fix: TxApproval prices a site's ETH send from its exact amount on its own network (closes #614)
TxApproval worked out the USD value of a site's ETH send from the
four-decimal figure it shows, at the price on the active network, so the
same value read a few cents different than on ConfirmTx, and a
transaction on another network was priced at the active one's rate, its
network fee too. It now prices the exact value with getPrice("ETH",
chainId) for the transaction's chain id, as ConfirmTx does, and passes
that chain id to formatFee() for the fee.
getPrice() now returns null for a chain id that names no network instead
of throwing, so such a transaction is still shown, with no USD value.
Model: opus-5-5
This commit was merged in pull request #635.
This commit is contained in:
@@ -2397,7 +2397,10 @@ view would leave a wallet one click from deletion.
|
||||
- Password input and an error line
|
||||
- "Confirm" / "Reject" buttons
|
||||
- **Behavior**: Every etherscan link on the screen opens the explorer of the
|
||||
network the transaction was populated on, whichever network is active.
|
||||
network the transaction was populated on, whichever network is active. The
|
||||
Value and Network fee lines each give the USD value of the exact amount at the
|
||||
price on that network, as on ConfirmTx: none for a testnet transaction or one
|
||||
on a chain id that names no network.
|
||||
- **Transitions**:
|
||||
- "Confirm" (correct password) → decrypts and signs the transaction it was
|
||||
shown, exactly as shown, hands the signed transaction to the background to
|
||||
|
||||
@@ -231,10 +231,11 @@ function showTxFee(approvedTx) {
|
||||
const feePerGas = BigInt(approvedTx.maxFeePerGas || approvedTx.gasPrice);
|
||||
// Through formatFee(), as the confirmation screen's fee is, so the same
|
||||
// fee reads the same on both. In the native currency of the network shown
|
||||
// above, as the value is.
|
||||
// above, and at the price on it, as the value is.
|
||||
$("approve-tx-fee").textContent = formatFee(
|
||||
gasLimit * feePerGas,
|
||||
nativeCurrencyByChainId(approvedTx.chainId),
|
||||
approvedTx.chainId,
|
||||
);
|
||||
|
||||
let detail =
|
||||
@@ -367,11 +368,12 @@ function showTxApproval(details) {
|
||||
$("approve-tx-to").innerHTML = escapeHtml(CONTRACT_CREATION_TEXT);
|
||||
}
|
||||
|
||||
const ethValueFormatted = formatTxValue(
|
||||
formatEther(approvedTx.value || "0"),
|
||||
);
|
||||
const ethPrice = getPrice("ETH");
|
||||
const ethUsd = ethPrice ? parseFloat(ethValueFormatted) * ethPrice : null;
|
||||
const ethValueFormatted = formatTxValue(ethValue);
|
||||
// Priced from the exact value, not the four-decimal figure, at the price
|
||||
// on the network the transaction is for, as the confirmation screen prices
|
||||
// its amount, so the same value reads the same on both.
|
||||
const ethPrice = getPrice("ETH", approvedTx.chainId);
|
||||
const ethUsd = ethPrice ? parseFloat(ethValue) * ethPrice : null;
|
||||
const usdStr = formatUsd(ethUsd);
|
||||
// In the native currency of the network the transaction is for, which the
|
||||
// Network line names, not the active network's: the active network can
|
||||
|
||||
@@ -38,11 +38,12 @@ function clearPrices() {
|
||||
}
|
||||
|
||||
// Return the USD price for a symbol on the network with `chainId`, or on the
|
||||
// active network when no chain id is given; null on testnet / unknown.
|
||||
// active network when no chain id is given; null on testnet, on a chain id
|
||||
// that names no network, or for a symbol with no price.
|
||||
function getPrice(symbol, chainId) {
|
||||
const { currentNetwork } = require("./state");
|
||||
const network = chainId ? networkByChainId(chainId) : currentNetwork();
|
||||
if (network.isTestnet) return null;
|
||||
if (!network || network.isTestnet) return null;
|
||||
return prices[symbol] || null;
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,192 @@
|
||||
// The USD value on the approval screen's Value and Network fee lines, for a
|
||||
// site's ETH send.
|
||||
//
|
||||
// It was worked out from the four-decimal figure the line shows, at the price
|
||||
// on the active network, so the same value read a few cents different here than
|
||||
// on the confirmation screen, which prices the exact amount at the price on the
|
||||
// transaction's own network, and a transaction on another network than the
|
||||
// active one was priced at the active one's rate, its fee too
|
||||
// (https://git.eeqj.de/sneak/AutistMask/issues/614).
|
||||
//
|
||||
// Driven through the real approval screen, against a minimal DOM stub in the
|
||||
// shape tests/approvalTokenNetwork.test.js uses.
|
||||
|
||||
globalThis.chrome = {
|
||||
storage: { local: { get: async () => ({}), set: async () => {} } },
|
||||
};
|
||||
|
||||
const { state } = require("../src/shared/state");
|
||||
const { NETWORKS } = require("../src/shared/networks");
|
||||
const { prices, clearPrices } = require("../src/shared/prices");
|
||||
const approval = require("../src/popup/views/approval");
|
||||
|
||||
const FROM = "0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266";
|
||||
const RECIPIENT = "0x711e691962E5c054b78071C789A0C8CE657377aD";
|
||||
|
||||
function makeElement(id) {
|
||||
const classes = new Set();
|
||||
const el = {
|
||||
id,
|
||||
textContent: "",
|
||||
value: "",
|
||||
innerHTML: "",
|
||||
disabled: false,
|
||||
style: {},
|
||||
dataset: {},
|
||||
listeners: {},
|
||||
classList: {
|
||||
add: (...names) => names.forEach((n) => classes.add(n)),
|
||||
remove: (...names) => names.forEach((n) => classes.delete(n)),
|
||||
contains: (n) => classes.has(n),
|
||||
toggle: (n, force) => {
|
||||
const on = force === undefined ? !classes.has(n) : force;
|
||||
if (on) classes.add(n);
|
||||
else classes.delete(n);
|
||||
return on;
|
||||
},
|
||||
},
|
||||
addEventListener: (name, fn) => {
|
||||
el.listeners[name] = el.listeners[name] || [];
|
||||
el.listeners[name].push(fn);
|
||||
},
|
||||
querySelectorAll: () => [],
|
||||
appendChild: () => {},
|
||||
};
|
||||
// Views reach for .parentElement to hide whole sections.
|
||||
Object.defineProperty(el, "parentElement", {
|
||||
get: () => node(id + "-parent"),
|
||||
});
|
||||
return el;
|
||||
}
|
||||
|
||||
function makeDocument() {
|
||||
const els = new Map();
|
||||
return {
|
||||
getElementById(id) {
|
||||
// The debug banner is created on demand by helpers.js; absent
|
||||
// is the state a non-debug, non-testnet popup is in.
|
||||
if (id === "debug-banner") return null;
|
||||
if (!els.has(id)) els.set(id, makeElement(id));
|
||||
return els.get(id);
|
||||
},
|
||||
createElement: () => makeElement("created"),
|
||||
body: { prepend: () => {} },
|
||||
};
|
||||
}
|
||||
|
||||
function node(id) {
|
||||
return globalThis.document.getElementById(id);
|
||||
}
|
||||
|
||||
// Open the approval screen, with the network `networkId` active, for the
|
||||
// site's transaction sending 1.23414 ETH to RECIPIENT on the chain `chainId`.
|
||||
async function showEthSend(chainId, networkId) {
|
||||
globalThis.document = makeDocument();
|
||||
globalThis.window = { location: { search: "" }, close: () => {} };
|
||||
globalThis.chrome.runtime = {
|
||||
connect: () => ({ postMessage: () => {} }),
|
||||
sendMessage: (msg, reply) => {
|
||||
if (!reply) return;
|
||||
reply({
|
||||
type: "tx",
|
||||
origin: "https://dapp.example",
|
||||
isPhishingDomain: false,
|
||||
approvedFrom: FROM,
|
||||
approvedTx: {
|
||||
type: 2,
|
||||
from: FROM,
|
||||
chainId,
|
||||
nonce: "0x7",
|
||||
gasLimit: "0x5208",
|
||||
maxPriorityFeePerGas: "0x3b9aca00",
|
||||
maxFeePerGas: "0x77359400",
|
||||
to: RECIPIENT,
|
||||
value: "1234140000000000000",
|
||||
data: "0x",
|
||||
accessList: [],
|
||||
},
|
||||
});
|
||||
},
|
||||
};
|
||||
state.networkId = networkId;
|
||||
state.activeAddress = FROM;
|
||||
state.wallets = [
|
||||
{
|
||||
type: "hd",
|
||||
name: "Wallet 1",
|
||||
xpub: "xpub-wallet-1",
|
||||
encryptedSecret: "encrypted-secret-1",
|
||||
nextIndex: 1,
|
||||
addresses: [{ address: FROM, index: 0, balance: "0" }],
|
||||
},
|
||||
];
|
||||
state.trackedTokens = [];
|
||||
state.networkTrackedTokens = {};
|
||||
state.currentView = null;
|
||||
approval.init({});
|
||||
await approval.show(1);
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
prices.ETH = 2000;
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
clearPrices();
|
||||
});
|
||||
|
||||
// 1.23414 ETH reads 1.2341 ETH. At $2,000 the exact value is $2,468.28, the
|
||||
// figure the confirmation screen shows for it; the shown figure would be
|
||||
// $2,468.20.
|
||||
test.each([
|
||||
[
|
||||
"a mainnet transaction is priced from its exact value",
|
||||
NETWORKS.mainnet.chainId,
|
||||
"mainnet",
|
||||
"1.2341 ETH ($2,468.28)",
|
||||
],
|
||||
[
|
||||
"a mainnet transaction shown with Sepolia active is priced at the mainnet price",
|
||||
NETWORKS.mainnet.chainId,
|
||||
"sepolia",
|
||||
"1.2341 ETH ($2,468.28)",
|
||||
],
|
||||
[
|
||||
"a Sepolia transaction shown with mainnet active has no USD value",
|
||||
NETWORKS.sepolia.chainId,
|
||||
"mainnet",
|
||||
"1.2341 SepoliaETH",
|
||||
],
|
||||
// Chain id 1337 names no network here, so nothing knows its price.
|
||||
[
|
||||
"a transaction on a chain id that names no network has no USD value",
|
||||
"0x539",
|
||||
"mainnet",
|
||||
"1.2341 ETH",
|
||||
],
|
||||
])("%s", async (_label, chainId, networkId, expected) => {
|
||||
await showEthSend(chainId, networkId);
|
||||
expect(state.currentView).toBe("approve-tx");
|
||||
expect(node("approve-tx-value").textContent).toBe(expected);
|
||||
});
|
||||
|
||||
// The Network fee line is priced on the transaction's network too. 21000 gas at
|
||||
// up to 2 gwei is 0.000042 ETH, $0.08 at $2,000.
|
||||
test.each([
|
||||
[
|
||||
"the fee of a mainnet transaction shown with Sepolia active is priced at the mainnet price",
|
||||
NETWORKS.mainnet.chainId,
|
||||
"sepolia",
|
||||
"0.00004 ETH ($0.08)",
|
||||
],
|
||||
[
|
||||
"the fee of a Sepolia transaction shown with mainnet active has no USD value",
|
||||
NETWORKS.sepolia.chainId,
|
||||
"mainnet",
|
||||
"0.00004 SepoliaETH",
|
||||
],
|
||||
])("%s", async (_label, chainId, networkId, expected) => {
|
||||
await showEthSend(chainId, networkId);
|
||||
expect(state.currentView).toBe("approve-tx");
|
||||
expect(node("approve-tx-fee").textContent).toBe(expected);
|
||||
});
|
||||
Reference in New Issue
Block a user