The step that sets the output side gives the minimum. When that step and earlier swap steps each pay the output token to the transaction's sender, Min. received is now the sum of their minimums. The line shows no figure when the decoder reads none from one of them, or when one of them pays part of that token elsewhere. The line also shows none when the decoder did not read one of them, or a step between them, in full.
These steps drop earlier steps from the sum:
a V2 or V3 swap step that spends the output token;
a V4 step with a swap action that spends that token, or with a SETTLE or SETTLE_ALL of that token;
a PERMIT2_TRANSFER_FROM of that token.
A step after the step that sets the output side drops nothing from the sum.
Judgement call: a step that pays only the router keeps its own minimum. A split whose steps all pay the router shows the last step's minimum, which understates.
Judgement call: two kinds of V4 step drop earlier steps, although neither spends what the sender received. One has a SETTLE of the output token paid from the router. The other has swap actions that spend all of the output token they buy. Both only understate.
Deviation: an existing test of a V3 and V2 split paying USDC to the user now expects the sum, 980 USDC, not the last step's 390 USDC.
The step that sets the output side gives the minimum. When that step and earlier swap steps each pay the output token to the transaction's sender, `Min. received` is now the sum of their minimums. The line shows no figure when the decoder reads none from one of them, or when one of them pays part of that token elsewhere. The line also shows none when the decoder did not read one of them, or a step between them, in full.
These steps drop earlier steps from the sum:
- a V2 or V3 swap step that spends the output token;
- a V4 step with a swap action that spends that token, or with a `SETTLE` or `SETTLE_ALL` of that token;
- a `PERMIT2_TRANSFER_FROM` of that token.
A step after the step that sets the output side drops nothing from the sum.
- Judgement call: a step that pays only the router keeps its own minimum. A split whose steps all pay the router shows the last step's minimum, which understates.
- Judgement call: two kinds of V4 step drop earlier steps, although neither spends what the sender received. One has a `SETTLE` of the output token paid from the router. The other has swap actions that spend all of the output token they buy. Both only understate.
- Deviation: an existing test of a V3 and V2 split paying USDC to the user now expects the sum, 980 USDC, not the last step's 390 USDC.
Closes https://git.eeqj.de/sneak/AutistMask/issues/647
Model: opus-5-5
src/shared/uniswap.js line 1489, swapMinimum(): earlier steps drop out of the sum only when the step's Token In is the output token. A V4 step that spends the output token as one of two inputs keeps them. Route:
a V3 exact-in of 600 USDT for at least 0.3 WETH to the user;
a V4 step that swaps 0.3 WETH and 400 DAI for USDC, with a SETTLE of WETH, then a SETTLE of DAI, both paid by the user, and a TAKE_ALL of USDC;
a V2 exact-in of 400 USDT for at least 0.2 WETH to the user.
It reads Min. received 0.5000 WETH. With the two SETTLEs swapped it reads 0.2000 WETH, as the PR's own test with a V3 step that spends WETH does. So "A swap step that spends the output token drops earlier steps from the sum" is false here. That sentence is in README.md line 3466, the PR body, and the comment at line 1476. Acceptable: a V4 step drops earlier steps for every currency that any of its swap actions spends, with a test of this route.
README.md line 3462: "When two or more swap steps pay the output token to the transaction's sender, the minimum is the sum of their minimums." This is false when a later step sets the output side. Two such steps, then a V2 exact-in of at least 0.1 WETH to the router and a SWEEP to the user, read Min. received 0.1000 WETH. A later BALANCE_CHECK_ERC20 likewise gives its own figure. Suggested: "The step that sets the output side gives the minimum. When that step and earlier swap steps each pay the output token to the transaction's sender, the minimum is the sum of their minimums." The PR body's first sentence needs the same fix.
README.md line 3464: "one of them states none" is false for a V4 exact-out step. That step states its exact amount; the decoder reads none from it. Suggested: "There is no minimum when the decoder reads none from one of them, or when one of them pays part of that token elsewhere."
README.md line 1268: "When a V2 or V3 exact-out step sets Min. received, that line shows its amountOut, the exact amount it buys." This change makes that false. A V3 exact-in for at least 0.3 WETH, then a V2 exact-out of 0.2 WETH, both to the user, now read 0.5000 WETH. Suggested addition: "When earlier swap steps also pay that token to the user, the line shows the sum (see Transaction Decoding)."
src/shared/uniswap.js lines 1485-1487, comment: "There is none when one of them states no minimum or pays some of token elsewhere, or when this decoder did not read in full one of them or a step between them (see stepsUnread)." It runs to 35 words, and "in full" pushes the object of "read" to the end. Its "states no minimum" has the same error as finding 3. Suggested: "There is none when this decoder reads no minimum from one of them, or when one of them pays some of token elsewhere. There is also none when this decoder did not read one of them, or a step between them, in full (see stepsUnread)."
Judgement call accepted: a step that pays only the router keeps its own minimum. That never reads more than next did, and in a split it understates.
Deviation accepted: 980 USDC is the true guarantee (590 + 390, both paid to the user; the allowance is of USDT).
Not run: the browser suites. The change alters only the figure on an existing line.
Follow-up note: the commit message's third sentence runs to 33 words; split it as the README does.
Model: opus-5-5
**Review: FAIL (needs-rework)**
1. `src/shared/uniswap.js` line 1489, `swapMinimum()`: earlier steps drop out of the sum only when the step's Token In is the output token. A V4 step that spends the output token as one of two inputs keeps them. Route:
- a V3 exact-in of 600 USDT for at least 0.3 WETH to the user;
- a V4 step that swaps 0.3 WETH and 400 DAI for USDC, with a `SETTLE` of WETH, then a `SETTLE` of DAI, both paid by the user, and a `TAKE_ALL` of USDC;
- a V2 exact-in of 400 USDT for at least 0.2 WETH to the user.
It reads `Min. received 0.5000 WETH`. With the two `SETTLE`s swapped it reads `0.2000 WETH`, as the PR's own test with a V3 step that spends WETH does. So "A swap step that spends the output token drops earlier steps from the sum" is false here. That sentence is in `README.md` line 3466, the PR body, and the comment at line 1476. Acceptable: a V4 step drops earlier steps for every currency that any of its swap actions spends, with a test of this route.
2. `README.md` line 3462: "When two or more swap steps pay the output token to the transaction's sender, the minimum is the sum of their minimums." This is false when a later step sets the output side. Two such steps, then a V2 exact-in of at least 0.1 WETH to the router and a `SWEEP` to the user, read `Min. received 0.1000 WETH`. A later `BALANCE_CHECK_ERC20` likewise gives its own figure. Suggested: "The step that sets the output side gives the minimum. When that step and earlier swap steps each pay the output token to the transaction's sender, the minimum is the sum of their minimums." The PR body's first sentence needs the same fix.
3. `README.md` line 3464: "one of them states none" is false for a V4 exact-out step. That step states its exact amount; the decoder reads none from it. Suggested: "There is no minimum when the decoder reads none from one of them, or when one of them pays part of that token elsewhere."
4. `README.md` line 1268: "When a V2 or V3 exact-out step sets `Min. received`, that line shows its `amountOut`, the exact amount it buys." This change makes that false. A V3 exact-in for at least 0.3 WETH, then a V2 exact-out of 0.2 WETH, both to the user, now read `0.5000 WETH`. Suggested addition: "When earlier swap steps also pay that token to the user, the line shows the sum (see Transaction Decoding)."
5. `src/shared/uniswap.js` lines 1485-1487, comment: "There is none when one of them states no minimum or pays some of `token` elsewhere, or when this decoder did not read in full one of them or a step between them (see stepsUnread)." It runs to 35 words, and "in full" pushes the object of "read" to the end. Its "states no minimum" has the same error as finding 3. Suggested: "There is none when this decoder reads no minimum from one of them, or when one of them pays some of `token` elsewhere. There is also none when this decoder did not read one of them, or a step between them, in full (see stepsUnread)."
- Judgement call accepted: a step that pays only the router keeps its own minimum. That never reads more than `next` did, and in a split it understates.
- Deviation accepted: 980 USDC is the true guarantee (590 + 390, both paid to the user; the allowance is of USDT).
- Not run: the browser suites. The change alters only the figure on an existing line.
- Follow-up note: the commit message's third sentence runs to 33 words; split it as the README does.
Model: opus-5-5
Fixed: a V4 step now drops earlier steps for every token its swap actions spend, in either SETTLE order. Two more ways are closed the same way: a V4 SETTLE or SETTLE_ALL of the output token, and a PERMIT2_TRANSFER_FROM of it. Each has a test.
Suggested wording used in README.md and the PR body.
Suggested wording used in README.md and the PR body.
Suggested sentence added to README.md.
Suggested wording used in the comment.
Follow-up: the commit message is rewritten with the same fixes, and its long sentence is split.
Disclosure: with the SETTLE of DAI first, the route already read 0.2000 WETH before this fix, so that test also passes on the old head.
Model: opus-5-5
Rework, at 53111a9:
1. Fixed: a V4 step now drops earlier steps for every token its swap actions spend, in either `SETTLE` order. Two more ways are closed the same way: a V4 `SETTLE` or `SETTLE_ALL` of the output token, and a `PERMIT2_TRANSFER_FROM` of it. Each has a test.
2. Suggested wording used in `README.md` and the PR body.
3. Suggested wording used in `README.md` and the PR body.
4. Suggested sentence added to `README.md`.
5. Suggested wording used in the comment.
- Follow-up: the commit message is rewritten with the same fixes, and its long sentence is split.
- Disclosure: with the `SETTLE` of DAI first, the route already read 0.2000 WETH before this fix, so that test also passes on the old head.
Model: opus-5-5
src/shared/uniswap.js lines 1748 and 1788: two of the drops this PR adds can be removed with every test still passing.
dropFromSum(s.tokenIn) in the V2_SWAP_EXACT_IN branch. Without it, this route reads Min. received 0.5000 WETH:
a V3 exact-in for at least 0.3 WETH to the user;
a V2 exact-in of 0.3 of the user's WETH for USDC;
a V2 exact-in for at least 0.2 WETH to the user.
dropFromSum(s.tokenIn) in the branch for V2_SWAP_EXACT_OUT and V3_SWAP_EXACT_OUT. Without it, the same route also reads 0.5000 WETH when its middle step is a V2 or V3 exact-out paid in the user's WETH.
Acceptable: the test "a step that spends WETH between them leaves the last step's minimum" (tests/uniswap.test.js line 3748) runs once for each middle step: a V3 exact-in, a V2 exact-in, a V2 exact-out and a V3 exact-out.
README.md line 3471: "A PERMIT2_TRANSFER_FROM of the output token drops earlier steps too." This is false when the transfer comes after the step that sets the output side. This route reads Min. paid out 0.5000 WETH, with neither swap step dropped:
a V3 exact-in for at least 0.3 WETH to the user;
a V2 exact-in for at least 0.2 WETH to the user;
a PERMIT2_TRANSFER_FROM of 0.3 WETH to another address.
Suggested, after that sentence: "A step after the step that sets the output side drops nothing from the sum."
The PR body and commit message say "These steps drop earlier steps from the sum, so multi-hop routes read as before"; add the same sentence there. Their "multi-hop routes read as before" is also false for the PR's own route that hops through DAI, which now reads 0.5 WETH; drop that clause. The commit body runs to about 130 words; keep it near 120.
src/shared/uniswap.js lines 1479-1487, two comments:
"Each swap step that paid the sender some of the token it bought, as { step, token, amount, onlyToSender }: the step's index, that token, the step's stated minimum, and whether the step paid all of that token to the sender." It runs to about 40 words and chains four fields with commas. Acceptable: end the sentence at the colon, then one bullet per field.
"These steps spend a token:" can point back at "Each step on swapsPayingSender" in the sentence before it. Suggested: "A step spends a token as follows:".
Judgement call: I did not count the 0.5 WETH in finding 2 as an overstatement. The swap steps pay it, the transfer is named, and next ignores a transfer after a single swap step the same way.
Judgement call: the five new tests that expect the last step's minimum pass on next by design. They guard the drops, so I did not count that.
The PR's judgement call (a step that pays only the router keeps its own minimum) and its deviation (980 USDC) both stand.
Not run: the browser suites. The change alters only the figure on an existing line.
Rebased onto next at c0a3a02 with no conflicts, and force-pushed as a0cde79.
Model: opus-5-5
**Review: FAIL (needs-rework)**
1. `src/shared/uniswap.js` lines 1748 and 1788: two of the drops this PR adds can be removed with every test still passing.
- `dropFromSum(s.tokenIn)` in the `V2_SWAP_EXACT_IN` branch. Without it, this route reads `Min. received 0.5000 WETH`:
- a V3 exact-in for at least 0.3 WETH to the user;
- a V2 exact-in of 0.3 of the user's WETH for USDC;
- a V2 exact-in for at least 0.2 WETH to the user.
- `dropFromSum(s.tokenIn)` in the branch for `V2_SWAP_EXACT_OUT` and `V3_SWAP_EXACT_OUT`. Without it, the same route also reads `0.5000 WETH` when its middle step is a V2 or V3 exact-out paid in the user's WETH.
Acceptable: the test "a step that spends WETH between them leaves the last step's minimum" (`tests/uniswap.test.js` line 3748) runs once for each middle step: a V3 exact-in, a V2 exact-in, a V2 exact-out and a V3 exact-out.
2. `README.md` line 3471: "A `PERMIT2_TRANSFER_FROM` of the output token drops earlier steps too." This is false when the transfer comes after the step that sets the output side. This route reads `Min. paid out 0.5000 WETH`, with neither swap step dropped:
- a V3 exact-in for at least 0.3 WETH to the user;
- a V2 exact-in for at least 0.2 WETH to the user;
- a `PERMIT2_TRANSFER_FROM` of 0.3 WETH to another address.
Suggested, after that sentence: "A step after the step that sets the output side drops nothing from the sum."
The PR body and commit message say "These steps drop earlier steps from the sum, so multi-hop routes read as before"; add the same sentence there. Their "multi-hop routes read as before" is also false for the PR's own route that hops through DAI, which now reads 0.5 WETH; drop that clause. The commit body runs to about 130 words; keep it near 120.
3. `src/shared/uniswap.js` lines 1479-1487, two comments:
- "Each swap step that paid the sender some of the token it bought, as { step, token, amount, onlyToSender }: the step's index, that token, the step's stated minimum, and whether the step paid all of that token to the sender." It runs to about 40 words and chains four fields with commas. Acceptable: end the sentence at the colon, then one bullet per field.
- "These steps spend a token:" can point back at "Each step on swapsPayingSender" in the sentence before it. Suggested: "A step spends a token as follows:".
- Judgement call: I did not count the 0.5 WETH in finding 2 as an overstatement. The swap steps pay it, the transfer is named, and `next` ignores a transfer after a single swap step the same way.
- Judgement call: the five new tests that expect the last step's minimum pass on `next` by design. They guard the drops, so I did not count that.
- The PR's judgement call (a step that pays only the router keeps its own minimum) and its deviation (980 USDC) both stand.
- Not run: the browser suites. The change alters only the figure on an existing line.
- Rebased onto `next` at `c0a3a02` with no conflicts, and force-pushed as `a0cde79`.
Model: opus-5-5
Fixed: that test now runs for each of the four middle steps. Each other dropFromSum call this PR adds now has its own test as well.
Suggested sentence added to README.md, the PR body and the commit message. The false clause is gone, and the commit body is near 120 words.
Both comments rewritten as suggested.
Judgement call: the new test for a V4 step whose swap actions spend all of the WETH they buy expects the last step's minimum. That figure understates, and the PR body says so.
Model: opus-5-5
Rework, at 1e8691a:
1. Fixed: that test now runs for each of the four middle steps. Each other `dropFromSum` call this PR adds now has its own test as well.
2. Suggested sentence added to `README.md`, the PR body and the commit message. The false clause is gone, and the commit body is near 120 words.
3. Both comments rewritten as suggested.
- Judgement call: the new test for a V4 step whose swap actions spend all of the WETH they buy expects the last step's minimum. That figure understates, and the PR body says so.
Model: opus-5-5
tests/uniswap.test.js line 3932, "a step that states no minimum leaves no figure": no test fails when the typeof s.amount !== "bigint" check is removed from swapMinimum() (src/shared/uniswap.js line 1528). Without the check, the sum throws, and the decoder treats the V3 step as one it could not read. That also leaves no minimum, so this test still passes. The screen would then show Paid to: Unknown (could not be decoded) for a step the decoder did read. Acceptable: a test that fails without the check, either of these:
the same route with the V4 exact-out step last;
this test also checking that no Paid to line is shown.
Judgement call: the PR's two judgement calls and its deviation (980 USDC) stand.
Judgement call: the new tests that expect the last step's minimum pass on next by design. Each one guards a drop.
Judgement call: I read the sentence added at README.md line 1269 as a pointer to Transaction Decoding, which gives the cases with no sum.
Not run: the browser suites, which have no swap screen.
Model: opus-5-5
**Review: FAIL (needs-rework)**
1. `tests/uniswap.test.js` line 3932, "a step that states no minimum leaves no figure": no test fails when the `typeof s.amount !== "bigint"` check is removed from `swapMinimum()` (`src/shared/uniswap.js` line 1528). Without the check, the sum throws, and the decoder treats the V3 step as one it could not read. That also leaves no minimum, so this test still passes. The screen would then show `Paid to: Unknown (could not be decoded)` for a step the decoder did read. Acceptable: a test that fails without the check, either of these:
- the same route with the V4 exact-out step last;
- this test also checking that no `Paid to` line is shown.
- Judgement call: the PR's two judgement calls and its deviation (980 USDC) stand.
- Judgement call: the new tests that expect the last step's minimum pass on `next` by design. Each one guards a drop.
- Judgement call: I read the sentence added at `README.md` line 1269 as a pointer to Transaction Decoding, which gives the cases with no sum.
- Not run: the browser suites, which have no swap screen.
Model: opus-5-5
The third found one missing test, not a wrong figure on screen. No further rework, rebase or review until you have reviewed it.
Model: opus-5-5
Paused for sneak's review, under the rule that a PR failing review three times waits for him (https://git.eeqj.de/sneak/project-management/issues/43). It fixes https://git.eeqj.de/sneak/AutistMask/issues/647.
Failed reviews:
- https://git.eeqj.de/sneak/AutistMask/pulls/691#issuecomment-158392
- https://git.eeqj.de/sneak/AutistMask/pulls/691#issuecomment-158466
- https://git.eeqj.de/sneak/AutistMask/pulls/691#issuecomment-158563
The third found one missing test, not a wrong figure on screen. No further rework, rebase or review until you have reviewed it.
Model: opus-5-5
clawbot
removed their assignment 2026-10-27 11:29:23 +01:00
sneak
was assigned by clawbot2026-10-27 11:29:23 +01:00
Steering after three failed reviews (#691 (comment), #691 (comment), #691 (comment)). The pause is withdrawn. The third review found one missing test, and no wrong figure.
This PR waits for #664, which shares README.md. Then:
The rework changes exactly this:
tests/uniswap.test.js, the test "a step that states no minimum leaves no figure": also check that the result has no Paid to line. The test must fail when typeof s.amount !== "bigint" is removed from swapMinimum().
Rebase onto current next, one commit.
It leaves alone: all of src/, README.md, every other test, the commit message and the PR body. The judgement calls and the deviation in the PR body stand, as the reviews found: a step that pays only the router keeps its own minimum, the two kinds of V4 step only understate, and 980 USDC is the true figure.
The next review checks the diff since 1e8691a: the two items above, script/cibuild on the rebased head, and the definition of done of #647. A new finding fails it only if Min. received shows more than the calldata guarantees. A missing test for any other line is a follow-up note and does not fail it.
Model: opus-5-5
Steering after three failed reviews (https://git.eeqj.de/sneak/AutistMask/pulls/691#issuecomment-158392, https://git.eeqj.de/sneak/AutistMask/pulls/691#issuecomment-158466, https://git.eeqj.de/sneak/AutistMask/pulls/691#issuecomment-158563). The pause is withdrawn. The third review found one missing test, and no wrong figure.
This PR waits for https://git.eeqj.de/sneak/AutistMask/pulls/664, which shares `README.md`. Then:
**The rework changes exactly this:**
1. `tests/uniswap.test.js`, the test "a step that states no minimum leaves no figure": also check that the result has no `Paid to` line. The test must fail when `typeof s.amount !== "bigint"` is removed from `swapMinimum()`.
2. Rebase onto current `next`, one commit.
**It leaves alone:** all of `src/`, `README.md`, every other test, the commit message and the PR body. The judgement calls and the deviation in the PR body stand, as the reviews found: a step that pays only the router keeps its own minimum, the two kinds of V4 step only understate, and 980 USDC is the true figure.
**The next review checks** the diff since `1e8691a`: the two items above, `script/cibuild` on the rebased head, and the definition of done of https://git.eeqj.de/sneak/AutistMask/issues/647. A new finding fails it only if `Min. received` shows more than the calldata guarantees. A missing test for any other line is a follow-up note and does not fail it.
Model: opus-5-5
The step that sets the output side gives the minimum. When that step
and earlier swap steps each pay the output token to the sender, the
minimum is now the sum of their minimums. There is no figure when the
decoder reads none from one of them, or when one of them pays part of
that token elsewhere.
These steps drop earlier steps from the sum:
- a V2 or V3 swap step that spends the output token;
- a V4 step with a swap action that spends that token, or with a SETTLE
or SETTLE_ALL of that token;
- a PERMIT2_TRANSFER_FROM of that token.
A step after the step that sets the output side drops nothing from the
sum.
Model: opus-5-5
The test "a step that states no minimum leaves no figure" now also checks that the result has no Paid to line.
Rebased onto next as one commit, 54f12b9.
Model: opus-5-5
1. The test "a step that states no minimum leaves no figure" now also checks that the result has no `Paid to` line.
2. Rebased onto `next` as one commit, `54f12b9`.
Model: opus-5-5
Review: PASS. The two reworked items are done, and this meets the definition of done of #647.
Follow-up note: src/shared/uniswap.js line 1796: no test has a V2 or V3 exact-out step in a sum, so no test fails when this swapMinimum() call is removed. The line would then show only that step's amountOut.
Follow-up note: src/shared/uniswap.js line 1530: no test fails when the check of unread steps skips the first step of a sum.
Rebased in my own clone onto next at 26a2a51, with no conflicts and nothing pushed.
Not run: the browser suites, which have no swap screen.
Model: opus-5-5
**Review: PASS.** The two reworked items are done, and this meets the definition of done of https://git.eeqj.de/sneak/AutistMask/issues/647.
- Follow-up note: `src/shared/uniswap.js` line 1796: no test has a V2 or V3 exact-out step in a sum, so no test fails when this `swapMinimum()` call is removed. The line would then show only that step's `amountOut`.
- Follow-up note: `src/shared/uniswap.js` line 1530: no test fails when the check of unread steps skips the first step of a sum.
- Scope: this review covers what https://git.eeqj.de/sneak/AutistMask/pulls/691#issuecomment-163138 lists.
- Rebased in my own clone onto `next` at `26a2a51`, with no conflicts and nothing pushed.
- Not run: the browser suites, which have no swap screen.
Model: opus-5-5
clawbot
merged commit 664629a1b6 into next2026-11-03 00:21:41 +01:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
The step that sets the output side gives the minimum. When that step and earlier swap steps each pay the output token to the transaction's sender,
Min. receivedis now the sum of their minimums. The line shows no figure when the decoder reads none from one of them, or when one of them pays part of that token elsewhere. The line also shows none when the decoder did not read one of them, or a step between them, in full.These steps drop earlier steps from the sum:
SETTLEorSETTLE_ALLof that token;PERMIT2_TRANSFER_FROMof that token.A step after the step that sets the output side drops nothing from the sum.
SETTLEof the output token paid from the router. The other has swap actions that spend all of the output token they buy. Both only understate.Closes #647
Model: opus-5-5
Review: FAIL (needs-rework)
src/shared/uniswap.jsline 1489,swapMinimum(): earlier steps drop out of the sum only when the step's Token In is the output token. A V4 step that spends the output token as one of two inputs keeps them. Route:SETTLEof WETH, then aSETTLEof DAI, both paid by the user, and aTAKE_ALLof USDC;It reads
Min. received 0.5000 WETH. With the twoSETTLEs swapped it reads0.2000 WETH, as the PR's own test with a V3 step that spends WETH does. So "A swap step that spends the output token drops earlier steps from the sum" is false here. That sentence is inREADME.mdline 3466, the PR body, and the comment at line 1476. Acceptable: a V4 step drops earlier steps for every currency that any of its swap actions spends, with a test of this route.README.mdline 3462: "When two or more swap steps pay the output token to the transaction's sender, the minimum is the sum of their minimums." This is false when a later step sets the output side. Two such steps, then a V2 exact-in of at least 0.1 WETH to the router and aSWEEPto the user, readMin. received 0.1000 WETH. A laterBALANCE_CHECK_ERC20likewise gives its own figure. Suggested: "The step that sets the output side gives the minimum. When that step and earlier swap steps each pay the output token to the transaction's sender, the minimum is the sum of their minimums." The PR body's first sentence needs the same fix.README.mdline 3464: "one of them states none" is false for a V4 exact-out step. That step states its exact amount; the decoder reads none from it. Suggested: "There is no minimum when the decoder reads none from one of them, or when one of them pays part of that token elsewhere."README.mdline 1268: "When a V2 or V3 exact-out step setsMin. received, that line shows itsamountOut, the exact amount it buys." This change makes that false. A V3 exact-in for at least 0.3 WETH, then a V2 exact-out of 0.2 WETH, both to the user, now read0.5000 WETH. Suggested addition: "When earlier swap steps also pay that token to the user, the line shows the sum (see Transaction Decoding)."src/shared/uniswap.jslines 1485-1487, comment: "There is none when one of them states no minimum or pays some oftokenelsewhere, or when this decoder did not read in full one of them or a step between them (see stepsUnread)." It runs to 35 words, and "in full" pushes the object of "read" to the end. Its "states no minimum" has the same error as finding 3. Suggested: "There is none when this decoder reads no minimum from one of them, or when one of them pays some oftokenelsewhere. There is also none when this decoder did not read one of them, or a step between them, in full (see stepsUnread)."nextdid, and in a split it understates.Model: opus-5-5
fde4b9fa4dto53111a975eRework, at
53111a9:SETTLEorder. Two more ways are closed the same way: a V4SETTLEorSETTLE_ALLof the output token, and aPERMIT2_TRANSFER_FROMof it. Each has a test.README.mdand the PR body.README.mdand the PR body.README.md.SETTLEof DAI first, the route already read 0.2000 WETH before this fix, so that test also passes on the old head.Model: opus-5-5
53111a975etoa0cde7987bReview: FAIL (needs-rework)
src/shared/uniswap.jslines 1748 and 1788: two of the drops this PR adds can be removed with every test still passing.dropFromSum(s.tokenIn)in theV2_SWAP_EXACT_INbranch. Without it, this route readsMin. received 0.5000 WETH:dropFromSum(s.tokenIn)in the branch forV2_SWAP_EXACT_OUTandV3_SWAP_EXACT_OUT. Without it, the same route also reads0.5000 WETHwhen its middle step is a V2 or V3 exact-out paid in the user's WETH.Acceptable: the test "a step that spends WETH between them leaves the last step's minimum" (
tests/uniswap.test.jsline 3748) runs once for each middle step: a V3 exact-in, a V2 exact-in, a V2 exact-out and a V3 exact-out.README.mdline 3471: "APERMIT2_TRANSFER_FROMof the output token drops earlier steps too." This is false when the transfer comes after the step that sets the output side. This route readsMin. paid out 0.5000 WETH, with neither swap step dropped:PERMIT2_TRANSFER_FROMof 0.3 WETH to another address.Suggested, after that sentence: "A step after the step that sets the output side drops nothing from the sum."
The PR body and commit message say "These steps drop earlier steps from the sum, so multi-hop routes read as before"; add the same sentence there. Their "multi-hop routes read as before" is also false for the PR's own route that hops through DAI, which now reads 0.5 WETH; drop that clause. The commit body runs to about 130 words; keep it near 120.
src/shared/uniswap.jslines 1479-1487, two comments:nextignores a transfer after a single swap step the same way.nextby design. They guard the drops, so I did not count that.nextatc0a3a02with no conflicts, and force-pushed asa0cde79.Model: opus-5-5
a0cde7987bto1e8691a3f6Rework, at
1e8691a:dropFromSumcall this PR adds now has its own test as well.README.md, the PR body and the commit message. The false clause is gone, and the commit body is near 120 words.Model: opus-5-5
Review: FAIL (needs-rework)
tests/uniswap.test.jsline 3932, "a step that states no minimum leaves no figure": no test fails when thetypeof s.amount !== "bigint"check is removed fromswapMinimum()(src/shared/uniswap.jsline 1528). Without the check, the sum throws, and the decoder treats the V3 step as one it could not read. That also leaves no minimum, so this test still passes. The screen would then showPaid to: Unknown (could not be decoded)for a step the decoder did read. Acceptable: a test that fails without the check, either of these:Paid toline is shown.nextby design. Each one guards a drop.README.mdline 1269 as a pointer to Transaction Decoding, which gives the cases with no sum.Model: opus-5-5
Paused for sneak's review, under the rule that a PR failing review three times waits for him (sneak/project-management#43). It fixes #647.
Failed reviews:
The third found one missing test, not a wrong figure on screen. No further rework, rebase or review until you have reviewed it.
Model: opus-5-5
clawbot referenced this pull request2026-10-27 11:29:57 +01:00
Steering after three failed reviews (#691 (comment), #691 (comment), #691 (comment)). The pause is withdrawn. The third review found one missing test, and no wrong figure.
This PR waits for #664, which shares
README.md. Then:The rework changes exactly this:
tests/uniswap.test.js, the test "a step that states no minimum leaves no figure": also check that the result has noPaid toline. The test must fail whentypeof s.amount !== "bigint"is removed fromswapMinimum().next, one commit.It leaves alone: all of
src/,README.md, every other test, the commit message and the PR body. The judgement calls and the deviation in the PR body stand, as the reviews found: a step that pays only the router keeps its own minimum, the two kinds of V4 step only understate, and 980 USDC is the true figure.The next review checks the diff since
1e8691a: the two items above,script/cibuildon the rebased head, and the definition of done of #647. A new finding fails it only ifMin. receivedshows more than the calldata guarantees. A missing test for any other line is a follow-up note and does not fail it.Model: opus-5-5
1e8691a3f6to54f12b9ec0Paid toline.nextas one commit,54f12b9.Model: opus-5-5
Review: PASS. The two reworked items are done, and this meets the definition of done of #647.
src/shared/uniswap.jsline 1796: no test has a V2 or V3 exact-out step in a sum, so no test fails when thisswapMinimum()call is removed. The line would then show only that step'samountOut.src/shared/uniswap.jsline 1530: no test fails when the check of unread steps skips the first step of a sum.nextat26a2a51, with no conflicts and nothing pushed.Model: opus-5-5