Replace CAffectedKeysVisitor with descriptor based logic #14821

1. 
   sipa commented at 8:30 pm on November 27, 2018: member

   It seems we don’t need a custom visitor pattern anymore to find what keys are affected by a script. Instead, infer the descriptor, and see which keys it expands to.
2. Replace CAffectedKeysVisitor with descriptor based logic 0e75f44a09
3. MarcoFalke added the label Refactoring on Nov 27, 2018
4. MarcoFalke added the label Wallet on Nov 27, 2018
5. 
   instagibbs commented at 8:51 pm on November 27, 2018: member

   +13 −67

   concept ACK, note that this will now include pubkeys you have, not only private keys, which I think is a more sane general anyways.

6. 
   DrahtBot commented at 10:31 pm on November 27, 2018: 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:

   • #14144 (Refactoring: Clarify code using encrypted_batch in CWallet by domob1812)

   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.

7. 
   meshcollider commented at 6:16 am on November 28, 2018: contributor

   Concept ACK
8. in src/wallet/wallet.cpp:108 in 0e75f44a09

   159-    {
   160-        CKeyID id(keyid);
   161-        if (keystore.HaveKey(id)) {
   162-            vKeys.push_back(id);
   163-        }
   164+std::vector<CKeyID> GetAffectedKeys(const CScript& spk, const SigningProvider& provider)
   

   ---

   ---

   

   promag commented at 10:10 am on November 28, 2018:

   nit, static. Could also keep some comment.

   ---

   

   instagibbs commented at 3:42 pm on November 28, 2018:

   Could even make it more specific with the updated behavior.

   ---

   

   Empact commented at 7:35 am on December 7, 2018:

   nit: script_pub_key would be a more readable arg name. Granted I’m a noob but I had to check the callers.
9. promag changes_requested
10. 
    promag commented at 10:20 am on November 28, 2018: member

    utACK 0e75f44.
11. 
    meshcollider commented at 9:17 pm on November 30, 2018: contributor

    utACK https://github.com/bitcoin/bitcoin/pull/14821/commits/0e75f44a09ed07ff5eeeb142a4cefc4aba0ddf32
12. 
    instagibbs commented at 8:17 am on December 2, 2018: member

    @sipa if you’d like you could take this test: https://github.com/instagibbs/bitcoin/commit/a80edd7e689e89e40d4ba98f3770c5da2bc8d174
13. 
    jonasschnelli commented at 11:18 pm on December 9, 2018: contributor

    utACK 0e75f44a09ed07ff5eeeb142a4cefc4aba0ddf32 also in favour to pull in https://github.com/instagibbs/bitcoin/commit/a80edd7e689e89e40d4ba98f3770c5da2bc8d174
14. 
    instagibbs commented at 2:09 pm on December 10, 2018: member

    @jonasschnelli I ended up PRing it, that commit is bad anyways: https://github.com/bitcoin/bitcoin/pull/14857
15. 
    sipa commented at 10:04 pm on December 13, 2018: member

    @instagibbs #14857 is merged, any suggested improvements to this PR left?
16. 
    instagibbs commented at 10:07 pm on December 13, 2018: member

    sorry! no

    utACK https://github.com/bitcoin/bitcoin/pull/14821/commits/0e75f44a09ed07ff5eeeb142a4cefc4aba0ddf32

17. meshcollider merged this on Dec 14, 2018
18. meshcollider closed this on Dec 14, 2018

    ---

19. meshcollider referenced this in commit 7a30e0f6c5 on Dec 14, 2018
20. deadalnix referenced this in commit 3f4c090205 on Jun 6, 2020
21. christiancfifi referenced this in commit 8c68b28d09 on Aug 24, 2021
22. christiancfifi referenced this in commit e10ba2fa03 on Aug 24, 2021
23. MarcoFalke locked this on Sep 8, 2021

Contributors
sipa instagibbs DrahtBot meshcollider promag Empact jonasschnelli

Labels
Refactoring Wallet