createwallet allows you to create many wallets for the same external signer

josibake commented at 4:59 pm on August 5, 2021: member

While testing external signers in 22.0rc2, I noticed that I can create many watch only wallets with the external signer option, each with a different name, but they will all be tied to the same external signer. Not sure if this should be a bug, but it is definitely unexpected and leads to misleading information in the GUI/RPC commands. At the very least, perhaps all that’s needed is better documentation on how external signers work with Bitcoin Core to avoid confusion, which I’d be happy to write.

Expected behavior

If createwallet allows me to create 3 different wallets with different names, I expect them to be different wallets:

0./src/bitcoin-cli -datadir=$DATA_DIR -signet listwallets
1[
2  "trezor_1",
3  "trezor_2",
4  "trezor_3"
5]

If a wallet has already been created with the existing external signer, I would expect new wallet creation to fail, even if the wallet name is different, or create a new wallet on the same external signer but be under a different account (derviation path)

**Actual behavior

The three wallets above are all attached to the same external signer and account, even tho they are shown as separate wallets in the cli and the gui.

 0./src/bitcoin-cli -datadir=$DATA_DIR -signet -rpcwallet=trezor_1 getwalletinfo
 1{
 2  "walletname": "trezor_1",
 3  "walletversion": 169900,
 4  "format": "sqlite",
 5  "balance": 0.01799890,
 6  "unconfirmed_balance": 0.00000000,
 7  "immature_balance": 0.00000000,
 8  "txcount": 7,
 9  "keypoolsize": 3000,
10  "keypoolsize_hd_internal": 3000,
11  "paytxfee": 0.00000000,
12  "private_keys_enabled": false,
13  "avoid_reuse": true,
14  "scanning": false,
15  "descriptors": true
16}

and

 0./src/bitcoin-cli -datadir=$DATA_DIR -signet -rpcwallet=trezor_2 getwalletinfo
 1{
 2  "walletname": "trezor_2",
 3  "walletversion": 169900,
 4  "format": "sqlite",
 5  "balance": 0.01799890,
 6  "unconfirmed_balance": 0.00000000,
 7  "immature_balance": 0.00000000,
 8  "txcount": 7,
 9  "keypoolsize": 3000,
10  "keypoolsize_hd_internal": 3000,
11  "paytxfee": 0.00000000,
12  "private_keys_enabled": false,
13  "avoid_reuse": true,
14  "scanning": false,
15  "descriptors": true
16}

To reproduce

Setup HW wallet using an external signer. The steps I followed are here: https://gist.github.com/josibake/9799b348e261e81881fa621d0c9d8dfb#testing-wallet-changes
Create two separate wallets with the same external signer

0./src/bitcoin-cli -datadir=$DATA_DIR -signet createwallet "trezor_1" true true "" true true true true
1./src/bitcoin-cli -datadir=$DATA_DIR -signet createwallet "trezor_2" true true "" true true true true

Send a transaction to one of the wallets
Verify both wallets have the same transaction listed using getwalletinfo
If you have previous transactions on the external signer, you will need to run rescanblockchain for each and you can also verify they have exactly the same information

System information

Using https://github.com/bitcoin/bitcoin/releases/tag/v22.0rc2 (self compiled) on:

OS: Pop!_OS 20.10 (Ubuntu 20.10) OS Type: 64 bit GNOME: 3.83.3

josibake added the label Bug on Aug 5, 2021

achow101 commented at 5:34 pm on August 5, 2021: member

I don’t think this is really a bug or an issue that needs fixing. We’ve always allowed users to have multiple wallets containing the same keys. You could always import your keys and descriptors into another wallet that’s already loaded.

Furthermore, wallets are independent containers. I don’t think we should disallow what one wallet can contain just because another wallet is loaded that has the same things in it. This would also be easily bypassable by simply closing the other wallets before creating the duplicates.

josibake commented at 6:45 pm on August 5, 2021: member

We’ve always allowed users to have multiple wallets containing the same keys

