Repository navigation
chore: release v0.45.0 - #1208
Merged
Merged
Conversation
…ield/accessor renames) (#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 #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>
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>
This reverts commit 0b056c2.
…#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>
…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 #800. * fix: emit `tracing::warn!` message as format string in `dash-spv` quorum lookup Addresses CodeRabbit review comment on PR #801 #801 (comment) * refactor: drop redundant `QuorumHash` import in `helpers` test module Addresses CodeRabbit review comment on PR #801 #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 (#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.
) `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 (#823)
* 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>
…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
* 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 (#839) * refactor(key-wallet): move extended_public_key out of Signer into ExtendedPubKeySigner Signer::extended_public_key (added in #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>
) * 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 #838 #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 #838 #838 (comment)
…ported (#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 #840 Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
… after a restart (#835) Fixes #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>
* 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 #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 #832 (U1). --------- Co-authored-by: lklimek <842586+lklimek@users.noreply.github.com>
* 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 #844 #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 #836 #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>
…1149) Partial cherry-pick of the dev commit below: only its `CoinbasePayload` part, which #1137 needs so coinbase payloads persisted through binary serde keep decoding. Its `PlatformNodeId` and `BLSPublicKey` parts repair regressions from #1056 and #1036, which this branch does not contain, so the formats they restore are already the ones here. Original message: fix(dash)!: restore the serde formats of `PlatformNodeId`, `BLSPublicKey` and pre-v4 `CoinbasePayload` (#1149) * fix(dash)!: restore `PlatformNodeId` and its canonical-order serialization #1056 replaced `PlatformNodeId` with an alias of `EddsaPkHash`, which holds the wire order. Its binary serde writes the in-memory bytes, so `bincode::serde` started storing node IDs byte-reversed, silently misreading records persisted before (Platform stores wallet transaction records this way). Restore the earlier `PlatformNodeId`, which holds the canonical order and reverses only on the consensus wire, so JSON, binary serde, native bincode and consensus encodings match the pre-#1056 ones again, as do `from_byte_array`/`to_byte_array`. `EddsaPkHash` stays as the Ed25519 public key hash and converts into a `PlatformNodeId` in canonical order. A test pins the binary serde bytes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(dash): keep the pre-v4 serde shape of `CoinbasePayload` #1137 added `merkle_root_asset_unlocks` to the derived serde impl, which writes it for every version, so coinbase payloads persisted through binary serde before (Platform stores wallet transaction records this way) no longer decode. Write the serde impls by hand with the earlier shape, adding the field only from version 4, as the bincode impl already does. Tests check that a version 3 payload serializes to the same bytes and JSON as the earlier derived impl and decodes from them. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(dash)!: restore `BLSPublicKey` as a type serialized as a hex string #1036 made `BLSPublicKey` an alias of `BlsPkBytes`, whose serde impl writes raw bytes in binary formats, while the earlier type wrote its hex string in every format. Records persisted through binary serde before (Platform stores wallet transaction records this way, with operator keys in ProRegTx and ProUpRegTx payloads) no longer decode. `BLSPublicKey` is its own 48-byte type again, serialized as a hex string in every format as before. Native bincode and the consensus encoding are unchanged. It converts to and from `BlsPkBytes` for cryptographic operations, which the quorum aggregate verification and the key-wallet BLS derivation now do explicitly. A test pins the binary serde bytes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(dash): reject the CbTx asset unlocks root before version 4 in serde maps The hand-written `visit_map` accepted `merkle_root_asset_unlocks` at any version, so a version 3 map carrying it decoded into a payload whose root was dropped on the next encode. Read the version first and reject the field before version 4, as `visit_seq` already ignores it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * style(dash): use the compact license header in platform_node_id Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 66f86e8)
…1155) Core reads the version as an int16_t and a payload only when nVersion >= 3 and nType != 0. Below version 3, non-zero type bits are kept as ClassicalWithNonStandardVersionTypeBytes, without a payload. Closes #1135 Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 90359ed)
…1154) * feat(dash)!: keep special payloads raw when their layout is unknown Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(dash): cover a known payload kept raw because it re-encodes differently Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(dash): keep a payload raw instead of panicking when its encoder fails Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(dash): report a raw payload of type 0 as Unknown, not Classic Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs(key-wallet-ffi): a raw provider payload leaves every pointer null Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 2232580)
Dash Core v24 moved deployment status out of getblockchaininfo into the
new getdeploymentinfo RPC (dash#6888): getblockchaininfo carries
`softforks` only with -deprecatedrpc=softforks. GetBlockchainInfoResult
required the field, so decoding any Core v24 answer failed with
"missing field `softforks`", which stops Platform's InitChain and
empties DAPI's blockchain status.
- `GetBlockchainInfoResult.softforks` defaults to an empty map when
Core leaves it out; its type is unchanged.
- `RpcApi::get_deployment_info(block_hash)` calls getdeploymentinfo at
the given block, or at the chain tip for `None`, and decodes
`{hash, height, deployments}` into `GetDeploymentInfoResult`. Its deployments are the `SoftforkInfo`
entries getblockchaininfo reported, built by the same Core function.
Core v23 answers it with method not found (-32601).
Tests decode real answers of Dash Core v24.0.0-rc.3 and v23.1.8 on
regtest, and pin that each buried deployment v23 reported in softforks
v24 reports identically in deployments.
Test would have caught this in CI: the Core v24 getblockchaininfo test
fails before the fix ("missing field `softforks`", the error seen on a
devnet) and passes after it.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
(cherry picked from commit 985dc93)
`Transaction::deserialize_ignoring_asset_lock_payload_length` decodes an asset lock as this crate did before special payloads were read strictly through their length prefix (#1154): the prefix must be a minimal CompactSize, its value is ignored, and the payload must end the bytes. Any other transaction goes to `deserialize`. Dash Platform needs it for instant asset lock proofs. Neither the InstantSend lock (checked against the re-encoded txid) nor the transition signature covers the prefix, and Platform releases built on earlier pins accepted and charged for proofs whose prefix differs from the payload length. Platform v5.0.0-rc.1 restored that with a byte-level workaround on its side (dashpay/platform#5403); this gives it a named, frozen decoder. The function is standalone rather than a mode of `Decodable for Transaction`, and writes the payload layout out instead of calling `AssetLockPayload::consensus_decode`, so a later transaction rule or a new asset lock payload layout cannot change what it accepts. It makes the same finite-reader calls as the transaction decoder. `dash/tests/legacy_asset_lock.rs` pins absolute results (fixed bytes, txids, error variants): 1/3/5/9-byte prefixes, versions 4 and 0x7fff, the witness form (including a witness on one of two inputs) and the zero-input form, a long payload read after a short prefix, a payload without credit outputs, payload versions 0/2/3/255 with appended fields counted by the prefix or not, and the refusals (non-minimal prefix or credit-output count, truncation, trailing bytes, segwit flags 0 and 2, a classic transaction). A coinbase, a ProRegTx, an unknown type and type 8 below version 3 are checked against `deserialize`, and a seeded round-trip property covers mutated inputs. Each of these changes to the function turns tests red: delegating everything to `deserialize` (22 of 46), dropping the segwit rule (12), checking `any` instead of `all` witnesses empty (2), accepting only version 3 (2), comparing the version unsigned (1), refusing empty credit outputs (2), dropping the trailing-bytes check (6). A differential run against Platform's workaround over 16M seeded mutations, on dev and on 7302d0a, found no Ok/Err, decoded-transaction or error mismatch. Adapted for this branch: the `AssetLockPayload` import no longer sits next to the `AssetUnlockPayload` import from #1157, which this branch does not have, and the tests decode hex with `hex`, since `hex-conservative` is not a dependency here. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 28c1a04)
`Transaction::deserialize_ignoring_asset_lock_payload_length` becomes `Transaction::deserialize_with_legacy_asset_lock_framing` and moves to `dash/src/blockdata/transaction/legacy_asset_lock.rs`. The transaction module keeps only the `mod` line. The name and the doc comments now say plainly that this is a legacy decoder whose framing must never change. It is neither `#[deprecated]` nor behind a cargo feature: Dash Platform has to call it for as long as it must agree with its older binaries and replay its chain, so there is nothing to migrate to, and a feature would only allow building Platform without it. The function body is byte-for-byte the one it replaces. The 46 tests in `dash/tests/legacy_asset_lock.rs` pass with only the name changed. No red/green step applies: nothing behaves differently. Adapted for this branch: the removed `AssetLockPayload` import no longer sits next to the `AssetUnlockPayload` import from #1157, which this branch does not have. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit f9f195c)
…ubmitchainlock` (#1148) `submit_chain_lock` sent the hex of the hash's internal bytes, while Dash Core reads `blockHash` with `ParseHashV`, which expects the reversed display order. Core then verified the signature against the wrong hash and answered `invalid signature`, except when it already held a ChainLock at that height, where it returns before checking. Pass the hash through `into_json` as the other methods do. Closes #1128 Adapted for this branch: the signature argument here is still built with `hex::encode`, not `to_lower_hex_string`, so only the block hash line changes. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit f4abb26)
… masternode list diff (#1204) NOT SECURE, AND TEMPORARY. MasternodeListEngine keeps the coinbase payload of the highest block an applied masternode list diff carried one for, and exposes it as MasternodeListEngine::latest_coinbase() and DashSpvClient::latest_coinbase(). Nothing authenticates it: neither the diff's merkle proof of the coinbase nor the payload's merkle roots are checked, and quorum verification does not gate it, so one peer can serve a payload of its choosing. On dev, #1202 (on top of #1200) replaces it with a version that keeps only verified coinbases, under the same names and return types. Platform v5.0's wallet reads the payload version to tell whether Dash Core's V24 deployment is active (version 4 from V24), so it builds the ProUpServTx version Core accepts for an evonode. The payload is kept in memory only: serde skips it and the engine's bincode Encode/Decode are hand-written with the derived layout minus the field, so persisted engines decode unchanged. Breaking: the field is pub, so struct literals and patterns listing every field must add it. When this branch is reconciled with dev, drop this change rather than port it; see the PR description for the conflict resolutions. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`observed_spent_outpoints` records every input of every transaction in every block the wallet processes, false positives included, and is pruned only up to a chainlock the wallet has applied. The client defers chainlocks until `SyncComplete`, so nothing is pruned during an initial sync. On a mainnet restore of the bench wallet the map reached 1 487 967 entries (every input of 10 295 blocks, 505 946 transactions), above the 1 000 000 entries the serde adapter accepts on load, so a wallet persisted mid-sync could not be loaded back. The dispatcher still defers applying chainlocks, but now passes the deferred chainlock's height on through `note_chain_lock_height`. The wallet uses it only as a finality boundary for pruning: entries at or below min(synced_height, highest chainlock applied or noted) are evicted. As before, only chain-locked spends are forgotten, and no record is promoted. Pruning mid-sync opened one path: redelivering a funding transaction whose output sits in `spent_before_funded`, after its observed-spend entry was evicted, re-inserted the spent coin into `utxos`. `update_utxos` now keeps such an output held; the new test fails without that guard. Validated with 5 consecutive mainnet restores at 100 Mbit / 100 ms, all ending at 7112 records, 14 114 383 sat and 13 389 addresses. Peak RSS was 1378–1724 MiB against 1488 MiB before, within run-to-run spread. Claude-Session: https://claude.ai/code/session_017DruChNTWXwJoWPartZwCf Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> (cherry picked from commit 6533320)
…sactions (#1009) The four `Provider*` arms of `get_relevant_account_types` listed the standard and CoinJoin accounts but not the two DashPay ones. That list is not only a discovery hint: `check_core_transaction` marks a UTXO spent only for the account types it returns, so a provider transaction funded from a DashPay contact account had its inputs never debited. The spent coins kept counting toward the balance while the change output was credited on top - the same failure the `AssetLock` arm already documents from dashpay/platform#4073. Nothing in the protocol confines a provider transaction's funding to one kind of account. DIP-3 lets a ProRegTx's collateral be an output of the ProRegTx itself, so its inputs pay for it; the three update transactions carry no collateral and DIP-3 is silent on their fee, which DIP-2 settles instead - "the inputs and outputs of special transactions usually have the same meaning as with classical transactions". Coins paid to the DIP-15 contact addresses a DashPay account receives on are ordinary UTXOs, spendable like any other. Every arm now names the DIP that defines its transaction type, from DIP-2's type table, including `Ignored` (quorum commitments, DIP-6; masternode hard fork signals, DIP-23), which said nothing before. `Coinbase` is deliberately left narrow, and its comment corrected: it claimed the payout is always a standard address, which DIP-3 contradicts - it requires only P2PKH or P2SH, and a CoinJoin or DashPay address is P2PKH too. A coinbase has no wallet inputs to debit, so a missed account costs an uncredited reward rather than a stale UTXO, and there is one in every block of a scan, making it the one arm where widening is paid for on the whole chain. That is a cost trade-off, not a correctness claim. `fund_bearing_with` replaces the hand-rolled extension the `AssetLock` arm was doing. Verified: the four provider routing tests now assert the DashPay types, 709 key-wallet and 647 dash-spv tests pass, clippy and rustfmt clean. Claude-Session: https://claude.ai/code/session_01SbgCpMiBjnpvW4CyEEsKXw Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> (cherry picked from commit 59f656c)
…#1035) * fix(key-wallet): extend DIP-15 contact pools as their payments arrive A DashPay contact chain monitored only the addresses built at construction, so once a contact had used them all, its next payment landed on an address the wallet never derived and was never seen: invisible to balances, and unrecoverable by rescan since the address does not exist client-side. check_core_transaction marks the matched address used, then asks the wallet for a key source to run maintain_gap_limit with. It asked via AccountTypeToCheck, whose Dashpay variants carry no data: a contact account is keyed by its index plus the friendship's two identity ids, so no lookup by (variant, index) can find one. The helper returned None for both Dashpay variants, that became KeySource::NoKeySource, and the NoKeySource guard skipped pool maintenance altogether. Resolve the key source from the fully-keyed AccountType, which the matched account already carries, via AccountCollection::account_of_type. That is the same per-variant `.map(|a| a.account_xpub)` lookup extended_public_key_for_account_type performed, against a table that does hold the contact chains, so every other account type resolves exactly as before: the BLS operator arm is untouched, and ProviderPlatformKeys still yields NoKeySource because account_of_type returns None for the BLS/EdDSA accounts by design. The test pays 21 consecutive addresses of one friendship and asserts the monitored window advances with each. It reads pool.gap_limit rather than a literal, so it holds at whatever the contact gap limit is set to. Closes #1032 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor(key-wallet): name the DashPay contact gap limit, per DIP-15 The contact pool size was the literal 20, repeated at both Dashpay arms of ManagedAccountType::from_account_type with nothing to say where it came from. No DIP mandates 20 for contact chains: DIP-15 recommends 10 ("load 10 addresses past the last used address"), and the 20 that DIP-17 recommends covers Platform payment keys, a different chain already served by DIP17_GAP_LIMIT. Name it DEFAULT_DASHPAY_GAP_LIMIT and set it to what DIP-15 recommends. This narrows each friendship's initial pool from 20 addresses to 10, in both the receiving and the external direction; raising it again is a policy call that deserves its own discussion. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(key-wallet): cover both directions of a DIP-15 friendship The regression test exercised only DashpayReceivingFunds. The external direction reaches gap maintenance by its own route — account_of_type reads it from dashpay_external_accounts, a separate collection — so a regression confined to that arm would have gone unnoticed (#1035 review). Run both chains of one friendship, and assert up front that reversing the identity ids gives the two directions distinct address chains, since DIP-15 derives them from the same pair in opposite order. Address the window by gap limit rather than by a literal index. The first address past the construction window is the one a contact's payment goes missing on, and it moved from 20 to 10 when the pool size became DEFAULT_DASHPAY_GAP_LIMIT; indices relative to pool.gap_limit keep the test aimed at that boundary whatever the limit is set to. Two payments per chain now cover it: one to the last address built at construction, which must widen the window, and one to the address that only exists because it did. Verified by mutation: returning NoKeySource from key_source_for_account for both Dashpay variants fails the test on the receiving chain, and doing it for DashpayExternalAccount alone fails it on the external chain, where it previously passed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(key-wallet): assert a contact pool keeps sliding, not widens once The payment past the construction window was only checked for relevance, so gap maintenance that ran a single time would have satisfied the test (#1035 review). Assert the window moved again after that payment: an address derived by maintenance must, once used, widen the window in its turn. Verified by mutation: capping maintain_gap_limit's target at one widening fails this assertion alone, with every other assertion in the file still passing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> (cherry picked from commit 44fb0b1)
… arrives late (#1037) mark_instant_send_utxos overwrote the record's context with InstantSend unconditionally. For a transaction the wallet already held as InBlock that throws away its BlockInfo, since InstantSend carries no height, and apply_chain_lock only promotes InBlock records, so the transaction never reached InChainLockedBlock (#1020). This is the path dash-spv takes for a lock arriving after the transaction is already tracked (process_instant_send_lock). The other entry points already refuse to downgrade a confirmed record: check_transaction treats an IS lock on a confirmed transaction as stale, and chainlock promotion skips confirmed records. mark_instant_send_utxos now does the same and leaves a confirmed context alone. The UTXOs are still flagged as IS-locked. Two tests: a late lock on a mined record keeps its InBlock context and height and a later chainlock promotes it; a lock on an unconfirmed record still applies as before. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> (cherry picked from commit ea0b24d)
* fix(key-wallet): re-apply a spend whose coin was funded after it Two mainnet restores of the same wallet ended at the same balance with 7112 and 7111 wallet records. Replaying each run's logged block applications offline, against the blocks it stored, reproduced both results exactly, with no divergence from the logs. The missing record was e66553f3…8c9e at height 2 185 057: it pays change to the BIP44 account and spends a CoinJoin coin funded at 2 182 877. When the spend is applied before its funding, the CoinJoin account cannot recognise it. The funding then parks the coin in `spent_before_funded` (#1001), and that only attributes the spend if its block is delivered again. In one run it was (funding at step 1494, spend at 1516); in the other it was not (spend at 1608, funding at 1627, no redelivery), so the result depended on delivery order. `WalletInfoInterface::unrecorded_spend_heights` reports, for a transaction, the heights of the blocks that spent its outputs before it arrived and that the owning account has not recorded yet. `process_block_for_wallets` returns them per wallet in `BlockProcessingResult::reapply_heights`, only heights above the block being applied, so re-applying cannot loop. `BlocksManager` re-applies those blocks from block storage straight away; every downloaded block is stored on arrival. Re-applications emit no `SyncEvent::BlockProcessed` and so never touch a batch's pending-block accounting. Offline, both orderings now end with identical per-account records (7112), with about 105 blocks re-applied from disk per restore. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017DruChNTWXwJoWPartZwCf * fix(dash-spv): don't count block re-applications as processed blocks Re-applying a stored block went through the same accounting as a first pass, so every re-application added to `processed` and `relevant`: a mainnet restore reported 11 294 processed blocks for 10 326 downloaded plus 855 loaded from storage. Only new transactions are still counted on re-application, since a transaction first recorded there was never counted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> (cherry picked from commit 59dd41b)
…es (#1059) `verify_chain_lock` checks the ChainLock against the list at or below its signing height and, when that fails, retries against the list above it if that list's ChainLock quorums differ. With no list above, it fell through to `Ok(())` and dropped the error it had just computed. So any ChainLock signed at or above the engine's newest list verified, whatever its signature. `ChainLockManager::process_chainlock` only skips heights at or below the best ChainLock and accepts a block hash it has no header for, so one forged `clsig` from any peer at a height ahead of the tip was validated, persisted as the best ChainLock, and made every genuine ChainLock below it be ignored until the chain caught up. The retry path is unchanged; when nothing can do better than the list below, its error is returned. Tests: - dash: a genuine mainnet ChainLock (the one `chain_lock_verification` checks) verifies at its own height and is rejected when replayed at newest + 1000. - dash-spv: `process_chainlock` of a zero-signature ChainLock above the only list reports `validated: false` and leaves no best ChainLock. Both fail without the fix. Verified: fmt, clippy --workspace --all-features --all-targets -D warnings, dashcore 617, dash-spv 571 + dashd_masternode 10 + dashd_sync 32, dash-spv-ffi 49 + dashd_sync 7. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit defa02a)
The `headers` decoder reserved space for up to 16384 headers from the peer-controlled count before reading any, so a few-byte frame with a huge count triggered a ~1.3 MB allocation. `headers2` did the same, capped at `MAX_VEC_SIZE`. Core never sends more than 2000 headers in `headers` or 8000 in `headers2` (`MAX_HEADERS_UNCOMPRESSED_RESULT` and `MAX_HEADERS_COMPRESSED_RESULT`) and treats more as misbehaviour. Reject larger counts before reserving, which also bounds the allocation. Closes #1124 Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 644d373)
…1147) The DEBUG request log wrote every argument and the TRACE response log every result, which include wallet secrets such as the `importprivkey` key, wallet passphrases and the `dumpprivkey` result. Log only the command. Closes #1122 Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit fefeafa)
* fix(key-wallet): sweep a conflict loser's keys-only records too `ManagedWalletInfo::sweep_conflicts` removed a loser from funds accounts only. An asset lock is recorded twice — in the funds account that pays for it and in the identity keys account it funds — so the second record outlived the sweep, on both entry points (`check_core_transaction` with a final context and `mark_instant_send_utxos`) and for descendants swept along with a loser. The leftover record was more than clutter: - the identity account and `transaction_history()` kept listing a transaction that can never confirm; - `retain_unclaimed` counted its inputs as still claimed, so `released_outpoints` came back empty and a caller mirroring wallet state was never told which coin the sweep freed; - a later sighting of the same transaction was not new, so the funds account never recorded it again. Every swept txid is now removed from keys-only accounts before the released set is filtered. Wallets without keys-only records see the same `txids`, `released_outpoints` and balance as before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(key-wallet): pin the released outpoint of a swept asset lock on its own The assertion that the sweep names the coin only the swept asset lock spent sat behind the record check in the block-winner test, so it never failed on its own. In a test of its own it fails without the fix with an empty `released_outpoints`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(key-wallet): cover the keys-only sweep in the existing rescan test `sweep_conflicts` is the one place both sweep entry points go through, so a single loser that is an asset lock exercises the keys-only removal. `test_a_swept_losers_extra_input_is_recoverable_by_rescan` already has the shape that matters — a loser spending two coins, a winner spending one — and asserts the released set, so its loser becomes an asset lock paying the identity registration account and the record check widens from the BIP44 account to every account. The dedicated `sweep_keys_only_records_tests` module is dropped in favour of it. Without the fix the reworked test fails on the record check. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit d036d84)
`LLMQType::params()` disagreed with Dash Core's `src/llmq/params.h`: - llmq_test_platform, llmq_devnet_platform: DKG interval 24, not 288. - llmq_devnet_dip0024: mining window 12 to 20, not 10 to 18. - llmq_test: size 3, not 4. - llmq_devnet: keepOldConnections 5, not 4. - llmq_devnet_platform: keepOldConnections 5, recoveryMembers 6, and its name is llmq_devnet_platform. Closes #1180 Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit f35115b)
…` release branch
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #1208 +/- ##
==========================================
+ Coverage 72.87% 78.13% +5.25%
==========================================
Files 323 330 +7
Lines 71994 86826 +14832
==========================================
+ Hits 52463 67838 +15375
+ Misses 19531 18988 -543
|
3 of 4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Releases
v0.45.0frommasterfor Dash Platform v5.0. It isintegration/v5.0-core-v24(the Dash Core v24 backports on top ofdevat #989) plus a set of clear bugfixes backported fromdev, the0.45.0changelog and one test import fix. It stacks on #1207, which caughtmasterup tov0.44.0.