aboutsummaryrefslogtreecommitdiff
path: root/src
AgeCommit message (Collapse)Author
2019-11-14Merge #17449: fix uninitialized variable nMinerConfirmationWindowfanquake
edb6b768a4185a4aaa6281ee50a6538f7426cb1e fix uninitialized variable nMinerConfirmationWindow (NullFunctor) Pull request description: It is used for the computation of `BIP9WarningHeight`, and by that time it isn't initialized. ACKs for top commit: jnewbery: utACK edb6b768a promag: ACK edb6b768a4185a4aaa6281ee50a6538f7426cb1e, commit description could be cleaned up though. MarcoFalke: ACK edb6b768a4185a4aaa6281ee50a6538f7426cb1e, used python3 to do the addition locally šŸ“ practicalswift: ACK edb6b768a4185a4aaa6281ee50a6538f7426cb1e, used `clang++ -O2` on the previous version^W^W^W^W^W^W`bc` to verify the addition locally šŸ“ Sjors: Code review ACK edb6b76. Nit: commit description has duplicate text. Tree-SHA512: 6fa0be0ecfbfd5d537f2c5b4a9333c76530c1f3182f777330cc7939b0496e37b75d8f8810cdaf471a9bd3247b425f2e239578300dfa0d5a87cd14a6ccfafa619
2019-11-12fix uninitialized variable nMinerConfirmationWindowNullFunctor
fix uninitialized variable hard code the MinBIP9WarningHeight fix uninitialized var hard code the MinBIP9WarningHeight instead
2019-11-12Merge #17437: rpc: Expose block height of wallet transactionsMarcoFalke
a5e77959c8ff6a8bffa1621d7ea29ee8603c5a14 rpc: Expose block height of wallet transactions (JoĆ£o Barbosa) Pull request description: Closes #17296. ACKs for top commit: practicalswift: ACK a5e77959c8ff6a8bffa1621d7ea29ee8603c5a14 -- diff looks correct now (good catch @theStack!) theStack: ACK a5e77959c8ff6a8bffa1621d7ea29ee8603c5a14 ryanofsky: Code review ACK a5e77959c8ff6a8bffa1621d7ea29ee8603c5a14. Changes since last review getblockhash python test fixes, and removing the last hardcoded height Tree-SHA512: 57dcd0e4e7083f34016bf9cf8ef578fbfde49e882b6cd8623dd1c64716e096e62f6177a4c2ed94f5de304e751fe23fb9d11cf107a86fbf0a3c5f539cd2844916
2019-11-12Merge #17450: util: Add missing headers to util/fees.cppMarcoFalke
b1315241375211563e9ec00391c4ac5310e1146f util: Add missing headers to util/fees.cpp (Hennadii Stepanov) Pull request description: ACKs for top commit: laanwj: code review ACK b1315241375211563e9ec00391c4ac5310e1146f MarcoFalke: ACK b131524 jnewbery: ACK b1315241375211563e9ec00391c4ac5310e1146f Tree-SHA512: a1ad36bff12219912c6aaacd7d9dcbeccf0fa3373280fa6e804d7a4d267b485433d6e1c01134cfa6732d2fb30ec1ab4629dff6e4bea2fe4c1976180064a3c6ca
2019-11-12util: Add missing headers to util/fees.cppHennadii Stepanov
2019-11-11rpc: Expose block height of wallet transactionsJoĆ£o Barbosa
2019-11-12fix Typo: "merkelRoot" -> "merkleRoot"ianliu
2019-11-10Merge #17427: qt: Fix missing qRegisterMetaType for size_tWladimir J. van der Laan
1828c6f05fcbed9ed432b042cc36eee0d80b113d refactor: Styling w/ clang-format, comment update (Hennadii Stepanov) 88a94f7bb8ba2b0257315d70717f9af928ca6561 qt: Fix missing qRegisterMetaType for size_t (Hennadii Stepanov) Pull request description: On master (a7aec7ad97949a82f870c033d8fd8b65d772eacb) this connection https://github.com/bitcoin/bitcoin/blob/a7aec7ad97949a82f870c033d8fd8b65d772eacb/src/qt/rpcconsole.cpp#L587 fails due to `ClientModel::mempoolSizeChanged()` signal has unregistered parameter type `size_t`: https://github.com/bitcoin/bitcoin/blob/a7aec7ad97949a82f870c033d8fd8b65d772eacb/src/qt/clientmodel.h#L102 More: ``` $ QT_FATAL_WARNINGS=1 lldb src/qt/bitcoin-qt -- -debug=qt ... (lldb) bt * thread #17, name = 'QThread', stop reason = signal SIGABRT * frame #0: 0x00007ffff35fce97 libc.so.6`__GI_raise(sig=2) at raise.c:51 frame #1: 0x00007ffff35fe801 libc.so.6`__GI_abort at abort.c:79 frame #2: 0x00007ffff5901352 libQt5Core.so.5`QMessageLogger::warning(char const*, ...) const + 354 frame #3: 0x00007ffff5b216fe libQt5Core.so.5`___lldb_unnamed_symbol2329$$libQt5Core.so.5 + 334 frame #4: 0x00007ffff5b2456d libQt5Core.so.5`QMetaObject::activate(QObject*, int, int, void**) + 1933 frame #5: 0x000055555566872e bitcoin-qt`ClientModel::mempoolSizeChanged(this=<unavailable>, _t1=<unavailable>, _t2=<unavailable>) at moc_clientmodel.cpp:260 ... ``` `debug.log`: ``` [] GUI: QObject::connect: Cannot queue arguments of type 'size_t' (Make sure 'size_t' is registered using qRegisterMetaType().) ``` This PR fixes it. Refs: - [Qt docs: qRegisterMetaType](https://doc.qt.io/qt-5/qmetatype.html#qRegisterMetaType) - #16348 --- Side NOTE: Also I believe this line https://github.com/bitcoin/bitcoin/blob/a7aec7ad97949a82f870c033d8fd8b65d772eacb/src/qt/bitcoin.cpp#L63 is redundant since long `CAmount` is a `typedef`. ACKs for top commit: laanwj: Tested ACK 1828c6f05fcbed9ed432b042cc36eee0d80b113d Tree-SHA512: 2c7f9fe6a5ae70f2e1dd86b07f95d4b00c85c5706a9d722f063f80beb71880d012ec46556963fb1544c2af53d006936c2f7612eae60d9193f67db62ba3d86129
2019-11-10Merge #17431: Remove unnecessary forward declarationWladimir J. van der Laan
3d133482b20c0acc03cdbe5f9104ae8dedde8e4d Remove unnecessary forward declaration (Mark Erhardt) Pull request description: This removes an unnecessary forward declaration. ACKs for top commit: jnewbery: Tested ACK 3d133482b20c0acc03cdbe5f9104ae8dedde8e4d laanwj: ACK 3d133482b20c0acc03cdbe5f9104ae8dedde8e4d Tree-SHA512: 9e5b14e861c2b9fa2d7707ed67c4667540e9812a762e00f5039691eeca82390eb7462d20ad781d4e8c9111517e989da7aef60b112ab33abb774e32d9845b5459
2019-11-10refactor: Styling w/ clang-format, comment updateHennadii Stepanov
2019-11-10qt: Fix missing qRegisterMetaType for size_tHennadii Stepanov
It is required in order to use size_t in QueuedConnections.
2019-11-10Merge #17410: Rename `db` log category to `walletdb` (like `coindb`)Wladimir J. van der Laan
e2c03c1156a1a8cb2c04c180f2ddbd3535126a46 doc: Add relase note for dbā†’walletdb rename (Wladimir J. van der Laan) 4c1d263d93988ceed53e8f6b5decaf034b68137e scripted-diff: Change `BCLog::DB` to `BCLog::WALLETDB` (Wladimir J. van der Laan) 6b42b3ba9087225fddb91dd764c42c28d0c42d0f Rename `db` log category to `walletdb` (like `coindb`) (Wladimir J. van der Laan) Pull request description: Rename the `db` log category to `walletdb` (in the style of, and to distinguish from `coindb`). Deprecate (but still accept) '-debug=db'. Second commit is a scripted commit that changes the enum item name. ACKs for top commit: hebasto: ACK e2c03c1156a1a8cb2c04c180f2ddbd3535126a46, tested on Linux Mint 19.2: Tree-SHA512: a044de6f9a70e735cbb1caa4ed6bf75bc2269b2d5bc3241a25b6a6d69c1fc1d83456e252b431388ae61f4821e4fc06ecc1b634816ceadbe9a3c0e494bee6c11e
2019-11-09Remove unnecessary forward declarationMark Erhardt
2019-11-08Merge #15934: Merge settings one place instead of five placesWladimir J. van der Laan
083c954b02a4e7d0708349eeaf3bac2b5947fb0e Add settings_tests (Russell Yanofsky) 7f40528cd50fc43ac0bd3e785de24d661adddb7a Deduplicate settings merge code (Russell Yanofsky) 9dcb952fe5f85529ab28e091af7534e72c21c90f Add util::Settings struct and helper functions. (Russell Yanofsky) e2e37cfe8af088bd8ea884be2f79f0f3cac555d5 Remove includeconf nested scope (Russell Yanofsky) 5a84aa880f6da0bac0e2144733fdef3b8558c761 Rename includeconf variables for clarity (Russell Yanofsky) dc8e1e75487461ec9bff433144f0db831b682403 Clarify emptyIncludeConf logic (Russell Yanofsky) Pull request description: This is a refactoring-only change that makes it easier to add a new settings source. This PR doesn't change behavior. The [`util_ArgsMerge`](https://github.com/bitcoin/bitcoin/blob/deb2327b435925c6a39ca654a79283b8eb6aeb86/src/test/util_tests.cpp#L626-L822) and [`util_ChainMerge`](https://github.com/bitcoin/bitcoin/blob/deb2327b435925c6a39ca654a79283b8eb6aeb86/src/test/util_tests.cpp#L843-L924) tests added in #15869 and #15988 were written specifically to confirm that ArgsManager settings are parsed, merged, and returned the same way before and after this change. This change: - Makes it easier to add new settings sources that can get merged with existing sources (see 70675c3e4975203ad6222ba2b00c83b4e4213793 from #15935). - Separates parsing of settings from merging of settings, and deduplicates merging code so it doesn't happen five different places ([GetArg](https://github.com/bitcoin/bitcoin/blob/c459c5f70176928adcee4935813a2dbe7f4dbd51/src/util/system.cpp#L221-L244), [GetNetBoolArg](https://github.com/bitcoin/bitcoin/blob/c459c5f70176928adcee4935813a2dbe7f4dbd51/src/util/system.cpp#L255-L261), [GetArgs](https://github.com/bitcoin/bitcoin/blob/c459c5f70176928adcee4935813a2dbe7f4dbd51/src/util/system.cpp#L460-L467), [IsArgNegated](https://github.com/bitcoin/bitcoin/blob/c459c5f70176928adcee4935813a2dbe7f4dbd51/src/util/system.cpp#L482-L491), [GetUnsuitableSectionOnlyArgs](https://github.com/bitcoin/bitcoin/blob/c459c5f70176928adcee4935813a2dbe7f4dbd51/src/util/system.cpp#L343-L352)) in inconsistent ways. - Documents and tests current strange merging behaviors, so they be cleaned up in the future if resulting code simplifications and UX improvements warrant loss of backwards compatibility. The newly documented behaviors are: command line [ignored arguments](https://github.com/ryanofsky/bitcoin/blob/69d44f3cc75a68d404ca0e1ca2b4831fd2bac4bb/src/util/system.cpp#L323-L326) and [more ignored arguments](https://github.com/ryanofsky/bitcoin/blob/69d44f3cc75a68d404ca0e1ca2b4831fd2bac4bb/src/util/settings.cpp#L67-L72), and config file [reverse precedence](https://github.com/ryanofsky/bitcoin/blob/69d44f3cc75a68d404ca0e1ca2b4831fd2bac4bb/src/util/settings.cpp#L61-L65), [inconsistently applied top-level settings](https://github.com/ryanofsky/bitcoin/blob/69d44f3cc75a68d404ca0e1ca2b4831fd2bac4bb/src/util/settings.cpp#L55-L59), and [zombie values](https://github.com/ryanofsky/bitcoin/blob/69d44f3cc75a68d404ca0e1ca2b4831fd2bac4bb/src/util/settings.cpp#L101-L108). The original motivation for this change was to make it easy to add a new persistent setting source without introducing more bugs and inconsistencies. Two commits building on top of this to add a persistent `-wallet` setting are pretty straightforward and show how the new code can be extended: * 70675c3e4975203ad6222ba2b00c83b4e4213793 from #15935 ā€“ _Add \<datadir>/settings.json persistent settings storage_ * 04c80c40df9fc6f4734ba238ea7f65607cf88089 from #15937 ā€“ _Add loadwallet and createwallet RPC load_on_startup options_ ACKs for top commit: ariard: ACK 083c954 jnewbery: ACK 083c954b02a4e7d0708349eeaf3bac2b5947fb0e jamesob: ACK 083c954b02a4e7d0708349eeaf3bac2b5947fb0e Tree-SHA512: 5d106746a44d64d3963c4ef3f4a2fa668a4bedcc9018d3ea12c86beae2fda48a0b036241665837f68685712366f70f2e1faba84d193fa1f456013503097b7659
2019-11-08scripted-diff: Change `BCLog::DB` to `BCLog::WALLETDB`Wladimir J. van der Laan
-BEGIN VERIFY SCRIPT- git grep -l "BCLog::DB" src | xargs sed -i "s/BCLog::DB/BCLog::WALLETDB/g" sed -i "s/DB =/WALLETDB =/g" src/logging.h -END VERIFY SCRIPT-
2019-11-08Rename `db` log category to `walletdb` (like `coindb`)Wladimir J. van der Laan
Deprecate (but still accept) '-debug=db'.
2019-11-08Merge #17405: wallet: Remove unused boost::this_thread::interruption_pointfanquake
fad1de66a29bf6bd348a932150dad7d472feb3d0 wallet: Remove unused boost::this_thread::interruption_point (MarcoFalke) Pull request description: `BerkeleyEnvironment::Open` is only called from the main thread (init) or an http rpc thread, neither of which can be interrupted, so remove the useless interruption point. `BerkeleyEnvironment{}` is only used in tests, which run in a single process/thread, so remove the useless interruption point. ACKs for top commit: laanwj: ACK fad1de66a29bf6bd348a932150dad7d472feb3d0 fanquake: ACK fad1de66a29bf6bd348a932150dad7d472feb3d0 Tree-SHA512: dacd8398e966e4a6ce5cf7d3ed821c9c267eff40b14c0635085441647cdb72d1642807f89355419f1710f814c7963e35a10d102d0b985c7198261dfc736256f8
2019-11-08Merge #17354: wallet: Tidy CWallet::SetUsedDestinationStatefanquake
0b75a7f0680d16a41043864a897470324917b1e8 wallet: Reuse existing batch in CWallet::SetUsedDestinationState (JoĆ£o Barbosa) 01f45dd00eb032a19d142026e4d019944192da19 wallet: Avoid recursive lock in CWallet::SetUsedDestinationState (JoĆ£o Barbosa) Pull request description: This PR makes 2 distinct changes around `CWallet::SetUsedDestinationState`: - 1st the recursive lock is removed and now it requires the lock to be held; - 2nd change is to support, in the best case, just a wallet database flush when transaction is added to the wallet. ACKs for top commit: achow101: ACK 0b75a7f0680d16a41043864a897470324917b1e8 MarcoFalke: ACK 0b75a7f0680d16a41043864a897470324917b1e8 ryanofsky: Code review ACK 0b75a7f0680d16a41043864a897470324917b1e8. Code changes looks fine but PR description should be updated to say what benefits of the change are. I might have missed something, but I didn't see a place where multiple batches were used previously and a single batch was used now. So the main benefit of this change appears to be removing a recursive lock? And maybe moving toward a consistent convention for passing batch instances? Tree-SHA512: abcf23a5850d29990668db20d6f624cca3e89629cc9ed003e0d05cde1b58ab2ff365034f156684ad13e55764b54c6c0c2bc7d5f96b8af7dc5e45a3be955d6b15
2019-11-08Merge #15931: Remove GetDepthInMainChain dependency on locked chain interfaceSamuel Dobson
36b68de5b2938722911db900ca299f7008780d01 Remove getBlockDepth method from Chain::interface (Antoine Riard) b66c429c56c85fa16c309be0b2bca9c25fdd3e1a Remove locked_chain from GetDepthInMainChain and its callers (Antoine Riard) 0ff03871add000f8b4d8f82aeb168eed2fc9dc5f Use CWallet::m_last_block_processed_height in GetDepthInMainChain (Antoine Riard) f77b1de16feee097a88e99d2ecdd4d84beb4f915 Only return early from BlockUntilSyncedToCurrentChain if current tip is exact match (Antoine Riard) 769ff05e48fb53d4b62c59060424a0fea71d0aab Refactor some importprunedfunds checks with guard clause (Antoine Riard) 5971d3848e09abf571e5308185275296127efca4 Add block_height field in struct Confirmation (Antoine Riard) 9700fcb47feca9d78e005b8d18b41148c8f6b25f Replace CWalletTx::SetConf by Confirmation initialization list (Antoine Riard) 5aacc3eff15b9b5bdc951f1e274f00d581f63bce Add m_last_block_processed_height field in CWallet (Antoine Riard) 10b4729e33f76092bd8cfa06d1a5e0a066436f76 Pass block height in Chain::BlockConnected/Chain::BlockDisconnected (Antoine Riard) Pull request description: Work starter to remove Chain::Lock interface by adding m_last_block_processed_height in CWallet and m_block_height in CMerkleTx to avoid GetDepthInMainChain having to keep a lock . Once this one done, it should ease work to wipe out more cs_main locks from wallet code. I think it's ready for a first round of review before to get further. - `BlockUntilSyncedToCurrent` : restrain isPotentialTip to isTip because we want to be sure that wallet see BlockDisconnected callbacks if its height differs from the Chain one. It means during a reorg, an RPC could return before the BlockDisconnected callback had been triggered. This could cause a tx that had been included in the disconnected block to be displayed as confirmed, for example. ~~- `AbandonTransaction` : in case of conflicted tx (nIndex = -1), we set its m_block_height to the one of conflicting blocks, but if this height is superior to CWallet::m_last_block_processed_height, that means tx isn't conflicted anymore so we return 0 as tx is again unconfirmed~~ After #16624, we instead rely on Confirmation. ~~- `AddToWalletIfInvolvingMe`: in case of block disconnected, transactions are added to mempool again, so we need to replace old txn in `mapWallet` with a height set to zero so we remove check on block_hash.IsNull~~ Already done in #16624 ACKs for top commit: jnewbery: @jkczyz you've ACKed an intermediate commit (github annoyingly orders commits in date order, not commit order). Did you mean to ACK the final commit in this branch (36b68de5b2938722911db900ca299f7008780d01). jkczyz: > @jkczyz you've ACKed an intermediate commit (github annoyingly orders commits in date order, not commit order). Did you mean to ACK the final commit in this branch ([36b68de](https://github.com/bitcoin/bitcoin/commit/36b68de5b2938722911db900ca299f7008780d01)). meshcollider: utACK 36b68de5b2938722911db900ca299f7008780d01 ryanofsky: Code review ACK 36b68de5b2938722911db900ca299f7008780d01. Changes since last review: new jkczyz refactor importprunedfunds commit, changed BlockUntilSyncedToCurrentChainChanges commit title and description, changed Confirmation struct field order and line-wrapped comment jnewbery: utACK 36b68de5b2938722911db900ca299f7008780d01 promag: Code review ACK 36b68de5b2938722911db900ca299f7008780d01. Tree-SHA512: 08b89a0bcc39f67c82a6cb6aee195e6a11697770c788ba737b90986b4893f44e90d1ab9ef87239ea3766508b7e24ea882b7199df41173ab27a3d000328c14644
2019-11-07Add settings_testsRussell Yanofsky
Co-authored-by: James O'Beirne <james.obeirne@gmail.com>
2019-11-07Deduplicate settings merge codeRussell Yanofsky
Get rid of settings merging code in util/system.cpp repeated 5 places, inconsistently: - ArgsManagerHelper::GetArg - ArgsManagerHelper::GetNetBoolArg - ArgsManager::GetArgs - ArgsManager::IsArgNegated - ArgsManager::GetUnsuitableSectionOnlyArgs Having settings merging code separated from parsing simplifies parsing somewhat (for example negated values can simply be represented as false values instead of partially cleared or emply placeholder lists). Having settings merge happen one place instead of 5 makes it easier to add new settings sources and harder to introduce new inconsistencies in the way settings are merged. This commit does not change behavior in any way.
2019-11-07Add util::Settings struct and helper functions.Russell Yanofsky
Implement merging of settings from different sources (command line and config file) separately from parsing code in system.cpp, so it is easier to add new sources. Document current inconsistent merging behavior without changing it. This commit only adds new settings code without using it. The next commit calls the new code to replace existing code in system.cpp. Co-authored-by: John Newbery <john@johnnewbery.com>
2019-11-07Remove includeconf nested scopeRussell Yanofsky
Easier to review ignoring whitespace Suggestion from John Newbery <john@johnnewbery.com> in https://github.com/bitcoin/bitcoin/pull/15934#discussion_r343806860
2019-11-07Rename includeconf variables for clarityRussell Yanofsky
includeconf -> conf_file_names to_include -> conf_file_name include_config -> conf_file_stream Suggestion from John Newbery <john@johnnewbery.com> in https://github.com/bitcoin/bitcoin/pull/15934#discussion_r343905138
2019-11-07Clarify emptyIncludeConf logicRussell Yanofsky
Suggestion from John Newbery <john@johnnewbery.com> in https://github.com/bitcoin/bitcoin/pull/15934#discussion_r343795528
2019-11-07wallet: Remove unused boost::this_thread::interruption_pointMarcoFalke
2019-11-07Merge #16978: test: Seed test RNG context for each test case, print seedMarcoFalke
fae43a97ca947cd0802392e9bb86d9d0572c0fba test: Seed test RNG context for each test case, print seed (MarcoFalke) Pull request description: Debugging failing unit tests is hard if the failure is non-deterministic and the seed is not known. Fix that by printing the seed and making it possible to set the seed from outside. ACKs for top commit: davereikher: Tested ACK fae43a97ca947cd0802392e9bb86d9d0572c0fba Tree-SHA512: 33d848dd1f4180d3664ecf60e9810c2a93590c05276b2c46b1e4fe6e376b45916a46b90c803bb602750ab666da3a05ce499e550024685a90b8cc38fab6667cb8
2019-11-07Merge #17342: refactor: Clean up nScriptCheckThreadsMarcoFalke
5506ecfe7a65d5705616bc048f2f1735b89993fb [refactor] Replace global int nScriptCheckThreads with bool (John Newbery) d9957623b48a7c3eff0ac750d1245fabfb1843a2 [tests] Don't use TestingSetup in the checkqueue_tests (John Newbery) Pull request description: The meaning of this value is confusing. Refactor it and add comments. ACKs for top commit: sipa: ACK 5506ecfe7a65d5705616bc048f2f1735b89993fb promag: ACK 5506ecfe7a65d5705616bc048f2f1735b89993fb, only change was addressing my nits. laanwj: Code review ACK 5506ecfe7a65d5705616bc048f2f1735b89993fb MarcoFalke: ACK 5506ecfe7a65d5705616bc048f2f1735b89993fb šŸ„ Tree-SHA512: 78536727c98d2c23f3c0f3f169131474fef9a4486ae65029011caf06eab30f6f70ff73a65b2fb04a5d969fc1150858d1c6ea4767f04d48c1eea6b829316d0e63
2019-11-07Merge #17384: test: Create new test libraryMarcoFalke
fa4c6fa9b1139791f45f1495d662c1c7cd2f7ed6 doc: Add documentation for new test/lib (MarcoFalke) faec28252cf4f8e754c689a7a44fd421f631db50 scripted-diff: test: Move setup_common to test library (MarcoFalke) Pull request description: Sorry for clickbait, this is only a move-only scripted-diff commit and one documentation commit. Longer term, someone who knows something about build systems can make this an actual library. Motivation for this is that each module gets compiled for each target that includes it. For example, setup_common is compiled 27 times (for the fuzz suite) and another 3 times for the other tests (bench, unit test, gui) ACKs for top commit: practicalswift: ACK fa4c6fa9b1139791f45f1495d662c1c7cd2f7ed6 -- diff looks correct and Travis is happy jonatack: ACK fa4c6fa9b1139791f45f1495d662c1c7cd2f7ed6 with the reserve that the commit messages (and PR description) contain the motivation for this change. Built, ran tests, light code review. ryanofsky: Code review ACK fa4c6fa9b1139791f45f1495d662c1c7cd2f7ed6. I didn't realize `lib` was actually name of existing directory, not a new name. But in any case this looks good and nice to have one scripted diff instead of two. Tree-SHA512: 2e176df90c60578276e4a6dc83ff57ff59d8e666ecf30c5ceacb8c326725da91baa4cac3dfa7a2e1605f58122a3e3e27e4938ff33e3a0ce7ea53afffebbf57a4
2019-11-06[refactor] Replace global int nScriptCheckThreads with boolJohn Newbery
The global nScriptCheckThreads int is confusing and is only needed for its int-ness in AppInitMain. Move all `-par` parsing logic there and replace the int nScriptCheckThreads with a bool g_parallel_script_checks. Also tidy up logic and improve comments.
2019-11-06[tests] Don't use TestingSetup in the checkqueue_testsJohn Newbery
It's only needed for a hardcoded int, which we can define locally.
2019-11-06Remove getBlockDepth method from Chain::interfaceAntoine Riard
Pass conflicting height in CWallet::MarkConflicted
2019-11-06Remove locked_chain from GetDepthInMainChain and its callersAntoine Riard
We don't remove yet Chain locks as we need to preserve lock order with CWallet one until swapping at once to avoid deadlock failures (spotted by --enable-debug)
2019-11-06Use CWallet::m_last_block_processed_height in GetDepthInMainChainAntoine Riard
Avoid to lock chain to query state thanks to tracking last block height in CWallet.
2019-11-06Only return early from BlockUntilSyncedToCurrentChain if current tipAntoine Riard
is exact match In the next commit, we start using BlockConnected/BlockDisconnected callbacks to establish tx depth, rather than querying the chain directly. Currently, BlockUntilSyncedToCurrentChain will return early if the best block processed by the wallet is a descendant of the node'tip. That means that in the case of a re-org, it won't wait for the BlockDisconnected callbacks that have been enqueued during the re-org but have not yet been triggered in the wallet. Change BlockUntilSyncedToCurrentChain to only return early if the wallet's m_last_block_processed matches the tip exactly. This ensures that there are no BlockDisconnected or BlockConnected callbacks in-flight.
2019-11-06Refactor some importprunedfunds checks with guard clauseAntoine Riard
Credit to jkczyz
2019-11-06Add block_height field in struct ConfirmationAntoine Riard
At wallet loading, we rely on chain state querying to retrieve height of txn, to do so we ensure that lock order is respected between cs_main and cs_wallet. If wallet loaded is the wallet-tool one, all wallet txn will show up with a height of zero. It doesn't matter as confirmation height is not used by wallet-tool. Reorder arguments and document Confirmation calls to avoid ambiguity. Fixes nits left from #16624
2019-11-06doc: Add documentation for new test/libMarcoFalke
2019-11-06scripted-diff: test: Move setup_common to test libraryMarcoFalke
-BEGIN VERIFY SCRIPT- # Move files for f in $(git ls-files src/test/lib/); do git mv $f src/test/util/; done git mv src/test/setup_common.cpp src/test/util/ git mv src/test/setup_common.h src/test/util/ # Replace Windows paths sed -i -e 's|\\setup_common|\\util\\setup_common|g' $(git grep -l '\\setup_common') sed -i -e 's|src\\test\\lib\\|src\\test\\util\\|g' build_msvc/test_bitcoin/test_bitcoin.vcxproj # Everything else sed -i -e 's|/setup_common|/util/setup_common|g' $(git grep -l 'setup_common') sed -i -e 's|test/lib/|test/util/|g' $(git grep -l 'test/lib/') # Fix include guard sed -i -e 's|BITCOIN_TEST_SETUP_COMMON_H|BITCOIN_TEST_UTIL_SETUP_COMMON_H|g' ./src/test/util/setup_common.h sed -i -e 's|BITCOIN_TEST_LIB_|BITCOIN_TEST_UTIL_|g' $(git grep -l 'BITCOIN_TEST_LIB_') -END VERIFY SCRIPT-
2019-11-06Replace CWalletTx::SetConf by Confirmation initialization listAntoine Riard
2019-11-06Merge #17381: LegacyScriptPubKeyMan code cleanupsWladimir J. van der Laan
05b224a175065aee4d6d9c471722bc4503f01fdf Add missing SetupGeneration error handling in EncryptWallet (Russell Yanofsky) bfd826a675445801adec86a469040f3ceb8172ee Clean up nested scope in GetReservedDestination (Russell Yanofsky) 491a599b37f3e3a648e52aebed677ca11b0615e2 Get rid of confusing LegacyScriptPubKeyMan::TopUpKeyPool method (Russell Yanofsky) 4a0abf694ee10cf186f25a67ca35c3fce0c10874 Pass CTxDestination to ScriptPubKeyMan::GetMetadata (Russell Yanofsky) b07b07cd8779355ba1dd16e7eb4af42e0ae1c587 Add EnsureLegacyScriptPubKeyMan and use in rpcwallet.cpp (Russell Yanofsky) Pull request description: This PR implements suggested code cleanups from #17300 and #17304 review comments ACKs for top commit: Sjors: re-ACK 05b224a laanwj: Code review ACK 05b224a175065aee4d6d9c471722bc4503f01fdf Tree-SHA512: 12fd86637088515b744c028e0501c5d21a9cf9ee9c9cfd70e9cb65d44611ea5643abd5f6f101105caa5aff015d74de606f074f08af7dae8429f929d21288ab45
2019-11-06Merge #17390: test: Add util_ArgParsing testWladimir J. van der Laan
286f197704e82045c762d332aba5d1ac52e0212d Add util_ArgParsing test (Russell Yanofsky) Pull request description: ArgsManager test coverage for parsing of integer and boolean values is currently very poor and doesn't give us a way of knowing whether changes to ArgsManager may unintentionally break backwards compatibility, so this adds a new test to catch regressions. ACKs for top commit: promag: ACK 286f197, more surprising results šŸ˜± laanwj: ACK 286f197704e82045c762d332aba5d1ac52e0212d Tree-SHA512: 9e1db3ef87e55abbc280af60c088f35765a1f9e2ec20507ad0c1992027b875490016868dcb8cc287e6df279dd0e00f10550901af3de3d36287867249e0bd8207
2019-11-06Merge #17368: cli: fix -getinfo output when compiled with no walletWladimir J. van der Laan
3d05d332693ec860626fc77e6ba50dec94e4e83c cli: fix -getinfo output when compiled with no wallet (fanquake) Pull request description: master (33b155f28732487854cf0ca29ca17c50f8c6872e): ```bash src/bitcoin-cli -getinfo { "version": 199900, "protocolversion": 70015, "blocks": 602348, "headers": 602348, "verificationprogress": 0.9999995592310106, "timeoffset": 0, "connections": 10, "proxy": "", "difficulty": 13691480038694.45, "chain": "main", "walletversion": null, "balance": null, "keypoololdest": null, "keypoolsize": null, "paytxfee": null, "relayfee": 0.00001000, "warnings": "This is a pre-release test build - use at your own risk - do not use for mining or merchant applications" } ``` This PR (3d05d332693ec860626fc77e6ba50dec94e4e83c): ```bash { "version": 199900, "protocolversion": 70015, "blocks": 602348, "headers": 602348, "verificationprogress": 0.9999996313568186, "timeoffset": 0, "connections": 10, "proxy": "", "difficulty": 13691480038694.45, "chain": "main", "relayfee": 0.00001000, "warnings": "This is a pre-release test build - use at your own risk - do not use for mining or merchant applications" } ``` ACKs for top commit: MarcoFalke: ouch ACK 3d05d332693ec860626fc77e6ba50dec94e4e83c laanwj: ACK 3d05d332693ec860626fc77e6ba50dec94e4e83c darosior: ACK 3d05d332693ec860626fc77e6ba50dec94e4e83c Tree-SHA512: 055424e122a082cbfea410da287d9ceb7ed405fd68d53e2f5bef62beea80bc374a7d00366de0479d23faecb7f063b232aca52e9fdbdb97c58ddf46e7749136a9
2019-11-06Merge #17388: Add missing newline in util_ChainMerge testWladimir J. van der Laan
3645e4ca0033bb6365f41ef710111780c139370f Add missing newline in util_ChainMerge test (Russell Yanofsky) Pull request description: This was causing a lot of test cases not not be very meaningful because multiple configuration options were combined into one line. The changes in test output with this fix make sense and look like: ```diff - testnet=1 regtest=1 || test + testnet=1 regtest=1 || error: Invalid combination of -regtest, -testnet and -chain. Can use at most one. ``` Issue was reported and debugged by Wladimir J. van der Laan <laanwj@protonmail.com> in https://github.com/bitcoin/bitcoin/pull/17385#issuecomment-550033222 <!-- *** Please remove the following help text before submitting: *** Pull requests without a rationale and clear improvement may be closed immediately. --> <!-- Please provide clear motivation for your patch and explain how it improves Bitcoin Core user experience or Bitcoin Core developer experience significantly: * Any test improvements or new tests that improve coverage are always welcome. * All other changes should have accompanying unit tests (see `src/test/`) or functional tests (see `test/`). Contributors should note which tests cover modified code. If no tests exist for a region of modified code, new tests should accompany the change. * Bug fixes are most welcome when they come with steps to reproduce or an explanation of the potential issue as well as reasoning for the way the bug was fixed. * Features are welcome, but might be rejected due to design or scope issues. If a feature is based on a lot of dependencies, contributors should first consider building the system outside of Bitcoin Core, if possible. * Refactoring changes are only accepted if they are required for a feature or bug fix or otherwise improve developer experience significantly. For example, most "code style" refactoring changes require a thorough explanation why they are useful, what downsides they have and why they *significantly* improve developer experience or avoid serious programming bugs. Note that code style is often a subjective matter. Unless they are explicitly mentioned to be preferred in the [developer notes](/doc/developer-notes.md), stylistic code changes are usually rejected. --> <!-- Bitcoin Core has a thorough review process and even the most trivial change needs to pass a lot of eyes and requires non-zero or even substantial time effort to review. There is a huge lack of active reviewers on the project, so patches often sit for a long time. --> ACKs for top commit: laanwj: ACK 3645e4ca0033bb6365f41ef710111780c139370f practicalswift: ACK 3645e4ca0033bb6365f41ef710111780c139370f -- diff looks correct Tree-SHA512: ca5bde9b9f553811d4827113f4880d15d7b8f4f1455b95bbf34c9a1512fdd53062f1a2133c50d9b54f94160a1ee77a54bc82681a5f3bf25d2b0d01f8a8e95165
2019-11-05Add util_ArgParsing testRussell Yanofsky
ArgsManager test coverage for parsing of integer and boolean values is currently very poor and doesn't give us a way of knowing whether changes to ArgsManager may unintentionally break backwards compatibility, so this adds a new test to catch regressions.
2019-11-06Merge #17382: rpc: Remove unused boost::this_thread::interruption_pointWladimir J. van der Laan
fa5facd3e72b6d61374b0b93b722b55e2b090020 rpc: Remove unused boost::this_thread::interruption_point (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, the rpc threads are `std::thread`, which does not have an `std::thread::interrupt` member function to request interruption: https://dev.visucore.com/bitcoin/doxygen/httpserver_8cpp.html#ae1a63374e18b9abd348eb74e4243ea34 Thus, the interruption points can be removed. ACKs for top commit: laanwj: ACK fa5facd3e72b6d61374b0b93b722b55e2b090020, this does nothing. practicalswift: ACK fa5facd3e72b6d61374b0b93b722b55e2b090020 jamesob: ACK https://github.com/bitcoin/bitcoin/pull/17382/commits/fa5facd3e72b6d61374b0b93b722b55e2b090020 Tree-SHA512: 4e29a44df1f2702cbd1ffdffa559440a8bb800baab64b4116e2c3d27cd64d8d1e8aafe1dc21b1a4e3988470d03be19cae294bd5669f7abf6d487685dc8fd8d7e
2019-11-05Merge #16805: logs: add timing information to FlushStateToDisk()Wladimir J. van der Laan
dcef9a2922317cb2849f397366b6c56d755db6c9 logs: add timing information to FlushStateToDisk() (James O'Beirne) 41edaf227a69bc4846d5996675e8763fdfe0f367 logs: add BCLog::Timer and related macros (James O'Beirne) Pull request description: It's currently annoying to detect FlushStateToDisk() calls when benchmarking since they have to be inferred from a drop in coins count from the `UpdateTip: ` log messages. This adds a new logging utility, `BCLog::Timer`, and some related macros that are generally useful for printing timing-related logging messages, and a message that is unconditionally written when the coins cache is flushed to disk. ``` 2019-09-04T20:17:51Z FlushStateToDisk: write block and undo data to disk completed (3ms) 2019-09-04T20:17:51Z FlushStateToDisk: write block index to disk completed (370ms) 2019-09-04T20:17:51Z FlushStateToDisk: write coins cache to disk (2068451 coins, 294967kB) completed (21481ms) ``` ACKs for top commit: laanwj: Thanks, ACK dcef9a2922317cb2849f397366b6c56d755db6c9 ryanofsky: Code review ACK dcef9a2922317cb2849f397366b6c56d755db6c9. No changes since last review other than moving code to new timer.h header Tree-SHA512: 6d61e48a062d3edb48d0e056a6f0b1f8031773cc99289ee4544f8349d24526b88519e1e304009d56e428f1eaf76c857bf8e7e1c0b6873a6f270306accb5edc3d
2019-11-05Add missing newline in util_ChainMerge testRussell Yanofsky
This was causing a lot of test cases not not be very meaningful because multiple configuration options were combined into one line. The changes in test output with this fix make sense and look like: ```diff - testnet=1 regtest=1 || test + testnet=1 regtest=1 || error: Invalid combination of -regtest, -testnet and -chain. Can use at most one. ``` Issue was reported and debugged by Wladimir J. van der Laan <laanwj@protonmail.com> in https://github.com/bitcoin/bitcoin/pull/17385#issuecomment-550033222
2019-11-05Merge #17360: gui: Improve "Hide" button tool-tip messageWladimir J. van der Laan
1c26c16065182ca2d2cdbb05fae79cac8c75f17d Improve "Hide" button tool-tip message (Danny-Scott) Pull request description: Cleaned up the tool tip text, it looks as though it just got included back in 2014 when the whole section was added. Changed hide button tool tip within transaction fee settings area from "collapse fee-settings" to "Hide transaction fee settings" to be more user friendly and fit with other tool tips. ![hide-transaction-fee-tool-tip](https://user-images.githubusercontent.com/17258195/68086415-b7b70680-fe43-11e9-82cb-567b9730c1b9.png) ACKs for top commit: laanwj: ACK 1c26c16065182ca2d2cdbb05fae79cac8c75f17d Tree-SHA512: e2c83271c273f785ac625da9f88e095076043e21a9c59792049c271747837d19483e0cae5466c26ef3231947b6245680c4c136a530ba6f1885f9ddc18f2560d6
2019-11-05Merge #16540: test: Add ASSERT_DEBUG_LOG to unit test frameworkMarcoFalke
fa2c44c3ccc3e7a54e2afc862addd555d5a6e51b test: Add ASSERT_DEBUG_LOG to unit test framework (MarcoFalke) fa1936f57bbf5aebb1f8fc18701441d79219d443 logging: Add member for arbitrary print callbacks (MarcoFalke) Pull request description: Similar to `assert_debug_log` in the functional test framework Top commit has no ACKs. Tree-SHA512: aa9eaeca386b61d806867c04a33275f6eb4624fa5bf50f2928d16c83f5634bac96bcac46f9e8eda3b00b4251c5f12d7b01d6ffd84ba8e05c09eeec810cc31251