Strange behaviour when importing `pk()` descriptors, changed from 0.19 to 0.20

shesek commented at 2:09 pm on June 10, 2020: contributor

In v0.19.1, importing a pk() descriptor resulted in a pkh() descriptor being imported into the wallet with the p2pkh base58 address:

 0$ bitcoin-cli importmulti '[{"desc":"pk(0279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798)#gn28ywm7","watchonly":true,"timestamp":0}]'
 1
 2$ getaddressinfo mrCDrCybB6J1vRfbwM5hemdJz73FwDBC8r
 3{
 4  "address": "mrCDrCybB6J1vRfbwM5hemdJz73FwDBC8r",
 5  "scriptPubKey": "76a914751e76e8199196d454941c45d1b3a323f1433bd688ac",
 6  "ismine": false,
 7  "solvable": true,
 8  "desc": "pkh([751e76e8]0279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798)#6ewytvca",
 9  "iswatchonly": false,
10  "isscript": false,
11  "iswitness": false,
12  "pubkey": "0279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798",
13  "iscompressed": true,
14  "label": "",
15  "ischange": false,
16  "timestamp": 0,
17  "hdkeypath": "m",
18  "hdseedid": "0000000000000000000000000000000000000000",
19  "hdmasterfingerprint": "751e76e8",
20  "labels": [ { "name": "", "purpose": "receive" }
21  ]
22}

I would consider this an unexpected behavior (p2pk should have no address format), but its somewhat consistent with deriveaddresses returning p2pkh addresses for pk() descriptors (which itself also seems wrong?).

Beginning in v0.20, it appears like the p2pkh address is still being added to the wallet, but without any associated descriptor information and with solvable being reported as false:

 0$ bitcoin-cli importmulti '[{"desc":"pk(0279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798)#gn28ywm7","watchonly":true,"label":"L","timestamp":0}]'
 1
 2$ bitcoin-cli getaddressesbylabel L
 3{
 4  "mrCDrCybB6J1vRfbwM5hemdJz73FwDBC8r": {
 5    "purpose": "receive"
 6  }
 7}
 8
 9$ bitcoin-cli getaddressinfo mrCDrCybB6J1vRfbwM5hemdJz73FwDBC8r
10{
11  "address": "mrCDrCybB6J1vRfbwM5hemdJz73FwDBC8r",
12  "scriptPubKey": "76a914751e76e8199196d454941c45d1b3a323f1433bd688ac",
13  "ismine": false,
14  "solvable": false,
15  "iswatchonly": false,
16  "isscript": false,
17  "iswitness": false,
18  "ischange": false,
19  "labels": [
20    "L"
21  ]
22}

Which seems even more unexpected. Also of note is that iswatchonly is reported as false, for both v0.19 and v0.20.

achow101 commented at 4:00 pm on June 10, 2020: member

It looks like this is due to the ScriptPubKeyMan refactor. Previously, 0.19 would just use the CWallet anyways and try to get pubkey info. Now we check whether the address belongs to the wallet before fetching the pubkey info.

Specifically the P2PKH address for a pk() descriptor is not considered watchonly. Because of the refactor, an extra call to IsMine is done before getting a SolvingProvider that it can use to get the pubkey and key origin. Because the P2PKH is not watchonly or spendable, IsMine returns ISMINE::NO, no SolvingProvider is returned so there is no access to the pubkey information.

If you import the address as watchonly, then getaddressinfo works as it does previously.

I’m not sure that this is worth fixing as pk() doesn’t have an address so getaddressinfo really shouldn’t work for the p2pkh address.

shesek commented at 5:50 pm on June 10, 2020: contributor

I’m not sure that this is worth fixing as pk() doesn’t have an address so getaddressinfo really shouldn’t work for the p2pkh address.

Agreed, but I would consider not supporting p2pkh addresses for pk() scripts entirely as fixing it. :) IMHO it shouldn’t work with deriveaddresses and shouldn’t show up on getaddressesbylabel either.

Edit: it would, however, be nice if it was possible to derive scriptPubKeys for descriptors that don’t have an address representation. This would probably require a new RPC call, something like derivescript?

adamjonas added the label Wallet on Aug 3, 2022

aureleoules commented at 6:42 pm on August 11, 2022: member

This issue still persists as of e5d8b654239789d2302bee3eb55d37ec1a3339e6. I was able to reproduce the bug with the steps given in the PR description.

0$ bitcoin-cli -regtest importmulti '[{"desc":"pk(0279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798)#gn28ywm7","watchonly":true,"timestamp":0}]'
1$ bitcoin-cli -regtest getaddressinfo mrCDrCybB6J1vRfbwM5hemdJz73FwDBC8r

achow101 commented at 11:05 pm on October 26, 2022: member

Closing as there is little support for working on fixing non-critical issues in legacy wallets. This is, for the most part, unsupported behavior.

achow101 closed this on Oct 26, 2022

bitcoin locked this on Oct 26, 2023

Strange behaviour when importing pk() descriptors, changed from 0.19 to 0.20 #19236

Strange behaviour when importing `pk()` descriptors, changed from 0.19 to 0.20 #19236