3351c91ed4 refactor: Make CScriptVisitor stateless (João Barbosa)
Pull request description:
`CScriptVisitor` was added in 1025440184 (#1357) and the visitor return type was never used. Now `CScriptVisitor` is stateless and `CScript` is the return type.
ACKs for top commit:
MarcoFalke:
ACK 3351c91ed4🏤
sipa:
utACK 3351c91ed4
Tree-SHA512: d158ad2ebe8ea4dc8cc090b943dd66fa5421a84f9443e16ab2d661df38e1a85de16ff13cbaa56924489d8d43cba25fa3cd8b6904bbbcbf356b886ffe8ffba19a
26acc8dd9b Add sanity check asserts to span when -DDEBUG (Pieter Wuille)
2676aeadfa Simplify usage of Span in several places (Pieter Wuille)
ab303a16d1 Add Span constructors for arrays and vectors (Pieter Wuille)
bb3d38fc06 Make pointer-based Span construction safer (Pieter Wuille)
1f790a1147 Make Span size type unsigned (Pieter Wuille)
Pull request description:
This improves our Span class by making it closer to the C++20 `std::span` one:
* ~~Support conversion between compatible Spans (e.g. `Span<char>` to `Span<const char>`).~~ (done in #18591)
* Make the size type `std::size_t` rather than `std::ptrdiff_t` (the C++20 one underwent the same change).
* Support construction of Spans directly from arrays, `std::string`s, `std::array`s, `std::vector`s, `prevector`s, ... (for all but arrays, this only works for const containers to prevent surprises).
And then make use of those improvements in various call sites.
I realize the template magic used looks scary, but it's only needed to make overload resultion make the right choices. Note that the operations done on values are all extremely simple: no casts, explicit conversions, or warning-silencing constructions. That should hopefully make it simpler to review.
ACKs for top commit:
laanwj:
Code review ACK 26acc8dd9b
promag:
Code review ACK 26acc8dd9b.
Tree-SHA512: 5a5bd346a140edf782b5b3b3f04d9160c7b9e9def35159814a07780ab1dd352545b88d3cc491e0f80d161f829c49ebfb952fddc9180f1a56f1257aa51f38788a
152e8baf08 Use salted hasher instead of nonce in sigcache (Jeremy Rubin)
5495fa5850 Add Hash Padding Microbenchmarks (Jeremy Rubin)
Pull request description:
This PR replaces nonces in two places with pre-salted hashers.
The nonce is chosen to be 64 bytes long so that it forces the SHA256 hasher to process the chunk. This leaves the next 64 (or 56 depending if final chunk) open for data. In the case of the script execution cache, this does not make a big performance improvement because the nonce was already properly padded to fit into one buffer, but does make the code a little simpler. In the case of the sig cache, this should reduce the hashing overhead slightly because we are less likely to need an additional processing step.
I haven't benchmarked this, but back of the envelope it should reduce the hashing by one buffer for all combinations except compressed public keys with compact signatures.
ACKs for top commit:
ryanofsky:
Code review ACK 152e8baf08. No code changes, just rebase since last review and expanded commit message
Tree-SHA512: b133e902fd595cfe3b54ad8814b823f4d132cb2c358c89158842ae27daee56ab5f70cde2585078deb46f77a6e7b35b4cc6bba47b65302b7befc2cff254bad93d
c57f03ce17 refactor: Replace const char* to std::string (Calvin Kim)
Pull request description:
Rationale: Addresses #19000
Some functions should be returning std::string instead of const char*.
This commit changes that.
Main benefits/reasoning:
1. The functions never return nullptr, so returning a string makes code at call sites easier to review (reviewers don't have to read the source code to verify that a nullptr is never returned)
2. All call sites convert to string anyway
ACKs for top commit:
MarcoFalke:
re-ACK c57f03ce17 (no changes since previous review) 🚃
Empact:
Fair enough, Code Review ACK c57f03ce17
practicalswift:
ACK c57f03ce17 -- patch looks correct
hebasto:
re-ACK c57f03ce17
Tree-SHA512: 9ce99bb38fe399b54844315048204cafce0f27fd8f24cae357fa7ac6f5d8094d57bbf5f5c1f5878a65f2d35e4a3f95d527eb17f49250b690c591c0df86ca84fd
f9ee0f37c2 Add comments to CustomUintFormatter (Pieter Wuille)
4eb5643e35 Convert everything except wallet/qt to new serialization (Pieter Wuille)
2b1f85e8c5 Convert blockencodings_tests to new serialization (Pieter Wuille)
73747afbbe Convert merkleblock to new serialization (Pieter Wuille)
d06fedd1bc Add SER_READ and SER_WRITE for read/write-dependent statements (Russell Yanofsky)
6f9a1e5ad0 Extend CustomUintFormatter to support enums (Russell Yanofsky)
769ee5fa00 Merge BigEndian functionality into CustomUintFormatter (Pieter Wuille)
Pull request description:
The next step of changes from #10785.
This:
* Adds support for enum serialization to `CustomUintFormatter`, used in `CAddress` for service flags.
* Merges `BigEndian` into `CustomUintFormatter`, used in `CNetAddr` for port numbers.
* Converts everything (except wallet and gui) to use the new serialization framework.
ACKs for top commit:
MarcoFalke:
re-ACK f9ee0f37c2, only change is new documentation commit for CustomUintFormatter 📂
ryanofsky:
Code review ACK f9ee0f37c2. Just new commit adding comment since last review
jonatack:
Code review re-ACK f9ee0f37c2 only change since last review is an additional commit adding Doxygen documentation for `CustomUintFormatter`.
Tree-SHA512: e7a0a36afae592d5a4ff8c81ae04d858ac409388e361f2bc197d9a78abca45134218497ab2dfd6d031e0cce0ca586cf857077b7c6ce17fccf67e2d367c1b6cd4
Use salted hasher instead of nonce in Script Execution Cache
Don't read more than 32 bytes from GetRand
Apply g_* naming convention to scriptExecutionCache in validation.cpp
Fully apply g_* naming convention to scriptCacheHasher
Write same uint256 nonce twice for cache hash rather than calling getrand twice
Use salted hasher instead of nonce in sigcache
Use salted hasher instead of nonce in Script Execution Cache
Don't read more than 32 bytes from GetRand
Apply g_* naming convention to scriptExecutionCache in validation.cpp
Fully apply g_* naming convention to scriptCacheHasher
Write same uint256 nonce twice for cache hash rather than calling getrand twice
ccccd51908 script: Remove undocumented and unused operator+ (MarcoFalke)
Pull request description:
This operator has no documented use case and is also unused outside of test code. The test code and all other (imaginary) code that might use this operator is written more clear and concise by the existing CScript push operators for opcodes and data.
Removing the operator is also going to protect against accidentally reintroducing bugs like this 6ff5f718b6 (diff-8458adcedc17d046942185cb709ff5c3L1135) (last time it was used).
ACKs for top commit:
laanwj:
ACK ccccd51908
Tree-SHA512: 43898ac77e4d9643d9f8ac6f8f65497a4f0bbb1fb5dcaecc839c3719aa36181ba77befb213e59a9f33a20a29e0173a0e9c4763b1930940b32c3d1598b3e39af9
f63dec189c [REFACTOR] Initialize PrecomputedTransactionData in CheckInputScripts (Pieter Wuille)
Pull request description:
This is a single commit taken from the Schnorr/Taproot PR #17977.
Add a default constructor to `PrecomputedTransactionData`, which doesn't initialize the struct's members. Instead they're initialized inside the `CheckInputScripts()` function. This allows a later commit to add the spent UTXOs to that structure. The spent UTXOs are required for the schnorr signature hash, since it commits to the scriptPubKeys. See https://github.com/bitcoin/bips/blob/master/bip-0341.mediawiki#common-signature-message for details.
By itself, this isn't really an improvement to the code, but I think it makes sense to separate out the refactor/moveonly commits from PR #17977 so that PR is only the logical changes needed for Schnorr/Taproot.
ACKs for top commit:
jonatack:
Re-ACK f63dec1 `git diff 851908d f63dec1` shows no change since last ACK.
sipa:
utACK f63dec189c
theStack:
re-ACK f63dec189c
fjahr:
Re-ACK f63dec189c
ariard:
Code Review ACK f63dec1
Tree-SHA512: ecf9154077824ae4c274b4341e985797f3648c0cb0c31cb25ce382163b923a3acbc7048683720be4ae3663501801129cd0f48c441a36f049cc304ebe9f30994e
Add a default constructor to `PrecomputedTransactionData`, which doesn't
initialize the struct's members. Instead they're initialized inside the
`CheckInputScripts()` function. This allows a later commit to add the
spent UTXOs to that structure.
14e8cf974a [consensus] MOVEONLY: Move single-sig checking EvalScript code to EvalChecksig (Pieter Wuille)
Pull request description:
This is another small refactor pulled out of the Schnorr/Taproot PR #17977.
This is in preparation for adding different signature verification rules,
specifically tapscript (BIP 342), which interprets opcode 0xac and 0xad
as Schnorr signature verifications.
ACKs for top commit:
sipa:
ACK 14e8cf974a, verified move-only.
MarcoFalke:
ACK 14e8cf974a, reviewed with "git show 14e8cf974a --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space -W" 👆
fjahr:
Code-review ACK 14e8cf974a, verified that it's move-only.
instagibbs:
code review ACK 14e8cf974a, verified move-only
theStack:
Code-Review ACK 14e8cf974a
jonatack:
ACK 14e8cf974a
Tree-SHA512: af2efce9ae39d5ec01db5b9ef0ff383fe252ef5f33b3483927308ae17d91a619266cb45951f32ea1ce54807a4c0f052bcdefb47e244465d3a726393221c227b1
2b0fcff7f2 Make VerifyWitnessProgram use a Span stack (Pieter Wuille)
Pull request description:
Here is a follow-up to #18002, again with the goal of simplifying (potential) BIP341 code.
Instead of passing a begin and end iterator of the initial stack to `ExecuteWitnessScript`, they are turned into a `Span<const valtype>`, representing a span of `valtype`s in memory. This allows `VerifyWitnessProgram` to operate on that span directly, instead of juggling iterators around (which would be exacerbated by #17977 if trying to avoid copying the stack).
ACKs for top commit:
ajtowns:
ACK 2b0fcff7f2
elichai:
ReACK on the diff 2b0fcff7f2
instagibbs:
re-ACK 2b0fcff7f2
theStack:
re-ACK 2b0fcff7f2
Empact:
ACK 2b0fcff7f2
jnewbery:
utACK 2b0fcff7f2
Tree-SHA512: 38eb4ce17f1947674c1c274caa40feb6ea8266bd96134d9cf1bc41e6fbf1114d4dde6c7a9e26e1ca8f3d0155429ef0911cc8ec0c1037d8fe7d6ec7f9e7184e93
This is in preparation for adding different signature verification rules,
specifically tapscript (BIP 342), which interprets opcode 0xac and 0xad
as Schnorr signature verifications.
e6e622e5a0 Implement O(1) OP_IF/NOTIF/ELSE/ENDIF logic (Pieter Wuille)
d0e8f4d5d8 [refactor] interpreter: define interface for vfExec (Anthony Towns)
89fb241c54 Benchmark script verification with 100 nested IFs (Pieter Wuille)
Pull request description:
While investigating what mechanisms are possible to maximize the per-opcode verification cost of scripts, I noticed that the logic for determining whether a particular opcode is to be executed is O(n) in the nesting depth. This issue was also pointed out by Sergio Demian Lerner in https://bitslog.wordpress.com/2017/04/17/new-quadratic-delays-in-bitcoin-scripts/, and this PR implements a variant of the O(1) algorithm suggested there.
This is not a problem currently, because even with a nesting depth of 100 (the maximum possible right now due to the 201 ops limit), the slowdown caused by this on my machine is around 70 ns per opcode (or 0.25 s per block) at worst, far lower than what is possible with other opcodes.
This PR mostly serves as a proof of concept that it's possible to avoid it, which may be relevant in discussions around increasing the opcode limits in future script versions. Without it, the execution time of scripts can grow quadratically with the nesting depth, which very quickly becomes unreasonable.
This improves upon #14245 by completely removing the `vfExec` vector.
ACKs for top commit:
jnewbery:
Code review ACK e6e622e5a0
MarcoFalke:
ACK e6e622e5a0🐴
fjahr:
ACK e6e622e5a0
ajtowns:
ACK e6e622e5a0
laanwj:
concept and code review ACK e6e622e5a0
jonatack:
ACK e6e622e5a0 code review, build, benches, fuzzing
Tree-SHA512: 1dcfac3411ff04773de461959298a177f951cb5f706caa2734073bcec62224d7cd103767cfeef85cd129813e70c14c74fa8f1e38e4da70ec38a0f615aab1f7f7
09e25071f4 Cache parent xpub inside of BIP32PubkeyProvider (Andrew Chow)
deb791c7ba Only cache xpubs that have a hardened last step (Andrew Chow)
f76733eda5 Cache the immediate derivation parent xpub (Andrew Chow)
58f54b686f Add DescriptorCache* read_cache and DescriptorCache* write_cache to Expand and GetPubKey (Andrew Chow)
66c2cadc91 Rename BIP32PubkeyProvider.m_extkey to m_root_extkey (Andrew Chow)
df55d44d0d Track the index of the key expression in PubkeyProvider (Andrew Chow)
474ea3b927 Introduce DescriptorCache struct which caches xpubs (Andrew Chow)
Pull request description:
Improves the descriptor cache by changing it from a `std::vector<unsigned char>` to a newly introduced `DescriptorCache` class. Instead of serializing pubkeys and whatever else we would want to cache in a way that may not be backwards compatible, we instead create a `DescriptorCache` object and populate it. This object contains only an xpub cache. Since the only `PubkeyProvider` that used the cache is the `BIP32PubkeyProvider` we just have it store the xpubs instead of the pubkeys. This allows us to have both the parent xpub and the child xpubs in the same container. The map is keyed by `KeyOriginInfo`.
Sine we are caching `CExtPubKey`s in `DescriptorCache`, `BIP32PubKeyProviders` can use the cached parent xpubs to derive the children if unhardened derivation is used in the last step. This also means that we can still derive the keys for a `BIP32PubkeyProvider` that has hardened derivation steps. When combined with descriptor wallets, this should allow us to be able to import a descriptor with an `xprv` and hardened steps and still be able to derive from it. In that sense, this is an alternative to #18163
To test that this works, the tests have been updated to do an additional `Expand` at the `i + 1` position. This expansion is not cached. We then do an `ExpandFromCache` at `i + 1` and use the cache that was produced by the expansion at `i`. This way, we won't have the child xpubs for `i + 1` but we will have the parent xpubs. So this checks whether the parent xpubs are being stored and can be used to derive the child keys. Descriptors that have a hardened last step are skipped for this part of the test because that will always require private keys.
ACKs for top commit:
instagibbs:
code review re-re-ACK 09e25071f4
Sjors:
re-ACK 09e25071f4
Tree-SHA512: 95c8d0092274cdf115ce39f6d49dec767679abf3758d5b9e418afc308deca9dc6f67167980195bcc036cd9c09890bbbb39ec1dacffbfacdc03efd72a7e23b276
c8e24ddce3 [REFACTOR] Abstract out script execution out of VerifyWitnessProgram() (Pieter Wuille)
Pull request description:
This is a refactoring cherry-picked out of #17977. As it touches consensus code, I don't think this would ordinarily meet the bar for review cost vs benefit. However, it simplifies the changes for Taproot significantly, and if it's going to be necessitated by inclusion of that code, I may as well give it some additional attention by PRing it independently.
ACKs for top commit:
fjahr:
Re-ACK c8e24ddce3
theStack:
re-ACK c8e24ddce3
Empact:
Code Review Re-ACK c8e24ddce3
ajtowns:
ACK c8e24ddce3
jnewbery:
ACK c8e24ddce3
jonatack:
ACK c8e24dd
Tree-SHA512: 96c2aa5d2f9c7c802bcc008f5cde55b1dfedfaf42e34101331e6c0d594acdf6437661102dc939718f0877c20451336855dfbaa8aa8f57d9e722a7fa7329e3a46
Make sure that there are no errors set for an input after it is signed.
This is useful for when there are multiple ScriptPubKeyMans. Some may
fail to sign, but one may be able to sign, and after it does, we don't
want there to be any more errors there.
Optimize Expand by having BIP32PubkeyProvider also cache the parent
(or only) xpub within itself. Since Expand does not provide a read
cache, it is useful to internally cache this xpub to avoid re-deriving
the same xpub.
Also adds tests for this:
For ranged descriptors with unhardened derivation, we expect to
find parent keys in the cache but no child keys.
For descriptors containing an xpub but do not have unhardened derivation
(i.e. hardened derivation or single xpub with or without derivation),
we expect to find all of the keys in the cache, and the same
number of keys in the cache as in the SigningProvider.
For everything else (no xpub), nothing should be cached at all.
Have Expand, ExpandFromCache, and ExpandHelper take additional DescriptorCache
parameters. These are then passed into PubkeyProvider::GetPubKey which
also takes them as arguments.
Reading and writing to the cache is pushed down into GetPubKey. The old cache where
pubkeys are serialized to a vector is completely removed and instead xpubs are being
cached in DescriptorCache.
7e80f646b2 Get the OutputType for a descriptor (Andrew Chow)
Pull request description:
Adds a `GetOutputType()` method to get the OutputType of a descriptor. Some descriptors don't have a determinate OutputType, so we actually use an `Optional<OutputType>`. For descriptors with indeterminate OutputType, we return `nullopt`.
`addr()` and `raw()` use OutputTypes as determined by the CTxDestination they have. For simplicity, `ScriptHash` destinations are `LEGACY` even though they could be `P2SH_SEGWIT`.
`combo()`, `pk()`, and `multi()` are `nullopt` as they either don't have an OutputType or they have multiple. `DescriptorImpl` defaults to `nullopt`.
`pkh()` is `LEGACY` as expected
`wpkh()` and `wsh()` are `BECH32` as expected.
`sh()` checks whether the sub-descriptor is `BECH32`. If so, it is `P2SH_SEGWIT`. Otherwise it is `LEGACY`.
The descriptor tests are updated to check the OutputType too.
ACKs for top commit:
fjahr:
ACK 7e80f646b2
meshcollider:
utACK 7e80f646b2
instagibbs:
cursory ACK 7e80f646b2
Sjors:
Code review ACK 7e80f646b2
jonatack:
ACK 7e80f64 code review/build/tests
Tree-SHA512: c5a813447b62e982435e1c948066f8d6c148c9ebffb0a5eb5a9028b173b01d5ead2f076a5ca3f7f37698538baa346f82a977ee48f583d89cb4e5ebd9111b2341
Make LegacyScriptPubKeyMan::CanProvide method able to recognize p2sh scripts
when the redeem script is present in the mapScripts map without the p2sh script
also having to be added to the mapScripts map. This restores behavior prior to
https://github.com/bitcoin/bitcoin/pull/17261, which I think broke backwards
compatibility with old wallet files by no longer treating addresses created by
`addmultisigaddress` calls before #17261 as solvable.
The reason why tests didn't fail with the CanProvide implementation in #17261
is because of a workaround added in 4a7e43e846
"Store p2sh scripts in AddAndGetDestinationForScript", which masked the problem
for new `addmultisigaddress` RPC calls without fixing it for multisig addresses
already created in old wallet files.
This change adds a lot of comments and allows reverting commit
4a7e43e846 "Store p2sh scripts in
AddAndGetDestinationForScript", so the AddAndGetDestinationForScript() function,
CanProvide() method, and mapScripts map should all be more comprehensible
3f373659d7 Refactor: Replace SigningProvider pointers with unique_ptrs (Andrew Chow)
3afe53c403 Cleanup: Drop unused GUI learnRelatedScripts method (Andrew Chow)
e2f02aa59e Refactor: Copy CWallet signals and print function to LegacyScriptPubKeyMan (Andrew Chow)
c729afd0a3 Box the wallet: Add multiple keyman maps and loops (Andrew Chow)
4977c30d59 refactor: define a UINT256_ONE global constant (Andrew Chow)
415afcccd3 HD Split: Avoid redundant upgrades (Andrew Chow)
01b4511206 Make UpgradeKeyMetadata work only on LegacyScriptPubKeyMan (Andrew Chow)
4a7e43e846 Store p2sh scripts in AddAndGetDestinationForScript (Andrew Chow)
501acb5538 Always try to sign for all pubkeys in multisig (Andrew Chow)
81610eddbc List output types in an array in order to be iterated over (Andrew Chow)
eb81fc3ee5 Refactor: Allow LegacyScriptPubKeyMan to be null (Andrew Chow)
fadc08ad94 Locking: Lock cs_KeyStore instead of cs_wallet in legacy keyman (Andrew Chow)
f5be479694 wallet: Improve CWallet:MarkDestinationsDirty (João Barbosa)
Pull request description:
Continuation of wallet boxes project.
Actually makes ScriptPubKeyMan an interface which LegacyScriptPubkeyMan. Moves around functions and things from CWallet into LegacyScriptPubKeyMan so that they are actually separate things without circular dependencies.
***
Introducing the `ScriptPubKeyMan` (short for ScriptPubKeyManager) for managing scriptPubKeys and their associated scripts and keys. This functionality is moved over from `CWallet`. Instead, `CWallet` will have a pointer to a `ScriptPubKeyMan` for every possible address type, internal and external. It will fetch the correct `ScriptPubKeyMan` as necessary. When fetching new addresses, it chooses the `ScriptPubKeyMan` based on address type and whether it is change. For signing, it takes the script and asks each `ScriptPubKeyMan` for whether that `ScriptPubKeyMan` considers that script `IsMine`, whether it has that script, or whether it is able to produce a signature for it. If so, the `ScriptPubKeyMan` will provide a `SigningProvider` to the caller which will use that in order to sign.
There is currently one `ScriptPubKeyMan` - the `LegacyScriptPubKeyMan`. Each `CWallet` will have only one `LegacyScriptPubKeyMan` with the pointers for all of the address types and change pointing to this `LegacyScriptPubKeyMan`. It is created when the wallet is loaded and all keys and metadata are loaded into it instead of `CWallet`. The `LegacyScriptPubKeyMan` is primarily made up of all of the key and script management that used to be in `CWallet`. For convenience, `CWallet` has a `GetLegacyScriptPubKeyMan` which will return the `LegacyScriptPubKeyMan` or a `nullptr` if it does not have one (not yet implemented, but callers will check for the `nullptr`). For purposes of signing, `LegacyScriptPubKeyMan`'s `GetSigningProvider` will return itself rather than a separate `SigningProvider`. This will be different for future `ScriptPubKeyMan`s.
The `LegacyScriptPubKeyMan` will also handle the importing and exporting of keys and scripts instead of `CWallet`. As such, a number of RPCs have been limited to work only if a `LegacyScriptPubKeyMan` can be retrieved from the wallet. These RPCs are `sethdseed`, `addmultisigaddress`, `importaddress`, `importprivkey`, `importpubkey`, `importmulti`, `dumpprivkey`, and `dumpwallet`. Other RPCs which relied on the wallet for scripts and keys have been modified in order to take the `SigningProvider` retrieved from the `ScriptPubKeyMan` for a given script.
Overall, these changes should not effect how everything actually works and the user should experience no difference between having this change and not having it. As such, no functional tests were changed, and the only unit tests changed were those that were directly accessing `CWallet` functions that have been removed.
This PR is the last step in the [Wallet Structure Changes](https://github.com/bitcoin-core/bitcoin-devwiki/wiki/Wallet-Class-Structure-Changes).
ACKs for top commit:
instagibbs:
re-utACK 3f373659d7
Sjors:
re-utACK 3f373659d7 (it still compiles on macOS after https://github.com/bitcoin/bitcoin/pull/17261#discussion_r370377070)
meshcollider:
Tested re-ACK 3f373659d7
Tree-SHA512: f8e2b8d9efa750b617691e8702d217ec4c33569ec2554a060141d9eb9b9a3a5323e4216938e2485c44625d7a6e0925d40dea1362b3af9857cf08860c2f344716
Identified via -Wdocumentation, e.g.:
./rpc/rawtransaction_util.h:31:13: error: parameter 'prevTxs' not found in the function declaration [-Werror,-Wdocumentation]
* @param prevTxs Array of previous txns outputs that tx depends on but may not yet be in the block chain
^~~~~~~
./rpc/rawtransaction_util.h:31:13: note: did you mean 'prevTxsUnival'?
* @param prevTxs Array of previous txns outputs that tx depends on but may not yet be in the block chain
^~~~~~~
prevTxsUnival
netbase.cpp:766:11: error: parameter 'outProxyConnectionFailed[out]' not found in the function declaration [-Werror,-Wdocumentation]
* @param outProxyConnectionFailed[out] Whether or not the connection to the
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
netbase.cpp:766:11: note: did you mean 'outProxyConnectionFailed'?
* @param outProxyConnectionFailed[out] Whether or not the connection to the
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
outProxyConnectionFailed
6dd59d2e49 Don't allow implementers to think ScriptHash(Witness*()) results in nesting computation (Gregory Sanders)
4b8f1e989f IsUsedDestination shouldn't use key id as script id for ScriptHash (Gregory Sanders)
Pull request description:
Regression introduced in https://github.com/bitcoin/bitcoin/pull/17621 which causes p2sh-segwit addresses to be erroneously missed.
Tests are only failing in 0.19 branch, likely because that release still uses p2sh-segwit addresses rather than bech32 by default.
I'll devise a test case to catch this going forward.
ACKs for top commit:
achow101:
ACK 6dd59d2e49
MarcoFalke:
ACK 6dd59d2
meshcollider:
Code review ACK 6dd59d2e49
Tree-SHA512: b3e0f320c97b8c1f814cc386840240cbde2761fee9711617b713d3f75a4a5dce2dff2df573d80873df42a1f4b74e816ab8552a573fa1d62c344997fbb6af9950
