Compare commits

..
1 Commits
Author SHA1 Message Date
sneak 864eda03eb fix: the connection prompt shows the shared address whole (closes #517)
check / check (push) Waiting to run
e2e / e2e-chrome (push) Waiting to run
e2e / e2e-firefox (push) Waiting to run
The element holding the address on the connection prompt was a flex row,
so the title line and the address that renderAddressHtml() returns sat
side by side and the address got only the width left over, which hid its
last nine characters. It is now a plain block, as on the signature
screen, so the address has a row of its own. The Chrome browser suite's
connection test now measures that row with the check the other screens
already use.

Model: opus-5-5
2026-10-14 08:08:21 +00:00
10 changed files with 26 additions and 131 deletions
+2 -5
View File
@@ -2075,9 +2075,7 @@ view would leave a wallet one click from deletion.
artifact can be compared with it field for field. A nonce the site supplies is
ignored: the nonce is always the account's next nonce from the node, so a site
cannot replace one of the user's pending transactions or leave this one stuck
behind a gap. A gas limit the site supplies, as `gas` or `gasLimit`, is used
in place of the node's estimate; a request giving both with different values
is refused. A request that cannot be populated — unreachable node, reverting
behind a gap. A request that cannot be populated — unreachable node, reverting
gas estimate — opens no window and is failed back to the site. Only one
transaction approval exists at a time: populating fixes the nonce, so a second
`eth_sendTransaction` arriving while one is unanswered is refused with
@@ -2665,8 +2663,7 @@ plaintext list of anyone's domain names.
When a dApp on a blocklisted domain requests a wallet connection, transaction
approval, or signature, the approval popup displays a prominent red warning
banner alerting the user. The domain checker matches exact hostnames and all
parent domains (subdomain matching), and checks a hostname written with a
trailing dot (`name.`) as `name`.
parent domains (subdomain matching).
#### Transaction Decoding
+1 -4
View File
@@ -1716,10 +1716,7 @@
<div class="text-xs text-muted mb-1">
Address that will be shared:
</div>
<div
id="approve-address"
class="text-xs flex items-center mb-2"
></div>
<div id="approve-address" class="text-xs mb-2"></div>
</div>
<div class="mb-3">
<label
+12 -39
View File
@@ -1,14 +1,13 @@
// Preparation of the transaction an approval screen displays.
//
// A dApp's eth_sendTransaction normally fixes `to`, `value` and `data`, and
// sometimes the gas limit. The nonce, the fees, and the gas limit when the
// site gives none have to be filled in from the network before anything can
// be signed, and whoever fills them in decides what the user is shown. That
// work used to happen in the popup, after the user had already approved: the
// numbers on the approval screen came from the popup and were compared against
// nothing, so a compromised popup could display one fee and sign another, and
// the ceilings in approvalVerify.js were all that stood between the user and a
// fee that hands the validator the balance.
// A dApp's eth_sendTransaction normally fixes only `to`, `value` and `data`.
// The nonce, the gas limit and the fees have to be filled in from the network
// before anything can be signed, and whoever fills them in decides what the
// user is shown. That work used to happen in the popup, after the user had
// already approved: the numbers on the approval screen came from the popup and
// were compared against nothing, so a compromised popup could display one fee
// and sign another, and the ceilings in approvalVerify.js were all that stood
// between the user and a fee that hands the validator the balance.
//
// So it happens here instead, in the background, before the approval window is
// opened. The background populates the transaction, shows that object, and
@@ -32,7 +31,6 @@ const {
VoidSigner,
accessListify,
getAddress,
getBigInt,
getBytes,
hexlify,
toQuantity,
@@ -49,11 +47,10 @@ const {
// minutes long, which is not a wait anyone will sit through.
const POPULATE_TIMEOUT_MS = 20000;
// The request fields taken from the page, plus `gas`, which requestFrom()
// takes as the gas limit. Anything else is dropped rather than passed to
// ethers: the object is page-controlled, and a future ethers that learns to
// carry a new transaction field must not start picking one up out of it
// without this module knowing.
// The request fields taken from the page. Anything else is dropped rather than
// passed to ethers: the object is page-controlled, and a future ethers that
// learns to carry a new transaction field must not start picking one up out of
// it without this module knowing.
//
// The nonce is not taken from the page; it is always the account's next nonce
// from the network. A page that chose it could replace one of the user's
@@ -87,16 +84,6 @@ function present(v) {
return v !== null && v !== undefined && v !== "";
}
// Whether two quantities from the page are the same number. One that is not a
// number matches nothing.
function sameQuantity(a, b) {
try {
return getBigInt(a) === getBigInt(b);
} catch {
return false;
}
}
// These strings reach the user through the requesting page, so they are full
// sentences even when the tail of one came from ethers or from the node.
function sentence(text) {
@@ -124,20 +111,6 @@ function requestFrom(txParams, from) {
for (const key of REQUEST_FIELDS) {
if (present(txParams[key])) request[key] = txParams[key];
}
// JSON-RPC calls the gas limit `gas`, and that is the name sites send.
// ethers only reads `gasLimit`: left as `gas` it would be dropped and an
// estimate signed in its place.
if (present(txParams.gas)) {
if (
present(request.gasLimit) &&
!sameQuantity(request.gasLimit, txParams.gas)
) {
throw fail(
"The site asked for two different gas limits for this transaction.",
);
}
request.gasLimit = txParams.gas;
}
if (
present(request.type) &&
!ALLOWED_TX_TYPES.includes(Number(request.type))
+1 -4
View File
@@ -110,14 +110,11 @@ function hashListed(hash) {
* Generate hostname variants for subdomain matching.
* "sub.evil.com" yields ["sub.evil.com", "evil.com"].
*
* A trailing dot is dropped: "evil.com." is the fully qualified spelling of
* "evil.com", and the browser loads the same site from either.
*
* @param {string} hostname
* @returns {string[]}
*/
function hostnameVariants(hostname) {
const h = hostname.toLowerCase().replace(/\.$/, "");
const h = hostname.toLowerCase();
const variants = [h];
const parts = h.split(".");
// Parent domains: a.b.c.d -> b.c.d, c.d
+5 -7
View File
@@ -36,13 +36,11 @@ const DEFECTS = {
"derivation path beneath that key instead of from a master " +
"key, so the addresses listed here are not the ones that key " +
"produces under the standard path.",
"Signing, sending and private key export are disabled for this " +
"wallet. Anything these addresses hold can be reached outside " +
"this wallet with the extended private key you imported, if " +
"you kept a copy of it, or with the master extended private " +
"key or recovery phrase it came from, in software that " +
"repeats the same non-standard derivation. Check the " +
"addresses in a block explorer before deciding what to do.",
"Signing and sending are disabled for this wallet. The addresses " +
"do descend from the extended private key you imported, so " +
"anything they hold is still reachable by software that " +
"repeats the same non-standard derivation. Check them in a " +
"block explorer before deciding what to do.",
"To see the addresses this key produces under the standard path, " +
"import the master extended private key, or the recovery " +
"phrase it came from, as a new wallet. Nothing here has been " +
+1 -47
View File
@@ -9,7 +9,7 @@
// requesting page or the RPC node can say turns it into an approval that
// should never have been raised.
const { Network, Transaction, Wallet, toQuantity } = require("ethers");
const { Network, Wallet } = require("ethers");
const {
prepareApprovalTx,
serializeApprovedTx,
@@ -138,43 +138,6 @@ describe("prepareApprovalTx", () => {
expect(approved.maxFeePerGas).toBe("0x12a05f200");
});
// `gas` is the JSON-RPC name for the gas limit and the one sites send.
// The node would estimate 21000; the site's 200000 is what is shown and
// what is signed.
test("uses the gas limit a site sends as gas", async () => {
const approved = await prepareApprovalTx(
providerWith(),
signer.address,
{ ...TX_PARAMS, gas: "0x30d40" },
);
expect(approved.gasLimit).toBe("0x30d40");
const signed = Transaction.from(
await signer.signTransaction({ ...approved }),
);
expect(signed.gasLimit).toBe(200000n);
});
test("accepts gas and gasLimit that name the same number", async () => {
const approved = await prepareApprovalTx(
providerWith(),
signer.address,
{ ...TX_PARAMS, gas: "0x30d40", gasLimit: 200000 },
);
expect(approved.gasLimit).toBe("0x30d40");
});
test("refuses gas and gasLimit that disagree", async () => {
await expect(
prepareApprovalTx(providerWith(), signer.address, {
...TX_PARAMS,
gas: "0x30d40",
gasLimit: "0x5208",
}),
).rejects.toThrow(
"The site asked for two different gas limits for this transaction.",
);
});
test("carries an access list the request asked for", async () => {
const approved = await prepareApprovalTx(
providerWith(),
@@ -249,15 +212,6 @@ describe("prepareApprovalTx", () => {
).rejects.toThrow(/gas limit no network this wallet supports/);
});
test("refuses a gas limit the site sent above the ceiling", async () => {
await expect(
prepareApprovalTx(providerWith(), signer.address, {
...TX_PARAMS,
gas: toQuantity(MAX_GAS_LIMIT + 1n),
}),
).rejects.toThrow(/gas limit no network this wallet supports/);
});
// The combined bound at population: a gas limit and a fee that are each
// under their own ceiling but multiply to thousands of ETH is refused
// before the approval window opens, so the user is never shown a
+1
View File
@@ -58,6 +58,7 @@ const TX_PARAMS = {
to: RECIPIENT,
value: "0x2386f26fc10000",
data: "0xdeadbeef",
gas: "0x5208",
};
// The values populateTransaction() fills in when the dApp fixed none of them.
+3
View File
@@ -3857,6 +3857,9 @@ test("eth_requestAccounts approved returns the selected address (#183)", async (
shown.toLowerCase().includes(env.expectedAddress.toLowerCase()),
"the site prompt shows the wrong address: " + JSON.stringify(shown),
);
// The text above is there even when the row is too narrow to show it
// all; this measures that the whole address is visible (#517).
await assertAddressesFit(popup, "the connection prompt");
// Remembered, so the connection survives a background worker that MV3
// terminates after 30 seconds idle. The in-memory connectedSites map
-13
View File
@@ -89,12 +89,6 @@ describe("vendored blocklist", () => {
expect(isPhishingDomain(LISTED[0].toUpperCase())).toBe(true);
});
test("a hostname ending in a dot is the same host", () => {
expect(isPhishingDomain(LISTED[1] + ".")).toBe(true);
expect(isPhishingDomain("wallet." + LISTED[1] + ".")).toBe(true);
expect(isPhishingDomain(CLEAN[0] + ".")).toBe(false);
});
test("returns false for an empty or missing hostname", () => {
expect(isPhishingDomain("")).toBe(false);
expect(isPhishingDomain(null)).toBe(false);
@@ -139,13 +133,6 @@ describe("hostnameVariants", () => {
test("lowercases hostnames", () => {
expect(hostnameVariants("Evil.COM")).toEqual(["evil.com"]);
});
test("drops the trailing dot of a fully qualified hostname", () => {
expect(hostnameVariants("sub.evil.com.")).toEqual([
"sub.evil.com",
"evil.com",
]);
});
});
describe("domain hashing", () => {
-12
View File
@@ -153,18 +153,6 @@ describe("the explanatory copy", () => {
expect(all).not.toMatch(/\blost\b|\bstolen\b|\bgone\b/i);
});
// The wallet holds the imported key but will not export it, so the copy
// must name every key the user may hold that reaches the addresses
// outside it.
test("it names the keys that reach the addresses outside this wallet", () => {
const body = defect.paragraphs.join(" ");
expect(body).toMatch(/private key export are disabled/);
expect(body).toMatch(
/with the extended private key you imported, if you kept a copy of it, or with the master extended private key or recovery phrase it came from, in software that repeats the same non-standard derivation/,
);
});
test("it says the wallet is not deleted and what the user can do", () => {
const body = defect.paragraphs.join(" ");