aboutsummaryrefslogtreecommitdiff
AgeCommit message (Collapse)Author
2020-12-17Merge #20658: ci: Move linter task to cirrusMarcoFalke
4045a6722c884be779e86016313061ac6ff80808 ci: Use cpu=1 for linter (Dhruv Mehta) 739d39022d2959c1114c14a0065daebf4fe812c1 ci: Move linter task to cirrus (Dhruv Mehta) Pull request description: Solves #20467: Move linter to Cirrus-CI as Travis-CI.org is shutting down ACKs for top commit: MarcoFalke: ACK 4045a6722c884be779e86016313061ac6ff80808 Tree-SHA512: 9aa7487ac86c91fc68bb584d29134e304dbd46702514a5d47d1ef0de6b877d96d42b7589870fc67ad9a31f5d3a789728446da4418688f336111a9ba0f8de5feb
2020-12-16ci: Use cpu=1 for linterDhruv Mehta
2020-12-16Merge #20575: Do not run functions with necessary side-effects in assert()Wladimir J. van der Laan
5021810650afc3073c2af6953ff046ad4d27a1fc Make CanFlushToDisk a const member function (practicalswift) 281cf995547f7683a9e9186bc6384a9fb6035d10 Do not run functions with necessary side-effects in assert() (practicalswift) Pull request description: Do not run functions with necessary side-effects in `assert()`. ACKs for top commit: laanwj: Code review ACK 5021810650afc3073c2af6953ff046ad4d27a1fc sipa: utACK 5021810650afc3073c2af6953ff046ad4d27a1fc theStack: Code Review ACK 5021810650afc3073c2af6953ff046ad4d27a1fc 🟢 Tree-SHA512: 38b7faccc2f16a499f9b7b1b962b49eb58580b2a2bbf63ea49dcc418a5ecc8f21a0972fa953f66db9509c7239af67cfa2f9266423fd220963d091034d7332b96
2020-12-16Merge #20470: build: Replace genisoimage with xorrisoWladimir J. van der Laan
7587d11ec959f15f469bd396d4ad2697729b4ccd build: remove cdrkit package from depends (fanquake) 0df98191268fe641dd5d12e3a9f517c0c5cfacdd build: Replace genisoimage with xorriso (fanquake) 22437fc72e78ba3845a3953853d40093de32c395 build: Run libdmg-hfsplus's DMG tool in make deploy (Carl Dong) Pull request description: This is a redo of fanquake's https://github.com/bitcoin/bitcoin/pull/18151, which, aside from switching us from the deprecated `genisoimage` to the maintained `xorriso`, is also necessary for Guix to achieve determinism without using faketime. > xorriso and its mkisofs/genisoimage emulation alter-ego xorrisofs are > more maintained, and has the right toggles for us to achieve output > determinism without using blunt tools like faketime. > > In this commit, we use xorrisofs from the build environment rather than > building it ourselves using depends. This is not necessary and can be > changed in the future. > > From wiki.debian.org/genisoimage?action=recall&rev=11 : > > > The classical command line interface for production of ISO 9660 > > filesystem images is the option set established by program mkisofs. > > For reasons of licensing and other problems with its author, Debian > > ships a fork of mkisofs, called genisoimage, which was split off in > > 2006 and then developed independently. > > > > Meanwhile, genisoimage gets no new features and not even bug fixes. It > > is first choice only if its options -udf or -hfs are needed. > > > > Replacement in most uses cases, especially for bootable ISO 9660 > > filesystems, archiving, and backup, is xorrisofs which starts the -as > > mkisofs emulation mode of program xorriso. ACKs for top commit: laanwj: ACK 7587d11ec959f15f469bd396d4ad2697729b4ccd Tree-SHA512: 62f3aad08fa8bf21192e951d7dd33b24975586d76834cfa3498f4b8cdb586cefec8cab2c073d1951a0884b5e182fd71ef2cf3accad98f84455016776ad3c5422
2020-12-16Merge #20651: net: Make p2p recv buffer timeout 20 minutes for all peersMarcoFalke
ea36a453e3d06657679459e1168347648fa7c5c0 [net] Make p2p recv buffer timeout 20 minutes for all peers (John Newbery) Pull request description: The timeout interval for the send and recv buffers was changed from 90 minutes to 20 minutes in commit f1920e86 in 2013, except for peers that did not support the pong message (where the recv buffer timeout remained at 90 minutes). A few observations: - for peers that support BIP 31 (pong messages), this recv buffer timeout is almost redundant with the ping timeout. We send a ping message every two minutes, and set a timeout of twenty minutes to receive the pong response. If the recv buffer was really timing out, then the pong response would also time out. - BIP 31 is supported by all nodes of p2p version 60000 and higher, and has been in widespread use since 2013. I'd be very surprised if there are many nodes on the network that don't support pong messages. - The recv buffer timeout is not specified in any p2p BIP. We're free to set it at any value we want. - A peer that doesn't support BIP 31 and hasn't sent any message to us at all in 90 minutes is unlikely to be useful for us, and is more likely to be evicted AttemptToEvictConnection() since it'll have the worst possible ping time and isn't providing blocks/transactions. Therefore, we remove this check, and set the recv buffer timeout to 20 minutes for all peers. This removes the final p2p version dependent logic from the net layer, so all p2p version data can move into the net_processing layer. Alternative approaches: - Set the recv buffer timeout to 90 minutes for all peers. This almost wouldn't be a behaviour change at all (pre-BIP 31 peers would still have the same recv buffer timeout, and we can't ever reach a recv buffer timeout higher than 21 minutes for post-BIP31 peers, because the pong timeout would be hit first). - Stop supporting peers that don't support BIP 31. BIP 31 has been in use since 2012, and implementing it is trivial. ACKs for top commit: MarcoFalke: review ACK ea36a453e3d06657679459e1168347648fa7c5c0 promag: Code review ACK ea36a453e3d06657679459e1168347648fa7c5c0. practicalswift: cr ACK ea36a453e3d06657679459e1168347648fa7c5c0: patch looks correct ajtowns: ACK ea36a453e3d06657679459e1168347648fa7c5c0 sipa: utACK ea36a453e3d06657679459e1168347648fa7c5c0 jonatack: Code review ACK ea36a453e3d06657679459e1168347648fa7c5c0 Tree-SHA512: df290bb32d2b5d9e59a0125bb215baa92787f9d01542a7437245f1c478c7f9b9831e5f170d3cd0db2811e1b11b857b3e8b2e03376476b8302148e480d81aab19
2020-12-16Merge #20248: test: fix length of R check in key_signature_testsWladimir J. van der Laan
89895773b72275a620951730aef0b52e9437bc13 Fix length of R check in test/key_tests.cpp:key_signature_tests (Dmitry Petukhov) Pull request description: The code before the fix only checked the length of R value of the last signature in the loop, and only for equality (but the length can be less than 32) The fixed code checks that length of the R value is less than or equal to 32 on each iteration of the loop ACKs for top commit: laanwj: ACK 89895773b72275a620951730aef0b52e9437bc13 Tree-SHA512: 73eb2bb4a6c1c5fc11dd16851b28b43037ac06ef8cfc3b1f957429a1fca295c9422d67ec6c73c0e4bb4919f92e22dc5d733e84840b0d01ad1a529aa20d906ebf
2020-12-16Merge #20171: Add functional test test_txid_inv_delayWladimir J. van der Laan
bc4a23008762702ffcd6868bcdb8fe2a732640ba Remove redundant p2p lock tacking for tx download functional tests (Antoine Riard) d3b5eac9a989878e2e09e5fde71c49149b123f18 Add mutation for functional test test_preferred_inv (Antoine Riard) 06efb3163cdf30e74df3f78afc4896b0f55ce937 Add functional test test_txid_inv_delay (Antoine Riard) a07910abcd580ed07187794cf0e1faf040bb4212 test: Makes wtxidrelay support a generic P2PInterface option (Antoine Riard) Pull request description: This is a simple functional test to increase coverage of #19988, checking that txid announcements from txid-relay peers are delayed by TXID_RELAY_DELAY, assuming we have at least another wtxid-relay peer. You can verify new test with the following diff : ``` diff --git a/src/net_processing.cpp b/src/net_processing.cpp index f14db379f..2a2805df5 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -773,7 +773,7 @@ void PeerManager::AddTxAnnouncement(const CNode& node, const GenTxid& gtxid, std auto delay = std::chrono::microseconds{0}; const bool preferred = state->fPreferredDownload; if (!preferred) delay += NONPREF_PEER_TX_DELAY; - if (!gtxid.IsWtxid() && g_wtxid_relay_peers > 0) delay += TXID_RELAY_DELAY; + //if (!gtxid.IsWtxid() && g_wtxid_relay_peers > 0) delay += TXID_RELAY_DELAY; const bool overloaded = !node.HasPermission(PF_RELAY) && m_txrequest.CountInFlight(nodeid) >= MAX_PEER_TX_REQUEST_IN_FLIGHT; if (overloaded) delay += OVERLOADED_PEER_TX_DELAY; ``` ACKs for top commit: laanwj: ACK bc4a23008762702ffcd6868bcdb8fe2a732640ba Tree-SHA512: 150e806bc5289feda94738756ab375c7fdd23c80c12bd417d3112043e26a91a717dc325a01079ebd02a88b90975ead5bd397ec86eb745c7870ebec379a8aa711
2020-12-16build: remove cdrkit package from dependsfanquake
2020-12-16build: Replace genisoimage with xorrisofanquake
xorriso and its mkisofs/genisoimage emulation alter-ego xorrisofs are more maintained, and has the right toggles for us to achieve output determinism without using blunt tools like faketime. In this commit, we use xorrisofs from the build environment rather than building it ourselves using depends. This is not necessary and can be changed in the future. From https://wiki.debian.org/genisoimage?action=recall&rev=11 : > The classical command line interface for production of ISO 9660 > filesystem images is the option set established by program mkisofs. > For reasons of licensing and other problems with its author, Debian > ships a fork of mkisofs, called genisoimage, which was split off in > 2006 and then developed independently. > > Meanwhile, genisoimage gets no new features and not even bug fixes. It > is first choice only if its options -udf or -hfs are needed. > > Replacement in most uses cases, especially for bootable ISO 9660 > filesystems, archiving, and backup, is xorrisofs which starts the -as > mkisofs emulation mode of program xorriso.
2020-12-16Merge #20661: Only select from addrv2-capable peers for torv3 address relayWladimir J. van der Laan
37fe80e6267094f6051ccf9bec0c7f1a6b9e15da Only consider addrv2 peers for relay of non-addrv1 addresses (Pieter Wuille) 83f8821a6f41854edd5c0b11deabba658890cde1 refactor: add IsAddrCompatible() to CNode (Pieter Wuille) Pull request description: When selecting peers to relay an address to, only pick addrv2-capable ones if the address cannot be represented in addr(v1). Without this I expect that propagation of torv3 addresses over the cleartext network will be very hard for a while. ACKs for top commit: jonatack: ACK 37fe80e6267094f6051ccf9bec0c7f1a6b9e15da vasild: ACK 37fe80e6267094f6051ccf9bec0c7f1a6b9e15da Tree-SHA512: 18a854ea43ad473cf89b9c5193b524109d7af75c26f7aa7e26cd72ad0db52f19c8001d566c607a7e6772bc314f770f09b6c3e07282d110c5daea193edc592cd2
2020-12-16Merge #20365: wallettool: add parameter to create descriptors walletMarcoFalke
173cc9b7be335b5dd2cc1bb112dfa6ec5c13ec12 test: walettool create descriptors (Ivan Metlushko) 345e88eecf1b28607d5da3af38e19794a8a115ce wallettool: add param to create descriptors wallet (Ivan Metlushko) 6d3af3ab627096a824cb6a7ca1ebeddc7530361c wallettool: pass in DatabaseOptions into MakeWallet (Ivan Metlushko) Pull request description: Rationale: expose and promote descriptor wallets in more places; make cli tool more consistent with `createwallet` rpc. Add `-descriptors` parameter which is off by default. When specified it will create a new descriptors wallet with sqlite backend, which is consistent with `createwallet` rpc. This PR is based on a suggestion from **ryanofsky** https://github.com/bitcoin/bitcoin/pull/19137#discussion_r516779603 Example: ``` $ ./src/bitcoin-wallet -wallet=fewty -descriptors create Topping up keypool... Wallet info =========== Name: fewty Format: sqlite Descriptors: yes Encrypted: no HD (hd seed available): yes Keypool Size: 6000 Transactions: 0 Address Book: 0 ``` ``` $ ./src/bitcoin-wallet -wallet=fewty create Topping up keypool... Wallet info =========== Name: fewty Format: bdb Descriptors: no Encrypted: no HD (hd seed available): yes Keypool Size: 2000 Transactions: 0 Address Book: 0 ``` ACKs for top commit: achow101: ACK 173cc9b7be335b5dd2cc1bb112dfa6ec5c13ec12 ryanofsky: Code review ACK 173cc9b7be335b5dd2cc1bb112dfa6ec5c13ec12. This seems pretty nicely implemented now, with opportunities to clean up more and dedup later MarcoFalke: Concept ACK 173cc9b7be335b5dd2cc1bb112dfa6ec5c13ec12 🌠 Tree-SHA512: cc32ba336ff709de2707ee15f495b4617908e8700ede8401a58e894f44cda485c544d644023c9a6604d88a62db9d92152383ee2e8abf691688c25cf6e222c622
2020-12-16Merge #20605: init: Signal-safe instant shutdownWladimir J. van der Laan
cd03513dc2fcccaa142e9632a28b38efd0056436 init: Signal-safe instant shutdown (Wladimir J. van der Laan) Pull request description: Replace the 200ms polling loop with a faster and more efficient waiting operation. This should speed up short RPC tests. This change has been tried a few times before, but abandoned every time because solutions used a condition variable which is not safe for use in signals, as they need to be reentrant. On UNIX-ish OSes, use a safe way: a pipe. When shutdown is requested write a dummy byte to the pipe. Waiting for shutdown is a matter of a blocking read from the pipe. On Windows, there are no signals so using a condition variable is safe. This only affects bitcoind. The GUI is unaffected by this change, and keeps polling as before in `BitcoinGUI::detectShutdown()`. It might be possible to listen to a pipe there, too, but I'm not sure, and it's complicated by the GUI-node abstraction. ACKs for top commit: jonatack: ACK cd03513dc2fcccaa142e9632a28b38efd0056436 tested on Debian 5.9.11-1 (2020-11-27) x86_64 GNU/Linux Tree-SHA512: ed2f532f69fec4855c17bf7b8f3d0eb96e78ee2a3c13d374dd2c6add06e3ad6a190da8ed8f9d7a76532cf998222d67f57e35b206aec29675e96437448ae7e13c
2020-12-16Merge #20569: test: Fix intermittent wallet_multiwallet issue with ↵MarcoFalke
got_loading_error fab48da908f3f81135b9163edf5011d1e5f6ef6e test: Fix intermittent wallet_multiwallet issue with got_loading_error (MarcoFalke) fa8e15f7b75e35846b86e8627a3612e31eb22dcb test: pep8 wallet_multiwallet.py (MarcoFalke) Pull request description: Failing the test after 10 iterations without a loading error is problematic because it may take 11 iterations to get a loading error. Fix that by running until a loading error occurs, which should happen in almost all runs within the first 10 iterations. ACKs for top commit: ryanofsky: Code review ACK fab48da908f3f81135b9163edf5011d1e5f6ef6e. This seems like a good workaround. I think more ideally think load and unload RPCs would not have racy status reporting (suggested previously https://github.com/bitcoin/bitcoin/pull/19300#pullrequestreview-435362710 and Tree-SHA512: 6b80b26d916276efe2a01af93bca7dbf71a3e67db9d3deb15175070719bf7d1325a1410d93e74c0316942e388faa2ba185dc9d3759c82d1c73c3c509b9997f05
2020-12-16Merge #20601: doc: Update for FreeBSD 12.2, add GUI Build InstructionsWladimir J. van der Laan
c17569056105d221053a839d5430df5b3e94f746 doc: Update for FreeBSD 12.2, add GUI Build Instructions (Jarol Rodriguez) Pull request description: The current FreeBSD Build documentation is a little outdated and underwhelming. This PR intends to keep the build-freebsd.md doc up to date. Here are the main improvements: - Introduce dependency information - New instructions for building the GUI - Instructions for supporting descriptor wallets - Various notes on the build and compile process **Before/Master:** [render](https://github.com/bitcoin/bitcoin/blob/master/doc/build-freebsd.md) **After/PR:** [render](https://github.com/bitcoin/bitcoin/blob/2e8b9a5aac2a2c8926216a971c87ea8f8c00cb1b/doc/build-freebsd.md) ACKs for top commit: laanwj: utACK c17569056105d221053a839d5430df5b3e94f746 Tree-SHA512: 64aeb743c7f4ed167451454564b53d13c5c30d82bd2423799655c7b5b465a75733072fb0c574927c2588a46e89a68c6c57b008220acfd25336d445c9dc309f3b
2020-12-16Merge #20276: test: run mempool_expiry.py even with wallet disabledMarcoFalke
3b064fcb9dd3df6c438a440f0fea86e9cf7b5f57 test: run mempool_expiry.py even with wallet disabled (Michael Dietz) Pull request description: Run the mempool expiry test even when the wallet was not compiled, as proposed in https://github.com/bitcoin/bitcoin/issues/20078. ACKs for top commit: MarcoFalke: ACK 3b064fcb9dd3df6c438a440f0fea86e9cf7b5f57 Tree-SHA512: 5860dc021d02bc3752268ec1e859505bec87174953223b34b1af8a8e4ab66d645458fbf9571c0b816a9de891c3ff41314996e580869671fccd6972c093e78154
2020-12-16Merge #20650: depends: Drop workaround for a fixed bug in Qt build systemJonas Schnelli
267f259c0dfbd348340d49e9a89b8684b994e22a depends: Drop workaround for a fixed bug in Qt build system (Hennadii Stepanov) Pull request description: This PR drops workaround that was [introduced](https://github.com/bitcoin/bitcoin/pull/4592/commits/1dec09b341f61836147d87656aea7f7be02aab6d) for Qt 5.2.1 for a bug in Qt build system that has been fixed in Qt 5.3.0. The bug reports: - https://bugreports.qt.io/browse/QTBUG-35444 - https://bugreports.qt.io/browse/QTBUG-32519 I've noted this change is a part of the #19716, but I think that a separate commit with the documented reason will benefit it. ACKs for top commit: laanwj: Code review ACK 267f259c0dfbd348340d49e9a89b8684b994e22a jonasschnelli: code Review ACK 267f259c0dfbd348340d49e9a89b8684b994e22a practicalswift: cr ACK 267f259c0dfbd348340d49e9a89b8684b994e22a: patch looks correct Tree-SHA512: b994f94776b4f8bb2f996095c87c7fef55e74d1e64852a890d664275e3739ec890ee388b10baa15445dd24ec7b971ce57d396cb062dbed933c18b6b69525349f
2020-12-16Merge #20477: net: Add unit testing of node eviction logicMarcoFalke
fee88237e03c21bf81f21098e6b89ecfa5327cee Assert eviction at >= 29 candidates. Assert non-eviction at <= 20 candidates. (practicalswift) 685c428de0fb63ca6ec1419bb112f07d27bcdf14 test: Add unit testing of node eviction logic (practicalswift) ed73f8cee0d7b7facbd2e8dde24a237f20c48c0c net: Move eviction node selection logic to SelectNodeToEvict(...) (practicalswift) Pull request description: Add unit testing of node eviction logic. Closes #19966. ACKs for top commit: jonatack: ACK fee88237e03c21bf81f21098e6b89ecfa5327cee MarcoFalke: ACK fee88237e03c21bf81f21098e6b89ecfa5327cee 🐼 Tree-SHA512: 0827c35609122ca42bfabb17feaaee35c191ab4dc2e428282af425a6c176eaeaff2789b74a4f7eb4ca6b8cb10603068e90ca800e6ef3bc3904d50e76616f7a2b
2020-12-16Assert eviction at >= 29 candidates. Assert non-eviction at <= 20 candidates.practicalswift
2020-12-16test: Add unit testing of node eviction logicpracticalswift
2020-12-16net: Move eviction node selection logic to SelectNodeToEvict(...)practicalswift
2020-12-15Merge #20611: Move TX_MAX_STANDARD_VERSION to policyWladimir J. van der Laan
fade6195b1c230edd561443637a7bde81c2594a4 Move TX_MAX_STANDARD_VERSION to policy (MarcoFalke) Pull request description: `primitives` should only be used for the raw datastructures (parsing and format). It is not the right place to document relay policy. ACKs for top commit: laanwj: Code review ACK fade6195b1c230edd561443637a7bde81c2594a4 lontivero: Concept ACK https://github.com/bitcoin/bitcoin/pull/20611/commits/fade6195b1c230edd561443637a7bde81c2594a4 Tree-SHA512: f809c4aecd14d7e9feaa7b50b9c0697232991eef36190cd960bcfb0ad6e20c71a4f6aab48c7747cf8a681eb14feda60c55b09a37f128673d519567224f29cd97
2020-12-15Merge #20615: cirrus: Schedule one task with paid credits for faster CI feedbackWladimir J. van der Laan
faf2c6e32e5b27cf936c9b26e6059027dc020374 cirrus: Schedule one task with paid credits for faster CI feedback (MarcoFalke) Pull request description: During times of high activity in the repo, the scheduling of Cirrus CI tasks might put them a few hours in the future. This is fine when all the tasks eventually pass. Though for failing tasks, a failure should ideally be shown to the author and reviewer as soon as possible. Compute credits can be used to schedule immediately: https://cirrus-ci.org/pricing/#compute-credits. Running all tasks with compute credits will probably be more expensive than our previous CI invoice. However, they are also more flexible. As a start we could enable only a single task and revisit/re-evaluate the next steps in a month. ACKs for top commit: laanwj: Concept ACK faf2c6e32e5b27cf936c9b26e6059027dc020374 fanquake: ACK faf2c6e32e5b27cf936c9b26e6059027dc020374 practicalswift: cr ACK faf2c6e32e5b27cf936c9b26e6059027dc020374: patch looks correct Tree-SHA512: df599e1c4cc0394f7f03413ad29954ddc87b163b02640d8bfc0497a5dc8d237b8c963c1dd9d01ac83c4a044f575300763097dac2ea6d1a4a163a1cece342b743
2020-12-15Only consider addrv2 peers for relay of non-addrv1 addressesPieter Wuille
2020-12-15refactor: add IsAddrCompatible() to CNodePieter Wuille
2020-12-15Merge #20660: Move signet onion seed from v2 to v3Wladimir J. van der Laan
3e6657a14d501c6315ab46ffe7d204684491c710 Move signet onion seed from v2 to v3 (Sjors Provoost) Pull request description: Since v0.21 hidden services use the longer v3 address format. It may make sense to backport this to the v0.21 branch, although onion nodes can always use the non-onion seeds. ACKs for top commit: MarcoFalke: Concept ACK 3e6657a14d501c6315ab46ffe7d204684491c710 laanwj: ACK 3e6657a14d501c6315ab46ffe7d204684491c710 Saibato: tACK https://github.com/bitcoin/bitcoin/commit/3e6657a14d501c6315ab46ffe7d204684491c710 :+1: Tree-SHA512: 9bb4d82345ab25d6ea971f8f106c30778ade2ba11a292e8d7449ea39581b91e51c4b34851b5c06f5dfbe07e7b04e5dc92c48c98c47a1c7ef3d5f350c3a3ad4f7
2020-12-15ci: Move linter task to cirrusDhruv Mehta
2020-12-15Merge #20560: fuzz: Link all targets onceMarcoFalke
fa13e1b0c52738492310b6b421d8e38cb04da5b1 build: Add option --enable-danger-fuzz-link-all (MarcoFalke) 44444ba759480237172d83f42374c5c29c76eda0 fuzz: Link all targets once (MarcoFalke) Pull request description: Currently the linker is invoked more than 150 times when compiling with `--enable-fuzz`. This is problematic for several reasons: * It wastes disk space north of 20 GB, as all libraries and sanitizers are linked more than 150 times * It wastes CPU time, as the link step can practically not be cached (similar to ccache for object files) * It makes it a blocker to compile the fuzz tests by default for non-fuzz builds #19388, for the aforementioned reasons * The build file is several thousand lines of code, without doing anything meaningful except listing each fuzz target in a highly verbose manner * It makes writing new fuzz tests unnecessarily hard, as build system knowledge is required; Compare that to boost unit tests, which can be added by simply editing an existing cpp file * It encourages fuzz tests that re-use the `buffer` or assume the `buffer` to be concatenations of seeds, which increases complexity of seeds and complexity for the fuzz engine to explore; Thus reducing the effectiveness of the affected fuzz targets Fixes #20088 ACKs for top commit: practicalswift: Tested ACK fa13e1b0c52738492310b6b421d8e38cb04da5b1 sipa: ACK fa13e1b0c52738492310b6b421d8e38cb04da5b1. Reviewed the code changes, and tested the 3 different test_runner.py modes (run once, merge, generate). I also tested building with the new --enable-danger-fuzz-link-all Tree-SHA512: 962ab33269ebd51810924c51266ecc62edd6ddf2fcd9a8c359ed906766f58c3f73c223f8d3cc49f2c60f0053f65e8bdd86ce9c19e673f8c2b3cd676e913f2642
2020-12-15Merge #20653: doc: Move addr relay comment in net to correct placeMarcoFalke
fa86217e97234aac6f815a6768afc1b87b8b2ae8 doc: Move add relay comment in net to correct place (MarcoFalke) Pull request description: The comment was previously attached to `m_addr_known`, but now it is attached to `id`, which is wrong. Fix that by moving the comment to `RelayAddrsWithConn`. ACKs for top commit: practicalswift: cr ACK fa86217e97234aac6f815a6768afc1b87b8b2ae8: patch looks correct jnewbery: ACK fa86217e97 theStack: Code review ACK fa86217e97234aac6f815a6768afc1b87b8b2ae8 🌳 Tree-SHA512: ec3d5f1996aded38947d2a5fd0bb63539e88f83964cd3254984002edfd51abb4dde813c7c81619a8a3a5c55b7e9ae83c8c5be8ad6c84b4593ed3bbf463fe8979
2020-12-15Merge #20616: Check CJDNS address is validWladimir J. van der Laan
f7264fff0a098f8b6354c7373b8790791c25dd07 Check if Cjdns address is valid (Lucas Ontivero) Pull request description: CJDNS addresses start with 0xFC and for that reason if a netaddr was unserialized with network type cjdns but its address prefix is not 0xFC then that netaddr should be considered invalid. ACKs for top commit: jonatack: ACK f7264fff0a098f8b6354c7373b8790791c25dd07 practicalswift: cr ACK f7264fff0a098f8b6354c7373b8790791c25dd07: patch looks correct theStack: ACK f7264fff0a098f8b6354c7373b8790791c25dd07 ✔️ Tree-SHA512: 5300df2ffbbd69c40271b6d8df96cca98eb3e1ee76aba62c9c76025d083788ab1f1332775890c63b06e02ca593863a867cd53956bce5962383e8450487898669
2020-12-15init: Signal-safe instant shutdownWladimir J. van der Laan
Replace the 200ms polling loop with a faster and more efficient waiting operation. This was tried a few times before, but given up every time because solutions use a condition variable which is not safe for use in signals as they need to be reentrant. On UNIX-ish OSes, use a safe way: a pipe. When shutdown is requested write a dummy byte to the pipe. Waiting for shutdown is a matter of a blocking read from the pipe. On Windows, there are no signals so using a condition variable is safe.
2020-12-15Merge #20437: fuzz: Avoid time-based "non-determinism" in fuzzing harnesses ↵MarcoFalke
by using mocked GetTime() 8c09c0c1d18885ef94f79b3f2d073f43269bc95d fuzz: Avoid time-based "non-determinism" in fuzzing harnesses by using mocked GetTime() (practicalswift) Pull request description: Avoid time-based "non-determinism" in fuzzing harnesses by using mocked `GetTime()`. Prior to this commit the fuzzing harnesses `banman`, `connman`, `net` and `rbf` had time-based "non-determinism". `addrman` is fixed in #20425. `process_message` and `process_messages` are left to fix: simply using mock time is not enough for them due to interaction with `IsInitialBlockDownload()`. 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 :) ACKs for top commit: MarcoFalke: review ACK 8c09c0c1d18885ef94f79b3f2d073f43269bc95d practicalswift: > review ACK [8c09c0c](https://github.com/bitcoin/bitcoin/commit/8c09c0c1d18885ef94f79b3f2d073f43269bc95d) Tree-SHA512: 32dfbead3dfd18cf4ff56dc2ea341aa977441b4e19a54879cf54fa5820c7e2b14b92c7e238d32fd785654f3b28cc82826ae66c03e94c292633c63c41196ba9a8
2020-12-15Move signet onion seed from v2 to v3Sjors Provoost
2020-12-15Merge #20253: net: use std::chrono throughout maxOutbound logicfanquake
0475c8ba4d10dce79092361bc4c23c11dadba39a net: use std::chrono throughout maxOutbound logic (fanquake) f805933e70d4320e62739f4cecfc08bdbad8afaa init: set nMaxOutboundLimit connection option directly (fanquake) 173d0d35f1bb271175047c9eb7dd58cc46ed35bf net: remove nMaxOutboundTimeframe from connection options (fanquake) b117eb148678b0fc5be02346cef29d87d4f81af9 net: remove SetMaxOutboundTimeframe (fanquake) 2f3f1aec1f8aadd4a6fb08ca5da7eeda31eb388f net: remove SetMaxOutboundTarget (fanquake) Pull request description: Switch to using `std::chrono` types for the max outbound related logic. Removes some unnecessary code from init. ACKs for top commit: jnewbery: utACK 0475c8ba4d10dce79092361bc4c23c11dadba39a MarcoFalke: review ACK 0475c8ba4d10dce79092361bc4c23c11dadba39a 🎭 Tree-SHA512: 5a6d5b61e0d4c08a235cfc0257dae65d09a5df019d8d230b1a58a3e2483ddf4a31efdefc885c4a02e4715e4180b0ed92ebc0a1c08b2bf476a391945114593514
2020-12-15Merge bitcoin-core/gui#115: Replace "Hide tray icon" option with positive ↵Jonas Schnelli
"Show tray icon" one 03edb52eee5a87af16161c23bdc6cde91a2e5b8b qt: Remove redundant BitcoinGUI::setTrayIconVisible (Hennadii Stepanov) 17174f8328c44ae84479e8365c122ad8502bd7da gui: Replace "Hide tray icon" option with positive "Show tray icon" one (Hennadii Stepanov) Pull request description: This change makes easier both (1) using this option, and (2) reasoning about the code. ACKs for top commit: jonasschnelli: utACK 03edb52eee5a87af16161c23bdc6cde91a2e5b8b Tree-SHA512: 38e317492210d4fb13302dea383bd1f4f0ae1219d7ff2fdcb78607f15ac61a51969acaadb59b72c3f075b6356ef54368eb46fb49e6e1bd42db6d5804b97e232b
2020-12-14Merge #20594: Fix getauxval calls in randomenv.cppWladimir J. van der Laan
836a3dc02c72f917db5be386b9b4787a59d48610 Avoid weak-linked getauxval support on non-linux platforms (like macOS) (Jonas Schnelli) 41a413b31746cc749f3c64ed8070cea9cc6cfdbe Define correct symbols for getauxval (Jonas Schnelli) Pull request description: PR #20358 made use of the two preprocessor symbols `HAVE_STRONG_GETAUXVAL` as well as `HAVE_WEAK_GETAUXVAL`. These symbols have not been defined in configure.ac. They where only passed selective as CRC32 CPPFLAGS in https://github.com/bitcoin/bitcoin/blob/master/src/Makefile.crc32c.include#L16. PR #20358 would have broken the macOS build since `getauxval` is not supported on macOS (but weak-linking does pass). This PR defines the two symbols correctly and reduces calls to `getauxval` to linux. ACKs for top commit: laanwj: Code review ACK 836a3dc02c72f917db5be386b9b4787a59d48610 jonatack: utACK 836a3dc02c72f917db5be386b9b4787a59d48610 Tree-SHA512: 6527f4a617b937f4c368a3cb1c162f1ac38a6f5e6341295554961eaf322906e9b27398a6f7b00819854ceebb5c828d3e6ce0a779edd769adc4053ce8beda3739
2020-12-14Check if Cjdns address is validLucas Ontivero
2020-12-14doc: Move add relay comment in net to correct placeMarcoFalke
Can be reviewed with --ignore-all-space --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space
2020-12-14Merge #20599: net processing: Tolerate sendheaders and sendcmpct messages ↵MarcoFalke
before verack b316dcb758e3dbd302fbb5941a1b5b0ef5f1f207 [net processing] Tolerate sendheaders and sendcmpct messages before verack (John Newbery) Pull request description: BIP 130 (sendheaders) and BIP 152 (compact blocks) do not specify at which stage the `sendheaders` or `sendcmpct` messages should be sent. Therefore we should tolerate them being sent before the version-verack handshake is complete. ACKs for top commit: MarcoFalke: review ACK b316dcb758e3dbd302fbb5941a1b5b0ef5f1f207 📒 jonatack: Code review re-ACK b316dcb758e3dbd302fbb5941a1b5b0ef5f1f207 per `git range-diff b103fdcb 82d0a43 b316dcb`, rebase only luke-jr: utACK b316dcb758e3dbd302fbb5941a1b5b0ef5f1f207, only code movement from after verack check to before Tree-SHA512: a8da3990a786a53c195a33cd278eb9d1b6b09e9a33717674d5bc6ef5629811189216f3eda1a3dd11f6972b9680c7b7a532bf7343b6e7440009dd831bef128e1d
2020-12-14build: Add option --enable-danger-fuzz-link-allMarcoFalke
2020-12-14[net processing] Tolerate sendheaders and sendcmpct messages before verackJohn Newbery
BIP 130 (sendheaders) and BIP 152 (compact blocks) do not specify at which stage the `sendheaders` or `sendcmpct` messages should be sent. Therefore we should tolerate them being sent before the version-verack handshake is complete.
2020-12-14[net] Make p2p recv buffer timeout 20 minutes for all peersJohn Newbery
The timeout interval for the send and recv buffers was changed from 90 minutes to 20 minutes in commit f1920e86 in 2013, except for peers that did not support the pong message (where the recv buffer timeout remained at 90 minutes). A few observations: - for peers that support BIP 31 (pong messages), this recv buffer timeout is almost redundant with the ping timeout. We send a ping message every two minutes, and set a timeout of twenty minutes to receive the pong response. If the recv buffer was really timing out, then the pong response would also time out. - BIP 31 is supported by all nodes of p2p version 60000 and higher, and has been in widespread use since 2013. I'd be very surprised if there are many nodes on the network that don't support pong messages. - The recv buffer timeout is not specified in any p2p BIP. We're free to set it at any value we want. - A peer that doesn't support BIP 31 and hasn't sent any message to us at all in 90 minutes is unlikely to be useful for us, and is more likely to be evicted AttemptToEvictConnection() since it'll have the worst possible ping time and isn't providing blocks/transactions. Therefore, we remove this check, and sent the recv buffer timeout to 20 minutes for all peers. This removes the final p2p version dependent logic from the net layer, so all p2p version data can move into the net_processing layer. Alternative approaches: - Set the recv buffer timeout to 90 minutes for all peers. This almost wouldn't be a behaviour change at all (pre-BIP 31 peers would still have the same recv buffer timeout, and we can't ever reach a recv buffer timeout higher than 21 minutes for post-BIP31 peers, because the pong timeout would be hit first). - Stop supporting peers that don't support BIP 31. BIP 31 has been in use since 2012, and implementing it is trivial.
2020-12-14Merge #19763: net: don't try to relay to the address' originatorMarcoFalke
7fabe0f359ae16ed36ce4ca2c33631d038c21448 net: don't relay to the address' originator (Vasil Dimov) Pull request description: For each address to be relayed we "randomly" pick 2 nodes to send the address to (in `RelayAddress()`). However we do not take into consideration that it does not make sense to relay the address back to its originator (`CNode::PushAddress()` will do nothing in that case). This means that if the originator is among the "randomly" picked nodes, then we will relay to one node less than intended. Fix this by skipping the originating node when choosing candidates to relay to. ACKs for top commit: sdaftuar: ACK 7fabe0f359ae16ed36ce4ca2c33631d038c21448 (this time I looked at the test, and verified the test breaks in expected ways if I break the code). jnewbery: utACK 7fabe0f359ae16ed36ce4ca2c33631d038c21448 (only net_processing changes. I haven't reviewed the test changes) jonatack: re-ACK 7fabe0f359ae16ed36ce4ca2c33631d038c21448 per `git range-diff b76abae fd897f8 7fabe0f`, change since last review is rebase and more readable Doxygen documentation Tree-SHA512: c6a9d11c7afc97ab4e8960513f6416648d4a8c0c64b713c145a7482a7b9e54946f81386a3351e3ec0011e5594ba5ccff4d10c6f656bb80680d9f0d0a63366165
2020-12-14depends: Drop workaround for a fixed bug in Qt build systemHennadii Stepanov
The bug reports: - https://bugreports.qt.io/browse/QTBUG-35444 - https://bugreports.qt.io/browse/QTBUG-32519 Fixed in Qt 5.3.0 (the workaround was introduced for Qt 5.2.1).
2020-12-14Merge #20624: net processing: Remove nStartingHeight check from block relayMarcoFalke
f6360088de8ca02f9d198da2f8cca4ea8d64c992 [net processing] Clarify UpdatedBlockTip() (John Newbery) 94d2cc35be98d3b20db88b2a3745322e5b0aa9d4 [net processing] Remove unnecesary nNewHeight variable in UpdatedBlockTip() (John Newbery) 8b57013473c44fc87c9f1c4611224a89187c289a [net processing] Remove nStartingHeight check from block relay (John Newbery) Pull request description: nStartingHeight was introduced in commit 7a47324c7 (Bitcoin version 0.2.9, P2P version 209) with the comment "better prevention of inventory relaying during initial download". At that time, there was no function to determine whether the node was still in Initial Block Download, so to prevent syncing nodes from relaying old blocks to their peers, a check was added to never relay a block to a peer where the height was lower than 2000 less than the peer's best block. That check was updated several times in later commits to ensure that we weren't relaying blocks before the latest checkpoint if the peer didn't provide a startingheight. The checkpoint comparison was changed to compare with an estimate of the highest block in commit eae82d8e. In commit 202e0194, all block relay was gated on being out of Initial Block Download. In commit 0278fb5f, the comparison to nBlockEstimate was removed since "we already checked IsIBD()". We can remove the check against nStartingHeight entirely. If the node is out of Initial Block Download, then its tip height must have been within 24 hours of current time, so should not be more than ~144 blocks behind the most work tip. This simplifies moving block inventory state into the `Peer` object (#19829). ACKs for top commit: Sjors: utACK f636008 jonatack: ACK f6360088de8ca02f9d198da2f8cca4ea8d64c992 MarcoFalke: ACK f6360088de8ca02f9d198da2f8cca4ea8d64c992 💽 ariard: Code Review ACK f636008 Tree-SHA512: 4959cf35f1dcde46f34bffec1375729a157e1b2a1fd8a8ca33da9771c3c89a6c43e7050cdeeab8d90bb507b0795703db8c8bc304a1a5065ef00aae7a6992ca4f
2020-12-14Merge #20592: doc: update wtxidrelay documentation per BIP339MarcoFalke
4b7b58b3fef5b9566a475871c441ed6ae73af89e Update net_processing WTXID documentation per BIP339 (Jon Atack) Pull request description: BIP339 currently states: *The wtxidrelay message MUST be sent in response to a version message from a peer whose protocol version is >= 70016 and prior to sending a verack. A wtxidrelay message received after a verack message MUST be ignored or treated as invalid.* ACKs for top commit: MarcoFalke: ACK 4b7b58b3fef5b9566a475871c441ed6ae73af89e practicalswift: ACK 4b7b58b3fef5b9566a475871c441ed6ae73af89e RiccardoMasutti: ACK 4b7b58b Tree-SHA512: 58ca6b197618cc73c70aa5de0a2d9d89a68b4cad9d5a708278ef17a9d6854d4362bcc384b6d29696642924977204a8fc120b31e91e2d97b6072b7b0d41c9f2dc
2020-12-14Merge #20617: p2p: Remove m_is_manual_connection from CNodeStatefanquake
a33442fdc73eabd1c5596ab92954344edc9517e6 Remove m_is_manual_connection from CNodeState (Antoine Riard) Pull request description: Currently, this member is only used to exclude MANUAL peers from discouragement in MaybePunishNodeForBlock(). Manual connections are already protected in MaybeDiscourageAndDisconnect(), independently from their network processing behaviors. ACKs for top commit: MarcoFalke: cr ACK a33442fdc73eabd1c5596ab92954344edc9517e6 promag: Code review ACK a33442fdc73eabd1c5596ab92954344edc9517e6. jnewbery: utACK a33442fdc73eabd1c5596ab92954344edc9517e6 amitiuttarwar: code review ACK a33442fdc73eabd1c5596ab92954344edc9517e6 Tree-SHA512: cfe3f3dfa131373e3299002d34ae9e22ca6e1a966831bab32fcf06ff1d08f06095b4ab020cc4d267f3ec05ae23fbdc22373382ab828b999c0db11b8c842a4f0c
2020-12-13net: use std::chrono throughout maxOutbound logicfanquake
2020-12-13init: set nMaxOutboundLimit connection option directlyfanquake
DEFAULT_MAX_UPLOAD_TARGET is a compile time constant.
2020-12-13net: remove nMaxOutboundTimeframe from connection optionsfanquake
It's not actually possible to change this value, so remove the indirection of it being a conn option. DEFAULT_MAX_UPLOAD_TIMEFRAME is a compile time constant.
2020-12-13net: remove SetMaxOutboundTimeframefanquake
This was introduced in 872fee3fccc8b33b9af0a401b5f85ac5504b57eb and it's unclear if it's ever been used.