aboutsummaryrefslogtreecommitdiff
path: root/src
AgeCommit message (Collapse)Author
2020-05-29Merge #18452: qt: Fix shutdown when waitfor* cmds are called from RPC consoleJonas Schnelli
da73f1513a637a9f347b64de66564d6cdb2541f8 qt: Fix shutdown when waitfor* cmds are called from RPC console (Hennadii Stepanov) Pull request description: On master (7eed413e72a236b6f1475a198f7063fd24929e23), if the GUI has been started with`-server=1`, `bitcoin-qt` hangs on shutdown during calling any of the `waitfor*` commands in the GUI RPC console. This PR suggests minimal changes to fix this bug. Fix #17495 ACKs for top commit: jonasschnelli: utACK da73f1513a637a9f347b64de66564d6cdb2541f8 Tree-SHA512: 469f5332945a5f2c57d19336cda5df79b123ccc494aea6d58a85eb1293be52708b2b9c5bb6bc2c402a90b7b4e9e8d7ab8fe84cf201cf7ce612c9290c57e43681
2020-05-29Merge #14988: wallet: Fix for confirmed column in csv export for payment to ↵Jonas Schnelli
self transactions 9760293ce632e09f0175368ebf0c8502ac9b10d4 wallet: Fix for exported confirmation field in payment to self transactions (Ben Carman) Pull request description: Closes #3455 ACKs for top commit: jonasschnelli: Tested ACK 9760293ce632e09f0175368ebf0c8502ac9b10d4 Tree-SHA512: 8207768771ad787f716b966c4aa7aeef2da8a602e32e3510e41c7b49ec5ec679a3835d248be5016d4b37764f9914846f7c41c11cf48cddb617cb7ef831318fd7
2020-05-29Merge #16939: p2p: Delay querying DNS seedsfanquake
96954d17948662672cababc940e453dff08e8cbb DNS seeds: don't query DNS while network is inactive (Anthony Towns) fa5894f7f581718ea28bb34b52fcd3b33ff3e644 DNS seeds: wait for 5m instead of 11s if 1000+ peers are known (Anthony Towns) Pull request description: Changes the logic for querying DNS seeds: after this PR, if there's less than 1000 entries in addrman, it will still usually query DNS seeds after 11s (unless the first few peers tried mostly succeed), but if there's more than 1000 entries it won't try DNS seeds until 5 minutes have passed without getting multiple outbound peers. (If there's 0 entries in addrman, it will still immediately query the DNS seeds). Additionally, delays querying DNS seeds while the p2p network is not active. Fixes #15434 ACKs for top commit: fanquake: ACK 96954d17948662672cababc940e453dff08e8cbb - Ran some tests of different scenarios. More documentation is being added in #19084. ariard: Tested ACK 96954d1, on Debian 9.1. Both MANY_PEERS/FEW_PEERS cases work. Sjors: tACK 96954d1 (rebased on master) on macOS 10.15.4. It found it useful to run with `-debug=addrman` and change `DNSSEEDS_DELAY_MANY_PEERS` to something lower to test the behaviour, as well as renaming `peers.dat` to test the peer threshold. naumenkogs: utACK 96954d17948662672cababc940e453dff08e8cbb Tree-SHA512: 73693db3da73bf8e76c3df9e9c82f0a7fb08049187356eac2575c4ffa455f76548dd1c86a11fc6beea8a3baf0ba020e047bebe927883c731383ec72442356005
2020-05-29Merge #18424: qt: Use parent-child relation to manage lifetime of ↵Jonas Schnelli
OptionsModel object 8e08d005989c6b5f7f05e0a1e0ba84f544a76d01 qt: Use parent-child relation to manage lifetime of OptionsModel object (Hennadii Stepanov) Pull request description: Both `BitcoinApplication` and `OptionsModel` classes are derived from the `QObject` class, therefore a parent-child relation could be established to manage the lifetime of an `OptionsModel` object: https://github.com/bitcoin/bitcoin/blob/5236b2e267a58870239673c7ec85e5df0cb8fc8e/src/qt/optionsmodel.cpp#L29-L30 This PR does not change behavior. ACKs for top commit: jonasschnelli: utACK 8e08d005989c6b5f7f05e0a1e0ba84f544a76d01 promag: ACK 8e08d005989c6b5f7f05e0a1e0ba84f544a76d01. Tree-SHA512: 0223dddf5ba28b0bfaefeda1b03b4ff95bf7e7d0c1e7b32368171e561813e22129f2a664f09279fa3b4fa63259b7680d55aa3fe66db9c7ae0039b7f529777ec3
2020-05-29Merge #18165: Consolidate service flag bit-to-name conversion to a shared ↵Jonas Schnelli
serviceFlagToStr function c31bc5bcfddf440e9a1713f7ba2ca2bf9cfa8e2e Consolidate service flag bit-to-name conversion to a shared serviceFlagToStr function (Luke Dashjr) cea91a1e40e12029140ebfba969ce3ef2965029c Bugfix: GUI: Use unsigned long long type to avoid implicit conversion of MSB check (Luke Dashjr) Pull request description: Side effect: this results in the RPC showing unknown service bits as "UNKNOWN[n]" like the GUI. Note that there is no common mask-to-`vector<string>` function because both GUI and RPC would need to iterate through it to convert to their desired target formats. ACKs for top commit: jonasschnelli: utACK ~~cea91a1e40e12029140ebfba969ce3ef2965029c~~ c31bc5bcfddf440e9a1713f7ba2ca2bf9cfa8e2e Tree-SHA512: 32c7ba8ac7ef2d4087f4f317447ae93a328ec9fb9ad81301df2fbaeeb21a3db7a503187a369552b05a9414251b7cf8e15bcde74c1ea2ef36591ea7ffb6721f60
2020-05-29Merge #17993: gui: Balance/TxStatus polling update based on last block hash.Jonas Schnelli
a06e845e826acaeb0db7cf02b2519c177e94dee5 BlockTip struct created and connected to notifyHeaderTip and notifyBlockTip signals. (furszy) 2f867203b0c7a4438ce484be4cfa2b29dbf1abf0 Added best block hash to the NotifyHeaderTip and NotifyBlockTip signals. (furszy) Pull request description: Rationale: The height based polling in the GUI is an issue on chain reorgs. Any new tip signal with the same height as the one that it's cached in the model was not triggering the GUI update (interpreting it as the same same block when it could receive a different one). Ending up with bad information presented in the GUI. This PR essentially changes the last cached height to be a last cached block hash. --- Old historical information of this PR. As the tip height is cached and updated via signaling in clientModel, there is no need to continue locking `cs_main` on every balance poll (`m_node.getNumBlocks()` method call). Extra topic: Would suggest to change the `cachedNumBlocks` field inside `walletModel` to a more understandable name, maybe `nLastBalanceUpdateHeight`. And finally, this will have the equal height reorg issue mentioned [here](https://github.com/bitcoin/bitcoin/pull/17905#issuecomment-577324304), whatever is presented to fix it, this should use the same flow too. **[Edit - 24/01/2020]** Have added #[17905](https://github.com/bitcoin/bitcoin/pull/17905#issuecomment-577324304) comment fix here too. ACKs for top commit: jonasschnelli: utACK a06e845e826acaeb0db7cf02b2519c177e94dee5 - it would be great to have QT unit tests (in this case for a reorg) that either automatically inspect the window content based on accessibility and tests for expected values or at least allow for quick manual re-testing (screenshots, automatically create UI situations). hebasto: re-ACK a06e845e826acaeb0db7cf02b2519c177e94dee5, suggested style changes implemented since the [previous](https://github.com/bitcoin/bitcoin/pull/17993#pullrequestreview-417249705) review. ryanofsky: Code review ACK a06e845e826acaeb0db7cf02b2519c177e94dee5. A lot of changes since the last review: rebase after sync_state introduction #18152 and tryGetBalances revert #18587, reverting getLastBlockTime change, fixing spacing and initializations and renaming some variables Tree-SHA512: 835e587a8296df9899cccd7b3e598a5970942b640e432e6a32de0b4eaea5b40f9271258f089ec033595311707b74a0f7187ecf8ed397c713e1153e2714072975
2020-05-29Merge #17968: qt: Ensure that ModalOverlay is resized properlyJonas Schnelli
4fc1df41d570ab631a8b47e4427a0b84305e37d1 qt: Track QEvent::Resize during animation (Hennadii Stepanov) Pull request description: In certain circumstances the `ModalOverlay` widget is not sized properly: - #17269 - #17967 - https://github.com/bitcoin/bitcoin/pull/17968#pullrequestreview-350753107 On master (f018d0c9cd7f408dac016b6bfc873670de713d27) this bug looks like this: ![DeepinScreenshot_bitcoin-qt_20200120193402](https://user-images.githubusercontent.com/32963518/72748165-298b2a80-3bbf-11ea-810d-2966f08e496a.png) With this PR the wallet frame looks ok: ![DeepinScreenshot_bitcoin-qt_20200120195241](https://user-images.githubusercontent.com/32963518/72748388-c64dc800-3bbf-11ea-8875-1ba1899b3513.png) Fix #17269 Fix #17967 ACKs for top commit: promag: Code review ACK 4fc1df41d570ab631a8b47e4427a0b84305e37d1. jonasschnelli: utACK 4fc1df41d570ab631a8b47e4427a0b84305e37d1 Tree-SHA512: b5d303fbc139c9383cd22edecba05e51b0d6115631aeb7d4474e973e7250a84019c11c0e41b5200e4d9ab10e17908774b45234317535857dc5314c3c28614ad4
2020-05-29Merge #17956: gui: Disable unavailable context menu items in transactions tabJonas Schnelli
2b18fd2242a589988fbb68205dae4afa0b8b3d34 Disable unavailable context menu items in transactions tab (Kristaps Kaupe) Pull request description: Fixes #9192. ACKs for top commit: jonatack: Re-ACK 2b18fd2242a5899 jonasschnelli: codereview utACK 2b18fd2242a589988fbb68205dae4afa0b8b3d34 Tree-SHA512: 4ea19c7b5976f1f0b1baecccb3077cf82f078af7257f92162686bcce2188efe49511a5f557853bc5fe0b10616708957d61c006944babbe60b8105e78751e865f
2020-05-29Merge #17918: qt: Hide non PKHash-Addresses in signing address bookJonas Schnelli
c4ea501e96363e937200bc97b8e2d78162bdb699 qt: Hide non PKHash-Addresses in signing address book (Emil Engler) Pull request description: [Video Demo](https://www.youtube.com/watch?v=T-Rp2pFRmzY) This PR hides all non PKHash addresses in the signing GUI in the Address Book when it is opened through the signing dialog, as non PKHash addresses are useless there. ACKs for top commit: jonasschnelli: Code Review ACK c4ea501e96363e937200bc97b8e2d78162bdb699 Tree-SHA512: e321d45e15534b2d68da5a1297b1c7551cdd784f03203f54c9385c2ce0bb2b7316c09f9e8c3eb41bfa1e7207ecc94c8ed08f012e2d6c117b803996ade26feb2f
2020-05-29Merge #17908: qt: Remove QFont warnings with QT_QPA_PLATFORM=minimalJonas Schnelli
1122817c194ed49abf896e68604e725c3b5c8569 qt: Remove QFont warnings with QPA=minimal (Hennadii Stepanov) Pull request description: This PR removes massive warnings like: ``` QWARN : ... QFont::setPointSizeF: Point size <= 0 (...), must be greater than 0 ``` from `test_bitcoin-qt` output. On master (e258ce792a4849927a6db51786732d71cbbb65fc): ``` $ ./src/qt/test/test_bitcoin-qt | grep QFont | wc -l ~BitcoinApplication : Stopping thread ~BitcoinApplication : Stopped thread 57 ``` With this PR: ``` $ ./src/qt/test/test_bitcoin-qt | grep QFont | wc -l ~BitcoinApplication : Stopping thread ~BitcoinApplication : Stopped thread 0 ``` ACKs for top commit: promag: Code review ACK 1122817c194ed49abf896e68604e725c3b5c8569. jonasschnelli: utACK 1122817c194ed49abf896e68604e725c3b5c8569 Tree-SHA512: 32fa72a5d3db1d4c73a2a324aa9cad807ee46f23fc5319f7d71202987dc73ea7c90082904489b323a432e1afaebd9976b7dd0374236a16153162aa75fe368723
2020-05-29Merge #17597: qt: Fix height of QR-less ReceiveRequestDialogJonas Schnelli
73529f0859c060087dd41e9e176fd4198d0f385f qt: Rename slot to updateDisplayUnit() (Hennadii Stepanov) 68288ef0c15522799d4bf6239d2ae1e8086b5abf qt: Overhaul ReceiveRequestDialog (Hennadii Stepanov) Pull request description: If master (89a1f7a250ef70ff2d65701564f1e24bb9280d90) is compiled without QR support, the "Request payment to..." dialog looks ugly: ![Screenshot from 2019-11-25 19-58-59](https://user-images.githubusercontent.com/32963518/69566647-3d9c1c80-0fc0-11ea-8ff6-183cea9372c5.png) With this PR: ![Screenshot from 2019-12-26 00-42-46](https://user-images.githubusercontent.com/32963518/71451226-221c6100-277a-11ea-94ae-c19a5c6256f7.png) ![Screenshot from 2019-12-26 00-44-34](https://user-images.githubusercontent.com/32963518/71451228-29436f00-277a-11ea-8ac5-1bafe6d73b5c.png) ![Screenshot from 2019-12-26 00-48-51](https://user-images.githubusercontent.com/32963518/71451230-2e082300-277a-11ea-8c4c-726ca7b776e7.png) Other minor changes: - "URI" abbreviation is not translated now - wallet name is shown if available ACKs for top commit: jonasschnelli: Tested ACK 73529f0859c060087dd41e9e176fd4198d0f385f Tree-SHA512: 45f9a41d3c72978d78eb2e8ca98e274b8be5abf5352fd3e1f4f49514ce744994545fb3012e80600ded936ae41c6be4d4825a0c5f7bcb3db671d69e0299f0f65b
2020-05-29Merge #16432: qt: Add privacy to the Overview pageJonas Schnelli
8d75115844baefe5cad4d82ec8dce001dd16eb9c qt: Add privacy feature to Overview page (Hennadii Stepanov) 73d8ef72742ab9193e9e95158b26176bfaab3f66 qt: Add BitcoinUnits::formatWithPrivacy() function (Hennadii Stepanov) Pull request description: This PR allows to hide/reveal values on the Overviewpage by checking/unchecking Menu->Settings-> Mask Values Closes #16407 Privacy mode is OFF (the default behavior): ![Screenshot from 2020-01-02 15-08-28](https://user-images.githubusercontent.com/32963518/71669074-28ab6980-2d74-11ea-8e54-4973aa307192.png) Privacy mode is ON: ![Screenshot from 2020-01-02 15-10-23 cropped](https://user-images.githubusercontent.com/32963518/71669082-2d701d80-2d74-11ea-9df5-d4acc4982dbe.png) ACKs for top commit: jonatack: Tested ACK 8d75115 jonasschnelli: Tested ACK 8d75115844baefe5cad4d82ec8dce001dd16eb9c Tree-SHA512: 42f396d5bf0d343b306fb7e925f86f66b3fc3a257af370a812f4be181b5269298f9b23bd8a3ce25ab61de92908c4018d8c2dc8591d11bc58d79c4eb7206fc6ec
2020-05-28Add missing QPainterPath includeAndrew Chow
This is needed to compile with Qt 5.15.
2020-05-28Merge #18700: Fix locking on WSL using flock instead of fcntlWladimir J. van der Laan
e8fa0a3d2025509fcddc59fc618e91371542cf87 Fix WSL file locking by using flock instead of fcntl (Samuel Dobson) Pull request description: Fixes #18622 A bug in WSL means that fcntl does not exclusively lock files, allowing multiple instances of bitcoin to use the same datadir. If we instead use flock, it works correctly. Passes Travis, but testing on some OS variety would be sensible. From what I can tell, flock and fcntl don't work with each other on linux, so it would still be possible to run a node with this code change and a node before it with the same datadir (this isn't true for Mac/FreeBSD). flock also doesn't support NFS on MacOS and linux<2.6.12 while fcntl did. See here for example: https://gavv.github.io/articles/file-locks/ If changing to flock for all systems is inadvisable, it would also be possible to just detect WSL and use flock when on that platform to avoid the bug. ACKs for top commit: laanwj: Code review ACK e8fa0a3d2025509fcddc59fc618e91371542cf87 Tree-SHA512: ca1009e171970101f1dc2332c5e998717aee00eebc80bb586b826927a74bd0d4c94712e46d1396821bc30533d76deac391b6e1c406c406865661f57fa062c702
2020-05-28Get rid of -Wthread-safety-precise warningsHennadii Stepanov
2020-05-28Use template function instead of void* parameterHennadii Stepanov
This change gets rid of -Wthread-safety-attributes warning spam.
2020-05-28refactor: Rename LockGuard to StdLockGuard for consistency with StdMutexHennadii Stepanov
2020-05-28Add thread safety annotated wrapper for std::mutexHennadii Stepanov
Co-authored-by: Anthony Towns <aj@erisian.com.au>
2020-05-27Merge #16127: More thread safety annotation coverageMarcoFalke
5478d6c099e76fe070703cc5383cba7b91468b0f logging: thread safety annotations (Anthony Towns) e685ca19928eec4e687c66f5edfcfff085a42c27 util/system.cpp: add thread safety annotations for dir_locks (Anthony Towns) a7887899480db72328784009181d93904e6d479d test/checkqueue_tests: thread safety annotations (Anthony Towns) 479c5846f7477625ec275fbb8a076c7ef157172b rpc/blockchain.cpp: thread safety annotations for latestblock (Anthony Towns) 8b5af3d4c1270267ad85e78f661bf8fab06f3aad net: fMsgProcWake use LOCK instead of lock_guard (Anthony Towns) de7c5f41aba860751ef7824245e6d9d5088a1200 wallet/wallet.h: Remove mutexScanning which was only protecting a single atomic bool (Anthony Towns) c3cf2f55013c4ea1c1ef4a878fc7ff8e92f2c42d rpc/blockchain.cpp: Remove g_utxosetscan mutex that is only protecting a single atomic variable (Anthony Towns) Pull request description: In a few cases we need to use `std::mutex` rather than the sync.h primitives. But `std::lock_guard<std::mutex>` doesn't include the clang thread safety annotations unless you also use clang's C library, which means you can't indicate when variables should be guarded by `std::mutex` mutexes. This adds an annotated version of `std::lock_guard<std::mutex>` to threadsafety.h to fix that, and modifies places where `std::mutex` is used to take advantage of the annotations. It's based on top of #16112, and turns the thread safety comments included there into annotations. It also changes the RAII classes in wallet/wallet.h and rpc/blockchain.cpp to just use the atomic<bool> flag for synchronisation rather than having a mutex that doesn't actually guard anything as well. ACKs for top commit: MarcoFalke: ACK 5478d6c099e76fe070703cc5383cba7b91468b0f 🗾 hebasto: re-ACK 5478d6c099e76fe070703cc5383cba7b91468b0f, only renamed s/`MutexGuard`/`LockGuard`/, and dropped the commit "test/util_threadnames_tests: add thread safety annotations" since the [previous](https://github.com/bitcoin/bitcoin/pull/16127#pullrequestreview-414184113) review. ryanofsky: Code review ACK 5478d6c099e76fe070703cc5383cba7b91468b0f. Thanks for taking suggestions! Only changes since last review are dropping thread rename test commit d53072ec730d8eec5a5b72f7e65a54b141e62b19 and renaming mutex guard to lock guard Tree-SHA512: 7b00d31f6f2b5a222ec69431eb810a74abf0542db3a65d1bbad54e354c40df2857ec89c00b4a5e466c81ba223267ca95f3f98d5fbc1a1d052a2c3a7d2209790a
2020-05-27Merge #19004: refactor: Replace const char* to std::stringMarcoFalke
c57f03ce1741b38af448bec7b22ab9f8ac21f067 refactor: Replace const char* to std::string (Calvin Kim) Pull request description: Rationale: Addresses #19000 Some functions should be returning std::string instead of const char*. This commit changes that. Main benefits/reasoning: 1. The functions never return nullptr, so returning a string makes code at call sites easier to review (reviewers don't have to read the source code to verify that a nullptr is never returned) 2. All call sites convert to string anyway ACKs for top commit: MarcoFalke: re-ACK c57f03ce17 (no changes since previous review) 🚃 Empact: Fair enough, Code Review ACK https://github.com/bitcoin/bitcoin/pull/19004/commits/c57f03ce1741b38af448bec7b22ab9f8ac21f067 practicalswift: ACK c57f03ce1741b38af448bec7b22ab9f8ac21f067 -- patch looks correct hebasto: re-ACK c57f03ce1741b38af448bec7b22ab9f8ac21f067 Tree-SHA512: 9ce99bb38fe399b54844315048204cafce0f27fd8f24cae357fa7ac6f5d8094d57bbf5f5c1f5878a65f2d35e4a3f95d527eb17f49250b690c591c0df86ca84fd
2020-05-27Merge #19073: Remove outdated comment about DER encodingfanquake
4c825792dd9f4eaf4936c3e376ac7a5c177528e2 Remove outdated comment about DER encoding (Elichai Turkel) Pull request description: This comment got me confused about the status of BIP66 (Thanks jnewbery for explaining) The comment was added in: https://github.com/bitcoin/bitcoin/pull/3843 But in https://github.com/bitcoin/bitcoin/pull/5713 strict DER encoding was enforced in consensus, and is now it's buried and enforced by the height of the block here: https://github.com/bitcoin/bitcoin/blob/4af01b37d40246cd1fdb54719855927e36a36b46/src/validation.cpp#L1889 P.S. This is also quite confusing: https://github.com/bitcoin/bitcoin/blob/4af01b37d40246cd1fdb54719855927e36a36b46/src/validation.cpp#L1560-L1563 But seems to be intentional: https://github.com/bitcoin/bitcoin/blob/4af01b37d40246cd1fdb54719855927e36a36b46/src/validation.cpp#L1510-L1511 ACKs for top commit: laanwj: ACK 4c825792dd9f4eaf4936c3e376ac7a5c177528e2 Tree-SHA512: 7afbbae84ed4ecfaa0a273ae024b14f2b7ffe65307f078086fe0b5b645c57722bc2952fb15d167d9e4fa5b052d1d0ac6e5e33f57e8fc881c0ea611d352bccc1e
2020-05-27qt: Add privacy feature to Overview pageHennadii Stepanov
2020-05-27Merge #18918: wallet: Move salvagewallet into wallettoolSamuel Dobson
84ae0578b6c68dda145ca65fef510ce0fdac0d7b Add release notes about salvage changes (Andrew Chow) ea337f2d0318a860f695698cfb3aa91c03ded858 Move RecoverKeysOnlyFilter into RecoverDataBaseFile (Andrew Chow) 9ea2d258b46e8a9776100633585ed0feede5c2a4 Move RecoverDatabaseFile and RecoverKeysOnlyFilter into salvage.{cpp/h} (Andrew Chow) b426c7764d26e280e1f814cf36e050743c45cd12 Make BerkeleyBatch::Recover and WalletBatch::RecoverKeysOnlyFilter standalone (Andrew Chow) 2741774214168eb287c7066d6823afe5e570381d Expose a version of ReadKeyValue and use it in RecoverKeysOnlyFilter (Andrew Chow) ced95d0e43389fe62b5d30fcc7c42dbca0e88242 Move BerkeleyEnvironment::Salvage into BerkeleyBatch::Recover (Andrew Chow) 07250b8dcebe2b97ed0fd900ad35cba4091b8ecf walletdb: remove fAggressive from Salvage (Andrew Chow) 8ebcbc85c652665b78dcfd2ad55fa67cafd42c73 walletdb: don't automatically salvage when corruption is detected (Andrew Chow) d321046f4bb4887742699c586755a21f3a2edbe1 wallet: remove -salvagewallet (Andrew Chow) cdd955e580dff99f3fa440494ed2b348f7f094af Add basic test for bitcoin-wallet salvage (Andrew Chow) c87770915b88d195d264b58111c64142b1965cfa wallettool: Add a salvage command (Andrew Chow) Pull request description: Removes the `-salvagewallet` startup option and adds a `salvage` command to the `bitcoin-wallet` tool. As such, `-salvagewallet` is removed. Additionally, the automatic salvage that is done if the wallet file fails to load is removed. Lastly the salvage code entirely is moved out entirely into `bitcoin-wallet` from `walletdb.{cpp/h}` and `db.{cpp/h}`. ACKs for top commit: jonatack: ACK 84ae0578b6c68dda145ca65fef510ce0fdac0d7b feedback taken, and compared to my previous review, the bitcoin-wallet salvage command now seems to run and it exits without raising. The new test passes at both 9454105 and 84ae057 so as a sanity check I'd agree there is room for improvement, if possible. MarcoFalke: re-ACK 84ae0578b6 🏉 Empact: Code Review ACK https://github.com/bitcoin/bitcoin/pull/18918/commits/84ae0578b6c68dda145ca65fef510ce0fdac0d7b ryanofsky: Code review ACK 84ae0578b6c68dda145ca65fef510ce0fdac0d7b. Lot of small changes since previous review: added verify step before salvage, added basic test in new commit, removed unused scanstate variable and warnings parameter, tweaked various comments and strings, moved fsuccess variable declaration meshcollider: Concept / light code review ACK 84ae0578b6c68dda145ca65fef510ce0fdac0d7b Tree-SHA512: 05be116b56ecade1c58faca1728c8fe4b78f0a082dbc2544a3f7507dd155f1f4f39070bd1fe90053444384337bc48b97149df5c1010230d78f8ecc08e69d93af
2020-05-26Remove outdated comment about DER encodingElichai Turkel
2020-05-27logging: thread safety annotationsAnthony Towns
Adds LockGuard helper in threadsafety.h to replace lock_guard<mutex> when LOCK(Mutex) isn't available for use.
2020-05-26Merge #19032: Serialization improvements: final stepWladimir J. van der Laan
71f016c6eb42e1ac2c905e04ba4d20c2009e533f Remove old serialization primitives (Pieter Wuille) 92beff15d3ae2646c00bd78146d7592a7097ce9c Convert LimitedString to formatter (Pieter Wuille) ef17c03e074b6c3f185afa4eff572ba687c2a171 Convert wallet to new serialization (Pieter Wuille) 65c589e45e8b8914698a0fd25cd5aafdda30869c Convert Qt to new serialization (Pieter Wuille) Pull request description: This is the final step 🥳 of the serialization improvements extracted from #10785. It converts the LimitedString wrapper to a new-style formatter, and updates the wallet and Qt code to use the new serialization framework. Finally all remaining old primitives are removed. ACKs for top commit: jonatack: ACK 71f016c6eb42e1ac2 reviewed diff, builds/tests/re-fuzzed. laanwj: Code review ACK 71f016c6eb42e1ac2c905e04ba4d20c2009e533f Tree-SHA512: d952194bc73259f6510bd4ab1348a1febbbf9862af30f905991812fb0e1f23f15948cdb3fc662be54d648e8f6d95b11060055d2e7a8c2cb5bf008224870b1ea1
2020-05-26util/system.cpp: add thread safety annotations for dir_locksAnthony Towns
2020-05-26Merge #18881: Prevent UB in DeleteLock() functionMarcoFalke
90eb027204f5a9d7c00fa97d4112243bd37a9012 doc: Add and fix comments about never destroyed objects (Hennadii Stepanov) 26c093a9957756f3743c2347fe0abd90f81159c4 Replace thread_local g_lockstack with a mutex-protected map (Hennadii Stepanov) 58e6881bc5be002e8ddbc9b75422c0deae66a2df refactor: Refactor duplicated code into LockHeld() (Hennadii Stepanov) f511f61dda4e860079153d5e51d64658cc265283 refactor: Add LockPair type alias (Hennadii Stepanov) 8d8921abd35c3ac1b8ebacb11de8e1bbc7b28d66 refactor: Add LockStackItem type alias (Hennadii Stepanov) 458992b06d80eb568141f60a33d38e12e894e27a Prevent UB in DeleteLock() function (Hennadii Stepanov) Pull request description: Tracking our instrumented mutexes (`Mutex` and `RecursiveMutex` types) requires that all involved objects should not be destroyed until after their last use. On master (ec79b5f86b22ad8f77c736f9bb76c2e4d7faeaa4) we have two problems related to the object destroying order: - the function-local `static` `lockdata` object that is destroyed at [program exit](https://en.cppreference.com/w/cpp/utility/program/exit) - the `thread_local` `g_lockstack` that is destroyed at [thread exit](https://en.cppreference.com/w/cpp/language/destructor) Both cases could cause UB at program exit in so far as mutexes are used in other static object destructors. Fix #18824 ACKs for top commit: MarcoFalke: re-ACK 90eb027204, only change is new doc commit 👠 ryanofsky: Code review ACK 90eb027204f5a9d7c00fa97d4112243bd37a9012 because all the changes look correct and safe. But I don't know the purpose of commit 26c093a9957756f3743c2347fe0abd90f81159c4 "Replace thread_local g_lockstack with a mutex-protected map (5/6)." It seems like it could have a bad impact on debug performance, and the commit message and PR description don't give a reason for the change. Tree-SHA512: 99f29157fd1278994e3f6eebccedfd9dae540450f5f8b980518345a89d56b635f943a85b20864cef087027fd0fcdb4880b659ef59bfe5626d110452ae22031c6
2020-05-26Merge #19056: rpc: Make gettxoutsetinfo/GetUTXOStats interruptibleMarcoFalke
fa756928c3f455943086051c5fe1d5bb09962248 rpc: Make gettxoutsetinfo/GetUTXOStats interruptible (MarcoFalke) fa7fc5a8e0fcf9ca81e84b3631f18ae40502be60 rpc: factor out RpcInterruptionPoint from dumptxoutset (MarcoFalke) Pull request description: Make it interruptible, so that shutdown doesn't block for up to one hour. Fixes (partially) #13217 ACKs for top commit: Empact: Code Review ACK https://github.com/bitcoin/bitcoin/pull/19056/commits/fa756928c3f455943086051c5fe1d5bb09962248 laanwj: Code review ACK fa756928c3f455943086051c5fe1d5bb09962248 Tree-SHA512: 298261e0ff7d79fab542b8f6828cc0ac451cbafe396d5f0816c9d36437faba1330f5c4cb2a25c5540e202bfb9783da6ec858bd453056ce488d21e36335d3d42c
2020-05-26Merge #18867: tests: Add fuzzing harness for CCoinsViewCacheMarcoFalke
f9b22e3bdb54acb2f830b3ebbad47ff17dfb5781 tests: Add fuzzing harness for CCoinsViewCache (practicalswift) Pull request description: Add fuzzing harness for `CCoinsViewCache`. 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 f9b22e3bdb 📫 Tree-SHA512: 4fa79aab683875eef128b672cf199909c86e4d2ed7c406f006fa27a546dafc9cb0061c4de5e660e622458072f1dab69dbf6b6b03d5b863f81c5710bf4cee6c0c
2020-05-26Merge #19010: net processing: Add support for getcfheadersMarcoFalke
5308c97ccaf0955e5840956bc1636108a43e6f46 [test] Add test for cfheaders (Jim Posen) f6b58c150686e90bc4952976e488b1605f3ae02a [net processing] Message handling for getcfheaders. (Jim Posen) 3bdc7c2d3977a7864aacea80bffc4df7f37cac51 [doc] Add comment for m_headers_cache (John Newbery) Pull request description: Support `getcfheaders` requests when `-peerblockfilters` is set. Does not advertise compact filter support in version messages. ACKs for top commit: jkczyz: ACK 5308c97ccaf0955e5840956bc1636108a43e6f46 MarcoFalke: re-ACK 5308c97cca , only change is doc related 🗂 theStack: ACK 5308c97ccaf0955e5840956bc1636108a43e6f46 :rocket: Tree-SHA512: 240fc654f6f634c191d9f7628b6c4801f87ed514a1dd55c7de5d454d4012d1c09509a2d5a246bc7da445cd920252b4cd56a493c060cdb207b04af4ffe53b95f7
2020-05-25Move RecoverKeysOnlyFilter into RecoverDataBaseFileAndrew Chow
2020-05-25Move RecoverDatabaseFile and RecoverKeysOnlyFilter into salvage.{cpp/h}Andrew Chow
2020-05-25Make BerkeleyBatch::Recover and WalletBatch::RecoverKeysOnlyFilter standaloneAndrew Chow
Instead of having these be class static functions, just make them be standalone. Also removes WalletBatch::Recover which just passed through to BerkeleyBatch::Recover.
2020-05-25Expose a version of ReadKeyValue and use it in RecoverKeysOnlyFilterAndrew Chow
We need this exposed for BerkeleyBatch::Recover to be moved out.
2020-05-25Move BerkeleyEnvironment::Salvage into BerkeleyBatch::RecoverAndrew Chow
2020-05-25walletdb: remove fAggressive from SalvageAndrew Chow
The only call to Salvage set fAggressive = true so remove that parameter and always use DB_AGGRESSIVE
2020-05-25walletdb: don't automatically salvage when corruption is detectedAndrew Chow
2020-05-25wallet: remove -salvagewalletAndrew Chow
2020-05-25wallettool: Add a salvage commandAndrew Chow
2020-05-25tests: Add fuzzing harness for CCoinsViewCachepracticalswift
2020-05-24Remove old serialization primitivesPieter Wuille
2020-05-24Convert LimitedString to formatterPieter Wuille
2020-05-24Convert wallet to new serializationPieter Wuille
2020-05-23BlockTip struct created and connected to notifyHeaderTip and notifyBlockTip ↵furszy
signals.
2020-05-23Added best block hash to the NotifyHeaderTip and NotifyBlockTip signals.furszy
[ClientModel] best header/block hash cached.
2020-05-23qt: Add BitcoinUnits::formatWithPrivacy() functionHennadii Stepanov
2020-05-24Merge #18594: cli: display multiwallet balances in -getinfoSamuel Dobson
5edad5ce5d3f15b694bf3fad0300c6446674b554 test: add -getinfo multiwallet functional tests (Jon Atack) 903b6c117f541ea9258d3234ffcf59427344e668 rpc: drop unused JSONRPCProcessBatchReply size arg, refactor (Jon Atack) afce85eb994384246e455b766549c3206cb059e0 cli: use GetWalletBalances() functionality for -getinfo (Jon Atack) 9f01849a498a70616506bdcda8ce6897aa29e664 cli: create GetWalletBalances() to fetch multiwallet balances (Jon Atack) 743077544b5420246ef29e0b708c90e3a8dfeeb6 cli: lift -rpcwallet logic up to CommandLineRPC() (Jon Atack) 29f2cbdeb7afdde87d108adf80cffad17d112632 cli: extract connection exception handler, -rpcwait logic (Jon Atack) Pull request description: This PR is a client-side version of #18453, per review feedback there and [review club discussions](https://bitcoincore.reviews/18453#meeting-log). It updates `bitcoin-cli -getinfo` on the client side to display wallet name and balance for the loaded wallets when more than one is loaded (e.g. you are in "multiwallet mode") and `-rpcwallet=` is not passed; otherwise, behavior is unchanged. before ```json $ bitcoin-cli -getinfo -regtest { "version": 199900, "blocks": 15599, "headers": 15599, "verificationprogress": 1, "timeoffset": 0, "connections": 0, "proxy": "", "difficulty": 4.656542373906925e-10, "chain": "regtest", "balance": 0.00001000, "relayfee": 0.00001000 } ``` after ```json $ bitcoin-cli -getinfo -regtest { "version": 199900, "blocks": 15599, "headers": 15599, "verificationprogress": 1, "timeoffset": 0, "connections": 0, "proxy": "", "difficulty": 4.656542373906925e-10, "chain": "regtest", "balances": { "": 0.00001000, "Encrypted": 0.00003500, "day-to-day": 0.00000120, "side project": 0.00000094 } } ``` ----- `Review club` discussion about this PR is here: https://bitcoincore.reviews/18453 This PR can be manually tested by building, creating/loading/unloading several wallets with `bitcoin-cli createwallet/loadwallet/unloadwallet` and running `bitcoin-cli -getinfo` and `bitcoin-cli -rpcwallet=<wallet-name> -getinfo`. `wallet_multiwallet.py --usecli` provides regression test coverage on this change, along with `interface_bitcoin_cli.py` where this PR adds test coverage. Credit to Wladimir J. van der Laan for the idea in https://github.com/bitcoin/bitcoin/issues/17314 and https://github.com/bitcoin/bitcoin/pull/18453#issuecomment-605431806. ACKs for top commit: promag: Tested ACK 5edad5ce5d3f15b694bf3fad0300c6446674b554. jnewbery: utACK 5edad5ce5d3f15b694bf3fad0300c6446674b554 meshcollider: Code review ACK 5edad5ce5d3f15b694bf3fad0300c6446674b554 Tree-SHA512: 4ca36c5f6c49936b40afb605c44459c1d5b80b5bd84df634007ca276b3f6c102a0cb382f9d528370363ee32c94b0d7ffa15184578eaf8de74179e566c5c5cee5
2020-05-23Merge #18698: Make g_chainman internal to validationMarcoFalke
fab6b9d18fd48bbbd1939b1173723bc04c5824b5 validation: Mark g_chainman DEPRECATED (MarcoFalke) fa1d97b25686a5caca623599f6d608fd08616fe8 validation: Make ProcessNewBlock*() members of ChainstateManager (MarcoFalke) fa24d4909864096934577abc26cfa9be47f634ba validation: Make PruneOneBlockFile() a member of ChainstateManager (MarcoFalke) fa84b1cd846f6499b741710fd478ec9ad49b5120 validation: Make LoadBlockIndex() a member of ChainstateManager (MarcoFalke) fa05fdf0f19fa4b557cc5e9ba436e3215b83c4e6 net: Pass chainman into PeerLogicValidation (MarcoFalke) fa7b626d7a150e5cbd4d163d2dab6f8a55fc2cc4 node: Add chainman alias for g_chainman (MarcoFalke) Pull request description: The global `g_chainman` has recently been introduced in #17737. The chainstate manager is primarily needed for the assumeutxo feature, but it can also simplify testing in the future. The goal of this pull is to make the global chainstate manager internal to validation, so that all external code does not depend on globals and that unit or fuzz tests can pass in their (potentially mocked) chainstate manager. I suggest reviewing the pull request commit-by-commit. It should be relatively straightforward refactoring that does not change behavior at all. ACKs for top commit: ryanofsky: Code review ACK fab6b9d18fd48bbbd1939b1173723bc04c5824b5. Had to be rebased but still looks good Tree-SHA512: dcbf114aeef4f8320d466369769f22ce4dd8f46a846870354df176c3de9ff17c64630fbd777e7121d7470d7a8564ed8d37b77168746e8df7489c6877e55d7b4f
2020-05-22rpc: Make gettxoutsetinfo/GetUTXOStats interruptibleMarcoFalke
Also, add interruption points to scantxoutset