fix(escrow_wip): apply 2026-05-09 internal audit findings
Two HIGH validator-side bugs + several MED/LOW off-chain issues found in the subagent-driven audit on this branch. New validator hash: a8081acef26935d9b5f44b92052178e17301b6d6e6808c91c5b56f5d. ## HIGH-1: Deposit redeemer let depositors drain tokens aiken-escrow/validators/escrow.ak Deposit branch now requires `value_geq_value(new_value, in_value)` before computing net_added. Previously net_added could carry negative quantities (when new_value < in_value component-wise), letting a depositor write a matching new_d.deposits with reduced values and pocket the difference as wallet change. Latent under v1 ADA-only MCP usage but the validator must hold against all callers. ## HIGH-2: Empty/partial deposits enabled funds drain via Veto/Refund Veto and Refund branches now require `value_eq(deposits_to_value(d.deposits), in_value)` — the tracked deposits must account for the full locked value. Previously `refund_outputs_satisfy(_, [])` was vacuously true on empty deposits, so a driver could fire Veto/Refund on an escrow opened with `initial_contributor=None` (deposits=[], in_value>0) and pocket the input's lovelace as change. Defense in depth: escrow_open builder now refuses `initial_contributor=None`. New helper `deposits_to_value` folds deposit FlatValues into a Value via `assets.add` for the equality check. ## MED: off-chain fixes - escrow_open min-utxo bumped 1M → 2M (Conway-era inline-datum + script-address outputs need ~1.4-1.7 ADA, NOT the 1 ADA default). - escrow_settle_unsigned + escrow_refund_timeout_unsigned now derive `validity_lower_ms` via slot_to_posix_ms(network, slot) instead of Koios's `block_time*1000` — the chain reconstructs `lower` from the slot, so Koios's ~1s drift could pass off-chain preflight while the chain rejects at the strict-`>` boundary. - escrow_open_unsigned MCP tool no longer accepts (and silently discards) `fee_lovelace` — the unsigned-tx builder auto-estimates. ## LOW: defensive depth - escrow_veto + escrow_refund_timeout: `qty as u64` → `u64::try_from` so a corrupt or adversarial datum with negative i128 qty can't slip through with a wraparound. ## Tests - 36 escrow builder tests pass (added rejects_no_initial_contributor) - 132 dao tests pass under --features escrow_wip - aldabra-mcp release build clean ## Infra - Validator artifact files (plutus.json, validator.cbor.hex) regenerated. Dockerfile already wired to bake them at /etc/aldabra/escrow/ for MCP tools' validator_script_path arg. - Internal audit findings written up at aiken-escrow/README.md including the v2-deferred items (multi-asset spend-input, lovelace-not-cross-checked, etc.) Third-party audit still required before any mainnet deployment.
This commit is contained in:
Sulkta
parent
ef38ff0e57
commit
7daa62b5e5
8 changed files with 192 additions and 65 deletions
|
|
@ -88,41 +88,60 @@ pub struct UnsignedEscrowOpen {
|
|||
/// Build the unsigned escrow_open tx.
|
||||
pub fn build_unsigned_escrow_open(args: EscrowOpenArgs) -> DaoResult<UnsignedEscrowOpen> {
|
||||
// ---- preflight ----
|
||||
if let Some(c) = args.initial_contributor {
|
||||
if c != args.party_a_pkh && c != args.party_b_pkh {
|
||||
return Err(DaoError::State(
|
||||
"initial_contributor must be party_a or party_b".to_string(),
|
||||
));
|
||||
}
|
||||
//
|
||||
// HIGH-2 fix (2026-05-09 audit): refuse `initial_contributor=None`
|
||||
// entirely. Previously this opened an escrow with `deposits=[]` and
|
||||
// `initial_lovelace > 0`, which the validator (pre-fix) treated as
|
||||
// a refundable-by-anyone escrow because `refund_outputs_satisfy(_, [])`
|
||||
// is vacuously true. Even with the validator now enforcing
|
||||
// `sum(deposits) == in_value`, the empty-deposits path produces a
|
||||
// permanently-stuck escrow (no Veto/Refund possible until somebody
|
||||
// deposits). Cleaner v1 invariant: every escrow has at least one
|
||||
// contributor at open. The "open empty, top up later" UX was never
|
||||
// useful anyway — party_a can just open + deposit in a single call.
|
||||
let initial_contributor = args.initial_contributor.ok_or_else(|| {
|
||||
DaoError::State(
|
||||
"initial_contributor is required — open an escrow with at least one contributor (party_a or party_b)"
|
||||
.into(),
|
||||
)
|
||||
})?;
|
||||
if initial_contributor != args.party_a_pkh && initial_contributor != args.party_b_pkh {
|
||||
return Err(DaoError::State(
|
||||
"initial_contributor must be party_a or party_b".to_string(),
|
||||
));
|
||||
}
|
||||
// The output must clear the validator's min-utxo for an inline-datum
|
||||
// + asset-bearing output. We use the protocol-param floor as a lower
|
||||
// bound; the actual min depends on serialized output size and is
|
||||
// computed by pallas-txbuilder when building the tx.
|
||||
if args.initial_lovelace < args.params.min_utxo_lovelace {
|
||||
|
||||
// MED-5 fix (2026-05-09 audit): the escrow output bears an inline
|
||||
// datum + sits at a script address — Conway-era min-utxo computes
|
||||
// to ~1.4-1.7 ADA depending on datum size, NOT the 1 ADA default.
|
||||
// Bump our floor to match the deposit/spend builders' constant
|
||||
// (2 ADA) so an open tx that passes preflight also passes
|
||||
// pallas-txbuilder's actual min-utxo computation.
|
||||
const ESCROW_OPEN_MIN_LOVELACE: u64 = 2_000_000;
|
||||
if args.initial_lovelace < ESCROW_OPEN_MIN_LOVELACE {
|
||||
return Err(DaoError::State(format!(
|
||||
"initial_lovelace {} below min_utxo_lovelace {}",
|
||||
args.initial_lovelace, args.params.min_utxo_lovelace
|
||||
"initial_lovelace {} below escrow output min-utxo floor {ESCROW_OPEN_MIN_LOVELACE}",
|
||||
args.initial_lovelace
|
||||
)));
|
||||
}
|
||||
|
||||
// ---- build the datum ----
|
||||
let deposits = match args.initial_contributor {
|
||||
Some(c) => {
|
||||
// Build the EscrowValue mirroring what's actually paid into
|
||||
// the script output: lovelace + any native assets.
|
||||
let mut value = EscrowValue::ada(args.initial_lovelace);
|
||||
for a in &args.initial_assets {
|
||||
let policy = hex::decode(&a.policy_id_hex)
|
||||
.map_err(|e| DaoError::Config(format!("policy_id_hex parse: {e}")))?;
|
||||
let name = hex::decode(&a.asset_name_hex)
|
||||
.map_err(|e| DaoError::Config(format!("asset_name_hex parse: {e}")))?;
|
||||
value.policies.push((policy, vec![(name, a.quantity as i128)]));
|
||||
}
|
||||
vec![EscrowDeposit { contributor: c, value }]
|
||||
}
|
||||
None => vec![],
|
||||
};
|
||||
//
|
||||
// initial_contributor is now mandatory (see HIGH-2 fix above). Build
|
||||
// the EscrowValue mirroring what's actually paid into the script
|
||||
// output: lovelace + any native assets.
|
||||
let mut value = EscrowValue::ada(args.initial_lovelace);
|
||||
for a in &args.initial_assets {
|
||||
let policy = hex::decode(&a.policy_id_hex)
|
||||
.map_err(|e| DaoError::Config(format!("policy_id_hex parse: {e}")))?;
|
||||
let name = hex::decode(&a.asset_name_hex)
|
||||
.map_err(|e| DaoError::Config(format!("asset_name_hex parse: {e}")))?;
|
||||
value.policies.push((policy, vec![(name, a.quantity as i128)]));
|
||||
}
|
||||
let deposits = vec![EscrowDeposit {
|
||||
contributor: initial_contributor,
|
||||
value,
|
||||
}];
|
||||
let datum = EscrowDatum {
|
||||
party_a: args.party_a_pkh,
|
||||
party_b: args.party_b_pkh,
|
||||
|
|
@ -151,21 +170,13 @@ pub fn build_unsigned_escrow_open(args: EscrowOpenArgs) -> DaoResult<UnsignedEsc
|
|||
)
|
||||
.map_err(|e| DaoError::State(format!("escrow_open payment builder: {e}")))?;
|
||||
|
||||
let summary = match args.initial_contributor {
|
||||
Some(c) => format!(
|
||||
"escrow_open: lock {} lovelace + {} assets at {} (initial contributor {})",
|
||||
args.initial_lovelace,
|
||||
args.initial_assets.len(),
|
||||
args.escrow_script_address,
|
||||
hex::encode(c),
|
||||
),
|
||||
None => format!(
|
||||
"escrow_open: lock {} lovelace + {} assets at {} (no initial contributor)",
|
||||
args.initial_lovelace,
|
||||
args.initial_assets.len(),
|
||||
args.escrow_script_address,
|
||||
),
|
||||
};
|
||||
let summary = format!(
|
||||
"escrow_open: lock {} lovelace + {} assets at {} (initial contributor {})",
|
||||
args.initial_lovelace,
|
||||
args.initial_assets.len(),
|
||||
args.escrow_script_address,
|
||||
hex::encode(initial_contributor),
|
||||
);
|
||||
|
||||
Ok(UnsignedEscrowOpen {
|
||||
tx_cbor_hex: payment.cbor_hex,
|
||||
|
|
@ -184,6 +195,33 @@ mod tests {
|
|||
[seed; PKH_LEN]
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rejects_no_initial_contributor() {
|
||||
// HIGH-2 fix: opening an escrow with `None` initial_contributor
|
||||
// is now refused (formerly produced a deposits=[] escrow that
|
||||
// could be drained on Veto / Refund via vacuous-true
|
||||
// refund_outputs_satisfy).
|
||||
let args = EscrowOpenArgs {
|
||||
network: Network::Preprod,
|
||||
escrow_script_address: "addr_test1wpyt48l...".to_string(),
|
||||
party_a_pkh: pkh(0xa1),
|
||||
party_b_pkh: pkh(0xb2),
|
||||
recipient_pkh: pkh(0xb2),
|
||||
open_deadline_ms: 1_700_000_000_000,
|
||||
lock_period_ms: 30 * 60 * 1000,
|
||||
initial_contributor: None,
|
||||
initial_lovelace: 5_000_000,
|
||||
initial_assets: vec![],
|
||||
change_address: "addr_test1...".to_string(),
|
||||
wallet_utxos: vec![],
|
||||
params: ProtocolParams::default(),
|
||||
};
|
||||
let r = build_unsigned_escrow_open(args);
|
||||
assert!(matches!(r, Err(DaoError::State(_))));
|
||||
let msg = r.unwrap_err().to_string();
|
||||
assert!(msg.contains("initial_contributor is required"), "got: {msg}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rejects_unauthorized_initial_contributor() {
|
||||
let args = EscrowOpenArgs {
|
||||
|
|
|
|||
|
|
@ -258,8 +258,16 @@ pub fn build_unsigned_escrow_refund_timeout(
|
|||
policy.len()
|
||||
)));
|
||||
};
|
||||
// LOW fix (2026-05-09 audit): `qty as u64` silently truncates
|
||||
// negative i128 from CBOR. Use try_from so a corrupt or
|
||||
// adversarial datum can't slip through with a wraparound.
|
||||
let qty_u64 = u64::try_from(qty).map_err(|_| {
|
||||
DaoError::State(format!(
|
||||
"deposit qty {qty} not representable as u64 (negative or > u64::MAX)"
|
||||
))
|
||||
})?;
|
||||
out = out
|
||||
.add_asset(policy_hash, name, qty as u64)
|
||||
.add_asset(policy_hash, name, qty_u64)
|
||||
.map_err(|e| DaoError::Backend(format!("refund add_asset: {e}")))?;
|
||||
}
|
||||
staging = staging.output(out);
|
||||
|
|
|
|||
|
|
@ -298,8 +298,16 @@ pub fn build_unsigned_escrow_veto(args: EscrowVetoArgs) -> DaoResult<UnsignedEsc
|
|||
policy.len()
|
||||
)));
|
||||
};
|
||||
// LOW fix (2026-05-09 audit): `qty as u64` silently truncates
|
||||
// negative i128 from CBOR. Use try_from so a corrupt or
|
||||
// adversarial datum can't slip through with a wraparound.
|
||||
let qty_u64 = u64::try_from(qty).map_err(|_| {
|
||||
DaoError::State(format!(
|
||||
"deposit qty {qty} not representable as u64 (negative or > u64::MAX)"
|
||||
))
|
||||
})?;
|
||||
out = out
|
||||
.add_asset(policy_hash, name, qty as u64)
|
||||
.add_asset(policy_hash, name, qty_u64)
|
||||
.map_err(|e| DaoError::Backend(format!("refund add_asset: {e}")))?;
|
||||
}
|
||||
staging = staging.output(out);
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue