aboutsummaryrefslogtreecommitdiff
path: root/src
AgeCommit message (Collapse)Author
2021-02-11fuzz: version handshakeMarcoFalke
2021-02-11Merge #21043: net: Avoid UBSan warning in ProcessMessage(...)MarcoFalke
3ddbf22ed179a2db733af4b521bec5d2b13ebf4b util: Disallow negative mocktime (MarcoFalke) f5f2f9716885e7548809e77f46b493c896a019bf net: Avoid UBSan warning in ProcessMessage(...) (practicalswift) Pull request description: Avoid UBSan warning in `ProcessMessage(...)`. Context: https://github.com/bitcoin/bitcoin/pull/20380#issuecomment-770427182 (thanks Crypt-iQ!) ACKs for top commit: MarcoFalke: re-ACK 3ddbf22ed179a2db733af4b521bec5d2b13ebf4b only change is adding patch written by me ajtowns: ACK 3ddbf22ed179a2db733af4b521bec5d2b13ebf4b -- code review only Tree-SHA512: e8d7af0457ca86872b75a4e406c0a93aafd841c2962e244e147e748cc7ca118c56be0fdafe53765f4b291410030b2c3cc8f76f733b37a955d34fc885ab6037b9
2021-02-11Merge #20211: Use -Wswitch for TxoutType where possibleMarcoFalke
fa650ca7f19307a9237e64ac311488c8947fc12a Use -Wswitch for TxoutType where possible (MarcoFalke) fa59e0b5bd2aed8380cc9b9e52791f662aecd6a6 test: Add missing script_standard_Solver_success cases (MarcoFalke) Pull request description: This removes unused `default:` cases for all `switch` statements on `TxoutType` and adds the cases (`MULTISIG`, `NULL_DATA`, `NONSTANDARD`) to `ExtractDestination` for clarity. Also, the compiler is now able to use `-Wswitch`. ACKs for top commit: practicalswift: cr ACK fa650ca7f19307a9237e64ac311488c8947fc12a: patch looks correct and `assert(false);` is better than UB :) hebasto: ACK fa650ca7f19307a9237e64ac311488c8947fc12a, I have reviewed the code and it looks OK, I agree it can be merged. Tree-SHA512: 282458b6523bd8923a0c0f5c423d1db2dce2a2d1b1d1dae455415c6fc995bb41ce82c1f9b0a1c0dcc6d874d171e04c30eca585f147582f52c7048c140358630a
2021-02-11fuzz: Fail if message type is not fuzzedMarcoFalke
2021-02-11fuzz: Count message type fuzzers before main()MarcoFalke
2021-02-11Merge #20663: fuzz: Hide script_assets_test_minimizerMarcoFalke
fac726b1b8331b267973138bbd2bff5304774315 doc: Fixup docs in fuzz/script_assets_test_minimizer.cpp (MarcoFalke) fafca47adc2476f19f7926de4d55b64b0286e41c fuzz: Hide script_assets_test_minimizer (MarcoFalke) Pull request description: This is not an actual fuzz target. It is a hack to exploit the built-in capability of fuzz engines to measure coverage. ACKs for top commit: practicalswift: cr ACK fac726b1b8331b267973138bbd2bff5304774315: patch looks correct and touches only `src/test/fuzz/` Tree-SHA512: 0652dd8d9e95746b0906be4044467435d8204a34a30366ae9bdb75b9cb0788d429db7cedf2760fd543565d9d4f7ee206873ed10a29dd715a792a26337f65b53c
2021-02-10Merge #21114: Deduplicate some block-to-JSON codeWladimir J. van der Laan
fa2c52111544b0de93ac1002f5395bceeb8fea0e Deduplicate some block-to-JSON code. (Daniel Kraft) Pull request description: Some of the logic converting blocks and block headers to JSON for the blockchain RPC methods (`getblock`, `getlockheader`) was duplicated. Instead of that, the `blockToJSON` RPC method now calls `blockheaderToJSON` first, and then fills in the missing, block-specific bits explicitly. ACKs for top commit: laanwj: Code review ACK fa2c52111544b0de93ac1002f5395bceeb8fea0e Tree-SHA512: 1b9b269e251d9c8c1056f253cfc2a795170d609f4c26ecc85b1ff9cdd567095a673dd89564e0d587b32dfc152ea01b2682169b018f2c9c3004c511a9998d772e
2021-02-10Merge #21125: test: Change BOOST_CHECK to BOOST_CHECK_EQUAL for pathsMarcoFalke
059e8ccc1eba6cd92f4c434325cb56b0533eb744 Change BOOST_CHECK to BOOST_CHECK_EQUAL to see mismatched values when a check fails. (Kiminuo) Pull request description: This is useful to see mismatched values when a check fails as specified in the [Boost documentation](https://www.boost.org/doc/libs/1_71_0/libs/test/doc/html/boost_test/utf_reference/testing_tool_ref/assertion_boost_level.html). This PR would make #20744 PR's diff smaller by a bit. ACKs for top commit: MarcoFalke: review ACK 059e8ccc1eba6cd92f4c434325cb56b0533eb744 theStack: Code Review ACK 059e8ccc1eba6cd92f4c434325cb56b0533eb744 Tree-SHA512: 82359ef38e0d1926f12a34aeff6fde6d1d307c703a080547749b908f873c2a2f894f6f094c33470b32987c229e3a1f17f7d1e877663c53293c023bde0e7272c1
2021-02-10Deduplicate some block-to-JSON code.Daniel Kraft
Some of the logic converting blocks and block headers to JSON for the blockchain RPC methods (getblock, getlockheader) was duplicated. Instead of that, the blockToJSON RPC method now calls blockheaderToJSON first, and then fills in the missing, block-specific bits explicitly.
2021-02-09Merge #21052: refactor: Replace fs::unique_path with GetUniquePath(path) callsWladimir J. van der Laan
1bca2aa694cd85984c09699ae28daec313077462 Introduce GetUniquePath(base) helper method to replace boost::filesystem::unique_path() which is not available in std::filesystem. (Kiminuo) Pull request description: This PR makes it easier in #20744 to remove our dependency on the `boost::filesystem::unique_path()` function which does not have a direct equivalent in C++17. This PR attempts to re-implement `boost::filesystem::unique_path()` as `GetUniquePath(path)` but the implementations are not meant to be the same. Note: * Boost 1.75.0 implementation of `unique_path`: https://github.com/boostorg/filesystem/blob/9cab675b71e98706886a87afe7c19eb9da568961/src/unique_path.cpp#L235 * In the previous implementation, I attempted to add: ```cpp fs::path GetUniquePath(const fs::path& base) { FastRandomContext rnd; fs::path tmpFile = base / HexStr(rnd.randbytes(8)); return tmpFile; } ``` to `fs.cpp` but this leads to a circular dependency: "fs -> random -> logging -> fs". That is why the modified implementation adds a new file. ACKs for top commit: laanwj: Code review ACK 1bca2aa694cd85984c09699ae28daec313077462 ryanofsky: Code review ACK 1bca2aa694cd85984c09699ae28daec313077462. It's a simple change and extra test coverage is nice Tree-SHA512: f324bdf0e254160c616b5033c3ece33d87db23eb0135acee99346ade7b5cf0d30f3ceefe359a25a8e9b53ba8e4419f459c2bdd369e10fc0152ce95031d1f221c
2021-02-09Change BOOST_CHECK to BOOST_CHECK_EQUAL to see mismatched values when a ↵Kiminuo
check fails. See https://www.boost.org/doc/libs/1_71_0/libs/test/doc/html/boost_test/utf_reference/testing_tool_ref/assertion_boost_level_eq.html
2021-02-09Merge #20557: addrman: Fix new table bucketing during unserializationWladimir J. van der Laan
4676a4fb5be0f6ef0b3f71c1f4361c20f7cb0e0b [addrman] Don't repeat "Bucketing method was updated" log multiple times (John Newbery) 436292367c1d737cf73bd985293539500d1206f5 [addrman] Improve serialization comments (John Newbery) ac3547eddd8a7d67b4103508f30d5d02a9c1f148 [addrman] Improve variable naming/code style of touched code. (John Newbery) a5c9b04959f443372400f9a736c6eaf5502284a1 [addrman] Don't rebucket new table entries unnecessarily (John Newbery) 8062d928ce5c495c1b6ecd18e4b30c12da822d90 [addrman] Rename asmap version to asmap checksum (John Newbery) 009b8e0fdf3bfb11668edacced5d8b70726d5d0e [addrman] Improve variable naming/code style of touched code. (John Newbery) b4c5fda417dd9ff8bf9fe24a87d384a649e3730d [addrman] Fix new table bucketing during unserialization (John Newbery) Pull request description: This fixes three issues in addrman unserialization. 1. An addrman entry can appear in up to 8 new table buckets. We store this entry->bucket indexing during shutdown so that on restart we can restore the entries to their correct buckets. Commit ec45646de9e62b3d42c85716bfeb06d8f2b507dc broke the deserialization code so that each entry could only be put in up to one new bucket. 2. Unserialization may result in an entry appearing in a 9th bucket. If the entry already appears in 8 buckets don't try to place it in another bucket. 3. We unnecessarily rebucket when reading a peers.dat with file version 1. Don't do that. ACKs for top commit: vasild: ACK 4676a4fb5be0f6ef0b3f71c1f4361c20f7cb0e0b glozow: re-ACK https://github.com/bitcoin/bitcoin/commit/4676a4fb5be0f6ef0b3f71c1f4361c20f7cb0e0b, changes were a rename, comments, and removing repeat-logging. naumenkogs: ACK 4676a4f laanwj: Code review ACK 4676a4fb5be0f6ef0b3f71c1f4361c20f7cb0e0b dhruv: ACK 4676a4fb5be0f6ef0b3f71c1f4361c20f7cb0e0b ryanofsky: Code review ACK 4676a4fb5be0f6ef0b3f71c1f4361c20f7cb0e0b. I'm not previously familiar with this code but all the changes here do make sense and seem like improvements. Left some notes and comments, but they aren't important so feel to ignore. Tree-SHA512: b228984f6dec5910be23c3740ae20258da33bcf66ceb7edb10e5a53163450f743bab349e47f09808b7e8d40f27143119ec3e0981d7e678aa494d8559a1c99c23
2021-02-09Merge #21115: test: Fix Windows cross buildfanquake
723eb4326bac4a3906cbfe278bb6b8bee17e7790 test: Fix Windows cross build (Hennadii Stepanov) Pull request description: On master (e51f6c4dee3d42b2707c31fa1c93a337b6bf5ba7, after #20936 merge), Windows cross compiling fails: ``` $ make > /dev/null In file included from ./policy/fees.h:12, from policy/fees.cpp:6: policy/fees.cpp: In member function ‘unsigned int CBlockPolicyEstimator::HighestTargetTracked(FeeEstimateHorizon) const’: ./sync.h:232:104: warning: control reaches end of non-void function [-Wreturn-type] 232 | #define LOCK(cs) DebugLock<decltype(cs)> PASTE2(criticalblock, __COUNTER__)(cs, #cs, __FILE__, __LINE__) | ^ policy/fees.cpp:680:5: note: in expansion of macro ‘LOCK’ 680 | LOCK(m_cs_fee_estimator); | ^~~~ test/fuzz/netaddress.cpp:12:10: fatal error: netinet/in.h: No such file or directory 12 | #include <netinet/in.h> | ^~~~~~~~~~~~~~ compilation terminated. make[2]: *** [Makefile:13039: test/fuzz/fuzz-netaddress.o] Error 1 make[2]: *** Waiting for unfinished jobs.... libtool: warning: undefined symbols not allowed in x86_64-w64-mingw32 shared libraries; building static only test/fuzz/string.cpp: In function ‘void string_fuzz_target(FuzzBufferType)’: test/fuzz/string.cpp:81:11: error: ‘ShellEscape’ was not declared in this scope 81 | (void)ShellEscape(random_string_1); | ^~~~~~~~~~~ make[2]: *** [Makefile:13543: test/fuzz/fuzz-string.o] Error 1 make[1]: *** [Makefile:15078: all-recursive] Error 1 make: *** [Makefile:812: all-recursive] Error 1 ``` This PR fixes both of errors. ACKs for top commit: MarcoFalke: cr ACK 723eb4326bac4a3906cbfe278bb6b8bee17e7790 Tree-SHA512: 5d2fba5ca806e64bf92011786d1f868c6624f786bfa753a10316feab7a802a28ec27a4bd25fc26dc289a399895a521c3878ffa1efeff0e540c7245cdb8e4942c
2021-02-08Merge #20944: rpc: Return total fee in getmempoolinfoMarcoFalke
fa362064e383163a2585ffbc71ac1ea3bcc92663 rpc: Return total fee in mempool (MarcoFalke) Pull request description: This avoids having to loop over the whole mempool to query each entry's fee ACKs for top commit: achow101: ACK fa362064e383163a2585ffbc71ac1ea3bcc92663 glozow: ACK https://github.com/bitcoin/bitcoin/pull/20944/commits/fa362064e383163a2585ffbc71ac1ea3bcc92663 🧸 jnewbery: ACK fa362064e383163a2585ffbc71ac1ea3bcc92663 Tree-SHA512: e2fa1664df39c9e187f9229fc35764ccf436f6f75889c5a206d34fff473fc21efbf2bb143f4ca7895c27659218c22884d0ec4195e7a536a5a96973fc9dd82d08
2021-02-08test: Fix Windows cross buildHennadii Stepanov
2021-02-08doc: Fixup docs in fuzz/script_assets_test_minimizer.cppMarcoFalke
2021-02-08fuzz: Hide script_assets_test_minimizerMarcoFalke
Can be reviewed with --ignore-all-space
2021-02-05build: build fuzz tests by default.Dan Benjamin
This fixes issue #19388. The changes are as follows: - Add a new flag to configure, --enable-fuzz-binary, which allows building test/fuzz/fuzz regardless of whether we are building to do actual fuzzing - Set -DPROVIDE_MAIN_FUNCTION whenever --enable-fuzz is no - Add the following libraries to FUZZ_SUITE_LD_COMMON: - LIBBITCOIN_WALLET - SQLLITE_LIBS - BDB_LIBS - if necessary, some or all of: - NATPMP_LIBS - MINIUPNPC_LIBS - LIBBITCOIN_ZMQ / ZMQ_LIBS
2021-02-05Merge bitcoin-core/gui#203: Display plain "Inbound" in peer detailsJonas Schnelli
506e6585a54818d0613a067f91c0bac2f308a48c gui: display plain "Inbound" in peer details (Jon Atack) Pull request description: Alternative version to #201. ACKs for top commit: MarcoFalke: ACK 506e6585a54818d0613a067f91c0bac2f308a48c jonasschnelli: utACK 506e6585a54818d0613a067f91c0bac2f308a48c Tree-SHA512: 88d141b14684c1dcdff47f7ba241e5a7c42c14da3d9aaa89f1649235a64fd26bc5a6055707dc07992cd9d8c05d143754f6dd51ccee69fd4309336dd07c52e61c
2021-02-05Merge #20839: fuzz: Avoid extraneous copy of input data, using Span<>Wladimir J. van der Laan
faf7d7418cf01cb04cd457bcc630654da958a777 fuzz: Avoid extraneous copy of input data, using Span<> (MarcoFalke) Pull request description: Seeing speedup here in the fuzz framework part (non-fuzz-target part). Speedup is only visible for input data larger than 100kB. ACKs for top commit: practicalswift: cr ACK faf7d7418cf01cb04cd457bcc630654da958a777: patch looks correct :) laanwj: Code review ACK faf7d7418cf01cb04cd457bcc630654da958a777 Tree-SHA512: 41af7118846e0dfee237a6d5269a6c7cfbc775d7bd1cc2a85814cb60f6c2b37fe7fd35f1a788d4f08e6e0202c48b71054b67d2931160c445c79fc59e5347dadf
2021-02-05Merge #20764: cli -netinfo peer connections dashboard updates 🎄 ✨Wladimir J. van der Laan
747cb5b9949f80b3b4516f382a0ce80e41f3f5a6 netinfo: display only outbound block relay counts (Jon Atack) 76d198a5c15a9376c7d3a91754320334337a9e50 netinfo: add i2p network (Jon Atack) 9d6aeca2c5ec1df579c27c39e82fa3ddf1d25986 netinfo: add bip152 high-bandwidth to/from fields (Jon Atack) 5de7a6cf63ef39b0474ea9c90a968f867635d98e netinfo: display manual peers count (Jon Atack) d3cca3be63afeb19a41e9892444fc6e02ea1c7c8 netinfo: update to use peer connection types (Jon Atack) 62bf5b785087981d9c0f8ddc8a3ceda911845a53 netinfo: add ConnectionTypeForNetinfo member helper function (Jon Atack) Pull request description: Merry Bitcoin Christmas! Ho ho ho 🎄 ✨ This PR updates `-netinfo` to: - use the getpeerinfo `connection_type` field (and no longer use getpeerinfo `relaytxes` for block-relay detection) - display manual peers count, if any, in the outbound row - display the block relay counts in the outbound row only - display high-bandwidth BIP152 compact block relay peers (`hb` column, to `.` and from `*`) - add support for displaying I2P network peers, if any are present Testing and review welcome! How to test: - to run the full live dashboard (on Linux): `$ watch --interval 1 --no-title ./src/bitcoin-cli -netinfo 4` - to run the full dashboard: ``$ ./src/bitcoin-cli -netinfo 4`` - to see the help: `$ ./src/bitcoin-cli -netinfo help` - to see the help summary: `$ ./src/bitcoin-cli -help | grep -A4 netinfo` ACKs for top commit: laanwj: re-ACK 747cb5b9949f80b3b4516f382a0ce80e41f3f5a6 michaelfolkson: ACK 747cb5b9949f80b3b4516f382a0ce80e41f3f5a6 jonasschnelli: Tested ACK 747cb5b9949f80b3b4516f382a0ce80e41f3f5a6 - works nicely. Great that this PR only changes bitcoin-cli. Tree-SHA512: 48fe23dddf3005a039190fcbc84167cd25b0a63489617fe14ea5db9a641a829b46b6e8dc7924aab6577d82a13909d157e82f715bd2ed3a8a15071957c35c19f3
2021-02-05Merge #20646: doc: refer to BIPs 339/155 in feature negotiationWladimir J. van der Laan
e1e67148321cff0de9eb5e63d2604f05c12e69d1 doc: refer to BIPs 339/155 in feature negotiation (Jon Atack) Pull request description: of `wtxidrelay` and `addrv2`/`sendaddrv2`, and add `fSuccessfullyConnected` doxygen documentation to clarify that it is set to true on VERACK. ACKs for top commit: laanwj: re-ACK e1e67148321cff0de9eb5e63d2604f05c12e69d1 Tree-SHA512: 3e6af5b246e4ee1ec68ee34db525746717871bc986ad4840f5a8edce55740768389f6fd0ec69046eda2fb4c69939440a96571f79d36e6cbff4fd3b7f2ebc74c0
2021-02-05Merge #21077: doc: clarify -timeout and -peertimeout config optionsMarcoFalke
eecb7ab105a4a59d09cd55b124c5ad563846fe11 [doc] clarify -peertimeout and -timeout descriptions (gzhao408) Pull request description: The debug-only option `-peertimeout` is used to delay `InactivityCheck()`, whereas the `-timeout` option specifies socket timeouts (`nConnectTimeout`). The current descriptions are a bit misleading and hard to tell apart. I think it would save dev/review time to update them 🤷 ACKs for top commit: MarcoFalke: ACK eecb7ab105a4a59d09cd55b124c5ad563846fe11 nice doc fixup jnewbery: ACK eecb7ab105a4a59d09cd55b124c5ad563846fe11 Tree-SHA512: 71d2e6c31664b9f7f0b053ecf3be21c6c55472553fa7478d8526ba3be8d54979bceafca63d87b8b2488c11f409c332ac795da613ff8101546b18d9cd8bcceb50
2021-02-04[doc] clarify -peertimeout and -timeout descriptionsgzhao408
2021-02-04refactor: Treat ArgsManager::Flags as uint32_t explicitlyMarcoFalke
2021-02-04Introduce GetUniquePath(base) helper method to replace ↵Kiminuo
boost::filesystem::unique_path() which is not available in std::filesystem.
2021-02-04Merge #21025: validation: Guard chainman chainstates with cs_mainMarcoFalke
20677ffa22e93e7408daadbd15d433f1e42faa86 validation: Guard all chainstates with cs_main (Carl Dong) Pull request description: ``` This avoids a potential race-condition where a thread is reading the ChainstateManager::m_active_chainstate pointer while another one is writing to it. There is no portable guarantee that reading/writing the pointer is thread-safe. This is also done in way that mimics ::ChainstateActive(), so the transition from that function to this method is easy. More discussion: 1. https://github.com/bitcoin/bitcoin/pull/20749#discussion_r559544027 2. https://github.com/bitcoin/bitcoin/pull/19806#discussion_r561023961 3. https://github.com/bitcoin/bitcoin/pull/19806#issuecomment-768946522 4. https://github.com/bitcoin/bitcoin/pull/19806#issuecomment-768955695 ``` Basically this PR removes the loaded-but-unfired footgun, which: - Is multiplied (but still unshot) in the chainman deglobalization PRs (#20158) - Is shot in the test framework in the au.activate PR (#19806) ACKs for top commit: jnewbery: code review ACK 20677ffa22e93e7408daadbd15d433f1e42faa86. I've verified by eye that neither of these members are accessed without cs_main. ryanofsky: Code review ACK 20677ffa22e93e7408daadbd15d433f1e42faa86. It is safer to have these new `GUARDED_BY` annotations and locks than not to have them, but in the longer run I think every `LOCK(cs_main)` added here and added earlier in f92dc6557a153b390a1ae1d0808ff7ed5d02c66e from #20749 should be removed and replaced with `EXCLUSIVE_LOCKS_REQUIRED(cs_main)` on the accessor methods instead. `cs_main` is a high level lock that should be explicitly acquired at a high level to prevent the chain state from changing. It shouldn't be acquired recursively in low-level methods just to read pointer values atomically. Tree-SHA512: 68a3a46d79a407b774eab77e1d682a97e95f1672db0a5fcb877572e188bec09f3a7b47c5d0cc1f2769ea276896dcbe97cb35c861acf7d8e3e513e955dc773f89
2021-02-04Merge #20715: util: Add ArgsManager::GetCommand() and use it in bitcoin-walletMarcoFalke
fa61b9d1a68820758f9540653920deaeae6abe79 util: Add ArgsManager::GetCommand() and use it in bitcoin-wallet (MarcoFalke) 7777105a24a36b62df35d12ecf6c6370671568c8 refactor: Move all command dependend checks to ExecuteWalletToolFunc (MarcoFalke) fa06bce4ac17f93decd4ee38c956e7aa55983f0d test: Add tests (MarcoFalke) fac05ccdade8b34c969b9cd9b37b355bc0aabf9c wallet: [refactor] Pass ArgsManager to WalletAppInit (MarcoFalke) Pull request description: This not only moves the parsing responsibility out from the wallet tool, but it also makes it easier to implement bitcoin-util #19937 Fixes: #20902 ACKs for top commit: ajtowns: ACK fa61b9d1a68820758f9540653920deaeae6abe79 fjahr: Code review ACK fa61b9d1a68820758f9540653920deaeae6abe79 Tree-SHA512: 79622b806e8bf9dcd0dc24a8a6687345710df57720992e83a41cd8d6762a6dc112044ebc58fcf6e8fbf45de29a79b04873c5b8c2494a1eaaf902a2884703e47b
2021-02-03fuzz: Avoid extraneous copy of input data, using Span<>MarcoFalke
2021-02-03netinfo: display only outbound block relay countsJon Atack
2021-02-03netinfo: add i2p networkJon Atack
the i2p peer counts column is displayed iff the node is connected to at least one i2p peer, so this doesn't add clutter for users who are not running an i2p service
2021-02-03netinfo: add bip152 high-bandwidth to/from fieldsJon Atack
2021-02-03netinfo: display manual peers countJon Atack
2021-02-03netinfo: update to use peer connection typesJon Atack
2021-02-03Merge #21060: doc: More precise -debug and -debugexclude docfanquake
572fd0f7382bd0e6c7acc27dc354fae8489ab0a0 doc: More precise -debug and -debugexclude doc (wodry) Pull request description: I wondered how one could enable debug logging with `-debug=<category>` for multiple categories. Found out solution is to specify that option multiple times for each wanted category. This PR documents this behavior and uses the same wording for the same behavior of `-debugexclude=<category>` to make that also clear and stringent. ACKs for top commit: laanwj: ACK 572fd0f7382bd0e6c7acc27dc354fae8489ab0a0 MarcoFalke: ACK 572fd0f7382bd0e6c7acc27dc354fae8489ab0a0 theStack: ACK 572fd0f7382bd0e6c7acc27dc354fae8489ab0a0 Tree-SHA512: 8d93db37602fd5ff4247e7c11478e55b99c0e3d47eaa2bb901937805d8f2a466b3a198b713b760981c5411576b74c52e2909c46c6d3f2e0e04215fd521b65cf7
2021-02-02netinfo: add ConnectionTypeForNetinfo member helper functionJon Atack
2021-02-02doc: refer to BIPs 339/155 in feature negotiationJon Atack
and add fSuccessfullyConnected doxygen documentation to clarify that it is set to true on VERACK
2021-02-02Merge #19509: Per-Peer Message CaptureMarcoFalke
bff7c66e67aa2f18ef70139338643656a54444fe Add documentation to contrib folder (Troy Giorshev) 381f77be858d7417209b6de0b7cd23cb7eb99261 Add Message Capture Test (Troy Giorshev) e4f378a505922c0f544b4cfbfdb169e884e02be9 Add capture parser (Troy Giorshev) 4d1a582549bc982d55e24585b0ba06f92f21e9da Call CaptureMessage at appropriate locations (Troy Giorshev) f2a77ff97bec09dd5fcc043d8659d8ec5dfb87c2 Add CaptureMessage (Troy Giorshev) dbf779d5deb04f55c6e8493ce4e12ed4628638f3 Clean PushMessage and ProcessMessages (Troy Giorshev) Pull request description: This PR introduces per-peer message capture into Bitcoin Core. 📓 ## Purpose The purpose and scope of this feature is intentionally limited. It answers a question anyone new to Bitcoin's P2P protocol has had: "Can I see what messages my node is sending and receiving?". ## Functionality When a new debug-only command line argument `capturemessages` is set, any message that the node receives or sends is captured. The capture occurs in the MessageHandler thread. When receiving a message, it is captured as soon as the MessageHandler thread takes the message off of the vProcessMsg queue. When sending, the message is captured just before the message is pushed onto the vSendMsg queue. The message capture is as minimal as possible to reduce the performance impact on the node. Messages are captured to a new `message_capture` folder in the datadir. Each node has their own subfolder named with their IP address and port. Inside, received and sent messages are captured into two binary files, msgs_recv.dat and msgs_sent.dat, like so: ``` message_capture/203.0.113.7:56072/msgs_recv.dat message_capture/203.0.113.7:56072/msgs_sent.dat ``` Because the messages are raw binary dumps, included in this PR is a Python parsing tool to convert the binary files into human-readable JSON. This script has been placed on its own and out of the way in the new `contrib/message-capture` folder. Its usage is simple and easily discovered by the autogenerated `-h` option. ## Future Maintenance I sympathize greatly with anyone who says "the best code is no code". The future maintenance of this feature will be minimal. The logic to deserialize the payload of the p2p messages exists in our testing framework. As long as our testing framework works, so will this tool. Additionally, I hope that the simplicity of this tool will mean that it gets used frequently, so that problems will be discovered and solved when they are small. ## FAQ "Why not just use Wireshark" Yes, Wireshark has the ability to filter and decode Bitcoin messages. However, the purpose of the message capture added in this PR is to assist with debugging, primarily for new developers looking to improve their knowledge of the Bitcoin Protocol. This drives the design in a different direction than Wireshark, in two different ways. First, this tool must be convenient and simple to use. Using an external tool, like Wireshark, requires setup and interpretation of the results. To a new user who doesn't necessarily know what to expect, this is unnecessary difficulty. This tool, on the other hand, "just works". Turn on the command line flag, run your node, run the script, read the JSON. Second, because this tool is being used for debugging, we want it to be as close to the true behavior of the node as possible. A lot can happen in the SocketHandler thread that would be missed by Wireshark. Additionally, if we are to use Wireshark, we are at the mercy of whoever it maintaining the protocol in Wireshark, both as to it being accurate and recent. As can be seen by the **many** previous attempts to include Bitcoin in Wireshark (google "bitcoin dissector") this is easier said than done. Lastly, I truly believe that this tool will be used significantly more by being included in the codebase. It's just that much more discoverable. ACKs for top commit: MarcoFalke: re-ACK bff7c66e67aa2f18ef70139338643656a54444fe only some minor changes: 👚 jnewbery: utACK bff7c66e67aa2f18ef70139338643656a54444fe theStack: re-ACK bff7c66e67aa2f18ef70139338643656a54444fe Tree-SHA512: e59e3160422269221f70f98720b47842775781c247c064071d546c24fa7a35a0e5534e8baa4b4591a750d7eb16de6b4ecf54cbee6d193b261f4f104e28c15f47
2021-02-02Merge #21059: Drop boost/preprocessor dependenciesWladimir J. van der Laan
e99db77a6e73996d33d7108f8336938dd57037a7 Drop boost/preprocessor dependencies (Hennadii Stepanov) 12f5028d4957bce3ba176e80527894b497c04a3b refactor: Move STRINGIZE macro to macros.h (Hennadii Stepanov) Pull request description: Use own macros instead of boost's ones. ACKs for top commit: laanwj: Code review ACK e99db77a6e73996d33d7108f8336938dd57037a7 practicalswift: cr ACK e99db77a6e73996d33d7108f8336938dd57037a7 Tree-SHA512: 7ec15c2780a661e293c990f64c41b5b451d894cc191aa7872fbcaf96da91915a351209b1f1003ab12a7a16cb464e50ac58a028db02beeedfa5f6931752c2d9e2
2021-02-02util: Disallow negative mocktimeMarcoFalke
Signed-off-by: practicalswift <practicalswift@users.noreply.github.com>
2021-02-02net: Avoid UBSan warning in ProcessMessage(...)practicalswift
2021-02-01validation: Guard all chainstates with cs_mainCarl Dong
Since these chainstates are: 1. Also vulnerable to the race condition described in the previous commit 2. Documented as having similar semantics as m_active_chainstate we should also protect them with ::cs_main.
2021-02-02Merge #21051: Fix -Wmismatched-tags warningsfanquake
b6aadcd5b4350a6ebcd57e88e7a0853cedf7c2fb build: Add -Werror=mismatched-tags (Hennadii Stepanov) 1485124291368c4a2ca8ea09c18e813f1dbabf5c Fix -Wmismatched-tags warnings (Hennadii Stepanov) Pull request description: Warnings were introduced in #20749: ``` ./validation.h:43:1: warning: class 'CCheckpointData' was previously declared as a struct; this is valid, but may result in linker errors under the Microsoft C++ ABI [-Wmismatched-tags] class CCheckpointData; ^ ./chainparams.h:24:8: note: previous use is here struct CCheckpointData { ^ ./validation.h:43:1: note: did you mean struct here? class CCheckpointData; ^~~~~ struct 1 warning generated. ``` This change fixes AppVeyor build: https://ci.appveyor.com/project/DrahtBot/bitcoin/builds/37547435 ACKs for top commit: glozow: utACK https://github.com/bitcoin/bitcoin/pull/21051/commits/b6aadcd5b4350a6ebcd57e88e7a0853cedf7c2fb 🚗 practicalswift: cr ACK b6aadcd5b4350a6ebcd57e88e7a0853cedf7c2fb: patch looks correct Tree-SHA512: 3ac887ebdbf9a1ae33c1fd5381b3b8d83388ad557ddeb55013acd42bb9752a5bd009e3a0eed52644a023a7a0dda1c159277981af82f58fb0abfe60b84e01bf29
2021-02-01doc: More precise -debug and -debugexclude docwodry
I wondered how one could enable debug logging with `-debug=<category>` for multiple categories. Found out solution is to specify that option multiple times for each wanted category. This PR documents this behavior and uses the same wording for the same behavior of `-debugexclude=<category>` to make that also clear and stringent.
2021-02-01Drop boost/preprocessor dependenciesHennadii Stepanov
2021-02-01refactor: Move STRINGIZE macro to macros.hHennadii Stepanov
This is a move-only change.
2021-02-01Merge #20464: refactor: Treat CDataStream bytes as uint8_tWladimir J. van der Laan
fa292724598c273867bc6dbf311f1440fe2541ba Remove redundant MakeUCharSpan wrappers (MarcoFalke) faf4aa2f47c0de4f3a0c5f5fe5b3ec32f611eefd Remove CDataStream::Init in favor of C++11 member initialization (MarcoFalke) fada14b948cac147198e3b685b5dd8cb72dc2911 Treat CDataStream bytes as uint8_t (MarcoFalke) fa8bdb048e65cae2d26bea3f991717a856e2fb39 refactor: Drop CDataStream constructors in favor of one taking a Span of bytes (MarcoFalke) faa96f841fe45bc49ebb6e07ac82a129fa9c40bf Remove unused CDataStream methods (MarcoFalke) Pull request description: Using `uint8_t` for raw bytes has a style benefit: * The signedness is clear from reading the code, as it does not depend on the architecture Other clean-ups in this pull include: * Remove unused methods * Constructor is simplified with `Span` * Remove `Init()` member in favor of C++11 member initialization ACKs for top commit: laanwj: code review ACK fa292724598c273867bc6dbf311f1440fe2541ba theStack: ACK fa292724598c273867bc6dbf311f1440fe2541ba 🍾 Tree-SHA512: 931ee28bd99843d7e894b48e90e1187ffb0278677c267044b3c0c255069d9bbd9298ab2e539b1002a30b543d240450eaec718ef4ee95a7fd4be0a295e926343f
2021-02-01Fix -Wmismatched-tags warningsHennadii Stepanov
2021-02-01Merge #21016: refactor: remove boost::thread_group usageWladimir J. van der Laan
dc8be12510c2fd5a809d9a82d2c14b464b5e5a3f refactor: remove boost::thread_group usage (fanquake) Pull request description: Post #18710, there isn't much left using `boost::thread_group`, so should just be able to replace it with the standard library. This also removes the last use of `boost::thread_interrupted`. After this change, last piece of Boost Thread we'd be using is `boost::shared_mutex`. See the commentary [here](https://github.com/bitcoin/bitcoin/issues/16684#issuecomment-726214696) as to why it may be non-trivial to swap that for `std::shared_mutex` in the near future. Closes #17307 ACKs for top commit: laanwj: Code review re-ACK dc8be12510c2fd5a809d9a82d2c14b464b5e5a3f MarcoFalke: review ACK dc8be12510c2fd5a809d9a82d2c14b464b5e5a3f 🔁 jonatack: Non-expert code review ACK dc8be12510c2fd5a809d9a82d2c14b464b5e5a3f, also checked range-diff since last review and that local debug build is clean with gcc 10.2.1-6 on Debian Tree-SHA512: 5510e2d760cce824234207dc86b1551ca8f21cbf3a2ce753c0254a0d03ffd83c94e449aec202fb7bd76e6fc64df783a6b70a736b0add9ece3734bb9c8ce8fc2f
2021-02-01Merge #20749: [Bundle 1/n] Prune g_chainman usage related to ::LookupBlockIndexWladimir J. van der Laan
67c9a83df19c6e2a2edb32336879204e7770b4a7 style-only: Remove redundant sentence in ActivateBestChain comment (Carl Dong) b8e95658d5909f93dfc7d1e6532661db8919e5b7 style-only: Make TestBlockValidity signature readable (Carl Dong) 0cdad753903640ff4240b715dec9d62f68e51407 validation: Use accessible chainstate in ChainstateManager::ProcessNewBlock (Carl Dong) ea4fed90219be17160136313c68c06d84176af08 validation: Use existing chainstate in ChainstateManager::ProcessNewBlockHeaders (Carl Dong) e0dc3057277c9576ddbfb8541599db0149e08bb6 validation: Move LoadExternalBlockFile to CChainState (Carl Dong) 5f8cd7b3a527999512161956db4d718688cb956f validation: Remove global ::ActivateBestChain (Carl Dong) 2a696472a1423e877bfa83f016f66c7e45be7369 validation: Pass in chainstate to ::NotifyHeaderTip (Carl Dong) 9c300cc8b3ce3d82874982fbf3087e48a6ac0ef2 validation: Pass in chainstate to TestBlockValidity (Carl Dong) 0e17c833cda67cdba5338bd7409061772b6d5edb validation: Make CChainState.m_blockman public (Carl Dong) d363d06bf7d6c3736140672ba8a7f82f4d6fb6ab validation: Pass in blockman to ContextualCheckBlockHeader (Carl Dong) f11d11600ddb0ddff6538250ae2a35df6112c3db validation: Move GetLastCheckpoint to BlockManager (Carl Dong) e4b95eefbc700ebc915bec312f77477ce3e87a7e validation: Move GetSpendHeight to BlockManager (Carl Dong) b026e318c39f59a06e29f1b25c7f577e01b25ccb validation: Move FindForkInGlobalIndex to BlockManager (Carl Dong) 3664a150ac7547c9336b571557af223d9e31aac9 validation: Remove global LookupBlockIndex (Carl Dong) eae54e6e60d7ed05b29d8345c0bb055397149ce8 scripted-diff: Use BlockManager::LookupBlockIndex (Carl Dong) 15d20f40e1321b24963b40c12958c7d30ad112ec validation: Move LookupBlockIndex to BlockManager (Carl Dong) f92dc6557a153b390a1ae1d0808ff7ed5d02c66e validation: Guard the active_chainstate with cs_main (Carl Dong) Pull request description: Overall PR: #20158 (tree-wide: De-globalize ChainstateManager) Note to reviewers: 1. This bundle may _apparently_ introduce usage of `g_chainman` or `::Chain(state|)Active()` globals, but these are resolved later on in the overall PR. [Commits of overall PR](https://github.com/bitcoin/bitcoin/pull/20158/commits) 2. There may be seemingly obvious local references to `ChainstateManager` or other validation objects which are not being used in callers of the current function in question, this is done intentionally to **_keep each commit centered around one function/method_** to ease review and to make the overall change systematic. We don't assume anything about our callers. Rest assured that once we are considering that particular caller in later commits, we will use the obvious local references. [Commits of overall PR](https://github.com/bitcoin/bitcoin/pull/20158/commits) 3. When changing a function/method that has many callers (e.g. `LookupBlockIndex` with 55 callers), it is sometimes easier (and less error-prone) to use a scripted-diff. When doing so, there will be 3 commits in sequence so that every commit compiles like so: 1. Add `new_function`, make `old_function` a wrapper of `new_function`, divert all calls to `old_function` to `new_function` **in the local module only** 2. Scripted-diff to divert all calls to `old_function` to `new_function` **in the rest of the codebase** 3. Remove `old_function` ACKs for top commit: jnewbery: utACK 67c9a83df19c6e2a2edb32336879204e7770b4a7 laanwj: re-ACK 67c9a83df19c6e2a2edb32336879204e7770b4a7 ryanofsky: Code review ACK 67c9a83df19c6e2a2edb32336879204e7770b4a7. Changes since last review: Tree-SHA512: 8744aba2dd57a40cd2fedca809b0fe24d771bc60da1bffde89601999384aa0df428057a86644a3f72fbeedbc8b04db6c4fd264ea0db2e73c279e5acc6d056cbf
generated by cgit v1.2.3 (git 2.26.2) at 2025-03-21 20:24:16 +0000