fix: bind SwapConfirm to reserve-time commitment (#61)

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

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