fix: one transaction approval at a time, and honest copy for a nonce collision (closes #271)
All checks were successful
check / check (push) Successful in 40s
All checks were successful
check / check (push) Successful in 40s
Populating the transaction in the background before the approval window opens is what makes the displayed object the verified object. It also fixes the nonce before the user has answered anything, so two eth_sendTransaction calls populated concurrently took the same nonce from a node that had seen neither of them broadcast, and the second could never be sent: its approved nonce is spent, and the only way to give it a fresh one is to populate it again after the user has read the old one off the screen. A second transaction approval is now refused while one is unanswered, with EIP-1193 code -32002. The refusal happens before anything is populated — no second nonce is allocated, no window opens — and the slot is released when the requesting page has its answer. Signature approvals are not gated; a signature consumes no nonce. A collision that does happen is now reported for what it is. A broadcast the node refused for the nonce, and an approval carrying a nonce this worker has already broadcast (caught before the node is asked at all), both report that the transaction did not reach the network and to send it again, instead of the standing broadcast wording that warns it may have sent. "already known" keeps that ambiguous wording deliberately: a node that says it has the transaction has it. Nothing about verification is weakened. The approval still carries the transaction the screen displayed, and the artifact is still compared against that object field for field.
This commit is contained in:
@@ -14,8 +14,10 @@ const {
|
||||
assertWithinCeilings,
|
||||
sameAddress,
|
||||
failureIsRetryable,
|
||||
isNonceCollision,
|
||||
describeTxFailure,
|
||||
describeSigningFailure,
|
||||
NONCE_COLLISION_MESSAGE,
|
||||
ALLOWED_TX_TYPES,
|
||||
SERIALIZED_FIELDS,
|
||||
FORBIDDEN_FIELDS,
|
||||
@@ -23,6 +25,7 @@ const {
|
||||
TX_STAGE_SIGN,
|
||||
TX_STAGE_VERIFY,
|
||||
TX_STAGE_BROADCAST,
|
||||
TX_STAGE_NONCE,
|
||||
MAX_GAS_LIMIT,
|
||||
MAX_FEE_PER_GAS,
|
||||
} = require("../src/shared/approvalVerify");
|
||||
@@ -1191,7 +1194,6 @@ describe("signing failure and retry", () => {
|
||||
"already known",
|
||||
"timeout of 30000ms exceeded",
|
||||
"could not coalesce error",
|
||||
"replacement transaction underpriced",
|
||||
]) {
|
||||
const outcome = describeTxFailure(
|
||||
TX_STAGE_BROADCAST,
|
||||
@@ -1199,10 +1201,76 @@ describe("signing failure and retry", () => {
|
||||
);
|
||||
expect(outcome.retryable).toBe(false);
|
||||
expect(outcome.spendApproval).toBe(true);
|
||||
expect(outcome.stage).toBe(TX_STAGE_BROADCAST);
|
||||
expect(outcome.error).toBe(message);
|
||||
}
|
||||
});
|
||||
|
||||
// The one broadcast failure that is not ambiguous. The node answered, and
|
||||
// its answer was that the nonce was already spoken for, so this
|
||||
// transaction is not in a mempool anywhere.
|
||||
test("a nonce the node refused is classified however it was worded", () => {
|
||||
for (const err of [
|
||||
new Error("nonce too low"),
|
||||
new Error("replacement transaction underpriced"),
|
||||
Object.assign(new Error("could not coalesce error"), {
|
||||
code: "NONCE_EXPIRED",
|
||||
}),
|
||||
Object.assign(new Error("could not coalesce error"), {
|
||||
code: "REPLACEMENT_UNDERPRICED",
|
||||
}),
|
||||
// The shape ethers hands up when it could not classify the node's
|
||||
// error itself: the node's own words are nested underneath.
|
||||
Object.assign(new Error("could not coalesce error"), {
|
||||
info: { error: { code: -32000, message: "OldNonce" } },
|
||||
}),
|
||||
]) {
|
||||
const outcome = describeTxFailure(TX_STAGE_BROADCAST, err);
|
||||
expect(
|
||||
describeSigningFailure(
|
||||
outcome,
|
||||
"The transaction could not be sent.",
|
||||
).message,
|
||||
).toMatch(/did not reach the network/);
|
||||
expect(outcome.retryable).toBe(false);
|
||||
expect(outcome.spendApproval).toBe(true);
|
||||
expect(outcome.error).toBe(NONCE_COLLISION_MESSAGE);
|
||||
expect(outcome.stage).toBe(TX_STAGE_NONCE);
|
||||
expect(isNonceCollision(err)).toBe(true);
|
||||
}
|
||||
});
|
||||
|
||||
// A node that says it knows the transaction has it, so it did reach the
|
||||
// network and the ambiguous wording is the correct one.
|
||||
test("already known is not a nonce collision", () => {
|
||||
const err = new Error("already known");
|
||||
const outcome = describeTxFailure(TX_STAGE_BROADCAST, err);
|
||||
expect(
|
||||
describeSigningFailure(
|
||||
outcome,
|
||||
"The transaction could not be sent.",
|
||||
).message,
|
||||
).toMatch(/may still have reached the network/);
|
||||
expect(outcome.stage).toBe(TX_STAGE_BROADCAST);
|
||||
expect(isNonceCollision(err)).toBe(false);
|
||||
});
|
||||
|
||||
test("a nonce collision says the transaction did not reach the network", () => {
|
||||
const outcome = describeTxFailure(
|
||||
TX_STAGE_BROADCAST,
|
||||
new Error("nonce too low"),
|
||||
);
|
||||
const copy = describeSigningFailure(
|
||||
outcome,
|
||||
"The transaction could not be sent.",
|
||||
);
|
||||
expect(copy.retryable).toBe(false);
|
||||
expect(copy.message).toMatch(/did not reach the network/);
|
||||
expect(copy.message).not.toMatch(/may still have reached the network/);
|
||||
expect(copy.message).toMatch(/Please send it again from the site\.$/);
|
||||
expect(copy.message).toMatch(/^[A-Z].*\.$/);
|
||||
});
|
||||
|
||||
test("a failed broadcast does not tell the user to send it again", () => {
|
||||
const outcome = describeSigningFailure(
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user