Repository navigation
feat(platform)!: support Core v24 masternode identities at protocol version 14 - #5227
Conversation
Core v24 leaves `service`, `platformP2PPort` and `platformHTTPPort` out of its masternode list RPC output unless started with `-deprecatedrpc=service`, and Drive and DAPI read those fields. Render `deprecatedrpc=service` unconditionally in the default section of dash.conf so it applies on every network. Core v23 accepts the option and ignores it. Rollout follows earlier template-only changes to dash.conf (adding and removing `deprecatedrpc=hpmn`): no migration. Every command re-renders the service configs when loading migrates the config format version, so an upgrade from 4.1.x or earlier picks the line up on the first dashmate command. Every 4.2.0 prerelease stamps the format version as 4.2.0 (the newest migration key), so an upgrade from a 4.2.0 prerelease to a later prerelease or to 4.2.0 itself runs no migration and does not re-render. Those nodes need `dashmate config render --config=<name>` and a Core restart before they run Core v24. The new test renders dash.conf for mainnet, testnet, devnet and local and checks the option sits before the first network section. Red to green: 4 failing before the template change, 4 passing after. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Pin rust-dashcore to 538522bc (dashpay/rust-dashcore#1093), whose rpc-json parses the masternode lists Dash Core v24 prints, and adapt drive-abci to it. Only rpc-json differs from the previous pin e4208c90. The re-exported `dpp::dashcore_rpc` (and `dash_sdk::dashcore_rpc`) masternode types change in a breaking way for Rust consumers; nothing nodes agree on changes. DMNState and DMNStateDiff rename the flat platform ports to the deprecated `legacy_platform_p2p_port` / `legacy_platform_http_port`, add the nested `addresses` and the `payouts` list, and make `owner_address`, `payout_address` and `collateral_address` optional, since a shared masternode has none of them. A Tor or I2P `service` reads as `[::]:0` and a `-1` platform port as absent, instead of failing the whole list. Adapt every reader and struct literal in drive-abci, behaviour-identical for every masternode list Core printed before v24 (all three addresses always present): - platform ports are read from the legacy fields exactly as before, with `#[allow(deprecated)]` on the single statement, pattern or field that touches one; `addresses` is not read; - create_owner_identity v0/v1 and transition_to_version_4 need a value from the now optional owner/payout address and fail with DashCoreBadResponseError when it is absent, instead of inventing one; what an added masternode without them gets is decided separately; - the stored MasternodeV0/MasternodeStateV0 format is unchanged: the addresses are stored as before (an absent one as zero bytes, which only a list the node could not process until now can produce) and read back as present; `payouts` and `addresses` are not stored and read back as `None`; - test fixtures wrap the same values in `Some`, drawing the same random numbers in the same order, so seeded strategy tests are unchanged. No behaviour change for pre-v24 lists, so no new test; the drive-abci tests, including the mainnet listdiff fixture test, pass unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Dash Core v24 lists masternodes every shipped Drive fails to parse, which
halts the chain: a shared masternode has no owner, payout or collateral
address, and an extended-address masternode has a `payouts` list instead
of `payoutAddress`. Now that rust-dashcore parses them, decide what an
added one gets. Its single payout address is the payout address, else
the only entry of a one-entry payout list (Core turns a payout address
into such a list when a masternode moves to extended addresses), else
none.
- No owner address (shared): no owner identity. Voter and operator
identities as for any masternode.
- Owner address and a single payout address: unchanged, TRANSFER key 0
and OWNER key 1, byte-identical to a legacy payout address.
- Owner address and several payouts: an owner identity with only the
OWNER key 1; key id 0 stays free.
- create_owner_identity v0 (protocol versions 1-3), whose identity holds
only the TRANSFER key: no owner identity without a single payout
address.
- transition_to_version_4 adds no OWNER key for a masternode without an
owner address, after the existing skip of masternodes without owner
identity keys.
- Diffs are unchanged: only `payout_address` rotates the TRANSFER key;
a `payouts` change is parsed but not acted on.
- Stored masternodes keep their format: an absent owner, payout or
collateral address is stored as zero bytes and now reads back as
absent; `payouts` is not stored.
- Drive logs at debug level, with the ProTx hash and the reason, each
added masternode that gets no owner identity or one with only the
OWNER key, and each masternode transition_to_version_4 gives no OWNER
key.
In-place change to shipped generations: create_owner_identity v0
(protocol versions 1-3) and v1 (4-14), update_masternode_identities v0
and perform_events_on_first_block_of_protocol_change v0
(transition_to_version_4, every version). Every input that takes a new
branch is an added masternode without an owner address or without a
single payout address, which only a Core v24 masternode list contains
and every shipped binary rejects at deserialization, so no committed
block was produced from one. Pre-v24 inputs are unchanged: Core prints
an owner and payout address for every earlier masternode and refuses a
null owner key, and a single payout gives the legacy identity byte for
byte. The stored-state conversion is not consensus state; nothing that
decides a block reads a stored payout or collateral address or the
payout list.
Tests, red to green:
- should_update_masternode_identities_from_a_core_v24_masternode_list
runs Core v24 `protx listdiff` JSON (shared, one payout, Tor primary
address with two payouts, Evo with `-1` ports, IPv6 Evo with two
payouts, then a legacy-to-extended-address diff and payout-list-only
diffs) through update_masternode_list with the mock Core RPC. Against
rust-dashcore e4208c90 it fails parsing:
CoreRpc(Json(Error("missing field `payoutAddress`", line: 74, column: 7)))
on the parent commit it fails with
DashCoreBadResponseError("masternode 27978dd8...0bef has no payout address")
and it passes here.
- create_owner_identity: shared (protocol versions 1 and latest), single
payout equals payout address byte for byte (1 and latest), several
payouts give OWNER key 1 only, several payouts on version 1 give no
owner identity. All four fail on the parent commit with
DashCoreBadResponseError and pass here.
- should_reload_absent_addresses_as_absent fails on the parent commit
(Some([0; 20]) read back) and passes here.
- should_add_no_owner_key_for_a_masternode_without_owner_address_on_transition_to_version_4
fails on the parent commit with DashCoreBadResponseError and passes
here.
- should_not_change_owner_identity_keys_on_a_payout_list_change and
should_reload_a_masternode_with_every_address_unchanged pin unchanged
behaviour and pass on both.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
github-actions
Bot
added
the
waiting-bots
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 59 minutes. View limit detailsLimit details: You’ve used the included review currently available. Review configuration: ⚙️ Run configuration
⛔ Files ignored due to path filters (2)
📒 Files selected for processing (29)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe pull request adapts masternode state handling to optional addresses and payout lists. It adds versioned owner identity creation and updates, and adds Core v24 list-diff fixtures and regression tests. ChangesCore v24 Masternode State and Identity Handling
Priority: ⬆️ High Estimated code review effort: 4 (Complex) | ~45 minutes Change: Feature Sequence Diagram(s)sequenceDiagram
participant CoreRPC
participant update_masternode_list
participant update_state_masternode_list
participant update_masternode_identities
participant Drive
CoreRPC->>update_masternode_list: return Core v24 list diff
update_masternode_list->>update_state_masternode_list: apply masternode and validator changes
update_masternode_list->>update_masternode_identities: apply identity changes
update_state_masternode_list->>Drive: persist updated platform state
update_masternode_identities->>Drive: apply identity operations
Suggested reviewers: Merge Risk: ⚪ Minimal · up to No actionable merge blocker is established by these changes; merge after normal checks complete. Security Architecture ReviewSecurity architecture risk: 🟡 Moderate · up to The change affects network consensus and withdrawal authority. The inspected payout controls preserve ownership and revoke obsolete transfer keys, but safe operation depends on activating protocol version 14 before Core v24 and confirming the allowed lifecycle of ownerless masternodes. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
✅ Final review complete — no blockers (commit bc2a201) · triage: critical |
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
⚠️ DEGRADED — Final validation — Phase 1 + Phase 2
⚠️ DEGRADED review. The primary review models were unavailable (gpt-6.1-solunavailable: 503 auth_unavailable: no auth available (providers=codex, model=gpt-6.1-sol; last upstream error: usage_limit_reached: T), so this review ran on stand-in models:gpt-5.6-luna→muse-spark-1.3-contributor,gpt-5.6-sol→muse-spark-1.3-contributor,gpt-5.6-terra→muse-spark-1.3-contributor,gpt-6-astra→muse-spark-1.3-contributor,gpt-6.1-sol→muse-spark-1.3-contributor. Both review phases and the independent verifiers still ran, but on weaker models, with Phase 1 capped athigheffort. Treat the verdict as provisional; a full-strength re-review will run on the next push once the primary models are back.
Core v24 compatibility fix is consensus-safe: new branches are reachable only by input old binaries reject at deserialization, with pre-v24 behavior byte-identical. Two in-scope polish items remain: pin the zero-byte sentinel collision with a test and fix the stale accessor doc.
🟡 1 suggestion(s) | 💬 1 nitpick(s)
1 finding(s) not shown inline (the lines are not part of this PR's diff)
💬 Nitpick: collateral_address accessor doc omits zero-bytes-absent sentinel
packages/rs-drive-abci/src/platform_types/masternode/v0/accessors.rs:14-15
MasternodeV0::collateral_address now uses zero bytes to mean absent for shared masternodes, documented on the struct field and on stored_address/loaded_address. The MasternodeAccessorsV0::collateral_address trait method returns the same raw [u8; 20] but its doc still promises 'The address where the collateral is stored', so a future caller reading only the trait cannot learn that all-zero means absent. Nothing calls the accessor today, so there is no behavior bug; the trait contract should carry the same caveat the struct field does.
/// The address where the collateral is stored; zero bytes when it has none, as for a
/// shared masternode.
fn collateral_address(&self) -> [u8; 20];
source: glm-5.3-flash (phase1-reviewer: general, architecture-layering, platform-versioning, rust-quality, security-auditor)
Review provenance
Source: reviewer 1: glm-5.3-flash (agent: phase1-reviewer, role: general); reviewer 2: glm-5.3-flash (agent: phase1-reviewer, role: architecture-layering); reviewer 3: glm-5.3-flash (agent: phase1-reviewer, role: platform-versioning); reviewer 4: glm-5.3-flash (agent: phase1-reviewer, role: rust-quality); reviewer 5: glm-5.3-flash (agent: phase1-reviewer, role: security-auditor); reviewer 6: muse-spark-1.3-contributor (standing in for gpt-6.1-sol) (agent: phase2-reviewer, role: general); reviewer 7: muse-spark-1.3-contributor (standing in for gpt-6.1-sol) (agent: phase2-reviewer, role: architecture-layering); reviewer 8: muse-spark-1.3-contributor (standing in for gpt-6.1-sol) (agent: phase2-reviewer, role: platform-versioning); reviewer 9: muse-spark-1.3-contributor (standing in for gpt-6.1-sol) (agent: phase2-reviewer, role: rust-quality); reviewer 10: muse-spark-1.3-contributor (standing in for gpt-6.1-sol) (agent: phase2-reviewer, role: security-auditor); final verifier: muse-spark-1.3-contributor (standing in for gpt-6.1-sol) (agent: sol-verifier, role: final-verifier)
- Degraded mode:
gpt-6.1-solunavailable: 503 auth_unavailable: no auth available (providers=codex, model=gpt-6.1-sol; last upstream error: usage_limit_reached: T (detected by probe, since 2026-09-30T21:27:42Z); stand-insgpt-5.6-luna→muse-spark-1.3-contributor,gpt-5.6-sol→muse-spark-1.3-contributor,gpt-5.6-terra→muse-spark-1.3-contributor,gpt-6-astra→muse-spark-1.3-contributor,gpt-6.1-sol→muse-spark-1.3-contributor; Phase 1 effort capped athigh - Triage:
criticalbymuse-spark-1.3-contributor(standing in forgpt-6.1-sol) (effort low) — Large 25-file change that alters consensus block execution and key handling in create_owner_identity/update_masternode_identities to define owner/operator identities for new Core v24 masternode variants. - Phase 1 reviewers:
glm-5.3-flash— general (completed, effort high); agentphase1-reviewer,glm-5.3-flash— architecture-layering (completed, effort high); agentphase1-reviewer,glm-5.3-flash— platform-versioning (completed, effort high); agentphase1-reviewer,glm-5.3-flash— rust-quality (completed, effort high); agentphase1-reviewer,glm-5.3-flash— security-auditor (completed, effort high); agentphase1-reviewer - Phase 1 model:
glm-5.3-flash— zai quota: 5h 99% left, weekly 78% left; passed overgemini-3.8-flash-high(antigravity below 15% reserve: weekly 13% left, 5h 100% left) - Fresh verifier:
muse-spark-1.3-contributor(standing in forgpt-6.1-sol) — final-verifier; agentsol-verifier - Phase 2 reviewers:
muse-spark-1.3-contributor(standing in forgpt-6.1-sol) — general (completed, effort xhigh); agentphase2-reviewer,muse-spark-1.3-contributor(standing in forgpt-6.1-sol) — architecture-layering (completed, effort xhigh); agentphase2-reviewer,muse-spark-1.3-contributor(standing in forgpt-6.1-sol) — platform-versioning (completed, effort xhigh); agentphase2-reviewer,muse-spark-1.3-contributor(standing in forgpt-6.1-sol) — rust-quality (completed, effort xhigh); agentphase2-reviewer,muse-spark-1.3-contributor(standing in forgpt-6.1-sol) — security-auditor (completed, effort xhigh); agentphase2-reviewer
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `packages/rs-drive-abci/src/platform_types/masternode/v0/mod.rs`:
- [SUGGESTION] packages/rs-drive-abci/src/platform_types/masternode/v0/mod.rs:97-109: Zero-address sentinel collapses Some([0;20]) into None without a pinning test
stored_address maps None to [0;20] and loaded_address maps [0;20] back to None, so a present zero address does not round-trip. Core currently never emits a present zero owner key and nothing reads stored payout/collateral addresses, so this is safe today, but the invariant lives only in prose. The new round-trip tests cover non-zero and absent addresses but never exercise Some([0;20]), so a future emitter of a zero address would regress silently. Pin the intended collision with an explicit edge-case test.
In `packages/rs-drive-abci/src/platform_types/masternode/v0/accessors.rs`:
- [NITPICK] packages/rs-drive-abci/src/platform_types/masternode/v0/accessors.rs:14-15: collateral_address accessor doc omits zero-bytes-absent sentinel
MasternodeV0::collateral_address now uses zero bytes to mean absent for shared masternodes, documented on the struct field and on stored_address/loaded_address. The MasternodeAccessorsV0::collateral_address trait method returns the same raw [u8; 20] but its doc still promises 'The address where the collateral is stored', so a future caller reading only the trait cannot learn that all-zero means absent. Nothing calls the accessor today, so there is no behavior bug; the trait contract should carry the same caveat the struct field does.
|
Your move: thepastaclaw left review threads unresolved; resolve them. |
github-actions
Bot
added
waiting-self-review
Moves the rust-dashcore pin to 40268cc0 (dashpay/rust-dashcore#1093), whose platform address accessors bracket an IPv6 node IP in the legacy port fallback, as nested entries already are. Drive takes only the port from these accessors, so nothing it decides changes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…as absent The stored masternode list writes an absent owner, payout or collateral address as zero bytes and reads zero bytes back as absent, so a present address of zero bytes does not round-trip. Core never prints one (it refuses a null owner key for every masternode but a shared one, and nothing reads a stored payout or collateral address), so this is the intended collision; pin it with a test instead of leaving it to prose, and say on the collateral_address accessor that zero bytes mean absent. No behaviour change; the test passes before and after. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
github-actions
Bot
added
waiting-bots
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Re-review — Final validation — Phase 2 only (queue backlog)
Verified the complete PR diff at 7c67e01, including the pinned RPC decoder changes; no in-scope blocking defect was confirmed. Both prior findings are fixed, and local validation passed 149 masternode-filtered Rust tests (one ignored), all four dashmate template tests, and git diff --check. One suggestion remains: identify the source-breaking re-exported Rust API changes in release metadata.
🟡 1 suggestion(s)
Review provenance
Source: reviewer 1: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 8: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 9: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 10: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)
- Triage:
criticalbygpt-6.1-sol(effort low) — The large, intricate diff changes consensus-critical masternode identity creation, protocol-upgrade behavior, peer-facing Core masternode-list deserialization, and stored masternode-list handling in drive-abci. - Phase 1 reviewers: not run (skipped for throughput: 13 PRs queued, above the 10 limit)
- Fresh final gate: an independent Phase-2 review ran after iterative findings were reconciled
- Fresh verifier:
gpt-6.1-sol— final-verifier; agentsol-verifier - Phase 2 reviewers:
gpt-6.1-sol— general (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— architecture-layering (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— platform-versioning (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— rust-quality (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— security-auditor (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— general (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— architecture-layering (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— platform-versioning (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— rust-quality (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— security-auditor (completed, effort xhigh); agentphase2-reviewer
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `Cargo.toml`:
- [SUGGESTION] Cargo.toml:75: Mark the re-exported Rust API break in release metadata
This dependency bump changes public types exposed through `dpp::dashcore_rpc` when `core_rpc_client` is enabled: address fields become optional, platform-port fields are renamed, and struct literals require additional fields. Existing Rust consumers therefore need source changes even though Platform consensus compatibility is preserved. The PR description acknowledges these breaks but explicitly omits `!` because consensus does not change; the repository's PR template requires the marker for breaking changes generally. The changelog generator uses the Dash conventional-commit preset, which recognizes `!` or a `BREAKING CHANGE` footer, not the PR's Markdown section. Change the PR/squash title to `fix(platform)!:` and retain the API migration explanation so release notes identify the source-compatibility break. This does not require a PlatformVersion or GroveVersion bump.
Out-of-scope follow-up suggestions (2)
These are valid observations, but they are outside this PR's scope and should be handled in separate issues or author/maintainer-requested PRs rather than blocking this review.
- Versioned payout changes must revoke stale withdrawal authority — The owner-identity update path rotates the TRANSFER key only for
payout_address; apayouts-only change leaves the previous key enabled, as the new regression test confirms. That preserves withdrawal authority for the previous payout-key holder after Core changes recipients. The shipped decoder already accepts these diffs while ignoring their payout lists, and the production key-update branch is unchanged by this PR, so this is a concrete security requirement for the explicitly deferred payout-authorization work rather than an in-scope blocker.- Follow-up: Track revocation and migration of legacy TRANSFER keys in the separate protocol-versioned payout-authorization work, with tests confirming that previous payout-key holders lose withdrawal authority.
- PV1–PV3 can accept a multi-payout Evo without an owner identity — Outside the supported deployment scope: create_owner_identity_v0 does return None for multiple payouts, and proposer reward distribution targets the ProTxHash identity, supporting the stated failure mechanism. However, both the source comment and PR description explicitly require Core v24-from-genesis deployments to start at protocol version 4 or later. An additional deployment guard for this excluded configuration is not required for the stated PV13 compatibility goal.
- Follow-up: Consider creating a separate issue or author/maintainer-requested PR for this.
|
Bots are done — your move: post |
github-actions
Bot
added
waiting-self-review
shumkov
changed the title
The rendered-dash.conf test for `deprecatedrpc=service` is not needed; the template change stays. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…changed
Shipped generations may change in place only where the new code cannot
run under the protocol versions that select them. A node at protocol
version 13 running against Dash Core v24 does receive shared and
extended-address masternodes, so the branches the previous commit added
to them were reachable and would have created state where every
released binary stops.
Restore create_owner_identity v0 (protocol versions 1-3) and v1 (4-13),
update_masternode_identities v0 and transition_to_version_4 to the
mechanical adaptation of the rust-dashcore bump: a masternode without the
payout or owner address they read fails with DashCoreBadResponseError,
the same halt, without state, as a release that cannot parse the list.
The three create_owner_identity and update_masternode_identities files
are byte-identical to that commit again; transition_to_version_4 differs
only in its test. The stored masternode conversion (absent address <->
zero bytes) stays: it is aux storage, not consensus state.
Tests now pin the shipped behaviour:
- create_owner_identity at protocol versions 3 and 13: shared, single
payout list, several payouts (and, at 13, no owner address) fail with
DashCoreBadResponseError; a legacy masternode gets its identity.
- The Core v24 listdiff fixture at protocol version 13 fails with
DashCoreBadResponseError ("... has no payout address").
- transition_to_version_4 gives a masternode with an owner address its
OWNER key after its TRANSFER key, skips a masternode without an owner
identity, and fails on one with an owner identity but no owner
address.
- The payout-list-diff test moves to the update_masternode_identities
dispatcher and runs at protocol version 13 and the latest version.
The tests for the new owner identities move to the new generations in
the next commit.
Red to green: the three new error tests fail on the parent commit
("expected DashCoreBadResponseError, got Ok(None)" / "got Ok(())") and
pass here.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…lti-payout masternodes at protocol version 14
From v24 on, Dash Core lists shared masternodes, which have no owner,
payout or collateral address, and extended-address masternodes, which
have a `payouts` list instead of `payoutAddress`. create_owner_identity 1
fails on both with DashCoreBadResponseError, which fails the block.
New generations, selected only by protocol version 14
(DRIVE_ABCI_METHOD_VERSIONS_V10, amended in place; only PLATFORM_V14
references it):
- create_owner_identity 2, a copy of 1: no owner address gives no owner
identity; an owner address and a single payout address (payoutAddress,
else the only entry of a one-entry payout list) give the version 1
identity byte for byte (TRANSFER key 0, OWNER key 1); several payouts
give an owner identity with only the OWNER key 1, key id 0 free. The
effective-payout helper lives in v2, its only user.
- update_masternode_identities 1, a copy of 0 that adds no owner identity
when create_owner_identity gives none. Updates are processed as in 0:
only a payoutAddress change rotates the TRANSFER key, and a `payouts`
change is not acted on.
The create_owner_identity dispatcher now returns Option<Identity>.
Versions 0 and 1 are unchanged and wrapped in Some; the only change to
update_masternode_identities 0 unwraps it and treats None as
CorruptedCodeExecution, unreachable because every protocol version that
selects it selects create_owner_identity 0 or 1. transition_to_version_4
is unchanged; it only runs when crossing protocol version 4.
Item 66 of PLATFORM_V14's doc records the change: protocol version 14
has to be active on a network before its Dash Core activates V24, since
earlier versions keep failing on these masternodes.
Tests at the latest version, through the dispatchers:
- create_owner_identity v2: shared gives None; a single payout gives the
payout address's identity, which equals version 1's at protocol
version 13, byte for byte; several payouts give OWNER key 1 only.
- update_masternode_identities v1: a shared masternode gets voter and
operator identities and no owner identity; a change of its voting
address and operator key, and its removal, succeed without an owner
identity and leave its voter and operator keys disabled.
- update_masternode_list with the Core v24 listdiff fixture: processed,
with the identities above and no key change on payout-list diffs
(protocol version 13 keeps failing on it, pinned in the previous
commit).
- The shipped-generation test now also checks the key layout a legacy
masternode gets at protocol versions 3 (TRANSFER 0) and 13 (TRANSFER 0,
OWNER 1), and the payout-list-diff test describes both generations.
Red to green: with create_owner_identity 1 and
update_masternode_identities 0 left in DRIVE_ABCI_METHOD_VERSIONS_V10 the
six new-generation tests fail with DashCoreBadResponseError ("... has no
payout address"); with the table change they pass.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Pin the historical transition regression and its initial storage to PV4 so the successful OWNER-key insertion checks the protocol that first shipped it. The targeted test passes. Test-only change; no production behavior delta.
shumkov
changed the title
github-actions
Bot
added
waiting-bots
|
@thepastaclaw review Fixed the payout-list authorization defect in 0084f1a (PV14 only). Supported sole P2PKH payout replacements rotate/reuse/re-enable TRANSFER authority; splits, empty lists, and unsupported/inconsistent scripts disable every enabled TRANSFER key while preserving OWNER and balance. Creation uses the same script rule. Shipped generations and legacy rotation are unchanged. Observed red → green with genuinely signed stale-key withdrawals after replacement and a split: SuccessfulExecution before the fix, node-side PublicKeyIsDisabledError after it, with later credits protected and no withdrawal debit/document on rejection. Replacement/OWNER withdrawals, key reuse/restoration, overflow, and rollback of both keys and withdrawal references are tested. Local checks: 163 masternode tests passed (1 existing ignored), 202 withdrawal tests passed (none skipped), formatting and all-target/all-feature Clippy passed. Independent security, architecture and test reviews found no remaining blockers. Broader shared/multi-recipient/P2SH withdrawals, the bounded default-destination lookup after many rotations, and pre-PV14 stale-state reconciliation remain explicitly documented limitations. This supersedes the earlier deferral reply. Finding. Please verify the complete new head and publish a fresh verdict. The description includes the corrected scope and verification evidence. |
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Re-review — Final validation — Phase 1 + Phase 2
PV14 isolates the new masternode policy, and payout reconciliation now revokes obsolete stored TRANSFER authority before withdrawal validation; all four prior findings are fixed. Two non-blocking suggestions remain: correcting the protocol reference and covering the initially OWNER-only identity receiving its first TRANSFER key. This was a static review: current-head Rust workspace CI passes, Swift SDK CI fails, and two test-suite jobs remain pending.
🟡 2 suggestion(s)
Review provenance
Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 7: gpt-6-astra (agent: phase2-reviewer, role: general); reviewer 8: gpt-6-astra (agent: phase2-reviewer, role: architecture-layering); reviewer 9: gpt-6-astra (agent: phase2-reviewer, role: platform-versioning); reviewer 10: gpt-6-astra (agent: phase2-reviewer, role: rust-quality); reviewer 11: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 12: gpt-6-astra (agent: phase2-reviewer, role: security-auditor); reviewer 13: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 14: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 15: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 16: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 17: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)
- Triage:
criticalbygpt-6.1-sol(effort low) — The intricate new create_owner_identity/v2 and update_masternode_identities/v1 implementations change PV14 consensus state creation and payout TRANSFER-key authorization, including disabling and re-enabling withdrawal authority. - Phase 1 reviewers:
muse-spark-1.3-contributor— general (completed, effort xhigh); agentphase1-reviewer,muse-spark-1.3-contributor— rust-quality (completed, effort xhigh); agentphase1-reviewer - Phase 1 model:
muse-spark-1.3-contributor— not quota-gated; passed overgemini-3.8-flash-high(antigravity below 15% reserve: weekly 15% left, 5h 100% left),glm-5.3-flash(not used above high effort; tier asks max) - Single stage: Phase 1 and Phase 2 reviewed this head side by side, with no blocker gate between them (triage tier)
- Fresh final gate: an independent Phase-2 review ran after iterative findings were reconciled
- Fresh verifier:
gpt-6.1-sol— final-verifier; agentsol-verifier - Phase 2 reviewers:
gpt-6.1-sol— general (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— architecture-layering (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— platform-versioning (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— rust-quality (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— security-auditor (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— general (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— architecture-layering (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— platform-versioning (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— rust-quality (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— security-auditor (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— general (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— architecture-layering (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— platform-versioning (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— rust-quality (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— security-auditor (completed, effort xhigh); agentphase2-reviewer - Model comparison: every Phase-2 reviewer also ran on
gpt-6-astra; the verifier weighed both sets without knowing which model wrote which
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `packages/rs-platform-version/src/version/v14.rs`:
- [SUGGESTION] packages/rs-platform-version/src/version/v14.rs:1967-1974: Update the PV14 reference to describe payout-key reconciliation
Item 73 says updates are unchanged and `payouts` changes are not acted on, but the PV14-selected updater now calls `reconcile_owner_payout_keys`: it retains, re-enables, or adds the sole supported P2PKH recipient's TRANSFER key and disables obsolete TRANSFER authority. The creation description also incorrectly grants a TRANSFER key to every one-entry list; `effective_payout_address` requires the script to match that recipient's P2PKH address. This permanent protocol reference therefore describes the superseded authorization policy. Update item 73 and the corresponding V10 table documentation to record the supported-script restriction and payout-list reconciliation, while distinguishing the unchanged historical updater and legacy payout-address rotation. No additional version bump is needed.
In `packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_identities/update_masternode_identities/v1/payout_tests.rs`:
- [SUGGESTION] packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_identities/update_masternode_identities/v1/payout_tests.rs:113-116: Cover an initially OWNER-only identity receiving its first TRANSFER key
Every payout-update fixture registers a supported sole recipient, so TRANSFER key 0 and its withdrawal-index reference already exist. Restoration tests reuse that retained key, and the Core v24 list test leaves initially multi-payout identities OWNER-only. No test exercises an initially multiple- or unsupported-payout registration later becoming a supported sole recipient. This new lifecycle starts with only OWNER key 1 and an empty withdrawal index; reconciliation must allocate TRANSFER key 2 and create its reference without changing OWNER or balance. Add a dispatcher-level regression starting from that registration, apply a sole-P2PKH update, assert the key layout and RecentWithdrawalKeys reference, and process a genuinely signed withdrawal successfully. The implementation appears correct; the missing coverage concerns a distinct state introduced by this PR.
Out-of-scope follow-up suggestions (3)
These are valid observations, but they are outside this PR's scope and should be handled in separate issues or author/maintainer-requested PRs rather than blocking this review.
- Audit historical payout authority if Core v24 preceded PV14 — The historical updater ignores payout-list changes, the PV14 reconciler runs only on subsequent payout-list diffs, and the protocol-14 transition contains no activation-time key repair. A network that processed a payout replacement or split before PV14 could therefore retain obsolete withdrawal authority until another payout-list change triggers reconciliation. This conditional, security-relevant historical-state limitation is outside the intended PV14-before-Core-v24 rollout and is explicitly documented; it is not a remaining defect in the corrected update path.
- Follow-up: If any deployment used Core v24 payout-list changes before PV14, track a separate state audit and maintainer-approved reconciliation procedure.
- Default withdrawal destination selection remains a Drive-owned follow-up — Outside scope — the unchanged withdrawal transformer searches the latest TRANSFER reference and five older entries. The PR explicitly documents this inherited limitation, and the new regression distinguishes successful explicit-destination signing from failed default-destination lookup after historical-key reuse. It does not undermine obsolete-key revocation and does not warrant another review follow-up here.
- Follow-up: Consider creating a separate issue or author/maintainer-requested PR for this.
- Default withdrawal destination lookup remains bounded after key reuse — Outside scope — this duplicates the documented, unchanged bounded destination lookup. A separate Drive destination-selection redesign is not required for this PR's authority-reconciliation goal.
- Follow-up: Consider creating a separate issue or author/maintainer-requested PR for this.
|
Bots are done — your move: post |
github-actions
Bot
added
waiting-self-review
Register a multi-payout masternode with only OWNER key 1, then apply a sole P2PKH payout through the dispatcher. Verify allocation of TRANSFER key 2, its withdrawal-index reference, unchanged OWNER and balance, and a genuinely signed withdrawal. Preserve the original sole-payout fixtures. Correct the permanent PV14 and V10 protocol references to describe supported scripts and payout-list reconciliation. No production behavior or table values change; the regression covers behavior already fixed in 0084f1a. Validation: 164 masternode tests passed (1 existing ignored), formatting and all-target/all-feature drive-abci Clippy passed. Independent test and protocol documentation reviews found no remaining issues.
|
Waiting for bot review — coderabbitai not yet · thepastaclaw ✓. Wait for the missing reviews, or a writer can post |
github-actions
Bot
added
waiting-bots
|
@thepastaclaw review Both non-blocking suggestions from your final review are addressed in bc2a201:
Local verification: 164 masternode tests passed (1 existing ignored), formatting and all-target/all-feature Clippy passed, and independent test/documentation reviews passed. This follow-up changes tests and documentation only; production behavior is unchanged from the head where you confirmed all four blockers fixed. Documentation thread · lifecycle-test thread. Prior-head Rust CI passed 18,386 main tests and 913 shielded tests; other test jobs passed except testCaptureHistoricalV2Fixture's known SQLite-lock flake (774 tests, 1 skip, 1 failure). The Swift/FFI source review confirmed unchanged Swift files and the identical earlier failure that cleared on retry. CI is now running against this final follow-up head. Please verify the final head and publish a fresh verdict. |
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Re-review — Final validation — Phase 1 + Phase 2
Static verification of the complete base-to-head diff found no remaining in-scope defects at bc2a201. All six prior findings are addressed: PV14 alone selects the new identity behavior, obsolete payout authority is revoked through transactional Drive operations, and the final follow-up adds the OWNER-only lifecycle regression and corrected protocol documentation. No builds or tests were run locally; the supplied 2026-10-04T19:28:57Z CI snapshot still had Rust workspace tests, JS and Docker builds, and PR Hygiene pending.
🔴 0 blocking | 🟡 0 suggestion(s) | 💬 0 nitpick(s)
Review provenance
Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 3: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 8: gpt-6-astra (agent: phase2-reviewer, role: general); reviewer 9: gpt-6-astra (agent: phase2-reviewer, role: architecture-layering); reviewer 10: gpt-6-astra (agent: phase2-reviewer, role: platform-versioning); reviewer 11: gpt-6-astra (agent: phase2-reviewer, role: rust-quality); reviewer 12: gpt-6-astra (agent: phase2-reviewer, role: security-auditor); reviewer 13: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 14: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 15: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 16: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 17: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)
- Triage:
criticalbygpt-6.1-sol(effort low) — The large, intricate changes in create_owner_identity/v2/mod.rs and update_masternode_identities/v1/mod.rs alter consensus-selected identity creation and payout TRANSFER key authorization, including revoking and re-enabling withdrawal authority. - Phase 1 reviewers:
muse-spark-1.3-contributor— general (completed, effort xhigh); agentphase1-reviewer,muse-spark-1.3-contributor— rust-quality (completed, effort xhigh); agentphase1-reviewer - Phase 1 model:
muse-spark-1.3-contributor— not quota-gated; passed overgemini-3.8-flash-high(antigravity below 15% reserve: weekly 15% left, 5h 100% left),glm-5.3-flash(not used above high effort; tier asks max) - Single stage: Phase 1 and Phase 2 reviewed this head side by side, with no blocker gate between them (triage tier)
- Fresh final gate: an independent Phase-2 review ran after iterative findings were reconciled
- Fresh verifier:
gpt-6.1-sol— final-verifier; agentsol-verifier - Phase 2 reviewers:
gpt-6.1-sol— general (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— architecture-layering (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— platform-versioning (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— rust-quality (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— security-auditor (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— general (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— architecture-layering (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— platform-versioning (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— rust-quality (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— security-auditor (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— general (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— architecture-layering (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— platform-versioning (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— rust-quality (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— security-auditor (completed, effort xhigh); agentphase2-reviewer - Model comparison: every Phase-2 reviewer also ran on
gpt-6-astra; the verifier weighed both sets without knowing which model wrote which
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify the current code and confirm that no unresolved issues remain.
No unresolved findings remain from the prior review on this head.
Out-of-scope follow-up suggestions (1)
These are valid observations, but they are outside this PR's scope and should be handled in separate issues or author/maintainer-requested PRs rather than blocking this review.
- Audit historical payout authority if Core v24 preceded PV14 — The historical updater ignores payout-list changes, and PV14 reconciliation runs on subsequent Core payout-list diffs rather than protocol activation. If a network enabled those Core features before PV14, an obsolete TRANSFER key could therefore remain authorized after activation until another payout-list change reconciles it. This is a concrete historical-state concern outside the PR's explicitly required PV14-before-Core-v24 rollout.
- Follow-up: Separately establish whether any network violated the activation prerequisite; if so, audit affected identities and decide whether a versioned reconciliation migration is needed.
|
@coderabbitai review Please review final head bc2a201. Your receipt currently covers 0084f1a; the follow-up adds only the initially OWNER-only lifecycle test and protocol documentation. Pasta Claw has approved the complete final diff with zero findings. I verified the retained conditional ownerless-payout concern against Core's producer contract at a31fcf26f1444f91f4e5881f2735eb53546c155d:
Thus a valid shared-masternode payout-list diff does not reach owner-key reconciliation. Missing owner keys on a genuine owner-payout diff remain an invariant error rather than silently hiding corrupted state. The supported shared registration/update/removal lifecycle is tested. The final added test also verifies a non-shared multi-payout OWNER-only identity gains its first TRANSFER key and exact withdrawal reference, then completes a genuinely signed withdrawal. CI's network, browser, JS and Docker jobs have passed; Rust and its dependent Swift job are still pending. |
✅ Action performedReview finished.
|
|
Bots are done — your move: post |
github-actions
Bot
added
waiting-self-review
|
/self-reviewed |
|
Ready for review — |
github-actions
Bot
added
ready-for-human
Retain the Core compatibility, shielding nullifier and reference-validation documentation entries. Manual conflict resolution changes comments only; no behavioral test added.
Port of #4932 (closed) from v5.0-dev to v5.1-dev, reorganized as a single commit on top of 6499c68. It replaces the original history: the #4932 commits, the port merge, two v5.1-dev merges and the interim Node 20 change, which #5310 later superseded. - Bump the rust-dashcore git dependencies (secp256k1 0.30 -> 0.33, rand 0.9, getrandom 0.4) and migrate call sites mechanically: no secp256k1 context arguments; key, signature and RNG API renames; hex rendering through the `hex` crate. Seeded test keys are unchanged. - Enable getrandom 0.3/0.4 `wasm_js` on wasm32-unknown-unknown in rs-dpp. - Keep the Core v24 masternode handling and persistence from #5227/#5228, replacing the migration's superseded strict legacy conversions. - Preserve the existing cryptography implementations, legacy RPC projection and FFI ABI. SQLite replay, accounting repairs and SwiftData accounting changes remain in the dependent PR #5150. Co-Authored-By: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Basic explanation
What this does: At protocol version 14, Drive accepts Core v24 shared masternodes and masternodes with payout lists. Earlier protocol versions retain their owner-identity behavior. Dashmate also enables
deprecatedrpc=service, which Core v24 requires to return fields still used by Platform.Value: Enables Core v24 masternode registration without deciding the future ownership and withdrawal model for shared or multi-payout masternodes.
Risks: Consensus behavior changes only at PV14. PV14 must activate before Core V24 activates on the network. PV13 still fails on shared/extended-address registrations; a halted chain cannot vote in PV14. Withdrawal limitations are listed below.
Issue being fixed or feature implemented
Core v24 omits owner/payout/collateral addresses for shared masternodes and replaces
payoutAddresswithpayoutsfor extended-address registrations. The old RPC parser rejects these lists, causing block execution to fail on every Drive node. Tor/I2P services and negative platform ports also fail parsing. Related network-address work: #3227 and #5228.What was done?
40268cc0402a8933ec539f16b2d634c4e25876adfrom feat(rpc-json)!: parse Core v24 masternode lists rust-dashcore#1093. The commit remains onintegration/v4.2-core-v24; it adds RPC compatibility on top of the previous pin without importing newerdevdependencies.create_owner_identityv2 andupdate_masternode_identitiesv1, selected only by PV14's existingDRIVE_ABCI_METHOD_VERSIONS_V10table. Shared masternodes get voter/operator identities and no owner identity. A sole supported P2PKH payout gets TRANSFER key 0 and OWNER key 1; other payout lists get OWNER key 1 only. Legacy input yields the same identities.create_owner_identityv0/v1 andtransition_to_version_4behavior: missing required addresses cause explicit errors. The dispatcher wraps historical identities inSome.payoutschanges in the PV14 updater: retain/re-enable/add the sole supported P2PKH TRANSFER key and disable obsolete TRANSFER keys. Split, empty, P2SH, or inconsistent payout lists disable all TRANSFER authority while preserving OWNER and balance. Key reuse matches purpose/type/data deterministically; allocation is checked. Conflicting payout representations fail before applying operations. Shipped payout-list and legacy rotation behavior remains unchanged.payoutsis not persisted. New identity creation reads fresh Core RPC entries.deprecatedrpc=serviceto dashmate's Core template. The template-only test was removed as requested. Platform ports continue using the flat fields here; feat(drive-abci)!: resolve Core v24 platform ports at protocol version 14 #5228 handles nested addresses separately.In-place changes to shipped generations
create_owner_identityv0 (PV1–3), v1 (PV4–13), andtransition_to_version_4: mechanicalOptionadaptation preserves keys for every previously valid input; absent required fields still fail instead of creating new state. The version-4 transition is tested at PV4.update_masternode_identitiesv0 (PV1–13): unwraps the dispatcher's optional identity with an explicit error.Noneis unreachable because every selecting protocol version pairs it with owner-creation v0 or v1, both wrapped inSome.How Has This Been Tested?
update_masternode_identities/v1/payout_tests.rsproves red → green with genuinely signed obsolete-key withdrawals: replacement/split executed those requests before the fix and now return disabled-key errors without withdrawal debits or documents. Coverage includes later credits, OWNER/default-destination controls, restoration/reuse, unsupported scripts, checked allocation, full-key/withdrawal-index rollback/retry, and an initially OWNER-only registration receiving its first TRANSFER key and completing a signed withdrawal.cargo clippy -p drive-abci --all-targets --all-features -- -D warningspassed. Independent correctness, security, test and documentation reviews found no remaining issues.bc2a201c6a: 18,387 main Rust tests passed / 1,388 excluded; 913 shielded tests passed / 3 ignored. Swift SDK: 789 tests / 16 skipped / 0 failures, including the historical-fixture test; simulatorxcodebuild testsucceeded. Node suite: 95 passing / 17 pending. Docker builds, JS packages/linting, functional tests, both browser suites, Dashmate E2E and the separate Kotlin check passed. Existing filters and gated tests remain; the full chain-simulation phase is skipped for PR runs. One failed-job retry cleared an unchanged wallet timing-test flake; no wallet source or assertions were changed.Breaking Changes
PV14 introduces new consensus behavior; shipped protocol versions do not gain the new owner-identity rules. The re-exported RPC types also change: optional address fields, new
payouts/addresses, and renamedlegacy_platform_*_portfields.Before switching Core to v24, render the config with
dashmate config render --config=<name>and restart the node sodeprecatedrpc=serviceis active. Config-format upgrades render migrated configs automatically, but a same-format upgrade does not. The target release is now 5.0.0-beta.1; the protocol boundary remains PV14.Known follow-ups:
Checklist:
structure.rs, regeneratedgrovedb-structure.json, and checked the structure viewer link posted on this pull requestFor repository code-owners and collaborators only
PR Hygiene ·
bc2a201dashmate— you own itrs-drive-abci(packages/rs-drive-abci/src/execution/platform_events/block_end/update_state_cache/v0/mod.rs,packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_identities/create_operator_identity/v0/mod.rs,packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_identities/create_owner_identity/mod.rsand 23 more) — QuantumExplorerWhen every box is checked the
PR Hygienecheck passes and this can merge.Summary by CodeRabbit
New Features
Bug Fixes
Tests