Category: bug
Problem
In dabdub_contracts/contracts/batch_payments/src/lib.rs, create_batch's payment ID derivation (lines 137-143) is:
let seed: u64 = (env.ledger().sequence() as u64) * 1000 + i as u64;
let id_bytes: BytesN<32> = env.crypto().sha256(&Bytes::from_slice(&env, &seed.to_be_bytes())).into();
The seed depends only on the current ledger sequence number and the item's index within its own batch (i, 0..19) — it does not include the calling merchant, any per-invocation nonce, or the batch's contents. The author's own comment acknowledges this: "In production this would be a proper UUID or hash of inputs." Separately, the PaymentRecord struct (lines 30-38) is defined but never constructed or stored anywhere in the contract — grep -n "PaymentRecord" src/lib.rs only matches its own definition.
Impact
Two different merchants (or the same merchant twice) invoking create_batch within the same ledger — plausible any time multiple batch-payment transactions land in the same ~5-second ledger close — will independently compute identical seed values for equal indices and therefore generate identical id_bytes for unrelated payments. Since the contract emits these IDs as the sole PaymentCreated event identifier and stores no on-chain record to detect or reject a duplicate, an off-chain indexer keying on this ID can silently conflate two different merchants' payments.
Suggested fix
Include merchant and a monotonically increasing per-contract counter (or the full PaymentInput contents) in the hash preimage, matching the approach already used in admin_timelock::schedule_change (Counter in storage, hashed together with content). Either persist PaymentRecords on-chain for auditability or remove the unused struct.
Category: bug
Problem
In
dabdub_contracts/contracts/batch_payments/src/lib.rs,create_batch's payment ID derivation (lines 137-143) is:The seed depends only on the current ledger sequence number and the item's index within its own batch (
i, 0..19) — it does not include the callingmerchant, any per-invocation nonce, or the batch's contents. The author's own comment acknowledges this: "In production this would be a proper UUID or hash of inputs." Separately, thePaymentRecordstruct (lines 30-38) is defined but never constructed or stored anywhere in the contract —grep -n "PaymentRecord" src/lib.rsonly matches its own definition.Impact
Two different merchants (or the same merchant twice) invoking
create_batchwithin the same ledger — plausible any time multiple batch-payment transactions land in the same ~5-second ledger close — will independently compute identicalseedvalues for equal indices and therefore generate identicalid_bytesfor unrelated payments. Since the contract emits these IDs as the solePaymentCreatedevent identifier and stores no on-chain record to detect or reject a duplicate, an off-chain indexer keying on this ID can silently conflate two different merchants' payments.Suggested fix
Include
merchantand a monotonically increasing per-contract counter (or the fullPaymentInputcontents) in the hash preimage, matching the approach already used inadmin_timelock::schedule_change(Counterin storage, hashed together with content). Either persistPaymentRecords on-chain for auditability or remove the unused struct.