Let a bunker code be re-used: wait for a person, and close what we replace #9
Loading…
Reference in a new issue
No description provided.
Delete branch "fix/bunker-relogin"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Reported by @laoc42 from a real phone: signing in with a
bunker://code worked, and signing in again with the same code did not.Stacks on #8 (
feat/nsec-login).What is actually happening
A
bunker://code carries a secret, and NIP-46 spends it on the first connection:So the first pairing is approved by a machine and is instant. Every later one arrives with nothing to pre-approve it and has to be approved by a person — unlock another device, find the prompt, tap it.
Stash waited 20 seconds for that. The comment on the constant said so out loud: "the unauthorised-secret path spends exactly this before giving up." It was chosen for the machine.
What this changes
The pairing wait is now two minutes, with a Cancel button. A long wait with no way out is its own defect, which is why the button is part of the same change rather than a follow-up. Restoring a stored pairing keeps the short bound and gains a name that says why —
RESTORE_TIMEOUT, because nobody is being asked anything there and it runs on every cold start.A replaced signer is now closed.
NostrConnectSignerowns live relay connections and dropping the reference does not drop them: every sign-out and every re-pairing left a websocket open for the life of the process, still subscribed under the signed-out account's client key. Closing happens in thesignersetter rather than at three call sites, andsignEventrefuses on a closed signer instead of leaving "signed out means cannot sign" to whatever the binding does with a disposed handle.The failure message named neither cause. It now names the spent secret first, because it is the one a user cannot deduce.
What the tests say, and what they do not
BunkerReloginE2ETestis not a red-first regression test and is not presented as one. Grafted onto the base commit73d51e1it passes there too —expected=1 started=1 passed=1 failed=0, in 2.3 seconds.That is the point of it.
nak bunkerre-approves its authorised secret, so against that fixture the protocol permits the second pairing and only Stash could refuse it. A green says the sign-in path itself was never the blocker — which is what narrows the report to the approval window, and is the only way I could separate "our code cannot do this" from "the signer would not let it".The two tests that wait on a bunker that will not answer now pass their own 20s bound, so the suite does not spend four minutes proving it. The shipped constants get their own assertions instead: a constant nothing varies is a constant nothing is pinning.
What I have not proven
That this fixes it on @laoc42's phone. If their signer prompts when a used code is re-pasted, two minutes is enough and this is the fix. If it silently ignores the reused code — which the spec says it SHOULD — then no wait is long enough, and the answer is a fresh code, which is what the new message now says. Those two look identical from here and different from there.
Test APK is in Nextcloud:
stash-0.12.0-bunker-relogin-debug.apk, versionCode 19, shabfd6c57d….Adjacent, not fixed here
UiState.bunkerUnavailableis set byrestoreBunkerand read by nothing — zero references outsideMainViewModel. So a cold start against a sleeping bunker currently tells the user nothing at all. Left alone deliberately; it is a separate report.Reported from a real phone (laoc, 2026-08-06): signing in with a `bunker://` code worked, and signing in again with the same code did not. Two defects, both ours, both producing exactly that. **The pairing wait was 20 seconds.** NIP-46 spends the secret in a bunker code on the first connection — "Optional secret can be used for single successfully established connection only, remote-signer SHOULD ignore new attempts to establish connection with old secret" — so the first pairing is approved by a machine and is instant, and every later one has to be approved by a *person*: unlock another device, find the prompt, tap it. 20s was chosen for the machine, and the comment on the constant said so out loud ("the unauthorised-secret path spends exactly this before giving up"). It is now two minutes, with a Cancel button on the login screen, because a long wait without a way out is its own defect. Restoring a stored pairing keeps the short bound and gains a name that says why: nobody is being asked anything there. **A replaced signer was never closed.** `NostrConnectSigner` owns live relay connections and dropping the reference does not drop them, so every sign-out and every re-pairing left a websocket open for the life of the process, still subscribed for the signed-out account's client key. Closing now happens in the `signer` setter rather than at three call sites, and `signEvent` refuses on a closed signer rather than leaving "signed out means cannot sign" to whatever the binding does with a disposed handle. The failure message named neither cause. It now names the spent secret first, because it is the one the user cannot deduce. Measured, device suite at this commit: 81/81, started == numtests. `BunkerReloginE2ETest` is not a red-first regression test and is not presented as one: grafted onto the base commit it passes there too. That is its point. `nak bunker` re-approves its authorised secret, so against that fixture the protocol permits the second pairing and only Stash could refuse it — a green says the sign-in path itself was never the blocker, which is what narrows the report to the approval window. The two tests that wait on a bunker that will not answer now pass their own 20s bound, so the suite does not spend four minutes proving it. The shipped constants get their own assertions instead: a constant nothing varies is a constant nothing is pinning.Verification re-run and re-attributed. The 81/81 in the description was measured before the final commit existed (an import reorder and the versionCode bump landed after it), so it was quoted against a SHA that had not been run.
Re-run at the pushed tip, clean tree, same shell:
Same numbers, now attributable. One attempt in between died with
DEVICE NEVER APPEARED/protocol fault (couldnt read status)— adb contention with a concurrent run on the same AVD, not a test result; noted so a reader of the logs does not count it as a red.