| Age | Commit message (Collapse) | Author |
|---|
|
09e25071f40c564af08a1386c39c4f2d8eb484b6 Cache parent xpub inside of BIP32PubkeyProvider (Andrew Chow)
deb791c7ba057a3765d09b12bf3e55547a5298e4 Only cache xpubs that have a hardened last step (Andrew Chow)
f76733eda5f4c161e9eb47c74b949582ab8f448a Cache the immediate derivation parent xpub (Andrew Chow)
58f54b686f663e4c46a2cf7a64560409007c7eb3 Add DescriptorCache* read_cache and DescriptorCache* write_cache to Expand and GetPubKey (Andrew Chow)
66c2cadc91d26074b89e5ada68350b5c8676efac Rename BIP32PubkeyProvider.m_extkey to m_root_extkey (Andrew Chow)
df55d44d0de2174ba74ed3a28bef5e83b0a51b47 Track the index of the key expression in PubkeyProvider (Andrew Chow)
474ea3b927ddc67e64ae78e08c20c9264817e84d 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 https://github.com/bitcoin/bitcoin/pull/18204/commits/09e25071f40c564af08a1386c39c4f2d8eb484b6
Sjors:
re-ACK 09e25071f40c564af08a1386c39c4f2d8eb484b6
Tree-SHA512: 95c8d0092274cdf115ce39f6d49dec767679abf3758d5b9e418afc308deca9dc6f67167980195bcc036cd9c09890bbbb39ec1dacffbfacdc03efd72a7e23b276
|
|
c8e24ddce31a8de6255b23c19d958c1cd44a8847 [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 c8e24ddce31a8de6255b23c19d958c1cd44a8847
theStack:
re-ACK https://github.com/bitcoin/bitcoin/commit/c8e24ddce31a8de6255b23c19d958c1cd44a8847
Empact:
Code Review Re-ACK https://github.com/bitcoin/bitcoin/pull/18002/commits/c8e24ddce31a8de6255b23c19d958c1cd44a8847
ajtowns:
ACK c8e24ddce31a8de6255b23c19d958c1cd44a8847
jnewbery:
ACK c8e24ddce31a8de6255b23c19d958c1cd44a8847
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.
|
|
If unhardened derivation is used, cache the immediate derivation
parent xpub and use it for unhardened derivation
|
|
and GetPubKey
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.
|
|
Renaming clarifies that m_extkey is actually the root
extkey that keys are derived from.
|
|
|
|
|
|
7e80f646b24a2abf3c031a649bcc706a695f80da 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 7e80f646b24a2abf3c031a649bcc706a695f80da
meshcollider:
utACK 7e80f646b24a2abf3c031a649bcc706a695f80da
instagibbs:
cursory ACK https://github.com/bitcoin/bitcoin/pull/18034/commits/7e80f646b24a2abf3c031a649bcc706a695f80da
Sjors:
Code review ACK 7e80f646b24a2abf3c031a649bcc706a695f80da
jonatack:
ACK 7e80f64 code review/build/tests
Tree-SHA512: c5a813447b62e982435e1c948066f8d6c148c9ebffb0a5eb5a9028b173b01d5ead2f076a5ca3f7f37698538baa346f82a977ee48f583d89cb4e5ebd9111b2341
|
|
This removes the unclear reliance on "falling through" to get to the
script execution part.
Also fix some code style issues.
|
|
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 4a7e43e8460127a40a7895519587399feff3b682
"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
4a7e43e8460127a40a7895519587399feff3b682 "Store p2sh scripts in
AddAndGetDestinationForScript", so the AddAndGetDestinationForScript() function,
CanProvide() method, and mapScripts map should all be more comprehensible
|
|
|
|
have multiple
3f373659d732a5b1e5fdc692a45b2b8179f66bec Refactor: Replace SigningProvider pointers with unique_ptrs (Andrew Chow)
3afe53c4039103670cec5f9cace897ead76e20a8 Cleanup: Drop unused GUI learnRelatedScripts method (Andrew Chow)
e2f02aa59e3402048269362ff692d49a6df35cfd Refactor: Copy CWallet signals and print function to LegacyScriptPubKeyMan (Andrew Chow)
c729afd0a3b74a3943e4c359270beaf3e6ff8a7b Box the wallet: Add multiple keyman maps and loops (Andrew Chow)
4977c30d59e88a3e5ee248144bcc023debcd895b refactor: define a UINT256_ONE global constant (Andrew Chow)
415afcccd3e5583defdb76e3a280f48e98983301 HD Split: Avoid redundant upgrades (Andrew Chow)
01b4511206e399981a77976deb15785d18db46ae Make UpgradeKeyMetadata work only on LegacyScriptPubKeyMan (Andrew Chow)
4a7e43e8460127a40a7895519587399feff3b682 Store p2sh scripts in AddAndGetDestinationForScript (Andrew Chow)
501acb5538008d98abe79288b92040bc186b93f3 Always try to sign for all pubkeys in multisig (Andrew Chow)
81610eddbc57c46ae243f45d73e715d509f53a6c List output types in an array in order to be iterated over (Andrew Chow)
eb81fc3ee58d3e88af36d8091b9e4017a8603b3c Refactor: Allow LegacyScriptPubKeyMan to be null (Andrew Chow)
fadc08ad944cad42e805228cdd58e0332f4d7184 Locking: Lock cs_KeyStore instead of cs_wallet in legacy keyman (Andrew Chow)
f5be479694d4dbaf59eef562d80fbeacb3bb7dc1 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 https://github.com/bitcoin/bitcoin/pull/17261/commits/3f373659d732a5b1e5fdc692a45b2b8179f66bec
Sjors:
re-utACK 3f373659d732a5b1e5fdc692a45b2b8179f66bec (it still compiles on macOS after https://github.com/bitcoin/bitcoin/pull/17261#discussion_r370377070)
meshcollider:
Tested re-ACK 3f373659d732a5b1e5fdc692a45b2b8179f66bec
Tree-SHA512: f8e2b8d9efa750b617691e8702d217ec4c33569ec2554a060141d9eb9b9a3a5323e4216938e2485c44625d7a6e0925d40dea1362b3af9857cf08860c2f344716
|
|
Instead of having a uint256 representations of one scattered throughout
where it is used, define it globally in uint256.h
|
|
|
|
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
|
|
ScriptHash
6dd59d2e491bc11ab26498668543e65440a3a931 Don't allow implementers to think ScriptHash(Witness*()) results in nesting computation (Gregory Sanders)
4b8f1e989f3b969dc628b0801d5c31ebd373719c 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 6dd59d2e491bc11ab26498668543e65440a3a931
MarcoFalke:
ACK 6dd59d2
meshcollider:
Code review ACK 6dd59d2e491bc11ab26498668543e65440a3a931
Tree-SHA512: b3e0f320c97b8c1f814cc386840240cbde2761fee9711617b713d3f75a4a5dce2dff2df573d80873df42a1f4b74e816ab8552a573fa1d62c344997fbb6af9950
|
|
computation
|
|
-BEGIN VERIFY SCRIPT-
./contrib/devtools/copyright_header.py update ./
-END VERIFY SCRIPT-
|
|
-BEGIN VERIFY SCRIPT-
# Delete outdated alias for RecursiveMutex
sed -i -e '/CCriticalSection/d' ./src/sync.h
# Replace use of outdated alias with RecursiveMutex
sed -i -e 's/CCriticalSection/RecursiveMutex/g' $(git grep -l CCriticalSection)
-END VERIFY SCRIPT-
|
|
3bd8db80d8d335ab63ece4f110b0fadd562e80b7 [validation] fix comments in CheckInputScripts() (John Newbery)
6f6465cefcd599c89c00f7b51f42a4b87a5ffb0b scripted-diff: [validation] Rename CheckInputs to CheckInputScripts (John Newbery)
Pull request description:
CheckInputs() used to check no double spends, scripts & sigs and amounts. Since
832e074, the double spend and amount checks
have been moved to CheckTxInputs(), and CheckInputs() now just validates
input scripts. Rename the function to CheckInputScripts().
Also fix incorrect comments.
ACKs for top commit:
MarcoFalke:
re-ACK 3bd8db80d8d335ab63ece4f110b0fadd562e80b7, did the rebase myself, checked the scripted diff 👡
promag:
ACK 3bd8db80d8d335ab63ece4f110b0fadd562e80b7 :trollface:
Tree-SHA512: 7b3f8597d210492798fb784ee8ea47ea6377519111190161c7cc34a967509013f4337304f52e9bedc97b7710de7b0ff8880e08cd7f867754567f82e7b02c794c
|
|
-BEGIN VERIFY SCRIPT-
./contrib/devtools/copyright_header.py update ./
-END VERIFY SCRIPT-
|
|
consistently
cb9d830a00995ee60e71780c04f6193efd02c511 test: Use proper MAX_SCRIPT_ELEMENT_SIZE (Hennadii Stepanov)
402ee706d8afab3d8d883cd15a660740fcebeb55 refactor: Use proper MAX_SCRIPT_ELEMENT_SIZE const (Hennadii Stepanov)
Pull request description:
This PR replaces well-known "magic" numbers with proper `MAX_SCRIPT_ELEMENT_SIZE` constants.
ACKs for top commit:
practicalswift:
ACK cb9d830a00995ee60e71780c04f6193efd02c511 -- diff looks correct and change appears to be complete
instagibbs:
utACK https://github.com/bitcoin/bitcoin/pull/17439/commits/cb9d830a00995ee60e71780c04f6193efd02c511
Tree-SHA512: 5fa033275d6df7e35962c38bfdf09a7b5cd7ef2ccdd5e30a39ba47d0c21ac779a5559c23f5ef5bfd4293be0fc639e836a308bbedf0e34717e1eead983b389bbd
|
|
class
0580f86bb48004b797d4cb6273e7ffee0b0a0584 Fixup whitespace (Ben Woosley)
47101bbb27d0e13ea2b40ce1c7ff0dba9030f369 scripted-diff: Rename CPubKey and CKey::*_KEY_SIZE and COMPRESSED_*_KEY_SIZE (Ben Woosley)
Pull request description:
~~And introduce CPubKeySig to host code relative to key sigs.~~
ACKs for top commit:
meshcollider:
utACK https://github.com/bitcoin/bitcoin/pull/12461/commits/0580f86bb48004b797d4cb6273e7ffee0b0a0584
Tree-SHA512: 29aa0be54912358b138e391b9db78639786f56580493e590ec9f773c0e1b421740133d05a79be247c7ee57e71c9c9e41b9cb54088cb3c0e3f813f74f0895287b
|
|
|
|
CheckInputs() used to check no double spends, scripts & sigs and amounts. Since
832e0744cb8b1e1625cdb19b257f97316ac16a90, the double spend and amount checks
have been moved to CheckTxInputs(), and CheckInputs() now just validates
input scripts. Rename the function to CheckInputScripts().
-BEGIN VERIFY SCRIPT-
sed -i -E -e 's/CheckInputs\b/CheckInputScripts/g' $(git grep -l CheckInputs | grep -v doc/)
-END VERIFY SCRIPT-
|
|
|
|
f201ba59ffd2e071a36a688b80d2cff9a9c44bb2 Refactor: Split up CWallet and LegacyScriptPubKeyMan and classes (Andrew Chow)
6702048f91089d7a565e5ca5f7c8dcd2ca405a85 MOVEONLY: Move key handling code out of wallet to keyman file (Andrew Chow)
ab053ec6d1e766402f88947d29cd875a285e7280 Move wallet enums to walletutil.h (Andrew Chow)
Pull request description:
Moves key management functions into a new class LegacyScriptPubKeyMan. First two commits are move-only commits which move stuff out of wallet.{h/cpp} and into newly created scriptpubkeyman.{h/cpp}. Third commit changes several things in CWallet to use LegacyScriptPubKeyMan.
First step in the wallet boxes refactor. Note that LegacyScriptPubKeyMan and ScriptPubKeyMan cannot be used standalone yet and are still very much tied into CWallet with both accessing functions within each other. This PR is to help reduce review burden.
ACKs for top commit:
Sjors:
Code review ACK f201ba5.
promag:
Code review ACK f201ba59ffd2e071a36a688b80d2cff9a9c44bb2.
ryanofsky:
Code review ACK f201ba59ffd2e071a36a688b80d2cff9a9c44bb2
MarcoFalke:
ACK f201ba59ffd2e071a36a688b80d2cff9a9c44bb2
Tree-SHA512: bdc0d8595a06233fe003afcf968a38e0e8cc584a6a89c5bcd05309ac29dca852391802d46763ef81a108d146d0f40c79ea5438e87234ed12b4b8360c9aec94c0
|
|
Our formatter, tinyformat, *never* needs `c_str()` for strings.
Remove redundant `c_str()` calls for:
- `strprintf`
- `LogPrintf`
- `tfm::format`
|
|
Start moving wallet and ismine code to scriptpubkeyman.h, scriptpubkeyman.cpp
The easiest way to review this commit is to run:
git log -p -n1 --color-moved=dimmed_zebra
And check that everything is a move (other than includes and copyrights comments).
This commit is move-only and doesn't change code or affect behavior.
|
|
7d8d3e6a2ad827fa916e3909a18dedb9f7fdce43 Add tests for util/vector.h's Cat and Vector (Pieter Wuille)
e65e61c812df90a56e3ce4a8e76c4b746766f387 Add some general std::vector utility functions (Pieter Wuille)
Pull request description:
This is another general improvement extracted from #16800 .
Two functions are added are:
* Vector(arg1,arg2,arg3,...) constructs a vector with the specified arguments as elements. The vector's type is derived from the arguments. If some of the arguments are rvalue references, they will be moved into place rather than copied (which can't be achieved using list initialization).
* Cat(vector1,vector2) returns a concatenation of the two vectors, efficiently moving elements when relevant.
Vector generalizes (and replaces) the `Singleton` function in src/descriptor.cpp, and `Cat` replaces the function in bech32.cpp
ACKs for top commit:
laanwj:
ACK 7d8d3e6a2ad827fa916e3909a18dedb9f7fdce43
MarcoFalke:
ACK 7d8d3e6a2ad827fa916e3909a18dedb9f7fdce43 (enjoyed reading the tests, but did not compile)
Tree-SHA512: 92325f14e90d7e7d9d920421979aec22bb0d730e0291362b4326cccc76f9c2d865bec33a797c5c0201773468c3773cb50ce52c8eee4c1ec1a4d10db5cf2b9d2a
|
|
Added are:
* Vector(arg1,arg2,arg3,...) constructs a vector with the specified
arguments as elements. The vector's type is derived from the
arguments. If some of the arguments are rvalue references, they
will be moved into place rather than copied (which can't be achieved
using list initialization).
* Cat(vector1,vector2) returns a concatenation of the two vectors,
efficiently moving elements when relevant.
Vector generalizes (and replaces) the Singleton function in
src/descriptor.cpp, and Cat replaces the Cat function in bech32.cpp
|
|
|
|
15ac916642f20918f66e32729bb6b0b674e3bc24 doc: Doxygen-friendly descriptor.h comments (Jon Layton)
Pull request description:
Closes #16942.
- Make `Descriptor` overview subtext of `Interface for parsed descriptor objects.`
- Conform to `@param[in, out] argname: Info` in parameter comments. Present in code: https://github.com/bitcoin/bitcoin/blob/feb162d500273040c8a2e5cdd3393edf34dff45b/src/net_processing.cpp#L1001
- Remove redundant argument type, `in` vs `out` mentions
- Removed unnecessary backticks around `IsSolvable()`, since Doxygen builds a link to the known function's docs
- Add backticks to refer to `argname`s
`descriptor.cpp` has more documentation, but Doxygen's output doesn't include anything inside unnamed namespaces for some reason. Tried to access them via searchbar.
Top commit has no ACKs.
Tree-SHA512: 587cc7596de46358a08b0321a7cf08a08785945715dbdce8945d837e1bee0664d1e11b1e47b7be85c4f35262f7ea173fb1f6202efcacc2023e2c6b0bd44133b3
|
|
bb36372b8f2bd675313ae8553ceb61f28c2c1afd test: add unit tests for Span-parsing helpers (Sebastian Falbesoner)
5e69aeec3f2a0fafd5e591b7222716f00145761d Add documenting comments to spanparsing.h (Pieter Wuille)
230d43fdbc41b356700b0d8a6984d69e00279ade Abstract out some of the descriptor Span-parsing helpers (Pieter Wuille)
Pull request description:
As suggested here: https://github.com/bitcoin/bitcoin/pull/16800#issuecomment-531605482.
This moves the Span parsing functions out of the descriptor module, making them more easily usable for other parsers (in particular, in preparation for miniscript parsing).
ACKs for top commit:
MarcoFalke:
ACK bb36372b8f2bd675313ae8553ceb61f28c2c1afd
Tree-SHA512: b5c5c11a9bc3f0a1c2c4cfa22755654ecfb8d4b69da0dc1fb9f04e1556dc0f6ffd87ad153600963279ac465d587d7971b53d240ced802d12693682411ac73deb
|
|
|
|
73aaf4ecf825a4d18c802ca5dd3856ce9096915e Make SignatureExtractorChecker private to its own file (Ben Woosley)
Pull request description:
~If we add a CTxIn constructor to SignatureData, then constructing the
SignatureData directly is no more verbose than calling DataFromTransaction,
and grants the caller additional flexibiliy in how to provide the CTxIn.~
A simple change to enhance encapsulation.
ACKs for top commit:
MarcoFalke:
utACK 73aaf4ecf825a4d18c802ca5dd3856ce9096915e
laanwj:
ACK 73aaf4ecf825a4d18c802ca5dd3856ce9096915e
Tree-SHA512: f7eafbce22b0e9917a8487e88d1f5a1061f2a0959ae1a097cbd9c8ea0d774edfb807da56813cb5fb26f6ca98499a0604a8ff024c198a7c8dc755164de66d972a
|
|
|
|
|
|
p2p_invalid_block test.
0c62e3aa73839e97e65a3155e06a98d84b700a1e New regression testing for CVE-2018-17144, CVE-2012-2459, and CVE-2010-5137. (lucash-dev)
38bfca6bb2ad68719415e9c54a981441052da072 Added comments referencing multiple CVEs in tests and production code. (lucash-dev)
Pull request description:
This functional test includes two scenarios that test for regressions of vulnerabilities, but they are only briefly described. There are freely available documents explaining in detail the issues, but without explicit mentions, the developer trying to maintain the code needs an additional step of digging in commit history and PR conversations to figure it out.
Added comments to explicitly mention CVE-2018-17144 and CVE-2012-2459, for more complete documentation.
This improves developer experience by making understanding the tests easier.
ACKs for top commit:
laanwj:
ACK 0c62e3aa73839e97e65a3155e06a98d84b700a1e, checked the CVE numbers, thanks for adding documentation
Tree-SHA512: 3ee05351745193b8b959e4a25d50f25a693b2d24b0732ed53cf7d5882df40b5dd0f1877bd5c69cffb921d4a7acf9deb3cc1160b96dc730d9b5984151ad06b7c9
|
|
Some failure conditions implicitly fail by failing some other check.
But the error messages are more helpful if they say explicitly what
actually caused the failure, so add those as failure conditions and
errors.
|
|
failed
|
|
26d3fad1093dfc697048313be7a96c9adf723654 Add unmodified-but-with-checksum to getdescriptorinfo (Pieter Wuille)
104b3a5069c937383e6f88f2f3fb804ef61b208f Factor out checksum checking from descriptor parsing (Pieter Wuille)
Pull request description:
ACKs for top commit:
achow101:
Code Review ACK 26d3fad1093dfc697048313be7a96c9adf723654
meshcollider:
re-Code Review ACK 26d3fad1093dfc697048313be7a96c9adf723654
Sjors:
ACK 26d3fad1093dfc697048313be7a96c9adf723654
Tree-SHA512: b7a7f89b64a184927d6f9a0c183a087609983f0c5d5593f78e12db4714e930a4af655db9da4b0c407ea2e24d3b926cef6e1f2a15de502d0d1290a6e046826b99
|
|
|
|
2e68ffaf205866e4cea71f64e79bbfb89e17280a [doc] descriptor: explain GetPubKey() usage with cached public key (Sjors Provoost)
2290269759ad10cc2e35958c7b0a63f3a7608621 scripted-diff: rename DescriptorImpl m_script_arg to m_subdescriptor_arg (Sjors Provoost)
Pull request description:
I found the name `m_script_arg` to be confusing while reviewing https://github.com/bitcoin/bitcoin/pull/14646#discussion_r240677238. @sipa let me know if `m_subdescriptor_arg` is completely wrong.
I also added an explanation of why we call `GetPubKey` when we don't ask it for a public key.
ACKs for top commit:
laanwj:
ACK 2e68ffaf205866e4cea71f64e79bbfb89e17280a
Tree-SHA512: 06698e9a91cdda93c043a82732793f0ad3cd91daa2513565953e9fa048d5573322fb534e9d0ea9ab736e6366be5921e2b8699c4f4b3693edab48039aaae06f78
|
|
|
|
|
