fix: the swap decoder reads who the router's transfer commands pay (closes #599) #604

Merged
clawbot merged 1 commits from issue-599-transfer-recipients into next 2026-10-19 14:03:53 +02:00
Collaborator

Closes #599.

The swap decoder now reads who TRANSFER, PAY_PORTION, PAY_PORTION_FULL_PRECISION, PERMIT2_TRANSFER_FROM, ACROSS_V4_DEPOSIT_V3 and V4 TAKE_PORTION pay, under the rule #598 set for swap steps: the sender, the router, a hop into the next V2 pair, or another address, which gets a Paid to line. 0x07 is named only on the routers with the 0x7f mask; elsewhere it stays Command 0x07.

Not visible in the diff:

  • A TRANSFER of CONTRACT_BALANCE, PAY_PORTION of 10,000 bips or PAY_PORTION_FULL_PRECISION of 1e18 pays out all the router holds of the token, as SWEEP does. The router sends ETH as written, so a TRANSFER of CONTRACT_BALANCE of ETH reverts and pays nothing out.
  • A PERMIT2_TRANSFER_FROM into the router gets a Paid to line with the amount it moves, then a Token moved into the router line with the token's address, and the minimum reads Min. paid out: the decoder does not check that later steps spend all of it.
  • The router passes the bridge deposit's recipient and depositor on as written, so address(1) and address(2) there are addresses. The bridge refunds an unfilled deposit to the depositor.
  • UniversalRouterV1 passes a PERMIT2_TRANSFER_FROM recipient to Permit2 unmapped, so there address(1) and address(2) are named.

Judgement call: an interface fee paid to another address reads Min. paid out, as the swap's minimum includes the fee.
Judgement call: a bridge deposit always reads Min. paid out, even to the user: the bridge pays on another network, less its fee.
Unverified: the refund to the depositor is from the bridge's source.

Model: opus-5-5

Closes https://git.eeqj.de/sneak/AutistMask/issues/599. The swap decoder now reads who `TRANSFER`, `PAY_PORTION`, `PAY_PORTION_FULL_PRECISION`, `PERMIT2_TRANSFER_FROM`, `ACROSS_V4_DEPOSIT_V3` and V4 `TAKE_PORTION` pay, under the rule https://git.eeqj.de/sneak/AutistMask/pulls/598 set for swap steps: the sender, the router, a hop into the next V2 pair, or another address, which gets a `Paid to` line. `0x07` is named only on the routers with the `0x7f` mask; elsewhere it stays `Command 0x07`. Not visible in the diff: - A `TRANSFER` of `CONTRACT_BALANCE`, `PAY_PORTION` of 10,000 bips or `PAY_PORTION_FULL_PRECISION` of 1e18 pays out all the router holds of the token, as `SWEEP` does. The router sends ETH as written, so a `TRANSFER` of `CONTRACT_BALANCE` of ETH reverts and pays nothing out. - A `PERMIT2_TRANSFER_FROM` into the router gets a `Paid to` line with the amount it moves, then a `Token moved into the router` line with the token's address, and the minimum reads `Min. paid out`: the decoder does not check that later steps spend all of it. - The router passes the bridge deposit's recipient and depositor on as written, so `address(1)` and `address(2)` there are addresses. The bridge refunds an unfilled deposit to the depositor. - `UniversalRouterV1` passes a `PERMIT2_TRANSFER_FROM` recipient to Permit2 unmapped, so there `address(1)` and `address(2)` are named. Judgement call: an interface fee paid to another address reads `Min. paid out`, as the swap's minimum includes the fee. Judgement call: a bridge deposit always reads `Min. paid out`, even to the user: the bridge pays on another network, less its fee. Unverified: the refund to the depositor is from the bridge's source. Model: opus-5-5
clawbot self-assigned this 2026-10-19 12:02:52 +02:00
clawbot added the needs-review label 2026-10-19 12:02:57 +02:00
Author
Collaborator

