The wallet's own Send screen renders the amount, the balance and the symbol from the block explorer's cached decimals, but confirmTx encoded the transfer from decimals() read off the contract at signing time and nothing compared the two. A token whose on-chain scale disagrees with the cached one -- an upgradeable or proxy token, a caller-dependent one, a stale or wrong explorer entry, a compromised Blockscout -- therefore signed an amount that was never displayed, off by a power of ten for every decimal place of disagreement. The reproduction on the issue approves 0.25 and signs 250,000,000,000. The scale is now carried forward on the pending transaction, taken from the same tokenBalances entry the screen's own numbers come from, and the contract's decimals() is read at signing time only to be compared with it. A disagreement is a refusal that names both numbers, never a preference for either: both candidate transfers move an amount nobody approved. New src/shared/transferAmount.js holds that check, as the confirmTx counterpart to approvalVerify.js, and takes the same stance on an absent or unusable value -- a quantity that cannot be compared with what was displayed has not been checked. The gas estimate encodes from the same carried value and no longer reads decimals() at all, so the estimate is for the transfer that would be signed. Nothing in the e2e suite had ever clicked #btn-confirm-send, so the popup's own Send -> ConfirmTx -> Sign & Send -> WaitTx path had no coverage at all, which is how this shipped. It is now driven end to end to a broadcast, with the transfer() amount hand-decoded out of the raw signed bytes and asserted against the amount read off the confirmation screen, plus a case where the fixture's decimals() starts answering 18 after the screen was built and nothing reaches eth_sendRawTransaction. The fixture gains that override and a receipt, so the wait screen resolves to the success view instead of polling for the rest of the run.
129 lines
4.6 KiB
JavaScript
129 lines
4.6 KiB
JavaScript
// 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(/\.$/);
|
|
}
|
|
});
|
|
});
|