fix(drive): unify shielded per-action processing fee across all protocol versions#3877
Conversation
…col versions The shielded per-action processing fee was only calibrated to its real value (22M credits, the ~1.1 ms/action Halo 2 verification cost) in validation v8 (#3800); validation v1-v7 kept a 3M placeholder. The shielded family does not activate until platform v12 (validation v8), so no block under an earlier protocol version has ever priced a shielded action — making the v1-v7 value a dormant placeholder, not deployed consensus behavior. Unify all versions to 22M so a client computing the pool fee under a stale protocol version (e.g. an SDK whose auto-detect ratchet hasn't yet observed v12 metadata) still reserves the consensus-correct amount. Without this, a ShieldFromAssetLock built with the 3M constant under-reserves by 19M/action and is rejected by consensus with OutPointNotEnoughBalance (observed on testnet: required 10,038,000,000 vs 10,000,000,000 reserved for a 2-action bundle). The live network value (v8 = 22M) is unchanged, so this alters no deployed consensus outcome. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThe pull request updates protocol validation configurations across seven ABCI validation version files (v1–v7). The primary change unifies the ChangesProtocol Fee and Validation Updates
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
|
✅ Review complete (commit cdb9b30) |
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
Narrow, correct fix unifying the dormant placeholder shielded_per_action_processing_fee (3M → 22M) in validation versions v1-v7 to match v8. Shielded transitions activate only at platform v12 (validation v8, already 22M), so no historical block ever priced a shielded action under the old constant — change is not consensus-affecting. The fix closes a real stale-protocol-version client-side under-reservation hazard documented in the commit message with an observed testnet failure mode.
|
✅ DashSDKFFI.xcframework built for this PR.
SwiftPM (host the zip at a stable URL, then use): .binaryTarget(
name: "DashSDKFFI",
url: "https://your.cdn.example/DashSDKFFI.xcframework.zip",
checksum: "1ce84a2ea7f0e5f0282a7a9fa003e3e9a07918a76831c9a35a2ffd1460d5be09"
)Xcode manual integration:
|
Issue being fixed or feature implemented
The shielded per-action processing fee (
shielded_per_action_processing_fee) was only calibrated to its real value — 22,000,000 credits, pricing the ~1.1 ms/action Halo 2 verification CPU — in validation v8 (#3800). Validation v1–v7 still carried a 3,000,000 placeholder.This skew is a live hazard for any client whose protocol-version view lags the network. The SDK auto-detect ratchet seeds at an older protocol version and only advances after it parses a metadata-bearing platform response. A wallet computing the
ShieldFromAssetLockpool fee while still seeded below v12 uses the 3M constant, so it reserveslock_value − (proof_fee + actions × 3M + albc)instead of the22M-priced amount consensus actually charges. The transition then under-funds by19M × num_actionsand is rejected withIdentityAssetLockTransactionOutPointNotEnoughBalanceError.Observed on testnet (4.0.0-rc.2): shielding 0.1 tDASH built a valid 2-action bundle but was rejected —
credits_required: 10,038,000,000vscredits_left: 10,000,000,000. The 38M shortfall is exactly2 actions × 19M, the v8 bump the client never saw (its shielded sync queries were failing UNIMPLEMENTED against stale evonodes, so no v12 metadata ever arrived to advance the ratchet).What was done?
Set
shielded_per_action_processing_feeto22_000_000in validation versions v1 through v7 (v8 already had it), making the constant uniform across every protocol version. Each changed site carries a comment explaining why a frozen older version's constant is being updated.Because the per-action fee is now version-invariant, a client that computes the fee under any protocol version reserves the consensus-correct amount — the version-skew can no longer produce a wrong reservation. This is a more robust fix than forcing every fee-sensitive client path to refresh its protocol version before building.
How Has This Been Tested?
cargo check -p platform-version— clean.cargo test -p dpp --lib compute_minimum_shielded_fee— all 4 shielded-fee tests pass (incl.compute_minimum_shielded_fee_v0_equals_historical_formula, which reads the constant dynamically viaPlatformVersion::latest()).cargo fmtapplied.3_000_000value; the only references are the version constants themselves and the formula that reads them generically.Breaking Changes
None to any deployed network. The shielded family does not activate until platform v12 (validation v8), which already used
22M— so no block under any protocol version has ever priced a shielded action at the old3Mplaceholder. Re-validating historical blocks is unaffected (they contain no shielded transitions), and the live network's fee is unchanged. This is why the change is not marked consensus-breaking (fix:, notfix!:).Checklist:
For repository code-owners and collaborators only
🤖 Generated with Claude Code
Summary by CodeRabbit