Repository navigation
feat(platform): price storage refunds at the storing epoch's rate and drop the fee history map - #5798
Open
QuantumExplorer wants to merge 2 commits into
Open
QuantumExplorer wants to merge 2 commits into
QuantumExplorer wants to merge 2 commits into
Conversation
…le and drop the fee history from Drive Drive::calculate_fee and everything above it took previous_fee_versions, a map from epoch to fee schedule. Its only reader was the refund branch of consume_to_fees_v0, which ignored it for fee_version_number 1, the number of every shipped schedule. Every shipped schedule also has the same storage rate, so no block on any network reads the map. FeeRefunds::from_storage_removal now takes the block's FeeVersion and the parameter is gone from 45 functions and about 860 calls, along with KnownCostItem and EpochCosts, which only read through the map. Platform state still records and saves the map in the same format. No fee or refund changes at any protocol version. should_price_a_refund_the_same_at_every_protocol_version checks that, and should_keep_one_storage_rate_on_every_protocol_version fails on a schedule that changes the rate, since refunds would then need the rate of the epoch that stored the bytes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
|
Important Review skippedToo many files! This PR contains 343 files, which is 243 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. Usage-priced reviews support at most 300 files.
 |
Contributor
|
📖 Book Preview built successfully. Download the preview from the workflow artifacts. Updated at 2026-11-07T09:16:13.796Z |
Contributor
|
Waiting for bot review — thepastaclaw not yet. Wait for the missing reviews, or a writer can post |
github-actions
Bot
added
the
waiting-bots
Collaborator
|
⛔ Final review complete — 2 blocking finding(s) (commit f61087d) · triage: critical |
thepastaclaw
reviewed
Nov 7, 2026
thepastaclaw
left a comment
thepastaclaw
left a comment
Collaborator
There was a problem hiding this comment.
Final validation — Phase 1 + Phase 2
At head 3e41c7f, the refund-pricing refactor preserves results for all configured protocol versions because their storage rates match. One non-blocking regression remains: the public refund helper loses its current-epoch range validation. This was a static review; current CI reports passing code checks, including Rust workspace and wasm32 tests, with PR Hygiene and automated review still pending.
🟡 1 suggestion(s)
1 finding(s) not shown inline (GitHub refused the PR diff as too large)
🟡 Suggestion: Preserve epoch validation in the public refund helper
packages/rs-dpp/src/fee/fee_result/refunds.rs:60-62
Removing Epoch::new(current_epoch_index)? also removes the range validation previously performed by this public, fallible API. EpochIndex is a u16 alias, so callers can supply values above MAX_EPOCH. For example, refunding 100 bytes stored in epoch 1 with current_epoch_index = u16::MAX, epochs_per_era = 20, and the first fee schedule previously returned ProtocolError::Overflow. It now reaches current_epoch_index + 1 in calculate_storage_fee_refund_amount_and_leftovers, which panics with overflow checks enabled or wraps to zero and produces an incorrect positive refund without them. The existing Drive execution path supplies a validated epoch, so this is not a current consensus blocker. Retain the epoch validation independently of the removed history lookup and add a regression test asserting that an invalid current epoch returns an error.
source: gpt-6.1-sol (phase2-reviewer: rust-quality)
Review provenance
Source: reviewer 1: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 3: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: tenderdash-consensus-security); reviewer 8: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)
- Triage:
criticalbygpt-6.1-sol(effort low) — The cross-cutting refactor directly changes funds-refund pricing in FeeRefunds::from_storage_removal and its consensus-execution callers, requiring verification that historical protocol versions retain identical monetary results. - Phase 1 reviewers:
muse-spark-1.3-contributor— general (completed, effort xhigh); agentphase1-reviewer,muse-spark-1.3-contributor— rust-quality (completed, effort xhigh); agentphase1-reviewer - Phase 1 model:
muse-spark-1.3-contributor— not quota-gated; passed overgemini-3.8-flash-high(antigravity quota lookup failed),glm-5.3-flash(not used above high effort; tier asks max) - Single stage: Phase 1 and Phase 2 reviewed this head side by side, with no blocker gate between them (triage tier)
- Fresh verifier:
gpt-6.1-sol— final-verifier; agentsol-verifier - Phase 2 reviewers:
gpt-6.1-sol— general (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— architecture-layering (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— platform-versioning (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— rust-quality (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— security-auditor (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— tenderdash-consensus-security (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-dpp/src/fee/fee_result/refunds.rs`:
- [SUGGESTION] packages/rs-dpp/src/fee/fee_result/refunds.rs:60-62: Preserve epoch validation in the public refund helper
Removing `Epoch::new(current_epoch_index)?` also removes the range validation previously performed by this public, fallible API. `EpochIndex` is a `u16` alias, so callers can supply values above `MAX_EPOCH`. For example, refunding 100 bytes stored in epoch 1 with `current_epoch_index = u16::MAX`, `epochs_per_era = 20`, and the first fee schedule previously returned `ProtocolError::Overflow`. It now reaches `current_epoch_index + 1` in `calculate_storage_fee_refund_amount_and_leftovers`, which panics with overflow checks enabled or wraps to zero and produces an incorrect positive refund without them. The existing Drive execution path supplies a validated epoch, so this is not a current consensus blocker. Retain the epoch validation independently of the removed history lookup and add a regression test asserting that an invalid current epoch returns an error.
Contributor
|
Bots are done — your move: post |
github-actions
Bot
added
waiting-self-review
…m protocol version 15 A refund returns the part of an owner's storage fee not yet paid out to epochs, and that fee was charged at the storage rate of the epoch that stored the bytes. calculate_fee 1, selected by DRIVE_VERSION_V10, reads the protocol version that epoch recorded at its first block (get_epoch_protocol_version) and takes the rate from that version's fee schedule. Bytes stored in the block's own epoch use the block's rate, since that epoch is only recorded at the end of its first block. consume_to_fees_v1 wraps generation 0, handing it each sectioned removal as a basic removal of as many bytes, and prices the refunds itself. calculate_fee reads state now, so it is a Drive method and drops epochs_per_era, which all 171 callers passed from that Drive's config. FeeRefunds::from_storage_removal takes a rate per storing epoch, and generation 0 passes the block's rate for every epoch, as before. Every schedule so far has the same storage rate, so no fee changes at any protocol version. TEST_PLATFORM_V4 (mock-versions only) doubles it, so should_price_a_refund_at_the_storage_rate_of_the_epoch_that_stored_the_bytes can tell the epochs apart. The pin on one storage rate is no longer needed and is removed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
QuantumExplorer
changed the title
Contributor
|
Waiting for bot review — thepastaclaw not yet. Wait for the missing reviews, or a writer can post |
github-actions
Bot
added
waiting-bots
thepastaclaw
suggested changes
Nov 7, 2026
thepastaclaw
left a comment
thepastaclaw
left a comment
Collaborator
There was a problem hiding this comment.
Re-review — Final validation — Phase 1 + Phase 2
At f61087d, historical refund pricing is correctly gated to protocol version 15 and storage access remains in Drive. Two blockers remain: historical metadata reads bypass processing-cost accounting, and the new metadata prerequisite breaks a cross-epoch fixture, as confirmed by the current-head Rust workspace CI log. The prior public-helper validation concern also remains a suggestion; this verification was static, with no local builds or tests.
🔴 2 blocking | 🟡 1 suggestion(s)
3 finding(s) not shown inline (GitHub refused the PR diff as too large)
🔴 Blocking: Account for historical epoch reads in the processing fee
packages/rs-drive/src/fees/calculate_fee/v1/mod.rs:64-65
This introduces user-dependent GroveDB reads without carrying their costs into the fee result. get_epoch_protocol_version_v0 uses grove.get(...).unwrap(); in the pinned GroveDB dependency, this is CostContext::unwrap, which explicitly discards OperationCost. Generation 1 computes processing fees from the original operations and then replaces only refunds and removed-system-byte counts, so the additional seeks and loaded bytes are never charged. The callback also runs for every qualifying owner/epoch entry in every sectioned-removal operation, repeating the same historical lookup across entries. Preserve the read cost in a Drive-owned accessor and include its deterministic processing charge in generation 1, with estimation and execution coverage while leaving generation 0 unchanged. A per-calculation cache of successful historical rates would additionally avoid repeated reads of the same immutable record; caching is an optimization, not a substitute for accounting for the reads that remain.
source: muse-spark-1.3-contributor (phase1-reviewer: general, rust-quality); gpt-6.1-sol (phase2-reviewer: general, architecture-layering, platform-versioning, rust-quality, security-auditor, tenderdash-consensus-security); gpt-6-astra (phase2-reviewer: general, architecture-layering, platform-versioning, rust-quality, security-auditor, tenderdash-consensus-security)
🔴 Blocking: Initialize historical epoch metadata in cross-epoch fixtures
packages/rs-drive/src/fees/calculate_fee/v1/mod.rs:64-65
The new mandatory lookup breaks the existing latest-version cross-epoch replacement test. In replacement.rs, test_double_document_replace_different_height_different_epoch enters epoch 1 with fast_forward_to_block(..., 1, false) at line 2861, then creates and replaces its document there. That branch updates platform state without processing epoch initialization. Genesis records epoch 0's protocol version; the later processed transition into epoch 2 initializes epoch 2 and its next-epoch record, not epoch 1. Refunding epoch-1 bytes therefore fails here. The current-head Rust workspace tests / Tests log confirms failure at replacement.rs:3091: valid_count() is 0 instead of 1, cancelling the remaining run (https://github.com/dashpay/platform/actions/runs/42358995597/job/128363462330). Update this fixture and equivalent synthetic-epoch setups to initialize and commit the storing epoch's metadata before leaving it. Retain the production missing-record error rather than substituting a guessed rate.
source: muse-spark-1.3-contributor (phase1-reviewer: general, rust-quality); gpt-6.1-sol (phase2-reviewer: general, architecture-layering, platform-versioning, rust-quality, security-auditor, tenderdash-consensus-security); gpt-6-astra (phase2-reviewer: general, architecture-layering, platform-versioning, rust-quality, security-auditor, tenderdash-consensus-security)
🟡 Suggestion: Preserve epoch validation in the public refund helper
packages/rs-dpp/src/fee/fee_result/refunds.rs:62-64
Drive supplies the current index from an existing Epoch, which protects normal block execution, but this public DPP helper still accepts an unrestricted EpochIndex (u16). Replacing Epoch::new(current_epoch_index)? with the storing-rate callback removes the helper's previous current-epoch validation; the callback receives only the storing index and cannot establish that invariant. With a qualifying 100-byte removal from epoch 0, a positive rate, and epochs_per_era = 20, MAX_EPOCH + 1 now returns a result instead of the former ProtocolError::Overflow. At u16::MAX, the distribution callback reaches unchecked current_epoch_index + 1, which panics with overflow checks or wraps without them. Restore current-epoch validation on the retained, non-dust path, or require a validated Epoch, and add boundary regression coverage. This is a public-API robustness issue, not a demonstrated network-reachable consensus defect.
source: muse-spark-1.3-contributor (phase1-reviewer: general, rust-quality); gpt-6.1-sol (phase2-reviewer: general, architecture-layering, platform-versioning, rust-quality); gpt-6-astra (phase2-reviewer: general, architecture-layering, platform-versioning, rust-quality)
Review provenance
Source: reviewer 1: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: tenderdash-consensus-security); reviewer 7: gpt-6-astra (agent: phase2-reviewer, role: general); reviewer 8: gpt-6-astra (agent: phase2-reviewer, role: architecture-layering); reviewer 9: gpt-6-astra (agent: phase2-reviewer, role: platform-versioning); reviewer 10: gpt-6-astra (agent: phase2-reviewer, role: rust-quality); reviewer 11: gpt-6-astra (agent: phase2-reviewer, role: security-auditor); reviewer 12: gpt-6-astra (agent: phase2-reviewer, role: tenderdash-consensus-security); reviewer 13: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 14: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)
- Triage:
criticalbygpt-6.1-sol(effort low) — The cross-cutting refactor also changes funds-refund calculation in Drive::calculate_fee v1 and LowLevelDriveOperation::consume_to_fees_v1, introducing protocol-gated historical epoch reads on block execution paths. - Phase 1 reviewers:
muse-spark-1.3-contributor— general (completed, effort xhigh); agentphase1-reviewer,muse-spark-1.3-contributor— rust-quality (completed, effort xhigh); agentphase1-reviewer - Phase 1 model:
muse-spark-1.3-contributor— not quota-gated; passed overgemini-3.8-flash-high(antigravity quota lookup failed),glm-5.3-flash(not used above high effort; tier asks max) - Single stage: Phase 1 and Phase 2 reviewed this head side by side, with no blocker gate between them (triage tier)
- Fresh verifier:
gpt-6.1-sol— final-verifier; agentsol-verifier - Phase 2 reviewers:
gpt-6.1-sol— general (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— architecture-layering (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— platform-versioning (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— rust-quality (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— security-auditor (completed, effort xhigh); agentphase2-reviewer,gpt-6.1-sol— tenderdash-consensus-security (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— general (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— architecture-layering (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— platform-versioning (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— rust-quality (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— security-auditor (completed, effort xhigh); agentphase2-reviewer,gpt-6-astra— tenderdash-consensus-security (completed, effort xhigh); agentphase2-reviewer - Model comparison: every Phase-2 reviewer also ran on
gpt-6-astra; the verifier weighed both sets without knowing which model wrote which
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `packages/rs-drive/src/fees/calculate_fee/v1/mod.rs`:
- [BLOCKING] packages/rs-drive/src/fees/calculate_fee/v1/mod.rs:64-65: Account for historical epoch reads in the processing fee
This introduces user-dependent GroveDB reads without carrying their costs into the fee result. `get_epoch_protocol_version_v0` uses `grove.get(...).unwrap()`; in the pinned GroveDB dependency, this is `CostContext::unwrap`, which explicitly discards `OperationCost`. Generation 1 computes processing fees from the original operations and then replaces only refunds and removed-system-byte counts, so the additional seeks and loaded bytes are never charged. The callback also runs for every qualifying owner/epoch entry in every sectioned-removal operation, repeating the same historical lookup across entries. Preserve the read cost in a Drive-owned accessor and include its deterministic processing charge in generation 1, with estimation and execution coverage while leaving generation 0 unchanged. A per-calculation cache of successful historical rates would additionally avoid repeated reads of the same immutable record; caching is an optimization, not a substitute for accounting for the reads that remain.
- [BLOCKING] packages/rs-drive/src/fees/calculate_fee/v1/mod.rs:64-65: Initialize historical epoch metadata in cross-epoch fixtures
The new mandatory lookup breaks the existing latest-version cross-epoch replacement test. In `replacement.rs`, `test_double_document_replace_different_height_different_epoch` enters epoch 1 with `fast_forward_to_block(..., 1, false)` at line 2861, then creates and replaces its document there. That branch updates platform state without processing epoch initialization. Genesis records epoch 0's protocol version; the later processed transition into epoch 2 initializes epoch 2 and its next-epoch record, not epoch 1. Refunding epoch-1 bytes therefore fails here. The current-head `Rust workspace tests / Tests` log confirms failure at `replacement.rs:3091`: `valid_count()` is 0 instead of 1, cancelling the remaining run (https://github.com/dashpay/platform/actions/runs/42358995597/job/128363462330). Update this fixture and equivalent synthetic-epoch setups to initialize and commit the storing epoch's metadata before leaving it. Retain the production missing-record error rather than substituting a guessed rate.
In `packages/rs-dpp/src/fee/fee_result/refunds.rs`:
- [SUGGESTION] packages/rs-dpp/src/fee/fee_result/refunds.rs:62-64: Preserve epoch validation in the public refund helper
Drive supplies the current index from an existing `Epoch`, which protects normal block execution, but this public DPP helper still accepts an unrestricted `EpochIndex` (`u16`). Replacing `Epoch::new(current_epoch_index)?` with the storing-rate callback removes the helper's previous current-epoch validation; the callback receives only the storing index and cannot establish that invariant. With a qualifying 100-byte removal from epoch 0, a positive rate, and `epochs_per_era = 20`, `MAX_EPOCH + 1` now returns a result instead of the former `ProtocolError::Overflow`. At `u16::MAX`, the distribution callback reaches unchecked `current_epoch_index + 1`, which panics with overflow checks or wraps without them. Restore current-epoch validation on the retained, non-dust path, or require a validated `Epoch`, and add boundary regression coverage. This is a public-API robustness issue, not a demonstrated network-reachable consensus defect.
Contributor
|
Your move: thepastaclaw requested changes on this head; dismiss the review or push a fix. |
github-actions
Bot
added
waiting-self-review
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Basic explanation
What this does: when stored data is deleted, its owner gets part of the storage fee back. That refund should use the price the owner paid when the data was stored. Every stored item already records the epoch it was stored in, and Drive records each epoch's protocol version when the epoch starts. That version's fee schedule has the price. From protocol version 15, refunds look the price up that way. That makes the fee history map, which was passed through the whole fee code for this, unnecessary, so it is removed.
Value: refunds stay right if a future protocol version changes the storage price, and callers have nothing to pass in or forget. About 1,200 fewer lines. The map argument is gone from 45 functions and about 860 calls, and
Drive::calculate_feebecomes a Drive method without itsepochs_per_eraargument. The saved map also no longer feeds block execution, so it cannot make nodes disagree (#4647).Risks: Low.
Issue being fixed or feature implemented
A refund returns the part of an owner's storage fee that has not yet been paid out to epochs. That fee was charged at the storage rate of the epoch that stored the bytes, so the refund has to use that rate.
The code passed
previous_fee_versions, a map from epoch to fee schedule kept in platform state, throughDrive::calculate_feeand everything above it, 45 functions in all. The refund code then ignored it forfee_version_number1, which every shipped schedule has, and priced against an empty map. Even its other branch looked up the removing epoch, not the storing one. The numbers were only right because the storage rate (27,000 credits per byte) never changed.Drive already has what it needs. The storage flags give the storing epoch, and the first block of every epoch records the epoch's protocol version in the epoch's tree (
KEY_PROTOCOL_VERSION, read byDrive::get_epoch_protocol_version, which fee distribution already uses). That version's fee schedule has the rate.What was done?
Refunds at the storing epoch's rate (protocol version 15)
Drive::calculate_fee1 (calculate_fee/v1, selected byDRIVE_VERSION_V10, protocol v15 note 25) prices each refund at the storage rate of the protocol version that ran the epoch that stored the bytes. It reads the version withget_epoch_protocol_versionand takes the rate fromPlatformVersion::get(version).fee_version.LowLevelDriveOperation::consume_to_fees_v1wraps generation 0 instead of copying it. Each sectioned removal reaches generation 0 as a basic removal of as many bytes, so processing and storage fees are computed exactly as before, and the refund and the system's removed bytes are then set from the lookup.FeeRefunds::from_storage_removaltakes the rate of each storing epoch from a closure instead of a schedule.Example: 1,000 bytes stored in epoch 12, removed in epoch 15, at protocol version 15, if epoch 12 had run a schedule at twice today's rate.
Before:
After:
Today every epoch ran a schedule at 27,000 per byte, so both give the same refund.
Generation 0 keeps its numbers
consume_to_fees_v0prices every refund at the block's rate, through the new closure argument. It used to price against the first schedule; for every protocol version that selects it (up to 14) the two rates are equal. See "In-place changes to shipped generations".The map is gone
The parameter is removed from
Drive::calculate_fee,apply_drive_operations,update_contract, the document, identity and token balance methods that forwarded it, and fromexecute_event,validate_fees_of_eventandprocess_validation_resultin drive-abci.check_tx,process_raw_state_transitions,process_block_fees_and_validate_sum_treesandupdate_masternode_identitiesstop reading it.dpp::fee::default_costs::{KnownCostItem, EpochCosts}only read through the map and are removed.Platform state still records the map at each protocol upgrade and saves it in the same format, so saved states load as before. Nothing in block execution reads it.
calculate_feeis a Drive methodIt reads state now, so it takes
&self, and it no longer takesepochs_per_era, which every one of its 171 callers passed as that same Drive'sconfig.epochs_per_era.Before:
After:
A test-only protocol version
TEST_PLATFORM_V4(mocks/v4_test.rs,mock-versionsonly) is protocol version 15 with twice its storage rate, so a test can tell which epoch's schedule priced a refund. No network runs it.Book
fees/overview.mdandcontributing/coding-conventions.mdstate the new refund rule.drive/cost-tracking.mddescribes generation 1.calculate_feesamples indrive/batch-operations.md,drive/finalize-tasks.md,state-transitions/drive-operations.md,versioning/versioned-dispatch.mdanddata-model/key-limits.mdfollow the new signature.platform_versionis the last parameter".Live network check
Each epoch's record was read with
getEpochsInfo, which only returns an epoch that holds its whole record. An epoch's first block writes that record (start height, start time, fee multiplier and protocol version) in one batch.Open PRs affected
calculate_feev1 and the price-before-commit generations no longer have a purpose. It also adds acalculate_fee1. Its recorded-owner refund credits are unaffected but will conflict.mocks/v4_test.rs. With nothing reading the map, its tests no longer exercise it, and it conflicts with this PR's mock.How Has This Been Tested?
New tests:
rs-drive,fees/calculate_fee:should_price_a_refund_at_the_storage_rate_of_the_epoch_that_stored_the_bytesrecords epoch 12 at the doubled-rate mock and epoch 14 at the latest version. It removes bytes stored in 12, 14 and 15 at epoch 15. At protocol version 15 epoch 12 refunds at the doubled rate and 14 and 15 at the normal rate; at protocol version 14 all three use the normal rate.should_price_a_refund_the_same_at_every_protocol_versionruns a removal through the dispatcher at every entry ofPLATFORM_VERSIONSand compares it with the first schedule's amount.should_not_price_a_refund_from_an_epoch_that_recorded_no_protocol_version: protocol version 15 refuses to price bytes from an epoch with no record, and protocol version 14 still prices them.rs-dpp,fee_result/refunds.rs:should_price_each_storing_epoch_at_its_own_storage_rategives two storing epochs different rates.Local checks, all passing:
cargo fmt --all,cargo clippy -p platform-version -p dpp -p drive -p drive-abci --all-targets -- -D warnings,cargo check --workspace --all-targetsandcargo check -p drive --no-default-features --features verify.platform-versionwithmock-versions: 30 tests.dppfee: 71 tests.drive: fees, documents, contracts, identities, tokens, groups and batches, 1,655 tests.drivequery test binaries (query_tests,query_tests_history): 63 tests.drive-abci: block end events and refund clawbacks, fee validation, event execution, fee pools, masternode identity updates, shielded identity creation and protocol upgrade, 159 tests.The strategy tests were only compiled locally, so CI is their first run.
In-place changes to shipped generations
Except for
calculate_fee1 andconsume_to_fees_v1, which only protocol version 15 selects, every edit is in place and none changes an output:LowLevelDriveOperation::consume_to_fees_v0,Drive::calculate_fee_v0, andFeeRefunds::from_storage_removal(unversioned)FeeVersion::first()'s rate because every shipped schedule has number 1. Now they use the block schedule's rate through the closure. All schedules useFEE_STORAGE_VERSION1, so it is the same number. Theelsebranch and itsCorruptedCodeExecutionneeded a number other than 1, so they were unreachable.Drive::calculate_feedispatcherself.config.epochs_per_era, which is what every caller passed.apply_drive_operationsv0, v1, v2;update_contractv0, v1, v2add_document_for_contract,update_document_for_contract,update_document_for_contract_id,update_document_with_serialization_for_contract,delete_document_for_contract,delete_document_for_contract_id,delete_index_only_document_for_contract,remove_from_identity_balance,update_identity_revision,remove_from_identity_token_balance,add_to_identity_token_balanceexecute_eventv0, v1;validate_fees_of_eventv0, v1;process_validation_resultv0, v1;update_masternode_identitiesv0, v1;process_block_fees_and_validate_sum_treesv0;process_raw_state_transitionsv0;check_txv0Every other edited generation only changes how it calls
calculate_fee.from_storage_removalalso losesEpoch::new(current_epoch_index)?. That can only fail aboveMAX_EPOCH(65,279), and the index it receives comes from an existingEpoch.Breaking Changes
None for the protocol: no fee or refund changes at any protocol version, and saved platform state is unchanged. Rust API only: the signatures above change,
Drive::calculate_feebecomes a method, anddpp::fee::default_costs::{KnownCostItem, EpochCosts}are removed. No other crate in this workspace uses them.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 ·
f61087d/self-reviewedWhen every merge requirement is met, the
PR Hygienecheck passes. Reviewer limits do not block merging; other required GitHub checks and protections still apply.