feat(dpp): add max_asset_lock_transaction_inputs limit to prevent stuck funds#3491
feat(dpp): add max_asset_lock_transaction_inputs limit to prevent stuck funds#3491
Conversation
…ck funds Asset lock transactions with too many inputs (from many small UTXOs) can exceed Platform's 20KB state transition size limit. Core accepts the transaction but Platform rejects it, leaving funds stuck. This adds a configurable input count limit (default 100) that validates early with a clear error message advising users to consolidate UTXOs first. Closes #3399 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 59 minutes and 57 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (13)
✨ Finishing Touches🧪 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 |
|
Needs to be tested with dash-evo-tool |
There was a problem hiding this comment.
Pull request overview
Adds a protocol-configured cap on Asset Lock transaction input count so oversized asset-lock state transitions are rejected before Core broadcast, avoiding “stuck funds” scenarios and providing a clearer, actionable consensus error.
Changes:
- Introduces
max_asset_lock_transaction_inputs(set to100) in platform version configs (v1–v3). - Enforces the new limit during asset lock transaction structure validation and adds unit tests around the boundary.
- Wires a new consensus error (code
10534) through Rust + WASM error surfaces.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/rs-platform-version/src/version/dpp_versions/dpp_state_transition_versions/mod.rs | Adds max_asset_lock_transaction_inputs to identity asset-lock version config struct. |
| packages/rs-platform-version/src/version/dpp_versions/dpp_state_transition_versions/v1.rs | Sets max asset-lock inputs to 100 for v1. |
| packages/rs-platform-version/src/version/dpp_versions/dpp_state_transition_versions/v2.rs | Sets max asset-lock inputs to 100 for v2. |
| packages/rs-platform-version/src/version/dpp_versions/dpp_state_transition_versions/v3.rs | Sets max asset-lock inputs to 100 for v3. |
| packages/rs-dpp/src/identity/state_transition/asset_lock_proof/validate_asset_lock_transaction_structure/mod.rs | Threads platform-version limit into v0 validator call. |
| packages/rs-dpp/src/identity/state_transition/asset_lock_proof/validate_asset_lock_transaction_structure/v0/mod.rs | Implements input-count guard and adds unit tests for 1/100/101 inputs. |
| packages/rs-dpp/src/errors/consensus/basic/identity/mod.rs | Exposes the new identity consensus error type. |
| packages/rs-dpp/src/errors/consensus/basic/basic_error.rs | Adds the new BasicError enum variant for the consensus error. |
| packages/rs-dpp/src/errors/consensus/codes.rs | Assigns consensus error code 10534. |
| packages/rs-dpp/src/errors/consensus/basic/identity/identity_asset_lock_transaction_too_many_inputs_error.rs | Defines the new consensus error type and message. |
| packages/wasm-dpp/src/errors/consensus/basic/identity/mod.rs | Registers the new WASM error module + re-export. |
| packages/wasm-dpp/src/errors/consensus/basic/identity/identity_asset_lock_transaction_too_many_inputs_error.rs | Adds WASM bindings/accessors for the new error type. |
| packages/wasm-dpp/src/errors/consensus/consensus_error.rs | Maps the new BasicError variant into the WASM consensus error conversion. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -0,0 +1,47 @@ | |||
| use crate::consensus::basic::BasicError; | |||
| use crate::consensus::ConsensusError; | |||
| use crate::errors::ProtocolError; | |||
There was a problem hiding this comment.
ProtocolError is imported but never used in this error type, which will generate an unused-import warning. Please remove the import (or use it if it was intended).
| use crate::errors::ProtocolError; |
| return ConsensusValidationResult::new_with_error( | ||
| IdentityAssetLockTransactionTooManyInputsError::new(input_count as u16, max_inputs) | ||
| .into(), |
There was a problem hiding this comment.
input_count is a usize but is cast to u16 when constructing the error. If a transaction ever has more than u16::MAX inputs, actual_inputs will wrap/truncate and the error will report the wrong count. Consider using a fallible conversion (u16::try_from) and clamping, or store actual_inputs as a wider type (e.g. u32) in the error.
| return ConsensusValidationResult::new_with_error( | |
| IdentityAssetLockTransactionTooManyInputsError::new(input_count as u16, max_inputs) | |
| .into(), | |
| let actual_inputs = u16::try_from(input_count).unwrap_or(u16::MAX); | |
| return ConsensusValidationResult::new_with_error( | |
| IdentityAssetLockTransactionTooManyInputsError::new(actual_inputs, max_inputs).into(), |
| fn should_accept_transaction_at_max_inputs() { | ||
| let tx = make_asset_lock_transaction(100); | ||
| let result = validate_asset_lock_transaction_structure_v0(&tx, 0, 100); | ||
|
|
||
| let has_too_many_inputs_error = result.errors.iter().any(|e| { |
There was a problem hiding this comment.
should_accept_transaction_at_max_inputs only asserts that the "too many inputs" error is absent; it would still pass if other validation errors were returned. To make the test actually verify acceptance, assert result.is_valid() (similar to should_accept_transaction_with_one_input).
|
⏳ Review in progress (commit 3656c69) |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## v3.1-dev #3491 +/- ##
=========================================
Coverage 84.83% 84.83%
=========================================
Files 2476 2476
Lines 267733 267822 +89
=========================================
+ Hits 227123 227202 +79
- Misses 40610 40620 +10
🚀 New features to boost your workflow:
|
Issue being fixed or feature implemented
Fixes #3399
Imagine you are a user funding a Platform address with tDASH accumulated from many small mining rewards. You send 400 tDASH, the Core transaction succeeds — but Platform rejects the state transition with a cryptic "Tx too large" error. Your funds are stuck: gone from Core wallet, never credited to Platform. This change catches the problem before the Core broadcast, with a clear error message telling you to consolidate UTXOs first.
What was done?
Added a
max_asset_lock_transaction_inputslimit (100) to prevent asset lock transactions from exceeding Platform's 20KBmax_state_transition_sizelimit.Size math: Each input adds ~184 bytes to the state transition (148 bytes for the transaction input + 36 bytes for the InstantLock outpoint). At 100 inputs the total is ~18,877 bytes — fits within the 20,480 byte limit with ~1,600 bytes (~8%) margin.
Changes across 3 packages:
rs-platform-version:max_asset_lock_transaction_inputs: u16toIdentityTransitionAssetLockVersions100in all version configs (v1, v2, v3)rs-dpp:IdentityAssetLockTransactionTooManyInputsError(code 10534) with actionable message: "Consolidate UTXOs before creating the asset lock"validate_asset_lock_transaction_structure_v0wasm-dpp:getMaxInputs(),getActualInputs(),getCode(), andmessageaccessorsHow Has This Been Tested?
cargo check -p dpp --features=validation— cleancargo check -p dash-sdk— cleancargo check -p drive-abci— cleancargo test -p dpp --features=validation -- asset_lock_transaction— 5/5 pass (3 new + 2 existing)Breaking Changes
None. The new limit (100 inputs) is well above typical transaction sizes. Only transactions that would have been rejected by Platform's existing 20KB state transition size limit are now caught earlier with a better error message.
Checklist:
For repository code-owners and collaborators only
🤖 Co-authored by Claudius the Magnificent AI Agent