aboutsummaryrefslogtreecommitdiff
path: root/src
AgeCommit message (Collapse)Author
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-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-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.
2023-02-27refactor: Use new GetConfigFilePath functionRyan Ofsky
New function was introduced by willcl-ark <will@256k1.dev> in commit 56e370fbb9413260723c598048392219b1055ad0 from https://github.com/bitcoin/bitcoin/pull/27073 and removes some duplicate code.
2023-02-27Merge bitcoin/bitcoin#26032: wallet: skip R-value signature grinding for ↵Andrew Chow
external signers 807de2cebdad960c2b52185528ca8960ec694f49 wallet: skip R-value grinding for external signers (Sjors Provoost) 72b763e4521e674990da5dd1999b7a8c7bd3ba8c wallet: annotate bools in descriptor SPKM FillPSBT() (Sjors Provoost) Pull request description: When producing a dummy signature for the purpose of estimating the transaction fee, do not assume an external signer performs R-value grinding on the signature. In particular, this avoids a scenario where the fee rate is 1 sat / vbyte and a transaction with a 72 byte signature is not accepted into our mempool. Suggested testing: 1. On master, launch with `-signet` and create an external signer wallet using e.g. a Trezor and HWI, see [guide](https://github.com/bitcoin/bitcoin/blob/master/doc/external-signer.md#example-usage) (with the GUI it should "just work" once you have the HWI path configured). 2. Create a few addresses and fund them from the faucet: https://signet.bc-2.jp/ (wait for confirmation) 3. Create another address, and now send the entire wallet to it, set the fee to 1 sat/byte 4. Most likely this transaction never gets broadcast and you won't see it on the [signet explorer](https://explorer.bc-2.jp) 5. With this PR, try again. 6. Check the explorer and inspect the transaction. Each input witness starts with either `30440220` (R has 32 bytes) or `30440221` (R has 33 bytes). See this explainer for [DER encoding](https://bitcoin.stackexchange.com/questions/92680/what-are-the-der-signature-and-sec-format). Fixes #26030 ACKs for top commit: S3RK: ACK 807de2cebdad960c2b52185528ca8960ec694f49 achow101: ACK 807de2cebdad960c2b52185528ca8960ec694f49 furszy: ACK 807de2ce ishaanam: utACK 807de2cebdad960c2b52185528ca8960ec694f49 Tree-SHA512: 64f626a3030ef0ab1e43af86d8fba113151512561baf425e6e5182af53df3a64fa9e85c7f67bf4ed15b5ad6e5d5afc7fbba8b6e1f3bad388e48db51cb9446074
2023-02-27Merge bitcoin/bitcoin#27146: Fix various libbitcoinkernel DLL build problemsfanquake
5da7c0b3e34626ca57d1f0773db61e7d8351d8c7 build: allow libitcoinkernel dll builds now that exports are fixed (Cory Fields) 130490aef95e4b352a47dfbd55df855db56760c7 build: always build bitcoin-chainstate against static libbitcoinkernel (Cory Fields) 545a74ef320d0abb1e45f88ed857ccee951e81c3 build: fix bitcoin-chainstate when libbitcoinkernel is static (Cory Fields) 9c253d2398005d852cab77c4456bc1f44831a16b build: don't define DLL_EXPORT for windows (Cory Fields) Pull request description: Fixes #25008. Fixes #19772. 1. Fixup the build defines so that exports are clean. 2. Work around a libtool issue wrt dependency calculation 3. Simplify everything by only ever building in-tree bitcoin-chainstate against a static libbitcoinkernel 4. Remove Windows-only hack that disabled dll creation ACKs for top commit: TheCharlatan: ACK 5da7c0b3e34626ca57d1f0773db61e7d8351d8c7 Tree-SHA512: 61bab457e13842946387240da703d313509af30d4ca3371a19a26a5ef1716e4d7107b09567323041b549ab1fc97a064aa1d6992406936ab9c491a616bc7f4e7f
2023-02-27Merge bitcoin/bitcoin#25227: Handle invalid hex encoding in ParseHexfanquake
faab273e060d27e166b5fb7fe7692614ec9e5c76 util: Return empty vector on invalid hex encoding (MarcoFalke) fa3549a77bf6a15d8309d36056237f3126baf721 test: Add hex parse unit tests (MarcoFalke) Pull request description: Seems a bit confusing to happily accept random bytes and pretend they are hex encoded strings. ACKs for top commit: stickies-v: re-ACK faab273e060d27e166b5fb7fe7692614ec9e5c76 Tree-SHA512: a808135f744f50aece03d4bf5a71481c7bdca1fcdd0d5b113abdb0c8b382bf81cafee6d17c239041fb49b59f4e19970f24a475378e7f711c3a47d6438de2bdab
2023-02-27qt: Update translation source fileHennadii Stepanov
The diff is produced by running `make -C src translate`.
2023-02-27qt: Periodic translation updates from TransifexHennadii Stepanov
Pulled from 24.x resource. Changes to "de", "es_MX" and "nl" have been ignored as they remove translations altogether.
2023-02-27util: Return empty vector on invalid hex encodingMarcoFalke
2023-02-27test: Add hex parse unit testsMarcoFalke
2023-02-24doc: add explanation for fail_on_insufficient_dbcacheRyan Ofsky
2023-02-24init: Return more fitting ChainStateLoadStatus if verification was interruptedMartin Zumsande
This also avoids a misleading block index loadtime log entry in init. Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
2023-02-23util: Remove duplicate includeAndrew Chow
Duplicate `#include <utility>` is upsetting the linter.
2023-02-23Merge bitcoin/bitcoin#27073: Convert ArgsManager::GetDataDir to a read-only ↵Andrew Chow
function 64c105442ce8c10900ea6fbecdbcfebe42f2d387 util: make GetDataDir read-only & create datadir.. (willcl-ark) 56e370fbb9413260723c598048392219b1055ad0 util: add ArgsManager datadir helper functions (willcl-ark) Pull request description: Fixes #20070 Currently `ArgsManager::GetDataDir()` ensures it will always return a datadir by creating one if necessary. The function is shared between `bitcoind` `bitcoin-qt` and `bitcoin-cli` which results in the undesirable behaviour described in #20070. This PR splits out the part of the function which creates directories and adds it as a standalone function, only called as part of `bitcoind` and `bitcoin-qt` init, but not `bitcoin-cli`. `ReadConfigFiles`' behavior is changed to use the absolute path of the config file in error and warning messages instead of a relative path. This was inadvertantly the form being tested [here](https://github.com/bitcoin/bitcoin/blob/73966f75f67fb797163f0a766292a79d4b2c1b70/test/functional/feature_config_args.py#L287), whilst we were _not_ testing that a relative path was returned by the message even though we passed a relative path in as argument. ACKs for top commit: achow101: ACK 64c105442ce8c10900ea6fbecdbcfebe42f2d387 hebasto: re-ACK 64c105442ce8c10900ea6fbecdbcfebe42f2d387, only comments have been adjusted as requsted since my previous [review](https://github.com/bitcoin/bitcoin/pull/27073#pullrequestreview-1307435890). TheCharlatan: Re-ACK 64c105442ce8c10900ea6fbecdbcfebe42f2d387 ryanofsky: Code review ACK 64c105442ce8c10900ea6fbecdbcfebe42f2d387. Only comment changes since last review Tree-SHA512: b129501346071ad62551c9714492b21536d0558a94117d97218e255ef4e948d00df899a4bc2788faea27d3b1f20fc6136ef9d03e6a08498d926d9ad8688d6c96
2023-02-23Merge bitcoin/bitcoin#16195: util: Use void* throughout support/lockedpool.hAndrew Chow
f36d1d5b8934aac60d3097047ecedeb58bae2185 Use void* throughout support/lockedpool.h (Jeffrey Czyz) Pull request description: Replace uses of char* with void* in Arena's member variables. Instead, cast to char* where needed in the implementation. Certain compiler environments disallow std::hash<char*> specializations to prevent hashing the pointer's value instead of the string contents. Thus, compilation fails when std::unordered_map is keyed by char*. Explicitly using void* is a workaround in such environments. For consistency, void* is used throughout all member variables similarly to the public interface. Changes to this code are covered by src/test/allocator_tests.cpp. ACKs for top commit: achow101: ACK f36d1d5b8934aac60d3097047ecedeb58bae2185 theStack: Code-review ACK f36d1d5b8934aac60d3097047ecedeb58bae2185 jonatack: ACK f36d1d5b8934aac60d3097047ecedeb58bae2185 review, debug build, unit tests, checked clang 15 raises "error: arithmetic on a pointer to void" without the conversions here from the generic void* pointer back to char* Tree-SHA512: f9074e6d29ef78c795a512a6e00e9b591e2ff34165d09b73eae9eef25098c59e543c194346fcd4e83185a39c430d43744b6f7f9d1728a132843c67bd27ea5189
2023-02-23Merge bitcoin/bitcoin#25943: rpc: Add a parameter to sendrawtransaction ↵Andrew Chow
which sets a maximum value for unspendable outputs. 7013da07fbcddb04abae9759767a9419ab90444c Add release note for PR#25943 (David Gumberg) 04f270b4358417fc2827b9f91717816062b1864e Add test for unspendable transactions and parameter 'maxburnamount' to sendrawtransaction. (David Gumberg) Pull request description: This PR adds a user configurable, zero by default parameter — `maxburnamount` — to `sendrawtransaction`. This PR makes bitcoin core reject transactions that contain unspendable outputs which exceed `maxburnamount`. closes #25899. As a result of this PR, `sendrawtransaction` will by default block 3 kinds of transactions: 1. Those that begin with `OP_RETURN` - (datacarriers) 2. Those whose lengths exceed the script limit. 3. Those that contain invalid opcodes. The user is able to configure a `maxburnamount` that will override this check and allow a user to send a potentially unspendable output into the mempool. I see two legitimate use cases for this override: 1. Users that deliberately use `OP_RETURN` for datacarrier transactions that embed data into the blockchain. 2. Users that refuse to update, or are unable to update their bitcoin core client would be able to make use of new opcodes that their client doesn't know about. ACKs for top commit: glozow: reACK 7013da07fbcddb04abae9759767a9419ab90444c achow101: re-ACK 7013da07fbcddb04abae9759767a9419ab90444c Tree-SHA512: f786a796fb71a587d30313c96717fdf47e1106ab4ee0c16d713695e6c31ed6f6732dff6cbc91ca9841d66232166eb058f96028028e75c1507324426309ee4525
2023-02-23wallet: skip R-value grinding for external signersSjors Provoost
When producing a dummy signature for the purpose of estimating the transaction fee, do not assume an external signer performs R-value grinding on the signature. In particular, this avoids a scenario where the fee rate is 1 sat / vbyte and a transaction with a 72 byte signature is not accepted into our mempool. This commit also drops the nullptr default for CCoinControl arguments for functions that it touches. This is because having a boolean argument right next to an optional pointer is error prone. Co-Authored-By: S3RK <1466284+S3RK@users.noreply.github.com>
2023-02-23wallet: annotate bools in descriptor SPKM FillPSBT()Sjors Provoost
2023-02-23util: make GetDataDir read-only & create datadir..willcl-ark
.. only in bitcoind and bitcoin-qt This changes behaviour of GetConfigFilePath which now always returns the absolute path of the provided -conf argument.
2023-02-23util: add ArgsManager datadir helper functionswillcl-ark
* Add ArgsManager::EnsureDataDir() Creates data directory if it doesn't exist * Add ArgsManager::GetConfigFilePath() Return config file path (read-only)
2023-02-22build: allow libitcoinkernel dll builds now that exports are fixedCory Fields
Symbol visibility issues are not actually fixed yet because we have not yet defined an api and exported symbols, but everything is now in place for that.
2023-02-22build: always build bitcoin-chainstate against static libbitcoinkernelCory Fields
Building binaries against our uninstalled shared libs is impractical. Instead, to test them, we'll need to work on a runtime shared-lib execution harness.
2023-02-22build: fix bitcoin-chainstate when libbitcoinkernel is staticCory Fields
Libtool is unable to calculate dependencies correctly so give it some help.
2023-02-22Merge bitcoin/bitcoin#25574: validation: Improve error handling when ↵Andrew Chow
VerifyDB dosn't finish successfully 0af16e7134459e0820ab95d751093876c1ec4c6d doc: add release note for #25574 (Martin Zumsande) 57ef2a4812f443b2d734f43cebf3ef5038da83f2 validation: report if pruning prevents completion of verification (Martin Zumsande) 0c7785bb2540b69564104767d38342704230cbc2 init, validation: Improve handling if VerifyDB() fails due to insufficient dbcache (Martin Zumsande) d6f781f1cfcbc2c2ad5ee289a0642ed00386d013 validation: return VerifyDBResult::INTERRUPTED if verification was interrupted (Martin Zumsande) 6360b5302d2675788de5c4a28ea77d823f6d809e validation: Change return value of VerifyDB to enum type (Martin Zumsande) Pull request description: `VerifyDB()` can fail to complete due to insufficient dbcache at the level 3 checks. This PR improves the error handling in this case in the following ways: - The rpc `-verifychain` now returns false if the check can't be completed due to insufficient cache - During init, we only log a warning if the default values for `-checkblocks` and `-checklevel` are taken and the check doesn't complete. However, if the user actively specifies one of these args, we return with an InitError if we can't complete the check. This PR also changes `-verifychain` RPC to return `false` if the verification didn't finish due to missing block data (pruning) or due to being interrupted by the node being shutdown. Previously, this PR also included a fix for a possible assert during verification - this was done in #27009 (now merged). ACKs for top commit: achow101: ACK 0af16e7134459e0820ab95d751093876c1ec4c6d ryanofsky: Code review ACK 0af16e7134459e0820ab95d751093876c1ec4c6d. Only small suggested changes since the last review, like renaming some of the enum values. I did leave more suggestions, but they are not very important and could be followups john-moffett: ACK 0af16e7134459e0820ab95d751093876c1ec4c6d MarcoFalke: lgtm re-ACK 0af16e7134459e0820ab95d751093876c1ec4c6d 🎚 Tree-SHA512: 84b4f767cf9bfbafef362312757c9bf765b41ae3977f4ece840e40c52a2266b1457832df0cdf70440be0aac2168d9b58fc817238630b0b6812f3836ca950bc0e
2023-02-22Merge bitcoin/bitcoin#27068: wallet: SecureString to allow null charactersAndrew Chow
4bbf5ddd44bde15b328be131922123eaa3212a7e Detailed error message for passphrases with null chars (John Moffett) b4bdabc2238750a1f6e72cb1403f8b770fc4f365 doc: Release notes for 27068 (John Moffett) 4b1205ba37d6737722d2087696b1a054a852286a Test case for passphrases with null characters (John Moffett) 00a0861181cc7f4771ac2690ca6be5731c30b005 Pass all characters to SecureString including nulls (John Moffett) Pull request description: `SecureString` is a `std::string` specialization with a secure allocator. However, in practice it's treated like a C- string (no explicit length and null-terminated). This can cause unexpected and potentially insecure behavior. For instance, if a user enters a passphrase with embedded null characters (which is possible through Qt and the JSON-RPC), it will ignore any characters after the first null, potentially giving the user a false sense of security. Instead of assigning to `SecureString` via `std::string::c_str()`, assign it via a `std::string_view` of the original. This explicitly captures the size and still doesn't make any extraneous copies in memory. Note to reviewers, the following all compile identically in recent `GCC` (x86-64 and ARM64) with `-O2` (and `-std=c++17`): ```C++ std::string orig_string; std::cin >> orig_string; SecureString s; s.reserve(100); // The following all compile identically s = orig_string; s = std::string_view{orig_string}; s.assign(std::string_view{orig_string}); s.assign(orig_string.data(), orig_string.size()); ``` So it's largely a matter of preference. However, one thing to keep in mind is that we want to avoid making unnecessary copies of any sensitive data in memory. Something like `SecureString s{orig_string};` is still invalid and probably unwanted in our case, since it'd get treated as a short string and optimized away from the secure allocator. I presume that's the reason for the `reserve()` calls. Fixes #27067. ACKs for top commit: achow101: re-ACK 4bbf5ddd44bde15b328be131922123eaa3212a7e stickies-v: re-ACK [4bbf5dd](https://github.com/bitcoin/bitcoin/commit/4bbf5ddd44bde15b328be131922123eaa3212a7e) furszy: utACK 4bbf5ddd Tree-SHA512: 47a96905a82ca674b18076a20a388123beedf70e9de73e42574ea68afbb434734e56021835dd9b148cdbf61709926b487cc95e9021d9bc534a7c93b3e143d2f7
2023-02-22Merge bitcoin/bitcoin#27144: kernel: add missing includefanquake
49d01f32c9cc4de4fcd0d1f235e2c62e4acfc7a2 kernel: add missing include (Cory Fields) Pull request description: This syncs the cs_main definition/declaration. Noticed when experimenting with the external visibility of `cs_main`. Specifically, this is needed for the following to work as intended: ```c++ __attribute__ ((visibility ("default"))) extern RecursiveMutex cs_main; ``` ACKs for top commit: fanquake: ACK 49d01f32c9cc4de4fcd0d1f235e2c62e4acfc7a2 Tree-SHA512: ea0dbcf81959566f949d76c7dcd1e33de53e613519500c863bfb0ac8209665b1c12cff2daa7890d03b76debc4d046339ee7b3231adb71b128e9d5a8fa3132b6c
2023-02-22Merge bitcoin/bitcoin#26837: I2P network optimizationsfanquake
3c1de032de01e551992975eb374465300a655f44 i2p: use consistent number of tunnels with i2pd and Java I2P (Vasil Dimov) 801b405f85b413631427c2d8cc1f8447309ea5d8 i2p: lower the number of tunnels for transient sessions (Vasil Dimov) b906b64eb76643feaede1da5987a0c4d466c581b i2p: reuse created I2P sessions if not used (Vasil Dimov) Pull request description: * Reuse an I2P transient session instead of discarding it if we failed to connect to the desired peer. This means we never used the generated address (destination), whose creation is not cheap. This does not mean that we will use the same address for more than one peer. * Lower the number of tunnels for transient sessions. * Explicitly specify the number of tunnels for persistent sessions instead of relying on the defaults which differ between I2P routers. This way we get consistent behavior with all routers. Alleviates: https://github.com/bitcoin/bitcoin/issues/26754 (I have not tested this with i2pd, yet) ACKs for top commit: jonatack: ACK 3c1de032de01e551992975eb374465300a655f44 mzumsande: Light ACK 3c1de032de01e551992975eb374465300a655f44 Tree-SHA512: 477b4b9a5755e6a9a46bc0f7b268fa419dff4414e25445c750ae913f7552d9e2313f2aca4e3b70067b8390c2d0c2d68ec459f331765e939fc84139e454031cd4
2023-02-22Merge bitcoin/bitcoin#26595: wallet: be able to specify a wallet name and ↵fanquake
passphrase to migratewallet 9486509be65f09174a0cb50a337cac58a0c09de4 wallet, rpc: Update migratewallet help text for encrypted wallets (Andrew Chow) aaf02b5721a8b5d3d9280dc3146fa5e44ea671b6 tests: Tests for migrating wallets by name, and providing passphrase (Andrew Chow) 7fd125b27d48e410509f3009e2eb9fa5cd6729dd wallet: Be able to unlock the wallet for migration (Andrew Chow) 6bdbc5ff590de18dfb47c31190baad879f68fef7 rpc: Allow users to specify wallet name for migratewallet (Andrew Chow) dbfa34540372033d95036a02b7025ddd33f540aa wallet: Allow MigrateLegacyToDescriptor to take a wallet name (Andrew Chow) Pull request description: `migratewallet` currently operates on wallets that are already loaded, however this is not necessarily required, and in the future, not possible once the legacy wallet is removed. So we need to also be able to give the wallet name to migrate. Additionally, the passphrase is required when migrating a wallet. Since a wallet may not be loaded when we migrate, and as we currently unload wallets when migrating, we need the passphrase to be given to `migratewallet` in order to migrate encrypted wallets. Fixes #27048 ACKs for top commit: john-moffett: reACK 9486509be65f09174a0cb50a337cac58a0c09de4 pinheadmz: ACK 9486509be65f09174a0cb50a337cac58a0c09de4 furszy: ACK 9486509b Tree-SHA512: 35e2ba69a148e129a41e20d7fb99c4cab7947b1b7e7c362f4fd06ff8ac6e79e476e07207e063ba5b80e1a33e2343f4b4f1d72d7930ce80c34571c130d2f5cff4
2023-02-22kernel: add missing includeCory Fields
This syncs the cs_main definition/declaration. Noticed when experimenting with the external visibility of cs_main.
2023-02-22Merge bitcoin/bitcoin#27117: fuzz: avoid redundant dup key checks when ↵fanquake
creating Miniscript nodes c1b7bd047f47dcd3eb6897adfaf9a55594deff5d fuzz: avoid redundant dup key checks when creating Miniscript nodes (Antoine Poinsot) Pull request description: I thought i had done that already in #24149, but it must have slipped through the rebase. It's a 2x speed improvement against the existing corpora and will probably be much more as we extend them with larger nodes. ACKs for top commit: sipa: ACK c1b7bd047f47dcd3eb6897adfaf9a55594deff5d Tree-SHA512: 9e6ceb6254183964b6c5538e21ba6321df95a68acb343a15a6ecfef5c51a1980d2627df5aeef9aef1db41656e18cc4f3bc96e6f24314d12fa60368b04a350001
2023-02-21wallet, rpc: Update migratewallet help text for encrypted walletsAndrew Chow
2023-02-21Detailed error message for passphrases with null charsJohn Moffett
Since users may have thought the null characters in their passphrases were actually evaluated prior to this change, they may be surprised to learn that their passphrases no longer work. Give them feedback to explain how to remedy the issue.
2023-02-21Pass all characters to SecureString including nullsJohn Moffett
`SecureString` is a `std::string` specialization with a secure allocator. However, it's treated like a C- string (no explicit length and null-terminated). This can cause unexpected behavior. For instance, if a user enters a passphrase with an embedded null character (which is possible through Qt and the JSON-RPC), it will ignore any characters after the null, giving the user a false sense of security. Instead of assigning `SecureString` via `std::string::c_str()`, assign it via a `std::string_view` of the original. This explicitly captures the size and doesn't make any extraneous copies in memory.
2023-02-21Merge bitcoin/bitcoin#26347: wallet: ensure the wallet is unlocked when ↵Andrew Chow
needed for rescanning 6a5b348f2e526f048d0b448b01f6c4ab608569af test: test rescanning encrypted wallets (ishaanam) 493b813e171a389a8b6750b4f2e42e8363a0267e wallet: ensure that the passphrase is not deleted from memory when being used to rescan (ishaanam) 66a86ebabb26a055ca92af846bfa39dbd2f9f722 wallet: keep track of when the passphrase is needed when rescanning (ishaanam) Pull request description: Wallet passphrases are needed to top up the keypool of encrypted wallets during a rescan. The following RPCs need the passphrase when rescanning: - `importdescriptors` - `rescanblockchain` The following RPCs use the information about whether or not the passphrase is being used to ensure that full rescans are able to take place (meaning the following RPCs should not be able to run if a rescan requiring the wallet to be unlocked is taking place): - `walletlock` - `encryptwallet` - `walletpassphrasechange` `m_relock_mutex` is also introduced so that the passphrase is not deleted from memory when the timeout provided in `walletpassphrase` is up and the wallet is still rescanning. Fixes #25702, #11249 Thanks to achow101 for coming up with the idea of using a new mutex to solve this issue and for answering related questions. ACKs for top commit: achow101: ACK 6a5b348f2e526f048d0b448b01f6c4ab608569af hernanmarino: ACK 6a5b348f2e526f048d0b448b01f6c4ab608569af furszy: Tested ACK 6a5b348f Tree-SHA512: 0b6db692714f6f94594fa47249f5ee24f85713bfa70ac295a7e84b9ca6c07dda65df7b47781a2dc73e5b603a8725343a2f864428ae20d3e126c5b4802abc4ab5
2023-02-21Merge bitcoin/bitcoin#27122: script: BIP341 txdata cannot be precomputed ↵Andrew Chow
without spent outputs 95f12de92505522a32ba58acd5251c69e602d160 BIP341 txdata cannot be precomputed without spent outputs (Pieter Wuille) Pull request description: In `PrecomputedTransactionData::Init`, if `force` is set to `true`, `m_bip341_taproot_ready` is always set to true, suggesting that all its BIP341-relevant members (including `m_spent_amounts_single_hash`) are correct. If however no `spent` array of spent previous `CTxOut`s is provided, some of these members will be incorrect. This option was introduced in #21365. That doesn't actually hurt, as without prevout data, it's fundamentally impossible to generate correct BIP341 signatures anyway, and https://github.com/bitcoin/bitcoin/blob/f722a9bd132222d9d5cd503b5af25c905b205cdb/src/script/sign.cpp#L71 should prevent the logic from being used anyway. Still, don't set `m_bip341_taproot_ready` variable when we clearly don't have enough data to compute it. Discovered by Russell O'Connor. ACKs for top commit: ajtowns: ACK 95f12de92505522a32ba58acd5251c69e602d160 achow101: ACK 95f12de92505522a32ba58acd5251c69e602d160 instagibbs: ACK 95f12de92505522a32ba58acd5251c69e602d160 Tree-SHA512: 90acd2bfa50a7a0bde75a15a9f6c1f5c40f48fb5b870b1bbc4082777e24a482c8282463ef7d1245e53201dbcb5c196ef0386352f8e380e68cdf00c2111633b77
2023-02-20Add test for unspendable transactions and parameter 'maxburnamount' to ↵David Gumberg
sendrawtransaction. 'maxburnamount' sets a maximum value for outputs heuristically deemed unspendable including datacarrier scripts that begin with `OP_RETURN`.
2023-02-20Merge bitcoin/bitcoin#27053: wallet: reuse change dest when re-creating TX ↵fanquake
with avoidpartialspends 14b4921a91920df25b19ff420bfe2bff8c56f71e wallet: reuse change dest when recreating TX with avoidpartialspends (Matthew Zipkin) Pull request description: Closes https://github.com/bitcoin/bitcoin/issues/27051 When the wallet creates a transaction internally, it will also create an alternative that spends using destination groups and see if the fee difference is negligible. If it costs the user the same to send the grouped version, we send it (even if the user has `avoidpartialspends` set to `false` which is default). This patch ensures that the second transaction creation attempt re-uses the change destination selected by the first attempt. Otherwise, the first change address remains reserved, will not be used in the second attempt, and then will never be used by the wallet, leaving gaps in the BIP44 chain. If the user had `avoidpartialspends` set to true, there is no second version of the created transaction and the change addresses are not affected. I believe this behavior was introduced in https://github.com/bitcoin/bitcoin/pull/14582 ACKs for top commit: achow101: ACK 14b4921a91920df25b19ff420bfe2bff8c56f71e Tree-SHA512: a3d56f251ff4b333fc11325f30d05513e34ab0a2eb703fadd0ad98d167ae074493df1a24068298336c6ed2da6b31aa2befa490bc790bbc260ed357c8f2397659
2023-02-20Merge bitcoin/bitcoin#27127: rpc: fix successful broadcast count in ↵fanquake
`submitpackage` error msg 7554b1fd663fe2010edb0e8a93ab85a6cb10a323 rpc: fix successful broadcast count in `submitpackage` error msg (Sebastian Falbesoner) Pull request description: If a `submitpackage` RPC call errors due to any of the individual tx broadcasts failing, the returned error message is supposed to contain the number of successful broadcasts so far: https://github.com/bitcoin/bitcoin/blob/4395b7f0845d2dca60f3b4e007ef5770ce8e2aa9/src/rpc/mempool.cpp#L848-L849 Right now this is wrongly always shown as zero. Fix this by adding the missing increment of the counter. While touching that area, the variable is also renamed to better reflect its purpose (s/num_submitted/num_broadcast/; the submission has already happened at that point) and named arguments for the `BroadcastTransaction` call are added. (Note that the error should be really rare, as all txs have already been submitted succesfully to the mempool. IIUC this code-path could only hit if somehow a tx is being removed from the mempool between `ProcessNewPackage` and the `BroadcastTransaction` calls, e.g. if a new block is received which confirms any of the package's txs.) ACKs for top commit: glozow: utACK 7554b1fd663fe2010edb0e8a93ab85a6cb10a323, thanks! Tree-SHA512: e362e93b443109888e28d6facf6f52e67928e8baaa936e355bfdd324074302c4832e2fa0bd8745309a45eb729866d0513b928ac618ccc9432b7befc3aa2aac66