aboutsummaryrefslogtreecommitdiff
path: root/src
AgeCommit message (Collapse)Author
2023-03-09refactor: Replace GetTimeMicros by SystemClockMarcoFalke
2023-03-08Merge bitcoin/bitcoin#26968: doc: Fixup remove 'omitted...' doc for rpc ↵fanquake
getrawtransaction when verbose is 2 3e947d7117c97a3cc34cfa7e1f5515fa0192fbe7 doc: remove 'omitted...' doc for rpc getrawtransaction when verbose is 2 (dougEfish) Pull request description: Remove optional rpc doc for getrawtransaction when verbose is 2 ACKs for top commit: stickies-v: ACK 3e947d7117c97a3cc34cfa7e1f5515fa0192fbe7 Tree-SHA512: b9e970d6ef4a47ec7ca32f5ff1028cc901f1bfdc1571668208505d42f4160733530601b78e469de82a854d3b298a55a81d0a7916bc5db4a43ad6d6a299c55c9e
2023-03-08Merge bitcoin/bitcoin#27180: doc: DummySignInput mention external signerfanquake
6fc5f4fdb661eb9d42842227501106afcf7111e7 doc: DummySignInput mention external signer (Sjors Provoost) Pull request description: Followups for #26032. So far nothing major. ACKs for top commit: ishaanam: ACK 6fc5f4fdb661eb9d42842227501106afcf7111e7 S3RK: ACK 6fc5f4fdb661eb9d42842227501106afcf7111e7 Tree-SHA512: e27edde9853487fe3eef8213f991aae3724f318bbbe0b11da23759879adaf9a31771e6ea0c30baaebca149032780b89b32aa540ff456ca3d5ec6adb0371749c6
2023-03-08Merge bitcoin/bitcoin#27189: util: Use steady clock in SeedStrengthen, ↵fanquake
FindBestImplementation, FlushStateToDisk fa1b4e5c3294fc9aec033892a4a4d7b5cfc015aa Use steady clock in FlushStateToDisk (MarcoFalke) 1111e2f8b43cd9ed62dcf6b571a224b84fc421fd Use steady clock in SeedStrengthen and FindBestImplementation (MarcoFalke) Pull request description: There may be a theoretical deadlock for the duration of the offset when the system clock is adjusted into a past time while executing `SeedStrengthen`. Fix this by using steady clock. Do the same in `FindBestImplementation`, which shouldn't be affected, because it discards outlier measurements. However, doing the same there for consistency seems fine. Do the same in `FlushStateToDisk`, which should make the flushes more steady, if the system clock is adjusted by a large offset. ACKs for top commit: john-moffett: ACK fa1b4e5c3294fc9aec033892a4a4d7b5cfc015aa willcl-ark: ACK fa1b4e5c3 Tree-SHA512: cc625e796b186accd53222bd64eb57d0512bc7e588312d254349b542bbc5e5daac348ff2b3b3f7dc5ae0bbbae2ec11fdbf3022cf2164211633765a4b0108e83e
2023-03-07Merge bitcoin/bitcoin#25740: assumeutxo: background validation completionAndrew Chow
2b373fe49d64f04ceab2309d3f40da7bac6b37d6 docs: update assumeutxo.md (James O'Beirne) 87a1108c81fe0cb15c3860e3a67dc1f43ffec705 test: add snapshot completion unittests (James O'Beirne) d70919a88fc90a2662f9a844deb085d03ee7b5d8 refactor: make MempoolMutex() public (James O'Beirne) 7300ced9de22e6d1bff816e6538d3370cebe7501 log: add LoadBlockIndex() message for assumedvalid blocks (James O'Beirne) d96c59cc5cd2f73f1f55c133c52208671fe75ef3 validation: add ChainMan logic for completing UTXO snapshot validation (James O'Beirne) f2a4f3376f1476b38a79a549bd81ba3006225df6 move-only-ish: init: factor out chainstate initialization (James O'Beirne) 637a90b973f60555ea4fef4b845ffa7533dcb866 add Chainstate::HasCoinsViews() (James O'Beirne) c29f26b47b8ef978d8689dc0222aa663361ee6cb validation: add CChainState::m_disabled and ChainMan::isUsable (James O'Beirne) 5ee22cdafd2562bcb8bf0ae6025e4b53c826382d add ChainstateManager.GetSnapshot{BaseHeight,BaseBlock}() (James O'Beirne) Pull request description: This is part of the [assumeutxo project](https://github.com/bitcoin/bitcoin/projects/11) (parent PR: https://github.com/bitcoin/bitcoin/pull/15606) Part two of replacing https://github.com/bitcoin/bitcoin/pull/24232. --- When a user activates a snapshot, the serialized UTXO set data is used to create an "assumed-valid" chainstate, which becomes active in an attempt to get the node to network tip as quickly as possible. Simultaneously in the background, the already-existing chainstate continues "conventional" IBD to both accumulate full block data and serve as a belt-and-suspenders to validate the assumed-valid chainstate. Once the background chainstate's tip reaches the base block of the snapshot used, we set `m_stop_use` on that chainstate and immediately take the hash of its UTXO set; we verify that this matches the assumeutxo value in the source code. Note that while we ultimately want to remove this background chainstate, we don't do so until the following initialization process, when we again check the UTXO set hash of the background chainstate, and if it continues to match, we remove the (now unnecessary) background chainstate, and move the (previously) assumed-valid chainstate into its place. We then reinitialize the chainstate in the normal way. As noted in previous comments, we could do the filesystem operations "inline" immediately when the background validation completes, but that's basically just an optimization that saves disk space until the next restart. It didn't strike me as worth the risk of moving chainstate data around on disk during runtime of the node, though maybe my concerns are overblown. The final result of this completion process is a fully-validated chain, where the only evidence that the user synced using assumeutxo is the existence of a `base_blockhash` file in the `chainstate` directory. ACKs for top commit: achow101: ACK 2b373fe49d64f04ceab2309d3f40da7bac6b37d6 Tree-SHA512: a204e1d6e6932dd83c799af3606b01a9faf893f04e9ee1a36d63f2f1ccfa9118bdc1c107d86976aa0312814267e6a42074bf3e2bf1dead4b2513efc6d955e13d
2023-03-07test: add snapshot completion unittestsJames O'Beirne
Also adjusts the previous snapshot chainstate init tests to account for the fact that the init process is now attempting to validate and complete background chainstates whose tip is at the snapshot base block. We use a DisconnectTip() hack to preserve the nature of the test.
2023-03-07refactor: make MempoolMutex() publicJames O'Beirne
for use in the following unittests.
2023-03-07log: add LoadBlockIndex() message for assumedvalid blocksJames O'Beirne
I found this useful during unittest debugging.
2023-03-07validation: add ChainMan logic for completing UTXO snapshot validationJames O'Beirne
Trigger completion when a background validation chainstate reaches the same height as a UTXO snapshot, and handle cleaning up the chainstate on subsequent startup.
2023-03-07Merge bitcoin/bitcoin#27150: Deduplicate bitcoind and bitcoin-qt init codeAndrew Chow
802cc1ef536e11944608fe9ab782d3e962037703 Deduplicate bitcoind and bitcoin-qt init code (Ryan Ofsky) d172b5c6718b69200c8ad211fe709860081bd692 Add InitError(error, details) overload (Ryan Ofsky) 3db2874bd71d2391747b7385cabcbfef67218c4c Extend bilingual_str support for tinyformat (Ryan Ofsky) c361df90b9fd34e50bbf1db43b866930e5498357 scripted-diff: Remove double newlines after some init errors (Ryan Ofsky) Pull request description: Add common InitConfig function to deduplicate bitcoind and bitcoin-qt code reading config files and creating the datadir. Noticed the duplicate code while reviewing #27073 and want to remove it because difference in bitcoin-qt and bitcoind behavior make it hard to evaluate changes like #27073 There are a few minor changes in behavior: - In bitcoin-qt, when there is a problem reading the configuration file, the GUI error text has changed from "Error: Cannot parse configuration file:" to "Error reading configuration file:" to be consistent with bitcoind. - In bitcoind, when there is a problem reading the settings.json file, the error text has changed from "Failed loading settings file" to "Settings file could not be read" to be consistent with bitcoin-qt. - In bitcoind, when there is a problem writing the settings.json file, the error text has changed from "Failed saving settings file" to "Settings file could not be written" to be consistent with bitcoin-qt. - In bitcoin-qt, if there datadir is not accessible (e.g. no permission to read), there is an normal error dialog showing "Error: filesystem error: status: Permission denied [.../settings.json]", instead of an uncaught exception. ACKs for top commit: Sjors: Light review ACK 802cc1ef536e11944608fe9ab782d3e962037703 TheCharlatan: ACK 802cc1ef536e11944608fe9ab782d3e962037703 achow101: ACK 802cc1ef536e11944608fe9ab782d3e962037703 Tree-SHA512: 9c78d277e9ed595fa8ce286b97d2806e1ec06ddbbe7bd3434bd9dd7b456faf8d989f71231e97311f36edb9caaec645a50c730bd7514b8e0fe6e6f7741b13d981
2023-03-07Merge bitcoin/bitcoin#27218: util: Work around ParseHex gcc cross compiler bugfanquake
fa8481b05fae6a7e44ca526d930993f02549234e util: Work around ParseHex gcc cross compiler bug (MarcoFalke) Pull request description: I fail to see how an explicit `ParseHex` template instantiation fails to also instantiate `TryParseHex`. Nonetheless, to work around a compiler bug, change the explicit instantiation from `ParseHex` to `TryParseHex`. (`ParseHex` is inline anyway and will be instantiated by the compiler either way). Fixes https://github.com/bitcoin/bitcoin/pull/25227#issuecomment-1456009757 : ``` CXXLD bitcoind /usr/lib/gcc-cross/powerpc64le-linux-gnu/11/../../../../powerpc64le-linux-gnu/bin/ld: libbitcoin_node.a(libbitcoin_node_a-net_processing.o): in function `(anonymous namespace)::PeerManagerImpl::ProcessMessage(CNode&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, CDataStream&, std::chrono::duration<long, std::ratio<1l, 1000000l> >, std::atomic<bool> const&)': net_processing.cpp:(.text+0x29660): undefined reference to `std::optional<std::vector<unsigned char, std::allocator<unsigned char> > > TryParseHex<unsigned char>(std::basic_string_view<char, std::char_traits<char> >)' /usr/lib/gcc-cross/powerpc64le-linux-gnu/11/../../../../powerpc64le-linux-gnu/bin/ld: libbitcoin_node.a(libbitcoin_node_a-rest.o): in function `rest_getutxos(std::any const&, HTTPRequest*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)': rest.cpp:(.text+0x83b4): undefined reference to `std::optional<std::vector<unsigned char, std::allocator<unsigned char> > > TryParseHex<unsigned char>(std::basic_string_view<char, std::char_traits<char> >)' /usr/lib/gcc-cross/powerpc64le-linux-gnu/11/../../../../powerpc64le-linux-gnu/bin/ld: libbitcoin_node.a(libbitcoin_node_a-torcontrol.o): in function `std::vector<unsigned char, std::allocator<unsigned char> > ParseHex<unsigned char>(std::basic_string_view<char, std::char_traits<char> >)': torcontrol.cpp:(.text._Z8ParseHexIhESt6vectorIT_SaIS1_EESt17basic_string_viewIcSt11char_traitsIcEE[_Z8ParseHexIhESt6vectorIT_SaIS1_EESt17basic_string_viewIcSt11char_traitsIcEE]+0x2c): undefined reference to `std::optional<std::vector<unsigned char, std::allocator<unsigned char> > > TryParseHex<unsigned char>(std::basic_string_view<char, std::char_traits<char> >)' /usr/lib/gcc-cross/powerpc64le-linux-gnu/11/../../../../powerpc64le-linux-gnu/bin/ld: libbitcoin_common.a(libbitcoin_common_a-external_signer.o): in function `ExternalSigner::SignTransaction(PartiallySignedTransaction&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&)': external_signer.cpp:(.text+0x8d84): undefined reference to `std::optional<std::vector<unsigned char, std::allocator<unsigned char> > > TryParseHex<unsigned char>(std::basic_string_view<char, std::char_traits<char> >)' collect2: error: ld returned 1 exit status ACKs for top commit: gruve-p: ACK https://github.com/bitcoin/bitcoin/pull/27218/commits/fa8481b05fae6a7e44ca526d930993f02549234e hebasto: ACK fa8481b05fae6a7e44ca526d930993f02549234e, tested on Ubuntu 22.04, gcc 11.3 for the `riscv64-linux-gnu` host., Tree-SHA512: 53efa424e7e18d85a2c9ac2267b9370ae3594d9be73da5135a3a79bf07ab50fcc5357cbde09dc0b2a9eb78d78ec37beae0c9f876333b568e678b9d0067bc9e4e
2023-03-07util: Work around ParseHex gcc cross compiler bugMarcoFalke
2023-03-06Merge bitcoin/bitcoin#26742: http: Track active requests and wait for last ↵Andrew Chow
to finish - 2nd attempt 60978c8080ec13ff4571c8a89e742517b2aca692 test: Reduce extended timeout on abortnode test (Fabian Jahr) 660bdbf785a32024f0694915fa043968a0afb573 http: Release server before waiting for event base loop exit (João Barbosa) 8c6d007c80dc3fec5ce6c0196381444a5ed7e424 http: Track active requests and wait for last to finish (João Barbosa) Pull request description: This revives #19420. Since promag is not so active at the moment, I can support this to finally get it merged. The PR is rebased and comments by jonatack have been addressed. Once this is merged, I will also reopen #19434. ACKs for top commit: achow101: ACK 60978c8080ec13ff4571c8a89e742517b2aca692 stickies-v: re-ACK [60978c8](https://github.com/bitcoin/bitcoin/commit/60978c8080ec13ff4571c8a89e742517b2aca692) hebasto: ACK 60978c8080ec13ff4571c8a89e742517b2aca692 Tree-SHA512: eef0fe1081e9331b95cfafc71d82f2398abd1d3439dac5b2fa5c6d9c0a3f63ef19adde1c38c88d3b4e7fb41ce7c097943f1815c10e33d165918ccbdec512fe1c
2023-03-06Merge bitcoin/bitcoin#25806: wallet: group outputs only once, decouple it ↵Andrew Chow
from Coin Selection 6a302d40dfcc33b15bf27a1d3fcc04bfd3cbda44 wallet: single output groups filtering and grouping process (furszy) bd91ed1cb2cc804f824d1b204513ac2afb7d5e28 wallet: unify outputs grouping process (furszy) 55962001da4f8467e52da502b05f5c0a85128fb0 test: coinselector_tests refactor, use CoinsResult instead of plain std::vector (furszy) 34f54a0a3a54c59d3f062de69550ab3a3b077ea0 wallet: decouple outputs grouping process from each ChooseSelectionResult (furszy) 461f0821a2dff0a27f755464b0eb92314fba1acd refactor: make OutputGroup::m_outputs field a vector of shared_ptr (furszy) d8e749bb840cf65065ed00561998255156126278 test: wallet, add coverage for outputs grouping process (furszy) 06ec8f992890cac69cd0fd20224aa51fa311a181 wallet: make OutputGroup "positive_only" filter explicit (furszy) Pull request description: The idea originates from https://github.com/bitcoin/bitcoin/pull/24845#issuecomment-1130310321. Note: For clarity, it's recommended to start reviewing from the end result to understand the structure of the flow. #### GroupOutputs function rationale: If "Avoid Partial Spends" is enabled, the function gathers outputs with the same script together inside a container. So Coin Selection can treats them as if them were just one possible input and either select them all or not select them. #### How the Inputs Fetch + Selection process roughly works: ``` 1. Fetch user’s manually selected inputs. 2. Fetch wallet available coins (walks through the entire wallet txes map) and insert them into a set of vectors (each vector store outputs from a single type). 3. Coin Selection Process: Call `AttemptSelection` 8 times. Each of them expands the coin eligibility filter (accepting a larger subset of coins in the calculation) until it founds a solutions or completely fails if no solutions gets founds after the 8 rounds. Each `AttemptSelection` call performs the following actions: - For each output type supported by the wallet (P2SH, P2PK, P2WPKH, P2WSH and a combination of all of them): Call ‘ChooseSelectionResult’ providing the respective, filtered by type, coins vector. Which: I. Groups the outputs vector twice (one for positive only and a second one who includes the negative ones as well). - GroupOutputs walks-through the entire inputted coins vector one time at least, + more if we are avoiding partial spends, to generate a vector of OutputGroups. II. Then performs every coin selection algorithm using the recently created vector of OutputGroup: (1) BnB, (2) knapsack and (3) SRD. III. Then returns the best solution out of them. ``` We perform the general operation of gathering outputs, with the same script, into a single container inside: Each coins selection attempt (8 times —> each coin eligibility filter), for each of the outputs vector who were filtered by type (plus another one joining all the outputs as well if needed), twice (one for the positive only outputs effective value and a second one for all of them). So, in the worst case scenario where no solution is found after the 8 Coin Selection attempts, the `GroupOutputs` function is called 80 times (8 * 5 * 2). #### Improvements: This proposal streamlines the process so that the output groups, filtered by coin eligibility and type, are created in a single loop outside of the Coin Selection Process. The new process is as follows: ``` 1. Fetch user’s manually selected inputs. 2. Fetch wallet available coins. 3. Group outputs by each coin eligibility filter and each different output type found. 4. Coin Selection Process: Call AttemptSelection 8 times. Each of them expands the coin eligibility filter (accepting different output groups) until it founds a solutions or completely fails if no solutions gets founds after the 8 rounds. Each ‘AttemptSelection’ call performs the following actions: - For each output type supported by the wallet (P2SH, P2PK, P2WPKH, P2WSH and all of them): A. Call ‘ChooseSelectionResult’ providing the respective, filtered by type, output group. Which: I. Performs every coin selection algorithm using the provided vector of OutputGroup: (1) BnB, (2) knapsack and (3) SRD. II. Then returns the best solution out of them. ``` Extra Note: The next steps after this PR will be to: 1) Merge `AvailableCoins` and `GroupOutputs` processes. 2) Skip entire coin selection rounds if no new coins are added into the subsequent round. 3) Remove global feerates from the OutputGroup class. 4) Remove secondary "grouped" tx creation from `CreateTransactionInternal` by running Coin Selection results over the aps grouped outputs vs non-aps ones. ACKs for top commit: S3RK: ReACK 6a302d4 achow101: ACK 6a302d40dfcc33b15bf27a1d3fcc04bfd3cbda44 theStack: re-ACK 6a302d40dfcc33b15bf27a1d3fcc04bfd3cbda44 :coconut: Tree-SHA512: dff849063be328e7d9c358ec80239a6db2cd6131963b511b83699b95b337d3106263507eaba0119eaac63e6ac21c6c42d187ae23d79d9220b90c323d44b01d24
2023-03-06doc: remove 'omitted...' doc for rpc getrawtransaction when verbose is 2dougEfish
2023-03-06Merge bitcoin/bitcoin#27155: doc: Expand scantxoutset help text to cover ↵Andrew Chow
tr() and miniscript e4ede64fe87b1619f65da5753088d93e6f481b05 Expand scantxoutset help text to cover tr() and miniscript (Greg Sanders) Pull request description: ACKs for top commit: achow101: ACK e4ede64fe87b1619f65da5753088d93e6f481b05 darosior: webACK e4ede64fe87b1619f65da5753088d93e6f481b05 Tree-SHA512: 6b5d9e7fccc8242f4534861c1b438ec40fb03fbf5968c5a4af5ddbced73df6d666812053c2e12a1e0a34057f6f4fe11987c8c59d8cdfa48ca7ab7d2720b51ef9
2023-03-06Merge bitcoin/bitcoin#25491: wallet: use Mutex for g_sqlite_mutex instead of ↵Andrew Chow
GlobalMutex 4163093d6374e865dc57e33dbea0e7e359062e0a wallet: use Mutex for g_sqlite_mutex instead of GlobalMutex (Vasil Dimov) Pull request description: Using `Mutex` provides stronger guarantee than `GlobalMutex` wrt Clang's thread safety analysis. Thus it is better to reduce the usage of `GlobalMutex` in favor of `Mutex`. Using `Mutex` for `g_sqlite_mutex` is ok because its usage is limited in `wallet/sqlite.cpp` and it does not require propagating the negative annotations to not relevant code. ACKs for top commit: achow101: ACK 4163093d6374e865dc57e33dbea0e7e359062e0a hebasto: re-ACK 4163093d6374e865dc57e33dbea0e7e359062e0a TheCharlatan: ACK 4163093d6374e865dc57e33dbea0e7e359062e0a Tree-SHA512: 4913bcb8437ecf0e6b6cb781d02a6d24ffb4bf3e2e1899fa60785eab41c4c65dbdd9600bcb696290c873661b873ad61e5a4c4f205b7e66fdef2ae17c676cd12f
2023-03-06Expand scantxoutset help text to cover tr() and miniscriptGreg Sanders
2023-03-06wallet: single output groups filtering and grouping processfurszy
Optimizes coin selection by performing the "group outputs" procedure only once, outside the "attempt selection" process. Avoiding the repeated execution of the 'GroupOutputs' operation that occurs on each coin eligibility filters (up to 8 of them); then for every coin vector type plus one for all the coins together. This also let us not perform coin selection over coin eligibility filtered groups that don't add new elements. (because, if the previous round failed, and the subsequent one has the same coins, then this new round will fail again).
2023-03-06wallet: unify outputs grouping processfurszy
The 'GroupOutputs()' function performs the same calculations for only-positive and mixed groups, the only difference is that when we look for only-positive groups, we discard negative utxos. So, instead of wasting resources calling GroupOutputs() for positive-only first, then call it again to include the negative ones in the result, we can execute GroupOutputs() only once, including in the response both group types (positive-only and mixed).
2023-03-06test: coinselector_tests refactor, use CoinsResult instead of plain std::vectorfurszy
No functional changes. Only cosmetic changes to simplify the follow-up commit.
2023-03-06wallet: decouple outputs grouping process from each ChooseSelectionResultfurszy
Another step towards the single OutputGroups calculation goal
2023-03-06refactor: make OutputGroup::m_outputs field a vector of shared_ptrfurszy
Initial steps towards sharing COutput instances across all possible OutputGroups (instead of copying them time after time).
2023-03-06test: wallet, add coverage for outputs grouping processfurszy
The following scenarios are covered: 1) 10 UTXO with the same script: partial spends is enabled --> outputs must not be grouped. 2) 10 UTXO with the same script: partial spends disabled --> outputs must be grouped. 3) 20 UTXO, 10 one from scriptA + 10 from scriptB: a) if partial spends is enabled --> outputs must not be grouped. b) if partial spends is not enabled --> 2 output groups expected (one per script). 3) Try to add a negative output (value - fee < 0): a) if "positive_only" is enabled --> negative output must be skipped. b) if "positive_only" is disabled --> negative output must be added. 4) Try to add a non-eligible UTXO (due not fulfilling the min depth target for "not mine" UTXOs) --> it must not be added to any group 5) Try to add a non-eligible UTXO (due not fulfilling the min depth target for "mine" UTXOs) --> it must not be added to any group 6) Surpass the 'OUTPUT_GROUP_MAX_ENTRIES' size and verify that a second partial group gets created.
2023-03-04Merge bitcoin/bitcoin#27192: util: add missing include and fix function ↵fanquake
signature 8847ce44e0713350a6d3524f62eaeb10ba548bae util: add missing include and fix function signature (Cory Fields) Pull request description: ping hebasto Discovered while testing pre-compiled header support with CMake: https://github.com/theuni/bitcoin/commits/cmake-pch-poc. Compilation of that branch fails without this fix and succeeds with it. Similar to the fix in #27144. The problem of having a default argument in the definition was masked by the missing include. Using PCH forces that include, so we end up with the compiler error we should've been getting all along. ACKs for top commit: fanquake: ACK 8847ce44e0713350a6d3524f62eaeb10ba548bae Tree-SHA512: 5eb9a6691ee37cbc5033a48aedcbf5c93af3b234614ae14c3fcc858f1ee505f630ad68f8bbb69ffa280080c8d0f91451362cb3819290b741ce906b2b3224a622
2023-03-03util: add missing include and fix function signatureCory Fields
2023-03-03Fixed a couple of typos in comments to make linter happyhernanmarino
2023-03-03wallet: make OutputGroup "positive_only" filter explicitfurszy
And not hide it inside the `OutputGroup::Insert` method. This method does not return anything if insertion fails. We can know before calling `Insert` whether the coin will be accepted or not.
2023-03-03Merge bitcoin/bitcoin#26612: refactor: RPC: pass named argument value as ↵fanquake
string_view 545ff924ab6303ffabd91fdfc4f0a4962daf133c refactor: use string_view for RPC named argument values (stickies-v) 7727603e44f8f674e0fc8389e78047e2b56e6052 refactor: reduce unnecessary complexity in ParseNonRFCJSONValue (stickies-v) 1d02e599012721549d4c20b1b37fcc5ee7b961b6 test: add cases to JSON parsing (stickies-v) Pull request description: Inspired by MarcoFalke's [comment](https://github.com/bitcoin/bitcoin/pull/26506#discussion_r1036149426). Main purpose of this PR is to minimize copying (potentially large) RPC named arguments when calling `.substr()` by using `std::string_view` instead of `std::string`. Furthermore, cleans up the code by removing unnecessary complexity in `ParseNonRFCJSONValue()` (done first to avoid refactoring required to concatenate `string` and `string_view`), updates some naming and adds a few test cases. Should not introduce any behaviour change. ## Questions - ~Was there actually any merit to `ParseNonRFCJSONValue()` surrounding the value with brackets and then parsing it as an array? I don't see it, and the new approach doesn't fail any tests. Still a bit suspicious about it though.~ - Cleared up by https://github.com/bitcoin/bitcoin/pull/26506#pullrequestreview-1211984059 - If there are no objections to 7727603e44f8f674e0fc8389e78047e2b56e6052, I think we should follow up with a PR to rename `ParseNonRFCJSONValue()` to a local `Parse()` helper function (that throws if invalid), remove it from `client.h` and merge the test coverage we currently have on `ParseNonRFCJSONValue()` with the coverage we have on `UniValue::read()`. ACKs for top commit: ryanofsky: Code review ACK 545ff924ab6303ffabd91fdfc4f0a4962daf133c MarcoFalke: review ACK 545ff924ab6303ffabd91fdfc4f0a4962daf133c 📻 Tree-SHA512: b1c89fb010ac9c3054b023cac1acbba2a539a09cf39a7baffbd7f7571ee268d5a6d98701c7ac10d68a814526e8fd0fe96ac1d1fb072f272033e415b753f64a5c
2023-03-02Use steady clock in FlushStateToDiskMarcoFalke
2023-03-02Use steady clock in SeedStrengthen and FindBestImplementationMarcoFalke
2023-03-01Merge bitcoin/bitcoin#27169: Update translations for 25.0 soft translation ↵fanquake
string freeze 9172cc672ea99eac6d0210e8b793ca030c20e179 qt: Update translation source file (Hennadii Stepanov) 7b0cbf444d2ecdd2a6c0754990bf677ab1152dab qt: Bump Transifex slug for 25.x (Hennadii Stepanov) 369023d22def0917fd879f52f86cf6a4945498ca qt: Periodic translation updates from Transifex (Hennadii Stepanov) Pull request description: This PR follows our [Release Process](https://github.com/bitcoin/bitcoin/blob/master/doc/release-process.md). Required to open Transifex translations for 25.0 on 2023-03-01 as it's [planned](https://github.com/bitcoin/bitcoin/issues/26549). **NOTE.** Translations for the following languages for the latest 24.x Transifex resource have been effectively cancelled/damaged/vandalized: - German (de) by [nesbonk83](https://www.transifex.com/user/profile/nesbonk83/) on 2023-01-27 - Dutch (nl) by [bram00767](https://www.transifex.com/user/profile/bram00767/) on 2022-12-17 - Spanish, Mexico (es_MX) by [VCFNFT](https://www.transifex.com/user/profile/VCFNFT/) on 2022-08-08 The first commit ignores changes to translations mentioned above. ACKs for top commit: jarolrod: ACK 9172cc672ea99eac6d0210e8b793ca030c20e179 Tree-SHA512: 85641facecd11526bbcde934b43629aba1b856c4f97272a956c2ce194af8a1723325a160a0a518fc052af9373f853204848b58d3c0a3bea09788fccfc5d9f557
2023-03-01doc: DummySignInput mention external signerSjors Provoost
2023-02-28Deduplicate bitcoind and bitcoin-qt init codeRyan Ofsky
Add common InitConfig function to deduplicate bitcoind and bitcoin-qt code reading config files and creating the datadir. There are a few minor changes in behavior: - In bitcoin-qt, when there is a problem reading the configuration file, the GUI error text has changed from "Error: Cannot parse configuration file:" to "Error reading configuration file:" to be consistent with bitcoind. - In bitcoind, when there is a problem reading the settings.json file, the error text has changed from "Failed loading settings file" to "Settings file could not be read" to be consistent with bitcoin-qt. - In bitcoind, when there is a problem writing the settings.json file, the error text has changed from "Failed saving settings file" to "Settings file could not be written" to be consistent with bitcoin-qt. - In bitcoin-qt, if there datadir is not accessible (e.g. no permission to read), there is an normal error dialog showing "Error: filesystem error: status: Permission denied [.../settings.json]", instead of an uncaught exception
2023-02-28Merge bitcoin/bitcoin#27165: Make miniscript_{stable,smart} fuzzers avoid ↵fanquake
too large scripts 56e37e71a2538a240cc360678aeb752d17bd8f45 Make miniscript fuzzers avoid script size limit (Pieter Wuille) bcec5ab4ff1039c0c309dbbb9953adbd0a4f3e88 Make miniscript fuzzers avoid ops limit (Pieter Wuille) 213fffa5138229eac2d4a9eda0f643fe90870378 Enforce type consistency in miniscript_stable fuzz test (Pieter Wuille) e1f30414c6b9434048e089ccc3ec4f475f980c60 Simplify miniscript fuzzer NodeInfo struct (Pieter Wuille) 5abb0f5ac37e8a17072d5989a025227035fdc7e6 Do base type propagation in miniscript_stable fuzzer (Pieter Wuille) Pull request description: This adds a number of improvements to the miniscript fuzzers that all amount to rejecting invalid or overly big miniscripts early on: * Base type propagation in the miniscript_stable fuzzers prevents constructing a large portion of miniscripts that would be illegal, with just a little bit of type logic in the fuzzer. The fuzzer input format is unchanged. * Ops and script size tracking in GenNode means that too-large scripts (either due to script size limit or ops limit) will be detected on the fly during fuzz input processing, before actually constructing the scripts. Closes #27147. ACKs for top commit: darosior: re-ACK 56e37e71a2 dergoegge: tACK 56e37e71a2538a240cc360678aeb752d17bd8f45 Tree-SHA512: 245584adf9a6644a35fe103bc81b619e5b4f5d467571a761b5809d08b1dec48f7ceaf4d8791ccd8208b45c6b309d2ccca23b3d1ec5399df76cd5bf88f2263280
2023-02-28Add InitError(error, details) overloadRyan Ofsky
This is only used in the current PR to avoid ugly `strprintf(Untranslated("%s:\n%s"), str, MakeUnorderedList(details)` boilerplate in init code. But in the future the function could be extended and more widely used to include more details in GUI error messages or display them in a more readable way, see code comment.
2023-02-28Extend bilingual_str support for tinyformatRyan Ofsky
Previous bilingual_str tinyformat::format accepted bilingual format strings, but not bilingual arguments. Extend it to accept both. This is useful when embedding one translated string inside another translated string, for example: `strprintf(_("Error: %s"), message)` which would fail previously if `message` was a bilingual_str.
2023-02-28scripted-diff: Remove double newlines after some init errorsRyan Ofsky
Some InitError calls had trailing \n characters, causing double newlines in error output. After this change InitError calls consistently output one newline instead of two. Appearance of messages in the GUI does not seem to be affected. Can be tested with: src/bitcoind -regtest -datadir=noexist src/qt/bitcoin-qt -regtest -datadir=noexist -BEGIN VERIFY SCRIPT- git grep -l InitError src/ | xargs sed -i 's/\(InitError(.*\)\\n"/\1"/' -END VERIFY SCRIPT-
2023-02-28Merge bitcoin/bitcoin#23897: refactor: Move calculation logic out from ↵glozow
`CheckSequenceLocksAtTip()` 75db62ba4cae048e742ca02dc6a52b3b3d6727de refactor: Move calculation logic out from `CheckSequenceLocksAtTip()` (Hennadii Stepanov) 3bc434f4590758db673e1bd4ebf1906ea632f593 refactor: Add `CalculateLockPointsAtTip()` function (Hennadii Stepanov) Pull request description: This PR is follow up for bitcoin/bitcoin#22677 and bitcoin/bitcoin#23683. On master (013daed9acca1b723f599d63ab36b9c2a5c60e5f) it is not obvious that `CheckSequenceLocksAtTip()` function can modify its `LockPoints* lp` parameter which leads to https://github.com/bitcoin/bitcoin/pull/22677#discussion_r762040101. This PR: - separates the lockpoint calculate logic from `CheckSequenceLocksAtTip()` function into a new `CalculateLockPointsAtTip()` one - cleans up the `CheckSequenceLocksAtTip()` function interface - makes code easier to reason about (hopefully) ACKs for top commit: achow101: ACK 75db62ba4cae048e742ca02dc6a52b3b3d6727de stickies-v: re-ACK 75db62b Tree-SHA512: 072c3fd9cd1e1b0e0bfc8960a67b01c80a9f16d6778f374b6944ade03a020415ce8b8ab2593b0f5e787059c8cf90af798290b4c826785d41955092f6e12e7486
2023-02-28Merge bitcoin/bitcoin#27170: refactor: Stop using gArgs global in system.cppAndrew Chow
9a9d5da11fa6033f82dcf8e2298aee29587f5396 refactor: Stop using gArgs global in system.cpp (Ryan Ofsky) b20b34f5b33230fe253c81008496bd9b13fd6ecf refactor: Use new GetConfigFilePath function (Ryan Ofsky) Pull request description: Most of the code in `util/system.cpp` that was hardcoded to use the global `ArgsManager` instance `gArgs` has been changed to stop using it (for example in https://github.com/bitcoin/bitcoin/pull/20092). But a few hardcoded references to `gArgs` remain. This commit removes the last ones so these functions aren't reading or writing global state. Noticed these `gArgs` references while reviewing #27073 ACKs for top commit: achow101: ACK 9a9d5da11fa6033f82dcf8e2298aee29587f5396 stickies-v: ACK 9a9d5da11 willcl-ark: tACK 9a9d5da11 Tree-SHA512: 2c74b0d5fc83e9ed2ec6562eb26ec735512f75db8876a11a5d5f04e6cdbe0cd8beec19894091aa2cbf29319194d2429ccbf8036f5520ecc394f6fe89a0079a7b
2023-02-28Merge bitcoin/bitcoin#27176: docs: GetDataDirNet and GetDataDirBase don't ↵fanquake
create datadir fb0dbe94233ec509570cbba3118cf62d8e60842b docs: GetDataDirNet and GetDataDirBase don't create datadir (stickies-v) Pull request description: Since #27073, the behaviour of `GetDataDir()` [changed](https://github.com/bitcoin/bitcoin/pull/27073/files#diff-19427b0dd1a791adc728c82e88f267751ba4f1c751e19262cac03cccd2822216L435-L443) to only return the datadir path, but not create it if non-existent. This also changed the behaviour of `GetDataDirNet()` and `GetDataDirBase()` but the docs do not yet reflect that. ACKs for top commit: TheCharlatan: ACK fb0dbe94233ec509570cbba3118cf62d8e60842b theStack: ACK fb0dbe94233ec509570cbba3118cf62d8e60842b willcl-ark: ACK fb0dbe942 Tree-SHA512: 3f10f4871df59882f3649c6d3b2362cae2f8a01ad0bd0c636c5608b0d177d279a2e8712930b819d6d3912e91fa6447b9e54507c33d8afe427f7f39002b013bfb
2023-02-28Merge bitcoin/bitcoin#26533: prune: scan and unlink already pruned block ↵Andrew Chow
files on startup 3141eab9c669488a2e7fef5f60d356ac92294922 test: add functional test for ScanAndUnlinkAlreadyPrunedFiles (Andrew Toth) e252909e561e47d75cb3a892657662a139f6532c test: add unit test for ScanAndUnlinkAlreadyPrunedFiles (Andrew Toth) 77557dda4a123515d0fa2a545ee21d7c43a66988 prune: scan and unlink already pruned block files on startup (Andrew Toth) Pull request description: There are a few cases where we can mark a block and undo file as pruned in our block index, but not actually remove the files from disk. 1. If we call `FindFilesToPrune` or `FindFilesToPruneManual` and crash before `UnlinkPrunedFiles`. 2. If on Windows there is an open file handle to the file somewhere else when calling `fs::remove` in `UnlinkPrunedFiles` (https://en.cppreference.com/w/cpp/filesystem/remove, https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-deletefilew#remarks). This could be from another process, or if we are calling `ReadBlockFromDisk`/`ReadRawBlockFromDisk` without having a lock on `cs_main` (which has been allowed since https://github.com/bitcoin/bitcoin/commit/ccd8ef65f93ed82a87cee634660bed3ac17d9eb5). This PR mitigates this by scanning all pruned block files on startup after `LoadBlockIndexDB` and unlinking them again. ACKs for top commit: achow101: ACK 3141eab9c669488a2e7fef5f60d356ac92294922 pablomartin4btc: re-ACK with added functional test 3141eab9c669488a2e7fef5f60d356ac92294922. furszy: Code review ACK 3141eab9 theStack: Code-review ACK 3141eab9c669488a2e7fef5f60d356ac92294922 Tree-SHA512: 6c73bc57838ad1b7e5d441af3c4d6bf4c61c4382e2b86485e57fbb74a61240710c0ceeceb8b4834e610ecfa3175c6955c81ea4b2285fee11ca6383f472979d8d
2023-02-28Make miniscript fuzzers avoid script size limitPieter Wuille
Use the same technique as is using in the FromString miniscript parser to predict the final script size of the miniscript being generated in the miniscript_stable and miniscript_smart fuzzers (by counting every unexplored sub node as 1 script byte, which is possible because every leaf node always adds at least 1 byte). This allows bailing out early if the script being generated would exceed the maximum allowed size (before actually constructing the miniscript, as that may happen only significantly later potentially). Also add a self-check to make sure this predicted script size matches that of generated scripts.
2023-02-28Make miniscript fuzzers avoid ops limitPieter Wuille
Keep track of the total number of ops the constructed script will have during miniscript_stable and miniscript_smart fuzzers' GenNode, so it can abort early if the 201 ops limit would be exceeded. Also add a self-check that the final constructed node has the predicted ops size limit, so we know the fuzzer's logic for keeping track of this is correct.
2023-02-28Enforce type consistency in miniscript_stable fuzz testPieter Wuille
Add a self-check to the fuzzer that the constructed types match the expected types in the miniscript_stable fuzzer too.
2023-02-28Simplify miniscript fuzzer NodeInfo structPieter Wuille
Since we now keep track of all expected child node types (even if rudimentary) in both miniscript_stable and miniscript_smart fuzzers, there is no need anymore for the former shortcut NodeInfo constructors without sub types.
2023-02-28Do base type propagation in miniscript_stable fuzzerPieter Wuille
Keep track of which base type (B, K, V, or W) is desired in the miniscript_stable ConsumeStableNode function. This allows aborting early if the constructed node won't have the right type. Note that this does not change the fuzzer format; the meaning of inputs in ConsumeStableNode is unmodified. The only change is that often the fuzzer will abort early. The direct motivation is preventing recursing v: wrappers, which are the only fragment type that does not otherwise increase the overall minimum possible script size. In a later commit this will be exploited to prevent overly-large scripts from being constructed.
2023-02-28docs: GetDataDirNet and GetDataDirBase don't create datadirstickies-v
Since #27073, the behaviour of GetDataDir changed to only return the datadir path, but not create it. This also changed the behaviour of GetDataDirNet and GetDataDirBase but the docs do not yet reflect that.
2023-02-28Merge bitcoin/bitcoin#27157: init: Return ChainstateLoadStatus::INTERRUPTED ↵fanquake
when verification was interrupted. c5825e14f8999a8c5f5121027af9e07ac51ab42e doc: add explanation for fail_on_insufficient_dbcache (Ryan Ofsky) 7dff7da4f5eafa89546565a63362e57516e4064e init: Return more fitting ChainStateLoadStatus if verification was interrupted (Martin Zumsande) Pull request description: This addresses two outstanding comments by ryanofsky from #25574: * return `ChainstateLoadStatus::INTERRUPTED` instead of `ChainstateLoadStatus::SUCCESS` if verification was stopped by an interrupt. This would coincide with straightforward expectation, and it avoids a misleading [log entry](https://github.com/mzumsande/bitcoin/blob/c5825e14f8999a8c5f5121027af9e07ac51ab42e/src/init.cpp#L1526) in `init` for the block index load time (because that would include the verificiation, which didn't complete). It shouldn't affect node behavior otherwise because the shutdown signal would be caught in init anyway. In test, this would lead to an assert ([link](https://github.com/mzumsande/bitcoin/blob/c5825e14f8999a8c5f5121027af9e07ac51ab42e/src/test/util/setup_common.cpp#L230)), which also makes more sense because benign interrupts are not expected there during init. This can be tested by setting a large value for `-checkblocks`, interrupting the node during block verification and observing the log. https://github.com/bitcoin/bitcoin/pull/25574#discussion_r1110050930 * add documentation for `require_full_verification` https://github.com/bitcoin/bitcoin/pull/25574#discussion_r1110031541 ACKs for top commit: MarcoFalke: thanks lgtm ACK c5825e14f8999a8c5f5121027af9e07ac51ab42e Tree-SHA512: ca1c71a1b046d30083337dd9ef6d52e66fa1ac8c4ecd807716e4aa6a894179a81df41caee916fa30997fd6e0b284412a3c8f2919d19c29d826fb580ffb89fd73
2023-02-27refactor: Stop using gArgs global in system.cppRyan Ofsky
Most of the code in util/system.cpp that was hardcoded to use the global ArgsManager instance `gArgs` has been changed to work with explicit ArgsManager instances (for example in https://github.com/bitcoin/bitcoin/pull/20092). But a few hardcoded references to `gArgs` remain. This commit removes the last ones so these functions aren't reading or writing global state.
generated by cgit v1.2.3 (git 2.26.2) at 2025-01-23 20:46:04 +0000