wallet: deactivate descriptor #19774

pull S3RK wants to merge 1 commits into bitcoin:master from S3RK:wallet_deactivate_descriptor changing 6 files +58 −0

S3RK commented at 10:58 AM on August 21, 2020: member

Rationale: allow to deactivate wallet descriptor

Currently, it's not possible to deactivate wallet descriptor, active argument is just silently ignored if set to false. When descriptor is deactivated it's still present in the wallet, but can't be used to derive new addresses.

Follow up for #19651
wallet: deactivate descriptor 4deb2ef0e0
fanquake added the label Wallet on Aug 21, 2020
achow101 commented at 3:53 PM on August 21, 2020: member

ACK 4deb2ef0e0c0571a15a8be44bd43fa42f99c089d
DrahtBot commented at 4:06 PM on August 21, 2020: member


The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



Conflicts

Reviewers, this pull request conflicts with the following ones:
- #19651 (wallet: allow import same descriptor twice by S3RK)
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
in src/wallet/wallet.cpp:4492 in 4deb2ef0e0
```
4484 | @@ -4485,6 +4485,21 @@ void CWallet::LoadActiveScriptPubKeyMan(uint256 id, OutputType type, bool intern
4485 |      NotifyCanGetAddressesChanged();
4486 |  }
4487 |  
4488 | +void CWallet::DeactivateScriptPubKeyMan(uint256 id, OutputType type, bool internal)
4489 | +{
4490 | +    WalletLogPrintf("Deactivate spkMan: id = %s, type = %d, internal = %d\n", id.ToString(), static_cast<int>(type), static_cast<int>(internal));
4491 | +    WalletBatch batch(*database);
4492 | +    if (!batch.EraseActiveScriptPubKeyMan(static_cast<uint8_t>(type), internal)) {
```
instagibbs commented at 3:08 PM on August 25, 2020:

It's erasing a spkm of the same output type because the user is importing an inactive descriptor of the same output type? Am I reading this right?

S3RK commented at 10:05 AM on August 26, 2020:

🤦 Yes, unfortunately, you are correct. Another issue is that I can't fix this PR on it's own. I was under impression it can be separate from #19651 but alas. I can't just move EraseActiveScriptPubKeyMan under if (spk_mans[type] && spk_mans[type]->GetID() == id) because the map of active spk managers has been cleared already by this point. So it's impossible to know whether we need to write anything to the disk.

I'll fix this and combine with #19651 where I'm trying to make importdescriptors command do proper updates. Would you mind reviewing that instead?

instagibbs commented at 2:35 PM on August 26, 2020:

ill subscribe to that PR, sure!
S3RK commented at 11:17 AM on August 27, 2020: member

Closed in favor of #19651
S3RK closed this on Aug 27, 2020
S3RK deleted the branch on Sep 25, 2020
DrahtBot locked this on Feb 15, 2022

Contributors

Labels

Linked (view graph)

#19651 wallet: importdescriptors update existing

wallet: deactivate descriptor #19774

Conflicts