importprivkey unsets labels set with importaddress and importpubkey

[harding]

Expected behavior:

1. importaddress or importpubkey a watching-only address or pubkey with a label
2. Receive a transaction to that address; observe that the label has been applied to that transaction
3. importprivkey the private key for that address without specifying any change to the label
4. Have the previous label still applied to the transaction after the import.

Observed behavior on 0.15 and current master:

Steps 1-3 the same.
4. Previous label is replaced with the default empty label ("")

Problem seems to have occurred on 0.15 and affects the latest master (see subsequent details). I'd suspect all versions are affected, i.e. it's not a regression.

Steps to reproduce

## For testing
ADDRESS=2N6Y2wdhd91mQnZ8qCeH6hT5aZZygXFnNjP
PRIVKEY=cQriWp98xU9ghRvDKbzkRcgcecQLezjYwtPUjLNN5kzT8GbdnxQt

## Starting a fresh regtest (i.e. ~/.bitcoin/regtest/ previously deleted)
$ bitcoind -daemon -regtest
Bitcoin server starting

## Import the address with a label and no rescan
$ bitcoin-cli -regtest importaddress $ADDRESS "Watch-only test" false

## Mine a generation tx to the address (regular spending works too)
$ bitcoin-cli -regtest generatetoaddress 1 $ADDRESS > /dev/null

## Bury the generation tx so it shows up in the wallet
$ bitcoin-cli -regtest generate 100 > /dev/null

## See that the address label was applied to the tx
$ bitcoin-cli -regtest listunspent | jq .[].label
"Watch-only test"

## Import the private key.  Note the 'label' parameter is not explicitly passed
$ bitcoin-cli -regtest importprivkey $PRIVKEY

## See that the address label has been set to empty :-(
$ bitcoin-cli -regtest listunspent | jq .[].label

Same thing happens if importpubkey is substituted for importaddress. I haven't tested importmulti for either address/pubkey or privkey import, nor have I tested using named parameters.

Use case

I was watching-only addresses from my cold wallet, then moved my coins to new addresses and (after some confirmations) imported the old private keys so that I could sell fork coins. I didn't realize until now when I was trying to do some bookkeeping that all my old cold wallet labels were reset (happily, I have backups, so nothing is lost but time).

Possibly related issues

• On testnet, a dumpwallet/importwallet cycle loses all labels #12698 looks related but the poster there says that it works on mainnet. I discovered my problem from actual use on mainnet and reproduced on regtest.

[promag]

IIUC this is the intended behaviour:

importprivkey "privkey" ( "label" ) ( rescan )
...
2. "label"            (string, optional, default="") An optional label

Are you suggesting to change the default behaviour: default=current label if watch only address otherwise ""?

[harding]

@promag that's a good argument for it being the intended behavior, so perhaps this should be considered a feature request rather than a bug. However, it does seem undesirable to me for key import to destroy possibly-important metadata previously entered by the user.

[promag]

Yes, either way it should have a way to preserve the current label.

[marcoagner]

I'm trying to address this on #13381
Could you, please, check if the approach seems reasonable and the behavior is an improvement for the expected use case? Thanks.