-
Notifications
You must be signed in to change notification settings - Fork 0
Merge bitcoin/bitcoin#27501: mempool / rpc: add getprioritisedtransactions, delete a mapDeltas entry when delta==0 #813
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: backport-0.26-batch-602
Are you sure you want to change the base?
Conversation
…elete a mapDeltas entry when delta==0 67b7fec [mempool] clear mapDeltas entry if prioritisetransaction sets delta to 0 (glozow) c1061ac [functional test] prioritisation is not removed during replacement and expiry (glozow) 0e5874f [functional test] getprioritisedtransactions RPC (glozow) 99f8046 [rpc] add getprioritisedtransactions (glozow) 9e9ca36 [mempool] add GetPrioritisedTransactions (glozow) Pull request description: Add an RPC to get prioritised transactions (also tells you whether the tx is in mempool or not), helping users clean up `mapDeltas` manually. When `CTxMemPool::PrioritiseTransaction` sets a delta to 0, remove the entry from `mapDeltas`. Motivation / Background - `mapDeltas` entries are never removed from mapDeltas except when the tx is mined in a block or conflicted. - Mostly it is a feature to allow `prioritisetransaction` for a tx that isn't in the mempool {yet, anymore}. A user can may resbumit a tx and it retains its priority, or mark a tx as "definitely accept" before it is seen. - Since bitcoin#8448, `mapDeltas` is persisted to mempool.dat and loaded on restart. This is also good, otherwise we lose prioritisation on restart. - Note the removal due to block/conflict is only done when `removeForBlock` is called, i.e. when the block is received. If you load a mempool.dat containing `mapDeltas` with transactions that were mined already (e.g. the file was saved prior to the last few blocks), you don't delete them. - Related: dashpay#4818 and dashpay#6464. - There is no way to query the node for not-in-mempool `mapDeltas`. If you add a priority and forget what the value was, the only way to get that information is to inspect mempool.dat. - Calling `prioritisetransaction` with an inverse value does not remove it from `mapDeltas`, it just sets the value to 0. It disappears on a restart (`LoadMempool` checks if delta is 0), but that might not happen for a while. Added together, if a user calls `prioritisetransaction` very regularly and not all those transactions get mined/conflicted, `mapDeltas` might keep lots of entries of delta=0 around. A user should clean up the not-in-mempool prioritisations, but that's currently difficult without keeping track of what those txids/amounts are. ACKs for top commit: achow101: ACK 67b7fec theStack: Code-review ACK 67b7fec instagibbs: code review ACK 67b7fec ajtowns: ACK 67b7fec code review only, some nits Tree-SHA512: 9df48b622ef27f33db1a2748f682bb3f16abe8172fcb7ac3c1a3e1654121ffb9b31aeaad5570c4162261f7e2ff5b5912ddc61a1b8beac0e9f346a86f5952260a
|
Warning Rate limit exceeded@DashCoreAutoGuix has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 26 minutes and 47 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (7)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
✅ Backport Verification SuccessfulNo issues found! This backport is complete and faithful to the original Bitcoin change. Original Bitcoin commit: Verification Details:
Reviewer Feedback Analysis:PastaPastaPasta comments reviewed: 0 No human reviewer feedback found - all automated checks pass. This PR is ready for merge. ✅ |
|
Reason: CI has more than 1 failing job (2 lint failures exceed threshold) 🚫 CI Check: 2 jobs failing (threshold: max 1 allowed) ✅ Backport Validation Results:
Next Steps:
|
…627, #711, #658, #813, #825, #626, #831, #835 (qt backports: part 5) 20763f1 merge bitcoin-core/gui#835: Fix crash when closing wallet (Kittywhiskers Van Gogh) b47b1f6 merge bitcoin-core/gui#831: GUIUtil::bringToFront workaround for Wayland (Kittywhiskers Van Gogh) 493768c merge bitcoin-core/gui#626: Showing Local Addresses in Node Window (Kittywhiskers Van Gogh) 4e1b06b merge bitcoin-core/gui#825: Show maximum mempool size in information window (Kittywhiskers Van Gogh) 568753a merge bitcoin-core/gui#813: Don't permit port in proxy IP option (Kittywhiskers Van Gogh) dc3771e merge bitcoin-core/gui#658: Never change the prune checkbox after the user has touched it (Kittywhiskers Van Gogh) 9f103fb fix: hide the entire transaction widget if discreet mode is enabled (Kittywhiskers Van Gogh) e9b347f merge bitcoin-core/gui#711: Disable unused special members functions in `UnlockContext` (Kittywhiskers Van Gogh) 8829549 merge bitcoin-core/gui#627: Apply translator comments to reset options confirmation dialog (Kittywhiskers Van Gogh) d496df0 merge bitcoin-core/gui#612: Drop unused `QFrame`s in `SendCoinsEntry` (Kittywhiskers Van Gogh) 9e4ee65 merge bitcoin-core/gui#617: Reset options, notify user about backup creation (Kittywhiskers Van Gogh) cc2df46 merge bitcoin-core/gui#594: replace deprecated Q_OS_MAC with Q_OS_MACOS (Kittywhiskers Van Gogh) Pull request description: ## Additional Information | `develop` (e23a658) | This PR | | ------------------------------------------------------------ | ------------------------------------------------------------ | |  |  | |  |  | |  |  | ## Breaking Changes None expected. ## Checklist - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)** - [x] I have added or updated relevant unit/integration/functional/e2e tests - [x] I have made corresponding changes to the documentation **(note: N/A)** - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: UdjinM6: utACK 20763f1 Tree-SHA512: 6e047d42685f9f710cd966b838ba354f57628e31314f73fc3f6f9b48bbf8fadac1940a48d19b8cfcf9cee47ec00a16a6f21d43c905d1683fba20e42633054606
Backports bitcoin#27501
Original commit: 1af72e7
Backported from Bitcoin Core v0.26
This PR adds the
getprioritisedtransactionsRPC to get prioritised transactions and clears mapDeltas entries when prioritisation is set to 0. This helps users clean up mapDeltas manually and prevents accumulation of zero-delta entries.