Author SHA1 Message Date
sneak 01dce26807 fix: approval windows open as wide as the page (closes #518)
check / check (push) Waiting to run
e2e / e2e-chrome (push) Waiting to run
e2e / e2e-firefox (push) Waiting to run
The approval window was opened 360 pixels wide while the page is 396, so
it scrolled sideways and cut off the Reject button. It now opens at 396.
Both browsers count the window's frame in that width, so the approval
page also widens its own window by however far it would scroll sideways;
the toolbar popup's window is left alone. The Firefox suite checks that
the sign prompt does not scroll sideways. The Chrome suite cannot: its
approval windows open at 1280x720 whatever size is asked. README's Visual
Style line, which still said 360x600, now gives the page's real width.

Model: opus-5-5
2026-10-14 11:50:23 +00:00
clawbot 2442441830 fix: use the gas limit a site sends as gas (closes #207)
check / check (push) Waiting to run
e2e / e2e-chrome (push) Waiting to run
e2e / e2e-firefox (push) Waiting to run
Sites send the gas limit under its JSON-RPC name, `gas`. ethers only
reads `gasLimit`, so the field was dropped and the node's estimate was
shown and signed instead. The page's request now takes `gas` as the gas
limit, and the existing ceiling, display and comparison with the signed
transaction cover it as they already did `gasLimit`. A request that
gives both with different values is refused.

Model: opus-5-5
2026-10-14 13:48:52 +02:00
clawbot f1a13a50a6 fix: the defective xprv wallet notice names the keys that still reach its addresses (closes #255)
check / check (push) Waiting to run
e2e / e2e-chrome (push) Waiting to run
e2e / e2e-firefox (push) Waiting to run
The notice told the user that these addresses are reachable by software
that repeats the same derivation, while the same wallet blocks Export
Private Key, so there was no way to take that route from here. The
second paragraph now names private key export among the disabled
controls and says the addresses can be reached outside this wallet with
the extended private key the user imported, if they kept a copy of it,
or with the master extended private key or recovery phrase it came
from. No behaviour changes. A test checks the new wording.

Model: opus-5-5
2026-10-14 11:48:46 +02:00
16 changed files with 261 additions and 287 deletions
+22 -19
View File
@@ -464,6 +464,9 @@ papered over:
forced by clicking Reveal and the settings gear in one page task, which a forced by clicking Reveal and the settings gear in one page task, which a
person cannot do; the case a person can hit, the first decrypt after the popup person cannot do; the case a person can hit, the first decrypt after the popup
opens while libsodium is still loading, is not driven. opens while libsodium is still loading, is not driven.
- Approval windows open at 1280x720 whatever size the extension asks for, so
whether an approval screen fits its window is checked in the Firefox suite
only.
Any test that drives a failure path on purpose declares the `console.error` it Any test that drives a failure path on purpose declares the `console.error` it
is about to provoke, via `errors.expect()`. That is not a mute: the declaration is about to provoke, via `errors.expect()`. That is not a mute: the declaration
@@ -536,7 +539,8 @@ suite), wallet creation through the UI, the Add Token screen, and the dApp round
trips — `eth_requestAccounts`, `personal_sign`, `wallet_switchEthereumChain` trips — `eth_requestAccounts`, `personal_sign`, `wallet_switchEthereumChain`
rejected and then approved, `eth_sendTransaction`, and a closed approval window rejected and then approved, `eth_sendTransaction`, and a closed approval window
rejecting with EIP-1193 4001 — driven through the real content script, rejecting with EIP-1193 4001 — driven through the real content script,
background page and approval windows. background page and approval windows. The `personal_sign` prompt is also
required to fit its window without scrolling sideways.
The suite lives in `tests/e2e/firefox/`. Its WebDriver client (`driver.js`) has The suite lives in `tests/e2e/firefox/`. Its WebDriver client (`driver.js`) has
**no npm dependencies at all**: it is built on global `fetch` and **no npm dependencies at all**: it is built on global `fetch` and
@@ -893,8 +897,11 @@ discoverable.
enough that vanilla JS with simple view switching is sufficient. A framework enough that vanilla JS with simple view switching is sufficient. A framework
would add bundle size, build complexity, and attack surface for no benefit at would add bundle size, build complexity, and attack surface for no benefit at
this scale. this scale.
- **360x600 popup**: Standard browser extension popup dimensions. The UI is - **396x600 popup**: The page is 396 pixels wide (`body` in
designed for this fixed viewport. `src/popup/styles/main.css`), and 600 pixels is as tall as the browser lets
the toolbar popup grow. The UI is designed for this fixed viewport. The window
an approval opens in is given the same size; the browser counts the window's
frame in its width, so the page widens the window by whatever the frame takes.
#### No Layout Shift #### No Layout Shift
@@ -1110,16 +1117,14 @@ read:
The swap's `Token In` and `Token Out` lines name a currency, not an amount; each 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 reads `Unknown (not named in the calldata)` when the decoder found no token for
that side, or `Unknown (could not be decoded)` instead when a V4 swap action in that side. An `UNWRAP_WETH` step makes `Token Out` ETH only when the output side
the transaction could not be read (see Transaction Decoding below). An is WETH, on mainnet or Sepolia, or when no step set the output side; a WETH
`UNWRAP_WETH` step makes `Token Out` ETH only when the output side is WETH, on `Min. received` figure then reads in ETH. Otherwise `Token Out` and
mainnet or Sepolia, or when no step set the output side; a WETH `Min. received` `Min. received` keep the output side's own token and figure, whether the swap
figure then reads in ETH. Otherwise `Token Out` and `Min. received` keep the was paid in ETH or in a token: a V2 exact-out swap that buys USDC and then
output side's own token and figure, whether the swap was paid in ETH or in a unwraps the WETH it did not spend shows USDC. The token permission warning on
token: a V2 exact-out swap that buys USDC and then unwraps the WETH it did not the signature screen has its own amount wording, including `Unknown`; the
spend shows USDC. The token permission warning on the signature screen has its SignApproval section below describes it.
own amount wording, including `Unknown`; the SignApproval section below
describes it.
The swap's `Deadline` line is the router's deadline as a UTC date and time, e.g. The swap's `Deadline` line is the router's deadline as a UTC date and time, e.g.
`2026-02-27 08:25:51`. A JavaScript date reaches only to 275760-09-13 00:00:00 `2026-02-27 08:25:51`. A JavaScript date reaches only to 275760-09-13 00:00:00
@@ -2077,7 +2082,9 @@ view would leave a wallet one click from deletion.
artifact can be compared with it field for field. A nonce the site supplies is artifact can be compared with it field for field. A nonce the site supplies is
ignored: the nonce is always the account's next nonce from the node, so a site ignored: the nonce is always the account's next nonce from the node, so a site
cannot replace one of the user's pending transactions or leave this one stuck cannot replace one of the user's pending transactions or leave this one stuck
behind a gap. A request that cannot be populated — unreachable node, reverting behind a gap. A gas limit the site supplies, as `gas` or `gasLimit`, is used
in place of the node's estimate; a request giving both with different values
is refused. A request that cannot be populated — unreachable node, reverting
gas estimate — opens no window and is failed back to the site. Only one gas estimate — opens no window and is failed back to the site. Only one
transaction approval exists at a time: populating fixes the nonce, so a second transaction approval exists at a time: populating fixes the nonce, so a second
`eth_sendTransaction` arriving while one is unanswered is refused with `eth_sendTransaction` arriving while one is unanswered is refused with
@@ -2692,11 +2699,7 @@ Currently supported:
or recipient, and amount. or recipient, and amount.
- **Uniswap Universal Router**: `execute()` calls — shows swap direction (e.g. - **Uniswap Universal Router**: `execute()` calls — shows swap direction (e.g.
"Swap USDT → ETH"), token addresses, amounts, execution steps, and deadline. "Swap USDT → ETH"), token addresses, amounts, execution steps, and deadline.
Decodes Permit2, V2/V3/V4 swaps, wrap/unwrap, and balance checks. A V4 swap Decodes Permit2, V2/V3/V4 swaps, wrap/unwrap, and balance checks.
action is read only in the parameter shape deployed on mainnet and Sepolia,
not in the newer one on `v4-periphery` `main`, which adds a `minHopPriceX36`
field. An action in any other shape is not read, and its step is listed as
`V4 Swap (could not be decoded)`.
### Non-Goals Forever ### Non-Goals Forever
+5 -2
View File
@@ -423,7 +423,10 @@ function releaseApproval(approval) {
// the result on the floor, so a rejection here would be unhandled. // the result on the floor, so a rejection here would be unhandled.
async function openApprovalWindow(id) { async function openApprovalWindow(id) {
const popupUrl = runtime.getURL("src/popup/index.html?approval=" + id); const popupUrl = runtime.getURL("src/popup/index.html?approval=" + id);
const popupWidth = 360; // As wide as the page: body in src/popup/styles/main.css. The browser
// counts the window's frame in this width; the page itself adds back
// whatever the frame takes (src/popup/index.js).
const popupWidth = 396;
const popupHeight = 600; const popupHeight = 600;
let currentWin = null; let currentWin = null;
@@ -443,7 +446,7 @@ async function openApprovalWindow(id) {
// Centred on a browser window only. The last focused window can be // Centred on a browser window only. The last focused window can be
// another approval window still open, and centring on one can give a // another approval window still open, and centring on one can give a
// position the browser refuses ("Bounds must be at least 50% within // position the browser refuses ("Bounds must be at least 50% within
// visible screen space"): headless Chrome reports this 360x600 popup as // visible screen space"): headless Chrome reports this 396x600 popup as
// 1280x720. The request then failed with no window at all. Over a popup, // 1280x720. The request then failed with no window at all. Over a popup,
// the browser picks the position. // the browser picks the position.
if (currentWin && currentWin.type === "normal") { if (currentWin && currentWin.type === "normal") {
+17
View File
@@ -21,6 +21,7 @@ const {
goBack, goBack,
} = require("./views/helpers"); } = require("./views/helpers");
const { applyTheme } = require("./theme"); const { applyTheme } = require("./theme");
const { windowsGetCurrent, windowsUpdate } = require("../shared/browserApi");
// Renders a view the popup lands on without having navigated to it forward: // Renders a view the popup lands on without having navigated to it forward:
// on restore here, and on Back. Only the views that can be fully re-rendered // on restore here, and on Back. Only the views that can be fully re-rendered
// from persisted state (RESTORABLE_VIEWS, src/shared/restorableViews.js) go // from persisted state (RESTORABLE_VIEWS, src/shared/restorableViews.js) go
@@ -233,6 +234,22 @@ async function init() {
const params = new URLSearchParams(window.location.search); const params = new URLSearchParams(window.location.search);
const approvalId = params.get("approval"); const approvalId = params.get("approval");
if (approvalId) { if (approvalId) {
// The background opens this window as wide as the page, but the
// browser counts the window's frame in that width, and how wide the
// frame is depends on the browser and the system. The window is
// widened by however far the page would scroll sideways. The
// connection prompt can also show in the toolbar popup, whose window
// is the user's browser window; that one is left alone.
const sideways =
document.documentElement.scrollWidth -
document.documentElement.clientWidth;
if (sideways > 0) {
windowsGetCurrent().then((win) => {
if (win.type === "popup") {
windowsUpdate(win.id, { width: win.width + sideways });
}
});
}
// Deliberately not awaited, and deliberately not .catch()ed. show() // Deliberately not awaited, and deliberately not .catch()ed. show()
// is async, so a throw past its first await surfaces as an unhandled // is async, so a throw past its first await surfaces as an unhandled
// rejection rather than an uncaught error — measured as still failing // rejection rather than an uncaught error — measured as still failing
+39 -12
View File
@@ -1,13 +1,14 @@
// Preparation of the transaction an approval screen displays. // Preparation of the transaction an approval screen displays.
// //
// A dApp's eth_sendTransaction normally fixes only `to`, `value` and `data`. // A dApp's eth_sendTransaction normally fixes `to`, `value` and `data`, and
// The nonce, the gas limit and the fees have to be filled in from the network // sometimes the gas limit. The nonce, the fees, and the gas limit when the
// before anything can be signed, and whoever fills them in decides what the // site gives none have to be filled in from the network before anything can
// user is shown. That work used to happen in the popup, after the user had // be signed, and whoever fills them in decides what the user is shown. That
// already approved: the numbers on the approval screen came from the popup and // work used to happen in the popup, after the user had already approved: the
// were compared against nothing, so a compromised popup could display one fee // numbers on the approval screen came from the popup and were compared against
// and sign another, and the ceilings in approvalVerify.js were all that stood // nothing, so a compromised popup could display one fee and sign another, and
// between the user and a fee that hands the validator the balance. // the ceilings in approvalVerify.js were all that stood between the user and a
// fee that hands the validator the balance.
// //
// So it happens here instead, in the background, before the approval window is // So it happens here instead, in the background, before the approval window is
// opened. The background populates the transaction, shows that object, and // opened. The background populates the transaction, shows that object, and
@@ -31,6 +32,7 @@ const {
VoidSigner, VoidSigner,
accessListify, accessListify,
getAddress, getAddress,
getBigInt,
getBytes, getBytes,
hexlify, hexlify,
toQuantity, toQuantity,
@@ -47,10 +49,11 @@ const {
// minutes long, which is not a wait anyone will sit through. // minutes long, which is not a wait anyone will sit through.
const POPULATE_TIMEOUT_MS = 20000; const POPULATE_TIMEOUT_MS = 20000;
// The request fields taken from the page. Anything else is dropped rather than // The request fields taken from the page, plus `gas`, which requestFrom()
// passed to ethers: the object is page-controlled, and a future ethers that // takes as the gas limit. Anything else is dropped rather than passed to
// learns to carry a new transaction field must not start picking one up out of // ethers: the object is page-controlled, and a future ethers that learns to
// it without this module knowing. // carry a new transaction field must not start picking one up out of it
// without this module knowing.
// //
// The nonce is not taken from the page; it is always the account's next nonce // The nonce is not taken from the page; it is always the account's next nonce
// from the network. A page that chose it could replace one of the user's // from the network. A page that chose it could replace one of the user's
@@ -84,6 +87,16 @@ function present(v) {
return v !== null && v !== undefined && v !== ""; return v !== null && v !== undefined && v !== "";
} }
// Whether two quantities from the page are the same number. One that is not a
// number matches nothing.
function sameQuantity(a, b) {
try {
return getBigInt(a) === getBigInt(b);
} catch {
return false;
}
}
// These strings reach the user through the requesting page, so they are full // These strings reach the user through the requesting page, so they are full
// sentences even when the tail of one came from ethers or from the node. // sentences even when the tail of one came from ethers or from the node.
function sentence(text) { function sentence(text) {
@@ -111,6 +124,20 @@ function requestFrom(txParams, from) {
for (const key of REQUEST_FIELDS) { for (const key of REQUEST_FIELDS) {
if (present(txParams[key])) request[key] = txParams[key]; if (present(txParams[key])) request[key] = txParams[key];
} }
// JSON-RPC calls the gas limit `gas`, and that is the name sites send.
// ethers only reads `gasLimit`: left as `gas` it would be dropped and an
// estimate signed in its place.
if (present(txParams.gas)) {
if (
present(request.gasLimit) &&
!sameQuantity(request.gasLimit, txParams.gas)
) {
throw fail(
"The site asked for two different gas limits for this transaction.",
);
}
request.gasLimit = txParams.gas;
}
if ( if (
present(request.type) && present(request.type) &&
!ALLOWED_TX_TYPES.includes(Number(request.type)) !ALLOWED_TX_TYPES.includes(Number(request.type))
+18
View File
@@ -250,6 +250,22 @@ function windowsGetLastFocused() {
return invoke(windowsApi(), "getLastFocused"); return invoke(windowsApi(), "getLastFocused");
} }
/**
* @returns {Promise<Object>} the window the calling page is in.
*/
function windowsGetCurrent() {
return invoke(windowsApi(), "getCurrent");
}
/**
* @param {number} windowId
* @param {Object} updateInfo
* @returns {Promise<Object>} the updated window.
*/
function windowsUpdate(windowId, updateInfo) {
return invoke(windowsApi(), "update", windowId, updateInfo);
}
/** /**
* @param {number} windowId * @param {number} windowId
* @returns {Promise<void>} * @returns {Promise<void>}
@@ -275,6 +291,8 @@ module.exports = {
tabsSendMessage, tabsSendMessage,
windowsApi, windowsApi,
windowsCreate, windowsCreate,
windowsGetCurrent,
windowsGetLastFocused, windowsGetLastFocused,
windowsRemove, windowsRemove,
windowsUpdate,
}; };
+16 -58
View File
@@ -53,11 +53,6 @@ function formatAmount(raw, decimals) {
// on a scale — not as a token name, and not as a quantity. // on a scale — not as a token name, and not as a quantity.
const UNNAMED_CURRENCY = "Unknown (not named in the calldata)"; const UNNAMED_CURRENCY = "Unknown (not named in the calldata)";
// The same refusal when a V4 swap action could not be read (see
// decodeExactly()). That action names both of its currencies, so saying the
// calldata did not name one would be false.
const UNDECODED_CURRENCY = "Unknown (could not be decoded)";
// Explicit presence, never truthiness. Every gate in this file that guards a // Explicit presence, never truthiness. Every gate in this file that guards a
// decoded value goes through here: an address is never falsy once set, but an // decoded value goes through here: an address is never falsy once set, but an
// amount of 0n is, and a gate that cannot tell a genuine zero from an absent // amount of 0n is, and a gate that cannot tell a genuine zero from an absent
@@ -300,24 +295,6 @@ function v4ExactInAmount(raw) {
return raw === 0n ? OPEN_DELTA : raw; return raw === 0n ? OPEN_DELTA : raw;
} }
// The V4 swap actions are read only in the parameter structs V4Router has
// deployed on mainnet and Sepolia. v4-periphery `main` has since added a
// `minHopPriceX36` field to all four (`src/interfaces/IV4Router.sol`), and
// reading a shape nobody has deployed would be guessing. ethers alone would
// read the newer structs anyway: it takes whatever word sits where a
// deployed field would be, ignores bytes left over, and holds back an error
// in a field until that field is used. The newer multi-hop exact-in struct
// then yields the offset of its hop-price array as `amountIn`. So a decode
// counts only when encoding the result again gives back the same bytes;
// otherwise this throws, and decodeV4Swap() marks the step as not decoded.
function decodeExactly(types, data) {
const decoded = coder.decode(types, data);
if (coder.encode(types, decoded) !== data) {
throw new Error("Not in the deployed V4 parameter shape.");
}
return decoded;
}
// Decode V4_SWAP (command 0x10) input bytes. // Decode V4_SWAP (command 0x10) input bytes.
// The input is ABI-encoded as (bytes actions, bytes[] params). // The input is ABI-encoded as (bytes actions, bytes[] params).
// We extract token addresses from SETTLE (input) and TAKE (output) sub-actions, // We extract token addresses from SETTLE (input) and TAKE (output) sub-actions,
@@ -332,10 +309,6 @@ function decodeV4Swap(input) {
let takeToken = null; let takeToken = null;
let amountIn = null; let amountIn = null;
let amountOutMin = null; let amountOutMin = null;
// Set when a swap action's parameters are not in the deployed shape
// (see decodeExactly()), so nothing was read from that action. SETTLE
// and TAKE may still name both currencies.
let swapUndecoded = false;
for (let i = 0; i < actions.length; i++) { for (let i = 0; i < actions.length; i++) {
const actionId = actions[i]; const actionId = actions[i];
@@ -364,7 +337,7 @@ function decodeV4Swap(input) {
// tuple(address,uint24,int24,address,bytes)[] path, // tuple(address,uint24,int24,address,bytes)[] path,
// uint128 amountIn, uint128 amountOutMin) // uint128 amountIn, uint128 amountOutMin)
try { try {
const s = decodeExactly( const s = coder.decode(
[ [
"tuple(address,tuple(address,uint24,int24,address,bytes)[],uint128,uint128)", "tuple(address,tuple(address,uint24,int24,address,bytes)[],uint128,uint128)",
], ],
@@ -382,13 +355,13 @@ function decodeV4Swap(input) {
amountOutMin = s[0][3]; amountOutMin = s[0][3];
} }
} catch { } catch {
swapUndecoded = true; // Fall through — SETTLE/TAKE will provide tokens
} }
} else { } else {
// ExactInputSingleParams: (tuple(address,address,uint24,int24,address) poolKey, // ExactInputSingleParams: (tuple(address,address,uint24,int24,address) poolKey,
// bool zeroForOne, uint128 amountIn, uint128 amountOutMin, bytes hookData) // bool zeroForOne, uint128 amountIn, uint128 amountOutMin, bytes hookData)
try { try {
const s = decodeExactly( const s = coder.decode(
[ [
"tuple(tuple(address,address,uint24,int24,address),bool,uint128,uint128,bytes)", "tuple(tuple(address,address,uint24,int24,address),bool,uint128,uint128,bytes)",
], ],
@@ -411,7 +384,7 @@ function decodeV4Swap(input) {
amountOutMin = s[0][3]; amountOutMin = s[0][3];
} }
} catch { } catch {
swapUndecoded = true; // Fall through
} }
} }
} else if ( } else if (
@@ -420,7 +393,7 @@ function decodeV4Swap(input) {
) { ) {
if (actionId === V4_SWAP_EXACT_OUT) { if (actionId === V4_SWAP_EXACT_OUT) {
try { try {
const s = decodeExactly( const s = coder.decode(
[ [
"tuple(address,tuple(address,uint24,int24,address,bytes)[],uint128,uint128)", "tuple(address,tuple(address,uint24,int24,address,bytes)[],uint128,uint128)",
], ],
@@ -432,11 +405,11 @@ function decodeV4Swap(input) {
settleToken = path[path.length - 1][0]; settleToken = path[path.length - 1][0];
} }
} catch { } catch {
swapUndecoded = true; // Fall through
} }
} else { } else {
try { try {
const s = decodeExactly( const s = coder.decode(
[ [
"tuple(tuple(address,address,uint24,int24,address),bool,uint128,uint128,bytes)", "tuple(tuple(address,address,uint24,int24,address),bool,uint128,uint128,bytes)",
], ],
@@ -453,7 +426,7 @@ function decodeV4Swap(input) {
? poolKey[1] ? poolKey[1]
: poolKey[0]; : poolKey[0];
} catch { } catch {
swapUndecoded = true; // Fall through
} }
} }
} }
@@ -467,7 +440,6 @@ function decodeV4Swap(input) {
tokenOut: takeToken, tokenOut: takeToken,
amountIn, amountIn,
amountOutMin, amountOutMin,
swapUndecoded,
}; };
} catch { } catch {
return null; return null;
@@ -493,7 +465,6 @@ function decode(data, toAddress, sources) {
let minOutput = null; let minOutput = null;
let hasUnwrapWeth = false; let hasUnwrapWeth = false;
let hasV2ExactOut = false; let hasV2ExactOut = false;
let swapUndecoded = false;
const commandNames = []; const commandNames = [];
// THE INVARIANT: an amount and the token it is counted in always come // THE INVARIANT: an amount and the token it is counted in always come
@@ -597,12 +568,6 @@ function decode(data, toAddress, sources) {
if (cmdId === 0x10) { if (cmdId === 0x10) {
const v4 = decodeV4Swap(inputs[i]); const v4 = decodeV4Swap(inputs[i]);
if (v4) { if (v4) {
// Without this, a swap action that went unread would
// look like one that had nothing to show.
if (v4.swapUndecoded) {
swapUndecoded = true;
commandNames[i] += " (could not be decoded)";
}
setInputOnce(v4.tokenIn, v4.amountIn); setInputOnce(v4.tokenIn, v4.amountIn);
// Always update output: last swap step wins. A step // Always update output: last swap step wins. A step
// that carries the Min. received figure but decoded no // that carries the Min. received figure but decoded no
@@ -626,13 +591,12 @@ function decode(data, toAddress, sources) {
} }
} }
// Resolve token info. A null token on either side means no step that // Resolve token info. A null token on either side means the calldata
// was read named a currency for it; tokenInfo() refuses rather than // named no currency for it; tokenInfo() refuses rather than calling it
// calling it ETH. UNWRAP_WETH turns WETH into ETH, so it makes the // ETH. UNWRAP_WETH turns WETH into ETH, so it makes the output ETH
// output ETH when the output side is WETH, or when no step set the // when the output side is WETH, or when no step set the output side.
// output side. Any other output keeps its own token and figure: a swap // Any other output keeps its own token and figure: a swap that buys
// that buys USDC and then unwraps the WETH it did not spend receives // USDC and then unwraps the WETH it did not spend receives USDC.
// USDC.
const outputIsWeth = const outputIsWeth =
present(outputToken) && present(outputToken) &&
WETH_ADDRESSES.includes(outputToken.toLowerCase()); WETH_ADDRESSES.includes(outputToken.toLowerCase());
@@ -676,10 +640,7 @@ function decode(data, toAddress, sources) {
// rather than going missing or naming a token by default. Same // rather than going missing or naming a token by default. Same
// wording as the Token Out refusal below: the two sides of this // wording as the Token Out refusal below: the two sides of this
// screen must not describe the same condition in two ways. // screen must not describe the same condition in two ways.
details.push({ details.push({ label: "Token In", value: UNNAMED_CURRENCY });
label: "Token In",
value: swapUndecoded ? UNDECODED_CURRENCY : UNNAMED_CURRENCY,
});
} }
if (present(inputAmount)) { if (present(inputAmount)) {
@@ -742,10 +703,7 @@ function decode(data, toAddress, sources) {
// rather than going missing or naming a token by default, and a // rather than going missing or naming a token by default, and a
// Min. received figure below it is never attached to a token the // Min. received figure below it is never attached to a token the
// calldata did not state. // calldata did not state.
details.push({ details.push({ label: "Token Out", value: UNNAMED_CURRENCY });
label: "Token Out",
value: swapUndecoded ? UNDECODED_CURRENCY : UNNAMED_CURRENCY,
});
} }
if (present(minOutput)) { if (present(minOutput)) {
+7 -5
View File
@@ -36,11 +36,13 @@ const DEFECTS = {
"derivation path beneath that key instead of from a master " + "derivation path beneath that key instead of from a master " +
"key, so the addresses listed here are not the ones that key " + "key, so the addresses listed here are not the ones that key " +
"produces under the standard path.", "produces under the standard path.",
"Signing and sending are disabled for this wallet. The addresses " + "Signing, sending and private key export are disabled for this " +
"do descend from the extended private key you imported, so " + "wallet. Anything these addresses hold can be reached outside " +
"anything they hold is still reachable by software that " + "this wallet with the extended private key you imported, if " +
"repeats the same non-standard derivation. Check them in a " + "you kept a copy of it, or with the master extended private " +
"block explorer before deciding what to do.", "key or recovery phrase it came from, in software that " +
"repeats the same non-standard derivation. Check the " +
"addresses in a block explorer before deciding what to do.",
"To see the addresses this key produces under the standard path, " + "To see the addresses this key produces under the standard path, " +
"import the master extended private key, or the recovery " + "import the master extended private key, or the recovery " +
"phrase it came from, as a new wallet. Nothing here has been " + "phrase it came from, as a new wallet. Nothing here has been " +
+47 -1
View File
@@ -9,7 +9,7 @@
// requesting page or the RPC node can say turns it into an approval that // requesting page or the RPC node can say turns it into an approval that
// should never have been raised. // should never have been raised.
const { Network, Wallet } = require("ethers"); const { Network, Transaction, Wallet, toQuantity } = require("ethers");
const { const {
prepareApprovalTx, prepareApprovalTx,
serializeApprovedTx, serializeApprovedTx,
@@ -138,6 +138,43 @@ describe("prepareApprovalTx", () => {
expect(approved.maxFeePerGas).toBe("0x12a05f200"); expect(approved.maxFeePerGas).toBe("0x12a05f200");
}); });
// `gas` is the JSON-RPC name for the gas limit and the one sites send.
// The node would estimate 21000; the site's 200000 is what is shown and
// what is signed.
test("uses the gas limit a site sends as gas", async () => {
const approved = await prepareApprovalTx(
providerWith(),
signer.address,
{ ...TX_PARAMS, gas: "0x30d40" },
);
expect(approved.gasLimit).toBe("0x30d40");
const signed = Transaction.from(
await signer.signTransaction({ ...approved }),
);
expect(signed.gasLimit).toBe(200000n);
});
test("accepts gas and gasLimit that name the same number", async () => {
const approved = await prepareApprovalTx(
providerWith(),
signer.address,
{ ...TX_PARAMS, gas: "0x30d40", gasLimit: 200000 },
);
expect(approved.gasLimit).toBe("0x30d40");
});
test("refuses gas and gasLimit that disagree", async () => {
await expect(
prepareApprovalTx(providerWith(), signer.address, {
...TX_PARAMS,
gas: "0x30d40",
gasLimit: "0x5208",
}),
).rejects.toThrow(
"The site asked for two different gas limits for this transaction.",
);
});
test("carries an access list the request asked for", async () => { test("carries an access list the request asked for", async () => {
const approved = await prepareApprovalTx( const approved = await prepareApprovalTx(
providerWith(), providerWith(),
@@ -212,6 +249,15 @@ describe("prepareApprovalTx", () => {
).rejects.toThrow(/gas limit no network this wallet supports/); ).rejects.toThrow(/gas limit no network this wallet supports/);
}); });
test("refuses a gas limit the site sent above the ceiling", async () => {
await expect(
prepareApprovalTx(providerWith(), signer.address, {
...TX_PARAMS,
gas: toQuantity(MAX_GAS_LIMIT + 1n),
}),
).rejects.toThrow(/gas limit no network this wallet supports/);
});
// The combined bound at population: a gas limit and a fee that are each // The combined bound at population: a gas limit and a fee that are each
// under their own ceiling but multiply to thousands of ETH is refused // under their own ceiling but multiply to thousands of ETH is refused
// before the approval window opens, so the user is never shown a // before the approval window opens, so the user is never shown a
-1
View File
@@ -58,7 +58,6 @@ const TX_PARAMS = {
to: RECIPIENT, to: RECIPIENT,
value: "0x2386f26fc10000", value: "0x2386f26fc10000",
data: "0xdeadbeef", data: "0xdeadbeef",
gas: "0x5208",
}; };
// The values populateTransaction() fills in when the dApp fixed none of them. // The values populateTransaction() fills in when the dApp fixed none of them.
+47
View File
@@ -45,3 +45,50 @@ test("the approval window shows no screen until the background describes the app
expect(env.visibleViews()).toEqual(["approve-network"]); expect(env.visibleViews()).toEqual(["approve-network"]);
expect(env.pageErrors).toEqual([]); expect(env.pageErrors).toEqual([]);
}); });
// The background opens the approval window as wide as the page, but the
// browser counts the window's frame in that width
// (https://git.eeqj.de/sneak/AutistMask/issues/518). The browser suites run
// without frames, so the widening is checked here.
function windowsStub(type) {
const updates = [];
return {
updates,
getCurrent: (done) => done({ id: 7, type, width: 396 }),
update: (id, info, done) => {
updates.push({ id, info });
done({});
},
};
}
const approvalRuntime = {
connect: () => ({ postMessage: () => {} }),
sendMessage: () => {},
};
test("the approval window widens itself by whatever its frame takes", async () => {
const windows = windowsStub("popup");
const env = await bootPopup(unversionedValidProfile(), {
search: "?approval=approval-1",
runtime: approvalRuntime,
windows,
documentElement: { scrollWidth: 396, clientWidth: 380 },
});
expect(windows.updates).toEqual([{ id: 7, info: { width: 412 } }]);
expect(env.pageErrors).toEqual([]);
});
test("a connection prompt in the toolbar popup leaves the browser window alone", async () => {
const windows = windowsStub("normal");
const env = await bootPopup(unversionedValidProfile(), {
search: "?approval=approval-1",
runtime: approvalRuntime,
windows,
documentElement: { scrollWidth: 396, clientWidth: 380 },
});
expect(windows.updates).toEqual([]);
expect(env.pageErrors).toEqual([]);
});
+3 -3
View File
@@ -2805,7 +2805,7 @@ describe("where an approval window opens", () => {
await settle(); await settle();
expect(bg.created).toHaveLength(1); expect(bg.created).toHaveLength(1);
expect(bg.created[0]).toMatchObject({ left: 460, top: 60 }); expect(bg.created[0]).toMatchObject({ left: 442, top: 60 });
}); });
test("not centred on an approval window the user was last in", async () => { test("not centred on an approval window the user was last in", async () => {
@@ -2822,7 +2822,7 @@ describe("where an approval window opens", () => {
bg.requestSign(); bg.requestSign();
await settle(); await settle();
// Centred, it would be at left 900, the position the browser refused. // Centred, it would be at left 882, the position the browser refused.
expect(bg.created).toHaveLength(1); expect(bg.created).toHaveLength(1);
expect(bg.created[0].left).toBeUndefined(); expect(bg.created[0].left).toBeUndefined();
expect(bg.created[0].top).toBeUndefined(); expect(bg.created[0].top).toBeUndefined();
@@ -2844,7 +2844,7 @@ describe("where an approval window opens", () => {
await settle(); await settle();
expect(bg.created).toHaveLength(2); expect(bg.created).toHaveLength(2);
expect(bg.created[0]).toMatchObject({ left: 1520, top: 750 }); expect(bg.created[0]).toMatchObject({ left: 1502, top: 750 });
expect(bg.created[1].left).toBeUndefined(); expect(bg.created[1].left).toBeUndefined();
expect(bg.created[1].top).toBeUndefined(); expect(bg.created[1].top).toBeUndefined();
+13
View File
@@ -640,6 +640,19 @@ step(
await d.switchToWindow(popup); await d.switchToWindow(popup);
await d.waitVisible("#view-approve-sign"); await d.waitVisible("#view-approve-sign");
// The window is opened as wide as the page
// (https://git.eeqj.de/sneak/AutistMask/issues/518). This suite is
// where that is checked: the Chrome suite's approval windows open at
// 1280x720 whatever width the extension asks for.
const sideways = await d.execute(
"return document.documentElement.scrollWidth -" +
" document.documentElement.clientWidth;",
);
assert(
sideways <= 0,
"the sign prompt scrolls sideways by " + sideways + "px",
);
const screen = await d.execute( const screen = await d.execute(
`return { `return {
origin: document.getElementById("approve-sign-origin").textContent, origin: document.getElementById("approve-sign-origin").textContent,
+11
View File
@@ -242,6 +242,11 @@ async function settle() {
* "?approval=" and an id for the popup opened as an approval window. * "?approval=" and an id for the popup opened as an approval window.
* @param {object} [options.runtime] members of chrome.runtime that replace the * @param {object} [options.runtime] members of chrome.runtime that replace the
* stub's own, for a test that has to answer the background's messages. * stub's own, for a test that has to answer the background's messages.
* @param {object} [options.windows] chrome.windows, for a test of the approval
* window resizing itself.
* @param {object} [options.documentElement] members of document.documentElement
* that replace the stub's own: scrollWidth and clientWidth, which are equal
* unless a test says otherwise, so the page does not scroll sideways.
* @returns {Promise<object>} handles onto the booted page. * @returns {Promise<object>} handles onto the booted page.
*/ */
async function bootPopup(stored, options) { async function bootPopup(stored, options) {
@@ -277,6 +282,11 @@ async function bootPopup(stored, options) {
(options && options.storage) || (options && options.storage) ||
makeStorageStub(stored === undefined ? {} : { autistmask: stored }); makeStorageStub(stored === undefined ? {} : { autistmask: stored });
const document = makeDocument(POPUP_HTML); const document = makeDocument(POPUP_HTML);
Object.assign(
document.documentElement,
{ scrollWidth: 396, clientWidth: 396 },
options && options.documentElement,
);
const reloads = []; const reloads = [];
globalThis.chrome = { globalThis.chrome = {
@@ -287,6 +297,7 @@ async function bootPopup(stored, options) {
onMessage: { addListener: () => {} }, onMessage: { addListener: () => {} },
...(options && options.runtime), ...(options && options.runtime),
}, },
windows: options && options.windows,
}; };
globalThis.document = document; globalThis.document = document;
globalThis.window = { globalThis.window = {
+3 -181
View File
@@ -163,110 +163,6 @@ function encodeV4ExactInSingle(currency0, currency1, amountIn, amountOutMin) {
); );
} }
// Each V4 swap action's parameters in two shapes: the struct V4Router has
// deployed on mainnet and Sepolia, and the one v4-periphery `main` has since
// moved to, which adds a `minHopPriceX36` field. Each swaps WETH for USDC.
const POOL_KEY = "tuple(address,address,uint24,int24,address)";
const PATH_KEY = "tuple(address,uint24,int24,address,bytes)";
const WETH_USDC_POOL = [WETH_ADDR, USDC_ADDR, 100, 1, ZERO_ADDR];
const ONE_WETH = 1000000000000000000n;
const V4_SWAP_ACTION_SHAPES = [
{
action: "SWAP_EXACT_IN_SINGLE",
id: 0x06,
deployed: coder.encode(
[`tuple(${POOL_KEY},bool,uint128,uint128,bytes)`],
[[WETH_USDC_POOL, true, ONE_WETH, 990000n, "0x"]],
),
newer: coder.encode(
[`tuple(${POOL_KEY},bool,uint128,uint128,uint256,bytes)`],
[[WETH_USDC_POOL, true, ONE_WETH, 990000n, 0n, "0x"]],
),
},
{
action: "SWAP_EXACT_IN",
id: 0x07,
deployed: coder.encode(
[`tuple(address,${PATH_KEY}[],uint128,uint128)`],
[
[
WETH_ADDR,
[[USDC_ADDR, 100, 1, ZERO_ADDR, "0x"]],
ONE_WETH,
990000n,
],
],
),
newer: coder.encode(
[`tuple(address,${PATH_KEY}[],uint256[],uint128,uint128)`],
[
[
WETH_ADDR,
[[USDC_ADDR, 100, 1, ZERO_ADDR, "0x"]],
[],
ONE_WETH,
990000n,
],
],
),
},
{
action: "SWAP_EXACT_OUT_SINGLE",
id: 0x08,
deployed: coder.encode(
[`tuple(${POOL_KEY},bool,uint128,uint128,bytes)`],
[[WETH_USDC_POOL, true, 990000n, ONE_WETH, "0x"]],
),
newer: coder.encode(
[`tuple(${POOL_KEY},bool,uint128,uint128,uint256,bytes)`],
[[WETH_USDC_POOL, true, 990000n, ONE_WETH, 0n, "0x"]],
),
},
{
action: "SWAP_EXACT_OUT",
id: 0x09,
deployed: coder.encode(
[`tuple(address,${PATH_KEY}[],uint128,uint128)`],
[
[
USDC_ADDR,
[[WETH_ADDR, 100, 1, ZERO_ADDR, "0x"]],
990000n,
ONE_WETH,
],
],
),
newer: coder.encode(
[`tuple(address,${PATH_KEY}[],uint256[],uint128,uint128)`],
[
[
USDC_ADDR,
[[WETH_ADDR, 100, 1, ZERO_ADDR, "0x"]],
[],
990000n,
ONE_WETH,
],
],
),
},
];
// Helper: execute() with one V4_SWAP step holding the swap action, then
// SETTLE of WETH and TAKE of USDC.
function v4SwapCalldata(actionId, swapParams) {
return buildExecute(
"0x10",
[
encodeV4Swap(new Uint8Array([actionId, V4_SETTLE_ID, V4_TAKE_ID]), [
swapParams,
encodeV4Settle(WETH_ADDR),
encodeV4Take(USDC_ADDR),
]),
],
9999999999n,
);
}
function detail(result, label) { function detail(result, label) {
return result.details.find((d) => d.label === label); return result.details.find((d) => d.label === label);
} }
@@ -307,83 +203,11 @@ describe("uniswap decoder", () => {
result.details.find((d) => d.label === "Min. received").value, result.details.find((d) => d.label === "Min. received").value,
).toBe("0.0002 ETH"); ).toBe("0.0002 ETH");
// Real mainnet calldata is in the deployed shape, so its V4 step is
// not said to be undecoded.
const steps = result.details.find((d) => d.label === "Steps"); const steps = result.details.find((d) => d.label === "Steps");
expect(steps.value).toBe("Permit2 Permit → V4 Swap"); expect(steps.value).toContain("Permit2 Permit");
expect(steps.value).toContain("V4 Swap");
}); });
test.each(V4_SWAP_ACTION_SHAPES)(
"a V4 $action in the deployed shape is read",
({ id, deployed }) => {
const result = uniswap.decode(
v4SwapCalldata(id, deployed),
ROUTER_ADDR,
);
expect(detail(result, "Steps").value).toBe("V4 Swap");
},
);
// https://git.eeqj.de/sneak/AutistMask/issues/358 — before it, all four
// newer structs decoded as the deployed ones without an error, and the
// newer SWAP_EXACT_IN showed the offset of its hop-price array as Amount
// ("0.0000000000000004 WETH") and its amountIn as Min. received.
test.each(V4_SWAP_ACTION_SHAPES)(
"a V4 $action in the newer periphery shape is shown as not decoded",
({ id, newer }) => {
const result = uniswap.decode(
v4SwapCalldata(id, newer),
ROUTER_ADDR,
);
expect(detail(result, "Amount")).toBeUndefined();
expect(detail(result, "Min. received")).toBeUndefined();
expect(detail(result, "Steps").value).toBe(
"V4 Swap (could not be decoded)",
);
// SETTLE and TAKE keep their own shape and still name both sides.
expect(detail(result, "Token In").address).toBe(WETH_ADDR);
expect(detail(result, "Token Out").address).toBe(USDC_ADDR);
},
);
// The decoder does not read SETTLE_ALL or TAKE_ALL, so here only the
// unread swap action names the currencies. The token lines must not say
// the calldata did not name them.
test.each(V4_SWAP_ACTION_SHAPES)(
"a V4 $action in the newer shape, then SETTLE_ALL and TAKE_ALL, says both tokens could not be decoded",
({ id, newer }) => {
const data = buildExecute(
"0x10",
[
// 0x0c is SETTLE_ALL (currency, maxAmount), 0x0f is
// TAKE_ALL (currency, minAmount).
encodeV4Swap(new Uint8Array([id, 0x0c, 0x0f]), [
newer,
coder.encode(
["address", "uint256"],
[WETH_ADDR, ONE_WETH],
),
coder.encode(
["address", "uint256"],
[USDC_ADDR, 990000n],
),
]),
],
9999999999n,
);
const result = uniswap.decode(data, ROUTER_ADDR);
expect(detail(result, "Steps").value).toBe(
"V4 Swap (could not be decoded)",
);
expect(detail(result, "Token In").value).toBe(
"Unknown (could not be decoded)",
);
expect(detail(result, "Token Out").value).toBe(
"Unknown (could not be decoded)",
);
},
);
test("decodes V2_SWAP_EXACT_IN with known tokens", () => { test("decodes V2_SWAP_EXACT_IN with known tokens", () => {
const data = buildExecute( const data = buildExecute(
"0x08", // V2_SWAP_EXACT_IN "0x08", // V2_SWAP_EXACT_IN
@@ -724,11 +548,9 @@ describe("uniswap decoder", () => {
const result = uniswap.decode(data, ROUTER_ADDR); const result = uniswap.decode(data, ROUTER_ADDR);
expect(result).not.toBeNull(); expect(result).not.toBeNull();
// The V4 swap action's parameters are empty, so that step is said to
// be undecoded.
const steps = result.details.find((d) => d.label === "Steps"); const steps = result.details.find((d) => d.label === "Steps");
expect(steps.value).toBe( expect(steps.value).toBe(
"Permit2 Permit \u2192 V4 Swap (could not be decoded) \u2192 Unwrap WETH", "Permit2 Permit \u2192 V4 Swap \u2192 Unwrap WETH",
); );
}); });
+1 -5
View File
@@ -97,11 +97,7 @@ describe("an execute() whose input currency never decoded", () => {
]); ]);
test("says the input token is unknown instead of naming ETH", () => { test("says the input token is unknown instead of naming ETH", () => {
// The swap action that would name it could not be read, so the line expect(detail(data(), "Token In").value).toBe(REFUSAL);
// says that rather than REFUSAL's "not named in the calldata".
expect(detail(data(), "Token In").value).toBe(
"Unknown (could not be decoded)",
);
}); });
test("keeps a Token In line, so the screen never omits what is paid", () => { test("keeps a Token In line, so the screen never omits what is paid", () => {
+12
View File
@@ -153,6 +153,18 @@ describe("the explanatory copy", () => {
expect(all).not.toMatch(/\blost\b|\bstolen\b|\bgone\b/i); expect(all).not.toMatch(/\blost\b|\bstolen\b|\bgone\b/i);
}); });
// The wallet holds the imported key but will not export it, so the copy
// must name every key the user may hold that reaches the addresses
// outside it.
test("it names the keys that reach the addresses outside this wallet", () => {
const body = defect.paragraphs.join(" ");
expect(body).toMatch(/private key export are disabled/);
expect(body).toMatch(
/with the extended private key you imported, if you kept a copy of it, or with the master extended private key or recovery phrase it came from, in software that repeats the same non-standard derivation/,
);
});
test("it says the wallet is not deleted and what the user can do", () => { test("it says the wallet is not deleted and what the user can do", () => {
const body = defect.paragraphs.join(" "); const body = defect.paragraphs.join(" ");