fix: sign the ERC-20 amount the confirmation screen displayed (closes #305)
All checks were successful
check / check (push) Successful in 27s
e2e / e2e-chrome (push) Successful in 1m10s
e2e / e2e-firefox (push) Successful in 22s

The send screen was built from the indexer's decimals while the transfer was
encoded from the contract's decimals() read at signing time, with nothing
comparing them. A token whose scales disagree moved 10^12 times the approved
amount.

The displayed scale is now carried on pendingTx from the same tokenBalances
entry the amount, balance and symbol were rendered from, and both encode sites
use it. transferAmount.js refuses rather than falling back when the two scales
disagree or either is unusable.

Adds the first end-to-end coverage of the popup's own Send -> ConfirmTx ->
Sign & Send path; #btn-confirm-send had never been clicked by any test.
This commit was merged in pull request #314.
This commit is contained in:
2026-08-20 12:31:28 +02:00
parent ff3387d8cf
commit 2f80a9bdb4
8 changed files with 570 additions and 16 deletions

View File

@@ -0,0 +1,128 @@
// The scale an ERC-20 transfer from the wallet's own Send screen is encoded
// with (issue #305). The screen renders from the block explorer's cached
// decimals; the transfer used to be encoded from decimals() read off the
// contract at signing time, with nothing comparing the two, so a token whose
// on-chain scale differed signed an amount that was never displayed.
const { parseUnits } = require("ethers");
const {
displayedDecimals,
transferAmountUnits,
MAX_DECIMALS,
UNKNOWN_DISPLAYED_DECIMALS_MESSAGE,
UNREADABLE_CONTRACT_DECIMALS_MESSAGE,
} = require("../src/shared/transferAmount");
describe("displayedDecimals", () => {
test("accepts what the explorer and the contract each answer with", () => {
// A string is what fetchTokenBalances() parses out of Blockscout, a
// number is what it stores, and a bigint is what ethers hands back
// from a uint8 return.
expect(displayedDecimals("6")).toBe(6);
expect(displayedDecimals(6)).toBe(6);
expect(displayedDecimals(6n)).toBe(6);
expect(displayedDecimals(0)).toBe(0);
expect(displayedDecimals(MAX_DECIMALS)).toBe(MAX_DECIMALS);
});
test("refuses anything that is not a uint8", () => {
for (const bad of [
null,
undefined,
"",
"eighteen",
NaN,
6.5,
-1,
MAX_DECIMALS + 1,
true,
{},
[],
]) {
expect(() => displayedDecimals(bad)).toThrow(
UNKNOWN_DISPLAYED_DECIMALS_MESSAGE,
);
}
});
});
describe("transferAmountUnits", () => {
test("encodes with the displayed scale when the contract agrees", () => {
expect(transferAmountUnits("0.25", 6, 6n)).toBe(parseUnits("0.25", 6));
expect(transferAmountUnits("0.25", "6", 6n)).toBe(
parseUnits("0.25", 6),
);
expect(transferAmountUnits("1.5", 18, 18n)).toBe(parseUnits("1.5", 18));
});
// The reproduction on the issue: 0.25 of a token displayed at 6 decimals,
// signed against a contract answering 18, moves 10^12 times the amount
// that was approved.
test("refuses the reproduction rather than signing either amount", () => {
expect(() => transferAmountUnits("0.25", 6, 18n)).toThrow(
/contract reports 18 decimal places, but the amount was displayed using 6/,
);
});
test("refuses a disagreement in the other direction too", () => {
expect(() => transferAmountUnits("0.25", 18, 6n)).toThrow(
/contract reports 6 decimal places, but the amount was displayed using 18/,
);
});
test("never returns the amount at either scale on a disagreement", () => {
// The point of the refusal: both candidate encodings exist, and the
// wallet must produce neither.
let thrown = null;
try {
transferAmountUnits("0.25", 6, 18n);
} catch (e) {
thrown = e;
}
expect(thrown).toBeInstanceOf(Error);
expect(thrown.message).toMatch(/was not sent/);
});
test("refuses when the screen's scale is unknown", () => {
expect(() => transferAmountUnits("0.25", null, 6n)).toThrow(
UNKNOWN_DISPLAYED_DECIMALS_MESSAGE,
);
expect(() => transferAmountUnits("0.25", undefined, 6n)).toThrow(
UNKNOWN_DISPLAYED_DECIMALS_MESSAGE,
);
});
test("refuses when the contract's answer is not a uint8", () => {
for (const bad of [null, undefined, "", "eighteen", 6.5, -1, 256]) {
expect(() => transferAmountUnits("0.25", 6, bad)).toThrow(
UNREADABLE_CONTRACT_DECIMALS_MESSAGE,
);
}
});
test("rejects an amount finer than the token's scale", () => {
// parseUnits' own refusal, reached only once the scales agree: a
// fractional base unit cannot be sent and must not be truncated.
expect(() => transferAmountUnits("0.0000001", 6, 6n)).toThrow();
});
test("every refusal is a full sentence", () => {
const messages = [];
for (const args of [
["0.25", 6, 18n],
["0.25", null, 6n],
["0.25", 6, "eighteen"],
]) {
try {
transferAmountUnits(...args);
} catch (e) {
messages.push(e.message);
}
}
expect(messages).toHaveLength(3);
for (const m of messages) {
expect(m).toMatch(/^[A-Z]/);
expect(m).toMatch(/\.$/);
}
});
});