feat(key-wallet): add chainlock handling to the wallet#756
Merged
Conversation
Contributor
|
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR implements chainlock-driven transaction finalization across the Dash SPV wallet stack. It models chainlock state in wallet events, tracks applied chainlocks in wallet metadata, promotes in-block transactions to finalized when covered by a chainlock, coordinates chainlock dispatch during sync completion, and validates the behavior through integration and unit tests. ChangesChainlock Wallet Integration
Sequence DiagramsequenceDiagram
participant ChainLockManager
participant SyncManager
participant Wallet
participant FFI
ChainLockManager->>SyncManager: emit SyncEvent::ChainLockReceived{validated}
SyncManager->>Wallet: deliver ChainLock (subscribe/forward)
Wallet->>Wallet: apply_chain_lock(chain_lock) -> promote txs, update metadata
Wallet->>FFI: WalletEvent::TransactionsChainlocked(chain_lock, per_account)
Wallet->>FFI: WalletEvent::BlockProcessed(..., chain_lock: Option)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
xdustinface
force-pushed
the
feat/wallet-chainlock-finalization
branch
from
b2f1eaf to
053bdaf
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## v0.42-dev #756 +/- ##
===========================================
Coverage 72.25% 72.25%
===========================================
Files 320 320
Lines 69651 70275 +624
===========================================
+ Hits 50326 50780 +454
- Misses 19325 19495 +170
|
xdustinface
force-pushed
the
feat/wallet-chainlock-finalization
branch
from
053bdaf to
bbf78d1
Compare
Sets up the wallet-layer foundation for chainlock-driven transaction finalization, surfacing chainlock finality and the signing proof to consumers through wallet events. - `WalletMetadata::last_applied_chain_lock` persists the highest `ChainLock` applied, establishing the wallet's finality boundary. - `WalletEvent::TransactionsChainlocked` carries the chainlock and the net-new finalized txids per account. - `WalletEvent::BlockProcessed` carries `chain_lock: Option<ChainLock>`, `Some` only when the block is at or below the wallet's finality boundary.
xdustinface
force-pushed
the
feat/wallet-chainlock-finalization
branch
from
bbf78d1 to
1b6074c
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@dash-spv/src/client/event_handler.rs`:
- Around line 159-230: Add in-module unit tests for
spawn_chainlock_wallet_dispatch that exercise the state machine: implement a
test WalletInterface stub that records calls to apply_chain_lock, spawn the task
with broadcast::channel, a CancellationToken, and on_failure mpsc; then send
SyncEvent::ChainLockReceived events (both validated = false and validated =
true) to assert that non-validated chainlocks are ignored, when initial sync is
not complete only the highest deferred_chain_lock (compare block_height) is
kept, a SyncEvent::SyncComplete { cycle: 0 } causes exactly one apply_chain_lock
of the buffered chainlock (deferred_chain_lock is cleared), and subsequent
validated ChainLockReceived events are applied immediately; also assert no extra
calls and ensure the task is shut down via shutdown.cancelled(); use tests
inside the same module with #[cfg(test)] to follow guidelines and reference
spawn_chainlock_wallet_dispatch, apply_chain_lock, SyncEvent::ChainLockReceived,
SyncEvent::SyncComplete, and deferred_chain_lock to locate the code under test.
In `@dash-spv/src/sync/chainlock/manager.rs`:
- Around line 88-99: The deferred ChainLock in pending_validation is promoted
without re-checking the block hash, which can allow an outdated header to pass;
when taking pending_validation in the readiness path inside manager.rs, re-run
the same block-hash verification used by process_chainlock() (i.e., call
verify_block_hash(...) or the same helper used there) before
validate_signature(&pending). Only if verify_block_hash returns true and
validate_signature(&pending).await succeeds should you increment progress, set
best_chainlock = Some(pending), and call save_best_chainlock().await; otherwise
treat it as invalid and call progress.add_invalid(1).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ab2bc3ca-848b-4279-b53d-e4375e272fbb
📒 Files selected for processing (21)
dash-spv-ffi/src/bin/ffi_cli.rsdash-spv-ffi/src/callbacks.rsdash-spv-ffi/tests/dashd_sync/callbacks.rsdash-spv/src/client/event_handler.rsdash-spv/src/client/sync_coordinator.rsdash-spv/src/sync/chainlock/manager.rsdash-spv/src/sync/chainlock/sync_manager.rsdash-spv/tests/dashd_masternode/helpers.rsdash-spv/tests/dashd_masternode/main.rsdash-spv/tests/dashd_masternode/tests_chainlock.rsdash-spv/tests/dashd_masternode/tests_instantsend.rskey-wallet-manager/src/event_tests.rskey-wallet-manager/src/events.rskey-wallet-manager/src/process_block.rskey-wallet-manager/src/test_utils/mock_wallet.rskey-wallet-manager/src/wallet_interface.rskey-wallet/src/managed_account/managed_core_funds_account.rskey-wallet/src/managed_account/managed_core_keys_account.rskey-wallet/src/tests/keep_finalized_transactions_tests.rskey-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rskey-wallet/src/wallet/metadata.rs
… ChainLock `on_masternode_ready` was promoting `pending_validation` to `best_chainlock` after a successful BLS signature check only, but the cached chainlock's block-hash had been verified earlier under `process_chainlock`'s permissive rule: when the header for that height is still missing, `verify_block_hash` returns `true` so the chainlock isn't dropped before masternode state arrives. By the time `on_masternode_ready` fires, the header has usually resolved, and if its hash disagrees with the chainlock's claim the deferred path would persist a chainlock the local chain doesn't match. Re-run `verify_block_hash` on the pending chainlock before calling `validate_signature`. Both must pass for promotion; a mismatch is counted as invalid and dropped, same as the live path in `process_chainlock`.
ZocoLini
approved these changes
May 12, 2026
shumkov
added a commit
to dashpay/platform
that referenced
this pull request
May 12, 2026
Picks up dashpay/rust-dashcore#756 which adds chainlock-driven transaction finalization in the wallet layer. Previously, `WalletInterface` had no `process_chain_lock` method and `dash-spv`'s `SyncEvent::ChainLockReceived` was emitted but never consumed, so wallet records were stuck at `TransactionContext:: InBlock(_)` forever even when the network produced a chainlock for the containing block. The new pin promotes records `InBlock → InChainLockedBlock` on chainlock arrival and emits a new `WalletEvent::TransactionsChainlocked` variant carrying the chainlock proof and per-account net-new finalized txids. For our `wait_for_proof` poll loop this means the chainlock branch (`record.context.is_chain_locked()`) actually flips when peers deliver the chainlock — the iter-4 IS→CL fallback path now resolves correctly instead of timing out at the secondary 180 s deadline. The new `WalletEvent` variant forces match-arm coverage in two sites: - packages/rs-platform-wallet/src/changeset/core_bridge.rs `build_core_changeset` returns `CoreChangeSet::default()` for the new variant. The wallet has already mutated the in-memory record by the time the event fires (upstream is "mutate-then- emit"), and the poll loop reads `record.context.is_chain_locked()` directly, so no additional persister projection is needed today. A future enhancement could persist `WalletMetadata:: last_applied_chain_lock` for crash recovery, but that's out of scope here. - packages/rs-platform-wallet/src/wallet/core/balance_handler.rs `BalanceUpdateHandler::on_wallet_event` returns early for the new variant. Chainlocks promote finality (`InBlock → InChainLockedBlock`) without changing UTXO state, so there's no balance update to deliver. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
5 tasks
Draft
6 tasks
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Adds chainlock-driven transaction finalization in the wallet layer:
WalletMetadata::last_applied_chain_lockpersists the highestChainLockapplied, establishing the wallet's finality boundary.WalletEvent::TransactionsChainlockedcarries the chainlock and the net-new finalized txids per account.WalletEvent::BlockProcessedcarrieschain_lock: Option<ChainLock>,Someonly when the block is at or below the wallet's finality boundary.Summary by CodeRabbit
New Features
Tests