aboutsummaryrefslogtreecommitdiff
path: root/src
AgeCommit message (Collapse)Author
2021-02-12txdb: don't reset during in-memory cache resizeJames O'Beirne
We can't support a reset of the dbwrapper object when in-memory configuration is used because it results in the permanent loss of coins. This only affects unittest configurations (since that's the only place we use in-memory CCoinsViewDB instances).
2021-02-12simplify ChainstateManager::SnapshotBlockhash() return semanticsJames O'Beirne
Don't return null snapshotblockhash values to avoid caller complexity/confusion.
2021-02-12chainparams: add allowed assumeutxo valuesJames O'Beirne
Values for mainnet and testnet will be specified in a follow-up PR that can be scrutinized accordingly. This structure is required for use in snapshot activation logic.
2021-02-12Merge #19884: p2p: No delay in adding fixed seeds if -dnsseed=0 and ↵Wladimir J. van der Laan
peers.dat is empty fe3e993968d6b46777d5a16a662cd22790ddf5bb [p2p] No delay in adding fixed seeds if -dnsseed=0 and peers.dat is empty. Add -fixedseeds arg. (Dhruv Mehta) Pull request description: Closes #19795 Before PR: If `peers.dat` is empty and `-dnsseed=0`, bitcoind will fallback on to fixed seeds but only after a 60 seconds delay. After PR: There's no 60 second delay. To reproduce: `rm ~/.bitcoin/peers.dat && src/bitcoind -dnsseed=0` without and with patch code Other changes in the PR: - `-fixedseeds` command line argument added: `-dnsseed=0 -fixedseeds=0 -addnode=X` provides a trusted peer only setup. `-dnsseed=0 -fixedseeds=0` allows for a `addnode` RPC to add a trusted peer without falling back to hardcoded seeds. ACKs for top commit: LarryRuane: re-ACK fe3e993968d6b46777d5a16a662cd22790ddf5bb laanwj: re-ACK fe3e993968d6b46777d5a16a662cd22790ddf5bb Tree-SHA512: 79449bf4e83a315be6dbac9bdd226de89d2a3f7f76d9c5640a2cb3572866e6b0e8ed67e65674c9824054cf13119dc01c7e1a33848daac6b6c34dbc158b6dba8f
2021-02-12Merge #21064: refactor: use std::shared_mutex & remove Boost ThreadWladimir J. van der Laan
060a2a64d40d75fecb60b7d2b9946a67e46aa6fc ci: remove boost thread installation (fanquake) 06e1d7d81d5a56d136c6fc88f09a2b0654a164f9 build: don't build or use Boost Thread (fanquake) 7097add83c8596f81be9edd66971ffd2486357eb refactor: replace Boost shared_mutex with std shared_mutex in sigcache (fanquake) 8e55981ef834490c438436719f95cbaf888c4914 refactor: replace Boost shared_mutex with std shared_mutex in cuckoocache tests (fanquake) Pull request description: This replaces `boost::shared_mutex` and `boost::unique_lock` with [`std::shared_mutex`](https://en.cppreference.com/w/cpp/thread/shared_mutex) & [`std::unique_lock`](https://en.cppreference.com/w/cpp/thread/unique_lock). Even though [some concerns were raised](https://github.com/bitcoin/bitcoin/issues/16684#issuecomment-726214696) in #16684 with regard to `std::shared_mutex` being unsafe to use across some glibc versions, I still think this change is an improvement. As I mentioned in #21022, I also think trying to restrict standard library feature usage based on bugs in glibc is not only hard to do, but it's not currently clear exactly how we do that in practice (does it also extend to patching out use in our dependencies, should we be implementing more runtime checks for features we are using, when do we consider an affected glibc "old enough" not to worry about? etc). If you take a look through the [glibc bug tracker](https://sourceware.org/bugzilla/describecomponents.cgi?product=glibc) you'll no doubt find plenty of (active) bug reports for standard library code we already using. Obviously not to say we shouldn't try and avoid buggy code where possible. Two other points: [Cory mentioned in #21022](https://github.com/bitcoin/bitcoin/pull/21022#issuecomment-769274179): > It also seems reasonable to me to worry that boost hits the same underlying glibc bug, and we've just not happened to trigger the right conditions yet. Moving away from Boost to the standard library also removes the potential for differences related to Boosts configuration. Boost has multiple versions of `shared_mutex`, and what you end up using, and what it's backed by depends on: * The version of Boost. * The platform you're building for. * Which version of `BOOST_THREAD_VERSION` is defined: (2,3,4 or 5) default=2. (see [here](https://www.boost.org/doc/libs/1_70_0/doc/html/thread/build.html#thread.build.configuration) for some of the differences). * Is `BOOST_THREAD_V2_SHARED_MUTEX` defined? (not by default). If so, you might get the ["less performant, but more robust"](https://github.com/boostorg/thread/issues/230#issuecomment-475937761) version of `shared_mutex`. A lot of these factors are eliminated by our use of depends, but users will have varying configurations. It's also not inconceivable to think that a distro, or some package manager might start defining something like `BOOST_THREAD_VERSION=3`. Boost tried to change the default from 2 to 3 at one point. With this change, we no longer use Boost Thread, so this PR also removes it from depends, the build system, CI etc. Previous similar PRs were #19183 & #20922. The authors are included in the commits here. Also related to #21022 - pthread sanity checking. ACKs for top commit: laanwj: Code review ACK 060a2a64d40d75fecb60b7d2b9946a67e46aa6fc vasild: ACK 060a2a64d40d75fecb60b7d2b9946a67e46aa6fc Tree-SHA512: 572d14d8c9de20bc434511f20d3f431836393ff915b2fe9de5a47a02dca76805ad5c3fc4cceecb4cd43f3ba939a0508178c4e60e62abdbaaa6b3e8db20b75b03
2021-02-12Merge #19522: build: fix building libconsensus with reduced exports for ↵Wladimir J. van der Laan
Darwin targets de4238f92f4c067f099663f68d9772105de81d75 build: consolidate reduced export checks (fanquake) 012bdec1b7df01906566a6526e56f27d57d1653b build: add building libconsensus to end-of-configure output (fanquake) 8f360e349e365870b40a6873917c81de714ae41a build: remove ax_gcc_func_attribute macro (fanquake) f054a089ecfbdc4732e6f705a10e93189074f41c build: remove AX_GCC_FUNC_ATTRIBUTE test for dllimport (fanquake) 7cd0a696643a824ab6f6911278f116f01c5af662 build: test for __declspec(dllexport) in configure (fanquake) 1624e17b5430dfe808bb3b1b79dfa53bf45aa053 build: remove duplicate visibility attribute detection (fanquake) Pull request description: Darwin targets do not have a `protected` visibility function attribute, see [LLVM explanation](https://github.com/llvm/llvm-project/blob/8e9a505139fbef7d2e6e9d0adfe1efc87326f9ef/clang/lib/Basic/Targets/OSTargets.h#L131). This means that the `AX_GCC_FUNC_ATTRIBUTE` check for `visibility` fails: ```bash configure:24513: checking for __attribute__((visibility)) configure:24537: g++ -std=c++11 -o conftest -g -O2 -DHAVE_BUILD_INFO -D__STDC_FORMAT_MACROS -DMAC_OSX -DOBJC_OLD_DISPATCH_PROTOTYPES=0 -Wl,-headerpad_max_install_names conftest.cpp >&5 conftest.cpp:35:56: warning: target does not support 'protected' visibility; using 'default' [-Wunsupported-visibility] int foo_pro( void ) __attribute__((visibility("protected"))); ^ 1 warning generated. configure:24537: $? = 0 configure:24550: result: no ``` This leads to `EXPORT_SYMBOL` being [defined to nothing](https://github.com/bitcoin/bitcoin/blob/f4de89edfa8be4501534fec0c662c650a4ce7ef2/src/script/bitcoinconsensus.h#L29), as `HAVE_FUNC_ATTRIBUTE_VISIBILITY` is not defined, and when building with reduced exports, you end up with a libbitcoinconsensus.dylib that doesn't export any `_bitcoinconsensus_*` symbols. ```bash ➜ git:(master) nm -C src/.libs/libbitcoinconsensus.dylib | rg _bitcoinconsensus_ ➜ git:(master) ``` We do have a [second check](https://github.com/bitcoin/bitcoin/blob/f4de89edfa8be4501534fec0c662c650a4ce7ef2/configure.ac#L882) for the `visibility` attribute, which works for Darwin as it's only testing for default visibility, however the result of this check isn't used at all. It was added in #4725, along with the `--enable-reduce-exports` option, however when libbitcoinconsensus was added in #5235, it used the results of the added `AX_GCC_FUNC_ATTRIBUTE` calls. This PR removes our usage of the AX_GCC_FUNC_ATTRIBUTE macro entirely, in favour of our own checks in configure. This meant adding a check for `dllexport`, which I've tested as working with both [GCC](https://gcc.gnu.org/onlinedocs/gcc/Microsoft-Windows-Function-Attributes.html) and [Clang](https://releases.llvm.org/10.0.0/tools/clang/docs/AttributeReference.html#dllexport) when building for Windows. I haven't added an equivalent check for `dllimport`, as we weren't actually using the result of that check, we're just testing that `MSC_VER` was defined before using. With these changes building a libbitcoinconsensus with reduced exports, when targeting Darwin, works as expected: ```bash ./autogen.sh ./configure --disable-tests --disable-bench --with-utils=no --with-daemon=no --with-gui=no --disable-wallet --with-libs=yes --enable-reduce-exports make -j8 ... nm -C src/.libs/libbitcoinconsensus.dylib | rg _bitcoinconsensus_ 000000000000a340 T _bitcoinconsensus_verify_script 00000000000097e0 T _bitcoinconsensus_verify_script_with_amount 000000000000a3c0 T _bitcoinconsensus_version ``` ```python >>> import ctypes >>> consensus = ctypes.CDLL("src/.libs/libbitcoinconsensus.dylib") >>> print(consensus.bitcoinconsensus_version()) 1 >>> exit() ``` TODO: Modify a CI job to compile with --enable-reduce-exports and check for symbols in shared lib? ACKs for top commit: laanwj: Code review ACK de4238f92f4c067f099663f68d9772105de81d75 Tree-SHA512: d148f3c55d14dac6e9e5b718cc65bb557bcf6f663218d24bc9044b86281bd5dd3d931ebea79c336a58e8ed50d683218c0a9e75494f2267b91097665043e252ae
2021-02-12Merge #19145: Add hash_type MUHASH for gettxoutsetinfoWladimir J. van der Laan
e987ae5a554c9952812746c29f2766bacea4b727 test: Add test for deterministic UTXO set hash results (Fabian Jahr) 6ccc8fc067bf516cda7bc5d7d721945be5ac2003 test: Add test for gettxoutsetinfo RPC with MuHash (Fabian Jahr) 0d3b2f643d7da3202c0a0e757539208c4aa7c450 rpc: Add hash_type MUHASH to gettxoutsetinfo (Fabian Jahr) 2474645f3b15687e7f196b89eb935d6e6a98a9da refactor: Separate hash and stats calculation in coinstats (Fabian Jahr) a1fcceac69097a8e6540a6fd8121a5d53022528f refactor: Improve encapsulation between MuHash3072 and Num3072 (Fabian Jahr) Pull request description: This is another Pr in the series PRs for Coinstatsindex (see overview in #18000). This PR adds the `hash_type` option `muhash` to `gettxoutsetinfo` through which the user can calculate the serialized muhash of the utxo set. This PR does not use the index yet. ACKs for top commit: Sjors: tACK e987ae5 achow101: ACK e987ae5a554c9952812746c29f2766bacea4b727 jonatack: Tested re-ACK e987ae5a554c9952812746c29f2766bacea4b727 per `git diff 3506d90 e987ae5`, reviewed diff, debug built, ran gettxoutsetinfo -signet and help on this branch vs master, at height 23127 both returned `hash_serialized_2` of `2b72d65f3b6efb2311f58374ea2b939abf49684d44f4bafda45faa3b5452a454` and this branch returned `muhash` of `c9f1ff12d345ccf9939c6bbf087e6f7399b6115adee1569287e9c5c43dbb475c` ryanofsky: Code review ACK e987ae5a554c9952812746c29f2766bacea4b727. Looks very good. I left one suggestion to simplify code, but feel free to ignore it here and maybe consider it for later since PR has already had a lot of review. Tree-SHA512: 9a739ce375e73749fa69a467262b60d3e5314ef384e2d7150b3bbc8e4125cd9fd1db95306623bb9a632fcbaf5d9d2bf2f5cc43bf717d4ff5e2c9c4b52dd9296c
2021-02-12build: test for __declspec(dllexport) in configurefanquake
This should work for GCC and Clang when building for Windows targets.
2021-02-12build: remove duplicate visibility attribute detectionfanquake
We are already testing for this, and our test works correctly with a Darwin target, where the macro does not. Darwin targets do not support "protected" visibility.
2021-02-11[p2p] No delay in adding fixed seeds if -dnsseed=0 and peers.dat is empty. ↵Dhruv Mehta
Add -fixedseeds arg.
2021-02-11Merge #21041: log: Move "Pre-allocating up to position 0x[…] in […].dat" ↵Wladimir J. van der Laan
log message to debug category 25f899cc23a791c08e19acae91bebda6c3538d37 log: Move "Pre-allocating up to position 0x[...] in [...].dat" log message to debug category (practicalswift) acd7980b37b5a71f324f7772d72175c8bd7ab900 log: Move "Leaving block file [...]: [...]" log message to debug category (practicalswift) Pull request description: Move `Pre-allocating up to position 0x[…] in […].dat` log message to debug category. After the cleanup of `-debug=net` log messages PR (#20724) was merged recently the console log now has very high signal to noise ratio. That's great! :) This PR increases the signal to noise ratio slightly more by moving the most common remaining implementation detail log message (`Pre-allocating up to position 0x[…] in […].dat`) to the debug category where it belongs :) Expected standard output from `bitcoind` (when in steady state) before this patch: ``` $ src/bitcoind … 0000-00-00T00:00:00Z UpdateTip: new best=0000000000000000000000000000000000000000000000000000000000000000 height=000000 version=0x00000000 log0_work=00.000000 tx=000000000 date='0000-00-00T00:00:00Z' progress=0.000000 cache=000.0MiB(0000000txo) 0000-00-00T00:00:00Z UpdateTip: new best=0000000000000000000000000000000000000000000000000000000000000000 height=000000 version=0x00000000 log0_work=00.000000 tx=000000000 date='0000-00-00T00:00:00Z' progress=0.000000 cache=000.0MiB(0000000txo) 0000-00-00T00:00:00Z Pre-allocating up to position 0x0000000 in blk00000.dat 0000-00-00T00:00:00Z Pre-allocating up to position 0x000000 in rev00000.dat 0000-00-00T00:00:00Z UpdateTip: new best=0000000000000000000000000000000000000000000000000000000000000000 height=000000 version=0x00000000 log0_work=00.000000 tx=000000000 date='0000-00-00T00:00:00Z' progress=0.000000 cache=000.0MiB(0000000txo) 0000-00-00T00:00:00Z UpdateTip: new best=0000000000000000000000000000000000000000000000000000000000000000 height=000000 version=0x00000000 log0_work=00.000000 tx=000000000 date='0000-00-00T00:00:00Z' progress=0.000000 cache=000.0MiB(0000000txo) 0000-00-00T00:00:00Z UpdateTip: new best=0000000000000000000000000000000000000000000000000000000000000000 height=000000 version=0x00000000 log0_work=00.000000 tx=000000000 date='0000-00-00T00:00:00Z' progress=0.000000 cache=000.0MiB(0000000txo) ``` Expected standard output from `bitcoind` (when in steady state) after this patch: ``` $ src/bitcoind … 0000-00-00T00:00:00Z UpdateTip: new best=0000000000000000000000000000000000000000000000000000000000000000 height=000000 version=0x00000000 log0_work=00.000000 tx=000000000 date='0000-00-00T00:00:00Z' progress=0.000000 cache=000.0MiB(0000000txo) 0000-00-00T00:00:00Z UpdateTip: new best=0000000000000000000000000000000000000000000000000000000000000000 height=000000 version=0x00000000 log0_work=00.000000 tx=000000000 date='0000-00-00T00:00:00Z' progress=0.000000 cache=000.0MiB(0000000txo) 0000-00-00T00:00:00Z UpdateTip: new best=0000000000000000000000000000000000000000000000000000000000000000 height=000000 version=0x00000000 log0_work=00.000000 tx=000000000 date='0000-00-00T00:00:00Z' progress=0.000000 cache=000.0MiB(0000000txo) 0000-00-00T00:00:00Z UpdateTip: new best=0000000000000000000000000000000000000000000000000000000000000000 height=000000 version=0x00000000 log0_work=00.000000 tx=000000000 date='0000-00-00T00:00:00Z' progress=0.000000 cache=000.0MiB(0000000txo) 0000-00-00T00:00:00Z UpdateTip: new best=0000000000000000000000000000000000000000000000000000000000000000 height=000000 version=0x00000000 log0_work=00.000000 tx=000000000 date='0000-00-00T00:00:00Z' progress=0.000000 cache=000.0MiB(0000000txo) ``` I find the latter alternative much easier to visually scan for anomalies (and more aesthetically pleasing TBH!). Non-GUI users deserve nice interfaces too :) ACKs for top commit: laanwj: ACK 25f899cc23a791c08e19acae91bebda6c3538d37 Tree-SHA512: 5970798c41b041527ebdcbd843c5e136c257c28c3b21fc74102da8970406ca5c0c7e406305c5e6e67de5c1708dc1858af07a77a2e05f44159b7103423e8ab32f
2021-02-11Merge #20370: fuzz: version handshakeMarcoFalke
fabce459bb44e90dc7ae9c44eeedab707435af5b fuzz: version handshake (MarcoFalke) Pull request description: Not fuzzing the version handshake will limit fuzz coverage ACKs for top commit: practicalswift: cr ACK fabce459bb44e90dc7ae9c44eeedab707435af5b: patch looks very much correct Tree-SHA512: 4091d27d39edee781d033e471b352084bb54df250d0890e4821a325926a44dff9b26a2614d67dd0529f73bd366b075d7a0a1a570c2837de286a1b93a59a8fb91
2021-02-11Merge #21062: refactor: return MempoolAcceptResult from ATMPMarcoFalke
53e716ea119658c28935fee24eb50090907c500e [refactor] improve style for touched code (gzhao408) 174cb5330af4b09f3a66974d3bae783ea43b190e [refactor] const ATMPArgs and non-const Workspace (gzhao408) f82baf0762f60c2ca5ffc339b095f9271d7c2f33 [refactor] return MempoolAcceptResult (gzhao408) 9db10a55061e09021ff8ea1d6637d99f7959035f [refactor] clean up logic in testmempoolaccept (gzhao408) Pull request description: This is the first 4 commits of #20833, and does refactoring only. It should be relatively simple to review, and offers a few nice things: - It makes accessing values that don't make sense (e.g. fee) when the tx is invalid an error. - Returning `MempoolAcceptResult` from ATMP makes the interface cleaner. The caller can get a const instead of passing in a mutable "out" param. - We don't have to be iterating through a bunch of lists for package validation, we can just return a `std::vector<MempoolAcceptResult>`. - We don't have to refactor all ATMP call sites again if/when we want to return more stuff from it. ACKs for top commit: MarcoFalke: ACK 53e716ea119658c28935fee24eb50090907c500e 💿 jnewbery: Code review ACK 53e716ea119658c28935fee24eb50090907c500e ariard: Code Review ACK 53e716e, I did tweak a bit the touched paths to see if we had good test coverage. Didn't find holes. Tree-SHA512: fa6ec324a08ad9e6e55948615cda324cba176255708bf0a0a0f37cedb7a75311aa334ac6f223be7d8df3c7379502b1081102b9589f9a9afa1713ad3d9ab3c24f
2021-02-11Merge #20788: net: add RAII socket and use it instead of bare SOCKETWladimir J. van der Laan
615ba0eb96cf131364c1ceca9d3dedf006fa1e1c test: add Sock unit tests (Vasil Dimov) 7bd21ce1efc363b3e8ea1d51dd1410ccd66820cb style: rename hSocket to sock (Vasil Dimov) 04ae8469049e1f14585aabfb618ae522150240a7 net: use Sock in InterruptibleRecv() and Socks5() (Vasil Dimov) ba9d73268f9585d4b9254adcf54708f88222798b net: add RAII socket and use it instead of bare SOCKET (Vasil Dimov) dec9b5e850c6aad989e814aea5b630b36f55d580 net: move CloseSocket() from netbase to util/sock (Vasil Dimov) aa17a44551c03b00a47854438afe9f2f89b6ea74 net: move MillisToTimeval() from netbase to util/time (Vasil Dimov) Pull request description: Introduce a class to manage the lifetime of a socket - when the object that contains the socket goes out of scope, the underlying socket will be closed. In addition, the new `Sock` class has a `Send()`, `Recv()` and `Wait()` methods that can be overridden by unit tests to mock the socket operations. The `Wait()` method also hides the `#ifdef USE_POLL poll() #else select() #endif` technique from higher level code. ACKs for top commit: laanwj: Re-ACK 615ba0eb96cf131364c1ceca9d3dedf006fa1e1c jonatack: re-ACK 615ba0eb96cf131364c1ceca9d3dedf006fa1e1c Tree-SHA512: 3003e6bc0259295ca0265ccdeb1522ee25b4abe66d32e6ceaa51b55e0a999df7ddee765f86ce558a788c1953ee2009bfa149b09d494593f7d799c0d7d930bee8
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-11test: add Sock unit testsVasil Dimov
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-10log: Move "Pre-allocating up to position 0x[...] in [...].dat" log message ↵practicalswift
to debug category
2021-02-10log: Move "Leaving block file [...]: [...]" log message to debug categorypracticalswift
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-10style: rename hSocket to sockVasil Dimov
In the arguments of `InterruptibleRecv()`, `Socks5()` and `ConnectThroughProxy()` the variable `hSocket` was previously of type `SOCKET`, but has been changed to `Sock`. Thus rename it to `sock` to imply its type, to distinguish from other `SOCKET` variables and to abide to the coding style wrt variables' names.
2021-02-10net: use Sock in InterruptibleRecv() and Socks5()Vasil Dimov
Use the `Sock` class instead of `SOCKET` for `InterruptibleRecv()` and `Socks5()`. This way the `Socks5()` function can be tested by giving it a mocked instance of a socket. Co-authored-by: practicalswift <practicalswift@users.noreply.github.com>
2021-02-10net: add RAII socket and use it instead of bare SOCKETVasil Dimov
Introduce a class to manage the lifetime of a socket - when the object that contains the socket goes out of scope, the underlying socket will be closed. In addition, the new `Sock` class has a `Send()`, `Recv()` and `Wait()` methods that can be overridden by unit tests to mock the socket operations. The `Wait()` method also hides the `#ifdef USE_POLL poll() #else select() #endif` technique from higher level code.
2021-02-10net: move CloseSocket() from netbase to util/sockVasil Dimov
Move `CloseSocket()` (and `NetworkErrorString()` which it uses) from `netbase.{h,cpp}` to newly added `src/util/sock.{h,cpp}`. This is necessary in order to use `CloseSocket()` from a newly introduced Sock class (which will live in `src/util/sock.{h,cpp}`). `sock.{h,cpp}` cannot depend on netbase because netbase will depend on it.
2021-02-10net: move MillisToTimeval() from netbase to util/timeVasil Dimov
Move `MillisToTimeval()` from `netbase.{h,cpp}` to `src/util/system.{h,cpp}`. This is necessary in order to use `MillisToTimeval()` from a newly introduced `src/util/sock.{h,cpp}` which cannot depend on netbase because netbase will depend on it.
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-09[refactor] improve style for touched codegzhao408
2021-02-09[refactor] const ATMPArgs and non-const Workspacegzhao408
ATMPArgs should contain const arguments for validation. The Workspace should contain state that may change throughout validation.
2021-02-09[refactor] return MempoolAcceptResultgzhao408
This creates a cleaner interface with ATMP, allows us to make results const, and makes accessing values that don't make sense (e.g. fee when tx is invalid) an error.
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