aldabra/audits/2026-05-09-escrow-internal-audit.md

Escrow internal audit — 2026-05-09

Subagent-driven correctness/security audit of the escrow surface before preprod deployment. Two HIGH findings on the validator, several MED on the off-chain builders + MCP layer, LOW polish items.

This is internal review only — third-party audit still required before any mainnet deployment. Findings here are best-effort by an agent reviewing the code; assume they don't catch everything.

Validator hash before / after

	hash	UPLC chars
Pre-fix	223aa7ace4a98ff5b8f8988c1c07b846c046de1a2bc9e8dc77411486	7902
Post-fix	a8081acef26935d9b5f44b92052178e17301b6d6e6808c91c5b56f5d	8414

The CBOR grew ~6.5% from the two HIGH-fix checks. The script address derived from the new hash is the v1 deploy target.

HIGH findings

HIGH-1 — Deposit redeemer didn't bound net_added to non-negative

File: aiken-escrow/validators/escrow.ak Deposit branch.

What was wrong:

let net_added = value_to_flat(merge(new_value, negate(in_value)))
let expected = expected_deposits_after(d.deposits, contributor, net_added)
cbor.serialise(expected) == cbor.serialise(new_d.deposits)

flatten(merge(new_value, negate(in_value))) includes negative quantities when new_value < in_value component-wise. A depositor could construct a tx where new_value had MORE ADA but FEWER tokens than in_value, producing net_added = (+5 ADA, -50 TOKEN). expected_deposits_after would write the matching deposit entry as (value: ADA=15, TOKEN=50), and the cbor equality check would pass. The 50 missing tokens flow out as the depositor's wallet change — token theft.

Latent under v1 ADA-only MCP usage (no tokens ever land in the script UTxO), but the validator must hold against any caller, not just our MCP layer.

Fix:

expect Some((new_d, new_value)) =
  find_continuing_output(self.outputs, script_addr)
expect value_geq_value(new_value, in_value)  // NEW
let net_added = value_to_flat(merge(new_value, negate(in_value)))

HIGH-2 — Vacuous refund_outputs_satisfy([], _) on Veto + Refund

File: aiken-escrow/validators/escrow.ak Veto + Refund branches.

What was wrong:

refund_outputs_satisfy(tx_outputs, deposits) is list.all(deposits, ...). When deposits = [], it's vacuously True. The validator passes, the input UTxO is consumed, no refund outputs are required, and the input's lovelace flows out as the driver's wallet change — funds theft.

Trigger paths (all observed pre-fix):

1. escrow_open with initial_contributor=None and initial_lovelace>0 creates exactly this state (deposits=[], in_value>0).
2. Any escrow with sum(deposits.values) < in_value — e.g., an escrow griefed via a token send that landed in the same UTxO via a coin- selection edge case. Currently impossible under MCP usage but the validator must hold against direct CBOR construction.

Fix (validator-side):

Veto -> {
  expect Agreed { .. } = d.state
  expect signed_by(self, d.party_a) || signed_by(self, d.party_b)
  expect value_eq(deposits_to_value(d.deposits), in_value)  // NEW
  refund_outputs_satisfy(self.outputs, d.deposits)
}

Refund -> {
  expect d.state == Open
  expect Some(lower) = tx_lower_ms(self)
  expect lower > d.open_deadline_ms
  expect value_eq(deposits_to_value(d.deposits), in_value)  // NEW
  refund_outputs_satisfy(self.outputs, d.deposits)
}

deposits_to_value and value_eq are new helpers: fold every deposit's FlatValue into a Value via assets.add, then value_geq_value both directions for equality. Component-wise.

Fix (builder-side, defense in depth):

escrow_open now refuses initial_contributor=None. New test rejects_no_initial_contributor. The "open empty, top up later" UX was never useful — party_a can just open + first-deposit in a single call.

MED findings (addressed)

MED-2/3 — Settle/Refund used Koios block_time*1000 for lower_ms

File: crates/aldabra-mcp/src/tools.rs (escrow_settle_unsigned, escrow_refund_timeout_unsigned)

The chain reconstructs lower from valid_from_slot(slot) via slot↔ms math. Koios's block_time field can drift up to ~1s from the slot's true ms. At the strict-> validator boundary (lower > agreed_at + lock_period, lower > open_deadline_ms), the off-chain preflight could pass while the chain rejects. Fee + collateral burned.

Fix: derive validity_lower_ms = slot_to_posix_ms(network, validity_lower_slot) in both tools so the off-chain ms matches what the chain reconstructs.

MED-5 — escrow_open min-utxo floor too low

File: crates/aldabra-dao/src/builder/escrow_open.rs

Used params.min_utxo_lovelace (default 1_000_000). Conway-era inline- datum + script-address outputs need ~1.4-1.7 ADA depending on datum size. Tx could pass preflight, fail at pallas-txbuilder build.

Fix: hardcoded ESCROW_OPEN_MIN_LOVELACE = 2_000_000 to match the existing ESCROW_OUTPUT_MIN_LOVELACE used by Deposit.

MED-7 — escrow_open_unsigned silently discarded fee_lovelace

File: crates/aldabra-mcp/src/tools.rs (escrow_open_unsigned)

fee_lovelace was on the args struct with let _ = fee_lovelace; — caller's value ignored, payment-extras builder did its own estimation. Misleading.

Fix: removed fee_lovelace from EscrowOpenUnsignedArgs. The unsigned-tx builder still auto-estimates.

LOW findings (addressed)

LOW — qty as u64 truncated negative i128 in veto + refund_timeout

File: crates/aldabra-dao/src/builder/escrow_veto.rs, escrow_refund_timeout.rs

A corrupt or adversarial datum with negative qty would silently wraparound. Defensive depth.

Fix: u64::try_from(qty).map_err(...) — surfaces clear error.

Findings deferred to v2

These were flagged but not addressed in this round; v1 ADA-only scope side-steps them.

• MED-1, MED-6: build_escrow_spend_in hardcodes assets: vec![] on the spend input. Multi-asset escrows would have outputs missing tokens. v2 needs an escrow_in_assets arg or chain-side discovery.
• MED-4: EscrowUtxoIn.lovelace is caller-supplied, no chain cross-check. Wrong value silently produces a chain-rejecting tx. v2 should fetch via Koios.
• LOW: negative open_deadline_ms / lock_period_ms not rejected. Edge-case; v2 polish.
• LOW: change_address could theoretically equal escrow_script_address. MCP layer always uses wallet address so safe in practice.
• LOW: find_continuing_output returns None on DatumHash outputs, builders only emit InlineDatum so no real risk.
• LOW: fetch_tip_slot_ms error messages are generic.

Tests after fixes

• 36 escrow builder tests pass (added rejects_no_initial_contributor).
• 132 dao tests pass.
• aldabra-mcp release build clean.

Audit lineage

This was an automated audit by a subagent — it is NOT a third-party review. The validator must still go through external audit (TxPipe / Anastasia Labs / MLabs / Tweag) before any mainnet deployment.