Merge bitcoin/bitcoin#28920: wallet: birth time update during tx scanning#1040
Merge bitcoin/bitcoin#28920: wallet: birth time update during tx scanning#1040DashCoreAutoGuix wants to merge 2 commits into
Conversation
1ce45ba rpc: getwalletinfo, return wallet 'birthtime' (furszy) 83c6644 test: coverage for wallet birth time interaction with -reindex (furszy) 6f49737 wallet: fix legacy spkm default birth time (furszy) 75fbf44 wallet: birth time update during tx scanning (furszy) b4306e3 refactor: rename FirstKeyTimeChanged to MaybeUpdateBirthTime (furszy) Pull request description: Fixing bitcoin#28897. As the user may have imported a descriptor with a timestamp newer than the actual birth time of the first key (by setting 'timestamp=now'), the wallet needs to update the birth time when it detects a transaction older than the oldest descriptor timestamp. Testing Notes: Can cherry-pick the test commit on top of master. It will fail there. ACKs for top commit: Sjors: re-utACK 1ce45ba achow101: ACK 1ce45ba Tree-SHA512: 10c2382f87356ae9ea3fcb637d7edc5ed0e51e13cc2729c314c9ffb57c684b9ac3c4b757b85810c0a674020b7287c43d3be8273bcf75e2aff0cc1c037f1159f9 (cherry picked from commit 08e6aaa)
WalkthroughThis change introduces explicit tracking and updating of wallet birth time throughout the wallet codebase. A new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~18 minutes
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🧰 Additional context used📓 Path-based instructions (3)test/functional/**/*.py📄 CodeRabbit Inference Engine (CLAUDE.md)
Files:
**⚙️ CodeRabbit Configuration File
Files:
src/**/*.{cpp,h,cc,cxx,hpp}📄 CodeRabbit Inference Engine (CLAUDE.md)
Files:
🧠 Learnings (11)📓 Common learnings📚 Learning: in dash backports from bitcoin core test files, the `address_to_scriptpubkey` function should be imp...Applied to files:
📚 Learning: the `add_wallet_options` method is not available in dash's test framework and should be removed when...Applied to files:
📚 Learning: when backporting bitcoin core commits that use python's textwrap.dedent() function in test files, th...Applied to files:
📚 Learning: during dash backport verification of bitcoin core commit 06d469c, scope creep was detected when a...Applied to files:
📚 Learning: in dash backports from bitcoin core, witness transaction-related code (msg_wtx, wtxid) should be rep...Applied to files:
📚 Learning: during multiple verification attempts of bitcoin core commit 06d469c backport to dash pr #566, da...Applied to files:
📚 Learning: dashcoreautoguix successfully completed a complex bitcoin core backport (pr bitcoin#29412) for block mutati...Applied to files:
📚 Learning: in dash backports from bitcoin core, including necessary compilation fixes (such as api compatibilit...Applied to files:
📚 Learning: dashcoreautoguix successfully fixed scope creep in bitcoin core commit fcdb39d backport by removi...Applied to files:
📚 Learning: in dash backports from bitcoin core, the univalue method pushkvend() is not available and should be ...Applied to files:
🪛 Ruff (0.12.2)test/functional/wallet_reindex.py19-19: Missing return type annotation for public function Add return type annotation: (ANN201) 23-23: Missing return type annotation for public function Add return type annotation: (ANN201) 26-26: Missing return type annotation for public function Add return type annotation: (ANN201) 26-26: Missing type annotation for function argument (ANN001) 26-26: Missing type annotation for function argument (ANN001) 33-33: Missing return type annotation for public function Add return type annotation: (ANN201) 33-33: Missing type annotation for function argument (ANN001) 33-33: Missing type annotation for function argument (ANN001) 91-91: Missing return type annotation for public function Add return type annotation: (ANN201) ⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
🔇 Additional comments (3)
✨ 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. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
src/wallet/rpc/wallet.cpp(2 hunks)src/wallet/scriptpubkeyman.cpp(1 hunks)src/wallet/scriptpubkeyman.h(3 hunks)src/wallet/wallet.cpp(6 hunks)src/wallet/wallet.h(4 hunks)test/functional/test_runner.py(1 hunks)test/functional/wallet_reindex.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
src/**/*.{cpp,h,cc,cxx,hpp}
📄 CodeRabbit Inference Engine (CLAUDE.md)
C++20 codebase should be placed under src/
Files:
src/wallet/scriptpubkeyman.cppsrc/wallet/wallet.hsrc/wallet/scriptpubkeyman.hsrc/wallet/rpc/wallet.cppsrc/wallet/wallet.cpp
**
⚙️ CodeRabbit Configuration File
**: # CodeRabbit AI Review Instructions for Dash BackportsYour Role
You are reviewing Bitcoin Core backports to Dash Core. Your ONLY job is to validate that the Dash commit faithfully represents the original Bitcoin commit with minimal, necessary adaptations.
Critical Validation Rules
1. File Operations Must Match (AUTO-REJECT if violated)
- If Bitcoin modifies an existing file → Dash MUST modify (not create new)
- If Bitcoin creates a new file → Dash creates
- If Bitcoin deletes a file → Dash deletes
- Common failure: Bitcoin modifies keys.txt, Dash creates new file with 58 keys
2. Size Ratio Check (80-150% of Bitcoin)
- Count functional lines changed (exclude comments/whitespace)
- Dash changes should be 80-150% of Bitcoin's size
- Red flag: 2-line Bitcoin fix becoming 150+ lines in Dash
3. No Scope Creep
- Reject if you see: "TODO:", "FIXME:", "while we're here", "also fix"
- No unrelated refactoring or style changes
- Only Bitcoin's intended changes + minimal Dash adaptations
4. Bitcoin-Specific Code Detection
- Auto-reject witness/segwit code:
msg_wtxidrelay,MSG_WTX, witness imports- Auto-reject RBF (replace-by-fee) functionality
- Note: PSBT is supported in Dash (don't flag)
5. Mandatory Adaptations Only
bitcoin→dashin strings/pathsBitcoin→Dashin user-facing text- Port numbers: 8332→9998 (RPC), 8333→9999 (P2P)
- Hardcoded test values specific to Dash
- No other changes unless absolutely required
6. Completeness Check
- All files changed in Bitcoin must be present
- Extra files need clear justification (Dash-specific compatibility)
- Missing files = incomplete backport
Review Process
- First: Check file operations match exactly
- Second: Calculate size ratio
- Third: Scan for scope creep patterns
- Fourth: Detect Bitcoin-specific code
- Fifth: Verify all changes are minimal adaptations
Output Format
VALIDATION: [PASS/FAIL] File Operatio...
Files:
src/wallet/scriptpubkeyman.cppsrc/wallet/wallet.htest/functional/wallet_reindex.pysrc/wallet/scriptpubkeyman.htest/functional/test_runner.pysrc/wallet/rpc/wallet.cppsrc/wallet/wallet.cpp
test/functional/**/*.py
📄 CodeRabbit Inference Engine (CLAUDE.md)
Functional tests should be placed in test/functional/ and written in Python
Files:
test/functional/wallet_reindex.pytest/functional/test_runner.py
🧠 Learnings (9)
📓 Common learnings
Learnt from: DashCoreAutoGuix
PR: DashCoreAutoGuix/dash#0
File: :0-0
Timestamp: 2025-07-31T01:14:55.631Z
Learning: DashCoreAutoGuix successfully completed a complex Bitcoin Core backport (PR #29412) for block mutation detection by implementing the IsBlockMutated function, adding net processing integration, creating comprehensive unit tests, and properly adapting all Bitcoin-specific witness code for Dash compatibility. The backport maintains full security functionality while respecting Dash's non-witness transaction architecture.
Learnt from: DashCoreAutoGuix
PR: DashCoreAutoGuix/dash#0
File: :0-0
Timestamp: 2025-07-28T22:03:12.364Z
Learning: During multiple verification attempts of Bitcoin Core commit 06d469c26b backport to Dash PR #566, DashCoreAutoGuix consistently identified scope creep in interface_usdt_utxocache.py where additional pruning test functionality was added beyond the original Bitcoin commit. The user provided comprehensive fixes including both scope creep removal and missing mempool test file additions, but couldn't push due to authentication restrictions. The scope creep fix was identified as the priority to resolve CI failures.
Learnt from: DashCoreAutoGuix
PR: DashCoreAutoGuix/dash#0
File: :0-0
Timestamp: 2025-07-28T20:34:29.061Z
Learning: During Dash backport verification of Bitcoin Core commit 06d469c26b, scope creep was detected when additional pruning test functionality was added to interface_usdt_utxocache.py beyond what was in the original Bitcoin commit. The fix involved removing the extra test block while maintaining the core compiler flag fixes for USDT compilation errors.
Learnt from: DashCoreAutoGuix
PR: DashCoreAutoGuix/dash#0
File: :0-0
Timestamp: 2025-07-28T19:54:21.426Z
Learning: In Dash backports from Bitcoin Core, including necessary compilation fixes (such as API compatibility changes like UniValue get_int() → getInt<int>()) alongside the core backport is standard and expected practice. These compatibility fixes ensure the backported code compiles in Dash's evolved codebase while preserving Bitcoin's original functionality and intent.
Learnt from: DashCoreAutoGuix
PR: DashCoreAutoGuix/dash#0
File: :0-0
Timestamp: 2025-07-30T14:45:15.700Z
Learning: In Dash backports from Bitcoin Core test files, the `address_to_scriptpubkey` function should be imported from `test_framework.wallet` instead of `test_framework.address` as the import location differs between Bitcoin and Dash test frameworks.
Learnt from: DashCoreAutoGuix
PR: DashCoreAutoGuix/dash#0
File: :0-0
Timestamp: 2025-07-29T21:29:32.827Z
Learning: DashCoreAutoGuix successfully fixed scope creep in Bitcoin Core commit fcdb39d3ee backport by removing the parse test case from src/test/uint256_tests.cpp that was not part of the original Bitcoin commit. The fix was implemented in commit 16748115ce and verified through range-diff analysis.
Learnt from: DashCoreAutoGuix
PR: DashCoreAutoGuix/dash#0
File: :0-0
Timestamp: 2025-07-29T22:13:54.797Z
Learning: In Dash backports from Bitcoin Core, witness transaction-related code (MSG_WTX, wtxid) should be replaced with regular transaction handling (MSG_TX, txid) for compatibility, as demonstrated in the p2p_filter.py test fix where MSG_WTX was replaced with MSG_TX and irr_wtxid usage was replaced with irr_txid.
Learnt from: DashCoreAutoGuix
PR: DashCoreAutoGuix/dash#0
File: :0-0
Timestamp: 2025-07-30T14:45:15.700Z
Learning: The `add_wallet_options` method is not available in Dash's test framework and should be removed when backporting Bitcoin Core tests that use this method, as demonstrated in wallet_rescan_unconfirmed.py.
Learnt from: DashCoreAutoGuix
PR: DashCoreAutoGuix/dash#0
File: :0-0
Timestamp: 2025-07-29T17:13:35.087Z
Learning: When backporting Bitcoin Core commits that use Python's textwrap.dedent() function in test files, the textwrap import statement needs to be explicitly added if it's missing in the Dash test file.
Learnt from: DashCoreAutoGuix
PR: DashCoreAutoGuix/dash#0
File: :0-0
Timestamp: 2025-07-29T17:13:35.087Z
Learning: In Dash backports from Bitcoin Core, when the DIFFICULTY_ADJUSTMENT_INTERVAL constant is missing, it should be defined as 24 for Dash (different from Bitcoin's value), as seen in the getnetworkhashps RPC backport fix.
Learnt from: DashCoreAutoGuix
PR: DashCoreAutoGuix/dash#0
File: :0-0
Timestamp: 2025-07-27T22:35:10.176Z
Learning: In Dash backports, src/dashbls files are vendored dependencies that should not be modified during Bitcoin Core backports unless there is specific justification. Unauthorized modifications to vendored dependencies should be removed to maintain code integrity.
📚 Learning: in dash backports from bitcoin core test files, the `address_to_scriptpubkey` function should be imp...
Learnt from: DashCoreAutoGuix
PR: DashCoreAutoGuix/dash#0
File: :0-0
Timestamp: 2025-07-30T14:45:15.700Z
Learning: In Dash backports from Bitcoin Core test files, the `address_to_scriptpubkey` function should be imported from `test_framework.wallet` instead of `test_framework.address` as the import location differs between Bitcoin and Dash test frameworks.
Applied to files:
test/functional/wallet_reindex.pytest/functional/test_runner.py
📚 Learning: the `add_wallet_options` method is not available in dash's test framework and should be removed when...
Learnt from: DashCoreAutoGuix
PR: DashCoreAutoGuix/dash#0
File: :0-0
Timestamp: 2025-07-30T14:45:15.700Z
Learning: The `add_wallet_options` method is not available in Dash's test framework and should be removed when backporting Bitcoin Core tests that use this method, as demonstrated in wallet_rescan_unconfirmed.py.
Applied to files:
test/functional/wallet_reindex.pytest/functional/test_runner.py
📚 Learning: during dash backport verification of bitcoin core commit 06d469c, scope creep was detected when a...
Learnt from: DashCoreAutoGuix
PR: DashCoreAutoGuix/dash#0
File: :0-0
Timestamp: 2025-07-28T20:34:29.061Z
Learning: During Dash backport verification of Bitcoin Core commit 06d469c26b, scope creep was detected when additional pruning test functionality was added to interface_usdt_utxocache.py beyond what was in the original Bitcoin commit. The fix involved removing the extra test block while maintaining the core compiler flag fixes for USDT compilation errors.
Applied to files:
test/functional/wallet_reindex.pytest/functional/test_runner.py
📚 Learning: when backporting bitcoin core commits that use python's textwrap.dedent() function in test files, th...
Learnt from: DashCoreAutoGuix
PR: DashCoreAutoGuix/dash#0
File: :0-0
Timestamp: 2025-07-29T17:13:35.087Z
Learning: When backporting Bitcoin Core commits that use Python's textwrap.dedent() function in test files, the textwrap import statement needs to be explicitly added if it's missing in the Dash test file.
Applied to files:
test/functional/wallet_reindex.pytest/functional/test_runner.py
📚 Learning: in dash backports from bitcoin core, witness transaction-related code (msg_wtx, wtxid) should be rep...
Learnt from: DashCoreAutoGuix
PR: DashCoreAutoGuix/dash#0
File: :0-0
Timestamp: 2025-07-29T22:13:54.797Z
Learning: In Dash backports from Bitcoin Core, witness transaction-related code (MSG_WTX, wtxid) should be replaced with regular transaction handling (MSG_TX, txid) for compatibility, as demonstrated in the p2p_filter.py test fix where MSG_WTX was replaced with MSG_TX and irr_wtxid usage was replaced with irr_txid.
Applied to files:
test/functional/wallet_reindex.pysrc/wallet/wallet.cpp
📚 Learning: during multiple verification attempts of bitcoin core commit 06d469c backport to dash pr #566, da...
Learnt from: DashCoreAutoGuix
PR: DashCoreAutoGuix/dash#0
File: :0-0
Timestamp: 2025-07-28T22:03:12.364Z
Learning: During multiple verification attempts of Bitcoin Core commit 06d469c26b backport to Dash PR #566, DashCoreAutoGuix consistently identified scope creep in interface_usdt_utxocache.py where additional pruning test functionality was added beyond the original Bitcoin commit. The user provided comprehensive fixes including both scope creep removal and missing mempool test file additions, but couldn't push due to authentication restrictions. The scope creep fix was identified as the priority to resolve CI failures.
Applied to files:
test/functional/wallet_reindex.py
📚 Learning: dashcoreautoguix successfully completed a complex bitcoin core backport (pr bitcoin#29412) for block mutati...
Learnt from: DashCoreAutoGuix
PR: DashCoreAutoGuix/dash#0
File: :0-0
Timestamp: 2025-07-31T01:14:55.631Z
Learning: DashCoreAutoGuix successfully completed a complex Bitcoin Core backport (PR #29412) for block mutation detection by implementing the IsBlockMutated function, adding net processing integration, creating comprehensive unit tests, and properly adapting all Bitcoin-specific witness code for Dash compatibility. The backport maintains full security functionality while respecting Dash's non-witness transaction architecture.
Applied to files:
test/functional/wallet_reindex.py
📚 Learning: dashcoreautoguix successfully fixed scope creep in bitcoin core commit fcdb39d backport by removi...
Learnt from: DashCoreAutoGuix
PR: DashCoreAutoGuix/dash#0
File: :0-0
Timestamp: 2025-07-29T21:29:32.827Z
Learning: DashCoreAutoGuix successfully fixed scope creep in Bitcoin Core commit fcdb39d3ee backport by removing the parse test case from src/test/uint256_tests.cpp that was not part of the original Bitcoin commit. The fix was implemented in commit 16748115ce and verified through range-diff analysis.
Applied to files:
test/functional/wallet_reindex.py
🧬 Code Graph Analysis (1)
src/wallet/wallet.h (1)
src/wallet/wallet.cpp (2)
MaybeUpdateBirthTime(1656-1662)MaybeUpdateBirthTime(1656-1656)
🪛 Ruff (0.12.2)
test/functional/wallet_reindex.py
18-18: Missing return type annotation for public function add_options
Add return type annotation: None
(ANN201)
18-18: Missing type annotation for function argument parser
(ANN001)
21-21: Missing return type annotation for public function set_test_params
Add return type annotation: None
(ANN201)
25-25: Missing return type annotation for public function skip_test_if_missing_module
Add return type annotation: None
(ANN201)
28-28: Missing return type annotation for public function advance_time
Add return type annotation: None
(ANN201)
28-28: Missing type annotation for function argument node
(ANN001)
28-28: Missing type annotation for function argument secs
(ANN001)
35-35: Missing return type annotation for public function birthtime_test
Add return type annotation: None
(ANN201)
35-35: Missing type annotation for function argument node
(ANN001)
35-35: Missing type annotation for function argument miner_wallet
(ANN001)
93-93: Missing return type annotation for public function run_test
Add return type annotation: None
(ANN201)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Lint / Run linters
- GitHub Check: x86_64-pc-linux-gnu_nowallet / Build depends
- GitHub Check: x86_64-apple-darwin / Build depends
- GitHub Check: x86_64-w64-mingw32 / Build depends
- GitHub Check: arm-linux-gnueabihf / Build depends
- GitHub Check: x86_64-pc-linux-gnu / Build depends
🔇 Additional comments (24)
test/functional/test_runner.py (1)
196-197: Validate runtime classification of newly-addedwallet_reindex.pyentriesThe two new test invocations are correctly appended, but this block of
BASE_SCRIPTSis intended for tests that take ≤60 s.
wallet_reindex.pyperforms a full reindex and wallet rescan; on Bitcoin Core it typically runs ~2–3 minutes. Please time the Dash variant locally and, if it exceeds ~60 s, relocate these lines to the “< 2 m” or “< 5 m” section so that the parallel scheduler remains balanced.src/wallet/scriptpubkeyman.cpp (3)
821-821: LGTM! Proper signal emission for birth time tracking.The addition of
NotifyFirstKeyTimeChanged(this, nTimeFirstKey)correctly implements the notification mechanism for wallet birth time tracking, allowing the wallet to stay synchronized with ScriptPubKeyMan's first key time changes.
822-822: LGTM! Improved clarity with UNKNOWN_TIME constant.The change from
!nTimeFirstKeytonTimeFirstKey == UNKNOWN_TIMEprovides better code clarity and explicitly indicates when the time is unknown rather than relying on implicit zero/false checks.
824-824: LGTM! Consistent signal emission for birth time updates.The addition of
NotifyFirstKeyTimeChanged(this, nTimeFirstKey)ensures that both code paths that update the first key time properly notify the wallet, maintaining consistency in the birth time tracking mechanism.src/wallet/wallet.h (3)
282-283: LGTM - Clean atomic birth time implementationThe atomic
m_birth_timevariable with sentinel initialization follows Bitcoin Core patterns correctly. Thread-safe design with clear documentation.
731-732: LGTM - Correct method declarationThe
MaybeUpdateBirthTime(int64_t time)method declaration matches the expected signature for conditional birth time updates.
930-943: LGTM - Proper merge conflict resolutionThe merge conflict has been correctly resolved by preserving Dash-specific governance functionality while adding the Bitcoin
GetBirthTime()getter. The method correctly returns the atomic birth time value.src/wallet/scriptpubkeyman.h (3)
56-57: LGTM - Proper sentinel constant definitionThe
UNKNOWN_TIMEconstant definition usingstd::numeric_limits<int64_t>::max()is consistent with the birth time implementation and follows standard sentinel value patterns.
236-238: LGTM - Consistent signal patternThe
NotifyFirstKeyTimeChangedsignal follows the established pattern of other signals in the class, with appropriate parameters for birth time notifications.
261-262: LGTM - Improved default initializationChanging
nTimeFirstKeyinitialization toUNKNOWN_TIMEprevents unnecessary scanning until keys are actually generated/imported, which is more efficient than the previous default of 0.src/wallet/rpc/wallet.cpp (3)
194-194: LGTM: Appropriate addition of birthtime fieldThe addition of the optional "birthtime" field to the RPC result specification correctly implements the wallet birth time tracking feature from the Bitcoin Core backport.
288-290: LGTM: Correct conditional birthtime implementationThe runtime logic correctly checks for a valid birth time (not UNKNOWN_TIME) before adding the field to the JSON response, which matches the expected behavior for optional RPC fields.
152-294: Backport completeness verifiedAll files modified in Bitcoin Core PR bitcoin#28920 are present in this backport:
- src/wallet/rpc/wallet.cpp
- src/wallet/wallet.h
- src/wallet/wallet.cpp
- src/wallet/scriptpubkeyman.h
- src/wallet/scriptpubkeyman.cpp
- test/functional/test_runner.py
- test/functional/wallet_reindex.py
No missing files detected.
src/wallet/wallet.cpp (6)
944-946: LGTM! Clean birth time tracking implementation.The addition of
MaybeUpdateBirthTime(wtx.GetTxTime())inAddToWalletcorrectly updates the wallet's birth time when new transactions are discovered. The placement after successful insertion and the conditional execution only for new transactions is appropriate.
1053-1055: LGTM! Consistent birth time tracking for loaded transactions.The addition of birth time tracking in
LoadToWalletensures that the wallet's birth time is updated correctly when loading existing transactions from the database during wallet initialization. This complements theAddToWalletimplementation.
1656-1662: LGTM! Thread-safe birth time update implementation.The
MaybeUpdateBirthTimemethod correctly implements atomic birth time tracking. The compare-and-update logic ensures that the birth time reflects the oldest relevant timestamp, and the use of atomic operations provides thread safety.
3080-3086: LGTM! Proper birth time initialization during wallet creation.The birth time initialization correctly finds the earliest key time across all ScriptPubKeyMans and updates the wallet's birth time accordingly. This ensures that newly created or loaded wallets have accurate birth time tracking from the start.
3747-3755: LGTM! Clean ScriptPubKeyMan addition with birth time tracking.The new
AddScriptPubKeyManmethod correctly adds the ScriptPubKeyMan to the wallet and updates the birth time based on the manager's first key time. The implementation maintains proper order of operations and integrates well with the birth time tracking system.
3784-3784: LGTM! Proper signal connection for dynamic birth time updates.The connection of
NotifyFirstKeyTimeChangedsignal toMaybeUpdateBirthTimeensures that the wallet's birth time is dynamically updated whenever ScriptPubKeyMan first key times change (e.g., during key imports). This completes the comprehensive birth time tracking system.test/functional/wallet_reindex.py (5)
1-16: LGTM!File header, imports, and constants follow standard functional test patterns appropriate for Dash.
28-30: LGTM!Time advancement helper follows standard functional test patterns.
35-91: LGTM!Core test logic correctly implements wallet birth time validation for both descriptor and legacy wallets. The test appropriately handles reindex operations and verifies birth time persistence.
93-108: LGTM!Test runner setup follows standard functional test patterns with appropriate miner setup and test execution.
1-109: Backport Validation SummaryVALIDATION: PASS (with one fix required)
File Operations: MATCH - New file creation appropriate for functional test
Size Ratio: Appropriate (~109 lines for comprehensive wallet birth time testing)
Scope Creep: NONE - Focused solely on wallet birth time reindex testing
Bitcoin-Specific Code: NONE - No witness/segwit or RBF code detected
Completeness: COMPLETE - Full functional test implementationRequired Fix: Remove
add_wallet_optionsmethod call for Dash compatibility (already flagged above)The test correctly implements wallet birth time validation during reindex operations with proper handling of descriptor vs legacy wallet differences.
- Remove duplicated RPC field definitions in src/wallet/rpc/wallet.cpp - Remove incompatible add_wallet_options call in test/functional/wallet_reindex.py Addresses review feedback from CodeRabbit bot while preserving Bitcoin intent.
✅ Backport Verification - Issues FixedI found 2 issues and automatically applied fixes. Original Bitcoin commit: Issues Resolved:
Verification Details:
Reviewer Feedback Analysis:CodeRabbit bot comments reviewed: 2
The backport should now be complete and faithful to the original Bitcoin bitcoin#28920. CI Status Update:
|
DashCoreAutoGuix
added
the
fixes-applied
✅ Backport Verification SuccessfulNo issues found! This backport is complete and faithful to the original Bitcoin change. Original Bitcoin commit: Verification Details:
Reviewer Feedback Analysis:CodeRabbit comments reviewed: 2 All feedback items resolved:
CI Status:
This PR successfully implements Bitcoin Core's wallet birth time update feature with proper Dash adaptations. The backport is faithful to the original Bitcoin intent while resolving compatibility issues. Ready for merge pending lint fix. ✅ |
DashCoreAutoGuix
added
the
verified
PastaPastaPasta
added
ci-failed
|
Reason: CI has more than 1 failing job (7 CI jobs failing) 🚫 CI Check: 7 jobs failing (threshold: max 1 allowed) Failed CI Jobs:
Validation Results: Next Steps: |
DashCoreAutoGuix
added
the
needs-fixes
3 participants
Summary
Backport of Bitcoin Core PR bitcoin#28920
This PR implements wallet birth time tracking functionality that updates during transaction scanning.
Changes:
m_birth_timemember variable to track wallet creation timeMaybeUpdateBirthTime()to update birth time when older transactions are foundGetBirthTime()getter methodConflicts resolved:
src/wallet/wallet.h: Added birth time variable and methods alongside Dash-specific functionalitysrc/wallet/wallet.cpp: Integrated birth time updates with Dash's transaction handlingsrc/wallet/rpc/wallet.cpp: Added birthtime field to RPC output structuresrc/wallet/scriptpubkeyman.h/cpp: Added NotifyFirstKeyTimeChanged signal and updated UpdateTimeFirstKeytest/functional/test_runner.py: Added wallet_reindex.py test entriesBuild Status:
🤖 Generated with Claude Code
Summary by CodeRabbit