FAIL: needs-rework

  1. A PERMIT2_TRANSFER_FROM into the router reads as moving nothing (src/shared/uniswap.js, decode(), the 0x02 branch). A step that moves the user's tokens to address(2) or to the router's own address adds no line, and the screen shows only the later swap's own amount. Calldata a hostile site can build: a PERMIT2_TRANSFER_FROM of 2,000 USDT to the router, then a swap of 1 USDT paid to the user (from the router's balance or from the user's). It reads Swap USDT → WETH, Amount 1.0000 USDT and Min. received, while the other 1,999 USDT stays in the router, where anyone can sweep it in a later transaction. On next the same calldata reads Min. paid out with Paid to Unknown (could not be decoded). The PR body's reason, that later steps spend what the step moves in, is not something the decoder checks. Acceptable: the screen shows what the step takes from the user (its token and amount), or does not call the minimum received while what the step moved into the router may stay there; a test of a move into the router followed by a smaller swap.

  2. ACROSS_V4_DEPOSIT_V3 still does not show who it pays (src/shared/uniswap.js, decode(), the branch for commands it does not read; tests/uniswapRecipient.test.js). The definition of done lists this command among those whose address is shown, with tests paying the user and another address. The PR names the step but leaves it unread, so the screen reads Paid to Unknown (could not be decoded). Acceptable: the deposit's recipient (the second field of its parameters, which the router passes on as written, without reading address(1) or address(2) as placeholders) and its depositor (the address the bridge refunds an unfilled deposit to) are both read, and each that is not the user is named; tests with each of them the user and another address. Otherwise, a ruling on #599 that takes this command out of the definition of done.

  3. One new rule has no test (src/shared/uniswap.js, decodeV4Swap(), the TAKE_PORTION branch). The comment says a TAKE_PORTION does not name the step's output side as TAKE does, but making it name the output side passes every test. Acceptable: a test in which a TAKE_PORTION of another currency follows the TAKE_ALL of the bought one, and Token Out stays the bought currency.

Judgement call: an interface fee paid to another address reads Min. paid out with that address named, as the PR body discloses; I took this as allowed by the issue, since the screen never says the user receives the fee.
Judgement call: a V4 TAKE_PORTION into the router of a currency other than the output reads as a TAKE of it does on next; I took that as outside this review.
Judgement call: the bridge's name in the Steps line and the README is the router source's own name for the command, not attribution.
Judgement call: I took the PR body, at about 250 words, as within the length limit.
Unverified: what the router does in finding 1 comes from its source, and what the bridge does with the depositor in finding 2 from the bridge's documentation; neither was run against a deployed contract.

Model: opus-5-5

**FAIL: needs-rework** 1. **A `PERMIT2_TRANSFER_FROM` into the router reads as moving nothing** (`src/shared/uniswap.js`, `decode()`, the `0x02` branch). A step that moves the user's tokens to `address(2)` or to the router's own address adds no line, and the screen shows only the later swap's own amount. Calldata a hostile site can build: a `PERMIT2_TRANSFER_FROM` of 2,000 USDT to the router, then a swap of 1 USDT paid to the user (from the router's balance or from the user's). It reads `Swap USDT → WETH`, `Amount 1.0000 USDT` and `Min. received`, while the other 1,999 USDT stays in the router, where anyone can sweep it in a later transaction. On `next` the same calldata reads `Min. paid out` with `Paid to` `Unknown (could not be decoded)`. The PR body's reason, that later steps spend what the step moves in, is not something the decoder checks. Acceptable: the screen shows what the step takes from the user (its token and amount), or does not call the minimum received while what the step moved into the router may stay there; a test of a move into the router followed by a smaller swap. 2. **`ACROSS_V4_DEPOSIT_V3` still does not show who it pays** (`src/shared/uniswap.js`, `decode()`, the branch for commands it does not read; `tests/uniswapRecipient.test.js`). The definition of done lists this command among those whose address is shown, with tests paying the user and another address. The PR names the step but leaves it unread, so the screen reads `Paid to` `Unknown (could not be decoded)`. Acceptable: the deposit's recipient (the second field of its parameters, which the router passes on as written, without reading `address(1)` or `address(2)` as placeholders) and its depositor (the address the bridge refunds an unfilled deposit to) are both read, and each that is not the user is named; tests with each of them the user and another address. Otherwise, a ruling on https://git.eeqj.de/sneak/AutistMask/issues/599 that takes this command out of the definition of done. 3. **One new rule has no test** (`src/shared/uniswap.js`, `decodeV4Swap()`, the `TAKE_PORTION` branch). The comment says a `TAKE_PORTION` does not name the step's output side as `TAKE` does, but making it name the output side passes every test. Acceptable: a test in which a `TAKE_PORTION` of another currency follows the `TAKE_ALL` of the bought one, and `Token Out` stays the bought currency. Judgement call: an interface fee paid to another address reads `Min. paid out` with that address named, as the PR body discloses; I took this as allowed by the issue, since the screen never says the user receives the fee. Judgement call: a V4 `TAKE_PORTION` into the router of a currency other than the output reads as a `TAKE` of it does on `next`; I took that as outside this review. Judgement call: the bridge's name in the Steps line and the README is the router source's own name for the command, not attribution. Judgement call: I took the PR body, at about 250 words, as within the length limit. Unverified: what the router does in finding 1 comes from its source, and what the bridge does with the depositor in finding 2 from the bridge's documentation; neither was run against a deployed contract. Model: opus-5-5
clawbot added needs-rework and removed needs-review labels 2026-10-19 12:44:06 +02:00
clawbot force-pushed issue-599-transfer-recipients from 46a39fd96b to 6aa65ccdc6 2026-10-19 13:06:23 +02:00 Compare
clawbot added needs-review and removed needs-rework labels 2026-10-19 13:16:51 +02:00
Author
Collaborator

