Repository navigation
fix(drive)!: fee estimates size sum and count index trees as what they are (PV14) - #5377
Conversation
…y are (PV14) The apply: false estimate of a document insert under a summable or countable index came in below the applied storage fee (18 bytes for a summable index on small fixed-width properties). The value tree and its 0 bucket are created in the same GroveDB batch as the document's reference, and GroveDB's average-case estimator sized such a tree by the aggregate its level reported, which was none for every non-indexed tree, so each sum or count tree was estimated as a plain one. Bump grovedb to dashpay/grovedb#1018 (on top of develop b50a583377), which makes GROVE_V4 report every level's own aggregate. Only protocol version 14 selects GROVE_V4. Add estimate-vs-apply tests for summable, range-summable, countable and countable+summable indexes, update the five latest-version estimate pins that move by one sum tree root key replace (+3,600 credits of processing fee), and add PV14 note 93. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 29 minutes. View limit details
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 |
|
Waiting for bot review — coderabbitai not yet · thepastaclaw not yet. Wait for the missing reviews, or a writer can post |
github-actions
Bot
added
the
waiting-bots
|
🕓 Review not started yet because the new head is waiting for the 30-minute push debounce.
Commit af901c7. Normal review starts when eligible; priority review starts as soon as a slot is available. |
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Final validation — Phase 1 + Phase 2
No actionable defects were confirmed at the exact reviewed head. The estimator correction is gated to GROVE_V4/PV14, with four Drive fee-bound tests and upstream GROVE_V3 regression assertions; the proposed additional historical balance pin does not identify an uncovered change introduced by this PR. This was a static verification: Rust workspace CI remained in progress in the supplied snapshot, and the author-disclosed GroveDB merge-commit re-pin remains a pre-merge requirement.
Review provenance
Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: architecture-layering); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: platform-versioning); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)
- Triage:
normalbygpt-6.1-sol(effort low) — The GroveDB dependency bump, PV14 compatibility table changes, and fee-estimation regression tests warrant ordinary review but do not themselves constitute a large or intricate change to a critical surface. - Phase 1 reviewers:
muse-spark-1.3-contributor— general (completed, effort xhigh); agentphase1-reviewer,muse-spark-1.3-contributor— rust-quality (completed, effort xhigh); agentphase1-reviewer - Phase 1 model:
muse-spark-1.3-contributor— not quota-gated; passed overgemini-3.8-flash-high(antigravity below 15% reserve: weekly 14% left, 5h 100% left),glm-5.3-flash(not used above high effort; tier asks max) - Fresh verifier:
gpt-6.1-sol— final-verifier; agentsol-verifier - Phase 2 reviewers:
gpt-6.1-sol— general (completed, effort high); agentphase2-reviewer,gpt-6.1-sol— architecture-layering (completed, effort high); agentphase2-reviewer,gpt-6.1-sol— platform-versioning (completed, effort high); agentphase2-reviewer,gpt-6.1-sol— rust-quality (completed, effort high); agentphase2-reviewer
…tree-fee-estimates-pv14 # Conflicts: # Cargo.lock # Cargo.toml
… (PV14) dashpay/grovedb#1018 merged (99c0f4fb6f), followed by #1019 and #1020, which size inserted sum items (and replaced ItemWithSumItem) at the fixed width the apply charges in the average- and worst-case estimators, from GROVE_V4. Pin develop's head; the lockfile changes only the grovedb rev. PV14 note 93 now covers the sum item sizing too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Waiting for bot review — coderabbitai not yet · thepastaclaw not yet. Wait for the missing reviews, or a writer can post |
Basic explanation
What this does: Before Drive writes a document, it can work out the fee without writing anything (a "dry run"), and that figure is expected to be at least the real fee. For a document type with a summable or countable index, the dry run came in below the real storage fee (by 486,000 credits, 18 bytes, for a summable index), because GroveDB counted the index's new sum and count trees as plain trees. The fix is in GroveDB (dashpay/grovedb#1018, merged); this PR moves Platform to GroveDB
develop, which also carries #1019 and #1020 (sum items estimated at the size they are stored), adds tests that pin the dry run above the real fee, and updates five pinned protocol version 14 estimates.Value: The dry-run fee is the bound Platform checks a write against before applying it. With this change it covers the applied fee for documents under summable, range-summable, countable and countable-and-summable indexes, as it already did for other indexes.
Risks: Low. Only protocol version 14 changes (it alone selects grovedb's
GROVE_V4), and estimates only go up: by 9 bytes per aggregate for each sum or count tree a write creates, and by 9 replaced bytes (3,600 credits of processing fee) when a write rewrites a sum tree's root, which identity and token balance changes do. Each inserted sum item (an identity or token balance, a counter, a summable index-only entry) is also estimated up to 8 bytes higher, the bytes the apply already charges. Applied fees, stored data and proofs do not change.Issue being fixed or feature implemented
A contract at
PlatformVersion::latest()with aposttype:tier(integer 0..=255, sou8) andweight(integer -100..=100, soi8), both required, and an indexbyTieron[tier]withsummable: "weight". Inserting one post (tier 5, weight -3) into an empty drive:Before:
After:
The first document under an index value creates the value tree and its
0bucket, bothSumTrees here, in the same GroveDB batch as the document's reference. Drive's layer information describes both correctly, but GroveDB's average-case estimator sized a tree created together with its contents by the aggregate its level reported, and every non-indexed level reported none, so each was sized as a plain tree: 3 bytes instead of 12, 9 bytes short per tree. With small fixed-width properties nothing else in the estimate leaves room, so the shortfall showed. The same holds for count trees (9 bytes per tree) and count-sum trees (18 per tree, 36 bytes for a countable and summable index).What was done?
GroveDB bump (
Cargo.toml,Cargo.lock) fromb50a583377todevelophead5cb0962afd; the lockfile changes only the GroveDB rev. It brings threeGROVE_V4estimate fixes:average_case_propagated_aggregate_kind1: every level reports the aggregate of its own tree type, so a sum or count tree created with its contents is sized as that tree.average_case_sum_item_cost_size1 and #1020worst_case_sum_item_cost_size1: an insertedSumItemorItemWithSumItem, and a replacedItemWithSumItem, is sized with its sum at the fixed 9-byte width the apply charges.DRIVE_VERSION_V9(protocol version 14) is the only table selectingGROVE_V4, so no shipped protocol version's estimates change.Tests in
packages/rs-drive/src/drive/document/insert/add_document_for_contract/aggregating_index_fee_estimate_tests.rs: three posts (new tier, same tier, new tier) under asummable,summable+rangeSummable,countable, andcountable+summableindex; each dry run must upper-bound the applied storage and processing fees.Pinned estimates at the latest protocol version, +3,600 credits of processing fee each (one sum tree root key replace, now 12 bytes instead of 3):
should_add_to_balance_latest_version_estimated,should_remove_from_balance_latest_version_estimated,test_insert_identity_estimated_costs_latest_version, and the two token balance..._without_statetests. The first-version and v11 pins are unchanged.PV14 note 93 in
packages/rs-platform-version/src/version/v14.rs, covering all three (92 is taken by an open PR).How Has This Been Tested?
v5.0-devat8d5a5be0bb(by 18 bytes), and all four withGROVE_V4's new slot set back to 0 (18 bytes, 36 for countable and summable). They pass with this PR.cargo test --locked -p drive --libon this branch (merged withv5.0-dev7bd6be4217, GroveDB5cb0962afd): 4,175 passed, 0 failed, 6 ignored. The sum item sizing of feat(dapi-grpc): add protobuf clients for rust #1019 and fix(dapi-grpc): missing protobuf rust code #1020 moved no Drive pin.cargo check --locked --all-targetsfordrive,drive-abci,drive-proof-verifier,dash-sdkandplatform-version, andcargo fmt --all -- --check, are clean.Breaking Changes
Consensus, protocol version 14 only: the dry-run fee estimates Drive computes rise as described above. Estimates at protocol versions 1 to 13 and every applied fee are unchanged. No shipped generation is edited in place.
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 ·
af901c7/skip-botsproceeds without the ones not yet reported/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.