diff --git a/README.md b/README.md index 05bdf8b..1fd1ff2 100644 --- a/README.md +++ b/README.md @@ -1922,9 +1922,12 @@ view would leave a wallet one click from deletion. - Amount + symbol - To: color dot + full address + etherscan link, or for a contract creation the same sentence as on WaitTx - - Block number + - Block number + etherscan link - Transaction hash: full hash (tap to copy) + etherscan link - "Done" button +- **Behavior**: As on WaitTx, every etherscan link on the screen opens the + explorer of the network the transaction was sent on, whichever network is + active. - **Transitions**: - "Done" in the approval popup → closes the popup window - "Done" otherwise → resets the navigation stack, then → **AddressToken** @@ -1944,6 +1947,9 @@ view would leave a wallet one click from deletion. - Transaction hash section (hidden if broadcast failed before getting hash): full hash (tap to copy) + etherscan link - "Done" button +- **Behavior**: As on WaitTx, every etherscan link on the screen opens the + explorer of the network the transaction was sent on, whichever network is + active. - **Transitions**: - "Done" in the approval popup → closes the popup window - "Done" otherwise → resets the navigation stack, then → **AddressToken** diff --git a/src/popup/viewRouter.js b/src/popup/viewRouter.js index aa2b8a1..caefb26 100644 --- a/src/popup/viewRouter.js +++ b/src/popup/viewRouter.js @@ -205,11 +205,19 @@ function renderView(view, state, views) { if (!data.hash) return false; if (!isAddressText(data.to)) return false; if (!isRenderableDecoded(data.decoded)) return false; + // `chainId` picks the network whose explorer the links open, so + // when it is there it has to name one. Likewise on error-tx. + if (data.chainId && !SUPPORTED_CHAIN_IDS.has(data.chainId)) { + return false; + } views.txStatus.renderSuccess(); return true; case "error-tx": if (!data.message) return false; if (!isAddressText(data.to)) return false; + if (data.chainId && !SUPPORTED_CHAIN_IDS.has(data.chainId)) { + return false; + } views.txStatus.renderError(); return true; default: diff --git a/src/popup/views/txStatus.js b/src/popup/views/txStatus.js index 005c6c7..f6fcfb3 100644 --- a/src/popup/views/txStatus.js +++ b/src/popup/views/txStatus.js @@ -64,8 +64,8 @@ function endWait() { } // A contract creation reaches these screens with `to` as "" (approval.js -// writes `to: toAddr || ""`). The link opens the explorer of the network with -// `chainId`, or of the active network when none is given. +// writes `to: toAddr || ""`). The links below open the explorer of the network +// with `chainId`, or of the active network when none is given. function toAddressHtml(address, chainId) { if (!address) return escapeHtml(CONTRACT_CREATION_TEXT); const title = addressTitle(address, state.wallets); @@ -77,9 +77,9 @@ function txHashHtml(hash, chainId) { return copyableHtml(hash, "break-all") + etherscanLinkHtml(link); } -function blockNumberHtml(blockNumber) { +function blockNumberHtml(blockNumber, chainId) { const num = String(blockNumber); - const link = explorerUrl("block", num); + const link = explorerUrl("block", num, chainId); return copyableHtml(num) + etherscanLinkHtml(link); } @@ -95,11 +95,11 @@ function startWait(txInfo, txHash, broadcastTime, pollNow) { // native currency of txInfo.chainId, the network the transaction was sent // on, not the active network's: the active network can change, in // Settings or when the user approves a site's request, while this screen - // is open or before a later popup resumes it. This screen's links open - // that network's explorer, and its receipt is asked of that network's - // node, for the same reason. A null token is a swap whose amount was not - // decoded (see showTxApproval() in approval.js): the amount says so, and - // no symbol follows it. + // is open or before a later popup resumes it. The links on all three + // screens open that network's explorer, and this screen's receipt is + // asked of that network's node, for the same reason. A null token is a + // swap whose amount was not decoded (see showTxApproval() in + // approval.js): the amount says so, and no symbol follows it. let symbol = ""; if (txInfo.token === "ETH") { symbol = nativeCurrencyByChainId(txInfo.chainId); @@ -258,12 +258,13 @@ function showSuccess(txInfo, txHash, blockNumber) { hash: txHash, blockNumber: blockNumber, decoded: txInfo.decoded || null, + chainId: txInfo.chainId, }; renderSuccess(); ctx.doRefreshAndRender(); } -function decodedDetailsHtml(decoded) { +function decodedDetailsHtml(decoded, chainId) { if (!decoded || !decoded.details) return ""; let html = `
`; if (decoded.name) { @@ -281,9 +282,9 @@ function decodedDetailsHtml(decoded) { if (d.isToken) { const sym = tokenLabel(d.address) || "Unknown token"; html += `
${escapeHtml(sym)}
`; - html += toAddressHtml(d.address); + html += toAddressHtml(d.address, chainId); } else { - html += toAddressHtml(d.address); + html += toAddressHtml(d.address, chainId); } } else { html += `
${escapeHtml(d.value)}
`; @@ -312,16 +313,16 @@ function renderSuccess() { toSection.classList.remove("hidden"); $("success-tx-summary").textContent = d.amount + (d.symbol ? " " + d.symbol : ""); - $("success-tx-to").innerHTML = toAddressHtml(d.to); + $("success-tx-to").innerHTML = toAddressHtml(d.to, d.chainId); } - $("success-tx-block").innerHTML = blockNumberHtml(d.blockNumber); - $("success-tx-hash").innerHTML = txHashHtml(d.hash); + $("success-tx-block").innerHTML = blockNumberHtml(d.blockNumber, d.chainId); + $("success-tx-hash").innerHTML = txHashHtml(d.hash, d.chainId); // Show decoded calldata details if present const decodedEl = $("success-tx-decoded"); if (decodedEl && hasDecoded) { - decodedEl.innerHTML = decodedDetailsHtml(d.decoded); + decodedEl.innerHTML = decodedDetailsHtml(d.decoded, d.chainId); decodedEl.classList.remove("hidden"); } else if (decodedEl) { decodedEl.classList.add("hidden"); @@ -347,6 +348,7 @@ function showError(txInfo, txHash, message) { to: txInfo.to, hash: txHash || null, message: message, + chainId: txInfo.chainId, }; renderError(); } @@ -356,11 +358,11 @@ function renderError() { if (!d || !d.message) return; $("error-tx-summary").textContent = d.amount + (d.symbol ? " " + d.symbol : ""); - $("error-tx-to").innerHTML = toAddressHtml(d.to); + $("error-tx-to").innerHTML = toAddressHtml(d.to, d.chainId); $("error-tx-message").textContent = d.message; if (d.hash) { - $("error-tx-hash").innerHTML = txHashHtml(d.hash); + $("error-tx-hash").innerHTML = txHashHtml(d.hash, d.chainId); $("error-tx-hash-section").classList.remove("hidden"); attachCopyHandlers("view-error-tx"); } else { diff --git a/tests/persistedFieldContract.test.js b/tests/persistedFieldContract.test.js index 995a57b..b2f0c06 100644 --- a/tests/persistedFieldContract.test.js +++ b/tests/persistedFieldContract.test.js @@ -441,6 +441,17 @@ const CONTRACT = [ views: ["error-tx"], restored: false, }, + // A chain id naming no network has no explorer to link to. + { + value: { hash: "0x1", to: ADDRESS, chainId: "0x5" }, + views: ["success-tx"], + restored: false, + }, + { + value: { message: "boom", to: ADDRESS, chainId: "0x5" }, + views: ["error-tx"], + restored: false, + }, // transaction passes on `data.tx`. { value: { tx: { hash: "0x1" } }, diff --git a/tests/txStatus.test.js b/tests/txStatus.test.js index 8880c7c..a78e9b6 100644 --- a/tests/txStatus.test.js +++ b/tests/txStatus.test.js @@ -566,6 +566,85 @@ describe("WaitTx for a Sepolia transaction with mainnet active", () => { }); }); +// SuccessTx and ErrorTx follow WaitTx and describe the same transaction, but +// their links went to the active network's explorer +// (https://git.eeqj.de/sneak/AutistMask/issues/622). Each test checks the +// screen again after rendering it from its stored view data alone, as a +// reopened popup does. +describe("SuccessTx and ErrorTx for a Sepolia transaction with mainnet active", () => { + const SEPOLIA_EXPLORER = "https://sepolia.etherscan.io"; + const TOKEN = "0x1c7D4B196Cb0C7B01d743Fbc6116a902379C7238"; + const sepoliaTx = { ...TX_INFO, chainId: NETWORKS.sepolia.chainId }; + + test("the recipient, block and hash on SuccessTx link to the Sepolia explorer", async () => { + function expectSepoliaLinks() { + expect(getElement("success-tx-to").innerHTML).toContain( + `href="${SEPOLIA_EXPLORER}/address/${RECIPIENT}"`, + ); + expect(getElement("success-tx-block").innerHTML).toContain( + `href="${SEPOLIA_EXPLORER}/block/21000000"`, + ); + expect(getElement("success-tx-hash").innerHTML).toContain( + `href="${SEPOLIA_EXPLORER}/tx/${TX_HASH}"`, + ); + } + mockReceiptLookup.mockResolvedValue({ blockNumber: 21000000 }); + txStatus.showWait(sepoliaTx, TX_HASH); + await jest.advanceTimersByTimeAsync(10000); + expect(state.currentView).toBe("success-tx"); + expectSepoliaLinks(); + + elements.clear(); + txStatus.renderSuccess(); + expectSepoliaLinks(); + }); + + test("the addresses in SuccessTx's decoded action link to the Sepolia explorer", async () => { + function expectSepoliaLinks() { + const html = getElement("success-tx-decoded").innerHTML; + expect(html).toContain( + `href="${SEPOLIA_EXPLORER}/address/${TOKEN}"`, + ); + expect(html).toContain( + `href="${SEPOLIA_EXPLORER}/address/${RECIPIENT}"`, + ); + } + const decoded = { + name: "Transfer", + details: [ + { label: "Token", address: TOKEN, isToken: true }, + { label: "Recipient", address: RECIPIENT }, + ], + }; + mockReceiptLookup.mockResolvedValue({ blockNumber: 21000000 }); + txStatus.showWait({ ...sepoliaTx, decoded }, TX_HASH); + await jest.advanceTimersByTimeAsync(10000); + expect(state.currentView).toBe("success-tx"); + expectSepoliaLinks(); + + elements.clear(); + txStatus.renderSuccess(); + expectSepoliaLinks(); + }); + + test("the recipient and hash on ErrorTx link to the Sepolia explorer", () => { + function expectSepoliaLinks() { + expect(getElement("error-tx-to").innerHTML).toContain( + `href="${SEPOLIA_EXPLORER}/address/${RECIPIENT}"`, + ); + expect(getElement("error-tx-hash").innerHTML).toContain( + `href="${SEPOLIA_EXPLORER}/tx/${TX_HASH}"`, + ); + } + txStatus.showError(sepoliaTx, TX_HASH, "The transaction failed."); + expectSepoliaLinks(); + + elements.clear(); + txStatus.renderError(); + expectSepoliaLinks(); + }); +}); + 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