-
Notifications
You must be signed in to change notification settings - Fork 0
2026 dont backdate locktime rbf #3
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1016,14 +1016,23 @@ void DiscourageFeeSniping(CMutableTransaction& tx, FastRandomContext& rng_fast, | |
| // now we ensure code won't be written that makes assumptions about | ||
| // nLockTime that preclude a fix later. | ||
| if (IsCurrentForAntiFeeSniping(chain, block_hash)) { | ||
| uint32_t previous_locktime = tx.nLockTime; | ||
| tx.nLockTime = block_height; | ||
|
|
||
| // Secondly occasionally randomly pick a nLockTime even further back, so | ||
| // that transactions that are delayed after signing for whatever reason, | ||
| // e.g. high-latency mix networks and some CoinJoin implementations, have | ||
| // better privacy. | ||
| if (rng_fast.randrange(10) == 0) { | ||
| tx.nLockTime = std::max(0, int(tx.nLockTime) - int(rng_fast.randrange(100))); | ||
| // If a previous lockitme is passed (like in the bump fee case), the | ||
| // backdating is limited betwen the current height and the previous lockitme | ||
| if (previous_locktime > 0){ | ||
| // To avoid issues with the randrange, locktime_range cannot be 0 | ||
| int locktime_range = std::max(1, std::min(100, int(block_height - previous_locktime))); | ||
| tx.nLockTime = std::max(int(previous_locktime), int(tx.nLockTime) - int(rng_fast.randrange(locktime_range))); | ||
| }else{ | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't forget to run clang-tidy before opening PR on bitcoin/bitcoin :) https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#running-clang-tidy |
||
| tx.nLockTime = std::max(0, int(tx.nLockTime) - int(rng_fast.randrange(100))); | ||
| } | ||
| } | ||
| } else { | ||
| // If our chain is lagging behind, we can't discourage fee sniping nor help | ||
|
|
@@ -1317,10 +1326,13 @@ static util::Result<CreatedTransactionResult> CreateTransactionInternal( | |
| txNew.vin.back().scriptWitness = *scripts.second; | ||
| } | ||
| } | ||
|
|
||
| if (coin_control.m_locktime) { | ||
| txNew.nLockTime = coin_control.m_locktime.value(); | ||
| // If we have a locktime set, we can't use anti-fee-sniping | ||
| use_anti_fee_sniping = false; | ||
| } else if (coin_control.m_previous_locktime.has_value()) { | ||
| txNew.nLockTime = coin_control.m_previous_locktime.value(); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm a bit unsure between passing the previous locktime to the function by setting I know I pushed a bit against adding a lot of parameters throughout the code, but I feel like here it might make the code easier to follow. Otherwise people inside |
||
| } | ||
| if (use_anti_fee_sniping) { | ||
| DiscourageFeeSniping(txNew, rng_fast, wallet.chain(), wallet.GetLastBlockHash(), wallet.GetLastBlockHeight()); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of using max, I think it would be clearer to calculate locktime_range, and if it's 0, skip the next step (we can't backdate anyways). Like this:
Otherwise right now it's a bit convoluted: what happens if
block_height == previous_locktime? Thenlocktime_range = 1, and belowtx.nLockTimeis the max betweenprevious_locktimeandtx.nLockTime- 1, meaning that it will beprevious_locktime. It is still correct, but a bit harder to follow