aboutsummaryrefslogtreecommitdiff
AgeCommit message (Collapse)Author
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-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-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-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-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
2020-08-28Pass mempool reference to chainstate constructorMarcoFalke
2020-08-28Merge #19067: tests: Add fuzzing harness for CNodeMarcoFalke
cc26fab48d76a813d798657b18ae1af08a301150 tests: Add fuzzing harness for CNode (practicalswift) Pull request description: Add fuzzing harness for `CNode`. See [`doc/fuzzing.md`](https://github.com/bitcoin/bitcoin/blob/master/doc/fuzzing.md) for information on how to fuzz Bitcoin Core. Don't forget to contribute any coverage increasing inputs you find to the [Bitcoin Core fuzzing corpus repo](https://github.com/bitcoin-core/qa-assets). Happy fuzzing :) Top commit has no ACKs. Tree-SHA512: e6330e5de5b2eb44d3bd91a885e69ebb625bfd1cb2499338aeb3997ff0268848434e651126fe68a8cadd7235c391e61a40d6408ee26e457faf73572e0c375f6b
2020-08-27chain: Remove UB CChain comparisonCarl Dong
It was unused, and had UB
2020-08-27gui refactor: Inline SplashScreen::ConnectWalletRussell Yanofsky
Suggested https://github.com/bitcoin/bitcoin/pull/19099#discussion_r450522201
2020-08-27Merge bitcoin-core/gui#40: Clarify block height labelMarcoFalke
b6dcc6d74186eee15eda2cb6e8a7ab5b5b4a05f8 gui: Clarify block height label (Hennadii Stepanov) Pull request description: Prefer "block height" instead of "number of blocks". This was done while testing https://github.com/bitcoin/bitcoin/pull/16981. ACKs for top commit: michaelfolkson: ACK b6dcc6d74186eee15eda2cb6e8a7ab5b5b4a05f8. I don't think there are any other obvious examples in the GUI where "block height" should replace "number of blocks" except for translations. MarcoFalke: cr ACK b6dcc6d74186eee15eda2cb6e8a7ab5b5b4a05f8 Tree-SHA512: ec3b48c1af5d613ed657ad51f2caddea774376736ecc02343d54518986e35ec37f1745b059814b5be92b5e5c2bb2970d17159b24c6e88b9316803d4de5327c31
2020-08-27refactor: Move wallet methods out of chain.h and node.hRussell Yanofsky
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, 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. Also tweaks splash screen registration for load wallet events to be delayed until after wallet client is created.
2020-08-27refactor: Create interfaces earlier during initializationRussell Yanofsky
Add AppInitInterfaces function so wallet chain and chain client interfaces are created earlier during initialization. This is needed in the next commit to allow the gui splash screen to be able to register for wallet events through a dedicated WalletClient interface instead managing wallets indirectly through the Node interface. This only works if the wallet client interface is created before the splash screen needs to use it.
2020-08-27Merge #19169: rpc: Validate provided keys for query_options parameter in ↵MarcoFalke
listunspent a99a3c0bd6d5476503c015e23be569295fdd190c rpc: Validate provided keys for query_options parameter in listunspent (pasta) Pull request description: At Dash, one of our developers was working with the `listunspent` RPC command, but instead of saying "minimumAmount" he said "minimmumAmount" as such the RPC wasn't working as expected. In https://github.com/dashpay/dash/pull/3507 we implemented a check so that `listunspent` returns an error if an unrecognized option is given. I figured I might as well adapt the code and throw up a PR here. Cheers! ACKs for top commit: adaminsky: ACK `a99a3c0bd` meshcollider: Seems fine to me. utACK a99a3c0bd6d5476503c015e23be569295fdd190c Tree-SHA512: 9fccf14979849879a51b352afa3e1932ce4a6cfc2ee97b8d405ec6e65673fe94e302795e3ec0b440e6d252f13acda620e1f6a0e86c3fa918883c3fb4600a372c
2020-08-27tests: Add fuzzing harness for CNodepracticalswift
2020-08-27Merge #19289: wallet: GetWalletTx and IsMine require cs_wallet lockWladimir J. van der Laan
b8405b833ad28351c80fb10f6f896f974013fd9e wallet: IsChange requires cs_wallet lock (João Barbosa) d8441f30ff57e4ae98cff6694c995eaffc19c51a wallet: IsMine overloads require cs_wallet lock (João Barbosa) a13cafc6c6998baedf3c5766259c21fcd763b99e wallet: GetWalletTx requires cs_wallet lock (João Barbosa) Pull request description: This change removes some unlock/lock and lock/lock cases regarding `GetWalletTx` and `IsMine` overloads. ACKs for top commit: laanwj: Code review ACK b8405b833ad28351c80fb10f6f896f974013fd9e ryanofsky: Code review ACK b8405b833ad28351c80fb10f6f896f974013fd9e. Just new commit since last review changing IsChange GetChange locks and adding annotations Tree-SHA512: 40d37c4fe5d10a1407f57d899d5822bb285633d8dbfad8afcf15a9b41b428ed9971a9a7b1aae84318371155132df3002699a15dab56e004527d50c889829187d
2020-08-27Merge #19761: build: improve sed robustness by not using sedfanquake
3de365e4f15ddaaf6bdb5b3c31529156617d2835 build: replace wingenminiupnpcstrings sed with a patch in miniupnpc package (fanquake) bbc01a753d40488fa1469c87b31e7a12ddc8b329 build: replace qtranslations lrelease sed with a patch in qt package (fanquake) c723e4176ee3c1c2391a7c43b094a315bdd3c72e build: replace FreeType back-compat sed with a patch in qt package (fanquake) 3aaa39d436aa32489aa136d6b3d714220535ec1e build: replace pwd sed in qt package with a patch (fanquake) 9d440f4e11f7d610f3bcf1fb587d71ed2581a6c1 build: remove no-longer needed qt workaround (fanquake) bf85eace1a594862c4baa375fd3c8b741948cc47 build: remove no-longer needed qt configure workaround (fanquake) 4af59a407a207e7de327c1bb145d68ebc8ddedd8 build: use patch rather than sed in zeromq package (fanquake) cc107a3af17d821f66de9357efe73214a628803b build: use patch rather than sed in native_cctools package (fanquake) 865cb23a485d88be603c1d6bf8c32ef7a5edeaa2 build: use patch rather than sed in fontconfig package (fanquake) 335bd7f8bc415d6419763e17b013f94e5f0e5689 build: use patch rather than sed in Boost package (fanquake) f36140d00c937b5059cd952d759a77e25330388a build: use patch rather than sed in bdb package (fanquake) Pull request description: While using `sed` can be handy to use for a quick-fix, these instances accumulate, and can become unmaintainable. Not only that, but using sed isn't necessarily robust and it can fail silently. Most of our usage is also missing any documentation explaining why something is being done, when it should be updated/removed etc. Rather than relying on sed going forward, where possible, I've converted our sed usage into patches. These are easier to maintain, contain documentation, and should fail loudly when they don't apply. The remaining sed usage, (1 in miniupnpc, the rest in qt), are non-trivial to remove, as they are using build-time variables, or some input from the environment. This also steals 2 related commits out of #19716. Related to #16838. ACKs for top commit: hebasto: re-ACK 3de365e4f15ddaaf6bdb5b3c31529156617d2835, only `drop_lrelease_dependency.patch` updated. Travis makes ARM build without errors now. theuni: ACK 3de365e4f15ddaaf6bdb5b3c31529156617d2835. Tree-SHA512: b39afcb237e4421f9caabbd665af93fd2e749a1cdd42b1d5ee2261059120005c0e82994d315e679c317d23794eab5c7727f51cae403c94a9c4e4fd7eee9e7ee6
2020-08-27Merge #19752: test: Update wait_until usage in tests not to use the one from ↵MarcoFalke
utils d841301010914203fb5ef02627c76fad99cb11f1 test: Add docstring to wait_until() in util.py to warn about its usage (Seleme Topuz) 1343c86c7cc1fc896696b3ed87c12039e4ef3a0c test: Update wait_until usage in tests not to use the one from utils (Seleme Topuz) Pull request description: Replace global (from [test_framework/util.py](https://github.com/bitcoin/bitcoin/blob/master/test/functional/test_framework/util.py#L228)) `wait_until()` usages with the ones provided by `BitcoinTestFramework` and `P2PInterface` classes. The motivation behind this change is that the `util.wait_until()` expects a timeout, timeout_factor and lock and it is not aware of the context of the test framework. `BitcoinTestFramework` offers a `wait_until()` which has an understandable amount of default `timeout` and a shared `timeout_factor`. Moreover, on top of these, `mininode.wait_until()` also has a shared lock. closes #19080 ACKs for top commit: MarcoFalke: ACK d841301010914203fb5ef02627c76fad99cb11f1 🦆 kallewoof: utACK d841301010914203fb5ef02627c76fad99cb11f1 Tree-SHA512: 81604f4cfa87fed98071a80e4afe940b3897fe65cf680a69619a93e97d45f25b313c12227de7040e19517fa9c003291b232f1b40b2567aba0148f22c23c47a88
2020-08-26Merge #19778: test: Fix intermittent issue in wallet_bumpfeeMarcoFalke
fafc9d5af48c987212af4170bb8b2504be8138f8 test: Fix intermittent issue in wallet_bumpfee (MarcoFalke) fa347b2f2570a9436313a7c5d61015f2088662f4 test: Select at least the fee in wallet_bumpfee to avoid negative amounts (MarcoFalke) Pull request description: With a "dirty" mempool a transaction might fail to be accepted intermittently. For example, * https://travis-ci.org/github/bitcoin-core/gui/jobs/719916499#L6773 Fails acceptance * https://travis-ci.org/github/bitcoin-core/gui/jobs/719916499#L6954 Test fails Fix the issue by clearing the mempool between subtests ACKs for top commit: promag: Code review ACK fafc9d5af48c987212af4170bb8b2504be8138f8. Tree-SHA512: 23fb6decb6343d19eafddcbdb7da0551f6be11325d1c97c30e563944000aeb02bcc4b24904d204b132c093dc1acf28445fa1fd08bfe8d8b52ddd1de51c33eeb6
2020-08-26test: Add docstring to wait_until() in util.py to warn about its usageSeleme Topuz
2020-08-26test: Update wait_until usage in tests not to use the one from utilsSeleme Topuz
Replace "wait_until()" usage from utils, with the ones from BitcoinTestFramework and P2PInterface. closes #19080
2020-08-26Merge bitcoin-core/gui#35: Parse params directly instead of through node ↵MarcoFalke
(partial revert #10244) 519cae8fd6e44aef3470415d7c5e12acb0acd9f4 gui: Delay interfaces::Node initialization (Russell Yanofsky) 102abff9eb6c267af64f2a3560712147d1896e13 gui: Replace interface::Node references with pointers (Russell Yanofsky) 91aced7c7e6e75c1f5896b7e3843015177f32748 gui: Remove unused interfaces::Node references (Russell Yanofsky) e1336316250ab5cb0ed654b1e593378a6e0769ce gui: Partially revert #10244 gArgs and Params changes (Russell Yanofsky) Pull request description: This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/projects/10). --- This is a partial revert of https://github.com/bitcoin/bitcoin/pull/10244. It changes gui code to go back to using gArgs and Params() functions directly instead of using interfaces::Node to handle arguments. These changes were originally pushed as part of https://github.com/bitcoin/bitcoin/pull/19461. Motivation is to support a new GUI process connecting to an already running node process. Details are explained in commit messages, but in addition to spawning a new bitcoin-node process, we want bitcoin-gui to connect to an existing bitcoin-node process. So for that reason it should be able to parse its own parameters, rather than rely on the node. ACKs for top commit: MarcoFalke: re-ACK 519cae8fd6, only change is rebase and addressed nits of my previous review last week 🌄 Tree-SHA512: 9c339dd82ba78bcc7b887b84d872f35ccc7dfa3d271691e6eafe8a2048cbbe3bdde1e810ce33d0714d75d048c9de3470e9e9b6f8306a6047d1cb3548f6858dc8
2020-08-26bench: Prevent thread oversubscriptionHennadii Stepanov
This change decreases the variance of benchmark results.
2020-08-26Merge #19687: refactor: make EncodeBase{32,64} consume SpansMarcoFalke
e2aa1a585a83971639572cd2c84565ec360deac9 util: make EncodeBase64 consume Spans (Sebastian Falbesoner) 2bc207190e6f1818b04c9336f6e0d625b2a2a0ba util: make EncodeBase32 consume Spans (Sebastian Falbesoner) Pull request description: To simplify the interface of the Base32/Base64 encoding functions for raw data, this PR changes them from taking two arguments (pointer and length) to just one Span. Most calls to `EncodeBase64` pass data from `CDataStream` instances, which unfortunately internally work with `char*` pointers rather than `unsigned char*`, but thanks to the recently introduced `MakeUCharSpan` helper, converting them is quite easy. ACKs for top commit: MarcoFalke: ACK e2aa1a585a83971639572cd2c84565ec360deac9 🐮 vasild: ACK e2aa1a585 Tree-SHA512: 43bd3bd2ee8e3be2474db0a81dae9d9e88fac2464b96d1b042147106ed7433799dcba3000c69990511ecfc697b0c7306ce85f2ecb2293e2e44fd356c9694b150
2020-08-26gui: Delay interfaces::Node initializationRussell Yanofsky
This is needed to allow bitcoin-gui to connect to existing node process with -ipcconnect instead of spawning a new process. It's possible to spawn a new bitcoin-node process without knowing the current data dir or network, but connecting to an existing bitcoin-node requires knowing the datadir and network first.
2020-08-26gui: Replace interface::Node references with pointersRussell Yanofsky
No change in behavior. Replacing references with pointers allows Node interface creation to be delayed until later during gui startup next commit to support implementing -ipcconnect option
2020-08-26gui: Remove unused interfaces::Node referencesRussell Yanofsky
Remove Node references no longer needed after previous commit
2020-08-26gui: Partially revert #10244 gArgs and Params changesRussell Yanofsky
Change gui code to use gArgs, Params() functions directly instead of going through interfaces::Node. Remotely accessing bitcoin-node ArgsManager from bitcoin-gui works fine in https://github.com/bitcoin/bitcoin/pull/10102, when bitcoin-gui spawns a new bitcoin-node process and controls its startup, but for bitcoin-gui to support -ipcconnect option in https://github.com/bitcoin/bitcoin/pull/19461 and connect to an existing bitcoin-node process, it needs ability to parse arguments itself before connecting out. This change also simplifies https://github.com/bitcoin/bitcoin/pull/10102 a bit, by making the bitcoin-gui -> bitcoin-node startup sequence more similar to the bitcoin-node -> bitcoin-wallet startup sequence where the parent process parses arguments and passes them to the child process instead of the parent process using the child process to parse arguments.
2020-08-26Merge #19760: test: Remove confusing mininode terminologyfanquake
d5800da5199527a366024bc80cad7fcca17d5c4a [test] Remove final references to mininode (John Newbery) 5e8df3312e47a73e747ee892face55ed9ababeea test: resort imports (John Newbery) 85165d4332b0f72d30e0c584b476249b542338e6 scripted-diff: Rename mininode to p2p (John Newbery) 9e2897d020b114a10c860f90c5405be029afddba scripted-diff: Rename mininode_lock to p2p_lock (John Newbery) Pull request description: New contributors are often confused by the terminology in the test framework, and what the difference between a _node_ and a _peer_ is. To summarize: - a 'node' is a bitcoind instance. This is the thing whose behavior is being tested. Each bitcoind node is managed by a python `TestNode` object which is used to start/stop the node, manage the node's data directory, read state about the node (eg process status, log file), and interact with the node over different interfaces. - one of the interfaces that we can use to interact with the node is the p2p interface. Each connection to a node using this interface is managed by a python `P2PInterface` or derived object (which is owned by the `TestNode` object). We can open zero, one or many p2p connections to each bitcoind node. The node sees these connections as 'peers'. For historic reasons, the word 'mininode' has been used to refer to those p2p interface objects that we use to connect to the bitcoind node (the code was originally taken from the 'mini-node' branch of https://github.com/jgarzik/pynode/tree/mini-node). However that name has proved to be confusing for new contributors, so rename the remaining references. ACKs for top commit: amitiuttarwar: ACK d5800da519 MarcoFalke: ACK d5800da5199527a366024bc80cad7fcca17d5c4a 🚞 Tree-SHA512: 2c46c2ac3c4278b6e3c647cfd8108428a41e80788fc4f0e386e5b0c47675bc687d94779496c09a3e5ea1319617295be10c422adeeff2d2bd68378e00e0eeb5de
2020-08-26Merge #19779: Remove gArgs global from initfanquake
fa9d5902f7d72e8cce105dd1b1f5a1062e304b10 scripted-diff: gArgs -> args (MarcoFalke) fa33bc2dabbbd2d73961f9b0ce51420a3b6e4ad5 init: Capture copy of blocknotify setting for BlockNotifyCallback (MarcoFalke) fa40017706e08b4de111e8e57aabeced60881a57 init: Pass reference to ArgsManager around instead of relying on global (MarcoFalke) Pull request description: The gArgs global has several issues: * gArgs is used by each process (bitcoind, bitcoin-qt, bitcoin-wallet, bitcoin-cli, bitcoin-tx, ...), but it is hard to determine which arguments are actually used by each process. For example arguments that have never been registered, but are still used, will always return the fallback value. * Tests may run several sub-tests, which need different settings. So globals will have to be overwritten, but that is fragile on its own: e.g. https://github.com/bitcoin/bitcoin/pull/19704#issuecomment-678259092 or #19511 The goal is to remove gArgs, but as a first step in that direction this pull will change gArgs in init to use a passed-in reference instead. ACKs for top commit: ryanofsky: Code review ACK fa9d5902f7d72e8cce105dd1b1f5a1062e304b10. Looks good. Nice day to remove some globals, and add some lambdas :+1: fanquake: ACK fa9d5902f7d72e8cce105dd1b1f5a1062e304b10 - I'm not as familiar with the settings & argument handling code, but this make sense, and is a step in the right direction towards a reduction in the usage of globals. Not a huge fan of the clang-formatting in the scripted diff. jonasschnelli: Concept ACK fa9d5902f7d72e8cce105dd1b1f5a1062e304b10 Tree-SHA512: ed00db5f826566c7e3b4d0b3d2ee0fc1a49a6e748e04e5c93bdd694ac7da5598749e73937047d5fce86150d764a067d2ca344ba4ae3eb2704cc5c4fa0d20940f
generated by cgit v1.2.3 (git 2.26.2) at 2024-11-10 14:40:59 +0000