Fix(underwriter): bind the source-deposit hash to the caller's target amount - #557
Closed
heifner wants to merge 1 commit into
Closed
Fix(underwriter): bind the source-deposit hash to the caller's target amount#557heifner wants to merge 1 commit into
heifner wants to merge 1 commit into
Conversation
… amount The source outpost hashes the terms the CALLER submitted -- ReserveManager.requestSwap's abi.encodePacked on EVM, request_swap.rs::correlation_hash on SVM -- and the underwriter reproduces that preimage before it will commit. Both verifiers packed the UWREQ's dst_amount, which the depot now overwrites with its own AMM quote (WNS-02): a number minted after the deposit was hashed and one the outpost never saw. Every swap whose quote differs from the target -- every swap that pays a WIRE-leg fee -- therefore failed to verify, and the underwriter declined to commit with a bare "SwapDeposit hash mismatch", stalling the swap instead of failing loudly. Pack target_amount instead. The field already exists on the row and carries exactly what the outpost hashed. The packing moves into routing_detail.hpp as pack_swap_deposit_preimage, shared by both verifiers -- the EVM and SVM bodies were near-identical, differing only in depositor width, and had already drifted once. swap_deposit_terms carries no dst_amount field at all, so the quote cannot be packed here by mistake; the single uw_request adapter makes that choice once for both chains.
Contributor
Author
|
See #556 |
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.
The underwriter reproduces the source outpost's deposit hash before it will commit to a swap. Both verifiers packed the UWREQ's
dst_amount— which #550 repurposed to hold the depot's own AMM quote, a number minted long after the outpost hashed the deposit and one it never saw. The outpost hashedSwapRequest.target_amount, so the recompute now misses for every swap whose quote differs from the target: every swap that pays a WIRE-leg fee.The failure is silent.
verify_source_depositreturns false, the underwriter logsSwapDeposit hash mismatchand skips the request, and retries on the next scan cycle — forever. Nothing aborts, so the swap simply never gets a commitment.The fix
Pack
target_amount. The field already exists on the row — #550 added it for exactly this reason ("the destination amount the caller ASKED for") — and the plugin simply never read it.The packing moves into
routing_detail.hppaspack_swap_deposit_preimage, shared by both verifiers. The EVM and SVM bodies were near-identical, differing only in depositor width (20-byte address vs 32-byte Ed25519 pubkey), and they had already drifted once — this is the second time a change had to be made in two places and landed in neither.swap_deposit_termsdeliberately has nodst_amountfield, so the quote cannot be packed here by mistake; the singleuw_requestadapter makes that choice once for both chains.No compatibility fallback on the decode: the chain is unlaunched, so there are no pre-split rows to read.
Tests
underwriter_source_deposit_testspins the byte layout for both chains (depositor bytes, seven big-endian u64s in outpost order, then the tolerance), that the EVM and SVM preimages share identical trailing bytes so the two verifiers cannot drift, and that the preimage tracks the caller's target rather than anything the depot re-prices.The verifiers themselves need live RPC, so the end-to-end proof is a swap confirming on a cluster — that needs this merged and a gate run, and I have not claimed it here.
Scope
This affects any outpost-originated underwritten swap on master, not one flow.
flow-swap-with-underwritingshould be failing the same way right now. If a recent gate run on master is green, my model is incomplete and I would like to know why before this merges.Found while re-running the e2e gate for #549 after merging master: that branch's
flow-underwriter-slashingtimed out waiting for a commitment, with the log above. #549 stays gate-blocked until this lands.