Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 22 additions & 7 deletions src/pyrxd/btc_wallet/htlc_leg.py
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,17 @@ class BtcFundingReader(Protocol):
enforcing ``min_confirmations`` (raise/fail-closed if shallower).
``confirmations`` is the symmetric confirmation-depth reader (mirrors
``RadiantChainIO.confirmations``) the reorg gate consumes. ``txid_of`` resolves a
raw tx's canonical txid VIA THE NODE — never a local segwit parse (see the reorg
gate plan; the gated txid must be that of the exact bytes ``p`` was scraped from).
raw tx's canonical txid LOCALLY, by re-serialising the non-witness form
(``taproot.btc_txid_from_raw``) — every shipped implementation does, including the
Bitcoin Core one, because on mainnet there is no node to ``decoderawtransaction``.

Until 2026-09-03 this said "VIA THE NODE — never a local segwit parse", which had
the mechanism exactly backwards. The safety property is unchanged, and is if
anything better served by the local derivation: the gated txid must be that of the
EXACT bytes ``p`` was scraped from, never a counterparty-supplied id. Serialising
those bytes yields that txid without asking anyone; a node round-trip would be one
more party to trust. ``btc_txid_from_raw`` is fail-closed on any structural problem,
and a mis-derived txid reads 0 confs at the gate — never a false depth.
"""

async def read_output_amount_sats(self, txid: str, vout: int, *, min_confirmations: int) -> int:
Expand All @@ -156,7 +165,7 @@ async def confirmations(self, txid: str) -> int:
...

async def txid_of(self, raw_tx: bytes) -> str:
"""Resolve ``raw_tx``'s canonical txid via the node (NOT a local parse)."""
"""Derive ``raw_tx``'s canonical txid from the bytes themselves (no node round-trip)."""
...


