feat(dash-spv): floor mainnet sync at HD/BIP39 activation height#175
Open
xdustinface wants to merge 54 commits into
Open
feat(dash-spv): floor mainnet sync at HD/BIP39 activation height#175xdustinface wants to merge 54 commits into
xdustinface wants to merge 54 commits into
Conversation
* fix(dash): return known genesis hash for `Network::Devnet` The static height-0 devnet genesis block has a deterministic hash (verified by the existing `devnet_genesis_full_block` test). Returning `None` here caused `dash-spv` to fail with `"No known genesis hash for network"` when initializing storage for a devnet. Only the height-1 "devnet genesis" varies per devnet name, height 0 does not. * fix(dash): use `LlmqtypeDevnetPlatform` for `Network::Devnet` platform quorum `platform_type()` returned `LlmqtypeDevnet`, but Dash Core's devnet defaults use `LLMQ_DEVNET_PLATFORM` for `llmqTypePlatform` (see `chainparams.cpp:651`). This matches the standard devnet config `llmqplatform=llmq_devnet_platform`. * feat(dash-spv): add `devnet` to `--network` CLI Allows targeting `Network::Devnet` from the `dash-spv` binary. Required for SPV-syncing a devnet, since previously only mainnet, testnet, and regtest were exposed. * feat(dash-spv): add `--devnet-name` for devnet handshake compatibility Dash Core nodes on devnet check that incoming peers carry `devnet.<network-id>` in their user agent and disconnect otherwise (see `net_processing.cpp:3957-3967`). The `network-id` is `devnet-<name>` per `ArgsManager::GetDevNetName`, so the substring required for the `paloma` devnet is `devnet.devnet-paloma`. The new flag is required whenever `--network=devnet`, and the user agent is rebuilt as `/rust-dash-spv:<version>(devnet.devnet-<name>)/` so devnet peers accept the handshake. * feat: support `LLMQ_DEVNET` params override via `ClientConfig` Dash Core's `-llmqdevnetparams=<size>:<threshold>` lets a devnet adjust the `LLMQ_DEVNET` quorum size and threshold (`chainparams.cpp:UpdateLLMQDevnetParameters`). Without matching params, the SPV client picks the wrong masternodes when reconstructing the signing set, so ChainLock and legacy InstantSend signatures cannot be verified on devnets that use the override. `ClientConfig::llmq_devnet_params: Option<(u32, u32)>` carries the override and `DashSpvClient::new` applies it after `validate`. Internally this writes to a `OnceLock` in `dashcore::sml::llmq_type` that `LLMQType::params()` consults for `LlmqtypeDevnet`, mirroring Dash Core's chainparams-as-global model without exposing the setter beyond the lib. Only `LLMQ_DEVNET` is affected (matches Dash Core, the DIP0024 and platform devnet quorums are not adjusted by this flag). The `dash-spv` binary exposes the override as `--llmq-devnet-params=<size>:<threshold>`, valid only with `--network=devnet`. * fix(dash): make `set_llmq_devnet_params` idempotent for identical values The `apply_global_overrides` doc claimed idempotent behavior, but `OnceLock::set` rejected every second call regardless of value. Constructing multiple `DashSpvClient` instances with the same devnet config in one process therefore failed on the second call. Now the setter compares against the existing value and returns `Ok(())` when it matches, only erroring on a genuine conflict. Addresses CodeRabbit review comment on PR dashpay#784 dashpay#784 (comment) * test(dash): cover `LLMQ_DEVNET` override contract Add a single test that walks through valid initial set, idempotent reapplication of the same values, and rejection of a conflicting reapplication. The three checks share one test because `LLMQ_DEVNET_OVERRIDE` is a process-global `OnceLock` that cannot be reset between tests. Addresses CodeRabbit review comment on PR dashpay#784 dashpay#784 (comment) * refactor(dash): use `LlmqDevnetParams` struct instead of `(u32, u32)` tuple `Option<(u32, u32)>` was unreadable at call sites — `with_llmq_devnet_params(13, 9)` is one transposition away from a silent bug. Introduce a named struct in `dashcore::sml::llmq_type` so the size/threshold meaning is explicit everywhere it crosses an API boundary. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Quantum Explorer <quantum@dash.org> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
dashpay#781) `check_compact_filters_for_addresses` ran `address.script_pubkey()` per address per filter batch, pushing OP_DUP / OP_HASH160 / hash / OP_EQUALVERIFY / OP_CHECKSIG into a fresh `ScriptBuf` every call. A dhat profile of a full testnet sync showed `ScriptBuf::new_p2pkh` was hit ~7 million times for ~172 MB of lifetime bytes, dominated by this path. `AddressInfo` already stores `script_pubkey: ScriptBuf` from address generation. Read from that cache instead of reconstructing: - `AddressPool`, `ManagedAccountType`, `ManagedAccountRef`, and `ManagedAccountTrait` expose `all_script_pubkeys` alongside `all_addresses`. - `WalletInfoInterface` and `WalletInterface` expose `monitored_script_pubkeys` and `monitored_script_pubkeys_for`. The parallel `Vec<Address>` accessor is dropped, no remaining caller. - `BlockProcessingResult.new_scripts` and `SyncEvent::BlockProcessed.new_scripts` carry `Vec<ScriptBuf>` so the rescan path on `FiltersBatch` consumes cached scripts directly. - `check_compact_filters_for_addresses` becomes `check_compact_filters_for_script_pubkeys`, taking `&[ScriptBuf]` and handing the inner bytes to `BlockFilter::match_any`.
…ashpay#788) * fix(dash): decode `LlmqtypeDevnetPlatform` from `u8` `From<u8> for LLMQType` was missing the `107 => LlmqtypeDevnetPlatform` arm, so wire decoders silently mapped Platform quorum messages to `LlmqtypeUnknown`. Add it so devnet Platform routing can be observed by SPV when overrides are exercised. * feat(dash): add devnet LLMQ routing overrides Mirror Dash Core's `-llmqchainlocks`, `-llmqinstantsenddip0024`, and `-llmqplatform` flags with process-global `OnceLock` setters and getters. Wire them into `NetworkLLMQExt::{chain_locks_type, isd_llmq_type, platform_type}` so the `Network::Devnet` arms honor any active override. Setters reject types outside Dash Core's devnet allowlist (`50_60`, `60_75`, `400_60`, `400_85`, `100_67`, `devnet`, `devnet_dip0024`, `devnet_platform`) and enforce the rotation invariants Core applies in `chainparams.cpp`: ChainLocks must not rotate, InstantSend DIP24 must rotate, Platform has no rotation constraint. Setters are idempotent on identical re-set and error on conflict, matching the existing `set_llmq_devnet_params` contract. `enabled_llmq_types(Devnet)` expands to all eight devnet-registered LLMQ types so DKG window scheduling tracks any legal routing target. `devnet_llmq_type_from_name` parses the Dash-Core name set, accepting `llmq_dev_platform` as a defensive alias for this crate's local spelling of `LLMQ_DEV_PLATFORM`. * feat(dash-spv): consolidate devnet knobs into `DevnetConfig` Replace `ClientConfig.llmq_devnet_params` and the free-floating `--devnet-name` CLI flag with a single `Option<DevnetConfig>` field on `ClientConfig`. The struct collects every devnet-only setting in one place and surfaces the routing overrides added in the previous commit (`-llmqchainlocks`, `-llmqinstantsenddip0024`, `-llmqplatform`). `ClientConfig::validate` collapses to a biconditional: `devnet.is_some() == (network == Network::Devnet)`. The devnet user-agent suffix (`/rust-dash-spv:VERSION(devnet.devnet-NAME)/`) moves off `main.rs` and onto `DevnetConfig::user_agent`, so library consumers building a devnet client get the same handshake string without re-implementing it. `main.rs` gains `--llmq-chainlocks`, `--llmq-instantsend-dip0024`, `--llmq-platform` flags and extracts `build_client_config(&Args, PathBuf)` so the `Args` → `ClientConfig` mapping is unit-testable. * test: cover `DevnetConfig` validation and CLI mapping `config_test.rs` adds round-trip, user-agent format, table-driven validation matrix, empty-name rejection, and a single-test forwarding check that exercises all four `apply_global_overrides` slots in one shot. The forwarding test owns the process-global `OnceLock`s for the whole binary, comment block calls that out so future tests don't accidentally collide. `main.rs` gains a `tests` module that drives the extracted `build_devnet_config`, `build_client_config`, and `parse_llmq_devnet_params` helpers via clap's `try_parse_from`: covers missing `--devnet-name`, devnet flags rejected on non-devnet networks, minimal devnet build, full five-flag compose, parse-error shapes, and unknown quorum-name rejection. * chore: pr cleanup Hoist three duplicate `use crate::sml::llmq_type::network::NetworkLLMQExt;` lines out of individual lifecycle tests in `dash/src/sml/llmq_type/mod.rs` into the `mod tests` module top, and pull `use dashcore::sml::llmq_type::{...}` out of `test_apply_global_overrides_forwards_all_slots` in `dash-spv/src/client/config_test.rs`. Replace inline fully-qualified paths in `dash-spv/src/main.rs`: `dash_spv::ValidationMode` and `dash_spv::LLMQType::*` become normal imports; `tempfile::TempDir::new()` in the test module uses `use tempfile::TempDir;` instead. Trim cross-referencing comments that named sibling tests or caller flows. Fold `test_devnet_llmq_type_from_name_rejects_unknown_names` into `test_devnet_llmq_type_from_name`. Swap a `;` clause separator for `.` in a `DevnetConfig` doc comment.
* feat(key-wallet): add network-scoped wallet-id derivation The existing wallet id (`compute_wallet_id_from_root_extended_pub_key`) hashes only `root_public_key || root_chain_code`, which is identical across networks for a given seed — the same mnemonic yields the same id on mainnet/testnet/devnet/regtest. Some consumers need the option of a network-scoped id so one mnemonic maps to distinct ids per network. Add an opt-in, purely additive API: - `compute_network_scoped_wallet_id_from_root_extended_pub_key(root, Option<Network>)` hashes `pubkey || chain_code || DOMAIN_TAG || network_byte`. - `compute_network_scoped_wallet_id(&self)` instance method mirroring `compute_wallet_id`, folding in `self.network`. The network byte uses an explicit, wire-stable mapping (None=0xFF, Mainnet=0x00, Testnet=0x01, Devnet=0x02, Regtest=0x03) rather than `Network as u8`, since the enum repr can drift. A domain-separation tag (`DASH_WALLET_ID_NET_V1`) is appended before the network byte so a scoped id can never collide with the legacy unscoped digest. Network is optional. The legacy `compute_wallet_id_from_root_extended_pub_key` is unchanged byte-for-byte, and construction still stamps the legacy id into `self.wallet_id`. Flipping the default and any downstream re-keying are intentionally out of scope. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(key-wallet): cover scoped-id keyless-wallet branch Add a test asserting compute_network_scoped_wallet_id returns the construction-time id verbatim for WatchOnly and ExternalSignable wallets (no root key on hand), matching compute_wallet_id. This was the one documented behavior branch the existing tests did not exercise. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(key-wallet): avoid rustdoc private-intra-doc-link errors The public compute_network_scoped_wallet_id_from_root_extended_pub_key doc linked to two private items (NETWORK_SCOPED_WALLET_ID_DOMAIN and network_scoped_wallet_id_discriminant), which fails `cargo doc` under RUSTDOCFLAGS=-D warnings (rustdoc::private_intra_doc_links). Reference them as plain inline code instead of intra-doc links. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(key-wallet): make None network reproduce the legacy wallet id Per review feedback, the network-scoped derivation is now backwards compatible: passing `network = None` produces a digest byte-for-byte identical to the legacy unscoped id (`compute_wallet_id_from_root_extended_pub_key`). The domain tag + discriminant byte are only appended for a concrete `Some(network)`, so a scoped id still cannot collide with the legacy/`None` digest. - Drop the `None => 0xFF` sentinel; the discriminant helper now takes `Network` directly (still an exhaustive match enforcing a byte per variant). - Update docs to spell out the `None` == legacy contract. - Tests: `None` now asserts equality with the legacy id (test_no_network_matches_legacy_id); `Some(network)` still asserts inequality. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * refactor(key-wallet): shorten wallet-id domain tag to b"N" Replace the verbose b"DASH_WALLET_ID_NET_V1" domain-separation tag with b"N" and condense the constant's doc. The tag's only job is to make a Some(network) scoped preimage diverge from the legacy/None preimage; any non-empty suffix achieves that, since the legacy preimage is a strict prefix of the scoped one. No persisted scoped ids exist yet and no test hardcodes a scoped digest, so this is safe. The legacy/None digest is unaffected (the tag is never appended there). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * refactor(key-wallet): fold network scoping into the existing wallet-id fn Now that `None` reproduces the legacy digest byte-for-byte, a separate `compute_network_scoped_wallet_id_from_root_extended_pub_key` was pure redundancy: calling it with `None` was identical to the legacy `compute_wallet_id_from_root_extended_pub_key`. Merge them. - `compute_wallet_id_from_root_extended_pub_key` now takes `network: Option<Network>`. `None` = legacy unscoped id (unchanged bytes); `Some(net)` = network-scoped id (domain tag + discriminant). - Delete the redundant scoped free function. The instance accessors stay: `compute_wallet_id()` calls it with `None`, `compute_network_scoped_wallet_id()` with `Some(self.network)`. - Update the 3 in-crate call sites (initialization.rs) and tests to pass `None`. Drop the now-tautological no-network-vs-legacy test; the known-answer test already locks `fn(.., None)` to the legacy digest. API note: this changes the signature of the public free function (callers add `None`). The crate is 0.x; the digest output for existing `None` callers is unchanged, so no persisted ids shift. Only in-crate call sites exist; `key-wallet-manager` uses the no-arg instance method and is unaffected. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(key-wallet)!: make wallet id network-scoped by default Per design decision, abandon backwards compatibility and flip the default: a wallet's id is now network-scoped everywhere. - Construction (`from_wallet_type` → from_mnemonic/from_seed/new_random, plus `from_xpub`/`from_external_signable`) now stamps `compute_wallet_id_from_root_extended_pub_key(root, Some(network))`. - `compute_wallet_id(&self)` now folds in `self.network` (was unscoped), keeping it consistent with the stamped `self.wallet_id`. - Remove the now-redundant `compute_network_scoped_wallet_id(&self)` instance method — it would be identical to `compute_wallet_id`. The free function keeps its `Option<Network>` param: `Some(network)` is the scoped id (default), `None` still yields the network-independent digest (`pubkey || chain_code`) for callers that explicitly want one id shared across networks. BREAKING: every wallet id derived from a given seed now differs per network. Previously persisted ids (which used the unscoped derivation) will not match and must be re-keyed by consumers. The network-independent form remains available via `…_from_root_extended_pub_key(root, None)`. Tests updated: two pre-existing tests that asserted cross-network id equality now assert distinctness (matching their intent), and the network-scoped unit tests cover the new default plus the `None` known answer. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(key-wallet): pin a network-scoped wallet-id known answer The known-answer test only locked the `None` (network-independent) digest. Since the network-scoped id is now the default and the b"N" domain tag + discriminant bytes are a wire-stable contract, add a hard-coded expected hex for the Some(Network::Mainnet) digest so any change to the tag or a discriminant byte fails the test instead of silently shifting persisted ids. Merges both assertions into test_wallet_id_known_answers. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…pay#790) * feat(dash-spv): storage truncation primitives for reorg support Add `truncate_above(height)` to `BlockHeaderStorage`, `FilterHeaderStorage`, `FilterStorage`, and `BlockStorage`, backed by a new `SegmentCache::truncate_above` that drops segments above the target and resets the boundary segment's tail slots to the persistable sentinel. Dropped segment files are queued for deletion on the next `persist`, and `PersistentBlockHeaderStorage` also prunes its `header_hash_index` so orphaned hashes no longer resolve to a height. This is the foundational primitive for upcoming fork/reorg handling in storage. No callers are wired up yet. * fix(dash-spv): reject `get_items` ranges past tip in `SegmentCache` After a within-segment `truncate_above`, the boundary segment is not in `to_delete`, so the loop guard in `get_items` does not catch overruns into its sentinel tail. In debug builds this panicked on the `last_valid_offset` assertion, in release builds it returned sentinel-filled data. Add an entry guard that fails fast when the requested end exceeds `next_height()`, plus a within-segment regression test alongside the existing cross-boundary one. Addresses CodeRabbit review comment on PR dashpay#790 dashpay#790 (comment) * test(dash-spv): consolidate truncate_above storage tests Merge two noop edge cases in segments.rs into a single test. Fold the block_headers noop assertion into the index-prune test. Collapse the blocks, filter_headers, and filters wrappers to one smoke test each since the underlying SegmentCache::truncate_above engine is already exhaustively covered in segments.rs.
`QuorumSnapshot::consensus_encode` wrote the active-members bitset without the leading compact-size bit count that `consensus_decode` reads (and that Dash Core's `DYNBITSET(activeQuorumMembers)` serializes), so encode and decode were asymmetric and any re-serialized `QuorumSnapshot` desynced on decode. This never surfaced because the only consensus-encode path for the type is serializing an outgoing `QRInfo` response (`message.rs`), which an SPV client never produces, and the only existing test was decode-only. Adds a round-trip test.
* fix: decode SML ProTx v3 entries
Dash Core 23.1 introduced ProTx version 3 (`ProTxVersion::ExtAddr`). `CSimplifiedMNListEntry` gates two things on `nVersion >= 3`: the service address switches from the legacy fixed 18-byte `MnNetInfo` to a variable-length `ExtNetInfo`, and Evo nodes no longer serialize the inline `platformHTTPPort` (only `platformNodeID` is inline, the HTTP port lives in `ExtNetInfo` under `PLATFORM_HTTPS`). The previous decoder always read a fixed 18-byte address, desyncing the stream on v3 entries and failing with `Invalid MasternodeType variant`.
Add a faithful `net_info` module with `Bip155Network`, `NetInfoEntry`, `NetInfoPurpose` and `ExtNetInfo` types decoding the ADDRV2/BIP155 wire form (per-network address-length validation, big-endian ports, version short-circuit). Introduce `MasternodeNetInfo::{Legacy, Extended}` for `MasternodeListEntry::service_address` with a version-driven decode/encode branch and a `primary_service_address()` accessor that reconstructs IPv4/IPv6 `SocketAddr`s. Inline the `mn_type` codec into the entry path so v3 Evo entries read only `platform_node_id` and source `platform_http_port` from the `PLATFORM_HTTPS` entry, while v2 Evo entries keep the inline port.
`MasternodeNetInfo` carries a hand-written `bincode` codec whose `Legacy` arm reuses the bare `SocketAddr` layout so existing persisted engine snapshots keep decoding.
Also fix `QuorumSnapshot::consensus_encode` to write the compact-size `activeQuorumMembers` count that the decoder already consumed, which the byte-exact `qrinfo` round-trip requires.
Consumers (`peer_addresses`, `masternode_helpers`, `masternode-seeds-fetcher`) now go through `primary_service_address()`.
* fix: bound `ExtNetInfo` decode pre-allocation against malicious counts
`consensus_decode_ext` sized `purposes`/`entries` with attacker-controlled compact-size counts straight off the wire, letting a tiny `qrinfo` trigger an unbounded `Vec::with_capacity`. Cap both like the existing `Vec` decoder using `MAX_VEC_SIZE`.
* fix(key-wallet): correct CoinJoin derivation path and denominations CoinJoin addresses were derived at `m/9'/coin'/account'/index` instead of Dash Core's `m/9'/coin'/4'/account'/0/index`, so the SPV client monitored the wrong scriptPubKeys and matched none of the wallet's CoinJoin outputs against BIP158 filters. Insert the hardened `4'` feature level and derive the pool on the `/0` branch (via the external pool) to match Dash Core. The CoinJoin denomination constants in `has_denomination_outputs` omitted the per-round fee (`100_000_000` etc. instead of `100_001_000` etc.), so real mixing rounds were classified `Standard`. Use the protocol denominations from `coinjoin/common.h`. * fix(key-wallet): check all fund accounts for tx ownership, demote CoinJoin classification to a label `get_relevant_account_types` routed `TransactionType::Standard` and `TransactionType::CoinJoin` to disjoint account sets, so a transaction's shape gated which fund-bearing accounts were checked for ownership. A misclassified tx (for example a small denomination spend that the `>= 3 in/out` heuristic labels `Standard`, or a CoinJoin-shaped tx touching standard collateral and change) therefore skipped the account that actually owned its coins and was dropped. Match Dash Core, where ownership is membership-based and never depends on a transaction's type or shape: `AddToWalletIfInvolvingMe` records a tx iff `IsMine || IsFromMe`, and `IsMine` tests each scriptPubKey against all script-pubkey managers uniformly (regular external, internal, and the CoinJoin descriptor). Denomination/CoinJoin classification is purely a downstream annotation, never a precondition for discovery. Both the `Standard` and `CoinJoin` arms now return the full set of fund-bearing account types via a shared `fund_bearing_account_types` helper (StandardBIP44, StandardBIP32, CoinJoin, DashpayReceivingFunds, DashpayExternalAccount). The `is_coinjoin_transaction` heuristic is unchanged: it now only sets the stored `TransactionType` label, never which accounts are consulted. Checking extra accounts introduces no false positives because an account matches only when a scriptPubKey or spent UTXO genuinely belongs to it.
dashpay#806) * feat(key-wallet): add normalize_phrase and is_word_in_language helpers Add two BIP39 input helpers to Mnemonic so consumers don't re-implement wordlist/normalization logic on top of the bip39 crate: - normalize_phrase: NFKD + lowercase + whitespace-collapse (lenient input normalization for validation/membership; NOT BIP39 seed normalization, which to_seed already performs) - is_word_in_language: exact wordlist membership for a given language Re-export Language from the crate root + prelude, and add the unicode-normalization dependency. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(key-wallet): validate real phrase for each bundled language Mnemonic::validate was exercised only for English (test_mnemonic_validation) and Portuguese (test_portuguese_mnemonic); the other bundled languages were built via from_entropy in test_multiple_languages but never validate()'d. Add a test that builds a real 12-word phrase per language (French, Spanish, Italian, Japanese, Korean, Czech, ChineseSimplified, ChineseTraditional) and asserts validate() succeeds, plus a cross-language negative. This brings the per-language validate coverage home to key-wallet (it previously lived only in the dashwallet platform FFI facade). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(key-wallet): add word_list + cleanup_phrase recover-flow primitives Push BIP-39 recover-flow primitives down into key-wallet so the platform FFI layer can stay a thin C marshalling surface (per reviewer feedback on platform #3842): - Language::word_list(): public accessor for the raw 2048-word BIP-39 wordlist — the low-level primitive callers compose for word validation. - Mnemonic::cleanup_phrase(): minimal recover-input cleanup + CJK ideographic auto-split (DashSync DSBIP39Mnemonic cleanupPhrase: parity), built on module-private all-language wordlist/validate unions. Also drop the interim Mnemonic::is_word_in_language() helper (added earlier in this same unmerged branch) — too granular; callers use word_list() and do membership themselves. Net diff vs master is purely additive. cleanup_phrase / word_list tests live here since they exercise key-wallet logic. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(key-wallet): canonicalize all whitespace in cleanup_phrase + correct word_list doc Address CodeRabbit review on dashpay#806: - cleanup_phrase: canonicalize every Unicode whitespace to an ASCII space in step (2), not just '\n'. Step (1) keeps all whitespace and the valid-phrase early return hands the string back verbatim, so a pasted CRLF/tab-delimited recovery phrase previously came back with '\r' / '\t' still in it. Seed-equivalent (BIP-39 NFKD maps U+3000 et al. to U+0020). Adds a regression test (cleanup_canonicalizes_crlf_and_tabs). - word_list doc: drop the "accent tolerance" claim — normalize_phrase does NFKD + lowercase + whitespace folding but preserves diacritics as combining marks, so "cafe" still won't match "café". Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: PastaClaw <thepastaclaw@users.noreply.github.com>
…th AddressPrefix (dashpay#802)
…ield/accessor renames) (dashpay#808) * fix: read Core 23 nested platform addresses in rpc-json DMNState Dash Core 23.1 moved the platform P2P/HTTPS ports out of the deprecated top-level platformP2PPort/platformHTTPPort keys into a nested `addresses` object whose `platform_p2p`/`platform_https` sub-keys hold "host:port" arrays. Without reading them, platform_p2p_port deserialized to None on Core-23 entries, dropping or mis-porting validators downstream. Add MasternodeAddresses and backfill the existing port fields from the addresses object when the legacy port is absent or zero, via a private DMNStateIntermediate (mirroring DMNStateDiffIntermediate) and inside the existing TryFrom for DMNStateDiff. Legacy pre-23 JSON is untouched. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(rpc-json): resolve platform ports via accessors; drop in-deserialize backfill Match the plain-derive style of the rest of the crate: deserialize DMNState and DMNStateDiff with no logic in the deserialize path. The nested Core 23 `addresses` object now lands as a plain public field instead of being folded into the legacy port fields during deserialization. Port resolution moves into accessor methods on both DMNState and DMNStateDiff: resolved_platform_p2p_port / resolved_platform_http_port. They check the new `addresses` object first (first non-zero, parseable port) and fall back to the deprecated top-level platformP2PPort/platformHTTPPort — new-first, legacy-fallback. Removed: the DMNStateIntermediate mirror struct and its `#[serde(from)]`, the `backfill_port` helper, and the in-deserialize backfill blocks in both From/TryFrom impls. DMNState regains its plain `#[derive(Deserialize)]`; DMNStateDiff keeps its pre-existing `try_from` with `addresses` flowing through as raw data. apply_diff now propagates `addresses` so a stored DMNState stays resolvable after diffs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(rpc-json): platform address accessors return (host, port), clearable diff Address self-review on the Core 23 platform-address work. Rename the raw legacy fields on DMNState and DMNStateDiff to legacy_platform_p2p_port / legacy_platform_http_port (serde wire renames intact) so callers reach for the accessors instead of the deprecated fields. The resolver accessors become platform_p2p_address / platform_http_address and return Option<(String, u32)> instead of a bare port. On DMNState the legacy fallback pairs the port with the node IP from `service`, since Dash deploys platform services on the masternode's core IP; on DMNStateDiff there is no node IP to pair with, so the legacy path is not resolved. Zero ports never surface: they are dropped in both the addresses path and the legacy fallback. Ports parse through u16 then widen to u32, rejecting out-of-range values like "host:70000". DMNStateDiff::addresses becomes Option<Option<MasternodeAddresses>>, mirroring pose_ban_height: None = unchanged, Some(None) = cleared, Some(Some(_)) = set. A custom serde helper distinguishes absent / null / object on the wire. compare_to_newer_dmn_state now encodes a Some->None clear as Some(None) and apply_diff applies the inner value, so the clear survives the round-trip. Tests cover diff addresses propagation through apply_diff, the Some->None clear round-trip, zero-port-to-None, and out-of-range rejection. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore: add deprecated annotation to legacy fields * fix(rpc-json): tighten legacy platform-port resolution and deprecate DMNState fields Address a fresh review pass on the Core 23 platform-address work. - Mirror the `#[deprecated]` annotation onto DMNState::legacy_platform_p2p_port / legacy_platform_http_port so both DMNState and DMNStateDiff signal the same deprecation; add `#[allow(deprecated)]` to the two accessor methods, the apply_diff write sites, and the tests that read the fields to keep `clippy -D warnings` green. - legacy_platform_address now validates through `u16::try_from`, so an out-of-range legacy port (e.g. platformP2PPort: 70000) yields None instead of Some((ip, 70000)), matching the addresses path and the accessor's documented in-range contract. - parse_host_port rejects unbracketed multi-colon (bare IPv6) hosts rather than silently mangling them via rsplit_once; bracketed IPv6 and IPv4 still parse. The doc now states the supported forms accurately. Tests: wire-level `addresses: null` -> Some(None) (and absent -> None) round trip, out-of-range legacy port rejection, and parse_host_port IPv6 handling. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(rpc-json): reject empty host in parse_host_port; deprecate intermediate legacy ports Addresses CodeRabbit review on PR dashpay#808: - parse_host_port now rejects empty hosts (":36656" -> None) - DMNStateDiffIntermediate legacy_platform_*_port mirror the #[deprecated] of DMNState/DMNStateDiff Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…pay#813) AccountType::derivation_path() for DashpayReceivingFunds / DashpayExternalAccount pushed a hardcoded `0'` for the account segment and discarded the variant's own `index` field (destructured with `..`). The DashPay friendship path is m/9'/coin'/15'/account'/<id>/<id> (DIP-15 "Account Reference" + DIP-14), where the account segment is the sender's DashPay account — see DIP-15's multi-account example (personal = account 0, professional = account 1). dashj's FriendKeyChain already parameterizes this segment; we were the outlier. Effect: a multi-account wallet derived every relationship under account 0, so a non-zero account watched the wrong addresses and would miss funds. Now the segment uses `*index`. Account 0 — the only value in use today — is byte-identical to the old path, so this is backward-compatible; it only unlocks non-zero accounts. Test would have caught this in CI: test_dashpay_account_index_in_derivation_path asserts the account segment equals the index for accounts {0,1,7,42} on both the receiving and external channels (plus id ordering). ✖ before fix (1/7/42 derived 0'), ✔ after. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…dashpay#816) When a wallet boots already fully synced, `start_download` takes its early return before assigning `processing_height`, leaving it at the initial 0. The next block that extends the chain then drives `try_create_lookahead_batches`, which falls back to `processing_height` when no batches are active and creates a lookahead batch starting at height 0. Loading filters from height 0, which were never stored under checkpoint sync, reads an empty leading segment and panics in `SegmentCache::get_items` (debug) or returns sentinel filters as real data (release). Set `processing_height` to the computed `scan_start` before the early-return check so the scan frontier is recorded on every path. The genesis case (`committed_height == 0`) keeps starting at the wallet birth height because `scan_start` already encodes that distinction.
The Signer trait exposes sign_ecdsa + public_key, the latter returning only the compressed leaf point. A caller that needs a BIP-32 extended public key at a hardened path — e.g. DashPay contact-xpub derivation under m/9'/coin'/15'/account'/sender/recipient — could not obtain the chain code without the wallet's resident root private key, forcing the seed to be made resident. Add a required extended_public_key(path) -> ExtendedPubKey method so a Keychain/hardware-backed signer can return the point + chain code + parent fingerprint at a (possibly hardened) path, letting the caller non-hardened-derive descendants offline with no resident seed. A signer that cannot export an xpub at a hardened path returns an error rather than panicking. The two in-repo InMemorySigner test signers derive it from their root xprv (ExtendedPubKey::from_priv); the NoDigestSigner test stubs are unreachable. Pinned by a test asserting the signer's xpub at a hardened path equals the wallet's own derive_extended_public_key. Required (not defaulted): the trait's associated Error type is opaque, so a default body cannot construct a Self::Error to return; there are no production Signer impls in this crate. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: QuantumExplorer <11468583+QuantumExplorer@users.noreply.github.com>
…ashpay#801) * fix: resolve just-retired quorum by walking earlier masternode lists `get_quorum_at_height` and the FFI `ffi_dash_spv_get_quorum_public_key` resolved a quorum only through the single nearest masternode list at or below the lookup height. A Platform signing quorum is selected at a lagged height, so by the proof's `core_chain_locked_height` it can already have retired out of the active set. `apply_diff` drops a retired quorum from every later list, so the nearest list misses and the lookup fails with `Quorum not found`, surfacing downstream as `InvalidQuorum`. The failure is intermittent, firing only at the retirement edge. The full entry is not actually gone: `apply_diff` clones the base list without mutating earlier ones, and old per-height lists are retained, so the quorum still lives in every list from its mint height up to retirement. Add `MasternodeListEngine::quorum_entry_for_hash_at_or_before_height`, which walks `masternode_lists` backward from the lookup height and returns the first list still holding the quorum, skipping `Invalid` entries. Both consumer sites route through it, so the real full `QualifiedQuorumEntry` is returned with no synthesized data, no new storage, and no API change. The walk is floored at a few active windows below the height (derived from the type's `signing_active_quorum_count` and DKG interval): a legitimately referenced signing quorum cannot be older than that, so a miss scans a bounded span rather than every accumulated list. Closes dashpay#800. * fix: emit `tracing::warn!` message as format string in `dash-spv` quorum lookup Addresses CodeRabbit review comment on PR dashpay#801 dashpay#801 (comment) * refactor: drop redundant `QuorumHash` import in `helpers` test module Addresses CodeRabbit review comment on PR dashpay#801 dashpay#801 (comment) * docs: avoid private intra-doc link in public quorum walk-back docs The public `quorum_entry_for_hash_at_or_before_height` doc linked to the private `QUORUM_WALK_BACK_ACTIVE_WINDOWS` constant, which rustdoc rejects under `-D rustdoc::private-intra-doc-links`. Demote the link to a plain code span so the Documentation CI job passes.
…ts (dashpay#820) * fix(dash-spv): reprocess blocks on rescan against newly derived scripts The commit-time batch rescan re-matches scripts derived during block processing against the active batches, but `BlockMatchTracker::track` subtracted wallets that already had a block applied, so a block processed before those scripts existed was silently skipped. Scripts derived from processing a block could therefore never trigger a re-examination of any earlier block in the same batch, and partially recognized blocks stayed partial. Add `track_for_new_scripts`, which keeps the in-flight accounting of `track` but ignores processed records, and use it in `rescan_batch`: every match there is by construction against scripts that did not exist at first processing. Re-application is safe because wallet-side block processing is idempotent (transaction dedup, outpoint-keyed UTXO insertion, balance recomputed from the UTXO set). Termination is preserved since the rescan only queries newly collected scripts, and re-processing a block whose outputs are already recognized derives nothing further. The plain `scan_batch` path keeps the processed-record skip, late-added wallets still get their residual pass through plain `track`. * test(dash-spv): cover a burst payment beyond the gap window on cold sync One transaction pays a run of consecutive fresh addresses reaching past the gap window, mined before the client ever starts. The historical scan must climb the burst via fixpoint re-scanning and credit every output. Adds `DashCoreNode::send_many` so a regtest wallet can pay many addresses in a single transaction.
…ashpay#798) `MasternodeListEntry::calculate_entry_hash` hashed the full network consensus serialization, including the leading `version`. Dash Core's `CSimplifiedMNListEntry::CalcHash` uses `CHashWriter(SER_GETHASH, ...)`, and `SER_GETHASH` does not set `SER_NETWORK`, so the `SER_NETWORK`-gated leading `nVersion` is omitted from the hash pre-image. The two contexts therefore diverged for every entry. Extract the post-`version` body of the wire encoder into a shared `consensus_encode_body` helper. `consensus_encode` writes `version` then the body (byte-identical to before), while `calculate_entry_hash` hashes the body alone, matching Core's `CalcHash`. Verified against Core for both a `version` 1 and a `version` 2 Evo entry. Also fix `SocketAddr` decoding to use `to_ipv4_mapped` instead of `to_ipv4`, so an all-zero (`::`) service address round-trips to the same 16 bytes instead of acquiring an `::ffff:` mapped prefix. The prefix corruption changed the wire bytes and thus the entry hash for masternodes with an unset service.
…e all utxo available (dashpay#823)
* docs: add `0.43.0` changelog entry * docs: add `0.44.0` changelog entry
* feat(key-wallet): impl Zeroize for ExtendedPrivKey ExtendedPrivKey held its secp256k1::SecretKey (and ChainCode) with no Zeroize impl, so downstream code could not wrap it in zeroize::Zeroizing and had to scrub the scalar manually with SecretKey::non_secure_erase() on every path (error-prone; the scalar leaks un-wiped on `?`-early-return and panic-unwind paths). Add a hand-written `impl Zeroize for ExtendedPrivKey` mirroring the existing `RootExtendedPrivKey` and `ChainCode` impls. It is hand-written rather than `#[derive(Zeroize)]` because secp256k1::SecretKey exposes `non_secure_erase()` instead of a `Zeroize` impl, so a derive does not compile. The type stays `Copy` (no `Drop`), so this is additive and non-breaking; callers opt in via `zeroize::Zeroizing<ExtendedPrivKey>`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MoY6vhmqZuHzNsMfJ8wakQ <sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub> * feat(key-wallet): zeroize all ExtendedPrivKey fields; add Default to Network Expand `impl Zeroize for ExtendedPrivKey` to wipe the whole value rather than only the secret material. In addition to `private_key` (non_secure_erase) and `chain_code`, it now clears `depth` and `parent_fingerprint`, resets `child_number` to `Normal { index: 0 }`, and resets `network` to its default — matching how `RootExtendedPrivKey` zeroes all of its own fields. `dashcore::Network` had no `Default`, so add `#[derive(Default)]` to it with `Mainnet` (the `#[repr(u8)]` 0 discriminant) as the default. Additive and non-breaking. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MoY6vhmqZuHzNsMfJ8wakQ <sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub> * refactor(key-wallet): reset network via Network::Mainnet in zeroize; drop Network Default Revert the `#[derive(Default)]` added to `dashcore::Network` and instead reset `network` to `Network::Mainnet` directly in `ExtendedPrivKey::zeroize` — Mainnet is the `#[repr(u8)]` 0 discriminant, i.e. the natural "zero" value. Keeps the change confined to key-wallet with no cross-crate Default impl. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MoY6vhmqZuHzNsMfJ8wakQ <sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub> * fix(key-wallet)!: drop Copy from ExtendedPrivKey, zeroize on Drop Removing Copy lets ExtendedPrivKey implement Drop, so its secret material is wiped automatically on scope exit instead of relying on callers to remember to zeroize. Mirrors the RootExtendedPrivKey convention. BREAKING CHANGE: ExtendedPrivKey no longer implements Copy; downstream code relying on implicit copies must clone or borrow. Fallout fixed: - bip32::derive_priv clones self instead of *self. - taproot-psbt example takes &ExtendedPrivKey (borrow, no clone). - key-wallet tests reorder/capture-or-clone where the value is reused after move. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…ashpay#828) * refactor(key-wallet): ManagedAccountType::from_account_type refactor * refactor(key-wallet): ManagedAccountType::from_account_type refactor * refactor(key-wallet): ManagedAccountType::from_account_type refactor
…pay#812) * fix(key-wallet): reserve selected UTXOs to prevent concurrent double-spend A freshly built transaction is not reflected in the UTXO set until its broadcast is processed back into the wallet, so a second build in that window can select the same coins and produce a double-spend the network rejects. - Add an ephemeral `ReservationSet` on `ManagedCoreFundsAccount`: `set_funding` skips reserved outpoints during coin selection and `assemble_unsigned` reserves the inputs it selects. - Release the reservation when the spend is processed, and on a failed sign, so coin selection is unaffected by a build that never reaches the network. - Reclaim stale reservations via a roughly one-hour (24-block) TTL backstop, skipping the sweep at height 0 where elapsed time is unknown. - Share the set via `Arc<Mutex<_>>` so a reservation outlives the wallet lock, and never persist it: after a restart, chain and mempool sync are the source of truth. - Cover the read, write, release, TTL, and height-0 paths with unit tests, plus a `dashd` integration test asserting two pre-broadcast builds select disjoint inputs. * docs: trim and clarify `ReservationSet` doc comments Drop the data-shape restatement from the `ReservationSet` struct doc and lead the `lock` method doc with its rationale rather than the method name. Make the height-0 sweep comment explicit that a height-0 entry relies on a later non-zero height to ever be reclaimed. * test: cover reservation release on the confirmed spend path `processing_a_spend_releases_its_reservation` only exercised the `Mempool` context. Extend it so a second funded outpoint is reserved and then spent under `TransactionContext::InBlock`, asserting the reservation is released on the confirmed path as well. * fix(key-wallet): release reservations when fee encoding fails `build_unsigned` reserved the selected inputs in `assemble_unsigned` and then encoded the transaction with `unwrap`, leaking the reservation until the TTL backstop if the encode ever failed. Route both builders through a fallible `encoded_size` helper that returns a `BuilderError` instead of panicking, and release the reserved inputs on that failure path so `build_unsigned` matches the existing release behavior in `build_signed`. * docs: note the builder must not be held across an await in `set_funding` Strengthen the `set_funding` lock invariant to state that the builder must not be held across an `await` before `build_signed` or `assemble_unsigned`, since suspending there reopens the read-then-reserve window for a concurrent build. * feat(key-wallet): release UTXO reservations for abandoned builds Add `ManagedCoreFundsAccount::release_reservation` so a caller can release the reservations held for a built transaction's inputs when that transaction will not be broadcast (e.g. the user cancelled). Without this the inputs stay reserved until the TTL backstop reclaims them, making the funds appear unspendable for about an hour. Broadcast transactions still release on their own once the spend is processed back into the wallet, so this is only for abandoned builds.
…endedPubKeySigner subtrait (dashpay#839) * refactor(key-wallet): move extended_public_key out of Signer into ExtendedPubKeySigner Signer::extended_public_key (added in dashpay#817) forced every signer to implement xpub export, but exporting a chain code is a key-export capability, not a signing one: pure signing backends (HSMs, remote signing services whose policy forbids chain-code export) could not implement Signer without stubbing the method with an error. Move the method unchanged into a new ExtendedPubKeySigner: Signer subtrait. Callers that need offline descendant derivation (e.g. DashPay contact xpubs) bound on the subtrait; everything else keeps the minimal Signer. Reusing the Signer supertrait keeps a single associated Error type. No production code called the method, so only the two InMemorySigner test signers move their impl to the new trait, and the unreachable NoDigestSigner stubs are deleted outright — evidence the requirement never belonged on the base trait. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(key-wallet): pin asset-lock InMemorySigner xpub export against wallet derivation The asset_lock_builder test module duplicates the InMemorySigner ExtendedPubKeySigner impl but had no correctness check, unlike its twin in transaction_building.rs. Mirror the hardened-path assertion so both copies stay verified against Wallet::derive_extended_public_key. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…ashpay#838) * ci: fix `ready-for-review` label automation for fork pull requests Fork pull requests run the `pull_request` and `pull_request_review` events with a read-only `GITHUB_TOKEN` and no access to secrets, so the `evaluate` job could not mutate labels and failed with "Resource not accessible by integration" when it tried to add `ready-for-review` after CodeRabbit approved. The privileged `workflow_run` path that should have covered this was a silent no-op for forks because `github.event.workflow_run.pull_requests` is always empty for cross-repository PRs. Restructure the automation around the `workflow_run` relay pattern. A new lightweight `Ready for Review Trigger` workflow runs on `pull_request` and `pull_request_review`, does nothing but complete, and thereby fires a `workflow_run` event. The `Ready for Review Label` workflow now runs only on `workflow_run`, where it holds a write-capable token even for forks, resolves the PR by matching the head sha against the open pull requests (the commit-to-PR endpoint does not index fork heads), and applies or removes the label. No new secrets are required. Fold the dismissal, changes-requested and merge-conflict handling into the single re-evaluation path and exclude the relay job from the CI gate. * ci: grant `contents: read` to ready-for-review label workflow Setting a workflow-level `permissions` block drops the default `contents: read`, which both `actions/checkout@v4` steps need. Restore it alongside the existing scopes. Addresses CodeRabbit review comment on PR dashpay#838 dashpay#838 (comment) * ci: clear `ready-for-review` label when a PR is converted to draft The trigger workflow did not relay `converted_to_draft`, so converting a labeled PR back to draft fired no `workflow_run` and the label survived. Relay that event and make the draft branch remove the label instead of skipping, matching the defensive `|| true` used by the sibling merge-conflict and CodeRabbit branches so a `gh` hiccup on one PR does not abort a multi-PR batch. Addresses CodeRabbit review comment on PR dashpay#838 dashpay#838 (comment)
…ported (dashpay#841) `LockFile::new()` treated any `TryLockError::Error` from `File::try_lock()` as fatal. On Android (`target_os = "android"`) the Rust standard library does not implement `flock`-based advisory locking and returns `ErrorKind::Unsupported` at compile time, so SPV storage init always failed with `Write failed: Failed to acquire lock: try_lock() not supported`, blocking the entire Core/SPV layer on Android. The lock file is only a best-effort guard against multiple processes sharing one data directory. On single-process platforms that lack advisory locking, proceeding without it is safe. Treat `ErrorKind::Unsupported` as non-fatal: skip the lock with a warning and continue. Genuine I/O errors and the already-locked (`WouldBlock`) case remain fatal, and the fast path on desktop/iOS is unchanged. Extract the classification into a `classify_lock_result` helper so the degradation path can be unit-tested with a synthesized `Unsupported` error (which never occurs naturally on Linux/macOS CI). Fixes dashpay#840 Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
… after a restart (dashpay#835) Fixes dashpay#824. A fully synced client that restarted, or reconnected after losing all peers, came back with its filter download state uninitialized. The next new block then re-requested every filter from height 1 and wedged the in-order store loop, so `committed_height` froze one block below the tip and the client never reported synced again. `start_download` now anchors `next_batch_to_store` and `processing_height` and parks the pipeline at the download frontier before its already-synced early return. The already-synced reconnect branch of `start_sync` delegates to `start_download` instead of duplicating that logic, which also downloads the boundary filter body when the restored filter header tip is ahead of the stored bodies. `handle_new_filter_headers` now gives a new boundary block a processing batch even while a historical rescan is still draining `active_batches`. Covered by unit regression tests for both restart paths, a reconnect test with filter headers ahead of stored filter bodies, and a dashd integration test that mines a block right after a fully synced restart. Co-authored-by: lklimek <842586+lklimek@users.noreply.github.com>
…y#836) * fix(key-wallet): restrict asset-lock funding to final inputs Asset-lock builders selected funding UTXOs filtered only by `is_spendable`, which deliberately admits unconfirmed mempool outputs. A non-final input is not InstantSend-eligible per DIP-0010, so the resulting funding transaction could never receive the InstantSend lock or ChainLock Platform requires, and identity registration or top-up died on a 300s `AssetLockFinalityTimeout`. Add an opt-in `TransactionBuilder::require_final_inputs` that restricts coin selection to `is_confirmed || is_instantlocked` UTXOs and enable it in `build_asset_lock` and `build_asset_lock_with_signer`. Ordinary spends keep admitting unconfirmed inputs, now pinned by a regression test. Part of dashpay#832 (U2). * fix(key-wallet): require final parents for trusted self-send change `Utxo::is_trusted` was set from a flat `has_owned_input && change-addr` check, so change of a self-send spending a still-unconfirmed parent was credited to the confirmed balance bucket. `Utxo::is_trusted` documents Bitcoin Core `CWalletTx::IsTrusted` parity, which is recursive: a zero-conf output is trusted only if every parent is itself trusted. Set `is_trusted` only when every input spends one of our own UTXOs that is confirmed, InstantSend-locked, or itself trusted. The stored parent flag carries the recursion transitively, and the spent parents are still present in the UTXO map at that point because insertion runs before spent-outpoint removal. Unknown or non-final parents deny trust, so funds the network may still drop never surface as confirmed. Part of dashpay#832 (U1). --------- Co-authored-by: lklimek <842586+lklimek@users.noreply.github.com>
…#844) * ci: stop `evaluate` failing when a PR's CI is pending or red `check_ci_status.sh` runs `gh pr checks` under `set -euo pipefail`. That command exits `8` when any check is still pending and non-zero when any check fails, even with `--json`, so the script aborted before echoing its status whenever the PR's own CI was not fully green. The caller assigns its output with `CI_STATUS=$(check_ci_status.sh ...)`, also under `set -e`, so the failed command substitution took the whole `evaluate` job down with "exit code 1". This surfaced constantly after the `workflow_run` relay landed, because `evaluate` now re-runs on every review and label event, routinely while CI is still pending or red, which is exactly when `gh pr checks` returns non-zero. Capture the JSON with `|| true` and treat empty output as `no_checks`, so the `jq` rollup classifies `pending`/`has_failures`/`all_passed` instead of aborting the caller. * ci: surface unexpected `gh pr checks` failures instead of hiding them The previous `2>/dev/null || true` collapsed every `gh pr checks` failure into empty output, which `check_ci_status.sh` then reported as `no_checks`, stripping the `ready-for-review` label and turning the job green on a broken state with the error hidden. `gh pr checks --json` exits 0 when checks passed or failed (the failure exit code is suppressed by `--json`) and 8 when they are pending. Any other exit code means gh itself failed (auth, rate limit, network, unknown PR). Tolerate only the expected 0 and 8, propagate anything else with stderr intact, and treat empty output as `no_checks` only when gh actually succeeded. Addresses CodeRabbit review comment on PR dashpay#844 dashpay#844 (comment)
`update_utxos` rebuilds each output with a fresh `Utxo::new` on every reprocess, so a mempool→block confirmation silently reset `is_instantlocked` (and `is_trusted` and any coin reservation) back to false. An InstantSend lock is permanent for a txid per DIP-0010 and trust only ever settles, so both now latch monotonically when an entry for the outpoint already exists, and the coin-reservation flag is carried through unchanged. `is_confirmed` stays freshly derived so a reorg can still downgrade it. Addresses CodeRabbit review comment on PR dashpay#836 dashpay#836 (comment)
Move sync checkpoint seeding into `ManagedWalletInfo` construction so it can't be forgotten, and let callers control the scan range when importing. - `WalletInfoInterface::from_wallet` now takes `birth_height` and seeds `synced_height` and `last_processed_height` to `birth_height.saturating_sub(1)`. Previously every wallet-add path had to remember to call `set_birth_height` separately, and forgetting it dragged `WalletManager::synced_height` (a min across wallets) back to genesis on every add. - `WalletManager::import_wallet_from_extended_priv_key`, `import_wallet_from_xpub`, and `import_wallet_from_bytes` (plus the matching FFI) gain a `birth_height` parameter so previously-used keys can rescan from a chosen height instead of being silently anchored at the chain tip. - Unused `ManagedWalletInfo::with_birth_height` removed.
Co-authored-by: Kevin Rombach <35775977+xdustinface@users.noreply.github.com>
…y#848) When no explicit `start_from_height` is configured, `DashSpvClient::new` now derives the sync anchor from the wallet's `earliest_required_height` and starts from the nearest checkpoint at or before it. Previously the wallet birth height only short-circuited the compact-filter download, while block headers and filter headers always synced from genesis. Explicit `start_from_height` still takes precedence. This also fixes the checkpoint anchoring itself, which never worked. `initialize_genesis_block` rebuilt the checkpoint header with a hardcoded version `0x20000000` and verified the recomputed hash against the stored `block_hash`. Old blocks use version `2`, so the hash never matched, the code logged a warning and silently fell back to a full genesis sync, defeating `start_from_height` entirely. The `Checkpoint` does not store the block version, so the exact header bytes cannot be reconstructed. Since the checkpoint `block_hash` is a trusted constant and chain linkage only ever compares against the stored hash (never a recomputed one), we now store the anchor via the new `HashedBlockHeader::with_trusted_hash`, pairing the reconstructed header with the trusted hash. `time` and `bits` still come from the checkpoint for difficulty checks of later headers.
…yless wallet (dashpay#847) * fix(key-wallet): make add_account(_, None) fail actionably on keyless wallets Deriving an account from an in-wallet private key on a watch-only or external-signable wallet used to bubble up a cryptic InvalidParameter("External signable wallet has no private key"), which got misdiagnosed as a production bug three times instead of being read as a usage error. Add a typed, pattern-matchable Error::KeylessWalletRequiresAccountKey { account_type, required_key } and map ONLY the keyless-wallet failure of root_extended_priv_key() to it in the derive-from-root branch of add_account / add_bls_account / add_eddsa_account. Its Display names its own remedy: supply the account's xpub/seed via the Some(..) argument. The None hot-wallet convenience path, the Some(..) branch, and every other error path are left unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BqMDY7ZZNwSvdPgBrJQQNq * fix(key-wallet-ffi): handle KeylessWalletRequiresAccountKey in FFIError conversion New key_wallet::Error variant broke the exhaustive match in From<key_wallet::Error> for FFIError, failing CI (clippy, docs, MSRV, ASan, ffi builds on all platforms) with E0004 non-exhaustive patterns. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Extend `mainnet_checkpoints()` up to height 2,500,000 (previously 2,300,000) and `testnet_checkpoints()` up to height 1,500,000 (previously 1,400,000), keeping the existing 50k-block cadence. Mainnet entries were sourced from Blockchair and cross-verified against cryptoid so every block hash was confirmed by two independent providers, and Blockchair's raw-header fields were checked against its own dashboard `chainwork`. Testnet entries came straight from a local `dashd` via `getblockheader`. New entries carry `masternode_list_name: None` since that field has no consumer in `dash-spv`. Extend `test_mainnet_checkpoint_consistency` and `test_testnet_checkpoint_consistency` with a monotonic `chain_work` check, which confirms each new checkpoint chains onto the previous cumulative work.
…ashpay#862) DiskStorageManager::new used Path::set_extension("lock") to derive the data-directory lockfile. For directory names containing dots (e.g. probes/95.183.53.19-9999), set_extension replaces everything after the last dot, so distinct sibling directories collided on the same lockfile and spuriously failed with "Data directory ... is already in use by another process". Append ".lock" to the full final path component instead. Dot-free paths are unchanged (discovery -> discovery.lock). Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…y on drop (dashpay#859) The BLS and Ed25519 extended private key types held raw secret material but had no Zeroize/Drop implementations, so their secret bytes were left in freed heap memory when dropped. Their secp256k1 siblings (ExtendedPrivKey, RootExtendedPrivKey) already wipe themselves on drop. Mirror the secp256k1 pattern for both types: a hand-written Zeroize that wipes the secret key, chain code, and derivation metadata, plus a Drop impl that calls it. Add a Zeroize impl for Fingerprint so modules outside bip32 can wipe it too. Also replace the derived PartialEq on ExtendedEd25519PrivKey, which short-circuited over the raw 32-byte secret, with a constant-time comparison of the secret bytes. Fixes dashpay#858 Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…dashpay#863) During SPV historical sync, `dash-spv` chose which blocks to download by matching the wallet's watched scriptPubKeys against BIP157/158 compact filters. The matcher built its `FilterQuery` from scriptPubKeys only. Dash Core, however, also inserts a `ProRegTx`'s owner (`keyIDOwner`) and voting (`keyIDVoting`) key hashes into the block's compact filter as bare 20-byte `hash160` elements, not as P2PKH scripts (see `ExtractSpecialTxFilterElements`). A masternode owner who controls only the owner/voting keys, with no wallet-owned input or output in the `ProRegTx`, therefore never matched, the block was never downloaded, and the payload-aware wallet scan never ran on it. This adds each provider owner and voting key hash (bare 20-byte `hash160`) to the compact-filter query. `key-wallet` surfaces them via `WalletInfoInterface::monitored_filter_elements`, extracting the same bytes (same order) the BIP37 bloom path already inserts via `address_payload_bytes`. `key-wallet-manager` exposes `WalletInterface::monitored_filter_elements_for` and folds the extra elements into the shared `FilterQuery` (its `other` length bucket) in `check_compact_filters_for_elements` (renamed from `check_compact_filters_for_script_pubkeys`). `dash-spv`'s filter manager now passes each wallet's elements alongside its scripts in both the batch scan and the new-script rescan, including the per-wallet attribution queries. Step 1 of dashpay#861: owner/voting only. Collateral outpoint and `proTxHash` follow in stacked branches.
…ashpay#864) Dash Core inserts a `ProRegTx`'s `collateralOutpoint` into the block's BIP158 compact filter serialized the consensus way — 32-byte txid followed by the 4-byte little-endian vout, 36 bytes total (see `ExtractSpecialTxFilterElements`). A wallet that owns the 1000-DASH collateral output but holds none of the masternode's owner/voting keys — for example a third party registered the masternode against the wallet's collateral — never matches that `ProRegTx` on the scriptPubKey path, so the block is skipped during compact-filter sync. This extends `ManagedWalletInfo::monitored_filter_elements` to append each watched UTXO's consensus-serialized outpoint after the existing owner/voting hashes. The matcher and manager already fold `monitored_filter_elements` into the shared `FilterQuery`, so no change is needed there. The outpoint elements are gated on the wallet holding provider accounts (`ManagedAccountCollection::has_provider_accounts`). The collateral element only helps a masternode owner — it discovers a `ProRegTx` that references one of the wallet's UTXOs as collateral — and the common paths are already covered without it (an owner-created `ProRegTx` is matched by its fee input's script, a key-holding owner by the owner/voting hashes). A funding-only wallet can never hit that case, so it skips these elements rather than pay the per-UTXO false-positive cost on every compact-filter query. Within a provider wallet every UTXO is watched, to avoid hardcoding network-specific collateral amounts. Step 2 of dashpay#861: collateral outpoint. `proTxHash` follows in the next stacked branch.
…) structs (dashpay#870) * feat(key-wallet-ffi): one-shot transaction decode to (address, amount) structs Port the transaction-decode FFI from dashpay/platform#3981 (rs-platform-wallet-ffi/src/tx_decode.rs) into key-wallet-ffi, so wallet-less Core-chain decoding lives next to the library that owns the types. `transaction_decode` consensus-decodes raw tx bytes into `DecodedTransactionFFI` — per-input `(prev_txid, prev_vout, best-effort sender address)` and per-output `(address, value_duffs, scriptPubKey)` — in one struct-out call; `decoded_transaction_free` releases it. Adapted from the platform version to this crate's conventions: bool return with an `FFIError` out-parameter instead of a result struct, and `dash_network::ffi::FFINetwork`. Struct names are kept verbatim so the Swift wrapper migration stays a re-point. Closes dashpay#869 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * chore(key-wallet-ffi): regenerate FFI docs and apply typos fix Pre-commit fixes: regenerate FFI_API.md for the new transaction_decode / decoded_transaction_free entries, and "unparseable" -> "unparsable" per the typos hook. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…_signed (dashpay#872) * fix(key-wallet): return the fee the tx actually pays from build/build_signed TransactionBuilder::build_unsigned()/build_signed() returned a fee computed from the encoded size of the built transaction. When the builder drops a dust change remainder (0 < change <= 546 duffs), those duffs go to miners, so the real fee is the size fee plus the dust — the returned value under-reported by up to 546 duffs exactly when the fee got larger. Compute the returned fee as sum(selected input values) - sum(output values) instead, which is what the transaction actually pays in every case (normal change, exact change, dust drop, and drain). Remove the now-unused encoded_size helper and its error paths, and update the doc comments. Fixes dashpay#871 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(key-wallet): pin asset-lock fee semantics — locked credits are not fee An asset lock burns the locked amount into an on-chain OP_RETURN output mirroring the payload's credit outputs, so it is part of the output sum and inputs - outputs yields the miner fee only. Add a regression test proving the returned fee excludes the locked credits. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
dashpay#867) * fix(key-wallet): cover CoinJoin/DashPay accounts for AssetLock routing `TransactionRouter::get_relevant_account_types(AssetLock)` returned only the BIP44/BIP32 standard accounts plus the identity/asset-lock accounts, omitting the CoinJoin and DashPay fund-bearing accounts. Only `check_core_transaction` marks a UTXO spent, and it scopes spend detection to the account types this router returns. So an asset lock funded from a CoinJoin or DashPay UTXO never had its input debited: the spent UTXO kept counting toward the balance indefinitely while the (BIP44) change output was still credited, inflating the reported balance by exactly the spent amount — on both relay and rescan. Field evidence (Android SDK wallet, dashpay/dash-wallet#1507): outpoint-diff reconciliation against the dashj oracle showed the SDK balance high by exactly the value of asset-lock-spent CoinJoin inputs that were never debited. Fix: the AssetLock arm now consults the full `fund_bearing_account_types()` set (BIP44, BIP32, CoinJoin, DashpayReceivingFunds, DashpayExternalAccount) before the identity/asset-lock accounts. Discovery is membership-based like Dash Core's `IsMine`, so consulting extra accounts never yields false positives. Adds an end-to-end regression test that funds a CoinJoin UTXO, spends it via an asset-lock transaction through `check_core_transaction`, and asserts the CoinJoin UTXO is debited and removed (fails on the old routing: total_sent 0). Also extends the AssetLock routing unit test. Refs: dashpay/platform#4073, dashpay/platform#4074, dashpay/dash-wallet#1507 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(key-wallet): assert aggregate wallet balance in the asset-lock CoinJoin-debit test Addresses CodeRabbit's nitpick on dashpay#867. The regression the test guards against is balance inflation, but it previously asserted only the per-transaction totals and the CoinJoin UTXO removal. Add confirmed/spendable/ total balance assertions after the spend: a non-debited CoinJoin coin would report funding_value + change_value; a correct debit leaves only the confirmed BIP44 change (the asset-lock credit output is locked into Platform, never a spendable wallet UTXO). Existing assertions preserved. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…j parity (dashpay#868) Proposal: align the CoinJoin address discovery window with dashj's `DeterministicKeyChain` lookahead (`DEFAULT_LOOKAHEAD_SIZE = 100`), the effective window dashj-core watches for CoinJoin keychains. This is window dimensioning, not a scan-algorithm fix. The gap limit only bounds the largest run of consecutive UNUSED CoinJoin addresses the wallet can bridge before discovery stalls; the progressive rescan-to-quiescence algorithm in dash-spv + key-wallet is already correct at gap 30 for any spray whose unused runs are <= 30. See PR description for the two-sided evidence and the reference to the ongoing team discussion. Refs: dashpay/platform#4074, dashpay/dash-wallet#1507 Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
|
This PR has merge conflicts with the base branch. Please rebase or merge the base branch into your branch to resolve them. |
xdustinface
force-pushed
the
feat/mainnet-hd-wallet-sync-floor
branch
from
d6eb5ab to
c4e7e46
Compare
Floor mainnet sync at block 227121, the height from which HD/BIP39 wallets could first exist on chain. No address any HD wallet derives can appear before it, so syncing the pre-HD-wallet chain is wasted work. This matches the entry in DashSync's `mainnet_checkpoint_array` commented "first sync time (aka BIP39 creation time)", the value the official Dash mobile SPV client uses. Add `MAINNET_HD_ACTIVATION_HEIGHT` and a `NetworkExt::hd_wallet_sync_floor()` method to the `dash` crate, alongside `known_genesis_block_hash` where the other per-network chain constants live (mainnet returns the floor, other networks return `0`, meaning sync from genesis). When the caller sets no explicit `start_from_height`, `DashSpvClient::new` floors the wallet-derived birth height via `birth_height.max(config.network.hd_wallet_sync_floor())`, so a low or zero birth height can no longer drag mainnet sync back to genesis. Anchoring reuses the existing checkpoint machinery and snaps to the nearest checkpoint at or before the floor (the 200000 checkpoint), a safe undershoot that never skips a block that could hold an HD wallet transaction. An explicit `start_from_height` is still honored downward, and testnet/regtest/devnet are unaffected.
xdustinface
force-pushed
the
feat/mainnet-hd-wallet-sync-floor
branch
from
c4e7e46 to
202b0a1
Compare
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
8 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.
Summary
On mainnet the SPV client currently defaults to syncing from genesis (or from a wallet's birth height, which can be
0). But Dash HD/BIP39 wallets did not exist before block 227121, so no address any HD wallet derives can appear on chain earlier. Syncing the pre-HD-wallet chain is wasted work.This floors mainnet sync at height 227121 so a default client never anchors below it.
The number and its provenance
227121is the value the official Dash mobile SPV client uses. It is the entry right after genesis in DashSync's (dashpay/dashsync-iOS)mainnet_checkpoint_array, carrying the literal comment:Changes
MAINNET_HD_ACTIVATION_HEIGHT(227121) and aNetworkExt::hd_wallet_sync_floor() -> u32method to thedashcrate, alongsideknown_genesis_block_hashwhere the other per-network chain constants live. Mainnet returns the floor; other networks return0(sync from genesis).DashSpvClient::new, when no explicitstart_from_heightis set, floor the wallet-derived birth height viabirth_height.max(config.network.hd_wallet_sync_floor()).200000checkpoint), a safe undershoot that never skips a block that could hold an HD wallet transaction.Behavior
Per design clamp birth, honor explicit:
0) is clamped up on mainnet, anchoring at the200000checkpoint. A low or zero birth height can no longer drag mainnet sync back to genesis.start_from_heightstill wins and is honored downward.0).Tests
dash:test_hd_wallet_sync_floorcovers the per-network mapping.dash-spv: extendedbirth_height_anchors_chain_to_nearest_checkpoint— birth0and120_000anchor at200_000; birth560_000still anchors at550_000; explicitstart_from_heightbelow the floor is still honored.Notes
checkpoints.rsbut this change no longer does, so conflict risk is low.🤖 Generated with Claude Code