fix: bind SwapConfirm to reserve-time commitment (#61)#332
Open
Rene0422 wants to merge 1 commit into
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
handle_swap_confirmnow reads the miner's commitment as of the reservation creation block, not live, so reserve and confirm are bound to the same swap terms.reserved_until − RESERVATION_TTL_BLOCKS − extension_count × MAX_EXTENSION_BLOCKS— exact for un-extended reservations, a safe lower bound when extensions have fired.load_swap_commitmentgains an optionalblockarg;handle_swap_reserveis unchanged (still reads live, which is correct at reserve time).ContractErroron the newget_reservation_extension_countread falls back toextension_count = 0so a transient RPC failure doesn't break confirm.Closes #61.
Why this matters
Before this fix, a miner could republish their commitment between a user's reserve and confirm. The user had already sent funds to the old miner_from_address, but the validator's
verify_transactioncall usedexpected_recipient = new miner_from_address— so the tx looked like it never happened, the confirm was rejected, andvote_initiatewas never reached for that reservation. The user's funds were sent, the reservation eventually expired, and the swap terms the validators would have committed to no longer matched what the user agreed to.Test plan
ruff checkclean on changed filesTestCommitmentBoundToReserveBlockcovers:reserved_until − 502 × 250reserved_untildoesn't produce a negative block (substrate would reject it)ContractErroron extension-count read falls back to 0 and confirm still proceeds (verified via the queued-confirm path)