diff --git a/README.md b/README.md index c2c7bcc..25c7f61 100644 --- a/README.md +++ b/README.md @@ -787,6 +787,7 @@ Shown after a broadcast. The screen holds: - the heading "Transaction Broadcast", with no "Back" button; - the amount and symbol; - each `Moved from the sender` amount of a swap; +- each `Permit2 allowance` amount of a swap; - the recipient; - the transaction hash, with an explorer link; - the counter "Waiting for confirmation... Ns". @@ -826,6 +827,7 @@ The screen holds: - the heading "Transaction Failed"; - the amount; - each `Moved from the sender` amount of a swap; +- each `Permit2 allowance` amount of a swap; - the recipient; - the error message, in a dashed box; - the transaction hash, when one exists; diff --git a/src/popup/index.html b/src/popup/index.html index 2ccd7d0..4b4b418 100644 --- a/src/popup/index.html +++ b/src/popup/index.html @@ -810,7 +810,7 @@

Transaction Broadcast

Amount
-
+
@@ -857,7 +857,10 @@

Transaction Failed

Amount
-
+
diff --git a/src/popup/views/txStatus.js b/src/popup/views/txStatus.js index d0675fc..6955c9d 100644 --- a/src/popup/views/txStatus.js +++ b/src/popup/views/txStatus.js @@ -73,24 +73,25 @@ function toAddressHtml(address, chainId) { return renderAddressHtml(address, { title, chainId }); } -// The amount of each `Moved from the sender` line of a decoded swap, for the -// wait and error screens to show under the Amount figure. That figure can leave -// these amounts out, so alone it can read as less than can have left the -// wallet. `decoded` can come back from storage in any shape, so this shows only -// the lines whose value is text. -function movedFromSenderHtml(decoded) { +// The amount of each `Moved from the sender` and `Permit2 allowance` line of a +// decoded swap, for the wait and error screens to show under the Amount figure. +// That figure can leave these amounts out, so alone it can understate what can +// leave the wallet. `decoded` can come back from storage in any shape, so this +// shows only the lines whose value is text. +function movedOrAllowedHtml(decoded) { if (!decoded || !Array.isArray(decoded.details)) return ""; let html = ""; for (const d of decoded.details) { if ( !d || - d.label !== "Moved from the sender" || + (d.label !== "Moved from the sender" && + d.label !== "Permit2 allowance") || typeof d.value !== "string" ) continue; html += `
`; html += `
${escapeHtml(d.label)}
`; - html += `
${escapeHtml(d.value)}
`; + html += `
${escapeHtml(d.value)}
`; html += `
`; } return html; @@ -134,7 +135,7 @@ function startWait(txInfo, txHash, broadcastTime, pollNow) { txInfo.amount + (symbol ? " " + symbol : ""); $("wait-tx-token-note").innerHTML = blockscoutOffTokenNoteHtml(txInfo.token, txInfo.chainId) + - movedFromSenderHtml(txInfo.decoded); + movedOrAllowedHtml(txInfo.decoded); $("wait-tx-to").innerHTML = toAddressHtml(txInfo.to, txInfo.chainId); $("wait-tx-hash").innerHTML = txHashHtml(txHash, txInfo.chainId); attachCopyHandlers("view-wait-tx"); @@ -245,7 +246,7 @@ function showWait(txInfo, txHash) { // to name a network. txInfo.token and txInfo.tokenSymbol are not checked here. // blockscoutOffTokenNoteHtml() shows no line for a token that is not text. // Everything else only compares and coalesces them, and tokenSymbol is null for -// ETH. txInfo.decoded is not checked either: see movedFromSenderHtml(). +// ETH. txInfo.decoded is not checked either: see movedOrAllowedHtml(). function restoreWait() { const d = state.viewData; if (!d || !d.pendingWait) return false; @@ -391,7 +392,7 @@ function renderError() { d.amount + (d.symbol ? " " + d.symbol : ""); $("error-tx-token-note").innerHTML = blockscoutOffTokenNoteHtml(d.token, d.chainId) + - movedFromSenderHtml(d.decoded); + movedOrAllowedHtml(d.decoded); $("error-tx-to").innerHTML = toAddressHtml(d.to, d.chainId); $("error-tx-message").textContent = d.message; diff --git a/tests/swapStatusAmount.test.js b/tests/swapStatusAmount.test.js index dfac9a3..a54ff5c 100644 --- a/tests/swapStatusAmount.test.js +++ b/tests/swapStatusAmount.test.js @@ -412,3 +412,77 @@ describe("a swap with a Moved from the sender line of its own", () => { expectMovedLine("error-tx-token-note"); }); }); + +// https://git.eeqj.de/sneak/AutistMask/issues/733: a PERMIT2_PERMIT that gives +// another spender an unlimited USDC allowance, then a V4 swap of 1 ETH. The +// Amount line reads 1.0000 ETH and says nothing of the allowance. +describe("a swap that gives another spender an allowance", () => { + const OTHER_SPENDER = "0x1111111111111111111111111111111111111111"; + const MAX_UINT160 = (1n << 160n) - 1n; + const SEND_FAILS = { + error: "The transaction could not be sent.", + retryable: false, + }; + const data = routerIface.encodeFunctionData("execute", [ + "0x0a10", + [ + coder.encode( + [ + "tuple(tuple(address,uint160,uint48,uint48),address,uint256)", + "bytes", + ], + [ + [[USDC, MAX_UINT160, 0, 0], OTHER_SPENDER, 9999999999n], + "0x1234", + ], + ), + coder.encode( + ["bytes", "bytes[]"], + [ + new Uint8Array([SWAP_EXACT_IN_SINGLE, SETTLE, TAKE]), + [ + coder.encode( + [`tuple(${POOL_KEY},bool,uint128,uint128,bytes)`], + [[ETH_USDC_POOL, true, ONE_ETH, 990000n, "0x"]], + ), + settleEth, + takeUsdc, + ], + ], + ), + ], + 9999999999n, + ]); + + function expectAllowanceLine(id) { + const html = node(id).innerHTML; + expect(html).toContain("Permit2 allowance"); + expect(html).toContain("Unlimited"); + } + + test("the wait screen shows the allowance under the Amount figure", async () => { + await approveSwap(data, { txHash: TX_HASH }, ONE_ETH); + expect(node("wait-tx-summary").textContent).toBe("1.0000 ETH"); + expectAllowanceLine("wait-tx-token-note"); + }); + + test("the error screen shows the allowance under the Amount figure", async () => { + await approveSwap(data, SEND_FAILS, ONE_ETH); + expect(node("error-tx-summary").textContent).toBe("1.0000 ETH"); + expectAllowanceLine("error-tx-token-note"); + }); + + test("a reopened popup's wait screen still shows it", async () => { + await approveSwap(data, { txHash: TX_HASH }, ONE_ETH); + node("wait-tx-token-note").innerHTML = ""; + txStatus.restoreWait(); + expectAllowanceLine("wait-tx-token-note"); + }); + + test("a reopened popup's error screen still shows it", async () => { + await approveSwap(data, SEND_FAILS, ONE_ETH); + node("error-tx-token-note").innerHTML = ""; + txStatus.renderError(); + expectAllowanceLine("error-tx-token-note"); + }); +}); diff --git a/tests/txStatus.test.js b/tests/txStatus.test.js index d7a02ea..7c97782 100644 --- a/tests/txStatus.test.js +++ b/tests/txStatus.test.js @@ -101,6 +101,9 @@ global.document = { global.window = { location: { search: "" } }; +const fs = require("fs"); +const path = require("path"); + // Clones in both directions, as the real chrome.storage.local does; the stub // here used to hand back the live stored object, so an in-memory mutation // looked like a write that had reached storage. See @@ -743,6 +746,7 @@ describe("the Moved from the sender lines of a swap on WaitTx and ErrorTx", () = { details: [{ label: "Moved from the sender" }] }, { details: [moved(7)] }, { details: [moved({ toString: 1 })] }, + { details: [{ label: "Permit2 allowance", value: 7 }] }, ]) { expect(noteOnReopen(saved)).toBe(""); } @@ -752,6 +756,74 @@ describe("the Moved from the sender lines of a swap on WaitTx and ErrorTx", () = }); }); +describe("the Permit2 allowance lines of a swap on WaitTx and ErrorTx", () => { + const allowance = (value) => ({ label: "Permit2 allowance", value }); + const decoded = { + details: [ + allowance("Unlimited"), + { label: "Allowance token", value: "USDC" }, + allowance("400.0000 DAI"), + ], + }; + + function expectEveryLine(id) { + const html = getElement(id).innerHTML; + expect(html).toContain("Permit2 allowance"); + expect(html).toContain("Unlimited"); + expect(html).toContain("<b>400.0000</b> DAI"); + expect(html).not.toContain("Allowance token"); + } + + test("WaitTx shows each line's amount, escaped", () => { + mockReceiptLookup.mockResolvedValue(null); + txStatus.showWait({ ...TX_INFO, decoded }, TX_HASH); + expectEveryLine("wait-tx-token-note"); + }); + + test("ErrorTx shows each line's amount, escaped", () => { + txStatus.showError({ ...TX_INFO, decoded }, TX_HASH, "It failed."); + expectEveryLine("error-tx-token-note"); + }); + + test("a wait that ends in ErrorTx keeps them", async () => { + mockReceiptLookup.mockResolvedValue(null); + txStatus.showWait({ ...TX_INFO, decoded }, TX_HASH); + await jest.advanceTimersByTimeAsync(60000); + expect(state.currentView).toBe("error-tx"); + expectEveryLine("error-tx-token-note"); + }); +}); + +// An amount of about 47 digits or more is wider than the screen. Without the +// break-all class its last digits are cut off at the edge. +describe("a long amount on WaitTx and ErrorTx", () => { + test("each amount under the Amount figure wraps", () => { + const decoded = { + details: [ + { label: "Moved from the sender", value: "1.0000 ETH" }, + { label: "Permit2 allowance", value: "Unlimited" }, + ], + }; + txStatus.showError({ ...TX_INFO, decoded }, TX_HASH, "It failed."); + const html = getElement("error-tx-token-note").innerHTML; + expect(html).toContain('
1.0000 ETH<'); + expect(html).toContain('
Unlimited<'); + }); + + test("the Amount figure wraps", () => { + const html = fs.readFileSync( + path.join(__dirname, "..", "src", "popup", "index.html"), + "utf8", + ); + expect(html).toMatch( + /id="wait-tx-summary"\s+class="font-bold break-all"/, + ); + expect(html).toMatch( + /id="error-tx-summary"\s+class="font-bold break-all"/, + ); + }); +}); + describe("wait-tx is a view the popup may reopen onto", () => { // The resume feature is wired through RESTORABLE_VIEWS: restoreView() // refuses any view not in the set, so dropping "wait-tx" from it kills