the main difference here is now if I generate a new key in wallet A, that key will also show up in wallet B without me importing anything. AFAIK, this is different from before in that I could not create an HD wallet, and then create a new wallet based off the same seed such that all new addresses would now appear in both wallets. This seems to go against the idea of wallets being independent containers.

As I noted, perhaps this isn’t a bug and I’d be happy to write something in the documentation to avoid confusion for the user, but this does seem like new behavior introduced by external signers.

achow101 commented at 6:55 pm on August 5, 2021: member

the main difference here is now if I generate a new key in wallet A, that key will also show up in wallet B without me importing anything.

I disagree. IMO the act of creating an external signer wallet is equivalent to an explicit import. It’s not a default action - you have to explicitly choose to create an external signer wallet, in the same way that importing is an explicit decision. The difference is instead of looking at the thing being imported you are just telling Bitcoin Core to figure out what to import automatically. In both cases, you are choosing to make duplicate wallets; when importing, you choose to import the same thing, when using an external signer, you choose to use the same external signer.

AFAIK, this is different from before in that I could not create an HD wallet, and then create a new wallet based off the same seed such that all new addresses would now appear in both wallets. This seems to go against the idea of wallets being independent containers.

Actually you can, although it’s a little bit more complicated. You can get the seed for a wallet via dumpwallet, then create a new blank wallet, and then use sethdseed so the new wallet has exactly the same keys as the original.

Rspigler commented at 8:45 pm on August 5, 2021: contributor

I would think a user would expect a new wallet creation to be on a different account? (I believe that’s how it works on the HWW’s own software). Is there not a way to do that with HWI/Core, and if not, maybe documentation would be good.

achow101 commented at 9:06 pm on August 5, 2021: member

I would think a user would expect a new wallet creation to be on a different account?

I’m not sure that many users have a concept of what different accounts are. I don’t think users would assume or expect that, but if they do, we should clarify that in documentation. We should not inspect other wallets in order to limit what a new wallet can contain.

(I believe that’s how it works on the HWW’s own software).

Perhaps, but from my experience with hardware vendor wallet software, the wallets do not behave as independent entities as we have implemented. Electrum behaves similarly to us (if you leave the defaults, you will duplicate the wallet for the same hardware signer).

Is there not a way to do that with HWI/Core, and if not, maybe documentation would be good.

Not yet.

josibake commented at 11:25 am on August 9, 2021: member

I’m not sure that many users have a concept of what different accounts are. I don’t think users would assume or expect that, but if they do, we should clarify that in documentation.

This was my expectation, which is why I was confused when both wallet’s contained the same addresses. Perhaps this will be less confusing when HWI/Core does support creating separate accounts and the documentation is more clear?

We should not inspect other wallets in order to limit what a new wallet can contain.

Definitely agree with this and I think I understand your previous point about duplicate wallets a bit better.

Thanks for taking the time to respond, I learned quite a bit from the discussion. Happy to close this issue unless someone feels there is an actionable next step (i.e documentation edits)?

hebasto commented at 11:29 am on August 9, 2021: member

someone feels there is an actionable next step (i.e documentation edits)?

Probably, better to open a dedicated issue with clear goals and expectations?

Rspigler commented at 7:40 pm on August 9, 2021: contributor

I would, but I don’t have a HWW to test HWI with.

I agree I think documentation is the way forward.

josibake commented at 6:05 pm on August 10, 2021: member

@Rspigler here’s an emulator i used for testing, a bit of work to get setup but really helpful for testing: https://docs.trezor.io/trezor-firmware/core/emulator/index.html

there’s also some instructions on using it for testing HWI + core here: https://github.com/bitcoin-core/bitcoin-devwiki/wiki/22.0-Release-Candidate-Testing-Guide#testing-wallet-changes

closing this issue in favor of #22678

josibake closed this on Aug 10, 2021

DrahtBot locked this on Aug 18, 2022

createwallet allows you to create many wallets for the same external signer #22635