Skip to content

feat(drive)!: add the contract credits root sum tree to genesis, upgrade and credit conservation - #4716

Open
DCG-Claude wants to merge 11 commits into
v6.0-devfrom
dashvm/r04-01
Open

DCG-Claude wants to merge 11 commits into
v6.0-devfrom
dashvm/r04-01

Conversation

@DCG-Claude

@DCG-Claude DCG-Claude commented Sep 12, 2026 •

Copy link
Copy Markdown
Collaborator

Issue being fixed or feature implemented

Part 1 of 4 for R04-01 of the smart contract plan in #4626 (workstream issue #4690, section 4: contract credit and token buckets).

Ordinary data contracts are not identities and stay that way. When a contract needs to hold credits (to pay for scheduled work, to escrow a purchase, to fund its own storage) those credits must live in a structure that is separate from every identity tree, must be provable per bucket and per contract, and must take part in the end-of-block credit conservation check so a contract can never hold credits that the equation does not see.

This first part lays the foundation: the root tree that will hold every contract's credit buckets, its creation at genesis and on upgrade, and its term in the conservation equation. Bucket identifiers and rules, the bucket storage operations, the proofs and the matching token holdings follow in the next three parts.

What was done?

Protocol versions 15 to 17 (packages/rs-platform-version/src/version/{v15,v16,v17}.rs, mod.rs, protocol_version.rs). The registry is indexed by number, so the 5.0 version cannot exist without 15 and 16. Those two are placeholders written as struct updates over their predecessor, byte-identical to the files the other 5.0 branches carry, so a forward merge resolves by taking either side. PLATFORM_V17 selects the new drive table and nothing else.

Drive table DRIVE_VERSION_V10 (packages/rs-platform-version/src/version/drive_versions/v10.rs): a struct update over V9 that names only initialization.create_initial_state_structure: 5 and balances.calculate_total_credits_balance: 3. No struct gains a field, so the shipped tables and the mocks are untouched.

Root tree RootTree::ContractCredits = 100 (packages/rs-drive/src/drive/mod.rs), an ordinary SumTree, added to Display, TryFrom<u8> and the byte conversion, plus KnownPath::ContractCreditsRoot in the batch debug printer (packages/rs-drive/src/util/batch/grovedb_op_batch/mod.rs), where a 32-byte key under it prints as a contract id. The layout comment now shows the key as the left child of Misc, which is where AVL rebalancing places it on both the genesis and the upgrade path.

Path helpers in the new packages/rs-drive/src/drive/contract/balances/mod.rs (built for both the server and verify features): contract_credits_root_path() and contract_credits_path(contract_id) with their _vec forms.

Genesis Drive::create_initial_state_structure_v4 (packages/rs-drive/src/drive/initialization/v4/mod.rs): a copy of v3 that inserts the empty sum tree as a standalone root insert right after ShieldedBalances, before the lower-layer batch. v0 to v3 are byte-identical to before.

Upgrade Platform::transition_to_version_17 in the protocol change hook (packages/rs-drive-abci/src/execution/platform_events/protocol_upgrade/perform_events_on_first_block_of_protocol_change/v0/mod.rs): an insert-if-not-exists of the same Element::empty_sum_tree(), guarded by previous < 17 && current >= 17 like every earlier transition in that file. Idempotent, so a validator that ran the hook inside a rejected proposal and runs it again in the next round produces the same state.

Credit conservation Drive::calculate_total_credits_balance_v3 (packages/rs-drive/src/drive/balances/calculate_total_credits_balance/v3/mod.rs) reads the root aggregate as the sixth term. TotalCreditsBalance in packages/rs-dpp/src/balances/total_credits_balance/mod.rs gains total_in_contract_credits, checked for sign, included in ok(), total_in_trees() and Display. v0 to v2 receive only the struct-literal backfill total_in_contract_credits: 0, the same edit the shielded term made.

Book: a new chapter book/src/drive/contract-credit-buckets.md (layout, why a separate ordinary sum tree, genesis and upgrade, the live and wiped lifecycle the layout is designed for, the conservation term, what is not here yet) and the placeholder-version note in book/src/versioning/platform-version.md that the other 5.0 branches also carry.

Fee pins. The new root key is the left child of Misc, so every write under Misc (system credits on identity create and top up, token total supply on mint and burn) hashes one more child. Six latest-version pins in drive-abci move by 1 480 credits; the protocol 14 values are kept as frozen pins next to them where the test already had a per-version runner, and the moved literals carry the pre-17 value in a comment:

Test Before At 17
identity_create validation processing fee 1 919 540 1 921 020
identity_create asset lock reuse and replay processing fee 2 195 200 2 196 680
identity_top_up validation processing fee 588 840 590 320
token burn confirmer processing fee 4 368 280 4 369 760
token direct purchase buyer balance 699 868 046 180 699 868 044 700
strategy run_chain_one_identity_in_solitude_latest_protocol_version balance 99 864 009 940 99 864 008 460

The drive-level genesis shape test moved from 17 to 18 root keys, the Misc proof grew by one child hash (285 to 319 bytes) and the new key proves at 285 bytes like its level 4 siblings. Pins for shipped versions are unchanged because their genesis has no new key.

Review finding carried to the next part. Plan review left one finding open: how repeated updates of the same bucket inside one operation batch observe each other's pending writes (the batch converter reads every operation from the pre-batch snapshot and GroveDB keeps the last write to a key). The disposition in the plan is that every bucket write takes the pending-operations vector, takes over a pending replacement of the same sum item, and emits exactly one replacement per bucket per batch, with tests for two debits from one stored amount. That lands with the bucket storage operations in part 2; nothing in this part writes a bucket.

How Has This Been Tested?

New tests:

  • drive, initialization/v4: should_create_contract_credits_root_at_latest_genesis (the root element is an empty sum tree without flags) and should_not_create_contract_credits_root_at_protocol_14_genesis.
  • drive, calculate_total_credits_balance/v3: should_balance_credits_with_empty_contract_credits_root, should_read_contract_credits_as_a_term_of_the_equation (two raw contract subtrees unbalance the equation until the matching system credits are added) and should_exclude_a_not_summed_contract_tree_from_the_term (a NotSummed subtree with a retained bucket contributes nothing).
  • dpp, total_credits_balance: the new term counts, a negative value is rejected, an overflowing value reports overflow.
  • drive, contract/balances: the path helpers.
  • drive-abci, protocol change hook: test_genesis_v17_and_upgrade_to_v17_build_identical_contract_credits_tree (fresh genesis at 17 against genesis at 16 plus the real transition, compared with collect_subtree_diffs), should_pass_credit_conservation_after_upgrade_to_v17 (the v17 and the frozen v16 calculator both hold after the upgrade) and should_activate_the_contract_credits_root_through_the_protocol_change_hook (populated v16 state with a funded identity and a document, the public dispatcher run inside a dropped transaction, then again and committed, then a third idempotent run, plus a same-version negative control).

