From 184a5f3a2fe8f483a3e63a0f7072a835a4b1f701 Mon Sep 17 00:00:00 2001 From: Reinhard Hatko Date: Tue, 29 Sep 2026 16:38:16 +0200 Subject: [PATCH] fix(deploy): stop numbering re-sent phone requests as new steps When the phone doesn't answer a signing request, bulletin-deploy times out and calls signTx again for the same operation, often while the first call is still pending. The step counter numbered every request, so a user who had approved nothing saw "step 3: Link content" on a two-approval deploy. The DotNS label advanced per call too, so a re-sent commitment was shown as "Finalize domain". - createSigningCounter numbers approvals: a request that repeats the label of an uncompleted one is the same step, next attempt. - wrapDotnsSigner advances the DotNS label on completed signatures only, shared by deploy and decentralize (which carried its own copy). - phoneApprovalCopy.ts holds the re-send notice for PhoneApprovalCallout and the --yes output, replacing the label-keyed attempt map that only reached the headless deploy path. --- .changeset/phone-retry-labels.md | 18 ++ src/commands/contractDeployUi.tsx | 1 + src/commands/contractPipelineStatus.test.ts | 1 + .../decentralize/DecentralizeScreen.tsx | 6 +- src/commands/decentralize/index.ts | 5 +- src/commands/deploy/DeployScreen.tsx | 1 + src/commands/deploy/index.ts | 36 +-- src/commands/deploy/runningState.test.ts | 2 +- src/commands/deploy/signRequest.test.ts | 71 ------ src/utils/decentralize/run.ts | 23 +- src/utils/deploy/run.ts | 44 +--- src/utils/deploy/signingProxy.test.ts | 227 +++++++++++++++++- src/utils/deploy/signingProxy.ts | 90 ++++++- src/utils/ui/theme/PhoneApprovalCallout.tsx | 13 +- src/utils/ui/theme/phoneApprovalCopy.test.ts | 53 ++++ src/utils/ui/theme/phoneApprovalCopy.ts | 48 ++++ 16 files changed, 456 insertions(+), 183 deletions(-) create mode 100644 .changeset/phone-retry-labels.md delete mode 100644 src/commands/deploy/signRequest.test.ts create mode 100644 src/utils/ui/theme/phoneApprovalCopy.test.ts create mode 100644 src/utils/ui/theme/phoneApprovalCopy.ts diff --git a/.changeset/phone-retry-labels.md b/.changeset/phone-retry-labels.md new file mode 100644 index 00000000..da725e04 --- /dev/null +++ b/.changeset/phone-retry-labels.md @@ -0,0 +1,18 @@ +--- +"playground-cli": patch +--- + +Stop presenting an unanswered phone request as progress. + +When the phone doesn't answer a signing request (usually because the Polkadot +app isn't open in the foreground, which shows no prompt at all), the deploy +re-sends it. Every re-send used to be numbered as the next step, so a user who +had approved nothing could see "step 3: Link content" on a deploy that needs +two approvals. For a new domain, a re-sent "Reserve domain" request was also +relabelled "Finalize domain". + +Steps now count approvals rather than requests: a re-send keeps its step and +label and is shown as "No response yet — re-sent to your phone (attempt N)", +with a reminder to keep the app in the foreground. This applies to +`playground deploy`, `decentralize` and contract deploys, in both the +interactive screen and `--yes` output. diff --git a/src/commands/contractDeployUi.tsx b/src/commands/contractDeployUi.tsx index 4233e65d..7c99b5b6 100644 --- a/src/commands/contractDeployUi.tsx +++ b/src/commands/contractDeployUi.tsx @@ -209,6 +209,7 @@ export function ContractDeployStatusView({ )} {adapter.signingError && ( diff --git a/src/commands/contractPipelineStatus.test.ts b/src/commands/contractPipelineStatus.test.ts index e988db8c..e156e3e2 100644 --- a/src/commands/contractPipelineStatus.test.ts +++ b/src/commands/contractPipelineStatus.test.ts @@ -128,6 +128,7 @@ describe("ContractPipelineStatusAdapter", () => { kind: "sign-request", label: "Deploy and register contracts", step: 1, + attempt: 1, }); expect(adapter.signingPrompt?.label).toBe("Deploy and register contracts"); diff --git a/src/commands/decentralize/DecentralizeScreen.tsx b/src/commands/decentralize/DecentralizeScreen.tsx index 80297d0d..7530e18a 100644 --- a/src/commands/decentralize/DecentralizeScreen.tsx +++ b/src/commands/decentralize/DecentralizeScreen.tsx @@ -894,7 +894,11 @@ function RunningStage({ {signingPrompt && signingPrompt.kind === "sign-request" && ( - + )} ); diff --git a/src/commands/decentralize/index.ts b/src/commands/decentralize/index.ts index f16659f6..4cff079a 100644 --- a/src/commands/decentralize/index.ts +++ b/src/commands/decentralize/index.ts @@ -65,6 +65,7 @@ import { import type { SignerMode } from "../../utils/deploy/signerMode.js"; import { PLAYGROUND_TAGS } from "../../utils/deploy/tags.js"; import { onProcessShutdown } from "../../utils/process-guard.js"; +import { formatSignRequestLine } from "../../utils/ui/theme/phoneApprovalCopy.js"; interface DecentralizeOpts { site?: string; @@ -302,9 +303,7 @@ async function runHeadless({ } case "signing": if (ev.event.kind === "sign-request") { - process.stdout.write( - `\n ▸ Check your phone — approve step ${ev.event.step}: ${ev.event.label}\n`, - ); + process.stdout.write(`\n${formatSignRequestLine(ev.event)}`); } else if (ev.event.kind === "sign-error") { process.stdout.write(` ✖ signing failed: ${ev.event.message}\n`); } diff --git a/src/commands/deploy/DeployScreen.tsx b/src/commands/deploy/DeployScreen.tsx index 09b68fd6..2675d5a9 100644 --- a/src/commands/deploy/DeployScreen.tsx +++ b/src/commands/deploy/DeployScreen.tsx @@ -1223,6 +1223,7 @@ function RunningStage({ )} diff --git a/src/commands/deploy/index.ts b/src/commands/deploy/index.ts index 763081e1..231762fe 100644 --- a/src/commands/deploy/index.ts +++ b/src/commands/deploy/index.ts @@ -58,6 +58,7 @@ import { normalizeDomain } from "../../utils/deploy/playground.js"; import { assertBuildDirExists } from "../../utils/deploy/buildDir.js"; import { PLAYGROUND_TAGS } from "../../utils/deploy/tags.js"; import { NO_SESSION_HEADLESS_ERROR } from "./signerNotice.js"; +import { formatSignRequestLine } from "../../utils/ui/theme/phoneApprovalCopy.js"; interface DeployOpts { suri?: string; @@ -745,37 +746,6 @@ function runInteractive(ctx: { // ── Output helpers ─────────────────────────────────────────────────────────── -/** - * Render a phone signature request, distinguishing a RETRY from progress. - * - * The `step` on the event counts signature REQUESTS, not operations: when a - * request times out (the usual cause is the phone app not being in the - * foreground, which shows no prompt at all) it is re-sent with the next number. - * Printing that number made a stall look like progress — worse, it contradicted - * the "Phone approvals expected" plan above it, which numbers OPERATIONS. Seen - * live: `step 1: Link content` followed by `step 2: Link content`, while the - * plan said step 2 would be the registry publish. - * - * So we key off the label instead and say plainly when we are re-sending, plus - * the one remedy that actually fixes it. - */ -const phoneSignAttempts = new Map(); - -export function formatSignRequest(label: string): string { - const attempt = (phoneSignAttempts.get(label) ?? 0) + 1; - phoneSignAttempts.set(label, attempt); - if (attempt === 1) return ` 📱 Approve on your phone: ${label}\n`; - return ( - ` 📱 No response yet — re-sent to your phone (attempt ${attempt}): ${label}\n` + - ` If no prompt appeared, open the Polkadot app and keep it in the foreground.\n` - ); -} - -/** Test seam: the attempt counter is module state, so tests must reset it. */ -export function resetSignRequestAttempts(): void { - phoneSignAttempts.clear(); -} - function logHeadlessEvent(event: DeployEvent) { if (event.kind === "phase-start") { process.stdout.write(`▸ ${event.phase}…\n`); @@ -786,7 +756,7 @@ function logHeadlessEvent(event: DeployEvent) { } else if (event.kind === "storage-event" && event.event.kind === "chunk-progress") { process.stdout.write(` chunk ${event.event.current}/${event.event.total}\n`); } else if (event.kind === "signing" && event.event.kind === "sign-request") { - process.stdout.write(formatSignRequest(event.event.label)); + process.stdout.write(formatSignRequestLine(event.event)); } else if (event.kind === "error") { process.stderr.write(` ✖ ${event.phase}: ${event.message}\n`); } @@ -818,7 +788,7 @@ function logHeadlessContractInstallEvent(event: ContractInstallEvent) { function logHeadlessSigningEvent(event: SigningEvent) { if (event.kind === "sign-request") { - process.stdout.write(` approve on your phone (step ${event.step}): ${event.label}\n`); + process.stdout.write(formatSignRequestLine(event)); } } diff --git a/src/commands/deploy/runningState.test.ts b/src/commands/deploy/runningState.test.ts index f7f3f111..0f904585 100644 --- a/src/commands/deploy/runningState.test.ts +++ b/src/commands/deploy/runningState.test.ts @@ -168,7 +168,7 @@ describe("runningReducer — log/signing/plan events are no-ops", () => { it("signing sign-request -> state unchanged", () => { const s = runningReducer(s0, { kind: "signing", - event: { kind: "sign-request", step: 1, label: "DotNS register" }, + event: { kind: "sign-request", step: 1, attempt: 1, label: "DotNS register" }, }); expect(s).toBe(s0); }); diff --git a/src/commands/deploy/signRequest.test.ts b/src/commands/deploy/signRequest.test.ts deleted file mode 100644 index 0b04a6ae..00000000 --- a/src/commands/deploy/signRequest.test.ts +++ /dev/null @@ -1,71 +0,0 @@ -// Copyright (C) Parity Technologies (UK) Ltd. -// SPDX-License-Identifier: Apache-2.0 - -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -import { beforeEach, describe, expect, it } from "vitest"; -import { formatSignRequest, resetSignRequestAttempts } from "./index.js"; - -beforeEach(() => { - resetSignRequestAttempts(); -}); - -describe("formatSignRequest", () => { - it("announces the first request plainly", () => { - const line = formatSignRequest("Link content (DotNS setContenthash)"); - expect(line).toContain("Approve on your phone"); - expect(line).toContain("Link content (DotNS setContenthash)"); - expect(line).not.toMatch(/attempt/i); - }); - - /** - * The bug this replaces, observed live: the event's `step` counts signature - * REQUESTS, so a timed-out attempt re-sent as "step 2" read as progress — - * and contradicted the "Phone approvals expected" plan above it, which - * numbers OPERATIONS. A deploy printed `step 1: Link content` then - * `step 2: Link content` while the plan said step 2 was the registry - * publish, and the user approved four prompts for a two-approval deploy. - */ - it("calls a repeat of the same operation a retry, not the next step", () => { - const label = "Link content (DotNS setContenthash)"; - formatSignRequest(label); - const second = formatSignRequest(label); - - expect(second).toMatch(/attempt 2/); - expect(second).toMatch(/re-sent/i); - // Must not imply forward progress. - expect(second).not.toMatch(/step 2/); - }); - - it("keeps counting further retries of the same operation", () => { - const label = "Publish to Playground registry"; - formatSignRequest(label); - formatSignRequest(label); - expect(formatSignRequest(label)).toMatch(/attempt 3/); - }); - - it("treats a different operation as a fresh request", () => { - formatSignRequest("Link content (DotNS setContenthash)"); - const other = formatSignRequest("Publish to Playground registry"); - - expect(other).toContain("Approve on your phone"); - expect(other).not.toMatch(/attempt/i); - }); - - /** The one remedy worth surfacing when no prompt showed up at all. */ - it("tells the user what to check when a prompt never appeared", () => { - const label = "Link content (DotNS setContenthash)"; - formatSignRequest(label); - expect(formatSignRequest(label)).toMatch(/foreground/i); - }); -}); diff --git a/src/utils/decentralize/run.ts b/src/utils/decentralize/run.ts index 9c7e8e75..7c901dfa 100644 --- a/src/utils/decentralize/run.ts +++ b/src/utils/decentralize/run.ts @@ -44,6 +44,7 @@ import { createApprovalPrompt, type SigningCounter, type SigningEvent, + wrapDotnsSigner, wrapSignerWithEvents, } from "../deploy/signingProxy.js"; import { runStorageDeploy } from "../deploy/storage.js"; @@ -375,25 +376,5 @@ function wrapAuthForSigning( if (!auth.signer || !auth.signerAddress) return auth; const labels = approvals.filter((a) => a.phase === "dotns").map((a) => a.label); - const fallbackLabel = labels[labels.length - 1] ?? "DotNS step"; - const signer = auth.signer; - let seen = 0; - - return { - ...auth, - signer: { - publicKey: signer.publicKey, - signTx: (...args: Parameters) => { - const label = labels[seen] ?? fallbackLabel; - seen += 1; - return wrapSignerWithEvents(signer, { label, counter, onEvent }).signTx(...args); - }, - signBytes: (...args: Parameters) => - wrapSignerWithEvents(signer, { - label: "DotNS signBytes", - counter, - onEvent, - }).signBytes(...args), - }, - }; + return { ...auth, signer: wrapDotnsSigner(auth.signer, labels, counter, onEvent) }; } diff --git a/src/utils/deploy/run.ts b/src/utils/deploy/run.ts index dfb4c498..51f5a13e 100644 --- a/src/utils/deploy/run.ts +++ b/src/utils/deploy/run.ts @@ -34,6 +34,7 @@ import { } from "./signerMode.js"; import { wrapSignerWithEvents, + wrapDotnsSigner, createSigningCounter, createApprovalPrompt, type SigningCounter, @@ -326,15 +327,11 @@ export async function runDeploy(options: RunDeployOptions): Promise["bulletinDeployAuthOptions"], @@ -345,31 +342,10 @@ function maybeWrapAuthForSigning( if (!auth.signer || !auth.signerAddress) return auth; const labels = approvals.filter((a) => a.phase === "dotns").map((a) => a.label); - const fallbackLabel = labels[labels.length - 1] ?? "DotNS step"; - let seen = 0; - const wrapped = { - publicKey: auth.signer.publicKey, - signTx: (...args: Parameters) => { - const label = labels[seen] ?? fallbackLabel; - seen += 1; - const proxy = wrapSignerWithEvents(auth.signer!, { - label, - counter, - onEvent: (event) => options.onEvent({ kind: "signing", event }), - }); - return proxy.signTx(...args); - }, - signBytes: (...args: Parameters) => { - const proxy = wrapSignerWithEvents(auth.signer!, { - label: "DotNS signBytes", - counter, - onEvent: (event) => options.onEvent({ kind: "signing", event }), - }); - return proxy.signBytes(...args); - }, - }; - - return { ...auth, signer: wrapped }; + const signer = wrapDotnsSigner(auth.signer, labels, counter, (event) => + options.onEvent({ kind: "signing", event }), + ); + return { ...auth, signer }; } function wrapResolvedSigner( diff --git a/src/utils/deploy/signingProxy.test.ts b/src/utils/deploy/signingProxy.test.ts index b9c8af63..c77756b1 100644 --- a/src/utils/deploy/signingProxy.test.ts +++ b/src/utils/deploy/signingProxy.test.ts @@ -14,7 +14,14 @@ // limitations under the License. import { describe, it, expect } from "vitest"; -import { createSigningCounter, createApprovalPrompt, type SigningEvent } from "./signingProxy.js"; +import type { PolkadotSigner } from "polkadot-api"; +import { + createSigningCounter, + createApprovalPrompt, + wrapDotnsSigner, + wrapSignerWithEvents, + type SigningEvent, +} from "./signingProxy.js"; describe("createSigningCounter", () => { it("returns bare sequential step numbers with no predicted total", () => { @@ -23,11 +30,207 @@ describe("createSigningCounter", () => { // skipped) stranded users on "step 4 of 5" with no fifth step. The // counter now just numbers taps as they happen. const c = createSigningCounter(); - expect(c.next()).toEqual({ step: 1 }); - expect(c.next()).toEqual({ step: 2 }); - expect(c.next()).toEqual({ step: 3 }); + expect(c.next("a")).toEqual({ step: 1, attempt: 1 }); + expect(c.next("b")).toEqual({ step: 2, attempt: 1 }); + expect(c.next("c")).toEqual({ step: 3, attempt: 1 }); expect(c.count()).toBe(3); }); + + /** + * Seen live: a user who had approved NOTHING saw "step 3: Link content" on + * a deploy that needs two approvals — each unanswered request was re-sent + * and numbered as the next step. + */ + it("numbers a re-send of an uncompleted request as the same step, next attempt", () => { + const c = createSigningCounter(); + expect(c.next("Link content")).toEqual({ step: 1, attempt: 1 }); + expect(c.next("Link content")).toEqual({ step: 1, attempt: 2 }); + expect(c.next("Link content")).toEqual({ step: 1, attempt: 3 }); + expect(c.count()).toBe(1); + }); + + it("starts a new step for a repeated label once the previous one completed", () => { + // A legitimately repeated operation (e.g. the fallback DotNS label when + // bulletin-deploy fires more signatures than planned) is progress. + const c = createSigningCounter(); + const { step } = c.next("DotNS step"); + c.complete(step); + expect(c.next("DotNS step")).toEqual({ step: 2, attempt: 1 }); + }); + + it("moves on to a new step when the label changes", () => { + const c = createSigningCounter(); + c.next("Link content"); + expect(c.next("Publish to Playground registry")).toEqual({ step: 2, attempt: 1 }); + }); + + it("ignores a late completion of an earlier step", () => { + const c = createSigningCounter(); + c.complete(c.next("Link content").step); + c.next("Publish to Playground registry"); + c.complete(1); + expect(c.next("Publish to Playground registry")).toEqual({ step: 2, attempt: 2 }); + }); +}); + +/** A phone signer whose calls stay pending until the test settles them. */ +function fakePhoneSigner() { + const pending: { resolve: (v: Uint8Array) => void; reject: (e: Error) => void }[] = []; + const signer: PolkadotSigner = { + publicKey: new Uint8Array(32), + signTx: () => + new Promise((resolve, reject) => pending.push({ resolve, reject })), + signBytes: () => + new Promise((resolve, reject) => pending.push({ resolve, reject })), + }; + return { signer, pending }; +} + +const signTxArgs = [new Uint8Array(), {}, new Uint8Array(), 0] as unknown as Parameters< + PolkadotSigner["signTx"] +>; +const flush = () => new Promise((r) => setTimeout(r, 0)); +const requests = (events: SigningEvent[]) => + events + .filter((e) => e.kind === "sign-request") + .map(({ label, step, attempt }: any) => ({ + label, + step, + attempt, + })); + +describe("wrapSignerWithEvents", () => { + it("reports a re-send while the first request is still pending as a retry", async () => { + // bulletin-deploy times out and calls signTx again WITHOUT the first + // call ever settling, so "not completed" — not a rejection — is the signal. + const events: SigningEvent[] = []; + const { signer, pending } = fakePhoneSigner(); + const wrapped = wrapSignerWithEvents(signer, { + label: "Publish to Playground registry", + counter: createSigningCounter(), + onEvent: (e) => events.push(e), + }); + + void wrapped.signTx(...signTxArgs); + const retry = wrapped.signTx(...signTxArgs); + pending[1]!.resolve(new Uint8Array([1])); + await retry; + + expect(events).toEqual([ + { kind: "sign-request", label: "Publish to Playground registry", step: 1, attempt: 1 }, + { kind: "sign-request", label: "Publish to Playground registry", step: 1, attempt: 2 }, + { kind: "sign-complete", label: "Publish to Playground registry", step: 1 }, + ]); + }); + + it("reports a re-send after a rejected request as a retry", async () => { + const events: SigningEvent[] = []; + const { signer, pending } = fakePhoneSigner(); + const wrapped = wrapSignerWithEvents(signer, { + label: "Link content", + counter: createSigningCounter(), + onEvent: (e) => events.push(e), + }); + + const first = wrapped.signTx(...signTxArgs); + pending[0]!.reject(new Error("timed out")); + await expect(first).rejects.toThrow("timed out"); + void wrapped.signTx(...signTxArgs); + + expect(requests(events)).toEqual([ + { label: "Link content", step: 1, attempt: 1 }, + { label: "Link content", step: 1, attempt: 2 }, + ]); + }); +}); + +describe("wrapDotnsSigner", () => { + const REGISTER = ["Reserve domain", "Finalize domain", "Link content"]; + + it("keeps a re-sent DotNS request on its own label instead of the next one", async () => { + // Regression: labels advanced per signTx CALL, so a re-sent commitment + // was shown as "Finalize domain". + const events: SigningEvent[] = []; + const { signer, pending } = fakePhoneSigner(); + const wrapped = wrapDotnsSigner(signer, REGISTER, createSigningCounter(), (e) => + events.push(e), + ); + + void wrapped.signTx(...signTxArgs); // never answered + const retry = wrapped.signTx(...signTxArgs); + pending[1]!.resolve(new Uint8Array([1])); + await retry; + void wrapped.signTx(...signTxArgs); + + expect(requests(events)).toEqual([ + { label: "Reserve domain", step: 1, attempt: 1 }, + { label: "Reserve domain", step: 1, attempt: 2 }, + { label: "Finalize domain", step: 2, attempt: 1 }, + ]); + }); + + it("reproduces the live report: three unanswered Link content requests are ONE step", async () => { + const events: SigningEvent[] = []; + const counter = createSigningCounter(); + const { signer, pending } = fakePhoneSigner(); + const dotns = wrapDotnsSigner(signer, ["Link content"], counter, (e) => events.push(e)); + const publish = wrapSignerWithEvents(signer, { + label: "Publish to Playground registry", + counter, + onEvent: (e) => events.push(e), + }); + + void dotns.signTx(...signTxArgs); + void dotns.signTx(...signTxArgs); + const third = dotns.signTx(...signTxArgs); + pending[2]!.resolve(new Uint8Array([1])); + await third; + void publish.signTx(...signTxArgs); + + expect(requests(events)).toEqual([ + { label: "Link content", step: 1, attempt: 1 }, + { label: "Link content", step: 1, attempt: 2 }, + { label: "Link content", step: 1, attempt: 3 }, + { label: "Publish to Playground registry", step: 2, attempt: 1 }, + ]); + }); + + it("does not skip a label when an abandoned attempt completes late", async () => { + const events: SigningEvent[] = []; + const { signer, pending } = fakePhoneSigner(); + const wrapped = wrapDotnsSigner(signer, REGISTER, createSigningCounter(), (e) => + events.push(e), + ); + + const abandoned = wrapped.signTx(...signTxArgs); + const retry = wrapped.signTx(...signTxArgs); + pending[1]!.resolve(new Uint8Array([1])); + await retry; + pending[0]!.resolve(new Uint8Array([2])); + await abandoned; + await flush(); + void wrapped.signTx(...signTxArgs); + + expect(requests(events).at(-1)).toEqual({ label: "Finalize domain", step: 2, attempt: 1 }); + }); + + it("repeats the last label past the end of the plan", async () => { + const events: SigningEvent[] = []; + const { signer, pending } = fakePhoneSigner(); + const wrapped = wrapDotnsSigner(signer, ["Link content"], createSigningCounter(), (e) => + events.push(e), + ); + + const first = wrapped.signTx(...signTxArgs); + pending[0]!.resolve(new Uint8Array([1])); + await first; + void wrapped.signTx(...signTxArgs); + + expect(requests(events)).toEqual([ + { label: "Link content", step: 1, attempt: 1 }, + { label: "Link content", step: 2, attempt: 1 }, + ]); + }); }); describe("createApprovalPrompt", () => { @@ -37,13 +240,18 @@ describe("createApprovalPrompt", () => { const prompt = createApprovalPrompt(counter, (e) => events.push(e)); // A signing tap reserves step 1 elsewhere… - counter.next(); + counter.next("Link content"); // …so the allowance tap continues the same sequence at step 2. const handle = prompt("Grant Bulletin storage allowance"); handle.complete(); expect(events).toEqual([ - { kind: "sign-request", label: "Grant Bulletin storage allowance", step: 2 }, + { + kind: "sign-request", + label: "Grant Bulletin storage allowance", + step: 2, + attempt: 1, + }, { kind: "sign-complete", label: "Grant Bulletin storage allowance", step: 2 }, ]); }); @@ -56,7 +264,12 @@ describe("createApprovalPrompt", () => { handle.fail("declined on phone"); expect(events).toEqual([ - { kind: "sign-request", label: "Grant Bulletin storage allowance", step: 1 }, + { + kind: "sign-request", + label: "Grant Bulletin storage allowance", + step: 1, + attempt: 1, + }, { kind: "sign-error", label: "Grant Bulletin storage allowance", diff --git a/src/utils/deploy/signingProxy.ts b/src/utils/deploy/signingProxy.ts index ad8e4908..a3b48883 100644 --- a/src/utils/deploy/signingProxy.ts +++ b/src/utils/deploy/signingProxy.ts @@ -24,14 +24,21 @@ import type { PolkadotSigner } from "polkadot-api"; import type { AllowancePrompt } from "../allowances/bulletin.js"; export type SigningEvent = - | { kind: "sign-request"; label: string; step: number } + /** `attempt` is 1 for the first request of a step, 2+ for a re-send of the same approval. */ + | { kind: "sign-request"; label: string; step: number; attempt: number } | { kind: "sign-complete"; label: string; step: number } | { kind: "sign-error"; label: string; step: number; message: string }; export interface SigningCounter { - /** Reserve the next step number. */ - next(): { step: number }; - /** How many steps were reserved so far — useful for a final tally. */ + /** + * Reserve a step for a signature request labelled `label`. Repeating the + * label of a request that has not completed is a RE-SEND of the same + * approval — same step, next attempt — not a new step. + */ + next(label: string): { step: number; attempt: number }; + /** Mark `step` approved, so the next request is a new step even if it repeats the label. */ + complete(step: number): void; + /** How many distinct steps were reserved so far — useful for a final tally. */ count(): number; } @@ -42,13 +49,31 @@ export interface SigningCounter { * that runtime skipped left users on "step 4 of 5" with no fifth step), and * RFC-0010 allowance taps are demand-driven so they can't be counted up * front. The UI shows "step 1", "step 2", … and never has to guess. + * + * Steps count APPROVALS, not requests. When the phone does not answer (the + * usual cause is the app not being in the foreground, which shows no prompt + * at all), bulletin-deploy times out and calls `signTx` again — often while + * the first call is still pending, so a "not completed" request is the signal, + * not a rejection. Numbering each request made a stall read as progress: a + * user who had approved nothing saw "step 3: Link content" on a deploy that + * needs two approvals. */ export function createSigningCounter(): SigningCounter { let step = 0; + let current: { label: string; attempt: number; done: boolean } | null = null; return { - next() { + next(label) { + if (current && !current.done && current.label === label) { + current.attempt += 1; + return { step, attempt: current.attempt }; + } step += 1; - return { step }; + current = { label, attempt: 1, done: false }; + return { step, attempt: 1 }; + }, + complete(completed) { + // A late completion of an EARLIER step must not close the current one. + if (current && completed === step) current.done = true; }, count() { return step; @@ -73,10 +98,11 @@ export interface WrapOptions { */ export function wrapSignerWithEvents(inner: PolkadotSigner, options: WrapOptions): PolkadotSigner { const announce = async (fn: () => Promise): Promise => { - const { step } = options.counter.next(); - options.onEvent({ kind: "sign-request", label: options.label, step }); + const { step, attempt } = options.counter.next(options.label); + options.onEvent({ kind: "sign-request", label: options.label, step, attempt }); try { const value = await fn(); + options.counter.complete(step); options.onEvent({ kind: "sign-complete", label: options.label, step }); return value; } catch (err) { @@ -96,6 +122,45 @@ export function wrapSignerWithEvents(inner: PolkadotSigner, options: WrapOptions }; } +/** + * Wraps the phone signer handed to bulletin-deploy for DotNS, labelling each + * `signTx` with the operation it signs. `labels` lists the DotNS approvals in + * the order bulletin-deploy fires them (`signerMode.ts::dotnsApprovals`); past + * the end of the list we repeat the last label rather than invent an index. + * + * The label advances on COMPLETED signatures, not on calls: a timed-out + * request is re-sent as another `signTx` for the SAME operation (often while + * the first is still pending), and counting calls relabelled that retry as the + * next operation — a re-sent commitment showed up as "Finalize domain". `max` + * keeps a late completion of an abandoned attempt from skipping a label. + */ +export function wrapDotnsSigner( + inner: PolkadotSigner, + labels: string[], + counter: SigningCounter, + onEvent: (event: SigningEvent) => void, +): PolkadotSigner { + const fallbackLabel = labels[labels.length - 1] ?? "DotNS step"; + let completed = 0; + return { + publicKey: inner.publicKey, + signTx: async (...args) => { + const index = completed; + const signed = await wrapSignerWithEvents(inner, { + label: labels[index] ?? fallbackLabel, + counter, + onEvent, + }).signTx(...args); + completed = Math.max(completed, index + 1); + return signed; + }, + signBytes: (data) => + wrapSignerWithEvents(inner, { label: "DotNS signBytes", counter, onEvent }).signBytes( + data, + ), + }; +} + /** * Prompt factory for phone taps that are NOT signer calls: RFC-0010 * resource-allocation requests (the first-use Bulletin allowance grant). They @@ -114,10 +179,13 @@ export function createApprovalPrompt( onEvent: (event: SigningEvent) => void, ): AllowancePrompt { return (label) => { - const { step } = counter.next(); - onEvent({ kind: "sign-request", label, step }); + const { step, attempt } = counter.next(label); + onEvent({ kind: "sign-request", label, step, attempt }); return { - complete: () => onEvent({ kind: "sign-complete", label, step }), + complete: () => { + counter.complete(step); + onEvent({ kind: "sign-complete", label, step }); + }, fail: (message) => onEvent({ kind: "sign-error", label, step, message }), }; }; diff --git a/src/utils/ui/theme/PhoneApprovalCallout.tsx b/src/utils/ui/theme/PhoneApprovalCallout.tsx index 8b14f41c..a8f70c0a 100644 --- a/src/utils/ui/theme/PhoneApprovalCallout.tsx +++ b/src/utils/ui/theme/PhoneApprovalCallout.tsx @@ -15,6 +15,7 @@ import { Text } from "ink"; import { Callout } from "./Callout.js"; +import { PHONE_RESEND_HINT, resendNotice } from "./phoneApprovalCopy.js"; export interface PhoneApprovalCalloutProps { step: number; @@ -27,15 +28,25 @@ export interface PhoneApprovalCalloutProps { */ total?: number; label: string; + /** 2+ when this is a re-send of the same approval; omit (or 1) for a first request. */ + attempt?: number; } -export function PhoneApprovalCallout({ step, total, label }: PhoneApprovalCalloutProps) { +export function PhoneApprovalCallout({ + step, + total, + label, + attempt = 1, +}: PhoneApprovalCalloutProps) { + const notice = resendNotice(attempt); return ( approve step {step} {total !== undefined ? ` of ${total}` : ""}: {label} + {notice && {notice}} + {notice && {PHONE_RESEND_HINT}} ); } diff --git a/src/utils/ui/theme/phoneApprovalCopy.test.ts b/src/utils/ui/theme/phoneApprovalCopy.test.ts new file mode 100644 index 00000000..04bd0f8c --- /dev/null +++ b/src/utils/ui/theme/phoneApprovalCopy.test.ts @@ -0,0 +1,53 @@ +// Copyright (C) Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import { describe, expect, it } from "vitest"; +import { formatSignRequestLine, resendNotice } from "./phoneApprovalCopy.js"; + +const request = (step: number, attempt: number, label = "Link content (DotNS setContenthash)") => + ({ kind: "sign-request", label, step, attempt }) as const; + +describe("formatSignRequestLine", () => { + it("announces a first request with its step", () => { + const line = formatSignRequestLine(request(1, 1)); + expect(line).toContain("Approve on your phone (step 1)"); + expect(line).toContain("Link content (DotNS setContenthash)"); + expect(line).not.toMatch(/attempt|re-sent/i); + }); + + /** + * A re-send must not read as progress: it keeps its step number and says + * it is a re-send. Seen live before this: "step 3: Link content" for a + * user who had approved nothing on a two-approval deploy. + */ + it("calls a re-send a re-send, on the same step", () => { + const line = formatSignRequestLine(request(1, 3)); + expect(line).toMatch(/re-sent/i); + expect(line).toContain("step 1, attempt 3"); + expect(line).not.toMatch(/step [23]/); + }); + + it("tells the user what to check when a prompt never appeared", () => { + expect(formatSignRequestLine(request(2, 2))).toMatch(/foreground/i); + expect(formatSignRequestLine(request(2, 1))).not.toMatch(/foreground/i); + }); +}); + +describe("resendNotice", () => { + it("is null for a first request and names the attempt otherwise", () => { + expect(resendNotice(1)).toBeNull(); + expect(resendNotice(2)).toMatch(/attempt 2/); + }); +}); diff --git a/src/utils/ui/theme/phoneApprovalCopy.ts b/src/utils/ui/theme/phoneApprovalCopy.ts new file mode 100644 index 00000000..d7ec05af --- /dev/null +++ b/src/utils/ui/theme/phoneApprovalCopy.ts @@ -0,0 +1,48 @@ +// Copyright (C) Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +/** + * Copy for a phone-approval request, shared by `PhoneApprovalCallout` and the + * line-oriented (`--yes`) output so the two can't drift. Lifted out of the Ink + * component so it can be unit-tested without React. + * + * A re-send keeps its step number (the counter numbers approvals, not + * requests — see `signingProxy.ts::createSigningCounter`) and says plainly that + * it is a re-send, with the one remedy that actually fixes a missing prompt. + */ + +import type { SigningEvent } from "../../deploy/signingProxy.js"; + +type SignRequest = Extract; + +/** The usual cause of a request that shows no prompt at all. */ +export const PHONE_RESEND_HINT = + "If no prompt appeared, open the Polkadot app and keep it in the foreground."; + +/** `null` for a first request; otherwise the re-send notice for `attempt`. */ +export function resendNotice(attempt: number): string | null { + return attempt > 1 ? `No response yet — re-sent to your phone (attempt ${attempt}).` : null; +} + +/** One output line (plus the remedy on a re-send) for a sign request. */ +export function formatSignRequestLine(event: SignRequest): string { + if (event.attempt <= 1) { + return ` 📱 Approve on your phone (step ${event.step}): ${event.label}\n`; + } + return ( + ` 📱 No response yet — re-sent to your phone (step ${event.step}, attempt ${event.attempt}): ${event.label}\n` + + ` ${PHONE_RESEND_HINT}\n` + ); +}