aboutsummaryrefslogtreecommitdiff
AgeCommit message (Collapse)Author
2020-06-19build: add -fstack-clash-protection to hardening flagsfanquake
This option causes the compiler to insert probes whenever stack space is allocated statically or dynamically to reliably detect stack overflows and thus mitigate the attack vector that relies on jumping over a stack guard page as provided by the operating system. This option is now enabled by default in Ubuntu GCC as of 19.10. Available in GCC 8 and Clang 11.
2020-06-19build: add -fcf-protection=full to hardening optionsfanquake
Enables code instrumentation of control-flow transfers. Available in GCC 8 and Clang 7. This option is now on by default in Ubuntu GCC as of 19.10.
2020-06-19Merge #19293: net: Avoid redundant and confusing FAILED logfanquake
fa1904e5f0d164fbcf41398f9ebbaafe82c28419 net: Remove dead logging code (MarcoFalke) fac12ebf4f3b77b05112d2b00f8d3f4669621a4c net: Avoid redundant and confusing FAILED log (MarcoFalke) Pull request description: Remove a redundant and confusing "FAILED" log message and gets rid of the unused return type in `ProcessMessage` ACKs for top commit: jnewbery: utACK fa1904e5f0d164fbcf41398f9ebbaafe82c28419 gzhao408: utACK https://github.com/bitcoin/bitcoin/commit/fa1904e5f0d164fbcf41398f9ebbaafe82c28419 troygiorshev: ACK fa1904e5f0d164fbcf41398f9ebbaafe82c28419 naumenkogs: utACK fa1904e Tree-SHA512: bfa553d5efa022727ed17877fb7c08c14849d804fe6d6a7ce172d513857beba35de41ea40b27ff1aedf68b81e2cda7b2a948ac985fcaaf1b6cfb96cce4837c90
2020-06-19Merge #19322: [net] split PushInventory()fanquake
f52d403b81e758e9bc33847560b5740b22d95fff [net] split PushInventory() (John Newbery) Pull request description: PushInventory() is currently called with a CInv object, which can be a MSG_TX or MSG_BLOCK. PushInventory() only uses the type to determine whether to add the hash to setInventoryTxToSend or vInventoryBlockToSend. Since the caller always knows what type of inventory they're pushing, the CInv is wastefully constructed and thrown away, and tx/block relay is being split out, we split the function into PushTxInventory() and PushBlockInventory(). ACKs for top commit: amitiuttarwar: utACK f52d403b81. nice cleanup, this has bothered me :) naumenkogs: utACK f52d403 sipa: utACK f52d403b81e758e9bc33847560b5740b22d95fff Tree-SHA512: 331495199a3b1a2620e6a62beb336e494291b725d8fd64bb44726c02e80807f3974ff4f329bb0f059088e65cd7d41eff276c1065806d2dd6e72c5a9f368e82cd
2020-06-19Merge #19197: init: use std::thread for ThreadImport()fanquake
83fd3a6d73eee452dc5141bdf6826da62d7b2dbd init: use std::thread for ThreadImport() (fanquake) Pull request description: [Mentioned](https://github.com/bitcoin/bitcoin/pull/19142#issuecomment-638090759) in #19142, which removed the `boost::interruption_point()` in `ThreadImport()`. ACKs for top commit: hebasto: ACK 83fd3a6d73eee452dc5141bdf6826da62d7b2dbd, I have reviewed the code and it looks OK, I agree it can be merged. donaloconnor: ACK 83fd3a6 laanwj: Code review ACK 83fd3a6d73eee452dc5141bdf6826da62d7b2dbd MarcoFalke: ACK 83fd3a6d73eee452dc5141bdf6826da62d7b2dbd Tree-SHA512: 0644947d669feb61eed3a944012dad1bd3dd75cf994aa2630013043c213a335b162b63e20aa37e0997740d8e3a3ec367b660b5196007a09e13f0ac455b36c821
2020-06-18Merge #19310: wallet: BerkeleyDatabase make BerkeleyDatabase::Create, ↡MarcoFalke
CreateMock, and CreateDummy non-static functions da7a83c5ee6a51ff4c3eb35dbd447a310c4a0387 Remove WalletDatabase::Create, CreateMock, and CreateDummy (Andrew Chow) d6045d0ac615b5984b72e83cb25aa8a245a177a0 scripted-diff: Replace WalletDatabase::Create* with CreateWalletDatabase (Andrew Chow) 45c08f8a7b89dda6afb7d7cf9573a8ae8290ac92 Add Create*WalletDatabase functions (Andrew Chow) Pull request description: Instead of having `Create`, `CreateMock`, and `CreateDummy` being static functions in `BerkeleyDatabase`, move these to standalone functions in `walletdb.cpp`. This prepares us for having different `WalletDatabase` classes. Part of #18971. This was originally one commit but has been split into 3 to make it (hopefully) easier to review. ACKs for top commit: MarcoFalke: ACK da7a83c5ee6a51ff4c3eb35dbd447a310c4a0387 πŸŽ‚ ryanofsky: Code review ACK da7a83c5ee6a51ff4c3eb35dbd447a310c4a0387. Easy review, nice scripted-diff Tree-SHA512: 1feb7cb3889168c555154bf3701a49095fd6b8cab911d44b7f7efbf6fcee2280ccb3d4afec8a83755b39a592ecd13b90a318faa655c321f87bdabdf1e2312327
2020-06-18[net] split PushInventory()John Newbery
PushInventory() is currently called with a CInv object, which can be a MSG_TX or MSG_BLOCK. PushInventory() only uses the type to determine whether to add the hash to setInventoryTxToSend or vInventoryBlockToSend. Since the caller always knows what type of inventory they're pushing, the CInv is wastefully constructed and thrown away, and tx/block relay is being split out, we split the function into PushTxInventory() and PushBlockInventory().
2020-06-18Merge #19071: doc: Separate repository for the guiMarcoFalke
66666d55b16603287d30ee672061ced8fc6752ab doc: Mention repo split in the READMEs (MarcoFalke) faceed753a4d3d909985cdfc42b23f5dd395e168 doc: Add redirect for GUI issues and pull requests (MarcoFalke) Pull request description: ## πŸ₯… Goals Splitting up the GUI (and splitting out modules in general) has been brought up often in recent years. Now that the GUI is primarily connected through (internal) interfaces with the node, it seems an appropriate time to revive this discussion. Before looking for solutions, we should define a set of goals that we want to achieve. I will start with some ideas to get started and I hope that others will chime in to share and prioritize their goals. ### Separate issue and patch management It is currently not possible to subscribe to only a subset of modules in Bitcoin Core, or exclude modules from issue and patch notifications. While it is possible to reactively mute conversations in the stream of all ongoing discussions, there is no way to proactively achieve this. Moreover, the list of open issues and pull request will always include GUI related ones by default. Only with [filters](https://github.com/bitcoin/bitcoin/pulls?q=is%3Aopen+is%3Apr+-label%3AGUI) it is possible to hide them. ### More focused review and interests Long term goals of the GUI are partially unclear #17395 . Bitcoin Core developers are generally fluent on the command line. Thus, they might not be interested or motivated to review improvements to the GUI, which might not affect their workflow on the command line at all. Splitting up the GUI will hopefully attract similar minded people to a project whose primary goal is to build and improve the GUI. ### Maintain high quality assurance The quality of the GUI (and even more importantly Bitcoin Core in general) must not degrade. This means that code review itself is not negatively affected by splitting the GUI, but also the integration of the GUI into the rest of Bitcoin Core. One issue could arise when arbitrary version-combinations are allowed. We are struggling hard to test against all supported versions of Boost. Making the GUI version another dimension is going to make testing impossible. ### The GUI *is* Bitcoin Core When a user downloads Bitcoin Core from our website (or another package manager) they expect the GUI to be included. This should not change (at least not as a result of splitting up the GUI into another project). Similarly, when building Bitcoin Core, the gui should still be built when `--with-gui` is specified. ## 🌳 Proposed solution: Monotree TLDR. Everything stays the same, the development process for the GUI changes slightly. Long version: * An exact mirror of the master development branch is hosted at `bitcoin-core/gui`. The new repository is used to track gui-only issues and pull requests. Global changes that happen to touch gui code still go to the *main* repo. * All pull requests will be merged into `bitcoin/bitcoin`. * Decision making process and maintainers will be identical for both repos. ### Disadvantages * Review activity might decrease? * It doesn't go far enough. bitcoin/bitcoin#3440 is proposing a modularized Bitcoin Core. The GUI could be an "add-on", connected over RPC or capnproto (bitcoin/bitcoin#10102). Thus, the gui could even be hosted as a subtree or completely separate project. ### Advantages * Review activity might increase? It is impossible to predict the future, but for example the `libsecp256k1` subtree has a lot of domain specific experts, maintainers and reviewers. I think longer term it makes sense to at least try this route for the gui as well. * A smaller step is easier to undo when it turns out to come with any unforeseen downsides. * No substantial changes to the decision making progress. * Nothing changes in how developers set up their dev environment or how users build from the source. Also, the release binaries and process will stay exactly the same. No version drift. Finally, code sharing between the GUI and Bitcoin Core is not made any harder. * The organizational side. There are 72 open issues (~14%) and 61 open PRs (~16%) with the GUI label. If moved to its own repo, non-GUI developers wouldn't have to be distracted with GUI-only issues and PRs and GUI enhancements. GUI developers have their own repo to focus on GUI development exclusively. ### Implementation (outstanding TODOs) * Adjust maintainer merge script https://github.com/bitcoin-core/bitcoin-maintainer-tools/pull/57 * Create bitcoin-core/gui repository (empty or with master branch only) * Assign all existing bitcoin core maintainers to the new repo * Celebrate? :partying_face: * Long-term: Think how long the grace period is for existing GUI related issues and pull requests. Issues can be transferred with a script after a grace period of some months? ACKs for top commit: fjahr: ACK 66666d55b16603287d30ee672061ced8fc6752ab Sjors: ACK 66666d55b16603287d30ee672061ced8fc6752ab troygiorshev: re-ACK 66666d5 practicalswift: re-ACK 66666d55b16603287d30ee672061ced8fc6752ab hebasto: re-ACK 66666d55b16603287d30ee672061ced8fc6752ab Tree-SHA512: 2e1a8de945fa6995583059a2e322621763fccce74a869f9aa750f73546b26350487c4acc4222c03cb3ac1f88e80f0b9d9a3a80a200432fee0d785f52c5cb6174
2020-06-18Merge bitcoin-core/gui#3: scripted-diff: Make SeparatorStyle a scoped enumMarcoFalke
25f3554351a99a0a695fbd2a6a0f293b3adc3d98 scripted-diff: Make SeparatorStyle a scoped enum (Hennadii Stepanov) Pull request description: This PR is [split](https://github.com/bitcoin/bitcoin/pull/17877#issuecomment-644751515) from https://github.com/bitcoin/bitcoin/pull/17877 and makes `BitcoinUnits::SeparatorStyle` a scoped enum. ACKs for top commit: MarcoFalke: review ACK 25f3554351a99a0a695fbd2a6a0f293b3adc3d98 🚐 Tree-SHA512: 578f1340a476cf79faa109a83815d3c75e26d9c18873e653d7624b52428ccb2677293116db0a60ae14c949d63b64988fc5a39c7184c2352b87b00e8ddaaaf474
2020-06-18Merge #19292: wallet: Refactor BerkeleyBatch Read, Write, Erase, and Exists ↡MarcoFalke
functions into non-template functions a389ed52e8f4939ab5b4adcf93dcb7783d9006f1 walletdb: refactor Read, Write, Erase, and Exists into non-template func (Andrew Chow) Pull request description: In order to override these later, the specific details of how the Read, Write, Erase, and Exists functions interact with the actual database file need to go into functions that are not templated. The functions `ReadKey`, `WriteKey`, `EraseKey`, and `HasKey` are introduced to handle the actual interaction with the database. This is mostly a moveonly. Based on #19290 ACKs for top commit: ryanofsky: Code review ACK a389ed52e8f4939ab5b4adcf93dcb7783d9006f1. No changes since last review, just non-conflicting rebase Sjors: utACK a389ed52e8f4939ab5b4adcf93dcb7783d9006f1 MarcoFalke: ACK a389ed52e8f4939ab5b4adcf93dcb7783d9006f1 πŸ”³ Tree-SHA512: 73bd2fe9ddc4a132d4db6b97e77f5d5f8aa68b8cb25192384f3bacd826365947763a9eee73672331d34578e3f5ade85ee6aa550ff4d89eb62e482250dd5973e4
2020-06-18scripted-diff: Make SeparatorStyle a scoped enumHennadii Stepanov
-BEGIN VERIFY SCRIPT- # General rename helper: $1 -> $2 rename_global() { sed -i "s/\<$1\>/$2/g" $(git grep -l "$1"); } # Helper to rename SeparatorStyle enumerators rename_value() { sed -i "s/ $1/ $2/g" src/qt/bitcoinunits.h; rename_global $1 "SeparatorStyle::$2"; } rename_global 'enum SeparatorStyle' 'enum class SeparatorStyle' rename_value 'separatorNever' 'NEVER' rename_value 'separatorStandard' 'STANDARD' rename_value 'separatorAlways' 'ALWAYS' -END VERIFY SCRIPT-
2020-06-18Merge #18468: Span improvementsWladimir J. van der Laan
26acc8dd9b512f220c1facdba2c5de7976d3c258 Add sanity check asserts to span when -DDEBUG (Pieter Wuille) 2676aeadfa0e43dcaaccc4720623cdfe0beed528 Simplify usage of Span in several places (Pieter Wuille) ab303a16d114b1e94c6cf0e4c5db5389dfa197f6 Add Span constructors for arrays and vectors (Pieter Wuille) bb3d38fc061d8482e68cd335a45c9cd8bb66a475 Make pointer-based Span construction safer (Pieter Wuille) 1f790a1147ad9a5fe06987d84b6cd71f91cbec4b Make Span size type unsigned (Pieter Wuille) Pull request description: This improves our Span class by making it closer to the C++20 `std::span` one: * ~~Support conversion between compatible Spans (e.g. `Span<char>` to `Span<const char>`).~~ (done in #18591) * Make the size type `std::size_t` rather than `std::ptrdiff_t` (the C++20 one underwent the same change). * Support construction of Spans directly from arrays, `std::string`s, `std::array`s, `std::vector`s, `prevector`s, ... (for all but arrays, this only works for const containers to prevent surprises). And then make use of those improvements in various call sites. I realize the template magic used looks scary, but it's only needed to make overload resultion make the right choices. Note that the operations done on values are all extremely simple: no casts, explicit conversions, or warning-silencing constructions. That should hopefully make it simpler to review. ACKs for top commit: laanwj: Code review ACK 26acc8dd9b512f220c1facdba2c5de7976d3c258 promag: Code review ACK 26acc8dd9b512f220c1facdba2c5de7976d3c258. Tree-SHA512: 5a5bd346a140edf782b5b3b3f04d9160c7b9e9def35159814a07780ab1dd352545b88d3cc491e0f80d161f829c49ebfb952fddc9180f1a56f1257aa51f38788a
2020-06-18Merge #19304: test: Check that message sends successfully when header is ↡MarcoFalke
split across two buffers 80d4423f997e15780bfa3f91bf4b4bf656b8ea45 Test buffered valid message (Troy Giorshev) Pull request description: This PR is a tweak of #19302. This sends a valid message. Additionally, this test includes logging in the same vein as #19272. ACKs for top commit: MarcoFalke: tested ACK 80d4423f997e15780bfa3f91bf4b4bf656b8ea45 (added an assert(false) to observe deterministic coverage) 🌦 gzhao408: ACK https://github.com/bitcoin/bitcoin/commit/80d4423f997e15780bfa3f91bf4b4bf656b8ea45 πŸ‘Š Tree-SHA512: 3b1aa5ec480a1661917354788923d64595e2886448c9697ec0606a81293e8b4a4642b2b3cc9afb2206ce6f74e5c6d687308c5ad19cb73c5b354d3071ad8496f8
2020-06-18Merge #19301: build: don't warn when doxygen isn't foundfanquake
fa84edb93c85f7709fc53abf9c6daae5d1bb3b28 build: don't warn when doxygen isn't found (fanquake) Pull request description: Doxygen isn't so important that we need to warn when it is missing. I'd assume it might even be missing more often than not for most builds. ACKs for top commit: MarcoFalke: Fine with me ACK fa84edb93c85f7709fc53abf9c6daae5d1bb3b28 hebasto: ACK fa84edb93c85f7709fc53abf9c6daae5d1bb3b28, I have reviewed the code and it looks OK, I agree it can be merged. Tree-SHA512: 793ebf01a8a5d48b78a70fdef0022633fca59b30074c960ebb21589e3bd98992b8304621a2d999195d12172ed30fe9eefeeb2a952d58853cf58e8d9902b0090c
2020-06-17Add sanity check asserts to span when -DDEBUGPieter Wuille
2020-06-17Test buffered valid messageTroy Giorshev
A message can be broken across two buffers, with the split inside its header. Usually this will occur when sending many messages, such that the first buffer fills. This test uses the RPC to verify that the message is actually being received in two pieces. There is a very rare chance of a race condition where the test framework sends a message in between the two halves of the message under test. In this case the peer will almost certainly disconnect and the test will fail. An assert has been added to help debugging that rare case.
2020-06-17Merge #19298: test: Add missing sync_blocksMarcoFalke
fa195d4eba6397262e3e76c8525eb48dcb5e7f2d test: Add missing sync_blocks (MarcoFalke) Pull request description: Bitcoin Core does not sort block and tx announcements for other peers, so generating 100 blocks and then sending out a transaction might reject it if it arrives too early. (non-final) Fix that by syncing the blocks first. Fix #19265 Fix #19311 ACKs for top commit: Sjors: utACK fa195d4eba6397262e3e76c8525eb48dcb5e7f2d: sounds plausible Tree-SHA512: fdc46aed59595e4189509e71bd4a3607a93893933cc01d806cec2ee7701d54d7422c5f22dd83b81ddb021f9113b3119a688fdd8cf8a6474fc12fea422aedd064
2020-06-17Remove WalletDatabase::Create, CreateMock, and CreateDummyAndrew Chow
These are superseded by CreateWalletDatabase, CreateMockWalletDatabase, and CreateDummyWalletDatabase
2020-06-17scripted-diff: Replace WalletDatabase::Create* with CreateWalletDatabaseAndrew Chow
-BEGIN VERIFY SCRIPT- sed -i -e 's/WalletDatabase::Create(/CreateWalletDatabase(/g' `git grep -l "WalletDatabase::Create("` sed -i -e 's/WalletDatabase::CreateDummy(/CreateDummyWalletDatabase(/g' `git grep -l "WalletDatabase::CreateDummy("` sed -i -e 's/WalletDatabase::CreateMock(/CreateMockWalletDatabase(/g' `git grep -l "WalletDatabase::CreateMock("` -END VERIFY SCRIPT-
2020-06-17Add Create*WalletDatabase functionsAndrew Chow
These functions doing the same things as WalletDatabase::Create, CreateMock, and CreateDummy
2020-06-17walletdb: refactor Read, Write, Erase, and Exists into non-template funcAndrew Chow
In order to override these later, the specific details of how the Read, Write, Erase, and Exists functions interact with the actual database file need to go into functions that are not templated.
2020-06-17Merge #19295: refactor: Use AbortError in FatalErrorMarcoFalke
fa02b473132932c200be1750d1a5b1de14ea2383 refactor: Use AbortError in FatalError (MarcoFalke) Pull request description: `FatalError` has been copied from `AbortNode`, so the two should use the same style to avoid confusion. Follow-up to #18927 ACKs for top commit: hebasto: ACK fa02b473132932c200be1750d1a5b1de14ea2383, I have reviewed the code and it looks OK, I agree it can be merged. Tree-SHA512: 2cf6d18a6ffb5c2e5cf54f0a072a7cef6dc7e924152b2fee44e6ff2c6c53bad962afd364eda30d8a73883d656429ea68391090e6a27057e69eaefd7c4dad0a33
2020-06-17build: don't warn when doxygen isn't foundfanquake
Doxygen isn't so important that we need to warn when it is missing. I'd assume it might even be missing more often than not for most builds.
2020-06-17Merge #19252: test: wait for disconnect in disconnect_p2ps + bloomfilter ↡MarcoFalke
test followups 9a40cfc558b3f7fa4fff1270f969582af17479a5 [refactor] use waiting inside disconnect_p2ps (gzhao408) aeb9fb414e2d000830287d9dd3fed7fc2eb570d2 [test] wait for disconnect_p2ps to be reflected in getpeerinfo (gzhao408) e81942d2e1288367e8da94adb2b2a88be99e4751 [test] logging and style followups for bloomfilter tests (gzhao408) Pull request description: Followup to #19083 which adds bloomfilter-related tests. 1. Make test_node `disconnect_p2ps` wait until disconnection is complete to avoid race conditions (and not place the burden on tests) from MarcoFalke's [comment](https://github.com/bitcoin/bitcoin/pull/19083#discussion_r437383989). And clean up any redundant `wait_until`s in the functional tests. 2. Clean up style + logging in p2p_filter.py and p2p_nobloomfilter_messages.py and jonatack's other [comments](https://github.com/bitcoin/bitcoin/pull/19083#pullrequestreview-428955784) ACKs for top commit: jonatack: Code review ACK 9a40cfc from re-reviewing the diff and `git range-diff 5cafb46 8386ad5 9a40cfc` MarcoFalke: ACK 9a40cfc558b3f7fa4fff1270f969582af17479a5 πŸ‚ Tree-SHA512: 2e14b1c12fc08a355bd5ccad7a2a734a4ccda4bc7dc7bac171cb57359819fc1599d764290729af74832fac3e2be258c5d406c701e78ab6d7262835859b9a7d87
2020-06-17Merge #19249: Add means to handle negative capabilities in the Clang Thread ↡MarcoFalke
Safety annotations f8213c05f087e5fbb5d92a291f766b0baebc798f Add means to handle negative capabilities in thread safety annotations (Hennadii Stepanov) Pull request description: This commit is separated from #19238, and it adds support of [Negative Capabilities](https://clang.llvm.org/docs/ThreadSafetyAnalysis.html#negative) in the Clang Thread Safety Analysis attributes. > Negative requirements are an alternative `EXCLUDES` [`LOCKS_EXCLUDED`] that provide a stronger safety guarantee. A negative requirement uses the `REQUIRES` [`EXCLUSIVE_LOCKS_REQUIRED`] attribute, in conjunction with the ! operator, to indicate that a capability should not be held. Examples of usage: - #19238 (for a class) - https://github.com/hebasto/bitcoin/tree/200610-addrman-tsn (for the whole code base) ACKs for top commit: MarcoFalke: Approach ACK f8213c05f087e5fbb5d92a291f766b0baebc798f vasild: ACK f8213c05 Tree-SHA512: 86d992826b87579661bd228712ae5ee6acca6f70b885ef7e96458974eac184e4874a525c669607ba6b6c861aa4806409a8792d100e6914c858bcab43d31cfb1b
2020-06-17Merge #19290: wallet: move BDB specific classes to bdb.{cpp/h}Samuel Dobson
61c16339da4e80b1320a6296df6d96cd7a84bb4e walletdb: Move BDB specific things into bdb.{cpp/h} (Andrew Chow) 8f033642a8c6874184e297b97b951b9bd12ffd75 walletdb: moveonly: Move BerkeleyBatch Cursor and Txn funcs to cpp (Andrew Chow) 25a655794a0c495332dadedd88b87d694c1077c2 walletdb: move IsWalletLoaded to walletdb.cpp (Andrew Chow) f6fc5f3849bac48dfccd015bec7089cb711d0667 walletdb: Add IsBDBWalletLoaded to look for BDB wallets specifically (Andrew Chow) c3538f435af8c408759d9d005e80b2f1690e0659 walletdb: Make SpliWalletFilePath non-static (Andrew Chow) Pull request description: Moves the BDB specific classes from db.{cpp/h} to bdb.{cpp/h}. To do this, `SplitWalletFilePath` is first made non-static. Then `IsWalletLoaded` functionality is moved to `IsBDBWalletLoaded` which is called by `IsWalletLoaded`. Then the bulk of db.{cpp/h} is moved to a new file bdb.{cpp/h}. While doing some moveonly stuff, an additional commit moves the `*Cursor` and `Txn*` implementations out of the header file and into the cpp file. Part of #18971 ACKs for top commit: laanwj: Code review ACK 61c16339da4e80b1320a6296df6d96cd7a84bb4e promag: Code review ACK 61c16339da4e80b1320a6296df6d96cd7a84bb4e. meshcollider: utACK 61c16339da4e80b1320a6296df6d96cd7a84bb4e Tree-SHA512: cb676cd34c9cd3c838a4fef230d84711efe4cf0d2eefa64ebfd7f787ddc6f7379db0b29454874ddc46ca7ffee0f18f6f3fb96a85513cd10164048948fd03a80c
2020-06-17Merge #19287: contrib: Fix SyntaxWarning in Python base58 implementationfanquake
47b49a05eafddcaef373f70436d794e9f9f7495c contrib: Fix SyntaxWarning in Python base58 implementation (Alex Willmer) Pull request description: In Python integers should be compared for equality (`i == j`), not identity (`i is j`). Recent versions of CPython 3.x emit a SyntaxWarning when they encounter this incorrect usage, e.g. ``` $ python3 base58.py base58.py:110: SyntaxWarning: "is" with a literal. Did you mean "=="? assert get_bcaddress_version('15VjRaDX9zpbA8LVnbrCAFzrVzN7ixHNsC') is 0 Tests passed ``` ACKs for top commit: MarcoFalke: ACK 47b49a05eafddcaef373f70436d794e9f9f7495c Tree-SHA512: 9f8962025dcdfa062c0515c68a1864f5bbeb86bd0510c0ec0e413a5edb6afbfd5f41b4c0255784e53db8eaf39c68b7cfa7cc8a33a2e5214aae463fda374f8719
2020-06-17Merge #19299: refactor: Remove unused vars, Add missing includesfanquake
fa193c6b1b7da8f72a399bfddb1497655ce1685c Add missing includes to fix compile errors (MarcoFalke) fa09ec83f3f23dacb807c6b6393cabf2a984e4ff Remove unused variables (MarcoFalke) Pull request description: This is required for #19183, but seems like good cleanup that can go in upfront. ACKs for top commit: practicalswift: ACK fa193c6b1b7da8f72a399bfddb1497655ce1685c -- patch looks correct hebasto: ACK fa193c6b1b7da8f72a399bfddb1497655ce1685c, I have reviewed the code and it looks OK, I agree it can be merged. Tree-SHA512: 79b94e7f7ee3a1a8a8fb2ea1ecdf61f130f8b133a37865894da3dbbbf311979e7d1fc013b923fdd7dbf19a221e0232f664defbdb57aa44e0b8c45bfff3c71dcb
2020-06-16Merge #19294: test: refactor: Inline adjust_bitcoin_conf_for_pre_17MarcoFalke
fa41b0a6dac7afd77e2b94eca6520ab3d2adc231 pep-8 test/functional/test_framework/util.py (MarcoFalke) faa841bc979ca306f5ba4d5f7b78fcc427b8e413 test: refactor: Inline adjust_bitcoin_conf_for_pre_17 (MarcoFalke) Pull request description: This removes mental and code complexity as well as attack surface for bikeshedding ACKs for top commit: Sjors: utACK fa41b0a6dac7afd77e2b94eca6520ab3d2adc231 Tree-SHA512: 6e3c872e66d98ffaa7aecdfd64aa7dd8fbb51815a8fdaba170ce0772b4c3360084d0ebab4a5feac768ab5df50d04528d7daafc51ba07c15445c1ef94fa3efd34
2020-06-16Add missing includes to fix compile errorsMarcoFalke
fs.cpp:35:17: error: no member named 'strerror' in namespace 'std' return std::strerror(errno); ~~~~~^ fs.cpp:49:9: error: use of undeclared identifier 'close' close(fd); ^ 2 errors generated. ./interfaces/chain.h:265:55: error: β€˜std::function’ has not been declared virtual void rpcRunLater(const std::string& name, std::function<void()> fn, int64_t seconds) = 0; ^~~
2020-06-16Remove unused variablesMarcoFalke
2020-06-16test: Add missing sync_blocksMarcoFalke
2020-06-16Merge #18275: wallet: error if an explicit fee rate was given but the needed ↡MarcoFalke
fee rate differed 44cc75f80ee7805a117e9298a182af1a44bcbff4 wallet: error if an explicit fee rate was given but the needed fee rate differed (Karl-Johan Alm) Pull request description: This ensures that the code doesn't silently ignore too low fee reates. It will now trigger an error in the QT client, if the user provides a fee rate below the minimum, and becomes a necessary check for #11413. ACKs for top commit: Sjors: utACK 44cc75f80ee7805a117e9298a182af1a44bcbff4 (rebased) fjahr: re-ACK 44cc75f80ee7805a117e9298a182af1a44bcbff4 Tree-SHA512: cd5a60ee496e64f7ab37aaa53f7748a7393357b1629ccd9660839d366c6191b6413b871ce3aa7293fce1539336222c300ef6f86304f30a1ae8fe361b02310483
2020-06-16Merge #19284: [net] Add seed.bitcoin.wiz.biz to DNS seedsWladimir J. van der Laan
313a081b907bf0a5b56af99ec2d42814ef0638b0 [net] Add seed.bitcoin.wiz.biz to DNS seeds (wiz) Pull request description: I've created the `seed.bitcoin.wiz.biz` DNS seed for the benefit of the Bitcoin community, and will operate it in accordance with the [Bitcoin DNS seed operator policy](https://github.com/bitcoin/bitcoin/blob/master/doc/dnsseed-policy.md). Since this is my first PR to the Bitcoin Core project, I also ACK the [contributing guidelines](https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md). The data for this DNS seed is generated using redundant instances of TheBlueMatt's [dnsseed-rust implementation](https://github.com/TheBlueMatt/dnsseed-rust), which connects to all discoverable Bitcoin nodes to verify their capabilities and speed, and utilizes the full AS-MAP data from my network's BGP tables to select Bitcoin nodes which are fairly distributed across different networks. As for my qualifications, I currently operate Bitcoin nodes for the [mempool.space](https://mempool.space/) open-source block explorer project (mempool) and the [Bisq Network](https://bisq.network/) open-source P2P trading community (bisq-network). I have 20 years experience as a network engineer, and all of [my Bitcoin nodes](https://bitnodes.io/nodes/?q=AS54415) are hosted on [my own network](https://ipinfo.io/AS54415) across multiple datacenters. For personal references, the current Bitcoin DNS seed operators Emzy and TheBlueMatt can probably vouch for me. The DNS responses served from this instance are currently served with a TTL of 60 seconds, and the DNS resolvers do not log queries from users. Any inquiries related to the operation of this DNS seed can be sent to <noc@wiz.biz>. Here is a rough diagram of the `seed.bitcoin.wiz.biz` DNS seed architecture: ![seed bitcoin wiz biz](https://user-images.githubusercontent.com/232186/84641969-cb2c6300-af36-11ea-9e4c-392fe39f5f08.png) ACKs for top commit: jonasschnelli: Tested ACK 313a081b907bf0a5b56af99ec2d42814ef0638b0. laanwj: ACK 313a081b907bf0a5b56af99ec2d42814ef0638b0 Tree-SHA512: 9e4ea7a929b7888eba748933c1581328aefcba4de503af96f99630d797d794859b22c99999c25c3fc90f6efaed2598f32784d3acea3e428d84bae3aa37f92a25
2020-06-16Merge #18434: tests: add a test-security target and run it in CIWladimir J. van der Laan
9fe71a57a6780569e618cf9a8d4f1acf6321017f test: use subprocess.run() in test-security-check.py (fanquake) 968aaae940b064f21eddee6bb461aa08f777544c tests: run test-security-check.py in CI (fanquake) Pull request description: [Wladimir asked](https://github.com/bitcoin/bitcoin/pull/18415#issuecomment-603843094) about running the `test-security-check.py` script in our CI. This PR adds a target for that: `make test-security` and adds it to a few CI jobs. ACKs for top commit: laanwj: ACK 9fe71a57a6780569e618cf9a8d4f1acf6321017f Tree-SHA512: d00ebbefbd57ab22436f284837c320f73238ec9967495adc4f2f9a4d574b3b1595c19ce41d53ff4060d5cd7174dbc311235d5877c90e8af2f5587735e7236056
2020-06-16[refactor] use waiting inside disconnect_p2psgzhao408
-Use wait_for_disconnect instead of manual wait after calling disconnect_p2ps.
2020-06-16[test] wait for disconnect_p2ps to be reflected in getpeerinfogzhao408
-Waiting is important to avoid race conditions, especially if testing peer info through rpc later. -Wait for mininodes to be disconnected only, even though it's more complex, because we may still want to be connected to test nodes.
2020-06-16[test] logging and style followups for bloomfilter testsgzhao408
-Use peer to refer to mininodes instead of node because they are not bitcoind nodes. -Use log.debug for logs that give helpful but not super necessary information. -Adhere to style guidelines (newlines, capitalization).
2020-06-16refactor: Use AbortError in FatalErrorMarcoFalke
This is needed for consistency with AbortNode
2020-06-16Merge #18927: Pass bilingual_str argument to AbortNode()MarcoFalke
5527be06277647dffe7cda587c4bbfbec2a5c8ca refactor: Add AbortError alias (Hennadii Stepanov) d924f2a596c8f37deb2dd94069c578244823c31f Drop MSG_NOPREFIX flag (Hennadii Stepanov) 083daf7fbaf02de61f8d197ef6a8df98c1a57f7b Pass bilingual_str argument to AbortNode() (Hennadii Stepanov) d1cca129b4b5b8e4830e442ebaee55dd0660b48a refactor: Use bilingual_str::empty() (Hennadii Stepanov) Pull request description: This PR is a [followup](https://github.com/bitcoin/bitcoin/issues/16218#issuecomment-625919724) of #16224, and it adds `bilingual_str` type argument support to the `AbortNode()` functions. ACKs for top commit: MarcoFalke: ACK 5527be06277647dffe7cda587c4bbfbec2a5c8ca πŸ‘Ÿ Tree-SHA512: bf8b15b14912b1f672e6e588fffa1e6eb6f00b4b23d15d0ced7f18fbdf76919244427feb7217007fe29617049308e13def893a03a87358db819cca9692f59905
2020-06-16pep-8 test/functional/test_framework/util.pyMarcoFalke
Can be reviewed with --word-diff-regex=. -U0
2020-06-16test: refactor: Inline adjust_bitcoin_conf_for_pre_17MarcoFalke
2020-06-16test: use subprocess.run() in test-security-check.pyfanquake
2020-06-16tests: run test-security-check.py in CIfanquake
2020-06-16Merge #19153: test: mempool compatibility testMarcoFalke
16d4b3fd6d5aad18ebb731a5006a15180d3661ef test: mempool.dat compatibility between versions (Ivan Metlushko) Pull request description: Rationale: Verify mempool.dat compatibility between versions The format of mempool.dat has been changed in #18038 The tests verifies the fix made in #18807 and ensures that the file format is compatible between current version and v0.19.1 The test verifies both backward and forward compatibility. This PR also adds a log when we fail to add a tx loaded from mempool.dat. It was useful when debugging this test and could be potentially useful to debug other scenarios as well. Closes #19037 ACKs for top commit: Sjors: tACK 16d4b3fd6d5aad18ebb731a5006a15180d3661ef Tree-SHA512: 00a38bf528c6478cb0da467af216488f83c1e3ca4d9166c109202ea8284023e99d87a3d6e252c4d88d08d9b5ed1a730b3e1970d6e5c0aef526fa7ced40de7490
2020-06-16net: Remove dead logging codeMarcoFalke
fRet is never false, so the dead code can be removed and the return type can be made void
2020-06-16net: Avoid redundant and confusing FAILED logMarcoFalke
Every `return false` is preceeded by a detailed debug log message to explain that a disconnect or misbehavior happened. Logging another generic "FAILED" message seems redundant. Also, the size of the message and the message type has already been logged and is thus redundant as well. Finally, claiming that message processing FAILED seems odd, because the message was fully processed to the point where it was concluded that the peer should be either disconnected or marked as misbehaving.
2020-06-16Merge #19178: Make mininode_lock non-reentrantMarcoFalke
62068381a3b9c065d81300be79abba7aecfdb41b [tests] Make mininode_lock non-reentrant (John Newbery) c67c1f2c032a8efa141d776a7e5be58f052159ea [tests] Don't call super twice in P2PTxInvStore.on_inv() (John Newbery) 9d80762fa0931fe553fad241e95bcc1515ef0e95 [tests] Don't acquire mininode_lock twice in wait_for_broadcast() (John Newbery) edae6075aa3b1169c84b65e76fd48d68242a294e [tests] Only acquire lock once in p2p_compactblocks.py (John Newbery) Pull request description: There's no need for mininode_lock to be reentrant. Use a simpler non-recursive lock. ACKs for top commit: MarcoFalke: ACK 62068381a3b9c065d81300be79abba7aecfdb41b πŸ˜ƒ jonatack: ACK 62068381a3b9c0 Tree-SHA512: dcbc19e6c986970051705789be0ff7bec70c69cf76d5b468c2ba4cb732883ad512b1de5c3206c2eca41fa3f1c4806999df4cabbf67fc3c463bb817458e59a19c
2020-06-16Merge #19260: p2p: disconnect peers that send filterclear + update existing ↡fanquake
filter msg disconnect logic 3a10d935ac8ebabdfd336569d943f042ff84b13e [p2p/refactor] move disconnect logic and remove misbehaving (gzhao408) ff8c430c6589ea72b9e169455cf6437c8623cc52 [test] test disconnect for filterclear (gzhao408) 1c6b787e0319c44f0e0bede3f4a77ac7c2089db2 [netprocessing] disconnect node that sends filterclear (gzhao408) Pull request description: Nodes that don't have bloomfilters turned on (i.e. no `NODE_BLOOM` service) should disconnect peers that send them `filterclear` P2P messages. Non-bloomfilter nodes already disconnect peers for [`filteradd` and `filterload`](https://github.com/bitcoin/bitcoin/blob/19e919217e6d62e3640525e4149de1a4ae04e74f/src/net_processing.cpp#L2218), but #8709 removed `filterclear` so it could be used to reset tx relay. This isn't needed now because using `feefilter` message is much better for this purpose (See #19204). Also refactors existing disconnect logic for `filteradd` and `filterload` into respective message handlers and removes banning for them. ACKs for top commit: jnewbery: Code review ACK 3a10d935ac8ebabdfd336569d943f042ff84b13e naumenkogs: utACK 3a10d93 gillichu: tested ACK: quick test_runner on macOS [`3a10d93`](https://github.com/bitcoin/bitcoin/commit/3a10d935ac8ebabdfd336569d943f042ff84b13e) MarcoFalke: re-ACK 3a10d935ac only change is replacing false with true 🚝 Tree-SHA512: 7aad8b3c0b0e776a47ad52544f0c1250feb242320f9a2962542f5905042f77e297a1486f8cdc3bf0fb93cd00c1ab66a67b2ec426eb6da3fe4cda56b5e623620f
2020-06-15walletdb: Move BDB specific things into bdb.{cpp/h}Andrew Chow
Leave wallet/db.{cpp/h} for generic WalletDatabase stuff. The BDB specific stuff goes into bdb.{cpp/h}