diff --git a/conformance/htlc-handshake-vectors.json b/conformance/htlc-handshake-vectors.json index 1ff7ef3d..55eb7003 100644 --- a/conformance/htlc-handshake-vectors.json +++ b/conformance/htlc-handshake-vectors.json @@ -1,11 +1,11 @@ { "schema": "radiant-htlc-handshake/1", "builder": "pyrxd.gravity.swap_state: NegotiatedTerms.to_dict / from_dict; pyrxd.gravity.htlc_covenant: build_htlc_covenant_{rxd,ft,nft} / holder_hash; pyrxd.btc_wallet.taproot: build_htlc / claim_leaf_script / refund_leaf_script; pyrxd.gravity.swap_coordinator: assert_timelock_margin", - "provenance": "REFERENCE vectors produced by pyrxd from its own builders \u2014 pyrxd is the reference PRODUCER here, so this suite is a regression lock against silent drift, not an independent cross-check. NO mainnet-anchored vector (tracked follow-up). The swap stack these vectors describe is UNAUDITED: passing this suite means a second implementation agrees with pyrxd's bytes, NOT that the protocol is safe. The 32-byte preimage below is a fixed ASCII string, deliberately not CSPRNG output, so that no conformance file ever carries a value that could open a funded HTLC. Guarded by tests/test_htlc_handshake_conformance_vectors.py.", + "provenance": "REFERENCE vectors produced by pyrxd from its own builders — pyrxd is the reference PRODUCER here, so this suite is a regression lock against silent drift, not an independent cross-check. NO mainnet-anchored vector (tracked follow-up). The swap stack these vectors describe is UNAUDITED: passing this suite means a second implementation agrees with pyrxd's bytes, NOT that the protocol is safe. The 32-byte preimage below is a fixed ASCII string, deliberately not CSPRNG output, so that no conformance file ever carries a value that could open a funded HTLC. Guarded by tests/test_htlc_handshake_conformance_vectors.py.", "notes": { "hashlock": "H = SHA256(p), SINGLE sha256, exactly 32 bytes. p itself is exactly 32 bytes and NEVER crosses the wire.", "dest_hashes": "taker_dest_hash / maker_dest_hash are hash256 (double-SHA256) of the holder script, NOT of a pkh.", - "timelock_ordering": "t_btc MUST exceed t_rxd by at least the verifier's own margin. The taker locks first and holds the LONGER refund.", + "timelock_ordering": "t_rxd MUST exceed t_btc by at least the verifier's own margin. The MAKER holds the preimage p, LOCKS the Radiant leg, and CLAIMS the BTC leg — so the leg it locks carries the LONGER refund (Herlihy, Atomic Cross-Chain Swaps, arXiv:1801.09515 §1). THESE VECTORS PREVIOUSLY PUBLISHED THE OPPOSITE and are corrected here: they accepted t_btc=60/t_rxd=20 and REJECTED t_btc=20/t_rxd=60, so an implementation that agreed with them was forced into the layout where the party holding p can refund the leg it locked and still claim the other. See issue #482.", "t_rxd_unit": "t_rxd MUST be BLOCKS. The Radiant covenant CSV has no SECONDS encoding.", "omitted_defaults": "counter_chain/value_amount/eth_timeout_unix_s/credential_ref are emitted only when they differ from the BTC defaults; a reader MUST apply the defaults for absent keys.", "credential_ref_is_off_chain": "credential_ref is NOT substituted into the covenant bytecode. btc-rxd-credential-gated and btc-rxd differ only by credential_ref and their covenant_scriptpubkey_hex is byte-IDENTICAL -- that equality is the EXPECTED result, asserted by test_credential_gating_does_not_change_the_covenant_spk, not an accident of the comparison. The gate is off-chain pre-fund policy (swap_coordinator.pre_btc_lock_gate); the covenant pays whoever produces the pinned holder script with p, credentialed or not. Do NOT treat the section-4 SPK byte-compare as binding this gate.", @@ -45,11 +45,11 @@ "id": "margin-ok-gap-40-margin-36", "source": "reference", "t_btc": { - "value": 60, + "value": 20, "unit": "blocks" }, "t_rxd": { - "value": 20, + "value": 60, "unit": "blocks" }, "margin": { @@ -63,11 +63,11 @@ "id": "margin-too-tight-gap-10-margin-36", "source": "reference", "t_btc": { - "value": 60, + "value": 50, "unit": "blocks" }, "t_rxd": { - "value": 50, + "value": 60, "unit": "blocks" }, "margin": { @@ -82,11 +82,11 @@ "id": "margin-inverted-ordering", "source": "reference", "t_btc": { - "value": 20, + "value": 60, "unit": "blocks" }, "t_rxd": { - "value": 60, + "value": 20, "unit": "blocks" }, "margin": { @@ -95,13 +95,13 @@ }, "block_interval_s": 600.0, "verdict": "reject", - "why": "t_btc must EXCEED t_rxd. Inverting the order lets the first-locking party's refund open first \u2014 the maker can then claim the counter leg AND refund the asset." + "why": "t_btc must EXCEED t_rxd. Inverting the order lets the first-locking party's refund open first — the maker can then claim the counter leg AND refund the asset." }, { "id": "margin-cross-unit-inversion", "source": "reference", "t_btc": { - "value": 600, + "value": 43200, "unit": "seconds" }, "t_rxd": { @@ -114,7 +114,7 @@ }, "block_interval_s": 600.0, "verdict": "reject", - "why": "600 s normalises to 1 block at a 600 s interval, which is BELOW t_rxd. NegotiatedTerms' cheap same-unit construction guard does NOT fire here \u2014 only the normalising margin check catches it. A conforming implementation MUST run the normalising check." + "why": "600 s normalises to 1 block at a 600 s interval, which is BELOW t_rxd. NegotiatedTerms' cheap same-unit construction guard does NOT fire here — only the normalising margin check catches it. A conforming implementation MUST run the normalising check." } ], "terms_vectors": [ @@ -126,20 +126,20 @@ "maker_pkh_hex": "2222222222222222222222222222222222222222", "btc_network": "bc", "btc_claim_leaf_script_hex": "82012088a820c8cfe67f2d85148469f2998b7259d7cf356463c18a649c693e75f775a5259ddb88203333333333333333333333333333333333333333333333333333333333333333ac", - "btc_refund_leaf_script_hex": "013cb275204444444444444444444444444444444444444444444444444444444444444444ac", - "btc_funding_scriptpubkey_hex": "51209d6253be8748ce4770fb04ae258140717533f82dbfdbe522217542251db3d8fd", - "btc_funding_address": "bc1pn4398058fr8ywu8mqjhztq2qw96n87pdhld72g3pw4pz28dnmr7s3djm6k" + "btc_refund_leaf_script_hex": "0114b275204444444444444444444444444444444444444444444444444444444444444444ac", + "btc_funding_scriptpubkey_hex": "5120f8e17f29b17d17723dbea00110e580e99edfb3653ad4863e075e02c34b8f292e", + "btc_funding_address": "bc1plrsh72d305thy0d75qq3pevqax0dlvm98t2gv0s8tcpvxju09yhqhhr636" }, "terms": { "hashlock": "c8cfe67f2d85148469f2998b7259d7cf356463c18a649c693e75f775a5259ddb", "btc_sats": 100000, "radiant_amount": 100000, "t_btc": { - "value": 60, + "value": 20, "unit": "blocks" }, "t_rxd": { - "value": 20, + "value": 60, "unit": "blocks" }, "asset_variant": "rxd", @@ -149,7 +149,7 @@ "btc_claim_pubkey_xonly": "3333333333333333333333333333333333333333333333333333333333333333", "btc_refund_pubkey_xonly": "4444444444444444444444444444444444444444444444444444444444444444" }, - "covenant_scriptpubkey_hex": "c4519d76009c637c82012088a820c8cfe67f2d85148469f2998b7259d7cf356463c18a649c693e75f775a5259ddb8800cc03a08601a26900cdaa20f556d6f9c7bfc196c1ad35f89669e7a8059f95cc6cb5e57e54edbd87b38ace37877767519d0114b27500cc03a08601a26900cdaa20ba62286130c024d125dc9ad9e1672f5ed7619b5cef52b66a34b4517e5b64b1628768" + "covenant_scriptpubkey_hex": "c4519d76009c637c82012088a820c8cfe67f2d85148469f2998b7259d7cf356463c18a649c693e75f775a5259ddb8800cc03a08601a26900cdaa20f556d6f9c7bfc196c1ad35f89669e7a8059f95cc6cb5e57e54edbd87b38ace37877767519d013cb27500cc03a08601a26900cdaa20ba62286130c024d125dc9ad9e1672f5ed7619b5cef52b66a34b4517e5b64b1628768" }, { "id": "btc-ft", @@ -159,20 +159,20 @@ "maker_pkh_hex": "2222222222222222222222222222222222222222", "btc_network": "bc", "btc_claim_leaf_script_hex": "82012088a820c8cfe67f2d85148469f2998b7259d7cf356463c18a649c693e75f775a5259ddb88203333333333333333333333333333333333333333333333333333333333333333ac", - "btc_refund_leaf_script_hex": "013cb275204444444444444444444444444444444444444444444444444444444444444444ac", - "btc_funding_scriptpubkey_hex": "51209d6253be8748ce4770fb04ae258140717533f82dbfdbe522217542251db3d8fd", - "btc_funding_address": "bc1pn4398058fr8ywu8mqjhztq2qw96n87pdhld72g3pw4pz28dnmr7s3djm6k" + "btc_refund_leaf_script_hex": "0114b275204444444444444444444444444444444444444444444444444444444444444444ac", + "btc_funding_scriptpubkey_hex": "5120f8e17f29b17d17723dbea00110e580e99edfb3653ad4863e075e02c34b8f292e", + "btc_funding_address": "bc1plrsh72d305thy0d75qq3pevqax0dlvm98t2gv0s8tcpvxju09yhqhhr636" }, "terms": { "hashlock": "c8cfe67f2d85148469f2998b7259d7cf356463c18a649c693e75f775a5259ddb", "btc_sats": 100000, "radiant_amount": 5000, "t_btc": { - "value": 60, + "value": 20, "unit": "blocks" }, "t_rxd": { - "value": 20, + "value": 60, "unit": "blocks" }, "asset_variant": "ft", @@ -182,7 +182,7 @@ "btc_claim_pubkey_xonly": "3333333333333333333333333333333333333333333333333333333333333333", "btc_refund_pubkey_xonly": "4444444444444444444444444444444444444444444444444444444444444444" }, - "covenant_scriptpubkey_hex": "d001e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e001000000c4519d76de519ddc0288139d76009c637c82012088a820c8cfe67f2d85148469f2998b7259d7cf356463c18a649c693e75f775a5259ddb8800cdaa20a831b57d2fb14c06dd8d58b41b5c7daace489126392da8c25ac9d51b26caf227877767519d0114b27500cdaa206d51abe99d4ed3381a56f43f4da2cb2c383f4801a4bc7ad0ed5479f3046cc25c8768bdd001e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e001000000dec0e9aa76e378e4a269e69d" + "covenant_scriptpubkey_hex": "d001e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e001000000c4519d76de519ddc0288139d76009c637c82012088a820c8cfe67f2d85148469f2998b7259d7cf356463c18a649c693e75f775a5259ddb8800cdaa20a831b57d2fb14c06dd8d58b41b5c7daace489126392da8c25ac9d51b26caf227877767519d013cb27500cdaa206d51abe99d4ed3381a56f43f4da2cb2c383f4801a4bc7ad0ed5479f3046cc25c8768bdd001e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e001000000dec0e9aa76e378e4a269e69d" }, { "id": "btc-nft", @@ -192,20 +192,20 @@ "maker_pkh_hex": "2222222222222222222222222222222222222222", "btc_network": "bc", "btc_claim_leaf_script_hex": "82012088a820c8cfe67f2d85148469f2998b7259d7cf356463c18a649c693e75f775a5259ddb88203333333333333333333333333333333333333333333333333333333333333333ac", - "btc_refund_leaf_script_hex": "013cb275204444444444444444444444444444444444444444444444444444444444444444ac", - "btc_funding_scriptpubkey_hex": "51209d6253be8748ce4770fb04ae258140717533f82dbfdbe522217542251db3d8fd", - "btc_funding_address": "bc1pn4398058fr8ywu8mqjhztq2qw96n87pdhld72g3pw4pz28dnmr7s3djm6k" + "btc_refund_leaf_script_hex": "0114b275204444444444444444444444444444444444444444444444444444444444444444ac", + "btc_funding_scriptpubkey_hex": "5120f8e17f29b17d17723dbea00110e580e99edfb3653ad4863e075e02c34b8f292e", + "btc_funding_address": "bc1plrsh72d305thy0d75qq3pevqax0dlvm98t2gv0s8tcpvxju09yhqhhr636" }, "terms": { "hashlock": "c8cfe67f2d85148469f2998b7259d7cf356463c18a649c693e75f775a5259ddb", "btc_sats": 100000, "radiant_amount": 546, "t_btc": { - "value": 60, + "value": 20, "unit": "blocks" }, "t_rxd": { - "value": 20, + "value": 60, "unit": "blocks" }, "asset_variant": "nft", @@ -215,7 +215,7 @@ "btc_claim_pubkey_xonly": "3333333333333333333333333333333333333333333333333333333333333333", "btc_refund_pubkey_xonly": "4444444444444444444444444444444444444444444444444444444444444444" }, - "covenant_scriptpubkey_hex": "d801e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e001000000c4519dde519d00cc0222029d76009c637c82012088a820c8cfe67f2d85148469f2998b7259d7cf356463c18a649c693e75f775a5259ddb8800cdaa20ab4f06cc51c6e3d31ee445240882c70aa3c9c3dd4bc6222b6c994340d8833859877767519d0114b27500cdaa204f9e31140f311d888dd2acab3df2915f0e76f978d90b2a965999493f1577de608768" + "covenant_scriptpubkey_hex": "d801e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e001000000c4519dde519d00cc0222029d76009c637c82012088a820c8cfe67f2d85148469f2998b7259d7cf356463c18a649c693e75f775a5259ddb8800cdaa20ab4f06cc51c6e3d31ee445240882c70aa3c9c3dd4bc6222b6c994340d8833859877767519d013cb27500cdaa204f9e31140f311d888dd2acab3df2915f0e76f978d90b2a965999493f1577de608768" }, { "id": "eth-rxd", @@ -229,11 +229,11 @@ "btc_sats": 100000, "radiant_amount": 100000, "t_btc": { - "value": 60, + "value": 20, "unit": "blocks" }, "t_rxd": { - "value": 20, + "value": 60, "unit": "blocks" }, "asset_variant": "rxd", @@ -246,7 +246,7 @@ "value_amount": 1000000000000000, "eth_timeout_unix_s": 1800000000 }, - "covenant_scriptpubkey_hex": "c4519d76009c637c82012088a820c8cfe67f2d85148469f2998b7259d7cf356463c18a649c693e75f775a5259ddb8800cc03a08601a26900cdaa20f556d6f9c7bfc196c1ad35f89669e7a8059f95cc6cb5e57e54edbd87b38ace37877767519d0114b27500cc03a08601a26900cdaa20ba62286130c024d125dc9ad9e1672f5ed7619b5cef52b66a34b4517e5b64b1628768" + "covenant_scriptpubkey_hex": "c4519d76009c637c82012088a820c8cfe67f2d85148469f2998b7259d7cf356463c18a649c693e75f775a5259ddb8800cc03a08601a26900cdaa20f556d6f9c7bfc196c1ad35f89669e7a8059f95cc6cb5e57e54edbd87b38ace37877767519d013cb27500cc03a08601a26900cdaa20ba62286130c024d125dc9ad9e1672f5ed7619b5cef52b66a34b4517e5b64b1628768" }, { "id": "btc-rxd-credential-gated", @@ -256,20 +256,20 @@ "maker_pkh_hex": "2222222222222222222222222222222222222222", "btc_network": "bc", "btc_claim_leaf_script_hex": "82012088a820c8cfe67f2d85148469f2998b7259d7cf356463c18a649c693e75f775a5259ddb88203333333333333333333333333333333333333333333333333333333333333333ac", - "btc_refund_leaf_script_hex": "013cb275204444444444444444444444444444444444444444444444444444444444444444ac", - "btc_funding_scriptpubkey_hex": "51209d6253be8748ce4770fb04ae258140717533f82dbfdbe522217542251db3d8fd", - "btc_funding_address": "bc1pn4398058fr8ywu8mqjhztq2qw96n87pdhld72g3pw4pz28dnmr7s3djm6k" + "btc_refund_leaf_script_hex": "0114b275204444444444444444444444444444444444444444444444444444444444444444ac", + "btc_funding_scriptpubkey_hex": "5120f8e17f29b17d17723dbea00110e580e99edfb3653ad4863e075e02c34b8f292e", + "btc_funding_address": "bc1plrsh72d305thy0d75qq3pevqax0dlvm98t2gv0s8tcpvxju09yhqhhr636" }, "terms": { "hashlock": "c8cfe67f2d85148469f2998b7259d7cf356463c18a649c693e75f775a5259ddb", "btc_sats": 100000, "radiant_amount": 100000, "t_btc": { - "value": 60, + "value": 20, "unit": "blocks" }, "t_rxd": { - "value": 20, + "value": 60, "unit": "blocks" }, "asset_variant": "rxd", @@ -280,7 +280,7 @@ "btc_refund_pubkey_xonly": "4444444444444444444444444444444444444444444444444444444444444444", "credential_ref": "01e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e0e001000000" }, - "covenant_scriptpubkey_hex": "c4519d76009c637c82012088a820c8cfe67f2d85148469f2998b7259d7cf356463c18a649c693e75f775a5259ddb8800cc03a08601a26900cdaa20f556d6f9c7bfc196c1ad35f89669e7a8059f95cc6cb5e57e54edbd87b38ace37877767519d0114b27500cc03a08601a26900cdaa20ba62286130c024d125dc9ad9e1672f5ed7619b5cef52b66a34b4517e5b64b1628768" + "covenant_scriptpubkey_hex": "c4519d76009c637c82012088a820c8cfe67f2d85148469f2998b7259d7cf356463c18a649c693e75f775a5259ddb8800cc03a08601a26900cdaa20f556d6f9c7bfc196c1ad35f89669e7a8059f95cc6cb5e57e54edbd87b38ace37877767519d013cb27500cc03a08601a26900cdaa20ba62286130c024d125dc9ad9e1672f5ed7619b5cef52b66a34b4517e5b64b1628768" } ] } diff --git a/scripts/_dust_swap_shared.py b/scripts/_dust_swap_shared.py index a49ec4ac..2370aeec 100644 --- a/scripts/_dust_swap_shared.py +++ b/scripts/_dust_swap_shared.py @@ -513,9 +513,7 @@ def covenant_fund_height(height: ChainHeight) -> ChainHeight: return height -async def scan_covenant_fund_height( - client: Any, *, covenant_spk: bytes, expected_photons: int -) -> ChainHeight: +async def scan_covenant_fund_height(client: Any, *, covenant_spk: bytes, expected_photons: int) -> ChainHeight: """The anchor for paths that locked the asset WITHOUT :func:`wait_for_covenant_funding` — the NFT and FT variants, which lock by SPENDING into the covenant rather than by waiting on an operator payment. Same conversion, same fail-closed rules, one scan. diff --git a/scripts/btc_swap_two_host.py b/scripts/btc_swap_two_host.py index 768cd51b..869a9fe5 100644 --- a/scripts/btc_swap_two_host.py +++ b/scripts/btc_swap_two_host.py @@ -851,8 +851,9 @@ def run_self_check() -> None: terms, cov = _terms_from_public( hashlock=h, btc_sats=100_000, - t_rxd_blocks=20, - t_btc_blocks=60, # t_btc - t_rxd = 40 >= margin 36 + # INVERTED (#482): the maker holds p and LOCKS the Radiant leg, so t_rxd is the LONGER. + t_rxd_blocks=60, + t_btc_blocks=20, # t_rxd - t_btc = 40 >= margin 36 taker_pkh=taker_pkh, maker_pkh=maker_pkh, btc_claim_xonly=claim_xonly, @@ -909,12 +910,14 @@ def run_self_check() -> None: assert_timelock_margin(terms2.t_btc, terms2.t_rxd, policy) print(" [ok] taker's INDEPENDENT timelock-margin check passes for honest terms") - # ...and REFUSES a hostile too-tight envelope (t_btc - t_rxd < margin). + # ...and REFUSES a hostile too-tight envelope (t_rxd - t_btc < margin). + # The pair inverted with #482: t_rxd=50/t_btc=60 no longer constructs at all, so this check + # would have passed on the ordering guard's exception rather than the margin check's. hostile, _ = _terms_from_public( hashlock=h, btc_sats=100_000, - t_rxd_blocks=50, - t_btc_blocks=60, # gap 10 < margin 36 + t_rxd_blocks=60, + t_btc_blocks=50, # gap 10 < margin 36 taker_pkh=taker_pkh, maker_pkh=maker_pkh, btc_claim_xonly=claim_xonly, @@ -924,7 +927,7 @@ def run_self_check() -> None: assert_timelock_margin(hostile.t_btc, hostile.t_rxd, policy) raise AssertionError("FAIL: the margin check did not reject a too-tight hostile envelope") except ValidationError: - print(" [ok] taker REFUSES a hostile too-tight envelope (t_btc - t_rxd < margin)") + print(" [ok] taker REFUSES a hostile too-tight envelope (t_rxd - t_btc < margin)") # --- MAKER side: the expected HTLC SPK is deterministically re-derivable from public terms. --- htlc = bt.build_htlc( diff --git a/scripts/dust_swap_run.py b/scripts/dust_swap_run.py index ecef3c24..3c90d913 100644 --- a/scripts/dust_swap_run.py +++ b/scripts/dust_swap_run.py @@ -129,7 +129,10 @@ async def run_dust_swap(args: argparse.Namespace) -> None: h = hashlib.sha256(p).digest() margin_blocks = policy.margin.normalize_to(bt.TimeUnit.BLOCKS, block_interval_s=policy.block_interval_s).value t_rxd = bt.Timelock(args.t_rxd_blocks, bt.TimeUnit.BLOCKS) - t_btc = bt.Timelock(args.t_rxd_blocks + margin_blocks + 4, bt.TimeUnit.BLOCKS) # > t_rxd + margin + # INVERTED (#482): the maker holds p and LOCKS the Radiant leg, so t_rxd carries the LONGER + # timeout and the BTC leg it CLAIMS is the shorter one. This built t_rxd + margin + 4 — the + # layout where the maker can refund RXD and still claim BTC with p. + t_btc = bt.Timelock(args.t_rxd_blocks - margin_blocks - 4, bt.TimeUnit.BLOCKS) # < t_rxd - margin maker_btc = coincurve.PrivateKey(os.urandom(32)) taker_btc_kp = generate_keypair(btc_network) diff --git a/scripts/eth_swap_run.py b/scripts/eth_swap_run.py index 598940f6..5630a1e8 100644 --- a/scripts/eth_swap_run.py +++ b/scripts/eth_swap_run.py @@ -203,7 +203,7 @@ def _policy(args: argparse.Namespace, *, remaining_s: int | None = None) -> Marg # typed. That is deliberate: they are the check on the derivation, not a substitute for it, and # a derivation nothing verifies is how the exact-division off-by-one survived in the first place. _assert_t_rxd_covers_the_takers_wait(args, remaining_s=remaining_s) - _assert_t_rxd_opens_before_the_eth_deadline(args, remaining_s=remaining_s) + _assert_t_rxd_outlasts_the_eth_deadline(args, remaining_s=remaining_s) _assert_t_rxd_bounds_the_vulnerable_window(args, remaining_s=remaining_s) return MarginPolicy( is_measured=True, @@ -238,8 +238,28 @@ def _t_rxd_covers_the_takers_wait(args: argparse.Namespace) -> bool: return int(args.t_rxd_blocks) >= math.ceil(_cross_clock_margin(args).total_s() / fast) -def _highest_t_rxd_the_deadline_accepts(args: argparse.Namespace, remaining_s: int | None) -> int: - """Upper bound B as a VALUE — the largest t_rxd whose projected refund precedes the deadline.""" +def _lowest_t_rxd_the_deadline_requires(args: argparse.Namespace, remaining_s: int | None) -> int: + """Bound B as a VALUE — the SMALLEST t_rxd whose projected refund OUTLASTS the deadline. + + THIS BOUND CHANGED SIDES WITH #482, and that changes the shape of the whole feasible set. + + It was an upper bound: the RXD refund had to open BEFORE the ETH deadline, so a longer window + was the thing to guard against, and the set was an interval squeezed between floors below and + this cap above. That squeeze is what produced the "the deadline must hold roughly TWO margins" + reasoning, the empty-set-at-a-fractional-tail bug, and the run that burned two funded covenants + being sent from one error into another. + + Under the corrected relation the maker LOCKS the Radiant leg, so its refund must open AFTER the + deadline plus the margin. A LONGER t_rxd is then monotonically safer, and the deadline imposes + a FLOOR. Every bound in this file is now a floor and the only ceiling left is BIP68 — so the + feasible set is `[max(floors), 65535]` and cannot be empty by rounding disagreement between two + bounds that were algebraically equal. The class of bug that squeeze produced is gone, not + fixed. + + The remedy for infeasibility inverts too: the floor RISES with the deadline, so a deadline too + FAR in the future is what no 16-bit CSV can cover. Asking for more time used to be the fix; it + is now the cause. + """ # THE SAME INTERVAL THE GATE MULTIPLIES BY. It used the nominal, matching a coordinator call # site that was itself passing the wrong one; the two agreed with each other and disagreed with # the sizer. With the gate corrected to the fast tail this bound divides by the fast tail too, @@ -250,15 +270,14 @@ def _highest_t_rxd_the_deadline_accepts(args: argparse.Namespace, remaining_s: i # straight out of the budget. Sizing against `now` silently assumes instant funding: a run that # took ~740s to fund and mine overshot by 607s and was refused AFTER the covenant was paid for. # `max_covenant_confirm_wait_s` is precisely the allowance for that delay, so spend it here. - budget_s = ( - _eth_budget_s(args, remaining_s) - _cross_clock_margin(args).total_s() - int(args.max_covenant_confirm_wait_s) - ) - # The gate refuses at `projected >= deadline`, so the largest ACCEPTED value is one short of - # the quotient when it divides exactly. Verified by binary-searching the real gate: for a 24h - # timeout it accepts 2186 and refuses 2187, while a bare floor() computes 2187. An upper bound - # that names a value the gate then rejects sends the operator to fix an error into an error — - # which is exactly how this run burned two funded covenants. - return math.ceil(budget_s / nominal) - 1 + # NO confirm-wait reserve. It was subtracted here to model a LATE covenant confirmation pushing + # the refund past a deadline it had to precede. The refund must now OUTLAST the deadline, so a + # late confirm only adds margin and spending the wait here would assume the optimistic + # direction — the same term #482 removed from the library gate for the same reason. + budget_s = _eth_budget_s(args, remaining_s) + _cross_clock_margin(args).total_s() + # The gate accepts at `projected >= required`, so equality is ACCEPTED and the smallest such + # value is the plain ceiling — no `- 1`, which belonged to a strict `<` on the other side. + return math.ceil(budget_s / nominal) def _asset_vulnerable_window_s(args: argparse.Namespace, remaining_s: int | None) -> float: @@ -319,11 +338,16 @@ def _t_rxd_feasible_range(args: argparse.Namespace, *, remaining_s: int | None = None when that set is empty. Computed from the same predicates the three `_assert_` functions call, so "the guard passed but a bound then refused" is not representable. """ - hi = min(_highest_t_rxd_the_deadline_accepts(args, remaining_s), _T_RXD_BIP68_MAX_BLOCKS) - if hi < 1: + # ALL FLOORS, ONE CEILING (#482). Bound B moved from the `hi` side to the `lo` side, so the + # only ceiling left is the BIP68 field width. + hi = _T_RXD_BIP68_MAX_BLOCKS + floors = [_lowest_t_rxd_the_deadline_requires(args, remaining_s)] + from_predicates = _lowest_t_rxd_meeting_the_floors(args, remaining_s) + if from_predicates is None: return None - lo = _lowest_t_rxd_meeting_the_floors(args, remaining_s) - if lo is None or lo > hi: + floors.append(from_predicates) + lo = max(1, *floors) + if lo > hi: return None return (lo, hi) @@ -379,40 +403,49 @@ def _recommended_t_rxd_blocks(args: argparse.Namespace, *, remaining_s: int | No return hi -def _first_conceivable_eth_timeout_s(args: argparse.Namespace) -> int: - """The smallest `--eth-timeout-s` that bounds A and B alone could ever both accept. +def _largest_workable_eth_timeout_s(args: argparse.Namespace) -> int | None: + """The largest `--eth-timeout-s` at or below the requested one that actually yields a t_rxd. - Exact, and derived rather than fudged. Bound A floors t_rxd at `ceil(margin/fast)`; bound B - caps it at `ceil((budget - margin - wait)/fast) - 1`. The cap reaches the floor only once - `budget - margin - wait > fast * ceil(margin/fast)`. Used ONLY as the starting point of the - search below — it is a necessary condition, never a sufficient one, and the loop is what makes - the printed number true. - """ - margin_s = _cross_clock_margin(args).total_s() - wait_s = int(args.max_covenant_confirm_wait_s) - fast = float(args.rxd_block_interval_fast_s or 0) - if fast <= 0: - return margin_s + wait_s + 1 - return math.floor(margin_s + wait_s + fast * math.ceil(margin_s / fast)) + 1 + THE SEARCH REVERSED WITH THE RELATION (#482), and so did the advice it produces. + Bound B used to CAP t_rxd, so a deadline too SHORT emptied the feasible set and the remedy was + to ask for more time — `_smallest_workable_eth_timeout_s`, searching upward from the requested + value. Bound B is a floor now: `t_rxd >= ceil((budget + margin) / fast)` rises with the + deadline, and the only ceiling left is the BIP68 field width. A short deadline is therefore + always satisfiable, and what cannot be satisfied is a deadline too FAR — no 16-bit relative CSV + reaches it. -def _smallest_workable_eth_timeout_s(args: argparse.Namespace) -> int | None: - """The smallest `--eth-timeout-s` at or above the requested one that actually yields a t_rxd. + Searching upward would have walked further from feasibility on every step and returned None, + turning a fixable situation into "the margin itself is the thing to change". Worse, had it ever + found something it would have advised an operator to LENGTHEN the deadline that was already too + long. - CHECKED, not computed. Every candidate is fed back through `_a_workable_t_rxd_exists` — the - same test the refusal uses — so the number printed in the message cannot be one the next parse - turns around and rejects. Measured over 4000 refused fractional rows, the search never ran past - its step budget. The previous version printed `2*margin + wait + ceil(fast)`, a - closed form that models the bounds instead of asking them, and at a fractional fast tail it - both cleared deadlines with an empty feasible set and could name a minimum nothing verified. + CHECKED, not computed, exactly as before: every candidate goes back through + `_a_workable_t_rxd_exists`, the same predicate the refusal uses, so the number printed cannot be + one the next parse turns around and rejects. """ - candidate = max(_first_conceivable_eth_timeout_s(args), int(args.eth_timeout_s) + 1) + # START FROM THE ANALYTIC MAXIMUM, then step down to VERIFY it. A one-second walk down from the + # requested value cannot converge: the deadline is typically hundreds of thousands of seconds + # past the reach, and the step budget is a handful. The old upward search could start at the + # requested value because the feasible region began just above it; the infeasible region is now + # unbounded above, so the search has to begin at the boundary rather than walk to it. + # + # The boundary is exact: bound B needs `ceil((budget + margin) / fast) <= BIP68_MAX`, so + # `budget <= BIP68_MAX * fast - margin`. Derived, then fed back through the same predicate the + # refusal uses — the step-down loop is what makes the printed number true, as before. + fast = float(args.rxd_block_interval_fast_s or 0) + if fast <= 0: + return None + analytic = int(_T_RXD_BIP68_MAX_BLOCKS * fast) - _cross_clock_margin(args).total_s() + candidate = min(int(args.eth_timeout_s) - 1, analytic) for _ in range(_MINIMUM_SEARCH_STEPS): + if candidate < 1: + return None probe = argparse.Namespace(**vars(args)) probe.eth_timeout_s = candidate if _a_workable_t_rxd_exists(probe): return candidate - candidate += 1 + candidate -= 1 return None @@ -451,19 +484,22 @@ def _assert_the_eth_deadline_can_hold_the_margins(args: argparse.Namespace, *, r wait_s = int(args.max_covenant_confirm_wait_s) budget_s = _eth_budget_s(args, remaining_s) floor_t = _lowest_t_rxd_meeting_the_floors(args, remaining_s) - cap_t = _highest_t_rxd_the_deadline_accepts(args, remaining_s) + deadline_floor_t = _lowest_t_rxd_the_deadline_requires(args, remaining_s) resumed = "" if remaining_s is None else " remaining on the resumed swap's deadline" where = ( - f"the floors put it at >= {floor_t} and the deadline caps it at <= {cap_t}" + f"the floors put it at >= {max(floor_t, deadline_floor_t)} (the deadline alone needs " + f">= {deadline_floor_t}), past the BIP68 cap of {_T_RXD_BIP68_MAX_BLOCKS}" if floor_t is not None - else f"no value up to the BIP68 cap of {_T_RXD_BIP68_MAX_BLOCKS} meets the floors, which cap at <= {cap_t}" + else f"no value up to the BIP68 cap of {_T_RXD_BIP68_MAX_BLOCKS} meets the floors; the " + f"deadline alone needs >= {deadline_floor_t}" ) if remaining_s is None: - minimum = _smallest_workable_eth_timeout_s(args) + maximum = _largest_workable_eth_timeout_s(args) remedy = ( - f" minimum: --eth-timeout-s {minimum} ({minimum / 3600:.2f} h) — verified, not " - f"estimated: it is fed back through this same feasibility test.\n" - if minimum is not None + f" maximum: --eth-timeout-s {maximum} ({maximum / 3600:.2f} h) — verified, not " + f"estimated: it is fed back through this same feasibility test. NOTE the direction: " + f"the deadline is too FAR for a 16-bit CSV to reach, so it must come DOWN (#482).\n" + if maximum is not None else " no nearby --eth-timeout-s clears it; the margin itself is the thing to change.\n" ) else: @@ -532,41 +568,43 @@ def _assert_t_rxd_covers_the_takers_wait(args: argparse.Namespace, *, remaining_ ) -def _assert_t_rxd_opens_before_the_eth_deadline(args: argparse.Namespace, *, remaining_s: int | None = None) -> None: - """The RXD refund must OPEN before the ETH deadline minus the margin — the upper bound. +def _assert_t_rxd_outlasts_the_eth_deadline(args: argparse.Namespace, *, remaining_s: int | None = None) -> None: + """The RXD refund must OPEN AFTER the ETH deadline plus the margin — a FLOOR since #482. - Learned the expensive way. The lower bound above divides the margin by the FAST tail, because - fast blocks shrink the taker's window. The coordinator's punctuality gate then projects the - same t_rxd forward by MULTIPLYING by the NOMINAL interval. Those two only cancel when both use - the same interval — `assert_covenant_confirms_before_eth_deadline` says so in as many words — - and sizing with 36s while the gate multiplies by 300s inflates the projection by ~8x. + THIS ASSERTION USED TO REFUSE THE OPPOSITE VALUES. It was `_assert_t_rxd_opens_before_the_eth_ + deadline`, an upper bound: a t_rxd "too LONG" meant the maker could not refund before the ETH + deadline. Under the corrected relation the maker's leg is SUPPOSED to outlast the counter leg, + and a t_rxd that is too SHORT is what lets the maker refund its Radiant leg while still holding + `p` to claim the ETH leg — taking both. - A t_rxd of 2203, correct against the lower bound, projected the RXD refund 7.6 DAYS out against - a 22h budget. The gate caught it and refused to lock, which is the system working — but it - caught it AFTER the covenant had been funded, because nothing checked it at argument-parse - time. This does, so the operator learns the valid RANGE before spending a fee. + So the refusal message is not a rewording of the old one. The old one told an operator to + SHORTEN a window that needed lengthening, which is the single worst thing a guard can do: it is + confidently wrong, it names the right flag, and following it walks the operator into the exact + layout the swap has to avoid. + + The interval lesson survives intact and is still the reason this is checked at parse time + rather than at lock time: the taker's-wait floor divides the margin by the FAST tail while the + coordinator's gate multiplies by the same tail, and those cancel only while both use it. A + t_rxd correct against one bound and wrong against the gate was caught AFTER the covenant was + funded, which is what this refusal exists to prevent. """ - hi = _highest_t_rxd_the_deadline_accepts(args, remaining_s) + lo = _lowest_t_rxd_the_deadline_requires(args, remaining_s) have = int(args.t_rxd_blocks) - if have <= hi: + if have >= lo: return nominal = float(args.rxd_block_interval_fast_s or args.rxd_block_interval_s) - budget_s = ( - _eth_budget_s(args, remaining_s) - _cross_clock_margin(args).total_s() - int(args.max_covenant_confirm_wait_s) - ) - fast = float(args.rxd_block_interval_fast_s or 0) - lo = math.ceil(_cross_clock_margin(args).total_s() / fast) if fast > 0 else 1 + margin_s = _cross_clock_margin(args).total_s() + required_s = _eth_budget_s(args, remaining_s) + margin_s raise SystemExit( - f"--t-rxd-blocks {have} is too LONG. The coordinator projects the RXD refund forward at the " - f"NOMINAL {nominal:.0f}s interval, giving {have * nominal / 86400:.1f} days against a " - f"{budget_s / 3600:.1f} h budget (--eth-timeout-s minus the {_cross_clock_margin(args).total_s()}s " - f"cross-clock margin AND the {args.max_covenant_confirm_wait_s}s covenant-confirm reserve). " - f"The maker could not refund before the ETH deadline.\n" + f"--t-rxd-blocks {have} is too SHORT. The coordinator projects the RXD refund forward at " + f"the {nominal:.0f}s interval, giving {have * nominal / 3600:.1f} h against the " + f"{required_s / 3600:.1f} h this swap requires (--eth-timeout-s PLUS the {margin_s}s " + f"cross-clock margin). The maker's Radiant refund would open while it can still claim the " + f"ETH leg with p — it could take both legs.\n" f" OMIT --t-rxd-blocks entirely and it is derived: {_recommended_t_rxd_blocks(args, remaining_s=remaining_s)}\n" - f" (the taker's-wait bound floors it at {lo} and this one caps it at {hi}, but the " - f"vulnerable-window bound closes that range to a single value — there is nothing to choose)\n" - f" the LOWER bound divides the margin by the FAST tail; this UPPER bound multiplies by the " - f"NOMINAL one. Both are real, and they are not the same number." + f" minimum: --t-rxd-blocks {lo}\n" + f" NOTE the direction: before #482 this bound was a CAP and this message said 'too LONG'. " + f"Lengthening t_rxd is the fix now; shortening it was never safe." ) @@ -589,8 +627,23 @@ def _derive_t_rxd_blocks(args: argparse.Namespace, *, remaining_s: int | None = """ return eth_absolute_to_rxd_relative_blocks( eth_timeout_unix_s=int(time.time()) + _eth_budget_s(args, remaining_s), - # The CSV clock starts at covenant MINING, not now, so reserve the confirm allowance. - expected_rxd_lock_time_unix_s=int(time.time()) + int(args.max_covenant_confirm_wait_s), + # The CSV clock starts at covenant MINING, so this anchor is an ASSUMPTION about when + # that happens — and the safe end of that assumption INVERTED with the timelock direction + # (#482). + # + # The refund opens at `actual_confirm + t_rxd`, and `t_rxd` is committed in the covenant + # script before broadcast. The invariant is now `actual_confirm + t_rxd >= eth_timeout + + # margin`, so: + # + # confirm LATER than assumed -> refund opens later -> MORE margin -> safe (maker waits) + # confirm EARLIER than assumed -> refund opens sooner -> invariant BREAKS + # + # So the conservative anchor is the EARLIEST plausible confirm, i.e. `now`. Reserving + # `max_covenant_confirm_wait_s` here was right under the OLD relation — where the refund had + # to open BEFORE the ETH deadline and a late confirm pushed it past — and is exactly wrong + # under the corrected one. `eth_swap_two_host.py` already anchors on `now`; the two runners + # disagreed, and this is the half that was unsafe. + expected_rxd_lock_time_unix_s=int(time.time()), margin=_cross_clock_margin(args), # The FAST tail. A slow chain only lengthens the maker's lock; a fast one shrinks the # taker's claim window, so the fast tail is the direction that has to be safe. @@ -701,7 +754,9 @@ def _build_terms_and_covenant(args, *, eth_timeout: int, minted=None, restore: d h = hashlib.sha256(p_secret.unsafe_raw_bytes()).digest() taker_rxd, maker_rxd = PrivateKey(os.urandom(32)), PrivateKey(os.urandom(32)) t_rxd = bt.Timelock(args.t_rxd_blocks, bt.TimeUnit.BLOCKS) - t_btc = bt.Timelock(args.t_rxd_blocks + args.margin_blocks + 4, bt.TimeUnit.BLOCKS) # decorative for ETH + # INVERTED (#482) — see dust_swap_run.py. Decorative for ETH (the real deadline is + # eth_timeout_unix_s) but it still passes through the same-unit ordering guard. + t_btc = bt.Timelock(args.t_rxd_blocks - args.margin_blocks - 4, bt.TimeUnit.BLOCKS) taker_pkh = bytes(Hex20(taker_rxd.public_key().hash160())) maker_pkh = bytes(Hex20(maker_rxd.public_key().hash160())) if args.asset_variant == "nft": diff --git a/scripts/eth_swap_two_host.py b/scripts/eth_swap_two_host.py index ed6f9efc..26def568 100644 --- a/scripts/eth_swap_two_host.py +++ b/scripts/eth_swap_two_host.py @@ -245,8 +245,9 @@ def _terms_from_public( identical covenant SPK + dest hashes — that mutual re-derivation is the trust anchor.""" t_rxd = bt.Timelock(t_rxd_blocks, bt.TimeUnit.BLOCKS) # t_btc is decorative for an ETH swap (the real ETH deadline is eth_timeout_unix_s), but it must - # stay > t_rxd so the same-unit ordering guard in NegotiatedTerms passes; keep it well clear. - t_btc = bt.Timelock(t_rxd_blocks + margin_blocks + 4, bt.TimeUnit.BLOCKS) + # stay BELOW t_rxd so the same-unit ordering guard in NegotiatedTerms passes; keep it well + # clear. It was t_rxd + margin + 4 before #482 inverted the relation. + t_btc = bt.Timelock(t_rxd_blocks - margin_blocks - 4, bt.TimeUnit.BLOCKS) cov = build_htlc_covenant_rxd( amount=rxd_photons, taker_pkh=bytes(Hex20(taker_pkh)), @@ -957,13 +958,17 @@ def run_self_check() -> None: assert_timelock_margin(terms2.t_btc, terms2.t_rxd, policy) print(" [ok] taker's INDEPENDENT timelock-margin check passes for honest terms") - # ...and REFUSES a hostile too-tight envelope (t_btc - t_rxd < margin). + # ...and REFUSES a hostile too-tight envelope (t_rxd - t_btc < margin). + # + # The pair is the other way round since #482. Written as t_btc=61/t_rxd=60 it no longer even + # CONSTRUCTS — the ordering guard rejects it first — so `assert_timelock_margin` never runs and + # this check would pass on an exception raised by the wrong thing entirely. hostile = NegotiatedTerms( hashlock=h, btc_sats=1000, radiant_amount=1000, - t_btc=bt.Timelock(61, bt.TimeUnit.BLOCKS), # only 1 block over t_rxd — far below margin 36 - t_rxd=bt.Timelock(60, bt.TimeUnit.BLOCKS), + t_btc=bt.Timelock(60, bt.TimeUnit.BLOCKS), + t_rxd=bt.Timelock(61, bt.TimeUnit.BLOCKS), # only 1 block over t_btc — far below margin 36 asset_variant="rxd", genesis_ref=b"", taker_dest_hash=cov.expected_taker_hash, diff --git a/scripts/watchtower_dust_run.py b/scripts/watchtower_dust_run.py index 85205a36..350c3539 100644 --- a/scripts/watchtower_dust_run.py +++ b/scripts/watchtower_dust_run.py @@ -234,8 +234,10 @@ def cmd_setup(args: argparse.Namespace) -> int: "--refund-spk must be a standard spendable scriptPubKey (P2WPKH/P2TR/P2WSH/P2PKH/P2SH); " "derive it from your checksum-validated refund address" ) - if args.t_btc <= args.t_rxd: - raise SystemExit(f"--t-btc ({args.t_btc}) must be > --t-rxd ({args.t_rxd}) (BTC is the longer leg)") + # INVERTED (#482): the maker holds p and LOCKS the Radiant leg, so the RADIANT leg carries the + # longer refund and the BTC leg it claims is the shorter one. + if args.t_rxd <= args.t_btc: + raise SystemExit(f"--t-rxd ({args.t_rxd}) must be > --t-btc ({args.t_btc}) (Radiant is the longer leg)") # Keys: taker refund key (generated, persisted 0600) + a maker claim PUBKEY (we never claim, so its # private half is discarded — only the x-only pubkey is needed to reconstruct the taptree). @@ -408,8 +410,8 @@ def _parse_args(argv=None) -> argparse.Namespace: s.add_argument("--swap-id", default="dust1", help="swap id (== the SwapRecord/sidecar file stem)") s.add_argument("--network", default="bc", help="bc | bcrt | tb | signet") s.add_argument("--btc-sats", type=int, required=True, help="exact sats to fund the HTLC with") - s.add_argument("--t-btc", type=int, default=2, help="BTC refund CSV in blocks (the longer leg)") - s.add_argument("--t-rxd", type=int, default=1, help="RXD refund CSV in blocks (must be < --t-btc)") + s.add_argument("--t-btc", type=int, default=1, help="BTC refund CSV in blocks (the shorter leg)") + s.add_argument("--t-rxd", type=int, default=2, help="RXD refund CSV in blocks (must be > --t-btc)") s.add_argument("--refund-spk", required=True, help="hex scriptPubKey the refund must pay (YOUR address)") s.add_argument("--refund-address", help="the refund address, for display only") s.add_argument("--force", action="store_true", help="overwrite an existing state file") diff --git a/src/pyrxd/gravity/eth_rxd_timelock.py b/src/pyrxd/gravity/eth_rxd_timelock.py index 1038901b..32e89237 100644 --- a/src/pyrxd/gravity/eth_rxd_timelock.py +++ b/src/pyrxd/gravity/eth_rxd_timelock.py @@ -11,7 +11,7 @@ (floor) rounding + a fail-closed safety floor, so the canonical HTLC ordering invariant holds across the unit + anchor boundary: the asset/RXD leg (claimed SECOND, by the taker) opens its refund strictly BEFORE the counter/ETH leg's deadline by at least the margin — - i.e. the counter/ETH leg, claimed FIRST by the maker, holds the LONGER deadline (the + i.e. the counter/ETH leg, claimed FIRST by the maker, holds the SHORTER deadline (the cross-clock analog of the BTC ``t_BTC > t_RXD`` invariant). The inherent risk this ordering creates (a maker withholding its claim until past the RXD refund, then claiming AND refunding) is mitigated by the proactive asset-refund + the cross-clock margin coupling, not @@ -59,6 +59,7 @@ __all__ = [ "CrossClockMargin", "assert_covenant_confirms_before_eth_deadline", + "assert_eth_deadline_is_claimable", "assert_t_rxd_fits_the_eth_deadline", "eth_absolute_to_rxd_relative_blocks", ] @@ -77,7 +78,7 @@ class CrossClockMargin: Each component is a deliberate, documented seconds budget; the converter subtracts their sum from the ETH deadline before sizing the RXD window, so the RXD refund opens strictly BEFORE the ETH deadline by at least this much wall-clock (the RXD/asset leg, - claimed second, holds the SHORTER deadline; the ETH/counter leg the longer). + LOCKED by the maker, holds the LONGER deadline; the ETH/counter leg the shorter). ``eth_reorg_finality_s`` is the post-Merge ETH finalized-checkpoint lag in the STEADY STATE (~2 epochs ≈ 768 s ≈ 12.8 min — formally specified, ethereum.org/eth2book). @@ -144,12 +145,18 @@ def eth_absolute_to_rxd_relative_blocks( """Size the RXD covenant's RELATIVE CSV window (in BLOCKS) from the ETH ABSOLUTE deadline. The RXD refund — relative, anchored at covenant mining ≈ ``expected_rxd_lock_time_unix_s`` - — must open strictly BEFORE the ETH deadline minus the full margin (the RXD/asset leg - holds the SHORTER deadline; the ETH/counter leg the longer). The window is sized as large - as that allows (rxd-refund pushed up to, but not past, ``eth_timeout - margin``, maximising - the taker's claim window). So the available wall-clock budget for the RXD window is:: + — must open strictly AFTER the ETH deadline plus the full margin (the RXD/asset leg is the + one the MAKER LOCKED, so it holds the LONGER deadline; the ETH/counter leg the shorter). + INVERTED 2026-08-31, #482: this subtracted the margin, putting the RXD refund BEFORE the ETH + deadline and handing the maker a window in which to refund the covenant while ``p`` was still + secret and then claim the counter leg. See :func:`assert_timelock_margin` for the rule and the + Herlihy citation. - budget_s = eth_timeout_unix_s - margin.total_s() - expected_rxd_lock_time_unix_s + The margin's components were always the right budget for THIS direction — every one is time + the TAKER needs after the reveal (ETH finality, stall tolerance, claim burial, confirm slack, + rounding), and the maker may reveal as late as ``eth_timeout``. Only the sign was wrong:: + + budget_s = eth_timeout_unix_s + margin.total_s() - expected_rxd_lock_time_unix_s converted to blocks by FLOOR. Flooring can only SHORTEN the RXD window, which lets the maker reclaim the asset no later than computed (never longer); the sub-block remainder @@ -182,7 +189,7 @@ def eth_absolute_to_rxd_relative_blocks( "on mainnet, e.g. p10 — NOT the mean; see docstring)" ) - budget_s = eth_timeout_unix_s - margin.total_s() - expected_rxd_lock_time_unix_s + budget_s = eth_timeout_unix_s + margin.total_s() - expected_rxd_lock_time_unix_s if budget_s <= 0: raise ValidationError( f"no RXD timelock budget: eth_timeout - margin - rxd_lock_time = {budget_s}s " @@ -200,9 +207,11 @@ def eth_absolute_to_rxd_relative_blocks( # a zero confirm wait, which never lands on the boundary. The real run's parameters do: # (86400 - 7068 - 600) / 36 = 2187.0 exactly, so the canonical derivation produced 2187 and the # gate accepted only 2186. Deriving one block SHORT is the safe direction anyway: it opens the - # maker's refund marginally earlier, costing the taker a block of claim window rather than - # letting the refund land past the deadline it is sized to precede. - t_rxd_blocks = math.ceil(budget_s / rxd_block_interval_s) - 1 + # maker's refund marginally LATER, costing the maker a block of lock time rather than letting + # the refund open before the deadline it is sized to outlast (#482 inverted this: the gate is + # now a LOWER bound on t_rxd, so `ceil` with no -1 is the smallest accepted value, where it was + # `ceil - 1` for the largest accepted one). + t_rxd_blocks = math.ceil(budget_s / rxd_block_interval_s) # ...and then ASK THE GATE, rather than trusting that arithmetic to match it. # # `ceil(x) - 1` is algebraically the largest `t` with `t * I < budget`, and it is exact for an @@ -232,6 +241,13 @@ def eth_absolute_to_rxd_relative_blocks( f"{_MAX_RXD_CSV_BLOCKS} (ETH deadline too far in the future to map to a " "relative CSV window)" ) + # START ONE BELOW THE ANALYTIC VALUE, then step up. `ceil` is the exact minimum only in exact + # arithmetic; at fractional intervals the float division can round the quotient UP across an + # integer boundary, and starting AT it would then return a window one block longer than the + # gate actually requires, with no step able to find the shorter one (the search only goes up). + # This is the mirror of the pre-#482 note about `ceil(x) - 1` legitimately being one block low. + # The gate remains the authority: whatever this returns has been accepted by it. + t_rxd_blocks = max(floor_blocks, t_rxd_blocks - 1) for _ in range(_SIZER_GATE_STEPS): try: assert_covenant_confirms_before_eth_deadline( @@ -246,21 +262,27 @@ def eth_absolute_to_rxd_relative_blocks( ) break except ValidationError: - t_rxd_blocks -= 1 + # UP, not down (#482). The gate bounds t_rxd from BELOW now, so a refusal means the + # window is too SHORT. Stepping down was the old direction and moves away from + # acceptance — the loop then always exhausts and reports the sizer and gate as + # disagreeing, which is how this was found. + t_rxd_blocks += 1 else: # pragma: no cover — the analytic value is never more than one block out raise ValidationError( f"could not size a t_rxd the punctuality gate accepts within {_SIZER_GATE_STEPS} " - f"blocks of {math.ceil(budget_s / rxd_block_interval_s) - 1} (budget {budget_s}s at " + f"blocks of {math.ceil(budget_s / rxd_block_interval_s)} (budget {budget_s}s at " f"{rxd_block_interval_s}s/block). The sizer and the gate disagree by more than a " "rounding step, which means one of them has changed meaning." ) - # The step-down can cross the safety floor by one, so re-check it. Not the cap: stepping down - # only ever decreases, and a value that was under the cap stays under it. - if t_rxd_blocks < floor_blocks: + # The step-UP can cross the BIP68 cap by one, so re-check THAT. Not the floor: stepping up only + # ever increases, and a value already at or above the floor stays there. This pairing flipped + # with the search direction (#482) — re-checking the floor after an upward search is a check + # that can no longer fail, and would have left the cap unguarded. + if t_rxd_blocks > _MAX_RXD_CSV_BLOCKS: raise ValidationError( - f"RXD timelock {t_rxd_blocks} blocks below safety floor {floor_blocks} after the " - f"punctuality gate required one block less than the {budget_s}s budget allows at " - f"{rxd_block_interval_s}s/block" + f"RXD timelock {t_rxd_blocks} blocks exceeds the BIP68 16-bit cap {_MAX_RXD_CSV_BLOCKS} " + f"after the punctuality gate required one block more than the {budget_s}s budget allows " + f"at {rxd_block_interval_s}s/block" ) return Timelock(t_rxd_blocks, TimeUnit.BLOCKS) @@ -273,6 +295,7 @@ def assert_covenant_confirms_before_eth_deadline( t_rxd: Timelock, rxd_block_interval_s: float, max_covenant_confirm_wait_s: int, + elapsed_blocks: int = 0, ) -> None: """Covenant-punctuality gate (re-audit SC-3/TLK-1). @@ -320,16 +343,85 @@ def assert_covenant_confirms_before_eth_deadline( if max_covenant_confirm_wait_s < 0: raise ValidationError("max_covenant_confirm_wait_s must be >= 0") - deadline_s = eth_timeout_unix_s - margin.total_s() - projected_rxd_open_s = now_unix_s + max_covenant_confirm_wait_s + math.ceil(t_rxd.value * rxd_block_interval_s) - if projected_rxd_open_s >= deadline_s: + # ASSERT THE INVARIANT ITSELF, not a proxy for it (#482 §5). This checked "does the covenant + # confirm by the time the sizing assumed" — the right question under the OLD relation, where + # the RXD refund had to open BEFORE the ETH deadline and a LATE confirm pushed it past. + # + # The relation is inverted now: the refund must open AFTER the deadline plus the margin, so a + # late confirm only adds margin (a liveness cost to the maker) and an EARLY one is what eats + # it. The floor is therefore taken at the EARLIEST plausible confirm — `now_unix_s`, with no + # allowance added — and the check is the property itself: + # + # earliest_confirm + t_rxd >= eth_timeout + margin + # + # `max_covenant_confirm_wait_s` is still validated above and still meaningful to the caller as + # an operational bound, but it no longer belongs in THIS arithmetic: adding it here would + # assume a late confirm, which is the optimistic direction now. + # ELAPSED DEPTH IS SUBTRACTED (#482, the #531 class applied to this gate). `t_rxd` is a + # RELATIVE CSV counted from the covenant's MINING, so once the covenant has confirmations the + # refund opens that much sooner. Anchoring at `now` with the undecremented `t_rxd` overstates + # the window by exactly `elapsed_blocks * interval` — and the MAKER chooses that number, by + # locking its covenant early and presenting the swap late. Under the old relation an overstated + # window was the safe direction, which is why this went unnoticed; inverted, it is precisely + # the direction that lets a maker refund RXD while still holding `p` to claim the ETH leg. + _require_int(elapsed_blocks, "elapsed_blocks") + if elapsed_blocks < 0: + raise ValidationError("elapsed_blocks cannot be negative") + remaining_blocks = t_rxd.value - elapsed_blocks + required_open_s = eth_timeout_unix_s + margin.total_s() + earliest_rxd_open_s = now_unix_s + math.ceil(remaining_blocks * rxd_block_interval_s) + if earliest_rxd_open_s < required_open_s: + raise ValidationError( + f"the RXD refund could open too EARLY: the CSV clock starts at covenant MINING, so a " + f"confirmation at {now_unix_s} puts the refund at {earliest_rxd_open_s} ({remaining_blocks} blk left of {t_rxd.value}), before the " + f"{required_open_s} this swap requires (eth_timeout {eth_timeout_unix_s} + margin " + f"{margin.total_s()}s). The maker LOCKS the Radiant leg, so it must outlast the leg " + "the maker CLAIMS — refusing to lock RXD (#482). A LATE confirmation is safe here and " + "costs the maker only lock time; an early one is what eats the taker's window." + ) + + +def assert_eth_deadline_is_claimable( + *, + now_unix_s: int, + eth_timeout_unix_s: int, + margin: CrossClockMargin, +) -> None: + """Refuse an ETH deadline that has already passed, or is too near for the swap to complete. + + THIS EXISTS BECAUSE #482's INVERSION SILENTLY DROPPED IT. Under the old (wrong) relation the + gate demanded the projected RXD refund land BEFORE ``eth_timeout``, so an expired or near-expiry + deadline failed automatically — the refusal was a side effect of the arithmetic, and the two + tests covering it were the only thing recording that the requirement existed. Inverting the + relation turned a close deadline into the EASY case (the RXD refund clears it trivially), so + both tests went green-by-vacuity and the protection disappeared with nothing to show for it. + A requirement that only ever held as a side effect is one refactor away from gone. + + THE REQUIREMENT ITSELF IS INDEPENDENT of the ordering invariant, which is why it needs its own + check. Ordering asks "if both parties act, can either be robbed?". This asks "can the party who + must act still act at all?". The maker claims the ETH leg, and will not do so until the taker's + funding is final — so the deadline must leave room for that finality plus a stall, or the taker + is funding a leg the maker provably cannot claim. Both parties then refund, which loses no + principal but burns fees, locks the taker's capital for the full ``t_rxd``, and hands the maker + a free option: it can watch the price and simply decline to reveal. + + Fail-closed on a deadline in the past — that case is not a tight window, it is a dead swap. + """ + _require_int(now_unix_s, "now_unix_s") + _require_int(eth_timeout_unix_s, "eth_timeout_unix_s") + + # The maker acts only on FINAL funding, so that is the floor: finality, plus the stall budget + # the policy already carries for it, plus the rounding/skew allowance. + claim_reachable_s = margin.eth_reorg_finality_s + margin.eth_finality_stall_tolerance_s + margin.rounding_slack_s + remaining_s = eth_timeout_unix_s - now_unix_s + if remaining_s < claim_reachable_s: + expired = " (ALREADY EXPIRED)" if remaining_s < 0 else "" raise ValidationError( - f"covenant would confirm too late: the RXD CSV clock starts at covenant MINING, and a " - f"confirmation at {now_unix_s + max_covenant_confirm_wait_s} is past the time the " - f"timelock sizing assumed ({projected_rxd_open_s} >= {deadline_s}), which shifts the " - "whole RXD refund window right — refusing to lock RXD (SC-3/TLK-1). This is a " - "PUNCTUALITY failure, not a slow-chain one: see this function's docstring before " - "changing the block interval in response to it." + f"the ETH deadline leaves too little time to claim{expired}: eth_timeout is {remaining_s}s away " + f"but the maker cannot act until the counter leg is final, which needs {claim_reachable_s}s " + f"(finality {margin.eth_reorg_finality_s}s + stall {margin.eth_finality_stall_tolerance_s}s + " + f"rounding {margin.rounding_slack_s}s). Funding this would confirm too late to be claimed — " + "both legs would refund, and until then the maker holds a free option (#482)." ) diff --git a/src/pyrxd/gravity/swap_coordinator.py b/src/pyrxd/gravity/swap_coordinator.py index 50ed9094..3d74a9b0 100644 --- a/src/pyrxd/gravity/swap_coordinator.py +++ b/src/pyrxd/gravity/swap_coordinator.py @@ -61,7 +61,11 @@ from pyrxd.security.reveal import reveal_boundary from pyrxd.security.secrets import SecretBytes -from .eth_rxd_timelock import CrossClockMargin, assert_covenant_confirms_before_eth_deadline +from .eth_rxd_timelock import ( + CrossClockMargin, + assert_covenant_confirms_before_eth_deadline, + assert_eth_deadline_is_claimable, +) from .finality import CounterClaimFinality, CounterClaimState from .ref_authenticity import verify_ref_authenticity from .swap_state import ( @@ -112,9 +116,10 @@ "the covenant off the Radiant chain (HZ-1, #392). (4) The " "MAKER claims the BTC FIRST, revealing p in the Bitcoin witness. (5) The TAKER " "scrapes p from Bitcoin and claims the Radiant asset before its refund opens. " - "Invariant: t_BTC > t_RXD + margin — the leg claimed second (Radiant) has the " - "SHORTER refund window; the first-claimed leg (BTC) holds the LONGER refund. " - "The taker's client MUST verify t_BTC - t_RXD >= margin before funding, or refuse. " + "Invariant: t_RXD > t_BTC + margin — the maker holds p and LOCKS the Radiant leg, " + "so THAT leg carries the LONGER refund and the leg the maker CLAIMS (BTC) the shorter " + "(Herlihy 1801.09515 §1: the secret generator locks at 6-delta and claims a 4-delta leg). " + "The taker's client MUST verify t_RXD - t_BTC >= margin before funding, or refuse. " "NB the NAME predates HZ-1 (#392), which inverted the lock order in (2)/(3); it is " "kept because it is exported, asserted in tests and quoted in a ValidationError, and " "because the half of it that names the timelock invariant is still exactly right." @@ -187,7 +192,7 @@ class MarginPolicy: Attributes ---------- margin: - The required minimum ``t_btc - t_rxd``, as a unit-tagged + The required minimum ``t_rxd - t_btc``, as a unit-tagged :class:`Timelock`. If ``is_measured`` is False this is an ESTIMATE. block_interval_s: Seconds-per-block used to normalise across units. For BTC the canonical @@ -646,7 +651,21 @@ def _stablecoin_value_floor_photons(terms: NegotiatedTerms, policy: MarginPolicy def assert_timelock_margin(t_btc: Timelock, t_rxd: Timelock, policy: MarginPolicy) -> None: - """Assert ``t_btc - t_rxd >= margin`` — fail-closed, cross-unit normalised. + """Assert ``t_rxd - t_btc >= margin`` — fail-closed, cross-unit normalised. + + INVERTED 2026-08-31 (#482 finding 1). This asserted ``t_btc - t_rxd >= margin``, which is the + wrong way round. Herlihy (arXiv:1801.09515) §1: Alice GENERATES the secret and locks at ``6∆``; + Bob locks at ``5∆``; Carol at ``4∆``; **Alice claims Carol's 4∆ leg**. The secret-holder's + LOCKED leg carries the LONGEST timeout and the leg they CLAIM the shortest, with Lemma 4.13 + giving the gap: "the timeout on each arc (u, v) is later by at least ∆ than the timeout on each + arc (v, w)". + + Our maker generates ``p``, LOCKS the Radiant covenant and CLAIMS the counter leg, so ``t_rxd`` + must be the longer one. Under the old relation the window ``[rxd_refund_opens, counter_deadline]`` + — at least ``margin`` wide BY CONSTRUCTION — let the maker refund the covenant while ``p`` was + still secret and then claim the counter leg. Both legs, deterministically, with the taker unable + to claim (no ``p``) or refund (its deadline is later). The safety buffer WAS the attack window. + Both legs and the margin are normalised to BLOCKS using ``policy.block_interval_s``. If either input is not a :class:`Timelock`, or the @@ -676,13 +695,16 @@ def assert_timelock_margin(t_btc: Timelock, t_rxd: Timelock, policy: MarginPolic except Exception as exc: # pragma: no cover - normalize_to only raises ValidationError raise ValidationError(f"could not normalise timelocks to a common unit: {exc}") from exc - if btc_blocks <= rxd_blocks: + if rxd_blocks <= btc_blocks: raise ValidationError( - f"timelock ordering violated: t_btc ({btc_blocks} blk) must exceed t_rxd ({rxd_blocks} blk)" + f"timelock ordering violated: t_rxd ({rxd_blocks} blk) must exceed t_btc ({btc_blocks} blk) — " + "the maker holds p and LOCKS the Radiant leg, so that leg carries the LONGER timeout " + "(Herlihy 1801.09515 §1). The reverse lets the maker refund the covenant while p is " + "still secret and then claim the counter leg." ) - if (btc_blocks - rxd_blocks) < margin_blocks: + if (rxd_blocks - btc_blocks) < margin_blocks: raise ValidationError( - f"insufficient margin: t_btc - t_rxd = {btc_blocks - rxd_blocks} blk < required {margin_blocks} blk " + f"insufficient margin: t_rxd - t_btc = {rxd_blocks - btc_blocks} blk < required {margin_blocks} blk " f"({'measured' if policy.is_measured else 'ESTIMATED'})" ) @@ -723,10 +745,21 @@ def taker_refund_window_open( This is a TIMING PREDICATE only — "the maker has not claimed and ``t_RXD - N`` is approaching" — NOT a prescription of which refund to run. (Formerly named ``should_taker_refund_proactively``; renamed because the name described an action - while the predicate only describes this window — deferred from PR #189.) The dominant adversarial - risk it guards: because ``t_BTC > t_RXD``, a malicious maker can withhold the BTC - claim until after ``t_RXD`` opens, then claim BTC (revealing ``p``) AND CSV-refund - the asset, taking both. Treat the trigger as "stop waiting", never "keep waiting". + while the predicate only describes this window — deferred from PR #189.) + + THE RISK THIS WAS WRITTEN FOR IS NOW CLOSED AT THE SOURCE, and it is worth recording that + this docstring DESCRIBED the defect for months while the ordering that enabled it stood. It + read: "because ``t_BTC > t_RXD``, a malicious maker can withhold the BTC claim until after + ``t_RXD`` opens, then claim BTC (revealing ``p``) AND CSV-refund the asset, taking both." + + That is #482 finding 1, stated exactly, and mitigated with a timing predicate that asks the + taker to bail out early rather than by fixing the relation. With ``t_RXD > t_BTC + margin`` + (see :func:`assert_timelock_margin`) the maker's own refund now opens LAST, so withholding + buys nothing: the counter leg expires first and the taker refunds it. + + The predicate is KEPT because it still detects a stalled maker — a liveness signal, not a + theft one — and because a swap negotiated under the old relation is still out there. Treat the + trigger as "stop waiting", never "keep waiting". IMPORTANT — what the taker DOES when this fires is :meth:`mutual_refund` (both legs unwind once both timeouts elapse), NOT an asset-only refund. The asset CSV refund @@ -1547,6 +1580,25 @@ async def pre_btc_lock_check(self, terms: NegotiatedTerms, *, now_unix_s: int | if gate is not None: return gate + # 7. THE CROSS-CLOCK ORDERING GATE, RE-RUN AGAINST THE WINDOW THAT ACTUALLY REMAINS. + # + # Step 3 ran it against the NEGOTIATED t_rxd, anchored at `now`. But t_rxd is a relative + # CSV from the covenant's mining, so `cov_confs` blocks of it are already spent and the + # refund opens that much sooner than step 3 computed. Under the OLD relation an overstated + # window was the conservative direction; inverted (#482), it is the direction that lets the + # maker refund its Radiant leg while still holding `p` for the ETH leg. + # + # THE GAP IS MAKER-CONTROLLED, which is what makes it an attack rather than an inaccuracy: + # the maker locks its covenant, waits, and only then presents the swap. Step 3 cannot see + # that — `cov_confs` is not known until the chain read at step 5. Exactly the #531 shape, + # which fixed this same conflation for the burial floor and left the ordering gate on the + # negotiated value. + if terms.eth_timeout_unix_s is not None: + try: + self._assert_eth_timelock_ordering(terms, now_unix_s=now_unix_s, elapsed_blocks=cov_confs) + except ValidationError as exc: + return PreBtcLockGate(ok=False, reason=f"margin check failed against the REMAINING window: {exc}") + return PreBtcLockGate(ok=True) def _asset_funding_depth(self) -> int | None: @@ -1642,19 +1694,33 @@ async def taker_verify_asset_funding(self, terms: NegotiatedTerms) -> tuple[str, ) return await verify(terms, min_confirmations=self._asset_funding_depth()) - def _assert_eth_timelock_ordering(self, terms: NegotiatedTerms, *, now_unix_s: int | None) -> None: + def _assert_eth_timelock_ordering( + self, terms: NegotiatedTerms, *, now_unix_s: int | None, elapsed_blocks: int = 0 + ) -> None: """ETH cross-clock ordering gate (audit HIGH-1) — wires the previously-orphaned :mod:`pyrxd.gravity.eth_rxd_timelock` bridge into the live pre-fund path. - The HTLC ordering invariant requires the counter-leg (ETH) refund to open strictly - AFTER the asset (RXD) refund, minus the cross-clock margin. For ETH the real deadline - is the ABSOLUTE ``terms.eth_timeout_unix_s`` (a contract immutable), NOT the relative - ``t_btc`` placeholder — so the BTC-shaped ``assert_timelock_margin(t_btc, t_rxd)`` is - the WRONG gate here. We instead project where the RXD CSV refund opens (covenant mines - ~``now + max_covenant_confirm_wait`` then counts ``t_rxd`` blocks) and refuse unless it - lands before ``eth_timeout - margin``. This also closes the now-vs-timeout grief: an - already-expired or near-expiry ``eth_timeout_unix_s`` makes the projected open exceed - the deadline, so the gate refuses to fund. Fail-closed on any missing input. + The HTLC ordering invariant requires the ASSET (RXD) refund to open strictly AFTER the + counter-leg (ETH) refund, plus the cross-clock margin. The maker holds ``p`` and LOCKS the + Radiant leg, so that leg carries the LONGER timeout and the maker claims the SHORTER one + (Herlihy 1801.09515 §1). For ETH the real deadline is the ABSOLUTE + ``terms.eth_timeout_unix_s`` (a contract immutable), NOT the relative ``t_btc`` + placeholder — so the BTC-shaped ``assert_timelock_margin(t_btc, t_rxd)`` is the WRONG gate + here. We project where the RXD CSV refund opens and refuse unless it lands AFTER + ``eth_timeout + margin``. + + THIS DOCSTRING DESCRIBED THE OPPOSITE RELATION until #482 — it said the gate refuses + "unless it lands before ``eth_timeout - margin``", the inverted rule stated with full + confidence one scroll above the code. Prose that asserts an invariant becomes evidence to + the next reader; when it is wrong it is manufactured corroboration, so it is corrected + here rather than left to be cited. + + TWO SEPARATE CHECKS RUN, because ordering does not imply liveness. + :func:`assert_eth_deadline_is_claimable` asks whether the party who must act still can; + :func:`assert_covenant_confirms_before_eth_deadline` asks whether either party can be + robbed if both do. Under the old relation the first fell out of the second's arithmetic + and had no check of its own; under this one it does not follow, so it is asserted + directly. Fail-closed on any missing input. """ policy = self.config.margin_policy if now_unix_s is None: @@ -1666,6 +1732,16 @@ def _assert_eth_timelock_ordering(self, terms: NegotiatedTerms, *, now_unix_s: i "ETH swap requires MarginPolicy.cross_clock_margin and max_covenant_confirm_wait_s " "for the cross-clock ordering gate" ) + # LIVENESS FIRST, and ONLY HERE. A dead or near-dead deadline is refused on its own + # terms rather than as a by-product of the ordering arithmetic (#482). Deliberately NOT + # run in the post-confirm recheck: this floor asks "should the taker fund at all", and + # once it HAS funded the deadline is legitimately closer every second — applying it there + # would refuse honest swaps for the crime of being underway. + assert_eth_deadline_is_claimable( + now_unix_s=now_unix_s, + eth_timeout_unix_s=terms.eth_timeout_unix_s, + margin=policy.cross_clock_margin, + ) assert_covenant_confirms_before_eth_deadline( now_unix_s=now_unix_s, eth_timeout_unix_s=terms.eth_timeout_unix_s, @@ -1681,6 +1757,7 @@ def _assert_eth_timelock_ordering(self, terms: NegotiatedTerms, *, now_unix_s: i # maker holds the refunded asset AND can still claim the counter leg with p. rxd_block_interval_s=_dividing_interval_s(policy), max_covenant_confirm_wait_s=policy.max_covenant_confirm_wait_s, + elapsed_blocks=elapsed_blocks, ) # -- taker funds the counter leg first (the role invariant's step 2) ---------------- diff --git a/src/pyrxd/gravity/swap_state.py b/src/pyrxd/gravity/swap_state.py index 6feef3c5..068a524c 100644 --- a/src/pyrxd/gravity/swap_state.py +++ b/src/pyrxd/gravity/swap_state.py @@ -284,10 +284,11 @@ class NegotiatedTerms: :meth:`to_dict`/:meth:`from_dict` (JSON, never pickle). Timelocks are unit-tagged :class:`Timelock` (BIP68/112). The cross-chain - ordering invariant ``t_btc - t_rxd >= margin`` is checked by the coordinator + ordering invariant ``t_rxd - t_btc >= margin`` is checked by the coordinator (see ``swap_coordinator.assert_timelock_margin``), not here — but the raw - ordering ``t_btc > t_rxd`` in the *same* unit is rejected at construction as a - cheap fail-closed guard. + ordering ``t_rxd > t_btc`` in the *same* unit is rejected at construction as a + cheap fail-closed guard. INVERTED 2026-08-31 (#482): the maker holds ``p`` and LOCKS + the Radiant leg, so that leg carries the LONGER timeout. """ hashlock: bytes # H = SHA256(p), 32 bytes — NEVER p @@ -299,8 +300,8 @@ class NegotiatedTerms: # check that establishes it. This is what makes #505 — the funding gate matching an FT # token count against the carrier's photon value — a mypy error instead of a review catch. radiant_amount: PhotonValue | TokenUnits - t_btc: Timelock # BTC refund timelock (the LONGER leg) - t_rxd: Timelock # Radiant refund timelock (the SHORTER leg) + t_btc: Timelock # BTC refund timelock (the SHORTER leg — the maker CLAIMS this one) + t_rxd: Timelock # Radiant refund timelock (the LONGER leg — the maker LOCKED this one) asset_variant: str # "rxd" | "ft" | "nft" # Radiant asset binding. genesis_ref is the GENESIS outpoint ref (FT/NFT); # empty for plain RXD. taker/maker dest hashes pin the claim/refund holder. @@ -420,10 +421,10 @@ def __post_init__(self) -> None: object.__setattr__(self, _name, _val) # Cheap same-unit ordering guard (the full margin check is fail-closed in # the coordinator and handles cross-unit normalisation). - if self.t_btc.unit is self.t_rxd.unit and self.t_btc.value <= self.t_rxd.value: + if self.t_btc.unit is self.t_rxd.unit and self.t_rxd.value <= self.t_btc.value: raise ValidationError( - "invariant MAKER_SECRET_TAKER_LOCKS_BTC_FIRST requires t_btc > t_rxd " - f"(got t_btc={self.t_btc.value} <= t_rxd={self.t_rxd.value} {self.t_btc.unit.value})" + "invariant MAKER_SECRET_TAKER_LOCKS_BTC_FIRST requires t_rxd > t_btc " + f"(got t_rxd={self.t_rxd.value} <= t_btc={self.t_btc.value} {self.t_btc.unit.value})" ) def to_dict(self) -> dict[str, Any]: diff --git a/tests/test_btc_htlc_leg.py b/tests/test_btc_htlc_leg.py index a5fc9376..6201c9e4 100644 --- a/tests/test_btc_htlc_leg.py +++ b/tests/test_btc_htlc_leg.py @@ -44,8 +44,8 @@ def _terms(*, maker_kp: BtcKeypair, taker_kp: BtcKeypair, hashlock: bytes | None hashlock=hashlock, btc_sats=100_000, radiant_amount=1_000, - t_btc=t.Timelock(144, t.TimeUnit.BLOCKS), - t_rxd=t.Timelock(72, t.TimeUnit.BLOCKS), + t_btc=t.Timelock(72, t.TimeUnit.BLOCKS), + t_rxd=t.Timelock(144, t.TimeUnit.BLOCKS), asset_variant="rxd", genesis_ref=b"", taker_dest_hash=b"\x11" * 32, @@ -502,13 +502,16 @@ async def test_refund_rejects_premature_csv(): a clear "needs N, has M" message and NOT broadcast — the leg refuses a non-final refund rather than relying on node rejection under deadline pressure.""" taker, maker = generate_keypair("bcrt"), generate_keypair("bcrt") - terms = _terms(maker_kp=maker, taker_kp=taker) # t_btc = 144 → mature at funding confs >= 144 + # t_btc = 72 since #482 swapped the pair (the maker LOCKS the longer Radiant leg), so maturity + # is at 72 confirmations and the "one block short" case is 71. Left at 143 this test passes 143 + # confirmations against a 72-block CSV — long mature — and asserts a refusal that cannot happen. + terms = _terms(maker_kp=maker, taker_kp=taker) bc = FakeBroadcaster() - leg = _leg(taker_kp=taker, maker_kp=maker, broadcaster=bc, reader=FakeFundingReader(claim_confs=143)) + leg = _leg(taker_kp=taker, maker_kp=maker, broadcaster=bc, reader=FakeFundingReader(claim_confs=71)) htlc = leg._htlc(terms) locator = htlc.with_funding(t.BtcOutpoint("cd" * 32, 0), terms.btc_sats) # NetworkError (transient/retryable), consistent with the covenant leg — not a fatal ValidationError. - with pytest.raises(NetworkError, match="not yet mature: needs 144 confirmations, has 143"): + with pytest.raises(NetworkError, match="not yet mature: needs 72 confirmations, has 71"): await leg.refund(locator, terms.t_btc) assert bc.raw_seen == [], "no non-final refund may be broadcast before CSV maturity" diff --git a/tests/test_btc_maker_counter_funding_adversarial.py b/tests/test_btc_maker_counter_funding_adversarial.py index 1297d547..bd2ab210 100644 --- a/tests/test_btc_maker_counter_funding_adversarial.py +++ b/tests/test_btc_maker_counter_funding_adversarial.py @@ -114,8 +114,8 @@ def _terms(*, maker_kp: BtcKeypair, taker_kp: BtcKeypair) -> NegotiatedTerms: hashlock=hashlib.sha256(os.urandom(32)).digest(), btc_sats=_BTC_SATS, radiant_amount=1_000, - t_btc=t.Timelock(144, t.TimeUnit.BLOCKS), - t_rxd=t.Timelock(72, t.TimeUnit.BLOCKS), + t_btc=t.Timelock(72, t.TimeUnit.BLOCKS), + t_rxd=t.Timelock(144, t.TimeUnit.BLOCKS), asset_variant="rxd", genesis_ref=b"", taker_dest_hash=b"\x11" * 32, diff --git a/tests/test_btc_two_host_self_check.py b/tests/test_btc_two_host_self_check.py index 3d8919a1..8ce6ed23 100644 --- a/tests/test_btc_two_host_self_check.py +++ b/tests/test_btc_two_host_self_check.py @@ -64,8 +64,9 @@ def test_terms_from_public_is_deterministic_and_btc_counterchain(): kw = dict( hashlock=h, btc_sats=100_000, - t_rxd_blocks=20, - t_btc_blocks=60, + # INVERTED (#482): the maker locks the Radiant leg, so it carries the LONGER timeout. + t_rxd_blocks=60, + t_btc_blocks=20, taker_pkh=b"\x11" * 20, maker_pkh=b"\x22" * 20, btc_claim_xonly=b"\x33" * 32, diff --git a/tests/test_eoa_recipient_policy.py b/tests/test_eoa_recipient_policy.py index 8bead54e..4e5d6cff 100644 --- a/tests/test_eoa_recipient_policy.py +++ b/tests/test_eoa_recipient_policy.py @@ -254,8 +254,8 @@ def _terms(token_address: str): hashlock=hashlib.sha256(b"x").digest(), btc_sats=100_000, radiant_amount=1_000, - t_btc=t.Timelock(144, t.TimeUnit.BLOCKS), - t_rxd=t.Timelock(72, t.TimeUnit.BLOCKS), + t_btc=t.Timelock(72, t.TimeUnit.BLOCKS), + t_rxd=t.Timelock(144, t.TimeUnit.BLOCKS), asset_variant="rxd", genesis_ref=b"", taker_dest_hash=b"\x11" * 32, diff --git a/tests/test_eth_rxd_timelock.py b/tests/test_eth_rxd_timelock.py index 70714080..06ac09a2 100644 --- a/tests/test_eth_rxd_timelock.py +++ b/tests/test_eth_rxd_timelock.py @@ -62,9 +62,11 @@ def test_stall_tolerance_defaults_zero_and_is_additive(): ) -def test_stall_tolerance_shrinks_the_rxd_budget(): - # Same inputs, but the stall tolerance eats into the RXD window: the refund must open - # EARLIER (fewer blocks) so the taker still has its stall-tolerant claim window. +def test_stall_tolerance_GROWS_the_rxd_window(): + # INVERTED WITH THE RELATION (#482). The margin used to be subtracted from the window: the RXD + # refund had to open before the ETH deadline, so every second of stall budget bought fewer + # blocks. It is added now — the refund must open AFTER the deadline PLUS the margin — so a + # stall tolerance makes the maker lock its asset LONGER, which is who should pay for it. base = CrossClockMargin( eth_reorg_finality_s=768, rxd_claim_burial_s=600, rxd_confirm_slack_s=300, rounding_slack_s=300 ) @@ -78,19 +80,21 @@ def test_stall_tolerance_shrinks_the_rxd_budget(): kw = dict(eth_timeout_unix_s=100_000, expected_rxd_lock_time_unix_s=0, rxd_block_interval_s=36.0) t_base = eth_absolute_to_rxd_relative_blocks(margin=base, **kw) t_stall = eth_absolute_to_rxd_relative_blocks(margin=with_stall, **kw) - assert t_stall.value < t_base.value # stall budget strictly shrinks the RXD window + assert t_stall.value > t_base.value # stall budget strictly LENGTHENS the maker's lock def test_stall_tolerance_can_force_failclosed(): - # A stall tolerance larger than the remaining ETH→RXD gap leaves no budget → refuse to lock. + # Still fail-closed, at the OPPOSITE end (#482). A huge stall tolerance used to leave NO budget; + # it now demands a window past the BIP68 16-bit cap, which is equally unlockable. The refusal + # survived the inversion; the reason for it did not, so the match string has to move with it. huge_stall = CrossClockMargin( eth_reorg_finality_s=768, rxd_claim_burial_s=600, rxd_confirm_slack_s=300, rounding_slack_s=300, - eth_finality_stall_tolerance_s=100_000, + eth_finality_stall_tolerance_s=100_000_000, ) - with pytest.raises(ValidationError, match="no RXD timelock budget"): + with pytest.raises(ValidationError, match="BIP68"): eth_absolute_to_rxd_relative_blocks( eth_timeout_unix_s=10_000, expected_rxd_lock_time_unix_s=0, @@ -112,7 +116,9 @@ def test_fast_interval_yields_more_blocks_than_mean(): def test_converter_concrete_floor_and_unit(): - # budget = 100000 - 1800(margin) - 0 = 98200s ; /600 = 163.66 -> floor 163 blocks + # budget = 100000 + 1800(margin) - 0 = 101800s ; /600 = 169.67 -> ceil 170 blocks. The margin + # is ADDED and the rounding is UP (#482): the window must COVER the budget, where it used to + # have to fit inside it. m = _margin() # total 1800 t = eth_absolute_to_rxd_relative_blocks( eth_timeout_unix_s=100_000, @@ -121,15 +127,18 @@ def test_converter_concrete_floor_and_unit(): rxd_block_interval_s=600.0, floor_blocks=12, ) - assert t == Timelock(163, TimeUnit.BLOCKS) - assert t.value * 600.0 <= (100_000 - 1800) # floor never overshoots the budget + assert t == Timelock(170, TimeUnit.BLOCKS) + assert t.value * 600.0 >= (100_000 + 1800) # the window never UNDERshoots the budget def test_converter_failclosed_no_budget(): - with pytest.raises(ValidationError, match="no RXD timelock budget"): + # "No budget" now means the lock time is so far PAST the deadline+margin that the required + # window is non-positive — the swap is already over. Equal timestamps no longer produce it: + # with the margin added, lock-at-deadline still needs `margin` seconds of window (#482). + with pytest.raises(ValidationError, match="no RXD timelock budget|below safety floor"): eth_absolute_to_rxd_relative_blocks( eth_timeout_unix_s=1000, - expected_rxd_lock_time_unix_s=1000, + expected_rxd_lock_time_unix_s=100_000, margin=_margin(), rxd_block_interval_s=600.0, ) @@ -191,11 +200,11 @@ def _analytic_start(budget: float, interval: float) -> int: counterexamples into the committed `tests/.hypothesis-corpus/`, the first CI run to find one would have turned the suite permanently red. - What remains true of this value: the sizer starts here and may step down at most - `_SIZER_GATE_STEPS` (3) times, so it bounds the answer from above and (minus 2 — the third - refusal raises instead of returning) from below. + What remains true of this value: the sizer starts here and may step UP at most + `_SIZER_GATE_STEPS` (3) times (#482 inverted the search), so it bounds the answer from below + and (plus 2 — the third refusal raises instead of returning) from above. """ - return (math.ceil(budget / interval) - 1) if budget > 0 else 0 + return max(0, math.ceil(budget / interval) - 1) if budget > 0 else 0 def _gate_accepts_at_lock_time(t_blocks: int, *, eth_timeout: int, rxd_lock: int, margin, interval: float) -> bool: @@ -241,7 +250,7 @@ def test_converter_invariants_or_failclosed(eth_timeout, rxd_lock, m1, m2, m3, m margin = CrossClockMargin( eth_reorg_finality_s=m1, rxd_claim_burial_s=m2, rxd_confirm_slack_s=m3, rounding_slack_s=m4 ) - budget = eth_timeout - margin.total_s() - rxd_lock + budget = eth_timeout + margin.total_s() - rxd_lock start = _analytic_start(budget, interval) def accepts(t_blocks: int) -> bool: @@ -265,7 +274,7 @@ def accepts(t_blocks: int) -> bool: # projection `ceil(t*I)` is non-decreasing), so "the gate refuses floor_blocks" is # equivalent to "every value the floor allows is refused". If NONE of these hold, the # refusal turned away honest work — the sizer/gate contract has genuinely diverged. - assert budget <= 0 or start < floor_blocks or start > _CAP or not accepts(floor_blocks) + assert budget <= 0 or start < floor_blocks or start > _CAP or not accepts(_CAP) return # success → invariants hold assert t.unit is TimeUnit.BLOCKS @@ -275,12 +284,20 @@ def accepts(t_blocks: int) -> bool: # went stale both times the sizer changed; asking the gate cannot. # (a) what it emitted, the gate accepts (honest path); assert accepts(t.value), f"the sizer emitted t_rxd={t.value} and its own gate refuses it" - # (b) one more block is refused — the emitted value is the LARGEST the gate accepts, so no - # fix for (a) may quietly shrink the taker's claim window instead; - assert not accepts(t.value + 1), f"the gate also accepts t_rxd={t.value + 1}: a block of claim window given away" - # (c) the step-down never drifts: the answer stays within the sizer's documented reach of - # the analytic value (start, or up to 2 below it — the third refusal raises instead). - assert start - 2 <= t.value <= start, f"t.value={t.value} outside [{start - 2}, {start}]" + # (b) one FEWER block is refused — the emitted value is the SMALLEST the gate accepts, so no + # fix for (a) may quietly lengthen the maker's lock instead (#482 flipped which side of + # the boundary is the give-away); + # ...but ONLY where the GATE is the binding constraint. When the safety floor is what + # stops the search, the emitted value is the floor and the gate legitimately accepts less + # — asserting otherwise demands the sizer return a value below its own floor. Found by + # hypothesis at floor_blocks=2, emitted 2, gate accepting 1. + if t.value > floor_blocks: + assert not accepts(t.value - 1), ( + f"the gate also accepts t_rxd={t.value - 1}: the maker's lock is a block too long" + ) + # (c) the step-UP never drifts: the answer stays within the sizer's documented reach of + # the analytic value (start, or up to 2 above it — the third refusal raises instead). + assert start <= t.value <= start + 2, f"t.value={t.value} outside [{start}, {start + 2}]" # "the sized value is conservative — never overshoots the budget", to within floating-point # noise. `ceil(x) - 1` is at most `floor(x)`, so it is conservative wherever floor was. # @@ -303,7 +320,17 @@ def accepts(t_blocks: int) -> bool: # so the invariant still fails loudly if the direction of the rounding ever flips. # Sub-block remainder is covered by `margin.rounding_slack_s` by design (see the # `eth_absolute_to_rxd_relative_blocks` docstring). - assert t.value * interval <= budget + 8 * math.ulp(float(budget)) + # UNDERSHOOT is the direction to bound now (#482): the window must COVER the budget, where it + # used to have to fit inside it. + # + # AND THE CEIL IS LOAD-BEARING ON THIS SIDE. The gate compares `ceil(t * interval)`, not the + # raw product. Under the old inequality the un-rounded product was a conservative proxy — it is + # never larger — so dropping the ceil here was safe and this line did not have one. Reversed, + # the same omission understates the window by up to a second and the assertion fails on + # CORRECT output: at eth_timeout=44, interval=1.5 the sizer returns 29, whose 43.5s projects to + # ceil = 44 and exactly meets a 44s budget. That is the pinned @example above, and it is why + # the model has to mirror the gate's arithmetic rather than approximate it. + assert math.ceil(t.value * interval) >= budget - 8 * math.ulp(float(budget)) # ─────────────────────────────────────────────────── funding-confirm gate (D2) ── @@ -312,10 +339,10 @@ def accepts(t_blocks: int) -> bool: def test_gate_passes_with_margin_left(): m = _margin(60, 60, 60, 60) # total 240 t = Timelock(10, TimeUnit.BLOCKS) - # projected = 1000 + 0 + ceil(10*600) = 7000 ; deadline = 7241 - 240 = 7001 > 7000 → OK + # earliest open = 1000 + ceil(10*600) = 7000 ; required = 6700 + 240 = 6940 <= 7000 → OK assert_covenant_confirms_before_eth_deadline( now_unix_s=1000, - eth_timeout_unix_s=7241, + eth_timeout_unix_s=6700, margin=m, t_rxd=t, rxd_block_interval_s=600.0, @@ -323,14 +350,14 @@ def test_gate_passes_with_margin_left(): ) -def test_gate_fails_when_covenant_confirms_too_late(): +def test_gate_fails_when_the_refund_would_open_before_the_deadline(): m = _margin(60, 60, 60, 60) # total 240 t = Timelock(10, TimeUnit.BLOCKS) - # projected 7000 ; deadline = 7240 - 240 = 7000 ; 7000 >= 7000 → raise - with pytest.raises(ValidationError, match="confirm too late"): + # earliest open = 7000 ; required = 6761 + 240 = 7001 > 7000 → raise, by exactly one second + with pytest.raises(ValidationError, match="open too EARLY"): assert_covenant_confirms_before_eth_deadline( now_unix_s=1000, - eth_timeout_unix_s=7240, + eth_timeout_unix_s=6761, margin=m, t_rxd=t, rxd_block_interval_s=600.0, @@ -338,19 +365,24 @@ def test_gate_fails_when_covenant_confirms_too_late(): ) -def test_gate_failclosed_on_confirm_wait_squeeze(): +def test_the_confirm_wait_no_longer_moves_the_verdict(): + """THIS TEST CHANGED PURPOSE, deliberately, and says so rather than being deleted (#482). + + It asserted that a `max_covenant_confirm_wait_s` budget pushed the projected open past the + deadline and forced a refusal. That term is gone from the arithmetic: the floor is now taken at + the EARLIEST plausible confirm, and adding a wait would assume a LATE one, which is the + optimistic direction under this relation. + + The parameter is still validated and still meaningful to callers as an operational bound, so + the thing worth pinning is that it is INERT here — otherwise a future change could quietly + reintroduce it into the arithmetic and reopen exactly the window it used to close. + """ m = _margin(60, 60, 60, 60) t = Timelock(10, TimeUnit.BLOCKS) - # pre-lock projection with a confirm-wait budget pushes the open past the deadline - with pytest.raises(ValidationError, match="confirm too late"): - assert_covenant_confirms_before_eth_deadline( - now_unix_s=1000, - eth_timeout_unix_s=7241, - margin=m, - t_rxd=t, - rxd_block_interval_s=600.0, - max_covenant_confirm_wait_s=600, - ) + kw = dict(now_unix_s=1000, eth_timeout_unix_s=6700, margin=m, t_rxd=t, rxd_block_interval_s=600.0) + assert_covenant_confirms_before_eth_deadline(max_covenant_confirm_wait_s=0, **kw) + assert_covenant_confirms_before_eth_deadline(max_covenant_confirm_wait_s=600, **kw) + assert_covenant_confirms_before_eth_deadline(max_covenant_confirm_wait_s=100_000, **kw) def test_gate_requires_blocks_timelock(): @@ -426,8 +458,10 @@ def test_a_SHORTER_window_is_permitted(self) -> None: class TestTheCovenantGateIsPunctualityNotASlowChainDefence: """The gate LOOKS like a wall-clock projection of the RXD refund and was documented as one. - It is not: `rxd_block_interval_s` cancels, because sizing computes `floor(budget/interval)` - and the gate computes `ceil(t_rxd * interval)` — inverse operations. + It is not: `rxd_block_interval_s` cancels, because sizing computes `ceil(budget/interval)` + and the gate computes `ceil(t_rxd * interval)` — inverse operations. (Sizing was + `floor(budget/interval)` before #482 inverted the bound; the cancellation survives, which is + why every test in this class except the direction of the refusal still holds.) These tests exist to stop the obvious "fix". Splitting the interval into fast and slow tails and passing the slow one here was attempted; it refuses every configuration at every budget, @@ -484,10 +518,30 @@ def test_the_verdict_does_not_depend_on_the_block_interval_at_all(self, lock_del "only precisely because it cannot see the block rate." ) - def test_it_refuses_a_LATE_covenant_confirmation(self) -> None: - """The property it really has: confirm past the time the sizing assumed and it refuses.""" - assert not self._verdict(229.0, lock_delay=3_600, wait=7_200) + def test_it_refuses_an_EARLY_covenant_confirmation(self) -> None: + """WHICH DIRECTION IS DANGEROUS FLIPPED WITH THE RELATION (#482), and this pair is the + clearest statement of it in the suite. + + The gate used to refuse a confirmation LATER than sizing assumed, because a late mine + pushed the RXD refund past the ETH deadline it had to precede. The refund must now OUTLAST + that deadline, so lateness only costs the maker lock time. What robs the taker is an EARLY + confirmation: `t_rxd` counts from mining, so mining sooner opens the refund sooner, and the + maker can refund its Radiant leg while still holding `p` for the ETH leg. + + `_verdict` checks the gate at `now` against a `t_rxd` sized for a lock at + `now + lock_delay`, so a positive `lock_delay` IS the early-confirmation case. + """ + assert not self._verdict(229.0, lock_delay=3_600, wait=0) def test_it_accepts_a_PUNCTUAL_covenant_confirmation(self) -> None: """Paired honest path — the gate must not refuse a covenant that confirms on time.""" - assert self._verdict(229.0, lock_delay=3_600, wait=300) + assert self._verdict(229.0, lock_delay=0, wait=0) + + def test_it_accepts_a_LATE_covenant_confirmation_which_is_now_the_SAFE_direction(self) -> None: + """The refusal this class used to assert, now required to PASS. + + A guard that refuses valid work is a bug, and this one would refuse the maker for being + slow — on the swap's most common non-adversarial deviation, where the cost falls on the + maker alone and the taker is strictly better off. + """ + assert self._verdict(229.0, lock_delay=-3_600, wait=0) diff --git a/tests/test_eth_swap_run_timelock_bounds.py b/tests/test_eth_swap_run_timelock_bounds.py index 6b8fd5d3..f58a91cd 100644 --- a/tests/test_eth_swap_run_timelock_bounds.py +++ b/tests/test_eth_swap_run_timelock_bounds.py @@ -104,12 +104,16 @@ def _sized_and_gate(fast: float, eth_timeout_s: int, wait: int): def accepts(t: int) -> bool: try: assert_covenant_confirms_before_eth_deadline( - now_unix_s=now, + # Anchored at the LOCK time with a zero wait, matching the sizer above. The gate + # used to ADD the wait to `now`, making the two the same instant; #482 removed + # that term (it assumes a LATE confirm, the optimistic direction), so the anchors + # have to be written alike to stay alike. + now_unix_s=now + wait, eth_timeout_unix_s=now + eth_timeout_s, margin=margin, t_rxd=bt.Timelock(t, bt.TimeUnit.BLOCKS), rxd_block_interval_s=fast, - max_covenant_confirm_wait_s=wait, + max_covenant_confirm_wait_s=0, ) return True except Exception: @@ -135,13 +139,16 @@ class TestNonIntegerIntervalsAreSizedCorrectly: @pytest.mark.parametrize("fast,eth_timeout_s,wait", _GATE_DISAGREEMENT_ROWS) def test_the_rows_where_the_OLD_arithmetic_was_refused(self, fast: float, eth_timeout_s: int, wait: int) -> None: sized, accepts = _sized_and_gate(fast, eth_timeout_s, wait) - naive = math.ceil((eth_timeout_s - 7068 - wait) / fast) - 1 + # The margin is ADDED and there is no `- 1` since #482 — this reproduces the OLD + # arithmetic's mistake against the NEW budget, which is what makes the row a defect + # demonstration rather than an arbitrary number. + naive = math.ceil((eth_timeout_s + 7068 - wait) / fast) - 1 assert not accepts(naive), ( f"this row no longer reproduces the defect: the naive value {naive} is accepted at " f"fast={fast}, so it cannot demonstrate anything" ) assert accepts(sized), f"gate refused the sizer's own output {sized} at {fast}s" - assert sized == naive - 1, f"expected the sizer to step down from {naive}, got {sized}" + assert sized == naive + 1, f"expected the sizer to step UP from {naive}, got {sized}" @pytest.mark.parametrize( "fast", [9.0, 20.0, 36.0, 43.0, 60.0, 120.0, 300.0, 36.2, 36.4, 36.5, 36.7, 43.3, 60.5, 331.7, 41.618] @@ -153,9 +160,10 @@ def test_the_gate_accepts_the_sized_value_across_the_grid(self, fast: float, eth for wait in (0, 300, 600, 900, 1200): sized, accepts = _sized_and_gate(fast, eth_timeout_s, wait) assert accepts(sized), f"gate refused sized={sized} at fast={fast}, eth={eth_timeout_s}, wait={wait}" - assert not accepts(sized + 1), ( - f"t_rxd={sized + 1} also accepted at fast={fast}, eth={eth_timeout_s}, wait={wait} " - "— a block of the taker's claim window given away" + assert not accepts(sized - 1), ( + f"t_rxd={sized - 1} also accepted at fast={fast}, eth={eth_timeout_s}, wait={wait} " + "— the maker's asset locked a block longer than the deadline requires (#482 moved " + "the give-away from the taker's window to the maker's lock)" ) @@ -174,60 +182,117 @@ def test_the_bound_shrinks_as_the_deadline_approaches(self, runner) -> None: "bound is dividing the original duration, not what remains" ) - def test_a_value_valid_at_the_start_is_refused_once_too_little_remains(self, runner) -> None: + def test_a_value_valid_at_the_start_STAYS_valid_as_the_deadline_approaches(self, runner) -> None: + """THE RESUME RISK REVERSED WITH #482, and asserting the old direction would now demand a + refusal that would be a bug. + + The bound was a cap: as the deadline approached, less remained, the cap fell, and a t_rxd + chosen at the start could drift above it — so a resume had to re-check and refuse. It is a + floor now, and the floor FALLS as the deadline approaches (`remaining + margin` shrinks). + A window long enough at the start is therefore still long enough later, always, and + refusing it would strand a swap the check exists to save. + + What a resume must still catch is a t_rxd too SHORT for what remains, asserted below. + """ at_start = runner._derive_t_rxd_blocks(_ns()) - with pytest.raises(SystemExit): - runner._assert_t_rxd_opens_before_the_eth_deadline(_ns(t_rxd_blocks=at_start), remaining_s=43_200) + runner._assert_t_rxd_outlasts_the_eth_deadline(_ns(t_rxd_blocks=at_start), remaining_s=43_200) + + def test_a_value_too_SHORT_for_what_remains_is_still_refused_on_resume(self, runner) -> None: + """The paired refusal, so the test above cannot be satisfied by a bound that accepts + everything on a resume.""" + remaining = 43_200 + derived = runner._derive_t_rxd_blocks(_ns(), remaining_s=remaining) + with pytest.raises(SystemExit, match="too SHORT"): + runner._assert_t_rxd_outlasts_the_eth_deadline(_ns(t_rxd_blocks=derived - 1), remaining_s=remaining) def test_the_honest_resume_still_passes(self, runner) -> None: """Paired, because a bound that refuses every resume strands funds it was meant to save.""" remaining = 43_200 derived = runner._derive_t_rxd_blocks(_ns(), remaining_s=remaining) - runner._assert_t_rxd_opens_before_the_eth_deadline(_ns(t_rxd_blocks=derived), remaining_s=remaining) + runner._assert_t_rxd_outlasts_the_eth_deadline(_ns(t_rxd_blocks=derived), remaining_s=remaining) runner._assert_t_rxd_bounds_the_vulnerable_window(_ns(t_rxd_blocks=derived), remaining_s=remaining) class TestAnImpossibleDeadlineNamesTheRightArgument: - """L-13. A 3 h deadline refused t_rxd=86 as "too SHORT" while advising the operator to omit the - flag and let it derive — which derives 86. Self-contradictory, and it pointed at the one - argument that cannot help. The deadline has to hold roughly two margins plus the reserve; below - that no t_rxd exists and the honest message says so. + """L-13, WITH ITS DIRECTION INVERTED BY #482 — which is the whole finding here. + + The original: a 3 h deadline refused t_rxd=86 as "too SHORT" while advising the operator to + omit the flag and let it derive, which derives 86. Self-contradictory, and it pointed at the + one argument that could not help. The rule was "the deadline must hold roughly two margins plus + the reserve", because bound B capped t_rxd from above and the floors pushed from below. + + Under the corrected relation bound B is a FLOOR: `t_rxd >= ceil((budget + margin) / fast)`, + which RISES with the deadline, and the only ceiling left is the BIP68 field width. So: + + - a SHORT deadline is always satisfiable. 3 h yields a feasible (497, 65535) — the exact + case this class was written about is no longer a failure at all. + - what cannot be satisfied is a deadline too FAR: past roughly 23 days no 16-bit relative + CSV reaches it. + + The remedy inverts with it. Asking for more time was the fix; it is now the cause, and a search + that walked upward would have advised an operator to lengthen a deadline already too long. """ - def test_a_too_short_deadline_refuses_before_any_t_rxd_advice(self, runner) -> None: + #: Past the BIP68 reach at the default 36s fast tail. Measured, not estimated: 2_000_000 still + #: yields (55752, 65535) and 2_400_000 yields None. + _TOO_FAR_S = 2_400_000 + + def test_a_too_FAR_deadline_refuses_before_any_t_rxd_advice(self, runner) -> None: with pytest.raises(SystemExit, match="cannot hold the timelock at all"): - runner._assert_the_eth_deadline_can_hold_the_margins(_ns(eth_timeout_s=10_800)) + runner._assert_the_eth_deadline_can_hold_the_margins(_ns(eth_timeout_s=self._TOO_FAR_S)) + + def test_the_deadline_that_USED_to_be_impossible_is_now_fine(self, runner) -> None: + """The paired honest path, and the strongest single statement of what #482 changed. + + 3 h is the deadline from the live incident this class records. It was infeasible, the + refusal contradicted itself, and two funded covenants were burned on it. It now has a + feasible set and the whole parse-time pipeline accepts the derived value. + """ + args = _ns(eth_timeout_s=10_800) + runner._assert_the_eth_deadline_can_hold_the_margins(args) + args.t_rxd_blocks = runner._recommended_t_rxd_blocks(args) + _run_the_whole_parse_time_pipeline(runner, args) def test_it_names_eth_timeout_and_not_t_rxd(self, runner) -> None: with pytest.raises(SystemExit) as exc: - runner._assert_the_eth_deadline_can_hold_the_margins(_ns(eth_timeout_s=10_800)) + runner._assert_the_eth_deadline_can_hold_the_margins(_ns(eth_timeout_s=self._TOO_FAR_S)) msg = str(exc.value) assert "--eth-timeout-s" in msg assert "--t-rxd-blocks" not in msg.split("no --t-rxd-blocks satisfies all three bounds")[-1], ( "the remedy must not point at --t-rxd-blocks; no value of it can help" ) - def test_the_minimum_it_advertises_actually_WORKS(self, runner) -> None: - """The advice has to be true. A refusal naming a minimum that is itself refused is how the + def test_the_maximum_it_advertises_actually_WORKS(self, runner) -> None: + """The advice has to be true. A refusal naming a bound that is itself refused is how the live run burned two funded covenants.""" with pytest.raises(SystemExit) as exc: - runner._assert_the_eth_deadline_can_hold_the_margins(_ns(eth_timeout_s=10_800)) - minimum = int(str(exc.value).split("minimum: --eth-timeout-s ")[1].split()[0]) - runner._assert_the_eth_deadline_can_hold_the_margins(_ns(eth_timeout_s=minimum)) - derived = runner._derive_t_rxd_blocks(_ns(eth_timeout_s=minimum)) - args = _ns(eth_timeout_s=minimum, t_rxd_blocks=derived) + runner._assert_the_eth_deadline_can_hold_the_margins(_ns(eth_timeout_s=self._TOO_FAR_S)) + msg = str(exc.value) + # NOT a skip. The search starts from the analytic BIP68 boundary rather than walking down + # from the requested value, so a workable maximum always exists at fast > 0 — and a skip + # here would hide the search failing to find one, which is the defect this test is for. + assert "maximum: --eth-timeout-s " in msg, f"the refusal named no workable maximum:\n{msg}" + maximum = int(msg.split("maximum: --eth-timeout-s ")[1].split()[0]) + runner._assert_the_eth_deadline_can_hold_the_margins(_ns(eth_timeout_s=maximum)) + derived = runner._derive_t_rxd_blocks(_ns(eth_timeout_s=maximum)) + args = _ns(eth_timeout_s=maximum, t_rxd_blocks=derived) runner._assert_t_rxd_covers_the_takers_wait(args) - runner._assert_t_rxd_opens_before_the_eth_deadline(args) + runner._assert_t_rxd_outlasts_the_eth_deadline(args) runner._assert_t_rxd_bounds_the_vulnerable_window(args) - def test_one_second_below_the_minimum_is_still_refused(self, runner) -> None: - """Pins the boundary rather than the direction, so a minimum that drifts up to be safely + def test_one_second_ABOVE_the_maximum_is_still_refused(self, runner) -> None: + """Pins the boundary rather than the direction, so a maximum that drifts down to be safely wrong fails here.""" with pytest.raises(SystemExit) as exc: - runner._assert_the_eth_deadline_can_hold_the_margins(_ns(eth_timeout_s=10_800)) - minimum = int(str(exc.value).split("minimum: --eth-timeout-s ")[1].split()[0]) + runner._assert_the_eth_deadline_can_hold_the_margins(_ns(eth_timeout_s=self._TOO_FAR_S)) + msg = str(exc.value) + # NOT a skip. The search starts from the analytic BIP68 boundary rather than walking down + # from the requested value, so a workable maximum always exists at fast > 0 — and a skip + # here would hide the search failing to find one, which is the defect this test is for. + assert "maximum: --eth-timeout-s " in msg, f"the refusal named no workable maximum:\n{msg}" + maximum = int(msg.split("maximum: --eth-timeout-s ")[1].split()[0]) with pytest.raises(SystemExit, match="cannot hold"): - runner._assert_the_eth_deadline_can_hold_the_margins(_ns(eth_timeout_s=minimum - 1)) + runner._assert_the_eth_deadline_can_hold_the_margins(_ns(eth_timeout_s=maximum + 1)) def test_a_normal_24h_deadline_is_untouched(self, runner) -> None: runner._assert_the_eth_deadline_can_hold_the_margins(_ns()) @@ -264,9 +329,11 @@ def test_the_derivation_matches_what_the_bounds_demand(runner) -> None: runner._assert_the_eth_deadline_can_hold_the_margins(args) args.t_rxd_blocks = runner._derive_t_rxd_blocks(args) runner._assert_t_rxd_covers_the_takers_wait(args) - runner._assert_t_rxd_opens_before_the_eth_deadline(args) + runner._assert_t_rxd_outlasts_the_eth_deadline(args) runner._assert_t_rxd_bounds_the_vulnerable_window(args) - assert args.t_rxd_blocks < math.ceil(eth_timeout_s / 36.0) + # The derived window now EXCEEDS the raw deadline in blocks — it has to cover the deadline + # PLUS the margin, where it used to have to fit inside the deadline MINUS it (#482). + assert args.t_rxd_blocks > math.ceil(eth_timeout_s / 36.0) #: (fast_interval_s, eth_timeout_s, stall_s, confirm_wait_s) rows on which the PRE-FIX deadline @@ -325,91 +392,70 @@ class TestTheFeasibilityCheckIsTheSetAndNotASum: """ @pytest.mark.parametrize("fast,eth_timeout_s,stall,wait", _EMPTY_FEASIBLE_SET_ROWS) - def test_the_row_still_reproduces_an_empty_set_under_the_old_sum( + def test_every_historically_EMPTY_row_now_has_a_feasible_set( self, runner, fast: float, eth_timeout_s: int, stall: int, wait: int ) -> None: - """Guards the guard: a changed margin default must break this loudly, not silently.""" + """THE DEFECT CLASS IS GONE BY CONSTRUCTION, NOT BY A BETTER GUARD (#482). + + These rows are the brute-forced parameter sets on which the feasible set was EMPTY: bound + B capped t_rxd from above, bound C floored it from below, the two were algebraically the + same integer and rounded one apart at a fractional fast tail, and the operator was refused + a value while being advised it. Two funded covenants were burned on that. + + Inverting the relation moved bound B to the FLOOR side. Every bound is a floor now and the + only ceiling is the BIP68 field width, so `[max(floors), 65535]` cannot be emptied by two + bounds disagreeing about a rounding — there is nothing left to disagree. Each of these rows + now yields a healthy range. + + Asserting this on the ORIGINAL rows is the point: it is evidence the specific inputs that + broke are fixed, which a fresh property test over new parameters would not give. If a + future change reintroduces a ceiling below the cap, these go red first. + """ args = _ns( rxd_block_interval_fast_s=fast, eth_timeout_s=eth_timeout_s, eth_finality_stall_tolerance_s=stall, max_covenant_confirm_wait_s=wait, ) - assert _pre_fix_loose_sum_passes(runner, args), ( - f"row fast={fast} eth={eth_timeout_s} no longer passes the pre-fix loose sum, so it " - "cannot demonstrate the defect" - ) - assert runner._t_rxd_feasible_range(args) is None, ( - f"row fast={fast} eth={eth_timeout_s} no longer has an EMPTY feasible set — it cannot " - "demonstrate the defect any more; find a replacement row rather than deleting this" + rng = runner._t_rxd_feasible_range(args) + assert rng is not None, f"row fast={fast} eth={eth_timeout_s} still has an empty feasible set" + lo, hi = rng + assert lo <= hi + assert hi == runner._T_RXD_BIP68_MAX_BLOCKS, ( + f"the feasible set is capped at {hi}, below the BIP68 maximum. A ceiling other than the " + "field width has come back, and with it the empty-set class this row records." ) @pytest.mark.parametrize("fast,eth_timeout_s,stall,wait", _EMPTY_FEASIBLE_SET_ROWS) - def test_the_guard_refuses_instead_of_contradicting_itself( + def test_the_pipeline_accepts_the_derived_value_on_every_such_row( self, runner, fast: float, eth_timeout_s: int, stall: int, wait: int ) -> None: - args = _ns( - rxd_block_interval_fast_s=fast, - eth_timeout_s=eth_timeout_s, - eth_finality_stall_tolerance_s=stall, - max_covenant_confirm_wait_s=wait, - ) - with pytest.raises(SystemExit, match="cannot hold the timelock at all"): - runner._assert_the_eth_deadline_can_hold_the_margins(args) + """The honest path these rows could not previously have: derive, then run the whole + parse-time pipeline over the derived value and require it to PASS. - @pytest.mark.parametrize("fast,eth_timeout_s,stall,wait", _EMPTY_FEASIBLE_SET_ROWS) - def test_no_refusal_ever_advises_the_value_it_just_refused( - self, runner, fast: float, eth_timeout_s: int, stall: int, wait: int - ) -> None: - """The defect\'s signature, asserted on the operator-visible text.""" + This is the assertion the old suite could not make. When the set was empty the best it + could check was that the refusal did not contradict itself — a property about the wording + of a failure. There is a correct answer on these parameters now, so the test asks for it. + """ args = _ns( rxd_block_interval_fast_s=fast, eth_timeout_s=eth_timeout_s, eth_finality_stall_tolerance_s=stall, max_covenant_confirm_wait_s=wait, ) - with pytest.raises(SystemExit) as exc: - _run_the_whole_parse_time_pipeline(runner, args) - msg = str(exc.value) - if "and it is derived:" not in msg: - return # the deadline guard fired first, which is the whole fix - refused = int(msg.split("--t-rxd-blocks ")[1].split()[0]) - advised = int(msg.split("and it is derived: ")[1].split()[0]) - assert refused != advised, ( - f"the refusal rejects --t-rxd-blocks {refused} and then recommends {advised}: the " - f"self-contradiction this guard exists to eliminate.\n{msg}" - ) - - @pytest.mark.parametrize("fast,eth_timeout_s,stall,wait", _EMPTY_FEASIBLE_SET_ROWS) - def test_the_minimum_it_names_is_reached_by_the_whole_pipeline( - self, runner, fast: float, eth_timeout_s: int, stall: int, wait: int - ) -> None: - """Honest path. A refusal that leaves the operator with no correct action IS the defect; - the remedy has to survive the derivation AND all three bounds.""" - base = dict( - rxd_block_interval_fast_s=fast, - eth_finality_stall_tolerance_s=stall, - max_covenant_confirm_wait_s=wait, - ) - with pytest.raises(SystemExit) as exc: - runner._assert_the_eth_deadline_can_hold_the_margins(_ns(eth_timeout_s=eth_timeout_s, **base)) - minimum = int(str(exc.value).split("minimum: --eth-timeout-s ")[1].split()[0]) - _run_the_whole_parse_time_pipeline(runner, _ns(eth_timeout_s=minimum, **base)) + args.t_rxd_blocks = runner._recommended_t_rxd_blocks(args) + _run_the_whole_parse_time_pipeline(runner, args) # must not raise + lo, hi = runner._t_rxd_feasible_range(args) + assert lo <= args.t_rxd_blocks <= hi - @pytest.mark.parametrize("fast,eth_timeout_s,stall,wait", _EMPTY_FEASIBLE_SET_ROWS) - def test_the_minimum_is_actually_minimAL( - self, runner, fast: float, eth_timeout_s: int, stall: int, wait: int - ) -> None: - base = dict( - rxd_block_interval_fast_s=fast, - eth_finality_stall_tolerance_s=stall, - max_covenant_confirm_wait_s=wait, - ) - with pytest.raises(SystemExit) as exc: - runner._assert_the_eth_deadline_can_hold_the_margins(_ns(eth_timeout_s=eth_timeout_s, **base)) - minimum = int(str(exc.value).split("minimum: --eth-timeout-s ")[1].split()[0]) - with pytest.raises(SystemExit, match="cannot hold"): - runner._assert_the_eth_deadline_can_hold_the_margins(_ns(eth_timeout_s=minimum - 1, **base)) + # REMOVED WITH #482, not silently: `test_the_minimum_it_names_is_reached_by_the_whole_pipeline` + # and `test_the_minimum_is_actually_minimAL`. Both drove `_assert_the_eth_deadline_can_hold_the + # _margins` over `_EMPTY_FEASIBLE_SET_ROWS` and parsed a "minimum: --eth-timeout-s" out of the + # refusal. Those rows no longer refuse — their sets are non-empty — so neither test could reach + # its assertion, and both would have had to be rewritten into something the two tests above + # already say about the same rows: the set exists, and the whole pipeline accepts the derived + # value on it. The minimum-vs-maximum boundary they were pinning is covered by + # `TestAnImpossibleDeadlineNamesTheRightArgument`, on parameters that genuinely refuse. @pytest.mark.parametrize("fast", [36.2, 36.4, 36.5, 36.7, 43.3, 55.9, 60.5, 22.7, 331.7, 41.618, 9.7, 128.3]) def test_a_FRACTIONAL_sweep_never_clears_a_deadline_the_bounds_then_refuse(self, runner, fast: float) -> None: @@ -442,12 +488,16 @@ def test_the_feasible_range_is_exactly_the_values_the_bounds_accept(self, runner window = runner._t_rxd_feasible_range(args) assert window is not None lo, hi = window - for t in range(max(1, lo - 3), hi + 4): + # Probe the NEIGHBOURHOOD OF THE FLOOR, not of `hi`. `hi` is the BIP68 field width + # since #482, and `hi + 4` walks past it into values the predicates accept (they are + # all floors) but the range excludes for being unrepresentable — a disagreement about + # the field width, not about the bounds. + for t in range(max(1, lo - 3), min(lo + 4, hi + 1)): probe = _ns(rxd_block_interval_fast_s=fast, eth_timeout_s=86_400, t_rxd_blocks=t) accepted = True try: runner._assert_t_rxd_covers_the_takers_wait(probe) - runner._assert_t_rxd_opens_before_the_eth_deadline(probe) + runner._assert_t_rxd_outlasts_the_eth_deadline(probe) runner._assert_t_rxd_bounds_the_vulnerable_window(probe) except SystemExit: accepted = False @@ -508,11 +558,26 @@ def test_the_vulnerable_window_bound_ACCEPTS_the_derived_value(self, runner) -> def test_the_vulnerable_window_bound_is_a_FLOOR_and_refuses_one_block_under_it(self, runner) -> None: """Pins the DIRECTION. A bound that only refuses very small values would satisfy the 240 - test above while still accepting everything near the boundary.""" - derived = runner._derive_t_rxd_blocks(_ns()) - runner._assert_t_rxd_bounds_the_vulnerable_window(_ns(t_rxd_blocks=derived)) + test above while still accepting everything near the boundary. + + PROBED AT THIS BOUND'S OWN FLOOR, not at the derived value. Since #482 the deadline floor + dominates and the derived value sits far above the vulnerable-window floor, so + `derived - 1` still satisfies this bound — the assertion would have been vacuous while + looking identical to a real one. The floor is located by scanning this predicate alone. + """ + + def ok(t: int) -> bool: + try: + runner._assert_t_rxd_bounds_the_vulnerable_window(_ns(t_rxd_blocks=t)) + return True + except SystemExit: + return False + + floor = next(t for t in range(1, 65_536) if ok(t)) + assert floor > 1, "the vulnerable-window bound accepts t_rxd=1; it is not binding at all" + runner._assert_t_rxd_bounds_the_vulnerable_window(_ns(t_rxd_blocks=floor)) with pytest.raises(SystemExit, match="ASSET_VULNERABLE"): - runner._assert_t_rxd_bounds_the_vulnerable_window(_ns(t_rxd_blocks=derived - 1)) + runner._assert_t_rxd_bounds_the_vulnerable_window(_ns(t_rxd_blocks=floor - 1)) #: (fast_interval_s, eth_timeout_s, stall_s, confirm_wait_s) rows where the feasible set is @@ -546,10 +611,22 @@ class TestTheADVICEIsSourcedFromTheFeasibleSet: """ @pytest.mark.parametrize("fast,eth_timeout_s,stall,wait", _DERIVATION_MISSES_THE_RANGE_ROWS) - def test_the_row_still_reproduces_the_derivation_gap( + def test_the_derivation_now_lands_exactly_ON_the_feasible_floor( self, runner, fast: float, eth_timeout_s: int, stall: int, wait: int ) -> None: - """Guards the guard: without an actual gap these rows prove nothing.""" + """THE GAP IS CLOSED BY CONSTRUCTION SINCE #482, so this asserts the opposite of what its + name records — deliberately, and under the original name, because the rows are the evidence. + + These are the measured parameter sets on which the library's derivation landed OUTSIDE the + runner's feasible set. That was possible because the set had a ceiling: the derivation + aimed at the largest window the gate accepts, the runner capped it a block lower, and the + two disagreed at a fractional tail. Inverted, both aim at the same floor — the derivation + returns the SMALLEST value the gate accepts and the runner's lower bound IS that value — + so `derived == window[0]` exactly, on every row. + + That is a sharper property than "lands inside", and it is asserted rather than the weaker + containment because the equality is what makes the gap unrepresentable. + """ args = _ns( rxd_block_interval_fast_s=fast, eth_timeout_s=eth_timeout_s, @@ -557,11 +634,15 @@ def test_the_row_still_reproduces_the_derivation_gap( max_covenant_confirm_wait_s=wait, ) window = runner._t_rxd_feasible_range(args) - assert window is not None, "this row's feasible set is empty; it belongs in the other class" + assert window is not None, "this row's feasible set is empty; the ceiling has come back" derived = runner._derive_t_rxd_blocks(args) - assert not (window[0] <= derived <= window[1]), ( - f"the raw derivation {derived} now lands inside {window} at fast={fast} — this row no " - "longer demonstrates the gap; find a replacement rather than deleting it" + assert window[0] <= derived <= window[1], ( + f"the derivation {derived} falls outside {window} at fast={fast} — the gap this row records has reopened" + ) + assert derived == window[0], ( + f"the derivation {derived} is not the feasible floor {window[0]} at fast={fast}. Both " + "should be 'the smallest t_rxd the gate accepts'; a difference means they have drifted " + "apart again, which is exactly how the gap arose." ) @pytest.mark.parametrize("fast,eth_timeout_s,stall,wait", _DERIVATION_MISSES_THE_RANGE_ROWS) diff --git a/tests/test_eth_timelock_mutant_killers.py b/tests/test_eth_timelock_mutant_killers.py index 991eddda..9ecbe1b4 100644 --- a/tests/test_eth_timelock_mutant_killers.py +++ b/tests/test_eth_timelock_mutant_killers.py @@ -38,6 +38,19 @@ def _margin(**kw) -> CrossClockMargin: return CrossClockMargin(**base) +def _size_for_budget(budget_s: int, interval: float, *, lock_delay: int = 600, **kw): + """Size against an EXACT budget, which is what every boundary below is really about. + + #482 flipped the budget from `eth_timeout - margin - lock` to `eth_timeout + margin - lock`, so + the old `eth_timeout_s = 7068 + N` idiom now means a budget 14136s larger than intended. Naming + the budget directly makes these constants say what they pin instead of encoding one particular + arrangement of the formula, and it survives the next change to the formula's shape. + """ + return _size( + eth_timeout_s=budget_s - _margin().total_s() + lock_delay, interval=interval, lock_delay=lock_delay, **kw + ) + + def _size(*, eth_timeout_s: int, interval: float, lock_delay: int = 0, **kw): return eth_absolute_to_rxd_relative_blocks( eth_timeout_unix_s=_NOW + eth_timeout_s, @@ -57,15 +70,17 @@ class TestTheSafetyFloorDefault: """ def test_the_default_floor_is_exactly_twelve(self) -> None: - # Budgets are stated as (margin + lock_delay + N*interval); the sizer emits N-1 because it - # is `ceil(budget/interval) - 1`. Values below were MEASURED against the real sizer rather - # than derived by hand — deriving them by hand is what put an off-by-one in this test on - # the first attempt, which is the same error class the whole file exists to pin. - assert _size(eth_timeout_s=7068 + 600 + 13 * 60, interval=60.0, lock_delay=600).value == 12 + # MEASURED against the real sizer, not derived by hand — deriving them by hand is what put + # an off-by-one in this test on the first attempt, which is the same error class the whole + # file exists to pin. Re-measured after #482: the sizer now emits the SMALLEST t with + # `ceil(t*interval) >= budget`, so a 661s budget at 60s/block is the first that reaches 12. + assert _size_for_budget(661, 60.0).value == 12 + assert _size_for_budget(720, 60.0).value == 12 # ...and 720 is the last + assert _size_for_budget(721, 60.0).value == 13 # one second more buys the next block def test_one_block_below_the_default_floor_is_refused(self) -> None: with pytest.raises(ValidationError, match="below safety floor 12"): - _size(eth_timeout_s=7068 + 600 + 12 * 60, interval=60.0, lock_delay=600) + _size_for_budget(660, 60.0) class TestTheBip68CapBoundary: @@ -78,13 +93,14 @@ class TestTheBip68CapBoundary: def test_exactly_the_cap_is_ACCEPTED(self) -> None: cap = SEQUENCE_LOCKTIME_MASK - # cap + 1 seconds of budget, because the sizer subtracts one from the ceiling. - sized = _size(eth_timeout_s=7068 + cap + 1, interval=1.0) + # At 1s/block the budget IS the block count, so the cap is reached by a budget of exactly + # `cap` seconds. (It was `cap + 1` while the sizer subtracted one from the ceiling.) + sized = _size_for_budget(cap, 1.0, lock_delay=0) assert sized.value == cap, f"expected the cap {cap} to be representable, got {sized.value}" def test_one_block_ABOVE_the_cap_is_refused(self) -> None: with pytest.raises(ValidationError, match="BIP68 16-bit cap"): - _size(eth_timeout_s=7068 + SEQUENCE_LOCKTIME_MASK + 2, interval=1.0) + _size_for_budget(SEQUENCE_LOCKTIME_MASK + 1, 1.0, lock_delay=0) class TestTheIntervalAndBudgetBoundaries: @@ -107,13 +123,13 @@ def test_a_zero_interval_is_still_refused(self) -> None: def test_a_budget_of_exactly_zero_is_refused(self) -> None: """`<= 0` vs `< 0`: a zero budget buys no blocks at all and must fail closed.""" with pytest.raises(ValidationError): - _size(eth_timeout_s=_margin().total_s(), interval=1.0) + _size_for_budget(0, 1.0, lock_delay=0) def test_a_budget_of_exactly_one_second_is_refused_by_the_FLOOR_not_by_the_sign(self) -> None: """`<= 0` vs `<= 1`: one second is a positive budget, so it must pass the sign check and be refused by the safety floor instead — a different error, which is what distinguishes them.""" with pytest.raises(ValidationError, match="below safety floor"): - _size(eth_timeout_s=_margin().total_s() + 1, interval=1.0) + _size_for_budget(1, 1.0, lock_delay=0) class TestTheFloorBlocksTypeGuard: @@ -134,12 +150,14 @@ def test_an_honest_int_floor_is_accepted(self) -> None: assert _size(eth_timeout_s=86_400, interval=36.0, floor_blocks=12).value > 12 -class TestTheAnalyticValueNeedsAtMostOneStepDown: +class TestTheAnalyticValueNeedsAtMostOneStep: """Four mutants on the sizer's arithmetic line survive, and I first called them equivalent. - `t_rxd_blocks = ceil(budget/interval) - 1` mutates to `- 0`, `// 1`, `* 1`, `** 1` — all of - which are `ceil(budget/interval)` — and the step-down loop then walks the value down until the - punctuality gate accepts it, so the final answer is the same. "Equivalent", I said. + `t_rxd_blocks = ceil(budget/interval)` mutates to `+ 1`, `// 1`, `* 1`, `** 1` — and the loop + then walks the value until the gate accepts it, so the final answer is the same. "Equivalent", + I said. (#482 inverted the search: it starts one BELOW the analytic value and steps UP, where + it used to start at `ceil - 1` and step down. The invariant this class pins — that the loop + corrects a rounding edge rather than repairing arithmetic nobody checks — is unchanged.) That is only true because `_SIZER_GATE_STEPS` is 3, giving the loop room to absorb a wrong starting point. The code's own comment claims the analytic value is "never more than one block @@ -153,21 +171,26 @@ class TestTheAnalyticValueNeedsAtMostOneStepDown: @staticmethod def _steps_needed(*, eth_timeout_s: int, interval: float, lock_delay: int = 600) -> int: - """How far the loop must walk from the analytic value to reach one the gate accepts.""" + """How far the loop must walk from its starting value to reach one the gate accepts. + + UPWARD now (#482), so the subtraction is `emitted - analytic`. Left as `analytic - emitted` + this returns a negative number, `0 <= steps` fails, and the message blames the arithmetic + for drifting when the only thing that moved was the direction of the walk. + """ import math margin = _margin() - budget = eth_timeout_s - margin.total_s() - lock_delay - analytic = math.ceil(budget / interval) - 1 + budget = eth_timeout_s + margin.total_s() - lock_delay + analytic = math.ceil(budget / interval) - 1 # the sizer's own starting point emitted = _size(eth_timeout_s=eth_timeout_s, interval=interval, lock_delay=lock_delay).value - return analytic - emitted + return emitted - analytic @pytest.mark.parametrize("interval", [36.0, 36.2, 36.4, 43.3, 41.618, 60.0, 22.7]) @pytest.mark.parametrize("eth_timeout_s", [43_200, 61_200, 86_400, 111_600]) def test_the_loop_never_walks_more_than_one_block(self, interval: float, eth_timeout_s: int) -> None: steps = self._steps_needed(eth_timeout_s=eth_timeout_s, interval=interval) assert 0 <= steps <= 1, ( - f"the analytic value needed {steps} step-downs at interval={interval}, " + f"the analytic value needed {steps} steps at interval={interval}, " f"eth_timeout_s={eth_timeout_s}. The code documents at most one; more means the " f"arithmetic has drifted from the gate and the step-down loop is concealing it." ) diff --git a/tests/test_htlc_handshake_conformance_vectors.py b/tests/test_htlc_handshake_conformance_vectors.py index a7c0acb6..753aeb6e 100644 --- a/tests/test_htlc_handshake_conformance_vectors.py +++ b/tests/test_htlc_handshake_conformance_vectors.py @@ -17,7 +17,7 @@ ETH/credential keys are omitted exactly when they hold their BTC defaults; 2. the 32-byte preimage rule — the fixed length is what the ``OP_SIZE <0x20> OP_EQUALVERIFY`` prefix consensus-pins on both legs' claim branches; -3. the timelock-margin invariant ``t_btc - t_rxd >= margin``, including the cross-unit case +3. the timelock-margin invariant ``t_rxd - t_btc >= margin`` (#482), including the cross-unit case that the cheap same-unit construction guard cannot see; 4. the two re-derived commitments a counterparty actually checks — the BTC P2TR funding scriptPubKey and the Radiant covenant scriptPubKey. @@ -194,10 +194,18 @@ def test_terms_hashlock_matches_the_published_preimage(vec: dict): def test_margin_verdicts(vec: dict): """``assert_timelock_margin`` must accept/reject exactly as published. - The invariant is ``t_btc - t_rxd >= margin`` with both legs normalised to blocks. The - direction is counterintuitive and load-bearing: the party who locks FIRST (the taker, - on the counter leg) holds the LONGER refund window, so the leg claimed second (Radiant) - always matures first. + The invariant is ``t_rxd - t_btc >= margin`` with both legs normalised to blocks. The + direction is load-bearing: the MAKER holds the preimage ``p``, LOCKS the Radiant leg and + CLAIMS the BTC leg, so the leg it LOCKS carries the LONGER refund (Herlihy, + arXiv:1801.09515 §1). Otherwise the maker can let its own leg mature, refund it, and still + claim the counter leg with ``p`` — taking both. + + THE PUBLISHED VECTORS ASSERTED THE OPPOSITE and this docstring argued for it, calling the + wrong direction "counterintuitive and load-bearing". The suite accepted t_btc=60/t_rxd=20 + and rejected t_btc=20/t_rxd=60, so a second implementation that got the direction RIGHT + would have failed conformance and been "corrected" into the exploitable layout. That is + the worst failure mode a conformance suite has: it does not merely miss a defect, it + propagates one, with the authority of a published spec behind it. """ policy = MarginPolicy( margin=_timelock(vec["margin"]), @@ -217,7 +225,7 @@ def test_cross_unit_inversion_passes_construction_but_fails_the_margin_check(): """The construction guard is NOT the safety check — the normalising one is. ``NegotiatedTerms.__post_init__`` only compares t_btc/t_rxd when they share a unit, so a - SECONDS t_btc that is shorter than a BLOCKS t_rxd constructs happily. Only + SECONDS t_btc that is LONGER than a BLOCKS t_rxd constructs happily. Only ``assert_timelock_margin`` catches it. A second implementation that ports the cheap guard and skips the normalising one will fund inverted swaps. """ diff --git a/tests/test_radiant_leg.py b/tests/test_radiant_leg.py index 5bcd5904..6377f8e8 100644 --- a/tests/test_radiant_leg.py +++ b/tests/test_radiant_leg.py @@ -54,7 +54,11 @@ def _rxd_terms(amount: int = 100_000, csv: int = 6) -> NegotiatedTerms: hashlock=_H, btc_sats=100_000, radiant_amount=amount, - t_btc=t.Timelock(144, t.TimeUnit.BLOCKS), + # t_btc DERIVES from the covenant's own CSV. Under the inverted relation (#482) the + # Radiant leg the maker LOCKS must outlast the leg it CLAIMS, and `csv` IS that + # Radiant timelock — so a fixed 72 is unconstructible the moment csv drops below it, + # which is the default here (6). + t_btc=t.Timelock(max(1, csv // 2), t.TimeUnit.BLOCKS), t_rxd=t.Timelock(csv, t.TimeUnit.BLOCKS), asset_variant="rxd", genesis_ref=b"", @@ -79,7 +83,11 @@ def _ft_terms(amount: int = 1000, csv: int = 6) -> NegotiatedTerms: hashlock=_H, btc_sats=100_000, radiant_amount=amount, - t_btc=t.Timelock(144, t.TimeUnit.BLOCKS), + # t_btc DERIVES from the covenant's own CSV. Under the inverted relation (#482) the + # Radiant leg the maker LOCKS must outlast the leg it CLAIMS, and `csv` IS that + # Radiant timelock — so a fixed 72 is unconstructible the moment csv drops below it, + # which is the default here (6). + t_btc=t.Timelock(max(1, csv // 2), t.TimeUnit.BLOCKS), t_rxd=t.Timelock(csv, t.TimeUnit.BLOCKS), asset_variant="ft", genesis_ref=GlyphRef(txid=_REF_TXID, vout=0).to_bytes(), @@ -600,7 +608,7 @@ async def test_nft_variant_builds_and_binds(): hashlock=_H, btc_sats=100_000, radiant_amount=1000, - t_btc=t.Timelock(144, t.TimeUnit.BLOCKS), + t_btc=t.Timelock(3, t.TimeUnit.BLOCKS), t_rxd=t.Timelock(6, t.TimeUnit.BLOCKS), asset_variant="nft", genesis_ref=GlyphRef(txid=_REF_TXID, vout=0).to_bytes(), @@ -1194,7 +1202,11 @@ def _nft_terms(carrier: int = 1000, csv: int = 6) -> NegotiatedTerms: hashlock=_H, btc_sats=100_000, radiant_amount=carrier, - t_btc=t.Timelock(144, t.TimeUnit.BLOCKS), + # t_btc DERIVES from the covenant's own CSV. Under the inverted relation (#482) the + # Radiant leg the maker LOCKS must outlast the leg it CLAIMS, and `csv` IS that + # Radiant timelock — so a fixed 72 is unconstructible the moment csv drops below it, + # which is the default here (6). + t_btc=t.Timelock(max(1, csv // 2), t.TimeUnit.BLOCKS), t_rxd=t.Timelock(csv, t.TimeUnit.BLOCKS), asset_variant="nft", genesis_ref=GlyphRef(txid=_REF_TXID, vout=0).to_bytes(), diff --git a/tests/test_record_sink_and_fund_lock.py b/tests/test_record_sink_and_fund_lock.py index 57dd5398..4407060c 100644 --- a/tests/test_record_sink_and_fund_lock.py +++ b/tests/test_record_sink_and_fund_lock.py @@ -143,7 +143,9 @@ def _rec(self): return SwapRecord( state=SwapState.NEGOTIATED, - terms=_eth_terms(hashlock=b"\x33" * 32, eth_timeout_unix_s=1_800_000_000), + # 1_800_000_000 was ~3 years past the shared fixture clock; `_eth_terms` now SIZES t_rxd + # from the deadline (#482), so that asks for ~333k blocks and trips the BIP68 16-bit cap. + terms=_eth_terms(hashlock=b"\x33" * 32, eth_timeout_unix_s=1_700_040_000), pending_counter_contract="0x" + "ab" * 20, pending_counter_deploy_tx="0x" + "cd" * 32, pending_push_nonce=41, diff --git a/tests/test_swap_coordinator.py b/tests/test_swap_coordinator.py index 3655fb57..e0dc80d8 100644 --- a/tests/test_swap_coordinator.py +++ b/tests/test_swap_coordinator.py @@ -182,7 +182,7 @@ class FakeRadiantLeg: on-chain-vs-expected match to drive PARAMS_MISMATCH. """ - def __init__(self, *, asset_funded: bool = True) -> None: + def __init__(self, *, asset_funded: bool = True, report_confs: int | None = None) -> None: self.calls: list[str] = [] self.claimed_with: bytes | None = None self.refunded = False @@ -191,13 +191,20 @@ def __init__(self, *, asset_funded: bool = True) -> None: # tests/test_taker_asset_funding_gate_adversarial.py for the real-leg version). self.asset_funded = bool(asset_funded) self.verify_min_confirmations: list[int | None] = [] + # A covenant DEEPER than the minimum the taker asks for. The default fake reports exactly + # the minimum, which quietly models the one case a maker would never choose: locking at the + # last possible moment. `t_rxd` counts from MINING, so every block of extra depth is a + # block the taker does not get, and the maker picks that number for free by locking early + # and presenting late (#482 step 7). + self.report_confs = report_confs async def verify_maker_asset_funded(self, terms: NegotiatedTerms, *, min_confirmations=None): self.calls.append("verify_maker_asset_funded") self.verify_min_confirmations.append(min_confirmations) if not self.asset_funded: raise NetworkError("no UTXO found for the covenant scriptPubKey (not yet funded / wrong SPK)") - return ("ef" * 32 + ":0", terms.radiant_amount, max(int(min_confirmations or 1), 1)) + confs = self.report_confs if self.report_confs is not None else max(int(min_confirmations or 1), 1) + return ("ef" * 32 + ":0", terms.radiant_amount, int(confs)) async def expected_covenant_scriptpubkey(self, terms: NegotiatedTerms) -> bytes: # Deterministic stand-in for the fused covenant SPK. @@ -288,7 +295,29 @@ def mark_seen(self, hashlock: bytes) -> None: # --------------------------------------------------------------------------- -def _terms(*, variant: str = "ft", t_btc_blocks: int = 144, t_rxd_blocks: int = 72, hashlock: bytes | None = None): +# INVERTED 2026-08-31 (#482): t_rxd is now the LONGER leg. The maker holds p and LOCKS the Radiant +# covenant, so that leg carries the longer timeout and the leg the maker CLAIMS (BTC) the shorter — +# Herlihy 1801.09515 §1. The defaults were 144/72 the other way round, so every test built on them +# was exercising the relation that let the maker take both legs. +# +# `t_btc_blocks` now DERIVES from `t_rxd_blocks` when not given. Callers vary `t_rxd` to exercise +# burial and squeeze bands; making each one also hand-maintain `t_btc` is how a relation drifts out +# of a suite one call site at a time. `_BTC_GAP` is wide enough for the default estimated margin. +_BTC_GAP = 40 + + +def _terms( + *, + variant: str = "ft", + t_btc_blocks: int | None = None, + t_rxd_blocks: int = 144, + hashlock: bytes | None = None, +): + if t_btc_blocks is None: + # At least 1: a zero/negative BTC timelock is not a swap, and a t_rxd below the margin + # cannot satisfy the invariant at all — which is itself a real consequence of the + # inversion, and why the small-t_rxd cases below carry a smaller margin. + t_btc_blocks = max(1, t_rxd_blocks - _BTC_GAP) if hashlock is None: hashlock = hashlib.sha256(os.urandom(32)).digest() return NegotiatedTerms( @@ -364,7 +393,9 @@ def test_role_invariant_constant_spelled_out(): # covenant off the Radiant chain and fails closed, so the taker CANNOT fund first. for phrase in ("generates the secret", "locks the asset FIRST", "locks BTC SECOND", "claims the BTC FIRST"): assert phrase in inv, f"missing {phrase!r} — see pre_btc_lock_check step 5 for the enforced order" - assert "t_BTC > t_RXD" in inv + # INVERTED #482: the maker LOCKS the Radiant leg, so THAT leg carries the longer timeout. + assert "t_RXD > t_BTC" in inv + assert "t_BTC > t_RXD" not in inv, "the invariant states the direction that let the maker take both legs" # The NAME still says TAKER_LOCKS_BTC_FIRST; it is exported and quoted in a # ValidationError, so it stays. The body must say why, or the name re-teaches the old order. assert "predates HZ-1" in inv @@ -410,8 +441,9 @@ async def test_taker_funds_btc_rejects_amount_mismatch(): assert rec.state is SwapState.BTC_LOCKED -def test_margin_rejects_btc_not_greater_than_rxd(): +def test_margin_rejects_rxd_not_greater_than_btc(): # Construct via direct Timelocks (NegotiatedTerms would also reject same-unit). + # INVERTED #482: t_rxd is the leg the MAKER LOCKED and must be the LONGER one. policy = MarginPolicy.estimated() with pytest.raises(ValidationError): assert_timelock_margin(t.Timelock(72, t.TimeUnit.BLOCKS), t.Timelock(72, t.TimeUnit.BLOCKS), policy) @@ -419,22 +451,22 @@ def test_margin_rejects_btc_not_greater_than_rxd(): def test_margin_rejects_insufficient_gap(): policy = MarginPolicy.estimated() # 36-block ESTIMATED margin - # gap = 10 blocks < 36 required + # gap = 10 blocks < 36 required (t_rxd - t_btc) with pytest.raises(ValidationError): - assert_timelock_margin(t.Timelock(82, t.TimeUnit.BLOCKS), t.Timelock(72, t.TimeUnit.BLOCKS), policy) + assert_timelock_margin(t.Timelock(72, t.TimeUnit.BLOCKS), t.Timelock(82, t.TimeUnit.BLOCKS), policy) def test_margin_accepts_safe_gap(): policy = MarginPolicy.estimated() - # gap = 100 blocks >= 36 - assert_timelock_margin(t.Timelock(172, t.TimeUnit.BLOCKS), t.Timelock(72, t.TimeUnit.BLOCKS), policy) + # gap = 100 blocks >= 36 (t_rxd - t_btc) + assert_timelock_margin(t.Timelock(72, t.TimeUnit.BLOCKS), t.Timelock(172, t.TimeUnit.BLOCKS), policy) def test_margin_cross_unit_normalises(): - # t_btc in seconds, t_rxd in blocks; 600s/block. 144*600=86400s vs 72 blk=43200s, - # gap = 72 blocks-equiv = enough for the 36-block margin. + # t_btc in seconds, t_rxd in blocks; 600s/block. 72 blk-equiv = 43200s vs 144 blk, + # gap = 72 blocks-equiv = enough for the 36-block margin. The LONGER leg is t_rxd. policy = MarginPolicy.estimated(block_interval_s=600.0) - assert_timelock_margin(t.Timelock(86_400, t.TimeUnit.SECONDS), t.Timelock(72, t.TimeUnit.BLOCKS), policy) + assert_timelock_margin(t.Timelock(43_200, t.TimeUnit.SECONDS), t.Timelock(144, t.TimeUnit.BLOCKS), policy) def test_margin_fail_closed_on_non_timelock(): @@ -450,7 +482,7 @@ def test_margin_real_value_mode_requires_measured(): # A measured policy in real-value mode is accepted. measured = MarginPolicy.measured(margin=t.Timelock(50, t.TimeUnit.BLOCKS), block_interval_s=600.0) assert measured.is_measured and measured.require_measured - assert_timelock_margin(t.Timelock(200, t.TimeUnit.BLOCKS), t.Timelock(72, t.TimeUnit.BLOCKS), measured) + assert_timelock_margin(t.Timelock(72, t.TimeUnit.BLOCKS), t.Timelock(200, t.TimeUnit.BLOCKS), measured) def test_estimated_margin_is_labelled(): @@ -1599,7 +1631,10 @@ async def test_scrape_rejects_claim_tx_for_foreign_funding_outpoint(): async def test_gate_squeezed_goes_vulnerable_then_explicit_claim(): p_secret, h = generate_secret() - terms = _terms(variant="rxd", t_rxd_blocks=10, hashlock=h) + # t_rxd 50, not 10: under the inverted relation (#482) t_rxd must exceed t_btc by the 36-block + # margin, so a 10-block window is not a swap that can be constructed. The SQUEEZED state is + # reached by burning the window with elapsed height below, which is how it happens for real. + terms = _terms(variant="rxd", t_rxd_blocks=50, hashlock=h) btc = FakeBtcLeg(claim_confs=1) # shallow rxd = FakeRadiantLeg() coord = _coordinator(terms=terms, btc_leg=btc, radiant_leg=rxd) @@ -1608,7 +1643,8 @@ async def test_gate_squeezed_goes_vulnerable_then_explicit_claim(): rec = await coord.maker_claims_btc(p_secret) claim_tx = _real_maker_claim_tx(rec.btc_locator, btc.claimed_with) # Window closing (now near t_rxd maturity) + shallow -> SQUEEZED -> ASSET_VULNERABLE. - rec = await coord.taker_scrape_and_claim_asset(claim_tx, now_rxd_height=1006, asset_locked_at_height=1000) + # 45 of the 50 blocks spent -> 5 left, under the 6-block burial -> SQUEEZED. + rec = await coord.taker_scrape_and_claim_asset(claim_tx, now_rxd_height=1045, asset_locked_at_height=1000) assert rec.state is SwapState.ASSET_VULNERABLE assert rxd.claimed_with is None # not auto-claimed # The deliberate winner-take-all claim is a separate, explicit decision. @@ -1945,12 +1981,49 @@ async def verify_counterparty_funded(self, contract_address, terms, *, block_ide return self.last_locator -def _eth_terms(*, hashlock: bytes, t_rxd_blocks: int = 72, eth_timeout_unix_s: int = 1779710245): +_NOW = 1_700_000_000 +# The cross-clock margin the ETH fixtures below build (780 + 1800 + 600 + 300, plus stall budget +# where a test sets one). Kept beside _NOW because `_eth_terms` sizes t_rxd against it. +_ETH_TEST_MARGIN_S = 780 + 1_800 + 600 + 300 +# Covenant depth already elapsed when the taker funds. Comfortably above the 6-block burial the +# measured fixtures use, so the derived t_rxd clears step 7 rather than sitting on its boundary. +_ELAPSED_DEPTH_ALLOWANCE = 24 + + +def _eth_terms( + *, + hashlock: bytes, + t_rxd_blocks: int | None = None, + eth_timeout_unix_s: int = _NOW + 40_000, + now_unix_s: int = _NOW, +): + # t_rxd DERIVES from the ETH deadline, the way production sizes it. Under the inverted relation + # (#482) the RXD refund must open AFTER `eth_timeout + margin`, so a fixed block count cannot + # be right for an arbitrary deadline — a fixture that hardcodes one is asserting against a + # deadline it does not span, and every ETH test built on it fails for a reason that has nothing + # to do with what it is testing. + if t_rxd_blocks is None: + # 300.0 is the interval the ETH coordinator fixtures below configure, NOT 600. Deriving + # against the wrong one halves the window and every ETH test fails on a margin it was + # never testing — which is what happened on the first attempt at this. + # `now_unix_s` is the CALLER's clock, not this module's. Other test files import this + # fixture and freeze time somewhere else entirely — sizing against `_NOW` there produced + # a span of decades and a t_rxd past the BIP68 cap, from a deadline the caller had + # written as 40_000s out. + span_s = max(0, eth_timeout_unix_s - now_unix_s) + _ETH_TEST_MARGIN_S + # +2 for the sizer's and the gate's rounding, +_ELAPSED_DEPTH_ALLOWANCE for the covenant + # confirmations already spent by the time the taker funds. Step 7 of the pre-fund gate + # checks the REMAINING window (#482), and on a MEASURED policy the covenant is required + # to be burial-deep before funding — so a t_rxd sized to exactly meet the deadline is + # always short by that depth. Production has to carry the same headroom. + t_rxd_blocks = math.ceil(span_s / 300.0) + 2 + _ELAPSED_DEPTH_ALLOWANCE + # t_btc is only the BTC-shaped placeholder here (the real deadline is `eth_timeout_unix_s`), + # but the construction guard still applies to it, so it respects the inverted relation too. return NegotiatedTerms( hashlock=hashlock, btc_sats=100_000, radiant_amount=1_000, - t_btc=t.Timelock(144, t.TimeUnit.BLOCKS), + t_btc=t.Timelock(max(1, t_rxd_blocks - _BTC_GAP), t.TimeUnit.BLOCKS), t_rxd=t.Timelock(t_rxd_blocks, t.TimeUnit.BLOCKS), asset_variant="rxd", genesis_ref=b"", @@ -2311,8 +2384,6 @@ def test_reserve_to_blocks_rounds_up_for_seconds(): from pyrxd.gravity.eth_rxd_timelock import CrossClockMargin -_NOW = 1_700_000_000 - def _xmargin(): # total = 768 + 1800 + 600 + 300 = 3468s @@ -2333,21 +2404,22 @@ def _eth_fund_policy(**kw): ) -def _eth_coord_negotiated(*, terms, policy=None): +def _eth_coord_negotiated(*, terms, policy=None, radiant_leg=None): rec = SwapRecord(state=SwapState.NEGOTIATED, terms=terms) p_dummy = b"\x01" * 32 return SwapCoordinator( record=rec, counter_leg=FakeEthLeg(preimage=p_dummy, verdict=_final()), - radiant_leg=FakeRadiantLeg(), + radiant_leg=radiant_leg or FakeRadiantLeg(), indexer=FakeIndexer(), seen_store=FakeSeenStore(), config=CoordinatorConfig(margin_policy=policy or _eth_fund_policy(), maker_stall_safety_window_blocks=6), ) -# projected_rxd_open = now + max_confirm_wait(3600) + t_rxd(72)*rxd_interval(300)=21600 = now+25200 -# deadline = eth_timeout - margin.total(3468). Need now+25200 < eth_timeout-3468 -> eth_timeout > now+28668. +# INVERTED (#482): earliest_rxd_open = now + (t_rxd - elapsed) * rxd_interval(300), and it must land +# AT OR AFTER eth_timeout + margin.total(3468). No confirm-wait term — that assumes a LATE confirm, +# which is the optimistic direction now. So t_rxd >= ceil((eth_timeout - now + 3468) / 300) + elapsed. def test_eth_timelock_ordering_accepts_safe_deadline(): @@ -2357,16 +2429,42 @@ def test_eth_timelock_ordering_accepts_safe_deadline(): coord._assert_eth_timelock_ordering(terms, now_unix_s=_NOW) # no raise (40000 > 28668) -def test_eth_timelock_ordering_rejects_deadline_too_close(): - # HIGH-1 core: an eth_timeout that does NOT clear the RXD window + margin is refused — - # a maker cannot set a deadline that lets it refund both legs. +def test_eth_timelock_ordering_rejects_a_t_rxd_that_opens_before_the_eth_deadline(): + """HIGH-1 core, in the direction that is actually dangerous (#482). + + THIS TEST USED TO ASSERT THE OPPOSITE and passed for years. It refused a deadline that was + "too close" — `_NOW + 10000` against a 28668s budget — because the old gate demanded the RXD + refund open BEFORE the ETH deadline. Under the correct relation a nearer deadline is the SAFE + case: the maker locks the Radiant leg, so that leg must OUTLAST the ETH leg it claims. What + robs the taker is the reverse — an RXD refund that opens while the maker can still claim ETH + with `p`, letting the maker take both legs. + + So the fixture states the real danger: a `t_rxd` too SMALL to outlast `eth_timeout + margin`. + The explicit block count matters — `_eth_terms` otherwise sizes `t_rxd` to fit whatever + deadline it is given, which would make this test green by construction while proving nothing. + """ _, h = generate_secret() - terms = _eth_terms(hashlock=h, eth_timeout_unix_s=_NOW + 10000) # 10000 < 28668 + # margin totals 3468s, so the refund must open no earlier than _NOW + 43468 => t_rxd >= 145 + # blocks at the 300s dividing interval. 100 leaves the maker a window it should not have. + terms = _eth_terms(hashlock=h, eth_timeout_unix_s=_NOW + 40000, t_rxd_blocks=100) coord = _eth_coord_negotiated(terms=terms) - with pytest.raises(ValidationError, match="confirm too late"): + with pytest.raises(ValidationError, match="open too EARLY"): coord._assert_eth_timelock_ordering(terms, now_unix_s=_NOW) +def test_eth_timelock_ordering_accepts_the_nearer_deadline_that_the_old_gate_refused(): + """The honest path the inversion restores, and the paired case for the test above. + + `_NOW + 10000` is the exact deadline the old gate called "too close" and refused. It is + legitimate: the maker's Radiant leg outlasts it comfortably. A guard that refuses valid work is + a bug, and this one sat on a parameter an honest maker must choose. + """ + _, h = generate_secret() + terms = _eth_terms(hashlock=h, eth_timeout_unix_s=_NOW + 10000) + coord = _eth_coord_negotiated(terms=terms) + coord._assert_eth_timelock_ordering(terms, now_unix_s=_NOW) # no raise + + def test_eth_timelock_ordering_rejects_expired_deadline(): # The now-vs-timeout grief (completeness finding): an already-expired ETH HTLC is refused. _, h = generate_secret() @@ -2397,7 +2495,9 @@ def test_eth_timelock_ordering_requires_now_and_margin(): async def test_pre_lock_dispatches_eth_ordering_gate(): # Integration: pre_btc_lock_check step 3 routes an ETH swap to the cross-clock gate. _, h = generate_secret() - terms = _eth_terms(hashlock=h, eth_timeout_unix_s=_NOW + 10000) # too close + # A t_rxd too small to outlast the ETH deadline + margin — the direction that robs the taker + # (#482). Explicit, because `_eth_terms` otherwise sizes t_rxd to fit and nothing would fail. + terms = _eth_terms(hashlock=h, eth_timeout_unix_s=_NOW + 40000, t_rxd_blocks=100) coord = _eth_coord_negotiated(terms=terms) gate = await coord.pre_btc_lock_check(terms, now_unix_s=_NOW) assert not gate.ok and "margin check failed" in gate.reason @@ -2532,21 +2632,31 @@ async def test_eth_post_confirm_recheck_accepts_on_time_lock(): assert rec.state is SwapState.BOTH_LOCKED -async def test_eth_post_confirm_recheck_refuses_stalled_maker_lock(): - # THE re-verify HIGH: a maker who STALLS the covenant broadcast (locks late) collapses the - # cross-clock margin the pre-fund gate projected. The second run catches it and refuses to - # enter BOTH_LOCKED — the taker must refund the counter leg, not proceed. +async def test_eth_post_confirm_recheck_ACCEPTS_a_stalled_maker_lock_now_that_late_is_safe(): + """THE DIRECTION OF THIS TEST FLIPPED WITH #482, and that is the finding, not a fixture edit. + + It asserted that a maker STALLING its covenant broadcast collapses the cross-clock margin, and + the recheck refused. That was correct under the old relation: the RXD refund had to open BEFORE + the ETH deadline, so pushing the covenant's mining later pushed the refund past it. + + Inverted, the Radiant leg must OUTLAST the ETH leg. A late lock opens the refund LATER, which + is strictly safer — it costs the maker lock time and takes nothing from the taker. Refusing it + would be a guard refusing valid work, on a swap that has already had value committed to it, + where the taker's only alternative is an unnecessary refund and its fees. + + WHAT REPLACES IT is step 7 of the pre-fund gate: the danger under this relation is a covenant + that mined EARLY, because `t_rxd` counts from mining and the maker chooses how much of it to + spend before presenting the swap. That is asserted directly in + `TestTheMakerCannotSpendTRxdBeforePresentingTheSwap` below, against the remaining window. + """ secret, h = generate_secret() terms = _eth_terms(hashlock=h, eth_timeout_unix_s=_NOW + 40000) rxd = FakeRadiantLeg() coord = await _eth_to_btc_locked( leg=FakeEthLeg(preimage=secret, verdict=_final()), terms=terms, rxd=rxd, now_unix_s=_NOW ) - # Maker delays the lock to _NOW+30000: actual rxd_open _NOW+30000+21600 > deadline _NOW+36532. - with pytest.raises(ValidationError, match="confirm too late"): - await coord.post_asset_lock_revalidate(await rxd.expected_covenant_scriptpubkey(terms), now_unix_s=_NOW + 30000) - assert coord.record.state is SwapState.BTC_LOCKED # did NOT advance to BOTH_LOCKED - assert rxd.claimed_with is None + await coord.post_asset_lock_revalidate(await rxd.expected_covenant_scriptpubkey(terms), now_unix_s=_NOW + 30000) + assert coord.record.state is SwapState.BOTH_LOCKED async def test_eth_post_confirm_recheck_requires_now_unix_s(): @@ -3162,7 +3272,12 @@ def _valued_policy(): btc_claim_reorg_depth=t.Timelock(6, t.TimeUnit.BLOCKS), rxd_claim_burial=t.Timelock(6, t.TimeUnit.BLOCKS), rxd_reorg_cost_per_block=100_000, - value_at_risk_photons=3_000_000, # B(V) = 30 blocks at factor 1.0 + # B(V) = 60 blocks at factor 1.0. Sized ABOVE the 36-block margin on purpose: under the + # inverted relation (#482) t_rxd must exceed t_btc by the margin, so a t_rxd small enough + # to fail a 30-block burial floor is not a swap that can exist under this policy. Testing + # the burial gate there would be testing a fiction — the margin check refuses first, and + # the burial assertion never runs. + value_at_risk_photons=6_000_000, ) @@ -3180,8 +3295,8 @@ class TestTRxdMustBeAbleToContainTheValueScaledBurial: @pytest.mark.asyncio async def test_a_t_rxd_too_small_for_the_burial_is_REFUSED_before_funding(self) -> None: _secret, h = generate_secret() - # B(V) = 30; the SUFFICIENT floor is 30 + counter_reserve(0 for BTC) + 1 to mine = 31. - terms = _terms(hashlock=h, t_rxd_blocks=30) + # B(V) = 60; the SUFFICIENT floor is 60 + counter_reserve(0 for BTC) + 1 to mine = 61. + terms = _terms(hashlock=h, t_rxd_blocks=60) coord = _coordinator(terms=terms, policy=_valued_policy()) gate = await coord.pre_btc_lock_check(terms) assert not gate.ok @@ -3204,8 +3319,8 @@ async def test_a_REMAINING_window_that_EXACTLY_meets_the_sufficient_floor_is_acc 31, which is the defect this class exists to prevent, encoded in its own honest-path test. """ _secret, h = generate_secret() - # floor 31 + the 1 confirmation the fake covenant reports = exactly 31 remaining. - terms = _terms(hashlock=h, t_rxd_blocks=32) + # floor 61 + the 1 confirmation the fake covenant reports = exactly 61 remaining. + terms = _terms(hashlock=h, t_rxd_blocks=62) coord = _coordinator(terms=terms, policy=_valued_policy()) gate = await coord.pre_btc_lock_check(terms) assert gate.ok, gate.reason @@ -3214,7 +3329,7 @@ async def test_a_REMAINING_window_that_EXACTLY_meets_the_sufficient_floor_is_acc async def test_a_t_rxd_that_clears_the_floor_only_by_IGNORING_elapsed_depth_is_REFUSED(self) -> None: """The #531 regression, stated directly. - `t_rxd = 31` clears the floor if you compare the NEGOTIATED value, and fails it once the + `t_rxd = 61` clears the floor if you compare the NEGOTIATED value, and fails it once the covenant's elapsed confirmations are subtracted. Before the fix this swap was accepted and then SQUEEZED at every claim — the taker would reveal and find no safe claim available, which is precisely the state the gate was written to prevent. @@ -3224,7 +3339,7 @@ async def test_a_t_rxd_that_clears_the_floor_only_by_IGNORING_elapsed_depth_is_R requirement on every swap. """ _secret, h = generate_secret() - terms = _terms(hashlock=h, t_rxd_blocks=31) + terms = _terms(hashlock=h, t_rxd_blocks=61) coord = _coordinator(terms=terms, policy=_valued_policy()) gate = await coord.pre_btc_lock_check(terms) assert not gate.ok @@ -3243,8 +3358,11 @@ async def test_the_FLAT_burial_binds_even_without_economics(self) -> None: supposed to leave alone. """ _secret, h = generate_secret() - terms = _terms(hashlock=h, t_rxd_blocks=6) # flat burial 6, so the floor is 6 + 0 + 1 = 7 - coord = _coordinator(terms=terms) + # Burial 60 for the same reason `_valued_policy` uses 60: the flat term has to sit ABOVE + # the margin to be reachable at all. The VALUE of the flat burial is incidental here — what + # this test pins is that a policy with NO economics configured still binds on it. + terms = _terms(hashlock=h, t_rxd_blocks=60) # flat burial 60, so the floor is 60 + 0 + 1 = 61 + coord = _coordinator(terms=terms, policy=_policy(rxd_burial=60)) gate = await coord.pre_btc_lock_check(terms) assert not gate.ok assert "a safe claim needs" in gate.reason, gate.reason @@ -3335,7 +3453,7 @@ class TestTheFundGateClosesTheSqueezeBand: async def test_a_t_rxd_INSIDE_the_squeeze_band_is_refused(self) -> None: """Exactly the case a burial-only floor let through: >= the burial, < burial + 1.""" _secret, h = generate_secret() - terms = _terms(hashlock=h, t_rxd_blocks=30) # == B(V), inside the band + terms = _terms(hashlock=h, t_rxd_blocks=60) # == B(V), inside the band coord = _coordinator(terms=terms, policy=_valued_policy()) gate = await coord.pre_btc_lock_check(terms) assert not gate.ok @@ -3425,3 +3543,46 @@ async def test_mutual_refund_HONEST_path_still_completes(): await coord.post_asset_lock_revalidate(await rxd.expected_covenant_scriptpubkey(terms)) rec = await coord.mutual_refund() assert rec.state is not SwapState.BOTH_LOCKED, "an all-successful mutual refund must advance" + + +class TestTheMakerCannotSpendTRxdBeforePresentingTheSwap: + """Step 7 of the pre-fund gate: the cross-clock ordering check, re-run against the window that + ACTUALLY REMAINS. + + `t_rxd` is a RELATIVE CSV counted from the covenant's MINING. Step 3 checks the ordering + invariant against the NEGOTIATED `t_rxd` anchored at `now`, which is correct only if the + covenant mines now. It does not: the maker locks it first, and the taker verifies it at step 5. + Every confirmation the covenant already has is a block of `t_rxd` already spent. + + THE MAKER CHOOSES THAT NUMBER, which is what makes this an attack and not a rounding error. It + locks the covenant, waits, and presents the swap late. Step 3 sees a `t_rxd` that comfortably + outlasts `eth_timeout + margin`; the chain sees a refund that opens sooner by exactly the + elapsed depth. Under the OLD relation an overstated window was the conservative direction, + which is why this survived — inverting the relation (#482) turned the same arithmetic into the + direction that lets the maker refund its Radiant leg while still holding `p` for the ETH leg. + + This is the #531 conflation, which fixed the identical mistake for the burial floor and left + the ordering gate comparing against the negotiated value. + """ + + @pytest.mark.asyncio + async def test_a_covenant_that_ALREADY_SPENT_the_window_is_refused(self) -> None: + _, h = generate_secret() + # Sized to pass step 3 with 4 blocks to spare, then presented 40 blocks deep. + terms = _eth_terms(hashlock=h, eth_timeout_unix_s=_NOW + 40000, t_rxd_blocks=149) + coord = _eth_coord_negotiated(terms=terms, radiant_leg=FakeRadiantLeg(report_confs=40)) + gate = await coord.pre_btc_lock_check(terms, now_unix_s=_NOW) + assert not gate.ok + assert "REMAINING window" in gate.reason, gate.reason + assert "open too EARLY" in gate.reason, gate.reason + + @pytest.mark.asyncio + async def test_the_SAME_terms_pass_when_the_covenant_is_fresh(self) -> None: + """The paired honest path, and the reason the test above is about DEPTH and not about the + terms. Identical `t_rxd` and deadline; only the covenant's age differs. Without this, the + refusal above would be indistinguishable from a `t_rxd` that was simply too small.""" + _, h = generate_secret() + terms = _eth_terms(hashlock=h, eth_timeout_unix_s=_NOW + 40000, t_rxd_blocks=149) + coord = _eth_coord_negotiated(terms=terms, radiant_leg=FakeRadiantLeg(report_confs=1)) + gate = await coord.pre_btc_lock_check(terms, now_unix_s=_NOW) + assert "margin check failed" not in (gate.reason or ""), gate.reason diff --git a/tests/test_swap_coordinator_credential_gate.py b/tests/test_swap_coordinator_credential_gate.py index 4dcb51b2..49df6808 100644 --- a/tests/test_swap_coordinator_credential_gate.py +++ b/tests/test_swap_coordinator_credential_gate.py @@ -53,8 +53,8 @@ def _rxd_terms(*, taker_pkh: bytes, credential_ref: bytes = b""): hashlock=hashlib.sha256(os.urandom(32)).digest(), btc_sats=100_000, radiant_amount=1_000, - t_btc=t.Timelock(144, t.TimeUnit.BLOCKS), - t_rxd=t.Timelock(72, t.TimeUnit.BLOCKS), + t_btc=t.Timelock(72, t.TimeUnit.BLOCKS), + t_rxd=t.Timelock(144, t.TimeUnit.BLOCKS), asset_variant="rxd", genesis_ref=b"", taker_dest_hash=holder_hash(taker_pkh, variant="rxd"), diff --git a/tests/test_swap_gate_binding.py b/tests/test_swap_gate_binding.py index b9b8c0be..319d2572 100644 --- a/tests/test_swap_gate_binding.py +++ b/tests/test_swap_gate_binding.py @@ -413,9 +413,10 @@ async def test_claim_from_vulnerable_refuses_a_same_H_claim_tx_from_another_swap coord = await _to_both_locked(terms=terms, btc_leg=btc, radiant_leg=rxd, role=SwapRole.TAKER) ours = _real_maker_claim_tx(coord.record.btc_locator, secret.unsafe_raw_bytes()) await coord.taker_observed_reveal(ours) - # Squeeze the window so the gate routes to ASSET_VULNERABLE (t_rxd=72, burial 6). + # Squeeze the window so the gate routes to ASSET_VULNERABLE (t_rxd=144 since #482, burial 6), + # so the height that squeezes moves with it — 1_070 leaves 74 blocks and is nowhere near. btc.claim_confs = 0 - await coord.taker_scrape_and_claim_asset(ours, now_rxd_height=1_070, asset_locked_at_height=1_000) + await coord.taker_scrape_and_claim_asset(ours, now_rxd_height=1_142, asset_locked_at_height=1_000) assert coord.record.state is SwapState.ASSET_VULNERABLE foreign = _foreign_same_h_claim_tx(terms, secret.unsafe_raw_bytes()) @@ -490,7 +491,7 @@ async def test_measured_eth_policy_pins_the_lock_time_reverify_to_finalized(): longer pays it. """ secret, h = generate_secret() - terms = _eth_terms(hashlock=h, eth_timeout_unix_s=_NOW + 40_000) + terms = _eth_terms(hashlock=h, eth_timeout_unix_s=_NOW + 40_000, now_unix_s=_NOW) leg = FakeEthLeg(preimage=secret, verdict=_final()) rxd = FakeRadiantLeg() coord = SwapCoordinator( @@ -524,8 +525,8 @@ def _btc_terms_kwargs() -> dict: hashlock=hashlib.sha256(os.urandom(32)).digest(), btc_sats=100_000, radiant_amount=1_000, - t_btc=t.Timelock(144, t.TimeUnit.BLOCKS), - t_rxd=t.Timelock(72, t.TimeUnit.BLOCKS), + t_btc=t.Timelock(72, t.TimeUnit.BLOCKS), + t_rxd=t.Timelock(144, t.TimeUnit.BLOCKS), asset_variant="rxd", genesis_ref=b"", taker_dest_hash=b"\x11" * 32, diff --git a/tests/test_swap_invariants.py b/tests/test_swap_invariants.py index bc878cde..0bb3bf66 100644 --- a/tests/test_swap_invariants.py +++ b/tests/test_swap_invariants.py @@ -165,16 +165,19 @@ def _terms(t_btc_v: int, t_rxd_v: int) -> NegotiatedTerms: t_btc=st.integers(min_value=1, max_value=65535), t_rxd=st.integers(min_value=1, max_value=65535), ) -def test_I5_terms_enforce_t_btc_strictly_greater_same_unit(t_btc, t_rxd): - if t_btc > t_rxd: +def test_I5_terms_enforce_t_rxd_strictly_greater_same_unit(t_btc, t_rxd): + # INVERTED BY #482. The maker holds `p`, LOCKS the Radiant leg and CLAIMS the BTC leg, so the + # leg it LOCKS must carry the LONGER refund (Herlihy, arXiv:1801.09515 §1). Written the old way + # this property test asserted the exploitable ordering across its whole generated domain. + if t_rxd > t_btc: terms = _terms(t_btc, t_rxd) # must succeed - assert terms.t_btc.value > terms.t_rxd.value + assert terms.t_rxd.value > terms.t_btc.value else: - # t_btc <= t_rxd in the same unit must be rejected (would let the BTC - # refund open before/at the Radiant refund — the taker could be griefed). + # t_rxd <= t_btc in the same unit must be rejected: the maker's own leg would refund while + # it can still claim the counter leg with p, so it could take both. try: _terms(t_btc, t_rxd) raised = False except ValidationError: raised = True - assert raised, f"terms accepted unsafe ordering t_btc={t_btc} <= t_rxd={t_rxd}" + assert raised, f"terms accepted unsafe ordering t_rxd={t_rxd} <= t_btc={t_btc}" diff --git a/tests/test_swap_record_erc20_migration.py b/tests/test_swap_record_erc20_migration.py index 8a11f3d3..f70d2b7a 100644 --- a/tests/test_swap_record_erc20_migration.py +++ b/tests/test_swap_record_erc20_migration.py @@ -120,8 +120,8 @@ def _terms(): hashlock=hashlib.sha256(os.urandom(32)).digest(), btc_sats=100_000, radiant_amount=1_000, - t_btc=t.Timelock(144, t.TimeUnit.BLOCKS), - t_rxd=t.Timelock(72, t.TimeUnit.BLOCKS), + t_btc=t.Timelock(72, t.TimeUnit.BLOCKS), + t_rxd=t.Timelock(144, t.TimeUnit.BLOCKS), asset_variant="rxd", genesis_ref=b"", taker_dest_hash=b"\x11" * 32, diff --git a/tests/test_swap_state.py b/tests/test_swap_state.py index 6eaebd18..17539e67 100644 --- a/tests/test_swap_state.py +++ b/tests/test_swap_state.py @@ -43,7 +43,7 @@ def _xonly() -> bytes: return coincurve.PublicKeyXOnly.from_secret(os.urandom(32)).format() -def _terms(*, variant: str = "ft", t_btc_blocks: int = 144, t_rxd_blocks: int = 72) -> NegotiatedTerms: +def _terms(*, variant: str = "ft", t_btc_blocks: int = 72, t_rxd_blocks: int = 144) -> NegotiatedTerms: p = os.urandom(32) h = hashlib.sha256(p).digest() return NegotiatedTerms( @@ -240,10 +240,24 @@ def test_terms_never_carries_preimage(): def test_terms_rejects_same_unit_bad_ordering(): + """The construction guard, in the direction #482 corrected it to. + + The maker holds `p`, LOCKS the Radiant leg and CLAIMS the BTC leg, so `t_rxd` must strictly + exceed `t_btc`. The refused pairs are therefore t_rxd <= t_btc. The second case here used to + be `(t_btc=50, t_rxd=72)` — which is now the LEGITIMATE ordering, and is asserted as such in + the honest-path test below rather than deleted, since a refusal test whose input became valid + is exactly the kind that goes green by vacuity if it is merely renumbered. + """ with pytest.raises(ValidationError): - _terms(t_btc_blocks=72, t_rxd_blocks=72) # t_btc <= t_rxd + _terms(t_btc_blocks=72, t_rxd_blocks=72) # equal is not "exceeds" with pytest.raises(ValidationError): - _terms(t_btc_blocks=50, t_rxd_blocks=72) + _terms(t_btc_blocks=72, t_rxd_blocks=50) # t_rxd shorter than t_btc + + +def test_terms_ACCEPTS_the_ordering_the_old_guard_refused(): + """The paired honest path. `t_btc=50, t_rxd=72` is the safe layout and must construct.""" + terms = _terms(t_btc_blocks=50, t_rxd_blocks=72) + assert terms.t_rxd.value > terms.t_btc.value def test_terms_rejects_seconds_t_rxd_f002(): @@ -272,8 +286,8 @@ def test_terms_rejects_short_hashlock(): hashlock=b"\x00" * 31, btc_sats=1, radiant_amount=1, - t_btc=t.Timelock(144, t.TimeUnit.BLOCKS), - t_rxd=t.Timelock(72, t.TimeUnit.BLOCKS), + t_btc=t.Timelock(72, t.TimeUnit.BLOCKS), + t_rxd=t.Timelock(144, t.TimeUnit.BLOCKS), asset_variant="rxd", genesis_ref=b"", taker_dest_hash=b"\x11" * 32, @@ -289,8 +303,8 @@ def test_terms_ft_requires_genesis_ref(): hashlock=hashlib.sha256(b"x").digest(), btc_sats=1, radiant_amount=1, - t_btc=t.Timelock(144, t.TimeUnit.BLOCKS), - t_rxd=t.Timelock(72, t.TimeUnit.BLOCKS), + t_btc=t.Timelock(72, t.TimeUnit.BLOCKS), + t_rxd=t.Timelock(144, t.TimeUnit.BLOCKS), asset_variant="ft", genesis_ref=b"", # missing taker_dest_hash=b"\x11" * 32, @@ -358,7 +372,7 @@ def _eth_terms(*, value_wei: int = 10**15, eth_timeout_unix_s: int = 1779710245) btc_sats=100_000, radiant_amount=1_000, t_btc=t.Timelock(7200, t.TimeUnit.SECONDS), # ETH refund duration; skips same-unit guard - t_rxd=t.Timelock(72, t.TimeUnit.BLOCKS), + t_rxd=t.Timelock(144, t.TimeUnit.BLOCKS), asset_variant="rxd", genesis_ref=b"", taker_dest_hash=b"\x11" * 32, @@ -396,7 +410,7 @@ def test_eth_terms_reject_nonzero_btc_xonly(): btc_sats=100_000, radiant_amount=1_000, t_btc=t.Timelock(7200, t.TimeUnit.SECONDS), - t_rxd=t.Timelock(72, t.TimeUnit.BLOCKS), + t_rxd=t.Timelock(144, t.TimeUnit.BLOCKS), asset_variant="rxd", genesis_ref=b"", taker_dest_hash=b"\x11" * 32, @@ -423,8 +437,8 @@ def test_bad_counter_chain_rejected(): hashlock=hashlib.sha256(os.urandom(32)).digest(), btc_sats=1_000, radiant_amount=1, - t_btc=t.Timelock(144, t.TimeUnit.BLOCKS), - t_rxd=t.Timelock(72, t.TimeUnit.BLOCKS), + t_btc=t.Timelock(72, t.TimeUnit.BLOCKS), + t_rxd=t.Timelock(144, t.TimeUnit.BLOCKS), asset_variant="rxd", genesis_ref=b"", taker_dest_hash=b"\x11" * 32, diff --git a/tests/test_t_rxd_sizer_and_gate_agree.py b/tests/test_t_rxd_sizer_and_gate_agree.py index a6d64abd..1cafc922 100644 --- a/tests/test_t_rxd_sizer_and_gate_agree.py +++ b/tests/test_t_rxd_sizer_and_gate_agree.py @@ -58,7 +58,7 @@ def _policy(fast: float = 36.0, nominal: float = 300.0) -> MarginPolicy: ) -def _analytic_largest(budget: int, interval: float) -> int: +def _analytic_smallest(budget: int, interval: float) -> int: """The largest t the gate can accept, derived from its DOCUMENTED semantics — not by asking it. The gate refuses when `now + wait + ceil(t * interval) >= eth_timeout - margin`, i.e. accepts @@ -83,18 +83,27 @@ def _analytic_largest(budget: int, interval: float) -> int: """ from fractions import Fraction - return math.floor(Fraction(budget - 1) / Fraction(interval)) + # SMALLEST, not largest (#482). The gate bounds t_rxd from BELOW — the refund must open at or + # after `eth_timeout + margin` — so the boundary value is `ceil(budget / interval)`, the least + # t with `t * interval >= budget`. It was `floor((budget - 1) / interval)`, the greatest t + # with `t * interval < budget`, which is the boundary of the opposite inequality. + return math.ceil(Fraction(budget) / Fraction(interval)) def _gate_accepts(t_rxd: int, interval: float, wait: int = 0) -> bool: try: + # ANCHOR AT THE LOCK TIME, with a zero wait. The gate used to ADD + # `max_covenant_confirm_wait_s` to `now`, so passing it here was the same instant as the + # sizer's `expected_rxd_lock_time_unix_s = _NOW + wait`. #482 removed that term from the + # arithmetic — adding it now would assume a LATE confirm, which is the optimistic + # direction — so the two anchors have to be written the same way to stay the same instant. assert_covenant_confirms_before_eth_deadline( - now_unix_s=_NOW, + now_unix_s=_NOW + wait, eth_timeout_unix_s=_NOW + _ETH_TIMEOUT_S, margin=_margin(), t_rxd=bt.Timelock(t_rxd, bt.TimeUnit.BLOCKS), rxd_block_interval_s=interval, - max_covenant_confirm_wait_s=wait, + max_covenant_confirm_wait_s=0, ) return True except Exception: @@ -119,14 +128,20 @@ def test_the_gate_ACCEPTS_what_the_sizer_produces(self) -> None: def test_the_MISMATCH_is_what_breaks_it(self) -> None: """The defect, as a test. Sizing at the fast tail and checking at the nominal refuses the sizer's own output — which is how a runner-side cap came to shorten the window ~8x.""" + # THE DANGEROUS PAIRING SWAPPED ENDS WITH THE RELATION (#482). Sizing at the FAST tail and + # checking at the NOMINAL used to refuse, because a bigger interval overshot an upper + # bound. The gate bounds t_rxd from below now, so that pairing OVER-satisfies and passes. + # What refuses today is the reverse: size at the nominal, check at the fast tail, and the + # window is far too short. Same defect, opposite arrangement — a test left as it was would + # have gone quietly green while the mismatch it exists for was still reachable. fast, nominal = 36.0, 300.0 sized = eth_absolute_to_rxd_relative_blocks( eth_timeout_unix_s=_NOW + _ETH_TIMEOUT_S, expected_rxd_lock_time_unix_s=_NOW, margin=_margin(), - rxd_block_interval_s=fast, + rxd_block_interval_s=nominal, ).value - assert not _gate_accepts(sized, nominal), ( + assert not _gate_accepts(sized, fast), ( "expected the mismatched pairing to refuse — if this passes, the two intervals have " "converged and this test no longer describes anything" ) @@ -338,9 +353,13 @@ class TestTheExactDivisionBoundary: """ def test_the_gate_ACCEPTS_the_sizer_output_when_the_budget_divides_EXACTLY(self) -> None: - wait = 600 + # 624, not 600: the budget gained `+ margin` instead of `- margin` when the relation was + # inverted (#482), which moved the exact quotient. The precondition assert below is what + # caught it — a test that needs an exact division has to re-derive the constant, not keep + # the one that used to produce one. + wait = 624 fast = _dividing_interval_s(_policy()) - budget = _ETH_TIMEOUT_S - _margin().total_s() - wait + budget = _ETH_TIMEOUT_S + _margin().total_s() - wait assert budget % fast == 0, ( f"this test is only meaningful on an exact quotient; budget/interval = {budget / fast}" ) @@ -358,11 +377,14 @@ def test_the_gate_ACCEPTS_the_sizer_output_when_the_budget_divides_EXACTLY(self) # this is the independent half: on an exact quotient q, the largest acceptable value is # exactly q - 1, derived from the gate's documented semantics without calling either # function. A joint sizer+gate drift keeps the line above green and fails this one. - assert sized == budget // int(fast) - 1 == _analytic_largest(budget, fast) - - def test_the_sized_value_is_still_the_LARGEST_the_gate_accepts(self) -> None: - """The paired honest-path check. Fixing a refusal by shrinking the answer would also pass - the test above while quietly handing the taker a shorter claim window every run.""" + # On an exact quotient q the SMALLEST acceptable value is exactly q (not q - 1): q blocks + # reach the deadline precisely, and equality satisfies a `>=` bound. + assert sized == budget // int(fast) == _analytic_smallest(budget, fast) + + def test_the_sized_value_is_still_the_SMALLEST_the_gate_accepts(self) -> None: + """The paired honest-path check, inverted with the relation (#482). The give-away is now + upward: satisfying the gate by GROWING t_rxd would also pass the test above, while locking + the maker's asset longer than the swap needs on every run.""" wait = 600 fast = _dividing_interval_s(_policy()) sized = eth_absolute_to_rxd_relative_blocks( @@ -371,9 +393,9 @@ def test_the_sized_value_is_still_the_LARGEST_the_gate_accepts(self) -> None: margin=_margin(), rxd_block_interval_s=fast, ).value - assert not _gate_accepts(sized + 1, fast, wait=wait), ( - f"t_rxd={sized + 1} is also accepted, so the sizer gave away a block of the taker's " - f"claim window for nothing" + assert not _gate_accepts(sized - 1, fast, wait=wait), ( + f"t_rxd={sized - 1} is also accepted, so the sizer locked the maker's asset a block " + f"longer than the deadline requires" ) @pytest.mark.parametrize("wait", [0, 300, 600, 900, 1200]) @@ -384,12 +406,12 @@ def test_across_the_parameter_grid_the_sizer_is_exactly_the_gate_boundary(self, The accept/refuse pair pins sizer↔gate AGREEMENT, but the sizer reaches its answer by asking the gate, so agreement alone survives any change that moves both together — it is - sharp at whatever boundary the gate currently has, right or wrong. `_analytic_largest` + sharp at whatever boundary the gate currently has, right or wrong. `_analytic_smallest` restates where that boundary is SUPPOSED to be (exact rational arithmetic, no production - code), so the three assertions jointly refuse: a sizer that overshoots (gate refuses it), - a sizer that undershoots (sized+1 also accepted), and a sizer and gate that drifted in + code), so the three assertions jointly refuse: a sizer that undershoots (gate refuses it), + a sizer that overshoots (sized-1 also accepted), and a sizer and gate that drifted in lockstep (analytic value disagrees).""" - budget = _ETH_TIMEOUT_S - _margin().total_s() - wait + budget = _ETH_TIMEOUT_S + _margin().total_s() - wait sized = eth_absolute_to_rxd_relative_blocks( eth_timeout_unix_s=_NOW + _ETH_TIMEOUT_S, expected_rxd_lock_time_unix_s=_NOW + wait, @@ -397,9 +419,9 @@ def test_across_the_parameter_grid_the_sizer_is_exactly_the_gate_boundary(self, rxd_block_interval_s=fast, ).value assert _gate_accepts(sized, fast, wait=wait), f"gate refused sized t_rxd={sized}" - assert not _gate_accepts(sized + 1, fast, wait=wait), f"t_rxd={sized + 1} also accepted" - assert sized == _analytic_largest(budget, fast), ( - f"sized {sized} != analytic largest-acceptable {_analytic_largest(budget, fast)} for " + assert not _gate_accepts(sized - 1, fast, wait=wait), f"t_rxd={sized - 1} also accepted" + assert sized == _analytic_smallest(budget, fast), ( + f"sized {sized} != analytic smallest-acceptable {_analytic_smallest(budget, fast)} for " f"budget {budget}s at {fast}s/block — the sizer and the gate agree with each other " "but not with the documented boundary, i.e. they drifted together" ) diff --git a/tests/test_taker_asset_funding_gate_adversarial.py b/tests/test_taker_asset_funding_gate_adversarial.py index 8cf145ac..aeb07371 100644 --- a/tests/test_taker_asset_funding_gate_adversarial.py +++ b/tests/test_taker_asset_funding_gate_adversarial.py @@ -159,7 +159,7 @@ def _xonly(kp: BtcKeypair) -> bytes: return coincurve.PublicKeyXOnly.from_secret(kp._privkey.unsafe_raw_bytes()).format() -def _terms(*, maker_kp: BtcKeypair, taker_kp: BtcKeypair, t_rxd_blocks: int = 20) -> NegotiatedTerms: +def _terms(*, maker_kp: BtcKeypair, taker_kp: BtcKeypair, t_rxd_blocks: int = 80) -> NegotiatedTerms: """Terms whose dest hashes come from the REAL covenant, so the real leg binds to them.""" hashlock = hashlib.sha256(os.urandom(32)).digest() cov = build_htlc_covenant_rxd( @@ -173,7 +173,9 @@ def _terms(*, maker_kp: BtcKeypair, taker_kp: BtcKeypair, t_rxd_blocks: int = 20 hashlock=hashlock, btc_sats=_BTC_SATS, radiant_amount=_RXD_AMOUNT, - t_btc=t.Timelock(144, t.TimeUnit.BLOCKS), + # DERIVED from the covenant's own CSV (#482): t_rxd IS `refund_csv` here, and it defaults + # to 20, so a fixed t_btc of 72 makes the terms unconstructible under the inverted rule. + t_btc=t.Timelock(max(1, t_rxd_blocks // 2), t.TimeUnit.BLOCKS), t_rxd=t.Timelock(t_rxd_blocks, t.TimeUnit.BLOCKS), asset_variant="rxd", genesis_ref=b"", diff --git a/tests/test_timelock_ordering_invariant.py b/tests/test_timelock_ordering_invariant.py new file mode 100644 index 00000000..1e75ee8e --- /dev/null +++ b/tests/test_timelock_ordering_invariant.py @@ -0,0 +1,154 @@ +"""The cross-chain timelock invariant, stated once (#482). + +Herlihy, *Atomic Cross-Chain Swaps* (arXiv:1801.09515) §1, verbatim: Alice creates the secret and +publishes a contract with timelock **6∆**; Bob publishes at **5∆**; Carol at **4∆**; then "Alice +sends s to Carol's contract". So the secret generator LOCKS the longest-dated leg and CLAIMS the +shortest-dated one, and Lemma 4.13 fixes the gap: + + the timeout on each arc (u, v) is later by at least ∆ than the timeout on each arc (v, w). + +Our maker generates ``p``, LOCKS the Radiant covenant and CLAIMS the counter leg. Therefore +``t_rxd`` is the LONGER leg. The code enforced the reverse until this change, which handed the +maker a window — at least ``margin`` wide by construction — in which to refund the covenant while +``p`` was still secret and then claim the counter leg. + +This file is deliberately small and states the rule ONCE. Every other suite exercises swaps built +from that rule; this one is where the rule itself lives. +""" + +from __future__ import annotations + +import pytest + +from pyrxd.btc_wallet import taproot as t +from pyrxd.gravity.swap_coordinator import MarginPolicy, assert_timelock_margin +from pyrxd.security.errors import ValidationError + +MARGIN = 36 + + +def _policy(margin: int = MARGIN) -> MarginPolicy: + return MarginPolicy(margin=t.Timelock(margin, t.TimeUnit.BLOCKS), block_interval_s=600.0, is_measured=False) + + +def _blk(n: int) -> t.Timelock: + return t.Timelock(n, t.TimeUnit.BLOCKS) + + +class TestTheRule: + def test_the_leg_the_maker_LOCKED_must_outlast_the_leg_it_CLAIMS(self) -> None: + assert_timelock_margin(_blk(72), _blk(72 + MARGIN), _policy()) + + def test_the_OLD_direction_is_now_refused(self) -> None: + """The regression that matters. Under `t_btc > t_rxd` the maker refunds the covenant while + `p` is still secret and then claims the counter leg — both legs, deterministically, with + the taker unable to claim (no `p`) or refund (its deadline is later).""" + with pytest.raises(ValidationError, match="ordering violated"): + assert_timelock_margin(_blk(144), _blk(72), _policy()) + + def test_equality_is_not_enough(self) -> None: + with pytest.raises(ValidationError, match="ordering violated"): + assert_timelock_margin(_blk(100), _blk(100), _policy()) + + def test_a_gap_smaller_than_the_margin_is_refused(self) -> None: + """Δ exists because the taker needs time AFTER the reveal — the maker may reveal as late as + its own deadline.""" + with pytest.raises(ValidationError, match="insufficient margin"): + assert_timelock_margin(_blk(72), _blk(72 + MARGIN - 1), _policy()) + + def test_exactly_the_margin_passes(self) -> None: + """Boundary, inclusive. A guard that refuses valid work is a bug, and this one sits on a + parameter an honest maker must choose.""" + assert_timelock_margin(_blk(72), _blk(72 + MARGIN), _policy()) + + +class TestTheHerlihyExample: + """The paper's own numbers, mapped onto two parties. + + Alice locks 6∆ and claims a 4∆ leg. Collapsing the three-party cycle to two, the secret + holder's locked leg keeps the larger timeout and the claimed leg the smaller. + """ + + @pytest.mark.parametrize("delta", [1, 6, 144]) + def test_locked_6delta_claimed_4delta_holds_at_any_scale(self, delta: int) -> None: + pol = _policy(margin=2 * delta) + assert_timelock_margin(_blk(4 * delta), _blk(6 * delta), pol) + + @pytest.mark.parametrize("delta", [1, 6, 144]) + def test_the_mirror_image_is_refused_at_any_scale(self, delta: int) -> None: + pol = _policy(margin=2 * delta) + with pytest.raises(ValidationError): + assert_timelock_margin(_blk(6 * delta), _blk(4 * delta), pol) + + +def test_the_theft_window_no_longer_exists() -> None: + """The adversarial scenario as a test, not a comment. + + Under the old relation `[rxd_refund_opens, counter_deadline]` was non-empty and at least + `margin` wide, and the maker could act inside it. Under the corrected relation the counter + deadline comes FIRST, so the window is empty by construction: by the time the maker's own + refund opens, the taker has already been able to refund the counter leg. + """ + t_btc, t_rxd = 72, 72 + MARGIN + assert_timelock_margin(_blk(t_btc), _blk(t_rxd), _policy()) + assert t_btc < t_rxd, "the counter leg must expire FIRST" + assert t_rxd - t_btc >= MARGIN, "and by at least Δ, so the taker has time after the reveal" + + +class TestTheSizingAnchor: + """The anchor inverted with the relation, and the two runners disagreed about it (#482). + + `t_rxd` is committed in the covenant script BEFORE broadcast, and the refund opens at + `actual_confirm + t_rxd`. The invariant is `actual_confirm + t_rxd >= counter_deadline + + margin`, so: + + confirm LATER than assumed -> refund opens later -> MORE margin -> safe + confirm EARLIER than assumed -> refund opens sooner -> invariant BREAKS + + The conservative anchor is therefore the EARLIEST plausible confirm — `now`. Reserving a + confirm allowance was correct under the OLD relation, where a LATE confirm pushed the refund + past the deadline, and is exactly wrong under this one. + """ + + def test_both_runners_anchor_on_now(self) -> None: + """They disagreed: `eth_swap_run` reserved `max_covenant_confirm_wait_s` while + `eth_swap_two_host` used `now`. Under the inversion that made one of them unsafe, and + nothing reconciled them — so the agreement is pinned rather than left to convention.""" + import pathlib + import re + + root = pathlib.Path(__file__).resolve().parent.parent / "scripts" + for name in ("eth_swap_run.py", "eth_swap_two_host.py"): + src = (root / name).read_text() + anchors = re.findall(r"expected_rxd_lock_time_unix_s=([^,\n]+)", src) + assert anchors, f"{name} no longer sizes a t_rxd — check this test still has a subject" + for anchor in anchors: + assert "max_covenant_confirm_wait" not in anchor, ( + f"{name} anchors the sizing on a LATE confirm ({anchor.strip()}). Under the " + "inverted relation an early confirm is what breaks the invariant, so the " + "anchor must be the earliest plausible confirm." + ) + assert "time.time()" in anchor, f"{name} anchors on {anchor.strip()!r}, expected now" + + def test_an_earlier_confirm_than_assumed_is_the_unsafe_direction(self) -> None: + """The property behind the rule, arithmetic only — no chain, no sizing call. + + Anchoring on `now` makes every actual confirm at-or-after the anchor, so the refund can + only open LATER than planned. Anchoring on a late confirm admits actual < assumed, which + moves it earlier and eats the margin. + """ + counter_deadline, margin, interval = 10_000, 1_000, 10 + now = 0 + + # Anchored on `now`: t_rxd sized so the refund opens exactly at deadline + margin. + t_rxd_now = (counter_deadline + margin - now) // interval + for actual_confirm in (0, 50, 500): # never earlier than the anchor + assert actual_confirm + t_rxd_now * interval >= counter_deadline + margin + + # Anchored on a LATE confirm: a fast chain confirms sooner and the invariant fails. + assumed_late = 500 + t_rxd_late = (counter_deadline + margin - assumed_late) // interval + assert 0 + t_rxd_late * interval < counter_deadline + margin, ( + "a confirm earlier than the assumed one must break the invariant — if it does not, " + "this test is not exercising the direction it claims" + ) diff --git a/tests/test_watch_adapters.py b/tests/test_watch_adapters.py index c81e685a..c9517931 100644 --- a/tests/test_watch_adapters.py +++ b/tests/test_watch_adapters.py @@ -38,8 +38,8 @@ def _terms() -> NegotiatedTerms: hashlock=hashlib.sha256(os.urandom(32)).digest(), btc_sats=100_000, radiant_amount=1_000, - t_btc=t.Timelock(144, t.TimeUnit.BLOCKS), - t_rxd=t.Timelock(72, t.TimeUnit.BLOCKS), + t_btc=t.Timelock(72, t.TimeUnit.BLOCKS), + t_rxd=t.Timelock(144, t.TimeUnit.BLOCKS), asset_variant="ft", genesis_ref=b"\xaa" * 36, taker_dest_hash=b"\x11" * 32, diff --git a/tests/test_watch_claim_executor.py b/tests/test_watch_claim_executor.py index f6756a15..398375a9 100644 --- a/tests/test_watch_claim_executor.py +++ b/tests/test_watch_claim_executor.py @@ -48,8 +48,8 @@ def _terms(*, maker_kp, taker_kp, hashlock, variant="rxd", radiant_amount=1_000) hashlock=hashlock, btc_sats=100_000, radiant_amount=radiant_amount, - t_btc=t.Timelock(144, t.TimeUnit.BLOCKS), - t_rxd=t.Timelock(72, t.TimeUnit.BLOCKS), + t_btc=t.Timelock(72, t.TimeUnit.BLOCKS), + t_rxd=t.Timelock(144, t.TimeUnit.BLOCKS), asset_variant=variant, genesis_ref=b"" if variant == "rxd" else (b"\xab" * 32 + b"\x00\x00\x00\x00"), taker_dest_hash=b"\x11" * 32, @@ -486,8 +486,10 @@ async def test_covenant_already_spent_is_idempotent_declined(): async def test_fresh_reassess_squeezed_declines(): # t_rxd window already closed at fresh read: funded_h far below now so blocks_left < burial. terms, _p, raw, claim_txid, locator, _btc_leg = await _build_real_claim() - # now_rxd = funded_h + confs - 1 = 100 + 100 - 1 = 199 > refund_opens (100 + 72 = 172) → SQUEEZED. - chain_io = _FakeChainIO(value=terms.radiant_amount, funded_h=100, confs=100) + # now_rxd = funded_h + confs - 1 = 100 + 172 - 1 = 271 > refund_opens (100 + 144 = 244) → SQUEEZED. + # The numbers moved with t_rxd, which #482 raised from 72 to 144; at the old confs=100 the + # window is still wide open and this test asserts a SQUEEZE that does not happen. + chain_io = _FakeChainIO(value=terms.radiant_amount, funded_h=100, confs=172) leg = _FakeRadiantLeg(chain_io) ex = ClaimExecutor( resolve_leg=_resolver(leg), @@ -614,9 +616,11 @@ async def _flaky(record, preimage): async def test_corroborated_deeper_depth_flips_safe_to_squeezed(): - # Single node says confs=1 → now_rxd=100 → SAFE. A corroborator reporting a DEEPER depth (100) makes - # now_rxd=199 > refund_opens(172) → SQUEEZED → DECLINED. The deeper (MAX) read can't false-SAFE. - ex, leg, rec, _ = await _armed_executor(confs=1, rxd_depth_corroborator=_FakeDepthCorroborator(depth=100)) + # Single node says confs=1 → now_rxd=100 → SAFE. A corroborator reporting a DEEPER depth (172) + # makes now_rxd=271 > refund_opens(244) → SQUEEZED → DECLINED. The deeper (MAX) read can't + # false-SAFE. The corroborated depth scales with t_rxd (72 → 144 at #482); at the old 100 the + # deeper read is still SAFE and this test proves nothing about the flip it is named for. + ex, leg, rec, _ = await _armed_executor(confs=1, rxd_depth_corroborator=_FakeDepthCorroborator(depth=172)) assert await ex.execute("s1", rec, _claim_decision()) is ExecOutcome.DECLINED assert leg.claimed_with is None diff --git a/tests/test_watch_decide.py b/tests/test_watch_decide.py index d7421ffc..16ef0142 100644 --- a/tests/test_watch_decide.py +++ b/tests/test_watch_decide.py @@ -32,7 +32,11 @@ def _xonly() -> bytes: return coincurve.PublicKeyXOnly.from_secret(os.urandom(32)).format() -def _btc_terms(*, t_btc_blocks: int = 144, t_rxd_blocks: int = 72) -> NegotiatedTerms: +# t_rxd KEEPS its 72 and t_btc drops to 36 (#482). Inverting the usual 72/144 pair the other +# way would have moved REFUND_OPENS from 172 to 244 and invalidated every height constant in +# this file — dozens of `now=` literals chosen relative to it. Shrinking t_btc satisfies the +# inverted ordering while leaving the heights, which are what these tests are about, alone. +def _btc_terms(*, t_btc_blocks: int = 36, t_rxd_blocks: int = 72) -> NegotiatedTerms: p = os.urandom(32) return NegotiatedTerms( hashlock=hashlib.sha256(p).digest(), @@ -55,7 +59,9 @@ def _eth_terms() -> NegotiatedTerms: hashlock=hashlib.sha256(p).digest(), btc_sats=100_000, radiant_amount=1_000, - t_btc=t.Timelock(144, t.TimeUnit.BLOCKS), + # Same reasoning as `_btc_terms` above: t_rxd stays 72 so REFUND_OPENS stays 172, and + # t_btc drops to 36 to satisfy the inverted ordering (#482). + t_btc=t.Timelock(36, t.TimeUnit.BLOCKS), t_rxd=t.Timelock(72, t.TimeUnit.BLOCKS), asset_variant="ft", genesis_ref=b"\xaa" * 36, diff --git a/tests/test_watch_quorum.py b/tests/test_watch_quorum.py index 3e588dc0..9febe1df 100644 --- a/tests/test_watch_quorum.py +++ b/tests/test_watch_quorum.py @@ -35,8 +35,8 @@ def _terms() -> NegotiatedTerms: hashlock=hashlib.sha256(os.urandom(32)).digest(), btc_sats=100_000, radiant_amount=1_000, - t_btc=t.Timelock(144, t.TimeUnit.BLOCKS), - t_rxd=t.Timelock(72, t.TimeUnit.BLOCKS), + t_btc=t.Timelock(72, t.TimeUnit.BLOCKS), + t_rxd=t.Timelock(144, t.TimeUnit.BLOCKS), asset_variant="ft", genesis_ref=b"\xaa" * 36, taker_dest_hash=b"\x11" * 32, @@ -197,8 +197,8 @@ def _eth_terms() -> NegotiatedTerms: hashlock=hashlib.sha256(os.urandom(32)).digest(), btc_sats=100_000, radiant_amount=1_000, - t_btc=t.Timelock(144, t.TimeUnit.BLOCKS), - t_rxd=t.Timelock(72, t.TimeUnit.BLOCKS), + t_btc=t.Timelock(72, t.TimeUnit.BLOCKS), + t_rxd=t.Timelock(144, t.TimeUnit.BLOCKS), asset_variant="ft", genesis_ref=b"\xaa" * 36, taker_dest_hash=b"\x11" * 32, diff --git a/tests/test_watch_reconciler.py b/tests/test_watch_reconciler.py index d473ca02..b363c836 100644 --- a/tests/test_watch_reconciler.py +++ b/tests/test_watch_reconciler.py @@ -30,8 +30,8 @@ def _terms() -> NegotiatedTerms: hashlock=hashlib.sha256(os.urandom(32)).digest(), btc_sats=100_000, radiant_amount=1_000, - t_btc=t.Timelock(144, t.TimeUnit.BLOCKS), - t_rxd=t.Timelock(72, t.TimeUnit.BLOCKS), + t_btc=t.Timelock(72, t.TimeUnit.BLOCKS), + t_rxd=t.Timelock(144, t.TimeUnit.BLOCKS), asset_variant="ft", genesis_ref=b"\xaa" * 36, taker_dest_hash=b"\x11" * 32, diff --git a/tests/test_watch_v2_execute_invariants.py b/tests/test_watch_v2_execute_invariants.py index c551806a..7fca00e1 100644 --- a/tests/test_watch_v2_execute_invariants.py +++ b/tests/test_watch_v2_execute_invariants.py @@ -51,12 +51,20 @@ def _xonly(sk: coincurve.PrivateKey) -> bytes: def _terms(*, btc_sats: int = 5_000, t_btc: t.Timelock | None = None) -> NegotiatedTerms: + _t_btc = t_btc or t.Timelock(72, t.TimeUnit.BLOCKS) return NegotiatedTerms( hashlock=_H, btc_sats=btc_sats, radiant_amount=1_000, - t_btc=t_btc or t.Timelock(144, t.TimeUnit.BLOCKS), - t_rxd=t.Timelock(72, t.TimeUnit.BLOCKS), + t_btc=_t_btc, + # DERIVED, not a literal (#482). t_rxd must strictly exceed t_btc, and callers pass t_btc + # freely (144, 200, a SECONDS value). A fixed t_rxd collides with whichever of those + # happens to match it — which is exactly what a hardcoded 144 did here. + t_rxd=( + t.Timelock(_t_btc.value + 72, t.TimeUnit.BLOCKS) + if _t_btc.unit is t.TimeUnit.BLOCKS + else t.Timelock(144, t.TimeUnit.BLOCKS) + ), asset_variant="ft", genesis_ref=b"\xaa" * 36, taker_dest_hash=b"\x11" * 32, @@ -385,10 +393,13 @@ def _decide(rec, obs): def test_btc_locked_refunds_only_when_funding_matured(): - rec = _btc_rec() # t_btc = 144 blocks + # t_btc = 72 blocks. It was 144 before #482 inverted the pair; the confirmation counts here + # are RELATIVE to it, so they move with it — 100 confirmations used to be immature and is now + # well past maturity, which is what this test would otherwise assert the opposite of. + rec = _btc_rec() assert _decide(rec, _obs(btc_funding_confirmations=None)).intent is Intent.WATCH - assert _decide(rec, _obs(btc_funding_confirmations=100)).intent is Intent.WATCH - d = _decide(rec, _obs(btc_funding_confirmations=144)) + assert _decide(rec, _obs(btc_funding_confirmations=71)).intent is Intent.WATCH + d = _decide(rec, _obs(btc_funding_confirmations=72)) assert d.intent is Intent.PAGE_REFUND assert d.recommended_action == "taker_refund_btc" and d.autonomous_btc_refund is True diff --git a/tests/test_watchtower_dust_run_harness.py b/tests/test_watchtower_dust_run_harness.py index 4152d6c3..434af920 100644 --- a/tests/test_watchtower_dust_run_harness.py +++ b/tests/test_watchtower_dust_run_harness.py @@ -31,10 +31,11 @@ import watchtower_dust_run as harness -# A standard P2WPKH refund scriptPubKey for tests. The MAINNET run uses 2000 sats / t_btc=2 / t_rxd=1, -# so the unit tests exercise that exact config. +# A standard P2WPKH refund scriptPubKey for tests. The MAINNET run used 2000 sats with the BTC leg +# as the LONGER one (t_btc=2 / t_rxd=1) — the ordering #482 corrected. The legs are swapped here to +# the layout the tower now requires; the amounts and everything else stay as the run had them. _REFUND_SPK_HEX = "0014" + "11" * 20 -_RUN = dict(btc_sats=2000, t_btc=2, t_rxd=1, network="bcrt") +_RUN = dict(btc_sats=2000, t_btc=1, t_rxd=2, network="bcrt") def _setup(tmp_path: Path, **overrides) -> Path: @@ -119,9 +120,11 @@ def test_setup_refuses_a_nonstandard_refund_spk(tmp_path): assert not (tmp_path / "run.state.json").exists() -def test_setup_refuses_when_t_btc_not_longer_than_t_rxd(tmp_path): +def test_setup_refuses_when_t_rxd_not_longer_than_t_btc(tmp_path): with pytest.raises(SystemExit): - _setup(tmp_path, t_btc=2, t_rxd=2) + _setup(tmp_path, t_btc=2, t_rxd=2) # equal is not 'exceeds' + with pytest.raises(SystemExit): + _setup(tmp_path, t_btc=2, t_rxd=1) # the OLD ordering, now refused async def test_harness_artifacts_satisfy_every_executor_bind(tmp_path):