Enforce Tx unlock_time is Zero by Relay Rule [RELEASE] #9311
+186
−230
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 @tobtoht: undo rebase changes tx.dsts -> tx_dsts
rbrunner7
approved these changes
May 3, 2024
There was a problem hiding this comment.
I went through all the code again, after the review of the master branch PR. Again, saw no problems.
Encouraged by @selsta , beyond that code review I actually tested daemon and CLI wallet running this PR on testnet.
I tested the following:
- The daemon still accepts blocks with locked transactions in them, which is correct, as this PR does not yet change consensus rules
- The daemon ignores locked transactions sent to it from another daemon i.e. does not let them enter its pool
- The daemon rejects a locked transaction submitted by a wallet without this PR
- The CLI wallet without this PR correctly displays an error message when trying to submit a locked transaction that gets rejected by the daemon
- The CLI wallet with this PR does not know commands like
locked_transferanymore which makes it impossible to ask for locked transactions
Unfortunately the rejection message of a CLI wallet without this PR does not mention the reason for rejection because obviously it does not know it:
Error: transaction <75aa75f2adb89290b9d79590d2a511b3390c9ffc37aaff5dc59c18de60a36bf1> was rejected by daemon
I checked the responsible code wallet2::get_text_reason: Seems to me there is no way for a "new" daemon to give back error info in a way to make an "old" CLI wallet showing the reason. I don't think that this really matters however.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
#9151