external signer: PSBT error code `EXTERNAL_SIGNER_NOT

theStack commented at 1:25 pm on May 6, 2025: contributor

The error code PSBTError::EXTERNAL_SIGNER_NOT_FOUND is never returned anywhere in our codebase, hence code that handles it (currently done only in the GUI) is effectively dead. I have never looked deeper into external signing, but at a first glance the two obvious logical choices would be to either:

simply get rid of this error code and its handling (if e.g. a more generic error thrown in this case already provides enough information to the user)
implement the returning of the error condition; the areas that need to be touched are likely ExternalSignerScriptPubKeyMan::GetExternalSigner and its call-site in ::FillPSBT below

(Noticed while reviewing #31622, where new PSBTError codes are added in the third commit)

naiyoma commented at 10:52 am on June 3, 2025: contributor

simply get rid of this error code and its handling (if e.g. a more generic error thrown in this case already provides enough information to the user)

implement the returning of the error condition; the areas that need to be touched are likely ExternalSignerScriptPubKeyMan::GetExternalSigner and its call-site in ::FillPSBT below

I’m in favour of deleting PSBTError::EXTERNAL_SIGNER_NOT_FOUND

I’ve tested the error on GetExternalSigner in both walletdisplayaddress and walletprocesspsbt it seems sufficient to me.

0bitcoin-cli -regtest -rpcwallet=hww_working walletprocesspsbt "$PSBT"
1error code: -1
2error message:
3GetExternalSigner: No external signers found

0bitcoin-cli -regtest -rpcwallet=hww_working walletdisplayaddress $ADDRESS
1error code: -1
2error message:
3GetExternalSigner: No external signers found

Even if we return PSBTError::EXTERNAL_SIGNER_NOT_FOUND; in ExternalSignerScriptPubKeyMan::FillPSBT we’ll still need to keep the error in GetExternalSigner, since it’s also useful in other call sites.

maybe the intention of having PSBTError::EXTERNAL_SIGNER_NOT_FOUND error was to have a more PSBT-specific error, But honestly, that doesn’t seem very necessary.

I tried returning from FillPSBT directly, and it’s possible to do that without touching GetExternalSigner. But I still think deleting is the better approach

 0-    if (finalize) FinalizePSBT(psbt); // This won't work in a multisig setup
 1+
 2+    try {
 3+        ExternalSigner signer = GetExternalSigner();
 4+        if(!signer.SignTransaction(psbt, strFailReason)) {
 5+            tfm::format(std::cerr, "Failed to sign: %s\n", strFailReason);
 6+            return PSBTError::EXTERNAL_SIGNER_FAILED;
 7+        }
 8+    } catch (const std::runtime_error& e) {
 9+        return PSBTError::EXTERNAL_SIGNER_NOT_FOUND;
10+    }
11+    if (finalize) {
12+        FinalizePSBT(psbt); // This won't work in a multisig setup
13+
14+    }

fanquake commented at 11:05 am on June 3, 2025: member

@hebasto @Sjors

Sjors commented at 8:58 am on June 4, 2025: member

I made a note to refactor GetExternalSigner() and ExternalSignerScriptPubKeyMan::FillPSBT so the latter can return PSBTError::EXTERNAL_SIGNER_NOT_FOUND.

We call GetExternalSigner in three scenarios:

During initial wallet setup to fetch keys from the device
When attempting to display an address on the device
When signing a PSBT

For scenario (1) and (2) the current std::runtime_error approach seems fine.

In particular scenario (1) is unlikely, and definitely an error, if a user unplugs their device right after we fetched its name (via ExternalSigner::Enumerate).

Scenario (2) is more likely to occur if e.g. they plug in their device, forget to unlock it, or somehow HWI can’t find it.

That leaves (3) which is more nuanced. The external signer interface “officially” only supports single signature setup, but my longer term ambition still is to make this work well in multisig. And that’s where we should use PSBTError::EXTERNAL_SIGNER_NOT_FOUND.

Coincidentally I ran into the No external signers found std::error during recent MuSig2 testing. Let’s say you have a 2-of-2 setup with one key from Bitcoin Core and one from an external signer, and a delayed fallback where either can sign. Normally it’s an error when the device isn’t present, but for the fallback it’s not.

I’m not entirely sure yet how such a case should be handled. The signing code doesn’t know that the user lost their hardware wallet, and we don’t want to accidentally publish a scriptPath spend since that hurts privacy and is more expensive.

One approach could be that if PSBTError::EXTERNAL_SIGNER_NOT_FOUND is returned, we ask the user whether they want to try again or if we should only sign the fallback condition.

It looks like I introduced TransactionError::EXTERNAL_SIGNER_NOT_FOUND in d4b0107d68a91ed4d1a5c78c8ca76251329d3f3c as part of #16546, followed by https://github.com/bitcoin-core/gui/pull/4 that renders them. But no code actually triggered the error. I’m not sure why. Perhaps an earlier iteration did use them, but browsing through some of those versions I don’t see it. Neither did the immediate followup #16378.

Sjors referenced this in commit 0a4ee93529 on Jun 5, 2025

Sjors commented at 10:08 am on June 5, 2025: member

Fixed in #32682. It has the nice benefit of actually using the existing translations for “External signer not found”.

achow101 closed this on Jun 17, 2025

achow101 referenced this in commit 1be688f575 on Jun 17, 2025

external signer: PSBT error code EXTERNAL_SIGNER_NOT_FOUND is never returned #32426

external signer: PSBT error code `EXTERNAL_SIGNER_NOT_FOUND` is never returned #32426