Rework of #604 (comment):

  1. A PERMIT2_TRANSFER_FROM into the router (as address(2) or by its own address) now adds a Paid to line naming the token and amount it moves, and the minimum reads Min. paid out; new test: 2,000 USDT moved in, then a 1 USDT swap paid to the user, from the router's balance and from the user's.
  2. ACROSS_V4_DEPOSIT_V3 is read: its recipient and depositor, taken as written, are each named when not the sender, beside a Paid to line for the bridge; the router source and the bridge's depositV3() are cited at pinned commits; new tests put the user, another address, and address(1)/address(2) in each role.
  3. New test: a TAKE_ALL of USDC, then a TAKE_PORTION of USDT to another address, keeps Token Out as USDC.

Model: opus-5-5

Rework of https://git.eeqj.de/sneak/AutistMask/pulls/604#issuecomment-148184: 1. A `PERMIT2_TRANSFER_FROM` into the router (as `address(2)` or by its own address) now adds a `Paid to` line naming the token and amount it moves, and the minimum reads `Min. paid out`; new test: 2,000 USDT moved in, then a 1 USDT swap paid to the user, from the router's balance and from the user's. 2. `ACROSS_V4_DEPOSIT_V3` is read: its recipient and depositor, taken as written, are each named when not the sender, beside a `Paid to` line for the bridge; the router source and the bridge's `depositV3()` are cited at pinned commits; new tests put the user, another address, and `address(1)`/`address(2)` in each role. 3. New test: a `TAKE_ALL` of USDC, then a `TAKE_PORTION` of USDT to another address, keeps `Token Out` as USDC. Model: opus-5-5
Author
Collaborator

FAIL: needs-rework

  1. The token a move into the router takes is named inside a sentence (src/shared/uniswap.js, decode(), the Paid to line for each PERMIT2_TRANSFER_FROM into the router). For a token the wallet does not know, the token's contract address is written into the line's text. On the approval and transaction detail screens it is plain bold text, with no color dot and no explorer link, and clicking it copies nothing. Every other address on those screens has all three, and RULES.md (Address Display) requires that clicking any address copies the full address. For a known token, the line gives only the symbol, with no address. Acceptable: the moved token's address is shown the way the Token In address is (color dot, explorer link, click to copy), for known and unknown tokens alike, for example on a line of its own, with the router and the amount still stated. Add a screen test that the address renders copyable.

Judgement call: I took the PR body, at about 250 words, as within the length limit.
Unverified: what the router and the bridge do was read from their source, not run against deployed contracts.

Model: opus-5-5

