aboutsummaryrefslogtreecommitdiff
path: root/src/test
AgeCommit message (Collapse)Author
2021-04-14test: Use mocktime to avoid intermittent failureMarcoFalke
2021-04-13net: flag relevant Sock methods with [[nodiscard]]Vasil Dimov
2021-04-13Merge #21575: refactor: Create blockstorage modulefanquake
fadcd3f78e1dd1acd7a774f8fad68dc471ff9e1f doc: Remove irrelevant link to GitHub (MarcoFalke) fa121b628d51bb0e25eb3fbd716881fa55527dc7 blockstorage: [refactor] Use chainman reference where possible (MarcoFalke) fa0c7d9ad24d3c9515d3f9c136af4071cbd79055 move-only: Move *Disk functions to blockstorage (MarcoFalke) fa91b2b2b3447a3645e7958c7dc4e1946a69cb9c move-only: Move AbortNode to shutdown (MarcoFalke) fa413f07a14744e7d7f7746e861aabd9cf938f61 move-only: Move ThreadImport to blockstorage (MarcoFalke) faf843c07f99f91603e08ea858f972516f1d669a refactor: Move load block thread into ChainstateManager (MarcoFalke) Pull request description: This picks up the closed pull request #21030 and is the first step toward fixing #21220. The basic idea is to move all disk access into a separate module with benefits: * Breaking down the massive files init.cpp and validation.cpp into logical units * Creating a standalone-module to reduce the mental complexity * Pave the way to fix validation related circular dependencies * Pave the way to mock disk access for testing, especially where it is performance critical (like fuzzing) ACKs for top commit: promag: Code review ACK fadcd3f78e, checked (almost) moved only changes. This is a nice tidy up change and doesn't change behavior. Easily reviewed commit by commit. jamesob: ACK fadcd3f78e1dd1acd7a774f8fad68dc471ff9e1f ([`jamesob/ackr/21575.1.MarcoFalke.refactor_create_blocksto`](https://github.com/jamesob/bitcoin/tree/ackr/21575.1.MarcoFalke.refactor_create_blocksto)) ryanofsky: Code review ACK fadcd3f78e1dd1acd7a774f8fad68dc471ff9e1f. New organization makes sense, moves extraneous things outside of validation.cpp. PR is also easy to review with helpfully split up moveonly commits. Tree-SHA512: 917996592b6d8f9998289d8cb2b1b78b23d1fdb3b07216c9caec1380df33baa09dc2c1e706da669d440b497e79c9c62a01ca20dc202df5ad974a75f3ef7a143b
2021-04-13Merge #21633: refactor: add [[noreturn]] attribute where applicablefanquake
003929c0d55532038d5bf6fc0ff4a20628710fae refactor: add [[noreturn]] attribute where applicable (fanquake) Pull request description: Similar to #10843. We could build with `-Wmissing-noreturn`, however that would also mean modifying something like `--suppress-external-warnings` to suppress warnings for leveldb, which I don't think we want to do. In any case, the functions where this is applicable are only added/removed very rarely. ACKs for top commit: vasild: ACK 003929c0d55532038d5bf6fc0ff4a20628710fae Tree-SHA512: 33dfa6547d6b84f38a941f24d4c2effe8fde7b93dbc0b27a9309716420e4a879fdbe689d789fa5439d65f5f78292f89fd9dc1b61c97acf69316dfed954086705
2021-04-13Merge #21330: Deal with missing data in signature hashes more consistentlyfanquake
725d7ae0494d4a45f5a840bbbd19c008a7363965 Use PrecomputedTransactionData in signet check (Pieter Wuille) 497718b467330b2c6bb0d44786020c55f1aa75f9 Treat amount<0 also as missing data for P2WPKH/P2WSH (Pieter Wuille) 3820090bd619ac85ab35eff376c03136fe4a9f04 Make all SignatureChecker explicit about missing data (Pieter Wuille) b77b0cc507bdc716e5236b1d9880e648147e0af9 Add MissingDataBehavior and make TransactionSignatureChecker handle it (Pieter Wuille) Pull request description: Currently we have 2 levels of potentially-missing data in the transaction signature hashes: * P2WPKH/P2WSH hashes need the spent amount * P2TR hashes need all spent outputs (amount + scriptPubKey) Missing amounts are treated as -1 (thus leading to unexpected signature failures), while missing outputs in P2TR validation cause assertion failure. This is hard to extend for signing support, and also quite ugly in general. In this PR, an explicit configuration option to {Mutable,}TransactionSignatureChecker is added (MissingDataBehavior enum class) to either select ASSERT_FAIL or FAIL. Validation code passes ASSERT_FAIL (as at validation time all data should always be passed, and anything else is a serious bug in the code), while signing code uses FAIL. The existence of the ASSERT_FAIL option is really just an abundance of caution. Always using FAIL should be just fine, but if there were for some reason a code path in consensus code was introduced that misses certain data, I think we prefer as assertion failure over silently introducing a consensus change. Potentially useful follow-ups (not for this PR, in my preference): * Having an explicit script validation error code for missing data. * Having a MissingDataBehavior::SUCCEED option as well, for use in script/sign.cpp DataFromTransaction (if a signature is present in a witness, and we don't have enough data to fully validate it, we should probably treat it as valid and not touch it). ACKs for top commit: sanket1729: reACK 725d7ae0494d4a45f5a840bbbd19c008a7363965 Sjors: ACK 725d7ae0494d4a45f5a840bbbd19c008a7363965 achow101: re-ACK 725d7ae0494d4a45f5a840bbbd19c008a7363965 benthecarman: ACK 725d7ae0494d4a45f5a840bbbd19c008a7363965 fjahr: Code review ACK 725d7ae0494d4a45f5a840bbbd19c008a7363965 Tree-SHA512: d67dc51bae9ca7ef6eb9acccefd682529f397830f77d74cd305500a081ef55aede0e9fa380648c3a8dd4857aa7eeb1ab54fe808979d79db0784ac94ceb31b657
2021-04-13refactor: add [[noreturn]] attribute where applicablefanquake
2021-04-12versionbits: simplify state transitionsAnthony Towns
This removes the DEFINED->FAILED transition and changes the STARTED->FAILED transition to only occur if signalling didn't pass the threshold. This ensures that it is always possible for activation to occur, no matter what settings are chosen, or the speed at which blocks are found.
2021-04-12versionbits: Add explicit NEVER_ACTIVE deploymentsAnthony Towns
Previously we used deployments that would timeout prior to Bitcoin's invention, which allowed the deployment to still be activated in unit tests. This switches those deployments to be truly never active.
2021-04-12fuzz: test versionbits delayed activationAnthony Towns
2021-04-12tests: test versionbits delayed activationAnthony Towns
2021-04-12versionbits: Add support for delayed activationAnthony Towns
2021-04-12tests: clean up versionbits testAnthony Towns
Simplify the versionbits unit test slightly to make the next set of changes a little easier to follow.
2021-04-12tests: test ComputeBlockVersion for all deploymentsAnthony Towns
This generalises the ComputeBlockVersion test so that it can apply to any activation parameters we might set, and checks all the parameters set for each deployment on each chain, to simultaneously ensure that the deployments we have configured work sensibly, and that the test code does not suffer bitrot in the event that all interesting deployments are buried.
2021-04-12tests: pull ComputeBlockVersion test into its own functionAnthony Towns
The intent here is to allow checking ComputeBlockVersion behaviour with each deployment, rather than only testdummy on mainnet. This commit does the trivial refactoring component of that change.
2021-04-11Merge #21602: rpc: add additional ban time fields to listbannedMarcoFalke
d3b0b08b0f04d2f1dbebbafd7ab0384dfe045dec doc: release notes for new listbanned fields (Jarol Rodriguez) 60290d3f5ec8e7e3b8cb1ebae02d5d72f6005184 test: increase listbanned unit test coverage (Jon Atack) 3e978d1a5dbd43f85bd03e759984ab1f209d6e34 rpc: add time_remaining field to listbanned (Jarol Rodriguez) 5456b345312857981cb426712f0665800c682e09 rpc: add ban_duration field to listbanned (Jarol Rodriguez) c95c61657afd058b46549fb3d65633d7c736f5fc doc: improve listbanned help (Jarol Rodriguez) dd3c8eaa3399b28dc78a883ff78cbe7cc5c31b5b rpc: swap position of banned_until and ban_created fields (Jarol Rodriguez) Pull request description: This PR adds a `ban_duration` and `time_remaining` field to the `listbanned` RPC command. Thanks to jonatack, this PR also expands the `listbanned` test coverage to include these new fields It's useful to keep track of `ban_duration` as this is another data point on which to sort banned peers. I found this helpful in adding additional context columns to the GUI `bantablemodel` as part of a follow-up PR. As [suggested](https://github.com/bitcoin/bitcoin/pull/21602#issuecomment-813486134) by jonatack, `time_remaining` is another useful user-centric data point. Since a ban always expires after its created, the `ban_created` field is now placed before the `banned_until` field. This new ordering is more logical. This PR also improves the `help listbanned` output by providing additional context to the descriptions of the `address`, `ban_created`, and `banned_until` fields. **Master: listbanned** ``` [ { "address": "1.2.3.4/32", "banned_until": 1617691101, "ban_created": 1617604701 }, { "address": "135.181.41.129/32", "banned_until": 1649140716, "ban_created": 1617604716 } ] ``` **PR: listbanned** ``` [ { "address": "1.2.3.4/32", "ban_created": 1617775773, "banned_until": 1617862173, "ban_duration": 86400, "time_remaining": 86392 }, { "address": "3.114.211.172/32", "ban_created": 1617753165, "banned_until": 1618357965, "ban_duration": 604800, "time_remaining": 582184 } ] ``` ACKs for top commit: jonatack: re-ACK d3b0b08b0f04d2f1dbebbafd7ab0384dfe045dec hebasto: ACK d3b0b08b0f04d2f1dbebbafd7ab0384dfe045dec, tested on Linux Mint 20.1 (x86_64). MarcoFalke: review ACK d3b0b08b0f04d2f1dbebbafd7ab0384dfe045dec 🕙 Tree-SHA512: 5b83ed2483344e546d57e43adc8a1ed7a1fff292124b14c86ca3a1aa2aec8b0f7198212fabff2c5145e7f726ca04ae567fe667b141254c7519df290cf63774e5
2021-04-09Merge #21606: fuzz: Extend psbt fuzz target a bitMarcoFalke
faaf3954e2f0089b6c6b9965f15e7f9af09c6fb0 fuzz: Extend psbt fuzz target a bit (MarcoFalke) Pull request description: Previously it only merged the psbt with itself, now it tries to merge another. ACKs for top commit: practicalswift: Tested ACK faaf3954e2f0089b6c6b9965f15e7f9af09c6fb0 Tree-SHA512: e1b1d31a47d35e1767285bc2fda176c79cb0550d6d383fe467104272e61e1c83f6cbc0c7d6bbc0c3027729eec13ae1f289f8950117ee91e0fb3703e66d5e6918
2021-04-09fuzz: Extend psbt fuzz target a bitMarcoFalke
2021-04-09Merge #21592: test: Remove option to make TestChain100Setup non-deterministicMarcoFalke
fa6183d7761eef8bb815aa888f0396e92e275f05 test: Remove option to make TestChain100Setup non-deterministic (MarcoFalke) fa732bccb3cf9bc2cdc444975286df0e799917a2 test: Use compressed keys in TestChain100Setup (MarcoFalke) Pull request description: Seems odd to have an option for non-deterministic tests when the goal should be for all tests to be deterministic. ACKs for top commit: jamesob: ACK https://github.com/bitcoin/bitcoin/pull/21592/commits/fa6183d7761eef8bb815aa888f0396e92e275f05 practicalswift: cr ACK fa6183d7761eef8bb815aa888f0396e92e275f05: patch looks deterministic! Tree-SHA512: 6897a9f36e0dfb7d63b25dd6984414b3ee8a62458ad232cb21ed5077184fdb0bc626996e4ac84ef0bdd452b9f17c54aac75a71575b8e723b84cac07c9f9d5611
2021-04-08Merge #21574: Drop JSONRPCRequest constructors after #21366MarcoFalke
9044522ef76f880760165d98fab024802ccfc062 Drop JSONRPCRequest constructors after #21366 (Russell Yanofsky) Pull request description: This just makes an additional simplification after #21366 replaced util::Ref with std::any. It was originally suggested https://github.com/bitcoin/bitcoin/pull/21366#issuecomment-792044351 but delayed for a followup. It would have prevented usage bug https://github.com/bitcoin/bitcoin/pull/21572. ACKs for top commit: promag: ACK 9044522ef76f880760165d98fab024802ccfc062, fixed conflict in src/wallet/interfaces.cpp. Tree-SHA512: e909411b8f75013620b94e1a609296befb832fdcb574cd2e6689bfe3c636b03cd4ac1ccb2b32b532daf0f2131bb043464024966310fffc7e3cad77713d4bd0ef
2021-04-08test: Remove option to make TestChain100Setup non-deterministicMarcoFalke
Seems odd to have an option for non-deterministic tests when the goal should be for all tests to be deterministic. Can be reviewed with `--ignore-all-space`.
2021-04-08test: Use compressed keys in TestChain100SetupMarcoFalke
coinbaseKey.MakeNewKey(true); creates a compressed key and there is no reason for the deterministic setup to use uncompressed ones.
2021-04-07[test] check that mapFlagNames is up to dateglozow
There is no way to iterate through all script verification flags, and it's not guaranteed that every power of 2 is used. Just make sure that all flags in STANDARD_SCRIPT_VERIFY_FLAGS are present in mapFlagNames; this covers all consensus and policy flags. If mapFlagNames has more flags than STANDARD_SCRIPT_VERIFY_FLAGS, that's okay. Nonexistent flags will be caught by the compiler.
2021-04-07[test] remove unnecessary OP_1s from invalid testsglozow
Similar to 19db590d044efe7d474a16720e5b56e7b55db54c, which removed these for the valid tests. Not removing ones that cause a false/empty stack error because these tests should fail due to being invalid with CSV/CLTV
2021-04-07[test] minor improvements / followupsglozow
Add missing script verify flags to mapFlagNames. iterate through mapFlagNames values instead of bits. BOOST_CHECK_MESSAGE better reports which test failed exactly, whereas BOOST_ERROR was just incrementing the error counter.
2021-04-07[test] fix bug in ExcludeIndividualFlagsglozow
PR #19168 introduced this function but it always returns an empty vector.
2021-04-08Merge #21626: doc: Fix typos from codespellfanquake
94c7dd9ac810a60f9c818c494273ad798cbac34b doc: Fix typos from codespell lint (Yerzhan Mazhkenov) Pull request description: Typos from codespell linter: https://cirrus-ci.com/task/6677401661865984?logs=lint#L856 - txrequest.cpp: `annoucements` ==> `announcements` - contrib/guix/README.md:298: `stil` ==> `still` - contrib/guix/guix-build:18: `invokable` ==> `invocable` - contrib/guix/libexec/prelude.bash:12: `invokable` ==> `invocable` - src/test/fuzz/tx_pool.cpp:37: `acess` ==> `access` - src/txorphanage.h:29: `orginating` ==> `originating` ACKs for top commit: practicalswift: cr ACK 94c7dd9ac810a60f9c818c494273ad798cbac34b: thnaks fro fiixng tpyos! jarolrod: ACK 94c7dd9ac810a60f9c818c494273ad798cbac34b Tree-SHA512: e0fac462a2f9e68b6a161c9f5d95b4d0648ce5c618fd7cd243d57db8f0256138b8823b166ea406b21e95586eae43047df1ef0df04616858082a39c1d1eb13a86
2021-04-07doc: Fix typos from codespell lintYerzhan Mazhkenov
2021-04-07Drop JSONRPCRequest constructors after #21366Russell Yanofsky
This just makes an additional simplification after #21366 replaced util::Ref with std::any. It was originally suggested https://github.com/bitcoin/bitcoin/pull/21366#issuecomment-792044351 but delayed for a followup. It would have prevented usage bug https://github.com/bitcoin/bitcoin/pull/21572.
2021-04-07Merge #21617: fuzz: Fix uninitialized read in i2p testMarcoFalke
33333755f2edcbe88fcd136f6fef81f94819002e fuzz: Fix uninitialized read in test (MarcoFalke) Pull request description: Can be tested with: ``` ./test/fuzz/test_runner.py -l DEBUG --valgrind ../btc_qa_assets/fuzz_seed_corpus/ i2p ``` ``` ==22582== Conditional jump or move depends on uninitialised value(s) ==22582== at 0x6BB2D8: __sanitizer_cov_trace_const_cmp1 (in /tmp/bitcoin-core/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/test/fuzz/fuzz) ==22582== by 0xB305DB: ConnectSocketDirectly(CService const&, Sock const&, int, bool) (netbase.cpp:570) ==22582== by 0x8AAA5D: i2p::sam::Session::Hello() const (i2p.cpp:284) ==22582== by 0x8A6FA0: i2p::sam::Session::CreateIfNotCreatedAlready() (i2p.cpp:352) ==22582== by 0x8A6742: i2p::sam::Session::Listen(i2p::Connection&) (i2p.cpp:134) ==22582== by 0x7A6C42: i2p_fuzz_target(Span<unsigned char const>) (i2p.cpp:37) ACKs for top commit: sipa: utACK 33333755f2edcbe88fcd136f6fef81f94819002e vasild: ACK 33333755f2edcbe88fcd136f6fef81f94819002e Tree-SHA512: 36073582b26b541324b3e55f3fd4a44abf89cb3081f36d361525daf8c27602fbc25f736510ec30df7cb4ca0c4e395e8d8a60f531bf6af358b5a3e65dbabf72c0
2021-04-07test: increase listbanned unit test coverageJon Atack
Add test coverage for the new ban_duration and time_remaining fields. While here, some code improvements.
2021-04-07Merge #21582: Fix assumeutxo crash due to missing base_blockhashMarcoFalke
fa9b74f5ea89624e052934c48391b5076a87ffef Fix assumeutxo crash due to missing base_blockhash (MarcoFalke) fa8fffebe8ac126f31143619843dd6578a2f4e3c refactor: Prefer clean assert over UB in coinstats (MarcoFalke) Pull request description: This fixes an UB (which results in a crash with sanitizers enabled). Can be reproduced by cherry-picking the test without the other code changes. The fix: * Adds an `Assert` to transform the UB into a clean crash, even when sanitizers are disabled * Adds an early-fail condition to avoid the crash ACKs for top commit: jamesob: ACK fa9b74f5ea89624e052934c48391b5076a87ffef ([`jamesob/ackr/21582.1.MarcoFalke.fix_assumeutxo_crash_due`](https://github.com/jamesob/bitcoin/tree/ackr/21582.1.MarcoFalke.fix_assumeutxo_crash_due)) ryanofsky: Code review ACK fa9b74f5ea89624e052934c48391b5076a87ffef with no code changes since last review, just splitting up combocommit a little. Tree-SHA512: dd36808a09f49c647543a9eaa6fdb785b3f1109af48ba4cc983153b22a144da9ca61af22034dcfaa0e192a65b1ee7de744f187555079aff55bec0efa0ce87cd4
2021-04-06fuzz: Fix uninitialized read in testMarcoFalke
2021-04-06Merge #21571: test: make sure non-IP peers get discouraged and disconnected ↵MarcoFalke
(vasild) 81747b21719b3fa6b0fdfc3b084c0104d64903f9 test: make sure non-IP peers get discouraged and disconnected (Vasil Dimov) 637bb6da368b87711005b909f451f94909400092 test: also check disconnect in denialofservice_tests/peer_discouragement (Vasil Dimov) 4d6e246fa46f2309e2998b542e4c104d73d29071 test: use pointers in denialofservice_tests/peer_discouragement (Vasil Dimov) Pull request description: Split up from #20966, so that it can be backported easier. Merging this ahead of #20966 will also reduce the number of conflicts for that pull. ACKs for top commit: jonatack: ACK 81747b21719b3fa6b0fdfc3b084c0104d64903f9 Tree-SHA512: 8f0e30b95baba7f056920d7fc3b37bd49ee13e69392fe80e2d333c6bb09fd25f4603249301b8795cca26a2f2d15b9f8904798a55cd9c04fd28afb316e95c551c
2021-04-05move-only: Move *Disk functions to blockstorageMarcoFalke
Can be reviewed with the git options --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space
2021-04-05miner: Pass in previous CBlockIndex to RegenerateCommitmentsCarl Dong
2021-04-05Merge #21302: wallet: createwallet examples for descriptor walletsW. J. van der Laan
5039e0e55a7c9dd63a38c12fa60d244592de69e0 test: HelpExampleCliNamed and HelpExampleRpcNamed (Ivan Metlushko) 591735ef0bf13b94643b794518406f981fa5dcb7 rpc: Add HelpExampleCliNamed and use it for `createwallet` doc (Wladimir J. van der Laan) 5d5a90e819d23a302f9bec6b995a3116ead6ae94 rpc: Add HelpExampleRpcNamed (Ivan Metlushko) Pull request description: Rationale: make descriptor wallets more visible and just a bit easier to setup `bitcoin-cli help createwallet` **Before**: ``` Examples: > bitcoin-cli createwallet "testwallet" > curl --user myusername --data-binary '{"jsonrpc": "1.0", "id": "curltest", "method": "createwallet", "params": ["testwallet"]}' -H 'content-type: text/plain;' http://127.0.0.1:8332/ ``` **After** ``` Examples: > bitcoin-cli createwallet "testwallet" > curl --user myusername --data-binary '{"jsonrpc": "1.0", "id": "curltest", "method": "createwallet", "params": ["testwallet"]}' -H 'content-type: text/plain;' http://127.0.0.1:8332/ > bitcoin-cli createwallet "descriptors" false false "" true true true > curl --user myusername --data-binary '{"jsonrpc": "1.0", "id": "curltest", "method": "createwallet", "params": ["descriptors", false, false, "", true, true, true]}' -H 'content-type: text/plain;' http://127.0.0.1:8332/ ``` ACKs for top commit: laanwj: Tested ACK 5039e0e55a7c9dd63a38c12fa60d244592de69e0 Tree-SHA512: d37210e6ce639addee881377092d8f6fb2a537a60a259c561899e24cf68a0254d7ff45a213573c938f626677e46770cd21113aae5974f26c66b9a2e137699c14
2021-04-04Fix assumeutxo crash due to missing base_blockhashMarcoFalke
2021-04-02test: make sure non-IP peers get discouraged and disconnectedVasil Dimov
2021-04-02test: also check disconnect in denialofservice_tests/peer_discouragementVasil Dimov
Use `CConnmanTest` instead of `CConnman` and add the nodes to it so that their `fDisconnect` flag is set during disconnection.
2021-04-02test: use pointers in denialofservice_tests/peer_discouragementVasil Dimov
This is a non-functional change that replaces the `CNode` on-stack variables with `CNode` pointers. The reason for this is that it would allow us to add those `CNode`s to `CConnman::vNodes[]` which in turn would allow us to check that they are disconnected properly - a `CNode` object must be in `CConnman::vNodes[]` in order for its `fDisconnect` flag to be set. If we store pointers to the on-stack variables in `CConnman` then it would crash at the end, trying to `delete` them.
2021-04-01Merge #21525: [Bundle 4.5/n] Followup fixups to bundle 4MarcoFalke
693414d27181cf967f787a2ca72344e52c58c7f0 node/ifaces: ChainImpl: Use an accessor for ChainMan (Carl Dong) 98c4e252f0d09bebb2e4ad3289407459c2cda5d5 node/ifaces: NodeImpl: Use an accessor for ChainMan (Carl Dong) 7e8b5ee814b0b8c34acb20637ed4fc988ccba555 validation: Make BlockManager::LookupBlockIndex const (Carl Dong) 88aead263c61d86e5f836028f517cfbf2a575498 node: Avoid potential UB by asserting assumptions (Carl Dong) 1dd8ed7a8491e51b76eeb236b15b794d9254f674 net_processing: Move comments to declarations (Carl Dong) 07156eb387ea580be5e2ce4a1744992ce7575903 node/coinstats: Replace #include with fwd-declaration (Carl Dong) 7b8e976cd5ac78a22f1be2b2fed8562c693af5d9 miner: Add chainstate member to BlockAssembler (Carl Dong) e62067e7bcad5a559899afff2e4a8e8b7e9f4301 Revert "miner: Pass in chainstate to BlockAssembler::CreateNewBlock" (Carl Dong) eede0647b06b6009080c4e536a2705e911d6ee19 Revert "scripted-diff: Invoke CreateNewBlock with chainstate" (Carl Dong) 0c1b2bc549aec77b247f0103652d883227841ac5 Revert "miner: Remove old CreateNewBlock w/o chainstate param" (Carl Dong) Pull request description: Chronological history of this changeset: 1. Bundle 4 (#21270) got merged 2. Posthumous reviews were posted 3. These changes were prepended in bundle 5 4. More reviews were added in bundle 5 5. Someone suggested that we split the prepended changes up to another PR 6. This is that PR In the future, I will just do posthumous review changes in another PR instead. I apologize for the confusion. Addresses posthumous reviews on bundle 4: - From jnewbery: - https://github.com/bitcoin/bitcoin/pull/21270#issuecomment-796738048 - I didn't fix this one, but I added a `TODO` comment so that we don't lost track of it - https://github.com/bitcoin/bitcoin/pull/21270#discussion_r592291225 - https://github.com/bitcoin/bitcoin/pull/21270#discussion_r592296942 - https://github.com/bitcoin/bitcoin/pull/21270#discussion_r592299738 - https://github.com/bitcoin/bitcoin/pull/21270#discussion_r592301704 - From MarcoFalke: - https://github.com/bitcoin/bitcoin/pull/21270#discussion_r593096212 - https://github.com/bitcoin/bitcoin/pull/21270#discussion_r593097032 - https://github.com/bitcoin/bitcoin/pull/21270#discussion_r593097867 - https://github.com/bitcoin/bitcoin/pull/21270#discussion_r593100570 Addresses reviews on bundle 5: - Checking chainman existence before locking cs_main - MarcoFalke - https://github.com/bitcoin/bitcoin/pull/21391#discussion_r596601776 - https://github.com/bitcoin/bitcoin/pull/21391#discussion_r596601876 - Appropriate locking, usage of chainman, and control flow in `src/node/interfaces.cpp` - MarcoFalke - https://github.com/bitcoin/bitcoin/pull/21391#discussion_r596601383 - jnewbery - https://github.com/bitcoin/bitcoin/pull/21391#discussion_r597029360 - https://github.com/bitcoin/bitcoin/pull/21391#discussion_r597029921 - ryanofsky - https://github.com/bitcoin/bitcoin/pull/21391#discussion_r597163828 - Style/comment formatting changes - jnewbery - https://github.com/bitcoin/bitcoin/pull/21391#discussion_r597026552 - https://github.com/bitcoin/bitcoin/pull/21391#discussion_r597027186 - Making LookupBlockIndex const - jnewbery - https://github.com/bitcoin/bitcoin/pull/21391#discussion_r597035062 ACKs for top commit: MarcoFalke: review ACK 693414d27181cf967f787a2ca72344e52c58c7f0 🛐 ryanofsky: Code review ACK 693414d27181cf967f787a2ca72344e52c58c7f0. I reviewed this previously as part of #21391. I am a fan of the increasingly complicated bundle numbering, and kind of hope there in the next round there is some way we can get bundles 5.333333 and 5.666667! jamesob: ACK 693414d27181cf967f787a2ca72344e52c58c7f0 ([`jamesob/ackr/21525.1.dongcarl.bundle_4_5_n_followup_f`](https://github.com/jamesob/bitcoin/tree/ackr/21525.1.dongcarl.bundle_4_5_n_followup_f)) Tree-SHA512: 9bdc199f70400d01764e1bd03c25bdb6cff26dcef60e4ca3b649baf8d017a2dfc1f058099067962b4b6ccd32d078002b1389d733039f4c337558cb70324c0ee3
2021-03-31Merge #21366: refactor: replace util::Ref with std::any (C++17)W. J. van der Laan
916ab0195d567fd0a9097045e73a6654c453adea remove unused class util::Ref and its unit test (Sebastian Falbesoner) 8dbb87a3932f81e23ba7afd865b9aeeb535f0c20 refactor: replace util::Ref by std::any (C++17) (Sebastian Falbesoner) 95cccf8a4b392959c1fd7ec0647e04eb13880865 util: introduce helper AnyPtr to access std::any instances (Sebastian Falbesoner) Pull request description: As described in `util/ref.h`: "_This implements a small subset of the functionality in C++17's std::any class, and **can be dropped when the project updates to C++17**_". For accessing the contained object of a `std::any` instance, a helper template function `AnyPtr` is introduced (thanks to ryanofsky). ACKs for top commit: hebasto: re-ACK 916ab0195d567fd0a9097045e73a6654c453adea, with command ryanofsky: Code review ACK 916ab0195d567fd0a9097045e73a6654c453adea. Changes since last review: rebase and replacing types with `auto`. I might have used `const auto*` and `auto*` instead of plain `auto` because I think the qualifiers are useful, but this is all good. Tree-SHA512: fe2c3e4f5726f8ad40c61128339bb24ad11d2c261f71f7b934b1efe3e3279df14046452b0d9b566917ef61d5c7e0fd96ccbf35ff810357e305710f5002c27d47
2021-03-31[test] remove invalid test from tx_valid.jsonglozow
This exact test is also already in tx_invalid.json#L29 It will also test with no-P2SH flags, so duplicating with different flags is not necessary.
2021-03-30Merge #21553: fuzz: Misc refactorMarcoFalke
fa4926cca68db2433e579d9b4a642248a9314673 fuzz: [refactor] Use IsValidFlagCombination in signature_checker fuzz target (MarcoFalke) eeee8f5be1d4ccfb7e237248be5c6bef45b0fbb8 fuzz: Removed unused try-catch in coins_view (MarcoFalke) fa98f3f66ecb0a0c6805dcdcf828f12a1a11c3e6 fuzz: [refactor] Use ConsumeScript in signature_checker fuzz target (MarcoFalke) Pull request description: Some small refactors to remove unused and redundant fuzz code ACKs for top commit: practicalswift: cr re-ACK fa4926cca68db2433e579d9b4a642248a9314673 Tree-SHA512: eb07a2140caad7b31495b76385fc7634cf5b6daa4947f430ebb127eb1375583dc11e541a0a42d0e5d93d430480b8a815b93974450fd5ed897528a2d47c752f86
2021-03-30Merge #21387: p2p: Refactor sock to add I2P fuzz and unit testsWladimir J. van der Laan
40316a37cb02cf8a9a8b2cbd4d7153ffa57e7ec5 test: add I2P test for a runaway SAM proxy (Vasil Dimov) 2d8ac779708322e1235e823edfc9c8f6e2dd65e4 fuzz: add tests for the I2P Session public interface (Vasil Dimov) 9947e44de0cbd79e99d883443a9ac441d8c69713 i2p: use pointers to Sock to accommodate mocking (Vasil Dimov) 82d360b5a88d9057b6c09b61cd69e426c7a2412d net: change ConnectSocketDirectly() to take a Sock argument (Vasil Dimov) b5861100f85fef77b00f55dcdf01ffb4a2a112d8 net: add connect() and getsockopt() wrappers to Sock (Vasil Dimov) 5a887d49b2b39e59d7cce8e9d5b89c21ad694f8b fuzz: avoid FuzzedSock::Recv() repeated errors with EAGAIN (Vasil Dimov) 3088f83d016e7ebb6e6aa559e6326fa0ef0d6282 fuzz: extend FuzzedSock::Recv() to support MSG_PEEK (Vasil Dimov) 9b05c49ade729311a0f4388a109530ff8d0ed1f9 fuzz: implement unimplemented FuzzedSock methods (Vasil Dimov) Pull request description: Change the networking code and the I2P code to be fully mockable and use `FuzzedSocket` to fuzz the I2P methods `Listen()`, `Accept()` and `Connect()`. Add a mocked `Sock` implementation that returns a predefined data on reads and use it for a regression unit test for the bug fixed in https://github.com/bitcoin/bitcoin/pull/21407. ACKs for top commit: practicalswift: Tested ACK 40316a37cb02cf8a9a8b2cbd4d7153ffa57e7ec5 MarcoFalke: Concept ACK 40316a37cb jonatack: re-ACK 40316a37cb02cf8a9a8b2cbd4d7153ffa57e7ec5 reviewed `git range-diff 01bb3afb 23c861d 40316a3` and the new unit test commit, debug built, ran unit tests, ran bitcoind with an I2P service and network operation with seven I2P peers (2 in, 5 out) is looking nominal laanwj: Code review ACK 40316a37cb02cf8a9a8b2cbd4d7153ffa57e7ec5 Tree-SHA512: 7fc4f129849e16e0c7e16662d9f4d35dfcc369bb31450ee369a2b97bdca95285533bee7787983e881e5a3d248f912afb42b4a2299d5860ace7129b0b19623cc8
2021-03-30Merge #20197: p2p: protect onions in AttemptToEvictConnection(), add ↵Wladimir J. van der Laan
eviction protection test coverage 0cca08a8ee33b4e05ff586ae4fd914f5ea860cea Add unit test coverage for our onion peer eviction protection (Jon Atack) caa21f586f951d626a67f391050c3644f1057f57 Protect onion+localhost peers in ProtectEvictionCandidatesByRatio() (Jon Atack) 8f1a53eb027727a4c0eaac6d82f0a8279549f638 Use EraseLastKElements() throughout SelectNodeToEvict() (Jon Atack) 8b1e156143740a5548dc7b601d40fb141e6aae1c Add m_inbound_onion to AttemptToEvictConnection() (Jon Atack) 72e30e8e03f880eba4bd1c3fc18b5558d8cef680 Add unit tests for ProtectEvictionCandidatesByRatio() (Jon Atack) ca63b53ecdf377ce777fd959d400748912266748 Use std::unordered_set instead of std::vector in IsEvicted() (Jon Atack) 41f84d5eccd4c2620bf6fee616f2f8f717dbd6f6 Move peer eviction tests to a separate test file (Jon Atack) f126cbd6de6e1a8fee0e900ecfbc14a88e362541 Extract ProtectEvictionCandidatesByRatio from SelectNodeToEvict (Jon Atack) Pull request description: Now that #19991 and #20210 have been merged, we can determine inbound onion peers using `CNode::m_inbound_onion` and add it to the localhost peers protection in `AttemptToEvictConnection`, which was added in #19670 to address issue #19500. Update 28 February 2021: I've updated this to follow gmaxwell's suggestion in https://github.com/bitcoin/bitcoin/pull/20197#issuecomment-713865992. This branch now protects up to 1/4 onion peers (connected via our tor control service), if any, sorted by longest uptime. If any (or all) onion slots remain after that operation, they are then allocated to protect localhost peers, or a minimum of 2 localhost peers in the case that no onion slots remain and 2 or more onion peers were protected, sorted as before by longest uptime. This patch also adds test coverage for the longest uptime, localhost, and onion peer eviction protection logic to build on the welcome initial unit testing of #20477. Suggest reviewing the commits that move code with `colorMoved = dimmed-zebra` and `colorMovedWs = allow-indentation-change`. Closes #11537. ACKs for top commit: laanwj: Code review ACK 0cca08a8ee33b4e05ff586ae4fd914f5ea860cea vasild: ACK 0cca08a8ee33b4e05ff586ae4fd914f5ea860cea Tree-SHA512: 2f5a63f942acaae7882920fc61f0185dcd51da85e5b736df9d1fc72343726dd17da740e02f30fa5dc5eb3b2d8345707aed96031bec143d48a2497a610aa19abd
2021-03-30Merge #20228: addrman: Make addrman a top-level componentMarcoFalke
3fc06d3d7b43dc1143fe0850db23c4e7ffbfe682 [net] remove fUpdateConnectionTime from FinalizeNode (John Newbery) 7c4cc67c0c3c50df004ee53cac5b2884b7fbab29 [net] remove CConnman::AddNewAddresses (John Newbery) bcd7f30b7944892db7ae37069175804567bb0cdf [net] remove CConnman::MarkAddressGood (John Newbery) 8073673dbcb2744fcc9c011edf2d61388ca929cd [net] remove CConnman::SetServices (John Newbery) 392a95d393a9af01b53e5e68197e81968efb84fc [net_processing] Keep addrman reference in PeerManager (John Newbery) 1c25adf6d278eb1a1f018986a126d0eb8137e0ee [net] Construct addrman outside connman (John Newbery) Pull request description: Addrman is currently a member variable of connman. Make it a top-level component with lifetime owned by node.context, and add a reference to addrman in peerman. This allows us to eliminate some functions in connman that are simply forwarding requests to addrman, and simplifies the connman-peerman interface. By constructing the addrman in init, we can also add parameters to the ctor, which allows us to test it better. See #20233, where we enable consistency checking for addrman in our functional tests. ACKs for top commit: MarcoFalke: re-ACK 3fc06d3d7b43dc1143fe0850db23c4e7ffbfe682 only change is squash 🏀 vasild: ACK 3fc06d3d7b43dc1143fe0850db23c4e7ffbfe682 Tree-SHA512: 17662c65cbedcd9bd1c194914bc4bb4216f4e3581a06222de78f026d6796f1da6fe3e0bf28c2d26a102a12ad4fbf13f815944a297f000e3acf46faea42855e07
2021-03-30fuzz: [refactor] Use IsValidFlagCombination in signature_checker fuzz targetMarcoFalke
Can be reviewed with --color-moved=dimmed-zebra
2021-03-30fuzz: Removed unused try-catch in coins_viewMarcoFalke
2021-03-30fuzz: [refactor] Use ConsumeScript in signature_checker fuzz targetMarcoFalke
generated by cgit v1.2.3 (git 2.26.2) at 2025-03-16 17:44:30 +0000