diff --git a/README.md b/README.md index 6bb4aa9..e4a7310 100644 --- a/README.md +++ b/README.md @@ -264,6 +264,8 @@ The shared modules, each in one line: messaging, windows and the lock. - `buildInfo.js`: the version, license, author, commit and date compiled into About. +- `calldataDecoder.js`: the ERC-20 `approve()` and `transfer()` decoder, which + hands other calldata to `uniswap.js`. Popup only. - `chainSwitch.js`: the popup's network switch. - `chainSwitchFields.js`: the fields a network switch changes. - `constants.js`: `DEBUG`, the default endpoints, the ERC-20 ABI and the burn @@ -1379,7 +1381,7 @@ approval screens show a red warning. ### Transaction decoding TxApproval and TransactionDetail try to decode calldata into a summary -(`decodeCalldata()` in `src/popup/views/approval.js`). SuccessTx repeats +(`decodeCalldata()` in `src/shared/calldataDecoder.js`). SuccessTx repeats TxApproval's summary. Decoding is a display aid. When it fails, the raw calldata is shown in full. No contract gets any treatment beyond a legible confirmation screen. diff --git a/src/popup/views/approval.js b/src/popup/views/approval.js index 0907895..61b377b 100644 --- a/src/popup/views/approval.js +++ b/src/popup/views/approval.js @@ -12,9 +12,13 @@ const { formatFee, tokenLabel, blockscoutOffTokenNoteHtml, - tokenSourcesByChainId, } = require("./helpers"); -const { state, saveState, currentNetwork } = require("../../shared/state"); +const { + state, + saveState, + currentNetwork, + tokenSourcesByChainId, +} = require("../../shared/state"); const { networkById, networkByChainId, @@ -26,21 +30,24 @@ const { getAddress, getBigInt, getBytes, - Interface, MaxUint256, toUtf8String, } = require("ethers"); const { getPrice, formatUsd } = require("../../shared/prices"); -const { ERC20_ABI } = require("../../shared/constants"); const { INVISIBLE_CHARACTERS } = require("../../shared/symbolSpoof"); const { resolveTokenDecimals, resolveTokenSymbol, unknownDecimalsAmount, } = require("../../shared/approvalAmount"); +const { + decodeCalldata, + tokenAmountText, +} = require("../../shared/calldataDecoder"); // Four decimals, with the nonzero floor these screens hold: every amount this // view renders — the ERC-20 line, the ETH value, the max fee — and every one -// it carries forward to the wait/success/error screens goes through it. +// it carries forward to the wait/success/error screens goes through it. The +// ERC-20 line gets it from tokenAmountText(). const { truncateAmountNeverZero: formatTxValue, } = require("../../shared/amountDisplay"); @@ -54,11 +61,8 @@ const { typedDataChainRefusal, } = require("../../shared/approvalVerify"); const txStatus = require("./txStatus"); -const uniswap = require("../../shared/uniswap"); const { runtimeApi, sendMessage } = require("../../shared/browserApi"); -const erc20Iface = new Interface(ERC20_ABI); - // The link opens the explorer of the network with `chainId`, or of the active // network when none is given. A chain id that names no network has no // explorer, so the address is drawn without a link. @@ -70,141 +74,6 @@ function approvalAddressHtml(address, chainId) { return renderAddressHtml(address, { title, chainId }); } -// The amount line for a decoded ERC-20 call. With a known scale it is the -// token quantity; with `decimals` null it is the base-unit integer with the -// unknown scale stated, because formatting it with an assumed scale is what -// showed a 5,000-token transfer as `0.0000`. `raw` is what the status screens -// carry, `display` is what the approval screen shows. -function tokenAmountText(rawAmount, decimals, symbol) { - if (decimals === null) { - const unknown = unknownDecimalsAmount(rawAmount); - return { raw: unknown, display: unknown }; - } - const formatted = formatTxValue(formatUnits(rawAmount, decimals)); - return { - raw: formatted, - display: formatted + (symbol ? " " + symbol : ""), - }; -} - -// Try to decode calldata sent from `fromAddress` to `toAddress` on the network -// with chain id `chainId`, using known ABIs. -// Returns { name, description, details } or null. -function decodeCalldata(data, toAddress, chainId, fromAddress) { - if (!data || data === "0x" || data.length < 10) return null; - - // Where a token's name and scale are looked for, for every decoder below: - // what the wallet knows of the tokens on the network with `chainId`, which - // need not be the active one. The ERC-20 amount line and the swap's Amount - // and Min. received lines resolve the scale the same way, and refuse to - // format the same way when it is nowhere. - const decimalsSources = tokenSourcesByChainId(chainId); - - // Try ERC-20 (approve / transfer) - try { - const parsed = erc20Iface.parseTransaction({ data }); - if (parsed) { - const tokenSymbol = resolveTokenSymbol(toAddress, decimalsSources); - // null when no source knows this token's scale. It is not - // defaulted to 18: an amount formatted with a guessed scale is - // the wrong number, and for a token with fewer decimals than the - // guess it is the wrong number in the direction that reads as - // zero. See tokenAmountText(). - const tokenDecimals = resolveTokenDecimals( - toAddress, - decimalsSources, - ); - const contractLabel = tokenSymbol - ? tokenSymbol + " (" + toAddress + ")" - : toAddress; - - if (parsed.name === "approve") { - const spender = parsed.args[0]; - const rawAmount = parsed.args[1]; - const maxUint = BigInt( - "0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", - ); - const isUnlimited = rawAmount === maxUint; - // An unbounded allowance needs no scale to describe, so it is - // still named rather than refused. - const amount = isUnlimited - ? { raw: "Unlimited", display: "Unlimited" } - : tokenAmountText(rawAmount, tokenDecimals, tokenSymbol); - - return { - name: "Token Approval", - description: tokenSymbol - ? "Approve spending of your " + tokenSymbol - : "Approve spending of an ERC-20 token", - details: [ - { - label: "Token", - value: contractLabel, - address: toAddress, - isToken: true, - }, - { - label: "Spender", - value: spender, - address: spender, - }, - { - label: "Amount", - value: amount.display, - rawValue: amount.raw, - }, - ], - }; - } - - if (parsed.name === "transfer") { - const to = parsed.args[0]; - const rawAmount = parsed.args[1]; - const amount = tokenAmountText( - rawAmount, - tokenDecimals, - tokenSymbol, - ); - - return { - name: "Token Transfer", - description: tokenSymbol - ? "Transfer " + tokenSymbol - : "Transfer ERC-20 token", - details: [ - { - label: "Token", - value: contractLabel, - address: toAddress, - isToken: true, - }, - { label: "Recipient", value: to, address: to }, - { - label: "Amount", - value: amount.display, - rawValue: amount.raw, - }, - ], - }; - } - } - } catch { - // Not ERC-20 — fall through - } - - // Try Uniswap Universal Router - const routerResult = uniswap.decode( - data, - toAddress, - chainId, - decimalsSources, - fromAddress, - ); - if (routerResult) return routerResult; - - return null; -} - function showPhishingWarning(elementId, isPhishing) { const el = $(elementId); if (!el) return; @@ -1298,7 +1167,6 @@ function init(_ctx) { module.exports = { init, show, - decodeCalldata, formatTypedDataHtml, typedDataRefusal, }; diff --git a/src/popup/views/helpers.js b/src/popup/views/helpers.js index ceb40db..af027c0 100644 --- a/src/popup/views/helpers.js +++ b/src/popup/views/helpers.js @@ -31,7 +31,12 @@ const { escapeHtml } = require("../../shared/html"); const { isDebug } = require("../../shared/log"); const { networkByChainId } = require("../../shared/networks"); const { formatUsd, getPrice } = require("../../shared/prices"); -const { state, saveState, currentNetwork } = require("../../shared/state"); +const { + state, + saveState, + currentNetwork, + tokenSourcesByChainId, +} = require("../../shared/state"); const { displaySymbol } = require("../../shared/symbolDisplay"); const { isSpoofedSymbol } = require("../../shared/symbolSpoof"); const { walletHasRecoveryPhrase } = require("../../shared/wallet"); @@ -305,32 +310,6 @@ function nativeCurrency() { return currentNetwork().nativeCurrency; } -// What the wallet knows of the tokens on the network with this chain id, or -// on the active network when no chain id is given, as the `sources` that -// resolveTokenSymbol() and resolveTokenDecimals() take. A contract address -// names a token on one network only, and every token balance held is the -// active network's, so another network gets the tokens tracked on it -// (state.networkTrackedTokens, see applyChainSwitchFields()) and no balances. -// -// The wallets' balances hold what Blockscout reported for each token. So the -// wallets are returned only while "Use Blockscout" is on. -function tokenSourcesByChainId(chainId) { - const chainIdOrActive = chainId || currentNetwork().chainId; - const net = networkByChainId(chainIdOrActive); - if (net && net.id === state.networkId) { - return { - chainId: chainIdOrActive, - trackedTokens: state.trackedTokens, - wallets: state.useBlockscout ? state.wallets : [], - }; - } - return { - chainId: chainIdOrActive, - trackedTokens: (net && state.networkTrackedTokens[net.id]) || [], - wallets: [], - }; -} - // The line shown under a token while "Use Blockscout" is off. The token gets // the line only when the bundled list and the tracked tokens lack its name or // decimals. Otherwise this returns "". The token is a contract address on the @@ -866,7 +845,6 @@ module.exports = { keyMaterialName, unknownableAmount, nativeCurrency, - tokenSourcesByChainId, blockscoutOffTokenNoteHtml, tokenLabel, formatFee, diff --git a/src/popup/views/send.js b/src/popup/views/send.js index 8895370..1f28f73 100644 --- a/src/popup/views/send.js +++ b/src/popup/views/send.js @@ -10,9 +10,13 @@ const { renderAddressHtml, attachCopyHandlers, goBack, - tokenSourcesByChainId, } = require("./helpers"); -const { state, currentAddress, currentNetwork } = require("../../shared/state"); +const { + state, + currentAddress, + currentNetwork, + tokenSourcesByChainId, +} = require("../../shared/state"); let ctx; const { getProvider } = require("../../shared/balances"); const { resolveTokenDecimals } = require("../../shared/approvalAmount"); diff --git a/src/popup/views/transactionDetail.js b/src/popup/views/transactionDetail.js index 0d2d1c1..5052dab 100644 --- a/src/popup/views/transactionDetail.js +++ b/src/popup/views/transactionDetail.js @@ -27,7 +27,7 @@ const { nativeCurrencyByChainId } = require("../../shared/networks"); const { formatEther, formatUnits } = require("ethers"); const { log, debugFetch } = require("../../shared/log"); const { storedSettingIsOn } = require("../../shared/storedSettings"); -const { decodeCalldata } = require("./approval"); +const { decodeCalldata } = require("../../shared/calldataDecoder"); /** * Determine a human-readable transaction type string from tx fields. diff --git a/src/shared/amountDisplay.js b/src/shared/amountDisplay.js index 5232574..262409a 100644 --- a/src/shared/amountDisplay.js +++ b/src/shared/amountDisplay.js @@ -2,7 +2,7 @@ // the one exception to it, in one place. Three call sites had grown their own // copy of the truncation — the history and balance lists // (`src/shared/transactions.js`), the approval screen's ERC-20 amount line -// (`src/popup/views/approval.js`) and its Uniswap swap detail lines +// (`src/shared/calldataDecoder.js`) and its Uniswap swap detail lines // (`src/shared/uniswap.js`) — and a fix applied to one of them left the other // two showing a different number for the same value. // diff --git a/src/shared/calldataDecoder.js b/src/shared/calldataDecoder.js new file mode 100644 index 0000000..025c9cb --- /dev/null +++ b/src/shared/calldataDecoder.js @@ -0,0 +1,155 @@ +// Decode a transaction's calldata into the summary that TxApproval and +// TransactionDetail show: ERC-20 `approve` and `transfer` calls here, and +// Uniswap Universal Router calls through uniswap.js. The summary is data; the +// views turn its addresses into HTML. + +const { formatUnits, Interface } = require("ethers"); +const { ERC20_ABI } = require("./constants"); +const { tokenSourcesByChainId } = require("./state"); +const { + resolveTokenDecimals, + resolveTokenSymbol, + unknownDecimalsAmount, +} = require("./approvalAmount"); +// Four decimals, with the nonzero floor the approval screens hold. +const { truncateAmountNeverZero } = require("./amountDisplay"); +const uniswap = require("./uniswap"); + +const erc20Iface = new Interface(ERC20_ABI); + +// The amount line for a decoded ERC-20 call. With a known scale it is the +// token quantity; with `decimals` null it is the base-unit integer with the +// unknown scale stated, because formatting it with an assumed scale is what +// showed a 5,000-token transfer as `0.0000`. `raw` is what the status screens +// carry, `display` is what the approval screen shows. +function tokenAmountText(rawAmount, decimals, symbol) { + if (decimals === null) { + const unknown = unknownDecimalsAmount(rawAmount); + return { raw: unknown, display: unknown }; + } + const formatted = truncateAmountNeverZero(formatUnits(rawAmount, decimals)); + return { + raw: formatted, + display: formatted + (symbol ? " " + symbol : ""), + }; +} + +// Try to decode calldata sent from `fromAddress` to `toAddress` on the network +// with chain id `chainId`, using known ABIs. +// Returns { name, description, details } or null. +function decodeCalldata(data, toAddress, chainId, fromAddress) { + if (!data || data === "0x" || data.length < 10) return null; + + // Where a token's name and scale are looked for, for every decoder below: + // what the wallet knows of the tokens on the network with `chainId`, which + // need not be the active one. The ERC-20 amount line and the swap's Amount + // and Min. received lines resolve the scale the same way, and refuse to + // format the same way when it is nowhere. + const decimalsSources = tokenSourcesByChainId(chainId); + + // Try ERC-20 (approve / transfer) + try { + const parsed = erc20Iface.parseTransaction({ data }); + if (parsed) { + const tokenSymbol = resolveTokenSymbol(toAddress, decimalsSources); + // null when no source knows this token's scale. It is not + // defaulted to 18: an amount formatted with a guessed scale is + // the wrong number, and for a token with fewer decimals than the + // guess it is the wrong number in the direction that reads as + // zero. See tokenAmountText(). + const tokenDecimals = resolveTokenDecimals( + toAddress, + decimalsSources, + ); + const contractLabel = tokenSymbol + ? tokenSymbol + " (" + toAddress + ")" + : toAddress; + + if (parsed.name === "approve") { + const spender = parsed.args[0]; + const rawAmount = parsed.args[1]; + const maxUint = BigInt( + "0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", + ); + const isUnlimited = rawAmount === maxUint; + // An unbounded allowance needs no scale to describe, so it is + // still named rather than refused. + const amount = isUnlimited + ? { raw: "Unlimited", display: "Unlimited" } + : tokenAmountText(rawAmount, tokenDecimals, tokenSymbol); + + return { + name: "Token Approval", + description: tokenSymbol + ? "Approve spending of your " + tokenSymbol + : "Approve spending of an ERC-20 token", + details: [ + { + label: "Token", + value: contractLabel, + address: toAddress, + isToken: true, + }, + { + label: "Spender", + value: spender, + address: spender, + }, + { + label: "Amount", + value: amount.display, + rawValue: amount.raw, + }, + ], + }; + } + + if (parsed.name === "transfer") { + const to = parsed.args[0]; + const rawAmount = parsed.args[1]; + const amount = tokenAmountText( + rawAmount, + tokenDecimals, + tokenSymbol, + ); + + return { + name: "Token Transfer", + description: tokenSymbol + ? "Transfer " + tokenSymbol + : "Transfer ERC-20 token", + details: [ + { + label: "Token", + value: contractLabel, + address: toAddress, + isToken: true, + }, + { label: "Recipient", value: to, address: to }, + { + label: "Amount", + value: amount.display, + rawValue: amount.raw, + }, + ], + }; + } + } + } catch { + // Not ERC-20 — fall through + } + + // Try Uniswap Universal Router + const routerResult = uniswap.decode( + data, + toAddress, + chainId, + decimalsSources, + fromAddress, + ); + if (routerResult) return routerResult; + + return null; +} + +module.exports = { decodeCalldata, tokenAmountText }; diff --git a/src/shared/state.js b/src/shared/state.js index dbe0903..9ed2a61 100644 --- a/src/shared/state.js +++ b/src/shared/state.js @@ -22,7 +22,7 @@ // same thing in the editor is fast feedback in front of the build, not the // guarantee. -const { networkById } = require("./networks"); +const { networkById, networkByChainId } = require("./networks"); const { copyChainSwitchFields } = require("./chainSwitchFields"); const { DEFAULT_STATE, @@ -125,6 +125,32 @@ function currentNetwork() { return networkById(state.networkId); } +// What the wallet knows of the tokens on the network with this chain id, or +// on the active network when no chain id is given, as the `sources` that +// resolveTokenSymbol() and resolveTokenDecimals() take. A contract address +// names a token on one network only, and every token balance held is the +// active network's, so another network gets the tokens tracked on it +// (state.networkTrackedTokens, see applyChainSwitchFields()) and no balances. +// +// The wallets' balances hold what Blockscout reported for each token. So the +// wallets are returned only while "Use Blockscout" is on. +function tokenSourcesByChainId(chainId) { + const chainIdOrActive = chainId || currentNetwork().chainId; + const net = networkByChainId(chainIdOrActive); + if (net && net.id === state.networkId) { + return { + chainId: chainIdOrActive, + trackedTokens: state.trackedTokens, + wallets: state.useBlockscout ? state.wallets : [], + }; + } + return { + chainId: chainIdOrActive, + trackedTokens: (net && state.networkTrackedTokens[net.id]) || [], + wallets: [], + }; +} + // `baseline` holds the persisted fields as this page held them when its last // loadState() finished, or when its last successful saveState() began. // takeStoredNetwork(), takeStoredTrackedTokens() and @@ -820,5 +846,6 @@ module.exports = { loadState, currentAddress, currentNetwork, + tokenSourcesByChainId, StateNotLoadedError, }; diff --git a/tests/approvalAmount.test.js b/tests/approvalAmount.test.js index 229f00a..a6d66fd 100644 --- a/tests/approvalAmount.test.js +++ b/tests/approvalAmount.test.js @@ -23,7 +23,7 @@ const { resolveTokenDecimals, unknownDecimalsAmount, } = require("../src/shared/approvalAmount"); -const { decodeCalldata } = require("../src/popup/views/approval"); +const { decodeCalldata } = require("../src/shared/calldataDecoder"); const iface = new Interface(ERC20_ABI); diff --git a/tests/approvalDisplayFloor.test.js b/tests/approvalDisplayFloor.test.js index 0fb67f2..bb1b3c4 100644 --- a/tests/approvalDisplayFloor.test.js +++ b/tests/approvalDisplayFloor.test.js @@ -13,7 +13,7 @@ // asserted on `rawValue` alongside the displayed line. // // Both amount paths of that screen are covered: the ERC-20 line decoded by -// `src/popup/views/approval.js`, and the swap's `Amount` and `Min. received` +// `src/shared/calldataDecoder.js`, and the swap's `Amount` and `Min. received` // lines decoded by `src/shared/uniswap.js`. globalThis.chrome = { @@ -23,7 +23,7 @@ globalThis.chrome = { const { AbiCoder, Interface } = require("ethers"); const { ERC20_ABI } = require("../src/shared/constants"); const { state } = require("../src/shared/state"); -const { decodeCalldata } = require("../src/popup/views/approval"); +const { decodeCalldata } = require("../src/shared/calldataDecoder"); const uniswap = require("../src/shared/uniswap"); const { truncateAmount, diff --git a/tests/fabricatedDecimals.test.js b/tests/fabricatedDecimals.test.js index 7c2f8bb..616f498 100644 --- a/tests/fabricatedDecimals.test.js +++ b/tests/fabricatedDecimals.test.js @@ -47,7 +47,7 @@ const { fetchTokenBalances } = require("../src/shared/balances"); const { debugFetch } = require("../src/shared/log"); const { state } = require("../src/shared/state"); const { unknownDecimalsAmount } = require("../src/shared/approvalAmount"); -const { decodeCalldata } = require("../src/popup/views/approval"); +const { decodeCalldata } = require("../src/shared/calldataDecoder"); const { TOKEN_BY_ADDRESS } = require("../src/shared/tokenList"); const HOLDER = "0x" + "a".repeat(40); diff --git a/tests/resolveTokenSymbol.test.js b/tests/resolveTokenSymbol.test.js index ef97e49..b18410e 100644 --- a/tests/resolveTokenSymbol.test.js +++ b/tests/resolveTokenSymbol.test.js @@ -22,7 +22,7 @@ const { Interface } = require("ethers"); const { ERC20_ABI } = require("../src/shared/constants"); const { state } = require("../src/shared/state"); const { resolveTokenSymbol } = require("../src/shared/approvalAmount"); -const { decodeCalldata } = require("../src/popup/views/approval"); +const { decodeCalldata } = require("../src/shared/calldataDecoder"); const iface = new Interface(ERC20_ABI); diff --git a/tests/uniswapRouterAddress.test.js b/tests/uniswapRouterAddress.test.js index 9fbcf83..23a660f 100644 --- a/tests/uniswapRouterAddress.test.js +++ b/tests/uniswapRouterAddress.test.js @@ -47,7 +47,7 @@ globalThis.chrome = { const { state } = require("../src/shared/state"); const { fetchRecentTransactions } = require("../src/shared/transactions"); const approval = require("../src/popup/views/approval"); -const { decodeCalldata } = approval; +const { decodeCalldata } = require("../src/shared/calldataDecoder"); const transactionDetail = require("../src/popup/views/transactionDetail"); const MAINNET = "0x1"; diff --git a/tests/uniswapUnknownDecimals.test.js b/tests/uniswapUnknownDecimals.test.js index 892d867..b85b5ea 100644 --- a/tests/uniswapUnknownDecimals.test.js +++ b/tests/uniswapUnknownDecimals.test.js @@ -18,7 +18,7 @@ globalThis.chrome = { const { AbiCoder, Interface } = require("ethers"); const { state } = require("../src/shared/state"); const { unknownDecimalsAmount } = require("../src/shared/approvalAmount"); -const { decodeCalldata } = require("../src/popup/views/approval"); +const { decodeCalldata } = require("../src/shared/calldataDecoder"); const ROUTER = "0x66a9893cc07d91d95644aedd05d03f95e1dba8af"; const RECIPIENT = "0xC0FfEE0000000000000000000000000000c0fFEe";