aboutsummaryrefslogtreecommitdiff
path: root/src
AgeCommit message (Collapse)Author
2020-05-19Merge #18999: log: Remove "No rpcpassword set" from logsfanquake
fa243be1dc49385fff847f8a784c7a9c9f07c939 log: Remove "No rpcpassword set" from logs (MarcoFalke) Pull request description: rpcpassword is deprecated and not recommended anymore. So remove it from the logs, which indicate that an rpcpassword should be set and cause confusion. See #18998. ACKs for top commit: ryanofsky: Code review ACK fa243be1dc49385fff847f8a784c7a9c9f07c939. New log message makes more sense elichai: Re Code Review ACK (Checked the diff) fa243be1dc49385fff847f8a784c7a9c9f07c939 Tree-SHA512: de3e0800a204b15a59a59a7e6f345013ee9d38e8c5d0c9a94d6142780faa9cce672ed358c7571f53c1eb843bf5afb0b7bcbfd289d3b9e2e0bf8ff2fd361e98a9
2020-05-19Merge #18861: Do not answer GETDATA for to-be-announced txfanquake
2896c412fadbc03916a33028f4f50fd87ac48edb Do not answer GETDATA for to-be-announced tx (Pieter Wuille) f2f32a3dee9a965c8198f9ddd3aaebc627c273e4 Push down use of cs_main into FindTxForGetData (Pieter Wuille) c6131bf407c1ada78a0e5509a702bc7da0bfd57d Abstract logic to determine whether to answer tx GETDATA (Pieter Wuille) Pull request description: This PR intends to improve transaction-origin privacy. In general, we should try to not leak information about what transactions we have (recently) learned about before deciding to announce them to our peers. There is a controlled transaction dissemination process that reveals our transactions to peers that has various safeguards for privacy (it's rate-limited, delayed & batched, deterministically sorted, ...), and ideally there is no way to test which transactions we have before that controlled process reveals them. The handling of the `mempool` BIP35 message has protections in this regard as well, as it would be an obvious way to bypass these protections (handled asynchronously after a delay, also deterministically sorted). However, currently, if we receive a GETDATA for a transaction that we have not yet announced to the requester, we will still respond to it if it was announced to *some* other peer already (because it needs to be in `mapRelay`, which only happens on the first announcement). This is a slight privacy leak. Thankfully, this seems easy to solve: `setInventontoryTxToSend` keeps track of the txids we have yet to announce to a peer - which almost(*) exactly corresponds to the transactions we know of that we haven't revealed to that peer. By checking whether a txid is in that set before responding to a GETDATA, we can filter these out. (*) Locally resubmitted or rebroadcasted transactions may end up in setInventoryTxToSend while the peer already knows we have them, which could result in us incorrectly claiming we don't have such transactions if coincidentally requested right after we schedule reannouncing them, but before they're actually INVed. This is made even harder by the fact that filterInventoryKnown will generally keep known reannouncements out of setInventoryTxToSend unless it overflows (which needs 50000 INVs in either direction before it happens). The condition for responding now becomes: ``` (not in setInventoryTxToSend) AND ( (in relay map) OR ( (in mempool) AND (old enough that it could have expired from relay map) AND (older than our last getmempool response) ) ) ``` ACKs for top commit: naumenkogs: utACK 2896c41 ajtowns: ACK 2896c412fadbc03916a33028f4f50fd87ac48edb amitiuttarwar: code review ACK 2896c412fa jonatack: ACK 2896c412fadbc03916 per `git diff 2b3f101 2896c41` only change since previous review is moving the recency check up to be verified first in `FindTxForGetData`, as it was originally in 353a391 (good catch), before looking up the transaction in the relay pool. jnewbery: code review ACK 2896c412fadbc03916a33028f4f50fd87ac48edb Tree-SHA512: e7d5bc006e626f60a2c108a9334f3bbb67205ace04a7450a1e4d4db1d85922a7589e0524500b7b4953762cf70554c4a08eec62c7b38b486cbca3d86321600868
2020-05-17log: Remove "No rpcpassword set" from logsMarcoFalke
2020-05-17Merge #18938: tests: Fill fuzzing coverage gaps for functions in ↵MarcoFalke
consensus/validation.h, primitives/block.h and util/translation.h cd34038cbda4864e4770734c44b18d3e01aa2a28 Switch from Optional<T> to std::optional<T> (C++17). Run clang-format. (practicalswift) fb559c1170773360afb9d05daaccd57d18636ee9 tests: Fill fuzzing coverage gaps for functions in util/translation.h (practicalswift) b74f3d6c452d9ad7013c70a91216220917978f66 tests: Fill fuzzing coverage gaps for functions in consensus/validation.h (practicalswift) c0bbf8193d92ba85d62092c4fd886ff4461f65bf tests: Fill fuzzing coverage gaps for functions in primitives/block.h (practicalswift) Pull request description: * Fill fuzzing coverage gaps for functions in `consensus/validation.h` * Fill fuzzing coverage gaps for functions in `primitives/block.h` * Fill fuzzing coverage gaps for functions in `util/translation.h` * Switch from `Optional<T>` to `std::optional<T>` (C++17). Run `clang-format`. See [`doc/fuzzing.md`](https://github.com/bitcoin/bitcoin/blob/master/doc/fuzzing.md) for information on how to fuzz Bitcoin Core. Don't forget to contribute any coverage increasing inputs you find to the [Bitcoin Core fuzzing corpus repo](https://github.com/bitcoin-core/qa-assets). Happy fuzzing :) Top commit has no ACKs. Tree-SHA512: d6aa4634c3953ade173589a8239bd230eb317ef897835a8557acb73df01b25e5e17bf46f837838e59ec04c1f3d3b7d1309ba68c8a264d17b938215512c9e6085
2020-05-15Merge #18781: Add templated GetRandDuration<>MarcoFalke
0000ea32656833efa3d2ffd9bab66c88c83334f0 test: Add test for GetRandMillis and GetRandMicros (MarcoFalke) fa0e5b89cf742df56c6c8f49fe9b3c54d2970a66 Add templated GetRandomDuration<> (MarcoFalke) Pull request description: A naive implementation of this template is dangerous, because the call site might accidentally omit the template parameter: ```cpp template <typename D> D GetRandDur(const D& duration_max) { return D{GetRand(duration_max.count())}; } BOOST_AUTO_TEST_CASE(util_time_GetRandTime) { std::chrono::seconds rand_hour = GetRandDur(std::chrono::hours{1}); // Want seconds to be in range [0..1hour), but always get zero :(((( BOOST_CHECK_EQUAL(rand_hour.count(), 0); } ``` Luckily `std::common_type` is already specialised in the standard lib for `std::chrono::duration` (https://en.cppreference.com/w/cpp/chrono/duration/common_type). And its effect seem to be that the call site must always specify the template argument explicitly. So instead of implementing the function for each duration type by hand, replace it with a templated version that is safe to use. ACKs for top commit: laanwj: Code review ACK 0000ea32656833efa3d2ffd9bab66c88c83334f0 promag: Code review ACK 0000ea32656833efa3d2ffd9bab66c88c83334f0. jonatack: ACK 0000ea3 thanks for the improved documentation. Code review, built, ran `src/test/test_bitcoin -t random_tests -l test_suite` for the new unit tests, `git diff fa05a4c 0000ea3` since previous review: hebasto: ACK 0000ea32656833efa3d2ffd9bab66c88c83334f0 with non-blocking [nit](https://github.com/bitcoin/bitcoin/pull/18781#discussion_r424924671). Tree-SHA512: e89d46e31452be6ea14269ecbbb2cdd9ae83b4412cd14dff7d1084283092722a2f847cb501e8054394e4a3eff852f9c87f6d694fd008b3f7e8458cb5a3068af7
2020-05-15Merge #18975: test: Remove const to work around compiler error on xenialfanquake
050e2ee6f28e7b31c167013be7313726e34084e9 test: Remove const to work around compiler error on xenial (Wladimir J. van der Laan) Pull request description: Fix the following error in travis: test/validationinterface_tests.cpp:26:36: error: default initialization of an object of const type 'const BlockValidationState' without a user-provided default constructor const BlockValidationState state_dummy; ACKs for top commit: MarcoFalke: Tested ACK 050e2ee6f28e7b31c167013be7313726e34084e9 on xenial with clang version 3.8.0-2ubuntu4 (tags/RELEASE_380/final) fanquake: ACK 050e2ee6f28e7b31c167013be7313726e34084e9 - I see why we didn't hit this on master. We are installing the `clang-8` packages for the tsan job. However on the 0.20 branch we are still just installing `clang`, which is 3.8. Tree-SHA512: 8a1d57289dbe9895ab79f81ca87b4fd723426b8d72f3a34bec9553226fba69f6dc19551c1f1d52db6c4b2652164a02ddc60f3187c3e2ad7bcacb0aaca7fa690a
2020-05-14Switch from Optional<T> to std::optional<T> (C++17). Run clang-format.practicalswift
2020-05-14tests: Fill fuzzing coverage gaps for functions in util/translation.hpracticalswift
2020-05-14tests: Fill fuzzing coverage gaps for functions in consensus/validation.hpracticalswift
2020-05-14tests: Fill fuzzing coverage gaps for functions in primitives/block.hpracticalswift
2020-05-14Merge #18962: net processing: Only send a getheaders for one block in an INVWladimir J. van der Laan
746736639e6d05acdb85c866d4c605c947d4c500 [net processing] Only send a getheaders for one block in an INV (John Newbery) Pull request description: Headers-first is the primary method of announcement on the network. If a node fell back sending blocks by inv, it's probably for a re-org. The final block hash provided should be the highest, so send a getheaders and then fetch the blocks we need to catch up. Sending many GETHEADERS messages to the peer would cause them to send a large number of potentially large HEADERS messages with redundant data, which is a waste of bandwidth. ACKs for top commit: sipa: utACK 746736639e6d05acdb85c866d4c605c947d4c500 mzumsande: utACK 746736639e6d05acdb85c866d4c605c947d4c500 as per ajtowns' reasoning. naumenkogs: utACK 7467366 ajtowns: ACK 746736639e6d05acdb85c866d4c605c947d4c500 jonatack: ACK 746736639e6d05acdb85c866d4c605c947d4c500 Tree-SHA512: 59e243b80d3f0873709dfacb2e4ffba34689aad7de31ec7f69a64e0e3a0756235a0150e4082ff5de823949ba4411ee1aed2344b4749b62e0eb1ea906e41f5ea9
2020-05-14Merge #18946: rpcwallet: Replace boost::optional<T>::emplace with simple ↵Wladimir J. van der Laan
assignment of T{} fa1f840596554ed264d11ee3b3643bf99eb57eb5 rpcwallet: Replace pwallet-> with wallet. (MarcoFalke) fa182a8794cbf9be1aa91d1d75e65bc7800156bc rpcwallet: Replace boost::optional<T>::emplace with simple assignment of T{} (MarcoFalke) Pull request description: Closes #18943 ACKs for top commit: laanwj: ACK fa1f840596554ed264d11ee3b3643bf99eb57eb5 ryanofsky: Code review ACK fa1f840596554ed264d11ee3b3643bf99eb57eb5 and thanks for using a standalone commit for the fix promag: Code review ACK fa1f840596554ed264d11ee3b3643bf99eb57eb5. hebasto: ACK fa1f840596554ed264d11ee3b3643bf99eb57eb5, tested on Linux Mint 19.3. Tree-SHA512: 0838485d1f93f737ce5bf12740669dcafeebb78dbc3fa15dbcc511edce64bf024f60f0497a04149a1e799d893d57b0c9ffe442020c1b9cfc3c69db731f50e712
2020-05-14test: Remove const to work around compiler error on xenialWladimir J. van der Laan
Fix the following error in travis: test/validationinterface_tests.cpp:26:36: error: default initialization of an object of const type 'const BlockValidationState' without a user-provided default constructor const BlockValidationState state_dummy;
2020-05-14Merge #18742: miner: Avoid stack-use-after-return in validationinterfacefanquake
7777f2a4bb1f9d843bc50a4e35085cfbb2808780 miner: Avoid stack-use-after-return in validationinterface (MarcoFalke) fa5ceb25fce2200edf6b8ebfa6d4f01ed6774b95 test: Remove UninterruptibleSleep from test and replace it by SyncWithValidationInterfaceQueue (MarcoFalke) fa770ce7fe67685c43780e219d8232efbee0bb8e validationinterface: Rework documentation, Rename pwalletIn to callbacks (MarcoFalke) fab6d060ce5f580db538070beec1c5518c8c777c test: Add unregister_validation_interface_race test (MarcoFalke) Pull request description: When a validationinterface has itself unregistered in one thread, but is about to get executed in another thread [1], there is a race: * The validationinterface destructing itself * The validationinterface getting dereferenced for execution [1] https://github.com/bitcoin/bitcoin/blob/64139803f1225dab26197a20314109d37fa87d5f/src/validationinterface.cpp#L82-L83 This happens in the miner. More generally it happens everywhere where at least one thread is generating notifications and another one is unregistering a validationinterface. This issue has been fixed in commit ab31b9d6fe7b39713682e3f52d11238dbe042c16, but the fix has not been applied to the miner. Example where this happened in practice: https://travis-ci.org/github/bitcoin/bitcoin/jobs/675322230#L4414 ACKs for top commit: promag: Code review ACK 7777f2a4bb1f9d843bc50a4e35085cfbb2808780. laanwj: Code review ACK 7777f2a4bb1f9d843bc50a4e35085cfbb2808780 Tree-SHA512: 8087119243c71ba18a823a63515f3730d127162625d8729024278b447af29e2ff206f4840ee3d90bf84f93a2c5ab73b76c7e7044c83aa93b5b51047a166ec3d3
2020-05-13miner: Avoid stack-use-after-return in validationinterfaceMarcoFalke
This is achieved by switching to a shared_ptr. Also, switch the validationinterfaces in the tests to use shared_ptrs for the same reason.
2020-05-13test: Remove UninterruptibleSleep from test and replace it by ↵MarcoFalke
SyncWithValidationInterfaceQueue For the purpose of this test the two have the same outcome, but this one is shorter and avoids a sleep for 0.1 seconds.
2020-05-13validationinterface: Rework documentation, Rename pwalletIn to callbacksMarcoFalke
2020-05-13test: Add unregister_validation_interface_race testMarcoFalke
This commit is (intentionally) adding a broken test. The test is broken because it registering a subscriber object that can go out of scope while events are still being sent. To run the broken test and reproduce the bug: - Remove comment /** and */ - ./configure --with-sanitizers=address - export ASAN_OPTIONS=detect_leaks=0 - make - while ./src/test/test_bitcoin -t validationinterface_tests/unregister_validation_interface_race --catch_system_errors=no ; do true; done
2020-05-13Merge #18922: gui: Do not translate InitWarning messages in debug.logJonas Schnelli
78be8d97d3d750fcfbbe509a72639b7b30b7bd18 util: Drop OpOriginal() and OpTranslated() (Hennadii Stepanov) da16f95c3fecf4ee1e9a1dc4333b0b92cd981afd gui: Do not translate InitWarning messages in debug.log (Hennadii Stepanov) 4c9b9a4882e68835f16a52f1f0657efe47e589b5 util: Enhance Join() (Hennadii Stepanov) fe05dd0611cfc2929a7fdec04ca5948bccf0233b util: Enhance bilingual_str (Hennadii Stepanov) Pull request description: This PR forces the `bitcoin-qt` to write `InitWarning()` messages to the `debug.log` file in untranslated form, i.e., in English. On master (376294cde6b1588cb17055d8fde567eaf5848c3c): ``` $ ./src/qt/bitcoin-qt -lang=nl -debug=vladidation -printtoconsole | grep 'vladi' Warning: Niet-ondersteunde logcategorie -debug=vladidation. 2020-05-09T12:39:59Z Warning: Niet-ondersteunde logcategorie -debug=vladidation. 2020-05-09T12:40:02Z Command-line arg: debug="vladidation" ``` With this PR: ``` $ ./src/qt/bitcoin-qt -lang=nl -debug=vladidation -printtoconsole | grep 'vladi' Warning: Unsupported logging category -debug=vladidation. 2020-05-09T12:42:04Z Warning: Unsupported logging category -debug=vladidation. 2020-05-09T12:42:35Z Command-line arg: debug="vladidation" ``` ![Screenshot from 2020-05-09 15-42-31](https://user-images.githubusercontent.com/32963518/81474073-c7a50e00-920b-11ea-8775-c41122dacafe.png) Related to #16218. ACKs for top commit: laanwj: ACK 78be8d97d3d750fcfbbe509a72639b7b30b7bd18 jonasschnelli: utACK 78be8d97d3d750fcfbbe509a72639b7b30b7bd18 MarcoFalke: ACK 78be8d97d3d750fcfbbe509a72639b7b30b7bd18 📢 Tree-SHA512: 48e9ecd23c4dd8ec262e3eb94f8e30944bcc9c6c163245fb837b2e0c484d4d0b4f47f7abc638c14edc27d635d340ba3ee4ba4506b062399e9cf59a1564c98755
2020-05-13Merge #18616: refactor: Cleanup clientversion.cppWladimir J. van der Laan
c269e618cf53634d3cf270273ab1b0354dc3c119 Drop unused GIT_COMMIT_DATE macro (Hennadii Stepanov) 8f9f4ba5e2b738fb63c2d6cdcfdce5c169d4e88c refactor: Remove duplicated code (Hennadii Stepanov) 35f1189ea7365c1fdaf7dd9ac5e90fc8af69eae7 build: Rename BUILD_* macros and the code self-descriptive (Hennadii Stepanov) dc1fba9389de130809834b7832d780ecf4161496 scripted-diff: Rename share/genbuild.sh macros to more meaningful ones (Hennadii Stepanov) 1e06bb68bed84e525c55022f789416ffd4793e8d Drop unused CLIENT_VERSION_SUFFIX macro (Hennadii Stepanov) Pull request description: This PR: - removes unused macros and duplicated code - renames macros in a way, that makes the code self-descriptive. ACKs for top commit: dongcarl: Yup! ACK c269e618cf53634d3cf270273ab1b0354dc3c119 Tree-SHA512: c469f6269b578ccfae33d960e317eca8efaf27d49638f4c3830948c11b12ef728494d7e18c31e4a410945b7d83af5b246c7b83661b4eca17cf41ee4c4583649b
2020-05-13Merge #18862: Remove fdelt_chk back-compat code and sanity checkWladimir J. van der Laan
df6bde031b24112abf3a94337a2c096698acde6e test: remove glibc fdelt sanity check (fanquake) 8bf1540cc235fb8fb5330a7ae8ab638247ceb177 build: remove fdelt_chk backwards compatibility code (fanquake) Pull request description: https://github.com/bitcoin/bitcoin/commit/ae30d40e50e9d63d875d29d54d22147b09fc420c The return type of [`fdelt_chk`](https://sourceware.org/git/?p=glibc.git;a=blob;f=debug/fdelt_chk.c;h=f62ce7349707cb68f55831c1c591fd7387a90258;hb=HEAD) changed from `unsigned long int` to `long int` in glibc 2.16. See [this commit](https://sourceware.org/git/?p=glibc.git;a=commit;h=ceb9e56b3d1f8c1922e0526c2e841373843460e2). Now that we require [glibc >=2.17](https://github.com/bitcoin/bitcoin/pull/17538) we can remove our back-compat code. https://github.com/bitcoin/bitcoin/commit/ab7bce584ae02bf25e2e91aa54f9b0249427127d While looking at the above changes, I noticed that our glibc fdelt sanity check doesn't seem to be checking anything. `fdelt_warn()` also isn't something we'd want to actually "trigger" at runtime, as doing so would cause `bitcoind` to abort. The comments: > // trigger: Call FD_SET to trigger __fdelt_chk. FORTIFY_SOURCE must be defined > // as >0 and optimizations must be set to at least -O2. suggest calling FD_SET to check the invocation of `fdelt_chk` (this is [aliased with fdelt_warn in glibc](https://sourceware.org/git/?p=glibc.git;a=blob;f=debug/fdelt_chk.c;h=f62ce7349707cb68f55831c1c591fd7387a90258;hb=HEAD)). However just calling `FD_SET()` will not necessarily cause the compiler to insert a call to `fd_warn()`. Whether or not GCC (recent Clang should work, but may use different heuristics) inserts a call to `fdelt_warn()` depends on if the compiler can determine if the value passed in is a compile time constant (using [`__builtin_constant_p`](https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html)) and whether the value is < 0 or >= `FD_SETSIZE`. The glibc implementation is [here](https://sourceware.org/git/?p=glibc.git;a=blob;f=misc/bits/select2.h;h=7e17430ed94dd1679af10afa3d74795f9c97c0e8;hb=HEAD). This means our check should never cause a call to be inserted. Compiling master without `--glibc-back-compat` (if you do pass `--glibc-back-compat` the outcome is still the same; however the abort will only happen with >=`FD_SETSIZE` as that is what our [fdelt_warn()](https://github.com/bitcoin/bitcoin/blob/master/src/compat/glibc_compat.cpp#L24) checks for), there are no calls to `fdelt_warn()` inserted by the compiler: ```bash objdump -dC bitcoind | grep sanity_fdelt ... 0000000000399d20 <sanity_test_fdelt()>: 399d20: 48 81 ec 98 00 00 00 sub $0x98,%rsp 399d27: b9 10 00 00 00 mov $0x10,%ecx 399d2c: 64 48 8b 04 25 28 00 mov %fs:0x28,%rax 399d33: 00 00 399d35: 48 89 84 24 88 00 00 mov %rax,0x88(%rsp) 399d3c: 00 399d3d: 31 c0 xor %eax,%eax 399d3f: 48 89 e7 mov %rsp,%rdi 399d42: fc cld 399d43: f3 48 ab rep stos %rax,%es:(%rdi) 399d46: 48 8b 84 24 88 00 00 mov 0x88(%rsp),%rax 399d4d: 00 399d4e: 64 48 33 04 25 28 00 xor %fs:0x28,%rax 399d55: 00 00 399d57: 75 0d jne 399d66 <sanity_test_fdelt()+0x46> 399d59: b8 01 00 00 00 mov $0x1,%eax 399d5e: 48 81 c4 98 00 00 00 add $0x98,%rsp 399d65: c3 retq 399d66: e8 85 df c8 ff callq 27cf0 <__stack_chk_fail@plt> 399d6b: 0f 1f 44 00 00 nopl 0x0(%rax,%rax,1) ``` If you modify the sanity test to pass `-1` or `FD_SETSIZE` to `FD_SET`, you'll see calls to `fdelt_warn` inserted, and the runtime behaviour is an abort as expected. ```diff diff --git a/src/compat/glibc_sanity_fdelt.cpp b/src/compat/glibc_sanity_fdelt.cpp index 87140d0c7..16974bfa0 100644 --- a/src/compat/glibc_sanity_fdelt.cpp +++ b/src/compat/glibc_sanity_fdelt.cpp @@ -20,7 +20,7 @@ bool sanity_test_fdelt() { fd_set fds; FD_ZERO(&fds); - FD_SET(0, &fds); + FD_SET(FD_SETSIZE, &fds); return FD_ISSET(0, &fds); } #endif ``` ```bash 0000000000399d20 <sanity_test_fdelt()>: 399d20: 48 81 ec 98 00 00 00 sub $0x98,%rsp 399d27: b9 10 00 00 00 mov $0x10,%ecx 399d2c: 64 48 8b 04 25 28 00 mov %fs:0x28,%rax 399d33: 00 00 399d35: 48 89 84 24 88 00 00 mov %rax,0x88(%rsp) 399d3c: 00 399d3d: 31 c0 xor %eax,%eax 399d3f: 48 89 e7 mov %rsp,%rdi 399d42: fc cld 399d43: f3 48 ab rep stos %rax,%es:(%rdi) 399d46: 48 c7 c7 ff ff ff ff mov $0xffffffffffffffff,%rdi 399d4d: e8 3e ff ff ff callq 399c90 <__fdelt_warn> 399d52: 0f b6 04 24 movzbl (%rsp),%eax 399d56: 83 e0 01 and $0x1,%eax 399d59: 48 8b 94 24 88 00 00 mov 0x88(%rsp),%rdx 399d60: 00 399d61: 64 48 33 14 25 28 00 xor %fs:0x28,%rdx 399d68: 00 00 399d6a: 75 08 jne 399d74 <sanity_test_fdelt()+0x54> 399d6c: 48 81 c4 98 00 00 00 add $0x98,%rsp 399d73: c3 retq 399d74: e8 77 df c8 ff callq 27cf0 <__stack_chk_fail@plt> 399d79: 0f 1f 80 00 00 00 00 nopl 0x0(%rax) ``` ```bash src/bitcoind *** buffer overflow detected ***: src/bitcoind terminated Aborted ``` I think the test should should be removed and replaced (if possible) with additional checks in security-check.py. I was thinking about adding a version of [this script](https://github.com/fanquake/core-review/blob/master/fortify.py) as part of the output, but that needs more thought. I'll address this in a follow up. ACKs for top commit: laanwj: ACK df6bde031b24112abf3a94337a2c096698acde6e Tree-SHA512: d8b3af4f4eb2d6c767ca6e72ece51d0ab9042e1bbdfcbbdb7ad713414df21489ba3217662b531b8bfdac0265d2ce5431abfae6e861b6187d182ff26c6e59b32d
2020-05-13Merge #18814: rpc: Relock wallet only if most recent callbackfanquake
9f59dde9740d065118bdddde75ef9f4e4603a7b1 rpc: Relock wallet only if most recent callback (João Barbosa) a2e6db5c4f1bb52a8814102b628e51652493d06a rpc: Add mutex to guard deadlineTimers (João Barbosa) Pull request description: This PR fixes an early relocking race condition from #18811 where old relock callback runs after new wallet unlock code and nRelockTime update but before rpcRunLater call, causing early relock and incorrect nRelockTime time Issue introduced in #18487. Fixes #18811. ACKs for top commit: MarcoFalke: ACK 9f59dde9740d065118bdddde75ef9f4e4603a7b1 ryanofsky: Code review ACK 9f59dde9740d065118bdddde75ef9f4e4603a7b1. No changes since last review except squashing commits. jonatack: ACK 9f59dde9740d065118bdddde75ef Tree-SHA512: 2f7fc03e5ab6037337f2d82dfad432495cc337c77d07c968ee2355105db6292f24543c03456f5402e0e759577a4327758f9372f7ea29de6d56dc3695fda9b379
2020-05-13Merge #18894: gui: Fix manual coin control with multiple wallets loadedJonas Schnelli
a8b5f1b133d4f23975a3fbfb7a415b17261466ee gui: Fix manual coin control with multiple wallets loaded (João Barbosa) Pull request description: This PR ensures each loaded wallet has a dedicated coin control in the send view which is manipulated by the coin control dialog. This is an alternative to #17457. Two main differences are: - scope reduced - no unnecessary changes unrelated to the fix; - approach taken - coin control instance now belongs to the send view. All problems raised in #17457 reviews no longer apply due to the approach taken - https://github.com/bitcoin/bitcoin/pull/17457#pullrequestreview-319297589 and https://github.com/bitcoin/bitcoin/pull/17457#issuecomment-555920829) No change in behavior if only one wallet is loaded. Closes #15725. ACKs for top commit: jonasschnelli: utACK a8b5f1b133d4f23975a3fbfb7a415b17261466ee ryanofsky: Code review ACK a8b5f1b133d4f23975a3fbfb7a415b17261466ee. Code changes are very straightforward, just replacing global CCoinControl object with SendCoinsDialog member. Not sure if this means coin control settings are reset between payments. It would be good to note in the PR description or release notes if single wallet behavior is affected hebasto: ACK a8b5f1b133d4f23975a3fbfb7a415b17261466ee Sjors: tACK a8b5f1b133d4f23975a3fbfb7a415b17261466ee Tree-SHA512: 3ad9c51bab6f28ec0e90efbd6f43fa510c81dafb2eff0b8c3724efcee3e030054a10be013e27cefe35763374c5f6d7af8c02658736964f733d7e38b646b5df65
2020-05-13Merge #18578: gui: Fix leak in CoinControlDialog::updateViewJonas Schnelli
e8123eae40eb264bbb71007d0eb074901f0e2fe5 gui: Fix itemWalletAddress leak when not tree mode (João Barbosa) Pull request description: Taken from #17457, the first commit is a similar to 88a94f7bb8ba2b0257315d70717f9af928ca6561 but for test binary, and the second commit fixes a leak where `CCoinControlWidgetItem` are unnecessarily created and leaked. ACKs for top commit: jonasschnelli: utACK e8123eae40eb264bbb71007d0eb074901f0e2fe5 hebasto: ACK e8123eae40eb264bbb71007d0eb074901f0e2fe5, tested on Linux Mint 19.3. Tree-SHA512: 8b43cb29de103842ce5f048de51222919540d3212d2873c16731145e856178644041924ad0e9a58c2ff08f209a9b4ac26dc9965289eb719da233c0984f93631e
2020-05-13Merge #16710: build: Enable -Wsuggest-override if availablefanquake
839add193b13c17a40f42ff69d973caeb800d3f2 build: Enable -Wsuggest-override (Hennadii Stepanov) de5e91c3034f320f84ee0308a3c31659635d136a refactor: Add BerkeleyDatabaseVersion() function (Hennadii Stepanov) Pull request description: From GCC [docs](https://gcc.gnu.org/onlinedocs/gcc-9.2.0/gcc/Warning-Options.html): > `-Wsuggest-override` > Warn about overriding virtual functions that are not marked with the override keyword. ~This PR is based on #16722 (the first commit).~ See: https://github.com/bitcoin/bitcoin/pull/16722#issuecomment-584111086 ACKs for top commit: fanquake: ACK 839add193b13c17a40f42ff69d973caeb800d3f2 vasild: ACK 839add193 practicalswift: ACK 839add193b13c17a40f42ff69d973caeb800d3f2 assuming Travis is happy: patch looks correct Tree-SHA512: 1e8cc085da30d41536deff9b181962c1882314ab252c2ad958294087ae1e5a0dfa4886bdbe36f21cf6ae71df776a8420f349f007d4b5b49fd79ba98ce308965a
2020-05-12Do not answer GETDATA for to-be-announced txPieter Wuille
2020-05-12[net processing] Only send a getheaders for one block in an INVJohn Newbery
Headers-first is the primary method of announcement on the network. If a node fell back sending blocks by inv, it's probably for a re-org. The final block hash provided should be the highest, so send a getheaders and then fetch the blocks we need to catch up.
2020-05-12Push down use of cs_main into FindTxForGetDataPieter Wuille
2020-05-12Abstract logic to determine whether to answer tx GETDATAPieter Wuille
2020-05-12build: Enable -Wsuggest-overrideHennadii Stepanov
2020-05-12Merge #18910: p2p: add MAX_FEELER_CONNECTIONS constantfanquake
e3047edfb63c3d098cb56ba9f9a1e7e0a795d552 test: use p2p constants in denial of service tests (fanquake) 25d8264c95eaf98a66df32addb0bf32d795a35bd p2p: add MAX_FEELER_CONNECTIONS constant (tryphe) Pull request description: Extracted from #16003. ACKs for top commit: naumenkogs: utACK e3047ed Tree-SHA512: 14fc15292be4db2e825a0331dd189a48713464f622a91c589122c1a7135bcfd37a61e64af1e76d32880ded09c24efd54d3c823467d6c35367a380e0be33bd35f
2020-05-12Merge #18877: Serve cfcheckpt requestsMarcoFalke
23083856a551ca13e8b142791c296ecb25cc4e7f [test] Add test for cfcheckpt (Jim Posen) f9e00bb25ac4039056808affeb5ffa86a2c317fe [net processing] Message handling for getcfcheckpt. (Jim Posen) 9ccaaba11e94571fe984857494042ac292c17156 [init] Add -peerblockfilters option (Jim Posen) Pull request description: Serve cfcheckpt messages if basic block filter index is enabled and `-peercfilters` is set. `NODE_COMPACT_FILTERS` is not signaled to peers, but functionality can be used for testing and serving pre-configured clients. ACKs for top commit: jonatack: Code review re-ACK 23083856a551ca13e8b142791c296ecb25cc4e7f the only change since my review @ 967e2b1 is an update required for #16224 that was merged yesterday. fjahr: re-ACK 23083856a551ca13e8b142791c296ecb25cc4e7f jkczyz: re-ACK 23083856a551ca13e8b142791c296ecb25cc4e7f ariard: re-Code Review ACK 2308385 clarkmoody: Tested ACK 23083856a MarcoFalke: re-ACK 23083856a5 🌳 theStack: ACK https://github.com/bitcoin/bitcoin/commit/23083856a551ca13e8b142791c296ecb25cc4e7f Tree-SHA512: 8c751bbd7d1c31a413096462ae025c3d2f3163c7016cbec472a5f5ec267f8dd19a2dfc4d749876d7409c1db546e6fdd16461c6863effcfa0d3e993edcfa92a08
2020-05-12test: use p2p constants in denial of service testsfanquake
2020-05-12p2p: add MAX_FEELER_CONNECTIONS constanttryphe
2020-05-12Merge #18931: net: use CMessageHeader::HEADER_SIZE, add missing includefanquake
83da576f4416c64b5d520819208a722b2273739a net: use CMessageHeader::HEADER_SIZE, add missing include (Jon Atack) Pull request description: as suggested 16 months ago by Gleb Naumenko in https://github.com/bitcoin/bitcoin/pull/15197#issuecomment-456181865. `static constexpr CMessageHeader::HEADER_SIZE` is already used in this file, `src/net.cpp`, in 2 instances. This commit replaces the remaining 2 integer values in the file with it and adds the explicit include header. Co-authored by: Gleb Naumenko <naumenko.gs@gmail.com> ACKs for top commit: naumenkogs: utACK 83da576 practicalswift: ACK 83da576f4416c64b5d520819208a722b2273739a -- patch looks correct theStack: ACK 83da576f4416c64b5d520819208a722b2273739a -- verified that its just magic number elimination refactoring and additionally checked that all tests pass :+1: Tree-SHA512: 5b915483bca4ea162c259865a1b615d73b88a1b1db3f82db05f770d10b8a42494d948f5b21badbcce2d9efa5915b8cbb6af83073867c23d2f152c0d35ac37b96
2020-05-12Merge #18808: [net processing] Drop unknown types in getdatafanquake
9847e205bf7edcac4c30ce4b6d62f482aa7bc1b7 [docs] Improve commenting in ProcessGetData() (John Newbery) 2f032556e08a04807c71eb02104ca9589eaadf1b [test] test that an invalid GETDATA doesn't prevent processing of future messages (Amiti Uttarwar) e257cf71c851e25e1a533bf1d4296f6b55c81332 [net processing] ignore unknown INV types in GETDATA messages (Amiti Uttarwar) 047ceac142246b5d51056a51dbf4645b31802be4 [net processing] ignore tx GETDATA from blocks-only peers (Amiti Uttarwar) Pull request description: Currently we'll stall peers that send us an unknown INV type in a GETDATA message. Be a bit more friendly and just drop the invalid request. Ditto for blocks-relay-only peers that send us a GETDATA for a transaction. There's a test for the first part. The second is difficult to test in the functional test framework since we aren't able to make blocks-relay-only connections. ACKs for top commit: sipa: utACK 9847e205bf7edcac4c30ce4b6d62f482aa7bc1b7 brakmic: ACK https://github.com/bitcoin/bitcoin/commit/9847e205bf7edcac4c30ce4b6d62f482aa7bc1b7 luke-jr: utACK 9847e205bf7edcac4c30ce4b6d62f482aa7bc1b7 naumenkogs: utACK 9847e20 ajtowns: utACK 9847e205bf7edcac4c30ce4b6d62f482aa7bc1b7 Tree-SHA512: 6007f2fd839ffe737727f6fb8e8f083b2d9e05a510748f1d40b8f9be8fdf7b5419a36d8f1039923eec1ba2983e8f6f0436ec5fc196d9f6dcb0657f2ff8ff8e4c
2020-05-11refactor: Add BerkeleyDatabaseVersion() functionHennadii Stepanov
2020-05-11Merge #18914: refactor: Apply override specifier consistentlyMarcoFalke
d044e0ec7d37bbcdf10bbdb903b9119741c7297d refactor: Remove override for final overriders (Hennadii Stepanov) 1551cea2d52cac403ff506a7cc955d8de8fd6f3e refactor: Use override for non-final overriders (Hennadii Stepanov) Pull request description: Two commits are split out from #16710 to make reviewing [easier](https://github.com/bitcoin/bitcoin/pull/16710#issuecomment-625760894). From [C++ FAQ](https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines.html#c128-virtual-functions-should-specify-exactly-one-of-virtual-override-or-final): > C.128: Virtual functions should specify exactly one of virtual, override, or final > **Reason** Readability. Detection of mistakes. Writing explicit `virtual`, `override`, or `final` is self-documenting and enables the compiler to catch mismatch of types and/or names between base and derived classes. However, writing more than one of these three is both redundant and a potential source of errors. ACKs for top commit: practicalswift: ACK d044e0ec7d37bbcdf10bbdb903b9119741c7297d: consistent use of `override` prevents bugs + patch looks correct + Travis happy MarcoFalke: ACK d044e0ec7d37bbcdf10bbdb903b9119741c7297d, based on my understanding that adding `override` or `final` to a function must always be correct, unless it doesn't compile!? vasild: ACK d044e0ec7 Tree-SHA512: 245fd9b99b8b5cbf8694061f892cb3435f3378c97ebed9f9401ce86d21890211f2234bcc39c9f0f79a4d2806cb31bf8ce41a0f9c2acef4f3a2ac5beca6b077cf
2020-05-11rpcwallet: Replace pwallet-> with wallet.MarcoFalke
pwallet is never null everywhere where it is dereferenced, so simply replace it with a reference, which can not be null by definition.
2020-05-11rpcwallet: Replace boost::optional<T>::emplace with simple assignment of T{}MarcoFalke
Optional::emplace() was only added in boost 1.56, see https://github.com/boostorg/optional/commit/2e583aaf30964afd66c1284aa71d0d8b42fa440d To simply work around https://github.com/bitcoin/bitcoin/issues/18943, replace it with assignment of T{}
2020-05-11Merge #18216: test, build: Enable -Werror=sign-comparefanquake
68537275bd91d1dc14a69609ae443f955bfdbd64 build: Enable -Werror=sign-compare (Ben Woosley) eac6a3080d38cfd4eb7204ecd327df213958e51a refactor: Rework asmap Interpret to avoid ptrdiff_t (Ben Woosley) df37377e30678ac9b8338ea920e50b7296da6bd5 test: Fix outstanding -Wsign-compare errors (Ben Woosley) Pull request description: Disallowing sign-comparison mismatches can help to prevent the introduction of overflow and interpretation bugs. In this case, ~all~ most existing violations are in the tests, and most simply required annotating the literal as unsigned for comparison. This was previously prevented by violations in leveldb which were fixed upstream and merged in #17398. You can test that by building this branch against: 22d11187ee3c7abfe9d43c9eb68f102498cc2b9a vs 75fb37ce68289eb7e00e2ccdd2ef7f9271332545 ACKs for top commit: fjahr: re-ACK 68537275bd91d1dc14a69609ae443f955bfdbd64 practicalswift: ACK 68537275bd91d1dc14a69609ae443f955bfdbd64 Tree-SHA512: 14b5daa38c496fb51548feb30fb4dd179e6f76a8d355f52bc8e2a18f2f9340f0bc98dcf36d8b3d6521045d013891c3103749a4eda88ceef00202a6a0cf93f73c
2020-05-10util: Drop OpOriginal() and OpTranslated()Hennadii Stepanov
The current implementation of the Join() allows do not use OpOriginal() and OpTranslated() unary operators at all.
2020-05-10gui: Do not translate InitWarning messages in debug.logHennadii Stepanov
2020-05-10util: Enhance Join()Hennadii Stepanov
2020-05-10net: use CMessageHeader::HEADER_SIZE, add missing includeJon Atack
static constexpr CMessageHeader::HEADER_SIZE is already used in this file, src/net.cpp, in 2 instances. This commit replaces the remaining 2 integer values with it and adds the explicit include header. Co-authored by: Gleb Naumenko <naumenko.gs@gmail.com>
2020-05-09util: Enhance bilingual_strHennadii Stepanov
2020-05-09fuzz: use std::optional for sep_pos variableHarris
Co-authored-by: MarcoFalke <falke.marco@gmail.com>
2020-05-09refactor: Rework asmap Interpret to avoid ptrdiff_tBen Woosley
2020-05-08[net processing] Message handling for getcfcheckpt.Jim Posen
If -peerblockfilters is configured, handle requests for cfcheckpt.
2020-05-08[init] Add -peerblockfilters optionJim Posen
When a node is configured with --blockfilterindex=basic and -peerblockfilters it can serve compact block filters to its peers. This commit adds the configuration option handling. Future commits add compact block serving and service bits signaling.
generated by cgit v1.2.3 (git 2.26.2) at 2024-11-05 21:30:09 +0000