harden: a V4 step with a zero amountOutMin leaves an earlier hop's Min. received on screen #359

Closed
opened 2026-08-23 18:13:38 +02:00 by clawbot · 0 comments
Collaborator

Found by the review of #356 (#356 (comment)), executed rather than inferred. Pre-existing; not introduced there.

src/shared/uniswap.js:442 gates on truthiness: if (v4.amountOutMin) minOutput = v4.amountOutMin;. A V4 step whose amountOutMin is 0n therefore updates neither the figure nor — via the else if that inherits the same gate — the token.

Measured: a V3 USDT→WETH hop followed by a V4 step with minOut = 0 renders Token Out: WETH (0xc02a…) and Min. received: 0.5000 WETHan earlier hop's guarantee displayed for a transaction whose final leg guarantees nothing. A zero minimum means "no slippage floor at all", which is precisely the case a user most needs stated, and it is the one case the screen hides behind a stale number.

This is the falsy-collapse trap this repo keeps hitting — see #246, #306, #322 — where a legitimate 0 is indistinguishable from absent.

Note #356 is consistent here (stale token pairs with stale figure, so it never shows a figure against the wrong token), which is why the reviewer did not hold that PR for it.

Definition of done

  • Both minOutput and the output token are gated on !== null / explicit presence rather than truthiness, so a genuine 0n minimum is carried through as a real value.
  • A zero slippage floor is stated as such on the approval screen — a swap that guarantees nothing must say so, not inherit an earlier hop's figure.
  • Test: a V3 hop followed by a V4 step with amountOutMin of 0n. Fails against current head — state the mutation and observed result.
  • Also add coverage for the population #356 changed without testing: a non-swap execute() carrying only PERMIT2_PERMIT now renders the refusal and titles itself Uniswap Swap rather than inventing Token Out: ETH. That behaviour is correct and should be pinned so it cannot silently revert.
  • make check green.
Found by the review of https://git.eeqj.de/sneak/AutistMask/pulls/356 (https://git.eeqj.de/sneak/AutistMask/pulls/356#issuecomment-69205), executed rather than inferred. Pre-existing; not introduced there. `src/shared/uniswap.js:442` gates on truthiness: `if (v4.amountOutMin) minOutput = v4.amountOutMin;`. A V4 step whose `amountOutMin` is `0n` therefore updates neither the figure nor — via the `else if` that inherits the same gate — the token. Measured: a V3 USDT→WETH hop followed by a V4 step with `minOut = 0` renders `Token Out: WETH (0xc02a…)` and `Min. received: 0.5000 WETH` — **an earlier hop's guarantee displayed for a transaction whose final leg guarantees nothing.** A zero minimum means "no slippage floor at all", which is precisely the case a user most needs stated, and it is the one case the screen hides behind a stale number. This is the falsy-collapse trap this repo keeps hitting — see https://git.eeqj.de/sneak/AutistMask/issues/246, https://git.eeqj.de/sneak/AutistMask/issues/306, https://git.eeqj.de/sneak/AutistMask/issues/322 — where a legitimate `0` is indistinguishable from absent. Note https://git.eeqj.de/sneak/AutistMask/pulls/356 is *consistent* here (stale token pairs with stale figure, so it never shows a figure against the wrong token), which is why the reviewer did not hold that PR for it. ## Definition of done - [ ] Both `minOutput` and the output token are gated on `!== null` / explicit presence rather than truthiness, so a genuine `0n` minimum is carried through as a real value. - [ ] A zero slippage floor is stated as such on the approval screen — a swap that guarantees nothing must say so, not inherit an earlier hop's figure. - [ ] Test: a V3 hop followed by a V4 step with `amountOutMin` of `0n`. Fails against current head — state the mutation and observed result. - [ ] Also add coverage for the population https://git.eeqj.de/sneak/AutistMask/pulls/356 changed without testing: a non-swap `execute()` carrying only `PERMIT2_PERMIT` now renders the refusal and titles itself `Uniswap Swap` rather than inventing `Token Out: ETH`. That behaviour is correct and should be pinned so it cannot silently revert. - [ ] `make check` green.
clawbot added this to the 1.0.0 milestone 2026-08-23 18:13:38 +02:00
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/AutistMask#359