Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enforce Tx unlock_time is Zero by Relay Rule #9151

Merged
merged 2 commits into from
May 21, 2024

Conversation

jeffro256
Copy link
Contributor

@jeffro256 jeffro256 commented Feb 4, 2024

Related to monero-project/research-lab#78

Added a relay rule that enforces the unlock_time field is equal to 0 for non-coinbase transactions. This is treated as a "no-drop" offense which means nodes will not drop peers for suggesting transactions with non-zero unlock times, but will quietly not accept those transactions, as to cause the least disruption to the network as possible.

UIs changed:

  • Removed locked_transfer and locked_sweep_all commands from monero-wallet-cli

APIs changed:

  • Removed unlock_time parameters from wallet2 transfer methods
  • Wallet RPC transfer endpoints send error codes when requested unlock time is not 0
  • Removed unlock_time parameters from construct_tx* cryptonote core functions

Further discussion is welcome...

@Cactii1
Copy link
Contributor

Cactii1 commented Feb 9, 2024

Monero should not be removing functionality, it should be adding functionality.

Time locks have the potential to be used for important things.

One example: I'm the owner of playmonero.com and I could use that feature to allow a player to decide when a bet is placed. No interaction from me, other than programing it into the game (right now this is not implemented and it ignores any time locked bets).

The point is that there are use cases for it. Maybe these things are currently apparent, but someone else might be able to think of something.

Don't remove it if it's not broken.

@johnr365
Copy link

+1 on not wanting this functionality to be removed. I won't labor on potential utility, but it's there, as Cactii1 for instance shared.

@plowsof
Copy link
Contributor

plowsof commented Feb 10, 2024

I could use that feature to allow a player to decide when a bet is placed.

Lets not base actions on fantasy. The feature has been around long enough for all these "coulds" to be put to use but where are they? All ive personally seen is people targeting merchants who use software that is unaware of "unlock time" e.g. an instant swapper that has around 20kusd worth of monero locked for several years that came to IRC for help.

KISS - if you receive Monero, it is spendable in 10 blocks. But what if.. no.

@Cactii1
Copy link
Contributor

Cactii1 commented Feb 10, 2024

But what if..

Many of the pro-Monero arguments are based on "what ifs".

What if the government...
What if quantum computers...
What if a business's competition...
What if a 51%...
What if a mining pool...
What if a hostile developer...
What if another blockchain tech adopts...
What if fluffy or moneromoo's accounts get compromised...
What if GitHub does...
What if someone sends me locked XMR...

Don't dismiss things without thinking them through.

@plowsof
Copy link
Contributor

plowsof commented Feb 10, 2024

Its about to be removed, lets think of use cases/potential after all these years lol devs/researchers are calling for its removal. the time for dreaming is over

@Cactii1
Copy link
Contributor

Cactii1 commented Feb 10, 2024

You're not god here plowsof. This is a community.

@Cactii1
Copy link
Contributor

Cactii1 commented Feb 10, 2024

This makes me wonder if you can be impartial in your work as CCS coordinator.

@fullmetalScience
Copy link

I used to voice criticism regarding this functionality (for change unexpectedly getting locked too), but there are use cases.

A simple one: Lets say you pay child-support, want to get it over with and therefore program the money to arrive over the next couple of years, every month, on time.
You can proof your send while the recipient cannot spend it all carelessly, which is particularly useful if the recipient lacked budgeting skills. In such a case, if you paid lump, the money could be long gone while the children still need to be fed.

The Monero-esque one: Banking, specifically non-fractional reserve banking

There once was a discussion about tackling the fact of Monero not being a bearer asset, meaning you cannot give funds to someone who isn't set-up for receiving them.

Some argued that certain amounts could be burnt and paper money printed, representing that provably burnt amount.

Personally, I'd consider it much more acceptable to deposit the amount with a trusted third-party in exchange for counterfeit-resistant paper.

The trusted party would send the denominated amount to itself, as locked_transfer and print the unlock block height (as date of expiry) on that paper.

Until that date, those handling such papers could trust that it's most likely backed by real XMR, if the third-party is trustworthy, both ethically and security-wise.

For now it's a "what if" scenario. I failed to obtain a reasonable source of paper money - the idea was to use otherwise worthless Venezuelan bolívares as a base. Besides, it was still far too early back then anyway. I still hope though, that some day, somebody picks up the idea and builds this "tool" to allow us to pass value without having to rely on electronics.

