fix: answer eth_chainId and net_version from loaded state (closes #317) #319

Merged
clawbot merged 1 commits from issue-317-cold-worker-chainid into next 2026-08-20 13:11:49 +02:00
3 changed files with 303 additions and 7 deletions
Showing only changes of commit 5c4a671d4a - Show all commits

18
TODO.md
View File

@@ -44,6 +44,24 @@ but the review is broader than any of them.
# Completed Steps # Completed Steps
- 2026-08-20: A page asking which chain the wallet is on is told the chain the
user is actually on ([#317](https://git.eeqj.de/sneak/AutistMask/issues/317)).
`eth_chainId` and `net_version` answered from `currentNetwork()`, which reads
the module-level `state` singleton that nothing populates at module scope, so
a service worker revived by the page's own message answered out of
`DEFAULT_STATE` and reported mainnet `0x1`/`1` to a user on Sepolia — a dApp
building its interaction for the wrong chain. Both now answer from
`getState()`, the per-call detached storage read the other read handlers use,
rather than from the singleton: these two are reachable by any page on every
provider init, and mutating the shared singleton on that path would detach the
wallet objects an in-flight `backgroundRefresh()` is mutating. The read side
of the background was audited with it: the remaining singleton reads are the
chain switch, the transaction verification path and `backgroundRefresh`, which
each already load, and everything else answers from storage per call through
`getState()`. One stale read is left named but unfixed, outside this issue's
scope: `handleSendTransaction` builds its provider with no network name, so
`getProvider()` falls back to the same unloaded singleton for ethers' static
network hint.
- 2026-08-20: The dApp approval screen no longer shows a token transfer it - 2026-08-20: The dApp approval screen no longer shows a token transfer it
cannot scale as `0.0000` cannot scale as `0.0000`
([#306](https://git.eeqj.de/sneak/AutistMask/issues/306)). `decodeCalldata` ([#306](https://git.eeqj.de/sneak/AutistMask/issues/306)). `decodeCalldata`

View File

@@ -3,7 +3,11 @@
// non-sensitive calls to the configured Ethereum JSON-RPC endpoint. // non-sensitive calls to the configured Ethereum JSON-RPC endpoint.
const { DEFAULT_RPC_URL } = require("../shared/constants"); const { DEFAULT_RPC_URL } = require("../shared/constants");
const { SUPPORTED_CHAIN_IDS, networkByChainId } = require("../shared/networks"); const {
SUPPORTED_CHAIN_IDS,
networkById,
networkByChainId,
} = require("../shared/networks");
const { onChainSwitch } = require("../shared/chainSwitch"); const { onChainSwitch } = require("../shared/chainSwitch");
const { const {
state, state,
@@ -663,12 +667,28 @@ async function handleRpc(method, params, origin) {
return { result: [] }; return { result: [] };
} }
if (method === "eth_chainId") { // Both answered from currentNetwork(), which reads the module-level state
return { result: currentNetwork().chainId }; // singleton, and nothing populates that at module scope. A worker revived
} // by the page's own message therefore held DEFAULT_STATE and told a page
// it was on mainnet while the user was on Sepolia
if (method === "net_version") { // (https://git.eeqj.de/sneak/AutistMask/issues/317).
return { result: currentNetwork().networkVersion }; //
// Answered from getState() rather than by loading the singleton. Any page
// reaches these two — neither is gated on a connection, and the injected
// provider sends eth_chainId on every page load — and loadState() replaces
// state.wallets wholesale, which would detach the address objects an
// in-flight backgroundRefresh() is mutating across its network round trip,
// so its saveState() would persist the pre-refresh balances while still
// stamping lastBalanceRefresh. getState() is the detached per-call storage
// read the other read handlers here already use.
// networkById(undefined) falls back to mainnet, matching the default for a
// profile with no stored networkId.
if (method === "eth_chainId" || method === "net_version") {
const s = await getState();
const net = networkById(s.networkId);
return {
result: method === "eth_chainId" ? net.chainId : net.networkVersion,
};
} }
if (method === "wallet_switchEthereumChain") { if (method === "wallet_switchEthereumChain") {

View File

@@ -0,0 +1,258 @@
// What eth_chainId and net_version answer on a worker that has not loaded
// state yet.
//
// The MV3 service worker is terminated when idle and revived by the next
// message, and nothing loads state at module scope. Both methods answered from
// currentNetwork(), which reads the module-level `state` singleton, so a
// worker revived by the page's own message answered out of DEFAULT_STATE and
// told a page it was on mainnet while the user was on Sepolia
// (https://git.eeqj.de/sneak/AutistMask/issues/317).
//
// This file therefore uses the REAL state module and never calls loadState()
// itself: the handler has to answer from storage on its own. Same shape as
// tests/coldWorkerChainSwitch.test.js, which covers the write side.
const { networkById } = require("../src/shared/networks");
const ADDRESS = "0x66133E8ea0f5D1d612D2502a968757D1048c214a";
const CONNECTED_ORIGIN = "https://dapp.example";
const CONNECTED_HOSTNAME = "dapp.example";
const UNKNOWN_ORIGIN = "https://stranger.example";
const MAINNET = networkById("mainnet");
const SEPOLIA = networkById("sepolia");
const REFRESHED_BALANCE = "1.5";
function storedProfile(networkId) {
return {
hasWallet: true,
wallets: [
{
name: "Wallet 1",
type: "hd",
addresses: [
{ address: ADDRESS, balance: "0", tokenBalances: [] },
],
},
],
activeAddress: ADDRESS,
networkId,
rpcUrl: networkById(networkId).defaultRpcUrl,
blockscoutUrl: networkById(networkId).defaultBlockscoutUrl,
allowedSites: { [ADDRESS]: [CONNECTED_HOSTNAME] },
deniedSites: {},
trackedTokens: [],
};
}
async function settle() {
for (let i = 0; i < 50; i++) await Promise.resolve();
}
afterEach(() => {
delete global.chrome;
});
// Load the background worker with the real state module behind it, over a
// storage stub that keeps what is written.
//
// The stub structured-clones in both directions, as the real
// chrome.storage.local does. A stub that handed back the live stored object
// would alias it into whatever read it, so an in-place mutation of a detached
// copy would appear to have reached storage and this whole class of defect
// would be invisible here.
//
// opts.refreshBalances replaces the balances stub, so a test can hold a
// refresh open across a message.
function loadColdWorker(networkId, opts) {
jest.resetModules();
const options = opts || {};
jest.doMock("../src/shared/balances", () => ({
getProvider: () => ({}),
refreshBalances: options.refreshBalances || jest.fn(async () => {}),
}));
jest.doMock("../src/shared/phishingDomains", () => ({
isPhishingDomain: () => false,
}));
let alarmHandlers = {};
jest.doMock("../src/shared/alarms", () => ({
BALANCE_REFRESH_ALARM: "balance",
BALANCE_REFRESH_PERIOD_MINUTES: 1,
ensureRecurringAlarms: jest.fn(async () => {}),
registerAlarmHandlers: jest.fn((handlers) => {
alarmHandlers = handlers;
}),
}));
const store = { autistmask: storedProfile(networkId) };
let messageListener = null;
const set = jest.fn(async (items) => {
store.autistmask = structuredClone(items.autistmask);
});
global.chrome = {
storage: {
local: {
get: jest.fn(async () => structuredClone(store)),
set,
},
},
runtime: {
getURL: (path) => "chrome-extension://autistmask/" + path,
onMessage: {
addListener: (fn) => {
messageListener = fn;
},
},
onConnect: { addListener: () => {} },
lastError: null,
},
windows: {
getLastFocused: (cb) => cb(null),
create: (options, cb) => cb({ id: 1 }),
remove: (id, cb) => {
if (cb) cb();
},
onRemoved: { addListener: () => {} },
},
tabs: {
query: (queryInfo, cb) => cb([{ id: 1 }]),
sendMessage: (tabId, message, cb) => {
if (cb) cb();
},
},
action: { setPopup: () => {} },
};
require("../src/background/index");
async function rpc(method, origin) {
let result = null;
messageListener(
{ type: "AUTISTMASK_RPC", method, params: [] },
{ origin: origin || CONNECTED_ORIGIN },
(r) => {
result = r;
},
);
await settle();
return result;
}
return {
rpc,
persisted: () => store.autistmask,
storageSet: set,
fireBalanceAlarm: () => alarmHandlers.balance(),
};
}
describe("chain identity read by a worker that never loaded state", () => {
test("eth_chainId answers the stored chain, not the default", async () => {
// The first message this worker ever sees. Reading the unloaded
// singleton answers mainnet's 0x1 to a user who is on Sepolia.
const bg = loadColdWorker("sepolia");
expect(await bg.rpc("eth_chainId")).toEqual({
result: SEPOLIA.chainId,
});
});
test("net_version answers the stored chain, not the default", async () => {
const bg = loadColdWorker("sepolia");
expect(await bg.rpc("net_version")).toEqual({
result: SEPOLIA.networkVersion,
});
});
test("answers the stored chain to an origin that never connected", async () => {
// Neither method is gated on a connection, so the stale answer reached
// any page at all; the fixed answer has to as well.
const bg = loadColdWorker("sepolia");
expect(await bg.rpc("eth_chainId", UNKNOWN_ORIGIN)).toEqual({
result: SEPOLIA.chainId,
});
expect(await bg.rpc("net_version", UNKNOWN_ORIGIN)).toEqual({
result: SEPOLIA.networkVersion,
});
});
test("answers mainnet for a profile stored on mainnet", async () => {
// The default and the stored value agree here, so this case cannot
// catch the defect; it is what keeps the fix from being a swap.
const bg = loadColdWorker("mainnet");
expect(await bg.rpc("eth_chainId")).toEqual({
result: MAINNET.chainId,
});
expect(await bg.rpc("net_version")).toEqual({
result: MAINNET.networkVersion,
});
});
test("persists nothing: these are reads", async () => {
// The load must not turn a read into a write. saveState() persists
// every field of the singleton, and a read path that reached it would
// be the wipe https://git.eeqj.de/sneak/AutistMask/issues/316 fixed.
const bg = loadColdWorker("sepolia");
await bg.rpc("eth_chainId");
await bg.rpc("net_version");
expect(bg.storageSet).not.toHaveBeenCalled();
expect(bg.persisted()).toEqual(storedProfile("sepolia"));
});
test("a chain read arriving mid-refresh does not discard the refresh", async () => {
// Any page reaches these two methods, and the injected provider sends
// eth_chainId on every page load, so this overlap is ordinary traffic
// rather than a contrived race.
//
// backgroundRefresh() hands the singleton's wallets to
// refreshBalances(), which mutates those address objects in place once
// the network round trip resolves, and only then saves. Answering the
// page by calling loadState() would replace state.wallets mid-flight,
// so the refreshed balances would land on detached objects and the
// save that follows would persist the pre-refresh values — while still
// stamping lastBalanceRefresh, suppressing the redo.
let releaseRoundTrip;
const roundTrip = new Promise((resolve) => {
releaseRoundTrip = resolve;
});
let refreshReachedNetwork;
const inFlight = new Promise((resolve) => {
refreshReachedNetwork = resolve;
});
const bg = loadColdWorker("sepolia", {
refreshBalances: async (wallets) => {
refreshReachedNetwork();
await roundTrip;
// In place, on the objects handed in — as balances.js does.
wallets[0].addresses[0].balance = REFRESHED_BALANCE;
},
});
const refresh = bg.fireBalanceAlarm();
await inFlight;
expect(await bg.rpc("eth_chainId", UNKNOWN_ORIGIN)).toEqual({
result: SEPOLIA.chainId,
});
releaseRoundTrip();
await refresh;
expect(bg.persisted().wallets[0].addresses[0].balance).toBe(
REFRESHED_BALANCE,
);
});
});