fix: store an absent explorer decimals as unknown instead of fabricating 18 (closes #349)
fetchTokenBalances() did parseInt(item.token.decimals || "18", 10) before writing to state.wallets[].addresses[].tokenBalances[].decimals, so a token whose decimals() reverts -- one the block explorer reports no scale for -- was stored with a fabricated 18 that no reader could tell from a real one. That is upstream of a rule already merged. #306 made the ERC-20 approval amount line resolve the real scale or refuse to format, and #340 extended it to the swap lines; both read this stored value as an authoritative source, so the guess walked straight past refusals that were intact and simply never fired. A 1,000-unit approval of such a token rendered 0.000000001 on the one screen whose job is to state what is being authorized. The stored value is now the explorer's own answer or null, never a default. Both approval paths reach unknownDecimalsAmount() on a null, using the refusal that was already there. The history list's token transfers carried the same || "18" and now state exact base units with the scale unknown rather than a quantity at a guessed one. A holding whose scale nothing knows has no quantity either, so its balance is stored as null -- unknown, never zero -- and the balance list, the address USD total, the Send screen and the confirmation screen each say so rather than printing 0.0000 for money that is really there. The zero-balance filter moved onto the base-unit integer, where it needs no scale at all. The bundled token list and the user's tracked tokens already outrank the explorer, so a token either of them knows still displays its real quantity when the explorer's entry omits decimals; only what none of the three knows is unknown. Which makes the stored field the explorer's answer alone, and NOT the scale a screen renders at. Those are two questions, and every screen that needs the second one asks resolveTokenDecimals(). The Send screen did not: it read tokenBalances[].decimals raw and carried it onto the pending transaction, so a bundled or tracked token whose explorer row omits decimals reached displayedDecimals(null) inside estimateGas(). That throws, is caught as an unavailable fee, and disables Send behind "The network fee could not be estimated ... Please go back and try again" -- untrue, unactionable, and for a token such as WETH whose scale was never in doubt. The balance and the amount on the same screen were correct throughout, and validateTransfer() had nothing to object to, so nothing named the real reason. Before this change the fabricated 18 happened to be that token's real scale and the send completed, so this is a capability regression and not an inherited one. Send now resolves the scale through resolveTokenDecimals(), with no fallback. The two resolutions are deliberately not identical, and where they differ the balance follows the scale. balances.js resolves without wallets, because it is formatting one explorer row during a fetch that is about to replace the very state it would be consulting; its explorer leg is therefore that row's own value. send.js resolves with wallets, which adds explorerDecimals()'s cross-address check, so a contract two addresses report different scales for answers null rather than picking one -- a check that must apply to a value which goes on to encode a transfer. For a token neither bundled nor tracked whose explorer rows disagree, that leaves a stored quantity computed at a scale Send has just refused. Stating it would leave validateTransfer() checking the amount against a number the wallet does not vouch for, and, since the unknown-balance path is gated on the balance rather than on the scale, would again leave the fee-estimate failure as the only thing on the confirmation screen. So Send withdraws the stored quantity along with the scale: an unknown scale is an unknown balance. Only a stored quantity is withdrawn -- the "0" for a token with no row at all is an absence of holdings, which is true at every scale. The uint8 check is one shared toDecimals() rather than three copies of it, and it answers 0 for a real scale of zero: || "18" collapsed that to eighteen, the falsy-collapse trap of #246. The reader half is asserted, not just the writer half. Each of the six sites that now distinguishes an unknown quantity from a zero one -- balanceLine(), balanceLinesForAddress(), addressHoldsFunds(), getAddressValue()'s partial flag, the Send balance line and the confirmation screen's balance and insufficient-balance wording -- is tested on the PAIR, because an assertion about null alone still passes on a build that renders both as zero. The Send and confirmation cases run the real explorer response through the real fetcher, the real review handler and the real confirmation screen, so they show which of the two scale questions each screen is asking, including a two-address fixture whose explorer rows report 6 and 18 for one contract. Existing installs hold 18s that cannot be told apart retroactively -- that is the defect, and no migration can undo it. They display exactly as they do today until the next balance refresh, which rewrites tokenBalances wholesale and needs no user action. The schema version is not bumped: version 1 records stay valid and are read exactly as before. No || 18 or ?? 18 fallback remains anywhere in src/. The literal 18s that do remain are real data rather than defaults: 432 per-token decimals: 18 entries in the bundled src/shared/tokenList.js, and, outside that file, only native ETH's protocol-defined scale in src/shared/uniswap.js and the fixed-point comparison scale in src/shared/txValidation.js.
This commit is contained in:
282
tests/fabricatedDecimals.test.js
Normal file
282
tests/fabricatedDecimals.test.js
Normal file
@@ -0,0 +1,282 @@
|
||||
// What the balance fetcher stores when the block explorer reports no decimals
|
||||
// for a token, and what the approval screens then display.
|
||||
//
|
||||
// https://git.eeqj.de/sneak/AutistMask/issues/349: `fetchTokenBalances()` did
|
||||
// `parseInt(item.token.decimals || "18", 10)` BEFORE writing the row, so a
|
||||
// token whose `decimals()` reverts — and which the explorer therefore reports
|
||||
// no scale for — was stored with a fabricated 18. Nothing downstream could
|
||||
// tell that from a real 18.
|
||||
//
|
||||
// That matters because it is upstream of two refusals that were already built
|
||||
// and already merged. https://git.eeqj.de/sneak/AutistMask/issues/306 made the
|
||||
// ERC-20 amount line resolve the real scale or refuse to format, and
|
||||
// https://git.eeqj.de/sneak/AutistMask/issues/340 did the same for the swap
|
||||
// lines. Both read this stored value as an authoritative source, so the guess
|
||||
// walked straight past them: the refusal was intact and simply never fired.
|
||||
//
|
||||
// So these tests run a real explorer response through the real fetcher and
|
||||
// assert on the real approval screens. A test that hand-writes `decimals: null`
|
||||
// onto state would pass on the broken build, because the fabrication is in the
|
||||
// writer, not the readers.
|
||||
|
||||
jest.mock("../src/shared/log", () => ({
|
||||
log: {
|
||||
debugf: () => {},
|
||||
infof: () => {},
|
||||
warnf: () => {},
|
||||
errorf: () => {},
|
||||
},
|
||||
debugFetch: jest.fn(),
|
||||
setRuntimeDebug: () => {},
|
||||
isDebug: () => false,
|
||||
}));
|
||||
|
||||
global.fetch = jest.fn(() => {
|
||||
throw new Error("tests must not perform network requests");
|
||||
});
|
||||
|
||||
const { makeStorageStub } = require("./support/storageStub");
|
||||
global.chrome = { storage: makeStorageStub() };
|
||||
|
||||
const { AbiCoder, Interface } = require("ethers");
|
||||
const { ERC20_ABI } = require("../src/shared/constants");
|
||||
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 { TOKEN_BY_ADDRESS } = require("../src/shared/tokenList");
|
||||
|
||||
const HOLDER = "0x" + "a".repeat(40);
|
||||
const BLOCKSCOUT = "https://blockscout.example/api/v2";
|
||||
const ROUTER = "0x66a9893cc07d91d95644aedd05d03f95e1dba8af";
|
||||
const RECIPIENT = "0xC0FfEE0000000000000000000000000000c0fFEe";
|
||||
const SPENDER = "0x1111111111111111111111111111111111111111";
|
||||
// Outside the bundled list and untracked, so the explorer is the only source
|
||||
// of a scale for it — which is the case the fabrication was hiding.
|
||||
const NOVEL = "0xE2E0000000000000000000000000000000000E2e";
|
||||
// In the bundled list, at 18 decimals, for the other side of a swap.
|
||||
const WETH = "0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2";
|
||||
|
||||
// The holding the explorer reports, in base units. Large enough that it does
|
||||
// not round to zero even when divided by 10^18, which is what makes it the
|
||||
// case the laundering actually REACHED: a smaller holding formatted at the
|
||||
// fabricated 18 comes out "0.0", the balance list drops the row as dust, and
|
||||
// the approval screens then find no source for the scale and refuse anyway —
|
||||
// for the wrong reason, and only by luck.
|
||||
const HOLDING = 5000000000000000000n;
|
||||
|
||||
// The amount in the dApp's calldata, which is a separate number from the
|
||||
// holding. 1,000.00 of a 6-decimal token; formatted at the fabricated 18 it
|
||||
// reads 0.000000001, and at a real scale of 0 it reads 1000000000.
|
||||
const THOUSAND_AT_SIX = 1000000000n;
|
||||
const HALF_WETH = 500000000000000000n;
|
||||
|
||||
const erc20Iface = new Interface(ERC20_ABI);
|
||||
const coder = AbiCoder.defaultAbiCoder();
|
||||
const routerIface = new Interface([
|
||||
"function execute(bytes commands, bytes[] inputs, uint256 deadline)",
|
||||
]);
|
||||
|
||||
// One Blockscout token-balances row. `token` is spread last so a test can
|
||||
// override or blank a field; the base row carries no `decimals` at all, which
|
||||
// is exactly what a token whose decimals() reverts produces.
|
||||
function row(token = {}, value = HOLDING) {
|
||||
return {
|
||||
value: String(value),
|
||||
token: {
|
||||
type: "ERC-20",
|
||||
address_hash: NOVEL,
|
||||
symbol: "NOVEL",
|
||||
name: "Novel Token",
|
||||
// Well clear of the balance list's own spam floor, so the row is
|
||||
// admitted on its holder count alone: neither the bundled list nor
|
||||
// a tracked entry can supply a scale for it.
|
||||
holders_count: "50000",
|
||||
...token,
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
function respondWith(items) {
|
||||
debugFetch.mockImplementation(async () => ({
|
||||
ok: true,
|
||||
status: 200,
|
||||
statusText: "OK",
|
||||
json: async () => items,
|
||||
}));
|
||||
}
|
||||
|
||||
// Fetch and place the result exactly where refreshBalances() places it, so the
|
||||
// approval screens read what a real refresh would have left on state.
|
||||
async function fetchOnto(items, trackedTokens = []) {
|
||||
respondWith(items);
|
||||
const balances = await fetchTokenBalances(
|
||||
HOLDER,
|
||||
BLOCKSCOUT,
|
||||
trackedTokens,
|
||||
);
|
||||
state.trackedTokens = trackedTokens;
|
||||
state.wallets = [
|
||||
{
|
||||
name: "Wallet 1",
|
||||
addresses: [
|
||||
{ address: HOLDER, balance: "1.0", tokenBalances: balances },
|
||||
],
|
||||
},
|
||||
];
|
||||
return balances;
|
||||
}
|
||||
|
||||
// The ERC-20 approval screen's Amount line, and the swap decoder's.
|
||||
function erc20AmountLine(data, tokenAddress) {
|
||||
return decodeCalldata(data, tokenAddress).details.find(
|
||||
(d) => d.label === "Amount",
|
||||
).value;
|
||||
}
|
||||
|
||||
function swapAmountLine(data) {
|
||||
return decodeCalldata(data, ROUTER).details.find(
|
||||
(d) => d.label === "Amount",
|
||||
).value;
|
||||
}
|
||||
|
||||
function transferData(amount) {
|
||||
return erc20Iface.encodeFunctionData("transfer", [RECIPIENT, amount]);
|
||||
}
|
||||
|
||||
function approveData(amount) {
|
||||
return erc20Iface.encodeFunctionData("approve", [SPENDER, amount]);
|
||||
}
|
||||
|
||||
function swapData(tokenIn, amountIn, tokenOut, amountOutMin) {
|
||||
const input = coder.encode(
|
||||
["address", "uint256", "uint256", "address[]", "bool"],
|
||||
[RECIPIENT, amountIn, amountOutMin, [tokenIn, tokenOut], true],
|
||||
);
|
||||
return routerIface.encodeFunctionData("execute", [
|
||||
"0x08",
|
||||
[input],
|
||||
9999999999n,
|
||||
]);
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
debugFetch.mockReset();
|
||||
state.trackedTokens = [];
|
||||
state.wallets = [];
|
||||
});
|
||||
|
||||
describe("what fetchTokenBalances stores for an absent scale", () => {
|
||||
test("the token is not in the bundled list, so the explorer is the only source", () => {
|
||||
expect(TOKEN_BY_ADDRESS.has(NOVEL.toLowerCase())).toBe(false);
|
||||
});
|
||||
|
||||
test("an absent decimals is stored as null, not as 18", async () => {
|
||||
const balances = await fetchOnto([row()]);
|
||||
expect(balances).toHaveLength(1);
|
||||
expect(balances[0].decimals).toBeNull();
|
||||
});
|
||||
|
||||
test("an explicit null decimals is stored as null too", async () => {
|
||||
const balances = await fetchOnto([row({ decimals: null })]);
|
||||
expect(balances[0].decimals).toBeNull();
|
||||
});
|
||||
|
||||
// The same explorer row twice, differing only in whether it reports a
|
||||
// scale of 18. Before the fix both stored 18 and no reader could tell
|
||||
// which one had actually been reported.
|
||||
test("a real 18 is stored as 18, and so is distinguishable from absent", async () => {
|
||||
const real = await fetchOnto([row({ decimals: "18" })]);
|
||||
expect(real[0].decimals).toBe(18);
|
||||
expect(real[0].balance).toBe("5.0");
|
||||
const absent = await fetchOnto([row()]);
|
||||
expect(absent[0].decimals).toBeNull();
|
||||
expect(real[0].decimals).not.toBe(absent[0].decimals);
|
||||
});
|
||||
|
||||
// The falsy-collapse trap of
|
||||
// https://git.eeqj.de/sneak/AutistMask/issues/246. `decimals || "18"` reads
|
||||
// a real scale of zero as absent and then as eighteen, which is eighteen
|
||||
// orders of magnitude of error in the direction that displays as nothing.
|
||||
test("a real scale of zero is stored as zero, not collapsed", async () => {
|
||||
for (const reported of ["0", 0]) {
|
||||
const balances = await fetchOnto([row({ decimals: reported })]);
|
||||
expect(balances[0].decimals).toBe(0);
|
||||
expect(balances[0].balance).toBe("5000000000000000000.0");
|
||||
}
|
||||
});
|
||||
|
||||
test("no quantity is stated for a holding whose scale is unknown", async () => {
|
||||
const balances = await fetchOnto([row()]);
|
||||
// Not "0.0": the holding is real and nonzero, and a zero here is the
|
||||
// same lie the approval screens refuse to tell.
|
||||
expect(balances[0].balance).toBeNull();
|
||||
});
|
||||
|
||||
// Zero base units is zero tokens at every scale, so this filter never
|
||||
// needed a scale in the first place and does not acquire one now.
|
||||
test("a holding of zero base units is still dropped without a scale", async () => {
|
||||
expect(await fetchOnto([row({}, 0n)])).toEqual([]);
|
||||
});
|
||||
|
||||
test("the bundled list still supplies a quantity the explorer omitted", async () => {
|
||||
const balances = await fetchOnto([
|
||||
row({ address_hash: WETH, symbol: "WETH" }),
|
||||
]);
|
||||
// The stored decimals stay the explorer's own answer — absent. Copying
|
||||
// another source in here would make explorerDecimals()'s disagreement
|
||||
// check compare something other than explorer values.
|
||||
expect(balances[0].decimals).toBeNull();
|
||||
// The displayed quantity still comes out right, because the bundled
|
||||
// list knows this token's scale and outranks the explorer anyway.
|
||||
expect(balances[0].balance).toBe("5.0");
|
||||
});
|
||||
});
|
||||
|
||||
describe("the ERC-20 approval line reaches its refusal", () => {
|
||||
test("a transfer of a token the explorer gave no scale for is not formatted", async () => {
|
||||
await fetchOnto([row()]);
|
||||
const line = erc20AmountLine(transferData(THOUSAND_AT_SIX), NOVEL);
|
||||
expect(line).toBe(unknownDecimalsAmount(THOUSAND_AT_SIX));
|
||||
// The defect: a fabricated 18 renders this as 0.000000001, a quantity,
|
||||
// and a wrong one.
|
||||
expect(line).not.toMatch(/^0\./);
|
||||
});
|
||||
|
||||
test("an approve of the same token is not formatted either", async () => {
|
||||
await fetchOnto([row()]);
|
||||
const line = erc20AmountLine(approveData(THOUSAND_AT_SIX), NOVEL);
|
||||
expect(line).toBe(unknownDecimalsAmount(THOUSAND_AT_SIX));
|
||||
expect(line).not.toMatch(/^0\./);
|
||||
});
|
||||
|
||||
test("a scale the explorer did report still formats", async () => {
|
||||
await fetchOnto([row({ decimals: "6" })]);
|
||||
expect(erc20AmountLine(transferData(THOUSAND_AT_SIX), NOVEL)).toBe(
|
||||
"1000.0000",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("the swap approval line reaches its refusal", () => {
|
||||
test("a swap of a token the explorer gave no scale for is not formatted", async () => {
|
||||
await fetchOnto([row()]);
|
||||
const line = swapAmountLine(
|
||||
swapData(NOVEL, THOUSAND_AT_SIX, WETH, HALF_WETH),
|
||||
);
|
||||
expect(line).toBe(unknownDecimalsAmount(THOUSAND_AT_SIX));
|
||||
expect(line).not.toMatch(/^0\./);
|
||||
});
|
||||
|
||||
test("a scale the explorer did report still formats", async () => {
|
||||
await fetchOnto([row({ decimals: "6" })]);
|
||||
expect(
|
||||
swapAmountLine(swapData(NOVEL, THOUSAND_AT_SIX, WETH, HALF_WETH)),
|
||||
).toBe("1000.0000");
|
||||
});
|
||||
});
|
||||
|
||||
test("no test in this file performed a network request", () => {
|
||||
expect(global.fetch).not.toHaveBeenCalled();
|
||||
});
|
||||
185
tests/unknownScaleDisplay.test.js
Normal file
185
tests/unknownScaleDisplay.test.js
Normal file
@@ -0,0 +1,185 @@
|
||||
// What the screens that READ a stored token balance do with a holding whose
|
||||
// scale nothing knows.
|
||||
//
|
||||
// https://git.eeqj.de/sneak/AutistMask/issues/349 stopped `fetchTokenBalances()`
|
||||
// fabricating a scale of 18, so a row it cannot state a quantity for is now
|
||||
// stored with `balance: null`. Every reader of that field therefore has two
|
||||
// distinct inputs where it used to have one, and the property that has to hold
|
||||
// at each of them is the same one this codebase keeps losing:
|
||||
//
|
||||
// null (unknown) and 0 (genuinely zero) must produce DIFFERENT output.
|
||||
//
|
||||
// Losing it is what https://git.eeqj.de/sneak/AutistMask/issues/246,
|
||||
// https://git.eeqj.de/sneak/AutistMask/issues/306,
|
||||
// https://git.eeqj.de/sneak/AutistMask/issues/322,
|
||||
// https://git.eeqj.de/sneak/AutistMask/issues/359 and
|
||||
// https://git.eeqj.de/sneak/AutistMask/issues/364 each were. So every case
|
||||
// below asserts the pair, not just that the null branch does something
|
||||
// reasonable: an assertion on the null alone still passes on a build that
|
||||
// renders both as zero, which is precisely the build being guarded against.
|
||||
//
|
||||
// The writer half — that the fetcher stores null rather than 18 — is in
|
||||
// tests/fabricatedDecimals.test.js, and the Send and confirmation screens are
|
||||
// in tests/unknownScaleSend.test.js.
|
||||
|
||||
"use strict";
|
||||
|
||||
// helpers.js reaches for both at module scope through the modules it pulls in.
|
||||
globalThis.chrome = {
|
||||
storage: {
|
||||
local: {
|
||||
get: () => Promise.resolve({}),
|
||||
set: () => Promise.resolve(),
|
||||
},
|
||||
},
|
||||
runtime: { sendMessage: () => {} },
|
||||
};
|
||||
globalThis.document = {
|
||||
getElementById: () => null,
|
||||
createElement: () => ({ style: {}, classList: { toggle() {} } }),
|
||||
body: { prepend: () => {} },
|
||||
addEventListener: () => {},
|
||||
};
|
||||
|
||||
const {
|
||||
balanceLine,
|
||||
balanceLinesForAddress,
|
||||
addressHoldsFunds,
|
||||
} = require("../src/popup/views/helpers");
|
||||
const {
|
||||
prices,
|
||||
clearPrices,
|
||||
getAddressValue,
|
||||
} = require("../src/shared/prices");
|
||||
const { state } = require("../src/shared/state");
|
||||
|
||||
const NOVEL = "0x1111111111111111111111111111111111111111";
|
||||
|
||||
// One stored tokenBalances row. `balance: null` is what balances.js writes for
|
||||
// a holding whose scale nothing knows; "0.0" is a quantity that was actually
|
||||
// established and is zero.
|
||||
function holding(balance) {
|
||||
return {
|
||||
address: NOVEL,
|
||||
symbol: "NOVEL",
|
||||
decimals: balance === null ? null : 18,
|
||||
balance,
|
||||
holders: 50000,
|
||||
};
|
||||
}
|
||||
|
||||
function address(balance) {
|
||||
return {
|
||||
address: "0x" + "a".repeat(40),
|
||||
balance: "0",
|
||||
tokenBalances: [holding(balance)],
|
||||
};
|
||||
}
|
||||
|
||||
// The quantity cell of a rendered row, which is the second of the two spans
|
||||
// inside the fixed-width span.
|
||||
function quantities(html) {
|
||||
return [...html.matchAll(/<span>([^<]*)<\/span>/g)].map((m) => m[1]);
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
clearPrices();
|
||||
state.wallets = [];
|
||||
state.trackedTokens = [];
|
||||
state.activeAddress = null;
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
clearPrices();
|
||||
});
|
||||
|
||||
describe("balanceLine", () => {
|
||||
test("an unknown quantity and a zero one render differently", () => {
|
||||
const unknown = balanceLine("NOVEL", null, null, NOVEL);
|
||||
const zero = balanceLine("NOVEL", 0, null, NOVEL);
|
||||
expect(unknown).not.toBe(zero);
|
||||
expect(quantities(unknown)).toEqual(["NOVEL", "quantity unknown"]);
|
||||
expect(quantities(zero)).toEqual(["NOVEL", "0.0000"]);
|
||||
});
|
||||
|
||||
test("an unknown quantity produces no fiat figure, a zero one does", () => {
|
||||
prices.NOVEL = 3;
|
||||
const unknown = balanceLine("NOVEL", null, 3, NOVEL);
|
||||
const zero = balanceLine("NOVEL", 0, 3, NOVEL);
|
||||
// A price times an unknown quantity is not $0.00: that is the same
|
||||
// claim of "nothing here" the quantity cell just refused to make.
|
||||
expect(unknown).toContain(
|
||||
'<span class="text-right text-muted flex-1"> </span>',
|
||||
);
|
||||
expect(zero).toContain(
|
||||
'<span class="text-right text-muted flex-1">$0.00</span>',
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("balanceLinesForAddress", () => {
|
||||
// The show-zero setting is a statement about zeroes. An unknown quantity
|
||||
// is not one, so hiding the row would assert the zero nobody established
|
||||
// and the holding would vanish from the list entirely.
|
||||
test("hiding zero balances hides the zero row and keeps the unknown one", () => {
|
||||
const unknown = balanceLinesForAddress(address(null), [], false);
|
||||
const zero = balanceLinesForAddress(address("0.0"), [], false);
|
||||
expect(unknown).not.toBe(zero);
|
||||
expect(unknown).toContain("quantity unknown");
|
||||
expect(unknown).toContain("NOVEL");
|
||||
expect(zero).not.toContain("NOVEL");
|
||||
});
|
||||
|
||||
test("showing zero balances still tells the two apart", () => {
|
||||
const unknown = balanceLinesForAddress(address(null), [], true);
|
||||
const zero = balanceLinesForAddress(address("0.0"), [], true);
|
||||
expect(unknown).not.toBe(zero);
|
||||
expect(quantities(unknown)).toEqual([
|
||||
"ETH",
|
||||
"0.0000",
|
||||
"NOVEL",
|
||||
"quantity unknown",
|
||||
]);
|
||||
expect(quantities(zero)).toEqual(["ETH", "0.0000", "NOVEL", "0.0000"]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("addressHoldsFunds", () => {
|
||||
// Read by deleteAddress.js to decide whether removing the address is
|
||||
// warned about. balances.js drops a row of zero base units before any
|
||||
// scale is consulted, so a row that survived with no quantity is holding
|
||||
// something, and the warning must err towards warning.
|
||||
test("an unknown balance holds funds, a zero balance does not", () => {
|
||||
expect(addressHoldsFunds(address(null))).toBe(true);
|
||||
expect(addressHoldsFunds(address("0.0"))).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("getAddressValue", () => {
|
||||
// `usd` is the value of what could be priced and `partial` says it is a
|
||||
// floor rather than the total. An unpriceable holding is exactly what
|
||||
// `partial` exists for; a holding of zero can neither add to the total nor
|
||||
// make it incomplete.
|
||||
test("an unknown balance makes the total partial, a zero balance does not", () => {
|
||||
prices.ETH = 2000;
|
||||
prices.NOVEL = 3;
|
||||
const unknown = getAddressValue(address(null));
|
||||
const zero = getAddressValue(address("0.0"));
|
||||
expect(unknown).not.toEqual(zero);
|
||||
expect(unknown).toEqual({ usd: 0, partial: true });
|
||||
expect(zero).toEqual({ usd: 0, partial: false });
|
||||
});
|
||||
|
||||
test("an unknown balance is not priced as zero of the token", () => {
|
||||
prices.ETH = 2000;
|
||||
prices.NOVEL = 3;
|
||||
// The same row with a real quantity of 10 is worth $30. Neither that
|
||||
// figure nor a confident $0.00 may be stated for the unknown one.
|
||||
expect(getAddressValue(address("10.0"))).toEqual({
|
||||
usd: 30,
|
||||
partial: false,
|
||||
});
|
||||
expect(getAddressValue(address(null)).usd).toBe(0);
|
||||
expect(getAddressValue(address(null)).partial).toBe(true);
|
||||
});
|
||||
});
|
||||
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