f036dfbb692c4d44d0f59194d089ed0aa1096347 [addrman] Remove unused test_before_evict argument from Good() (John Newbery)
Pull request description:
This has never been used in the public interface method since it was
introduced in #9037.
ACKs for top commit:
lsilva01:
Tested ACK f036dfbb69 on Ubuntu 20.04.
theStack:
Code-review ACK f036dfbb692c4d44d0f59194d089ed0aa1096347
Tree-SHA512: 98145d9596b4ae1f354cfa561be1a54c6b8057c920e0ac3d4c1d42c9326b2dad2d44320f4171bb701d97088b216760cca8017b84c8b5dcd2b1dc8f158f28066d
faa54e375782b21cbc2761c763128131c569e903 Move pblocktree global to BlockManager (MarcoFalke)
fa27f03b4943540aa2eab283d4cf50ad4a1a01f8 Move LoadBlockIndexDB to BlockManager (MarcoFalke)
Pull request description:
The block tree db is used within BlockManager to write and read the block index, so make the db global a member variable of BlockManager.
ACKs for top commit:
jamesob:
ACK faa54e375782b21cbc2761c763128131c569e903 ([`jamesob/ackr/22371.1.MarcoFalke.move_pblocktree_global_t`](https://github.com/jamesob/bitcoin/tree/ackr/22371.1.MarcoFalke.move_pblocktree_global_t))
theStack:
re-ACK faa54e375782b21cbc2761c763128131c569e903 🥧
ryanofsky:
Code review ACK faa54e375782b21cbc2761c763128131c569e903. I was thinking this looked like a change Carl would like, so no surprised he [Mega-acked](https://github.com/bitcoin/bitcoin/pull/22371#pullrequestreview-696450475)
Tree-SHA512: 1b7badbf503d53f5d4dbd9ed8f2e5c1ebfe48102665197048cc9e37bc87b5cec5f2277f3aae9f73a1095bfe879b19d288286ca3daa28031f5f1b64b1184439a9
fa621ededdfe31a200b77a8787de7e3d2e667aec refactor: Pass script verify flags as uint32_t (MarcoFalke)
Pull request description:
The flags are cast to unsigned in the interpreter anyway, so avoid the confusion (and fuzz crashes) by just passing them as unsigned from the beginning.
Also, the flags are often inverted bit-wise with the `~` operator, which also works on signed integers, but might cause confusion as the sign bit is flipped.
Fixes#22233
ACKs for top commit:
theStack:
Concept and code review ACK fa621ededdfe31a200b77a8787de7e3d2e667aec
kristapsk:
ACK fa621ededdfe31a200b77a8787de7e3d2e667aec
jonatack:
ACK fa621ededdfe31a200b77a8787de7e3d2e667aec
Tree-SHA512: ea0720f32f823fa7f075309978672aa39773c6019d12b6c1c9d611fc1983a76115b7fe2a28d50814673bb6415c311ccc05b99d6e871575fb6900faf75ed17769
facd56750c8a6aee88eeef75d8c8233778d35757 scripted-diff: Revert "fuzz: Add Temporary debug assert for oss-fuzz issue" (MarcoFalke)
Pull request description:
No longer needed, as it wouldn't help to debug this issue. See https://github.com/bitcoin/bitcoin/pull/22472#issuecomment-882692900
ACKs for top commit:
fanquake:
ACK facd56750c8a6aee88eeef75d8c8233778d35757
Tree-SHA512: 13352b3529c43d6e65ab127134b32158d3072dc2fbbb326fea9adfeada5a8610d0477ea75748b8b68e7abb3b9869a989df3a3169e92bdd458053d64bae6ed379
a2aca207b1ad00ec05d7533dbd75bbff830e1d75 Move implementations of non-template fuzz helpers (Sriram)
Pull request description:
There are 78 cpp files that include `util.h` (`grep -iIr "#include <test/fuzz/util.h>" src/test/fuzz | wc -l`). Modifying the implementation of a fuzz helper in `src/test/fuzz/util.h` will cause all fuzz tests to be recompiled. Keeping the declarations of these non-template fuzz helpers in `util.h` and moving their implementations to `util.cpp` will skip the redundant recompilation of all the fuzz tests, and builds these helpers only once in `util.cpp`.
Functions moved from `util.h` to `util.cpp`:
- `ConsumeTxMemPoolEntry`
- `ContainsSpentInput`
- `ConsumeNetAddr`
- Methods of `FuzzedFileProvider::(open, read, write, seek, close)`
ACKs for top commit:
MarcoFalke:
review ACK a2aca207b1ad00ec05d7533dbd75bbff830e1d75 🍂
Tree-SHA512: e7037ebb86d0fc56048e4f3d8733eefc21da11683b09d2b22926bda410719628d89c52ddd9b4c18aa243607a66fdb4d13a63e62ca010e66b3ec9174fd18107f0
b1d905c225e87a4a289c0cd3593c6c21cea3fba7 p2p: earlier continuation when no remaining eviction candidates (Vasil Dimov)
c9e8d8f9b168dec2bc7b845da38449e96708cf8e p2p: process more candidates per protection iteration (Jon Atack)
02e411ec456af80d1da76085a814c68bb3aca6de p2p: iterate eviction protection only on networks having candidates (Jon Atack)
5adb06457403f8c1d874e9c6748ecbb78ef8fa2b bench: add peer eviction protection benchmarks (Jon Atack)
566357f8f7471f74729297868917aa32f6d3c390 refactor: move GetRandomNodeEvictionCandidates() to test utilities (Jon Atack)
Pull request description:
This follow-up to #21261 improves `ProtectEvictionCandidatesByRatio()` for better performance.
Benchmarks are added; the performance improvement is between 2x and 5x for the benchmarked cases (CPU 2.50GHz, Turbo off, performance mode, Debian Clang 11 non-debug build).
```
$ ./src/bench/bench_bitcoin -filter="EvictionProtection*.*"
```
The refactored code is well-covered by existing unit tests and also a fuzzer.
- `$ ./src/test/test_bitcoin -t net_peer_eviction_tests`
- `$ FUZZ=node_eviction ./src/test/fuzz/fuzz ../qa-assets/fuzz_seed_corpus/node_eviction`
ACKs for top commit:
klementtan:
Tested and code review ACK b1d905c2.
vasild:
ACK b1d905c225e87a4a289c0cd3593c6c21cea3fba7
jarolrod:
ACK b1d905c225e87a4a289c0cd3593c6c21cea3fba7
Tree-SHA512: a3a6607b9ea2fec138da9780c03f63e177b6712091c5a3ddc3804b896a7585216446310280791f5e20cc023d02d2f03a4139237e12b5c1d7f2a1fa1011610e96
ceb7b35a39145717e2d9d356fd382bd1f95d2a5a refactor: move UpdateTip into CChainState (James O'Beirne)
4abf0779d6594e97222279110c328b75b5f3db7b refactor: no mempool arg to GetCoinsCacheSizeState (James O'Beirne)
46e3efd1e4ae2f058ecfffdaee7e882c4305eb35 refactor: move UpdateMempoolForReorg into CChainState (James O'Beirne)
617661703ac29e0744f21de74501d033fdc53ff6 validation: make CChainState::m_mempool optional (James O'Beirne)
Pull request description:
Make `CChainState::m_mempool` optional by making it a pointer instead of a reference. This will allow a simplification to assumeutxo semantics (see https://github.com/bitcoin/bitcoin/pull/15606#pullrequestreview-692965905) and help facilitate the `-nomempool` option.
ACKs for top commit:
jnewbery:
ACK ceb7b35a39145717e2d9d356fd382bd1f95d2a5a
naumenkogs:
ACK ceb7b35a39145717e2d9d356fd382bd1f95d2a5a
ryanofsky:
Code review ACK ceb7b35a39145717e2d9d356fd382bd1f95d2a5a (just minor style and test tweaks since last review)
lsilva01:
Code review ACK and tested on Signet ACK ceb7b35a39
MarcoFalke:
review ACK ceb7b35a39145717e2d9d356fd382bd1f95d2a5a 😌
Tree-SHA512: cc445ad33439d5918cacf80a6354eea8f3d33bb7719573ed5b970fad1a0dab410bcd70be44c862b8aba1b71263b82d79876688c553e339362653dfb3d8ec81e6
Moved implementations of `ConsumeTxMemPoolEntry`, `ContainsSpentInput`, `ConsumeNetAddr`, and the methods(open, read, write, seek, close) of FuzzedFileProvider from test/fuzz/util.h to test/fuzz/util.cpp.
Since we now have multiple chainstate objects, only one of them is active at any given
time. An active chainstate has a mempool, but there's no point to others having one.
This change will simplify proposed assumeutxo semantics. See the discussion here:
https://github.com/bitcoin/bitcoin/pull/15606#pullrequestreview-692965905
Co-authored-by: Russell Yanofsky <russ@yanofsky.org>
This is a temporary change to convert I2P addresses that have propagated
with port 8333 to ones with port 0.
It would cause a problem some day if indeed some bitcoin software is
listening on port 8333 only and rejects connections to port 0 and we are
still using SAM 3.1 which only supports port 0. In this case we would
replace 8333 with 0 and try to connect to such nodes.
This commit should be included in 22.0 and be reverted before 23.0 is
released.
4e44f5bac4481d49ac53c458dcc5ca48e8b28414 test: Correct outstanding -Werror=sign-compare errors (Ben Woosley)
Pull request description:
I'm unclear on why these aren't failing on CI, but they failed for me locally, e.g.:
```
In file included from /usr/local/include/boost/test/test_tools.hpp:46:
/usr/local/include/boost/test/tools/old/impl.hpp:107:17: error: comparison of integers of different signs: 'const unsigned int' and 'const int' [-Werror,-Wsign-compare]
return left == right;
~~~~ ^ ~~~~~
/usr/local/include/boost/test/tools/old/impl.hpp:130:16: note: in instantiation of function template specialization 'boost::test_tools::tt_detail::equal_impl<unsigned int, int>' requested here
return equal_impl( left, right );
^
/usr/local/include/boost/test/tools/old/impl.hpp:145:16: note: in instantiation of function template specialization 'boost::test_tools::tt_detail::equal_impl_frwd::call_impl<unsigned int, int>' requested here
return call_impl( left, right, left_is_array() );
^
/usr/local/include/boost/test/tools/old/impl.hpp:92:50: note: in instantiation of function template specialization 'boost::test_tools::tt_detail::equal_impl_frwd::operator()<unsigned int, int>' requested here
BOOST_PP_REPEAT( BOOST_TEST_MAX_PREDICATE_ARITY, IMPL_FRWD, _ )
^
/usr/local/include/boost/preprocessor/repetition/repeat.hpp:30:26: note: expanded from macro 'BOOST_PP_REPEAT'
^
/usr/local/include/boost/preprocessor/cat.hpp:22:32: note: expanded from macro 'BOOST_PP_CAT'
^
/usr/local/include/boost/preprocessor/cat.hpp:29:34: note: expanded from macro 'BOOST_PP_CAT_I'
^
<scratch space>:153:1: note: expanded from here
BOOST_PP_REPEAT_1
^
test/streams_tests.cpp:122:5: note: in instantiation of function template specialization 'boost::test_tools::tt_detail::check_frwd<boost::test_tools::tt_detail::equal_impl_frwd, unsigned int, int>' requested here
BOOST_CHECK_EQUAL(varint, 54321);
^
/usr/local/include/boost/test/tools/old/impl.hpp:107:17: error: comparison of integers of different signs: 'const unsigned long long' and 'const long' [-Werror,-Wsign-compare]
return left == right;
~~~~ ^ ~~~~~
/usr/local/include/boost/test/tools/old/impl.hpp:130:16: note: in instantiation of function template specialization 'boost::test_tools::tt_detail::equal_impl<unsigned long long, long>' requested here
return equal_impl( left, right );
^
/usr/local/include/boost/test/tools/old/impl.hpp:145:16: note: in instantiation of function template specialization 'boost::test_tools::tt_detail::equal_impl_frwd::call_impl<unsigned long long, long>' requested here
return call_impl( left, right, left_is_array() );
^
/usr/local/include/boost/test/tools/old/impl.hpp:92:50: note: in instantiation of function template specialization 'boost::test_tools::tt_detail::equal_impl_frwd::operator()<unsigned long long, long>' requested here
BOOST_PP_REPEAT( BOOST_TEST_MAX_PREDICATE_ARITY, IMPL_FRWD, _ )
^
/usr/local/include/boost/preprocessor/repetition/repeat.hpp:30:26: note: expanded from macro 'BOOST_PP_REPEAT'
^
/usr/local/include/boost/preprocessor/cat.hpp:22:32: note: expanded from macro 'BOOST_PP_CAT'
^
/usr/local/include/boost/preprocessor/cat.hpp:29:34: note: expanded from macro 'BOOST_PP_CAT_I'
^
<scratch space>:161:1: note: expanded from here
BOOST_PP_REPEAT_1
^
test/serfloat_tests.cpp:41:5: note: in instantiation of function template specialization 'boost::test_tools::tt_detail::check_frwd<boost::test_tools::tt_detail::equal_impl_frwd, unsigned long long, long>' requested here
BOOST_CHECK_EQUAL(TestDouble(std::numeric_limits<double>::infinity()), 0x7ff0000000000000);
^
ACKs for top commit:
theStack:
ACK 4e44f5bac4481d49ac53c458dcc5ca48e8b28414
Tree-SHA512: 8d9e5245676c61207ceacdf78c78a78ccc9fd2a2551d4d8df023513795591334aa2f5e1f4a2a8ed2bfeb381f1e226b6ba84c07e0de29a1f3f00da71f3a257bc1
00b875ba9414463d0041da6924fd9b54d6a06dee addrman: remove invalid addresses when unserializing (Vasil Dimov)
bdb62096f0109b2ec76849d33d6cf7187dea299f fuzz: reduce possible networks check (Vasil Dimov)
a164cd3ba694ffeba03b2887a411b7f82f6c087e net: simplify CNetAddr::IsRoutable() (Vasil Dimov)
Pull request description:
* Simplify some code, now that we know `CNetAddr::IsRFC4193()` and `CNetAddr::IsTor()` cannot be `true` at the same time.
* Drop Tor v2 addresses when loading addrman from `peers.dat` - they would have been loaded as dummy-all-zeros IPv6 addresses and linger in addrman, wasting space.
ACKs for top commit:
sipa:
ACK 00b875ba9414463d0041da6924fd9b54d6a06dee. Reviewed the code, and tested with -DDEBUG_ADDRMAN (unit tests + mainnet run with peers.dat that contained v2 onions).
laanwj:
Code review and lightly tested ACK 00b875ba9414463d0041da6924fd9b54d6a06dee
jonatack:
ACK 00b875ba9414463d0041da6924fd9b54d6a06dee reviewed, debug-built with -DEBUG_ADDRMAN rebased to current master, restarted node on mainnet/signet/testnet and verified that on each chain -addrinfo shows no change in address counts (as expected). Added some sanity check asserts, rebuilt/re-ran test. Checked that the new test fails on master with "test/addrman_tests.cpp(824): error: in "addrman_tests/remove_invalid": check addrman.size() == 2 has failed [4 != 2]"
jarolrod:
ACK 00b875ba9414463d0041da6924fd9b54d6a06dee
Tree-SHA512: 6ed8e6745134b1b94fffaba28482de909ea39483b46b7f57bda61cdbae7a51251d15cb674de3631772fbeabe153d77a19269f96e62a89102a2d5c01e48f0ba06
* Assert when a type is missing
* Add missing WitnessV1Taproot
* Limit WitnessUnknown to version [2, 16], to avoid abiguity
* Limit WitnessUnknown to size [2, 40], to avoid invalid sizes
e48826ad87b4f92261f7433e84f48dac9bd9e5c3 tests: remove ComputeBlockVersion shortcut from versionbits tests (Anthony Towns)
c5f36725e877d8eb492383844f8ef7535466b366 [refactor] Move ComputeBlockVersion into VersionBitsCache (Anthony Towns)
4a69b4dbe0d7f504811b67c399da7e6d11e4f805 [move-only] Move ComputeBlockVersion from validation to versionbits (Anthony Towns)
0cfd6c6a8f929d5567ac41f95c21548f115efee5 [refactor] versionbits: make VersionBitsCache a full class (Anthony Towns)
8ee3e0bed5bf2cd3c7a68ca6ba6c65f7b9a72cca [refactor] rpc/blockchain.cpp: SoftForkPushBack (Anthony Towns)
92f48f360da5f425428b761219301f509826bec4 deploymentinfo: Add DeploymentName() (Anthony Towns)
ea68b3a5729f5d240e968388c4f88acffeb27228 [move-only] Rename versionbitsinfo to deploymentinfo (Anthony Towns)
c64b2c6a0f79369624ae96b2e3d579d50aae4de6 scripted-diff: rename versionbitscache (Anthony Towns)
de55304f6e7a8b607e6b3fc7436de50910747b0c [refactor] Add versionbits deployments to deploymentstatus.h (Anthony Towns)
2b0d291da8f479739ff394dd92801da8c40b9f8e [refactor] Add deploymentstatus.h (Anthony Towns)
eccd736f3dc231ac0306ca763c3b72cf8247230a versionbits: Use dedicated lock instead of cs_main (Anthony Towns)
36a4ba0aaaa9b35185d7178994e36bc02cca9887 versionbits: correct doxygen comments (Anthony Towns)
Pull request description:
Introduces helper functions to make it easy to bury future deployments, along the lines of the suggestion from [11398](https://github.com/bitcoin/bitcoin/pull/11398#issuecomment-335599326) "I would prefer it if a buried deployment wouldn't require all code paths that check the BIP9 status to require changing".
This provides three functions: `DeploymentEnabled()` which tests if a deployment can ever be active, `DeploymentActiveAt()` which checks if a deployment should be enforced in the given block, and `DeploymentActiveAfter()` which checks if a deployment should be enforced in the block following the given block, and overloads all three to work both with buried deployments and versionbits deployments.
This adds a dedicated lock for the versionbits cache, which is acquired internally by the versionbits functions, rather than relying on `cs_main`. It also moves moves versionbitscache into deploymentstatus to avoid a circular dependency with validation.
ACKs for top commit:
jnewbery:
ACK e48826ad87b4f92261f7433e84f48dac9bd9e5c3
gruve-p:
ACK e48826ad87
MarcoFalke:
re-ACK e48826ad87b4f92261f7433e84f48dac9bd9e5c3 🥈
Tree-SHA512: c846ba64436d36f8180046ad551d8b0d9e20509b9bc185aa2639055fc28803dd8ec2d6771ab337e80da0b40009ad959590d5772f84a0bf6199b65190d4155bed
e6cf0ed92de31a5ac35a271b0da8f0a8364d1175 wallet, rpc: listdescriptors does not need unlocked (Andrew Chow)
3280704886b60644d103a5eb310691c003a39328 Pass in DescriptorCache to ToNormalizedString (Andrew Chow)
7a26ff10c2f2e139fbc63e2f37fb33ea4efae088 Change DescriptorImpl::ToStringHelper to use an enum (Andrew Chow)
75530c93a83f3e94bcb78b6aa463c5570c1e737e Remove priv option for ToNormalizedString (Andrew Chow)
74fede3b8ba69e2cc82c617cdf406ab79df58825 wallet: Upgrade existing descriptor caches (Andrew Chow)
432ba9e5434da90d2cf680f23e8c7b7164c9f945 wallet: Store last hardened xpub cache (Andrew Chow)
d87b544b834077f102724415e0fada6ee8b2def2 descriptors: Cache last hardened xpub (Andrew Chow)
cacc3910989c4f3d7afa530dbab042461426abce Move DescriptorCache writing to WalletBatch (Andrew Chow)
0b4c8ef75cd03c8f0a8cfadb47e0fbcabe3c5e59 Refactor Cache merging and writing (Andrew Chow)
976b53b085d681645fd3a008fe382de85647e29f Revert "Cache parent xpub inside of BIP32PubkeyProvider" (Andrew Chow)
Pull request description:
Currently fetching a normalized descriptor requires the wallet to be unlocked as it needs the private keys to derive the last hardened xpub. This is not very user friendly as normalized descriptors shouldn't require and don't involve the private keys except for derivation. We solve this problem by caching the last hardened xpub (which has to be derived at some point when generating the address pool).
However the last hardened xpub was not already being cached. We only cached the immediate parent xpub and derived child keys. For example, with a descriptor derivation path of `/84'/0'/0'/0/*`, the parent xpub that is cached is `m/84'/0'/0'/0`, and the child keys of `m/84'/0'/0'/0/i` (note that child keys would not be cached in this case). This parent xpub is not suitable for the normalized descriptor form as we want the key at `m/84'/0'/0'`. So this PR adds another field to `DescriptorCache` to cache the last hardened xpub so that we can use them for normalized descriptors.
Since `DescriptorCache` is changing, existing descriptor wallets need to be upgraded to use this new cache. The upgrade will occur in the background either at loading time (if the wallet is not encrypted) or at unlocking time in the same manner that `UpgradeKeyMetadata` operates. It will use a new wallet flag `WALLET_FLAG_LAST_HARDENED_XPUB_CACHED` to indicate whether the descriptor wallet has the last hardened xpub cache.
Lastly `listdescriptors` will not require the wallet to be locked and `getaddressinfo`'s `parent_desc` will always be output (assuming the upgrade has occurred).
ACKs for top commit:
fjahr:
tACK e6cf0ed92de31a5ac35a271b0da8f0a8364d1175
S3RK:
reACK e6cf0ed
jonatack:
Semi ACK e6cf0ed92de31a5ac35a271b0da8f0a8364d1175 reviewed, debug-built and ran unit tests and some of the descriptor functional tests at each commit. I'm not very familiar with this code and it could be clearer to the uninitiated IMHO, so I'm not confident enough to give a full ACK. Various minor suggestions follow, most of them for readability, feel free to pick and choose.
meshcollider:
Code review + functional test run ACK e6cf0ed92de31a5ac35a271b0da8f0a8364d1175
Tree-SHA512: ac27aade8644525cd65bfcaf27ff32afb974085b1451faf4ff68c6671a690bd6a41d4f39a33cbf461ae0fbe85995c0a4c08dbd36171da1c1d2a1d00053ad298d
Adds support for versionbits deployments to DeploymentEnabled,
DeploymentActiveAfter and DeploymentActiveAt. Also moves versionbitscache
from validation to deploymentstatus.
fa0d9211ef87a682573aaae932c0c440acbcb8a8 refactor: Remove chainparams arg from CChainState member functions (MarcoFalke)
fa389471251f043ec25e7b01e59b37d3b921ce54 refactor: Remove ::Params() global from inside CChainState member functions (MarcoFalke)
Pull request description:
The `::Params()` global is verbose and confusing. Also it makes tests a bit harder to write because they'd have to mock a global.
Fix all issues by simply using a member variable that points to the right params.
(Can be reviewed with `--word-diff-regex=.`)
ACKs for top commit:
jnewbery:
ACK fa0d9211ef87a682573aaae932c0c440acbcb8a8
kiminuo:
utACK fa0d9211
theStack:
ACK fa0d9211ef87a682573aaae932c0c440acbcb8a8 🍉
Tree-SHA512: 44676b19c9ed471ccb536331d3029bad192d7d50f394fd7b8527ec431452aeec8c4494164b9cf8e16e0123c4463b16be864366c6b599370032c17262625a0356
7ad414f4bfa74595ee5726e66f3527045c02a977 doc: add comment about CCoinsViewDBCursor constructor (James O'Beirne)
0f8a5a4dd530549d37c43da52c923ac3b2af1a03 move-only(ish): don't expose CCoinsViewDBCursor (James O'Beirne)
615c1adfb07b9b466173166dc2e53ace540e4b32 refactor: wrap CCoinsViewCursor in unique_ptr (James O'Beirne)
Pull request description:
I tripped over this one for a few hours at the beginning of the week, so I've sort of got a personal vendetta against `CCoinsView::Cursor()` returning a raw pointer.
Specifically in the case of CCoinsViewDB, if a raw cursor is allocated and not freed, a cryptic leveldb assertion failure occurs on CCoinsViewDB destruction (`Assertion 'dummy_versions_.next_ == &dummy_versions_' failed.`).
This is a pretty simple change.
Related to: https://github.com/bitcoin/bitcoin/issues/21766
See also: https://github.com/google/leveldb/issues/142#issuecomment-414418135
ACKs for top commit:
MarcoFalke:
review ACK 7ad414f4bfa74595ee5726e66f3527045c02a977 🔎
jonatack:
re-ACK 7ad414f4bfa74595ee5726e66f3527045c02a977 modulo suggestion
ryanofsky:
Code review ACK 7ad414f4bfa74595ee5726e66f3527045c02a977. Two new commits look good and thanks for clarifying constructor comment
Tree-SHA512: 6471d03e2de674d84b1ea0d31e25f433d52aa1aa4996f7b4aab1bd02b6bc340b15e64cc8ea07bbefefa3b5da35384ca5400cc230434e787c30931b8574c672f9
Save the banlist in `banlist.json` instead of `banlist.dat`.
This makes it possible to store Tor v3 entries in the banlist on disk
(and any other addresses that cannot be serialized in addrv1 format).
Only read `banlist.dat` if it exists and `banlist.json` does not
exist (first start after an upgrade).
Supersedes https://github.com/bitcoin/bitcoin/pull/20904
Resolves https://github.com/bitcoin/bitcoin/issues/19748
458a345b0590fd2fa04c7d8d70beb8d57e34bbc8 Add support for SIGHASH_DEFAULT in RPCs, and make it default (Pieter Wuille)
c0f0c8eccb04f90940007e0c6aaff56bf2ab35b5 tests: check spending of P2TR (Pieter Wuille)
a2380127e905e5849f90acc7c69832859d8336aa Basic Taproot signing logic in script/sign.cpp (Pieter Wuille)
49487bc3b6038393c1b9c2dbdc04a78ae1178f1a Make GetInputUTXO safer: verify non-witness UTXO match (Pieter Wuille)
fd3f6890f3dfd683f6f13db912caf5c4288adf08 Construct and use PrecomputedTransactionData in PSBT signing (Pieter Wuille)
5cb6502ac5730ea453edbec4c46027ac2ada97e0 Construct and use PrecomputedTransactionData in SignTransaction (Pieter Wuille)
5d2e22437b22e7465ae4be64069443bcc1769dc9 Don't nuke witness data when signing fails (Pieter Wuille)
ce9353164bdb6215a62b2b6dcb2121d331796f60 Permit full precomputation in PrecomputedTransactionData (Pieter Wuille)
e841fb503d7a662bde01ec2e4794faa989265950 Add precomputed txdata support to MutableTransactionSignatureCreator (Pieter Wuille)
a91d532338ecb66ec5bed164929d878dd55d63a4 Add CKey::SignSchnorr function for BIP 340/341 signing (Pieter Wuille)
e77a2839b54fa2039bba468e8c09dbbbf19b150a Use HandleMissingData also in CheckSchnorrSignature (Pieter Wuille)
dbb0ce9fbff01ffe4dd29da465f43ecaddc2854c Add TaprootSpendData data structure, equivalent to script map for P2[W]SH (Pieter Wuille)
Pull request description:
Builds on top of #22051, adding signing support after derivation support.
Nothing is changed in descriptor features. Signing works for key path and script path spending, through the normal sending functions, and PSBT-based RPCs. However, PSBT usability is rather low as no extensions have been defined to convey Taproot-specific information, so all script information must be known to the signing wallet.
ACKs for top commit:
achow101:
re-ACK 458a345b0590fd2fa04c7d8d70beb8d57e34bbc8
fjahr:
Code review ACK 458a345b0590fd2fa04c7d8d70beb8d57e34bbc8
Sjors:
ACK 458a345b0590fd2fa04c7d8d70beb8d57e34bbc8
Tree-SHA512: 30ed212cf7754763a4a81624ebc084c51727b8322711ac0b390369213c1a891d367ed8b123882ac08c99595320c11ec57ee42304ff22a69afdc3d1a0d55cc711
9550dffa0c61df6d1591c62d09629b4c5731e1b7 fuzz: Assert roundtrip equality for `CPubKey` (Sebastian Falbesoner)
Pull request description:
This PR is a (quite late) follow-up to #19237 (https://github.com/bitcoin/bitcoin/pull/19237#issuecomment-642203251). Looking at `CPubKey::Serialize` and `CPubKey::Unserialize` I can't think of a scenario where the roundtrip (serialization/deserialization) equality wouldn't hold.
ACKs for top commit:
jamesob:
crACK 9550dffa0c pending CI
Tree-SHA512: 640fb9e777d249769b22ee52c0b15a68ff0645b16c986e1c0bce9742155d14f1be601e591833e1dc8dcffebf271966c6b861b90888a44aae1feae2e0248e2c55
f8866e8c324be3322fa507c2ceb1de35d148d0f1 Add roundtrip fuzz tests for CAddress serialization (Pieter Wuille)
e2f0548b52a4b2ba3edf77e3f21365f1e8f270a4 Use addrv2 serialization in anchors.dat (Pieter Wuille)
8cd8f37dfe3ffb73a09f3ad773603d9d89452245 Introduce well-defined CAddress disk serialization (Pieter Wuille)
Pull request description:
Alternative to #20509.
This makes the `CAddress` disk serialization format well defined, and uses it to enable addrv2 support in anchors.dat (in a way that's compatible with older software). The new format is:
- The first 4 bytes store a format version number. Its low 19 bits are ignored (as those historically stored the `CLIENT_VERSION`), but its high 13 bits specify the actual serialization:
- 0x00000000: LE64 encoding for `nServices`, V1 encoding for `CService` (like pre-BIP155 network serialization).
- 0x20000000: CompactSize encoding for `nServices`, V2 encoding for `CService` (like BIP155 network serialization).
- Any other value triggers an unsupported format error on deserialization, and can be used for future format changes.
- The `ADDRV2_FORMAT` flag in the stream's version does not determine the actual serialization format; it only sets whether or not V2 encoding is permitted.
ACKs for top commit:
achow101:
ACK f8866e8c324be3322fa507c2ceb1de35d148d0f1
laanwj:
Code review ACK f8866e8c324be3322fa507c2ceb1de35d148d0f1
vasild:
ACK f8866e8c324be3322fa507c2ceb1de35d148d0f1
jonatack:
ACK f8866e8c324be3322fa507c2ceb1de35d148d0f1 tested rebased to master and built/run/restarted with DEBUG_ADDRMAN, peers.dat and anchors ser/deser seems fine
hebasto:
ACK f8866e8c324be3322fa507c2ceb1de35d148d0f1, tested on Linux Mint 20.1 (x86_64).
Tree-SHA512: 3898f8a8c51783a46dd0aae03fa10060521f5dd6e79315fe95ba807689e78f202388ffa28c40bf156c6f7b1fc2ce806b155dcbe56027df73d039a55331723796
faf1af58f85da74f94c6b5f6910c7faf7b47cc88 fuzz: Add Temporary debug assert for oss-fuzz issue (MarcoFalke)
Pull request description:
oss-fuzz is acting weird, so add an earlier assert to help troubleshooting
ACKs for top commit:
practicalswift:
cr ACK faf1af58f85da74f94c6b5f6910c7faf7b47cc88
Tree-SHA512: 85830d7d47cf6b4edfe91a07bd5aa8f7110db0bade8df93868cf276ed04d5dd17e671f769e6a0fb5092012b86aa82bb411fb171411f15746981104ce634c88c1
The unit test is single threaded, so there's no need to hold the mutex
between Good() and Attempt().
This change avoids recursive locking in the CAddrMan::Attempt function.
Co-authored-by: John Newbery <john@johnnewbery.com>
1b1088d52fbff8b1c9438d6aa8c6edcbdd471457 test: add combined I2P/onion/localhost eviction protection tests (Jon Atack)
7c2284eda22a08dbf2a560894e496e245d026ee0 test: add tests for inbound eviction protection of I2P peers (Jon Atack)
ce02dd1ef1f7f54f33780b32f195d31c1cc87318 p2p: extend inbound eviction protection by network to I2P peers (Jon Atack)
70bbc62711643ec57cce620f9f7a0e1fe5fb6346 test: add combined onion/localhost eviction protection coverage (Jon Atack)
045cb40192bf3dfa6c42916237e55f86bbc788d4 p2p: remove unused m_is_onion member from NodeEvictionCandidate struct (Jon Atack)
310fab49282d507e5fa710afb20d036604bbf3a2 p2p: remove unused CompareLocalHostTimeConnected() (Jon Atack)
9e889e8a5c021b0ec7e4c4d17d418ab4a0accad4 p2p: remove unused CompareOnionTimeConnected() (Jon Atack)
787d46bb2a39fb39166882cc6e0afbc34424d88e p2p: update ProtectEvictionCandidatesByRatio() doxygen docs (Jon Atack)
1e15acf478ae071234350c9b38dc823dfe2e3419 p2p: make ProtectEvictionCandidatesByRatio() fully ratio-based (Jon Atack)
3f8105c4d251e0e81bdd31f0999004e36f8990b2 test: remove combined onion/localhost eviction protection tests (Jon Atack)
38a81a8e20b0e5ad9fef0eae8abd914619f05b25 p2p: add CompareNodeNetworkTime() comparator struct (Jon Atack)
4ee7aec47ebf6b59b4d930e6e4025e91352c05b4 p2p: add m_network to NodeEvictionCandidate struct (Jon Atack)
7321e6f2fe1641eb331f30e68646f5984d4bcbb3 p2p, refactor: rename vEvictionCandidates to eviction_candidates (Jon Atack)
ec590f1d91325404383d74098a5b42a2cd67dad9 p2p, refactor: improve constness in ProtectEvictionCandidatesByRatio() (Jon Atack)
4a19f501abac4adb476a6f2a30dfdf1a35892ccc test: add ALL_NETWORKS to test utilities (Jon Atack)
519e76bb64d03ecac175ec33c31e37d0e90f037f test: speed up and simplify peer_eviction_test (Jon Atack)
1cde8005233d163723d4d5bf1bacf22e6cb7a07e p2p, refactor: rm redundant erase_size calculation in SelectNodeToEvict() (Jon Atack)
Pull request description:
Continuing the work in #20197 and #20685, this pull updates and abstracts our inbound eviction protection to make it fully ratio-based and easily extensible to peers connected via high-latency privacy networks that we newly support, like I2P and perhaps others soon, as these peers are disadvantaged by the latency criteria of our eviction logic.
It then adds eviction protection for peers connected over I2P. As described in https://github.com/bitcoin/bitcoin/pull/20685#issuecomment-767486499, we've observed over the past few months that I2P peers have a min ping latency similar to or greater than that of onion peers.
The algorithm is a basically a multi-pass knapsack:
- Count the number of eviction candidates in each of the disadvantaged
privacy networks.
- Sort the networks from lower to higher candidate counts, so that
a network with fewer candidates will have the first opportunity
for any unused slots remaining from the previous iteration. In
the case of a tie in candidate counts, priority is given by array
member order from first to last, guesstimated to favor more unusual
networks.
- Iterate through the networks in this order. On each iteration,
allocate each network an equal number of protected slots targeting
a total number of candidates to protect, provided any slots remain
in the knapsack.
- Protect the candidates in that network having the longest uptime,
if any in that network are present.
- Continue iterating as long as we have non-allocated slots
remaining and candidates available to protect.
The goal of this logic is to favorise the diversity of our peer connections.
The individual commit messages describe each change in more detail.
Special thank you to Vasil Dimov for the excellent review feedback and the algorithm improvement that made this change much better than it would have been otherwise. Thanks also to Antoine Riard, whose review feedback nudged this change to protect disadvantaged networks having fewer, rather than more, eviction candidates.
ACKs for top commit:
laanwj:
Code review re-ACK 1b1088d52fbff8b1c9438d6aa8c6edcbdd471457
vasild:
ACK 1b1088d52fbff8b1c9438d6aa8c6edcbdd471457
Tree-SHA512: 722f790ff11f2969c79e45a5e0e938d94df78df8687e77002f32e3ef5c72a9ac10ebf8c7a9eb7f71882c97ab0e67b2778191effdb747d9ca54d7c23c2ed19a90
