feat(key-wallet): stamp retained transaction records with their in-block position#891
Conversation
…ock position Fixes #890. Consumers aggregating a wallet's retained provider special transactions need to replay Core's same-block apply order (RebuildListFromBlock applies provider updates in block.vtx order), but the retained records carried no in-block index: BlockInfo held only height/hash/timestamp, account.transactions() iterates in txid order, and the index visible during block processing was discarded. - BlockInfo gains `position: Option<u32>` (serde default; None for contexts built without the containing block, e.g. FFI-supplied contexts and legacy persisted records), with a `position()` accessor and a `with_position` builder. `new()` keeps its 3-arg signature. - process_block_for_wallets enumerates block.txdata and stamps each record's context with its index. Chainlock promotion already copies the whole BlockInfo, so the position survives InBlock → InChainLockedBlock transitions. - Test pins that a two-tx block yields records stamped 0 and 1. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughBlock processing now stamps each wallet-relevant transaction with its index in the block. ChangesBlock Position Tracking
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
|
Reviewed |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #891 +/- ##
==========================================
- Coverage 74.34% 74.32% -0.02%
==========================================
Files 327 327
Lines 74771 74779 +8
==========================================
- Hits 55591 55582 -9
- Misses 19180 19197 +17
|
Issue being fixed or feature implemented
Fixes #890. Retained
TransactionRecords carried no in-block position:BlockInfoheld only{height, block_hash, timestamp},account.transactions()iterates in txid order, and the index visible during block processing was discarded. Downstream consumers that aggregate a wallet's retained provider special transactions (dashpay/platform#4120's masternode list) therefore cannot replay Core's same-block apply order —RebuildListFromBlockapplies same-block provider updates inblock.vtxorder, so with two updates for one proTxHash in a single block, the winner per field must be the later-positioned one, not whichever txid sorts higher.What was done?
BlockInfogainsposition: Option<u32>(serde(default);Nonefor contexts built without the containing block — FFI-supplied contexts, legacy persisted records), aposition()accessor, and awith_positionbuilder.BlockInfo::newkeeps its 3-arg signature, so existing call sites are untouched.process_block_for_walletsenumeratesblock.txdataand stamps each record's context with its index. Chainlock promotion (apply_chain_lock) already copies the wholeBlockInfo, so the position survivesInBlock→InChainLockedBlock.test_block_processing_stamps_in_block_position: a two-tx block yields records stamped positions 0 and 1.How Has This Been Tested?
cargo test -p key-wallet -p key-wallet-manager -p key-wallet-ffi— 548 + 233 + FFI suites, 0 failures.Breaking Changes
None — additive field with a defaulted deserialization and an unchanged constructor signature.
🤖 Generated with Claude Code
Summary by CodeRabbit