Skip to content

fix(drive-abci)!: pay every reward share of a masternode and credit each identity once in the epoch payout (PV15) - #4986

Draft
QuantumExplorer wants to merge 9 commits into
v4.3-devfrom
claude/epoch-payout-credit-once
Draft

QuantumExplorer wants to merge 9 commits into
v4.3-devfrom
claude/epoch-payout-credit-once

Conversation

@QuantumExplorer

@QuantumExplorer QuantumExplorer commented Sep 24, 2026 •

Copy link
Copy Markdown
Member

Issue being fixed or feature implemented

When an epoch is paid out, add_epoch_pool_to_proposers_payout_operations builds one AddToIdentityBalance operation for each masternode reward share (payToId) and one for each proposer. Each is converted against the identity's balance before the batch and writes previous balance + credit.

Two gaps in how that payout credits its recipients:

  • Only one reward share per masternode was ever read. fetch_reward_shares_list_for_masternode v0 queries with limit: Some(1). The contract's only unique index is (ownerId, payToId), so a masternode may have several shares, and every share after the first was never paid; its part stayed with the masternode.
  • An identity owed two credits in one payout. Two masternodes naming the same payToId, or a proposer that is also a payToId, give one identity two operations that both start from the same balance. Generation 0 converted them into one grove batch, which keeps only the last write to a key, so the identity received one of its credits and the payout no longer added up to what left the epoch's pools.

#4985 (merged) already gave the payout a generation 1 that hands its credits to the block's apply_drive_operations, which at protocol version 14 merges every write of one identity balance into one, skips a share whose payToId has no balance, and caps each share at what is left of the masternode's payout. This PR builds on that generation in place.

No rewardShare document can be written at any protocol version so far (the reject data trigger covers create, replace and delete in every trigger binding list), so every payout made so far credited distinct proposer identities and is unaffected.