Expand Down Expand Up @@ -507,10 +516,16 @@ async def verify_counterparty_funded(
async def confirmations_of_claim(self, claim_tx_bytes: bytes) -> int:
"""Confirmation depth of the maker's BTC claim tx (the reorg gate's input).

The txid is resolved VIA THE NODE from the exact ``claim_tx_bytes`` ``p`` was
scraped from (never a local segwit parse) — so an attacker can't reveal ``p``
in a shallow tx while pointing the gate at a deep unrelated tx. Fail-closed:
any read/derivation error propagates (the coordinator then refuses to claim).
The txid is derived LOCALLY, by re-serialising the exact ``claim_tx_bytes`` ``p``
was scraped from (``taproot.btc_txid_from_raw``) — so an attacker can't reveal
``p`` in a shallow tx while pointing the gate at a deep unrelated tx. Only the
DEPTH of that txid is then read from the reader. Fail-closed: any read/derivation
error propagates (the coordinator then refuses to claim).

Until 2026-09-03 this said the txid was "resolved VIA THE NODE ... never a local
segwit parse", contradicting the comment in its own body and every shipped
``BtcFundingReader.txid_of``. Serialising the bytes in hand is what makes the
property hold: no third party gets to name the tx whose depth the gate trusts.
"""
if not isinstance(claim_tx_bytes, (bytes, bytearray)) or len(claim_tx_bytes) == 0:
raise ValidationError("claim_tx_bytes must be non-empty bytes")
Expand Down
15 changes: 13 additions & 2 deletions src/pyrxd/btc_wallet/taproot.py
Original file line number Diff line number Diff line change
Expand Up @@ -351,8 +351,19 @@ def refund_leaf_script(refund_pubkey_xonly: bytes, timeout: Timelock) -> bytes:
#
# It became reachable when #482 made `t_btc` a SUBTRACTION (`t_rxd - margin - 4`); the old
# addition could not underflow. The first fix put a `< 1` refusal in three RUNNER scripts —
# fix-at-the-demonstrated-site, which is the shape this codebase keeps repeating. Any caller
# that builds a leaf, now or later, gets the floor by construction.
# fix-at-the-demonstrated-site, which is the shape this codebase keeps repeating. Moving it
# here gives every caller that builds a BLOCKS-tagged leaf the floor by construction.
#
# SCOPE, HONESTLY: the refusal below is BLOCKS-ONLY, so "every caller gets the floor" is NOT
# true of a SECONDS-tagged `Timelock` — which this comment asserted until 2026-09-03. BIP68
# time locks are quantised to 512 s, so `Timelock(0..511, SECONDS)` all encode to
# `nSequence = 0x00400000` (zero time units): the SAME no-op relative lock, emitted without a
# word of complaint. Nothing in `pyrxd` or `scripts/` CONSTRUCTS a SECONDS `t_btc` today, but
# `NegotiatedTerms.from_dict` takes the unit tag straight off the wire
# (`swap_state._timelock_from_dict`) and `swap_state`'s own `t_btc` floor is scoped to BLOCKS
# in exactly the same way, so a counterparty-authored envelope can carry one end to end. Left
# as-is deliberately: widening the refusal is a behaviour change on fund-moving code and needs
# its own review (what the SECONDS floor should be, and which honest terms it would refuse).
#
# `build_htlc_covenant_*` has enforced the same floor on the Radiant side all along
# (`htlc_covenant.py`, "a 0 CSV is a no-op timelock"). This is the BTC-side twin it was missing.
Expand Down
13 changes: 10 additions & 3 deletions src/pyrxd/eth_wallet/erc20_leg.py
Original file line number Diff line number Diff line change
Expand Up @@ -565,9 +565,16 @@ async def claim(self, locator: EthHtlcLocator, preimage: bytes) -> str:
#468. Putting it in front of the dangerous action makes it unskippable rather than
merely available.

The contract address is checked alongside both parties, because that is the freeze with
no way out: measured on a mainnet fork, freezing the HTLC makes ``claim`` AND ``refund``
revert permanently, so no timeout rescues it.
The addresses checked here are the HTLC CONTRACT and the CLAIMANT — not the refundee. The
contract is the freeze with no way out: measured on a mainnet fork, freezing the HTLC makes
``claim`` AND ``refund`` revert permanently, so no timeout rescues it. The refundee is
deliberately absent, because a ``claim`` sweeps to the claimant and never touches it; see
the reasoning at the call site below, and :func:`assert_not_frozen_before_funding`, the
pre-FUND gate that is where a frozen refundee actually matters.

Until 2026-09-03 this read "checked alongside both parties", describing round 4's list
rather than round 5's — which removed the refundee precisely because refusing on it was a
guard refusing valid work and handed the counterparty a free unilateral veto.

The claim itself is the parent's, unchanged — this adds a precondition, not a different
settlement path.
Expand Down
8 changes: 7 additions & 1 deletion src/pyrxd/eth_wallet/htlc_leg.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,14 @@
Security gates enforced here (off-chain, per the security review):
* pre-fund: ``eth_getCode`` runtime-bytecode == the committed artifact's, the
contract immutables (hashlock/claimant/refundee/timeout) == negotiated, and the
funded balance == negotiated amount. Run inside the funder's own ``fund()``, and again
funded balance >= the negotiated amount. Run inside the funder's own ``fund()``, and again
on the maker's side before the maker reveals p (the maker's RXD lock precedes both).
The balance is a LOWER BOUND on purpose — anyone can force-send wei to a contract
(selfdestruct/coinbase), so an ``== expected`` check is griefable into a permanent
verify failure; over-funding is safe because claim/refund sweep the whole balance to
the winner. This line said "== negotiated amount" until 2026-09-03; the check has been
``bal < expected_amount_wei`` since the red-team LOW that introduced it, and
:meth:`EthHtlcContractLeg.verify_funded` documents it correctly at the check itself.
* EOA-only claimant/refundee (a recipient contract that reverts on receive would lock
funds via the contract's ``require(ok)``).
"""
Expand Down
19 changes: 15 additions & 4 deletions src/pyrxd/eth_wallet/locator.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,9 +120,16 @@ class EthHtlcLocator:
Attributes
----------
chain_id:
EIP-155 chain id (Sepolia = 11155111, mainnet = 1). Pins which network this
locator belongs to; a claim/refund built for the wrong chain is rejected by the
node via EIP-155 signing, and the leg refuses a chain_id mismatch up front.
EIP-155 chain id (Sepolia = 11155111, mainnet = 1). RECORDS which network this
locator was created on, so a durable record read back later says which chain its
``contract_address`` lives on. It is a note, not a gate: as of 2026-09-03 NO code
compares it to anything. ``EthRpc.assert_chain`` checks the NODE against the leg's
own ``expected_chain_id``, and ``_sign_and_send`` signs with the LEG's ``chain_id``
— so a leg pointed at the wrong network is caught, but a locator from a DIFFERENT
network driven by a correctly-configured leg is not. This entry claimed "the leg
refuses a chain_id mismatch up front" until 2026-09-03; it never did. Adding the
comparison is a behaviour change on fund-moving code and is deliberately left for
its own review rather than smuggled in with a docstring correction.
contract_address:
The deployed ``EthHtlc`` instance (deploy-per-swap).
deploy_tx_hash:
Expand All @@ -137,7 +144,11 @@ class EthHtlcLocator:
timeout:
Absolute unix deadline (matches the contract immutable).
amount_wei:
The funded value (verified == negotiated before the maker reveals p).
The NEGOTIATED value. Before the maker reveals p, ``verify_funded`` asserts the
contract's on-chain balance is ``>=`` this — a LOWER bound, because anyone can
force-send wei to a contract and an ``==`` check would be griefable into a
permanent verify failure. This said "verified == negotiated" until 2026-09-03; a
reader who took that literally would have believed an over-funded HTLC is refused.
"""

#: The wire tag this locator serialises under inside ``counterchain_locator``.
Expand Down
12 changes: 11 additions & 1 deletion src/pyrxd/gravity/swap_state.py
Original file line number Diff line number Diff line change
Expand Up @@ -429,7 +429,17 @@ def __post_init__(self) -> None:
# `p` is public. Reachable on the dust defaults (t_rxd=20) at a measured margin of 16.
#
# Refused here rather than clamped at each builder: clamping silently hands back a swap
# nobody asked for, and this is the layer that makes it unrepresentable for every caller.
# nobody asked for.
#
# SCOPE, HONESTLY: this refusal is BLOCKS-ONLY. Until 2026-09-03 the sentence above ended
# "and this is the layer that makes it unrepresentable for every caller", which is not what
# the condition says. `t_rxd` IS pinned to BLOCKS a few lines up; `t_btc` is not, and
# `from_dict` takes the unit tag off the wire, so a SECONDS-tagged `t_btc` skips this floor,
# skips the same-unit ordering guard below (the units differ), normalises to 0 blocks in
# `assert_timelock_margin`, and reaches `refund_leaf_script`, whose own floor is scoped the
# same way. BIP68 quantises time locks to 512 s, so `Timelock(0..511, SECONDS)` is the same
# no-op lock as `Timelock(0, BLOCKS)`. Not fixed here: pinning `t_btc` to BLOCKS (or giving
# SECONDS a floor) is a behaviour change on fund-moving code and needs its own review.
if self.t_btc.unit is TimeUnit.BLOCKS and self.t_btc.value < 1:
raise ValidationError(
f"t_btc is {self.t_btc.value} blocks — a counter leg that matures in its own funding "
Expand Down
Loading