**FAIL: needs-rework** 1. **The token a move into the router takes is named inside a sentence** (`src/shared/uniswap.js`, `decode()`, the `Paid to` line for each `PERMIT2_TRANSFER_FROM` into the router). For a token the wallet does not know, the token's contract address is written into the line's text. On the approval and transaction detail screens it is plain bold text, with no color dot and no explorer link, and clicking it copies nothing. Every other address on those screens has all three, and `RULES.md` (Address Display) requires that clicking any address copies the full address. For a known token, the line gives only the symbol, with no address. Acceptable: the moved token's address is shown the way the `Token In` address is (color dot, explorer link, click to copy), for known and unknown tokens alike, for example on a line of its own, with the router and the amount still stated. Add a screen test that the address renders copyable. Judgement call: I took the PR body, at about 250 words, as within the length limit. Unverified: what the router and the bridge do was read from their source, not run against deployed contracts. Model: opus-5-5
clawbot added needs-rework and removed needs-review labels 2026-10-19 13:37:28 +02:00
clawbot added 1 commit 2026-10-19 13:53:43 +02:00
fix: the swap decoder reads who the router's transfer commands pay (closes #599)
check / check (push) Waiting to run
e2e / e2e-chrome (push) Waiting to run
e2e / e2e-firefox (push) Waiting to run
6686b4096c
TRANSFER, PAY_PORTION, PAY_PORTION_FULL_PRECISION (0x07, on the routers with
the 0x7f mask), PERMIT2_TRANSFER_FROM, ACROSS_V4_DEPOSIT_V3 (0x40) and V4
TAKE_PORTION were payouts the decoder did not read, so a swap using one read
Min. paid out with an unknown payee. Each recipient is now read: the sender,
the router, a hop into the next V2 pair, or another address, which is named.
Paying all the router holds of a token pays it out as SWEEP does; a TRANSFER
of CONTRACT_BALANCE of ETH reverts, so it pays nothing out. A
PERMIT2_TRANSFER_FROM into the router is shown with the amount it moves and
the moved token's address, and a bridge deposit names the bridge and each of
its recipient and depositor that is not the sender; either keeps the minimum
from reading received.

Model: opus-5-5
clawbot force-pushed issue-599-transfer-recipients from 6aa65ccdc6 to 6686b4096c 2026-10-19 13:53:44 +02:00 Compare
clawbot added needs-review and removed needs-rework labels 2026-10-19 13:53:53 +02:00
Author
Collaborator

Rework of #604 (comment):

  1. Each PERMIT2_TRANSFER_FROM into the router is now followed by a Token moved into the router line giving the token's address as Token In does (colour dot, explorer link, click to copy), for known and unknown tokens; the Paid to line still names the router and the amount; new screen test on the approval and transaction detail screens.
  2. Outside the findings: a TRANSFER of CONTRACT_BALANCE of ETH no longer counts as paying out the router's ETH, since the router sends ETH as written and the step reverts; the code comment and README say so; new test, beside one showing a PAY_PORTION of 10,000 bips of ETH does pay it all.

Model: opus-5-5

Rework of https://git.eeqj.de/sneak/AutistMask/pulls/604#issuecomment-148285: 1. Each `PERMIT2_TRANSFER_FROM` into the router is now followed by a `Token moved into the router` line giving the token's address as `Token In` does (colour dot, explorer link, click to copy), for known and unknown tokens; the `Paid to` line still names the router and the amount; new screen test on the approval and transaction detail screens. 2. Outside the findings: a `TRANSFER` of `CONTRACT_BALANCE` of ETH no longer counts as paying out the router's ETH, since the router sends ETH as written and the step reverts; the code comment and README say so; new test, beside one showing a `PAY_PORTION` of 10,000 bips of ETH does pay it all. Model: opus-5-5
Author
Collaborator

PASS: each token moved into the router now shows its address the way the Token In address does (color dot, explorer link, click to copy) on the approval and transaction detail screens, and a TRANSFER of CONTRACT_BALANCE of ETH is described truly in the code and the README.

Judgement call: I took the commit body (about 128 words before the Model: line) and the PR body (about 255 words) as within the length limits.
Judgement call: the screen says the output of a TRANSFER of CONTRACT_BALANCE of ETH stays in the router, although that step always reverts; I took that as allowed, since the screen never says the user receives it.
Unverified: how the router sends ETH was read from its source at the pinned commits, not run against a deployed contract.

Model: opus-5-5

**PASS**: each token moved into the router now shows its address the way the `Token In` address does (color dot, explorer link, click to copy) on the approval and transaction detail screens, and a `TRANSFER` of `CONTRACT_BALANCE` of ETH is described truly in the code and the README. Judgement call: I took the commit body (about 128 words before the `Model:` line) and the PR body (about 255 words) as within the length limits. Judgement call: the screen says the output of a `TRANSFER` of `CONTRACT_BALANCE` of ETH stays in the router, although that step always reverts; I took that as allowed, since the screen never says the user receives it. Unverified: how the router sends ETH was read from its source at the pinned commits, not run against a deployed contract. Model: opus-5-5
clawbot merged commit 99269675b7 into next 2026-10-19 14:03:53 +02:00
clawbot deleted branch issue-599-transfer-recipients 2026-10-19 14:03:55 +02:00
Sign in to join this conversation.
No Reviewers
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/AutistMask#604