Filed together from round 7. The fixes landed in 124cabe; these are what remains.
1. Cross-host exclusion does not exist (HIGH if the corridor is ever driven from two hosts). FileFundLock is flock — advisory, host-local. 124cabe removed it from eth_swap_two_host.py, so a cross-host resume is now refused rather than silently unprotected. But that is a restriction, not a solution. Two hosts, a shared/NFS mount, a container restart with a different mount, or an operator copying a keys directory all still have no exclusion available, and the code cannot detect any of them.
Fix directions, in preference order:
- Make funding idempotent by construction: pin the push to a deterministic nonce recorded in the swap record, so a re-send is a REPLACEMENT rather than an addition. This removes the need for a distributed lock entirely.
- Or a fenced lease keyed to H, not to a keys path.
Until one exists, treat this corridor as single-host and cross-host resume as unshipped.
2. The in-flight nonce read is single-source (MEDIUM). rpc.get_transaction_count reads pending/latest from one provider — the same single-source class this repo already flags for finality. Where the lock is absent, that read is the ONLY thing standing between a resume and a double-fund, and a lagging or load-balanced node defeats it: host B sees balance=0 and pending==latest while host A's push is in flight, computes the full shortfall, and sends an additive transfer. Both mine, the HTLC holds 2x, and claim sweeps the whole balance to the counterparty. Fix: require the read to agree across the quorum already used for finality before it authorises a push.
3. The record and lock paths are per-RUN, not per-swap (MEDIUM). Both derive from keys_out. Two concurrent swaps sharing a funding key get two independent locks (so the lock is not per-swap, contrary to what its docstring claims) and one shared .swaprec.json (so one swap can clobber the other's pending address). The sink also never refuses an existing file, so rerunning with the same --keys-out overwrites a record whose contract still holds value. Today this is blocked only incidentally by the keys file's O_EXCL, and the runbook tells operators to delete that file after sweep. Fix: key the record and lock by H, or make the first write O_EXCL.
4. Completed swap records are never cleaned up (LOW). After a swap completes, .swaprec.json persists indefinitely at mode 600 holding terms and the funded locator. No runner deletes it.
5. FileFundLock docstring still misdescribes its key (LOW). It says the path is derived from the caller's key path "so two swaps sharing a funding key share a lock — conservative". All three runners derive it from keys_out, which is per-RUN, so two concurrent swaps on one funding key get two independent locks. Harmless in effect, but the documented invariant is false — and a false invariant in a lock is how the next person reasons wrongly about it.
Filed together from round 7. The fixes landed in 124cabe; these are what remains.
1. Cross-host exclusion does not exist (HIGH if the corridor is ever driven from two hosts).
FileFundLockisflock— advisory, host-local. 124cabe removed it frometh_swap_two_host.py, so a cross-host resume is now refused rather than silently unprotected. But that is a restriction, not a solution. Two hosts, a shared/NFS mount, a container restart with a different mount, or an operator copying a keys directory all still have no exclusion available, and the code cannot detect any of them.Fix directions, in preference order:
Until one exists, treat this corridor as single-host and cross-host resume as unshipped.
2. The in-flight nonce read is single-source (MEDIUM).
rpc.get_transaction_countreadspending/latestfrom one provider — the same single-source class this repo already flags for finality. Where the lock is absent, that read is the ONLY thing standing between a resume and a double-fund, and a lagging or load-balanced node defeats it: host B seesbalance=0andpending==latestwhile host A's push is in flight, computes the full shortfall, and sends an additive transfer. Both mine, the HTLC holds 2x, and claim sweeps the whole balance to the counterparty. Fix: require the read to agree across the quorum already used for finality before it authorises a push.3. The record and lock paths are per-RUN, not per-swap (MEDIUM). Both derive from
keys_out. Two concurrent swaps sharing a funding key get two independent locks (so the lock is not per-swap, contrary to what its docstring claims) and one shared.swaprec.json(so one swap can clobber the other's pending address). The sink also never refuses an existing file, so rerunning with the same--keys-outoverwrites a record whose contract still holds value. Today this is blocked only incidentally by the keys file'sO_EXCL, and the runbook tells operators to delete that file after sweep. Fix: key the record and lock byH, or make the first writeO_EXCL.4. Completed swap records are never cleaned up (LOW). After a swap completes,
.swaprec.jsonpersists indefinitely at mode 600 holding terms and the funded locator. No runner deletes it.5.
FileFundLockdocstring still misdescribes its key (LOW). It says the path is derived from the caller's key path "so two swaps sharing a funding key share a lock — conservative". All three runners derive it fromkeys_out, which is per-RUN, so two concurrent swaps on one funding key get two independent locks. Harmless in effect, but the documented invariant is false — and a false invariant in a lock is how the next person reasons wrongly about it.