fix(platform)!: credit repaid identity debt to the processing fee pool - #4985
Merged
Merged
Conversation
…l (PV14) An identity whose balance could not cover the processing part of a fee keeps the unpaid part as a debt, and credits it receives while its balance is empty repay that debt first. The repaid part was credited nowhere: only the remainder reached a balance. From protocol version 14 the repaid part goes to the processing fee pool of the epoch it is repaid in, where the unpaid fee would have gone. add_to_identity_balance_operations 1 marks it with a new LowLevelDriveOperation::RepaidIdentityDebt, and every apply routes it: apply_drive_operations 1 writes the pool after its batch (unbilled, and adding to a fee distribution the same batch wrote), the fee path (apply_balance_change_from_fee_to_identity 1, which now takes the block info) writes it in its own batch, and the epoch payout (generation 1) hands its credits to the block's apply_drive_operations instead of a plain grove batch. A batch still holding one is refused by apply_batch_low_level_drive_operations and by convert_drive_operations_to_grove_operations. add_to_previous_balance 0 reports the repaid part it already computed. Fees do not change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
|
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 configurationConfiguration used: Repository: dashpay/platform/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (30)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughProtocol version 14 changes identity credit and fee balance handling so credits that repay identity debt are recorded in the applicable epoch’s processing-fee pool. The change also adds batch routing and version-1 proposer payout logic, with tests for repayment and payout cases. ChangesIdentity debt repayment
Priority: ➖ Normal Estimated code review effort: 4 (Complex) | ~60 minutes Change: Bug fix Sequence Diagram(s)sequenceDiagram
participant Event as State transition event
participant Drive as Identity balance processing
participant Batch as Drive operation batch
participant Pool as Epoch processing-fee pool
Event->>Drive: Apply credit or fee balance change
Drive->>Batch: Add balance operations and repayment markers
Batch->>Pool: Route repaid debt credits for the block epoch
Suggested reviewers: Merge Risk: 🔵 Low · up to From protocol version 14, credits that repay an identity's debt are added to the epoch's processing fee pool. Proposer payouts now skip recipients without balances and cap reward shares at the proposer's remaining payout. No outstanding defects were found, and the earlier atomicity and double-counting concerns appear resolved. Because this changes consensus-level credit accounting, owners should keep the new repayment and payout tests in place when making related changes. Security Architecture ReviewSecurity architecture risk: 🟡 Moderate · up to The change affects how credits move through the platform’s shared fee pools. The reviewed paths contain controls for keeping balance and pool updates together, but the breadth of the accounting change and incomplete recovery coverage warrant design review. No PR-introduced security issue was established. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
|
📖 Book Preview built successfully. Download the preview from the workflow artifacts. Updated at 2026-09-25T14:39:05.136Z |
github-actions
Bot
added
the
waiting-bots
Collaborator
|
🕓 Queued for automated review — 5th in line, estimated start in ~2.3 h (commit 0bd4f1c)
|
8 tasks
github-actions
Bot
added
the
bot-review-skipped
thepastaclaw
suggested changes
Sep 24, 2026
thepastaclaw
left a comment
thepastaclaw
left a comment
Collaborator
There was a problem hiding this comment.
Final validation — Phase 2 only (queue backlog)
The PV14 debt-routing logic correctly preserves the intended accounting for distinct indebted identities, but it does not handle repeated credits to the same identity within one batch. The standalone balance API also commits the identity mutation before its separate processing-pool write when no caller transaction is supplied, allowing a failed pool update to leave the credit accounting inconsistent.
🔴 2 blocking
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:
criticalbygpt-6-astra(effort low) — This is a large, intricate change to consensus-critical funds movement and fee accounting across Drive and ABCI operation application, including identity debt repayment, processing fee pools, and protocol-version behavior. - Phase 1 reviewers: not run (skipped for throughput: 18 PRs queued, above the 10 limit)
- Fresh verifier:
gpt-6-astra— final-verifier; agentastra-verifier - Phase 2 reviewers:
gpt-6-astra— general (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— architecture-layering (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— platform-versioning (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— rust-quality (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— security-auditor (completed, effort xhigh); agentphase2-reviewer
🤖 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/util/batch/drive_op_batch/drive_methods/apply_drive_operations/v1/mod.rs`:
- [BLOCKING] packages/rs-drive/src/util/batch/drive_op_batch/drive_methods/apply_drive_operations/v1/mod.rs:113-114: Aggregate repeated credits to one indebted identity before routing repayment
Each `add_to_identity_balance_operations_v1` call is converted against the same pre-batch state, and this code only sums the resulting `RepaidIdentityDebt` markers. If two operations credit the same indebted identity, both observe the original debt and emit updates based on it. With the production default `batching_consistency_verification = false`, the duplicate debt writes leave the final identity state at the result of one credit, while both markers are routed to the processing pool. For example, a debt of 100 receiving two credits of 60 ends with balance 0, debt 40, and pool credit 120 instead of balance 20, debt 0, and pool credit 100. This can occur in epoch payouts when multiple reward-share payments target the same recipient. Coalesce credits per identity before calculating repayment, or otherwise apply pending balance/debt updates when constructing the batch, and add a shared-recipient payout regression.
In `packages/rs-drive/src/drive/identity/update/methods/add_to_identity_balance/v1/mod.rs`:
- [BLOCKING] packages/rs-drive/src/drive/identity/update/methods/add_to_identity_balance/v1/mod.rs:47-69: Commit the identity update and processing-pool credit atomically
When `transaction` is `None`, the first `apply_batch_low_level_drive_operations` call commits the identity balance and debt changes before the processing-pool operation is constructed and applied. The pool operation can fail while reading or updating the epoch pool—for example, an uninitialized or malformed pool returns an error at lines 57–62—after the identity mutation has already persisted. A process interruption between the two calls has the same effect. The direct public API can therefore clear debt and reduce the identity's incoming balance without recording the corresponding pool credit, recreating the conservation defect this PR addresses. When applying without a caller transaction, create an owned transaction spanning the reads and both writes, commit only after the pool update succeeds, and preserve caller-supplied transactions.
Contributor
|
Your move: thepastaclaw requested changes on this head; dismiss the review or push a fix; thepastaclaw left review threads unresolved; resolve them. |
github-actions
Bot
added
waiting-self-review
…ce, and credit the pool atomically (PV14) apply_drive_operations 1 converted every AddToIdentityBalance against the balance and debt committed before the batch, so two credits to one indebted identity each reported repaying the same debt while the later balance write replaced the earlier one: the pool got the repayment twice and the identity kept one credit. It now merges every credit the batch makes to one identity into one, in the place of the first, before converting; a batch crediting each identity once is unchanged. This also covers an epoch payout in which two masternodes pay one recipient. add_to_identity_balance 1 committed the balance write before the pool write when the caller passed no transaction. Both now share an owned transaction, committed only after the pool write applied. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
github-actions
Bot
added
waiting-bots
QuantumExplorer
changed the title
github-actions
Bot
added
the
bot-review-skipped
…nd harden the epoch payout (PV14) apply_drive_operations 1 now nets every AddToIdentityBalance and RemoveFromIdentityBalance of one identity in a batch into one write (or none), in the place of the first, skipping the map when a batch writes at most one balance. A credit and a debit of one identity no longer lose a write, and an indebted identity repays its debt once. The epoch payout (generation 1) skips a reward share whose payToId has no balance, leaving it with its masternode, and caps each share at what is left of the masternode's payout, so neither fails the batch that ends the block. The unbilled repaid debt pool write is one helper, apply_repaid_identity_debt_to_processing_pool. The guard refusing an unrouted RepaidIdentityDebt moves into the protocol version 14 arm of apply_batch_low_level_drive_operations and joins the partial batch apply; the consume helpers document that they drop it. The debt test helpers are shared, the payout and transfer tests assert balance and debt apart, and the protocol version 14 notes describe the merge. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
github-actions
Bot
removed
the
bot-review-skipped
…erations 0 and 1 Generation 0's operations body moves into add_to_identity_balance_operations_and_repaid_debt_v0, which also returns the part of the credits that repaid the identity's debt; generation 0 drops it and generation 1 appends the RepaidIdentityDebt marker, instead of carrying a copy of the whole body. Generation 0 builds the same operations, in the same order, as before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
QuantumExplorer
added a commit
that referenced
this pull request
Sep 25, 2026
#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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue being fixed or feature implemented
When an identity cannot cover the processing part of a fee, the unpaid part stays behind as a debt (its negative credit balance), and the fee pools only receive what was paid. Credits the identity receives later while its balance is empty repay that debt first (
add_to_previous_balance): the identity's balance gets only what is left.The repaid part was then credited nowhere. The credits left their source (a top-up's new system credits, a transfer's sender, a refund's storage pool, an epoch payout) but only the remainder reached a balance, so the platform's credit total and the sum of its balances no longer agreed after a repayment.
What was done?
The repaid part now goes to the processing fee pool of the epoch it is repaid in, where the unpaid fee would have gone. Protocol version 14 only.
add_to_identity_balance1 (add_to_identity_balance_operations1): the same arithmetic as 0, and when the added credits repay a debt the operations end with a newLowLevelDriveOperation::RepaidIdentityDebt(credits). It is not a GroveDB operation and has no cost; whoever applies the operations owes those credits to the current epoch's processing fee pool.apply_drive_operations1 (every state transition's operations, and the end of block batch) takes them out of the batch and, once the batch applied, adds their total to the processing fee pool of the block's epoch in a follow-up write. The write reads the state the batch left, so at the end of a block it adds to the fee distribution the same batch wrote instead of racing it (two writes of one pool item in one batch would lose one). It is not billed, so fees do not move.apply_balance_change_from_fee_to_identity1 routes the debt the payer's own refund repays and the debts other identities' refunds repay, in its own batch. It now takes the block info (the dispatcher gainedblock_info; generation 0 ignores it). The fee it reports as paid is unchanged.add_epoch_pool_to_proposers_payout_operations1 hands the epoch payouts to the block'sapply_drive_operationsas the identity credits they are, instead of converting them to a plain grove batch withconvert_drive_operations_to_grove_operations, which cannot carry the marker. The conversion reads the same state either way.add_to_identity_balance1 (the applying method) writes the pool after its batch too. Without a caller transaction both writes share an owned transaction, committed only once the pool write applied, so a failed pool write leaves the balance and debt as they were.apply_drive_operations1 first merges everyAddToIdentityBalanceandRemoveFromIdentityBalanceof one identity in the batch into one net write (or none when they cancel), in the place of the first. Each write is converted against the balance and debt committed before the batch, so unmerged, a second write to one identity replaced the first, and two credits to an indebted identity each reported repaying the same debt. A debt exists only at a zero balance, so writes that would succeed in turn net to a credit that repays it once. A batch writing each identity's balance at most once is unchanged, and skips the merge map. This covers an epoch payout in which two masternodes pay the same recipient.payToIdhas no balance (the share stays with its masternode) and caps each share at what is left of the masternode's payout, so neither fails the batch that ends the block.apply_batch_low_level_drive_operations(in its protocol version 14 arm, generation 1), the unused partial-batch apply andconvert_drive_operations_to_grove_operationsrefuse a batch that still holds aRepaidIdentityDebt, withCorruptedCodeExecution, and pricing one isCorruptedCodeExecutionas well.grovedb_operations_consumeandgrovedb_operations_batch_consumedocument that they drop it. The unbilled pool write after a batch is one helper,apply_repaid_identity_debt_to_processing_pool.AddToPreviousBalanceOutcomeV0reportsrepaid_debt, the part of the added credits that repaid the debt, whichadd_to_previous_balance0 already computed.DRIVE_IDENTITY_METHOD_VERSIONS_V2(protocol version 14's) setsadd_to_identity_balanceandapply_balance_change_from_fee_to_identityto 1, andDRIVE_ABCI_METHOD_VERSIONS_V10setsadd_epoch_pool_to_proposers_payout_operationsto 1, both amended in place since protocol version 14 is unreleased.v14.rs, theDRIVE_IDENTITY_METHOD_VERSIONS_V2doc comment, and the identity debt paragraph of the book's address fees chapter.Before and after
An identity with an empty balance owes 100 credits. It is topped up with 300 credits in a block of epoch 2:
The same holds when the credits come from a transfer, a document or token purchase, a moderators pot settlement, a storage refund (the payer's own or another identity's) or an epoch payout. When the added credits do not cover the debt (60 credits against 100), all 60 go to the pool and 40 stay owed, as before. An estimate reads no debt and repays none, as before.
How Has This Been Tested?
Drive (
add_to_identity_balance/v1,apply_balance_change_from_fee_to_identity/v1):should_credit_the_repaid_debt_to_the_processing_fee_pool_and_keep_the_credit_sum: the top-up above;calculate_total_credits_balancebalances.should_credit_all_added_credits_to_the_pool_when_they_do_not_cover_the_debtshould_leave_the_repaid_debt_in_no_counted_balance_at_protocol_version_13: the same top-up through the dispatcher at protocol version 13 still repays and credits no pool, and the credit sum does not balance.should_credit_every_debt_one_batch_repays_to_the_processing_fee_pool_once: two indebted identities credited in oneapply_drive_operationsbatch; the pool gets both repayments.should_add_the_repaid_debt_to_a_processing_fee_pool_write_of_the_same_batch: a batch that writes the epoch's processing fees itself (as the end of a block does) and repays a debt ends with both.should_repay_nothing_when_only_estimatingshould_repay_a_debt_once_when_one_batch_credits_the_identity_twice: a debt of 100 and two credits of 60 in one batch end with balance 20, debt 0 and 100 in the pool, and the credit sum balances.should_keep_every_credit_one_batch_makes_to_one_identity: two credits to one identity without a debt both land.should_merge_a_credit_and_a_debit_of_one_indebted_identity_into_one_net_write: a debt of 100, a credit of 300 and a debit of 7 in one batch end with balance 193, debt 0 and 100 in the pool, and the credit sum balances.should_commit_neither_write_when_the_pool_write_fails_without_a_caller_transaction: the pool write fails (an epoch with no pool tree) and the balance and debt are unchanged.should_refuse_to_apply_a_batch_holding_an_unrouted_repaid_debt,should_refuse_to_convert_a_debt_repaying_credit_into_a_plain_batchshould_credit_the_debts_that_the_payer_and_another_refund_repay_to_the_processing_fee_pool: the payer's refund and another identity's refund both repay; the fee reported as paid is the fee result, unchanged.should_credit_the_debt_another_refund_repays_when_the_payers_balance_does_not_changeshould_leave_the_processing_fee_pool_alone_at_protocol_version_13apply_balance_change_from_fee_to_identity_operations(identity and token balance modules) now expect the trailingRepaidIdentityDebtwith the repaid amount.drive-abci:
should_credit_the_debt_a_transfer_repays_to_the_processing_fee_pool(identity credit transfer): a real transfer to an indebted identity throughprocess_raw_state_transitions; the recipient keeps the rest and the epoch's processing fee pool holds exactly the repaid debt.should_pay_proposers_and_shares_and_credit_a_repaid_debt_to_the_block_epochs_pool(payout 1): the payout test with one share recipient in debt; the recipient gets its share less the debt, and the pool of the block's epoch gets the debt.should_repay_a_shared_recipients_debt_once_from_every_share_one_payout_pays_it(payout 1): two masternodes each pay half their reward to one indebted recipient; it receives both shares less the debt once, and the pool of the block's epoch gets the debt.should_keep_a_share_without_a_recipient_and_cap_a_share_above_the_reward(payout 1): a share naming no identity stays with its masternode, and a 120% share pays its recipient the masternode's whole reward.Results, run locally:
cargo test -p drive --lib -- add_to_identity_balance apply_balance_change_from_fee_to_identity balance::update identity::update: 85 passed.cargo test -p drive-abci --lib: 3514 passed, 0 failed. After the review fixes, on a clean build of the workspace crates: the Drive filter above 90 passed,cargo test -p drive-abci --lib -- add_epoch_pool_to_proposers_payout_operations identity_credit_transfer fee_pool process_block_fees102 passed (the payout tests 6 runs in a row), and after the second round (debits merged, payout skip and cap): the Drive filter 91 passed, the drive-abci filter 103 passed,cargo test -p drive-abci --lib3518 passed, and the strategy tests below green; clippy, fmt and the verify build clean. With the debit mapping, the skip or the cap removed, the matching new test fails. With the merge disabled the three repeated-credit tests fail; without the owned transaction the atomicity test fails.run_chain_insert_one_new_identity_per_block_with_epoch_change,run_chain_comprehensive_mixed_operations_with_epoch_change_and_quorum_rotation,run_chain_insert_many_new_identity_per_block_many_document_insertions_updates_transfers_and_deletions_with_epoch_change,run_chain_top_up_identities,run_chain_top_up_identities_from_addresses,run_chain_transfer_between_identities,run_chain_core_height_randomly_increasing_with_quick_epoch_change,should_record_the_total_credits_history_after_epoch_core_rewards: all passed. The first overflows the default test thread stack in a local debug build on the base branch too, so it ran withRUST_MIN_STACK=16777216.cargo clippy -p drive -p drive-abci -p platform-version --all-features --all-targets -- -D warnings,cargo check -p drive --no-default-features --features verifyandcargo fmt --all -- --checkare clean.Breaking Changes
Consensus, at protocol version 14 only (unreleased): credits that repay an identity's debt are credited to the processing fee pool of the current epoch. No fee changes. Earlier protocol versions keep generation 0 of every method involved.
In-place changes to shipped generations
add_to_previous_balance0 (every protocol version): its outcome also reportsrepaid_debt, the part of the added credits its arithmetic already kept from the balance. The balance and debt writes are unchanged; only the protocol version 14 generations read the new field.add_to_identity_balance0 (every protocol version): the body ofadd_to_identity_balance_operations_v0moves intoadd_to_identity_balance_operations_and_repaid_debt_v0, which also returns the repaid partadd_to_previous_balancereports, so generation 1 reuses it and only appends the marker. Generation 0 returns the same operations, in the same order; the existing fee-pinning tests of generation 0 pass unchanged.apply_balance_change_from_fee_to_identity0 (protocol versions 1 to 13): sets the new outcome field to 0 in its two literals, and its dispatcher takes ablock_infothat generation 0 ignores. No behaviour change.convert_drive_operations_to_grove_operations0 (every protocol version) and the partial-batch apply dispatcher (unused): refuse an operation list holding aRepaidIdentityDebt. Onlyadd_to_identity_balance_operations1 produces one, which protocol version 14 alone selects, so no earlier batch meets the check. The same guard inapply_batch_low_level_drive_operationssits in its generation 1 arm, which only protocol version 14 selects.apply_drive_operations1: selected by protocol version 14 alone (DRIVE_VERSION_V9), so edited in place as that version's generation.Checklist:
structure.rs, regeneratedgrovedb-structure.json, and checked the structure viewer link posted on this pull requestFor repository code-owners and collaborators only
🤖 Generated with Claude Code
PR Hygiene ·
0bd4f1c/skip-botsproceeds without the ones not yet reported/self-reviewedonce the bots are doneWhen every box is checked the
PR Hygienecheck passes and this can merge.Summary by CodeRabbit