TODO

  • Move from protocol version 14 to protocol version 15. Reward shares are not part of 4.2, so this PR now targets v4.3-dev and everything it gates at 14 moves to 15, once protocol version 15 exists (v4.3-dev only has v14.rs so far):
    • bring the branch onto v4.3-dev (against v4.3-dev the diff also shows the v4.2-dev commits it does not have yet, #4982, #4983 and #4985);
    • add_epoch_pool_to_proposers_payout_operations v1 ships with protocol version 14 as #4985 left it, so it goes back to that version, and the per-identity sum, the usize proposer index and the checked last-payout add move into a new v2 selected at 15;
    • version tables: DRIVE_ABCI_METHOD_VERSIONS_V10 goes back to its shipped values (fetch_reward_shares_list_for_masternode 0, the payout slot and its comment as fix(platform)!: credit repaid identity debt to the processing fee pool #4985 left them), and the protocol version 15 table selects fetch_reward_shares_list_for_masternode 1 and the payout v2;
    • v14.rs item 43 moves to the protocol version 15 notes;
    • the dispatcher tests on the latest version follow the latest version (15), and a protocol version 14 test shows generation 1 unchanged;
    • update the comments and docs that say protocol version 14.

What was done?

  • New generation fetch_reward_shares_list_for_masternode v1, selected at protocol version 14 for now (DRIVE_ABCI_METHOD_VERSIONS_V10; moves to 15, see TODO). It returns every reward share of the masternode (limit: None) and moves the queried documents instead of cloning them. v0 is unchanged; its tests are pinned to protocol version 13.
  • add_epoch_pool_to_proposers_payout_operations v1 (from fix(platform)!: credit repaid identity debt to the processing fee pool #4985, edited in place for now; moves to a v2 at 15, see TODO):
    • adds up everything the payout owes each identity (its reward shares and its own proposer payout) in a BTreeMap, then hands the block one AddToIdentityBalance per identity. The payout no longer depends on the batch merging its writes.
    • shares are paid in the order they are read (document id order); with fix(platform)!: credit repaid identity debt to the processing fee pool #4985's cap, a share larger than what is left of the masternode's payout gets what is left.
    • the last proposer's payout uses a checked add, and the proposer index and count stay usize instead of being cast to u16.
  • v0 of the payout is unchanged. Its test is pinned to protocol version 13.
  • v14.rs records the change as item 43, after fix(platform)!: credit repaid identity debt to the processing fee pool #4985's item 42.
  • create_test_mn_share_document in the drive-abci test helpers is now pub and takes the recipient's id, so a test can name any identity, or none. It derives the document id from the owner and payToId (a unique pair under the contract's index, exposed as test_mn_share_document_id) instead of Identifier::random().

Example: two shares from one masternode, P1 sharing 30% with R and 20% with P2. Each masternode's payout is m.

Before: only the first share read is paid; the other stays with P1
After:  R receives 0.30 m, P2 receives 0.20 m on top of its own m, P1 keeps 0.50 m

Shares over the whole payout, P1 sharing 80% with R and 50% with P2:

After: the share read first (lower document id) is paid in full, the other gets the rest, P1 keeps 0

One identity owed several credits, where P1 and P2 each share with R (30% and 20%) and P3 shares 10% with P1:

Generation 0 (protocol version 13):
R:  receives 0.20 m           (P2's share; P1's 0.30 m share is lost)
P1: receives 0.10 m           (P3's share; its own 0.70 m is lost)
credits in trees < total credits in platform

Generation 1 (protocol version 14):
R:  receives 0.30 m + 0.20 m, in one operation
P1: receives 0.70 m + 0.10 m, in one operation
credits in trees == total credits in platform

How Has This Been Tested?

The dispatcher tests below run through the distribution dispatcher and apply the payout the way the block does (in the epoch after the paid one), so the epoch is marked paid and the credit sum can be checked. They run on the Drive batching configuration a node ships with (batching_consistency_verification off).

  • should_credit_an_identity_everything_one_payout_owes_it (latest protocol version): the shared-recipient payout above. Each identity's balance equals what it is owed, and calculate_total_credits_balance balances.

  • should_repay_a_recipients_debt_once_from_everything_one_payout_owes_it (latest): the same payout with R holding a debt. The debt comes out of R's combined credit once, R's negative balance ends at 0, and the credit sum balances (the repaid debt reaches the processing fee pool).

  • should_leave_a_share_naming_no_identity_with_its_masternode (latest): P1 shares 30% with an id no identity holds. Every proposer gets its whole payout and the credit sum balances.

  • should_pay_every_reward_share_of_one_masternode (latest): P1 shares with R and P2. Both are paid and the credit sum balances. It fails with fetch_reward_shares_list_for_masternode at 0.

  • should_pay_shares_over_the_whole_payout_until_it_is_used_up (latest): P1 shares 130% of its payout. The share with the lower document id is paid in full, the other gets the rest, P1 keeps 0, and the credit sum balances. The shares are written in both orders and the balances are the same.

  • should_reproduce_the_lost_credits_of_generation_0_at_protocol_version_13: the shared-recipient payout at protocol version 13. R and P1 each miss a credit, and the credit sum is short by exactly the missing credits.

  • fix(platform)!: credit repaid identity debt to the processing fee pool #4985's generation 1 tests in v1/mod.rs keep passing.

  • cargo test -p drive-abci --lib -- add_epoch_pool_to_proposers_payout_operations fetch_reward_shares_list_for_masternode: 13 passed.

  • cargo test -p drive-abci --lib -- fee_pool block_processing_end_events: 44 passed.

  • cargo test -p platform-version: passed.

  • cargo fmt --all, cargo clippy -p drive-abci --all-features --all-targets -- -D warnings and cargo check --workspace --all-targets are clean.

Breaking Changes

Consensus change at protocol version 15 once the TODO is done (at 14 on the branch today): every reward share of a masternode is paid (up to its payout), and the payout credits each identity with one operation. Earlier protocol versions keep generation 0 of both methods.

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
  • If I added or changed GroveDB structure, I described it in the area's structure.rs, regenerated grovedb-structure.json, and checked the structure viewer link posted on this pull request

For repository code-owners and collaborators only

  • I have assigned this pull request to a milestone

🤖 Generated with Claude Code

… it (PV14)

The epoch payout built one AddToIdentityBalance per reward share and per
proposer. Each is converted against the balance before the batch, so an
identity owed two credits in one payout (two masternodes naming the same
payToId, or a proposer that is also a payToId) received only one of them.

add_epoch_pool_to_proposers_payout_operations v1 (protocol version 14)
adds up the credits per identity first and builds one operation each.
Generation 0 stays as it is; no reward share document can be written at
any protocol version so far.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added this to the v4.2.0 milestone Sep 24, 2026
@coderabbitai

coderabbitai Bot commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

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

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: 881856ac-4314-405f-b0de-9b4c83b83f6e

📥 Commits

Reviewing files that changed from the base of the PR and between 9eb59ec and 8c73d84.

📒 Files selected for processing (6)
  • packages/rs-drive-abci/src/execution/platform_events/fee_pool_outwards_distribution/add_epoch_pool_to_proposers_payout_operations/mod.rs
  • packages/rs-drive-abci/src/execution/platform_events/fee_pool_outwards_distribution/add_epoch_pool_to_proposers_payout_operations/v0/mod.rs
  • packages/rs-drive-abci/src/execution/platform_events/fee_pool_outwards_distribution/add_epoch_pool_to_proposers_payout_operations/v1/mod.rs
  • packages/rs-drive-abci/src/test/helpers/fee_pools.rs
  • packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_method_versions/v10.rs
  • packages/rs-platform-version/src/version/v14.rs

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


📝 Walkthrough

Walkthrough

The payout method now supports generation 1. It allocates proposer and reward-share credits, combines credits owed to the same identity, and uses the configured method version to select the implementation.

Changes

Epoch proposer payouts

Layer / File(s) Summary
Calculate and accumulate payouts
packages/rs-drive-abci/src/execution/platform_events/fee_pool_outwards_distribution/add_epoch_pool_to_proposers_payout_operations/v1/mod.rs, packages/rs-drive-abci/src/test/helpers/fee_pools.rs, packages/rs-drive-abci/src/execution/platform_events/fee_pool_outwards_distribution/add_epoch_pool_to_proposers_payout_operations/mod.rs
Generation 1 allocates proposer and reward-share credits, combines credits by identity, and batches balance operations. Tests cover payout balances, shared recipients, recipient debt, and missing recipient identities.
Route and select the payout generation
packages/rs-drive-abci/src/execution/platform_events/fee_pool_outwards_distribution/add_epoch_pool_to_proposers_payout_operations/mod.rs, packages/rs-drive-abci/src/execution/platform_events/fee_pool_outwards_distribution/add_epoch_pool_to_proposers_payout_operations/v0/mod.rs, packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_method_versions/v10.rs, packages/rs-platform-version/src/version/v14.rs
The dispatcher accepts generation 1, and the method version configuration selects it. The release note describes its payout behavior. Tests record generation 0 behavior at protocol version 13.

Priority: ➖ Normal

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

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant PayoutMethod as Generation 1 payout method
  participant IdentityBalance as Identity balance lookup
  participant BalanceOperations as AddToIdentityBalance operations
  PayoutMethod->>IdentityBalance: Check payToId balance
  IdentityBalance-->>PayoutMethod: Balance availability
  PayoutMethod->>BalanceOperations: Accumulate credits by identity
  PayoutMethod->>BalanceOperations: Batch converted operations
Loading

Suggested reviewers: thepastaclaw

Merge Risk: ⚪ Minimal · up to 8c73d

The version-14 payout change has no identified merge-blocking issue and is ready for normal merge checks.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 16 functions across 6 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
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.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main changes: paying reward shares and aggregating each identity’s epoch payout into one credit. The parenthetical “PV15” does not match the provided objective, which i…
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

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 added the waiting-bots Waiting for the review bots to report on this head label Sep 24, 2026
@thepastaclaw

thepastaclaw commented Sep 24, 2026 •

Copy link
Copy Markdown
Collaborator

🕓 Review not started yet because this PR is a draft.

  • Request normal review — click when the PR is ready for review.
  • Request priority review — click to move this review to the front of the queue.

Commit e231b28. Normal review starts when eligible; priority review starts as soon as a slot is available.

@github-actions github-actions Bot added the bot-review-skipped A required review bot did not report; it was skipped by the window or by a person. label Sep 24, 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.

Final validation — Phase 2 only (queue backlog)

The protocol-14 payout implementation correctly aggregates all credits owed to each identity before creating balance operations, while protocol 13 remains on the unchanged generation 0 path. The dispatcher, version table, deterministic ordering, checked arithmetic, and regression coverage are consistent with the stated fix; no actionable in-scope defects were identified.

Review provenance

Source: reviewer 1: gpt-6-astra (agent: phase2-reviewer, role: general); reviewer 2: gpt-6-astra (agent: phase2-reviewer, role: architecture-layering); reviewer 3: gpt-6-astra (agent: phase2-reviewer, role: platform-versioning); reviewer 4: gpt-6-astra (agent: phase2-reviewer, role: rust-quality); reviewer 5: 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) — The diff is a substantial change to add_epoch_pool_to_proposers_payout_operations that alters consensus-visible funds movement and identity balance credits across protocol generations.
  • Phase 1 reviewers: not run (skipped for throughput: 18 PRs queued, above the 10 limit)
  • 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 — 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

@github-actions

github-actions Bot commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Bots are done — your move: post /self-reviewed.
Full checklist in the description.

@github-actions github-actions Bot added waiting-self-review Waiting for the author to post /self-reviewed and removed waiting-bots Waiting for the review bots to report on this head labels Sep 24, 2026
…sternode (PV14)

Review follow-ups on the payout's generation 1:

- a reward share whose payToId names no identity stays with its
  masternode instead of failing the payout
- the last proposer's payout uses a checked add, and the proposer index
  and count are no longer truncated to u16
- tests: a recipient with a debt repays it once from its combined credit,
  a share naming no identity, the PV13 test framed as a reproduction, the
  v1 test named with "should"
- the share document helper derives its id from the owner and payToId
  instead of an unseeded random id
- the dispatcher and v1 docs name what the method returns

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added waiting-bots Waiting for the review bots to report on this head and removed waiting-self-review Waiting for the author to post /self-reviewed bot-review-skipped A required review bot did not report; it was skipped by the window or by a person. labels Sep 25, 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.

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

The PV14 dispatch and per-identity payout aggregation are correctly versioned, and the prior-generation behavior remains isolated. One blocking correctness gap remains: the new implementation still uses a reward-share query capped at one document, so it does not process every reward share permitted for a masternode owner.

🔴 1 blocking

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: gpt-6-astra (agent: phase2-reviewer, role: general); reviewer 6: gpt-6-astra (agent: phase2-reviewer, role: architecture-layering); reviewer 7: gpt-6-astra (agent: phase2-reviewer, role: platform-versioning); reviewer 8: gpt-6-astra (agent: phase2-reviewer, role: rust-quality); reviewer 9: gpt-6-astra (agent: phase2-reviewer, role: general); reviewer 10: gpt-6-astra (agent: phase2-reviewer, role: architecture-layering); reviewer 11: gpt-6-astra (agent: phase2-reviewer, role: platform-versioning); reviewer 12: gpt-6-astra (agent: phase2-reviewer, role: rust-quality); final verifier: gpt-6-astra (agent: astra-verifier, role: final-verifier)

  • Triage: critical by gpt-6-astra (effort low) — The substantial v1 rewrite of add_epoch_pool_to_proposers_payout_operations changes consensus payout and identity-balance crediting logic, directly affecting funds movement in a critical Drive ABCI surface.
  • 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
  • Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (antigravity below 15% reserve: weekly 67% left, 5h 0% 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-astra — final-verifier; agent astra-verifier
  • Phase 2 reviewers: 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 — 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
🤖 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/execution/platform_events/fee_pool_outwards_distribution/add_epoch_pool_to_proposers_payout_operations/v1/mod.rs`:
- [BLOCKING] packages/rs-drive-abci/src/execution/platform_events/fee_pool_outwards_distribution/add_epoch_pool_to_proposers_payout_operations/v1/mod.rs:104-110: v1 still processes only one reward-share document per owner
  This generation obtains reward shares through `fetch_reward_shares_list_for_masternode`, whose v0 query sets `limit: Some(1)`. The reward-share schema's `ownerId` index is non-unique; only the combined `(ownerId, payToId)` index is unique, so one owner can have multiple valid reward-share documents with different recipients. In that state, every document after the first is omitted: its recipient is not credited and its percentage is not subtracted from `masternode_payout_leftover`, leaving the masternode with funds that should have been distributed. That contradicts v1's stated guarantee of crediting everything owed by the payout. Remove the one-document limit through a versioned all-documents query path and add a regression test with multiple shares owned by the same masternode.
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.

  • Drive-side balance additions remain non-compositional outside the payout path — The Drive implementation of add_to_identity_balance_operations_v0 reads the pre-batch balance and materializes a replacement value, so repeated AddToIdentityBalance operations for one identity can still lose credits. Batch transition conversion flattens multiple document and token actions, including purchase actions that can pay the same seller or contract owner. This is a concrete pre-existing issue, but this PR correctly fixes the epoch-payout caller by aggregating credits in rs-drive-abci and does not introduce or widen the Drive-side behavior.
    • Follow-up: Track a separate versioned rs-drive fix that coalesces repeated identity-balance additions, with tests covering multiple purchases and other callers.

@github-actions

github-actions Bot commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

Your move: thepastaclaw requested changes on this head; dismiss the review or push a fix; thepastaclaw left review threads unresolved; resolve them.
Full checklist in the description.

@github-actions github-actions Bot added waiting-self-review Waiting for the author to post /self-reviewed and removed waiting-bots Waiting for the review bots to report on this head labels Sep 25, 2026
QuantumExplorer and others added 3 commits September 25, 2026 17:14
…n every replace of a transferable document (PV14) (#4982)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…nce (PV14) (#4983)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
… payout (PV14)

fetch_reward_shares_list_for_masternode v0 asked for one document, so a
masternode's other reward shares were never paid. v1 (protocol version
14) returns all of them.

With every share counted, a masternode's shares could add up to more
than its payout, which would fail the payout on the leftover
subtraction. The payout's generation 1 now pays the shares in the order
they are read until the masternode's payout is used up; a share larger
than what is left gets what is left.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added waiting-bots Waiting for the review bots to report on this head and removed waiting-self-review Waiting for the author to post /self-reviewed labels Sep 25, 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.

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

Verified the complete diff at dfd3fa7: the prior single-share blocker is fixed, both new generations activate only at PV14, and earlier production behavior is preserved. Two non-blocking suggestions remain: pin the order-sensitive test's expected balances and avoid cloning owned query results. Local validation passed 11 focused payout/query tests, 41 surrounding fee-pool/block-end tests, 25 platform-version tests, and git diff --check; the worktree remains unchanged.

🟡 2 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-astra (agent: phase2-reviewer, role: general); reviewer 7: gpt-6-astra (agent: phase2-reviewer, role: architecture-layering); reviewer 8: gpt-6-astra (agent: phase2-reviewer, role: platform-versioning); reviewer 9: gpt-6-astra (agent: phase2-reviewer, role: rust-quality); reviewer 10: gpt-6-astra (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); final verifier: gpt-6-astra (agent: astra-verifier, role: final-verifier)

  • Triage: critical by gpt-6-astra (effort low) — The diff is large and intricate and directly changes consensus-critical funds movement in add_epoch_pool_to_proposers_payout_operations, including epoch reward allocation and identity balance credits.
  • 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 35% left, 5h 0% 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-astra — final-verifier; agent astra-verifier
  • Phase 2 reviewers: 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-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
🤖 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/execution/platform_events/fee_pool_outwards_distribution/add_epoch_pool_to_proposers_payout_operations/mod.rs`:
- [SUGGESTION] packages/rs-drive-abci/src/execution/platform_events/fee_pool_outwards_distribution/add_epoch_pool_to_proposers_payout_operations/mod.rs:467-472: Pin the recipient balances in the order-sensitive payout test
  This assertion accepts either share being paid first, although those outcomes produce different identity balances and state roots. The new cap makes query traversal order part of payout behavior, so accidentally reversing the document iterator would still pass both this assertion and the credit-conservation check. The fixture now uses deterministic document IDs; assert the exact balances for its defined read order so the regression protects payment priority as well as conservation. Running the same fixture with reversed insertion order would additionally check that insertion order does not determine priority.

In `packages/rs-drive-abci/src/execution/platform_events/fee_pool_outwards_distribution/fetch_reward_shares_list_for_masternode/v1/mod.rs`:
- [SUGGESTION] packages/rs-drive-abci/src/execution/platform_events/fee_pool_outwards_distribution/fetch_reward_shares_list_for_masternode/v1/mod.rs:83: Move the queried documents instead of cloning the entire result
  The query outcome owns the returned documents and is discarded immediately, but `documents().to_owned()` deep-clones the vector and its documents, including property maps and buffers. With v1 removing the one-document limit, these avoidable allocations now grow with every reward share returned for each proposer. The already-imported `QueryDocumentsOutcomeV0Methods` trait provides `documents_owned(self)`, which moves the existing vector without changing query behavior.

…uery results

- should_pay_shares_over_the_whole_payout_until_it_is_used_up asserts the
  exact balances for document id read order, and runs the shares written
  in both orders
- test_mn_share_document_id exposes the helper's document id
- fetch_reward_shares_list_for_masternode v1 moves the queried documents
  instead of cloning them

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 the exact head and found no remaining in-scope issues. Both new generations activate only at PV14, preserve historical production implementations, and address all three prior findings. Local validation passed: 11 focused payout/query tests, 41 surrounding fee-pool/block-end tests, 25 platform-version tests, and git diff --check; the worktree remains unchanged.

🔴 0 blocking | 🟡 0 suggestion(s) | 💬 0 nitpick(s)

Review provenance

Source: reviewer 1: gemini-3.8-flash-high (agent: phase1-reviewer, role: general); reviewer 2: gemini-3.8-flash-high (agent: phase1-reviewer, role: architecture-layering); reviewer 3: gemini-3.8-flash-high (agent: phase1-reviewer, role: platform-versioning); reviewer 4: gemini-3.8-flash-high (agent: phase1-reviewer, role: rust-quality); reviewer 5: gpt-6-astra (agent: phase2-reviewer, role: general); reviewer 6: gpt-6-astra (agent: phase2-reviewer, role: architecture-layering); reviewer 7: gpt-6-astra (agent: phase2-reviewer, role: platform-versioning); reviewer 8: gpt-6-astra (agent: phase2-reviewer, role: rust-quality); reviewer 9: gpt-6-astra (agent: phase2-reviewer, role: general); reviewer 10: gpt-6-astra (agent: phase2-reviewer, role: architecture-layering); reviewer 11: gpt-6-astra (agent: phase2-reviewer, role: platform-versioning); reviewer 12: gpt-6-astra (agent: phase2-reviewer, role: rust-quality); final verifier: gpt-6-astra (agent: astra-verifier, role: final-verifier)

  • Triage: critical by gpt-6-astra (effort low) — The new add_epoch_pool_to_proposers_payout_operations/v1/mod.rs implements intricate consensus-critical funds distribution, aggregating overlapping identity credits, retaining missing-recipient shares, and capping ordered reward shares against remaining payouts, alongside protocol-version gating and expanded share retrieval.
  • Phase 1 reviewers: gemini-3.8-flash-high — general (completed, effort high); agent phase1-reviewer, gemini-3.8-flash-high — architecture-layering (completed, effort high); agent phase1-reviewer, gemini-3.8-flash-high — platform-versioning (completed, effort high); agent phase1-reviewer, gemini-3.8-flash-high — rust-quality (completed, effort high); agent phase1-reviewer
  • Phase 1 model: gemini-3.8-flash-high — antigravity quota: weekly 35% left, 5h 100% left
  • Fresh final gate: an independent Phase-2 review ran after iterative findings were reconciled
  • 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 — 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 — 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
🤖 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.

QuantumExplorer and others added 2 commits September 25, 2026 22:25
#4985)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
#4985 landed first with its own add_epoch_pool_to_proposers_payout_operations
v1. This PR folds into it in place: the per-identity sum (add_owed_credits),
usize proposer index and checked last-payout add join #4985's block-batch
hand-off, skip of a payToId without a balance and share cap. #4985's v1
tests are kept; the copied should_pay_every_proposer_and_its_reward_share
is dropped as covered by them. The v14.rs item moves to 43. The dispatcher
tests now apply the payout in epoch 1, and the debt test asserts that the
credit sum balances.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@QuantumExplorer QuantumExplorer changed the title fix(drive-abci)!: credit an identity everything one epoch payout owes it (PV14) fix(drive-abci)!: pay every reward share of a masternode and credit each identity once in the epoch payout (PV14) Sep 25, 2026
@QuantumExplorer QuantumExplorer changed the title fix(drive-abci)!: pay every reward share of a masternode and credit each identity once in the epoch payout (PV14) fix(drive-abci)!: pay every reward share of a masternode and credit each identity once in the epoch payout (PV15) Sep 25, 2026
@QuantumExplorer
QuantumExplorer changed the base branch from v4.2-dev to v4.3-dev September 25, 2026 16:07
@QuantumExplorer
QuantumExplorer marked this pull request as draft September 25, 2026 16:07
@github-actions github-actions Bot modified the milestones: v4.2.0, v4.3.0 Sep 25, 2026
@github-actions github-actions Bot removed the waiting-bots Waiting for the review bots to report on this head label Sep 25, 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.

3 participants