fix(swap): instrument bridge quote after intent verification (#752) - #823
Merged
Conversation
instrumentBridgeQuote ran before verifyLifiBridgeIntent in the bridge branch of prepareSwap, contradicting design §4.2 step 6. A bridge route later REFUSEd by verifyLifiBridgeIntent (destination chain or receiver mismatch) still incremented lifiBridgeSuspectedUnreachableCount, mildly inflating the exact metric PROD condition 2 uses to promote #745. Move the instrumentBridgeQuote call to after verifyLifiBridgeIntent, still gated on swapClass === "bridge".
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Closes #752
What changed and why
#752item 2:instrumentBridgeQuotewas called inside theswapClass === "bridge"branch ofprepareSwap(src/modules/swap/index.ts), BEFOREverifyLifiBridgeIntentran later in the same function. This contradicted design §4.2 step 6, whose own docstring oninstrumentBridgeQuotealready says it must "never sit before ... the existing bridge guards."Effect of the bug: a bridge route that
verifyLifiBridgeIntentlater REFUSEs (destination-chain mismatch or non-wallet EVM receiver, #798) still got counted towardlifiBridgeSuspectedUnreachableCountbefore the throw. That counter is the exact metric PROD condition 2 uses to promote #745, so a REFUSEd route was inflating a promotion signal it should never touch.Fix: moved the
instrumentBridgeQuotecall out of the pre-guard bridge branch to a newif (swapClass === "bridge")block placed immediately afterverifyLifiBridgeIntent(args, txRequest.data), so it only runs once that guard has passed.Scope
Only item 2 of #752 is addressed here. Items 1 (native-in LiFi topology sentinel — needs a live li.quest quote to verify) and 3 (commit real probe-output JSON) both need live network access unavailable to this authoring session; left as follow-up per the issue's own framing ("accept the residual" / "commit... probe output" are separable asks).
Falsifier test
Added
test/swap-lifi-minout.test.ts—describe("#752 — bridge instrumentation runs AFTER verifyLifiBridgeIntent"):bridgeSuspectedUnreachablewould flag it), but withBridgeData.receiverset to a non-wallet address soverifyLifiBridgeIntent'sassertEvmReceiverIsWallet(security: prepare_swap(toAddress=ATTACKER) drains to the LiFi Diamond unstamped — #760 closed on a partial fix; intent check is tautological against a rogue agent #798) REFUSEs it.prepareSwaprejects with/receiver mismatch/, AND thatlifiBridgeSuspectedUnreachableCountis still0afterward.main(instrumentation called before the guard), the counter would already be1by the time the guard throws — this test fails onmainand passes with this fix.Tests were NOT run locally — Node/npm are not installed on the authoring machine. CI (Build & Test on Node 20 + 22) is the authoritative check for this PR. The test was written by close reading of the existing test file's mocks (
fetchQuotemock,evmClientStub,makeBridgeQuotehelper,bridgeArgsfixture) and matching its exact style/imports; no existing test's assertions were run to confirm.Existing tests updated
None. The reorder doesn't change behavior for any bridge route that passes
verifyLifiBridgeIntent(the existing T6 tests all use the default wallet-matching receiver and unaffecteddestinationChainId, so they still increment/don't-increment the counter exactly as before).Blast radius
Single function (
prepareSwap), bridge-class branch only. No change to generic-swap handling, no change toverifyLifiBridgeIntentitself, no change toinstrumentBridgeQuote's own logic — only where it's called from. Diff is 51 lines across 2 files.Residual concerns
verifyLifiBridgeIntentcall and the receiver-mismatch case avoids anyINTERMEDIATE_CHAIN_BRIDGESinteraction, making it the cleaner/more deterministic falsifier. Happy to add a second case if reviewers want both REFUSE paths covered explicitly.