test(drive-abci): pin that a token config update cannot set a perpetual distribution - #4836
Conversation
…al distribution Drive writes a token's perpetual distribution storage only when the token is created (insert_contract). A TokenConfigUpdate goes through ApplyContract with its token already present in the original contract, so nothing would create [Tokens, distributions, perpetual, token_id] if a config update could introduce a distribution, and the first claim would fail inside the grove batch. That path is not reachable: TokenConfigUpdateTransition::validate_structure has refused every PerpetualDistribution change item with UnsupportedFeatureError since #2627, which shipped before v2.0.0, and a data contract update refuses any change to an existing token. The rs-dpp unit test covers the structure check in isolation; this test runs the whole flow on a token whose perpetual_distribution_rules authorize the owner: the update is an unpaid basic rejection, the stored contract is unchanged, and a later claim is an ordinary InvalidTokenClaimPropertyMismatch. Whoever lifts the gate has to make the config update maintain the distribution storage, and decide what happens to stored last-claim moments when the RewardDistributionType changes. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
PR HygieneState: waiting-slot · commit
Self-review is an author attestation that you have read the diff: This report does not bypass CI or repository protection rules. |
|
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 (1)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe token configuration update tests now reject perpetual distribution settings, verify that the token contract remains unchanged, and confirm the expected error from a later perpetual claim. ChangesToken configuration update validation
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Other 🚥 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 |
|
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## v4.2-dev #4836 +/- ##
============================================
+ Coverage 76.96% 77.18% +0.22%
============================================
Files 2963 2963
Lines 429535 429535
============================================
+ Hits 330609 331557 +948
+ Misses 98926 97978 -948
🚀 New features to boost your workflow:
|
…ument-token-costs #4826 landed on v4.2-dev as a squash whose tree is identical to the head this branch already carried, so every conflict is resolved to this branch's side: the squashed base plus the optional token cost changes. The merge brings in nothing else but the token config update test of #4836. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
2 participants
Issue being fixed or feature implemented
While fixing the missing distribution storage for tokens added by a contract update (#4834), a sibling gap was suspected: a
TokenConfigUpdatecarryingTokenConfigurationChangeItem::PerpetualDistribution(..)goes throughApplyContractintoupdate_contract, and nothing on that path creates or refreshes[Tokens, distributions, perpetual, token_id], because Drive only writes that storage when the token is created.Investigation result: the Drive-side hazard is real, but it is not reachable, so there is nothing to fix in consensus code.
TokenConfigUpdateTransition::validate_structurev0 refuses everyPerpetualDistribution(_)change item withUnsupportedFeatureError. It is called from batch basic structure validation, only v0 exists, and every protocol version selects it, including 14.v2.0.0-dev.1andv2.0.0-rc.1torc.11tags had tokens without it, so mainnet never accepted such an update.validate_update_tokens).The existing rs-dpp unit test covers the structure check in isolation. Nothing covered the whole flow, and nothing recorded that Drive relies on this gate. This PR adds that test.
What was done?
Added
test_token_config_update_by_owner_setting_perpetual_distribution_is_unsupportedto the token config update tests in drive-abci. It creates a token with no perpetual distribution whoseperpetual_distribution_rulesauthorize the contract owner, then:UnpaidConsensusErrorwithBasicError::UnsupportedFeatureError(it fails at basic structure, before the identity contract nonce is bumped);StateError::InvalidTokenClaimPropertyMismatch, not an internal error.The doc comment on the test states what Drive depends on, so that whoever lifts the gate sees it.
For whoever lifts the gate later
To check that the hazard is real I disabled the gate locally (not part of this PR) and ran the same scenario. The config update succeeded, and the claim then failed with:
This is the same signature as the bug fixed in #4834. Lifting the gate needs, at minimum:
add_perpetual_distributionv0 returnsCorruptedCodeExecutionwhen the tree already exists, so a change needs its own operation);RewardDistributionTypechanges: block height and timestamp are both 8 bytes and would be silently misread bymoment_from_bytes, epoch is 2 bytes and would raiseCorruptedDriveState;TOKEN_PERPETUAL_DISTRIBUTIONS_INFO_KEYhas no reader (claims use the contract's token configuration), so a stale info item would be inert.How Has This Been Tested?
Passes. The change is test only, inside drive-abci, so that build is also the compile check. The experiment with the gate disabled was run locally and fully reverted; the branch contains one commit touching one test file.
Breaking Changes
None. Test only.
Checklist:
For repository code-owners and collaborators only
🤖 Generated with Claude Code
Summary by CodeRabbit