From 1f53b9e3f304e8c1d4a517d25900278a8db4ac0a Mon Sep 17 00:00:00 2001 From: rene0422 Date: Fri, 15 May 2026 16:08:24 -0700 Subject: [PATCH] fix: bind SwapConfirm to reserve-time commitment (#61) --- allways/validator/axon_handlers.py | 40 +++++++++++++++-- tests/test_axon_handlers.py | 70 ++++++++++++++++++++++++++++++ 2 files changed, 107 insertions(+), 3 deletions(-) diff --git a/allways/validator/axon_handlers.py b/allways/validator/axon_handlers.py index 08e35b0a..e9d48ca7 100644 --- a/allways/validator/axon_handlers.py +++ b/allways/validator/axon_handlers.py @@ -17,7 +17,7 @@ from allways.classes import MinerPair from allways.commitments import read_miner_commitment -from allways.constants import RESERVATION_COOLDOWN_BLOCKS +from allways.constants import MAX_EXTENSION_BLOCKS, RESERVATION_COOLDOWN_BLOCKS, RESERVATION_TTL_BLOCKS from allways.contract_client import AllwaysContractClient, ContractError from allways.synapses import MinerActivateSynapse, SwapConfirmSynapse, SwapReserveSynapse from allways.utils.proofs import reserve_proof_message, swap_proof_message @@ -117,17 +117,28 @@ def resolve_swap_direction( return from_chain, to_chain, deposit_addr, fulfillment_addr, rate, rate_str -def load_swap_commitment(validator: 'Validator', miner_hotkey: str) -> Optional[MinerPair]: +def load_swap_commitment( + validator: 'Validator', + miner_hotkey: str, + block: Optional[int] = None, +) -> Optional[MinerPair]: """Read miner commitment and validate chains differ. Returns commitment or None. Passes the validator's cached metagraph so read_miner_commitment skips the full subnet metagraph download — that sync takes 30s+ on testnet finney and was the real source of axon handler timeouts. + + When ``block`` is supplied the commitment is read at that historical + block — used by swap-confirm to bind the confirm vote to the same + commitment that was active at reserve time, so a miner can't redirect + a user's fulfillment by changing their commitment between reserve and + confirm. """ commitment = read_miner_commitment( subtensor=validator.axon_subtensor, netuid=validator.config.netuid, hotkey=miner_hotkey, + block=block, metagraph=validator.metagraph, ) if commitment is None or commitment.from_chain == commitment.to_chain: @@ -452,7 +463,30 @@ async def handle_swap_confirm( res_tao_amount, res_source_amount, res_dest_amount = res_data - commitment = load_swap_commitment(validator, miner) + # Bind the confirm to the commitment that was active at reserve + # time, not the live one. Without this, a miner could republish + # their commitment after the user signed and sent funds, and the + # confirm would validate against the new addresses/rate — making + # verify_transaction reject a perfectly-valid user tx (and shifting + # the swap's miner_to_address / rate that vote_initiate commits to). + # + # The on-chain reservation does not store its creation block, but + # ``reserved_until - RESERVATION_TTL_BLOCKS - extension_count * + # MAX_EXTENSION_BLOCKS`` is a safe lower bound: it lands at-or- + # before the original vote_reserve and is exact for un-extended + # reservations (the common case). Reading commitment at this block + # returns the terms the user reserved under whenever the miner has + # not republished in the interim. + try: + extension_count = contract.get_reservation_extension_count(miner) + except ContractError: + extension_count = 0 + reservation_block = max( + 1, + reserved_until - RESERVATION_TTL_BLOCKS - extension_count * MAX_EXTENSION_BLOCKS, + ) + + commitment = load_swap_commitment(validator, miner, block=reservation_block) if commitment is None: reject_synapse(synapse, 'No valid commitment', ctx) return synapse diff --git a/tests/test_axon_handlers.py b/tests/test_axon_handlers.py index fc52708d..5eed386d 100644 --- a/tests/test_axon_handlers.py +++ b/tests/test_axon_handlers.py @@ -97,6 +97,7 @@ def make_validator( contract = MagicMock() contract.get_miner_reserved_until.return_value = reserved_until contract.get_reservation_data.return_value = reservation_data + contract.get_reservation_extension_count.return_value = 0 validator.axon_contract_client = contract if providers is None: @@ -328,6 +329,75 @@ def test_queued_entry_uses_reservation_amounts(self): # --------------------------------------------------------------------------- +class TestCommitmentBoundToReserveBlock: + """Issue #61: confirm must read commitment as-of reserve time, not live. + + Without this binding, a miner could republish their commitment between + reserve and confirm, redirecting fulfillment or changing the rate the + swap initiates under. The reservation creation block is derived from + ``reserved_until - RESERVATION_TTL_BLOCKS - extension_count * MAX_EXTENSION_BLOCKS``, + which is exact for un-extended reservations and a safe lower bound + otherwise. + """ + + def test_reads_commitment_at_reservation_block_no_extensions(self): + """No extensions: read at reserved_until - 50 (RESERVATION_TTL_BLOCKS).""" + validator = make_validator(block=1000, reserved_until=1040) + with patch( + 'allways.validator.axon_handlers.read_miner_commitment', + return_value=make_commitment(), + ) as mock_read: + asyncio.run(handle_swap_confirm(validator, make_synapse())) + # 1040 (reserved_until) - 50 (TTL) - 0 (extensions) = 990 + assert mock_read.call_args.kwargs['block'] == 990 + + def test_reads_commitment_at_reservation_block_with_extensions(self): + """Each extension subtracts MAX_EXTENSION_BLOCKS (250) — keeps the + read at-or-before the original reserve regardless of how far + reserved_until has been pushed forward by extension finalizations.""" + validator = make_validator(block=2000, reserved_until=2500) + validator.axon_contract_client.get_reservation_extension_count.return_value = 2 + with patch( + 'allways.validator.axon_handlers.read_miner_commitment', + return_value=make_commitment(), + ) as mock_read: + asyncio.run(handle_swap_confirm(validator, make_synapse())) + # 2500 - 50 - 2 * 250 = 1950 + assert mock_read.call_args.kwargs['block'] == 1950 + + def test_reservation_block_floor_at_one(self): + """Newly-bootstrapped chain (low reserved_until) must not produce a + negative block — substrate would reject the historical read.""" + validator = make_validator(block=10, reserved_until=20) + with patch( + 'allways.validator.axon_handlers.read_miner_commitment', + return_value=make_commitment(), + ) as mock_read: + asyncio.run(handle_swap_confirm(validator, make_synapse())) + assert mock_read.call_args.kwargs['block'] == 1 + + def test_extension_count_contract_error_falls_back_to_zero(self): + """RPC failure on extension_count must not crash confirm — fall back + to zero, which equals the no-extension lower bound. Verified via the + queued-confirm path so we don't depend on SS58 keypair construction.""" + validator = make_validator(block=1000, reserved_until=1040) + validator.axon_contract_client.get_reservation_extension_count.side_effect = ContractError('rpc down') + validator.axon_chain_providers['btc'].verify_transaction.return_value = make_tx_info( + confirmed=False, + confirmations=1, + ) + with patch( + 'allways.validator.axon_handlers.read_miner_commitment', + return_value=make_commitment(), + ) as mock_read: + result = asyncio.run(handle_swap_confirm(validator, make_synapse())) + # Falls back to 0 extensions → 1040 - 50 - 0 = 990 + assert mock_read.call_args.kwargs['block'] == 990 + # And the confirm still proceeds along the queued path. + assert result.accepted is True + assert 'Queued' in (result.rejection_reason or '') + + class TestErrorHandling: def test_contract_rejection_surfaces_variant_detail(self): """Contract rejections must carry the variant + description through to