Area
contracts/pool/src/lib.rs (deposit_batch, __constructor)
Problem
Two small but real input-validation gaps in the pool contract:
deposit_batch (lib.rs:357-410) only checks the aggregate next_index + count > MAX_LEAVES; there's no independent cap on commitments.len(). Each iteration does two persistent writes plus up to TREE_DEPTH (20) instance writes, so a large-enough batch can exceed Soroban's per-transaction CPU/resource budget and revert with an opaque host error instead of a clear PoolError.
__constructor (lib.rs:296-311) stores deposit_amount: i128 with no check that it's > 0. A zero or negative deposit_amount would let deposit/deposit_batch mint a valid leaf while transferring nothing (or attempting an invalid transfer), and contracts/compliance's amount_to_field_bytes casts amount as u128, which mishandles a negative i128 silently. Low likelihood (it's a one-time, operator-controlled deploy parameter) but cheap to guard.
Proposed change
Add a MAX_BATCH_SIZE constant and a BatchTooLarge error checked up front in deposit_batch, documented alongside TREE_DEPTH/MAX_LEAVES. Add an InvalidDepositAmount error returned by the constructor if deposit_amount <= 0.
Acceptance Criteria
Open your PR against the dev branch, not main. All active development merges into dev.
Discuss this issue / coordinate work: join the DShield contributor Telegram group: https://t.me/+SiGHH24No9U2MDJk
Area
contracts/pool/src/lib.rs (
deposit_batch,__constructor)Problem
Two small but real input-validation gaps in the pool contract:
deposit_batch(lib.rs:357-410) only checks the aggregatenext_index + count > MAX_LEAVES; there's no independent cap oncommitments.len(). Each iteration does two persistent writes plus up toTREE_DEPTH(20) instance writes, so a large-enough batch can exceed Soroban's per-transaction CPU/resource budget and revert with an opaque host error instead of a clearPoolError.__constructor(lib.rs:296-311) storesdeposit_amount: i128with no check that it's> 0. A zero or negativedeposit_amountwould letdeposit/deposit_batchmint a valid leaf while transferring nothing (or attempting an invalid transfer), andcontracts/compliance'samount_to_field_bytescastsamount as u128, which mishandles a negativei128silently. Low likelihood (it's a one-time, operator-controlled deploy parameter) but cheap to guard.Proposed change
Add a
MAX_BATCH_SIZEconstant and aBatchTooLargeerror checked up front indeposit_batch, documented alongsideTREE_DEPTH/MAX_LEAVES. Add anInvalidDepositAmounterror returned by the constructor ifdeposit_amount <= 0.Acceptance Criteria
deposit_batchrejects batches overMAX_BATCH_SIZEwith a clearBatchTooLargeerror (tested)deposit_amount <= 0withInvalidDepositAmount(tested)just test,pnpm test,cargo test, ornargo testas applicable) pass locally and in CIcargo testoutput for both new checks) — required before this can be merged.Open your PR against the
devbranch, notmain. All active development merges intodev.Discuss this issue / coordinate work: join the DShield contributor Telegram group: https://t.me/+SiGHH24No9U2MDJk