fix: show the true token amount on the approval screen, or none at all (closes #306)
decodeCalldata resolved an ERC-20 amount's scale from the 512-entry bundled token list alone and fell back to 18 decimals for everything else. Most tokens are outside that list, including anything the user added by contract address, so a transfer of 5000000000 units of a 6-decimal token - 5,000 tokens - was drawn as "Amount 0.0000". A user who reads zero confirms, and loses the balance or grants the allowance. The new src/shared/approvalAmount.js resolves decimals from the bundled list, then state.trackedTokens, then the decimals the block explorer already reported in addr.tokenBalances, accepting only a real uint8 from any of them and refusing a scale the explorer's own entries disagree about. Where no source knows the scale the amount is not formatted at all: the line reads "5000000000 base units (decimals unknown)". A formatted number computed from a guessed scale is the defect itself, and for a token with fewer decimals than the guess it is wrong in the direction that reads as zero. approve is covered alongside transfer; an unbounded allowance still reads "Unlimited", which needs no scale. The same string is carried to the status screens, so no formatted figure reappears downstream. Verified failing first: restoring only the old lookup fails 6 of the 15 new tests, with the unknown-decimals transfer case receiving exactly "0.0000".
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