WIP: feat/platform-wallet2 — do not review#655
Draft
QuantumExplorer wants to merge 35 commits intov0.42-devfrom
Draft
WIP: feat/platform-wallet2 — do not review#655QuantumExplorer wants to merge 35 commits intov0.42-devfrom
QuantumExplorer wants to merge 35 commits intov0.42-devfrom
Conversation
… bls/eddsa features Add insert_wallet() for inserting pre-built Wallet + info pairs, and get_wallet_and_info_mut() for split-borrowing (&Wallet, &mut T) from the two-map design. Also forward bls/eddsa feature flags to key-wallet. Needed by platform-wallet to integrate with the v0.42-dev WalletManager without modifying any existing WalletManager code. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Phase 1 (prep): - Fix AddressInfo::mark_used() hardcoded used_at=Some(0) → real timestamp - Make ManagedCoreAccount::current_timestamp() public - Add first_seen: u64 to TransactionRecord (restores pre-#582 behavior for mempool tx timestamps, needed by evo-tool wallet_transactions table) - Add helper accessors: is_instant_locked, is_chain_locked, block_height, block_hash - Derive PartialEq + Eq on TransactionRecord, InputDetail, OutputDetail (needed by changeset types) - Make Wallet::add_account() idempotent for xpub=None (deterministic derivation replays safely); still errors on explicit-xpub conflict - Add AddressPool::set_last_revealed(index) for apply() restore - Add ManagedCoreAccount::insert_utxo/remove_utxo idempotent wrappers - Add ManagedWalletInfo::find_account_by_address_mut/find_account_with_utxo_mut routing helpers for apply() Phase 2 (changeset module): - New key-wallet/src/changeset/ module with BDK-style atomic changesets - Merge trait + blanket impls for BTreeMap/BTreeSet/Option/Vec - WalletChangeSet with sub-changesets: AccountKeyChangeSet, AccountStateChangeSet, ChainChangeSet, UtxoChangeSet, TransactionChangeSet, BalanceChangeSet - Changesets carry native types directly (Utxo, TransactionRecord, AccountType) — no flattened *Entry wrappers, simpler apply logic - Merge semantics: max-wins for heights/last_revealed, sum for balance deltas, dedup for account types, extend for maps/sets Tests: 32 new (14 merge, 5 changeset, 4 address_pool, 11 changeset_support). Updated 3 existing tests that asserted add_account errors on duplicate. 478 total, all passing. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…reAccount Every state-mutating method on ManagedCoreAccount / ManagedWalletInfo now returns `(value, WalletChangeSet)` (or just `WalletChangeSet` for void returns). The changeset is built directly via `cs.field = Some(...)` -- no intermediate sub-changeset variables -- so callers uniformly do `result.changeset.merge(cs)` to accumulate. `TransactionCheckResult` grows a `changeset: WalletChangeSet` field populated by the orchestrator. Mutation methods covered: - mark_address_used -> (bool, WalletChangeSet) - update_utxos -> WalletChangeSet (private) - record_transaction -> (TransactionRecord, WalletChangeSet) - confirm_transaction -> (bool, WalletChangeSet) - mark_utxos_instant_send -> (bool, WalletChangeSet) - update_transaction_context(txid, new_context) -> WalletChangeSet (new) - update_balance -> WalletChangeSet - WalletInfoInterface::update_balance -> WalletChangeSet - WalletInfoInterface::mark_instant_send_utxos -> (bool, WalletChangeSet) Correctness fixes caught by review: - update_utxos now detects is_confirmed / is_instantlocked transitions on existing UTXOs and emits them to cs.utxos.added, so a persister replay cannot leave a stale Utxo after a mempool->block upgrade. - wallet_checker.rs gap-limit pass accumulates (account_index, pool_type) -> highest_used into cs.account_states.last_revealed so apply() can restore the reveal pointer without re-deriving. Required widening AccountStateChangeSet::last_revealed to BTreeMap<(u32, AddressPoolType), u32> and deriving PartialOrd / Ord / Hash on AddressPoolType. - update_transaction_context is idempotent: empty changeset when the record is missing or the context already matches. Both IS-lock orchestrators (wallet_checker.rs and mark_instant_send_utxos) now delegate to it instead of hand-rolling cs.transactions inserts. Call sites in key-wallet-manager and key-wallet-ffi updated for the new return shapes (mostly .0 tuple access where only the bool mattered). Tests: adds key-wallet/src/tests/changeset_mutation_tests.rs covering each mutation method's changeset shape, idempotency on replay, the update_transaction_context helper, the mempool->block UTXO context upgrade, and end-to-end accumulation into TransactionCheckResult. 486 key-wallet lib tests pass. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Introduces the restore path that mirrors the mutate-and-return changeset
API landed in the previous commit. A persister reads a WalletChangeSet
off disk and hands it to apply_changeset, which replays the deltas
idempotently onto the in-memory state.
Inverse of the mutation methods:
- account_keys -> wallet.add_account(ty, None) + mirror into
self.accounts via add_managed_account
(both idempotent from Phase 1)
- account_states -> mark_address_used per address + pool.set_last_revealed
per (account_index, pool_type) tuple
- utxos.added -> route by address, insert_utxo on owning account
(upsert: same path carries is_confirmed /
is_instantlocked upgrades)
- utxos.spent -> route by outpoint, remove_utxo
- utxos.instant_locked -> route by outpoint, flip is_instantlocked
- transactions.records -> insert into owning account; routed first by
txid membership, falling back to address
lookup on the record's output scripts
- chain.synced_height -> max-wins assignment on metadata
- balance -> recomputed from the restored UTXO set;
cached deltas in cs.balance are discarded
Invariants:
- Idempotent: apply(apply(cs)) produces the same state as apply(cs).
- No re-emission: apply does not return a WalletChangeSet, so the
returned changesets from inner calls (mark_address_used, etc.) are
intentionally discarded.
- Best-effort routing: unroutable entries (orphaned UTXOs, foreign
addresses) are silently skipped rather than panicking.
New files:
- key-wallet/src/wallet/managed_wallet_info/apply.rs — the ManagedWalletInfo
inherent impl plus 3 round-trip tests:
* funding changeset replays to sibling wallet (UTXO + tx + balance +
address-used mirror)
* double-apply is idempotent
* unroutable entries leave target state unchanged
key-wallet-manager additions:
- get_wallet_mut_and_info_mut(wallet_id) -> (&mut Wallet, &mut T) split
borrow on the two maps, required because apply_changeset takes
&mut Wallet for add_account.
- WalletManager<ManagedWalletInfo>::apply_changeset(wallet_id, cs) —
concrete-type wrapper that performs the split borrow, delegates to
ManagedWalletInfo::apply_changeset, and bumps structural_revision so
observers can detect the restore. PlatformWalletInfo will need its own
concrete impl in a follow-up (same pattern).
- 2 sanity tests: WalletNotFound path and empty-changeset no-op with
revision bump.
Tests: 489 key-wallet lib tests pass (was 489 — +3 new apply tests,
replacing some earlier test removals from the review pass).
38 key-wallet-manager tests pass (was 36 — +2 apply wrapper tests).
key-wallet-ffi and platform-wallet compile clean.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Addresses correctness, API, and consistency feedback from the three reviewers against commit e018f77. No behavior change for the happy path; closes several latent bugs on edge cases. ## Correctness - update_utxos applies state flags monotonically on replay. When a persisted changeset carries an unconfirmed Utxo and the in-memory entry is already confirmed, the existing state wins. is_confirmed and is_instantlocked are now OR'd together rather than overwritten, preventing a stale replay from regressing live state. - Transaction record routing now uses a single address-based strategy that scans both input and output addresses on the record. Replaces the previous primary/fallback split that silently dropped spending transactions whose outputs all went to external addresses (case 3: sweep/send-all with no change output). ## API - apply_changeset returns Result<(), ApplyError>. Account derivation failures (watch-only wallet missing the xpub, network mismatch) are now loud rather than silently skipped, because a missing account cascades into dropping every downstream entry. Orphan UTXOs and unroutable transaction records are still silently skipped. - New ApplyChangeSet trait. ManagedWalletInfo implements it; the WalletManager::apply_changeset wrapper is generic over it so PlatformWalletInfo can plug in without duplicating the split-borrow logic. - WalletError gains an ApplyChangeSet variant. key-wallet-ffi's error mapping learns the new variant. - WalletManager::apply_changeset only bumps structural_revision when the changeset is non-empty. Previously, no-op replays triggered spurious observer refresh cycles. ## Renames - AccountStateChangeSet.last_revealed -> highest_used. The field is populated from pool.highest_used, and the AddressPool helper that restores it (previously set_last_revealed, now set_highest_used) marks the address at the given index as used. The "last_revealed" name was misleading — it's a highest-used watermark, not a reveal watermark. ## Routing helpers in helpers.rs - find_account_for_transaction_record_mut — single-pass routing by any address on the record (inputs + outputs). Used by apply. - find_account_with_txid_mut — confirmation-upgrade routing for records already present on some account. - find_managed_account_by_index_mut — used by highest_used restoration to skip the inline all_accounts_mut().find(...) scan. ## Tests - apply_funding_changeset_mirrors_state_to_sibling_wallet now asserts G1: pool.highest_used matches between source and target after apply. - apply_account_keys_creates_new_managed_account (G2): applying a changeset with account_keys.added installs the managed account on a wallet that started without it. - apply_upgrades_mempool_utxo_to_confirmed (G3): a mempool->confirmed context upgrade carried via apply flips is_confirmed on the stored UTXO without losing the entry. - apply_does_not_downgrade_confirmed_utxo_on_stale_replay: F3 regression test for the monotonic flag merge. - apply_ignores_entries_that_cannot_be_routed now uses .expect on the Result return. - key-wallet-manager apply_tests: empty-changeset no-op no longer bumps structural_revision; added non-empty test that does bump. - apply.rs test file: all .unwrap() on TestWalletContext fields replaced with .expect(...) for consistency with the rest of the crate. ## Verification - key-wallet: 492 passed, 0 failed (+3 new apply tests) - key-wallet-manager: 39 passed, 0 failed (+1 empty vs non-empty bump) - key-wallet-ffi: clean - platform-wallet: clean (3 pre-existing dead-code warnings unrelated) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Restructures WalletChangeSet from a flat per-field design (utxos, transactions, account_states at the top level) to BDK's per-account bucketing: each account's deltas live in cs.per_account[AccountType], with wallet-level state (account_keys, chain, balance) staying at the top level. The old shape required address-based routing on every apply — each UTXO, every spent outpoint, every transaction record had to be scanned against every account's pool to find the owning account. The new shape pre-routes at mutation time: each ManagedCoreAccount::mark_* / record_* method writes directly into cs.per_account[self_type] using its own account_type, and apply_changeset delegates per-bucket via a direct get_by_account_type_mut lookup on the ManagedAccountCollection. Consequences: - The multi-account transaction collision bug is gone. A cross-account tx (CoinJoin, internal transfer) carries one record per account in its respective bucket, not overwriting each other in a shared BTreeMap<Txid, TransactionRecord>. - find_account_by_address_mut, find_account_with_utxo_mut, find_account_for_transaction_record_mut, find_account_with_txid_mut, and find_managed_account_by_index_mut are all deleted from helpers.rs — nothing calls them anymore. - The "best-effort fallback routing" in the old apply_changeset (primary txid lookup + secondary address-based fallback) is gone. Replaced by a single-line per-bucket delegation. - Mutation methods keep their uniform return type: (bool, WalletChangeSet) / (TransactionRecord, WalletChangeSet). Callers still do result.changeset.merge(cs) regardless of whether the mutation is account- or wallet-level. No new routing dance at accumulation sites. - AccountType and StandardAccountType now derive PartialOrd / Ord / Hash so AccountType can be used as a BTreeMap key. All fields are already Ord (u32, [u8; 32], StandardAccountType) so the derives are pure trait impls. - New AccountChangeSet struct scoped to one managed account: addresses_used, highest_used (per pool_type), utxos_added / utxos_spent / utxos_instant_locked, transactions. Deduplicates transaction records by txid on merge so re-merging the same cs is idempotent. - New ManagedCoreAccount::apply(&AccountChangeSet) inherent method mirrors the mutation methods — called by ManagedWalletInfo:: apply_changeset once it has routed a bucket. - New ManagedAccountCollection::get_by_account_type_mut — direct routing for apply's per-bucket delegation. Replaces the manual all_accounts_mut().find(|a| a.index() == ...) scan. - ApplyError / ApplyChangeSet trait stay — WalletManager<T>:: apply_changeset wrapper is still generic over them. Test coverage: - changeset_mutation_tests.rs rewritten to assert against the new cs.per_account[bip44_0()] bucket shape. - New account_changeset_merge_dedups_transactions_by_txid test in changeset.rs verifies the TransactionRecord last-write-wins. - New wallet_changeset_merge_merges_per_account_buckets test verifies max-wins highest_used and cross-account bucket addition. - apply.rs tests continue to exercise G1 (highest_used restoration), G2 (account_keys replay), G3 (mempool -> confirmed UTXO upgrade), and the F3 stale-replay monotonic flag regression. The apply_ignores_entries_that_cannot_be_routed test was rewritten as apply_skips_buckets_for_unknown_account_types to reflect the new bucket-based semantics. - changeset_support_tests.rs: the find_account_* tests are replaced with get_by_account_type_mut coverage. Verification: - key-wallet: 492 passed, 0 failed - key-wallet-manager: 39 passed, 0 failed - key-wallet-ffi: clean - platform-wallet: clean (unchanged — delegations to core_wallet continue to return WalletChangeSet, which still merges correctly under the new internal shape) Net -43 LOC across the diff (the deleted routing helpers dwarf the new AccountChangeSet definition). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Addresses the second reviewer round on commit c07207a. Correctness, consistency, observability, and documentation fixes for the per-account changeset shape. No behavior change on the happy path; closes several latent bugs on edge cases. ## Correctness - AccountChangeSet.transactions is now BTreeMap<Txid, TransactionRecord> rather than Vec. The map shape matches the invariant "one record per txid per account" directly: last-write-wins on merge is free via BTreeMap::extend, and the custom O(N²) dedup loop is gone along with its dedicated test. - mark_utxos_instant_send no longer assigns the whole utxos_instant_locked set via =, which clobbered any prior merged state on the bucket. Switched to per-outpoint insert inside the loop, matching every other mutation method. - update_utxos uses the account_bucket(ty).field.insert(...) inline pattern like its peers. Deleted the local-bucket-then-final-insert variant and its empty-bucket guard; WalletChangeSet::is_empty already handles empty buckets transitively. - apply_changeset's idempotent account_keys check was using get_by_account_type_mut().is_none(), which is hardcoded to return None for AccountType::PlatformPayment (platform accounts live in a separate map with a different managed type). A PlatformPayment replay would always call add_managed_account twice and fail the second time with "already exists". Introduced ManagedAccountCollection::contains_account_type which checks both collections and use it for the existence check. ## Observability - Unknown per_account buckets in apply are still silently skipped but now emit a tracing::warn!(?account_type, ...) so a split-brain between the account_keys step and the bucket loop is visible instead of invisibly dropping data. ## Naming + API - Renamed ManagedCoreAccount::apply → apply_changeset to match ManagedWalletInfo::apply_changeset and WalletManager::apply_changeset. apply_changeset also now takes an expected_account_type parameter and debug_asserts that the caller routed the bucket to the right account. - Deleted the ApplyChangeSet trait. Its only impl was ManagedWalletInfo and its only caller was WalletManager<T>::apply_changeset; platform- wallet does not use it (grep confirms zero hits). Collapsed the WalletManager wrapper back to a concrete impl WalletManager<ManagedWalletInfo> block. - Renamed ManagedAccountCollection::get_by_account_type_match[_mut] → get_by_account_type_matching[_mut]. The _match suffix was ambiguous with the new exact-lookup get_by_account_type[_mut] methods. - Added ManagedAccountCollection::get_by_account_type (immutable twin) for symmetry with the existing _mut variant. - Changed get_by_account_type_mut / get_by_account_type / contains_account_type to take AccountType by value instead of &AccountType — AccountType is Copy and the apply loop was doing &*key dance only because of the &-taking signature. ## Docs - AccountType gets a prominent comment explaining that the derived Ord follows variant + field declaration order, and that reordering here is a persistent storage-format break (affects BTreeMap iteration order and bincode discriminants). Add new variants at the end only. - Fixed stale doc comment in update_transaction_context that claimed it returned an empty AccountChangeSet (actual return is WalletChangeSet). - mark_utxos_instant_send doc now references cs.per_account[self.account_type.to_account_type()].utxos_instant_locked instead of the old flat cs.utxos_instant_locked path. - record_transaction and mark_address_used docs use uniform cs.per_account[self.account_type.to_account_type()].<field> notation. ## Tests - apply_skips_buckets_for_unknown_account_types → rewritten as apply_routes_known_bucket_and_skips_unknown. The old version only exercised the full-miss case on a wallet with zero accounts. The new version builds a changeset with one known bucket (BIP44-0, highest_used=5) and one unknown bucket (BIP44-99), applies both, and asserts the known bucket landed on the target's external pool while the unknown was silently dropped. - Updated changeset_mutation_tests.rs for BTreeMap-keyed transactions (contains_key / get(&txid) instead of .len() + index 0). ## Verification - key-wallet: 491 passed, 0 failed - key-wallet-manager: 39 passed, 0 failed - key-wallet-ffi: clean - platform-wallet: clean (3 pre-existing warnings unrelated) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Deletes the add_to_state: bool parameter from every address generation
method on AddressPool, ManagedCoreAccount, and ManagedPlatformAccount.
The flag had one behavior in practice — every real caller passed true.
The only false was a vestigial internal use in next_private_key that
relied on NoKeySource erroring before the flag mattered.
Methods that lost the parameter:
- AddressPool: generate_address_at_index, generate_addresses,
next_unused, next_unused_with_info, next_unused_multiple,
next_unused_multiple_with_info
- ManagedCoreAccount: next_receive_address, next_change_address,
next_receive_addresses, next_change_addresses, next_address,
next_address_with_info, next_bls_operator_key,
next_eddsa_platform_key
- ManagedPlatformAccount: next_unused_address,
next_unused_platform_address
Removed the `if add_to_state { ... }` branch in generate_address_at_index
that conditionally skipped inserting into self.addresses /
address_index / script_pubkey_index / highest_generated. Every
generated address is now unconditionally persisted into pool state —
that was the documented intent anyway.
No peek variants are being introduced. YAGNI: nothing in the codebase
needs pure-read address derivation right now. If a future caller does,
they can add a `peek_*` method then.
Call sites updated (~35 across the workspace):
- key-wallet-manager/src/lib.rs (18 sites)
- key-wallet/src/transaction_checking/wallet_checker.rs (6 sites)
- key-wallet/src/transaction_checking/transaction_router/tests/ (10 sites)
- key-wallet/src/tests/{address_pool_tests,performance_tests}.rs
- key-wallet-ffi/src/{managed_wallet,address_pool}.rs
- key-wallet/src/wallet/managed_wallet_info/asset_lock_builder.rs
- key-wallet-manager/tests/spv_integration_tests.rs
- key-wallet/src/test_utils/wallet.rs
Net -14 LOC. Verification: 491 key-wallet tests pass, 39
key-wallet-manager tests pass, key-wallet-ffi compiles clean.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Eliminates the hidden clones at every insert site in the apply path.
Three signatures flipped, three layers, no more `.clone()` cascade:
* `ManagedCoreAccount::apply_changeset(account_type, cs: AccountChangeSet)`
— destructures the bucket and drains every map / set. Each
`Utxo` and each `TransactionRecord` (which contains a full
`Transaction`) now moves directly from the persisted blob into
the in-memory map. Previously: `let mut merged = utxo.clone()`
on every UTXO and `record.clone()` on every transaction
(managed_account/mod.rs:745, :768).
The monotonic-flag merge on UTXOs (`is_confirmed |= existing`)
is preserved: the incoming UTXO is taken by value, then existing
flags are OR'd in before insertion. No clone of either side.
* `ManagedWalletInfo::apply_changeset(wallet, cs: WalletChangeSet)`
— destructures `WalletChangeSet` into its four sub-fields and
iterates `account_keys.added` / `per_account` by value. Each
per-account bucket moves into `ManagedCoreAccount::apply_changeset`
by value, completing the no-clone chain.
* `WalletManager::apply_changeset(wallet_id, changeset:
WalletChangeSet)` — concrete-type wrapper at the manager layer.
Captures `is_empty()` BEFORE consuming so the structural-revision
bump still fires conditionally; after that point the changeset
is moved straight into the apply path.
Single-variant API. The borrow form is gone, not co-existing — a
two-overload API would just invite the wrong one to be reached for.
Tests that legitimately need to apply the same changeset twice
(`apply_is_idempotent_on_sibling_wallet`,
`apply_does_not_downgrade_confirmed_utxo_on_stale_replay`)
explicitly `.clone()` the changeset at the call site. Those clones
are now visible at the call site instead of hidden inside the apply
path — exactly the place where the cost should be paid.
Deferred to `compound-engineering:research:performance-oracle` once
end-to-end persistence lands: profile the SPV-replay path and
confirm the `Transaction` clones inside `TransactionRecord` are
gone from flame graphs.
`cargo test -p key-wallet -p key-wallet-manager`: 570 tests pass
across all suites (491 lib + manager + integration). `cargo clippy`:
no new warnings (the 3 pre-existing ones in dashcore + key-wallet
style are untouched).
Refs: Phase 9a-3 follow-up "owned-cs apply" in
platform/packages/rs-platform-wallet/PERSISTENCE_REDESIGN.md.
Unblocks the matching platform-wallet flip and the persister
adapter rewrite (Phase 9a-5).
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Reviewer feedback on `b6479e9f` (owned-cs apply refactor): the second test site that explicit-clones a changeset for replay had the same 3-line comment as the first. The pattern is self-evident after the first occurrence — the reader who sees `cs.clone()` followed by `cs` in a double-apply test does not need it re-explained. `cargo test -p key-wallet -p key-wallet-manager`: all 570 tests still pass. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…minant
Item 6 prerequisite (Phase 10 uniform wallet state persistence).
Adds two helpers key-wallet consumers need when storing wallet state
in a persistent key/value form:
- `AccountType::to_db_key() -> Vec<u8>` and
`AccountType::from_db_key(&[u8]) -> Result<Self, _>` — uses the
bincode 2.x standard config to encode/decode the enum. Only
available when the `bincode` feature is enabled (same gate as
the existing `#[derive(Encode, Decode)]`). The type-level doc
comment already documents the storage-format stability contract
("variants are append-only, never reorder"), so the helpers
inherit that contract.
- `AddressPoolType::db_discriminant() -> u8` and
`AddressPoolType::from_db_discriminant(u8) -> Option<Self>` —
compact stable encoding for the unit enum. Doesn't need the
bincode feature because it's just a match on variants. Future
variants MUST be appended, never reordered.
No consumers in this commit — evo-tool's persister picks these up
in the Phase 10 6a schema + write path follow-up.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… bls/eddsa features Add insert_wallet() for inserting pre-built Wallet + info pairs, and get_wallet_and_info_mut() for split-borrowing (&Wallet, &mut T) from the two-map design. Also forward bls/eddsa feature flags to key-wallet. Needed by platform-wallet to integrate with the v0.42-dev WalletManager without modifying any existing WalletManager code. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Phase 1 (prep): - Fix AddressInfo::mark_used() hardcoded used_at=Some(0) → real timestamp - Make ManagedCoreAccount::current_timestamp() public - Add first_seen: u64 to TransactionRecord (restores pre-#582 behavior for mempool tx timestamps, needed by evo-tool wallet_transactions table) - Add helper accessors: is_instant_locked, is_chain_locked, block_height, block_hash - Derive PartialEq + Eq on TransactionRecord, InputDetail, OutputDetail (needed by changeset types) - Make Wallet::add_account() idempotent for xpub=None (deterministic derivation replays safely); still errors on explicit-xpub conflict - Add AddressPool::set_last_revealed(index) for apply() restore - Add ManagedCoreAccount::insert_utxo/remove_utxo idempotent wrappers - Add ManagedWalletInfo::find_account_by_address_mut/find_account_with_utxo_mut routing helpers for apply() Phase 2 (changeset module): - New key-wallet/src/changeset/ module with BDK-style atomic changesets - Merge trait + blanket impls for BTreeMap/BTreeSet/Option/Vec - WalletChangeSet with sub-changesets: AccountKeyChangeSet, AccountStateChangeSet, ChainChangeSet, UtxoChangeSet, TransactionChangeSet, BalanceChangeSet - Changesets carry native types directly (Utxo, TransactionRecord, AccountType) — no flattened *Entry wrappers, simpler apply logic - Merge semantics: max-wins for heights/last_revealed, sum for balance deltas, dedup for account types, extend for maps/sets Tests: 32 new (14 merge, 5 changeset, 4 address_pool, 11 changeset_support). Updated 3 existing tests that asserted add_account errors on duplicate. 478 total, all passing. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…reAccount Every state-mutating method on ManagedCoreAccount / ManagedWalletInfo now returns `(value, WalletChangeSet)` (or just `WalletChangeSet` for void returns). The changeset is built directly via `cs.field = Some(...)` -- no intermediate sub-changeset variables -- so callers uniformly do `result.changeset.merge(cs)` to accumulate. `TransactionCheckResult` grows a `changeset: WalletChangeSet` field populated by the orchestrator. Mutation methods covered: - mark_address_used -> (bool, WalletChangeSet) - update_utxos -> WalletChangeSet (private) - record_transaction -> (TransactionRecord, WalletChangeSet) - confirm_transaction -> (bool, WalletChangeSet) - mark_utxos_instant_send -> (bool, WalletChangeSet) - update_transaction_context(txid, new_context) -> WalletChangeSet (new) - update_balance -> WalletChangeSet - WalletInfoInterface::update_balance -> WalletChangeSet - WalletInfoInterface::mark_instant_send_utxos -> (bool, WalletChangeSet) Correctness fixes caught by review: - update_utxos now detects is_confirmed / is_instantlocked transitions on existing UTXOs and emits them to cs.utxos.added, so a persister replay cannot leave a stale Utxo after a mempool->block upgrade. - wallet_checker.rs gap-limit pass accumulates (account_index, pool_type) -> highest_used into cs.account_states.last_revealed so apply() can restore the reveal pointer without re-deriving. Required widening AccountStateChangeSet::last_revealed to BTreeMap<(u32, AddressPoolType), u32> and deriving PartialOrd / Ord / Hash on AddressPoolType. - update_transaction_context is idempotent: empty changeset when the record is missing or the context already matches. Both IS-lock orchestrators (wallet_checker.rs and mark_instant_send_utxos) now delegate to it instead of hand-rolling cs.transactions inserts. Call sites in key-wallet-manager and key-wallet-ffi updated for the new return shapes (mostly .0 tuple access where only the bool mattered). Tests: adds key-wallet/src/tests/changeset_mutation_tests.rs covering each mutation method's changeset shape, idempotency on replay, the update_transaction_context helper, the mempool->block UTXO context upgrade, and end-to-end accumulation into TransactionCheckResult. 486 key-wallet lib tests pass. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Introduces the restore path that mirrors the mutate-and-return changeset
API landed in the previous commit. A persister reads a WalletChangeSet
off disk and hands it to apply_changeset, which replays the deltas
idempotently onto the in-memory state.
Inverse of the mutation methods:
- account_keys -> wallet.add_account(ty, None) + mirror into
self.accounts via add_managed_account
(both idempotent from Phase 1)
- account_states -> mark_address_used per address + pool.set_last_revealed
per (account_index, pool_type) tuple
- utxos.added -> route by address, insert_utxo on owning account
(upsert: same path carries is_confirmed /
is_instantlocked upgrades)
- utxos.spent -> route by outpoint, remove_utxo
- utxos.instant_locked -> route by outpoint, flip is_instantlocked
- transactions.records -> insert into owning account; routed first by
txid membership, falling back to address
lookup on the record's output scripts
- chain.synced_height -> max-wins assignment on metadata
- balance -> recomputed from the restored UTXO set;
cached deltas in cs.balance are discarded
Invariants:
- Idempotent: apply(apply(cs)) produces the same state as apply(cs).
- No re-emission: apply does not return a WalletChangeSet, so the
returned changesets from inner calls (mark_address_used, etc.) are
intentionally discarded.
- Best-effort routing: unroutable entries (orphaned UTXOs, foreign
addresses) are silently skipped rather than panicking.
New files:
- key-wallet/src/wallet/managed_wallet_info/apply.rs — the ManagedWalletInfo
inherent impl plus 3 round-trip tests:
* funding changeset replays to sibling wallet (UTXO + tx + balance +
address-used mirror)
* double-apply is idempotent
* unroutable entries leave target state unchanged
key-wallet-manager additions:
- get_wallet_mut_and_info_mut(wallet_id) -> (&mut Wallet, &mut T) split
borrow on the two maps, required because apply_changeset takes
&mut Wallet for add_account.
- WalletManager<ManagedWalletInfo>::apply_changeset(wallet_id, cs) —
concrete-type wrapper that performs the split borrow, delegates to
ManagedWalletInfo::apply_changeset, and bumps structural_revision so
observers can detect the restore. PlatformWalletInfo will need its own
concrete impl in a follow-up (same pattern).
- 2 sanity tests: WalletNotFound path and empty-changeset no-op with
revision bump.
Tests: 489 key-wallet lib tests pass (was 489 — +3 new apply tests,
replacing some earlier test removals from the review pass).
38 key-wallet-manager tests pass (was 36 — +2 apply wrapper tests).
key-wallet-ffi and platform-wallet compile clean.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Addresses correctness, API, and consistency feedback from the three reviewers against commit e018f77. No behavior change for the happy path; closes several latent bugs on edge cases. ## Correctness - update_utxos applies state flags monotonically on replay. When a persisted changeset carries an unconfirmed Utxo and the in-memory entry is already confirmed, the existing state wins. is_confirmed and is_instantlocked are now OR'd together rather than overwritten, preventing a stale replay from regressing live state. - Transaction record routing now uses a single address-based strategy that scans both input and output addresses on the record. Replaces the previous primary/fallback split that silently dropped spending transactions whose outputs all went to external addresses (case 3: sweep/send-all with no change output). ## API - apply_changeset returns Result<(), ApplyError>. Account derivation failures (watch-only wallet missing the xpub, network mismatch) are now loud rather than silently skipped, because a missing account cascades into dropping every downstream entry. Orphan UTXOs and unroutable transaction records are still silently skipped. - New ApplyChangeSet trait. ManagedWalletInfo implements it; the WalletManager::apply_changeset wrapper is generic over it so PlatformWalletInfo can plug in without duplicating the split-borrow logic. - WalletError gains an ApplyChangeSet variant. key-wallet-ffi's error mapping learns the new variant. - WalletManager::apply_changeset only bumps structural_revision when the changeset is non-empty. Previously, no-op replays triggered spurious observer refresh cycles. ## Renames - AccountStateChangeSet.last_revealed -> highest_used. The field is populated from pool.highest_used, and the AddressPool helper that restores it (previously set_last_revealed, now set_highest_used) marks the address at the given index as used. The "last_revealed" name was misleading — it's a highest-used watermark, not a reveal watermark. ## Routing helpers in helpers.rs - find_account_for_transaction_record_mut — single-pass routing by any address on the record (inputs + outputs). Used by apply. - find_account_with_txid_mut — confirmation-upgrade routing for records already present on some account. - find_managed_account_by_index_mut — used by highest_used restoration to skip the inline all_accounts_mut().find(...) scan. ## Tests - apply_funding_changeset_mirrors_state_to_sibling_wallet now asserts G1: pool.highest_used matches between source and target after apply. - apply_account_keys_creates_new_managed_account (G2): applying a changeset with account_keys.added installs the managed account on a wallet that started without it. - apply_upgrades_mempool_utxo_to_confirmed (G3): a mempool->confirmed context upgrade carried via apply flips is_confirmed on the stored UTXO without losing the entry. - apply_does_not_downgrade_confirmed_utxo_on_stale_replay: F3 regression test for the monotonic flag merge. - apply_ignores_entries_that_cannot_be_routed now uses .expect on the Result return. - key-wallet-manager apply_tests: empty-changeset no-op no longer bumps structural_revision; added non-empty test that does bump. - apply.rs test file: all .unwrap() on TestWalletContext fields replaced with .expect(...) for consistency with the rest of the crate. ## Verification - key-wallet: 492 passed, 0 failed (+3 new apply tests) - key-wallet-manager: 39 passed, 0 failed (+1 empty vs non-empty bump) - key-wallet-ffi: clean - platform-wallet: clean (3 pre-existing dead-code warnings unrelated) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Restructures WalletChangeSet from a flat per-field design (utxos, transactions, account_states at the top level) to BDK's per-account bucketing: each account's deltas live in cs.per_account[AccountType], with wallet-level state (account_keys, chain, balance) staying at the top level. The old shape required address-based routing on every apply — each UTXO, every spent outpoint, every transaction record had to be scanned against every account's pool to find the owning account. The new shape pre-routes at mutation time: each ManagedCoreAccount::mark_* / record_* method writes directly into cs.per_account[self_type] using its own account_type, and apply_changeset delegates per-bucket via a direct get_by_account_type_mut lookup on the ManagedAccountCollection. Consequences: - The multi-account transaction collision bug is gone. A cross-account tx (CoinJoin, internal transfer) carries one record per account in its respective bucket, not overwriting each other in a shared BTreeMap<Txid, TransactionRecord>. - find_account_by_address_mut, find_account_with_utxo_mut, find_account_for_transaction_record_mut, find_account_with_txid_mut, and find_managed_account_by_index_mut are all deleted from helpers.rs — nothing calls them anymore. - The "best-effort fallback routing" in the old apply_changeset (primary txid lookup + secondary address-based fallback) is gone. Replaced by a single-line per-bucket delegation. - Mutation methods keep their uniform return type: (bool, WalletChangeSet) / (TransactionRecord, WalletChangeSet). Callers still do result.changeset.merge(cs) regardless of whether the mutation is account- or wallet-level. No new routing dance at accumulation sites. - AccountType and StandardAccountType now derive PartialOrd / Ord / Hash so AccountType can be used as a BTreeMap key. All fields are already Ord (u32, [u8; 32], StandardAccountType) so the derives are pure trait impls. - New AccountChangeSet struct scoped to one managed account: addresses_used, highest_used (per pool_type), utxos_added / utxos_spent / utxos_instant_locked, transactions. Deduplicates transaction records by txid on merge so re-merging the same cs is idempotent. - New ManagedCoreAccount::apply(&AccountChangeSet) inherent method mirrors the mutation methods — called by ManagedWalletInfo:: apply_changeset once it has routed a bucket. - New ManagedAccountCollection::get_by_account_type_mut — direct routing for apply's per-bucket delegation. Replaces the manual all_accounts_mut().find(|a| a.index() == ...) scan. - ApplyError / ApplyChangeSet trait stay — WalletManager<T>:: apply_changeset wrapper is still generic over them. Test coverage: - changeset_mutation_tests.rs rewritten to assert against the new cs.per_account[bip44_0()] bucket shape. - New account_changeset_merge_dedups_transactions_by_txid test in changeset.rs verifies the TransactionRecord last-write-wins. - New wallet_changeset_merge_merges_per_account_buckets test verifies max-wins highest_used and cross-account bucket addition. - apply.rs tests continue to exercise G1 (highest_used restoration), G2 (account_keys replay), G3 (mempool -> confirmed UTXO upgrade), and the F3 stale-replay monotonic flag regression. The apply_ignores_entries_that_cannot_be_routed test was rewritten as apply_skips_buckets_for_unknown_account_types to reflect the new bucket-based semantics. - changeset_support_tests.rs: the find_account_* tests are replaced with get_by_account_type_mut coverage. Verification: - key-wallet: 492 passed, 0 failed - key-wallet-manager: 39 passed, 0 failed - key-wallet-ffi: clean - platform-wallet: clean (unchanged — delegations to core_wallet continue to return WalletChangeSet, which still merges correctly under the new internal shape) Net -43 LOC across the diff (the deleted routing helpers dwarf the new AccountChangeSet definition). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Addresses the second reviewer round on commit c07207a. Correctness, consistency, observability, and documentation fixes for the per-account changeset shape. No behavior change on the happy path; closes several latent bugs on edge cases. ## Correctness - AccountChangeSet.transactions is now BTreeMap<Txid, TransactionRecord> rather than Vec. The map shape matches the invariant "one record per txid per account" directly: last-write-wins on merge is free via BTreeMap::extend, and the custom O(N²) dedup loop is gone along with its dedicated test. - mark_utxos_instant_send no longer assigns the whole utxos_instant_locked set via =, which clobbered any prior merged state on the bucket. Switched to per-outpoint insert inside the loop, matching every other mutation method. - update_utxos uses the account_bucket(ty).field.insert(...) inline pattern like its peers. Deleted the local-bucket-then-final-insert variant and its empty-bucket guard; WalletChangeSet::is_empty already handles empty buckets transitively. - apply_changeset's idempotent account_keys check was using get_by_account_type_mut().is_none(), which is hardcoded to return None for AccountType::PlatformPayment (platform accounts live in a separate map with a different managed type). A PlatformPayment replay would always call add_managed_account twice and fail the second time with "already exists". Introduced ManagedAccountCollection::contains_account_type which checks both collections and use it for the existence check. ## Observability - Unknown per_account buckets in apply are still silently skipped but now emit a tracing::warn!(?account_type, ...) so a split-brain between the account_keys step and the bucket loop is visible instead of invisibly dropping data. ## Naming + API - Renamed ManagedCoreAccount::apply → apply_changeset to match ManagedWalletInfo::apply_changeset and WalletManager::apply_changeset. apply_changeset also now takes an expected_account_type parameter and debug_asserts that the caller routed the bucket to the right account. - Deleted the ApplyChangeSet trait. Its only impl was ManagedWalletInfo and its only caller was WalletManager<T>::apply_changeset; platform- wallet does not use it (grep confirms zero hits). Collapsed the WalletManager wrapper back to a concrete impl WalletManager<ManagedWalletInfo> block. - Renamed ManagedAccountCollection::get_by_account_type_match[_mut] → get_by_account_type_matching[_mut]. The _match suffix was ambiguous with the new exact-lookup get_by_account_type[_mut] methods. - Added ManagedAccountCollection::get_by_account_type (immutable twin) for symmetry with the existing _mut variant. - Changed get_by_account_type_mut / get_by_account_type / contains_account_type to take AccountType by value instead of &AccountType — AccountType is Copy and the apply loop was doing &*key dance only because of the &-taking signature. ## Docs - AccountType gets a prominent comment explaining that the derived Ord follows variant + field declaration order, and that reordering here is a persistent storage-format break (affects BTreeMap iteration order and bincode discriminants). Add new variants at the end only. - Fixed stale doc comment in update_transaction_context that claimed it returned an empty AccountChangeSet (actual return is WalletChangeSet). - mark_utxos_instant_send doc now references cs.per_account[self.account_type.to_account_type()].utxos_instant_locked instead of the old flat cs.utxos_instant_locked path. - record_transaction and mark_address_used docs use uniform cs.per_account[self.account_type.to_account_type()].<field> notation. ## Tests - apply_skips_buckets_for_unknown_account_types → rewritten as apply_routes_known_bucket_and_skips_unknown. The old version only exercised the full-miss case on a wallet with zero accounts. The new version builds a changeset with one known bucket (BIP44-0, highest_used=5) and one unknown bucket (BIP44-99), applies both, and asserts the known bucket landed on the target's external pool while the unknown was silently dropped. - Updated changeset_mutation_tests.rs for BTreeMap-keyed transactions (contains_key / get(&txid) instead of .len() + index 0). ## Verification - key-wallet: 491 passed, 0 failed - key-wallet-manager: 39 passed, 0 failed - key-wallet-ffi: clean - platform-wallet: clean (3 pre-existing warnings unrelated) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Deletes the add_to_state: bool parameter from every address generation
method on AddressPool, ManagedCoreAccount, and ManagedPlatformAccount.
The flag had one behavior in practice — every real caller passed true.
The only false was a vestigial internal use in next_private_key that
relied on NoKeySource erroring before the flag mattered.
Methods that lost the parameter:
- AddressPool: generate_address_at_index, generate_addresses,
next_unused, next_unused_with_info, next_unused_multiple,
next_unused_multiple_with_info
- ManagedCoreAccount: next_receive_address, next_change_address,
next_receive_addresses, next_change_addresses, next_address,
next_address_with_info, next_bls_operator_key,
next_eddsa_platform_key
- ManagedPlatformAccount: next_unused_address,
next_unused_platform_address
Removed the `if add_to_state { ... }` branch in generate_address_at_index
that conditionally skipped inserting into self.addresses /
address_index / script_pubkey_index / highest_generated. Every
generated address is now unconditionally persisted into pool state —
that was the documented intent anyway.
No peek variants are being introduced. YAGNI: nothing in the codebase
needs pure-read address derivation right now. If a future caller does,
they can add a `peek_*` method then.
Call sites updated (~35 across the workspace):
- key-wallet-manager/src/lib.rs (18 sites)
- key-wallet/src/transaction_checking/wallet_checker.rs (6 sites)
- key-wallet/src/transaction_checking/transaction_router/tests/ (10 sites)
- key-wallet/src/tests/{address_pool_tests,performance_tests}.rs
- key-wallet-ffi/src/{managed_wallet,address_pool}.rs
- key-wallet/src/wallet/managed_wallet_info/asset_lock_builder.rs
- key-wallet-manager/tests/spv_integration_tests.rs
- key-wallet/src/test_utils/wallet.rs
Net -14 LOC. Verification: 491 key-wallet tests pass, 39
key-wallet-manager tests pass, key-wallet-ffi compiles clean.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Eliminates the hidden clones at every insert site in the apply path.
Three signatures flipped, three layers, no more `.clone()` cascade:
* `ManagedCoreAccount::apply_changeset(account_type, cs: AccountChangeSet)`
— destructures the bucket and drains every map / set. Each
`Utxo` and each `TransactionRecord` (which contains a full
`Transaction`) now moves directly from the persisted blob into
the in-memory map. Previously: `let mut merged = utxo.clone()`
on every UTXO and `record.clone()` on every transaction
(managed_account/mod.rs:745, :768).
The monotonic-flag merge on UTXOs (`is_confirmed |= existing`)
is preserved: the incoming UTXO is taken by value, then existing
flags are OR'd in before insertion. No clone of either side.
* `ManagedWalletInfo::apply_changeset(wallet, cs: WalletChangeSet)`
— destructures `WalletChangeSet` into its four sub-fields and
iterates `account_keys.added` / `per_account` by value. Each
per-account bucket moves into `ManagedCoreAccount::apply_changeset`
by value, completing the no-clone chain.
* `WalletManager::apply_changeset(wallet_id, changeset:
WalletChangeSet)` — concrete-type wrapper at the manager layer.
Captures `is_empty()` BEFORE consuming so the structural-revision
bump still fires conditionally; after that point the changeset
is moved straight into the apply path.
Single-variant API. The borrow form is gone, not co-existing — a
two-overload API would just invite the wrong one to be reached for.
Tests that legitimately need to apply the same changeset twice
(`apply_is_idempotent_on_sibling_wallet`,
`apply_does_not_downgrade_confirmed_utxo_on_stale_replay`)
explicitly `.clone()` the changeset at the call site. Those clones
are now visible at the call site instead of hidden inside the apply
path — exactly the place where the cost should be paid.
Deferred to `compound-engineering:research:performance-oracle` once
end-to-end persistence lands: profile the SPV-replay path and
confirm the `Transaction` clones inside `TransactionRecord` are
gone from flame graphs.
`cargo test -p key-wallet -p key-wallet-manager`: 570 tests pass
across all suites (491 lib + manager + integration). `cargo clippy`:
no new warnings (the 3 pre-existing ones in dashcore + key-wallet
style are untouched).
Refs: Phase 9a-3 follow-up "owned-cs apply" in
platform/packages/rs-platform-wallet/PERSISTENCE_REDESIGN.md.
Unblocks the matching platform-wallet flip and the persister
adapter rewrite (Phase 9a-5).
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Reviewer feedback on `b6479e9f` (owned-cs apply refactor): the second test site that explicit-clones a changeset for replay had the same 3-line comment as the first. The pattern is self-evident after the first occurrence — the reader who sees `cs.clone()` followed by `cs` in a double-apply test does not need it re-explained. `cargo test -p key-wallet -p key-wallet-manager`: all 570 tests still pass. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…rdcodes Mainnet `Address<NetworkChecked>::deserialize` was using `addr_unchecked.require_network(Network::Mainnet)`, which silently broke every testnet/regtest/devnet round-trip: - Any testnet `Address` serialized via serde would fail to deserialize with a "network mismatch" error. - Structs that embed `Address<NetworkChecked>` in serde-derived types (e.g. `InputDetail` in `key-wallet`'s `TransactionRecord`) would fail to decode on non-mainnet networks, and the failure cascaded as "skip entire TransactionRecord" for any consumer that log-and-skipped decode errors. The existing doc comment on the impl literally admitted it: "This is a limitation of deserializing without network context. Users should use Address<NetworkUnchecked> for serde when the network is not known at compile time." Meanwhile the native bincode `Decode` impl for `Address` at address.rs:864-874 already does the right thing — it decodes to `Address<NetworkUnchecked>` and calls `assume_checked()`. Align the serde impl with that behavior so the two serialization paths agree and `TransactionRecord` round-trips cleanly regardless of network. Callers that need actual network validation should deserialize as `Address<NetworkUnchecked>` and call `require_network(N)` explicitly. Flagged as a critical bug by rust-quality-engineer during the holistic review of Phase 10 Item 6c (evo-tool's `wallet_transactions.record` bincode-serde persistence path). Without this fix, Item 6c is broken on every non-mainnet deployment. 29 dashcore address tests pass. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
wallet-run had been rebased onto newer v0.42-dev than wallet2; this merge brings in the upstream fixes, plus resolves the mirror-commit overlap on key-wallet. No branch-specific logic — every wallet-run key-wallet commit has an identical mirror on wallet2. v0.42-dev upstream bits: - fix: announce tip to new peers when synced (#490) - fix: subscribe to SPV event monitors before startup (#636) - refactor: unify logging on tracing (#635) - chore: cleanup unused dependencies (#633) - fix: process broadcast transactions via dispatch_local (#626) - fix: gate FilterHeadersSyncComplete on block header sync (#631) - refactor: use String for TransactionRecord::label (#624) # Conflicts: # key-wallet/src/managed_account/transaction_record.rs
The asset-lock builder in key-wallet produced transactions that masternodes refused to IS-lock. Four issues: 1. `AssetLockPayload.version` was 0 — must be 1. 2. `tx.output[0]` was the credit output (p2pkh), but per DIP-00X that position must hold an OP_RETURN burn carrying the total locked amount. Credit outputs live only in the payload. Moved burn output construction into `ManagedWalletInfo::build_asset_lock`. 3. BIP-69 output sorting in `TransactionBuilder::build` moved the change output into position 0 (change is typically smaller than the burn), breaking the structural requirement. Added a `preserve_output_order` builder option and set it in `build_asset_lock`. 4. Tx wire version was the builder default (2) instead of the special-tx version 3. Symptom: self-broadcast asset-lock txs never received an `InstantLockReceived` event. Network-visible broadcasts succeeded, peers relayed them into mempool, but masternodes rejected them as invalid asset-lock transactions and never signed an IS-lock. Verified against `dash-evo-tool` e2e tests on testnet: identity_create and related tests go green after this fix.
The `dispatch_local` fix in #626 added a gate that rejected `broadcast_transaction` whenever `sync_progress().is_synced()` returned false. `is_synced()` requires *every* sync manager (headers / filter_headers / filters / blocks / masternodes) to be in `SyncState::Synced` simultaneously — any manager transitioning to `WaitForEvents` (e.g. the blocks manager briefly reacting to a new chain tip) flips the aggregate to "not synced" and blocks broadcasts. In end-to-end tests this manifested as intermittent `SpvBroadcastFailed { detail: "Client is not synced" }` errors in the middle of normal operation, hours after initial sync had completed. The gate served no purpose the underlying broadcast protocol doesn't already handle — peers themselves reject unfit-for-propagation txs. Drop it.
…adcast gate Brings in two bug fixes from wallet-run: 1. fix(key-wallet): correct AssetLockTx structure per DIP-00X The asset-lock builder produced transactions masternodes refused to IS-lock because of four structural issues: payload version was 0 instead of 1; the credit output was in tx.output[0] instead of an OP_RETURN burn; BIP-69 output sorting moved change into position 0; tx wire version was the builder default 2 instead of the special-tx version 3. Added preserve_output_order builder option and moved burn construction into build_asset_lock. 2. fix(dash-spv): remove overly-strict is_synced gate from broadcast The dispatch_local fix in #626 rejected broadcast_transaction when any sync manager briefly flipped to WaitForEvents (e.g. blocks manager reacting to a new tip). Symptom in e2e tests was intermittent 'Client is not synced' errors hours after initial sync. Drop the gate; peers enforce propagation fitness themselves.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…llet-sam-changes # Conflicts: # key-wallet/src/managed_account/mod.rs
DNS discovery alone may not return testnet peers. Add the 29 fixed hp-masternode IPs as fallback — they're always included in testnet peer discovery alongside DNS results. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
commit 5680cc36dc1406e918b976c237ad109f6df06c91
Author: xdustinface <xdustinfacex@gmail.com>
Date: Mon Apr 13 10:54:46 2026 +1000
fix(dash): store rotated quorums as unverified when CL sigs are missing
The rotation CL signature pre-check in `feed_qr_info` returned a hard
error when any historical CL signatures (sigm2, sigm1, sigm0) were
absent. This prevented `rotated_quorums_per_cycle` from being populated
on chains where ChainLock signatures weren't produced during data
generation (common in regtest pre-generated test chains).
IS lock verification needs `rotated_quorums_per_cycle` to find the
signing quorum by cyclehash. Without the cycle entry, every IS lock
fails with `CycleHashNotPresent` even though the quorum public keys
are valid.
Now, missing CL signatures produce a warning instead of an error. The
quorum entries are stored with `verifying_chain_lock_signature: None`
(unverified), allowing IS lock lookups to succeed. When CL sigs become
available in a later QRInfo (e.g. after a new DKG cycle produces
ChainLocks), the entries can be upgraded to fully verified.
commit 0f858245d2a37056f78aee2cb33e306ea19dc94d
Author: xdustinface <xdustinfacex@gmail.com>
Date: Mon Apr 13 10:08:13 2026 +1000
fix(dash): skip `sigmtip` requirement when tip diff has no rotating quorums
The rotation CL signature pre-check in `feed_qr_info` required all four
signatures including `sigmtip` even when the tip diff contained no
rotating-type quorum entries. This caused `MissingRotationSignatures`
errors whenever the tip was mid-cycle (before the DKG commitment is
mined), which is the common case.
Now the pre-check only requires `sigmtip` when
`tip_diff_has_rotating_quorums` is true. When the tip is mid-cycle,
the historical rotation quorums from `last_commitment_per_index` are
stored using `sigm0`/`sigm1`/`sigm2` (all present from cycle-boundary
diffs), and entries that cannot be fully validated are stored with
`verifying_chain_lock_signature: None` for later validation.
This enables IS lock verification on initial sync because
`rotated_quorums_per_cycle` is now populated even when the tip is
mid-cycle.
commit 154afa771cedc431867c66bd57e0848d696643a7
Author: xdustinface <xdustinfacex@gmail.com>
Date: Mon Apr 13 09:59:59 2026 +1000
fix(dash-spv): catch up MN list when headers arrive during initial sync
`BlockHeadersStored` events that arrive while the masternodes manager is
in `Syncing` state update `block_header_tip_height` but cannot trigger
incremental updates (the handler requires `Synced`). After the initial
sync completes and the state transitions to `Synced`, those events have
already been consumed and no more arrive.
The tick handler now detects this gap: when `Synced` with no pending
requests but `current_height < block_header_tip_height`, it triggers
the appropriate pipeline (QRInfo inside the mining window, or a targeted
MnListDiff otherwise) to catch up to the header tip.
commit 36d5561b4af4d6fc4286ba96628751f9f408055d
Author: xdustinface <xdustinfacex@gmail.com>
Date: Mon Apr 13 09:58:34 2026 +1000
fix(dash-spv): fall back to MnListDiff when QRInfo fails with missing tip sig
When the SPV is `Synced` and a QRInfo response fails with
`MissingRotationSignatures { [Tip] }` (the DKG commitment hasn't been
mined yet), fall back to a targeted MnListDiff instead of returning
empty. This keeps the tip masternode list fresh while the mining window
retries QRInfo on subsequent blocks.
Previously the code returned `Ok(vec![])` which blocked all MN list
updates inside the mining window when the DKG commitment was absent
(e.g. plain block generation without DKG orchestration, or when the
commitment block hasn't propagated yet).
Also relaxes the rotated quorum assertion in the end-to-end test since
`mine_dkg_cycle` mines 19 blocks which can overshoot the 18-block
mining window, causing a P2P timing-dependent race for QRInfo.
commit ad75763eb87c0b99f384aa8b88ae93ab09bec679
Author: xdustinface <xdustinfacex@gmail.com>
Date: Mon Apr 13 09:26:44 2026 +1000
test: remove rotated quorum assertion from initial sync check
The `rotated_quorums_per_cycle` assertion in the end-to-end test fired
during initial sync, before the DKG cycle is mined. With the partial
sync fix, rotated quorums are not stored until the mining window
mechanism fires a subsequent QRInfo after the commitment is mined.
commit 02711bf67eda72c3313179cf65478d640c0697b1
Author: xdustinface <xdustinfacex@gmail.com>
Date: Mon Apr 13 09:25:34 2026 +1000
tests
commit 59942005ace12140e902d8cb53e1d22312732afa
Author: xdustinface <xdustinfacex@gmail.com>
Date: Mon Apr 13 09:19:13 2026 +1000
fix(dash-spv): complete initial sync when tip rotation CL sig is missing
When `feed_qr_info` fails with `MissingRotationSignatures { [Tip] }`
during initial sync, the engine already has populated masternode lists
from the `apply_diff` calls that ran before the pre-check. Instead of
scheduling a retry loop (which never resolves on static chains or when
the tip is mid-cycle before the DKG commitment is mined), treat this as
a partial success: populate `known_mn_list_heights` from the engine and
call `complete_pipeline()` to run non-rotating quorum verification,
emit `MasternodeStateUpdated`, and transition to `Synced`.
The mining window mechanism picks up rotation validation later when the
DKG commitment is mined and new blocks arrive.
commit 752ff6c485d5e6e982abca2783f444825342930c
Author: xdustinface <xdustinfacex@gmail.com>
Date: Sat Apr 11 11:15:58 2026 +1000
fix(dash-spv): separate QRInfo and incremental mnlistdiff request flows
Splits the masternode-list catch-up path into two distinct flows:
- Full `getqrinfo` is fired at most once per rotation cycle plus a
short retry budget inside the cycle's DKG mining window. Any block
inside `[cycle_start + dkgMiningWindowStart, cycle_start +
dkgMiningWindowEnd]` can be the one that contains the rotated
commitment, so we fire QRInfo on every new header in that window
until one response succeeds. The mining window is short (e.g. 9
blocks for `llmq_60_75`), so per-cycle request volume is naturally
bounded by the window length.
- Targeted `GetMnListDiff` from the latest known masternode list tip
to the new header tip is fired for every other new header: before
the mining window opens, after the cycle has been freshly validated,
and after the mining window closes without a successful validation.
Keeps the tip masternode list fresh without re-running rotated
quorum validation or reapplying the historical cycle diffs.
Introduces two enums to keep the machinery explicit:
- `IncrementalUpdateAction { QRInfo, MnListDiffOnly }` -
`incremental_update_action()` consults the cycle's mining window,
validated-cycles set, and per-cycle tracking to pick one of the two
paths. Initial sync and explicit retry paths (ChainLock delay,
timeout) bypass it.
- `PipelineMode { QuorumValidation, Incremental }` -
`complete_pipeline()` dispatches on it when the mnlistdiff pipeline
drains:
- `QuorumValidation` runs the existing `verify_and_complete` flow
(hard-fails into Error state on verification failure, transitions
initial sync to Synced on success).
- `Incremental` runs `verify_non_rotating_masternode_list_quorums`
at the latest stored height; on failure logs a warning and stays
in `Synced` without emitting a spurious `MasternodeStateUpdated`.
Additional correctness fixes that go with the separation:
- `send_tip_mnlistdiff_update` sets `pipeline_mode = Incremental`
before queueing; the QRInfo response handler explicitly sets
`QuorumValidation` before queueing its historical diffs.
- `clear_pending()` resets `pipeline_mode` back to the default
`QuorumValidation`.
- On `apply_diff` failure for an in-flight mnlistdiff, if the pipeline
drains in `Incremental` mode we return `Ok(vec![])` instead of
calling `complete_pipeline()` - skipping the spurious completion
that would emit `MasternodeStateUpdated` against stale state.
- `last_synced_block_hash` is added as an explicit state field,
initialized from the engine's stored masternode lists in
`MasternodesManager::new` (so a restart resumes from the previous
run's last list), and refreshed on every successful pipeline
completion. It doubles as the base for `send_tip_mnlistdiff_update`.
- `mark_cycle_validated` is called from the QRInfo response handler
after `feed_qr_info` returns a summary where every rotated quorum
went through the fresh-validation path. Puts the cycle boundary
height in `validated_cycle_heights` so subsequent incremental
updates take the `MnListDiffOnly` path until the next cycle rollover.
- On `feed_qr_info` returning
`MissingRotationSignatures { missing: [Tip] }` while the sync state
is `Synced` (an incremental update), the handler logs at debug level
and returns `Ok(vec![])`, staying `Synced` so the next
`BlockHeadersStored` event drives the next attempt via the
incremental update action gate. Only when the state was not already
`Synced` (initial sync) does the tip-ChainLock time-based retry
path kick in. Uses the typed `RotationChainLockSignatureSlot::Tip`
enum variant rather than string-matching on sig names.
- A per-cycle `current_cycle_degraded_logged` flag ensures the
"mining window closed without a successful fresh validation"
warning fires exactly once per cycle, not on every header in the
degraded range.
commit 46a4c62a770cc76e2c44373c900e647b42678267
Author: xdustinface <xdustinfacex@gmail.com>
Date: Sat Apr 11 11:07:46 2026 +1000
fix(dash-spv): use single historical anchor for QRInfo base hashes
Replaces the previous "send every known block hash plus the last
processed tip" base hash policy with a single anchor computed from
the current tip and the engine's stored masternode lists.
Why this matters. The server's non-legacy `BuildQuorumRotationInfo`
chains historical cycle diffs by picking the highest client-provided
base less-than-or-equal to each diff's target. A cycle X's rotated
quorum commitment is mined unpredictably anywhere in
`[X + dkgMiningWindowStart, X + dkgMiningWindowEnd]`, so any base hash
that lands in or after the mining window can produce a
`(base, target]` range that misses the commit block entirely -
yielding empty `quorumsCLSigs` and missing `sigm0..sigm2` on the
client side. Clients that keep sending recent hashes
(`last_qrinfo_block_hash`, latest processed `MNLISTDIFF` endpoints)
walk straight into this trap.
A single anchor four full cycles behind the current cycle clears the
start of the oldest historical diff's mining window with a full cycle
of margin, letting the server's chaining produce clean one-cycle-wide
windows for every historical diff.
The anchor must also be a block where the engine already has a
masternode list stored - otherwise `apply_diff` on the historical
diffs fails with `MissingStartMasternodeList` because it has no base
list to apply against. `compute_qrinfo_anchor_hash` therefore picks
from `engine.masternode_lists`, not from header storage, selecting
the highest stored list with height `<= tip_cycle_start - 4 * dkg_interval`.
On a fresh restart with an empty engine, it returns `None` and the
caller sends an empty `baseBlockHashes` vector, letting the server
fall back to genesis, which `apply_diff` handles via its genesis-base
path.
Removes `known_block_hashes` and `last_qrinfo_block_hash` dead fields
and their write sites. Neither is read any more after this change.
commit 3572954b77750bd40bb7fcaa0771ba4ef4e76b8b
Author: xdustinface <xdustinfacex@gmail.com>
Date: Sat Apr 11 11:03:41 2026 +1000
fix(dash): require all four rotation CL sigs; reject incomplete QRInfos
Replaces the four sequential `ok_or` checks in `feed_qr_info` (which
errored on the first missing sig) with a single upfront pre-check that
collects every absent signature into one `MissingRotationSignatures`
error. Diagnostics on a broken QRInfo now name all missing sigs at
once instead of forcing operators to retry-and-rediscover them one at
a time.
The pre-check only runs when at least one quorum in
`last_commitment_per_index` is not already in
`known_qualified_quorum_entry`. Cycles whose rotated quorums are all
already validated still pass through without needing fresh sigs.
Uses the typed `RotationChainLockSignatureSlot` enum introduced in the
previous commit instead of stringly-typed sig names, so error-handling
code in `dash-spv` can match on the enum variants directly. The new
variant is appended at the end of `QuorumValidationError` to preserve
the existing `bincode` discriminant numbering for serialized fixtures.
commit 34df6447f47d603f1876d7ed0aa7aed081ab24b3
Author: xdustinface <xdustinfacex@gmail.com>
Date: Sat Apr 11 11:00:55 2026 +1000
refactor(dash): introduce `RotationChainLockSignatureSlot` enum
Adds a typed identifier for the four rotation ChainLock signature
slots carried by a QRInfo response: the three historical cycle diffs
(`mnListDiffAtHMinus2C`, `mnListDiffAtHMinusC`, `mnListDiffH`) and the
tip diff (`mnListDiffTip`).
This exists so error paths and diagnostic code across the `dash` /
`dash-spv` boundary can identify which slot is missing without
string-matching on the legacy short names (`sigm0`, `sigm1`, `sigm2`,
`sigmtip`). The `Display` impl still prints the legacy short names so
log output remains unchanged for operators.
Pure preparatory refactor for the upcoming strict sig-requirement
check in `feed_qr_info`. No behavior change.
commit bb76e8ac53c43a37c0d450638bb623384b3df6bb
Author: xdustinface <xdustinfacex@gmail.com>
Date: Sat Apr 11 00:47:52 2026 +1000
refactor(dash): return `QRInfoFeedSummary` from `feed_qr_info`
Introduces a push-model observability report returned from
`MasternodeListEngine::feed_qr_info`, replacing the pattern of
iterating engine state after-the-fact to derive log counters.
The summary captures the current cycle boundary hash and height, the
total number of rotated quorums in `last_commitment_per_index`, and
how many of those went through the fresh-validation path versus being
reused from a previously stored qualified quorum entry. Nothing is
persisted — it exists only for the duration of the call.
Benefits over storing provenance as a field on `QualifiedQuorumEntry`:
- No per-entry storage cost and no serde/bincode schema bump
- No conflation of "status" and "provenance" on a single enum
- Lets future cycle-aware schedulers decide "did this cycle freshly
validate this call" without peeking into engine state
`sync_manager::log_qrinfo_feed_summary` logs the summary at info level
when the cycle boundary is known, at warn level when it is not.
commit 285041a2d80a76661308b02be111ce44fd9eabf4
Author: xdustinface <xdustinfacex@gmail.com>
Date: Sat Apr 11 00:42:21 2026 +1000
fix(dash-spv): feed commitment quorum heights to engine
IS lock and rotated quorum formation verification both require the
engine to look up the masternode list at each quorum's block height.
The engine stores these heights in `block_container`, but
`feed_qrinfo_heights_to_engine` only fed heights for diff base/target
blocks and cycle boundaries.
Each `QuorumEntry::quorum_hash` in `last_commitment_per_index` IS a
block hash — the block where that quorum's DKG commitment was mined.
For rotating quorums these are sequential blocks near the cycle
boundary (Q[0] at C, Q[1] at C+1, ..., Q[31] at C+31). Only Q[0] and
any quorums that happened to be a diff endpoint were previously fed,
leaving ~30/32 heights missing and causing lookup failures downstream.
Include all `last_commitment_per_index` quorum hashes in the feed.
At most 32 extra header lookups per QRInfo call.
commit 987244ba23fb638d1c3b28423d7edc0dd2dffb73
Author: xdustinface <xdustinfacex@gmail.com>
Date: Sat Apr 11 23:20:52 2026 +1000
fix(dash): key rotated quorums by cycle boundary hash
IS locks set their `cyclehash` field to the block hash of the cycle
boundary (the most recent block at a height divisible by `dkgInterval`
at the time of signing). After a cycle boundary crosses, all new IS
locks reference the new cycle boundary hash.
Previously `rotated_quorums_per_cycle` was keyed by the first quorum's
`quorum_hash` - a block hash from the PREVIOUS cycle where the quorum
was committed. After a cycle boundary, IS lock lookups for the new
cycle always failed with `CycleHashNotPresent`, leaving transactions
permanently in `pending` state.
Derive the current cycle boundary hash from `mn_list_diff_h` (at work
block height `cycle_boundary - WORK_DIFF_DEPTH`) and use it as the map
key in both the fresh-validation and passive-storage branches of
`feed_qr_info`. Emit a loud warning if the derivation fails so silent
storage misses are visible in logs.
The inner container is already a `BTreeMap<u16, QualifiedQuorumEntry>`
as of the preceding commit, so `insert` replaces at the same
`quorum_index` - no `.clear()` workaround needed to prevent stale
entries from accumulating across repeated `feed_qr_info` calls for the
same cycle.
Extends `validate_from_qr_info_and_mn_list_diffs` to assert the new
keying invariant against the existing QRInfo fixture.
commit 7164b0644a938a85abad3e5719dc1aa37c8cc204
Author: xdustinface <xdustinfacex@gmail.com>
Date: Sun Apr 12 09:57:38 2026 +1000
Squashed commit of the following:
commit 31f7258
Author: xdustinface <xdustinfacex@gmail.com>
Date: Sun Apr 12 03:03:57 2026 +1000
fix: index rotated quorums by `quorum_index` and rebuild per cycle
The `Vec<QualifiedQuorumEntry>` had no size bound and no uniqueness check on `quorum_index`. Combined with QRInfo still being requested on every new block (will change soon), repeated `feed_qr_info` calls within a single rotation cycle accumulated duplicate entries that the lookup then returned as stale matches.
- Switch the inner container to `BTreeMap<u16, QualifiedQuorumEntry>` keyed by `quorum_index` so insertion replaces by key, lookup is direct, and re-feeds of the same cycle are not causing wrong behaviour anymore.
- Introduce `insert_cycle_quorum_by_index` as helper to centralize inserts.
- Both branches of `feed_qr_info` now `.clear()` the per-cycle map before refilling it and clearing is enforced via a debug assert.
- Migrates `tests/data/test_DML_diffs/masternode_list_engine.hex` to the new format.
commit f7d0c764e266bea2fe313c0a0b32827a438f98fd
Author: xdustinface <xdustinfacex@gmail.com>
Date: Sun Apr 12 20:57:53 2026 +1000
Squashed commit of the following:
commit e2581996de0dedccfdd148ee869b2bc9b0c3691f
Author: xdustinface <xdustinfacex@gmail.com>
Date: Sun Apr 12 00:05:49 2026 +1000
test: add IS-across-quorum-rotation test
Exercises InstantSend signing continuity across a DIP-0024 DKG cycle
boundary. Drives the chain to one block before the next DKG cycle
boundary, sends an IS tx that lands in the boundary block, then runs a
full rotation through `mine_dkg_cycle_with_hook` with a per-block
callback that sends one additional IS tx after every block mined during
phases 1-6, the commitment block, and the 8 maturity blocks. Finally
sends one more post-rotation IS tx on a subsequent block.
Every transaction — pre-cycle, in-cycle, and post-cycle — must produce
a validated `InstantLockReceived` event, and the
`InstantSendProgress.valid` counter must reflect the full count. This
proves signing quorum availability remains continuous across the
rotation transition window.
commit 77f38d08fdbb61fda477e8be1885c24dd26d59c6
Author: xdustinface <xdustinfacex@gmail.com>
Date: Sun Apr 12 00:03:51 2026 +1000
refactor: add `mine_dkg_cycle_with_hook` to masternode harness
Splits the existing `mine_dkg_cycle` body into a `_with_hook` variant
that invokes a caller-supplied closure after every block mined past the
cycle alignment point (phases 1-6 advance blocks, the commitment block,
and all 8 maturity blocks).
The hook receives a shared reference to the controller node so tests can
issue RPC calls (for example `send_to_address`) between DKG blocks
without fighting borrow checker constraints on `MasternodeTestContext`.
Existing `mine_dkg_cycle` becomes a thin wrapper passing a no-op hook,
so every current caller is unaffected.
commit 2c5bfe73605ba9f578539db076067c61a774d708
Author: xdustinface <xdustinfacex@gmail.com>
Date: Sat Apr 11 10:53:26 2026 +1000
test: add masternode InstantSend islock-before-tx ordering test
Shuts down the SPV client before sending, waits for the controller to
record an islock for the transaction, then reconnects a fresh SPV client
so the tx and the islock arrive together with no guaranteed ordering.
Exercises the `pending_is_locks` path in the mempool manager when the
islock lands before the matching mempool entry, and asserts the
end-to-end outcome (validated InstantLockReceived event + wallet
InstantSend status) regardless of internal ordering.
commit 9c5560f16d5d32d8c1fea5d526cd8ddd03de7a4d
Author: xdustinface <xdustinfacex@gmail.com>
Date: Sat Apr 11 10:52:09 2026 +1000
test: add masternode InstantSend full lifecycle test
Exercises the end-to-end InstantSend flow against the masternode harness:
initial sync, DKG cycle to form a live signing quorum, three concurrent
transactions from the controller wallet, validated InstantLockReceived
events, wallet InstantSend status propagation, and the transition to an
InChainLockedBlock context after a subsequent chainlock.
Adds shared wait helpers (wait_for_instant_lock_received,
wait_for_wallet_tx_status, wait_for_instantsend_valid_at_least), a
wallet-event receiver on ClientHandle, and a controller-mnemonic wallet
constructor.
commit 8e39abe7e79e5cda5a7d34ab6a765664851e6fbb
Author: xdustinface <xdustinfacex@gmail.com>
Date: Sat Apr 11 10:47:55 2026 +1000
refactor: split dashd_masternode tests into directory layout
Mirrors the existing dashd_sync/ test binary layout. Shared harness code
(TestContext, ClientHandle, create_and_start_client, wait helpers)
moves into setup.rs/helpers.rs so follow-up test files can reuse it
without duplication.
commit 8b53f8a98ca5e239f54ea28745415ae5d844e265
Author: xdustinface <xdustinfacex@gmail.com>
Date: Fri Apr 10 20:49:21 2026 +1000
test: require ChainLock in masternode sync end-to-end test
Change `test_masternode_list_sync_end_to_end`'s ChainLock wait from
warn-and-continue to a hard assertion. After a completed DKG cycle the
`llmq_test` quorum is expected to produce ChainLocks, so absence is a
real failure worth reporting. Also bump the ChainLock wait timeout
from 30s to 60s to give the fresh quorum time to start signing after
DKG completion.
commit d3e8f8ac068d6da03b6e24a37488e47110842305
Author: xdustinface <xdustinfacex@gmail.com>
Date: Fri Apr 10 20:22:49 2026 +1000
test: persist masternode test datadirs for debug log retention
Enable `-debug=all` / `-debuglogfile` on all nodes, leak the per-node
`TempDir`s (via `.keep()`), and retain each datadir via
`retain_test_dir` in `Drop` so `debug.log` is still available after
a test run finishes or fails.
commit 00e2279af02b319ca751ba5d80e935859a9f5590
Author: xdustinface <xdustinfacex@gmail.com>
Date: Fri Apr 10 20:22:23 2026 +1000
feat: add masternode network test infrastructure
Add `extra_args` to `DashCoreConfig` for passing custom dashd arguments.
Add `MasternodeTestContext` for managing a pre-generated masternode
network (controller + N masternodes) during integration tests. Reads
`network.json` metadata and starts dashd instances from copied
pre-generated datadirs with dynamic port allocation.
Includes DKG cycle orchestration helpers (`mine_dkg_cycle`,
`mine_blocks_and_wait_for_chainlock`) and end-to-end sync test exercising
initial masternode list sync, a controlled DKG cycle, and ChainLock
propagation to the SPV client.
…ck changesets Adds a pluggable WalletPersistence trait to WalletManager so SPV clients can persist wallet state (UTXOs, transactions, synced height) during block processing without a separate wrapper layer. - New `WalletPersistence` trait with `store(wallet_id, WalletChangeSet)` + `flush(wallet_id)` methods; `NoWalletPersistence` no-op for default usage - `WalletManager::new_with_persister(network, Arc<dyn WalletPersistence>)` constructor; existing `new()` delegates to it with the no-op persister - `check_transaction_in_all_wallets` now returns `(CheckTransactionsResult, BTreeMap<WalletId, WalletChangeSet>)` so per-wallet changesets flow up to the caller - `process_block` accumulates per-tx changesets, appends a height changeset for every wallet (so synced height is always persisted), calls `persister.store()` + `persister.flush()` and returns `Result<BlockProcessingResult, WalletError>` — persistence errors are propagated rather than silently suppressed - New `WalletError::Persistence` variant wrapping `Box<dyn Error + Send + Sync>` - dash-spv: adds `SyncError::Wallet(String)` variant and `From<WalletError>` impl; propagates `process_block` errors via `?` Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Address review findings from the WalletPersistence commit: - MockWallet/NonMatchingMockWallet: process_block returns Result now - FFI wallet_manager_process_transaction: destructure tuple return from check_transaction_in_all_wallets - FFI error.rs: add WalletError::Persistence match arm - Integration tests: unwrap process_block Result - event_tests/test_helpers: already used let _ = (no changes needed) - Batch persistence: store all wallets first, then flush all — backends that support batch-atomic writes can treat the flush pass as the commit boundary - Avoid unnecessary .clone() in changeset accumulation — use entry API - Fix stale doc comment referencing CorePersistence → WalletPersistence Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The persister implementation decides its own flush strategy (e.g. SQLite flushes inline on each store call). Having process_block call flush explicitly couples dashcore to a specific flush strategy and is redundant for existing backends. Simplify WalletPersistence to a single store() method — implementations handle flushing internally. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ore into feat/platform-wallet2
# Conflicts: # dash/src/sml/masternode_list_engine/mod.rs # dash/src/sml/quorum_validation_error.rs
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 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 |
This was referenced Apr 17, 2026
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
3 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.
This PR is not ready for review and should not be merged as-is. It tracks in-progress work on
feat/platform-wallet2(the dashpay/platform-facing wallet work) and exists purely to keep CI running againstv0.42-devand to surface integration breakage early.v0.42-dev; history is not cleaned up.Current state
v0.42-devintofeat/platform-wallet2. All masternode / SML code (dash/src/sml/,dash-spv/src/sync/masternodes/) taken fromv0.42-dev; theRotationChainLockSignatureSlot/QRInfoFeedSummary/MissingRotationSignaturesadditions that lived on this branch are gone in favor of PR fix: index rotated quorums byquorum_indexand rebuild per cycle #637's quorum-indexing fix.cargo build --workspace --all-featuresis clean.cargo test --workspace --all-features --libis clean.cargo test --workspace --all-features --no-runcompiles all integration binaries.Please do not
🤖 Draft managed with Claude Code