@plowsof
Copy link
Contributor

plowsof commented Feb 10, 2024

Transaction uniformity anyone? We should have a large ish tx_extra then which has alot of use cases. If MRL/-dev prove it harms.privacy then what is there left to discuss? Playing top trumps with ideas for it?

@Cactii1
Copy link
Contributor

Cactii1 commented Feb 10, 2024

Not even the official CLI Wallet currently supports tx_extra, locked_transfer is supported.

@jeffro256
Copy link
Contributor Author

jeffro256 commented Feb 10, 2024

A simple one: Lets say you pay child-support, want to get it over with and therefore program the money to arrive over the next couple of years, every month, on time.
You can proof your send while the recipient cannot spend it all carelessly, which is particularly useful if the recipient lacked budgeting skills. In such a case, if you paid lump, the money could be long gone while the children still need to be fed.

If you're actively enforcing the receiver follows some time-release schedule, a 2/2 multisig wallet would be much, much better solution to this problem since you can both agree to move money in an emergency.

Until that date, those handling such papers could trust that it's most likely backed by real XMR, if the third-party is trustworthy, both ethically and security-wise.

Except you can't back a currency with assets you can't use. If you plan on providing paper XMR, you need to be able to provide real XMR on demand to settle those notes. If your funds are locked, then you can't actually peg that paper XMR and the price will float around (probably going to 0) with nothing grounding it. The fact that their XMR assets are locked doesn't actually bring any accountability to the entity backing your paper XMR because they can still rug pull you or print more paper XMR than they have real XMR. In other words, the XMR being locked doesn't actually change your counterparty risk but just makes it harder for them to legitimately do their job.

Reserve proofs allow the backing entity to prove that they have a certain amount of XMR in reserves and doesn't impede their normal functioning.

@nahuhh
Copy link

nahuhh commented Feb 10, 2024

facepalm

fkn reddit

@nahuhh
Copy link

nahuhh commented Feb 10, 2024

Im dying over here.

if you want to lock your funds up, use multisig or self-control.

@jeffro256
Copy link
Contributor Author

Reposting my response to @Cactii1's original comment:

One example: I'm the owner of playmonero.com and I could use that feature to allow a player to decide when a bet is placed.

You could send the desired time info off-chain so that this transaction is indistinguishable from other transactions. You could also encode it in the bottom digits of the amount. You could encrypt it in tx_extra. There's a lot of ways to transmit data which don't involve time-locking funds.

Don't remove it if it's not broken.

But that's the point: it is broken, insofar as it actively makes the main use case of Monero, digital cash, worse. Even if you don't utilize the unlock_time feature, you must always be aware of it when you process payments, select decoys, or select inputs, else you will suffer the consequences.

@fullmetalScience
Copy link

Except you can't back a currency with assets you can't use. If you plan on providing paper XMR, you need to be able to provide real XMR on demand to settle those notes. If your funds are locked, then you can't actually peg that paper XMR and the price will float around (probably going to 0) with nothing grounding it.

In other words, the XMR being locked doesn't actually change your counterparty risk but just makes it harder for them to legitimately do their job.

I understand how it may not be immediately apparent, but there's no reason for any of these statements to be true.

Reserve proofs allow the backing entity to prove that they have a certain amount of XMR in reserves and doesn't impede their normal functioning.

There is no way of knowing the total value handed out on paper. Reserve proofs would fail to allow people to verify the backing of their specific paper and thus create the very problem of fractional banking all over again.

The fact that their XMR assets are locked doesn't actually bring any accountability to the entity backing your paper XMR because they can still rug pull you or print more paper XMR than they have real XMR.

They themselves should, in their own interest, rather not counterfeit their own paper, because users would notice ("oh look - this guy online sells the bills that reference the same xmr as mine here do!").

(In the concept, specific papers are matched 1:1 to specific locked transactions.)

@fullmetalScience
Copy link

(referring to previous example)

a 2/2 multisig wallet would be much, much better solution to this problem since you can both agree to move money in an emergency.

That use case relies on "withdrawals" being possible with one party being absent in the future.

Do you see any way (now or for Seraphis) of me pre-signing my part, with my signature being valid only from a specific date onwards?

Maybe if we could get such locking into multisig we could tackle the issue of people unexpectedly having to cope with incoming locked amounts while still satisfying those of us who use locking.

