fix: store an absent explorer decimals as unknown instead of fabricating 18 (closes #349)
parseInt(decimals || "18") ran before writing stored tokenBalances[].decimals, so an explorer reporting no decimals produced a fabricated 18 indistinguishable from a real one at read time. That defeated the resolve-or-refuse guarantees of #306 and #340: their refusal paths were intact but never fired, because the guess was laundered upstream of them. An absent scale is now stored as unknown, and a holding whose scale nothing knows carries a null balance -- unknown, never zero -- with six reader sites saying so rather than printing 0.0000. The Send screen resolves the display scale rather than reading the stored one, so a bundled token whose explorer row omits decimals still sends; when the scale cannot be resolved the stored quantity is withdrawn too, so the user is told the balance is unknown rather than only that the fee failed. Existing fabricated 18s cannot be told apart retroactively and are replaced wholesale on the next balance refresh. An explorer-sourced scale stays trusted -- only fabrication is removed; the reasoning is recorded on the issue.
This commit was merged in pull request #367.
This commit is contained in:
455
tests/unknownScaleSend.test.js
Normal file
455
tests/unknownScaleSend.test.js
Normal file
@@ -0,0 +1,455 @@
|
||||
// The Send and confirmation screens for a token whose explorer row carries no
|
||||
// decimals.
|
||||
//
|
||||
// https://git.eeqj.de/sneak/AutistMask/issues/349 made `fetchTokenBalances()`
|
||||
// store the explorer's own answer — `null` when it reported none — while the
|
||||
// scale a balance is DISPLAYED at is resolved separately: bundled list, then
|
||||
// the user's tracked tokens, then the explorer. The two are different
|
||||
// questions, and `tokenBalances[].decimals` only answers the second one.
|
||||
//
|
||||
// A reader that takes the stored field for the display scale therefore gets
|
||||
// `null` for a token the wallet does know the scale of. On the Send path that
|
||||
// null reaches `displayedDecimals()` inside `estimateGas()`, which throws, is
|
||||
// caught as an unavailable fee, and disables Send behind "The network fee could
|
||||
// not be estimated" — untrue, unactionable, and for a bundled token like WETH
|
||||
// or DAI whose scale was never in doubt. So the Send screen resolves the scale
|
||||
// the same way the balance list did, and only carries a null forward when that
|
||||
// resolution genuinely answers null.
|
||||
//
|
||||
// Driven through the real `fetchTokenBalances()`, the real Send review handler
|
||||
// and the real confirmation screen: a test that hand-wrote `decimals: null`
|
||||
// onto state would not show which of the two questions each screen is asking.
|
||||
//
|
||||
// The reader sites that are pure display are in tests/unknownScaleDisplay.test.js,
|
||||
// and what the fetcher stores is in tests/fabricatedDecimals.test.js.
|
||||
|
||||
"use strict";
|
||||
|
||||
jest.mock("../src/shared/log", () => ({
|
||||
log: {
|
||||
debugf: () => {},
|
||||
infof: () => {},
|
||||
warnf: () => {},
|
||||
errorf: () => {},
|
||||
},
|
||||
debugFetch: jest.fn(),
|
||||
setRuntimeDebug: () => {},
|
||||
isDebug: () => false,
|
||||
}));
|
||||
|
||||
// Everything the confirmation screen would reach the network for. The gas
|
||||
// estimate is the point: with a usable scale it must succeed, so that a failure
|
||||
// in these tests is a failure of the scale and not of the stub.
|
||||
const mockProvider = {
|
||||
getFeeData: async () => ({
|
||||
maxFeePerGas: 2000000000n,
|
||||
gasPrice: 1000000000n,
|
||||
}),
|
||||
estimateGas: async () => 21000n,
|
||||
getCode: async () => "0x",
|
||||
getTransactionCount: async () => 1,
|
||||
getBalance: async () => 0n,
|
||||
};
|
||||
|
||||
jest.mock("../src/shared/balances", () => {
|
||||
const actual = jest.requireActual("../src/shared/balances");
|
||||
return { ...actual, getProvider: () => mockProvider };
|
||||
});
|
||||
|
||||
// The confirmation screen's best-effort Etherscan label lookup is the one
|
||||
// thing here that reaches for fetch(). It is stubbed to fail, which is the
|
||||
// path it already takes offline; the assertion at the bottom of this file
|
||||
// pins that it is the ONLY fetch these screens make.
|
||||
global.fetch = jest.fn(() => {
|
||||
throw new Error("tests must not perform network requests");
|
||||
});
|
||||
|
||||
const { makeStorageStub } = require("./support/storageStub");
|
||||
global.chrome = { storage: makeStorageStub(), runtime: { sendMessage() {} } };
|
||||
|
||||
// A stub DOM. Every id in index.html that these two views touch resolves to a
|
||||
// fresh recording element; nothing here depends on layout, only on what the
|
||||
// views write into the elements and which handlers they register.
|
||||
const elements = new Map();
|
||||
|
||||
function makeEl(id) {
|
||||
const handlers = new Map();
|
||||
return {
|
||||
id,
|
||||
textContent: "",
|
||||
innerHTML: "",
|
||||
value: "",
|
||||
disabled: false,
|
||||
onclick: null,
|
||||
style: {},
|
||||
dataset: {},
|
||||
classList: {
|
||||
add() {},
|
||||
remove() {},
|
||||
toggle() {},
|
||||
contains: () => false,
|
||||
},
|
||||
handlers,
|
||||
addEventListener(name, fn) {
|
||||
handlers.set(name, fn);
|
||||
},
|
||||
appendChild(child) {
|
||||
return child;
|
||||
},
|
||||
querySelectorAll: () => [],
|
||||
querySelector: () => null,
|
||||
remove() {},
|
||||
focus() {},
|
||||
};
|
||||
}
|
||||
|
||||
global.document = {
|
||||
getElementById(id) {
|
||||
if (!elements.has(id)) elements.set(id, makeEl(id));
|
||||
return elements.get(id);
|
||||
},
|
||||
createElement: (tag) => makeEl(tag),
|
||||
body: { prepend() {}, appendChild() {} },
|
||||
addEventListener() {},
|
||||
};
|
||||
global.navigator = { clipboard: { writeText() {} } };
|
||||
|
||||
const { parseUnits } = require("ethers");
|
||||
const { fetchTokenBalances } = require("../src/shared/balances");
|
||||
const { debugFetch } = require("../src/shared/log");
|
||||
const { state } = require("../src/shared/state");
|
||||
const {
|
||||
displayedDecimals,
|
||||
transferAmountUnits,
|
||||
} = require("../src/shared/transferAmount");
|
||||
const send = require("../src/popup/views/send");
|
||||
const confirmTx = require("../src/popup/views/confirmTx");
|
||||
const { TOKEN_BY_ADDRESS } = require("../src/shared/tokenList");
|
||||
|
||||
const HOLDER = "0x" + "a".repeat(40);
|
||||
const SECOND_HOLDER = "0x" + "b".repeat(40);
|
||||
const RECIPIENT = "0xC0FfEE0000000000000000000000000000c0fFEe";
|
||||
const BLOCKSCOUT = "https://blockscout.example/api/v2";
|
||||
// Bundled, 18 decimals. The wallet knows this token's scale without asking
|
||||
// anyone, which is what makes an unsendable WETH a regression rather than a
|
||||
// refusal.
|
||||
const WETH = "0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2";
|
||||
// Neither bundled nor tracked, so the explorer is the only possible source and
|
||||
// an omission there really is an unknown scale.
|
||||
const NOVEL = "0xE2E0000000000000000000000000000000000E2e";
|
||||
|
||||
const FIVE_WETH = 5000000000000000000n;
|
||||
|
||||
function row(token = {}, value = FIVE_WETH) {
|
||||
return {
|
||||
value: String(value),
|
||||
token: {
|
||||
type: "ERC-20",
|
||||
address_hash: WETH,
|
||||
symbol: "WETH",
|
||||
name: "Wrapped Ether",
|
||||
holders_count: "50000",
|
||||
...token,
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
// Fetch the explorer's rows through the real fetcher and put them exactly where
|
||||
// refreshBalances() puts them.
|
||||
async function fetchOnto(items) {
|
||||
debugFetch.mockImplementation(async () => ({
|
||||
ok: true,
|
||||
status: 200,
|
||||
statusText: "OK",
|
||||
json: async () => items,
|
||||
}));
|
||||
const balances = await fetchTokenBalances(HOLDER, BLOCKSCOUT, []);
|
||||
state.wallets = [
|
||||
{
|
||||
name: "Wallet 1",
|
||||
addresses: [
|
||||
{ address: HOLDER, balance: "1.0", tokenBalances: balances },
|
||||
],
|
||||
},
|
||||
];
|
||||
state.selectedWallet = 0;
|
||||
state.selectedAddress = 0;
|
||||
return balances;
|
||||
}
|
||||
|
||||
// The same, for two addresses of one wallet holding the same contract. Sending
|
||||
// is from the first. Two addresses is what it takes to reach
|
||||
// explorerDecimals()'s disagreement check, which is only reachable across rows.
|
||||
async function fetchOntoBoth(itemsA, itemsB) {
|
||||
debugFetch.mockImplementation(async () => ({
|
||||
ok: true,
|
||||
status: 200,
|
||||
statusText: "OK",
|
||||
json: async () => itemsA,
|
||||
}));
|
||||
const a = await fetchTokenBalances(HOLDER, BLOCKSCOUT, []);
|
||||
debugFetch.mockImplementation(async () => ({
|
||||
ok: true,
|
||||
status: 200,
|
||||
statusText: "OK",
|
||||
json: async () => itemsB,
|
||||
}));
|
||||
const b = await fetchTokenBalances(SECOND_HOLDER, BLOCKSCOUT, []);
|
||||
state.wallets = [
|
||||
{
|
||||
name: "Wallet 1",
|
||||
addresses: [
|
||||
{ address: HOLDER, balance: "1.0", tokenBalances: a },
|
||||
{ address: SECOND_HOLDER, balance: "1.0", tokenBalances: b },
|
||||
],
|
||||
},
|
||||
];
|
||||
state.selectedWallet = 0;
|
||||
state.selectedAddress = 0;
|
||||
return { a, b };
|
||||
}
|
||||
|
||||
function el(id) {
|
||||
return global.document.getElementById(id);
|
||||
}
|
||||
|
||||
// Press Review on the Send screen and return the txInfo it hands the
|
||||
// confirmation screen.
|
||||
async function reviewSend(tokenAddress, amount) {
|
||||
let handed = null;
|
||||
send.init({ showConfirmTx: (info) => (handed = info) });
|
||||
state.selectedToken = tokenAddress;
|
||||
el("send-token").value = tokenAddress;
|
||||
el("send-to").value = RECIPIENT;
|
||||
el("send-amount").value = amount;
|
||||
await el("btn-send-review").handlers.get("click")();
|
||||
return handed;
|
||||
}
|
||||
|
||||
// show() kicks off the gas estimate without awaiting it; this lets it settle.
|
||||
async function settle() {
|
||||
for (let i = 0; i < 10; i++) await new Promise((r) => setTimeout(r, 0));
|
||||
}
|
||||
|
||||
function text(id) {
|
||||
return el(id).textContent;
|
||||
}
|
||||
|
||||
function errors() {
|
||||
return el("confirm-errors").innerHTML;
|
||||
}
|
||||
|
||||
function sendDisabled() {
|
||||
return el("btn-confirm-send").disabled;
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
elements.clear();
|
||||
debugFetch.mockReset();
|
||||
state.wallets = [];
|
||||
state.trackedTokens = [];
|
||||
state.selectedToken = null;
|
||||
state.fraudContracts = [];
|
||||
state.hideLowHolderTokens = false;
|
||||
state.currentView = null;
|
||||
});
|
||||
|
||||
describe("the Send screen resolves the scale rather than reading the stored one", () => {
|
||||
test("the bundled list knows WETH, and the explorer row does not report a scale", async () => {
|
||||
expect(TOKEN_BY_ADDRESS.get(WETH.toLowerCase()).decimals).toBe(18);
|
||||
const balances = await fetchOnto([row()]);
|
||||
// Stored: the explorer's own answer, which is nothing. Reading THIS is
|
||||
// what carried a null into the fee estimate.
|
||||
expect(balances[0].decimals).toBeNull();
|
||||
// Displayed: the bundled scale, so the quantity on screen is real.
|
||||
expect(balances[0].balance).toBe("5.0");
|
||||
});
|
||||
|
||||
test("the review hands the confirmation screen the resolved scale, not the stored null", async () => {
|
||||
const balances = await fetchOnto([row()]);
|
||||
const txInfo = await reviewSend(WETH, "1.5");
|
||||
expect(txInfo.tokenDecimals).toBe(18);
|
||||
expect(txInfo.tokenDecimals).not.toBe(balances[0].decimals);
|
||||
expect(txInfo.tokenBalance).toBe("5.0");
|
||||
});
|
||||
|
||||
test("that scale estimates a fee and leaves Send enabled", async () => {
|
||||
await fetchOnto([row()]);
|
||||
const txInfo = await reviewSend(WETH, "1.5");
|
||||
confirmTx.show(txInfo);
|
||||
await settle();
|
||||
// The regression: displayedDecimals(null) threw in estimateGas(), the
|
||||
// catch reported the fee as unknown, and Send stayed disabled behind a
|
||||
// message about the network fee that no retry could clear.
|
||||
expect(text("confirm-fee-amount")).not.toBe("Unable to estimate");
|
||||
expect(text("confirm-fee-amount")).toContain("ETH");
|
||||
expect(errors()).toBe("");
|
||||
expect(sendDisabled()).toBe(false);
|
||||
});
|
||||
|
||||
test("and the transfer encodes at the scale that was displayed", async () => {
|
||||
await fetchOnto([row()]);
|
||||
const txInfo = await reviewSend(WETH, "1.5");
|
||||
// The two calls confirmTx makes with this field: the gas estimate's
|
||||
// scale, and the encode, which compares it against the contract's own
|
||||
// decimals() before parsing.
|
||||
expect(displayedDecimals(txInfo.tokenDecimals)).toBe(18);
|
||||
expect(
|
||||
transferAmountUnits(txInfo.amount, txInfo.tokenDecimals, 18n),
|
||||
).toBe(parseUnits("1.5", 18));
|
||||
});
|
||||
|
||||
test("a token nothing knows the scale of is still refused, and says why", async () => {
|
||||
await fetchOnto([
|
||||
row({ address_hash: NOVEL, symbol: "NOVEL", name: "Novel Token" }),
|
||||
]);
|
||||
const txInfo = await reviewSend(NOVEL, "1.5");
|
||||
// No fallback was introduced: resolution answers null here, and the
|
||||
// null is what goes forward.
|
||||
expect(txInfo.tokenDecimals).toBeNull();
|
||||
expect(txInfo.tokenBalance).toBeNull();
|
||||
confirmTx.show(txInfo);
|
||||
await settle();
|
||||
expect(text("confirm-balance")).toBe("unknown (NOVEL)");
|
||||
expect(errors()).toContain("This token's balance is unknown");
|
||||
expect(sendDisabled()).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
// balances.js resolves the display scale WITHOUT `wallets`, so its explorer leg
|
||||
// is the row it is formatting. send.js resolves WITH `wallets`, so its explorer
|
||||
// leg is explorerDecimals(), which answers null when two addresses report
|
||||
// different scales for one contract — the check that must apply before a scale
|
||||
// encodes a transfer. The two therefore disagree exactly here, and a stored
|
||||
// balance formatted at a scale the Send screen just refused is not a balance it
|
||||
// may state: it would leave validateTransfer() satisfied, the unknown-balance
|
||||
// sentence unfired, and the fee-estimate failure as the only thing on screen.
|
||||
describe("a scale the explorer's own rows disagree about", () => {
|
||||
// 5000000 units at the "6" address A reports, 5e18 at the "18" address B
|
||||
// reports: both format to "5.0", so the disagreement is in the scale alone
|
||||
// and not in the quantity.
|
||||
function novel(decimals, value) {
|
||||
return row(
|
||||
{
|
||||
address_hash: NOVEL,
|
||||
symbol: "NOVEL",
|
||||
name: "Novel Token",
|
||||
decimals,
|
||||
},
|
||||
value,
|
||||
);
|
||||
}
|
||||
|
||||
test("is stored per row, because storage holds the explorer's own answer", async () => {
|
||||
const { a, b } = await fetchOntoBoth(
|
||||
[novel("6", 5000000n)],
|
||||
[novel("18", FIVE_WETH)],
|
||||
);
|
||||
expect(a[0].decimals).toBe(6);
|
||||
expect(a[0].balance).toBe("5.0");
|
||||
expect(b[0].decimals).toBe(18);
|
||||
});
|
||||
|
||||
test("resolves to null on the Send screen, and takes the balance with it", async () => {
|
||||
await fetchOntoBoth([novel("6", 5000000n)], [novel("18", FIVE_WETH)]);
|
||||
const txInfo = await reviewSend(NOVEL, "1.5");
|
||||
expect(txInfo.tokenDecimals).toBeNull();
|
||||
// The regression this closes: null scale alongside a non-null balance.
|
||||
expect(txInfo.tokenBalance).toBeNull();
|
||||
});
|
||||
|
||||
test("so the user is told the balance is unknown, not only that the fee failed", async () => {
|
||||
await fetchOntoBoth([novel("6", 5000000n)], [novel("18", FIVE_WETH)]);
|
||||
const txInfo = await reviewSend(NOVEL, "1.5");
|
||||
confirmTx.show(txInfo);
|
||||
await settle();
|
||||
// Before the fix: "5.0 NOVEL", an empty confirm-errors, and
|
||||
// confirm-fee-unknown-error — "the network fee could not be
|
||||
// estimated... please go back and try again" — as the only explanation
|
||||
// for a screen that can never proceed.
|
||||
expect(errors()).not.toBe("");
|
||||
expect(errors()).toContain("This token's balance is unknown");
|
||||
expect(text("confirm-balance")).toBe("unknown (NOVEL)");
|
||||
expect(sendDisabled()).toBe(true);
|
||||
// The fee line still reports the estimate as unavailable, because it
|
||||
// genuinely is — displayedDecimals() refuses the same missing scale.
|
||||
// What changed is that it is no longer the ONLY thing on the screen,
|
||||
// and no longer the only offered explanation. This is exactly how the
|
||||
// token nothing knows the scale of already behaved.
|
||||
expect(text("confirm-fee-amount")).toBe("Unable to estimate");
|
||||
expect(el("confirm-fee-unknown-error").style.visibility).toBe(
|
||||
"visible",
|
||||
);
|
||||
});
|
||||
|
||||
test("while agreeing rows leave the scale usable", async () => {
|
||||
await fetchOntoBoth([novel("6", 5000000n)], [novel("6", 5000000n)]);
|
||||
const txInfo = await reviewSend(NOVEL, "1.5");
|
||||
expect(txInfo.tokenDecimals).toBe(6);
|
||||
expect(txInfo.tokenBalance).toBe("5.0");
|
||||
confirmTx.show(txInfo);
|
||||
await settle();
|
||||
expect(text("confirm-balance")).toBe("5.0 NOVEL");
|
||||
expect(errors()).toBe("");
|
||||
expect(sendDisabled()).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("the confirmation screen tells an unknown balance from a zero one", () => {
|
||||
function txInfo(tokenBalance) {
|
||||
return {
|
||||
from: HOLDER,
|
||||
to: RECIPIENT,
|
||||
ensName: null,
|
||||
amount: "1.5",
|
||||
token: NOVEL,
|
||||
balance: "1.0",
|
||||
tokenSymbol: "NOVEL",
|
||||
tokenBalance,
|
||||
tokenDecimals: tokenBalance === null ? null : 18,
|
||||
};
|
||||
}
|
||||
|
||||
async function render(tokenBalance) {
|
||||
state.wallets = [
|
||||
{
|
||||
name: "Wallet 1",
|
||||
addresses: [
|
||||
{ address: HOLDER, balance: "1.0", tokenBalances: [] },
|
||||
],
|
||||
},
|
||||
];
|
||||
state.selectedWallet = 0;
|
||||
state.selectedAddress = 0;
|
||||
confirmTx.show(txInfo(tokenBalance));
|
||||
await settle();
|
||||
return { balance: text("confirm-balance"), errors: errors() };
|
||||
}
|
||||
|
||||
test("the balance line states unknown rather than a quantity of zero", async () => {
|
||||
const unknown = await render(null);
|
||||
const zero = await render("0.0");
|
||||
expect(unknown.balance).not.toBe(zero.balance);
|
||||
expect(unknown.balance).toBe("unknown (NOVEL)");
|
||||
expect(zero.balance).toBe("0.0 NOVEL");
|
||||
});
|
||||
|
||||
// Both hit INSUFFICIENT_TOKEN — an unknown balance is treated as nothing to
|
||||
// spend from, which is the fail-closed side — but "you have 0.0" is a claim
|
||||
// about the holding, and this one has no established quantity to claim.
|
||||
test("the insufficient-balance message names the reason, not a figure", async () => {
|
||||
const unknown = await render(null);
|
||||
const zero = await render("0.0");
|
||||
expect(unknown.errors).not.toBe(zero.errors);
|
||||
expect(unknown.errors).toContain("This token's balance is unknown");
|
||||
expect(unknown.errors).not.toContain("You have");
|
||||
expect(zero.errors).toContain("You have 0.0 NOVEL");
|
||||
expect(zero.errors).not.toContain("balance is unknown");
|
||||
});
|
||||
});
|
||||
|
||||
test("the only network these screens reached for is the Etherscan label lookup", () => {
|
||||
for (const [url] of global.fetch.mock.calls) {
|
||||
expect(String(url)).toMatch(/^https:\/\/etherscan\.io\/address\//);
|
||||
}
|
||||
});
|
||||
Reference in New Issue
Block a user