diff --git a/README.md b/README.md index ab1ff6b..dd2ca4e 100644 --- a/README.md +++ b/README.md @@ -1891,14 +1891,17 @@ view would leave a wallet one click from deletion. no recipient." instead - Transaction hash: full hash (tap to copy) + etherscan link - Count-up timer: "Waiting for confirmation... Ns" -- **Behavior**: Polls `getTransactionReceipt` every 10 seconds. The wait is - persisted: closing and reopening the popup resumes the poll, with the elapsed - counter and the timeout deadline still measured from the original broadcast. A - lookup that fails is retried on the next tick rather than counted as a missing - receipt, because a failed lookup says nothing about the transaction; but six - failures in a row (60 seconds at the poll cadence) end the wait, so an RPC - that never answers cannot leave it running indefinitely. Any lookup that - answers resets that count. +- **Behavior**: Polls `getTransactionReceipt` every 10 seconds. The screen + describes the transaction on the network it was sent on, whichever network is + active: the receipt is asked of that network's RPC endpoint, the one Settings + has for it, and the To and hash links open that network's explorer. The wait + is persisted: closing and reopening the popup resumes the poll, with the + elapsed counter and the timeout deadline still measured from the original + broadcast. A lookup that fails is retried on the next tick rather than counted + as a missing receipt, because a failed lookup says nothing about the + transaction; but six failures in a row (60 seconds at the poll cadence) end + the wait, so an RPC that never answers cannot leave it running indefinitely. + Any lookup that answers resets that count. - **Transitions**: - Receipt found → **SuccessTx** - A lookup that answers "no receipt" 60 seconds or more after broadcast → @@ -1963,7 +1966,8 @@ view would leave a wallet one click from deletion. #### TransactionDetail (`transaction`) - **When**: User tapped a transaction row on Home, AddressDetail, or - AddressToken. + AddressToken. Its etherscan links open the explorer of the network the + transaction was sent on, whichever network is active. - **Elements** (grouped into logical blocks using light well containers; field labels are self-explanatory so groups have no headings): - "Transaction" heading, "Back" button diff --git a/src/popup/viewRouter.js b/src/popup/viewRouter.js index 46bda0d..aa2b8a1 100644 --- a/src/popup/viewRouter.js +++ b/src/popup/viewRouter.js @@ -145,12 +145,15 @@ function isRenderablePendingTx(value) { // The stored transaction transactionDetail.render() shows. contractAddress is // optional on an ETH transfer, and reaches addressDotHtml() when it is there. +// `chainId` picks the network whose explorer the links open, so when it is +// there it has to name one. function isRenderableTx(value) { return ( isRecord(value) && isAddressText(value.from) && isAddressText(value.to) && - (!value.contractAddress || isAddressText(value.contractAddress)) + (!value.contractAddress || isAddressText(value.contractAddress)) && + (!value.chainId || SUPPORTED_CHAIN_IDS.has(value.chainId)) ); } diff --git a/src/popup/views/confirmTx.js b/src/popup/views/confirmTx.js index ef2d9d3..42a4ca8 100644 --- a/src/popup/views/confirmTx.js +++ b/src/popup/views/confirmTx.js @@ -23,12 +23,10 @@ const { goBack, onViewLeave, formatFee, + reviewedNetworkProvider, } = require("./helpers"); const { state, currentNetwork } = require("../../shared/state"); -const { - networkByChainId, - nativeCurrencyByChainId, -} = require("../../shared/networks"); +const { nativeCurrencyByChainId } = require("../../shared/networks"); const { getSignerForAddress } = require("../../shared/wallet"); const { walletDefect } = require("../../shared/walletDefects"); const { decryptWithPassword } = require("../../shared/vault"); @@ -458,20 +456,6 @@ function setVisible(id, visible) { $(id).style.visibility = visible ? "visible" : "hidden"; } -// The node of the network this transaction was reviewed for, which need not be -// the active one: the popup can reopen onto this screen after Settings switched -// the network. Its RPC URL is the one Settings has for that network: the -// active network's is state.rpcUrl, any other's the one remembered for it (see -// applyChainSwitchFields()). -function reviewedNetworkProvider(txInfo) { - const net = networkByChainId(txInfo.chainId); - if (net.id === state.networkId) { - return getProvider(state.rpcUrl, net.id); - } - const remembered = state.networkEndpoints[net.id] || {}; - return getProvider(remembered.rpcUrl || net.defaultRpcUrl, net.id); -} - async function estimateGas(txInfo) { try { const provider = reviewedNetworkProvider(txInfo); diff --git a/src/popup/views/helpers.js b/src/popup/views/helpers.js index f2585c2..9912b7b 100644 --- a/src/popup/views/helpers.js +++ b/src/popup/views/helpers.js @@ -19,6 +19,7 @@ const { isBelowOneMillionth, } = require("../../shared/amountDisplay"); const { resolveTokenSymbol } = require("../../shared/approvalAmount"); +const { getProvider } = require("../../shared/balances"); const { DEBUG } = require("../../shared/constants"); const { escapeHtml } = require("../../shared/html"); const { isDebug } = require("../../shared/log"); @@ -607,6 +608,20 @@ const EXT_ICON = `` + ``; +// The node of the network a transaction was reviewed for, the one its chain id +// names, which need not be the active one: the popup can reopen onto ConfirmTx +// or WaitTx after Settings switched the network. Its RPC URL is the one +// Settings has for that network: the active network's is state.rpcUrl, any +// other's the one remembered for it (see applyChainSwitchFields()). +function reviewedNetworkProvider(txInfo) { + const net = networkByChainId(txInfo.chainId); + if (net.id === state.networkId) { + return getProvider(state.rpcUrl, net.id); + } + const remembered = state.networkEndpoints[net.id] || {}; + return getProvider(remembered.rpcUrl || net.defaultRpcUrl, net.id); +} + // Block-explorer URLs. The origin is a per-network constant from // src/shared/networks.js, the explorer of the network with `chainId`, or of // the active network when no chain id is given; only the path segment is @@ -762,6 +777,7 @@ module.exports = { attachCopyHandlers, etherscanLinkHtml, explorerUrl, + reviewedNetworkProvider, EXT_ICON, truncateMiddle, isoDate, diff --git a/src/popup/views/transactionDetail.js b/src/popup/views/transactionDetail.js index c57a33b..15bf77d 100644 --- a/src/popup/views/transactionDetail.js +++ b/src/popup/views/transactionDetail.js @@ -48,16 +48,16 @@ function getTransactionType(tx) { return "Native " + nativeCurrencyByChainId(tx.chainId) + " Transfer"; } -function txAddressHtml(address, ensName, title) { +function txAddressHtml(address, ensName, title, chainId) { const blockie = blockieHtml(address); return ( `
${blockie}
` + - renderAddressHtml(address, { title, ensName }) + renderAddressHtml(address, { title, ensName, chainId }) ); } -function txHashHtml(hash) { - const link = explorerUrl("tx", hash); +function txHashHtml(hash, chainId) { + const link = explorerUrl("tx", hash, chainId); const extLink = etherscanLinkHtml(link); return copyableHtml(hash, "break-all") + extLink; } @@ -83,10 +83,10 @@ function show(tx) { method: tx.method || null, contractAddress: tx.contractAddress || null, // The network the history entry was read from. The type line and - // the fee are in its native currency, not the active network's: - // the active network can change, in Settings or when the user - // approves a site's request, before a later popup shows this - // screen again. + // the fee are in its native currency, not the active network's, + // and the links open its explorer: the active network can change, + // in Settings or when the user approves a site's request, before + // a later popup shows this screen again. chainId: tx.chainId, }, }; @@ -96,18 +96,24 @@ function show(tx) { function render() { const tx = state.viewData.tx; if (!tx) return; - $("tx-detail-hash").innerHTML = txHashHtml(tx.hash); + $("tx-detail-hash").innerHTML = txHashHtml(tx.hash, tx.chainId); const fromTitle = addressTitle(tx.from, state.wallets); $("tx-detail-from").innerHTML = txAddressHtml( tx.from, tx.fromEns, fromTitle, + tx.chainId, ); // A contract creation has no recipient: transactions.js gives it `to: ""`. if (tx.to) { const toTitle = addressTitle(tx.to, state.wallets); - $("tx-detail-to").innerHTML = txAddressHtml(tx.to, tx.toEns, toTitle); + $("tx-detail-to").innerHTML = txAddressHtml( + tx.to, + tx.toEns, + toTitle, + tx.chainId, + ); } else { $("tx-detail-to").innerHTML = escapeHtml(CONTRACT_CREATION_TEXT); } @@ -146,7 +152,7 @@ function render() { if (tokenContractSection && tokenContractEl) { if (tx.contractAddress) { const dot = addressDotHtml(tx.contractAddress); - const link = explorerUrl("token", tx.contractAddress); + const link = explorerUrl("token", tx.contractAddress, tx.chainId); // Hand-rolled rather than renderAddressHtml() because the // link goes to the explorer's /token/ page, not /address/. // Same two-row shape though: dot and link on the strip, the @@ -204,7 +210,11 @@ function showDetailField(sectionId, contentId, value) { function populateOnChainDetails(txData, chainId) { // Block number if (txData.block_number != null) { - const blockLink = explorerUrl("block", String(txData.block_number)); + const blockLink = explorerUrl( + "block", + String(txData.block_number), + chainId, + ); const blockSection = $("tx-detail-block-section"); const blockEl = $("tx-detail-block"); if (blockSection && blockEl) { @@ -340,9 +350,9 @@ async function loadFullTxDetails(txHash, chainId) { if (tokenSymbol) { detailsHtml += `
${escapeHtml(displaySymbol(tokenSymbol))}
`; } - detailsHtml += renderAddressHtml(d.address); + detailsHtml += renderAddressHtml(d.address, { chainId }); } else if (d.address) { - detailsHtml += renderAddressHtml(d.address); + detailsHtml += renderAddressHtml(d.address, { chainId }); } else { detailsHtml += `
${escapeHtml(d.value)}
`; } diff --git a/src/popup/views/txStatus.js b/src/popup/views/txStatus.js index bd31eee..005c6c7 100644 --- a/src/popup/views/txStatus.js +++ b/src/popup/views/txStatus.js @@ -14,10 +14,13 @@ const { displaySymbol, clearViewStack, tokenLabel, + reviewedNetworkProvider, } = require("./helpers"); const { state } = require("../../shared/state"); -const { nativeCurrencyByChainId } = require("../../shared/networks"); -const { getProvider } = require("../../shared/balances"); +const { + SUPPORTED_CHAIN_IDS, + nativeCurrencyByChainId, +} = require("../../shared/networks"); const { log } = require("../../shared/log"); // Receipt poll cadence and the deadline after which the wait is reported as @@ -61,15 +64,16 @@ function endWait() { } // A contract creation reaches these screens with `to` as "" (approval.js -// writes `to: toAddr || ""`). -function toAddressHtml(address) { +// writes `to: toAddr || ""`). The link opens 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); - return renderAddressHtml(address, { title }); + return renderAddressHtml(address, { title, chainId }); } -function txHashHtml(hash) { - const link = explorerUrl("tx", hash); +function txHashHtml(hash, chainId) { + const link = explorerUrl("tx", hash, chainId); return copyableHtml(hash, "break-all") + etherscanLinkHtml(link); } @@ -91,9 +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. 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. 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. let symbol = ""; if (txInfo.token === "ETH") { symbol = nativeCurrencyByChainId(txInfo.chainId); @@ -102,8 +108,8 @@ function startWait(txInfo, txHash, broadcastTime, pollNow) { } $("wait-tx-summary").textContent = txInfo.amount + (symbol ? " " + symbol : ""); - $("wait-tx-to").innerHTML = toAddressHtml(txInfo.to); - $("wait-tx-hash").innerHTML = txHashHtml(txHash); + $("wait-tx-to").innerHTML = toAddressHtml(txInfo.to, txInfo.chainId); + $("wait-tx-hash").innerHTML = txHashHtml(txHash, txInfo.chainId); attachCopyHandlers("view-wait-tx"); // Persisted so closing and reopening the popup resumes this wait @@ -128,7 +134,7 @@ function startWait(txInfo, txHash, broadcastTime, pollNow) { renderElapsed(); }, 1000); - const provider = getProvider(state.rpcUrl, state.networkId); + const provider = reviewedNetworkProvider(txInfo); let consecutiveFailures = 0; async function poll() { @@ -207,10 +213,11 @@ function showWait(txInfo, txHash) { // an object merely missing one of them throws a TypeError out of // restoreView() — which init() does not guard, skipping the rest of popup // init and leaving wait-tx on screen with no back control. A non-numeric -// broadcastTime leaves an unexitable wait counting "NaNs". txInfo.token, -// txInfo.tokenSymbol and txInfo.chainId are deliberately unchecked: they are -// compared and coalesced rather than dereferenced, and tokenSymbol is null for -// ETH. +// broadcastTime leaves an unexitable wait counting "NaNs". txInfo.chainId picks +// the node the receipt is asked of and the explorer the links open, so it has +// to name a network. txInfo.token and txInfo.tokenSymbol are deliberately +// unchecked: they are compared and coalesced rather than dereferenced, and +// tokenSymbol is null for ETH. function restoreWait() { const d = state.viewData; if (!d || !d.pendingWait) return false; @@ -226,6 +233,7 @@ function restoreWait() { // path itself created. if (typeof info.to !== "string") return false; if (typeof info.amount !== "string") return false; + if (!SUPPORTED_CHAIN_IDS.has(info.chainId)) return false; if (typeof w.broadcastTime !== "number" || !isFinite(w.broadcastTime)) { return false; } diff --git a/tests/contractCreation.test.js b/tests/contractCreation.test.js index ab4e5da..3d96d6c 100644 --- a/tests/contractCreation.test.js +++ b/tests/contractCreation.test.js @@ -192,6 +192,7 @@ describe("the wait, success and error screens", () => { amount: "0.0000", token: "ETH", tokenSymbol: null, + chainId: "0x1", }; const transfer = { ...creation, to: RECIPIENT }; diff --git a/tests/persistedFieldContract.test.js b/tests/persistedFieldContract.test.js index cabeaca..995a57b 100644 --- a/tests/persistedFieldContract.test.js +++ b/tests/persistedFieldContract.test.js @@ -459,6 +459,19 @@ const CONTRACT = [ views: ["transaction"], restored: false, }, + // A chain id naming no network has no explorer to link to. + { + value: { + tx: { + hash: "0x1", + from: ADDRESS, + to: ADDRESS, + chainId: "0x5", + }, + }, + views: ["transaction"], + restored: false, + }, // confirm-tx passes on `data.pendingTx`. { value: { pendingTx: { amount: "1" } }, @@ -821,7 +834,7 @@ const WELL_FORMED_DATA = { }, pendingWait: { hash: "0x1", - txInfo: { to: ADDRESS, amount: "1" }, + txInfo: { to: ADDRESS, amount: "1", chainId: "0x1" }, broadcastTime: BROADCAST_TIME, }, }; diff --git a/tests/stateRecovery.test.js b/tests/stateRecovery.test.js index 567fd94..e638219 100644 --- a/tests/stateRecovery.test.js +++ b/tests/stateRecovery.test.js @@ -225,7 +225,12 @@ describe("a popup already open when the stored profile becomes unreadable", () = viewData: { pendingWait: { hash: "0x1", - txInfo: { to: ADDRESS, amount: "1", token: "ETH" }, + txInfo: { + to: ADDRESS, + amount: "1", + token: "ETH", + chainId: "0x1", + }, broadcastTime: Date.now(), }, }, diff --git a/tests/tokenTransferDetail.test.js b/tests/tokenTransferDetail.test.js index 8a7c829..56979db 100644 --- a/tests/tokenTransferDetail.test.js +++ b/tests/tokenTransferDetail.test.js @@ -8,6 +8,10 @@ // tracks. It decodes against the address the explorer reports the transaction // was sent to. // +// Its links open the explorer of the network the transaction was sent on, +// whichever network is active +// (https://git.eeqj.de/sneak/AutistMask/issues/584). +// // The row is the one the real history parser makes from the explorer's answer, // opened through transactionDetail.show(), which is what a click on the row // calls. Driven against a minimal DOM stub in the shape @@ -35,6 +39,7 @@ globalThis.chrome = { }; const { state } = require("../src/shared/state"); +const { NETWORKS } = require("../src/shared/networks"); const { fetchRecentTransactions } = require("../src/shared/transactions"); const transactionDetail = require("../src/popup/views/transactionDetail"); @@ -77,7 +82,11 @@ function mockExplorerAnswer(url) { }; } if (url === EXPLORER + "/transactions/" + TX_HASH) { - return { to: { hash: TOKEN }, raw_input: TRANSFER_DATA }; + return { + to: { hash: TOKEN }, + raw_input: TRANSFER_DATA, + block_number: 100, + }; } return { items: [] }; } @@ -154,7 +163,11 @@ beforeEach(() => { }); test("a token transfer's decoded details name the token contract and the amount at its scale", async () => { - const [row] = await fetchRecentTransactions(HOLDER, EXPLORER, 1); + const [row] = await fetchRecentTransactions( + HOLDER, + EXPLORER, + NETWORKS.mainnet.chainId, + ); expect(row.to).toBe(RECIPIENT); expect(row.contractAddress).toBe(TOKEN.toLowerCase()); @@ -174,3 +187,33 @@ test("a token transfer's decoded details name the token contract and the amount '
1234.5000 TDOL
', ); }); + +// Mainnet is active and the row was read from Sepolia's explorer. The stub +// explorer answers whichever network asks, so every link on the screen is +// drawn. +test("with mainnet active, a Sepolia transaction's links open the Sepolia explorer", async () => { + state.networkId = "mainnet"; + const [row] = await fetchRecentTransactions( + HOLDER, + EXPLORER, + NETWORKS.sepolia.chainId, + ); + + transactionDetail.show(row); + for (let i = 0; i < 10; i++) await new Promise((r) => setTimeout(r, 0)); + + const sepolia = "https://sepolia.etherscan.io"; + const links = (id) => + [...node(id).innerHTML.matchAll(/href="([^"]*)"/g)].map((m) => m[1]); + expect(links("tx-detail-hash")).toEqual([`${sepolia}/tx/${TX_HASH}`]); + expect(links("tx-detail-from")).toEqual([`${sepolia}/address/${HOLDER}`]); + expect(links("tx-detail-to")).toEqual([`${sepolia}/address/${RECIPIENT}`]); + expect(links("tx-detail-token-contract")).toEqual([ + `${sepolia}/token/${TOKEN.toLowerCase()}`, + ]); + expect(links("tx-detail-block")).toEqual([`${sepolia}/block/100`]); + expect(links("tx-detail-calldata-details")).toEqual([ + `${sepolia}/address/${TOKEN}`, + `${sepolia}/address/${RECIPIENT}`, + ]); +}); diff --git a/tests/txStatus.test.js b/tests/txStatus.test.js index 8da3780..8880c7c 100644 --- a/tests/txStatus.test.js +++ b/tests/txStatus.test.js @@ -29,8 +29,13 @@ jest.mock("../src/shared/log", () => ({ })); const mockReceiptLookup = jest.fn(); +// The RPC URL and network of every node a wait was given. +const mockNodes = []; jest.mock("../src/shared/balances", () => ({ - getProvider: () => ({ getTransactionReceipt: mockReceiptLookup }), + getProvider: (rpcUrl, networkId) => { + mockNodes.push({ rpcUrl, networkId }); + return { getTransactionReceipt: mockReceiptLookup }; + }, refreshBalances: jest.fn(), })); @@ -107,6 +112,7 @@ global.chrome = { storage }; const txStatus = require("../src/popup/views/txStatus"); const { state } = require("../src/shared/state"); +const { NETWORKS } = require("../src/shared/networks"); const { RESTORABLE_VIEWS } = require("../src/shared/restorableViews"); const { log } = require("../src/shared/log"); @@ -118,6 +124,7 @@ const TX_INFO = { amount: "0.0050", token: "ETH", tokenSymbol: null, + chainId: NETWORKS.mainnet.chainId, }; // True when a view element is not hidden. @@ -137,7 +144,11 @@ beforeEach(() => { jest.setSystemTime(new Date("2026-08-11T12:00:00Z")); elements.clear(); mockReceiptLookup.mockReset(); + mockNodes.length = 0; log.errorf.mockClear(); + state.networkId = "mainnet"; + state.rpcUrl = NETWORKS.mainnet.defaultRpcUrl; + state.networkEndpoints = {}; state.wallets = []; state.viewData = {}; state.viewStack = []; @@ -349,14 +360,14 @@ describe("WaitTx persistence across popup close", () => { // out of restoreView(). { hash: TX_HASH, - txInfo: { to: 42, amount: "0.0050" }, + txInfo: { ...TX_INFO, to: 42 }, broadcastTime: Date.now(), }, // Otherwise complete but an array: only Array.isArray() rejects // it, since typeof [] is "object" and the fields are present. { hash: TX_HASH, - txInfo: Object.assign([], { to: RECIPIENT, amount: "0.0050" }), + txInfo: Object.assign([], TX_INFO), broadcastTime: Date.now(), }, { @@ -364,6 +375,18 @@ describe("WaitTx persistence across popup close", () => { txInfo: { to: RECIPIENT }, broadcastTime: Date.now(), }, + // Otherwise complete but for a chain id naming no network: there + // is no node to ask for the receipt, and no explorer to link to. + { + hash: TX_HASH, + txInfo: { ...TX_INFO, chainId: undefined }, + broadcastTime: Date.now(), + }, + { + hash: TX_HASH, + txInfo: { ...TX_INFO, chainId: "0x5" }, + broadcastTime: Date.now(), + }, ]) { state.viewData = { pendingWait: bad }; expect(txStatus.restoreWait()).toBe(false); @@ -499,6 +522,50 @@ describe("WaitTx when the popup closes", () => { }); }); +// The popup can reopen onto WaitTx after Settings switched the network. The +// wait asked the active network's node for the receipt, never found it, and +// reported a timeout, and its links went to the active network's explorer +// (https://git.eeqj.de/sneak/AutistMask/issues/584). +describe("WaitTx for a Sepolia transaction with mainnet active", () => { + const SEPOLIA_NODE = "https://sepolia.node.example.invalid"; + const sepoliaTx = { ...TX_INFO, chainId: NETWORKS.sepolia.chainId }; + + beforeEach(() => { + state.rpcUrl = "https://mainnet.node.example.invalid"; + state.networkEndpoints = { sepolia: { rpcUrl: SEPOLIA_NODE } }; + }); + + test("the receipt is asked of the Sepolia node Settings has", async () => { + mockReceiptLookup.mockResolvedValue({ blockNumber: 21000000 }); + state.viewData = { + pendingWait: { + txInfo: sepoliaTx, + hash: TX_HASH, + broadcastTime: Date.now(), + }, + }; + expect(txStatus.restoreWait()).toBe(true); + await jest.advanceTimersByTimeAsync(0); + + expect(mockNodes).toEqual([ + { rpcUrl: SEPOLIA_NODE, networkId: "sepolia" }, + ]); + expect(state.currentView).toBe("success-tx"); + }); + + test("the recipient and hash link to the Sepolia explorer", () => { + mockReceiptLookup.mockResolvedValue(null); + txStatus.showWait(sepoliaTx, TX_HASH); + + expect(getElement("wait-tx-to").innerHTML).toContain( + `href="https://sepolia.etherscan.io/address/${RECIPIENT}"`, + ); + expect(getElement("wait-tx-hash").innerHTML).toContain( + `href="https://sepolia.etherscan.io/tx/${TX_HASH}"`, + ); + }); +}); + 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