Repository navigation
chore: release v0.46.0 - #1402
Merged
Merged
Conversation
* feat(ci): route PR reviews through the shared ownership policy Pin the shared Platform evaluator while keeping repository policy local. Leave custom policy writes opt-in and preserve existing protections for staged activation. Validation: policy schema and paths, generated CODEOWNERS and Actionlint. Shared evaluator94 tests and policy CI passed. Application builds omitted because only workflow and ownership configuration changed. * fix(ci): pin the reviewed policy engine with feedback fixes * ci: read the review policy from dashpay/stale_prs_are_bad The policy is no longer copied here; the caller pins the shared engine in dashpay/stale_prs_are_bad and the engine reads this repository's policy live from that repository's protected branch. CODEOWNERS is regenerated from it: the fallback now names this repository's own owner instead of the Platform leads. The pin references the engine PR head and is re-pinned to its merge commit before activation. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * ci: pin the shared review engine to its merge commit dashpay/stale_prs_are_bad#5 landed as 25b9369077ef6394ffb5491be718a6f0740e6088. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * ci: re-pin the shared review engine dashpay/stale_prs_are_bad#7 landed as cc4d18484ceabf121886e47606b4b10628aafefa. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
The policy workflow now listens for pull_request_review itself, so the no-op signal workflow is gone. dashpay/stale_prs_are_bad#8 landed as d2978b05a15c8e29091958b0fa57b90a5ed5ff3b. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* test(key-wallet): pin the guard that keeps an explicit change address `fund_from` only asks the funding account for a change address when the builder has none, so a caller's `set_change_address` survives `add_funding`. Nothing tested that. The guard reads as a redundant `is_none` check next to an unconditional assignment, and removing it would compile, pass every test, and silently override the caller's address — while also advancing the change pool to derive one that is then discarded. The test asserts both halves: the explicit address is still there afterwards, and the account hands out the same next change address a pristine one would. Removing the guard fails it on the first assertion. The doc on `add_funding` now says the same thing in words, since the behaviour is a contract of the funding call rather than an implementation detail. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SbgCpMiBjnpvW4CyEEsKXw * test(key-wallet): test change address is preserved --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…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>
At the end of a run the client reads `VmHWM` from `/proc/self/status`, one read with no sampling, and writes `peak_rss_mib` to `summary.txt`. `run.sh` carries it into `results.tsv` and `report.md`. A 100 Mbit / 100 ms mainnet restore of the bench wallet peaks at 1488 MiB. Claude-Session: https://claude.ai/code/session_017DruChNTWXwJoWPartZwCf Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
The engine understands per-repository required_bots and publishes its status as `PR Hygiene`. dashpay/stale_prs_are_bad#10 and #11 landed as 36581c5a53672f06faf8ad2dc1c33b3ec5481d55. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Move rust-toolchain.toml from 1.95.0 to 1.98.1 and fix the clippy lints the newer toolchain raises under -D warnings. MSRV stays 1.89. Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
Drafts now record their state in the commit status without opening a comment. dashpay/stale_prs_are_bad#13 landed as 750eec13bc5e5f9c704f1a610aed51d376513fa2. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Progress now reports a green check instead of a permanent amber one, queued runs are no longer cancelled by events on other pull requests, and a bare `/self-reviewed` attests to everything pushed so far. dashpay/stale_prs_are_bad#15 and #16 landed as 8e36bbdd8daaeab02c108b976348fefaf7defba7. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
The engine can now ask a silent review bot once and stop waiting for it, and no longer treats a bot objection raised against an earlier commit as an outstanding one. dashpay/stale_prs_are_bad#17 landed as 64edd771b8a560c82f0351058eebadf2df94cdd9. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Ignores comments the engine never reads, rebuilds an author review slots with one query instead of two requests per pull request, reads collaborator access once per run, and sweeps hourly. dashpay/stale_prs_are_bad#20, #21, #22 and #24 landed as c115fa43691c6de989bd224989899d44f76bab89. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
The hourly sweep repairs pull requests whose events were missed. Its rotation cursor was bucketed by 15 minutes while the sweep runs hourly, so it advanced four batches per sweep and covered one; where the number of open pull requests shared a factor with that overshoot the same six were swept for ever. dashpay/stale_prs_are_bad#25 landed as 8192c065b4960cddb2244702ed1a1a9d7b8bed3c. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
A concurrency group holds one pending run and cancels it when the next event arrives, so a burst of events on one pull request cancelled the middle ones and a cancelled run renders as a failed check. Runs now queue in arrival order. dashpay/stale_prs_are_bad#26 and #27 landed as b378d1d7f9c3949a5f73822362ca02d251ab8141. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Rebuilding an author slots already read every candidate comments and latest inactive transition in one batched query, and the snapshot that followed read both again over REST. A reconcile of one platform pull request drops from 38 requests to 25. Also carries gh own message when an API call fails, instead of only its exit code, and keeps a GraphQL reply that only partly resolved. dashpay/stale_prs_are_bad#28 and #29 landed as 1ef768028f88b7f662fb260d9f8ba33fc0e48a44. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
) Every comment by either review bot started a full reconciliation. CodeRabbit receipt is a comment and has to be heard; thepastaclaw is a review, which arrives on its own event, and nothing reads its comments. dashpay/stale_prs_are_bad#30 landed as 17ce8d20f2ba1ac7103d01fcf2aea15ac7f1c01f. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
The clone of this repository was the slowest step in every run, nine of its twenty seconds, and it answers one question: do the policy area directories still exist. That can only change when a pull request merges, and the sweep asks every hour. dashpay/stale_prs_are_bad#31 landed as 6d57d4c22eeeeaff88554f6e78466be2aec7aed7. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…1033) A pull request now has to be green before reviewers are requested, and a build that goes red afterwards does not take that back. A second schedule looks again at the pull requests waiting on a build, because a build turning green raises no event this controller hears. dashpay/stale_prs_are_bad#32, #33 and #34 landed as 176b4072cb93c69a19c68ebdda2080b5ec12bea3. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…tion (#1034) One listing of collaborators does not name the people who reach a repository through the organisation rather than as collaborators of it, and reading their absence as no write access marked six platform pull requests a configuration error. Anyone the listing omits is asked about directly now. dashpay/stale_prs_are_bad#35 landed as 6636e320352dfafaa8187eb73b2d51c9975f2a48. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
The PR Hygiene status now passes only when the review policy is satisfied and is pending while anything is still waiting, so it can be required to merge. With it required, GitHub own approval count on this branch goes to zero and the policy is the one thing that decides: an author who owns every area they touched needs the bots, their attestation and a green build; anyone else also needs an owner approval. CODEOWNERS is a file with no rules now, on purpose. A rule there had GitHub request every owner the moment a pull request was opened, before the bots or the author had read it. The file stays, pointing at the policy. dashpay/stale_prs_are_bad#37 landed as 2a9705fbefd4294f490baa36f46f2b880a592502. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…#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>
Every state carries a label now, one at a time, worded as the status is. The waiver label reads bot-review-skipped. Anyone with write access can post /skip-bots to proceed without the bots that have not reported for the current head; the report names who did. This caller wakes on that comment. dashpay/stale_prs_are_bad#40 landed as ae7c249182b8ea29eb488d604d7ade9ed9af9e2c. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every governed pull request description now ends with the review checklist: every requirement, checked when met, the first unchecked line being the state. When the move changes -- to the author, to a reviewer, to merge -- one comment says so. Labels say whose move it is: waiting-bots, waiting-self-review, ready-for-human, too-many-open-prs. dashpay/stale_prs_are_bad#46 landed as 14572d41fab87ded04a8162a9934ed0f29972bc3. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…1044) dashpay/stale_prs_are_bad#47 landed as 1bc9f141f0d697a2cdcb8d7f020779e13997fedb. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
dashpay/stale_prs_are_bad#48 landed as dab95334e8c430313e6e20c24519f4b74ce52872. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
dashpay/stale_prs_are_bad#49 landed as ef64a3cc4b3d64953630dc029c38fc9d8d278f38. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…drop the `secp256k1` context (#1042) * build: bump `rand` to 0.9, `getrandom` to 0.4 * refactor: parse secp256k1 keys from byte arrays * build: upgrade `secp256k1` to 0.31 * build!: upgrade `secp256k1` to 0.33 * refactor(dash)!: drop the secp256k1 context * refactor(key-wallet)!: drop the secp256k1 context * refactor(key-wallet-ffi): drop the secp256k1 context
… and public keys off the curve (#1038) * refactor(key-wallet): isolate HMAC function used for key derivation * test(key-wallet): pin that the reduction is modulo the group order * test(key-wallet): pin paths that explicitly reduce their scalar * test(key-wallet): pin refusal of pk off the curve, zero scalar
Bumps the actions group with 5 updates: | Package | From | To | | --- | --- | --- | | [actions/checkout](https://github.com/actions/checkout) | `6.1.0` | `7.0.1` | | [actions/setup-python](https://github.com/actions/setup-python) | `6.3.0` | `7.1.0` | | [actions/cache](https://github.com/actions/cache) | `5.1.0` | `6.1.0` | | [codecov/codecov-action](https://github.com/codecov/codecov-action) | `6.0.2` | `7.1.1` | | [peter-evans/create-pull-request](https://github.com/peter-evans/create-pull-request) | `7.0.11` | `8.1.1` | Updates `actions/checkout` from 6.1.0 to 7.0.1 - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](actions/checkout@v6.1.0...v7.0.1) Updates `actions/setup-python` from 6.3.0 to 7.1.0 - [Release notes](https://github.com/actions/setup-python/releases) - [Commits](actions/setup-python@v6.3.0...v7.1.0) Updates `actions/cache` from 5.1.0 to 6.1.0 - [Release notes](https://github.com/actions/cache/releases) - [Changelog](https://github.com/actions/cache/blob/main/RELEASES.md) - [Commits](actions/cache@v5.1.0...v6.1.0) Updates `codecov/codecov-action` from 6.0.2 to 7.1.1 - [Release notes](https://github.com/codecov/codecov-action/releases) - [Changelog](https://github.com/codecov/codecov-action/blob/main/CHANGELOG.md) - [Commits](codecov/codecov-action@v6.0.2...v7.1.1) Updates `peter-evans/create-pull-request` from 7.0.11 to 8.1.1 - [Release notes](https://github.com/peter-evans/create-pull-request/releases) - [Commits](peter-evans/create-pull-request@v7.0.11...v8.1.1) --- updated-dependencies: - dependency-name: actions/checkout dependency-version: 7.0.1 dependency-type: direct:production update-type: version-update:semver-major dependency-group: actions - dependency-name: actions/setup-python dependency-version: 7.1.0 dependency-type: direct:production update-type: version-update:semver-major dependency-group: actions - dependency-name: actions/cache dependency-version: 6.1.0 dependency-type: direct:production update-type: version-update:semver-major dependency-group: actions - dependency-name: codecov/codecov-action dependency-version: 7.1.1 dependency-type: direct:production update-type: version-update:semver-major dependency-group: actions - dependency-name: peter-evans/create-pull-request dependency-version: 8.1.1 dependency-type: direct:production update-type: version-update:semver-major dependency-group: actions ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* chore(dash-spv)!: make the CLI and logging dependencies optional A library consumer had to build clap, tracing-subscriber, tracing-appender and chrono, and tokio's full feature set. clap now sits behind a `cli` feature, which the binaries require, and the logging module behind `logging`, which the examples require. Neither is a default feature. tokio only enables what the library uses. Also drops dependencies nothing used: `internals` from key-wallet, and `dash-spv` and `rustls-pki-types` from masternode-seeds-fetcher. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * chore: define the shared dependencies in the workspace Dependencies used by several crates had their versions repeated in each manifest, and two had drifted. They now live in `[workspace.dependencies]`, each crate keeping the features it needs. criterion moves to 0.8.1 in dash and hashes, whose benches take `black_box` from std, and thiserror to 2 in dash-spv. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * chore(dash-network): build cbindgen only for the ffi feature cbindgen was an unconditional build dependency, though only the `ffi` feature uses it, so every crate on top of dash-network built it and clap. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * chore: define the workspace crates in the workspace dependencies Each crate repeated the relative path of the crates it uses, and the `internals` rename. They now live in `[workspace.dependencies]`. key-wallet-manager and rpc-json drop `default-features = false` on key-wallet, whose only default is the `getrandom` they enable. The changed manifests are formatted with taplo. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * chore: drop the redundant cargo profile settings `panic = "unwind"` is the default, and `[profile.dev]` was left empty. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * chore: use the rust-version aware resolver With `Cargo.lock` out of git, every checkout resolves its dependencies. Resolver 3 prefers versions that build on the workspace's rust-version, so a dependency raising its MSRV doesn't break the MSRV job. No version changes today. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * chore(rpc-integration-test): replace lazy_static with std `NET` is a const, and the other two statics use `std::sync::LazyLock`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * chore: define the unexpected_cfgs lint in the workspace Each crate that had the lint set its own level and cfgs, and most had none. All of them now inherit one `deny` with the same cfgs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * chore: drop more dependencies that aren't needed tokio-test was only enabling tokio's test-util, which dash-spv's tests now ask for themselves. dashcore's derives come through bincode, so bincode_derive goes. clippy takes the MSRV from `rust-version`, which leaves nothing in the root clippy.toml. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Kevin Rombach <35775977+xdustinface@users.noreply.github.com>
* test(dash): pin the roots of a coinbase payload of a newer version The transaction decoder keeps a coinbase payload raw when its layout is newer than this crate decodes. That is the case for a later CbTx version that appends fields. `verify_coinbase_roots` refuses every coinbase whose payload is not typed, so the first block with such a layout would get every masternode list refused, on every network that activates it. `Transaction::with_newer_coinbase_payload` writes a coinbase's payload the way such a version would: the version 4 fields under version 5, plus a field of its own. - The roots of the result must be checked like those of a typed payload, and a payload off either root must be refused. - A raw payload of a version this crate decodes is refused. So is a raw payload of another transaction type. - A raw payload cut short inside its quorum root is refused, even against a list without quorums, whose root is all zeros. `verify_coinbase_roots_of_a_raw_payload` "of a newer version" fails on dev with `CoinbaseCommitmentMismatch`. The next commit fixes it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(dash): check the roots of a coinbase payload of a newer version The transaction decoder keeps a coinbase payload raw when its typed decode leaves bytes over or does not re-encode to the same bytes. A later CbTx version that appends fields does exactly that. `MasternodeList::verify_coinbase_roots` refused every coinbase whose payload was not typed. So the first block with a newer layout would have got every masternode list refused, and masternode sync would have stopped until a release of this library learned the layout. `CoinbasePayload::from_coinbase_tx` returns the payload of a coinbase as far as this crate knows its layout: - A typed payload is returned as it is. - A raw coinbase payload of a version newer than 4, the newest this crate decodes, is decoded from the front as version 4, and its newer fields are left out. Dash Core only appends fields in a new version (`CCbTx::SERIALIZE_METHODS`), and it refuses versions it does not know (`bad-cbtx-version`). So a new layout arrives with a Core release that this assumption can be checked against. - A raw payload of a known version gives `None`. Dash Core never produces one (`bad-cbtx-payload`), and reading it could skip the quorum root of a raw version 1. `verify_coinbase_roots` now reads its roots through it. The coinbase bytes are authenticated by the diff's merkle proof against the stored header before the roots are checked, and the roots still have to equal the ones recomputed from the list. Test would have caught this in CI: `verify_coinbase_roots_of_a_raw_payload` "of a newer version". ✖ before the fix (`CoinbaseCommitmentMismatch`), ✔ after. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(dash-spv): pin the ChainLock of a coinbase payload of a newer version The engine records the ChainLock a list's coinbase carries for member selection, through `valid_coinbase_chain_lock`, which reads only a typed payload. A coinbase payload of a version newer than this library decodes stays raw and yields no ChainLock. The non-rotating quorum check then derives its modifier from the block hash and selects the wrong members. So from the first block with such a layout, every new non-rotating quorum, the ChainLock quorums among them, would be marked invalid, even once its list is accepted. Dash Core only appends fields in a new version, so the version 3 ChainLock fields of such a payload are where they were. This test fails on top of the previous commit: it gets `None` where the typed coinbase gives the signature. The next commit fixes it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(dash-spv): read the ChainLock of a coinbase payload of a newer version `valid_coinbase_chain_lock` read only a typed coinbase payload. A payload of a version newer than this library decodes stays raw, so no ChainLock was recorded for its list. The non-rotating quorum check then derived the modifier from the block hash, selected the wrong members, and marked every new non-rotating quorum invalid, the ChainLock quorums among them. That would have happened from the first block with such a layout, even though the previous commit lets its list in. The ChainLock is now read through `CoinbasePayload::from_coinbase_tx`. Dash Core only appends fields in a new version, so the version 3 fields of a newer payload are read where they were. A misread signature would only select wrong members, so the quorum would fail its check, as it does today. It cannot add a quorum the coinbase does not commit to. Test would have caught this in CI: `a_newer_coinbase_payload_carries_its_chain_lock`. ✖ before the fix (`None`), ✔ after. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(dash): pin what `from_coinbase_tx` reads of a newer coinbase payload - A raw coinbase payload of version 5 or 0xffff gives the fields up to version 4, under its own version. - A raw payload of versions 0 to 4 gives nothing, and so does every cut of it short of those fields. - The roots test of a newer payload cut short now cuts before the quorum root, and checks that nothing is read, so a reader that filled missing bytes with zeros would fail it. - `with_newer_coinbase_payload` asserts the version 3 payload it needs. - The typed roots test shares the fixture setup. - The docs say when the decoder keeps a payload raw. They also say that the result of `from_coinbase_tx` does not encode to the payload's bytes, and that a later version changing a known field's meaning would need this reader to change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…1305) * ci: drop unused stable toolchain installs `rust-toolchain.toml` takes precedence over the toolchain these steps installed, so CI already built with the pinned version. `llvm-tools` moves to `rust-toolchain.toml` so coverage gets it too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * ci: read the MSRV from `Cargo.toml` `just msrv` installs the toolchain named by `rust-version`, so the MSRV job no longer repeats the version. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs: point MSRV mentions to `Cargo.toml` The README keeps the only hand-written copy, so bumping the MSRV no longer means updating seven files. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(dash-spv-bench): drop the MSRV fallback for the Rust image The image should use the pinned toolchain, and `rust-toolchain.toml` always exists, so the `1.89` fallback was wrong and unreachable. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * chore: remove unused `rustfmt.toml` rustfmt reads `.rustfmt.toml` when both exist, so this file never applied. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…ess (#1295) * fix(key-wallet): keep a funds account's output out of the other funds accounts A host can leave an outpoint in a funds account that does not own its address, for example a restore that loads a CoinJoin output into BIP44. When the owning account then records the transaction (a rescan), both accounts hold the outpoint and the wallet balance, a sum of the account balances, counts it twice. After each account records or confirms a transaction, take every output that pays one of its own addresses and that it holds out of the wallet's other funding accounts (BIP44, BIP32, CoinJoin, DashPay receiving). The removed copy's InstantSend-lock, trusted and locked flags carry over to the owner's copy. A DashPay external account holds a contact's coins and takes nothing from the wallet's own accounts. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(key-wallet): also take an output held as spent before funded out of other accounts When a block spending an output was seen before the transaction funding it, the owning account keeps the output as spent before funded, not in its UTXOs. A copy another funds account held then stayed there, counted and spendable. Treat such an output as held by its owner too; it is removed from the other accounts and credited nowhere. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * feat(key-wallet): restore UTXOs into the account that owns their address A host restoring UTXOs had to pick the account for each one itself. One whose address the stored pools no longer reach, such as a CoinJoin address first derived during a sync, could end up in another account, and the next rescan then credited it to its owner as well, so the wallet balance counted it twice. Add ManagedWalletInfo::restore_utxos. It places each UTXO in the funds account whose pools hold its script, extending the pools to their gap limit past the addresses used, the way a rescan finds them, and returns the UTXOs no account reaches instead of guessing. The wallet's own accounts are tried before DashPay contact accounts, and a copy another funds account already holds of a restored outpoint is taken out. This replaces the earlier pass that removed such a copy once the owning account recorded the transaction. Refs #1294 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(key-wallet): rematch restored UTXOs each pass and keep spent and lock state restore_utxos now matches every UTXO again on each pass after the pools grew, so an account tried earlier claims a UTXO once its pool reaches it, and marks the address used in every account holding it, as a rescan does. The assignment is applied once, after the pools stop growing. A restored UTXO keeps the InstantSend lock and trust of any copy it replaces or takes out of another account, and their coin lock replaces its own, as reprocessing a transaction keeps them. An outpoint the wallet already saw spent is not credited or returned: one spent in a processed block is held as spent before funded, as processing its funding transaction does, and one the owning account recorded spending is dropped. A spent-output claim still does not hold one back. The rustdoc also says the call belongs before sync resumes, since blocks already processed are not scanned again for newly derived addresses. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(key-wallet): cover a lock held only by the copy restore_utxos takes out restore_utxos takes the coin lock of the copies it replaces. Add the mixed case CodeRabbit suggested in its review of #1295: the CoinJoin copy is unlocked and the misplaced BIP44 copy is locked, so the restored UTXO stays locked. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(key-wallet): share the per-output credit decision of update_utxos and restore_utxos Move how a funds account takes in an output paying one of its addresses into ManagedCoreFundsAccount::take_in_output, which update_utxos and restore_utxos both call. A restored coin the owning account already recorded spending is now held under that spend, as update_utxos holds it, so a fork dropping the spend gives it back instead of losing it. Crediting an output also takes it out of claim_guarded_outputs, so a claimed coin restored after its funding is no longer in both. Document that a UTXO restore_utxos returns is credited only if the host rewinds synced_height below its height, that the host marks its stored used addresses first, and the call order with restore_spent_outpoints, whose guard description now says restore_utxos does not consult claims. Addresses xdustinface's review of #1295. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Kevin Rombach <35775977+xdustinface@users.noreply.github.com>
bip324 0.11.1's async `handshake` also returns the session id, so a fresh resolve of `bip324 = "0.11"` no longer compiles `dash-spv` (peer.rs:407, E0308). Destructure the fourth field and require 0.11.1. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Weekly with a 7 day cooldown, matching the Actions entry. Without a committed `Cargo.lock`, Dependabot only raises a requirement that no longer allows the newest release, so it proposes Cargo-incompatible bumps and stays quiet on compatible ones. Each gets its own PR because those bumps often need code changes.
Updates the requirements on jsonrpc to permit the latest version. --- updated-dependencies: - dependency-name: jsonrpc dependency-version: 0.21.0 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Updates the requirements on [base64](https://github.com/marshallpierce/rust-base64) to permit the latest version. - [Changelog](https://github.com/marshallpierce/rust-base64/blob/master/RELEASE-NOTES.md) - [Commits](marshallpierce/rust-base64@v0.22.0...v0.23.1) --- updated-dependencies: - dependency-name: base64 dependency-version: 0.23.1 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Dash Core computes each short ID from `GetInstanceHash()`, the hash of the full serialization (src/blockencodings.cpp:34). For an asset unlock with payload version 2 or higher that differs from the txid, which leaves out the quorum signing info, so `HeaderAndShortIds::from_block` gave such a transaction a short ID Core's peers can't match against their mempool. Use `instance_hash()`, which equals the txid for every other transaction. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
The optional `bitcoinconsensus` feature wrapped Bitcoin Core's libbitcoinconsensus, which checks scripts by Bitcoin's rules and parses transactions in Bitcoin's format. It does not work for Dash. A correctly signed special transaction fails with `ERR_TX_SIZE_MISMATCH`, because the extra payload after the locktime is not part of Bitcoin's format and Dash also signs it. A valid DIP-0020 script such as one using `OP_CAT` fails with `ERR_SCRIPT`, because those opcodes are disabled in Bitcoin and active on Dash mainnet since height 1516032. Bitcoin Core deprecated the library in v27 and removed it in v28, so `0.106.0+26.0` is the final crate release. Porting to it would also mean new flags, a new `spent_outputs` argument and dropping the `Ord`/`Hash` derives on `script::Error`. No downstream crate enables the feature: Platform, dash-evo-tool and dash-spv-wallet do not use it. This removes `Script::verify`, `Script::verify_with_flags`, `Transaction::verify`, `Transaction::verify_with_flags`, the `dashcore::bitcoinconsensus` re-export and the `script::Error` variants `BitcoinConsensus`, `UnknownSpentOutput` and `Serialization`, which only that API used.
Updates the requirements on [pprof](https://github.com/tikv/pprof-rs) to permit the latest version. - [Changelog](https://github.com/tikv/pprof-rs/blob/master/CHANGELOG.md) - [Commits](https://github.com/tikv/pprof-rs/commits) --- updated-dependencies: - dependency-name: pprof dependency-version: 0.15.0 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* fix(dash-spv): normalize user agent in `build_version` `build_version` sent the configured user agent unchanged. Dash Core reads it with `MAX_SUBVERSION_LENGTH` (256) and throws on a longer one, so our `version` was dropped and the handshake timed out. The agent is now wrapped in `/` delimiters and capped at 256 bytes on a UTF-8 char boundary, which also covers callers that set `ClientConfig::user_agent` directly. Part of #1242 * fix(dash-spv): use `dash-spv` name in devnet user agent `DevnetConfig::user_agent` still advertised `rust-dash-spv` while the default user agent is `/dash-spv:VER/`. Fixes #1242 * fix(dash-spv): reject a devnet name too long for the user agent On devnet, Dash Core bans an outbound peer whose user agent does not contain `devnet.<name>`. If a long devnet name pushed `DevnetConfig::user_agent` past 256 bytes, `normalize_user_agent` would cut into that suffix. `DevnetConfig::validate` now refuses such a name up front, like dashd refuses to start when its own user agent exceeds `MAX_SUBVERSION_LENGTH`. `MAX_USER_AGENT_LEN` is shared with the network module instead of duplicated.
Delete the `key-wallet-ffi` crate and remove the optional `ffi` feature, `ffi` module, `build.rs`, and `cbindgen.toml` from `dash-network`. Its remaining users, Platform and dashwallet-ios, move to Platform's own wallet FFI in `rs-platform-wallet-ffi`. Keeping it around for potential future use just eats time and tokens to keep it in sync with the Rust API, make sure it's tested and working, and review all the changes happening to it. I think it makes more sense to just generate a new FFI layer if we ever come up with a use case or if someone actually requests it. If someone is actually using it they will probably look into here and are welcome to open an issue, so we can consider reverting the PR. But at the moment all the effort we put into this is better put into something else.
* test(dash): pin that a partial merkle tree spans a full Dash block A Dash block holds up to 33,333 transactions, its 2 MB size limit over the 60 bytes of the smallest transaction. Dash Core's `CPartialMerkleTree::ExtractMatches` allows that many (`nTransactions > MaxBlockSize() / 60`). `extract_matches` still uses Bitcoin's BIP141 bound, `MAX_BLOCK_WEIGHT / MIN_TRANSACTION_WEIGHT` = 16,666, so it refuses the proof of any larger Dash block. That includes the coinbase proof of a masternode list diff at that block, which `coinbase_merkle_root` reports as `InvalidCoinbaseProof`, and that refusal bans the peer. This test fails on dev: 33,333 transactions are refused with `TooManyTransactions`. The next commit fixes it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(dash): bound partial merkle trees by Dash's block size `PartialMerkleTree::extract_matches` refused a tree of more than `MAX_BLOCK_WEIGHT / MIN_TRANSACTION_WEIGHT` = 16,666 transactions. That is Bitcoin's BIP141 bound. A Dash block holds up to 2 MB over the 60 bytes of the smallest transaction, so up to 33,333 transactions, and Dash Core's `CPartialMerkleTree::ExtractMatches` allows exactly that (`MaxBlockSize() / 60`, src/merkleblock.cpp). So a valid Dash block above 16,666 transactions could not be proven. A masternode list diff at that block failed its coinbase proof with `InvalidCoinbaseProof`. That refusal bans the peer, so every honest peer asked for that list was banned in turn, and masternode sync stopped. Any miner can build such a block. Raising the bound adds no work. The traversal is bounded by the hashes and flag bits the proof carries, and the depth of the tree is log2 of the count. The BIP141 constants stay public in `blockdata::constants`. Nothing in the crate uses them any more. Test would have caught this in CI: `a_partial_merkle_tree_spans_as_many_transactions_as_a_dash_block_holds`. ✖ before the fix (33,333 transactions refused with `TooManyTransactions`), ✔ after. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(dash)!: replace the BIP141 block weight constants with Dash's block size `MAX_BLOCK_WEIGHT` and `MIN_TRANSACTION_WEIGHT` described Bitcoin's BIP141 limit, the bound that refused large Dash blocks' partial merkle trees. Nothing in the workspace uses them any more, while Dash's block size sat as a bare `2_000_000` in `merkle_tree/block.rs`. `blockdata::constants::MAX_BLOCK_SIZE` is now Dash Core's `MAX_DIP0001_BLOCK_SIZE`, 2,000,000 bytes, and the partial merkle tree bound is derived from it. The bound stays at 33,333 transactions. BREAKING CHANGE: `blockdata::constants::MAX_BLOCK_WEIGHT` and `MIN_TRANSACTION_WEIGHT` are removed. Use `MAX_BLOCK_SIZE`, Dash's limit in bytes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
* build(hashes): move `dashcore_hashes` to `bitcoin_hashes` 1.x Depend on `bitcoin_hashes` from the same `rust-bitcoin` git revision as `bitcoin-crypto`, `bitcoin-units` and `base-sdk`, so the hash types unify across them. The 1.x `Hash` trait only converts to and from byte arrays. `hash_newtype!` implements it like upstream's macro and adds the conversions as `const` inherent functions. Newtypes keep `from_slice` and `all_zeros` as inherent functions, since Dash uses zero hashes as protocol values, but no longer get `hash`, `engine` or `from_engine`. The X11 hash gets the inherent surface of the upstream hash types plus `from_slice` and `all_zeros`, and its engine implements `finalize`. 1.x dropped `FromSliceError`, so this crate defines its own with the same shape and message, still also exported as `Error`. `sha256t_hash_newtype!` is removed. Nothing used it and upstream deprecated its counterpart in favour of implementing `sha256t::Tag` by hand. `sha256t` is now upstream's module. * refactor(crypto): adopt `bitcoin_hashes` 1.x hashing `PubkeyHash` no longer has `hash`, so hash the key through `hash160::Hash` and wrap the result. * refactor(dash): adopt `bitcoin_hashes` 1.x hashing Newtypes hash through their inner hash type and its engine: `T::from_raw_hash(sha256d::Hash::hash(..))`, or an engine from `sha256d::Hash::engine()` finished with `finalize`. Upstream hash types lost `all_zeros`, `from_slice` and indexing, so those call sites go through the byte array instead. `BlockHash` wraps X11 or SHA256d depending on `core-block-hash-use-x11`, so a crate private `BlockHashEngine` alias picks the matching engine for `Header::block_hash`. `merkle_tree::calculate_root` and `calculate_root_inline` now require `T: From<sha256d::Hash>` and always combine nodes with double SHA256. 1.x has no engine on the `Hash` trait, and every Dash merkle tree is SHA256d. `from_hex` on the hash newtypes parses through the inner hash's `FromStr`, which now reports the same `hex-conservative` 1.x error. * refactor(key-wallet): adopt `bitcoin_hashes` 1.x hashing `HmacEngine` is now parameterized by the engine type and finished with `finalize`. Upstream hash types no longer index into bytes, so fingerprints and chain codes read through `as_byte_array`. Tests hash newtypes through their inner hash and build upstream zero hashes with `from_byte_array`. * refactor(dash-spv): adopt `bitcoin_hashes` 1.x hashing Hash newtypes are built from their inner hash instead of the removed `hash`, and upstream zero hashes use `from_byte_array`. Imports of the `Hash` trait that only served the newtype functions are dropped. * refactor(rpc): adopt `bitcoin_hashes` 1.x hashing Drop the `Hash` trait imports the 1.x inherent functions no longer need and hash `Txid` through `sha256d::Hash`. * refactor(fuzz): adopt `bitcoin_hashes` 1.x hashing Finish engines with `finalize` and compare the hashes directly, since upstream hash types no longer index into bytes.
* fix(dash-spv): announce the peers `stop` disconnects `PeerNetworkManager::stop` emptied the connected set without sending any event, so subscribers that mirror peers from `PeerConnected`, `PeerDisconnected` and `PeersUpdated` kept peers that were gone. Departures are now announced by whoever removes a connection from the set, while holding its lock: `stop`, the timeout monitor when it kicks a peer, and the pump when a peer drops. The pump stays silent for a connection that already left the set, so the late disconnect of a peer that `stop` or the timeout monitor removed, or of a swapped out or probed peer, is not announced a second time. * fix(dash-spv): deliver network events until `stop` ends `stop_locked` ended the event monitors before it stopped the network, so the handlers never received the departures announced by `PeerNetworkManager::stop`. This held for a normal `stop` and for the restart after a fork. The monitors that deliver events to the handlers now have their own token, owned by `SyncLoop`. `stop_locked` still stops the sync loop and the coordinator first, then stops the network, and only then ends the monitors. A monitor told to stop first delivers the events already queued. * fix(dash-spv): announce peer changes under a `PeerSet` The rule that every change to the connected set is announced while its lock is held was only a comment, and the supervisor broke it. A handshake that finished just before `stop` could join the set after `stop` drained it and send a `PeerConnected` nobody would ever undo. The swap sent its events after unlocking, and `announce_update` read the count, unlocked and then sent it, so a stale `PeersUpdated` could land after the zero from `stop`. `NetworkState::connected` is now a crate-private `PeerSet` that owns the peers, the event sender and the best tip. Its mutating methods emit their own events under the lock, and everything else only reads through a guard: - `admit` adds a handshaken peer or swaps out a slow one. It refuses the peer once the caller's session is cancelled, which also turns away a late insert from the supervisor of an earlier session. - `remove` and `clear` announce only connections still in the set, keeping the same-address rule. - Every count event comes from the set, so `announce_update` is gone. * chore(dash-spv): warn on stop lag, tighten fork test A monitor draining its queue on shutdown now warns with the number of events it missed when it lagged, instead of skipping them silently. The fork test reset only the emptied flag before the reorg, so `connected_count` was already `Some(1)` and the wait for the peer to return could pass on its first check. It now forgets the count too.
…iff (#1202) * feat(dash-spv): expose the coinbase payload of the newest validated diff Platform needs the coinbase payload version of the tip to tell whether V24 is active, and fetched the block again over DAPI to get it. The masternode list engine already proves each diff's coinbase in its block, so it now keeps the payload of the highest one, and `DashSpvClient::latest_coinbase` returns it. Closes #1198 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(dash-spv): drop the latest coinbase above a followed fork The engine now drops lists built above a fork, but kept the coinbase of a dropped block as the latest one. It goes too when it is above the fork. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(dash-spv): put the latest coinbase back when a QRInfo feed is refused A QRInfo feed that fails or is dropped part way leaves the lists and rotated cycles as it found them, but kept the coinbase of a diff it had applied. The rollback now restores it too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * feat(dash-spv): keep the latest coinbase in the masternode engine snapshot The snapshot restored the lists at start-up but not the coinbase, so `latest_coinbase` was `None` until the next validated diff. It is written after the rotated cycles, and the snapshot version goes to 2: a version 1 snapshot is removed and the engine syncs again. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(dash-spv): cover the latest coinbase rollback and height rule Assert that a refused QRInfo puts the previous coinbase back, and that a lower diff arriving after a higher one leaves the newer coinbase. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(dash-spv): cover the latest coinbase in the engine snapshot Round-trip a version 4 coinbase through the masternode engine snapshot. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(dash-spv): cover DashSpvClient::latest_coinbase against dashd Assert it is None with masternode sync off and on a fresh engine, and that it returns the engine's coinbase after a masternode list sync. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs(dash-spv): note when latest_coinbase is missing or stale * test(dash-spv): drop the unused Hash import * fix(dash-spv): observe coinbases of a newer payload version --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…ow (#1308) * test(dash): pin the quorum root of commitments of types this crate does not know `LLMQType` decodes every type byte it has no variant for as `LlmqtypeUnknown`, and encodes that back as 0. A commitment's entry hash is the hash of its re-encoding, so for a commitment of a type Dash Core adds later it is not the hash Core puts in the coinbase's `merkleRootQuorums`. Once a network mines such a commitment, `verify_coinbase_roots` would refuse every masternode list, and masternode sync would stop until a release of this library learned the type. - Every type byte has to encode back to itself. - A list holding commitments of types 200 and 201 has to match a quorum root over Dash Core's hashes of them, and must be refused when one of them is served under type 202. `every_type_byte_encodes_back_to_itself` fails on dev: byte 7 encodes as 0. So does `verify_coinbase_roots_of_quorums_of_unknown_types` "as committed", with `CoinbaseCommitmentMismatch`. The next commit fixes both. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(dash)!: keep the type byte of a quorum type this crate does not know `LLMQType` decoded every type byte it had no variant for as `LlmqtypeUnknown`, and encoded that back as 0. So the entry hash of a commitment of a type Dash Core adds later was not the hash Core puts in the coinbase's `merkleRootQuorums`. Once a network mined such a commitment, `verify_coinbase_roots` would have refused every masternode list, and masternode sync would have stopped for every client until a release of this library learned the type. `LLMQType` gains `Unknown(u8)`, which keeps any byte without a variant (byte 0 still decodes as `LlmqtypeUnknown`). Every byte now encodes back to itself. That keeps the entry and commitment hashes of such a commitment Dash Core's, however many new types a list holds. Commitments of two unknown types also no longer share a key in a list's quorum map. `Unknown(_)` takes devnet parameters, as `LlmqtypeUnknown` does. A quorum commitment transaction of an unknown type now decodes typed rather than raw. The new variant comes last. Bincode writes a variant's position, so every existing value keeps its encoding, and stored masternode engine snapshots still load. Derived ordering is by discriminant, so existing types keep their order and `Unknown(_)` sorts after all of them. The alternative, trying each unknown byte until the quorum root matched, was rejected. Its work grows with commitments a peer can make about 250 KB each. Its refusals of a mismatch are not banned, so a peer could send such diffs again and again. And it fails with two new types at once. Test would have caught this in CI: `every_type_byte_encodes_back_to_itself` and `verify_coinbase_roots_of_quorums_of_unknown_types` "as committed". ✖ before the fix (byte 7 encoded as 0, `CoinbaseCommitmentMismatch`), ✔ after. BREAKING CHANGE: `LLMQType` gains the `Unknown(u8)` variant and becomes `#[repr(u8)]`. Exhaustive matches need an arm for it, `as` casts of an `LLMQType` no longer compile (use `index()` or `u8::from`), and `Unknown(b)` serializes with serde as a newtype variant. serde_json refuses a newtype variant as a map key, so a `MasternodeList` holding a quorum of an unknown type no longer serializes to JSON. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(dash): pin that a quorum kept under an unnamed type byte is its type's A list built while this crate has no name for a quorum type keeps that type's quorums under `LLMQType::Unknown(b)`. The dash-spv engine stores lists in its snapshot. Once a later release names the type, its diffs delete those quorums under the name. With the derived comparisons, `Unknown(b)` is not that name, so the deletion missed. The quorum would stay in the list forever, every later list would stop matching its coinbase, and masternode sync would stop until the snapshot was dropped. `a_quorum_kept_under_an_unnamed_type_byte_is_deleted_by_its_name` fails on top of the previous commit: the quorum is still there. The next commit fixes it. `bincode_writes_every_type_at_its_position` passes already. It pins that every named type keeps the bincode position stored snapshots hold, with `Unknown` after them all. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(dash): compare quorum types by their byte `LLMQType` derived its comparisons, so `Unknown(b)` and the variant that names `b` were different keys. A list stored while a type had no name keeps that type's quorums under `Unknown(b)`. Once a release named the type, diffs naming it could not delete those quorums, and the list would stop matching its coinbase for good. A quorum type is now its byte. `PartialEq`, `Ord` and `Hash` go by `index()`, so `Unknown(b)` is the variant that names `b`, in a map as anywhere else. The named types keep their order, and an unnamed byte sorts at its value. `params()` and `Display` also go by the byte: - the devnet type takes its runtime parameters; - a type Dash Core defines takes its fixed ones from dash-params, even before this enum names it; - any other type takes devnet parameters, as before. This also removes the `expect` that a named type always has parameters. Bincode and serde encodings are unchanged. So `Unknown(b)` built for a named byte, which serde, bincode or a literal can do, is no longer a second key for the same type. Test would have caught this in CI: `a_quorum_kept_under_an_unnamed_type_byte_is_deleted_by_its_name`. ✖ before the fix (the quorum stayed), ✔ after. `a_type_is_its_byte` covers every byte. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(dash): pin that a quorum list holding a commitment twice is refused A list keys each commitment by the type and quorum hash the commitment hashes. So a list built from diffs never holds one twice, but a list edited through its public maps, or restored from damaged storage, can. When the doubled leaf is the last one of an odd level, the merkle root of the sorted commitment hashes stays as it was (the shape of CVE-2012-2459). Dash Core refuses such a root (`mutated-calc-cbtx-quorummerkleroot`). This test fails without that check: the list holding the commitment twice matches the coinbase. The next commit adds the check. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(dash): refuse a quorum list holding a commitment twice `verify_coinbase_roots` compared the merkle root of a list's sorted commitment hashes with the coinbase's `merkleRootQuorums`. Doubling the last leaf of an odd level does not change that root. So a list holding a commitment twice, under a second quorum hash, matched the coinbase of the list without it. A list whose sorted commitment hashes hold a duplicate is now refused. That is stricter than Dash Core, which flags a root as mutated only when equal hashes are paired in its tree (`mutated-calc-cbtx-quorummerkleroot`). But any duplicate is enough to keep a root while changing the list, and an honest list never holds one: a commitment's map key is its type and quorum hash, and both are in the bytes it hashes. Test would have caught this in CI: `a_quorum_list_holding_a_commitment_twice_is_refused`. ✖ before the fix (the list holding the commitment twice matched), ✔ after. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(dash): keep the type byte test clear of the devnet override `a_type_is_its_byte` compared the parameters of `Unknown(b)` with those of the named type. For a byte that takes devnet parameters, both reads go through the devnet override, which another test in the same binary sets. If it was set between the two reads, the test would fail by chance. Only fixed parameters are compared now. The doc of `Unknown` now says that a value restored from storage stays `Unknown` after a release names its byte, so types are compared with `==` rather than matched by name. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(dash)!: keep an unnamed quorum type's byte in `LlmqtypeUnknown` `LlmqtypeUnknown` becomes `LlmqtypeUnknown(u8) = 0` and replaces the `Unknown(u8)` variant this branch added at the end. Dash Core gives byte 0 no meaning: no network defines it, and `LLMQ_NONE` is 0xff. So the two variants meant the same thing, and `From<u8>` needed a case for byte 0 to pick between them. The variant stays first, so every named type keeps its bincode position and stored engine snapshots and the bincode fixtures still decode. A type without a name writes position 0 and then its byte. No rule is left about where later variants go, and no implicit discriminant 108 is left for a later `= 108` variant to collide with. Equality, order, hash, `params()` and `Display` still go by the byte. Serde keeps every variant name; `LlmqtypeUnknown` becomes a newtype variant. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(dash): pin that the quorum modifier hashes the type as one byte Dash Core hashes `llmqParams.type` into the quorum modifier as the one byte of its `uint8_t` enum (`GetHashModifierFromWorkBlock`, src/llmq/utils.cpp, through the enum `Serialize` of serialize.h). `LLMQModifierType::build_llmq_hash` writes it as a CompactSize, which is three bytes from 253 on, `fd fd 00` for 253. Members of a quorum of such a type would be selected by another modifier than Core's. The test builds Core's bytes by hand for every type byte, in the block hash form and the signature form. It fails on this commit: assertion `left == right` failed: type byte 253, block hash form left: 0xe09e967b64864e329c7508aa684bc063591e113e5064a102f6223eb3f02b5329 right: 0x604054d4c569576e8366cc184ef1b9da27099875c4e693acdb38a7ca21ee7f8f The next commit fixes it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(dash): hash the quorum type into the modifier as one byte `LLMQModifierType::build_llmq_hash` wrote the quorum type as a CompactSize. Dash Core hashes `llmqParams.type` as the one byte of its `uint8_t` enum (`GetHashModifierFromWorkBlock`, src/llmq/utils.cpp). The two encodings agree up to byte 252, which covers every type this crate names, so no known type's modifier changes. From byte 253 on the CompactSize is three bytes, and members of such a type would have been selected by another modifier than Core's. The type now goes through its `Encodable`, which writes its byte. `the_modifier_hashes_the_type_as_one_byte` fails on the previous commit at type byte 253 and passes on this one: ✖ before the fix, ✔ after. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(dash): pin that a Platform type held as its byte is skipped A type is its byte, and `LlmqtypeUnknown(b)` for a named byte equals the named variant. `Network::should_skip_quorum_type` matches the Platform types by name, which a pattern does not see through, so `LlmqtypeUnknown(4)` on mainnet below Platform activation is not skipped while `Llmqtype100_67` is. Such a value comes from a stored list or a caller that builds the variant. The test fails on this commit: assertion failed: Network::Mainnet.should_skip_quorum_type(&LLMQType::LlmqtypeUnknown(4), The next commit fixes it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(dash): skip a Platform type held as its byte like its name `Network::should_skip_quorum_type` now matches on the named form of the type, `LLMQType::from(llmq_type.index())`, as `Display` does. A Platform type held as `LlmqtypeUnknown` with its byte is skipped before activation like the named variant, so the function agrees with the type's `==`. `a_platform_type_held_as_its_byte_is_skipped_before_activation` fails on the previous commit and passes on this one: ✖ before the fix, ✔ after. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(dash): pin each quorum type's bincode position and serde name The bincode test paired positions with the named types in declaration order, so a type declared in the middle would have moved every later position without the test noticing. Each named type now carries its position, and the test also decodes each encoding back to its type. Stored masternode engine snapshots hold those positions. A new test pins the serde name of each named type, and `LlmqtypeUnknown` as a newtype variant holding its byte. Test-only change: no behaviour changes, so there is no red run. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
`PeersUpdated` carried only a count, so a subscriber that wants the connected peers had to rebuild them from `PeerConnected` and `PeerDisconnected`. One missed event left its copy wrong for good. The event now carries `addresses`, the full list of connected peers, built by `PeerSet` under its lock, so a subscriber can keep the latest list and never drift. `connected_count` is removed, since it is always `addresses.len()`. Code that matched on it uses `addresses.is_empty()` or `addresses.len()` instead. `NetworkEvent` now derives `PartialEq` and `Eq`. The unit and dashd tests check every `PeersUpdated` list against the connect and disconnect events before it, and that the last list after `stop` or a fork restart is empty.
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
 |
|
🔴 Review failed: Sorry, the diff exceeded the maximum number of files (300). Consider using 'List pull requests files' API or locally cloning the repository instead.: {"resource":"PullRequest","field":"diff","code":"too_large"} - https://docs.github.com/rest/pulls/pulls#list-pull-requests-files |
|
🔴 Review failed: Sorry, the diff exceeded the maximum number of files (300). Consider using 'List pull requests files' API or locally cloning the repository instead.: {"resource":"PullRequest","field":"diff","code":"too_large"} - https://docs.github.com/rest/pulls/pulls#list-pull-requests-files Not reviewed yet · head
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1402 +/- ##
==========================================
+ Coverage 78.13% 85.17% +7.03%
==========================================
Files 330 268 -62
Lines 86826 68085 -18741
==========================================
- Hits 67838 57988 -9850
+ Misses 18988 10097 -8891
|
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.46.0frommaster. It isdevatc9aeac0d0(#1327), the revision Dash Platformv5.1-devpins since dashpay/platform#5729, plus the0.46.0changelog.masterholds thev0.45.0release, which was cut from theintegration/v5.0-core-v24branch and not fromdev. Every commit on it has an equivalent ondev, except #1204, which #1202 replaces. So the branch mergesmasterwith theoursstrategy: the merge commit recordsmasteras merged and keeps thedevtree, and this PR merges intomasterwithout conflicts. After the merge,masterholds exactlyc9aeac0d0plus the changelog.The changelog covers every PR on
devthat is not inv0.45.0, which reaches back to #989, where thev0.45.0integration branch started. Entries for code removed later in the same cycle (Taproot types, the FFI crates) are dropped.Merge with a merge commit, not a squash. The merge commit is what puts the
devhistory intomaster, so the next release merges without conflicts too. After the merge, tag the merge commitv0.46.0.