Compare commits
1 Commits
5c4a671d4a
...
issue-305-
| Author | SHA1 | Date | |
|---|---|---|---|
| 8fadc4107f |
@@ -992,6 +992,13 @@ on ConfirmTx, DeleteWallet, ApproveTx and ApproveSign.
|
||||
- **Transitions**:
|
||||
- "Sign & Send" (correct password) → broadcast tx → **WaitTx**
|
||||
- "Sign & Send" (correct password) → broadcast fails → **ErrorTx**
|
||||
- "Sign & Send" on an ERC-20 whose contract answers `decimals()` with a
|
||||
different number than the amount above was displayed with → nothing is
|
||||
signed → **ErrorTx** naming both numbers. The transfer is encoded from the
|
||||
decimals the screen rendered, carried forward on the pending transaction;
|
||||
the contract's own answer is read at signing time only to be compared with
|
||||
it, and a disagreement is a refusal rather than a preference for either
|
||||
value (`src/shared/transferAmount.js`)
|
||||
- "Sign & Send" (wrong password) → "Wrong password." on the password error
|
||||
line, no screen change
|
||||
- "Back" → **Send**
|
||||
|
||||
21
TODO.md
21
TODO.md
@@ -44,6 +44,27 @@ but the review is broader than any of them.
|
||||
|
||||
# Completed Steps
|
||||
|
||||
- 2026-08-20: The wallet's own ERC-20 send signs the amount it displayed
|
||||
([#305](https://git.eeqj.de/sneak/AutistMask/issues/305)). The confirmation
|
||||
screen renders from the block explorer's cached decimals; the transfer was
|
||||
encoded from `decimals()` read off the contract at signing time, and nothing
|
||||
compared the two, so a token whose on-chain scale disagreed — an upgradeable
|
||||
or proxy token, a stale explorer entry, a compromised Blockscout — signed an
|
||||
amount that was never on screen, off by a power of ten per decimal place of
|
||||
disagreement. The scale is now carried forward on the pending transaction from
|
||||
the same balance entry the screen's amount, balance and symbol come from, and
|
||||
the contract's answer is read at signing time only to be compared with it: a
|
||||
disagreement is a refusal naming both numbers, never a preference for either
|
||||
(`src/shared/transferAmount.js`, the `confirmTx` counterpart to
|
||||
`approvalVerify.js`). The gas estimate encodes from the same carried value and
|
||||
no longer reads `decimals()` at all. Nothing in the e2e suite had ever clicked
|
||||
`#btn-confirm-send`, which is how this shipped: the popup's own Send →
|
||||
ConfirmTx → Sign & Send → WaitTx path now runs end to end to a broadcast, with
|
||||
the `transfer()` amount decoded out of the raw signed bytes and asserted
|
||||
against what the screen displayed, and a companion case where the contract
|
||||
starts answering a different scale after the screen was built and nothing
|
||||
reaches the RPC. Reverting only the signing-side comparison turns that second
|
||||
case red and leaves the other 53 green.
|
||||
- 2026-08-17: The Settings screen is driven in a browser, and every element id
|
||||
the popup looks up is checked statically. Nothing exercised Settings in the
|
||||
e2e suite, and jest runs with no DOM, so the densest run of `$("...")` lookups
|
||||
|
||||
@@ -25,6 +25,10 @@ const {
|
||||
getFullWarnings,
|
||||
} = require("../../shared/addressWarnings");
|
||||
const { ERC20_ABI, isBurnAddress } = require("../../shared/constants");
|
||||
const {
|
||||
displayedDecimals,
|
||||
transferAmountUnits,
|
||||
} = require("../../shared/transferAmount");
|
||||
const {
|
||||
CODES,
|
||||
FEE_PENDING,
|
||||
@@ -302,8 +306,17 @@ async function estimateGas(txInfo) {
|
||||
});
|
||||
} else {
|
||||
const contract = new Contract(txInfo.token, ERC20_ABI, provider);
|
||||
const decimals = await contract.decimals();
|
||||
const amount = parseUnits(txInfo.amount, decimals);
|
||||
// The scale the screen is rendering with, not the contract's own
|
||||
// answer: the estimate has to be for the transfer that would be
|
||||
// signed, and that one is encoded from what was displayed. See
|
||||
// transferAmount.js. A pending transaction that carries no usable
|
||||
// scale throws here, which reports the fee as unknown and leaves
|
||||
// Send blocked — an amount that cannot be checked against the
|
||||
// screen is never estimated for, let alone sent.
|
||||
const amount = parseUnits(
|
||||
txInfo.amount,
|
||||
displayedDecimals(txInfo.tokenDecimals),
|
||||
);
|
||||
gasLimit = await contract.transfer.estimateGas(txInfo.to, amount, {
|
||||
from: txInfo.from,
|
||||
});
|
||||
@@ -445,8 +458,16 @@ function init(_ctx) {
|
||||
ERC20_ABI,
|
||||
connectedSigner,
|
||||
);
|
||||
const decimals = await contract.decimals();
|
||||
const amount = parseUnits(pendingTx.amount, decimals);
|
||||
// The contract's decimals() is read to be COMPARED with the
|
||||
// scale the screen rendered this amount at, not to encode with:
|
||||
// encoding from it signs whatever the contract answers now,
|
||||
// which is not what the user read. A disagreement throws and is
|
||||
// reported on the error screen. See transferAmount.js.
|
||||
const amount = transferAmountUnits(
|
||||
pendingTx.amount,
|
||||
pendingTx.tokenDecimals,
|
||||
await contract.decimals(),
|
||||
);
|
||||
tx = await contract.transfer(pendingTx.to, amount);
|
||||
}
|
||||
|
||||
|
||||
@@ -220,6 +220,11 @@ function init(_ctx) {
|
||||
|
||||
let tokenSymbol = null;
|
||||
let tokenBalance = null;
|
||||
// The scale the amount and the balance below are rendered at, carried
|
||||
// forward so the transfer is encoded with the number the user read
|
||||
// rather than with whatever the contract answers at signing time. See
|
||||
// src/shared/transferAmount.js.
|
||||
let tokenDecimals = null;
|
||||
if (token !== "ETH") {
|
||||
const tb = (addr.tokenBalances || []).find(
|
||||
(t) => t.address.toLowerCase() === token.toLowerCase(),
|
||||
@@ -230,6 +235,7 @@ function init(_ctx) {
|
||||
state.trackedTokens,
|
||||
);
|
||||
tokenBalance = tb ? tb.balance || "0" : "0";
|
||||
tokenDecimals = tb ? tb.decimals : null;
|
||||
}
|
||||
|
||||
ctx.showConfirmTx({
|
||||
@@ -241,6 +247,7 @@ function init(_ctx) {
|
||||
balance: addr.balance,
|
||||
tokenSymbol: tokenSymbol,
|
||||
tokenBalance: tokenBalance,
|
||||
tokenDecimals: tokenDecimals,
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
116
src/shared/transferAmount.js
Normal file
116
src/shared/transferAmount.js
Normal file
@@ -0,0 +1,116 @@
|
||||
// The base-unit amount an ERC-20 transfer from the wallet's own Send screen is
|
||||
// encoded with.
|
||||
//
|
||||
// A token amount is a decimal string plus a scale, and the two come from
|
||||
// different places. The confirmation screen renders the amount, the balance and
|
||||
// the symbol from the block explorer's cached metadata (see
|
||||
// fetchTokenBalances() in balances.js); the transfer used to be encoded from
|
||||
// decimals() read off the contract at signing time, and nothing compared the
|
||||
// two. A token whose on-chain scale differs from the cached one — an
|
||||
// upgradeable or proxy token, a caller-dependent one, a stale or wrong explorer
|
||||
// entry — therefore signed an amount that was never displayed, off by a power
|
||||
// of ten for every decimal place of disagreement.
|
||||
//
|
||||
// So the scale used to encode is the scale the screen rendered with, carried
|
||||
// forward on the pending transaction, and the contract's own answer is read
|
||||
// only to be compared with it. A disagreement is a refusal, never a preference
|
||||
// for either number: the wallet cannot tell which of the two the user meant,
|
||||
// and both candidate transfers move an amount nobody approved.
|
||||
//
|
||||
// This is the confirmTx counterpart to approvalVerify.js, which does the same
|
||||
// job for the dApp approval path, and it takes the same stance: a quantity that
|
||||
// cannot be compared with what was displayed has not been checked, so an absent
|
||||
// or unusable value is refused rather than filled in.
|
||||
//
|
||||
// Every message here is shown to the user on the transaction error screen, so
|
||||
// each is a full sentence and names the numbers it is refusing over.
|
||||
|
||||
const { parseUnits } = require("ethers");
|
||||
|
||||
// Solidity's decimals() returns a uint8, so anything outside that range is not
|
||||
// an answer this wallet can use.
|
||||
const MAX_DECIMALS = 255;
|
||||
|
||||
const UNKNOWN_DISPLAYED_DECIMALS_MESSAGE =
|
||||
"The transfer was not sent, because the number of decimal places this" +
|
||||
" amount was shown with is unknown, so the amount that would be signed" +
|
||||
" cannot be shown to be the amount that was displayed.";
|
||||
|
||||
const UNREADABLE_CONTRACT_DECIMALS_MESSAGE =
|
||||
"The transfer was not sent, because the token contract did not report a" +
|
||||
" usable number of decimal places, so the amount that would be signed" +
|
||||
" cannot be checked against the amount that was displayed.";
|
||||
|
||||
function mismatchMessage(displayed, onChain) {
|
||||
return (
|
||||
"The transfer was not sent. The token contract reports " +
|
||||
onChain +
|
||||
" decimal places, but the amount was displayed using " +
|
||||
displayed +
|
||||
", so signing it would move a different amount than the one shown." +
|
||||
" Reopen the wallet to reload this token's details and try again."
|
||||
);
|
||||
}
|
||||
|
||||
// A decimals value from either source as a number, or null if it is not one.
|
||||
// decimals() comes back from ethers as a bigint and the explorer's copy arrives
|
||||
// as a string, so both of those are accepted alongside a plain number; anything
|
||||
// fractional, negative, out of uint8 range, or of any other type at all is not.
|
||||
//
|
||||
// The types are enumerated rather than coerced because Number() is far too
|
||||
// willing: Number([]) is 0 and Number(true) is 1, so a coercing check would
|
||||
// admit an empty array as a scale of zero and encode a whole-token transfer
|
||||
// against it.
|
||||
function toDecimals(value) {
|
||||
let n;
|
||||
if (typeof value === "number") {
|
||||
n = value;
|
||||
} else if (typeof value === "bigint") {
|
||||
if (value < 0n || value > BigInt(MAX_DECIMALS)) return null;
|
||||
n = Number(value);
|
||||
} else if (typeof value === "string") {
|
||||
if (!/^[0-9]+$/.test(value)) return null;
|
||||
n = Number(value);
|
||||
} else {
|
||||
return null;
|
||||
}
|
||||
if (!Number.isInteger(n) || n < 0 || n > MAX_DECIMALS) return null;
|
||||
return n;
|
||||
}
|
||||
|
||||
// The decimals the confirmation screen rendered an amount with, as a number.
|
||||
// Throws when the pending transaction does not carry a usable one — which is
|
||||
// also what keeps the gas estimate from quietly estimating a different transfer
|
||||
// than the one that would be signed.
|
||||
function displayedDecimals(value) {
|
||||
const displayed = toDecimals(value);
|
||||
if (displayed === null) {
|
||||
throw new Error(UNKNOWN_DISPLAYED_DECIMALS_MESSAGE);
|
||||
}
|
||||
return displayed;
|
||||
}
|
||||
|
||||
// The transfer amount in the token's base units, or a throw. `amount` is the
|
||||
// decimal string the user typed and the screen displayed, `displayed` is the
|
||||
// scale it was displayed at, and `onChain` is what the contract's decimals()
|
||||
// answered at signing time. The two scales must agree.
|
||||
function transferAmountUnits(amount, displayed, onChain) {
|
||||
const shown = displayedDecimals(displayed);
|
||||
const reported = toDecimals(onChain);
|
||||
if (reported === null) {
|
||||
throw new Error(UNREADABLE_CONTRACT_DECIMALS_MESSAGE);
|
||||
}
|
||||
if (reported !== shown) {
|
||||
throw new Error(mismatchMessage(shown, reported));
|
||||
}
|
||||
return parseUnits(String(amount), shown);
|
||||
}
|
||||
|
||||
module.exports = {
|
||||
displayedDecimals,
|
||||
transferAmountUnits,
|
||||
mismatchMessage,
|
||||
MAX_DECIMALS,
|
||||
UNKNOWN_DISPLAYED_DECIMALS_MESSAGE,
|
||||
UNREADABLE_CONTRACT_DECIMALS_MESSAGE,
|
||||
};
|
||||
@@ -221,11 +221,6 @@ const RPC_RESULTS = {
|
||||
eth_estimateGas: hex(GAS_LIMIT),
|
||||
eth_getTransactionCount: "0x0",
|
||||
eth_maxPriorityFeePerGas: hex(PRIORITY_FEE_WEI),
|
||||
// "not mined yet", which is what a node answers for a transaction it has
|
||||
// only just accepted. The wait screen the dApp transaction approval hands
|
||||
// off to polls this every 10 seconds; leaving it unstubbed would report
|
||||
// the poll as escaping traffic the moment a test outlived one tick.
|
||||
eth_getTransactionReceipt: null,
|
||||
};
|
||||
|
||||
// The "latest" block, which ethers' getFeeData() reads baseFeePerGas from
|
||||
@@ -253,17 +248,22 @@ function latestBlock() {
|
||||
const SELECTOR_DECIMALS = "0x313ce567";
|
||||
|
||||
// Every eth_call still answers with a zero word except decimals() on the
|
||||
// stub token. ethers reads that before it can encode an ERC-20 transfer,
|
||||
// and a zero there makes parseUnits() reject any fractional amount — so the
|
||||
// ERC-20 confirmation path would fail its gas estimate for a reason that
|
||||
// has nothing to do with what is being tested.
|
||||
function ethCallResult(req) {
|
||||
// stub token, which the wallet reads back at signing time to compare with
|
||||
// the scale the confirmation screen rendered (issue #305).
|
||||
//
|
||||
// opts.tokenDecimalsOverride is the lying contract: set it and decimals()
|
||||
// answers something other than the value this same fixture reports through
|
||||
// Blockscout, which is exactly the disagreement the wallet must refuse to
|
||||
// sign over. It is read at request time, so a test flips it on the options
|
||||
// object the route was registered with — after the confirmation screen has
|
||||
// been built — without re-registering anything.
|
||||
function ethCallResult(req, opts) {
|
||||
const call = Array.isArray(req.params) ? req.params[0] : null;
|
||||
if (!call || typeof call !== "object") return ZERO_WORD;
|
||||
const data = String(call.data || call.input || "").toLowerCase();
|
||||
const to = String(call.to || "").toLowerCase();
|
||||
if (data.startsWith(SELECTOR_DECIMALS) && to === STUB_TOKEN.address) {
|
||||
return word(STUB_TOKEN.decimals);
|
||||
return word(opts.tokenDecimalsOverride || STUB_TOKEN.decimals);
|
||||
}
|
||||
return ZERO_WORD;
|
||||
}
|
||||
@@ -346,6 +346,38 @@ function transactionDetails(hash) {
|
||||
};
|
||||
}
|
||||
|
||||
// The receipt for a transaction this run broadcast.
|
||||
//
|
||||
// eth_getTransactionReceipt otherwise answers null — "not mined yet", which is
|
||||
// what a node says about a transaction it has only just accepted, and what the
|
||||
// wait screen has to keep polling through. opts.seedReceipt confirms it
|
||||
// instead, which is how a test that drives the popup's own send to a broadcast
|
||||
// gets off the wait screen: the wait resolves to the success view, which has a
|
||||
// Done button, rather than polling for a receipt for the rest of the suite.
|
||||
//
|
||||
// Every field ethers' receipt formatter requires is present. A receipt it
|
||||
// cannot parse throws inside the poll, which the wallet reports through
|
||||
// log.errorf — i.e. console.error — and the harness fails the run on, so a
|
||||
// half-populated fixture here would surface as an unrelated-looking failure.
|
||||
function transactionReceipt(hash) {
|
||||
return {
|
||||
transactionHash: hash,
|
||||
transactionIndex: "0x0",
|
||||
blockHash: "0x" + "33".repeat(32),
|
||||
blockNumber: hex(STUB_BLOCK_NUMBER),
|
||||
from: STUB_COUNTERPARTY,
|
||||
to: STUB_TOKEN.address,
|
||||
cumulativeGasUsed: hex(GAS_LIMIT),
|
||||
gasUsed: hex(GAS_LIMIT),
|
||||
effectiveGasPrice: hex(GAS_PRICE_WEI),
|
||||
contractAddress: null,
|
||||
logs: [],
|
||||
logsBloom: "0x" + "00".repeat(256),
|
||||
status: "0x1",
|
||||
type: "0x2",
|
||||
};
|
||||
}
|
||||
|
||||
function jsonResponse(route, body) {
|
||||
return route.fulfill({
|
||||
status: 200,
|
||||
@@ -408,7 +440,13 @@ function rpcReply(req, opts, report) {
|
||||
});
|
||||
}
|
||||
if (req.method === "eth_call") {
|
||||
return Object.assign(envelope, { result: ethCallResult(req) });
|
||||
return Object.assign(envelope, { result: ethCallResult(req, opts) });
|
||||
}
|
||||
if (req.method === "eth_getTransactionReceipt") {
|
||||
const hash = Array.isArray(req.params) ? req.params[0] : null;
|
||||
return Object.assign(envelope, {
|
||||
result: opts.seedReceipt && hash ? transactionReceipt(hash) : null,
|
||||
});
|
||||
}
|
||||
if (req.method === "eth_getBlockByNumber") {
|
||||
return Object.assign(envelope, { result: latestBlock() });
|
||||
@@ -555,6 +593,11 @@ function traceEnabled(raw) {
|
||||
* eth_estimateGas until this is cleared again.
|
||||
* @param {string[]} [opts.broadcastTransactions] every raw signed
|
||||
* transaction handed to eth_sendRawTransaction, appended in order.
|
||||
* @param {string} [opts.tokenDecimalsOverride] what decimals() answers for
|
||||
* the stub token, in place of the value Blockscout reports for it. This is
|
||||
* the token that lies about its scale; read at request time.
|
||||
* @param {boolean} [opts.seedReceipt] answer eth_getTransactionReceipt with a
|
||||
* confirmed receipt instead of null, so a wait screen resolves.
|
||||
* @returns {Promise<{waitForServiceWorkerTraffic: (ms: number) =>
|
||||
* Promise<string|null>}>}
|
||||
*/
|
||||
|
||||
211
tests/e2e/run.js
211
tests/e2e/run.js
@@ -12,10 +12,12 @@
|
||||
const {
|
||||
Transaction,
|
||||
formatEther,
|
||||
formatUnits,
|
||||
getAddress,
|
||||
getBytes,
|
||||
hexlify,
|
||||
parseEther,
|
||||
parseUnits,
|
||||
toQuantity,
|
||||
toUtf8Bytes,
|
||||
verifyMessage,
|
||||
@@ -1964,6 +1966,208 @@ test("ConfirmTx reports a failed ERC-20 estimate as unknown, not as a fee proble
|
||||
);
|
||||
});
|
||||
|
||||
// ------------------------- the popup's own send, end to end (#305)
|
||||
//
|
||||
// Everything above this point stops at the confirmation screen. Nothing in
|
||||
// the suite had ever clicked #btn-confirm-send, so the wallet's own Send ->
|
||||
// ConfirmTx -> Sign & Send -> WaitTx path had no coverage at all, and issue
|
||||
// #305 shipped through the gap: the screen was rendered from the explorer's
|
||||
// decimals while the transfer was encoded from decimals() read off the
|
||||
// contract at signing time, with nothing comparing the two.
|
||||
//
|
||||
// These two tests drive that path to a broadcast and read the amount out of
|
||||
// the bytes the node was handed. The first asserts those bytes against what
|
||||
// the screen displayed; the second makes the contract answer a different
|
||||
// scale after the screen was built, and requires that nothing is broadcast.
|
||||
|
||||
// keccak("transfer(address,uint256)")[0:4].
|
||||
const SELECTOR_TRANSFER = "0xa9059cbb";
|
||||
|
||||
// What decimals() starts answering once the confirmation screen has been
|
||||
// built. The explorer reports 6 for the same token, so a wallet that encodes
|
||||
// from the contract signs 10^12 times the amount it displayed.
|
||||
const LYING_DECIMALS = "18";
|
||||
|
||||
const TOKEN_DECIMALS = Number(STUB_TOKEN.decimals);
|
||||
|
||||
// The transfer() call inside a raw signed transaction, hand-decoded.
|
||||
//
|
||||
// Deliberately not run through an ethers Interface built from the
|
||||
// extension's own ABI: what is under assertion is the bytes that reached the
|
||||
// node, and the fewer assumptions the wallet and the assertion share, the
|
||||
// less room there is for both to be wrong in the same direction.
|
||||
function decodeTransfer(rawSignedTx) {
|
||||
const signed = Transaction.from(rawSignedTx);
|
||||
const data = signed.data.toLowerCase();
|
||||
assert(
|
||||
data.startsWith(SELECTOR_TRANSFER) && data.length === 10 + 128,
|
||||
"the broadcast transaction is not an ERC-20 transfer() call: " + data,
|
||||
);
|
||||
return {
|
||||
signed,
|
||||
recipient: getAddress("0x" + data.slice(34, 74)),
|
||||
rawAmount: BigInt("0x" + data.slice(74)),
|
||||
};
|
||||
}
|
||||
|
||||
// The amount the confirmation screen is showing, verbatim.
|
||||
async function shownAmount(page) {
|
||||
return (await page.locator("#confirm-amount").innerText()).trim();
|
||||
}
|
||||
|
||||
async function fillPasswordAndSend(page) {
|
||||
await page.fill("#confirm-tx-password", PASSWORD);
|
||||
await page.click("#btn-confirm-send");
|
||||
}
|
||||
|
||||
async function goToTokenConfirm(env) {
|
||||
await goToConfirm(env.page, {
|
||||
token: STUB_TOKEN.address,
|
||||
balance: TOKEN_BALANCE_TEXT + " " + STUB_TOKEN.symbol,
|
||||
amount: TOKEN_AMOUNT,
|
||||
});
|
||||
await waitForEstimate(env.page);
|
||||
const shown = await shownAmount(env.page);
|
||||
assert(
|
||||
shown === TOKEN_AMOUNT + " " + STUB_TOKEN.symbol,
|
||||
"the confirmation screen is not showing the amount that was entered: " +
|
||||
JSON.stringify(shown),
|
||||
);
|
||||
return shown;
|
||||
}
|
||||
|
||||
test("the popup's own ERC-20 send broadcasts the amount it displayed (#305)", async (env) => {
|
||||
// The previous test left the ETH balance at the fee-only fixture, which
|
||||
// blocks sending outright; this one has to be able to press Send.
|
||||
env.routeOpts.ethBalanceWei = toHexWei(FUNDED_ETH_WEI);
|
||||
await settleOnMain(env, { ethWei: FUNDED_ETH_WEI, expectToken: true });
|
||||
const shown = await goToTokenConfirm(env);
|
||||
|
||||
const before = env.routeOpts.broadcastTransactions.length;
|
||||
// Confirm the transaction once it is broadcast, so the wait screen
|
||||
// resolves to the success view instead of polling for the rest of the run.
|
||||
env.routeOpts.seedReceipt = true;
|
||||
await fillPasswordAndSend(env.page);
|
||||
await visible(env.page, "#view-wait-tx", 60000);
|
||||
|
||||
const broadcast = env.routeOpts.broadcastTransactions;
|
||||
assert(
|
||||
broadcast.length === before + 1,
|
||||
"expected exactly one raw transaction to reach the RPC, got " +
|
||||
(broadcast.length - before),
|
||||
);
|
||||
const { signed, recipient, rawAmount } = decodeTransfer(
|
||||
broadcast[broadcast.length - 1],
|
||||
);
|
||||
|
||||
// The measurement, printed on every run: the amount the user read, and
|
||||
// what the signed bytes mean at each of the two candidate scales. Under
|
||||
// the defect these three lines disagree.
|
||||
console.log(
|
||||
"# erc-20 send artifact: displayed=" +
|
||||
JSON.stringify(shown) +
|
||||
" rawAmount=" +
|
||||
rawAmount +
|
||||
" asIf" +
|
||||
TOKEN_DECIMALS +
|
||||
"Decimals=" +
|
||||
formatUnits(rawAmount, TOKEN_DECIMALS) +
|
||||
" asIf" +
|
||||
LYING_DECIMALS +
|
||||
"Decimals=" +
|
||||
formatUnits(rawAmount, Number(LYING_DECIMALS)),
|
||||
);
|
||||
|
||||
assert(
|
||||
getAddress(signed.to) === getAddress(STUB_TOKEN.address),
|
||||
"the broadcast transaction does not call the token contract: " +
|
||||
signed.to,
|
||||
);
|
||||
assert(
|
||||
recipient === getAddress(STUB_COUNTERPARTY),
|
||||
"the broadcast transfer goes to " + recipient,
|
||||
);
|
||||
// What the whole issue turns on: the signed amount, read back at the
|
||||
// scale the SCREEN rendered with, is the number the screen rendered.
|
||||
const wanted = parseUnits(shown.split(" ")[0], TOKEN_DECIMALS);
|
||||
assert(
|
||||
rawAmount === wanted,
|
||||
"the broadcast transfer moves " +
|
||||
rawAmount +
|
||||
" base units, which is " +
|
||||
formatUnits(rawAmount, TOKEN_DECIMALS) +
|
||||
" " +
|
||||
STUB_TOKEN.symbol +
|
||||
" at the scale the confirmation screen displayed — but the screen" +
|
||||
" displayed " +
|
||||
JSON.stringify(shown) +
|
||||
", i.e. " +
|
||||
wanted +
|
||||
" base units (#305)",
|
||||
);
|
||||
|
||||
const summary = (
|
||||
await env.page.locator("#wait-tx-summary").innerText()
|
||||
).trim();
|
||||
assert(
|
||||
summary === shown,
|
||||
"the wait screen summarises the send as " +
|
||||
JSON.stringify(summary) +
|
||||
", not as the approved " +
|
||||
JSON.stringify(shown),
|
||||
);
|
||||
|
||||
await visible(env.page, "#view-success-tx", 60000);
|
||||
await env.page.click("#btn-success-tx-done");
|
||||
await visible(env.page, "#view-address");
|
||||
env.routeOpts.seedReceipt = false;
|
||||
});
|
||||
|
||||
test("a token that lies about decimals() at signing time broadcasts nothing (#305)", async (env) => {
|
||||
const shown = await goToTokenConfirm(env);
|
||||
|
||||
// Only now, with the screen already built and its estimate already taken
|
||||
// at the explorer's scale, does the contract start answering differently.
|
||||
// This is the whole shape of the defect: a value read at signing time that
|
||||
// nothing on screen was ever derived from.
|
||||
env.routeOpts.tokenDecimalsOverride = LYING_DECIMALS;
|
||||
const before = env.routeOpts.broadcastTransactions.length;
|
||||
await fillPasswordAndSend(env.page);
|
||||
await visible(env.page, "#view-error-tx", 60000);
|
||||
env.routeOpts.tokenDecimalsOverride = null;
|
||||
|
||||
assert(
|
||||
env.routeOpts.broadcastTransactions.length === before,
|
||||
"a transfer encoded against a contract that contradicts the " +
|
||||
"confirmation screen still reached the RPC (#305)",
|
||||
);
|
||||
|
||||
const message = (
|
||||
await env.page.locator("#error-tx-message").innerText()
|
||||
).trim();
|
||||
console.log(
|
||||
"# erc-20 decimals refusal: displayed=" +
|
||||
JSON.stringify(shown) +
|
||||
" contract=" +
|
||||
LYING_DECIMALS +
|
||||
" message=" +
|
||||
JSON.stringify(message),
|
||||
);
|
||||
assert(
|
||||
message.includes("reports " + LYING_DECIMALS + " decimal places") &&
|
||||
message.includes("displayed using " + STUB_TOKEN.decimals),
|
||||
"the refusal does not name both scales it is refusing over: " +
|
||||
JSON.stringify(message),
|
||||
);
|
||||
assert(
|
||||
/^[A-Z].*\.$/s.test(message),
|
||||
"the refusal is not a full sentence: " + JSON.stringify(message),
|
||||
);
|
||||
|
||||
await env.page.click("#btn-error-tx-done");
|
||||
await visible(env.page, "#view-address");
|
||||
});
|
||||
|
||||
// ------------------------------------------- dApp round trips (#183)
|
||||
//
|
||||
// The seam. Everything above drives the popup on its own; this section is
|
||||
@@ -3094,6 +3298,13 @@ async function main() {
|
||||
ethBalanceWei: null,
|
||||
failGasEstimate: false,
|
||||
holdGasEstimate: false,
|
||||
// What decimals() answers for the stub token, when it is to answer
|
||||
// something other than the value the same fixture reports through
|
||||
// Blockscout. The token that lies about its scale (#305).
|
||||
tokenDecimalsOverride: null,
|
||||
// Whether eth_getTransactionReceipt confirms a transaction rather than
|
||||
// answering "not mined yet".
|
||||
seedReceipt: false,
|
||||
// Every raw signed transaction handed to eth_sendRawTransaction, in
|
||||
// order. The dApp transaction round trip asserts against these bytes
|
||||
// rather than against anything the extension reported about them.
|
||||
|
||||
128
tests/transferAmount.test.js
Normal file
128
tests/transferAmount.test.js
Normal 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(/\.$/);
|
||||
}
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user