Avoid expanding descriptor scriptPubKeys #14826

pull promag wants to merge 2 commits into bitcoin:master from promag:2018-11-faster-descriptor-expand changing 5 files +33 −88

promag commented at 11:16 AM on November 28, 2018: member

Output scripts aren't needed in GetAffectedKeys, this change avoids computing them.

Based on #14821.
Replace CAffectedKeysVisitor with descriptor based logic 0e75f44a09
Avoid expanding descriptor scriptPubKeys 02293e693d
promag force-pushed on Nov 28, 2018
DrahtBot commented at 12:39 PM on November 28, 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:
- #14646 (Add expansion cache functions to descriptors (unused for now) by sipa)
- #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.

in src/script/descriptor.cpp:414 in 02293e693d

 409 | @@ -410,8 +410,8 @@ class ComboDescriptor final : public Descriptor
 410 |              CScriptID p2wpkh_id(p2wpkh);
 411 |              CScript p2sh_p2wpkh = GetScriptForDestination(p2wpkh_id);
 412 |              out.scripts.emplace(p2wpkh_id, p2wpkh);
 413 | -            output_scripts.push_back(std::move(p2wpkh));
 414 | -            output_scripts.push_back(std::move(p2sh_p2wpkh));
 415 | +            if (output_scripts) output_scripts->push_back(std::move(p2wpkh));
 416 | +            if (output_scripts) output_scripts->push_back(std::move(p2sh_p2wpkh));

practicalswift commented at 6:38 PM on November 28, 2018:

Combine the two if statements :-)

meshcollider added the label Wallet on Dec 3, 2018

in src/wallet/wallet.cpp:108 in 02293e693d

 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)

Empact commented at 11:35 AM on December 4, 2018:

nit: could be static #14821 (review) ;P

Empact commented at 11:40 AM on December 4, 2018: member

Concept ACK
DrahtBot commented at 4:31 AM on December 12, 2018: member

Needs rebase
DrahtBot added the label Needs rebase on Dec 12, 2018
promag closed this on Mar 13, 2019
laanwj removed the label Needs rebase on Oct 24, 2019
DrahtBot locked this on Dec 16, 2021

Contributors

Labels

Linked (view graph)

#14821 Replace CAffectedKeysVisitor with descriptor based logic

Avoid expanding descriptor scriptPubKeys #14826

Conflicts