aboutsummaryrefslogtreecommitdiff
AgeCommit message (Collapse)Author
2020-09-02scripted-diff: Rename `OUTBOUND` ConnectionType to `OUTBOUND_FULL_RELAY`Amiti Uttarwar
-BEGIN VERIFY SCRIPT- sed -i 's/OUTBOUND, /OUTBOUND_FULL_RELAY, /g' src/net.h sed -i 's/ConnectionType::OUTBOUND/ConnectionType::OUTBOUND_FULL_RELAY/g' src/test/net_tests.cpp src/test/fuzz/process_message.cpp src/test/fuzz/process_messages.cpp src/net.cpp src/test/denialofservice_tests.cpp src/net.h src/test/fuzz/net.cpp -END VERIFY SCRIPT-
2020-09-02Merge #19840: Avoid callback when -blocknotify is emptyWladimir J. van der Laan
413e0d1d31ede6a9b539d63ec814b6e8044e35e2 Avoid callback when -blocknotify is empty (João Barbosa) Pull request description: ACKs for top commit: MarcoFalke: ACK 413e0d1d31ede6a9b539d63ec814b6e8044e35e2 practicalswift: ACK 413e0d1d31ede6a9b539d63ec814b6e8044e35e2 -- patch looks correct laanwj: Code review ACK 413e0d1d31ede6a9b539d63ec814b6e8044e35e2 Tree-SHA512: 915e796666b4e74dbb029ba5436e5573a4b881aad9e118f737bcff4024528b7ff3b00dd035138f63d30963cfd66195f6e53a2dbe429ee28cb6f0b9cc47218ecf
2020-09-02Merge #19685: depends: CMake invocation cleanupfanquake
b8936883573708059357a66f67fad9dc77a8bade depends: Specify LDFLAGS to cmake as well (Carl Dong) b3f541f618fe1f3d44baf6a0dd4299173c81f752 depends: Prepend CPPFLAGS to C{,XX}FLAGS for CMake (Carl Dong) 8e121e550953711cd03d7b6c221afd065c325c5e depends: Cleanup CMake invocation (Carl Dong) 8c7cd0c6d9f295bcb6913e3c69c9dac4ce2b25ce depends: More robust cmake invocation (Carl Dong) 3ecf0eca634601da216b06f091f95456c047f39c depends: Use $($(package)_cmake) instead of cmake (Carl Dong) Pull request description: - Use `$($(package)_cmake)` instead of invoking `cmake` directly - Use well-known env vars instead of overriding CMake variables ACKs for top commit: ryanofsky: Code review ACK b8936883573708059357a66f67fad9dc77a8bade. Only changes since last review are new commits adding whitespace, cppflags and ldflags to cmake invocation Tree-SHA512: cfcd8cc9dcd0b336cf48b82fca9fe4bbc7930ed397cb7a68a07066680eb4c1906a6a9b5bd2589b4b4999e8f16232fa30ee9b376b60f4456d0fff931fbf9cc19a
2020-09-02Merge #19857: net: improve nLastBlockTime and nLastTXTime documentationfanquake
d780293e1ee0f9e66bd2d88914694c17f9aaa0ca net: improve nLastBlockTime and nLastTXTime documentation (Jon Atack) Pull request description: Follow-up to #19731 to help alleviate confusion around `nLastBlockTime` and `nLastTXTime`, now also provided by the JSON-RPC API as `last_block` and `last_transaction` in `getpeerinfo` output. Thanks to John Newbery, credited in the commit, and to Dave Harding and Adam Jonas during discussions on how to best explain these in this week's Optech newsletter. ACKs for top commit: practicalswift: ACK d780293e1ee0f9e66bd2d88914694c17f9aaa0ca MarcoFalke: ACK d780293e1ee0f9e66bd2d88914694c17f9aaa0ca harding: ACK d780293e1ee0f9e66bd2d88914694c17f9aaa0ca . The added documentation matches my reading of the code and answers a question I had after seeing #19731 0xB10C: ACK d780293e1ee0f9e66bd2d88914694c17f9aaa0ca Tree-SHA512: 72d47cf50a099913c7e4753cb80e11785b26fb66fa3a8b6c382fde4ea725116f3d215f93d32a567246d269768e66159f8dcf017a1bbc6d5f2489a35f81c316fa
2020-09-02Merge #19610: p2p: refactor AlreadyHave(), CInv::type, INV/TX processingWladimir J. van der Laan
fb56d37612dea6666e7da73d671311a697570dae p2p: ensure inv is GenMsgTx before ToGenTxid in inv processing (John Newbery) aa3621385ee66c9dde5c632c0a79fba3a6ea2d62 test: use CInv::MSG_WITNESS_TX flag in p2p_segwit (Jon Atack) 24ee4f01eadb870435712950a1364cf0def06e9f p2p: make gtxid(.hash) and fAlreadyHave localvars const (Jon Atack) b1c855453bf2634e7fd9b53c4a76a8536fc9865d p2p: use CInv block message helpers in net_processing.cpp (Jon Atack) acd66421671e42a58e8e067868e1ab86268e3231 [net processing] Change AlreadyHaveTx() to take a GenTxid (John Newbery) 5fdfb80b861e0de3fcf8a57163b3f52af4b2df3b [net processing] Change AlreadyHaveBlock() to take block_hash argument (John Newbery) 430e183b89d00b4148f0b77a6fcacca2cd948202 [net processing] Remove mempool argument from AlreadyHaveBlock() (John Newbery) 42ca5618cae0fd9ef97d2006b17d896bc58cc17c [net processing] Split AlreadyHave() into separate block and tx functions (John Newbery) 39f1dc944554218911b0945fff7e6d06f3dab284 p2p: remove nFetchFlags from NetMsgType TX and INV processing (Jon Atack) 471714e1f024fb3b4892a7a8b34a76b83a13fa19 p2p: add CInv block message helper methods (Jon Atack) Pull request description: Building on #19590 and the recent `wtxid` and `GenTxid` changes, this is a refactoring and cleanup PR to simplify and improve some of the net processing code. Some of the diffs are best reviewed with `-w` to ignore spacing. Co-authored by John Newbery. ACKs for top commit: laanwj: Code review ACK fb56d37612dea6666e7da73d671311a697570dae jnewbery: utACK fb56d37612dea6666e7da73d671311a697570dae vasild: ACK fb56d3761 Tree-SHA512: ba39b58e6aaf850880a842fe5f6295e9f1870906ef690206acfc17140aae2ac854981e1066dbcd4238062478762fbd040ef772fdc2c50eea6869997c583e6a6d
2020-09-02Merge #14687: zmq: enable tcp keepaliveJonas Schnelli
c276df775914e4e42993c76e172ef159e3b830d4 zmq: enable tcp keepalive (mruddy) Pull request description: This addresses https://github.com/bitcoin/bitcoin/issues/12754. These changes enable node operators to address the silent dropping (by network middle boxes) of long-lived low-activity ZMQ TCP connections via further operating system level TCP keepalive configuration. For example, ZMQ sockets that publish block hashes can be affected in this way due to the length of time it sometimes takes between finding blocks (e.g.- sometimes more than an hour). Prior to this patch, operating system level TCP keepalive configurations would not take effect since the SO_KEEPALIVE option was not enabled on the underlying socket. There are additional ZMQ socket options related to TCP keepalive that can be set. However, I decided not to implement those options in this changeset because doing so would require adding additional bitcoin node configuration options, and would not yield a better outcome. I preferred a small, easily reviewable patch that doesn't add a bunch of new config options, with the tradeoff that the fine tuning would have to be done via well-documented operating system specific configurations. I tested this patch by running a node with: `./src/qt/bitcoin-qt -regtest -txindex -datadir=/tmp/node -zmqpubhashblock=tcp://127.0.0.1:28332 &` and connecting to it with: `python3 ./contrib/zmq/zmq_sub.py` Without these changes, `ss -panto | grep 28332 | grep ESTAB | grep bitcoin` will report no keepalive timer information. With these changes, the output from the prior command will show keepalive timer information consistent with the configuration at the time of connection establishment, e.g.-: `timer:(keepalive,119min,0)`. I also tested with a non-TCP transport and did not witness any adverse effects: `./src/qt/bitcoin-qt -regtest -txindex -datadir=/tmp/node -zmqpubhashblock=ipc:///tmp/bitcoin.block &` ACKs for top commit: adamjonas: Just to summarize for those looking to review - as of c276df775914e4e42993c76e172ef159e3b830d4 there are 3 tACKs (n-thumann, Haaroon, and dlogemann), 1 "looks good to me" (laanwj) with no NACKs or any show-stopping concerns raised. jonasschnelli: utACK c276df775914e4e42993c76e172ef159e3b830d4 Tree-SHA512: b884c2c9814e97e666546a7188c48f9de9541499a11a934bd48dd16169a900c900fa519feb3b1cb7e9915fc7539aac2829c7806b5937b4e1409b4805f3ef6cd1
2020-09-01net: improve nLastBlockTime and nLastTXTime documentationJon Atack
Co-authored-by: John Newbery <john@johnnewbery.com>
2020-09-01Merge #19105: Add Muhash3072 implementation in PythonWladimir J. van der Laan
36ec9801a4edb9663ef9ce9ad298233766b903e8 test: Add chacha20 test vectors in muhash (Fabian Jahr) 0e2b400fea890e769b75da5b55fa1902fd9f9851 test: Add basic Python/C++ Muhash implementation parity unit test (Fabian Jahr) b85543cb7361d6ba27c0eeca756eec9fd5395b36 test: Add Python MuHash3072 implementation to test framework (Pieter Wuille) ab30cece0e84a8b917cb9a219c2466574362f300 test: Move modinv to util and add unit test (Fabian Jahr) Pull request description: This is the second in a [series of pull requests](https://github.com/bitcoin/bitcoin/pull/18000) to implement an Index for UTXO set statistics. This pull request adds a Python implementation of Muhash3072, a homomorphic hashing algorithm to be used for hashing the UTXO set. The Python implementation can then be used to compare behavior with the C++ version. ACKs for top commit: jnewbery: utACK 36ec9801a laanwj: Code review ACK 36ec9801a4edb9663ef9ce9ad298233766b903e8 Tree-SHA512: a3519c6e11031174f1ae71ecd8bcc7f3be42d7fc9c84c77f2fbea7cfc5ad54fcbe10b55116ad8d9a52ac5d675640eefed3bf260c58a02f2bf3bc0d8ec208baa6
2020-09-01Merge #19668: Do not hide compile-time thread safety warningsMarcoFalke
ea74e10acf17903e44c85e3678853414653dd4e1 doc: Add best practice for annotating/asserting locks (Hennadii Stepanov) 2ee7743fe723227f2ea1b031eddb14fc6863f4c8 sync.h: Make runtime lock checks require compile-time lock checks (Anthony Towns) 23d71d171e6e22ba5e4a909d597a54595b2a2c1f Do not hide compile-time thread safety warnings (Hennadii Stepanov) 3ddc150857178bfb1c854c05bf9b526777876f56 Add missed thread safety annotations (Hennadii Stepanov) af9ea55a72c94678b343f5dd98dc78f3a3ac58cb Use LockAssertion utility class instead of AssertLockHeld() (Hennadii Stepanov) Pull request description: On the way of transit from `RecursiveMutex` to `Mutex` (see #19303) it is crucial to have run-time `AssertLockHeld()` assertion that does _not_ hide compile-time Clang Thread Safety Analysis warnings. On master (65e4ecabd5b4252154640c7bac38c92a3f3a7018) using `AssertLockHeld()` could hide Clang Thread Safety Analysis warnings, e.g., with the following patch applied: ```diff --- a/src/txmempool.h +++ b/src/txmempool.h @@ -607,7 +607,7 @@ public: void addUnchecked(const CTxMemPoolEntry& entry, setEntries& setAncestors, bool validFeeEstimate = true) EXCLUSIVE_LOCKS_REQUIRED(cs, cs_main); void removeRecursive(const CTransaction& tx, MemPoolRemovalReason reason) EXCLUSIVE_LOCKS_REQUIRED(cs); - void removeForReorg(const CCoinsViewCache* pcoins, unsigned int nMemPoolHeight, int flags) EXCLUSIVE_LOCKS_REQUIRED(cs, cs_main); + void removeForReorg(const CCoinsViewCache* pcoins, unsigned int nMemPoolHeight, int flags) EXCLUSIVE_LOCKS_REQUIRED(cs_main); void removeConflicts(const CTransaction& tx) EXCLUSIVE_LOCKS_REQUIRED(cs); void removeForBlock(const std::vector<CTransactionRef>& vtx, unsigned int nBlockHeight) EXCLUSIVE_LOCKS_REQUIRED(cs); ``` Clang compiles the code without any thread safety warnings. See "Add missed thread safety annotations" commit for the actual thread safety warnings that are fixed in this PR. ACKs for top commit: MarcoFalke: ACK ea74e10acf 🎙 jnewbery: ACK ea74e10acf17903e44c85e3678853414653dd4e1 ajtowns: ACK ea74e10acf17903e44c85e3678853414653dd4e1 Tree-SHA512: 8cba996e526751a1cb0e613c0cc1b10f027a3e9945fbfb4bd30f6355fd36b9f9c2e1e95ed3183fc254b42df7c30223278e18e5bdb5e1ef85db7fef067595d447
2020-09-01Merge #19671: wallet: Remove -zapwallettxesfanquake
3340dbadd38f5624642cf0e14dddbe6f83a3863b Remove -zapwallettxes (Andrew Chow) Pull request description: It's not clear what use there is to keeping `-zapwallettxes` given that it's intended usage has been superseded by `abandontransaction`. So this removes it outright. Alternative to #19700 ACKs for top commit: meshcollider: utACK 3340dbadd38f5624642cf0e14dddbe6f83a3863b fanquake: ACK 3340dbadd38f5624642cf0e14dddbe6f83a3863b - remaining manpage references will get cleaned up pre-release. Tree-SHA512: 3e58e1ef6f4f94894d012b93e88baba3fb9c2ad75b8349403f9ce95b80b50b0b4f443cb623cf76c355930db109f491b3442be3aa02972e841450ce52cf545fc8
2020-08-31Merge #19507: Expand functional zmq transaction testsWladimir J. van der Laan
7356292e1d7a44da8a2bd31c02c58d550bf38009 Have zmq reorg test cover mempool txns (Gregory Sanders) a0f4f9c983e57cc97ecbc56d0177eaf1854c842c Add zmq test for transaction pub during reorg (Gregory Sanders) 2399a0600ca9c4b676fa2f97520b8ecc44642246 Add test case for mempool->block zmq notification (Gregory Sanders) e70512a83c69bc85e96b08ade725594eda3e230f Make ordering of zmq consumption irrelevant to functional test (Gregory Sanders) Pull request description: Tests written to better define what messages are sent when. Also did a bit of refactoring to make sure the exact notification channel ordering doesn't matter. Confusions below aside, I believe having these more descriptive tests helps describe what behavior we expect from ZMQ notificaitons. Remaining confusion: 1) Notification patterns seem to vary wildly with the inclusion of mempool transactions being reorg'ed. See difference between "Add zmq test for transaction pub during reorg" and "Have zmq reorg test cover mempool txns" commits for specifics. 2) Why does a reorg'ed transaction get announced 3 times? From what I understand it can get announced once for disconnected block, once for mempool entry. What's the third? It occurs a 4th time when included in a block(not added in test) ACKs for top commit: laanwj: code review ACK 7356292e1d7a44da8a2bd31c02c58d550bf38009 promag: Code review ACK 7356292e1d7a44da8a2bd31c02c58d550bf38009. Tree-SHA512: 573662429523fd6a1af23dd907117320bc68cb51a93fba9483c9a2160bdce51fb590fcd97bcd2b2751d543d5c1148efa4e22e1c3901144f882b990ed2b450038
2020-08-31Remove -zapwallettxesAndrew Chow
-zapwallettxes is made a hidden option to inform users that it is removed and they should be using abandontransaction to do the stuck transaction thing.
2020-08-31Merge #19717: rpc: Assert that RPCArg names are equal to CRPCCommand ones ↵MarcoFalke
(mining,zmq,rpcdump) fa3d9ce3254882c545d700990fe8e9a678f31eed rpc: Assert that RPCArg names are equal to CRPCCommand ones (rpcdump) (MarcoFalke) fa32c1d5ec25bc53bf989a8ae68e688593d2859d rpc: Assert that RPCArg names are equal to CRPCCommand ones (zmq) (MarcoFalke) faaa46dc204d6d714f71dbc6f0bf02215dba0f0f rpc: Assert that RPCArg names are equal to CRPCCommand ones (mining) (MarcoFalke) fa93bc14c7411a108dd024d391344fabf0f76369 rpc: Remove unused return type from appendCommand (MarcoFalke) Pull request description: This is split out from #18531 to just touch the RPC methods in misc. Description from the main pr: ### Motivation RPCArg names in the rpc help are currently only used for documentation. However, in the future they could be used to teach the server the named arguments. Named arguments are currently registered by the `CRPCCommand`s and duplicate the RPCArg names from the documentation. This redundancy is fragile, and has lead to errors in the past (despite having linters to catch those kind of errors). See section "bugs found" for a list of bugs that have been found as a result of the changes here. ### Changes The changes here add an assert in the `CRPCCommand` constructor that the RPCArg names are identical to the ones in the `CRPCCommand`. ### Future work > Here or follow up, makes sense to also assert type of returned UniValue? Sure, but let's not get ahead of ourselves. I am going to submit any further works as follow-ups, including: * Removing the CRPCCommand arguments, now that they are asserted to be equal and thus redundant * Removing all python regex linters on the args, now that RPCMan can be used to generate any output, including the cli.cpp table * Auto-formatting and sanity checking the RPCExamples with RPCMan * Checking passed-in json in self-check. Removing redundant checks * Checking returned json against documentation to avoid regressions or false documentation * Compile the RPC documentation at compile-time to ensure it doesn't change at runtime and is completely static ### Bugs found * The assert identified issue #18607 * The changes itself fixed bug #19250 ACKs for top commit: fjahr: tested ACK fa3d9ce3254882c545d700990fe8e9a678f31eed promag: Code review ACK fa3d9ce3254882c545d700990fe8e9a678f31eed. Tree-SHA512: 068ade4b55cc195868d53b7f9a27151d45b440857bb069e261a49d102a49a38fdba5d68868516a1d66a54a73ba34681362f934ded7349e894042bde873b75719
2020-08-31Merge #19842: Update the vcpkg checkout commit ID in appveyor configMarcoFalke
a104caeb4008a6e0726ba604ece6f53549110354 Update the vcpkg checkout commit ID in appveyor config. (Aaron Clauson) Pull request description: A recent appveyor vm update broke the build of the `berkeleydb` vcpkg dependency, see #19839. The temporary resolution was to switch back to the previous appveyor vm. This PR updates the pegged vcpkg commit ID to the most recent commit as of 31 Aug 2020. That commit ID has been tested against the latest appveyor vm and is able to build Bitcoin Core successfully. The vcpkg bump includes a [patch](https://github.com/microsoft/vcpkg/pull/12870) to the `berkeleydb` build config which allows it to be built on the latest appveyor vm. ACKs for top commit: MarcoFalke: Concept ACK a104caeb4008a6e0726ba604ece6f53549110354 Tree-SHA512: 6d363d1615c51bb3d4b324eb96d53950648fc97fc81ffaef91ee6e92f1336776d150d89f6e859f354ee75ce66afcef07aa19ed39b725dbb3f47ba67d26e111db
2020-08-31Merge #19813: util, ci: Hard code previous release tarball checksumsMarcoFalke
0374e821bd9e9498ce9c03aa8e5435870019978b util: Hard code previous release tarball checksums (Hennadii Stepanov) bd897ce79f72a44a2e609f95433e251a3fd9eb9c scripted-diff: Move previous_release.py to test/get_previous_releases.py (Hennadii Stepanov) Pull request description: #19205 introduced signature verifying for the downloaded `SHA256SUMS.asc`. This approach is brittle and does not work in CI environment for many reasons: - https://github.com/bitcoin/bitcoin/issues/19812#issuecomment-680760663 - https://github.com/bitcoin/bitcoin/pull/19013#discussion_r459590779 This PR: - implements **Sjors**' [idea](https://github.com/bitcoin/bitcoin/pull/19205#pullrequestreview-426080048): > Alternatively we might as well hard code the checksum for each `tar.gz` release in the source code, here. - is an alternative to 5a2c31e528e6bd60635096f233252f3c717f366d (#19013) - fixes #19812 - updates v0.17.1 to v0.17.2 ACKs for top commit: MarcoFalke: cr ACK 0374e821bd9e9498ce9c03aa8e5435870019978b Sjors: tACK 0374e821bd9e9498ce9c03aa8e5435870019978b Tree-SHA512: cacdcf9f5209eae7da357abb3445585ad2f980920fd5bf75527ce89974d3f531a4cf8b5b35edfc116b23bfdfb45c0437cb14cbc416d76ed2dc5b9e6d33cdad71
2020-08-31Merge #18244: rpc: fundrawtransaction and walletcreatefundedpsbt also lock ↵Samuel Dobson
manually selected coins 6d1f51343cf11b07cd401fbd0c5bc3603e185a0e [rpc] fundrawtransaction, walletcreatefundedpsbt lock manually selected coins (Sjors Provoost) Pull request description: When using `fundrawtransaction` and `walletcreatefundedpsbt` with `lockUnspents`, it would only lock automatically selected coins, not manually selected coins. That doesn't make much sense to me if the goal is to prevent accidentally double-spending yourself before you broadcast a transaction. Note that when creating a transaction, manually selected coins are automatic "unlocked" (or more accurately: the lock is ignored). Earlier versions of this PR introduced an error when a locked coin is manually selected, but this idea was abandoned after some discussion. An application that uses this RPC should either rely on automatic coin selection (with `lockUnspents`) or handle lock concurrency itself with manual coin selection. In particular it needs to make sure to avoid/pause calls with automatic coin selection between calling `lockunspent` and the subsequent spending RPC. See #7518 for historical background. ACKs for top commit: meshcollider: Code review ACK 6d1f51343cf11b07cd401fbd0c5bc3603e185a0e fjahr: Code review ACK 6d1f51343cf11b07cd401fbd0c5bc3603e185a0e Tree-SHA512: 8773c788d92f2656952e1beac147ba9956b8c5132d474e0880e4c89ff53642928b4cbfcd1cb3d17798b9284f02618a8830c93a9f7a4733e5bded96adff1d5d4d
2020-08-31Merge #19773: wallet: Avoid recursive lock in IsTrustedSamuel Dobson
772ea4844c34ad70d02fd0bd6c0945baa8fff85c wallet: Avoid recursive lock in IsTrusted (João Barbosa) 819f10f6718659eeeec13af2ce831df3a0984090 wallet, refactor: Immutable CWalletTx::pwallet (João Barbosa) Pull request description: This change moves `CWalletTx::IsTrusted` to `CWallet` in order to have TSAN. So now `CWallet::IsTrusted` requires `cs_wallet` and the recursive lock no longer happens. Motivated by https://github.com/bitcoin/bitcoin/pull/19289/files#r473308226. ACKs for top commit: meshcollider: utACK 772ea4844c34ad70d02fd0bd6c0945baa8fff85c hebasto: ACK 772ea4844c34ad70d02fd0bd6c0945baa8fff85c, reviewed and tested on Linux Mint 20 (x86_64). Tree-SHA512: 702ffd928b2f42a8b90de398790649a5fd04e1ac3877558da928e94cdeb19134883f06c3a73a6826c11c912facf199173375a70200737e164ccaea1bec515b2a
2020-08-31Merge #19379: tests: Add fuzzing harness for SigHasLowR(...) and ↵MarcoFalke
ecdsa_signature_parse_der_lax(...) 46fcac1e4b9e0b1026bc0b663582148b2fd60390 tests: Add fuzzing harness for ec_seckey_import_der(...) and ec_seckey_export_der(...) (practicalswift) b667a90389cce7e1bf882f4ac78323c48858efaa tests: Add fuzzing harness for SigHasLowR(...) and ecdsa_signature_parse_der_lax(...) (practicalswift) Pull request description: Add fuzzing harness for `SigHasLowR(...)` and `ecdsa_signature_parse_der_lax(...)`. 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: Crypt-iQ: ACK 46fcac1e4b9e0b1026bc0b663582148b2fd60390 Tree-SHA512: 11a4856a1efd9a04030a8c8aee2413fd5be1ea248147e649a48a55bacdf732bb48a19ee1ce2761d47d4dd61c9598aec53061b961b319ad824d539dda11a8ccf4
2020-08-31Merge #19099: refactor: Move wallet methods out of chain.h and node.hMarcoFalke
24bf17602c620445f76c3b407937751c8a894d37 gui refactor: Inline SplashScreen::ConnectWallet (Russell Yanofsky) e4f435047121886edb6e6a6c4e4998e44ed2e36a refactor: Move wallet methods out of chain.h and node.h (Russell Yanofsky) b266b3e0bf29d0f3d5deaeec62d57c5025b35525 refactor: Create interfaces earlier during initialization (Russell Yanofsky) Pull request description: Add WalletClient interface so node interface is cleaner and don't need wallet-specific methods. The new NodeContext::wallet_client pointer will also be needed to eliminate global wallet variables like ::vpwallets in #19101, because createWallet(), loadWallet(), getWallets(), etc methods called by the GUI need a way to get a reference to the list of open wallets if it is no longer a global variable. ACKs for top commit: promag: Code review ACK 24bf17602c620445f76c3b407937751c8a894d37. MarcoFalke: ACK 24bf17602c620445f76c3b407937751c8a894d37 🐚 Tree-SHA512: a70d3776cd6723093db8912028c50075ec5fa0a48b961cb1a945f922658f5363754f8380dbb8378ed128c8c858913024f8264740905b8121a35c0d63bfaed7cf
2020-08-31Update the vcpkg checkout commit ID in appveyor config.Aaron Clauson
2020-08-31Merge #19710: bench: Prevent thread oversubscription and decreases the ↵MarcoFalke
variance of result values 3edc4e34fe2f92e7066c1455f5e42af2fdb43b99 bench: Prevent thread oversubscription (Hennadii Stepanov) ce3e6a7cb21d1aa455513970846e1f70c01472a4 bench: Allow skip benchmark (Hennadii Stepanov) Pull request description: Split out from #18710. Some results (borrowed from #18710): ![89121718-a3329800-d4c1-11ea-8bd1-66da20619696](https://user-images.githubusercontent.com/32963518/90146614-ecb89800-dd89-11ea-80fe-bac0e46e735e.png) ACKs for top commit: fjahr: Code review ACK 3edc4e34fe2f92e7066c1455f5e42af2fdb43b99 Tree-SHA512: df7413ec9ea326564a8e8de54752c9d1444ff7de34edb03e1e0c2120fc333e4640767fdbe3e87eab6a7b389a4863c02e22ad2ae0dbf139fad6a9b85e00f563b4
2020-08-31Merge #19826: Pass mempool reference to chainstate constructorMarcoFalke
fa0572d0f3b083b4c8e2e883a66e2b198c6779f1 Pass mempool reference to chainstate constructor (MarcoFalke) Pull request description: Next step toward #19556 Instead of relying on the mempool global, each chainstate is given a reference to a mempool to keep up to date with the tip (block connections, disconnections, reorgs, ...) ACKs for top commit: promag: Code review ACK fa0572d0f3b083b4c8e2e883a66e2b198c6779f1. darosior: ACK fa0572d0f3b083b4c8e2e883a66e2b198c6779f1 hebasto: ACK fa0572d0f3b083b4c8e2e883a66e2b198c6779f1, reviewed and tested on Linux Mint 20 (x86_64). Tree-SHA512: 12184d33ae5797438d03efd012a07ba3e4ffa0d817c7a0877743f3d7a7656fe279280c751554fc035ccd0058166153b6c6c308a98b2d6b13998922617ad95c4c
2020-08-31Merge #19803: Bugfix: Define and use HAVE_FDATASYNC correctly outside LevelDBfanquake
c4b85ba704a1b5257dc82786a84f676bacb7b027 Bugfix: Define and use HAVE_FDATASYNC correctly outside LevelDB (Luke Dashjr) Pull request description: Fixes a bug introduced in #19614 The LevelDB-specific fdatasync check was only using `AC_SUBST`, which works for Makefiles, but doesn't define anything for C++. Furthermore, the #define is typically 0 or 1, never undefined. This fixes both issues by defining it and checking its value instead of whether it is merely defined. Pulled out of #14501 by fanquake's request ACKs for top commit: fanquake: ACK c4b85ba704a1b5257dc82786a84f676bacb7b027 - thanks for catching and fixing my mistake. laanwj: Code review ACK c4b85ba704a1b5257dc82786a84f676bacb7b027 Tree-SHA512: 91d5d426ba000b4f3ee7e2315635e24bbb23ceff16269ddf4f65a63d25fc9e9cf94a3b236eed2f8031cc36ddcf78aeb5916efcb244f415943a8a12f907ede8f9
2020-08-31Merge #19828: wallet, refactor: Remove duplicate map lookups in ↵fanquake
GetAddressBalances b35e74ba379bdc12ea6d49a45469f0d6aa74cc27 wallet, refactor: Remove duplicate map lookups in GetAddressBalances (João Barbosa) Pull request description: Now just one lookup in `balances` instead of three. ACKs for top commit: achow101: ACK b35e74ba379bdc12ea6d49a45469f0d6aa74cc27 theStack: ACK b35e74ba379bdc12ea6d49a45469f0d6aa74cc27 practicalswift: ACK b35e74ba379bdc12ea6d49a45469f0d6aa74cc27 Tree-SHA512: a73c1b336406a569e3bb10290618c5950b944db58ed0b05ff202d097684bb3ba3a5942c8d30443960052aa16438c054e2d02977b67aa901cce665c4df0ee5602
2020-08-31Merge #19830: test: Add tsan supp for leveldb::DBImpl::DeleteObsoleteFilesfanquake
fa1fc536bb26471fd2a6fe8d12f98cf53c646306 test: Add tsan supp for leveldb::DBImpl::DeleteObsoleteFiles (MarcoFalke) Pull request description: Fixes #19712 ACKs for top commit: practicalswift: ACK fa1fc536bb26471fd2a6fe8d12f98cf53c646306 -- patch looks correct hebasto: ACK fa1fc536bb26471fd2a6fe8d12f98cf53c646306 Tree-SHA512: 24d6a4e871fda11196a9f88e2ddbd1c1461d895c503a04b103791233e46638421836200eaaa7d70689564e51dee0d68d32b880dd90a5c259fb6a906f21d07853
2020-08-30Merge #19839: Set appveyor VM version to previous Visual Studio 2019 releaseMarcoFalke
3ec8f4c080eab61563f11659c5f1370a34b77dc5 Set appveyor vm version to previous Visual Studio 2019 release. (Aaron Clauson) Pull request description: The Visual Studio 2019 Appveyor [update](https://www.appveyor.com/updates/2020/08/29/) includes a bump of `cmake` to `3.18.2`. The `berkeleydb` dependency from the pegged `vcpkg` commit now fails to build with the `cmake` update. Setting the Appveyor VM back to the previous version should fix the immediate issue while a solution is identified for updating the `berkeleydb` build configuration. ACKs for top commit: hebasto: ACK 3ec8f4c080eab61563f11659c5f1370a34b77dc5, AppVeyor build is ok now. Tree-SHA512: 61e10d0260c1c51dc6c552c857b3013f55ee324e26b1229bf7ef6f14c3e296762148db66a994ecc33fdc43e81592a7f8442d17b407964edb774b482d84735757
2020-08-30Avoid callback when -blocknotify is emptyJoão Barbosa
2020-08-30Set appveyor vm version to previous Visual Studio 2019 release.Aaron Clauson
Latest vm version has updated cmake and the berkeleydb vcpkg now fails to build.
2020-08-29doc: Add best practice for annotating/asserting locksHennadii Stepanov
2020-08-29sync.h: Make runtime lock checks require compile-time lock checksAnthony Towns
2020-08-29Do not hide compile-time thread safety warningsHennadii Stepanov
2020-08-29Add missed thread safety annotationsHennadii Stepanov
This is needed for upcoming commit "sync.h: Make runtime lock checks require compile-time lock checks" to pass.
2020-08-29Use LockAssertion utility class instead of AssertLockHeld()Hennadii Stepanov
This change prepares for upcoming commit "Do not hide compile-time thread safety warnings" by replacing AssertLockHeld() with LockAssertion() where needed.
2020-08-29Merge #18921: build: add stack-clash and control-flow protection options to ↵Wladimir J. van der Laan
hardening flags b536813cefc13f5c54a28a7c2fce8c69e89d6624 build: add -fstack-clash-protection to hardening flags (fanquake) 076183b36b76a11438463883ff916f17aef9e001 build: add -fcf-protection=full to hardening options (fanquake) Pull request description: Beginning with Ubuntu `19.10`, it's packaged GCC now has some additional hardening options enabled by default (in addition to existing defaults like `-fstack-protector-strong` and reducing the minimum ssp buffer size). The new additions are`-fcf-protection=full` and `-fstack-clash-protection`. > -fcf-protection=[full|branch|return|none] > Enable code instrumentation of control-flow transfers to increase program security by checking that target addresses of control-flow transfer instructions (such as indirect function call, function return, indirect jump) are valid. This prevents diverting the flow of control to an unexpected target. This is intended to protect against such threats as Return-oriented Programming (ROP), and similarly call/jmp-oriented programming (COP/JOP). > -fstack-clash-protection > Generate code to prevent stack clash style attacks. When this option is enabled, the compiler will only allocate one page of stack space at a time and each page is accessed immediately after allocation. Thus, it prevents allocations from jumping over any stack guard page provided by the operating system. If your interested you can grab `gcc-9_9.3.0-10ubuntu2.debian.tar.xz` from https://packages.ubuntu.com/focal/g++-9. The relevant changes are part of the `gcc-distro-specs` patches, along with the relevant additions to the gcc manages: > NOTE: In Ubuntu 19.10 and later versions, -fcf-protection is enabled by default for C, C++, ObjC, ObjC++, if none of -fno-cf-protection nor -fcf-protection=* are found. > NOTE: In Ubuntu 19.10 and later versions, -fstack-clash-protection is enabled by default for C, C++, ObjC, ObjC++, unless -fno-stack-clash-protection is found. So, if you're C++ using GCC on Ubuntu 19.10 or later, these options will be active unless you explicitly opt out. This can be observed with a small test: ```c++ int main() { return 0; } ``` ```bash g++ --version g++ (Ubuntu 9.3.0-10ubuntu2) 9.3.0 g++ test.cpp objdump -dC a.out .. 0000000000001129 <main>: 1129: f3 0f 1e fa endbr64 112d: 55 push %rbp 112e: 48 89 e5 mov %rsp,%rbp 1131: b8 00 00 00 00 mov $0x0,%eax 1136: 5d pop %rbp 1137: c3 retq 1138: 0f 1f 84 00 00 00 00 nopl 0x0(%rax,%rax,1) 113f: 00 # recompile opting out of control flow protection g++ test.cpp -fcf-protection=none objdump -dC a.out ... 0000000000001129 <main>: 1129: 55 push %rbp 112a: 48 89 e5 mov %rsp,%rbp 112d: b8 00 00 00 00 mov $0x0,%eax 1132: 5d pop %rbp 1133: c3 retq 1134: 66 2e 0f 1f 84 00 00 nopw %cs:0x0(%rax,%rax,1) 113b: 00 00 00 113e: 66 90 xchg %ax,%ax ``` Note the insertion of an `endbr64` instruction when compiling and _not_ opting out. This instruction is part of the Intel Control-flow Enforcement Technology [spec](https://software.intel.com/sites/default/files/managed/4d/2a/control-flow-enforcement-technology-preview.pdf), which the GCC control flow implementation is based on. If we're still doing gitian builds for the `0.21.0` and `0.22.0` releases, we'd likely update the gitian image to Ubuntu Focal, which would mean that the GCC used for gitian builds would also be using these options by default. So we should decide whether we want to explicitly turn these options on as part of our hardening options (although not just for this reason), or, we should be opting-out. GCC has supported both options since 8.0.0. Clang has supported `-fcf-protection` from 7.0.0 and will support `-fstack-clash-protection` in it's upcoming [11.0.0 release](https://clang.llvm.org/docs/ReleaseNotes.html#id6). ACKs for top commit: jamesob: ACK b536813cefc13f5c54a28a7c2fce8c69e89d6624 ([`jamesob/ackr/18921.1.fanquake.build_add_stack_clash_an`](https://github.com/jamesob/bitcoin/tree/ackr/18921.1.fanquake.build_add_stack_clash_an)) laanwj: Code review ACK b536813cefc13f5c54a28a7c2fce8c69e89d6624 Tree-SHA512: abc9adf23cdf1be384f5fb9aa5bfffdda86b9ecd671064298d4cda0440828b509f070f9b19c88c7ce50ead9ff32afff9f14c5e78d75f01241568fbfa077be0b7
2020-08-29util: Hard code previous release tarball checksumsHennadii Stepanov
2020-08-29scripted-diff: Move previous_release.py to test/get_previous_releases.pyHennadii Stepanov
-BEGIN VERIFY SCRIPT- OLD=contrib/devtools/previous_release.py NEW=test/get_previous_releases.py sed -i "s|$OLD|$NEW|g" $(git grep -l $OLD) git mv $OLD $NEW -END VERIFY SCRIPT-
2020-08-29Merge #18817: doc: Document differences in bitcoind and bitcoin-qt locale ↵MarcoFalke
handling ca185cf5a14b16d61814d7172284bc8efcd28b69 doc: Document differences in bitcoind and bitcoin-qt locale handling (practicalswift) Pull request description: Document differences in `bitcoind` and `bitcoin-qt` locale handling. Since this seems to be the root cause to the locale dependency issues we've seen over the years I thought it was worth documenting :) Note that 1.) `QLocale` (used by Qt), 2.) C locale (used by locale-sensitive C standard library functions/POSIX functions and some parts of the C++ standard library such as `std::to_string`) and 3.) C++ locale (used by the C++ input/output library) are three separate things. This comment is about the perhaps surprising interference with the C locale (2) that takes place as part of the Qt initialization. ACKs for top commit: hebasto: re-ACK ca185cf5a14b16d61814d7172284bc8efcd28b69 Tree-SHA512: e51c32f3072c506b0029a001d8b108125e1acb4f2b6a48a6be721ddadda9da0ae77a9b39ff33f9d9eebabe2244c1db09e8502e3e7012d7a5d40d98e96da0dc44
2020-08-29test: Add tsan supp for leveldb::DBImpl::DeleteObsoleteFilesMarcoFalke
2020-08-29doc: Document differences in bitcoind and bitcoin-qt locale handlingpracticalswift
2020-08-28Merge #19607: [p2p] Add Peer struct for per-peer data in net processingWladimir J. van der Laan
8e35bf59062b3a823182588e0bf809b3367c2be0 scripted-diff: rename misbehavior members (John Newbery) 1f96d2e673a78220eebf3bbd15b121c51c4cd97b [net processing] Move misbehavior tracking state to Peer (John Newbery) 7cd4159ac834432dadd60a5e8ee817f3cadbee55 [net processing] Add Peer (John Newbery) aba03359a6e62a376ae44914f609f82a1556fc89 [net processing] Remove CNodeState.name (John Newbery) Pull request description: We currently have two structures for per-peer data: - `CNode` in net, which should just contain connection layer data (eg socket, send/recv buffers, etc), but currently also contains some application layer data (eg tx/block inventory). - `CNodeState` in net processing, which contains p2p application layer data, but requires cs_main to be locked for access. This PR adds a third struct `Peer`, which is for p2p application layer data, and doesn't require cs_main. Eventually all application layer data from `CNode` should be moved to `Peer`, and any data that doesn't strictly require cs_main should be moved from `CNodeState` to `Peer` (probably all of `CNodeState` eventually). `Peer` objects are stored as shared pointers in a net processing global map `g_peer_map`, which is protected by `g_peer_mutex`. To use a `Peer` object, `g_peer_mutex` is locked, a copy of the shared pointer is taken, and the lock is released. Individual members of `Peer` are protected by different mutexes that guard related data. The lifetime of the `Peer` object is managed by the shared_ptr refcount. This PR adds the `Peer` object and moves the misbehaving data from `CNodeState` to `Peer`. This allows us to immediately remove 15 `LOCK(cs_main)` instances. For more motivation see #19398 ACKs for top commit: laanwj: Code review ACK 8e35bf59062b3a823182588e0bf809b3367c2be0 troygiorshev: reACK 8e35bf59062b3a823182588e0bf809b3367c2be0 via `git range-diff master 9510938 8e35bf5` theuni: ACK 8e35bf59062b3a823182588e0bf809b3367c2be0. jonatack: ACK 8e35bf59062b3a823182588e0bf809b3367c2be0 keeping in mind Cory's comment (https://github.com/bitcoin/bitcoin/pull/19607#discussion_r470173964) for the follow-up Tree-SHA512: ad84a92b78fb34c9f43813ca3dfbc7282c887d55300ea2ce0994d134da3e0c7dbc44d54380e00b13bb75a57c28857ac3236bea9135467075d78026767a19e4b1
2020-08-28wallet, refactor: Remove duplicate map lookups in GetAddressBalancesJoão Barbosa
2020-08-28Merge bitcoin-core/gui#39: Add visual accenting for the 'Create new ↵MarcoFalke
receiving address' button 4ec49f8d1e25b330e6a0f79ae897d98d29ff32f9 qt: Leverage the default "Create new receiving address" button (Hennadii Stepanov) 4227a8e1f3a4f94a5a4cb7adeecf967e14156bde qt: Make "Create new receiving address" default unconditionally (Hennadii Stepanov) Pull request description: Fix #24 The first commit: - visual improvement with no behavior change The second commit: - removes a bunch of LOCs - slightly change behavior and makes it standard With this PR: ![DeepinScreenshot_select-area_20200721213040](https://user-images.githubusercontent.com/32963518/88093294-7b2a6700-cb9a-11ea-89a2-a0e2678056a7.png) ACKs for top commit: Saibato: Concept tACK https://github.com/bitcoin-core/gui/pull/39/commits/4227a8e1f3a4f94a5a4cb7adeecf967e14156bde https://github.com/bitcoin-core/gui/pull/39/commits/4ec49f8d1e25b330e6a0f79ae897d98d29ff32f9 promag: Tested ACK 4ec49f8d1e25b330e6a0f79ae897d98d29ff32f9 on macos. Tree-SHA512: 3403d5ee96ec139491c7e23b24a24d9239fe55c58d99cbd4cd13bc877f76f992ed011c09e2af35b2a63be1a2371b95f6ac719325396dcc8333cf3eb7fa2e3d2c
2020-08-28Merge #19797: net: Remove old check for 3-byte shifted IP addresses from ↵MarcoFalke
pre-0.2.9 nodes 7b6d0f10a7af7998f7cfcf3aeaa0269b61a321ce Remove old check for 3-byte shifted IP addresses from pre-0.2.9 node messages (Raúl Martínez (RME)) Pull request description: The change removes an old check for IPv6 addresses in range ::ff:ff00:0:0:0/72 that were created due to a bug in size field of addr messages for 0.2.8 nodes and before. This check is no longer needed as they are no more pre 0.2.9 nodes on the network (as per bitnodes network snapshot). Credits for discovering this go to sipa in https://github.com/bitcoin/bitcoin/pull/19628#discussion_r475907453 Thanks for the attention! ACKs for top commit: sipa: utACK 7b6d0f10a7af7998f7cfcf3aeaa0269b61a321ce vasild: ACK 7b6d0f1 Tree-SHA512: c5fab59dda2acafe143f607a4c5b636a54ac76fba651cad1ad1b09c94e88ab39503a31c2244c8f2664da68456c2a870c601d8894139c55cde9ece8161913ed2e
2020-08-28Merge #19739: refactor: remove c-string interfaces for DecodeBase58{Check}Wladimir J. van der Laan
d3e8adfada889a3c9fba930086eda609509aca07 util: remove c-string interfaces for DecodeBase58{Check} (Sebastian Falbesoner) Pull request description: This micro-PR gets rid of base58 function interfaces that are redundant in terms of c-string / std::string variants; the c-string interface for `DecodeBase58Check` is completely unused outside the base58 module, while the c-string interface for `DecodeBase58` is only used in unit tests, where an implicit conversion to std::string is not problematic. ACKs for top commit: practicalswift: ACK d3e8adfada889a3c9fba930086eda609509aca07 -- patch looks correct laanwj: Code review ACK d3e8adfada889a3c9fba930086eda609509aca07 Tree-SHA512: 006a4a1e23b11385f60820c188b8e6b1634a182ca36e29a6580f72150214c65a3fdb273ec439165f26ba88a42d2bf5bab1cf3666a9eaee222fb4e1c00aeba433
2020-08-28Merge #19646: doc: Updated outdated help command for getblocktemplateWladimir J. van der Laan
c91b241b48d7f97b3e6b39d84ec780f2a3e3a0a7 Updated outdated help command for getblocktemplate (fixes #19625) (Jake Leventhal) Pull request description: **Summary of Changes** * Removed coinbasetxn from the help outputs * Added the missing name for transactions in the help outputs * Added help outputs for longpollid and default_witness_commitment * Added more clarity to capabilities, rules, and coinbaseaux **Rationale** The outputs from the help command for `getblocktemplate` are outdated and don't reflect the actual results from `getblocktemplate` (see #19625 for more details) Fixes #19625. ACKs for top commit: laanwj: ACK c91b241b48d7f97b3e6b39d84ec780f2a3e3a0a7 fjahr: utACK c91b241b48d7f97b3e6b39d84ec780f2a3e3a0a7 Tree-SHA512: ee443af4bc3b2838dfd92e2705f344256ee785ae720e505fffea9b0ec5b75930e3b1374bae59b36d5da57c85c9aefe4d62504b028b893d6f2914dccf1e34c658
2020-08-28Merge #19758: Drop deprecated and unused GUARDED_VAR and PT_GUARDED_VAR ↵fanquake
annotations 9034f6e30ee6d3db71049de4500e7ede648557fb Drop deprecated and unused GUARDED_VAR and PT_GUARDED_VAR annotations (Hennadii Stepanov) Pull request description: https://clang.llvm.org/docs/ThreadSafetyAnalysis.html#guarded-var-and-pt-guarded-var: > `GUARDED_VAR` and `PT_GUARDED_VAR` > Use of these attributes has been deprecated. ACKs for top commit: MarcoFalke: ACK 9034f6e30ee6d3db71049de4500e7ede648557fb They seem to be deprecated for a long time already https://releases.llvm.org/4.0.0/tools/clang/docs/ThreadSafetyAnalysis.html#guarded-var-and-pt-guarded-var Tree-SHA512: d86f55fe57c28d91eda4a0ad727e36a5b35ba4b50a557c59b83cf0c5291cc5ad37b6f4ba6daeba3c1aba143faadaea6bb21c723f4d221856d6e6c42d228e8aa2
2020-08-28Remove old check for 3-byte shifted IP addresses from pre-0.2.9 node messagesRaúl Martínez (RME)
The change removes an old check for IPv6 addresses in range ::ff:ff00:0:0:0/72 that were created due to a bug in size field of addr messages for 0.2.8 nodes and before. This check is no longer needed as they are no more pre 0.2.9 nodes on the network (as per bitnodes network snapshot). Credits for discovering this go to sipa.
2020-08-28Merge #19822: chain: Fix CChain comparison UB by removing it (it was unused)fanquake
df536883d263781c2abe944afc85f681cda635ed chain: Remove UB CChain comparison (Carl Dong) Pull request description: Comparing two empty `CChain`s is currently undefined behaviour, and resulted in false assertion failures when comparing identical empty `CChain`s in local testing. Let's just remove this comparison operator since it doesn't seem to be used anywhere. ACKs for top commit: practicalswift: ACK df536883d263781c2abe944afc85f681cda635ed -- patch is guaranteed to be correct :) MarcoFalke: cr ACK df536883d263781c2abe944afc85f681cda635ed Tree-SHA512: db10bac364fc965b56abf7a5bac48018786b14806ffe107e3e8eb24d5004a29331f3387dfe3409a3452a6750d3329e3f354265d787ebb3abfccabe77b28a54d5
2020-08-28wallet: Avoid recursive lock in IsTrustedJoão Barbosa
2020-08-28wallet, refactor: Immutable CWalletTx::pwalletJoão Barbosa