test(dpp,drive-abci): cover transfer-key signing rules for token transfers#3766
Conversation
…sfers Add DPP signing tests for standalone token transfers signed with critical authentication and transfer keys, plus rejection coverage for high authentication keys and transfer keys in multi-transition batches. Add ABCI coverage confirming a standalone token transfer signed with a critical transfer key is accepted end to end.
|
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 (2)
📝 WalkthroughWalkthroughThis PR adds test infrastructure and test cases to validate signing token transfer batches with transfer keys. It introduces a ChangesToken Transfer Key Signing
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## v3.1-dev #3766 +/- ##
=========================================
Coverage 87.17% 87.17%
=========================================
Files 2607 2607
Lines 319589 319589
=========================================
+ Hits 278602 278614 +12
+ Misses 40987 40975 -12
🚀 New features to boost your workflow:
|
|
✅ 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: "dde4b2f57e35999cf3f789f4e054b7431db6d163e25b42319c8b98078585f190"
)Xcode manual integration:
|
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
|
Actionable comments posted: 0 |
Issue being fixed or feature implemented
Token-transfer signing has a split validation path:
sign_externalenforces token-batch security up front, while Drive's signature validation and the batch's advanced structure validation re-derive purpose/security from the batched transitions. The declared rule — a standalone token transfer acceptsAUTHENTICATIONorTRANSFER(CRITICAL), while multi-transition batches requireAUTHENTICATION(CRITICAL) — was only thinly exercised:CRITICAL TRANSFERkey actually clears advanced structure validation.Without these, a manually signed transition could plausibly pass one validation layer and fail another, and we'd have no regression net for the boundary.
What was done?
DPP unit tests in
batch_transition/tests.rs:signing_single_token_transfer_accepts_critical_authentication_keysigning_single_token_transfer_accepts_critical_transfer_keysigning_single_token_transfer_rejects_high_authentication_key— expectsInvalidSignaturePublicKeySecurityLevelErrorsigning_multi_transition_batch_rejects_transfer_key— expectsWrongPublicKeyPurposeErrorDrive-abci end-to-end test in
batch/tests/token/transfer/mod.rs:test_token_transfer_signed_with_transfer_key— signs a standalone token transfer with anECDSA_HASH160CRITICAL TRANSFER key, processes the state transition, and asserts both sender and recipient balances post-execution.These nail down the declared rule at both layers and prove the transfer-key path actually clears advanced structure validation, not just
sign_external.How Has This Been Tested?
cargo test -p dash-sdk-dpp --lib state_transition::state_transitions::document::batch_transition::testscargo test -p drive-abci --lib execution::validation::state_transition::state_transitions::batch::tests::token::transferBreaking Changes
None. Tests only.
Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit
Release Notes