fix: resolve approval-screen token decimals, and refuse to format an unknown scale (closes #306)
decodeCalldata consulted only the 512-entry bundled list and defaulted to 18 decimals, so a transfer of 5,000 units of a 6-decimal token rendered "Amount 0.0000" and the user confirmed a drain reading zero. The same understatement applied to approve, where an unbounded allowance also rendered 0.0000. Decimals now resolve from the bundled list, then trackedTokens, then the address's explorer-reported entry, with uint8 validation and a refusal when sources for one contract disagree. When no source knows the scale, no formatUnits call is reached at all: the line renders raw base units with an explicit "decimals unknown" warning, and the same string reaches pendingTxDetails.amount so the status screens carry no formatted figure either. Verified failing first two independent ways: restoring the old `token ? token.decimals : 18` fails 6 of 15 new tests with the unknown case reporting "0.0000"; making the resolver return 18 rather than null on the unknown path fails a different 6, spanning resolver and render levels.
This commit was merged in pull request #321.
This commit is contained in:
208
tests/approvalAmount.test.js
Normal file
208
tests/approvalAmount.test.js
Normal file
@@ -0,0 +1,208 @@
|
||||
// The quantity the dApp approval screen shows for a decoded ERC-20 call.
|
||||
//
|
||||
// The screen's amount line is the only place a user sees how much a page is
|
||||
// asking for, and it is decoded from calldata, which carries base units and
|
||||
// no scale. Issue #306: decodeCalldata read decimals from the bundled token
|
||||
// list alone and fell back to 18, so a `transfer` of 5000000000 units of a
|
||||
// 6-decimal token — 5,000 tokens — was displayed as `0.0000` and confirmed.
|
||||
//
|
||||
// What is asserted here is that the scale is found wherever the wallet
|
||||
// already has it, and that where it is nowhere at all no formatted number is
|
||||
// produced: the amount line has to say base units and say the scale is
|
||||
// unknown, because a wrong quantity that reads as zero is worse than an
|
||||
// unwieldy correct one.
|
||||
|
||||
globalThis.chrome = {
|
||||
storage: { local: { get: async () => ({}), set: async () => {} } },
|
||||
};
|
||||
|
||||
const { Interface } = require("ethers");
|
||||
const { ERC20_ABI } = require("../src/shared/constants");
|
||||
const { state } = require("../src/shared/state");
|
||||
const {
|
||||
resolveTokenDecimals,
|
||||
unknownDecimalsAmount,
|
||||
} = require("../src/shared/approvalAmount");
|
||||
const { decodeCalldata } = require("../src/popup/views/approval");
|
||||
|
||||
const iface = new Interface(ERC20_ABI);
|
||||
|
||||
// Outside the bundled list, as the great majority of ERC-20s are.
|
||||
const NOVEL_TOKEN = "0xE2E0000000000000000000000000000000000E2e";
|
||||
// In the bundled list, at 6 decimals.
|
||||
const USDC = "0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48";
|
||||
const RECIPIENT = "0xC0FfEE0000000000000000000000000000c0fFEe";
|
||||
const SPENDER = "0x1111111111111111111111111111111111111111";
|
||||
|
||||
// 5,000 units of a 6-decimal token, the amount from the issue.
|
||||
const FIVE_THOUSAND_AT_SIX = 5000000000n;
|
||||
const MAX_UINT256 = (1n << 256n) - 1n;
|
||||
|
||||
function transferData(amount) {
|
||||
return iface.encodeFunctionData("transfer", [RECIPIENT, amount]);
|
||||
}
|
||||
|
||||
function approveData(amount) {
|
||||
return iface.encodeFunctionData("approve", [SPENDER, amount]);
|
||||
}
|
||||
|
||||
// The Amount line as the approval screen renders it.
|
||||
function amountLine(data, tokenAddress) {
|
||||
const decoded = decodeCalldata(data, tokenAddress);
|
||||
const detail = decoded.details.find((d) => d.label === "Amount");
|
||||
return detail.value;
|
||||
}
|
||||
|
||||
// A wallet holding `token` with the decimals the block explorer reported,
|
||||
// shaped as balances.js writes it onto state.
|
||||
function walletsHolding(token, decimals) {
|
||||
return [
|
||||
{
|
||||
name: "Wallet 1",
|
||||
addresses: [
|
||||
{
|
||||
address: "0x" + "a".repeat(40),
|
||||
balance: "1.0",
|
||||
tokenBalances: [
|
||||
{
|
||||
address: token,
|
||||
symbol: "NOVEL",
|
||||
decimals,
|
||||
balance: "5000.0",
|
||||
},
|
||||
],
|
||||
},
|
||||
],
|
||||
},
|
||||
];
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
state.trackedTokens = [];
|
||||
state.wallets = [];
|
||||
});
|
||||
|
||||
describe("resolveTokenDecimals", () => {
|
||||
test("prefers the bundled list", () => {
|
||||
state.trackedTokens = [{ address: USDC, symbol: "USDC", decimals: 2 }];
|
||||
expect(resolveTokenDecimals(USDC, state)).toBe(6);
|
||||
});
|
||||
|
||||
test("reads a token the user tracks", () => {
|
||||
state.trackedTokens = [
|
||||
{
|
||||
address: NOVEL_TOKEN.toLowerCase(),
|
||||
symbol: "NOVEL",
|
||||
decimals: 6,
|
||||
},
|
||||
];
|
||||
expect(resolveTokenDecimals(NOVEL_TOKEN, state)).toBe(6);
|
||||
});
|
||||
|
||||
test("reads the decimals the explorer reported", () => {
|
||||
// Blockscout's copy arrives as a string.
|
||||
state.wallets = walletsHolding(NOVEL_TOKEN, "6");
|
||||
expect(resolveTokenDecimals(NOVEL_TOKEN, state)).toBe(6);
|
||||
});
|
||||
|
||||
test("falls past a tracked entry whose decimals are unusable", () => {
|
||||
state.trackedTokens = [
|
||||
{ address: NOVEL_TOKEN, symbol: "NOVEL", decimals: NaN },
|
||||
];
|
||||
state.wallets = walletsHolding(NOVEL_TOKEN, 6);
|
||||
expect(resolveTokenDecimals(NOVEL_TOKEN, state)).toBe(6);
|
||||
});
|
||||
|
||||
test("refuses a scale the explorer's own entries disagree about", () => {
|
||||
const wallets = walletsHolding(NOVEL_TOKEN, 6);
|
||||
wallets[0].addresses.push({
|
||||
address: "0x" + "b".repeat(40),
|
||||
balance: "0.0",
|
||||
tokenBalances: [
|
||||
{ address: NOVEL_TOKEN, symbol: "NOVEL", decimals: 18 },
|
||||
],
|
||||
});
|
||||
state.wallets = wallets;
|
||||
expect(resolveTokenDecimals(NOVEL_TOKEN, state)).toBeNull();
|
||||
});
|
||||
|
||||
test("rejects values that are not a uint8", () => {
|
||||
for (const decimals of [-1, 256, 1.5, true, [], {}, null, "6.0", ""]) {
|
||||
state.trackedTokens = [{ address: NOVEL_TOKEN, decimals }];
|
||||
expect(resolveTokenDecimals(NOVEL_TOKEN, state)).toBeNull();
|
||||
}
|
||||
});
|
||||
|
||||
test("is null when nothing knows the token", () => {
|
||||
expect(resolveTokenDecimals(NOVEL_TOKEN, state)).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe("decodeCalldata amount", () => {
|
||||
test("transfer of a tracked 6-decimal token shows the true quantity", () => {
|
||||
state.trackedTokens = [
|
||||
{ address: NOVEL_TOKEN, symbol: "NOVEL", decimals: 6 },
|
||||
];
|
||||
expect(
|
||||
amountLine(transferData(FIVE_THOUSAND_AT_SIX), NOVEL_TOKEN),
|
||||
).toBe("5000.0000");
|
||||
});
|
||||
|
||||
test("transfer priced off the explorer's decimals shows the true quantity", () => {
|
||||
state.wallets = walletsHolding(NOVEL_TOKEN, "6");
|
||||
expect(
|
||||
amountLine(transferData(FIVE_THOUSAND_AT_SIX), NOVEL_TOKEN),
|
||||
).toBe("5000.0000");
|
||||
});
|
||||
|
||||
test("transfer of an unknown-decimals token shows base units, not a number", () => {
|
||||
const line = amountLine(
|
||||
transferData(FIVE_THOUSAND_AT_SIX),
|
||||
NOVEL_TOKEN,
|
||||
);
|
||||
expect(line).toBe("5000000000 base units (decimals unknown)");
|
||||
expect(line).toBe(unknownDecimalsAmount(FIVE_THOUSAND_AT_SIX));
|
||||
// The defect: any rendering that reads as a token quantity, and above
|
||||
// all one that reads as zero.
|
||||
expect(line).not.toMatch(/0\.0000/);
|
||||
});
|
||||
|
||||
test("approve of a tracked 6-decimal token shows the true quantity", () => {
|
||||
state.trackedTokens = [
|
||||
{ address: NOVEL_TOKEN, symbol: "NOVEL", decimals: 6 },
|
||||
];
|
||||
expect(amountLine(approveData(FIVE_THOUSAND_AT_SIX), NOVEL_TOKEN)).toBe(
|
||||
"5000.0000",
|
||||
);
|
||||
});
|
||||
|
||||
test("approve of an unknown-decimals token shows base units, not a number", () => {
|
||||
const line = amountLine(approveData(FIVE_THOUSAND_AT_SIX), NOVEL_TOKEN);
|
||||
expect(line).toBe("5000000000 base units (decimals unknown)");
|
||||
expect(line).not.toMatch(/0\.0000/);
|
||||
});
|
||||
|
||||
test("an unbounded allowance is still named, with or without a scale", () => {
|
||||
expect(amountLine(approveData(MAX_UINT256), NOVEL_TOKEN)).toBe(
|
||||
"Unlimited",
|
||||
);
|
||||
expect(amountLine(approveData(MAX_UINT256), USDC)).toBe("Unlimited");
|
||||
});
|
||||
|
||||
test("a bundled token keeps its symbol and its scale", () => {
|
||||
expect(amountLine(transferData(FIVE_THOUSAND_AT_SIX), USDC)).toBe(
|
||||
"5000.0000 USDC",
|
||||
);
|
||||
});
|
||||
|
||||
test("the amount carried to the status screens is the same string", () => {
|
||||
const decoded = decodeCalldata(
|
||||
transferData(FIVE_THOUSAND_AT_SIX),
|
||||
NOVEL_TOKEN,
|
||||
);
|
||||
const detail = decoded.details.find((d) => d.label === "Amount");
|
||||
expect(detail.rawValue).toBe(
|
||||
"5000000000 base units (decimals unknown)",
|
||||
);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user