[romchornyi]

Fixes #1190.

A wallet whose CoinJoin account holds more than 100 coins cannot shield. Every CoinJoin → Shielded attempt builds an asset lock that spends every mixed UTXO. Dash Core (v23.1.7+, and the v24 line) refuses an asset lock with more than 100 inputs as non-standard (IsStandardSpecialTx: MAX_STANDARD_ASSET_LOCK_INPUTS{100} → assetlocktx-too-many-inputs; above 20 480 bytes → assetlocktx-too-big). Peers drop such a lock without a reject, so the wallet only ever sees a 60 s Uncertain broadcast. Platform enforces the same 100-input cap in consensus (max_asset_lock_transaction_inputs, dashpay/platform#3491). key-wallet applied only the regular-transaction cap of 500 (MAX_STANDARD_TX_INPUTS) to asset locks. On mainnet this blocked a user with 104 mixed coins for a month (support case 32500); submitting the lock through DAPI returned assetlocktx-too-many-inputs.

The limits. MAX_ASSET_LOCK_INPUTS = 100 and MAX_ASSET_LOCK_TX_SIZE = 20_480 live next to MAX_STANDARD_TX_INPUTS in transaction_builder and are re-exported from asset_lock_builder. Any TransactionBuilder build that carries an asset-lock payload is held to them:

• Exact-amount locks. Coin selection stays within 100 inputs. When the configured strategy needs more, or runs short, the selector falls back to largest-first. An amount that cannot be funded within the cap returns BuilderError::TooManyInputs { count, max: 100 } before anything is reserved, with count the fewest inputs the amount actually needs.
• Size. It is checked before signing, against an upper bound on the signed size (assembled unsigned transaction plus a 107-byte P2PKH scriptSig per input). If an exact-amount pick is over the limit, the builder retries largest-first. The signed paths check the real size again after signing, so a signer that produces larger scriptSigs is caught, and they release their reservation (owner-guarded). Oversize is reported as the new typed BuilderError::AssetLockTooLarge { size, max }.
• P2PKH only. Non-P2PKH candidates are left out of an asset-lock build, because the size bound and the signer assume P2PKH.

Bounded drain. build_asset_lock / build_asset_lock_with_signer in drain mode no longer spend every final UTXO:

• When more than 100 are available, they lock the 100 largest. If those are still over the size limit, they drop the smallest of them until the lock fits.
• What was left behind is reported in two new AssetLockResult fields, remaining_utxo_count and remaining_duffs. They are 0 for a full drain and for exact-amount builds, and they count only final, unreserved, spendable P2PKH candidates.
• A drain through the generic TransactionBuilder is not bounded. Over 100 candidates it returns TooManyInputs, so no caller gets a partial drain without being told.
• Regular (non-asset-lock) builds are unchanged, including the 500-input refusal.

Not in scope.

• An InstantSend asset-lock proof embeds the whole transaction and an IS lock listing every outpoint. A transition built on a lock near the cap can therefore exceed Platform's 20 480-byte transition limit; fix(platform-wallet): retry an oversized InstantSend asset-lock transition with a ChainLock proof platform#5460 adds the ChainLock-proof fallback for that.
• platform-wallet has to read remaining_* to report a partial move (platform-wallet: asset-lock drains are not bounded by max_asset_lock_transaction_inputs, so CoinJoin → Shielded never completes for wallets with more than 100 mixed coins platform#5478).
• The canonical description of all of this is the "Asset-lock limits and the bounded drain" section on build_asset_lock.

Testing.

• cargo test -p key-wallet --all-features: 709 passed.
• cargo test -p key-wallet-ffi: 277 passed.
• fmt, clippy --all-features --all-targets -D warnings and cargo doc -D warnings are clean.
• New tests cover:
  • a 104-UTXO drain that takes the 100 largest and reports 4 left;
  • a drain at exactly 100;
  • the signer drain;
  • shrinking to the size limit and surfacing the reselection error;
  • exact amounts within and over the cap;
  • the size-aware fallback;
  • oversized signatures caught after signing, with the reservation released;
  • non-P2PKH filtering;
  • the generic-builder drain refusal;
  • the bounded selector's fallback, tie-break and zero bound.

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features
  • Asset-lock transactions now support bounded selection, using the largest eligible inputs that fit the input and transaction-size limits. Any eligible funds left unspent are reported.
  • Asset-lock builds that exceed the limits now return an error instead of producing an oversized transaction.
  • Drain transactions can include zero-value OP_RETURN outputs while retaining the requirement for exactly one value-carrying output.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: dashpay/rust-dashcore/.coderabbit.yaml
• Review profile: CHILL
• Plan: Advanced
• Run ID: 03bedab7-59c5-4ce3-9a7d-815e8809e855

📥 Commits

Reviewing files that changed from the base of the PR and between d35a8d6 and 7fad4a3.

📒 Files selected for processing: 4

• key-wallet/src/test_utils/utxo.rs
• key-wallet/src/wallet/managed_wallet_info/asset_lock_builder.rs
• key-wallet/src/wallet/managed_wallet_info/coin_selection.rs
• key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

Asset-lock builds now enforce input and serialized-size limits. Bounded drains select eligible UTXOs within those limits and report eligible UTXOs left unspent. Exact-amount builds remain capped and return errors when they exceed the limits.

Changes: Asset-lock limits and bounded drains

Layer / File(s)	Summary
Bounded coin selection key-wallet/src/wallet/managed_wallet_info/coin_selection.rs	CoinSelector accepts an optional input limit. Bounded drains select the largest spendable UTXOs; other strategies can fall back to largest-first selection.
Asset-lock transaction limits key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs	TransactionBuilder enforces asset-lock input and serialized-size limits. Oversized signed transactions release their reservations and return AssetLockTooLarge.
Bounded asset-lock reporting key-wallet/src/wallet/managed_wallet_info/asset_lock_builder.rs, key-wallet/src/test_utils/utxo.rs	Wallet-key and external-signer builders report eligible UTXOs left unselected. Tests cover bounded drains, exact-amount builds, size limits, and remainder reporting. Dummy UTXOs now use their address script.

Priority: ➖ Normal

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant AssetLockBuilder
  participant TransactionBuilder
  participant CoinSelector
  participant Signer
  AssetLockBuilder->>TransactionBuilder: request signed asset-lock build with reporting
  TransactionBuilder->>CoinSelector: select eligible inputs within configured limit
  CoinSelector-->>TransactionBuilder: selected inputs and unselected candidates
  TransactionBuilder->>TransactionBuilder: assemble transaction and reserve inputs
  TransactionBuilder->>Signer: sign asset-lock transaction
  Signer-->>TransactionBuilder: signed transaction
  TransactionBuilder->>TransactionBuilder: check serialized size and release reservation if oversized
  TransactionBuilder-->>AssetLockBuilder: return transaction, reservation, and unselected totals

Loading

Suggested reviewers: zocolini, quantumexplorer

Merge Risk: ⚪ Minimal · up to 7fad4

Asset-lock builds now stay within Dash Core's 100-input and 20,480-byte limits. Bounded drains report which eligible UTXOs were left unspent, and exact-amount builds that cannot fit return typed errors. No concrete defect was identified in the change. Downstream consumers may need small updates for the new error variant and result fields.

Pre-merge checks: 5

Check name	Status	Explanation
Linked Issues check		Issue #1190 requires asset-lock builds to stay within 100 inputs and 20,480 bytes, bounded drains, typed exact-amount failures, and reservation release after an oversized signed build. The reviewed ch…
Out of Scope Changes check		The changes remain connected to issue #1190. The transaction-builder and coin-selection changes enforce asset-lock limits and bounded-drain behavior. The result fields, documentation, fixtures, and te…
Docstring Coverage		Docstring coverage is 83.33% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 60 functions across 4 files.
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: enforcing Core's 100-input and 20,480-byte limits for key-wallet asset locks.

Full details: Linked Issues check

Explanation

Issue #1190 requires asset-lock builds to stay within 100 inputs and 20,480 bytes, bounded drains, typed exact-amount failures, and reservation release after an oversized signed build. The reviewed changes add and re-export both limits, apply asset-lock-specific selection and size checks in TransactionBuilder, retry exact-amount selection with largest-first, and return TooManyInputs or AssetLockTooLarge. Drain builders select bounded largest UTXOs, shrink for size, filter to P2PKH candidates, report remaining_utxo_count and remaining_duffs, and release reservations when the signed size fails. Generic asset-lock drains fail over the cap, while regular transaction behavior keeps the 500-input limit. The summary and focused source evidence also show bounded selector fallback behavior and coverage for the required cases.

Full details: Out of Scope Changes check

Explanation

The changes remain connected to issue #1190. The transaction-builder and coin-selection changes enforce asset-lock limits and bounded-drain behavior. The result fields, documentation, fixtures, and tests support partial-drain reporting and validation. The noted InstantSend proof, Platform transition, and platform-wallet reporting work is explicitly outside this pull request and is not presented as an unmet coding requirement.

✨ Finishing Touches

📝 Generate docstrings

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

Waiting for bot review — coderabbitai not yet. Wait for the missing reviews, or a writer can post /skip-bots to proceed without them; blocking findings still need addressing.
Full checklist in the description.

[github-actions]

Bots are done — your move: post /self-reviewed.
Full checklist in the description.

[codecov]

Codecov Report

❌ Patch coverage is 98.21216% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.05%. Comparing base (d35a8d6) to head (7fad4a3).

Files with missing lines	Patch %	Lines
.../wallet/managed_wallet_info/transaction_builder.rs	96.61%	11 Missing ⚠️
...c/wallet/managed_wallet_info/asset_lock_builder.rs	98.77%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##              dev    #1191      +/-   ##
==========================================
+ Coverage   78.86%   79.05%   +0.19%     
==========================================
  Files         304      304              
  Lines       81381    82126     +745     
==========================================
+ Hits        64178    64924     +746     
+ Misses      17203    17202       -1     

Flag	Coverage Δ
core	77.59% <ø> (ø)
ffi	50.69% <ø> (+<0.01%)	⬆️
rpc	52.05% <ø> (ø)
spv	92.82% <ø> (+0.04%)	⬆️
wallet	82.70% <98.21%> (+0.50%)	⬆️

Files with missing lines	Coverage Δ
...t/src/wallet/managed_wallet_info/coin_selection.rs	95.24% <100.00%> (+1.29%)	⬆️
...c/wallet/managed_wallet_info/asset_lock_builder.rs	95.06% <98.77%> (+1.24%)	⬆️
.../wallet/managed_wallet_info/transaction_builder.rs	94.57% <96.61%> (+0.45%)	⬆️

... and 7 files with indirect coverage changes

[ZocoLini]

1400 lines for somehting that we already have implemented for a different type of transaction its too much, i think this can reuse what we already have for the 500-input cap instead of adding new machinery:

• Make the existing TooManyInputs check use a per-type limit: 100 for asset locks, 500 otherwise.
• For the drain, seed the 100 largest coins with add_inputs + add_funding_reservation_only, so the build only spends those.
• If exact-amount selection picks more than the limit, retry largest-first.

100 P2PKH inputs stay well under 20 480 bytes, so the size checks aren't needed, and the remaining-funds fields can go: the caller can read them from the balance. That should be a few dozen lines plus a test.

[romchornyi]

Closing in favour of a wallet-side fix: the app consolidates CoinJoin coins before the drain asset lock (dashpay/dashwallet-ios#1221), the same approach as the Android wallet. The underlying SDK gap stays tracked in #1190.