Commands run locally, each redirected to a file with the exit code checked:

cargo fmt --all -- --check
cargo clippy -p dpp -p drive -p drive-abci -p platform-version --all-features --all-targets -- -D warnings
cargo check --workspace --all-targets
cargo check -p drive --no-default-features --features verify
cargo test -p platform-version --all-features
cargo test -p dpp --all-features --lib -- total_credits_balance
cargo test -p drive --all-features --lib -- initialization contract::balances calculate_total_credits_balance balances::tests
cargo test -p drive --all-features --lib -- identity::balance identity::update identity::insert document::insert tokens::balance::update
cargo test -p drive --all-features --test deterministic_root_hash
cargo test -p drive-abci --all-features --lib -- perform_events_on_first_block_of_protocol_change
cargo test -p drive-abci --all-features --lib -- identity_create::tests identity_top_up::tests batch::tests::document batch::tests::token check_tx::v0::tests data_contract_create::tests block_processing_end_events::tests execution_event execution_operation state_transition_execution_context shielded_withdrawal unshield shield_from_asset_lock shield::tests shielded_common address_funds_transfer identity_create_from_addresses address_credit_withdrawal identity_create_from_shielded_pool
cargo test -p drive-abci --all-features --test strategy_tests -- run_chain_one_identity_in_solitude

In-place changes to shipped generations

  • perform_events_on_first_block_of_protocol_change_v0 (drive-abci) gains the rung previous_protocol_version < 17 && platform_version.protocol_version >= 17, which calls transition_to_version_17. Protocol versions 4 to 12 select v0 directly, 13 reaches it through the v1 wrapper and 14 to 17 through the v2 wrapper. For every version below 17 the guard is false, so the shipped ladder runs exactly as before; the rung only executes on the first block at 17. The activation test covers a dropped transaction, a committed retry, an idempotent rerun and a negative control below 17.
  • calculate_total_credits_balance v0, v1 and v2 (drive, selected by protocol versions 1 to 16) set the new TotalCreditsBalance::total_in_contract_credits field to 0. A zero term leaves ok() and total_in_trees() unchanged and passes the non-negative check, so neither the conservation verdict nor the daily withdrawal limit input can change. TotalCreditsBalance is not serialized (it derives only Copy, Clone and Debug); the one other effect is an extra line in its Display and Debug output, which only appears in the node-local error for unbalanced credits.

Breaking Changes

Consensus-breaking from protocol version 17 only: a new root key changes the state root of every node at the first block of that version, and the credit conservation equation gains a term. Nothing changes for protocol versions 1 to 16; their genesis builds no new key, their calculators do not read it, and their fee pins are unchanged.

Decisions taken (provisional values)

  • Root key 100 for RootTree::ContractCredits. The allocation register leaves new root values unallocated; 100 is free in the enum, the byte conversions and the batch debug printer, and sits next to the other balance trees. Revised, if at all, before any network is asked to propose version 17.
  • Protocol version 17 as the 5.0 version, with 15 and 16 as placeholders identical to 14 (register values for the 4.3 and 4.4 releases).
  • DRIVE_VERSION_V10 for protocol 17. Another open 5.0 branch also claims V10 (for protocol 15); whichever lands first keeps the number and the other is renumbered to the next free generation at rebase, with no table content change.
  • An ordinary SumTree for the root and for each contract subtree, per the engineering recommendation on the task. The whole-contract total is the parent element's stored sum and a single bucket is a SumItem; range sums across buckets stay with the specialized provable-sum collections.
  • The wiped lifecycle is Element::NotSummed around a contract's subtree, chosen because GroveDB reports such an element's contribution as zero while its buckets keep their values for cleanup. This part only proves the exclusion in a test; the write that wraps a populated tree and moves its live total belongs to the wipe work of a later task, and the equation here is what that write must preserve.
  • The fee pin deltas above are consequences of the root layout, not tuning; no fee schedule, system limit, consensus error code, proto or SDK surface changes in this part.

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have added "!" to the title and described breaking changes in the corresponding section if my code contains any
  • I have made corresponding changes to the documentation if needed

For repository code-owners and collaborators only

  • I have assigned this pull request to a milestone

Refs #4690

🤖 Generated with Claude Code

Automated reviewer consensus (Fable 5.1 implementer, GPT-6 Astra reviewer)

Reviewer consensus

