fix: store an absent explorer decimals as unknown instead of fabricating 18 (closes #349)
fetchTokenBalances() did parseInt(item.token.decimals || "18", 10) before writing to state.wallets[].addresses[].tokenBalances[].decimals, so a token whose decimals() reverts -- one the block explorer reports no scale for -- was stored with a fabricated 18 that no reader could tell from a real one. That is upstream of a rule already merged. #306 made the ERC-20 approval amount line resolve the real scale or refuse to format, and #340 extended it to the swap lines; both read this stored value as an authoritative source, so the guess walked straight past refusals that were intact and simply never fired. A 1,000-unit approval of such a token rendered 0.000000001 on the one screen whose job is to state what is being authorized. The stored value is now the explorer's own answer or null, never a default. Both approval paths reach unknownDecimalsAmount() on a null, using the refusal that was already there. The history list's token transfers carried the same || "18" and now state exact base units with the scale unknown rather than a quantity at a guessed one. A holding whose scale nothing knows has no quantity either, so its balance is stored as null -- unknown, never zero -- and the balance list, the address USD total, the Send screen and the confirmation screen each say so rather than printing 0.0000 for money that is really there. The zero-balance filter moved onto the base-unit integer, where it needs no scale at all. The bundled token list and the user's tracked tokens already outrank the explorer, so a token either of them knows still displays its real quantity when the explorer's entry omits decimals; only what none of the three knows is unknown. Which makes the stored field the explorer's answer alone, and NOT the scale a screen renders at. Those are two questions, and every screen that needs the second one asks resolveTokenDecimals(). The Send screen did not: it read tokenBalances[].decimals raw and carried it onto the pending transaction, so a bundled or tracked token whose explorer row omits decimals reached displayedDecimals(null) inside estimateGas(). That throws, is caught as an unavailable fee, and disables Send behind "The network fee could not be estimated ... Please go back and try again" -- untrue, unactionable, and for a token such as WETH whose scale was never in doubt. The balance and the amount on the same screen were correct throughout, and validateTransfer() had nothing to object to, so nothing named the real reason. Before this change the fabricated 18 happened to be that token's real scale and the send completed, so this is a capability regression and not an inherited one. Send now resolves the scale through resolveTokenDecimals(), with no fallback. The two resolutions are deliberately not identical, and where they differ the balance follows the scale. balances.js resolves without wallets, because it is formatting one explorer row during a fetch that is about to replace the very state it would be consulting; its explorer leg is therefore that row's own value. send.js resolves with wallets, which adds explorerDecimals()'s cross-address check, so a contract two addresses report different scales for answers null rather than picking one -- a check that must apply to a value which goes on to encode a transfer. For a token neither bundled nor tracked whose explorer rows disagree, that leaves a stored quantity computed at a scale Send has just refused. Stating it would leave validateTransfer() checking the amount against a number the wallet does not vouch for, and, since the unknown-balance path is gated on the balance rather than on the scale, would again leave the fee-estimate failure as the only thing on the confirmation screen. So Send withdraws the stored quantity along with the scale: an unknown scale is an unknown balance. Only a stored quantity is withdrawn -- the "0" for a token with no row at all is an absence of holdings, which is true at every scale. The uint8 check is one shared toDecimals() rather than three copies of it, and it answers 0 for a real scale of zero: || "18" collapsed that to eighteen, the falsy-collapse trap of #246. The reader half is asserted, not just the writer half. Each of the six sites that now distinguishes an unknown quantity from a zero one -- balanceLine(), balanceLinesForAddress(), addressHoldsFunds(), getAddressValue()'s partial flag, the Send balance line and the confirmation screen's balance and insufficient-balance wording -- is tested on the PAIR, because an assertion about null alone still passes on a build that renders both as zero. The Send and confirmation cases run the real explorer response through the real fetcher, the real review handler and the real confirmation screen, so they show which of the two scale questions each screen is asking, including a two-address fixture whose explorer rows report 6 and 18 for one contract. Existing installs hold 18s that cannot be told apart retroactively -- that is the defect, and no migration can undo it. They display exactly as they do today until the next balance refresh, which rewrites tokenBalances wholesale and needs no user action. The schema version is not bumped: version 1 records stay valid and are read exactly as before. No || 18 or ?? 18 fallback remains anywhere in src/. The literal 18s that do remain are real data rather than defaults: 432 per-token decimals: 18 entries in the bundled src/shared/tokenList.js, and, outside that file, only native ETH's protocol-defined scale in src/shared/uniswap.js and the fixed-point comparison scale in src/shared/txValidation.js.
This commit is contained in:
@@ -12,6 +12,7 @@ const {
|
||||
displaySymbol,
|
||||
truncateMiddle,
|
||||
balanceLine,
|
||||
unknownableAmount,
|
||||
renderAddressHtml,
|
||||
attachCopyHandlers,
|
||||
goBack,
|
||||
@@ -118,7 +119,9 @@ function show() {
|
||||
addr.tokenBalances,
|
||||
state.trackedTokens,
|
||||
);
|
||||
amount = tb ? parseFloat(tb.balance || "0") : 0;
|
||||
// null when the scale is unknown: no quantity to show, and none to
|
||||
// price. balanceLine() states that rather than printing 0.0000.
|
||||
amount = tb ? unknownableAmount(tb.balance) : 0;
|
||||
price = getPrice(symbol);
|
||||
}
|
||||
|
||||
@@ -152,7 +155,7 @@ function show() {
|
||||
attachCopyHandlers($("address-token-line"));
|
||||
|
||||
// USD total for this token only
|
||||
const usdVal = price ? amount * price : null;
|
||||
const usdVal = price && amount !== null ? amount * price : null;
|
||||
const usdStr = formatUsd(usdVal);
|
||||
$("address-token-usd-total").innerHTML = usdStr || " ";
|
||||
|
||||
|
||||
@@ -139,12 +139,17 @@ function show(txInfo) {
|
||||
|
||||
// Balance (with inline USD)
|
||||
if (isErc20) {
|
||||
const bal = txInfo.tokenBalance || "0";
|
||||
const balUsd = tokenPrice ? parseFloat(bal) * tokenPrice : null;
|
||||
$("confirm-balance").textContent = valueWithUsd(
|
||||
bal + " " + symbol,
|
||||
balUsd,
|
||||
);
|
||||
// null is a balance whose scale nothing knows, not a balance of zero
|
||||
// (https://git.eeqj.de/sneak/AutistMask/issues/349). The send is
|
||||
// refused at encode time for the same missing scale; what this line
|
||||
// must not do is state a quantity nobody established.
|
||||
const bal = txInfo.tokenBalance;
|
||||
const balUsd =
|
||||
tokenPrice && bal != null ? parseFloat(bal) * tokenPrice : null;
|
||||
$("confirm-balance").textContent =
|
||||
bal == null
|
||||
? "unknown (" + symbol + ")"
|
||||
: valueWithUsd(bal + " " + symbol, balUsd);
|
||||
} else {
|
||||
const bal = txInfo.balance || "0";
|
||||
const balUsd = ethPrice ? parseFloat(bal) * ethPrice : null;
|
||||
@@ -235,17 +240,22 @@ function renderValidation(txInfo) {
|
||||
}
|
||||
if (codes.includes(CODES.INSUFFICIENT_TOKEN)) {
|
||||
messages.push(
|
||||
"Insufficient " +
|
||||
symbol +
|
||||
" balance. You have " +
|
||||
txInfo.tokenBalance +
|
||||
" " +
|
||||
symbol +
|
||||
" but are trying to send " +
|
||||
txInfo.amount +
|
||||
" " +
|
||||
symbol +
|
||||
".",
|
||||
txInfo.tokenBalance == null
|
||||
? "This token's balance is unknown, because nothing this" +
|
||||
" wallet can consult reports how many decimal places it" +
|
||||
" uses, so the amount you are trying to send cannot be" +
|
||||
" checked against it."
|
||||
: "Insufficient " +
|
||||
symbol +
|
||||
" balance. You have " +
|
||||
txInfo.tokenBalance +
|
||||
" " +
|
||||
symbol +
|
||||
" but are trying to send " +
|
||||
txInfo.amount +
|
||||
" " +
|
||||
symbol +
|
||||
".",
|
||||
);
|
||||
}
|
||||
if (codes.includes(CODES.INSUFFICIENT_ETH)) {
|
||||
|
||||
@@ -197,6 +197,15 @@ function showFlash(msg, duration = 2000) {
|
||||
}, duration);
|
||||
}
|
||||
|
||||
// A stored token balance as a number, or null when there is no number in it.
|
||||
// balances.js writes null for a holding whose scale nothing knows, and this
|
||||
// keeps that null from becoming a zero one dereference later.
|
||||
function unknownableAmount(balance) {
|
||||
if (balance == null) return null;
|
||||
const n = parseFloat(balance);
|
||||
return Number.isFinite(n) ? n : null;
|
||||
}
|
||||
|
||||
// One row of the balance list: symbol, quantity, fiat value.
|
||||
//
|
||||
// `symbol` is the ERC-20's own symbol() as the block explorer reported it,
|
||||
@@ -204,9 +213,18 @@ function showFlash(msg, duration = 2000) {
|
||||
// attacker-chosen length until it has been through displaySymbol. This is
|
||||
// the row that issue #307 was reported against: every screen that lists a
|
||||
// holding renders through here.
|
||||
//
|
||||
// `amount` is null for a holding whose scale nothing knows
|
||||
// (https://git.eeqj.de/sneak/AutistMask/issues/349). There is no quantity to
|
||||
// print for it and no fiat value to derive from one, and printing 0.0000 for
|
||||
// a real holding is the failure this whole rule exists to prevent, so the row
|
||||
// says so instead.
|
||||
function balanceLine(symbol, amount, price, tokenId) {
|
||||
const qty = amount.toFixed(4);
|
||||
const usd = price ? formatUsd(amount * price) || " " : " ";
|
||||
const qty = amount === null ? "quantity unknown" : amount.toFixed(4);
|
||||
const usd =
|
||||
price && amount !== null
|
||||
? formatUsd(amount * price) || " "
|
||||
: " ";
|
||||
// tokenId is a contract address out of the same explorer JSON, and it
|
||||
// lands inside a quoted attribute.
|
||||
const tokenAttr = tokenId ? ` data-token="${escapeHtml(tokenId)}"` : "";
|
||||
@@ -233,7 +251,12 @@ function balanceLinesForAddress(addr, trackedTokens, showZero) {
|
||||
);
|
||||
const seen = new Set();
|
||||
for (const t of addr.tokenBalances || []) {
|
||||
const bal = parseFloat(t.balance || "0");
|
||||
// A null balance is a holding of an unstatable amount, not a holding
|
||||
// of zero, so the show-zero setting has no say over it: hiding it
|
||||
// would be asserting the zero nobody established. Anything that does
|
||||
// not parse to a finite number is unknown for the same reason — the
|
||||
// `|| "0"` this replaced turned both into a confident zero.
|
||||
const bal = unknownableAmount(t.balance);
|
||||
if (bal === 0 && !showZero) continue;
|
||||
html += balanceLine(
|
||||
t.symbol,
|
||||
@@ -266,7 +289,12 @@ function addressHoldsFunds(addr) {
|
||||
if (!addr) return false;
|
||||
if (parseFloat(addr.balance || "0") > 0) return true;
|
||||
for (const t of addr.tokenBalances || []) {
|
||||
if (parseFloat(t.balance || "0") > 0) return true;
|
||||
// A null balance is a holding whose amount could not be stated —
|
||||
// balances.js drops a row of zero base units before the scale is
|
||||
// consulted, so a row that survived with no quantity is holding
|
||||
// something. Warning about funds must err towards warning.
|
||||
const bal = unknownableAmount(t.balance);
|
||||
if (bal === null || bal > 0) return true;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
@@ -525,6 +553,7 @@ module.exports = {
|
||||
balanceLine,
|
||||
balanceLinesForAddress,
|
||||
addressHoldsFunds,
|
||||
unknownableAmount,
|
||||
addressColor,
|
||||
addressDotHtml,
|
||||
escapeHtml,
|
||||
|
||||
@@ -12,6 +12,7 @@ const {
|
||||
const { state, currentAddress } = require("../../shared/state");
|
||||
let ctx;
|
||||
const { getProvider } = require("../../shared/balances");
|
||||
const { resolveTokenDecimals } = require("../../shared/approvalAmount");
|
||||
const { resolveSymbol } = require("../../shared/tokenList");
|
||||
const { isLowHolderCount } = require("../../shared/holders");
|
||||
const { isSpoofedSymbol } = require("../../shared/symbolSpoof");
|
||||
@@ -159,9 +160,14 @@ function updateSendBalance() {
|
||||
addr.tokenBalances,
|
||||
state.trackedTokens,
|
||||
);
|
||||
const bal = tb ? tb.balance || "0" : "0";
|
||||
// A null balance is a holding whose scale nothing knows. Saying "0"
|
||||
// for it would be a claim about the amount; the send itself is
|
||||
// refused later by transferAmountUnits() for the same missing scale.
|
||||
const bal = tb ? tb.balance : "0";
|
||||
$("send-balance").textContent =
|
||||
"Current balance: " + bal + " " + symbol;
|
||||
bal == null
|
||||
? "Current balance: unknown (" + symbol + ")"
|
||||
: "Current balance: " + bal + " " + symbol;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -235,8 +241,45 @@ function init(_ctx) {
|
||||
addr.tokenBalances,
|
||||
state.trackedTokens,
|
||||
);
|
||||
tokenBalance = tb ? tb.balance || "0" : "0";
|
||||
tokenDecimals = tb ? tb.decimals : null;
|
||||
// null carried through rather than flattened to "0": the confirm
|
||||
// screen states an unknown balance as unknown, and
|
||||
// validateTransfer() treats it as no balance to spend from, which
|
||||
// is the fail-closed side of an amount nobody can check.
|
||||
tokenBalance = tb ? (tb.balance ?? null) : "0";
|
||||
// Resolved the same way balances.js resolved the scale it
|
||||
// DISPLAYED this token's balance at: bundled list, then the user's
|
||||
// tracked tokens, then the explorer. The stored
|
||||
// tokenBalances[].decimals is the explorer's own answer alone, so
|
||||
// reading it raw carries a null forward for a token the wallet
|
||||
// does know the scale of — and displayedDecimals() then throws
|
||||
// inside estimateGas(), which the confirmation screen reports as
|
||||
// an unestimable fee. Unsendable, over a scale that was never in
|
||||
// doubt (https://git.eeqj.de/sneak/AutistMask/issues/349).
|
||||
// Still null when nothing knows: no fallback.
|
||||
//
|
||||
// Resolved WITH `wallets`, which balances.js does not pass: that
|
||||
// adds explorerDecimals()'s cross-address check, so a contract two
|
||||
// addresses report different scales for answers null rather than
|
||||
// picking one. That check has to apply here, because this value
|
||||
// encodes a transfer; balances.js is formatting one explorer row
|
||||
// at fetch time and cannot consult a state it is in the middle of
|
||||
// replacing.
|
||||
tokenDecimals = resolveTokenDecimals(token, {
|
||||
trackedTokens: state.trackedTokens,
|
||||
wallets: state.wallets,
|
||||
});
|
||||
// The two resolutions can therefore differ, and where they do, the
|
||||
// stored `balance` is a quantity computed at a scale this screen
|
||||
// has just declined to stand behind. Stating it would leave
|
||||
// validateTransfer() checking the amount against a number the
|
||||
// wallet does not vouch for, and — since the unknown-balance path
|
||||
// is gated on the balance, not on the scale — would leave the
|
||||
// fee-estimate failure as the only thing on the confirmation
|
||||
// screen, which says nothing about decimals. Unknown scale means
|
||||
// unknown balance. Only a stored quantity is withdrawn: the "0"
|
||||
// for a token that has no row at all is an absence of holdings,
|
||||
// which is true at every scale.
|
||||
if (tb && tokenDecimals === null) tokenBalance = null;
|
||||
}
|
||||
|
||||
ctx.showConfirmTx({
|
||||
|
||||
Reference in New Issue
Block a user