diff --git a/docs/developer/api.md b/docs/developer/api.md index 0f2ef019..e1c85412 100644 --- a/docs/developer/api.md +++ b/docs/developer/api.md @@ -663,11 +663,19 @@ history, and Core shows when its wallets are still catching up. The GUI runs the same check before `createwallet` for a restore, and `ms32 create --existing` asks when the seed was first used instead of rescanning from 0. +Core picks its first block by the highest block time so far, which no RPC +reports, so an out-of-order block time can still pass that check. Core then +reports "Rescan failed" for the descriptors. `initialize` then asks +`unloadwallet` to close the wallet and drop it from startup, and says not to +use it and to close it whenever it is open, because that unload can fail or be +undone at the next start. Only an error that blames pruning asks for a later +date; any other failed scan asks for the same date again. + The Core calls are fixed: `getnetworkinfo`, `getblockchaininfo`, `getblockstats`, `getchainstates`, `listwallets`, `getwalletinfo`, `listdescriptors`, `getdescriptorinfo`, `deriveaddresses`, -`validateaddress`, `importdescriptors`, `gethdkeys`, `derivehdkey`, and -`walletlock`. Bitcoin Core alone creates wallets, selects encryption, handles +`validateaddress`, `importdescriptors`, `gethdkeys`, `derivehdkey`, +`unloadwallet`, and `walletlock`. Bitcoin Core alone creates wallets, selects encryption, handles passphrases, stores keys, and provides normal wallet behavior. The wallet CLI is one leaf command: diff --git a/docs/user/gui.md b/docs/user/gui.md index 6955e97f..9971940a 100644 --- a/docs/user/gui.md +++ b/docs/user/gui.md @@ -175,7 +175,10 @@ blank to search all history. If the node has already pruned blocks the search would need, the window says so before it changes anything in Bitcoin Core; a date after the pruned blocks works, and otherwise the node must download the blockchain again. A node started from an AssumeUTXO snapshot lacks older blocks -until it finishes checking them, so the window asks you to restore after that. +until it finishes checking them, so the window asks you to restore after that. In +the rare case Bitcoin Core finds missing blocks only while it searches, the +window says so and asks Bitcoin Core to close that wallet. Do not use it, close +it if it opens again, and restore again into a new wallet. A new wallet has no history to search, so it is made and loaded at once; Bitcoin Core itself shows that it is still catching up. New wallets made here are loaded each time Bitcoin Core diff --git a/src/codex32/_bitcoin_core.py b/src/codex32/_bitcoin_core.py index 26380b1f..b2689ea5 100644 --- a/src/codex32/_bitcoin_core.py +++ b/src/codex32/_bitcoin_core.py @@ -1,6 +1,7 @@ from __future__ import annotations import calendar +import contextlib import hashlib import json import re @@ -359,16 +360,12 @@ def _target(self, name: str) -> tuple[bool, bool] | None: unlocked = info.get("unlocked_until") return (unlocked is not None, unlocked == 0) if eligible else None - def _select( - self, - ask: Callable[[str], str], - tell: Callable[[str], None], - ) -> str: + def _select(self, ask: Callable[[str], str], tell: Callable[[str], None]) -> str: + def agree(name: str) -> bool: + return ask(f"Use blank wallet {json.dumps(name)}? [y/N]").lower() in ("y", "yes") + choices = tuple(name for name in sorted(self._names()) if self._target(name) is not None) - if len(choices) == 1 and ask(f"Use blank wallet {json.dumps(choices[0])}? [y/N]").lower() in ( - "y", - "yes", - ): + if len(choices) == 1 and agree(choices[0]): return choices[0] while True: if choices: @@ -379,10 +376,7 @@ def _select( answer = ask("Choose a wallet number") if answer.isdecimal() and 1 <= int(answer) <= len(choices): name = choices[int(answer) - 1] - if ask(f"Use blank wallet {json.dumps(name)}? [y/N]").lower() in ( - "y", - "yes", - ): + if agree(name): return name continue if answer != str(len(choices) + 1): @@ -397,15 +391,11 @@ def _select( while True: sleep(1) names = set(self._names()) - new = names - before - appeared = tuple(name for name in sorted(new) if self._target(name) is not None) + appeared = tuple(name for name in sorted(names - before) if self._target(name) is not None) if appeared: break choices = tuple(name for name in sorted(names) if self._target(name) is not None) - if len(appeared) == 1 and ask(f"Use blank wallet {json.dumps(appeared[0])}? [y/N]").lower() in ( - "y", - "yes", - ): + if len(appeared) == 1 and agree(appeared[0]): return appeared[0] def initialize( @@ -478,6 +468,19 @@ def initialize( and all(isinstance(item, dict) and item.get("success") is True for item in imported) ) if not valid: + # Core keeps keys whose rescan hit missing blocks, so close that wallet; it lacks history. + if isinstance(imported, list) and any("Rescan failed" in str(i) for i in imported): + # Not loaded on startup either. Core may still fail, or warn that it will load it again, + # so the message below holds whether or not this closed it. + with contextlib.suppress(BitcoinCoreError): + self._rpc("unloadwallet", name, "false") + relock = False # An unloaded wallet holds no unlocked keys. + # Only Core's pruning hint means the date is too early; an aborted scan needs a retry. + date = "a later date, if recorded" if "pruning" in str(imported) else "the same date" + raise BitcoinCoreError( + f"Bitcoin Core could not search all of {name}'s history. Do not use that wallet, and close " + f"it in Bitcoin Core whenever it is open. Restore into a new wallet with {date}." + ) raise BitcoinCoreError("Bitcoin Core did not import every private descriptor.") public = self.public_descriptors( secret, diff --git a/src/codex32/cli.py b/src/codex32/cli.py index 7b10a8b0..7214cc9b 100644 --- a/src/codex32/cli.py +++ b/src/codex32/cli.py @@ -364,6 +364,8 @@ def _creation_start() -> int: while True: try: return parse_creation_date(_text(_FIRST_USED, optional=True)) + except KeyboardInterrupt as error: # Nothing was made yet: the existing cards stay valid. + raise _CoreSelectionInterrupted from error except ValueError as error: _print(str(error), err=True) diff --git a/src/codex32_gui/wallet_setup.py b/src/codex32_gui/wallet_setup.py index a9cf6d32..b8e540d6 100644 --- a/src/codex32_gui/wallet_setup.py +++ b/src/codex32_gui/wallet_setup.py @@ -7,6 +7,7 @@ from __future__ import annotations import codecs +import contextlib import json import locale import re @@ -228,14 +229,8 @@ def create(core: BitcoinCore, name: str, passphrase: str) -> tuple[str, ...]: raise BitcoinCoreError( "Bitcoin Core would not create a wallet with that name. A wallet of that name may exist already." ) from error - notes = created.get("warnings", []) if isinstance(created, dict) else None - if ( - not isinstance(created, dict) - or created.get("name") != name - or core._target(name) is None - or type(notes) is not list - or any(type(w) is not str for w in notes) - ): + notes = created.get("warnings", []) if isinstance(created, dict) and created.get("name") == name else None + if type(notes) is not list or any(type(w) is not str for w in notes) or core._target(name) is None: raise BitcoinCoreError("Bitcoin Core did not return an empty wallet and its warnings.") return tuple(notes) @@ -246,6 +241,9 @@ def relock(core: BitcoinCore, name: str) -> None: core._rpc("walletlock", wallet=name) state = core._rpc("getwalletinfo", wallet=name) except BitcoinCoreError as error: + with contextlib.suppress(BitcoinCoreError): # Only a listing without it proves it was unloaded. + if name not in core._names(): # As after a rescan that missed blocks: no keys are open. + return raise BitcoinCoreError(_STILL_OPEN) from error if not isinstance(state, dict) or state.get("unlocked_until") != 0: raise BitcoinCoreError(_STILL_OPEN) diff --git a/tests/test_bitcoin_core.py b/tests/test_bitcoin_core.py index 39bc291b..f0ecee7e 100644 --- a/tests/test_bitcoin_core.py +++ b/tests/test_bitcoin_core.py @@ -451,6 +451,40 @@ def fail( assert any(arguments == ("walletlock",) for arguments, _wallet, _stdin in rpc.calls) +@pytest.mark.parametrize("unloaded", [{"warnings": []}, {"warnings": ["load on startup not updated"]}, None]) +@pytest.mark.parametrize(("cause", "hint"), [(" caused by pruning", "a later date"), ("", "the same date")]) +def test_rescan_on_missing_blocks_closes_the_wallet( + monkeypatch: pytest.MonkeyPatch, unloaded: dict[str, list[str]] | None, cause: str, hint: str +) -> None: + rpc = _ImportRPC(locked=False) + original = rpc.__call__ + failed = {"success": False, "error": {"code": -1, "message": f"Rescan failed ...{cause}"}} + + def fail( + client: BitcoinCore, *arguments: str, wallet: str | None = None, stdin: str | None = None + ) -> object: + rpc.calls.append((arguments, wallet, stdin)) + if arguments == ("importdescriptors",): + return [failed] * 4 + if arguments[0] == "unloadwallet": + if unloaded is None: + raise BitcoinCoreError("Bitcoin Core rejected the requested wallet operation.") + return unloaded + return original(client, *arguments, wallet=wallet, stdin=stdin) + + monkeypatch.setattr(BitcoinCore, "_rpc", fail) + client = BitcoinCore("bitcoin-cli", "main", 300000) + with pytest.raises(BitcoinCoreError, match="all of signer's history. Do not use that wallet") as caught: + client.initialize( + _SEED, lambda _prompt: "yes", lambda _message: None, expected_fingerprint=_FINGERPRINT + ) + assert "close it in Bitcoin Core whenever it is open" in str(caught.value) + assert f"new wallet with {hint}" in str(caught.value) + assert (("unloadwallet", "signer", "false"), None, None) in rpc.calls + # A wallet that stayed loaded is locked again; an unloaded one has nothing left to lock. + assert any(arguments == ("walletlock",) for arguments, _wallet, _stdin in rpc.calls) is (unloaded is None) + + def test_fingerprint_uses_stateless_core_address_derivation(monkeypatch: pytest.MonkeyPatch) -> None: calls: list[tuple[tuple[str, ...], str | None]] = [] diff --git a/tests/test_cli.py b/tests/test_cli.py index a56476e6..24319580 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -2365,6 +2365,22 @@ def test_an_unusable_first_use_date_is_asked_again(monkeypatch: pytest.MonkeyPat assert "YYYY-MM-DD" in errors.getvalue() +def test_interrupting_the_first_use_date_does_not_void_existing_cards() -> None: + core = _FakeBitcoinCore() + errors = _TTYOutput() + with ( + patch.object(sys, "stdin", _TTYInput()), + patch("codex32.cli._creation_source", return_value=parse_codex32(VECTOR_1["secret_s"])), + patch("codex32.cli._text", side_effect=KeyboardInterrupt), + patch("codex32.cli.BitcoinCore.connect", return_value=core), + contextlib.redirect_stdout(_TTYOutput()), + contextlib.redirect_stderr(errors), + ): + assert ms_main(["create", "--existing"]) == 130 + assert "void" not in errors.getvalue() and "Interrupted." in errors.getvalue() + assert core.imported is None + + def test_create_existing_secret_does_not_silently_change_identifier() -> None: result = _invoke_confirmed_create(["create", "0test", "--existing"], VECTOR_4["secret_s"]) assert result.exit_code == 2 and result.stdout == "" diff --git a/tests/test_gui_wallet_setup.py b/tests/test_gui_wallet_setup.py index 97379a2b..fcda8da9 100644 --- a/tests/test_gui_wallet_setup.py +++ b/tests/test_gui_wallet_setup.py @@ -51,6 +51,8 @@ def run(self, command: list[str], **keywords: Any) -> subprocess.CompletedProces if supplied is not None and "-stdin" in options: arguments += lines name = next((item[11:] for item in options if item.startswith("-rpcwallet=")), None) + if name is not None and name not in self.wallets: # Core refuses a wallet that is not loaded. + return subprocess.CompletedProcess(command, 18, "", "Requested wallet does not exist") return subprocess.CompletedProcess(command, 0, self._reply(arguments[0], arguments[1:], name), "") def _reply(self, method: str, arguments: list[str], name: str | None) -> str: @@ -294,6 +296,31 @@ def refuse(*_arguments: object, **_keywords: object) -> str: assert fake.wallets["fresh"].locked +def test_a_wallet_closed_for_missing_history_keeps_its_own_message(monkeypatch: pytest.MonkeyPatch) -> None: + core, fake = _client(monkeypatch, {"fresh": _Wallet(True, True)}) + + def unload(*_arguments: object, **_keywords: object) -> str: + del fake.wallets["fresh"] + raise BitcoinCoreError("fresh misses history and was closed.") + + monkeypatch.setattr(wallet_setup, "initialize", unload) + with pytest.raises(BitcoinCoreError, match="misses history"): + wallet_setup.fill(core, _SEED, "fresh", PASSPHRASE, expected=None) + + +def test_a_failed_relock_still_asks_for_a_check_when_core_stops_answering( + monkeypatch: pytest.MonkeyPatch, +) -> None: + core, _fake = _client(monkeypatch, {"fresh": _Wallet(True, False)}) + + def down(*_arguments: object, **_keywords: object) -> object: + raise BitcoinCoreError("The local Bitcoin Core command did not complete.") + + monkeypatch.setattr(BitcoinCore, "_rpc", down) + with pytest.raises(BitcoinCoreError, match="Confirm in Bitcoin Core that the wallet is locked"): + wallet_setup.relock(core, "fresh") + + def test_identity_mismatch_is_refused_before_unlock(monkeypatch: pytest.MonkeyPatch) -> None: core, fake = _client(monkeypatch, {"fresh": _Wallet(True, True)})