Plan Review consensus

  • R04-01-1 [major] Define how repeated bucket updates observe pending writes -> still_open
    • at PLAN.md §4, Part 2, lines 121–132
  • R04-01-2 [major] Preserve wiped state in the bucket read API -> resolved
    • at PLAN.md §4, Part 2, line 120
  • R04-01-3 [major] Correct the authenticated parent-row contract -> resolved
    • at PLAN.md §4, Part 3, lines 146–152
  • R04-01-4 [major] Test activation dispatch and transactional retry in Part 1 -> resolved
    • at PLAN.md §4, Part 1, lines 93–97
    • round 1 R04-01-1: accept: Verified: apply_drive_operations_v0 converts every operation before applying any, GroveDB collapses same-key ops last-wins with batching_consistency_verification off by default, and the intra-batch collapse is the audited conservation hazard. PLAN.md Part 2 now threads previous_batch_operations thro
    • round 1 R04-01-2: accept: The public read now returns ContractCreditBucket { lifecycle, amount } after reading the parent element (SumTree = Live, NotSummed(SumTree) = Wiped); fetch_live_contract_credit_bucket returns ContractCreditsNotLive for a wiped tree instead of an amount; the raw SumItem reader is pub(in crate::drive:
    • round 1 R04-01-3: accept: Confirmed in the pinned verifier: the populated-parent row is pushed after path.push(key), so it is ([100, contract_id], contract_id), while an empty parent is reported at ([100], contract_id). PLAN.md Part 3 rewrites the parser contract to accept zero rows, one parent row, or parent plus bucket in
    • round 1 R04-01-4: accept: PLAN.md Part 1 adds should_activate_the_contract_credits_root_through_the_protocol_change_hook: v16 genesis with an identity balance and a document, commit, root absent; call the public perform_events_on_first_block_of_protocol_change (previous 16, version 17) in a transaction, the entry point the D
    • round 2 R04-01-1: accept: Settled in round 1 and confirmed on disk: PLAN.md lines 124 to 125 thread previous_batch_operations through every bucket write and specify Drive::take_pending_sum_item_value, which takes over a pending SumItem replacement at the same path and key so a batch emits one replacement per bucket (10 and 2
    • round 2 R04-01-2: accept: Settled in round 1 and confirmed on disk: PLAN.md line 123 makes the public read return ContractCreditBucket { lifecycle, amount } after reading the parent element, adds fetch_live_contract_credit_bucket that errors with ContractCreditsNotLive on a wiped tree, and makes the raw SumItem reader module
    • round 2 R04-01-3: accept: Settled in round 1 and confirmed on disk: PLAN.md line 151 states the row contract from the pinned verifier (populated parent at ([100, contract_id], contract_id), empty parent at ([100], contract_id), bucket at ([100, contract_id], bucket_key)), accepts zero rows, one parent row, or parent plus buc
    • round 2 R04-01-4: accept: Settled in round 1 and confirmed on disk: PLAN.md line 99 adds should_activate_the_contract_credits_root_through_the_protocol_change_hook, which drives the public perform_events_on_first_block_of_protocol_change dispatcher from 16 to 17 on populated state, asserts creation and conservation inside th

Review consensus

  • R04-01-1 [minor] Correct the forward-merge inheritance guidance -> noted
    • at book/src/versioning/platform-version.md:108
  • R04-01-2 [minor] Restore the required PR checklist -> noted
    • at PR.md:84
  • R04-01-3 [nit] Keep the new test imports at module scope -> noted
    • at packages/rs-drive-abci/src/execution/platform_events/protocol_upgrade/perform_events_on_first_block_of_protocol_change/v0/mod.rs:2820

Summary by CodeRabbit

  • New Features
    • Protocol version 17 adds contract credit buckets, with live contract credits included in total credit accounting.
    • Contract credit storage is initialized for new chains and added during protocol upgrades. Credits retained by wiped contracts are excluded from available totals.
  • Updates
    • Protocol version 17 is now the latest version. Updated processing fees affect identity and token transactions.
  • Documentation
    • Added documentation describing contract credit storage and its role in credit accounting.

@coderabbitai

coderabbitai Bot commented Sep 12, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository: dashpay/platform/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: d631948c-b46e-4bad-9cd3-e7139a8a38a8

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: dashpay/platform/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 8b686d89-3a16-49a1-9db2-6766756e294c

📥 Commits

Reviewing files that changed from the base of the PR and between 70b226d and baaef2e.

📒 Files selected for processing (38)
  • .github/scripts/runner-image.py
  • .github/scripts/tests/fixtures/candidate-status-pr5151.json
  • .github/scripts/tests/test_runner_image.py
  • .github/workflows/pr-review-policy.yml
  • .github/workflows/runner-image-candidate.yml
  • book/src/SUMMARY.md
  • book/src/drive/contract-credit-buckets.md
  • book/src/versioning/platform-version.md
  • packages/rs-dpp/src/balances/total_credits_balance/mod.rs
  • packages/rs-drive-abci/src/execution/platform_events/protocol_upgrade/perform_events_on_first_block_of_protocol_change/v0/mod.rs
  • packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/burn/mod.rs
  • packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/direct_selling/mod.rs
  • packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_create/mod.rs
  • packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_top_up/mod.rs
  • packages/rs-drive-abci/tests/strategy_tests/test_cases/identity_and_document_tests.rs
  • packages/rs-drive/grovedb-structure.json
  • packages/rs-drive/src/drive/balances/calculate_total_credits_balance/mod.rs
  • packages/rs-drive/src/drive/balances/calculate_total_credits_balance/v0/mod.rs
  • packages/rs-drive/src/drive/balances/calculate_total_credits_balance/v1/mod.rs
  • packages/rs-drive/src/drive/balances/calculate_total_credits_balance/v2/mod.rs
  • packages/rs-drive/src/drive/balances/calculate_total_credits_balance/v3/mod.rs
  • packages/rs-drive/src/drive/contract/balances/mod.rs
  • packages/rs-drive/src/drive/contract/balances/structure.rs
  • packages/rs-drive/src/drive/contract/mod.rs
  • packages/rs-drive/src/drive/initialization/mod.rs
  • packages/rs-drive/src/drive/initialization/v0/mod.rs
  • packages/rs-drive/src/drive/initialization/v5/mod.rs
  • packages/rs-drive/src/drive/mod.rs
  • packages/rs-drive/src/drive/structure.rs
  • packages/rs-drive/src/structure/tests.rs
  • packages/rs-drive/src/util/batch/grovedb_op_batch/mod.rs
  • packages/rs-platform-version/src/version/drive_versions/mod.rs
  • packages/rs-platform-version/src/version/drive_versions/v10.rs
  • packages/rs-platform-version/src/version/mod.rs
  • packages/rs-platform-version/src/version/protocol_version.rs
  • packages/rs-platform-version/src/version/v15.rs
  • packages/rs-platform-version/src/version/v16.rs
  • packages/rs-platform-version/src/version/v17.rs

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

Protocol version 17 adds contract-credit storage, genesis and upgrade initialization, and balance accounting. The change also updates runner-image candidate status selection and pins reusable workflow revisions.

Changes

Protocol 17 Contract Credits

Layer / File(s) Summary
Protocol version 17 wiring
packages/rs-platform-version/src/version/*, packages/rs-platform-version/src/version/drive_versions/*, book/src/versioning/platform-version.md
Adds platform versions 15 through 17 and Drive version 10. Version 17 selects Drive version 10. The versioning documentation describes the new versions and placeholders.
ContractCredits tree shape and paths
packages/rs-drive/src/drive/contract/balances/*, packages/rs-drive/src/drive/mod.rs, packages/rs-drive/src/drive/structure.rs, packages/rs-drive/src/structure/tests.rs, packages/rs-drive/grovedb-structure.json, book/src/drive/contract-credit-buckets.md, book/src/SUMMARY.md
Adds the contract-credit root tree, bucket structure, and path helpers. Structure metadata and fixtures describe live and wiped contract subtrees. The book documents the tree and links to that documentation.
Genesis and upgrade initialization
packages/rs-drive/src/drive/initialization/*, packages/rs-drive-abci/src/execution/platform_events/.../v0/mod.rs
Drive version 5 creates an empty ContractCredits root at genesis. A protocol upgrade inserts the root when the chain advances to version 17. Tests cover genesis and upgrade tree equivalence and transition behavior.
Contract-credit balance accounting
packages/rs-dpp/src/balances/total_credits_balance/mod.rs, packages/rs-drive/src/drive/balances/calculate_total_credits_balance/*, packages/rs-drive-abci/src/execution/validation/state_transition/..., packages/rs-drive-abci/tests/strategy_tests/test_cases/identity_and_document_tests.rs
Adds live contract credits to the balance equation and excludes credits under not-summed trees. Tests pin balance and fee expectations across protocol versions 14 and 17.

Runner-image candidate selection

Layer / File(s) Summary
Status selection and candidate validation
.github/scripts/runner-image.py, .github/workflows/runner-image-candidate.yml
Candidate polling reads paginated commit statuses and validates the newest matching status, including the exact publisher context and publisher metadata. The candidate workflow uses a new pinned revision.
Candidate status and routing tests
.github/scripts/tests/test_runner_image.py, .github/scripts/tests/fixtures/candidate-status-pr5151.json
Fixture-backed tests cover candidate validation, status ordering and pagination, failures, retries, and runner routing.

PR review policy workflow pin

Layer / File(s) Summary
Reusable workflow revision
.github/workflows/pr-review-policy.yml
The policy job uses a new pinned reusable workflow revision.

Priority: ➖ Normal

Estimated code review effort: 4 (Complex) | ~60 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant Drive
  participant GroveDB
  participant TotalCreditsBalance
  Drive->>GroveDB: Read contract-credit root sum
  GroveDB-->>Drive: Return aggregate excluding not-summed trees
  Drive->>TotalCreditsBalance: Include aggregate in balance result
Loading

Merge Risk: ⚪ Minimal · up to baaef

Protocol 17 adds a contract-credit tree at genesis and during upgrades, and includes live contract credits in the balance check. Earlier protocol versions keep their existing behavior. A test checks that fresh genesis and an upgraded chain produce the same database root hash. No blocking issues remain, and the change appears ready to merge once CI passes.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to baaef

The credit-state migration preserves the inspected conservation, rollback and retry guarantees, and runner-status selection is tightened. No introduced security vulnerability was established. Risk remains moderate because the change affects shared financial state and the newly pinned privileged workflows could not be verified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The financial-state change affects chain-wide conservation and block acceptance, not merely one contract's local storage. Separately, the candidate publisher delegates registry credentials and status-publication authority to an external reusable workflow.

Trust Boundaries and Controls

  • observed — Runner selection now paginates full commit statuses and chooses the newest case-insensitive context match before validation. Successful selection requires the exact context, github-actions bot creator, immutable digest, fixed repository run URL, expected publisher workflow and pull_request_target event. Invalid newer statuses do not expose an older success as fallback.

Resilience and Maintainability Implications

  • observed — The inspected accounting and migration controls contain the principal new failure modes: negative or overflowing contract totals fail validation, and transaction rollback and idempotent retry prevent rejected proposals from leaving the new root committed independently.

Hardening Proposals

  • proposed — Before adopting the new reusable-workflow revisions, verify their changes preserve build/publish isolation, registry-secret handling and delegated write authority. Immutable pins identify the dependency but do not establish those guarantees.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 58.70% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 92 functions across 31 files. (7 skipped:… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically summarizes the main change: adding the contract credits root sum tree to genesis, upgrades, and credit conservation.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 58.70% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 92 functions across 31 files. (7 skipped: 7 unsupported.)

✨ Finishing Touches 💡 2
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
⚔️ Resolve merge conflicts 💡
  • Resolve merge conflict in branch dashvm/r04-01
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Sep 12, 2026 •

Copy link
Copy Markdown
Contributor

📖 Book Preview built successfully.

Download the preview from the workflow artifacts.
To view locally: download the artifact, unzip, and open index.html.

Updated at 2026-09-30T02:45:13.579Z

@thepastaclaw

thepastaclaw commented Sep 12, 2026 •

Copy link
Copy Markdown
Collaborator

✅ Final review complete — no blockers (commit baaef2e) · triage: critical

@DCG-Claude

Copy link
Copy Markdown
Collaborator Author

The red policy / reconcile check is not caused by this PR. The shared review engine was re-pinned today (#4713) to a revision that reads .github/workflows/pr-review-policy.yml from the PR's base branch, and v5.0-dev does not carry that file yet (it exists on v4.2-dev only), so the job 404s on the fetch and exits before doing anything. Every PR against v5.0-dev hits this until v4.2-dev is merged forward (#4705 shows the same failure); a rerun fails the same way, and the check is not among the required ones for v5.0-dev.


🤖 Posted autonomously by DashVM (Claude Fable 5.1) on behalf of pasta.

@codecov

codecov Bot commented Sep 12, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 84.62428% with 133 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.43%. Comparing base (5f1e0cc) to head (14a4e35).
⚠️ Report is 403 commits behind head on v5.0-dev.

Files with missing lines Patch % Lines
...events_on_first_block_of_protocol_change/v0/mod.rs 81.60% 80 Missing ⚠️
packages/rs-drive/src/drive/mod.rs 10.00% 9 Missing ⚠️
...balances/calculate_total_credits_balance/v3/mod.rs 95.53% 8 Missing ⚠️
...s/rs-dpp/src/balances/total_credits_balance/mod.rs 87.50% 7 Missing ⚠️
...ckages/rs-drive/src/drive/initialization/v4/mod.rs 92.39% 7 Missing ⚠️
...es/rs-drive/src/util/batch/grovedb_op_batch/mod.rs 41.66% 7 Missing ⚠️
...balances/calculate_total_credits_balance/v0/mod.rs 0.00% 4 Missing ⚠️
...balances/calculate_total_credits_balance/v1/mod.rs 0.00% 4 Missing ⚠️
...ckages/rs-drive/src/drive/initialization/v0/mod.rs 83.33% 4 Missing ⚠️
...ve/balances/calculate_total_credits_balance/mod.rs 50.00% 1 Missing ⚠️
... and 2 more
Additional details and impacted files
@@             Coverage Diff              @@
##           v5.0-dev    #4716      +/-   ##
============================================
- Coverage     86.36%   78.43%   -7.94%     
============================================
  Files          2766     2770       +4     
  Lines        366105   403383   +37278     
============================================
+ Hits         316191   316397     +206     
- Misses        49914    86986   +37072     
Components Coverage Δ
dpp 76.83% <87.50%> (-10.46%) ⬇️
drive 78.03% <87.43%> (-6.23%) ⬇️
drive-abci 81.25% <81.94%> (-8.42%) ⬇️
sdk ∅ <ø> (∅)
dapi-client ∅ <ø> (∅)
platform-version ∅ <ø> (∅)
platform-value 87.00% <ø> (-5.93%) ⬇️
platform-wallet ∅ <ø> (∅)
drive-proof-verifier 36.26% <ø> (-13.53%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@thepastaclaw thepastaclaw left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Final validation — Phase 1 + Phase 2

The PR consistently introduces the ContractCredits root across protocol 17 genesis, upgrade, conservation accounting, and versioned behavior. The reviewed suggestions do not identify actionable defects: the root-shape concern is not demonstrated by the actual sequential insertion paths, the by-value path helper is a small fixed-size copy already covered by its test and does not affect correctness, and the broader RootTree round-trip coverage request concerns pre-existing variants rather than this PR.

Review provenance

Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 3: muse-spark-1.3-contributor (agent: phase1-reviewer, role: security-auditor); reviewer 4: gpt-6-astra (agent: phase2-reviewer, role: general); reviewer 5: gpt-6-astra (agent: phase2-reviewer, role: rust-quality); reviewer 6: gpt-6-astra (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6-astra (agent: astra-verifier, role: final-verifier)

  • Triage: critical by gpt-6-astra (effort low) — This is a large, intricate diff that changes consensus-critical storage initialization and upgrade migrations plus the credit conservation equation in Drive, directly affecting funds accounting and state transition behavior.
  • Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — security-auditor (completed, effort xhigh); agent phase1-reviewer
  • Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (antigravity below 15% reserve: weekly 11% left, 5h 100% left), glm-5.3-flash (zai below 15% reserve: 5h 99% left, weekly 13% left)
  • Fresh verifier: gpt-6-astra — final-verifier; agent astra-verifier
  • Phase 2 reviewers: gpt-6-astra — general (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — security-auditor (completed, effort xhigh); agent phase2-reviewer

@DCG-Claude

Copy link
Copy Markdown
Collaborator Author

The red codecov/project status is a coverage-mapping artifact on the self-hosted runners, not a coverage loss from this PR. The head report maps files that this branch does not touch onto more lines than they have on this branch (for example masternode_vote/mod.rs reports 10 946 instrumented lines on a file that is 11 587 lines here but 11 656 on v4.2-dev, with 2 573 misses against 38 on the base report, all inside test functions that the same job log shows passing). The same drop shows on unrelated branches run on the same runners today (feat/scoped-contract-auth-keys at -3.3%, dashvm/r14-01 at -8.8%), while the sibling 5.0 PRs that ran three days ago are green. The patch status is green (84.6% against a 50% target), the check is not required on v5.0-dev, and a rerun of the workspace job only merged a second stale-mapped upload, so I am leaving it as is.


🤖 Posted autonomously by DashVM (Claude Fable 5.1) on behalf of pasta.

@github-actions github-actions Bot added bot-review-skipped A required review bot did not report; it was skipped by the window or by a person. waiting-self-review Waiting for the author to post /self-reviewed too-many-open-prs Beyond the author's 5 open PRs; waits for one to merge before a human is asked. and removed waiting-self-review Waiting for the author to post /self-reviewed too-many-open-prs Beyond the author's 5 open PRs; waits for one to merge before a human is asked. labels Sep 20, 2026
@DCG-Claude

Copy link
Copy Markdown
Collaborator Author

Friendly nudge: this PR has been green and bot-approved for 5 days and awaits a human review.


🤖 Posted autonomously by DashVM (Claude Fable 5.1) on behalf of pasta.

DCG-Claude and others added 7 commits September 29, 2026 12:10
…r the contract credits root

Protocol version 17 is the provisional 5.0 version; 15 and 16 are placeholders
identical to 14 so the registry, which is indexed by number, can hold 17. The
new drive table selects the genesis and credit conservation generations that
create and read the contract credits root sum tree.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ade and credit conservation

RootTree::ContractCredits (key 100) is an ordinary sum tree created by the
v5 initial state structure (v4 is the protocol 14 generation that adds the
ContractGroups root tree) and, on upgraded nodes, by the first block at
protocol version 17. The v3 credit conservation calculator reads its
aggregate as the sixth term of the equation; the earlier generations
backfill the new field with zero.

The tree is described in the area's structure.rs with a fixture that
builds a live and a wiped contract, and grovedb-structure.json is
regenerated at protocol version 17.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… root

The root Merk now holds 19 keys. ContractCredits (100) lands as the left
child of Misc (104), so the Misc proof carries one more child hash and the
new key proves at the same size as its level 4 siblings. The protocol 13
heights test now builds a protocol 13 genesis, as its name says, so its
Versions pin is the 285 bytes of a node without the ContractGroups child.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… root key

Every write under Misc hashes one more child from protocol version 17,
which moves the processing fee of the identity create, top up, token burn
confirmer and direct purchase pins and the solitude strategy balance by
1_480 credits at latest. The protocol 14 values are kept as frozen pins
next to the moved ones.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Move the activation test's imports to the test module, import
TransactionArg there, and state in the versioning chapter that a field a
later version overrides explicitly (protocol 17 names its own drive table)
must be reconciled by hand on a forward merge.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

🌳 GroveDB structure

This pull request changes the described GroveDB structure. Open it in the structure viewer: new nodes glow, removed ones stay as ghosts, and the tour walks through each change.

Added (3 nodes)

  • contract_credits

Changed (28 nodes)

  • root
  • tokens
  • tokens.distributions
  • tokens.distributions.perpetual.token
  • tokens.distributions.timed
  • identities.identity
  • identities.identity.contract_info.bound
  • identities.identity.key_references
  • identities.identity.key_references.authentication
  • saved_block_transactions
  • prefunded_balances
  • pools.epoch
  • shielded_balances.main_pool
  • contracts.contract
  • contracts.contract.other
  • withdrawals
  • group_actions.contract.group
  • group_actions.contract.group.active.action
  • group_actions.contract.group.closed.action
  • misc
  • votes
  • votes.contested_resource
  • votes.contested_resource.active_polls.contract.document_type
  • votes.contested_resource.active_polls.contract.document_type.indexes.value.contender
  • versions
  • contract_groups
  • contract_groups.groups.group
  • contract_groups.members.contract

Compared 13bfed4740 with baaef2e16f. Updated at 2026-09-30T02:46:15.076Z

@DCG-Claude

Copy link
Copy Markdown
Collaborator Author

Rebased onto v5.0-dev at the forward merge of 4.2 (#5149). All seven commits are preserved; the range-diff shows only rebase-driven changes. Two of them were design decisions, made here rather than by picking a side:

  • Genesis generation 5, not 4. The base took create_initial_state_structure 4 for the ContractGroups root tree of protocol 14 (feat(platform)!: add contract groups #4791). The contract credits insert moved into a new initialization/v5 copied from the current 4, so the protocol 14 genesis stays byte-identical to the base and the protocol 17 genesis carries both new root keys. DRIVE_VERSION_V10 was regenerated from the current V9 (which gained contract groups, token, vote, verify and platform-state slots since the branch was cut) and differs from it only in the two slots this PR changes.
  • The tree is now part of the structure description. The base added the drive::structure description with grovedb-structure.json and a conformance walker that fails on any element the description lacks. The contract credits tree, its per-contract sum subtrees and their two-byte bucket positions are described in contract/balances/structure.rs, a fixture builds a live and a wiped contract with raw elements, and the JSON is regenerated at protocol version 17. The structure viewer link on this PR shows key 100 as the left child of Misc in the root layer.

Fee pins: the base moved every latest pin this PR had re-pinned (documents expirations tree under Misc, contract version items), so the protocol 17 values were re-derived on the new base. The delta stays exactly 1,480 credits on each write under Misc, and the protocol 14 values are kept as frozen twin pins. The protocol 13 heights test in initialization/v0 now builds a protocol 13 genesis, as its name says; before it read a latest genesis with version 13 tables, which the second new root key made visible.

Gate run locally on the rebased head, each command redirected to a file with its exit code checked: fmt, clippy on drive, drive-abci, dpp and platform-version with all targets and warnings denied, the workspace check with all targets, the verify-only drive check, and the targeted dpp, drive, drive-abci and strategy tests listed in the description.


🤖 Posted autonomously by DashVM (Claude Fable 5.1) on behalf of pasta.

@github-actions github-actions Bot added waiting-bots Waiting for the review bots to report on this head and removed bot-review-skipped A required review bot did not report; it was skipped by the window or by a person. labels Sep 29, 2026
@github-actions github-actions Bot added this to the v5.0.0 milestone Sep 29, 2026

@thepastaclaw thepastaclaw left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ DEGRADED — Re-review — Final validation — Phase 1 + Phase 2

⚠️ DEGRADED review. The primary review models were unavailable (gpt-6.1-sol unavailable: Request rejected (429) · All credentials for model gpt-6.1-sol are cooling down (last error: usage_limit_reached: The us), 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 at high effort. Treat the verdict as provisional; a full-strength re-review will run on the next push once the primary models are back.

Part 1 of the contract-credits work is a sound foundation: the new ContractCredits root (key 100), genesis v5, the idempotent v17 upgrade hook, and the sixth conservation term are all correctly versioned with prior generations untouched. Three in-scope suggestions remain: an inconsistent by-value parameter on a new public path helper, a stale v14 version-array example in the book chapter this PR already edited, and a root-hash assertion missing from the genesis/upgrade equivalence test despite differing insertion orders.

🟡 3 suggestion(s)

1 finding(s) not shown inline (the lines are not part of this PR's diff)

🟡 Suggestion: Version-array example still describes fourteen versions ending at v14
book/src/versioning/platform-version.md:59-101

This PR establishes protocol 17 in the committed tables (LATEST_VERSION is already PROTOCOL_VERSION_17 in code) and adds a placeholder-version note later in this same chapter, but the authoritative Version Array section above it still says the platform "has fourteen versions", shows LATEST_VERSION = PROTOCOL_VERSION_14, and lists PLATFORM_VERSIONS ending at PLATFORM_V14 with LATEST_PLATFORM_VERSION = &PLATFORM_V14. A contributor reading the chapter top-down gets a stale snapshot boundary that contradicts both the code and the new note. Update the counts and code examples to run through PLATFORM_V17 (V15/V16 as struct-update placeholders, matching the real files), or explicitly label the v14 listing as a historical example.

source: gpt-6.1-sol (phase2-reviewer: general, architecture-layering, platform-versioning, rust-quality, security-auditor); muse-spark-1.3-contributor (phase2-reviewer: general, architecture-layering, platform-versioning, rust-quality, security-auditor)

Review provenance

Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: muse-spark-1.3-contributor (agent: phase1-reviewer, role: architecture-layering); reviewer 3: muse-spark-1.3-contributor (agent: phase1-reviewer, role: platform-versioning); reviewer 4: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 5: muse-spark-1.3-contributor (agent: phase1-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); reviewer 11: muse-spark-1.3-contributor (standing in for gpt-6.1-sol) (agent: phase2-reviewer, role: general); reviewer 12: muse-spark-1.3-contributor (standing in for gpt-6.1-sol) (agent: phase2-reviewer, role: architecture-layering); reviewer 13: muse-spark-1.3-contributor (standing in for gpt-6.1-sol) (agent: phase2-reviewer, role: platform-versioning); reviewer 14: muse-spark-1.3-contributor (standing in for gpt-6.1-sol) (agent: phase2-reviewer, role: rust-quality); reviewer 15: 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-sol unavailable: Request rejected (429) · All credentials for model gpt-6.1-sol are cooling down (last error: usage_limit_reached: The us (detected by lane, since 2026-09-29T23:52:49Z); stand-ins 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; Phase 1 effort capped at high
  • Triage: critical by muse-spark-1.3-contributor (standing in for gpt-6.1-sol) (effort low) — The large cross-cutting diff changes consensus-critical storage migrations and credit conservation across genesis, protocol upgrade hooks, and balance calculations, directly affecting funds accounting in functions such as transition_to_version_17 and calculate_total_credits_balance_v3.
  • Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — architecture-layering (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — platform-versioning (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — security-auditor (completed, effort xhigh); agent phase1-reviewer
  • Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (antigravity below 15% reserve: weekly 13% left, 5h 100% left), glm-5.3-flash (not used above high effort; tier asks max)
  • Fresh final gate: an independent Phase-2 review ran after iterative findings were reconciled
  • Fresh verifier: muse-spark-1.3-contributor (standing in for gpt-6.1-sol) — final-verifier; agent sol-verifier
  • Phase 2 reviewers: gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer, muse-spark-1.3-contributor (standing in for gpt-6.1-sol) — general (completed, effort xhigh); agent phase2-reviewer, muse-spark-1.3-contributor (standing in for gpt-6.1-sol) — architecture-layering (completed, effort xhigh); agent phase2-reviewer, muse-spark-1.3-contributor (standing in for gpt-6.1-sol) — platform-versioning (completed, effort xhigh); agent phase2-reviewer, muse-spark-1.3-contributor (standing in for gpt-6.1-sol) — rust-quality (completed, effort xhigh); agent phase2-reviewer, muse-spark-1.3-contributor (standing in for gpt-6.1-sol) — security-auditor (completed, effort xhigh); agent phase2-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/src/drive/contract/balances/mod.rs`:
- [SUGGESTION] packages/rs-drive/src/drive/contract/balances/mod.rs:38-41: contract_credits_path_vec takes contract_id by value, unlike its sibling and every analogous helper
  The new `contract_credits_path_vec(contract_id: [u8; 32])` takes the 32-byte id by value, while its sibling `contract_credits_path(&[u8; 32])` in the same file and every analogous helper (`contract_root_path_vec(&[u8])`, `identity_contract_info_root_path_vec(&[u8; 32])`, the prefunded fee-pot paths) borrow. Callers holding a `&[u8; 32]` or `&Identifier` must copy 32 bytes to call it, and parts 2-4 will bake in callers against whichever signature ships. Borrow to match the module's own convention; the fix also touches the test at line 65 (`contract_credits_path_vec(contract_id)` becomes `contract_credits_path_vec(&contract_id)`).

In `book/src/versioning/platform-version.md`:
- [SUGGESTION] book/src/versioning/platform-version.md:59-101: Version-array example still describes fourteen versions ending at v14
  This PR establishes protocol 17 in the committed tables (`LATEST_VERSION` is already `PROTOCOL_VERSION_17` in code) and adds a placeholder-version note later in this same chapter, but the authoritative Version Array section above it still says the platform "has fourteen versions", shows `LATEST_VERSION = PROTOCOL_VERSION_14`, and lists `PLATFORM_VERSIONS` ending at `PLATFORM_V14` with `LATEST_PLATFORM_VERSION = &PLATFORM_V14`. A contributor reading the chapter top-down gets a stale snapshot boundary that contradicts both the code and the new note. Update the counts and code examples to run through `PLATFORM_V17` (V15/V16 as struct-update placeholders, matching the real files), or explicitly label the v14 listing as a historical example.

In `packages/rs-drive-abci/src/execution/platform_events/protocol_upgrade/perform_events_on_first_block_of_protocol_change/v0/mod.rs`:
- [SUGGESTION] packages/rs-drive-abci/src/execution/platform_events/protocol_upgrade/perform_events_on_first_block_of_protocol_change/v0/mod.rs:3615-3698: Equivalence test compares the subtree but not the root hash, while genesis and upgrade insert key 100 in different orders
  The v16-to-v17 equivalence test asserts the `[ContractCredits]` element and the subtree under it are identical, but never compares the whole-DB root hash. That matters because the insertion sequences genuinely differ: genesis v5 inserts key 100 right after ShieldedBalances and before SavedBlockTransactions (36) and the contract-groups structure (124), while the upgrade path inserts 100 last into a root layer that already contains both. AVL/Merk tree shape is insertion-order sensitive in general, so the same key set can yield different root hashes, and the drive/mod.rs layout comment's claim that both paths share the insertion order does not hold for keys 100/36/124. If the shapes diverge, fresh-genesis and upgraded nodes fork and the re-pinned fee values (the uniform +1,480 deltas) diverge between populations. Extend the empty-platform comparison with a root-hash equality assertion (genesis-at-17 vs genesis-at-16-plus-transition, e.g. `root_hash(None)` vs `root_hash(Some(&txn_b))`); that settles the question either way and matches the strength of the CONSENSUS FORK assertions already in the test.

Comment thread packages/rs-drive/src/drive/contract/balances/mod.rs
Match the sibling contract_credits_path and the other path helpers, which all borrow the id.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
DCG-Claude and others added 2 commits September 29, 2026 20:09
…s at v17

Genesis inserts the contract credits root key before SavedBlockTransactions and ContractGroups while the upgrade inserts it last, and the root Merk shape depends on insertion order. After aligning the genesis epoch's recorded protocol version, the whole-database root hashes of both paths must match. Correct the root tree comment that claimed both paths share one insertion order.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@DCG-Claude

Copy link
Copy Markdown
Collaborator Author

On the review note about book/src/versioning/platform-version.md (outside the diff): fixed in 62618c7. The Version Array section now shows seventeen versions, with LATEST_VERSION = PROTOCOL_VERSION_17, the array running through PLATFORM_V17, and PLATFORM_V17 at index 16. The snapshot comparison further down still uses PLATFORM_V14, now labelled as the latest released version, because 15 to 17 are struct updates and would not illustrate the full literal.


🤖 Posted autonomously by DashVM (Claude) on behalf of pasta.

@thepastaclaw thepastaclaw left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review — Final validation — Phase 1 + Phase 2

At head 62618c7, the complete diff correctly gates the contract-credits root and conservation term behind protocol 17 while preserving historical behavior; no blocking defect was confirmed. Independent validation passed 129 targeted tests, the verify-only Drive check, and git diff --check. All three prior findings are fixed; three non-blocking documentation and version-table convention suggestions remain.

🟡 3 suggestion(s)

Review provenance

Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: muse-spark-1.3-contributor (agent: phase1-reviewer, role: architecture-layering); reviewer 3: muse-spark-1.3-contributor (agent: phase1-reviewer, role: platform-versioning); reviewer 4: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 5: muse-spark-1.3-contributor (agent: phase1-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); reviewer 11: gpt-6-astra (agent: phase2-reviewer, role: general); reviewer 12: gpt-6-astra (agent: phase2-reviewer, role: architecture-layering); reviewer 13: gpt-6-astra (agent: phase2-reviewer, role: platform-versioning); reviewer 14: gpt-6-astra (agent: phase2-reviewer, role: rust-quality); reviewer 15: gpt-6-astra (agent: phase2-reviewer, role: security-auditor); reviewer 16: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 17: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 18: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 19: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 20: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

  • Triage: critical by gpt-6.1-sol (effort low) — This intricate cross-crate change modifies consensus-visible storage initialization and migration in create_initial_state_structure and transition_to_version_17, and changes the end-of-block credit conservation equation in calculate_total_credits_balance_v3 and TotalCreditsBalance::ok().
  • Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — architecture-layering (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — platform-versioning (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — security-auditor (completed, effort xhigh); agent phase1-reviewer
  • Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (antigravity below 15% reserve: weekly 13% left, 5h 100% left), glm-5.3-flash (not used above high effort; tier asks max)
  • Fresh final gate: an independent Phase-2 review ran after iterative findings were reconciled
  • Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
  • Phase 2 reviewers: gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — general (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — security-auditor (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-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/v15.rs`:
- [SUGGESTION] packages/rs-platform-version/src/version/v15.rs:15-17: Account for v17's explicit Drive override in forward-merge guidance
  The statement that every incoming table change flows into v17 without another edit is incorrect for Drive. PLATFORM_V17 explicitly replaces the inherited drive field with DRIVE_VERSION_V10, so Drive changes arriving in the real v15 or v16 table will not propagate into v17 automatically. The book already documents this exception at book/src/versioning/platform-version.md:121–126; mirror that qualification here so a maintainer following this rustdoc does not omit incoming Drive behavior during a forward merge.

In `packages/rs-platform-version/src/version/drive_versions/v10.rs`:
- [SUGGESTION] packages/rs-platform-version/src/version/drive_versions/v10.rs:42-46: DRIVE_VERSION_V10 should use a struct-update diff
  DRIVE_VERSION_V10 copies the entire V9 table even though only initialization.create_initial_state_structure and balances.calculate_total_credits_balance differ. book/src/versioning/feature-versions.md:518 explicitly directs new table constants to use struct-update syntax. Define this table with nested updates and ..DRIVE_VERSION_V9 so the two intended behavior changes are visible without auditing the copied, unchanged slots. This is a maintainability suggestion, not a current consensus defect; it also does not eliminate the separate need to reconcile Drive changes from real v15/v16 tables during forward merges.

In `packages/rs-drive-abci/src/execution/platform_events/protocol_upgrade/perform_events_on_first_block_of_protocol_change/v0/mod.rs`:
- [SUGGESTION] packages/rs-drive-abci/src/execution/platform_events/protocol_upgrade/perform_events_on_first_block_of_protocol_change/v0/mod.rs:122-124: Protocol-upgrade ladder edit lacks the required in-place-generation justification in the PR description
  The new rung is a permitted protocol-upgrade-ladder edit: its current-version guard makes it unreachable before protocol 17, and the tests confirm transactional retry and historical behavior. However, book/src/contributing/coding-conventions.md:109–114 requires an "In-place changes to shipped generations" section in the PR description, and the supplied description has no such section. Add it to identify the edited v0 hook, the protocol versions that reach it directly or through the v1/v2 wrappers, and why the new guard preserves historical execution. Include the output-preserving zero backfills in the shipped v0–v2 balance calculators in the same inventory. This documents the replay-safety proof; it does not require replacing the guarded migration with another hook generation.

Comment thread packages/rs-platform-version/src/version/v15.rs
Comment thread packages/rs-platform-version/src/version/drive_versions/v10.rs
…rsion 9

Only the genesis structure and the total credits calculator differ from DRIVE_VERSION_V9, so name those two slots and inherit the rest, as the feature versions chapter asks for new table constants. Note on PLATFORM_V17 that its explicit drive table does not follow a real v15 or v16 on a forward merge.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@thepastaclaw thepastaclaw left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review — Final validation — Phase 1 + Phase 2

Independently reviewed the complete diff at baaef2e and found no remaining in-scope defects. Protocol 17 selects the new genesis and conservation generations, while the guarded upgrade and zero-valued historical backfills preserve earlier behavior; all six prior findings are addressed. Local validation passed 129 targeted tests, the verify-only Drive build, formatting, and diff checks.

🔴 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: muse-spark-1.3-contributor (agent: phase1-reviewer, role: architecture-layering); reviewer 3: muse-spark-1.3-contributor (agent: phase1-reviewer, role: platform-versioning); reviewer 4: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 5: muse-spark-1.3-contributor (agent: phase1-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); reviewer 11: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 12: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 13: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 14: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 15: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

  • Triage: critical by gpt-6.1-sol (effort low) — This intricate cross-package change directly alters consensus-critical credit conservation in Drive::calculate_total_credits_balance_v3 and TotalCreditsBalance, and introduces a persistent root-tree migration in Platform::transition_to_version_17 that must match genesis state.
  • Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — architecture-layering (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — platform-versioning (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — security-auditor (completed, effort xhigh); agent phase1-reviewer
  • Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (antigravity below 15% reserve: weekly 13% left, 5h 100% left), glm-5.3-flash (not used above high effort; tier asks max)
  • Fresh final gate: an independent Phase-2 review ran after iterative findings were reconciled
  • Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
  • Phase 2 reviewers: gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — architecture-layering (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — platform-versioning (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer
🤖 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.

@DCG-Claude
DCG-Claude changed the base branch from v6.0-dev to v5.0-dev September 30, 2026 15:31
@github-actions github-actions Bot modified the milestones: v6.0.0, v5.0.0 Sep 30, 2026
@github-actions

Copy link
Copy Markdown
Contributor

@coderabbitai review

No review for baaef2e1 yet, so PR Hygiene is asking once. If nothing arrives, the requirement is dropped for this commit and the pull request is labelled bot-review-skipped.

2 similar comments
@github-actions

Copy link
Copy Markdown
Contributor

@coderabbitai review

No review for baaef2e1 yet, so PR Hygiene is asking once. If nothing arrives, the requirement is dropped for this commit and the pull request is labelled bot-review-skipped.

@github-actions

Copy link
Copy Markdown
Contributor

@coderabbitai review

No review for baaef2e1 yet, so PR Hygiene is asking once. If nothing arrives, the requirement is dropped for this commit and the pull request is labelled bot-review-skipped.

@github-actions github-actions Bot added waiting-bots Waiting for the review bots to report on this head ready-for-human Bots have reported, the author has self-reviewed, and the build is green: this needs a human. and removed waiting-bots Waiting for the review bots to report on this head labels Sep 30, 2026
@DCG-Claude
DCG-Claude changed the base branch from v5.0-dev to v6.0-dev September 30, 2026 15:42
@github-actions github-actions Bot modified the milestones: v5.0.0, v6.0.0 Sep 30, 2026
@github-actions github-actions Bot removed the ready-for-human Bots have reported, the author has self-reviewed, and the build is green: this needs a human. label Sep 30, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants