f6a6d912059c66792f48689632d2a7f14f8bdad9 test: add check for getting SigningProvider for a CPubKey (Sebastian Falbesoner)
62a95f5af9b998e241eb72c16ba581e77c480126 test: refactor: move `CreateDescriptor` helper to wallet test util module (Sebastian Falbesoner)
493656763f73e5ef1cfb979a513c12983dca99dd desc spkm: Return SigningProvider only if we have the privkey (Ava Chow)
Pull request description:
If we know about a pubkey that's in our descriptor, but we don't have the private key, don't return a SigningProvider for that pubkey.
This is specifically an issue for Taproot outputs that use the H point as the resulting PSBTs may end up containing irrelevant information because the H point was detected as a pubkey each unrelated descriptor knew about.
Split from #29675
ACKs for top commit:
fjahr:
ACK f6a6d912059c66792f48689632d2a7f14f8bdad9
theStack:
re-ACK f6a6d912059c66792f48689632d2a7f14f8bdad9
furszy:
utACK f6a6d912059. Only reviewed the actual change in detail, not the test commit.
Tree-SHA512: 30a196e611a0c5d9ebe5baf6d896caaa6af66f1615463dbb0c31e52604d53cf342922bb9967b3c697b47083d76b0485c77a5f545bd6381247c8bc44321c70f97
a96b84cb1b76e65a639e62f0224f534f89858c18 fuzz: Abort when calling system time without setting mock time (marcofleon)
ff21870e20b2391b684cc50fdd6879805055d6a1 fuzz: Add SetMockTime() to necessary targets (marcofleon)
Pull request description:
This PR expands the `CheckGlobals` utility that was introduced in https://github.com/bitcoin/bitcoin/pull/31486 and should help with fuzz stability (https://github.com/bitcoin/bitcoin/issues/29018).
System time shouldn't be used when running a fuzz test, as it is likely to introduce instability (non-determinism). This PR identifies and fixes the targets that were calling system time without setting mock time at the start of an iteration.
Removing`SetMockTime()` from any one of these targets should result in a crash and a message describing the issue.
ACKs for top commit:
achow101:
ACK a96b84cb1b76e65a639e62f0224f534f89858c18
dergoegge:
Code review ACK a96b84cb1b76e65a639e62f0224f534f89858c18
brunoerg:
crACK a96b84cb1b76e65a639e62f0224f534f89858c18
Tree-SHA512: e093a9feb8a397954f7b1416dfa8790b2733f09d5ac51fda5a9d225a55ebd8f99135aa52bdf5ab531653ad1a3739c4ca2b5349c1d989bb4b009ec8eaad684f7d
Verify that the DescriptorSPKM method `GetSigningProvider` should
only return a signing provider for the passed public key if its
corresponding private key of the passed public key is available.
This is possible and safe, because std::span can implicitly convert into
Span, if needed.
Changing this function is required, because std::span requires the
extent template parameter to be specified as well.
Instead of explicilty specifying them, just let the compiler derive the
template parameters correctly.
Otherwise, there would be a compile error later on:
src/wallet/test/db_tests.cpp:39:37: error: no matching function for call to ‘as_bytes<const char>(<brace-enclosed initializer list>)’
...
/usr/include/c++/11/span:420:5: note: candidate: ...
| as_bytes(span<_Type, _Extent> __sp) noexcept
| ^~~~~~~~
/usr/include/c++/11/span:420:5: note: template argument deduction/substitution failed:
src/wallet/test/db_tests.cpp:39:37: note: couldn’t deduce template parameter ‘_Extent’
| return std::as_bytes<const char>({str.data(), str.size()});
| ~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~
c495731a316d9c97ee05a08cf5087c5535f84bd4 fuzz: wallet: add target for `CreateTransaction` (brunoerg)
3db68e29ec632b29f5417dbef095520e75adc26d wallet: move `ImportDescriptors`/`FuzzedWallet` to util (brunoerg)
Pull request description:
This PR adds a fuzz target for the `CreateTransaction` function. It is a regression target for https://github.com/bitcoin/bitcoin/pull/27271 and can be testing by applying:
```diff
@@ -1110,7 +1110,7 @@ static util::Result<CreatedTransactionResult> CreateTransactionInternal(
// This can only happen if feerate is 0, and requested destinations are value of 0 (e.g. OP_RETURN)
// and no pre-selected inputs. This will result in 0-input transaction, which is consensus-invalid anyways
if (selection_target == 0 && !coin_control.HasSelected()) {
- return util::Error{_("Transaction requires one destination of non-0 value, a non-0 feerate, or a pre-selected input")};
+ // return util::Error{_("Transaction requires one destination of non-0 value, a non-0 feerate, or a pre-selected input")};
}
```
Also, it moves `ImportDescriptors` function to `src/wallet/test/util.h` to avoid to duplicate same code.
ACKs for top commit:
marcofleon:
ACK c495731a316d9c97ee05a08cf5087c5535f84bd4
maflcko:
ACK c495731a316d9c97ee05a08cf5087c5535f84bd4 🏻
Tree-SHA512: a439f947b91b01e327e18cd18e63d5ce49f2cb9ca16ca9d56fe337b8cff239b3af4db18fe89478fe5faa5549d37ca935bd321913db7646fbf6818f825cb5d878
5e190cd11f6ed8b7ab4db0f01192de63deaf6fd7 Replace CScript _hex_v_u8 appends with _hex (Lőrinc)
cac846c2fbf6fc69bfc288fd387aa3f68d84d584 Allow CScript's operator<< to accept spans, not just vectors (Lőrinc)
c78d8ff4cb83506413bb73833fc5c04885d0ece8 prevector: avoid GCC bogus warnings in insert method (Lőrinc)
Pull request description:
Split out of https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1722326803.
Replace `_hex_v_u8` for `CScript` appends to `_hex`, to skip vector conversion before serializing to the `prevector` in `CScript`.
To enable both `unsigned char` and `std::byte` values, I've extracted the existing serialization to append the size & data in separate private methods to clarify that it does more than just a simple data insertion.
There were also discussion on eliminating the operators here completely to obviate when we're serializing fixed-size collections as raw bytes, and when we're prefixing them with their size - should also be done in a separate PR.
ACKs for top commit:
achow101:
ACK 5e190cd11f6ed8b7ab4db0f01192de63deaf6fd7
ryanofsky:
Code review ACK 5e190cd11f6ed8b7ab4db0f01192de63deaf6fd7. Looks good!
hodlinator:
re-ACK 5e190cd11f6ed8b7ab4db0f01192de63deaf6fd7
Tree-SHA512: 27a646629e017b2a05416d5eb964dda8b25b900d466221eff7bfa1339ded443e1c5c4cf8ff20cb3bba915a2603787a9fa6f6ec12bc0b9415d9eb07b57289192b
84663291275248fd52da644b0c2566bbf9cc780b chain: simplify `deleteRwSettings` code and improve it's doc (ismaelsadeeq)
f8d91f49c7091102138fcb123850399e8fadfbc7 chain: dont check for null settings value in `overwriteRwSetting` (ismaelsadeeq)
df601993f2d7454f081316d6a8ddefbcefa49b3d chain: ensure `updateRwSetting` doesn't update to a null settings (ismaelsadeeq)
c8e2eeeffb494d99d95c1c45efeda5a98dce94cd chain: uniformly use `SettingsAction` enum in settings methods (ismaelsadeeq)
1e9e735670f029c975d3b7486a54e5bb67135df7 chain: move new settings safely in `overwriteRwSetting` (ismaelsadeeq)
1c409004c80bc5f2314e20cc922076e22931cf73 test: remove wallet context from `write_wallet_settings_concurrently` (ismaelsadeeq)
Pull request description:
This PR addresses the remaining review comments from #30697
1. Disallowed overwriting settings values with a `null` value.
2. Uniformly used the `SettingsAction` enum in all settings methods instead of a boolean parameter.
3. Updated `overwriteRwSetting` to receive the `common::SettingsValue` parameter by value, enabling it to be moved safely.
4. Removed wallet context from the `write_wallet_settings_concurrently` unit test, as it is not needed.
ACKs for top commit:
achow101:
ACK 84663291275248fd52da644b0c2566bbf9cc780b
ryanofsky:
Code review ACK 84663291275248fd52da644b0c2566bbf9cc780b. Looks good, thanks for taking suggestions and applying them to the right commits. Only changes since last review were documentation improvements and simplifying delete method.
furszy:
Code review ACK 84663291275248fd52da644b0c2566bbf9cc780b
Tree-SHA512: baf2f59ed5aac4a4bda0c84fb6554a466a40d1f7b52b61dc2ff293d83ae60e82b925b7003237b633fecb65eba3a4c108e69166046895d1295809fbe0de67b052
01960c53c7d71c70792abe19413315768dc2275a fuzz: make FuzzedDataProvider usage deterministic (Martin Leitner-Ankerl)
Pull request description:
There exist many usages of `fuzzed_data_provider` where it is evaluated directly in the function call.
Unfortunately, [the order of evaluation of function arguments is unspecified](https://en.cppreference.com/w/cpp/language/eval_order), and a simple example shows that it can differ e.g. between clang++ and g++: https://godbolt.org/z/jooMezWWY
When the evaluation order is not consistent, the same fuzzing/random input will produce different output, which is bad for coverage/reproducibility. This PR fixes all these cases I have found where unspecified evaluation order could be a problem.
Finding these has been manual work; I grepped the sourcecode for these patterns, and looked at each usage individually. So there is a chance I missed some.
* `fuzzed_data_provider`
* `.Consume`
* `>Consume`
* `.rand`
I first discovered this in https://github.com/bitcoin/bitcoin/pull/29013#discussion_r1420236394. Note that there is a possibility that due to this fix the evaluation order is now different in many cases than when the fuzzing corpus has been created. If that is the case, the fuzzing corpus will have worse coverage than before.
Update: In list-initialization the order of evaluation is well defined, so e.g. usages in `initializer_list` or constructors that use `{...}` is ok.
ACKs for top commit:
achow101:
ACK 01960c53c7d71c70792abe19413315768dc2275a
vasild:
ACK 01960c53c7d71c70792abe19413315768dc2275a
ismaelsadeeq:
ACK 01960c53c7d71c70792abe19413315768dc2275a
Tree-SHA512: e56d087f6f4bf79c90b972a5f0c6908d1784b3cfbb8130b6b450d5ca7d116c5a791df506b869a23bce930b2a6977558e1fb5115bb4e061969cc40f568077a1ad
Ideally all call sites should accept std::byte instead of uint8_t but those transformations are left to future PRs.
-BEGIN VERIFY SCRIPT-
sed -i --regexp-extended 's/\bParseHex\(("[^"]*")\)/\1_hex_u8/g' $(git grep -l ParseHex -- :src ':(exclude)src/test/util_tests.cpp')
sed -i --regexp-extended 's/\bParseHex<std::byte>\(("[^"]*")\)/\1_hex/g' $(git grep -l ParseHex -- :src ':(exclude)src/test/util_tests.cpp')
sed -i --regexp-extended 's/\bScriptFromHex\(("[^"]*")\)/ToScript(\1_hex)/g' src/test/script_tests.cpp
-END VERIFY SCRIPT-
Co-Authored-By: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
Co-Authored-By: Ryan Ofsky <ryan@ofsky.org>
The following scripted-diff commit will replace ParseHex("...") with "..."_hex_u8, but this replacement will not work in cases where vectors are needed instead of arrays, and is not ideal in cases where std::byte is accepted.
For example, it is currently necessary to use _hex_v_u8 when calling CScript operator<< because that operator does not currently support std::array or std::byte.
Conversely, it is incorrect to use _hex_v instead of _hex in net_processing.cpp for the MakeAndPushMessage argument, because if the argument is a std::vector it is considered variable-length and serialized with a size prefix, but if the argument is a std::array or Span is it considered fixed length and serialized without a prefix.
By the same logic, it is also safe to change the NUMS_H constant in pubkey.cpp from a std::vector to std::array because it is never serialized.
Beyond renaming it also adjusts whitespace and adds braces to conform to current doc/developer-notes.md.
TestEncrypt: Change iterator type to auto in ahead of vector -> span conversion.
Only touches functions that will be modified in next commit.
948238a683b6c99f4e91114aa75680c6c2d73714 test: Remove FastRandomContext global (Ryan Ofsky)
fa0fe08eca48064b2a42789571fea017e455d820 scripted-diff: [test] Use g_rng/m_rng directly (MarcoFalke)
fa54cab4734f02422f28fdffc0f11e6d3d51b8f0 test: refactor: Accept any RandomNumberGenerator in RandMoney (MarcoFalke)
68f77dd21e4aaf4f09d36d6e5ddd7d260824b94b test: refactor: Pass rng parameters to test functions (Ryan Ofsky)
fa19af555dff6d6c722caf36319b158699d2aa95 test: refactor: Move g_insecure_rand_ctx.Reseed out of the helper that calls MakeRandDeterministicDANGEROUS (MarcoFalke)
3dc527f4602297ffcec3a578eadc480a620d01ec test: refactor: Give unit test functions access to test state (Ryan Ofsky)
fab023e177d7eaef73902869ae1c95693f1e268b test: refactor: Make unsigned promotion explicit (MarcoFalke)
fa2cb654eca8dd6ed89101cd6d199ba1de0b81e0 test: Add m_rng alias for the global random context (MarcoFalke)
fae7e3791c9ed8053166773fcfb583ad19d006dd test: Correct the random seed log on a prevector test failure (MarcoFalke)
Pull request description:
This is mostly a style-cleanup for the tests' random generation:
1) `g_insecure_rand_ctx` in the tests is problematic, because the name is a leftover when the generator was indeed insecure. However, now the generator is *deterministic*, because the seed is either passed in or printed (c.f. RANDOM_CTX_SEED). Stating that deterministic randomness is insecure in the tests seems redundant at best. Fix it by just using `m_rng` for the name.
2) The global random context has many one-line aliases, such as `InsecureRand32`. This is problematic, because the same line of code may use the context directly and through a wrapper at the same time. For example in net_tests (see below). This inconsistency is harmless, but confusing. Fix it by just removing the one-line aliases.
```
src/test/net_tests.cpp: auto msg_data_1 = g_insecure_rand_ctx.randbytes<uint8_t>(InsecureRandRange(100000));
````
3) The wrapper for randmoney has the same problem that the same unit test uses the context directly and through a wrapper at the same time. Also, it has a single type of Rng hardcoded. Fix it by accepting any type.
ACKs for top commit:
hodlinator:
ACK 948238a683b6c99f4e91114aa75680c6c2d73714
ryanofsky:
Code review ACK 948238a683b6c99f4e91114aa75680c6c2d73714. Only changes since last review were changing a comments a little bit.
marcofleon:
Code review ACK 948238a683b6c99f4e91114aa75680c6c2d73714. Only changes since my last review are the improvements in `prevector_tests`.
Tree-SHA512: 69c6b46a42cb743138ee8c87ff26a588dbe083e3efb3dca49b8a133ba5d3b09e8bf01c590ec7e121a7d77cb1fd7dcacd927a9ca139ac65e1f7c6d1ec46f93b57
a0abcbd3822bd17a1d73c42ccd5b040a150b0501 doc: Mention multipath specifier (Ava Chow)
0019f61fc546b4d5f42eb4086f42560863fe0efb tests: Test importing of multipath descriptors (Ava Chow)
f97d5c137d605ac48f1122a836c9aa5f834957ba wallet, rpc: Allow importdescriptors to import multipath descriptors (Ava Chow)
32dcbca3fb918bc899a0637f876db31c3419aafd rpc: Allow importmulti to import multipath descriptors correctly (Ava Chow)
64dfe3ce4bed9ac168d0b08def8af7485db94ef1 wallet: Move internal to be per key when importing (Ava Chow)
16922455253f47fae0466c4ec6c3adfadcfe9182 tests: Multipath descriptors for scantxoutset and deriveaddresses (Ava Chow)
cddc0ba9a9dca3ca5873d768b3b504cdb2ab947b rpc: Have deriveaddresses derive receiving and change (Ava Chow)
360456cd221501fde3efe11bdba5c6d999dbb323 tests: Multipath descriptors for getdescriptorinfo (Ava Chow)
a90eee444c965bbd7bcddf9656eca9cee14c3aec tests: Add unit tests for multipath descriptors (Ava Chow)
1bbf46e2dae4599d04c79aaacf7c5db00b2e707f descriptors: Change Parse to return vector of descriptors (Ava Chow)
0d640c6f02bc20e5c1be773443dd74d8806d953b descriptors: Have ParseKeypath handle multipath specifiers (Ava Chow)
a5f39b103461a98689fd5d382e8da29037f55bea descriptors: Change ParseScript to return vector of descriptors (Ava Chow)
0d55deae157f4f8226b2419d55e7dc0dfb6e4aec descriptors: Add DescriptorImpl::Clone (Ava Chow)
7e86541f723d62c7ec6768f7f592c09ba2047d9e descriptors: Add PubkeyProvider::Clone (Ava Chow)
Pull request description:
It is convenient to have a descriptor which specifies both receiving and change addresses in a single string. However, as discussed in https://github.com/bitcoin/bitcoin/issues/17190#issuecomment-895515768, it is not feasible to use a generic multipath specification like BIP 88 due to combinatorial blow up and that it would result in unexpected descriptors.
To resolve that problem, this PR proposes a targeted solution which allows only a single pair of 2 derivation indexes to be inserted in the place of a single derivation index. So instead of two descriptor `wpkh(xpub.../0/0/*)` and `wpkh(xpub.../0/1/*)` to represent receive and change addresses, this could be written as `wpkh(xpub.../0/<0;1>/*)`. The multipath specifier is of the form `<NUM;NUM>`. Each `NUM` can have its own hardened specifier, e.g. `<0;1h>` is valid. The multipath specifier can also only appear in one path index in the derivation path.
This results in the parser returning two descriptors. The first descriptor uses the first `NUM` in all pairs present, and the second uses the second `NUM`. In our implementation, if a multipath descriptor is not provided, a pair is still returned, but the second element is just `nullptr`.
The wallet will not output the multipath descriptors (yet). Furthermore, when a multipath descriptor is imported, it is expanded to the two descriptors and each imported on its own, with the second descriptor being implicitly for internal (change) addresses. There is no change to how the wallet stores or outputs descriptors (yet).
Note that the path specifier is different from what was proposed. It uses angle brackets and the semicolon because these are unused characters available in the character set and I wanted to avoid conflicts with characters already in use in descriptors.
Closes#17190
ACKs for top commit:
darosior:
re-ACK a0abcbd3822bd17a1d73c42ccd5b040a150b0501
mjdietzx:
reACK a0abcbd3822bd17a1d73c42ccd5b040a150b0501
pythcoiner:
reACK a0abcbd
furszy:
Code review ACK a0abcbd
glozow:
light code review ACK a0abcbd3822
Tree-SHA512: 84ea40b3fd1b762194acd021cae018c2f09b98e595f5e87de5c832c265cfe8a6d0bc4dae25785392fa90db0f6301ddf9aea787980a29c74f81d04b711ac446c2
41051290ab3b6c36312cec26a27f787cea9961b4 cmake: Ignore build subdirectories within source directory (Hennadii Stepanov)
6ce50fd9d0ae6850d54bf883e7a7c1bcb6912c5c doc: Update for CMake-based build system (Hennadii Stepanov)
9730288a0cd3f33021ef00fb2d95e5216d10ab61 ci: Migrate CI scripts to CMake (Hennadii Stepanov)
c360837ca5c91c9878ae8088bb5482e96fd87c96 cmake, lint: Adjust `lint_includes_build_config` (Hennadii Stepanov)
3885441ee0d35a40904995ede68120fea471dde7 cmake: Add presets for native Windows builds (Hennadii Stepanov)
7681746b20dd58e7d3e6d2852f07fb876383a133 cmake: Add vcpkg manifest file (Hennadii Stepanov)
8b6f1c4353836bae6aa683cbc65251165bd031ba cmake: Add `Coverage` and `CoverageFuzz` scripts (Hennadii Stepanov)
65bdbc1ff23b0a817f4d9a4682e6f630c9bbdd59 cmake: Add `docs` build target (Hennadii Stepanov)
fb75ebbc33557ddd56f505100ad3631a0028eb86 cmake: Add compiler diagnostic flags (Hennadii Stepanov)
e821f0a37a026fa0480c7f6f6c938da7c77e0d52 cmake: Migrate Guix build scripts to CMake (Hennadii Stepanov)
747adb6ffe9b06d476fc5eaebbaf9a62b91a78c5 cmake: Add `Maintenance` module (Hennadii Stepanov)
1f60b30df0cb58a7381a1bfbd6d34f002232e862 cmake: Add `APPEND_{CPP,C,CXX,LD}FLAGS` cache variables (Hennadii Stepanov)
2b43c45b13ad00cfd9928a03da8a480977724cb1 cmake: Add `AddWindowsResources` module (Hennadii Stepanov)
973a3b0c5dcbf6b3fd155b2dda4c2e94a0b0ee5f cmake: Implement `install` build target (Hennadii Stepanov)
84ac35cfd4dfa6f235f6e5a00b571846358f45ce cmake: Add cross-compiling support (Hennadii Stepanov)
0d01c228a7d39bb4918b5cb9f6db25cb8c30567a build: Generate `toolchain.cmake` in depends (Hennadii Stepanov)
91a799247dc5e4627e6b2f221669c8ff9238bc8d depends: Add host-specific `cmake_system_version` variables (Hennadii Stepanov)
9b31209b4caaa02b3044acd2375a7f595cdbd520 depends: Rename `cmake_system` -> `cmake_system_name` (Hennadii Stepanov)
4a5208a81d5bfeef270c64d48dce3444d6d03511 Revert "build, qt: Do not install *.prl files" (Hennadii Stepanov)
6522af62af1c3a6e2525bfffdb2295751b6fa49b depends: Amend handling flags environment variables (Hennadii Stepanov)
90cec4d251a541adfc5953e24dc01840a8cb4af2 cmake: Add `MULTIPROCESS` option (Hennadii Stepanov)
bb1a450dcb111746869547c8b538b5d2472cf8e6 cmake: Build `bitcoin-chainstate` executable (Hennadii Stepanov)
aed38ea58cbde068fe12b5299b246b4e3649a09c cmake: Build `bitcoinkernel` library (Hennadii Stepanov)
975d67369b8f3a33a21fd7618c299c0ec138292c cmake: Build `test_bitcoin-qt` executable (Hennadii Stepanov)
10fcc668a3430b72eaf7effc83f00cedeb27c7dc cmake: Add `WITH_DBUS` option (Hennadii Stepanov)
5bb5a4bc75a523e30eab561763927252ce105c4d cmake: Add `libqrencode` optional package support (Hennadii Stepanov)
57a6e2ef4abbfd2b12ee6489366bc6609bead263 cmake: Build `bitcoin-qt` executable (Hennadii Stepanov)
30f642952cb5bf39479bdbe467b3950f0d09324a cmake: Add `WERROR` option (Hennadii Stepanov)
c98d4a4c341e524348d0342e145d439816a44c83 cmake: Add `REDUCE_EXPORTS` option (Hennadii Stepanov)
a01cb6e63ff3940f0773b37e2fe1e148f17acad9 cmake: Add `HARDENING` option (Hennadii Stepanov)
a8a2e364acf55bbe18404ab21f852d52257bcb6d cmake: Add Python-based tests (Hennadii Stepanov)
3d853795707c5a1828dcd09c1f68bb07dee472cd cmake: Add fuzzing options (Hennadii Stepanov)
908530e312a3d4561f9c1feeb2a76ce899f21c68 cmake: Add `SANITIZERS` option (Hennadii Stepanov)
8bb0e85631e7c1bee16e136454b2466776be1d65 cmake: Build `bench_bitcoin` executable (Hennadii Stepanov)
801735163a81650619a6c9587e8f1df9ee182694 cmake: Add external signer support (Hennadii Stepanov)
353e0c9e9679864a777e17c1bb7c6ba8b6eac96d cmake: Add `systemtap-sdt` optional package support (Hennadii Stepanov)
d2fda82b4954f4af7e7d340cf42b9cb34d96cde1 cmake: Add `libzmq` optional package support (Hennadii Stepanov)
ae7b39a0e106d798b6e9cc03ee783d9081e41480 cmake: Add `libminiupnpc` optional package support (Hennadii Stepanov)
6480e1dcdb03f43ce3d0aad96b8668d017d11750 cmake: Add `libnatpmp` optional package support (Hennadii Stepanov)
e73e9304a11af65f9b086460ff349f9f700709ce cmake: Build `bitcoin-util` executable (Hennadii Stepanov)
027c6d7caa0355c35b00f2689eddccc3d1227aef cmake: Build `bitcoin-tx` executable (Hennadii Stepanov)
d10c5c34c3d899db8bcff47ac8c6ba396a6da4b6 cmake: Add wallet functionality (Hennadii Stepanov)
ab2e99b0d95714e16a7d1a1313d7da938b0485cb cmake: Create test suite for `ctest` (Hennadii Stepanov)
959370bd76d30ced34208db45fb4fd097fbad31b cmake: Build `test_bitcoin` executable (Hennadii Stepanov)
b27bf9700dbbfa9a0243815f78c8b62abe78d8bc cmake: Build `bitcoin-cli` executable (Hennadii Stepanov)
a9813df826c885b1609e55a83d87cd9cbc90adfd cmake: Build `bitcoind` executable (Hennadii Stepanov)
97829ce2d5a8dc3b0307b5d57c6686b96b7cf850 cmake: Add `FindLibevent` module (Hennadii Stepanov)
3118e40c6157c8ab9e264518d1065d2b0fc07795 cmake: Build `bitcoin_consensus` library (Hennadii Stepanov)
809a2f192903145f88f30bc10d3cf1ab9ed06881 cmake: Build `bitcoin_util` static library (Hennadii Stepanov)
0a9a521a704ca8a27124c1498a86e87ad46d4c34 cmake: Build `bitcoin_crypto` library (Hennadii Stepanov)
958971f476a29cb5bb76f3ae80ae968317ca1930 cmake: Build `univalue` static library (Hennadii Stepanov)
752747fda801f2c0ecce91c96bcc9ef93e27462b cmake: Generate `obj/build.h` header (Hennadii Stepanov)
1f0a78edf3cd2c24236ac512acf420eb9ed86ab3 cmake: Build `minisketch` static library (Hennadii Stepanov)
12bfbc81540f037c95e7796ae0b9f05ce3fb1b4a cmake: Build `leveldb` static library (Hennadii Stepanov)
51985c5304dfc52bd45f505b3115989637d79ff5 cmake: Build `crc32c` static library (Hennadii Stepanov)
db7a198f29c62c5f762eaa25d1d83c57e2f1e211 cmake: Build `secp256k1` subtree (Hennadii Stepanov)
dbb7ed14e8562439238eec70b202c50f172e3def cmake: Add `ccache` support (Hennadii Stepanov)
cedfdf6c72535d0797a271c6bb9d84c4b406a8ea cmake: Redefine/adjust per-configuration flags (Hennadii Stepanov)
b6b5e732c8b49a2cc14f34ac72b2189389c6b27d cmake: Add global compiler and linker flags (Hennadii Stepanov)
f98327931bd0b5d90678ddd1770e9862266b396e cmake: Add `TryAppendLinkerFlag` module (Hennadii Stepanov)
4a0af29697b62d32af6f60d3ec70cd2ed4d7243c cmake: Add `TryAppendCXXFlags` module (Hennadii Stepanov)
35cffc497d8db3cf3eee35c1513e3435558f056b cmake: Add POSIX threads support (Hennadii Stepanov)
fd72d00ffe34c84e292b305f6797201040d31a72 cmake: Add position independent code support (Hennadii Stepanov)
07069e2bb0bbdacf16cf34efd3a33390de030217 cmake: Add introspection module (Hennadii Stepanov)
27d687fc1f6aceaed7725e1e904a093ead68d6e6 cmake: Add `config/bitcoin-config.h` support (Hennadii Stepanov)
fe5cdace5ffba46fb7981efb816621962d3873e3 cmake: Print compiler and linker flags in summary (Hennadii Stepanov)
70683884c5fd78dbf7816434464e6511b9d4e486 cmake: Introduce interface libraries to encapsulate common flags (Hennadii Stepanov)
a2317e27b7fb86df4e32cd1674c06e09cb808248 cmake: Add root `CMakeLists.txt` file (Hennadii Stepanov)
Pull request description:
This PR introduces a new CMake-based build system, which is a drop-in replacement for the current Autotools-based build system.
ML announcement: https://groups.google.com/g/bitcoindev/c/hgKkfQWzrTo
As discussed during the recent CoreDev meetup in April, the switch from Autotools to CMake is intended to happen as soon as possible after branching 28.x off, which means that 29.0 will be built using CMake.
This PR branch is essentially the [staging branch](https://github.com/hebasto/bitcoin/tree/cmake-staging), with every change reviewed and tested by a group of contributors, including (in alphabetical order):
- [**achow101**](https://github.com/achow101)
- [**fanquake**](https://github.com/fanquake)
- [**maflcko**](https://github.com/maflcko)
- [**m3dwards**](https://github.com/m3dwards)
- [**pablomartin4btc**](https://github.com/pablomartin4btc)
- [**real-or-random**](https://github.com/real-or-random)
- [**ryanofsky**](https://github.com/ryanofsky)
- [**sipsorcery**](https://github.com/sipsorcery)
- [**TheCharlatan**](https://github.com/TheCharlatan)
- [**theStack**](https://github.com/theStack)
- [**theuni**](https://github.com/theuni)
- [**vasild**](https://github.com/vasild)
Reviewing in a separate staging repo was suggested in https://github.com/bitcoin/bitcoin/pull/27060#issuecomment-1431798320.
The accompanying changes to the OSS-Fuzz project are available in https://github.com/hebasto/oss-fuzz/pull/8.
Please refer to the [build options parity table](https://gist.github.com/hebasto/2ef97d3a726bfce08ded9df07f7dab5e). The "auto" value is no longer available; non-default values must be specified explicitly. Additionally, the new default values have been chosen to suit the everyday build experience for the majority of developers.
System requirements for using the CMake-based build system:
- CMake >= 3.22 (if not available in your system's repository, it can be downloaded from https://cmake.org/download/)
- a build tool of your choice:
- any Make (GNU Make is no longer a requirement); GNU Make is still required to build depends
- Ninja (https://ninja-build.org/)
- MSBuild
- Xcode
A note for Windows users: The default installation of the latest version of MSVC 17.10.4 includes both CMake 3.28.3 and the vcpkg package manager).
---
We, the build system developers, kindly ask reviewers to refrain from making suggestions that are not directly related to the migration process or can be implemented separately. Bugs in the scripts and errors in the updated documentation should be the focus of this PR. Please be advised that comments not aligned with this PR's goal may be ignored.
Thank you all for your understanding.
ACKs for top commit:
maflcko:
review ACK 41051290ab3b6c36312cec26a27f787cea9961b4 🐥
sipsorcery:
ACK 41051290ab3b6c36312cec26a27f787cea9961b4.
vasild:
ACK 41051290ab3b6c36312cec26a27f787cea9961b4
TheCharlatan:
ACK 41051290ab3b6c36312cec26a27f787cea9961b4
pablomartin4btc:
tACK 41051290ab3b6c36312cec26a27f787cea9961b4
i-am-yuvi:
tACK [`4105129`](41051290ab)
theuni:
ACK 41051290ab3b6c36312cec26a27f787cea9961b4.
fanquake:
ACK 41051290ab3b6c36312cec26a27f787cea9961b4
Tree-SHA512: 6c1445054436c6c00ad63bfa0f19d64091a2b25c9bd694f85bf2218ac358ffb774d6c000685b3ca1e9b50401babed989fa2a0694b774c211d226bfd1944c9b39
- Settings updates were not thread-safe, as they were executed in
three separate steps:
1) Obtain settings value while acquiring the settings lock.
2) Modify settings value.
3) Overwrite settings value while acquiring the settings lock.
This approach allowed concurrent threads to modify the same base value
simultaneously, leading to data loss. When this occurred, the final
settings state would only reflect the changes from the last thread
that completed the operation, overwriting updates from other threads.
Fix this by making the settings update operation atomic.
- Add test coverage for this behavior.
Co-authored-by: furszy <matiasfurszyfer@protonmail.com>
401cc4ec70d67ba2aa0e078d2fab214e1c40742c fuzz: improve scriptpubkeyman target (brunoerg)
Pull request description:
Fixes#30541
This PR aims to improve `scriptpubkeyman` target to avoid timeouts. The input provided in #30541 takes too much time to run because it basically calls only `MarkUnusedAddresses` (300 times * number of spks). The following changes were made to improve it:
- Reduce keypool size.
- When calling `MarkUnusedAddresses`, do it with one of the spks per iteration.
- Remove the specific `AddDescriptorKey` call since it is already covered with `AddWalletDescriptor`.
- Limit number of iterations to a reasonable value.
ACKs for top commit:
maflcko:
lgtm ACK 401cc4ec70d67ba2aa0e078d2fab214e1c40742c
achow101:
ACK 401cc4ec70d67ba2aa0e078d2fab214e1c40742c
Tree-SHA512: 941812bc6d991dd03675a2974ce1b839494ca7f6e6d8a22c689d4bf4fed2dac5491246998f19cb15dbff516fdd8eeda27e7628c3206d45f57dc292bc05624a5c
Move `SetKeyFromPassphrase` to out of LIMITED_WHILE,
remove `SetKey` calls since it is already called
internally by other functions and reduce the number
of iterations.
734076c6de1781f957c8bc3bf7ed6951920cfcf6 [wallet, rpc]: add `max_tx_weight` to tx funding options (ismaelsadeeq)
b6fc5043c16c2467a2a6768a6ca9b18035fc400f [wallet]: update the data type of `change_output_size`, `change_spend_size` and `tx_noinputs_size` to `int` (ismaelsadeeq)
baab0d2d43049a71dc90176bc4d72062f7b2ce19 [doc]: update reason for deducting change output weight (ismaelsadeeq)
7f61d31a5cec8fc61328bee43f90d3f1dcb0a035 [refactor]: update coin selection algorithms input parameter `max_weight` name (ismaelsadeeq)
Pull request description:
This PR taken over from #29264
The PR added an option `max_tx_weight` to transaction funding RPC's that ensures the resulting transaction weight does not exceed the specified `max_tx_weight` limit.
If `max_tx_weight` is not given `MAX_STANDARD_TX_WEIGHT` is used as the max threshold.
This PR addressed outstanding review comments in #29264
For more context and rationale behind this PR see https://delvingbitcoin.org/t/lightning-transactions-with-v3-and-ephemeral-anchors/418/11?u=instagibbs
ACKs for top commit:
achow101:
ACK 734076c6de1781f957c8bc3bf7ed6951920cfcf6
furszy:
utACK 734076c6de1781f957c8bc3bf7ed6951920cfcf6
rkrux:
reACK [734076c](734076c6de)
Tree-SHA512: 013501aa443d239ee2ac01bccfc5296490c27b4edebe5cfca6b96c842375e895e5cfeb5424e82e359be581460f8be92095855763a62779a18ccd5bdfdd7ddce7
4383dc90bac1b5def73352fe222f99807d8ca4dd fuzz: fix key size in crypter target (brunoerg)
Pull request description:
Fixes#30251
This PR:
1. Limits `cipher_text_ed` and `random_string` (`SecureString`) size.
2. Replace `ConsumeRandomLengthByteVector` for keys to `ConsumeFixedLengthByteVector` with `WALLET_CRYPTO_KEY_SIZE`.
3. Replace `ConsumeRandomLengthByteVector` for `chSalt` to `ConsumeFixedLengthByteVector` with `WALLET_CRYPTO_SALT_SIZE`.
ACKs for top commit:
marcofleon:
Tested ACK 4383dc90bac1b5def73352fe222f99807d8ca4dd. I ran this:
dergoegge:
utACK 4383dc90bac1b5def73352fe222f99807d8ca4dd
Tree-SHA512: 6f09cca0b4627f49152b685ac03659c01004f2131c6aada7654606ea01f6619b1611b1d17624d2cddce277c1afdddda5f656d99f6ca8f72a22f5c0541762c964
This allows a transaction's weight to be bound under a certain
weight if possible and desired. This can be beneficial for future
RBF attempts, or whenever a more restricted spend topology is
desired.
Co-authored-by: Greg Sanders <gsanders87@gmail.com>
- This commit renames the coin selection algorithms input parameter `max_weight`
to `max_selection_weight` for clarity.
The parameter represent the maximum weight of the UTXOs the coin selection algorithm
should select, not the transaction maximum weight.
- The commit updates the parameter docstring to provide correct description.
- Also updates coin selection unit and fuzzing test variables to match the new name.
