fix: honour a transaction response only for a transaction approval (closes #262)
The liveness fix this issue describes — settle 4001 on release when the window a retry would use is gone — already landed with #271. This completes the rest. AUTISTMASK_TX_RESPONSE now refuses any approval that is not a transaction approval, so a reject can no longer retire a sign or connection approval, and a signed artifact never runs the broadcast path against one — which before only failed closed by throwing deeper in. Tests pin the site-connection port's approve, reject and disconnect paths against a transaction approval broadcasting behind them: each is declined and the dApp still receives its broadcast result. Model: opus-4-8
This commit is contained in:
@@ -45,6 +45,20 @@ but the review is broader than any of them.
|
|||||||
|
|
||||||
# Completed Steps
|
# Completed Steps
|
||||||
|
|
||||||
|
- 2026-09-21: A transaction response is honoured only for a transaction
|
||||||
|
approval, and the three remaining approval-settlement paths are pinned
|
||||||
|
([#262](https://git.eeqj.de/sneak/AutistMask/issues/262)). The liveness fix
|
||||||
|
the issue asks for — settle `4001` on release when the window it would be
|
||||||
|
retried in is gone — already landed with
|
||||||
|
[#271](https://git.eeqj.de/sneak/AutistMask/issues/271); this closes the rest.
|
||||||
|
`AUTISTMASK_TX_RESPONSE` now refuses any approval that is not a transaction
|
||||||
|
approval, so a reject no longer retires a sign or connection approval and a
|
||||||
|
signed artifact never runs the broadcast path against one, which before only
|
||||||
|
failed closed by throwing deeper in. Tests pin the site-connection port's
|
||||||
|
approve, reject and disconnect paths against a transaction approval
|
||||||
|
broadcasting behind them: each is declined and the dApp still receives its
|
||||||
|
broadcast result.
|
||||||
|
|
||||||
- 2026-08-30: An address no longer wraps, or is shortened to fit, in any of the
|
- 2026-08-30: An address no longer wraps, or is shortened to fit, in any of the
|
||||||
common views ([#380](https://git.eeqj.de/sneak/AutistMask/issues/380)). The
|
common views ([#380](https://git.eeqj.de/sneak/AutistMask/issues/380)). The
|
||||||
wallet list was the reported case: the address shared one row with the
|
wallet list was the reported case: the address shared one row with the
|
||||||
|
|||||||
@@ -1344,6 +1344,15 @@ runtime.onMessage.addListener((msg, sender, sendResponse) => {
|
|||||||
const approval = pendingApprovals[msg.id];
|
const approval = pendingApprovals[msg.id];
|
||||||
if (!approval) return false;
|
if (!approval) return false;
|
||||||
|
|
||||||
|
// This message signs and broadcasts a transaction, so it is honoured
|
||||||
|
// only for a transaction approval. A sign or connection approval
|
||||||
|
// carries no approvedTx, and reaching the broadcast path with one used
|
||||||
|
// to fail closed by throwing deeper in; refusing here keeps a future
|
||||||
|
// refactor from turning that incidental throw into a live path, and
|
||||||
|
// keeps a reject on this message from retiring an approval of another
|
||||||
|
// kind.
|
||||||
|
if (approval.type !== "tx") return false;
|
||||||
|
|
||||||
// A reject arriving while an attempt holds the approval is refused,
|
// A reject arriving while an attempt holds the approval is refused,
|
||||||
// not honoured: the attempt is on its way to broadcasting the
|
// not honoured: the attempt is on its way to broadcasting the
|
||||||
// transaction, and resolving 4001 here would tell the page the request
|
// transaction, and resolving 4001 here would tell the page the request
|
||||||
|
|||||||
@@ -1541,6 +1541,149 @@ describe("a claimed approval outlives every other retirement path", () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// The approval popup connects a port named for its approval whatever the
|
||||||
|
// approval's kind, so a decision or a disconnect on that port can reach a
|
||||||
|
// transaction approval. Both must be declined: the port decides only
|
||||||
|
// site-connection approvals, and settling a transaction approval it does not
|
||||||
|
// own — while an attempt is broadcasting behind it — is the round-3 fund-loss
|
||||||
|
// bug, where the page is told the request was rejected as the transaction goes
|
||||||
|
// out. These three paths route through settleApproval() and, before this
|
||||||
|
// suite, were exercised only against site approvals.
|
||||||
|
describe("the site-connection port never retires a transaction approval", () => {
|
||||||
|
async function txMidBroadcast() {
|
||||||
|
const bg = loadBackground();
|
||||||
|
const pending = bg.requestTx();
|
||||||
|
await settle();
|
||||||
|
const id = pending.id();
|
||||||
|
|
||||||
|
const inFlight = deferred();
|
||||||
|
bg.broadcastTransaction.mockReturnValue(inFlight.promise);
|
||||||
|
const first = bg.send(
|
||||||
|
{
|
||||||
|
type: "AUTISTMASK_TX_RESPONSE",
|
||||||
|
id,
|
||||||
|
approved: true,
|
||||||
|
rawSignedTx: await signedAtNonce(7),
|
||||||
|
},
|
||||||
|
{ url: bg.fromPopup.url },
|
||||||
|
);
|
||||||
|
await settle();
|
||||||
|
expect(bg.broadcastTransaction).toHaveBeenCalledTimes(1);
|
||||||
|
return { bg, pending, id, inFlight, first };
|
||||||
|
}
|
||||||
|
|
||||||
|
test("an approve on the port does not settle it", async () => {
|
||||||
|
const { bg, pending, id, inFlight, first } = await txMidBroadcast();
|
||||||
|
|
||||||
|
bg.connectApproval(id).decide(true, false);
|
||||||
|
await settle();
|
||||||
|
expect(pending.result()).toBeNull();
|
||||||
|
|
||||||
|
inFlight.resolve({ hash: "0xfeed" });
|
||||||
|
await settle();
|
||||||
|
expect(pending.result()).toEqual({ result: "0xfeed" });
|
||||||
|
expect(first.sendResponse).toHaveBeenCalledWith({ txHash: "0xfeed" });
|
||||||
|
});
|
||||||
|
|
||||||
|
test("a reject on the port does not settle it", async () => {
|
||||||
|
const { bg, pending, id, inFlight, first } = await txMidBroadcast();
|
||||||
|
|
||||||
|
bg.connectApproval(id).decide(false, false);
|
||||||
|
await settle();
|
||||||
|
expect(pending.result()).toBeNull();
|
||||||
|
|
||||||
|
inFlight.resolve({ hash: "0xfeed" });
|
||||||
|
await settle();
|
||||||
|
expect(pending.result()).toEqual({ result: "0xfeed" });
|
||||||
|
expect(first.sendResponse).toHaveBeenCalledWith({ txHash: "0xfeed" });
|
||||||
|
});
|
||||||
|
|
||||||
|
test("a port disconnect does not settle it", async () => {
|
||||||
|
const { bg, pending, id, inFlight, first } = await txMidBroadcast();
|
||||||
|
|
||||||
|
bg.connectApproval(id).disconnect();
|
||||||
|
await settle();
|
||||||
|
expect(pending.result()).toBeNull();
|
||||||
|
|
||||||
|
inFlight.resolve({ hash: "0xfeed" });
|
||||||
|
await settle();
|
||||||
|
expect(pending.result()).toEqual({ result: "0xfeed" });
|
||||||
|
expect(first.sendResponse).toHaveBeenCalledWith({ txHash: "0xfeed" });
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
// AUTISTMASK_TX_RESPONSE signs and broadcasts a transaction, so it is honoured
|
||||||
|
// only for a transaction approval. A reject shaped as this message used to
|
||||||
|
// retire a sign or connection approval outright, and an approve carrying a
|
||||||
|
// signed artifact used to run the broadcast path against an approval that names
|
||||||
|
// no transaction, failing closed only by throwing deeper in.
|
||||||
|
describe("a transaction response is honoured only for a transaction approval", () => {
|
||||||
|
test("a reject does not retire a sign approval", async () => {
|
||||||
|
const bg = loadBackground();
|
||||||
|
const pending = bg.requestSign();
|
||||||
|
await settle();
|
||||||
|
const id = pending.id();
|
||||||
|
|
||||||
|
bg.send(
|
||||||
|
{ type: "AUTISTMASK_TX_RESPONSE", id, approved: false },
|
||||||
|
{ url: bg.fromPopup.url },
|
||||||
|
);
|
||||||
|
await settle();
|
||||||
|
expect(pending.result()).toBeNull();
|
||||||
|
|
||||||
|
// Still live: its own reject settles it.
|
||||||
|
bg.send(
|
||||||
|
{ type: "AUTISTMASK_SIGN_RESPONSE", id, approved: false },
|
||||||
|
{ url: bg.fromPopup.url },
|
||||||
|
);
|
||||||
|
await settle();
|
||||||
|
expect(pending.result()).toEqual({
|
||||||
|
error: { code: 4001, message: "User rejected the request." },
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
test("a reject does not retire a connection approval", async () => {
|
||||||
|
const bg = loadBackground({ actionPopup: true });
|
||||||
|
const pending = bg.requestSite();
|
||||||
|
await settle();
|
||||||
|
const id = pending.id();
|
||||||
|
|
||||||
|
bg.send(
|
||||||
|
{ type: "AUTISTMASK_TX_RESPONSE", id, approved: false },
|
||||||
|
{ url: bg.fromPopup.url },
|
||||||
|
);
|
||||||
|
await settle();
|
||||||
|
expect(pending.result()).toBeNull();
|
||||||
|
|
||||||
|
// Still live: the port that owns it connects the site.
|
||||||
|
const port = bg.connectApproval(id);
|
||||||
|
port.decide(true, false);
|
||||||
|
port.disconnect();
|
||||||
|
await settle();
|
||||||
|
expect(pending.result()).toEqual({ result: [signer.address] });
|
||||||
|
});
|
||||||
|
|
||||||
|
test("an approve carrying a signed transaction never broadcasts against a sign approval", async () => {
|
||||||
|
const bg = loadBackground();
|
||||||
|
const pending = bg.requestSign();
|
||||||
|
await settle();
|
||||||
|
const id = pending.id();
|
||||||
|
|
||||||
|
bg.send(
|
||||||
|
{
|
||||||
|
type: "AUTISTMASK_TX_RESPONSE",
|
||||||
|
id,
|
||||||
|
approved: true,
|
||||||
|
rawSignedTx: await signedAtNonce(7),
|
||||||
|
},
|
||||||
|
{ url: bg.fromPopup.url },
|
||||||
|
);
|
||||||
|
await settle();
|
||||||
|
expect(bg.broadcastTransaction).not.toHaveBeenCalled();
|
||||||
|
expect(pending.result()).toBeNull();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
// A handler that throws must still answer. `sendResponse` is the only thing
|
// A handler that throws must still answer. `sendResponse` is the only thing
|
||||||
// that settles the page's window.ethereum.request() promise, so a throw that
|
// that settles the page's window.ethereum.request() promise, so a throw that
|
||||||
// escapes a handler leaves that promise pending forever — no error, no
|
// escapes a handler leaves that promise pending forever — no error, no
|
||||||
|
|||||||
Reference in New Issue
Block a user