Compare commits
1
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
969a40e898 |
@@ -2071,7 +2071,9 @@ view would leave a wallet one click from deletion.
|
|||||||
artifact can be compared with it field for field. A nonce the site supplies is
|
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
|
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
|
cannot replace one of the user's pending transactions or leave this one stuck
|
||||||
behind a gap. A request that cannot be populated — unreachable node, reverting
|
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
|
||||||
gas estimate — opens no window and is failed back to the site. Only one
|
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
|
transaction approval exists at a time: populating fixes the nonce, so a second
|
||||||
`eth_sendTransaction` arriving while one is unanswered is refused with
|
`eth_sendTransaction` arriving while one is unanswered is refused with
|
||||||
|
|||||||
@@ -31,6 +31,7 @@ const {
|
|||||||
VoidSigner,
|
VoidSigner,
|
||||||
accessListify,
|
accessListify,
|
||||||
getAddress,
|
getAddress,
|
||||||
|
getBigInt,
|
||||||
getBytes,
|
getBytes,
|
||||||
hexlify,
|
hexlify,
|
||||||
toQuantity,
|
toQuantity,
|
||||||
@@ -47,10 +48,11 @@ const {
|
|||||||
// minutes long, which is not a wait anyone will sit through.
|
// minutes long, which is not a wait anyone will sit through.
|
||||||
const POPULATE_TIMEOUT_MS = 20000;
|
const POPULATE_TIMEOUT_MS = 20000;
|
||||||
|
|
||||||
// The request fields taken from the page. Anything else is dropped rather than
|
// The request fields taken from the page, plus `gas`, which requestFrom()
|
||||||
// passed to ethers: the object is page-controlled, and a future ethers that
|
// takes as the gas limit. Anything else is dropped rather than passed to
|
||||||
// learns to carry a new transaction field must not start picking one up out of
|
// ethers: the object is page-controlled, and a future ethers that learns to
|
||||||
// it without this module knowing.
|
// 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
|
// 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
|
// from the network. A page that chose it could replace one of the user's
|
||||||
@@ -84,6 +86,16 @@ function present(v) {
|
|||||||
return v !== null && v !== undefined && 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
|
// 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.
|
// sentences even when the tail of one came from ethers or from the node.
|
||||||
function sentence(text) {
|
function sentence(text) {
|
||||||
@@ -111,6 +123,20 @@ function requestFrom(txParams, from) {
|
|||||||
for (const key of REQUEST_FIELDS) {
|
for (const key of REQUEST_FIELDS) {
|
||||||
if (present(txParams[key])) request[key] = txParams[key];
|
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 (
|
if (
|
||||||
present(request.type) &&
|
present(request.type) &&
|
||||||
!ALLOWED_TX_TYPES.includes(Number(request.type))
|
!ALLOWED_TX_TYPES.includes(Number(request.type))
|
||||||
|
|||||||
@@ -9,7 +9,7 @@
|
|||||||
// requesting page or the RPC node can say turns it into an approval that
|
// requesting page or the RPC node can say turns it into an approval that
|
||||||
// should never have been raised.
|
// should never have been raised.
|
||||||
|
|
||||||
const { Network, Wallet } = require("ethers");
|
const { Network, Transaction, Wallet, toQuantity } = require("ethers");
|
||||||
const {
|
const {
|
||||||
prepareApprovalTx,
|
prepareApprovalTx,
|
||||||
serializeApprovedTx,
|
serializeApprovedTx,
|
||||||
@@ -138,6 +138,43 @@ describe("prepareApprovalTx", () => {
|
|||||||
expect(approved.maxFeePerGas).toBe("0x12a05f200");
|
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 () => {
|
test("carries an access list the request asked for", async () => {
|
||||||
const approved = await prepareApprovalTx(
|
const approved = await prepareApprovalTx(
|
||||||
providerWith(),
|
providerWith(),
|
||||||
@@ -212,6 +249,15 @@ describe("prepareApprovalTx", () => {
|
|||||||
).rejects.toThrow(/gas limit no network this wallet supports/);
|
).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
|
// 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
|
// under their own ceiling but multiply to thousands of ETH is refused
|
||||||
// before the approval window opens, so the user is never shown a
|
// before the approval window opens, so the user is never shown a
|
||||||
|
|||||||
Reference in New Issue
Block a user