aboutsummaryrefslogtreecommitdiff
path: root/src
AgeCommit message (Collapse)Author
2020-06-08Merge #18898: gui: Display warnings as rich textMarcoFalke
a9d28afe23a94efdccc53f9f10716f3a0c9337eb qt: Display warnings as rich text (Hennadii Stepanov) Pull request description: On master (6621be53517d69ab855cee4a5978a44d6a133ba3), warnings that contain `<hr />` HTML tag are not displayed correctly: ![Screenshot from 2020-05-06 11-30-10](https://user-images.githubusercontent.com/32963518/81177281-0e49fc80-8faf-11ea-8cac-8847aa517e86.png) Fixed: ![Screenshot from 2020-05-07 07-30-48](https://user-images.githubusercontent.com/32963518/81255618-ca9ad580-9036-11ea-90ad-7f4d89c1880d.png) ACKs for top commit: jonasschnelli: utACK a9d28afe23a94efdccc53f9f10716f3a0c9337eb promag: Code review ACK a9d28afe23a94efdccc53f9f10716f3a0c9337eb. Tree-SHA512: ba5b3837d5f6ea15c3255a3120c9753fc58ee67a370c388556214048ab993c45be720af7cb8d43bb0f12088956cb78abc77546ed1fc691082880438072fe774b
2020-06-08Merge #19194: util: Don't reference errno when pthread fails.fanquake
cb38b069b0f41b1a26264784b1c1303c8ac6ab08 util: Don't reference errno when pthread fails. (MIZUTA Takeshi) Pull request description: Pthread library does not set errno. Pthread library's errno is returned by return value. ACKs for top commit: practicalswift: ACK cb38b069b0f41b1a26264784b1c1303c8ac6ab08 -- patch looks correct MarcoFalke: review ACK cb38b069b0f41b1a26264784b1c1303c8ac6ab08 hebasto: ACK cb38b069b0f41b1a26264784b1c1303c8ac6ab08, only squashed commits since the [previous](https://github.com/bitcoin/bitcoin/pull/19194#pullrequestreview-425831739) review. Tree-SHA512: e6c950e30726e5031db97a7b84c8a9215da5ad3e5d233bcc349f812ad15957ddfe378e26d18339b9e0a5dcac2f50b47a687b87a6a6beaf6139df84f31531321e
2020-06-08Merge #19192: doc: Extract net permissions docMarcoFalke
fa2c2b50d895ff3402b82ce3db69bfc43053b519 doc: Extract net permissions doc (MarcoFalke) Pull request description: Moving the documentation of each flag form the already over-large init.cpp into the net permissions module should clean up the code a bit. Moreover, making the documentation available is also required for an (currently imaginary) `setnetpermissions` RPC. ACKs for top commit: Sjors: re-utACK fa2c2b50d895ff3402b82ce3db69bfc43053b519 Tree-SHA512: c0a75facc9768913c28d2ffcdfaad8d60f7604d5584ee546adaf77d270563558d361aeaf354e49e349aca7e2e80814b27ffc24247e7b4f045c63cbdc079b449f
2020-06-08Merge #19189: refactor: Replace RecursiveMutex with Mutex in timedata.cppMarcoFalke
cc5c0d2299b09c58cd9962ca5075ffa53f2633c0 refactor: Fix formatting of timedata.cpp (Hennadii Stepanov) c2410ceb844a443caf6dd8c6df976b9e24724d06 refactor: Replace RecursiveMutex with Mutex in timedata.cpp (Hennadii Stepanov) Pull request description: Only `GetTimeOffset()` and `AddTimeData()` functions lock this mutex. They do not call itself recursively, and do not call each other either directly or indirectly. Therefore, the `g_timeoffset_mutex` could be a non-recursive mutex. Related to #19180. ACKs for top commit: MarcoFalke: ACK cc5c0d2299b09c58cd9962ca5075ffa53f2633c0 , checked the second commit with --word-diff-regex=. --ignore-all-space -U0 🦉 vasild: ACK cc5c0d22 verified that recursion is not happening Tree-SHA512: 38f6df689374d4a1a0e9aedb3ed5e885d8285c4da6b75f9bc84ae036936a159ef8276462db33b4f4dd5c71c6312fa9b45380f7a5726959665bc71dc39031be88
2020-06-08Merge #19190: refactor: Replace RecursiveMutex with Mutex in netbase.cppMarcoFalke
78c8f4fe11706cf5c165777c2ca122bd933b8b6a refactor: Replace RecursiveMutex with Mutex in netbase.cpp (Hennadii Stepanov) Pull request description: The functions that could lock this mutex, i.e., `{S,G}etProxy()`, `{S,G}etNameProxy()`, `HaveNameProxy()`, `IsProxy()`, do not call itself recursively, and do not call each other either directly or indirectly. Therefore, the `g_proxyinfo_mutex` could be a non-recursive mutex. Related to #19180. ACKs for top commit: MarcoFalke: ACK 78c8f4fe11706cf5c165777c2ca122bd933b8b6a , reviewed with the -W git option 👮 vasild: ACK 78c8f4fe verified that recursion does not happen Tree-SHA512: fc077fb371f38af5d05f1383c6bebf9926167c257892936fefd2d4fe6f679ca40124d25099e09f645d8ec266df222f96c5d0f9fd39eddcad15cbde0b427bc205
2020-06-08doc: Extract net permissions docMarcoFalke
2020-06-08Merge #19180: refactor: Replace RecursiveMutex with Mutex in Shutdown()MarcoFalke
1a9ef1d398dd14728b6bc67a89139cdf827c9753 refactor: Replace RecursiveMutex with Mutex in Shutdown() (Hennadii Stepanov) Pull request description: Step by step, going to replace all of the `RecursiveMutex` instances with the `Mutex` ones throughout the code base :) Not sure if it is possible in all cases though... This one is a low-hanging fruit. ACKs for top commit: MarcoFalke: ACK 1a9ef1d398dd14728b6bc67a89139cdf827c9753 Shutdown is not recursive, so the same thread can never lock twice (UB) vasild: ACK 1a9ef1d3 verified manually that `Shutdown()` is not called from places that could be called from inside `Shutdown()`. Tree-SHA512: 362a507b1a6f97dc351f708224aedbfe4bee03c4398f394d78ee31c24d76a7012ffff0e6766866cd5fd9a8e0d8840f05a2741111fe583aa20d45f0af3df0dcfa
2020-06-08util: Don't reference errno when pthread fails.MIZUTA Takeshi
Pthread library does not set errno. Pthread library's errno is returned by return value. Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
2020-06-08Merge #19188: test: Avoid overwriting the NodeContext member of the testing ↵fanquake
setup [-Wshadow-field] fac6b9b938230d24c13fcb6e9be28515d674c6c8 test: Avoid overwriting the NodeContext member of the testing setup (MarcoFalke) fa16e7816b886b82d36457b0d8edc773cba76421 build: Add -Wshadow-field (MarcoFalke) Pull request description: Adding this warning will eliminate unexpected test failures and hard to review code. Moreover, there shouldn't be a use case in Bitcoin Core that relies on fields to be shadowed. ACKs for top commit: fanquake: ACK fac6b9b938230d24c13fcb6e9be28515d674c6c8 - Warnings compiling fa16e7816b886b82d36457b0d8edc773cba76421 are below. No warnings with fac6b9b938230d24c13fcb6e9be28515d674c6c8. The `-Wshadow-field` diagnostic has been available in Clang since 5.0.0. It's not available for GCC. practicalswift: ACK fac6b9b938230d24c13fcb6e9be28515d674c6c8 -- patch looks correct hebasto: ACK fac6b9b938230d24c13fcb6e9be28515d674c6c8, tested on Linux Mint 19.3 (x86_64): Tree-SHA512: 824874ca10877efea7463cf934a2953147f3f99c486f04679426c14ff968975d8652cbba5729bfb7cb2c86c637ade5d1e5b873d611c06bad013a7cad8427e2bf
2020-06-07log: remove deprecated `db` log categoryJon Atack
2020-06-07refactor: Fix formatting of timedata.cppHennadii Stepanov
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-06refactor: Replace RecursiveMutex with Mutex in netbase.cppHennadii Stepanov
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-06test: Avoid overwriting the NodeContext member of the testing setupMarcoFalke
2020-06-06refactor: Replace RecursiveMutex with Mutex in timedata.cppHennadii Stepanov
2020-06-05refactor: Replace RecursiveMutex with Mutex in Shutdown()Hennadii Stepanov
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
generated by cgit v1.2.3 (git 2.26.2) at 2024-08-19 12:23:09 +0000