aboutsummaryrefslogtreecommitdiff
path: root/src
AgeCommit message (Collapse)Author
2020-06-07Merge #19005: doc: Add documentation for 'checklevel' argument in ↵MarcoFalke
'verifychain' RPC… 501e6ab4e778d8f4e95fdc807eeb8644df16203b doc: Add documentation for 'checklevel' argument in 'verifychain' RPC call (Calvin Kim) Pull request description: Rationale: When ```bitcoin-cli help verifychain``` is called, the user doesn't get any documentation about the ```checklevel``` argument, leading to issues like #18995. This PR addresses that issue and adds documentation for what each level does, and that each level includes the checks of the previous levels. ACKs for top commit: jonatack: ACK 501e6ab4e778d8f4e95fdc807eeb8644df16203b `git diff 292ed3c 501e6ab` shows only change since last review is the verifychain RPCHelpMan edit; rebuild and retested manually anyway MarcoFalke: ACK 501e6ab4e778d8f4e95fdc807eeb8644df16203b 🚝 Tree-SHA512: 09239f79c25b5c3022b8eb1f76198ba681305d7e8775038e46becffe5f6a14c572e0c5d06b0723fe9d4a015ec42c9f7ca7b80a2a93df0b1b66f5a84a80eeeeb1
2020-06-07doc: Add documentation for 'checklevel' argument in 'verifychain' RPC callCalvin Kim
2020-06-06Merge #18968: doc: noban precludes maxuploadtarget disconnectsMarcoFalke
fa9604c46f3245a704487c29b684caadffbf73bc doc: noban precludes maxuploadtarget disconnects (MarcoFalke) fa3999fe351b510bb141dff9ae4ecc8e717bf292 net: Reformat excessively long if condition into multiple lines (MarcoFalke) Pull request description: Whitelisting has been replaced by permission flags, so properly document this. See also #10131 ACKs for top commit: hebasto: ACK fa9604c46f3245a704487c29b684caadffbf73bc, I have reviewed the code and it looks OK, I agree it can be merged. ariard: ACK fa9604c Tree-SHA512: 5aee917ab9817719f01ec155487542118e17fa3d145ae7e4bc0e872b2cec39cde9e7fbdee2ae77e9a52700dd8bcc366de4224152e08e709d44d08e0d2f19c613
2020-06-05Merge #19096: Remove g_rpc_chain globalMarcoFalke
4a7253ab6c3bb323581cea54573529c2f823f035 Remove g_rpc_chain global (Russell Yanofsky) e783197bf0f7429f80fea94b44c59857bc8cfef9 refactor: replace RegisterWalletRPCCommands with GetWalletRPCCommands (Russell Yanofsky) Pull request description: Replace with RPC request reference to new WalletContext struct similar to the existing NodeContext struct and reference. This PR is a followup to #18740 removing the g_rpc_node global. Some later PRs will follow this up and move more wallet globals to the WalletContext struct. ACKs for top commit: MarcoFalke: ACK 4a7253ab6c3bb323581cea54573529c2f823f035 🎋 ariard: Code Review ACK 4a7253a, feel free to ignore comment it's super nit. Tree-SHA512: 5bb5561c89f81811ca5232a58bf450e230d4218e62471c03227d142395fd36131672e99cb88329b33b9680a235db01e8b9d1c1e2a18288349e57205528deabab
2020-06-05Merge #19132: qt: lock cs_main, m_cached_tip_mutex in that orderJonas Schnelli
f46b678acff0b2e75e26aa50b14d935b3d251a2a qt: lock cs_main, m_cached_tip_mutex in that order (Vasil Dimov) Pull request description: Always lock the mutexes `cs_main` and `m_cached_tip_mutex` in the same order: `cs_main`, `m_cached_tip_mutex`. Otherwise we may end up in a deadlock. `ClientModel::m_cached_tip_blocks` is protected by `ClientModel::m_cached_tip_mutex`. There are two access paths that lock the two mutexes in opposite order: ``` validation.cpp:2868 CChainState::ActivateBestChain(): lock cs_main validation.cpp:2916 CChainState::ActivateBestChain(): call uiInterface.NotifyBlockTip() ui_interface.cpp:52 CClientUIInterface::NotifyBlockTip(): go deep in boost ... qt/clientmodel.cpp:255 BlockTipChanged(): lock m_cached_tip_mutex ``` and ``` qt/clientmodel.cpp:119 ClientModel::getBestBlockHash(): lock m_cached_tip_mutex qt/clientmodel.cpp:121 ClientModel::getBestBlockHash(): call m_node.getBestBlockHash() interfaces/node.cpp:200 NodeImpl::getBestBlockHash(): lock cs_main ``` From `debug.log`: ``` POTENTIAL DEADLOCK DETECTED Previous lock order was: m_cs_chainstate validation.cpp:2851 (1) cs_main validation.cpp:2868 ::mempool.cs validation.cpp:2868 (2) clientmodel->m_cached_tip_mutex qt/clientmodel.cpp:255 Current lock order is: (2) m_cached_tip_mutex qt/clientmodel.cpp:119 (1) ::cs_main interfaces/node.cpp:200 ``` The possible deadlock was introduced in #17993 ACKs for top commit: jonasschnelli: Tested ACK f46b678acff0b2e75e26aa50b14d935b3d251a2a Tree-SHA512: 904f24b39bdc97c4d0ecb897a6980d8d479814535eb167e23105238800ea2f1f85273e3370cf894db58bc597f94c4f2e81fb68d0ff3362d468c16af5ce8f5d78
2020-06-05Merge #15202: gui: Add Close All Wallets actionJonas Schnelli
c4b574899abfa27f83c6948593838ed418c78f9c gui: Add Close All Wallets action (João Barbosa) f30960adc02877267cb6efeb378962ed96628741 gui: Add closeAllWallets to WalletController (João Barbosa) Pull request description: This PR adds the action to close all wallets. <img width="405" alt="Screenshot 2020-06-01 at 01 06 12" src="https://user-images.githubusercontent.com/3534524/83365986-25a8b980-a3a4-11ea-9613-24dcd8eaa55c.png"> ACKs for top commit: jonasschnelli: Tested ACK c4b574899abfa27f83c6948593838ed418c78f9c Tree-SHA512: 049ad77ac79949fb55f6bde47b583fbf946f4bfaf3d56d768e85f813d814cff0fe326b700f7b5e383cda4af7b5666e13043a6aaeee3798a69fc94385d88ce809
2020-06-05Merge #18758: Remove unused boost/threadfanquake
89f9fef1f71dfeff4baa59bc42bc9049a46d911b refactor: Specify boost/thread/thread.hpp explicitly (Hennadii Stepanov) fad8c890f5ae6e083e416781b4857a1a53ad5249 txdb: Remove unused boost/thread (MarcoFalke) faa958bc283023334b9377f5fb2210459ca16d82 txindex: Remove unused boost/thread (MarcoFalke) Pull request description: There are predefined interruption points for `boost::thread`: https://www.boost.org/doc/libs/1_71_0/doc/html/thread/thread_management.html#interruption_points However, non-boost threads such as `std::thread` or the `main()` thread can obviously not be interrupted. So remove all unused boost/thread from methods that are never executed in a `boost::thread`. Most of them were accompanied by a `ShutdownRequested` anyway. So even if the current thread was a `boost::thread`, the interruption point would be redundant. (We only interrupt threads during shutdown) ACKs for top commit: fanquake: ACK 89f9fef1f71dfeff4baa59bc42bc9049a46d911b hebasto: ACK 89f9fef1f71dfeff4baa59bc42bc9049a46d911b, tested on Linux Mint 19.3 (x86_64), verified shutdown in different scenarios. Tree-SHA512: 17221dadedf2d107e5bda9e4f371cc4f8ffce6ad27cae41aa2b8f1150d8f1adf23d396585ca4a2dd25b1dc6f0d5c81fecd950d8557966ccb45a6d4a85a331d90
2020-06-04doc: noban precludes maxuploadtarget disconnectsMarcoFalke
2020-06-04net: Reformat excessively long if condition into multiple linesMarcoFalke
Can be reviewed with the git option --word-diff-regex=.
2020-06-04Merge #19053: refactor: replace CNode pointers by references within ↵MarcoFalke
net_processing.{h,cpp} 8b3136bd307123a255b9166aa42a497a44bcce70 refactor: replace CNode pointers by references within net_processing.{h,cpp} (Sebastian Falbesoner) Pull request description: This PR is inspired by a [recent code review comment](https://github.com/bitcoin/bitcoin/pull/19010#discussion_r426954791) on a PR that introduced new functions to the net_processing module. The point of the discussion was basically that whenever we pass something not by value (in the concrete example it was about `CNode*` and `CConnman*`) we should either use * a pointer (```CType*```) with null pointer check or * a reference (```CType&```) To keep things simple, this PR for a first approach * only tackles `CNode*` pointers * only within the net_processing module, i.e. no changes that would need adaption in other modules * keeps the names of the variables as they are I'm aware that PRs like this are kind of a PITA to review, but I think the code quality would increase if we get rid of pointers without nullptr check -- bloating up the code by adding all the missing checks would be the worse alternative, in my opinion. Possible follow-up PRs, in case this is received well: * replace CNode pointers by references for net module * replace CConnman pointers by references for net_processing module * ... ACKs for top commit: MarcoFalke: ACK 8b3136bd307123a255b9166aa42a497a44bcce70 🔻 practicalswift: ACK 8b3136bd307123a255b9166aa42a497a44bcce70 Tree-SHA512: 15b6a569ecdcb39341002b9f4e09b38ed4df077e3a3a50dfb1b72d98bdc9f9769c7c504f106456aa7748af8591af7bb836b72d46086df715ab116e4ac3224b3b
2020-06-04Merge #19112: rpc: Remove special case for unknown service flagsWladimir J. van der Laan
fa1433ac1be8481f08c1a0a311a6b87d8a874c6a rpc: Remove special case for unknown service flags (MarcoFalke) Pull request description: The special case to return a bit as an integer is clumsy and undocumented. Probably also irrelevant because there shouldn't currently be a non-misbehaving client that connects to Bitcoin Core and advertises an unknown service flag. Thus, simply remove the code. ACKs for top commit: laanwj: ACK fa1433ac1be8481f08c1a0a311a6b87d8a874c6a Tree-SHA512: 942de6a577a9ee076ce12c92be121617640d53ee8c3424064c45a30a7ff789555d3722a4203670768faf81da2a40adfed3ec5cdeb5da06f04be81ddb53b9db7e
2020-06-04Merge #17994: validation: flush undo files after last block writeWladimir J. van der Laan
ac94141af0c16161afa68de1c3720f254ae4e12c validation: delay flushing undo files in syncing node case (Karl-Johan Alm) Pull request description: Fixes #17890. Replaces #17892. Data files (`{blk|rev}<number>.dat`) pre-allocate space as they are written, and then trims down to the final size once they move on to the next sequence ("finalized flush"). The code currently assumes (incorrectly) that blk and rev files finish at the same time, but because blk files are written as blocks come in, and rev files are written in block height order, rev files end up being written to for awhile after moving on to the next block file, resulting in pre-allocation and waste of up to 1 MB of space per rev file. The exact point at which rev file writing finishes is the highest height block found inside the corresponding block file, which is already available in the CBlockFileInfo vector. This PR moves finalized flushing of undo files to to directly after the undo data for the previous block file has been written. There is a branch with annotation that demonstrates how this is handling flushing here: https://github.com/kallewoof/bitcoin/tree/200124-rev-files-annotated ACKs for top commit: vasild: ACK ac94141af (no changes in the code since ed34e00da). fjahr: Code review re-ACK ac94141af0c16161afa68de1c3720f254ae4e12c jonatack: Code review ACK ac94141af0c16 Tree-SHA512: 1d4e3b3d1d99bd7ebe7a2f632b1231146dd4f9f993c54db3a4090d9c086d95d2e4c327fd936066392b3afc6277b8f3a908d5c5993d4c8e49f72b92a417716dd2
2020-06-04Merge #19131: refactor: Fix unreachable code in init arg checksWladimir J. van der Laan
eea81146571480b2acd12c8cd7f36b04d056c47f build: Enable unreachable-code-loop-increment (Jonathan Schoeller) d15db4b1fc988736b08c092d000ca1d1ff686975 refactor: Fix unreachable code in init arg checks (Jonathan Schoeller) Pull request description: Closes: #19017 In #19015 it's been suggested that we add some new compiler warnings to our build. Some of these, such as `-Wunreachable-code-loop-increment`, generate warnings. We'll likely want to fix these up if we're going to turn these warnings on. ```shell init.cpp:969:5: warning: loop will run at most once (loop increment never executed) [-Wunreachable-code-loop-increment] for (const auto& arg : gArgs.GetUnsuitableSectionOnlyArgs()) { ^~~ 1 warning generated. ``` https://github.com/bitcoin/bitcoin/blob/aa8d76806c74a51ec66e5004394fe9ea8ff0fac4/src/init.cpp#L968-L972 To fix this, collect all errors, and output them in a single error message after the loop completes. This resolves the unreachable code warning, and avoids popup hell that could result from outputting a seperate message for each error or warning one by one. ACKs for top commit: laanwj: Code review ACK eea81146571480b2acd12c8cd7f36b04d056c47f hebasto: re-ACK eea81146571480b2acd12c8cd7f36b04d056c47f, only suggested changes applied since the [previous](https://github.com/bitcoin/bitcoin/pull/19131#pullrequestreview-421772387) review. Tree-SHA512: 2aa3ceb7fab581b6ba2580900668388d8eba1c3001c8ff9c11c1f4a9a10fbc37f30e590249862676858446e3f4950140a252953ba1643ba3bfd772f8eae20583
2020-06-04refactor: Specify boost/thread/thread.hpp explicitlyHennadii Stepanov
2020-06-04txdb: Remove unused boost/threadMarcoFalke
2020-06-04txindex: Remove unused boost/threadMarcoFalke
2020-06-04Merge #19142: validation: Make VerifyDB level 4 interruptiblefanquake
fa3b4f9b8e54ec07aeb2e5e2333da3e784f7be12 validation: Make VerifyDB level 4 interruptible (MarcoFalke) fa1d5800d9c5e33942b76f6667839a818723dee9 validation: Remove unused boost interruption_point (MarcoFalke) Pull request description: level 0,1,2, and 3 are already interruptible, so make level 4 also interruptible ACKs for top commit: laanwj: Code review ACK fa3b4f9b8e54ec07aeb2e5e2333da3e784f7be12 fanquake: ACK fa3b4f9b8e54ec07aeb2e5e2333da3e784f7be12 Tree-SHA512: d302c84a17add1b5993dd78339c88670d27eee45ce208c4d046ae188b50be9843ee5a9584739d5d25453b54ae08fd1cb6eeee8cb1307d84c05cde8a54a7c445b
2020-06-03Merge #19088: validation: use std::chrono throughout some validation functionsMarcoFalke
789e9dd3aa727176797529c35b2848f994630a82 validation: use std::chrono in IsCurrentForFeeEstimation() (fanquake) 47be28c8bc475eafeebd4fc58ea92f0d3df0d8c6 validation: use std::chrono in CChainState::FlushStateToDisk() (fanquake) Pull request description: Probably up for debate as to which type is used for the constants. Personally, swapping these to hours is more readable. ACKs for top commit: MarcoFalke: ACK 789e9dd3aa727176797529c35b2848f994630a82 jonatack: ACK 789e9dd3aa727176797529c35b2848f994630a82 Tree-SHA512: f4a25cbd00a49a54b7783a1f588be83706dd2a475cecb5c2e8b97b2d4b27c0955a7454d7486f2454e96351c44f233b300c4f4b9ca62fc7336277f10da34dd5c3
2020-06-03Merge #18875: fuzz: Stop nodes in process_message* fuzzersMarcoFalke
fab860aed4878b831dae463e1ee68029b66210f5 fuzz: Stop nodes in process_message* fuzzers (MarcoFalke) 6666c828e072a5e99ea0c16394ca3e5b9de07409 fuzz: Give CNode ownership to ConnmanTestMsg in process_message fuzz harness (MarcoFalke) Pull request description: Background is that I saw an integer overflow in net_processing ``` #30629113 REDUCE cov: 25793 ft: 142917 corp: 3421/2417Kb lim: 4096 exec/s: 89 rss: 614Mb L: 1719/4096 MS: 1 EraseBytes- net_processing.cpp:977:25: runtime error: signed integer overflow: 2147483624 + 100 cannot be represented in type 'int' SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior net_processing.cpp:977:25 in net_processing.cpp:985:9: runtime error: signed integer overflow: -2147483572 - 100 cannot be represented in type 'int' SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior net_processing.cpp:985:9 in ``` Telling from the line numbers, it looks like `nMisbehavior` wrapped around. Fix that by calling `StopNodes` after each exec, which should clear the node state and thus `nMisbehavior`. ACKs for top commit: practicalswift: ACK fab860aed4878b831dae463e1ee68029b66210f5 Tree-SHA512: 891c081d5843565d891aec028b6c27ef3fa39bc40ae78238e81d8f784b4d4b49cb870998574725a5159dd03aeeb2e0b9bc3d3bb51d57d1231ef42e3394b2d639
2020-06-03validation: Make VerifyDB level 4 interruptibleMarcoFalke
2020-06-03validation: Remove unused boost interruption_pointMarcoFalke
ActivateBestChain (ABC) is only called in the "msghand" or one of the RPC threads, neither of which is a boost::thread. However, ABC is also called in ThreadImport (which currently happens to be a boost::thread). In all cases, the interruption_point is redundant with the breakpoint in ABC that triggers when ShutdownRequested() VerifyDB is only called in the main thread ("init") or one of the RPC threads, neither of which is a boost::thread.
2020-06-03Merge #19084: net: improve code documentation for dns seed behaviourfanquake
5cb7ee67a5c0e5da39eb698b64d23722fb2f7b3e net: improve code documentation for dns seed behaviour (Anthony Towns) Pull request description: Some better internal documentation post #16939 ACKs for top commit: hebasto: ACK 5cb7ee67a5c0e5da39eb698b64d23722fb2f7b3e naumenkogs: ACK 5cb7ee6 ariard: ACK 5cb7ee6 fanquake: ACK 5cb7ee67a5c0e5da39eb698b64d23722fb2f7b3e - thanks for following up. Tree-SHA512: 5a12680651cd1e0436aed1cadcbf50047ffeabfdc9f7f2f81fa176c30b10673fc960154c73ec34ed08ace1a000a81cedd44d67587883152654dee46065991b45
2020-06-02Merge #18982: wallet: Minimal fix to restore conflicted transaction ↵MarcoFalke
notifications 7eaf86d3bfc83f2beb3ef449707d5156853126fb trivial: Suggested cleanups to surrounding code (Russell Yanofsky) b604c5c8b5892842f13dee89ae31812a28ab25d1 wallet: Minimal fix to restore conflicted transaction notifications (Russell Yanofsky) Pull request description: This fix is a based on the fix by Antoine Riard (ariard) in https://github.com/bitcoin/bitcoin/pull/18600. Unlike that PR, which implements some new behavior, this just restores previous wallet notification and status behavior for transactions removed from the mempool because they conflict with transactions in a block. The behavior was accidentally changed in two `CWallet::BlockConnected` updates: a31be09bfd77eed497a8e251d31358e16e2f2eb1 and 7e89994133725125dddbfa8d45484e3b9ed51c6e from https://github.com/bitcoin/bitcoin/pull/16624, causing issue https://github.com/bitcoin/bitcoin/issues/18325. The change here could be improved and replaced with a more comprehensive cleanup, so it includes a detailed comment explaining future considerations. Fixes #18325 Co-authored-by: Antoine Riard (ariard) ACKs for top commit: jonatack: Re-ACK 7eaf86d3bfc83f ariard: ACK 7eaf86d, reviewed, built and ran tests. MarcoFalke: ACK 7eaf86d3bfc83f2beb3ef449707d5156853126fb 🍡 Tree-SHA512: 9a1efe975969bb522a9dd73c41064a9348887cb67883cd92c6571fd2df4321b9f4568363891abdaae14a3b9b168ef8142e95c373fc04677e46289b251fb84689
2020-06-02Merge #18792: wallet: Remove boost from PeriodicFlushfanquake
fa1c74fd0342b74d44cc4e41fff3890c1434e8f7 wallet: Remove unused boost::thread_interrupted (MarcoFalke) fa7b885f51ff848d3f913bc6e15d24528300c210 walletdb: Remove unsed boost/thread (MarcoFalke) 5555d978b056ab0e0e59faaf2d2067ec43fffaef wallet: Make PeriodicFlush uninterruptible (MarcoFalke) Pull request description: The `boost::this_thread::interruption_point()` in the code base currently block the replacement of `boost::thread` with `std::thread`. [1] Remove them from the wallet because they are either unused or useless. The feature to interrupt a periodic flush is useless because all wallets have just been flushed https://github.com/bitcoin/bitcoin/blob/9ccaee1d5e2e4b79b0a7c29aadb41b97e4741332/src/init.cpp#L194 and another flush should be a noop. Also, they will be flushed again shortly after https://github.com/bitcoin/bitcoin/blob/9ccaee1d5e2e4b79b0a7c29aadb41b97e4741332/src/init.cpp#L285, so even if repeated flushes weren't a noop, doing 3 instead of 2 shouldn't matter too much at this point. Also, the wallet is flushed every two seconds in the worst case, so if this is an expensive operation, that period should be readjusted. (Or bdb should be removed altogether #18916) [1] Replacement of `boost::thread` with `std::thread` should happen because: * The boost thread dependency is slow to compile * Boost thread is less maintained than the standard lib * Boost thread is mostly redundant to the standard lib * Global interruption points via exceptions are hard to keep track of during review and easy to get wrong during runtime (e.g. accidental `catch (...)`) ACKs for top commit: fanquake: ACK fa1c74fd0342b74d44cc4e41fff3890c1434e8f7 Tree-SHA512: b166619256de2ef4325480fa1367f68bc9371ad785ec503aed61eab41ba61f1a9807aab25451a24efda3db64855c9ba0025645b98bc58557bc3ec56c5b3297d0
2020-06-02Merge #13204: Faster sigcache nonceMarcoFalke
152e8baf08c7379e5cc09f90863e6309bdd4866c Use salted hasher instead of nonce in sigcache (Jeremy Rubin) 5495fa585007b40b2e9285c23be275de71708af8 Add Hash Padding Microbenchmarks (Jeremy Rubin) Pull request description: This PR replaces nonces in two places with pre-salted hashers. The nonce is chosen to be 64 bytes long so that it forces the SHA256 hasher to process the chunk. This leaves the next 64 (or 56 depending if final chunk) open for data. In the case of the script execution cache, this does not make a big performance improvement because the nonce was already properly padded to fit into one buffer, but does make the code a little simpler. In the case of the sig cache, this should reduce the hashing overhead slightly because we are less likely to need an additional processing step. I haven't benchmarked this, but back of the envelope it should reduce the hashing by one buffer for all combinations except compressed public keys with compact signatures. ACKs for top commit: ryanofsky: Code review ACK 152e8baf08c7379e5cc09f90863e6309bdd4866c. No code changes, just rebase since last review and expanded commit message Tree-SHA512: b133e902fd595cfe3b54ad8814b823f4d132cb2c358c89158842ae27daee56ab5f70cde2585078deb46f77a6e7b35b4cc6bba47b65302b7befc2cff254bad93d
2020-06-02wallet: Remove unused boost::thread_interruptedMarcoFalke
FindWalletTx is only called by zapwallet, which is never called in a boost::thread
2020-06-02Merge #19111: Limit scope of all global std::once_flagMarcoFalke
fa9c67559186f5416c1c0b26c0a1d5e72c234ccb Limit scope of all global std::once_flag (MarcoFalke) Pull request description: `once_flag` is a helper (as the name might suggest) to execute a callable only once. Thus, the scope of the flag does never need to extend beyond where the callable is called. Typically this is function scope. Move all the flags to function scope to * simplify code review * avoid mistakes where similarly named flags are accidentally exchanged * avoid polluting the global scope ACKs for top commit: hebasto: ACK fa9c67559186f5416c1c0b26c0a1d5e72c234ccb, tested on Linux Mint 19.3 (x86_64). promag: Code review ACK fa9c67559186f5416c1c0b26c0a1d5e72c234ccb. Tree-SHA512: 095a0c11d93d0ddcb82b3c71676090ecc7e3de3d5e7a2a63ab2583093be279242acac43523bbae2060b4dcfa8f92b54256a0e91fbbae78fa92d2d49e9db62e57
2020-06-02Merge #19104: gui, refactor: Register Qt meta types in application constructorJonas Schnelli
4f49d5222eca11c149713ad34113d5a3d1c577b1 gui, refactor: Register Qt meta types in application constructor (João Barbosa) Pull request description: Removes a warning when running `QT_QPA_PLATFORM=cocoa src/qt/test/test_bitcoin-qt`. ACKs for top commit: jonasschnelli: Re utACK 4f49d5222eca11c149713ad34113d5a3d1c577b1 hebasto: ACK 4f49d5222eca11c149713ad34113d5a3d1c577b1, tested on macOS 10.15.5. Tree-SHA512: e931a022ba83cb0ef04d82544ebd9b18242f8fc2b41443afce4d5c4222f222e8b3517bdb484a1a4f61377c5dceca067d8ccf250da3a727299448e54bec33ed6e
2020-06-02refactor: replace CNode pointers by references within net_processing.{h,cpp}Sebastian Falbesoner
2020-06-02refactor: Fix unreachable code in init arg checksJonathan Schoeller
Building with -Wunreachable-code-loop-increment causes a warning due to always returning on the first iteration of the loop that outputs errors on invalid args. Collect all errors, and output them in a single error message after the loop completes, resolving the warning and avoiding popup hell by outputting a seperate message for each error.
2020-06-01qt: lock cs_main, m_cached_tip_mutex in that orderVasil Dimov
Always lock the mutexes `cs_main` and `m_cached_tip_mutex` in the same order: `cs_main`, `m_cached_tip_mutex`. Otherwise we may end up in a deadlock. `ClientModel::m_cached_tip_blocks` is protected by `ClientModel::m_cached_tip_mutex`. There are two access paths that lock the two mutexes in opposite order: ``` validation.cpp:2868 CChainState::ActivateBestChain(): lock cs_main validation.cpp:2916 CChainState::ActivateBestChain(): call uiInterface.NotifyBlockTip() ui_interface.cpp:52 CClientUIInterface::NotifyBlockTip(): go deep in boost ... qt/clientmodel.cpp:255 BlockTipChanged(): lock m_cached_tip_mutex ``` and ``` qt/clientmodel.cpp:119 ClientModel::getBestBlockHash(): lock m_cached_tip_mutex qt/clientmodel.cpp:121 ClientModel::getBestBlockHash(): call m_node.getBestBlockHash() interfaces/node.cpp:200 NodeImpl::getBestBlockHash(): lock cs_main ``` From `debug.log`: ``` POTENTIAL DEADLOCK DETECTED Previous lock order was: m_cs_chainstate validation.cpp:2851 (1) cs_main validation.cpp:2868 ::mempool.cs validation.cpp:2868 (2) clientmodel->m_cached_tip_mutex qt/clientmodel.cpp:255 Current lock order is: (2) m_cached_tip_mutex qt/clientmodel.cpp:119 (1) ::cs_main interfaces/node.cpp:200 ``` The possible deadlock was introduced in https://github.com/bitcoin/bitcoin/pull/17993
2020-06-01Merge #19072: doc: Expand section on Getting Startedfanquake
facef3d4131f9980a4516282f11731361559509c doc: Explain that anyone can work on good first issues, move text to CONTRIBUTING.md (MarcoFalke) fae2fb2a196ee864e9a13fffc24a0279cd5d17e6 doc: Expand section on Getting Started (MarcoFalke) 100000d1b2c2e38d7a14a31b0af79e0e4316b04c doc: Add headings to CONTRIBUTING.md (MarcoFalke) fab893e0caf510d4836a20194892ef9c71426c51 doc: Fix unrelated typos reported by codespell (MarcoFalke) Pull request description: Some random doc changes: * Add sections to docs, so that they can be linked to * Explain that anyone (even maintainers) are allowed to work on good first issues * Expand section on Getting Started slightly ACKs for top commit: hebasto: ACK facef3d4131f9980a4516282f11731361559509c fanquake: ACK facef3d4131f9980a4516282f11731361559509c Tree-SHA512: 8998e273a76dbf4ca77e79374c14efe4dfcc5c6df6b7d801e1e1e436711dbe6f76b436f9cbc6cacb45a56827babdd6396f3bd376a9426ee7be3bb9b8a3b8e383
2020-06-01gui: Add Close All Wallets actionJoão Barbosa
2020-06-01gui: Add closeAllWallets to WalletControllerJoão Barbosa
2020-05-31Merge #18994: tests: Add fuzzing harnesses for functions in script/MarcoFalke
f898ef65c947776750e49d050633f830546bbdc6 tests: Add fuzzing harness for functions in script/sign.h (practicalswift) c91d2f06150cda258a17e78d9b7065b594d34a85 tests: Add fuzzing harness for functions in script/sigcache.h (practicalswift) d3d8adb79fbe34b15cf29334607f9b76d303aa1a tests: Add fuzzing harness for functions in script/interpreter.h (practicalswift) fa80117cfdeca7e5d3a2a09b385c0e938bf701e9 tests: Add fuzzing harness for functions in script/descriptor.h (practicalswift) 43fb8f0ca331a7f79f0d287817da7f4b894bdbfa tests: Add fuzzing harness for functions in script/bitcoinconsensus.h (practicalswift) 8de72711c685e638fa54d485694fb1b1af024adc tests: Fill fuzzing coverage gaps for functions in script/script.h, script/script_error.h and script/standard.h (practicalswift) c571ecb07145b4ce8c17ca80489f8f1497388c4d tests: Add fuzzing helper functions ConsumeDataStream, ConsumeTxDestination and ConsumeUInt160 (practicalswift) Pull request description: Add fuzzing harnesses for functions in `script/`: * Add fuzzing helper functions `ConsumeDataStream` and `ConsumeUInt160` * Fill fuzzing coverage gaps for functions in `script/script.h`, `script/script_error.h` and `script/standard.h` * Add fuzzing harness for functions in `script/bitcoinconsensus.h` * Add fuzzing harness for functions in `script/descriptor.h` * Add fuzzing harness for functions in `script/interpreter.h` * Add fuzzing harness for functions in `script/sigcache.h` * Add fuzzing harness for functions in `script/sign.h` 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: ACK f898ef65c947776750e49d050633f830546bbdc6 🔉 Tree-SHA512: f6e77b34dc79f23de5fa9e38ac06e6554b5b946ec3e9a67e2bd982e60aca37ce844f785457ef427a5e3b45e31c305456bca8587cc9f4a0b50b3852e39726eb04
2020-05-31Merge #19044: net processing: Add support for getcfiltersMarcoFalke
9e36067d8cd02830c7e5a88a391dff6ac3adbe0c [test] Add test for cfilters. (Jim Posen) 11106a4722558765a44ae45c7892724a73ce514c [net processing] Message handling for getcfilters. (Jim Posen) e535670726952e43483763dfca6fc6ec2f4b0691 [indexes] Fix default [de]serialization of BlockFilter. (Jim Posen) bb911ae7f5cbe4974ec61266d2334b95067fa49d [refactor] Pass CNode and CConnman by reference (John Newbery) Pull request description: Support `getcfilters` requests when `-peerblockfilters` is set. Does not advertise compact filter support in version messages. ACKs for top commit: Empact: re-Code Review ACK https://github.com/bitcoin/bitcoin/pull/19044/commits/9e36067d8cd02830c7e5a88a391dff6ac3adbe0c MarcoFalke: re-ACK 9e36067d8c , only change is adding commit "[refactor] Pass CNode and CConnman by reference" 🥑 jkczyz: ACK 9e36067d8cd02830c7e5a88a391dff6ac3adbe0c fjahr: Code review ACK 9e36067d8cd02830c7e5a88a391dff6ac3adbe0c Tree-SHA512: b45b42a25905ef0bd9e195029185300c86856c87f78cbe17921f4a25e159ae0f6f003e61714fa43779017eb97cd89d3568419be88e47d19dc8095562939e7887
2020-05-31Merge #16564: test: Always define the raii_event_tests test suiteMarcoFalke
9a19c9ada53e84d1ee8521b48f656faad48b737a Always define the raii_event_tests test suite (Craig Andrews) Pull request description: The test suite must always be defined (even when EVENT_SET_MEM_FUNCTIONS_IMPLEMENTED is not defined) so that the test harness doesn't fail due to not being able to find the raii_event_tests test. This improves upon 95f97f4 actually fixing https://github.com/bitcoin/bitcoin/issues/9493 ACKs for top commit: MarcoFalke: ACK 9a19c9ada53e84d1ee8521b48f656faad48b737a 🎹 Tree-SHA512: 3c42f17a9b5d56c8841f3aa9ac19da91c10aff210026266f31f7eb98a62528740d7c518c121452b68e8f801d6c80ecfb627d137ec6ed533289fa3beb08b4f176
2020-05-30Merge #18807: [doc / test / mempool] unbroadcast follow-upsMarcoFalke
9e1cb1adf1800efe429e348650931f2669b0d2c0 [trivial/doc] Fix comment type (Amiti Uttarwar) 8f30260a67166a6ab7c0f33f7ec1990d3c31761e [doc] Update unbroadcast description in RPC results (Amiti Uttarwar) 750456d6f29c63d57af05bfbdd6035bb9c965de2 [trivial] Remove misleading 'const' (Amiti Uttarwar) fa32e676e5833a5c5fc735ef00c0a80f5fab7a2c [test] Manage node connections better in mempool persist test (Amiti Uttarwar) 1f94bb0c744a103b633c1051e8fbc01e612097dc [doc] Provide rationale for randomization in scheduling. (Amiti Uttarwar) 9c8a55d9cb0ec73f10b196e79b637aa601c0a6b7 [mempool] Don't throw expected error message when upgrading (Amiti Uttarwar) ba5498318233ab81decbc585e9619d8ffe2df1b0 [test] Test that wallet transactions aren't rebroadcast before 12 hours (Amiti Uttarwar) 00d44a534b4e5ae249b8011360c6b0f7dc731581 [test] P2P connection behavior should meet expectations (Amiti Uttarwar) bd093ca15de762fdaf0937a0877d17b0c2bce16e [test] updates to unbroadcast test (Amiti Uttarwar) dab298d9ab5a5a41685f437db9081fa7b395fa73 [docs] add release notes (Amiti Uttarwar) Pull request description: This PR is a follow up to #18038 which introduced the idea of an unbroadcast set & focuses mostly on documentation updates and test fixes. One small functionality update to not throw an expected error in `LoadMempool` when you upgrade software versions. #18895 is another follow up to that addresses other functionality updates. Background context: The unbroadcast set is a mechanism for the mempool to track locally submitted transactions (via wallet or RPC). The node does a best-effort of delivering the transactions to the network via retries every 10-15 minutes until either a `GETDATA` is received or the transaction is removed from the mempool. ACKs for top commit: MarcoFalke: ACK 9e1cb1adf1 👁 gzhao408: ACK [`9e1cb1a`](https://github.com/bitcoin/bitcoin/pull/18807/commits/9e1cb1adf1800efe429e348650931f2669b0d2c0) Tree-SHA512: 0cd51c4ca368b9dce92d50d73ec6e9df278a259e609eef2858f24cb8595ad07acc3db781d9eb0c351715f18fca5a2b4526838981fdb34a522427e9dc868bdaa6
2020-05-30tests: Add fuzzing harness for functions in script/sign.hpracticalswift
2020-05-30tests: Add fuzzing harness for functions in script/sigcache.hpracticalswift
2020-05-30tests: Add fuzzing harness for functions in script/interpreter.hpracticalswift
2020-05-30tests: Add fuzzing harness for functions in script/descriptor.hpracticalswift
2020-05-30tests: Add fuzzing harness for functions in script/bitcoinconsensus.hpracticalswift
2020-05-30tests: Fill fuzzing coverage gaps for functions in script/script.h, ↵practicalswift
script/script_error.h and script/standard.h
2020-05-29rpc: Remove special case for unknown service flagsMarcoFalke
2020-05-29Limit scope of all global std::once_flagMarcoFalke
2020-05-29Merge #18926: test: Pass ArgsManager into getarg_testsMarcoFalke
f871f15c9d760b56a6a3b78b3941d3983d60810c scripted-diff: replace gArgs with argsman (glowang) 357f02bf2942f2944a04a1ceaa687f89d5da7d28 Create a local class inherited from BasicTestingSetup with a localized args manager and put it into the getarg_tests namespace (glowang) Pull request description: Replaced the global argsManager gArgs with a locally defined one in getarg_tests. This is to avoid confusion in arg settings between the test's ArgsManager and the #18804 ACKs for top commit: MarcoFalke: ACK f871f15c9d760b56a6a3b78b3941d3983d60810c ryanofsky: Code review ACK f871f15c9d760b56a6a3b78b3941d3983d60810c. Changes look good and thanks for updating. In future would recommend using clang-format-diff and following [coding style](https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#coding-style-c) notes, because it's atypical to indent namespace content, or indent protected keywords or put spaces around ::. Also it's fragile to define test setup class in a namespace, but test setup methods outside of the namespace and inside the test fixture instead. Would be simpler to just define the testing setup completely before using it without a namespace like: https://github.com/bitcoin/bitcoin/blob/8ad5f1c376fe99eeccac2696ac0e18e662323a4d/src/test/rpc_tests.cpp#L23 and it would have been a slightly smaller change too. Tree-SHA512: 016594639396d60667fadec8ea80ef7af634fbb2014c704f02406fe3251c5362757c21f1763d8bdb94ca4a3026ab9dc786a92a9a934efc8cd807655d9deee779
2020-05-29Merge #19106: util: simplify the interface of serviceFlagToStr()Jonas Schnelli
189ae0c38b7d4927c5c73b94664e9542b2b06ed9 util: dedup code in callers of serviceFlagToStr() (Vasil Dimov) fbacad1880341ace31f669530c66d4e322d19235 util: simplify the interface of serviceFlagToStr() (Vasil Dimov) Pull request description: Don't take two redundant arguments in `serviceFlagToStr()`. Introduce `serviceFlagsToStr()` which takes a mask (with more than one bit set) and returns a vector of strings. As a side effect this fixes an issue introduced in https://github.com/bitcoin/bitcoin/pull/18165 due to which the GUI could print something like `UNKNOWN[1033] & UNKNOWN[1033] & UNKNOWN[2^10]` instead of `NETWORK & WITNESS`. ACKs for top commit: MarcoFalke: ACK 189ae0c38b7d4927c5c73b94664e9542b2b06ed9 jonasschnelli: Tested ACK 189ae0c38b7d4927c5c73b94664e9542b2b06ed9 Tree-SHA512: 000c490f16ebbba04458c62ca4ce743abffd344d375d95f5bbd5008742012032787655db2874b168df0270743266261dccf1693761906567502dcbac902bda50
2020-05-29util: dedup code in callers of serviceFlagToStr()Vasil Dimov
Introduce `serviceFlagsToStr()` which hides the internals of the bitmask and simplifies callers of `serviceFlagToStr()`.
2020-05-29util: simplify the interface of serviceFlagToStr()Vasil Dimov
Don't take two redundant arguments in `serviceFlagToStr()`. As a side effect this fixes an issue introduced in https://github.com/bitcoin/bitcoin/pull/18165 due to which the GUI could print something like `UNKNOWN[1033] & UNKNOWN[1033] & UNKNOWN[2^10]` instead of `NETWORK & WITNESS`.
generated by cgit v1.2.3 (git 2.26.2) at 2024-11-07 15:54:13 +0000