aboutsummaryrefslogtreecommitdiff
path: root/src
AgeCommit message (Collapse)Author
2020-07-11Merge #19222: tests: Add fuzzing harness for BanManMarcoFalke
97846d7f5b47ef77469b9f961db77f770e8bcc0f tests: Add fuzzing harness for BanMan (practicalswift) deba199f1c88c2e5f754b0a4ec43b9ef28de8352 tests: Add ConsumeSubNet(...). Move and increase coverage in ConsumeNetAddr(...). (practicalswift) Pull request description: Add fuzzing harness for `BanMan`. 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: f4126c15bbb77638833367d73f58193c8f05d16bed0b1d6c33b39387d5b610ff34af78cd721adb51778062ce3ac5e79756d1c3895ef54c6c80c61dcf056e94ff
2020-07-11Merge #19474: doc: Use precise permission flags where possibleMarcoFalke
fab558612278909df93bdf88f5727b04f13aef0f doc: Use precise permission flags where possible (MarcoFalke) Pull request description: Instead of mentioning the all-encompassing `-whitelist*` settings, change the docs to mention the exact permission flag that will influence the behaviour. This is needed because in the future, the too-broad `-whitelist*` settings (they either include *all* permission flags or apply to *all* peers) might be deprecated to require the permission flags to be enumerated. Alternatively, in the future there could be an RPC to set the net permission flags on an existing connection, in which case the `-whitelist*` terminology is of no help. ACKs for top commit: jnewbery: reACK fab558612278909df93bdf88f5727b04f13aef0f fjahr: Code review ACK fab558612278909df93bdf88f5727b04f13aef0f jonatack: ACK fab558612278909df93bdf88f5727b04f13aef0f Tree-SHA512: c7dea3e577d90103bb2b0ffab7b7c8640b388932a3a880f69e2b70747fc9213dc1f437085671fd54c902ec2a578458b8a2fae6dbe076642fb88efbf9fa9e679c
2020-07-10Merge #19140: tests: Avoid fuzzer-specific nullptr dereference in libevent ↵MarcoFalke
when handling PROXY requests 20d31bdd92cc2ad9b8d26ed80da73bbcd6016144 tests: Avoid fuzzer-specific nullptr dereference in libevent when handling PROXY requests (practicalswift) Pull request description: Avoid constructing requests that will be interpreted by libevent as PROXY requests to avoid triggering a `nullptr` dereference. Split out from #19074 as suggested by MarcoFalke. The dereference (`req->evcon->http_server`) takes place in `evhttp_parse_request_line` and is a consequence of our hacky but necessary use of the internal function `evhttp_parse_firstline_` in the `http_request` fuzzing harness. The suggested workaround is not aesthetically pleasing, but it successfully avoids the troublesome code path. `" http:// HTTP/1.1\n"` was a crashing input prior to this workaround. Before this PR: ``` $ echo " http:// HTTP/1.1" > input $ src/test/fuzz/http_request input src/test/fuzz/http_request: Running 1 inputs 1 time(s) each. Running: input AddressSanitizer:DEADLYSIGNAL ================================================================= ==27905==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000108 (pc 0x55a169b7e053 bp 0x7ffd452f1160 sp 0x7ffd452f10e0 T0) ==27905==The signal is caused by a READ memory access. ==27905==Hint: address points to the zero page. #0 0x55a169b7e053 in evhttp_parse_request_line depends/work/build/x86_64-pc-linux-gnu/libevent/2.1.11-stable-36daee64dc1/http.c:1883:37 #1 0x55a169b7d9ae in evhttp_parse_firstline_ depends/work/build/x86_64-pc-linux-gnu/libevent/2.1.11-stable-36daee64dc1/http.c:2041:7 #2 0x55a1687f624e in test_one_input(std::vector<unsigned char, std::allocator<unsigned char> > const&) src/test/fuzz/http_request.cpp:51:9 … $ echo $? 1 ``` After this PR: ``` $ echo " http:// HTTP/1.1" > input $ src/test/fuzz/http_request input src/test/fuzz/http_request: Running 1 inputs 1 time(s) each. Running: input Executed input in 0 ms *** *** NOTE: fuzzing was not performed, you have only *** executed the target code on a fixed set of inputs. *** $ echo $? 0 ``` 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: 7a6b68e52cbcd6c117487e74e47760fe03566bec09b0bb606afb3b652edfd22186ab8244e8e27c38cef3fd0d4a6c237fe68b2fd22e0970c349e4ab370cf3e304
2020-07-10Merge #19453: refactor: reduce DefaultRequestHandler memory allocationsMarcoFalke
f20b359bb9dabc7be11c3e3319e435aa42a8f0f5 cli: reduce DefaultRequestHandler memory allocations (Jon Atack) Pull request description: per https://github.com/bitcoin/bitcoin/pull/16439#discussion_r443957125. Simpler code, fewer allocations. No change of behavior. The code has good test coverage in `interface_bitcoin_cli.py`. ACKs for top commit: MarcoFalke: review ACK f20b359bb9dabc7be11c3e3319e435aa42a8f0f5 fjahr: Code review ACK f20b359 Tree-SHA512: 745eab44dfdcc485ca2bbc1db8b8d364cbd3cf94982e46e033745ce05ab617c15320ee55da7fb930d365f4d26b172049d5f5efcf0b6d3af5b0a28185bdb93ea8
2020-07-10Merge #14033: p2p: Drop CADDR_TIME_VERSION checks now that ↵MarcoFalke
MIN_PEER_PROTO_VERSION is greater 57b0c0a93a243769beb306c89560d1eda61f54bd Drop CADDR_TIME_VERSION checks now that MIN_PEER_PROTO_VERSION is greater (Ben Woosley) Pull request description: We do not connect to peers older than 31800 ACKs for top commit: sipa: Code reivew ACK 57b0c0a93a243769beb306c89560d1eda61f54bd jnewbery: Code review ACK 57b0c0a93a243769beb306c89560d1eda61f54bd vasild: ACK 57b0c0a9 Tree-SHA512: e1ca7c9203cbad83ab7c7a2312777ad07ed6a16119169b256648b8a8738c260a5168acdd4fb33f6e4b17f51ec7e033e110b76bde55b4e3b2d444dc02c01bc2b1
2020-07-10Merge #18638: net: Use mockable time for ping/pong, add testsMarcoFalke
fa3365430c5fb57d7c0b5f2bce9fbbe290be93c3 net: Use mockable time for ping/pong, add tests (MarcoFalke) faab4aaf2fa1153c6d76efc8113fa01b06943ece util: Add count_microseconds helper (MarcoFalke) Pull request description: Switch `CNode::m_ping_start` and `CNetMessage::m_time` to mockable time, so that tests can be added. Mockable time is also type-safe, since it uses `std::chrono` ACKs for top commit: jonatack: Code review re-ACK fa33654 re-read code, verified rebase per `git range-diff 4b5c919 fa94d6f fa33654`, previous tested ACKs still valid troygiorshev: ACK fa3365430c5fb57d7c0b5f2bce9fbbe290be93c3 Tree-SHA512: 7d632bd6019ce7c882029e71b667a61517e783af82755a85dd979ef09380934e172dec8b8f91d57b200a30a6e096aeaf01f19fee7f3aed0e0e871c72eb44d70e
2020-07-10doc: Use precise permission flags where possibleMarcoFalke
2020-07-10Merge #19469: rpc: deprecate banscore field in getpeerinfofanquake
41d55d30579358c805036201664ad6a1c1d48681 doc: getpeerinfo banscore deprecation release note (Jon Atack) dd54e3796e633cfdf6954af306afd26eadc25116 test: getpeerinfo banscore deprecation test (Jon Atack) 8c7647b3fbbab03ea84071cf3cd2d0d2bf8be255 rpc: deprecate banscore field in rpc getpeerinfo (Jon Atack) Pull request description: Per https://github.com/bitcoin/bitcoin/pull/19219#discussion_r443074487 and https://github.com/bitcoin/bitcoin/pull/19219#issuecomment-652699592, this PR deprecates returning the `banscore` field in the `getpeerinfo` RPC, updates the help, adds a test, and updates the release notes. Related to #19464. ACKs for top commit: fanquake: ACK 41d55d30579358c805036201664ad6a1c1d48681 Tree-SHA512: 8eca08332581e2fe191a2aafff6ba89ce39413f0491ed0de8b86577739f0ec430b1a8fbff2914b0f3138a229563dfcc1981c0cf5b7dd6061b5c48680a28423bc
2020-07-09Merge #19283: refactor: Remove unused BlockAssembler::pblock member varMarcoFalke
fa6d5ab67444013f9db696cca2257c871e002594 refactor: Remove unused BlockAssembler::pblock member var (MarcoFalke) Pull request description: It seems odd to have a confusing and fragile "convenience pointer" member variable to be able to write `pblock->vtx` instead of `pblocktemplate->block.vtx` in a single place. ACKs for top commit: promag: Code review ACK fa6d5ab67444013f9db696cca2257c871e002594. Tree-SHA512: e9f032b5ab702dbefffd370db3768ebfb95c13acc732972b695281ea34c91d70cd0a1700bc2c6f106dbc9de68e81bc6bb06c68c2afd53c17cba8ebee4f9931b9
2020-07-09Merge #19191: net: Extract download permission from nobanMarcoFalke
fa0540cd46eaf44d9e1a9f91c3a937986826c4fa net: Extract download permission from noban (MarcoFalke) Pull request description: It should be possible to grant nodes in a local network (e.g. home, university, enterprise, ...) permission to download blocks even after the maxuploadtarget is hit. Currently this is only possible by setting the `noban` permission, which has some adverse effects, especially if the peers can't be fully trusted. Fix this by extracting a `download` permission from `noban`. ACKs for top commit: jonatack: ACK fa0540c Sjors: re-utACK fa0540cd46eaf44d9e1a9f91c3a937986826c4fa Tree-SHA512: 255566baa43ae925d93f5d0a3aa66b475a556d1590f662a88278a4872f16a1a05739a6119ae48a293011868042e05cb264cffe5822a50fb80db7333bf44376d9
2020-07-09Merge #19314: refactor: Use uint16_t instead of unsigned shortWladimir J. van der Laan
1cabbddbca615b26aa4510c75f459c28d6fe0afd refactor: Use uint16_t instead of unsigned short (Aaron Hook) Pull request description: I wanted to see if the `up for grabs` label works and looked at PR #17822 originally opend by ahook I saw it had many acks for example by jonatack and practicalswift but needed rebasing. So I checked out the remote branch rebased it resolved three conflicts and continued the rebase. Hope everything is as expected (: ACKs for top commit: sipsorcery: ACK 1cabbddbca615b26aa4510c75f459c28d6fe0afd. practicalswift: ACK 1cabbddbca615b26aa4510c75f459c28d6fe0afd -- patch looks correct :) laanwj: ACK 1cabbddbca615b26aa4510c75f459c28d6fe0afd hebasto: ACK 1cabbddbca615b26aa4510c75f459c28d6fe0afd, I have reviewed the code and it looks OK, I agree it can be merged. Tree-SHA512: 0e6bf64f274aae5dacb188358b4d5f65ccb207d4f70922f039bc4ed7934709418ddad19f8bfb7462517427837c3d2bb3f86ef284bb40e87119aad2a1e148d9d6
2020-07-09Merge #18993: qt: increase console command max lengthfanquake
fc6a637a013daeb14b2f93652d7f494f3b8462aa qt: increase console command max length (10xcryptodev) Pull request description: fix #17618 Tested the examples https://github.com/bitcoin/bitcoin/issues/17618#issuecomment-559538070 and works ACKs for top commit: MarcoFalke: Approach ACK fc6a637a013daeb14b2f93652d7f494f3b8462aa hebasto: ACK fc6a637a013daeb14b2f93652d7f494f3b8462aa, I have reviewed the code and it looks OK, I agree it can be merged. Tree-SHA512: 4975d7fa4c13a6b0f50f5754c3e04eb5a42b1411c385dc883d9948b6fc0dee38900ba2a418218a9a30ce39988a27d22f3ff3a02f0fa44f4136f01eef473efeca
2020-07-09Merge #19317: Add a left-justified width field to log2_work component for a ↵Wladimir J. van der Laan
uniform debug.log output c8583022800410afeb75e0154df7290d080d581d Change format of log2_work for uniform output (zero-padded) (jmorgan) Pull request description: Motivation: It's jarring to watch the output of `tail -f ~/btcdata/debug.log` scroll by and very frequently see columns not lining up correctly because `log2_work` somtimes has less precision than 8 digits. Current display: ``` 2020-06-18T02:54:42Z UpdateTip: new best=0000000000000000107f877e4920643f9fb06090fa7551cd1cdd83b857f520aa height=382038 version=0x00000003 log2_work=83.558653 tx=90953616 date='2015-11-04T17:11:44Z' progress=0.166675 cache=117.6MiB(966410txo) 2020-06-18T02:54:51Z UpdateTip: new best=0000000000000000019a4de585d30d1a8cc13c7a1972d11b4945635c9556acb5 height=382039 version=0x00000003 log2_work=83.55868 tx=90955936 date='2015-11-04T17:19:39Z' progress=0.166679 cache=117.9MiB(968799txo) ``` Display with this commit: ``` 2020-06-18T02:54:42Z UpdateTip: new best=0000000000000000107f877e4920643f9fb06090fa7551cd1cdd83b857f520aa height=382038 version=0x00000003 log2_work=83.558653 tx=90953616 date='2015-11-04T17:11:44Z' progress=0.166675 cache=117.6MiB(966410txo) 2020-06-18T02:54:51Z UpdateTip: new best=0000000000000000019a4de585d30d1a8cc13c7a1972d11b4945635c9556acb5 height=382039 version=0x00000003 log2_work=83.55868 tx=90955936 date='2015-11-04T17:19:39Z' progress=0.166679 cache=117.9MiB(968799txo) ``` ACKs for top commit: practicalswift: ACK c8583022800410afeb75e0154df7290d080d581d -- patch looks great :) achow101: ACK c8583022800410afeb75e0154df7290d080d581d laanwj: Tested ACK c8583022800410afeb75e0154df7290d080d581d Tree-SHA512: 16cbe419c4993ad51019c676e8ca409ef1025b803cc598437c780dd7ca003d7e4ad421f451e9a374e0070ee9b3ee601b7aba849e1f346798f9321d1bce5c4401
2020-07-09Merge #19468: refactor: Drop unused CDBWrapper methodsWladimir J. van der Laan
4b5ac258812a1e8848862689ff333587cf274892 Drop unused CDBWrapper methods (Hennadii Stepanov) Pull request description: `CDBWrapper::Flush()` and `CDBWrapper::Sync()` are not used in the code. ACKs for top commit: promag: ACK 4b5ac258812a1e8848862689ff333587cf274892. laanwj: ACK 4b5ac258812a1e8848862689ff333587cf274892 Tree-SHA512: 06115c59e75995d496173a64ceea1b9bb1b4fe3eac8bf4f59df68b87b112b5b3e8065298dcd5c4c7408544f76ee62922325acc2208619d830fd5dbb420cdda5c
2020-07-09Merge #18896: qt: Reset toolbar after all wallets are closedfanquake
1e9bfd4926a3cbb9db877c773a60b0bbbbe8bde0 qt: Reset toolbar after all wallets are closed (Hennadii Stepanov) Pull request description: If the last open wallet is closed from the non-"Overview" tab, that tab remains active when a new wallet is opened: ![Screenshot from 2020-05-06 09-00-26](https://user-images.githubusercontent.com/32963518/81142394-46821880-8f78-11ea-84b5-1d02f2b7f3f1.png) This PR fixes this bug. ACKs for top commit: promag: Code review ACK 1e9bfd4926a3cbb9db877c773a60b0bbbbe8bde0. luke-jr: utACK 1e9bfd4926a3cbb9db877c773a60b0bbbbe8bde0 jonasschnelli: utACK 1e9bfd4926a3cbb9db877c773a60b0bbbbe8bde0 Tree-SHA512: a8dfd7591267e9544ad40c87581d554e5cfaad4b2a5bbfdbaf2596dc6869d2ac6cf7877adfef3d528fc61b081d40c6e30d787bbd7280ef7946aa7f7d9bc8b18e
2020-07-09Merge #19470: banlist: log post-swept banlist size at startupfanquake
0b8ba84659b56b12d74587ea31b6062fce887ba3 banlist: log post-swept banlist size at startup (fanquake) Pull request description: We are currently logging the size of the banlist from before `SweepBanned()` has been called, meaning the value may be incorrect. i.e banlist.dat had `1`ban. That ban is swept on startup. We log "loaded 1 banned node..". Actual banlist size is `0`. ACKs for top commit: jonatack: Code review ACK 0b8ba84659b `m_banned` is set in SetBanned and is updated by SweepBanned before the logging. laanwj: Code review ACK 0b8ba84659b56b12d74587ea31b6062fce887ba3 jnewbery: Code review ACK 0b8ba84659b56b12d74587ea31b6062fce887ba3 Tree-SHA512: 1d6e363d6c68d7cc214dd685df3d2d27572f6a58a4c0e43c03cfbb03bc01badb6a10ecae403d137094bb316d27f33feb6be15b4e23ef1e9496cd0b3c23c21698
2020-07-09Merge #19454: tools: `.clang-format` compat with clang versions < 9fanquake
b9253c7d2089d3d159dcc10118ce5a219d9a6881 tools: clang-format 6 compatibility (Jon Atack) Pull request description: Our `.clang-format` settings inadvertently lost compatibility with Clang versions < 9 in #19095, including for Debian stable. This patch returns compatibility in the interim until the distros update. See discussion from https://github.com/bitcoin/bitcoin/pull/19095#issuecomment-651926138. ACKs for top commit: MarcoFalke: Approach ACK b9253c7d2089d3d159dcc10118ce5a219d9a6881 , haven't tested Tree-SHA512: 4af541a195f48d84ffb80e23aaefb624c66bc78f087c8d92b4af5a654420b69fedf25272c6e4fde2688ff88412d306b7a990ce1e15d8b24180374c625a253fb6
2020-07-09net: Extract download permission from nobanMarcoFalke
2020-07-09refactor: clean up PeriodicFlush()John Newbery
2020-07-09Merge #19320: wallet: Replace CDataStream& with CDataStream&& where appropriateMarcoFalke
fa8a341b88cabfd7f8d702db7cb9972b0804bf2a wallet: Replace CDataStream& with CDataStream&& where appropriate (MarcoFalke) fa021e9a5b7e930a3db0febb416942dea3a90a8f wallet: Remove confusing double return value ret+success (MarcoFalke) Pull request description: The keys and values are only to be used once because their memory is set to zero. Make that explicit by moving the bytes into the lower level methods. ACKs for top commit: sipa: utACK fa8a341b88cabfd7f8d702db7cb9972b0804bf2a ryanofsky: Code review ACK fa8a341b88cabfd7f8d702db7cb9972b0804bf2a. Nice changes. Tree-SHA512: 5c0218bae0f3cd2a07346f1bbf4ad232e5dde7ef2f807d82cc6cfd208d11fe60c8b0f37e7986087b52fbfc79cdfd33c3c8a5822b3d4d9a44d1c6b09e354fc424
2020-07-08Merge #19347: [net] Make cs_inventory nonrecursiveMarcoFalke
e8a2822119233ade0de84f791a9e92918a3d6896 [net] Don't try to take cs_inventory before deleting CNode (John Newbery) 3556227ddd3365cfac43b307204d73058b2943f0 [net] Make cs_inventory a non-recursive mutex (John Newbery) 344e831de54f7b864f03a90f6cb19692eafcd463 [net processing] Remove PushBlockInventory and PushBlockHash (John Newbery) Pull request description: - Remove PushBlockInventory() and PushBlockHash(). These are one-line functions that can easy be inlined into the calling code. Doing so also allows us to eliminate the one place that cs_inventory is recursively locked. - Make cs_inventory a nonrecursive mutex - Remove a redundant TRY_LOCK of cs_inventory when deleting CNode. ACKs for top commit: sipa: utACK e8a2822119233ade0de84f791a9e92918a3d6896 MarcoFalke: ACK e8a2822119233ade0de84f791a9e92918a3d6896 🍬 hebasto: re-ACK e8a2822119233ade0de84f791a9e92918a3d6896 Tree-SHA512: dbc721d102cdef7b5827a8f2549daf8b54f543050266999a7ea56c9f36618565b71e31ce0beb1209ba2db43d15388be173355a03fb6db8ad24e2475b145050bd
2020-07-08banlist: log post-swept banlist size at startupfanquake
We are currently logging the size of the banlist before SweepBanned() has been called, meaning the value may be incorrect.
2020-07-08Drop unused CDBWrapper methodsHennadii Stepanov
2020-07-08rpc: deprecate banscore field in rpc getpeerinfoJon Atack
2020-07-08tests: Add fuzzing harness for BanManpracticalswift
2020-07-08tests: Add ConsumeSubNet(...). Move and increase coverage in ↵practicalswift
ConsumeNetAddr(...).
2020-07-07Merge #19219: Replace automatic bans with discouragement filterPieter Wuille
2ad58381fffb33d611abf900b73d9e6b5a4e35f8 Clean up separated ban/discourage interface (Pieter Wuille) b691f2df5f7d443c0c9ee056ab94aa0fc19566d5 Replace automatic bans with discouragement filter (Pieter Wuille) Pull request description: This patch improves performance and resource usage around IP addresses that are banned for misbehavior. They're already not actually banned since #14929, as connections from them are still allowed, but they are preferred for eviction if the inbound connection slots are full. Stop treating these like manually banned IP ranges, and instead just keep them in a rolling Bloom filter of misbehaving nodes, which isn't persisted to disk or exposed through the ban framework. The effect remains the same: preferred for eviction, avoided for outgoing connections, and not relayed to other peers. Also change the name of this mechanism to "discouraged" to better reflect reality. ACKs for top commit: naumenkogs: utACK 2ad58381fffb33d611abf900b73d9e6b5a4e35f8 amitiuttarwar: code review ACK 2ad58381ff jonatack: ACK 2ad5838 per changes since last review `git range-diff 3276c14 1f7e0ca 2ad5838` jnewbery: Code review ACK 2ad58381fffb33d611abf900b73d9e6b5a4e35f8 Tree-SHA512: 5dedef401d9cbfa026812651303e6286223563dbeed7a10766ed536ac9e3f29ed4bd0df29cc6deadceeb35cbe9f066346add14ef0833958ca9f93d123fe7aab5
2020-07-06Merge #19328: Add gettxoutsetinfo hash_type optionMarcoFalke
40506bf93f955adfbc446c4d5fee4fa8bcfd7d9a test: Test gettxouttsetinfo hash_type option (Fabian Jahr) f17a4d1c4ddce6935a353004898fb4e8618a213e rpc: Add hash_type NONE to gettxoutsetinfo (Fabian Jahr) a712cf6f6801157667fcf36d1c498b6fff6d328a rpc: gettxoutsetinfo can specify hash_type (only legacy option for now) (Fabian Jahr) 605884ef21318fc3f326dbdf4901cb353ba63fab refactor: Extract GetBogoSize function (Fabian Jahr) Pull request description: This is another intermediate part of the Coinstats Index (tracked in #18000). Sjors suggested [here](https://github.com/bitcoin/bitcoin/pull/18000#issuecomment-641423019) that the part of the changes in #19145 that don't rely on the new `hash_type` muhash, i.e. that are for `hash_type=none`, could be merged separately from everything involving muhash. So these changes are extracted from #19145 here and can be merged without any other requirements. Building the index with no UTXO set hash is still valuable because `gettxoutsetinfo` can still be used to audit the `total_amount` for example. By itself this PR is not a huge improvement, `hash_type=none` is speeding up `gettxoutsetinfo` by about 10%, but it enables the implementation of an index on top of it in a follow-up and that means large parts of the index code of Coinstats Index can be merged while reviews for the hashing algorithm might take longer. ACKs for top commit: MarcoFalke: ACK 40506bf93f955adfbc446c4d5fee4fa8bcfd7d9a 🖨 Sjors: tACK 40506bf93f955adfbc446c4d5fee4fa8bcfd7d9a Tree-SHA512: 3964c2b8eed427511b1aa9b2ef285dff27dc4d1537d72c3911e435b6e6b40912232da4acb3a09bd19a0372ddffa44103388d8a650169d95a4a727b970d210add
2020-07-06tools: clang-format 6 compatibilityJon Atack
2020-07-05Merge #19324: wallet: Move BerkeleyBatch static functions to BerkeleyDatabaseMarcoFalke
d8e9ca66d119d80acfb2bb3c8940c386ce0fc226 walletdb: Move Rewrite into BerkeleyDatabase (Andrew Chow) 91d109156d63ff81cda534bd7bec8369af0027dd walletdb: Move PeriodicFlush into WalletDatabase (Andrew Chow) 8f1bcf8b7b6e47c05f2e43dd98ec3505b888d8b3 walletdb: Combine VerifyDatabaseFile and VerifyEnvironment (Andrew Chow) Pull request description: The `BerkeleyBatch` class has 4 static functions that operate on `BerkeleyDatabase` or `BerkeleyEnvironment`. It doesn't make sense for these to be standalone nor for them to be static functions. So instead, move them from `BerkeleyBatch` into `BerkeleyDatabase` and make them member functions instead of static. `BerkeleyBatch::VerifyEnvironment` and `BerkeleyBatch::VerifyDatabaseFile` are combined into a single `BerkeleyDatabase::Verify` function that operates on that `BerkeleyDatabase` object. `BerkeleyBatch::Rewrite` and `BerkeleyBatch::PeriodicFlush` both took a `BerkeleyDatabase` as an argument and did stuff on it. So we just make it a member function so it doesn't need to take a database as an argument. Part of #18971 ACKs for top commit: MarcoFalke: re-ACK d8e9ca66d1 only change is test fixup 🤞 promag: Code review ACK d8e9ca66d119d80acfb2bb3c8940c386ce0fc226, good stuff. Tree-SHA512: 9847e55b13d98bf4e5636cc14bc3f5351d56737f7e320fafffaed128606240765599e5400382c5aecac06690f7e36265ca3e1031f3f6d8a9688f6d5cb1bacd2a
2020-07-05cli: reduce DefaultRequestHandler memory allocationsJon Atack
2020-07-04Merge #19277: util: Add Assert identity functionMarcoFalke
fab80fef61ddd4afeff6e497c7e76bffcd05e8a4 refactor: Remove unused EnsureChainman (MarcoFalke) fa34587f1c811d99200453b0936219c473f514b0 scripted-diff: Replace EnsureChainman with Assert in unit tests (MarcoFalke) fa6ef701adba1cb48535cac25fd43c742a82e40d util: Add Assert identity function (MarcoFalke) fa457fbd3387661e1973a8f4e5cc2def79e0c625 move-only: Move NDEBUG compile time check to util/check (MarcoFalke) Pull request description: The utility function is primarily useful to dereference pointer types, which are known to be not null at that time. For example, the ArgsManager is known to exist when the wallets are started: https://github.com/bitcoin/bitcoin/pull/18923/files#diff-fdb2a1a1d8bc790fcddeb6cf5a42ac55R503 . Instead of silently relying on that assumption, `Assert` can be used to abort the program and avoid UB should the assumption ever be violated. ACKs for top commit: promag: Tested ACK fab80fef61ddd4afeff6e497c7e76bffcd05e8a4. ryanofsky: Code review ACK fab80fef61ddd4afeff6e497c7e76bffcd05e8a4 Tree-SHA512: 830fba10152ba17d47c4dd42809c7e26f9fe6d38e17a2d5b3f054fd644a5c4c9841286ac421ec9bb28cea9f5faeb659740fcf00de6cc589d423fee7694c42d16
2020-07-03Clean up separated ban/discourage interfacePieter Wuille
2020-07-03Replace automatic bans with discouragement filterPieter Wuille
This patch improves performance and resource usage around IP addresses that are banned for misbehavior. They're already not actually banned, as connections from them are still allowed, but they are preferred for eviction if the inbound connection slots are full. Stop treating these like manually banned IP ranges, and instead just keep them in a rolling Bloom filter of misbehaving nodes, which isn't persisted to disk or exposed through the ban framework. The effect remains the same: preferred for eviction, avoided for outgoing connections, and not relayed to other peers. Also change the name of this mechanism to better reflect reality; they're not banned, just discouraged. Contains release notes and several interface improvements by John Newbery.
2020-07-03Merge #19413: refactor: Remove confusing BlockIndex globalMarcoFalke
fa0dfdf447d5b84a1849dc823d8508463600136a refactor: Remove confusing BlockIndex global (MarcoFalke) Pull request description: The global `::BlockIndex()` is problematic for several reasons: * It returns a mutable reference to the block tree, without the appropriate lock annotation (`m_block_index` is guarded by `cs_main`). The current code is fine, but in the future this might lead to accidental races and data corruption. * The rpc server shouldn't rely on node globals, but rather a context that is passed in to the RPC method. * Tests might want to spin up their own block tree, and thus should also not rely on a single global. Fix all issues by removing the global ACKs for top commit: promag: Code review ACK fa0dfdf447d5b84a1849dc823d8508463600136a. jonatack: re-ACK fa0dfdf Tree-SHA512: 8f158fc5e1c67e73588a21c25677b3fa0fe442313b13ec24b87054806c59607d6ba0c062a865ce3e0ee568706bd0d1faa84febda21aff5bcd65dab172f74c52f
2020-07-03Merge #19215: psbt: Include and allow both non_witness_utxo and witness_utxo ↵Samuel Dobson
for segwit inputs 84d295e51341a126a6c3cbeea7a8caa04c7b5bc3 tests: Check that segwit inputs in psbt have both UTXO types (Andrew Chow) 46004790588c24174a0bec49b540d158ce163ffd psbt: always put a non_witness_utxo and don't remove it (Andrew Chow) 5279d8bc07d601fe6a67ad665fbc7591fe73c7de psbt: Allow both non_witness_utxo and witness_utxo (Andrew Chow) 72f6bec1da198764d4648a10a61c485e7ab65e9e rpc: show both UTXOs in decodepsbt (Andrew Chow) Pull request description: Due to recent changes to hardware wallets, the full previous transaction will need to be provided for segwit inputs. Since some software may be checking for the existence of a `witness_utxo` to determine whether to produce a segwit signature, we keep that field to ease the transition. Because all of the sanity checks implemented by the `IsSane` functions were related to having mixed segwit and non-segwit data in a PSBT, those functions are removed as those checks are no longer proper. Some tests are updated/removed to accommodate this and a simple test added to check that both UTXOs are being added to segwit inputs. As discussed in the wallet IRC meeting, our own signer will not require `non_witness_utxo` for segwit inputs. ACKs for top commit: Sjors: utACK 84d295e51341a126a6c3cbeea7a8caa04c7b5bc3 (didn't retest compared to 836d6fc, but fortunately HWI's CI tracks our master branch, with a bunch of hardware wallet simulators) ryanofsky: Code review re-ACK 84d295e51341a126a6c3cbeea7a8caa04c7b5bc3. No changes since last review, but now I understand the context better. I think it would good to improve the comments as suggested https://github.com/bitcoin/bitcoin/pull/19215#discussion_r447889473 and maybe refer to meshcollider: utACK 84d295e51341a126a6c3cbeea7a8caa04c7b5bc3 Tree-SHA512: ccc1fd3c16ac3859f5aca4fa489bd40f68be0b81bbdc4dd51188bbf28827a8642dc8b605a37318e5f16cf40f1c4910052dace2f27eca21bb58435f02a443e940
2020-07-02Merge #19396: refactor: Remove confusing OutputType::CHANGE_AUTOWladimir J. van der Laan
fa927ff884ae373c676deed63180a8d238872cdc Enable Wswitch for OutputType (MarcoFalke) faddad71f648ed99734f4f8811bd4bc7232ca670 Remove confusing OutputType::CHANGE_AUTO (MarcoFalke) fa2eb383522249a5f4d48726c520cec5de496614 interfaces: Remove unused getDefaultChangeType (MarcoFalke) Pull request description: `OutputType::CHANGE_AUTO` is problematic for several reasons: * An output that is not change must never be described by `CHANGE_AUTO`. Simply allowing that option makes the code confusing and review harder than it needs to be. * To make review even harder, `CHANGE_AUTO` requires `-Wswitch` to be disabled for `OutputType` Fix both issues by removing `CHANGE_AUTO` and then enabling `-Wswitch` for `OutputType` ACKs for top commit: promag: Code review ACK fa927ff884ae373c676deed63180a8d238872cdc. laanwj: Code review ACK fa927ff884ae373c676deed63180a8d238872cdc Tree-SHA512: 24fd809757aa343866c94dafe9a7130b50cda4f77c97666d407f99b813f75b115a7d8e688a6bc2a737e87cba64ddd4e43f2b3c5538fd35fabb5845807bb39134
2020-07-01Merge bitcoin-core/gui#17: doc: Remove outdated comment in TransactionTablePrivMarcoFalke
faebb60b8d009e52d585cffd0f124a0f2fd1a66d doc: Remove outdated comment in TransactionTablePriv (MarcoFalke) Pull request description: Locks are no longer taken upfront, so remove the outdated comment ACKs for top commit: hebasto: ACK faebb60b8d009e52d585cffd0f124a0f2fd1a66d, I have reviewed the code and it looks OK, I agree it can be merged. Tree-SHA512: cd6df24d49d17e58049ac9b261c5e07c8e85ed1aacb547b13c0e55139339d7fcc3b1f766ea2e27d758ea77deadc01f7e28781be1515323c82b9012cee8fd488b
2020-07-01Enable Wswitch for OutputTypeMarcoFalke
2020-07-01Remove confusing OutputType::CHANGE_AUTOMarcoFalke
2020-07-01wallet: Replace boost::none with nulloptMarcoFalke
2020-07-01script: Fix boost/C++17 compile failureMarcoFalke
script/standard.cpp:297:48: error: temporary of type 'boost::static_visitor<CScript>' has protected destructor return boost::apply_visitor(CScriptVisitor{}, dest); ^ /usr/include/boost/variant/static_visitor.hpp:53:5: note: declared protected here ~static_visitor() = default; ^ 1 error generated.
2020-07-01walletdb: Move Rewrite into BerkeleyDatabaseAndrew Chow
Make Rewrite actually a member of BerkeleyDatabase instead of a static function in BerkeleyBatch
2020-07-01walletdb: Move PeriodicFlush into WalletDatabaseAndrew Chow
Make PeriodicFlush a non-static member of WalletDatabase instead of WalletBatch.
2020-07-01walletdb: Combine VerifyDatabaseFile and VerifyEnvironmentAndrew Chow
Combine these two functions into a single Verify function that is a member of WalletDatabase. Additionally, these are no longer static.
2020-07-01Merge #19028: test: Set -logthreadnames in unit testsWladimir J. van der Laan
99993489da9bc003b823bcab10e5f5297b369431 test: Set -logthreadnames in unit tests (MarcoFalke) fa4ea997b4da1ae0afafba223fff9efbeefaf555 init: Setup scheduler in tests and init in exactly the same way (MarcoFalke) Pull request description: Generally the unit tests are single threaded, with the exception of the script check threads, the schedule, and optionally indexer threads. Like the functional tests, the thread name can serve additional debug information, so set `-logthreadnames` in unit tests. Can be tested with ``` ./src/test/test_bitcoin -l test_suite -t validation_tests/test_combiner_all -- DEBUG_LOG_OUT ACKs for top commit: laanwj: ACK 99993489da9bc003b823bcab10e5f5297b369431 Tree-SHA512: 3bdbfc211da146da64b50b0826246aff5c611a84b69ab896a55b3c9d1adc92c5975da36ab92aee577df82e229c4326b477f4105bfdd1a5df4c9a0b018cf61602
2020-07-01Merge #19256: gui: change combiner for signals to optional_last_valueWladimir J. van der Laan
f1a0314c537791f202dfb7c1209f0e04ba7988c3 gui: change combiner for signals to optional_last_value (Cory Fields) Pull request description: [`optional_last_value`](https://www.boost.org/doc/libs/1_73_0/doc/html/boost/signals2/optional_last_value.html), which does not throw, has replaced `last_value` as Boosts default combiner. Besides being better supported, it also doesn't trigger gcc's `-Wmaybe-unitialized` warning, presumably because exceptions no longer bubble-up out of signals: ```bash In file included from ui_interface.cpp:9: /bitcoin/depends/x86_64-pc-linux-gnu/share/../include/boost/signals2/last_value.hpp: In member function 'boost::signals2::detail::signal_impl<R(Args ...), Combiner, Group, GroupCompare, SlotFunction, ExtendedSlotFunction, Mutex>::result_type boost::signals2::detail::signal_impl<R(Args ...), Combiner, Group, GroupCompare, SlotFunction, ExtendedSlotFunction, Mutex>::operator()(Args ...) [with Combiner = boost::signals2::last_value<bool>; Group = int; GroupCompare = std::less<int>; SlotFunction = boost::function<bool(const bilingual_str&, const std::__cxx11::basic_string<char>&, unsigned int)>; ExtendedSlotFunction = boost::function<bool(const boost::signals2::connection&, const bilingual_str&, const std::__cxx11::basic_string<char>&, unsigned int)>; Mutex = boost::signals2::mutex; R = bool; Args = {const bilingual_str&, const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, unsigned int}]': /bitcoin/depends/x86_64-pc-linux-gnu/share/../include/boost/signals2/last_value.hpp:54:36: warning: '*((void*)& value +1)' may be used uninitialized in this function [-Wmaybe-uninitialized] if(value) return value.get(); ^ /bitcoin/depends/x86_64-pc-linux-gnu/share/../include/boost/signals2/last_value.hpp:43:21: note: '*((void*)& value +1)' was declared here optional<T> value; ^~~~~ /bitcoin/depends/x86_64-pc-linux-gnu/share/../include/boost/signals2/last_value.hpp: In member function 'boost::signals2::detail::signal_impl<R(Args ...), Combiner, Group, GroupCompare, SlotFunction, ExtendedSlotFunction, Mutex>::result_type boost::signals2::detail::signal_impl<R(Args ...), Combiner, Group, GroupCompare, SlotFunction, ExtendedSlotFunction, Mutex>::operator()(Args ...) [with Combiner = boost::signals2::last_value<bool>; Group = int; GroupCompare = std::less<int>; SlotFunction = boost::function<bool(const bilingual_str&, const std::__cxx11::basic_string<char>&, const std::__cxx11::basic_string<char>&, unsigned int)>; ExtendedSlotFunction = boost::function<bool(const boost::signals2::connection&, const bilingual_str&, const std::__cxx11::basic_string<char>&, const std::__cxx11::basic_string<char>&, unsigned int)>; Mutex = boost::signals2::mutex; R = bool; Args = {const bilingual_str&, const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, unsigned int}]': /bitcoin/depends/x86_64-pc-linux-gnu/share/../include/boost/signals2/last_value.hpp:54:36: warning: '*((void*)& value +1)' may be used uninitialized in this function [-Wmaybe-uninitialized] if(value) return value.get(); ^ /bitcoin/depends/x86_64-pc-linux-gnu/share/../include/boost/signals2/last_value.hpp:43:21: note: '*((void*)& value +1)' was declared here optional<T> value; ^~~~~ ``` The change in default happened in [Boost 1.39.0](https://www.boost.org/users/history/version_1_39_0.html) (along with the introduction of the Signals2 library. More information is also available here https://www.boost.org/doc/libs/1_73_0/doc/html/signals2/rationale.html#id-1.3.36.9.4: > The default combiner for Boost.Signals2 has changed from the last_value combiner used by default in the original Boost.Signals library. > This is because last_value requires that at least 1 slot be connected to the signal when it is invoked (except for the last_value<void> specialization). > In a multi-threaded environment where signal invocations and slot connections and disconnections may be happening concurrently, it is difficult to fulfill this requirement. When using optional_last_value, there is no requirement for slots to be connected when a signal is invoked, since in that case the combiner may simply return an empty boost::optional. ACKs for top commit: laanwj: ACK f1a0314c537791f202dfb7c1209f0e04ba7988c3 Tree-SHA512: 3600f85019a3591b141dc9207f8a7e66d16d9996cf97fdf08f5133a212d55c591955ab835ffbdca20b5d62711578bc305d5525c75546fa957f180192e2a80c1e
2020-07-01Merge #19308: wallet: BerkeleyBatch Handle cursor internallyWladimir J. van der Laan
ca24edfbc1941ed0a3c9586416dae4e84794eb66 walletdb: Handle cursor internally (Andrew Chow) Pull request description: Instead of returning a Dbc (BDB cursor object) and having the caller deal with the cursor, make BerkeleyBatch handle the cursor internally. Split from #18971 ACKs for top commit: ryanofsky: Code review ACK ca24edfbc1941ed0a3c9586416dae4e84794eb66. Changes since last review: StartCursor rename, moving CloseCursor calls near returns promag: Code review ACK ca24edfbc1941ed0a3c9586416dae4e84794eb66. Tree-SHA512: f029b498c7f275aedca53ce7ade7cb99c82975fd6cad17346a4990fb3bcc54e2a5309b32053bd13def9ee464d331b036ac79abb8fc4fa561170c6cfc85283447
2020-07-01gui: change combiner for signals to optional_last_valueCory Fields
optional_last_value, which does not throw, has replaced optional_value as boost's default combiner. Besides being better supported, it also doesn't trigger gcc's -Wmaybe-unitialized warning, presumably because exceptions no longer bubble-up out of signals: ```bash boost/signals2/last_value.hpp:54:36: warning: '*((void*)& value +1)' may be used uninitialized in this function [-Wmaybe-uninitialized] if(value) return value.get(); ``` The change in default happened in Boost 1.39.0 (along with the introduction of the signals 2 library. More information is available here: https://www.boost.org/doc/libs/1_73_0/doc/html/signals2/rationale.html#id-1.3.36.9.4 and here: https://www.boost.org/doc/libs/1_73_0/doc/html/boost/signals2/optional_last_value.html Co-authored-by: fanquake <fanquake@gmail.com>
2020-07-01Merge #19331: build: Do not include server symbols in walletWladimir J. van der Laan
faca73000fa8975c28f6be8be01957c1ae94ea62 ci: Install fixed version of clang-format for linters (MarcoFalke) fa4695da4c69646b58a8fa0b6b30146bb234deb8 build: Sort Makefile.am after renaming file (MarcoFalke) cccc2784a3bb10fa8e43be7e68207cafb12bd915 scripted-diff: Move ui_interface to the node lib (MarcoFalke) fa72ca6a9d90d66012765b0043fd819698b94ba8 qt: Remove unused includes (MarcoFalke) fac96e6450d595fe67168cb7afa7692da6cc9973 wallet: Do not include server symbols (MarcoFalke) fa0f6c58c1c6d10f04c4e65a424cc51ebca50a8c Revert "Fix link error with --enable-debug" (MarcoFalke) Pull request description: This reverts a hacky workaround from commit b83cc0f, which only happens to work due to compiler optimizations. Then, it actually fixes the linker error. The underlying problem is that the wallet includes symbols from the server (ui_interface), which usually results in linker failures. Though, in this specific case the linker failures have not been observed (unless `-O0`) because our compilers were smart enough to strip unused symbols. Fix the underlying problem by creating a new header-only with the needed symbol and move ui_interface to node to clarify that this is part of libbitcoin_server. ACKs for top commit: Sjors: ACK faca730 laanwj: ACK faca73000fa8975c28f6be8be01957c1ae94ea62 hebasto: re-ACK faca73000fa8975c28f6be8be01957c1ae94ea62, since the [previous](https://github.com/bitcoin/bitcoin/pull/19331#pullrequestreview-434420539) review: Tree-SHA512: e9731f249425aaea50b6db5fc7622e10078cf006721bb87989cac190a2ff224412f6f8a7dd83efd018835302337611f5839e29e15bef366047ed591cef58dfb4
generated by cgit v1.2.3 (git 2.26.2) at 2024-09-19 22:11:47 +0000