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 audits/2026-05-09-escrow-internal-audit.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:
parent
ad2d17e3d0
commit
eb192fa676
9 changed files with 378 additions and 65 deletions
10
Dockerfile
10
Dockerfile
|
|
@ -66,6 +66,16 @@ RUN apt-get update && \
|
|||
|
||||
COPY --from=builder /build/target/release/aldabra /usr/local/bin/aldabra
|
||||
|
||||
# Escrow V3 validator CBOR (escrow_wip surface). Baked at /etc/aldabra/
|
||||
# escrow/validator.cbor.hex so MCP escrow_*_unsigned tools can pass it
|
||||
# via `validator_script_path`. The MCP arg-truncation bug at >4500 hex
|
||||
# chars makes the inline `validator_script_cbor_hex` option unusable
|
||||
# for the 7902-char compiled validator. Validator hash:
|
||||
# a8081acef26935d9b5f44b92052178e17301b6d6e6808c91c5b56f5d — check
|
||||
# against the file's compiled hash before invoking on-chain ops.
|
||||
COPY aiken-escrow/validator.cbor.hex /etc/aldabra/escrow/validator.cbor.hex
|
||||
COPY aiken-escrow/plutus.json /etc/aldabra/escrow/plutus.json
|
||||
|
||||
# Default data dir — mount a volume here in compose / k8s / docker run.
|
||||
ENV ALDABRA_DATA=/var/lib/aldabra
|
||||
RUN mkdir -p /var/lib/aldabra && chmod 700 /var/lib/aldabra
|
||||
|
|
|
|||
File diff suppressed because one or more lines are too long
1
aiken-escrow/validator.cbor.hex
Normal file
1
aiken-escrow/validator.cbor.hex
Normal file
File diff suppressed because one or more lines are too long
|
|
@ -23,7 +23,7 @@ use aiken/crypto.{VerificationKeyHash}
|
|||
use aiken/interval.{Finite}
|
||||
use aiken/cbor
|
||||
use cardano/address.{Address, VerificationKey}
|
||||
use cardano/assets.{Value, flatten, merge, negate, quantity_of, zero}
|
||||
use cardano/assets.{Value, add, flatten, merge, negate, quantity_of, zero}
|
||||
use cardano/transaction.{
|
||||
Transaction, Output, OutputReference, InlineDatum, find_input,
|
||||
}
|
||||
|
|
@ -166,6 +166,44 @@ fn value_geq_flat(paid: Value, flat: FlatValue) -> Bool {
|
|||
)
|
||||
}
|
||||
|
||||
/// Sum every entry across all deposits into a single on-chain `Value`.
|
||||
/// Used by Veto / Refund-timeout to enforce the invariant
|
||||
/// `sum(deposits.value) == in_value` — i.e., the escrow's tracked
|
||||
/// deposits must account for every lovelace + token actually locked
|
||||
/// at the script. Without this, an escrow opened with empty deposits
|
||||
/// + non-zero locked value (or one griefed via a token send) lets the
|
||||
/// driver pocket untracked funds via vacuous-true `refund_outputs_satisfy`.
|
||||
/// HIGH-2 fix from 2026-05-09 internal audit.
|
||||
fn deposits_to_value(deposits: List<DepositEntry>) -> Value {
|
||||
list.foldr(
|
||||
deposits,
|
||||
zero,
|
||||
fn(d, acc) {
|
||||
list.foldr(
|
||||
d.value,
|
||||
acc,
|
||||
fn(policy_entry, acc2) {
|
||||
let Pair(policy, assets_) = policy_entry
|
||||
list.foldr(
|
||||
assets_,
|
||||
acc2,
|
||||
fn(asset_entry, acc3) {
|
||||
let Pair(name, qty) = asset_entry
|
||||
add(acc3, policy, name, qty)
|
||||
},
|
||||
)
|
||||
},
|
||||
)
|
||||
},
|
||||
)
|
||||
}
|
||||
|
||||
/// Component-wise equality on opaque `Value`. Cheaper than two-direction
|
||||
/// `value_geq_value` because we can short-circuit on the first mismatch.
|
||||
fn value_eq(a: Value, b: Value) -> Bool {
|
||||
value_geq_value(a, b) && value_geq_value(b, a)
|
||||
}
|
||||
|
||||
/// For each Deposit entry, an output to that contributor's base address
|
||||
/// must pay at least the entry's value.
|
||||
fn refund_outputs_satisfy(
|
||||
|
|
@ -299,6 +337,13 @@ validator escrow {
|
|||
expect signed_by(self, contributor)
|
||||
expect Some((new_d, new_value)) =
|
||||
find_continuing_output(self.outputs, script_addr)
|
||||
// HIGH-1 fix (2026-05-09 audit): without this, `net_added` from
|
||||
// `flatten(merge(new_value, negate(in_value)))` could carry
|
||||
// negative quantities and the depositor could DRAIN tokens
|
||||
// while writing a matching new_d.deposits with reduced values.
|
||||
// Forcing new_value ≥ in_value component-wise ensures every
|
||||
// net_added entry is non-negative.
|
||||
expect value_geq_value(new_value, in_value)
|
||||
// Datum unchanged except `deposits`
|
||||
expect new_d.party_a == d.party_a
|
||||
expect new_d.party_b == d.party_b
|
||||
|
|
@ -334,6 +379,14 @@ validator escrow {
|
|||
Veto -> {
|
||||
expect Agreed { .. } = d.state
|
||||
expect signed_by(self, d.party_a) || signed_by(self, d.party_b)
|
||||
// HIGH-2 fix (2026-05-09 audit): without this, an escrow whose
|
||||
// deposits don't account for every lovelace at the script
|
||||
// (e.g. opened with `initial_contributor=None`, or griefed via
|
||||
// a token-send) lets the driver pocket untracked funds as
|
||||
// change because `refund_outputs_satisfy` is vacuously true on
|
||||
// empty / partial deposits. Enforcing equality forces the
|
||||
// tracked deposits to be the FULL accounting of locked value.
|
||||
expect value_eq(deposits_to_value(d.deposits), in_value)
|
||||
refund_outputs_satisfy(self.outputs, d.deposits)
|
||||
}
|
||||
|
||||
|
|
@ -361,6 +414,10 @@ validator escrow {
|
|||
expect d.state == Open
|
||||
expect Some(lower) = tx_lower_ms(self)
|
||||
expect lower > d.open_deadline_ms
|
||||
// HIGH-2 fix (2026-05-09 audit): same invariant as Veto —
|
||||
// deposits must account for full in_value, else driver
|
||||
// pockets untracked funds via vacuous refund_outputs_satisfy.
|
||||
expect value_eq(deposits_to_value(d.deposits), in_value)
|
||||
refund_outputs_satisfy(self.outputs, d.deposits)
|
||||
}
|
||||
}
|
||||
|
|
|
|||
186
audits/2026-05-09-escrow-internal-audit.md
Normal file
186
audits/2026-05-09-escrow-internal-audit.md
Normal file
|
|
@ -0,0 +1,186 @@
|
|||
# Escrow internal audit — 2026-05-09
|
||||
|
||||
Subagent-driven correctness/security audit of the `escrow_wip` surface
|
||||
on `kayos/escrow-wip-v1` 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**:
|
||||
|
||||
```aiken
|
||||
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**:
|
||||
|
||||
```aiken
|
||||
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):
|
||||
|
||||
```aiken
|
||||
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 under `--features escrow_wip`.
|
||||
- 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.
|
||||
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -3584,11 +3584,8 @@ impl WalletService {
|
|||
lock_period_ms,
|
||||
initial_contributor_pkh_hex,
|
||||
initial_lovelace,
|
||||
fee_lovelace,
|
||||
}: EscrowOpenUnsignedArgs,
|
||||
) -> Result<String, String> {
|
||||
let _ = fee_lovelace; // accepted for forward compat; payment builder is fee-estimating
|
||||
|
||||
let party_a = decode_pkh28(&party_a_pkh_hex, "party_a_pkh_hex")?;
|
||||
let party_b = decode_pkh28(&party_b_pkh_hex, "party_b_pkh_hex")?;
|
||||
let recipient = decode_pkh28(&recipient_pkh_hex, "recipient_pkh_hex")?;
|
||||
|
|
@ -3906,13 +3903,17 @@ impl WalletService {
|
|||
|
||||
let cfg = self.dao_cfg_for_escrow()?;
|
||||
let wallet_utxos = pull_wallet_utxos(&self.inner.chain, &self.inner.address).await?;
|
||||
let (tip_slot, tip_ms) = fetch_tip_slot_ms(self).await?;
|
||||
let (tip_slot, _tip_ms) = fetch_tip_slot_ms(self).await?;
|
||||
|
||||
// lower bound: caller-driven from tip; we sanity-check it satisfies
|
||||
// the lock-window elapsed gate. Validator extracts `lower` from
|
||||
// valid_from_slot anyway, so we anchor lower at tip_slot.
|
||||
// MED-2/3 fix (2026-05-09 audit): derive `validity_lower_ms`
|
||||
// from the slot via the Shelley constants, NOT from Koios's
|
||||
// `block_time*1000`. The chain reconstructs `lower` from
|
||||
// `valid_from_slot(slot)` via slot↔ms math; using Koios's
|
||||
// block_time can drift up to ~1s, which at the strict-`>`
|
||||
// validator boundary makes the off-chain preflight pass while
|
||||
// the chain rejects → fees + collateral burned.
|
||||
let validity_lower_slot = tip_slot;
|
||||
let validity_lower_ms = tip_ms;
|
||||
let validity_lower_ms = slot_to_posix_ms(cfg.network, validity_lower_slot)?;
|
||||
|
||||
let validity_upper_slot = tip_slot + validity_window_seconds.unwrap_or(1800);
|
||||
|
||||
|
|
@ -3977,9 +3978,10 @@ impl WalletService {
|
|||
|
||||
let cfg = self.dao_cfg_for_escrow()?;
|
||||
let wallet_utxos = pull_wallet_utxos(&self.inner.chain, &self.inner.address).await?;
|
||||
let (tip_slot, tip_ms) = fetch_tip_slot_ms(self).await?;
|
||||
let (tip_slot, _tip_ms) = fetch_tip_slot_ms(self).await?;
|
||||
// MED-2/3 fix: same slot-derived ms as Settle (see escrow_settle_unsigned).
|
||||
let validity_lower_slot = tip_slot;
|
||||
let validity_lower_ms = tip_ms;
|
||||
let validity_lower_ms = slot_to_posix_ms(cfg.network, validity_lower_slot)?;
|
||||
let validity_upper_slot = tip_slot + validity_window_seconds.unwrap_or(1800);
|
||||
|
||||
let unsigned = build_unsigned_escrow_refund_timeout(EscrowRefundTimeoutArgs {
|
||||
|
|
@ -4286,13 +4288,16 @@ pub struct EscrowOpenUnsignedArgs {
|
|||
/// Veto-window length after Agree, in ms.
|
||||
pub lock_period_ms: i64,
|
||||
/// Optional pkh of the initial contributor (must equal party_a or party_b).
|
||||
/// If unset, opens with empty deposits — both parties top up later.
|
||||
/// **HIGH-2 fix 2026-05-09:** The builder now REJECTS `None` here — every
|
||||
/// escrow must have at least one initial contributor at open time.
|
||||
/// Sending `null` returns an error. Field stays `Option<String>` for
|
||||
/// schema-stability.
|
||||
#[serde(default)]
|
||||
pub initial_contributor_pkh_hex: Option<String>,
|
||||
/// Lovelace to lock at the escrow output. Must clear min-utxo floor (~1 ADA).
|
||||
/// Lovelace to lock at the escrow output. **MED-5 fix:** must clear
|
||||
/// 2_000_000 (≥2 ADA) — Conway-era inline-datum + script-address
|
||||
/// outputs need ~1.4-1.7 ADA min-utxo, with headroom.
|
||||
pub initial_lovelace: u64,
|
||||
/// Estimated total fee in lovelace. ~2_500_000 reasonable for v1.
|
||||
pub fee_lovelace: u64,
|
||||
}
|
||||
|
||||
// ─── escrow_wip spend-tool args (deposit / agree / veto / settle / refund) ──
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue