fix: a balance check of another address sets no swap minimum (closes #600)

BALANCE_CHECK_ERC20 takes (owner, token, minBalance), and every Universal
Router release that has it checks the owner's balance as written, without
map(), so address(1) and address(2) there are plain addresses, not the sender
and the router. The decoder read only the token and the minimum, so a check of
another address's balance read as Min. received. A check whose owner is not
the transaction's sender now sets neither Token Out nor the minimum, and its
step reads "Balance Check (of another address)". A check of the sender keeps
the reading it had, including the zero reading for one the router lets fail.

Model: opus-5-5

README.md

@@ -1198,11 +1198,13 @@ number. They can read:
   guarantees nothing: it sets `Token Out` and `Min. received` only when the
   output side holds no minimum, not even a zero one, at the point the check is
   reached, and otherwise leaves the current token and figure in place. A nonzero
-  `minBalance` sets both lines, as a swap step does. A `BALANCE_CHECK_ERC20`
+  `minBalance` sets both lines, as a swap step does. Only a check of the
-  step that the router lets fail without reverting the transaction (see
+  transaction's sender's own balance sets either line; a check of any other
-  Transaction Decoding) guarantees nothing either, whatever its `minBalance`,
+  address sets neither (see Transaction Decoding). A `BALANCE_CHECK_ERC20` step
-  and is read as one whose `minBalance` is zero. A minimum stated by a V4 swap
+  that the router lets fail without reverting the transaction (see Transaction
-  step the router lets fail is likewise read as zero.
+  Decoding) guarantees nothing either, whatever its `minBalance`, and is read as
+  one whose `minBalance` is zero. A minimum stated by a V4 swap step the router
+  lets fail is likewise read as zero.
 
 The swap's `Token In` and `Token Out` lines name a currency, not an amount; each
 reads `Unknown (not named in the calldata)` when the decoder found no token for
@@ -3018,11 +3020,17 @@ Currently supported:
   `ACROSS_V4_DEPOSIT_V3` pays the bridge, which pays the deposit's recipient on
   another network and refunds a deposit nobody fills to its depositor; the
   router passes both on as written, so `address(1)` and `address(2)` there are
-  addresses, and each that is not the transaction's sender is named. When a swap
+  addresses, and each that is not the transaction's sender is named. A
-  step, `TAKE` or `TAKE_PORTION` pays the router the swap's output token, the
+  `BALANCE_CHECK_ERC20` pays nobody: the router checks that its owner holds at
-  output stays in the router unless a later step pays all the router holds of it
+  least `minBalance` of its token, and reads the owner as written, so
-  to anyone but the router: a `SWEEP` of that token, an `UNWRAP_WETH` when it is
+  `address(1)` and `address(2)` there are addresses too. A check of any address
-  WETH, a `TRANSFER` of `CONTRACT_BALANCE`, a `PAY_PORTION` of 10,000 bips or a
+  but the transaction's sender's says nothing about what the user receives: it
+  sets neither `Token Out` nor the minimum, and it is listed as
+  `Balance Check (of another address)`. When a swap step, `TAKE` or
+  `TAKE_PORTION` pays the router the swap's output token, the output stays in
+  the router unless a later step pays all the router holds of it to anyone but
+  the router: a `SWEEP` of that token, an `UNWRAP_WETH` when it is WETH, a
+  `TRANSFER` of `CONTRACT_BALANCE`, a `PAY_PORTION` of 10,000 bips or a
   `PAY_PORTION_FULL_PRECISION` of 1e18 of that token, or an
   `ACROSS_V4_DEPOSIT_V3` of `CONTRACT_BALANCE` of that token, or of ETH when it
   sets `useNative`. The router sends ETH as written, so a `TRANSFER` of

src/shared/uniswap.js

@@ -388,7 +388,13 @@ function decodePermit2(input) {
     }
 }
 
-// Decode BALANCE_CHECK_ERC20 (command 0x0e) input bytes.
+// Decode BALANCE_CHECK_ERC20 (command 0x0e) input bytes. The router passes the
+// check when `owner` holds at least `minBalance` of `token`, and reads `owner`
+// as written, without map(), on every release that has the command:
+// - UniversalRouterV1_2_V2Support: https://github.com/Uniswap/universal-router/blob/41183d6eb154f0ab0e74a0e911a5ef9ea51fc4bd/contracts/base/Dispatcher.sol
+// - UniversalRouterV2: https://github.com/Uniswap/universal-router/blob/3663f6db6e2fe121753cd2d899699c2dc75dca86/contracts/base/Dispatcher.sol
+// - UniversalRouterV2_1_1 and UniversalRouterV2_1_2, the same file at both
+//   commits: https://github.com/Uniswap/universal-router/blob/802fe4c18f47300e0f183e2a42e9146ec2ea9fc3/contracts/base/Dispatcher.sol
 // ABI: (address owner, address token, uint256 minBalance)
 function decodeBalanceCheck(input) {
     try {
@@ -1087,15 +1093,26 @@ function decode(data, toAddress, chainId, sources, fromAddress) {
                     }
                 } else if (cmdId === 0x0e) {
                     const b = decodeBalanceCheck(inputs[i]);
-                    // The router passes this check whenever the owner holds at
+                    if (b && b.owner.toLowerCase() !== sender) {
-                    // least minBalance, so a zero one guarantees nothing and
+                        // The router reads the owner as written (see
-                    // does not replace a minimum an earlier step stated. A
+                        // decodeBalanceCheck()), so address(1) and address(2)
-                    // check the router lets fail guarantees nothing either, so
+                        // are not the sender and the router here. A check of
-                    // it is read as a zero one. Any other minBalance sets the
+                        // any address but the sender's says nothing about
-                    // output side as a swap does. It pays nobody.
+                        // what the user receives: it sets no side, and the
-                    const minBalance = b && !allowedToFail ? b.minBalance : 0n;
+                        // step says so.
-                    if (b && !(minBalance === 0n && present(minOutput))) {
+                        commandNames[i] += " (of another address)";
-                        setOutput(b.token, minBalance);
+                    } else if (b) {
+                        // The router passes this check whenever the owner
+                        // holds at least minBalance, so a zero one guarantees
+                        // nothing and does not replace a minimum an earlier
+                        // step stated. A check the router lets fail guarantees
+                        // nothing either, so it is read as a zero one. Any
+                        // other minBalance sets the output side as a swap
+                        // does. It pays nobody.
+                        const minBalance = allowedToFail ? 0n : b.minBalance;
+                        if (!(minBalance === 0n && present(minOutput))) {
+                            setOutput(b.token, minBalance);
+                        }
                     }
                 } else if (cmdId === 0x00) {
                     const s = decodeV3SwapExactIn(inputs[i]);

tests/uniswapRecipient.test.js

@@ -1,8 +1,10 @@
 // The address a swap's output is paid to
 // (https://git.eeqj.de/sneak/AutistMask/issues/594 and
-// https://git.eeqj.de/sneak/AutistMask/issues/599), and the allow-revert flag
+// https://git.eeqj.de/sneak/AutistMask/issues/599), the allow-revert flag
 // on the steps that pay it
-// (https://git.eeqj.de/sneak/AutistMask/issues/602).
+// (https://git.eeqj.de/sneak/AutistMask/issues/602), and the address whose
+// balance a BALANCE_CHECK_ERC20 checks
+// (https://git.eeqj.de/sneak/AutistMask/issues/600).
 //
 // Every swap command, SWEEP, WRAP_ETH, UNWRAP_WETH, TRANSFER, PAY_PORTION,
 // PAY_PORTION_FULL_PRECISION, PERMIT2_TRANSFER_FROM, ACROSS_V4_DEPOSIT_V3 and
@@ -47,6 +49,8 @@ const ROUTER = "0x66a9893cc07d91d95644aedd05d03f95e1dba8af";
 const ROUTER_V2_1_2 = "0x23617e59A5925b2A4Bf75d73ff6711cD0b29De85";
 // UniversalRouterV1 on Ethereum Mainnet.
 const ROUTER_V1 = "0xEf1c6E67703c7BD7107eed8303Fbe6EC2554BF6B";
+// UniversalRouterV1_2_V2Support on Ethereum Mainnet.
+const ROUTER_V1_2 = "0x3fC91A3afd70395Cd496C647d5a6CC9D4B2b7FAD";
 const USER = "0x66133E8ea0f5D1d612D2502a968757D1048c214a";
 const OTHER = "0x7777777777777777777777777777777777777777";
 const MSG_SENDER = "0x0000000000000000000000000000000000000001";
@@ -219,16 +223,20 @@ function unwrapWeth(recipient) {
     return [0x0c, coder.encode(["address", "uint256"], [recipient, 0n])];
 }
 
-function balanceCheck(token, minBalance) {
+// BALANCE_CHECK_ERC20 (0x0e) of `owner`'s balance of `token`.
+function balanceCheck(token, minBalance, owner = USER) {
     return [
         0x0e,
         coder.encode(
             ["address", "address", "uint256"],
-            [USER, token, minBalance],
+            [owner, token, minBalance],
         ),
     ];
 }
 
+// `step` with the allow-revert flag set on its command byte.
+const flagged = ([command, input]) => [command | 0x80, input];
+
 // `step`, a [command or action, input] pair, with the top byte of the input's
 // word `index` set. The decoder cannot read that word as an address.
 function withDirtyWord([id, input], index) {
@@ -1383,15 +1391,11 @@ describe("paths whose recipients were untested", () => {
 // and goes on to the next step only for some commands, none of them a swap or
 // a payment; on every other command the flag has no effect.
 describe("the allow-revert flag", () => {
-    // UniversalRouterV1_2_V2Support on Ethereum Mainnet.
-    const ROUTER_V1_2 = "0x3fC91A3afd70395Cd496C647d5a6CC9D4B2b7FAD";
     // UniversalRouterV2_1_1 on Ethereum Mainnet.
     const ROUTER_V2_1_1 = "0x4C82D1fBFe28C977cBB58D8C7FF8FCF9F70a2cCA";
     // The WETH/USDC pair on Ethereum Mainnet, as its V2 factory reports it.
     const PAIR = "0xB4e16d0168e52d35CaCD2c6185b44281Ec28C9Dc";
 
-    // `step` with the allow-revert flag set on its command byte.
-    const flagged = ([command, input]) => [command | 0x80, input];
     // `steps` with the flag set on the one at `index`.
     const flagStep = (steps, index) =>
         steps.map((s, i) => (i === index ? flagged(s) : s));
@@ -1714,6 +1718,86 @@ describe("the allow-revert flag", () => {
     );
 });
 
+// https://git.eeqj.de/sneak/AutistMask/issues/600: on every release that has
+// BALANCE_CHECK_ERC20, the router checks the balance of the owner as written,
+// without map(), so address(1) and address(2) there are those addresses, not
+// the sender and the router. Only a check of the sender's own balance says
+// anything about what the user receives.
+describe.each([
+    { router: "UniversalRouterV1_2_V2Support", address: ROUTER_V1_2 },
+    { router: "UniversalRouterV2", address: ROUTER },
+    { router: "UniversalRouterV2_1_2", address: ROUTER_V2_1_2 },
+])("on $router, the owner of a BALANCE_CHECK_ERC20", ({ address }) => {
+    // A swap of 2,000 USDT for WETH with no minimum, paid to the user.
+    const swap = v3SwapExactIn(MSG_SENDER, USDT_2000, 0n, USDT, WETH);
+    const NO_MINIMUM = "None (no minimum guaranteed)";
+
+    test("the sender: the check sets the minimum, unless the router lets it fail", () => {
+        const check = balanceCheck(WETH, ONE_WETH, USER);
+
+        const plain = decode(execute(swap, check), USER, address);
+        expect(detail(plain, "Steps").value).toBe(
+            "V3 Swap (Exact In) → Balance Check",
+        );
+        expect(detail(plain, "Min. received").value).toBe("1.0000 WETH");
+        // The sender as the From address gives it, in lower case.
+        expect(
+            decode(execute(swap, check), USER.toLowerCase(), address),
+        ).toEqual(plain);
+
+        const withFlag = decode(execute(swap, flagged(check)), USER, address);
+        expect(detail(withFlag, "Steps").value).toBe(
+            "V3 Swap (Exact In) → Balance Check (allowed to fail)",
+        );
+        expect(detail(withFlag, "Min. received").value).toBe(NO_MINIMUM);
+    });
+
+    const notTheSender = [
+        { kind: "address(1)", owner: MSG_SENDER },
+        { kind: "address(2)", owner: ADDRESS_THIS },
+        { kind: "another address", owner: OTHER },
+    ];
+
+    test.each(notTheSender)(
+        "$kind: after a swap paid to the user, the check sets no minimum and is said to be of another address",
+        ({ owner }) => {
+            const check = balanceCheck(WETH, ONE_WETH, owner);
+
+            const plain = decode(execute(swap, check), USER, address);
+            expect(detail(plain, "Steps").value).toBe(
+                "V3 Swap (Exact In) → Balance Check (of another address)",
+            );
+            expect(paidTo(plain)).toEqual([]);
+            expect(detail(plain, "Min. received").value).toBe(NO_MINIMUM);
+
+            const withFlag = decode(
+                execute(swap, flagged(check)),
+                USER,
+                address,
+            );
+            expect(detail(withFlag, "Steps").value).toBe(
+                "V3 Swap (Exact In) → Balance Check (of another address) (allowed to fail)",
+            );
+            expect(paidTo(withFlag)).toEqual([]);
+            expect(detail(withFlag, "Min. received").value).toBe(NO_MINIMUM);
+        },
+    );
+
+    test.each(notTheSender)(
+        "$kind: a check alone names no output token and states no minimum",
+        ({ owner }) => {
+            const check = balanceCheck(WETH, ONE_WETH, owner);
+
+            for (const step of [check, flagged(check)]) {
+                const result = decode(execute(step), USER, address);
+                expect(detail(result, "Token Out").value).toBe(UNNAMED);
+                expect(detail(result, "Min. received")).toBeUndefined();
+                expect(detail(result, "Min. paid out")).toBeUndefined();
+            }
+        },
+    );
+});
+
 // The DOM stub both screens are drawn into.
 function makeElement(id) {
     const classes = new Set();

tests/uniswapUndeterminedTokenIn.test.js

@@ -71,6 +71,8 @@ function exactInParams(currencyIn, path, amountIn, amountOutMin) {
 
 // BALANCE_CHECK_ERC20 (command 0x0e): (address owner, address token,
 // uint256 minBalance). It names the output token and nothing about the input.
+// Its owner is USER, the transaction's sender: a check of any other address
+// names no output token.
 function balanceCheck(token, minBalance) {
     return coder.encode(
         ["address", "address", "uint256"],
@@ -78,8 +80,13 @@ function balanceCheck(token, minBalance) {
     );
 }
 
+// A transaction from USER.
+function decode(data) {
+    return uniswap.decode(data, ROUTER, "0x1", {}, USER);
+}
+
 function detail(data, label) {
-    const decoded = uniswap.decode(data, ROUTER, "0x1", {});
+    const decoded = decode(data);
     expect(decoded).not.toBeNull();
     return decoded.details.find((d) => d.label === label);
 }
@@ -113,9 +120,7 @@ describe("an execute() whose input currency never decoded", () => {
     });
 
     test("does not name ETH in the swap title either", () => {
-        expect(uniswap.decode(data(), ROUTER, "0x1", {}).name).toBe(
+        expect(decode(data()).name).toBe("Uniswap Swap");
-            "Uniswap Swap",
-        );
     });
 });
 
@@ -125,9 +130,7 @@ describe("an execute() that names only an output token", () => {
 
     test("refuses the input rather than defaulting it to ETH", () => {
         expect(detail(data(), "Token In").value).toBe(REFUSAL);
-        expect(uniswap.decode(data(), ROUTER, "0x1", {}).name).toBe(
+        expect(decode(data()).name).toBe("Uniswap Swap");
-            "Uniswap Swap",
-        );
     });
 
     test("still states the output it did establish", () => {
@@ -175,9 +178,7 @@ describe("native ETH in, which V4 spells as the zero address", () => {
 
         expect(detail(data, "Token In").value).toBe("ETH (native)");
         expect(detail(data, "Amount").value).toBe("1.0000 ETH");
-        expect(uniswap.decode(data, ROUTER, "0x1", {}).name).toBe(
+        expect(decode(data).name).toBe("Swap ETH → USDT");
-            "Swap ETH → USDT",
-        );
     });
 
     test("WRAP_ETH is still ETH at 18 decimals", () => {