I am thinking of a "multisig" where A is a secret and B is a block height.

@Gingeropolous
Copy link
Collaborator

there are a lot of what-ifs and potential use cases here.

And yes, sure, there are use cases.

Just like there are use cases for tx_extra.

just because there are potential use cases doesn't necessarily mean that a feature should stick around, especially when it doesn't really help the fungibility thing.

and just because something gets removed, doesn't mean that it won't come back. My point is, there are probably better ways to do this type of lock time that could be implemented in the future.

As it stands now, it almost looks like it was bolted on. Everything was set up regarding the cryptonote protocol, and because cryptonote txs aren't scripts, there was no easy way to do a lock time. So they just added a field to the tx. Like, its not even bundled up and wrapped up in any cryptomath. It's just there. And it's also different than bitcoins unlocktime if i recall correctly.

Though its also present in the coinbase transaction, where there it makes sense. ( i haven't gone through the PR to see if / how this is handled).

Copy link
Contributor

@rbrunner7 rbrunner7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made a first pass through the code, with some questions resulting. In general, looks good.

I am reasonably certain that no tx construction related code was overlooked (ZMQ does not let you submit transactions, right?), and that no code was removed that is necessary to still process existing, mined transactions and honor their timelocks.

The high number of otherwise trivial changes that result from removing parameters of various methods are a bit unfortunate, but I still think it's the right way to eliminate these - many methods have almost too many parameters for their own good, and if we can reduce by one, it's a win.

{
explicit nonzero_unlock_time(std::string&& loc)
: transfer_error(std::move(loc), "transaction cannot have non-zero unlock time")
{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When first reading this error message I thought that it's awfully technical and a bit hard to read because of the double-negation, and wondered whether it should contain something like "timelocks are not supported anymore" so that people getting this error message can connect the dots or google the problem.

But then I realized that probably no normal user will ever get to see this message because the CLI wallet, and the RPC interface, simply will not let them set a non-zero unlock time in any way, so I think it's ok after all.

src/wallet/wallet_rpc_server.cpp Show resolved Hide resolved
tests/core_tests/tx_validation.cpp Show resolved Hide resolved
src/wallet/wallet2.cpp Outdated Show resolved Hide resolved
src/wallet/wallet2.cpp Show resolved Hide resolved
@SamsungGalaxyPlayer
Copy link
Collaborator

In my opinion, the current unlock_time is more of a footgun than anything. Use-cases have failed to materialize, as evidenced by the lack of on-chain adoption. And the potential for harm (someone unknowingly accepting a long-locked transaction) is real.

This proposal would not prevent the use of unlock_time by users who really want to use this function, but they will need to use custom software.

If in the future a lockup functionality is desired, then it should be re-implemented in a far more sane way (locking individual outputs instead of the entire transaction, etc).

@jeffro256
Copy link
Contributor Author

I understand how it may not be immediately apparent, but there's no reason for any of these statements to be true.

@fullmetalScience In this scenario, let's say that people suddenly started divesting from XMR because of economic conditions, especially those holding paper XMR. The real value of paper XMR drops 10% as compared to real XMR. How do you plan on stabilizing the paper XMR with no usable reserves?? Why would I want to hold your paper XMR if I can never cash it in for real XMR? And the merchants I interact with can't cash it in for real XMR?

There is no way of knowing the total value handed out on paper. Reserve proofs would fail to allow people to verify the backing of their specific paper and thus create the very problem of fractional banking all over again.

The locked enotes system also fails to solve this problem... it would just make it more obviously if the backing entity does it a lot. But you can say the same thing about reserve proofs: it's not true that reserve proofs can't allow for proving amounts in specific enotes. The most straight-forward proof of reserves is to just hand over the view key and all key images which would let you tie amounts and unspentness to individual transfers.

@vtnerd
Copy link
Contributor

vtnerd commented Feb 14, 2024

I am reasonably certain that no tx construction related code was overlooked (ZMQ does not let you submit transactions, right?), and that no code was removed that is necessary to still process existing, mined transactions and honor their timelocks.

ZMQ does allow you to send transactions.

@jeffro256
Copy link
Contributor Author

Should we update the ZMQ error handling code to include feedback that the transaction was rejected due to its unlock time?

@vtnerd
Copy link
Contributor

vtnerd commented Feb 18, 2024

Should we update the ZMQ error handling code to include feedback that the transaction was rejected due to its unlock time?

Yes, it already lists other error messages.

Copy link
Contributor

@UkoeHB UkoeHB left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no plan to support unlock times in Seraphis, so this is an anticipated feature regression.

I did not review the code.

@fullmetalScience
Copy link

@jeffro256

For the sake of clarity:

@fullmetalScience In this scenario, let's say that people suddenly started divesting from XMR because of economic conditions, especially those holding paper XMR. The real value of paper XMR drops 10% as compared to real XMR. How do you plan on stabilizing the paper XMR with no usable reserves?? Why would I want to hold your paper XMR if I can never cash it in for real XMR? And the merchants I interact with can't cash it in for real XMR?

You are getting a bit ahead of what I spelled out. The bank is by no means prevented from holding more funds apart from the locked ones and should definitely do so. They can take (and give) the paper in exchange for on-chain XMR any time, effectively preventing a separation based on this concern.

The most straight-forward proof of reserves is to just hand over the view key and all key images which would let you tie amounts and unspentness to individual transfers.

In comparison to the ability of checking individual bills, a drawback of revealing the lump sum is that the bank may make itself more of a target than necessary. The paper would be less trustworthy when the next block could reveal that it's no longer backed.

Related to monero-project/research-lab#78

Added a relay rule that enforces the `unlock_time` field is equal to 0 for non-coinbase transactions.

UIs changed:
* Removed `locked_transfer` and `locked_sweep_all` commands from `monero-wallet-cli`

APIs changed:
* Removed `unlock_time` parameters from `wallet2` transfer methods
* Wallet RPC transfer endpoints send error codes when requested unlock time is not 0
* Removed `unlock_time` parameters from `construct_tx*` cryptonote core functions
@jeffro256 jeffro256 force-pushed the no_relay_unlock_time branch from 8e682fb to 38f354e Compare February 24, 2024 20:27
@jeffro256
Copy link
Contributor Author

Rebased to fix conflicts with #8861

@plowsof
Copy link
Contributor

plowsof commented Mar 17, 2024

Thank you to the reviewers!

This makes me wonder if you can be impartial in your work as CCS coordinator.

@Cactii1 If you wish to make a complaint / escalate this, i've just recently posted an update for the final period of my current CCS proposal here

@@ -10383,10 +10382,10 @@ std::vector<wallet2::pending_tx> wallet2::create_transactions_2(std::vector<cryp
tx.selected_transfers.size() << " inputs");
auto tx_dsts = tx.get_adjusted_dsts(needed_fee);
if (use_rct)
transfer_selected_rct(tx_dsts, tx.selected_transfers, fake_outs_count, outs, valid_public_keys_cache, unlock_time, needed_fee, extra,
transfer_selected_rct(tx.dsts, tx.selected_transfers, fake_outs_count, outs, valid_public_keys_cache, needed_fee, extra,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tx_dsts -> tx.dsts here and further down. Is this a rebase error or intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a rebase error. Good catch! Will fix soon

Copy link

@SNeedlewoods SNeedlewoods left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed the changes in src/ and haven't looked at tests/ yet.
So far it looks good to me, just left a noob question.

ptx_vector = m_wallet->create_transactions_2(dsts, fake_outs_count, 0 /* unlock_time */, priority, extra, m_current_subaddress_account, subaddr_indices, subtract_fee_from_outputs);
break;
}
auto ptx_vector = m_wallet->create_transactions_2(dsts, fake_outs_count, priority, extra,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why use auto instead of std::vector<tools::wallet2::pending_tx>?
Is there a reason besides being shorter?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, just shorter ;)

@kayabaNerve
Copy link

kayabaNerve commented Apr 5, 2024

Time-based locks are annoying with FCMPs so I'm in great support of banning those under any FCMPs HF, and moving forward with this relay rule now.

@luigi1111
Copy link
Collaborator

Lots of discussion here...ultimately I didn't find any potential use case enumerated so far compelling enough not to move forward with this rule change.

@luigi1111 luigi1111 merged commit cdd7fc0 into monero-project:master May 21, 2024
18 checks passed
@xmrrmxntom
Copy link

@johnr365 @Cactii1 @rbrunner7 @kayabaNerve @Cactii1

A Plea to Restore a Crucial Feature in XMR

As a computer science student and a long-time follower of XMR & Dr. Daniel Kim (sweetwater.consulting), I'm compelled to share my thoughts on a feature that I believe is important to the value proposition of XMR. I've created an account specifically to express my disappointment and frustration with the removal of the locked_transfers and locked_sweep_all feature.

A Personal Journey with XMR

I've been following the XMR project since 2018, and its value proposition was evident to me even back then. However, I wasn't technical enough to fully appreciate its features. This year, I became proficient enough to run a full node and use the CLI, which is when I discovered the locked_transfers and locked_sweep_all features. I immediately utilized them, and they have been invaluable to me.

As someone who has impulsively sold assets like NVIDIA, BTC, and TSLA before they reached their full potential, I've come to realize that XMR is a long-term play that will appreciate in value over the next 5-20 years. The ability to lock transactions for an extended period has been a game-changer for me, allowing me to make sacrifices that my future self will appreciate.

The Value of Locked Transactions

The locked_transfers and locked_sweep_all feature was a unique selling point for XMR, offering me the ability to make long-term commitments to the blockchain. By removing this feature, we're depriving users of a valuable tool that would help them appreciate the embedded on-chain hodl properties of the XMR blockchain.

A Call to Action

I urge the XMR community to reconsider the removal of this feature and to implement safeguards to prevent similar decisions in the future. Specifically, I request:

  1. Reinstatement of the feature: Bring back the locked_transfers and locked_sweep_all feature to allow users to make long-term commitments to the blockchain.
  2. Safeguards for feature deprecation: Establish a formal, non-trivial process or procedure to determine whether a feature should be deprecated, ensuring that user feedback and concerns are taken into account and valued.
  3. Improvement or alternative: If the feature cannot be reinstated, explore alternative solutions that would provide similar functionality and benefits to users.

Conclusion

As more users join the XMR community, they will come to appreciate the unique properties of the blockchain. I firmly believe that the locked_transfers and locked_sweep_all feature helps users HODL with the long-term success of XMR. I hope that my plea will be heard, and we can work together to restore this important feature.

A Final Appeal

I've gone from hearing about XMR as the real privacy-focused vision of BTC, to buying some XMR on an exchange, to self-custodying on Exodus wallet, and finally to downloading and running the CLI. XMR is beautiful, and it's idealistic. Please keep or reimplement this feature.

https://reddit.com/r/Monero/comments/mwrm6g/how_to_lock_send_future_monero_to_yourself_with/

This was the post and feature that motivated me to dedicate a weekend last semester to read the documentation, compile from source, and use the CLI.

…Please keep this feature…

@kayabaNerve
Copy link

  1. The feature is effectively unused. 98% of transactions which used it used it improperly, fingerprinting them.

https://thecharlatan.ch/Monero-Unlock-Time-Privacy/

And then some effectively burn coins.

  1. The technical complexity of it isn't worthwhile for the effectively nil functionality. Improving its functionality is either non-trivial or a privacy issue.

  2. We had an issue tracking discussions on this for four years. This is a relay rule which obtained sufficient consensus. If there was demand for this functionality, it could be locally disabled and used. There's not been notable reports of large swaths of the network not adopting this rule and suddenly starting to use archaic, ill-thought functionality.

  3. You can locally replace this functionality with a TEE or a VDF. You do not need consensus rules to replace your personal lack of patience.

  4. Decade-long timelocks SHOULD NOT be considered safe. If you timelock an output for 20y, I'd consider a bet that 20y from now, a quantum computer scoops it up. The ability to rotate keys is of critical importance to user safety, and any functionality which does not go over the risks/implications, implying a lack of them, is unsafe. For Monero's inevitable PQ migration, we will need to:

A) Invalidate all existing timelocks
B) Kill off all existing timelocked outputs
C) Design a transition path aware of timelocks and enshrine timelocks into the new PQ protocol

I'd personally advocate option A at this time. There's already been a privacy issue with timelocks, and we've already decided against replacing that privacy issue with a performance issue. When discussing a PQ protocol, the performance concerns will increase by two orders of magnitude at least.


It's for all these reasons this decision makes sense. It's the best solution. If you want to propose a better solution, now or in the future, you're welcome to. This isn't about killing off functionality because we feel like it. It is about improving Monero.

The unfortunate thing for any advocate of timelocks on Monero is that it's very hard to argue any scheme successfully improves Monero. Functionality alone is an insufficient argument. If it was, we'd have shoved the EVM in already.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.