fix(dpp): bind SetPriceForDirectPurchase action_id to full pricing schedule#3357
Conversation
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughAdded a versioned action-id calculation for TokenSetPriceForDirectPurchaseTransition: a public dispatcher Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## v3.1-dev #3357 +/- ##
============================================
+ Coverage 75.50% 75.51% +0.01%
============================================
Files 2912 2912
Lines 281065 281224 +159
============================================
+ Hits 212211 212363 +152
- Misses 68854 68861 +7
🚀 New features to boost your workflow:
|
bc5bd95 to
9d271e2
Compare
…hedule The action_id for TokenSetPriceForDirectPurchaseTransition only hashed minimum_purchase_amount_and_price().1 (the credit price of the lowest tier). This meant different pricing schedules with the same minimum-tier price produced identical action_ids, enabling the same vote-swap attack as the token config update vulnerability fixed in PR #3346. Examples of collisions in the old code: - SetPrices({1: 100, 10: 800}) vs SetPrices({1: 100, 10: 9999}) - SinglePrice(100) vs SetPrices({1: 100}) - SetPrices({5: 100, ...}) vs SetPrices({10: 100, ...}) Fix: serialize the full TokenPricingSchedule with bincode and include it in the hash. This binds the voted-on pricing schedule into the action_id, preventing vote-swap attacks. Note: Unlike the config update fix, this is NOT gated behind a platform version because SetPriceForDirectPurchase is a new feature (v3.1) that has not been used in production yet. There is no backward compatibility concern. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- calculate_action_id_with_fields returns Result with map_err instead of using expect for bincode encoding - Update wrapper and v0 impls to match the AllowedAsMultiPartyAction trait signature from #3346 (platform_version + Result) - Fix all tests to pass platform_version and unwrap Results - Rebase on #3346 branch Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
9d271e2 to
b64180b
Compare
|
✅ 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: "94ddab4d078a45fa7a7bca7acb5121680abfb92ef0e020c1f6cf14e527c6fe90"
)Xcode manual integration:
|
Gate the action_id fix behind platform version, matching the pattern from #3346 (token config update): - Add token_set_price_action_id_version field to DPPTokenVersions - v0: hashes only minimum_purchase_amount_and_price().1 (existing) - v1: hashes the full serialized TokenPricingSchedule (fix) - Create TOKEN_VERSIONS_V3 with both fixes enabled - Update v12 to use TOKEN_VERSIONS_V3 - Add v0 vulnerability regression test and v0-vs-v1 differentiation test Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
TOKEN_VERSIONS_V2 hasn't activated in production yet, so there's no need for a separate V3. Set token_set_price_action_id_version: 1 directly in V2 alongside token_config_update_action_id_version: 1. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…hedule (#3357) Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Issue being fixed or feature implemented
Same class of vote-swap vulnerability as PR #3346, but for
TokenSetPriceForDirectPurchaseTransition.The action_id only hashed
minimum_purchase_amount_and_price().1(the credit price of the lowest tier), so different pricing schedules with the same minimum-tier price produced identical action_ids. A malicious group member could propose a fair pricing schedule, collect votes, then swap to a predatory schedule.Collision examples:
SetPrices({1: 100, 10: 800})vsSetPrices({1: 100, 10: 9999})— same min price, different tiersSinglePrice(100)vsSetPrices({1: 100})— different types, same(1, 100)resultSetPrices({5: 100, ...})vsSetPrices({10: 100, ...})— different minimum amounts, same priceWhat was done?
Replaced
minimum_purchase_amount_and_price().1.to_be_bytes()withbincode::encode_to_vec(price_per_token, bincode::config::standard())— the full serialized pricing schedule is now included in the hash.Not gated behind a platform version because
SetPriceForDirectPurchaseis a new v3.1 feature not yet used in production.How Has This Been Tested?
4 new tests proving:
SetPriceswith same minimum produce different action_idsSinglePrice(100)vsSetPrices({1: 100})produce different action_idsNoneprice vsSomeprice produce different action_idsBreaking Changes
This changes action_id computation for
SetPriceForDirectPurchasetransitions. Since this feature has not been used in production, there are no backward compatibility concerns.Checklist:
🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Chores
Tests