aboutsummaryrefslogtreecommitdiff
AgeCommit message (Collapse)Author
2023-12-14Merge bitcoin/bitcoin#29070: test: add TestNode wait_until helperAva Chow
bf0f7dbec6590a54ec890e7a2ca5d85427995334 test: add TestNode wait_until helper (Nikodemas Tuckus) Pull request description: Add `wait_until` method that wraps the `wait_until_helper_internal` call. Closes https://github.com/bitcoin/bitcoin/issues/29029. ACKs for top commit: maflcko: lgtm ACK bf0f7dbec6590a54ec890e7a2ca5d85427995334 mohamedawnallah: LGTM! Code Review ACK https://github.com/bitcoin/bitcoin/commit/bf0f7dbec6590a54ec890e7a2ca5d85427995334 achow101: ACK bf0f7dbec6590a54ec890e7a2ca5d85427995334 BrandonOdiwuor: Code review ACK bf0f7dbec6590a54ec890e7a2ca5d85427995334 Tree-SHA512: 05aab589c814f51a14e1483eb57c10b88385714e3eb2d0973c0ee2877f2b963a76837f34215fe2e6bd1c8d735f5af7dd2098331e1eda28587f39e513bc6e1a6a
2023-12-14Merge bitcoin/bitcoin#29022: Make bitcoin-tx replaceable value optionalAva Chow
98afe7866185ed4157ffc581763e11dc02fcbae0 doc: Update bitcoin-tx replaceable documentation (Kashif Smith) 94feaf2b66d68b3c849375e1d9d3a81c17cd2045 tests: Add unit tests for bitcoin-tx replaceable command (Kashif Smith) c2b836b119eeed8727d73bcca5e95055eb93fb1a bitcoin-tx: Make replaceable value optional (Kashif Smith) Pull request description: This fixes #28638. The issue was originally raised by dooglus, who also suggested the patch found in this code. Additionally, test coverage has been added and documentation has been updated. ACKs for top commit: achow101: ACK 98afe7866185ed4157ffc581763e11dc02fcbae0 pinheadmz: ACK 98afe7866185ed4157ffc581763e11dc02fcbae0 hernanmarino: Tested ACK 98afe7866185ed4157ffc581763e11dc02fcbae0 instagibbs: untested ACK https://github.com/bitcoin/bitcoin/pull/29022/commits/98afe7866185ed4157ffc581763e11dc02fcbae0 Tree-SHA512: ea1384aba7b0014c8cbeb7280d66b1e617d406fb02471dff33873057132b80518c94c7caa4b0426c26d17ce8aa393107de319dde781ace8df72f0314c8c75159
2023-12-14Merge bitcoin/bitcoin#29080: ci: Set ↵Hennadii Stepanov
`HOMEBREW_NO_INSTALLED_DEPENDENTS_CHECK` to avoid unrelated failures 43c3246af774bda284111056268a814477f9b256 ci: Set `HOMEBREW_NO_INSTALLED_DEPENDENTS_CHECK` to avoid failures (Hennadii Stepanov) Pull request description: Homebrew attempts to check for outdated dependents or those with broken linkage. Such behavior might lead to failures when Homebrew updates them on old macOS images. For example, https://github.com/bitcoin/bitcoin/actions/runs/7199058794/job/19609891263 using the macOS image version `20231025.2`. This PR prevents such behavior. ACKs for top commit: maflcko: lgtm ACK 43c3246af774bda284111056268a814477f9b256 ismaelsadeeq: re ACK https://github.com/bitcoin/bitcoin/commit/43c3246af774bda284111056268a814477f9b256 Tree-SHA512: cbe3cef5adf3f00eb618ba17aad3dc76c0c5d11142122a26b93619ae47dc50771e9e095caa898213325ed6ff41c07119429c0a9094bb98ead5601855d07bb2ea
2023-12-14ci: Set `HOMEBREW_NO_INSTALLED_DEPENDENTS_CHECK` to avoid failuresHennadii Stepanov
Homebrew attempts to check for outdated dependents or those with broken linkage. Such behavior might lead to failures when Homebrew updates them on old macOS images. This change prevents such behavior.
2023-12-13Merge bitcoin/bitcoin#29065: bench: wallet, fix change position out of range ↵Ava Chow
error 37c75c58202f89b752500f76c872d7f8caf6bdb3 test: wallet, fix change position out of range error (furszy) Pull request description: Fixes #29061. Only the benchmark is affected. Since #25273, the behavior of 'inserting change at a random position' is instructed by passing ´std::nullopt´ instead of -1. Also, added missing documentation about the meaning of 'change_pos=std::nullopt' inside 'CWallet::CreateTransaction()' ACKs for top commit: achow101: ACK 37c75c58202f89b752500f76c872d7f8caf6bdb3 kevkevinpal: ACK [37c75c5](https://github.com/bitcoin/bitcoin/pull/29065/commits/37c75c58202f89b752500f76c872d7f8caf6bdb3) BrandonOdiwuor: ACK 37c75c58202f89b752500f76c872d7f8caf6bdb3 Tree-SHA512: d9a8d8533540455716a5090fcf407573cad9f0d0018a05f903f89e51620302f9b256318db6f7338b85c047f7fab372d724e916b1721d7ed302dbf3d845b08734
2023-12-13Merge bitcoin/bitcoin#29075: msvc: Fix `test\config.ini` contentfanquake
f76e59d02ea819928b45bd18d9c3a5b1aab36fe2 msvc: Fix `test\config.ini` content (Hennadii Stepanov) Pull request description: This PR: 1. Closes https://github.com/bitcoin/bitcoin/issues/29074. 2. Enables the `tool_signet_miner.py` test. ACKs for top commit: fanquake: ACK f76e59d02ea819928b45bd18d9c3a5b1aab36fe2 Tree-SHA512: d6e4f5b1c2018426b5b3e3239d7a2e993ae4f6598e4afc7ea484a1d2cb203b5a9fa011bbc5cbda5c596d84f99a632a776b21fa48a1ed70930fbbec0241782f93
2023-12-13msvc: Fix `test\config.ini` contentHennadii Stepanov
2023-12-13Merge bitcoin/bitcoin#29066: Bump minimum required Boost version due to ↵fanquake
migration to C++20 49a90915aa3ee8e3a7e163f23a55de931faf8523 build: Bump minimum required Boost to 1.73.0 to support C++20 (Hennadii Stepanov) Pull request description: Boost versions <1.73 have C++20-specific bugs that were fixed in the following commits: - https://github.com/boostorg/signals2/commit/15fcf213563718d2378b6b83a1614680a4fa8cec - https://github.com/boostorg/test/commit/495c095dc063052ce54f2fe9217fe0fc69ced5f1 I tested [`libboost1.71-dev`](https://packages.ubuntu.com/focal/libboost1.71-dev) in Ubuntu 20.04 and Boost 1.71, 1.72, 1.73 in our depends build system. Closes https://github.com/bitcoin/bitcoin/issues/29063. ACKs for top commit: fanquake: ACK 49a90915aa3ee8e3a7e163f23a55de931faf8523 Tree-SHA512: b8ebc08af85abfa3fda70961bd1136ee9e5149dd76a3f901e43acba624d231971873cba5cbf30837f9e5ab58790b8330f241a76cb76d8cf5dce5ad0cca33fba8
2023-12-13Merge bitcoin/bitcoin#28967: build: disable external-signer for Windowsfanquake
308aec3e5655327d98e0428d8205d246f24d6af5 build: disable external-signer for Windows (fanquake) 35537318a19360ddf1ea8f0c1e6d8ad49e635516 ci: remove --enable-external-signer from win64 job (fanquake) Pull request description: It's come to light that Boost ASIO (a Boost Process sub dep) has in some instances, been quietly initialising our network stack on Windows (see PR https://github.com/bitcoin/bitcoin/pull/28486 and discussion in https://github.com/bitcoin/bitcoin/issues/28940). This has been shielding a bug in our own code, but the larger issue is that Boost Process/ASIO is running code before main, and doing things like setting up networking. This undermines our own assumptions about how our binary works, happens before we run any sanity checks, and before we call our own code to setup networking. Note that ASIO also calls WSAStartup with version `2.0`, whereas we call with `2.2`. It's also not clear why a feature like external signer would have a dependency that would be doing anything network/socket related, given it only exists to spawn a local process. See also the discussion in https://github.com/bitcoin/bitcoin/issues/24907. Note that the maintaince of Boost Process in general, has not really improved. For example, rather than fixing bugs like https://github.com/boostorg/process/issues/111, i.e, https://github.com/boostorg/process/pull/317, the maintainer chooses to just wrap exception causing overflows in try-catch blocks: https://github.com/boostorg/process/commit/0c42a58eacab6a96b19196e399307bad8a938a27. These changes get merged in large, unreviewed PRs, i.e https://github.com/boostorg/process/pull/319. This PR disables external-signer on Windows for now. If, in future, someone changes how Boost Process works, or replaces it entirely with some properly reviewed and maintained code, we could reenable this feature on Windows. ACKs for top commit: hebasto: re-ACK 308aec3e5655327d98e0428d8205d246f24d6af5. TheCharlatan: ACK 308aec3e5655327d98e0428d8205d246f24d6af5 Tree-SHA512: 7405f7fc9833eeaacd6836c4e5b1c1a7845a40c1fdd55c1060152f8d8189e4777464fde650e11eb1539556a75dddf49667105987078b1457493ee772945da66e
2023-12-13Merge bitcoin/bitcoin#29068: test: Actually fail when a python unit test failsfanquake
fa0534d7e47d44428d3f9dea6d2f6b8e86df22d4 test: Actually fail when a python unit test fails (MarcoFalke) Pull request description: Currently python unit test failures are ignored. Fix this. ACKs for top commit: theStack: ACK fa0534d7e47d44428d3f9dea6d2f6b8e86df22d4 BrandonOdiwuor: ACK fa0534d7e47d44428d3f9dea6d2f6b8e86df22d4 Tree-SHA512: c136be4c8d861d966f380e04d5d14b711b90c4011101302dae1332496e493207c5c673927586ed35b02b61a0b050bf45053a31e6ff766ec52f1d054caf0985e2
2023-12-13Merge bitcoin/bitcoin#28846: depends: fix libmultiprocess build on aarch64fanquake
bde8d63b17637c507a543cebe90f2998b5847373 depends: build libmultiprocess with position independant code (fanquake) 506634d79d6427925cd458f67799fe59e0ab14dd depends: always install libmultiprocess to /lib (fanquake) beb309626381bf189cd2ae8bde83078b9de47c6a depends: always install capnp to /lib (fanquake) Pull request description: Change to always install libmultiprocess into `lib/`. On some systems (my Fedora aarch64 box), libmultiprocess was being installed into `lib64/`, and then configure would fail to pick it up, because we only add `lib/` to pkgconfig/ldflags out of depends. Rather than adding lib64 to those, I opted for installing libmultiprocess into lib, with every other dependency we build. This was broken in our build after https://github.com/chaincodelabs/libmultiprocess/pull/79 upstream. ACKs for top commit: ryanofsky: Code review ACK bde8d63b17637c507a543cebe90f2998b5847373. Only changes since last review were reverting the native_capnp change as suggested, and changing the order of the first two commits. Tree-SHA512: ddd547e4ac224f2f199c569efd91104db7f2c243b124f9535aa0d9377315775ac566d699101580ce45ddd6676ad3e0c8cbe256334eeed9548205c2fa04d02102
2023-12-13test: add TestNode wait_until helperNikodemas Tuckus
2023-12-13Merge bitcoin/bitcoin#28075: util: Remove DirIsWritable, GetUniquePathfanquake
fa3da629a1aebcb4500803d7417feed8e34285b0 Remove DirIsWritable, GetUniquePath (MarcoFalke) fad3a9793b71df5bb0b17cc3758cf3466d08c015 Return LockResult::ErrorWrite in LockDirectory (MarcoFalke) fa0afe740843c308f6287b923f1f4d758cf2a3f6 refactor: Return enum in LockDirectory (MarcoFalke) Pull request description: `GetUniquePath` is only used in tests and in `DirIsWritable`. The check by `DirIsWritable` is redundant with the check done in `LockDirectory`. Fix the redundancy by removing everything, except `LockDirectory`. ACKs for top commit: TheCharlatan: Re-ACK fa3da629a1aebcb4500803d7417feed8e34285b0 hebasto: ACK fa3da629a1aebcb4500803d7417feed8e34285b0, I have reviewed the code and it looks OK. Tree-SHA512: e95f18cd586de7582e9c08ac7ddb860bfcfcbc8963804f45c5784c5e4c0598dc59ae7e45dd4daf30a5020dbf8433f5db2ad06e46a8676371982003790043c6c9
2023-12-12test: Actually fail when a python unit test failsMarcoFalke
2023-12-12build: Bump minimum required Boost to 1.73.0 to support C++20Hennadii Stepanov
Boost versions <1.73 have C++20-specific bugs that were fixed in the following commits: - https://github.com/boostorg/signals2/commit/15fcf213563718d2378b6b83a1614680a4fa8cec - https://github.com/boostorg/test/commit/495c095dc063052ce54f2fe9217fe0fc69ced5f1
2023-12-12test: wallet, fix change position out of range errorfurszy
Since #25273, the behavior of 'inserting change at a random position' is instructed by passing std::nullopt instead of -1. Also, added missing documentation about the meaning of 'change_pos=std::nullopt' inside 'CWallet::CreateTransaction()'
2023-12-12Merge bitcoin/bitcoin#28994: wallet: skip BnB when SFFO is enabledAndrew Chow
576bee88fd36e207b7288077626947a1fce0fc33 fuzz: disable BnB when SFFO is enabled (furszy) 05e5ff194c7722b4ebc2b9309fc0bf47b3cf1df7 test: add coverage for BnB-SFFO restriction (furszy) 0c5755761c3e544547899ad096121585dffa73df wallet: create tx, log resulting coin selection info (furszy) 5cea25ba795d6eb9ccc721d01560783ae576af34 wallet: skip BnB when SFFO is active (Murch) Pull request description: Solves #28918. Coming from https://github.com/bitcoin/bitcoin/issues/28918#issuecomment-1838626406 discussion. The intention is to decouple only the bugfix relevant commits from #28985, allowing them to be included in the 26.x release. This way, we can avoid disabling the coin selection fuzzing test for an entire release. Note: Have introduced few changes to the bug fix commit so that the unit tests pass without the additional burden introduced in #28985. ACKs for top commit: josibake: ACK https://github.com/bitcoin/bitcoin/commit/576bee88fd36e207b7288077626947a1fce0fc33 murchandamus: ACK https://github.com/bitcoin/bitcoin/commit/576bee88fd36e207b7288077626947a1fce0fc33 achow101: ACK 576bee88fd36e207b7288077626947a1fce0fc33 Tree-SHA512: f5d90eb3f3f524265afe4719495c9bf30f98b9af26cf039f7df5a7db977abae72caa7a3478cdd0ab10cd143bc1662e8fc5286b5bc10fc10f0dd582a45b45c31a
2023-12-12depends: build libmultiprocess with position independant codefanquake
This matches what we do with all other dependencies, see `--with-pic`, and fixes build failures, like #26943.
2023-12-12depends: always install libmultiprocess to /libfanquake
On some systems, libmultiprocess would be installed into `lib64`, I assume due to the use of GNUInstallDirs, however all other libs we build in depends, go into lib/. Rather than adding lib64/ to the pkg-config and link flags, I opted for always installing into lib/. This was changed in https://github.com/chaincodelabs/libmultiprocess/pull/79 upstream. Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
2023-12-12depends: always install capnp to /libfanquake
On some systems, capnp would be installed into `lib64`, I assume due to the use of GNUInstallDirs, however all other libs we build in depends, go into lib/. Rather than adding lib64/ to the pkg-config and link flags, I opted for always installing into lib/. Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
2023-12-12Merge bitcoin/bitcoin#29059: Revert "ci: Only run functional tests on ↵fanquake
windows in master" 7b22cd80e050b903b5765133b8116f4b17ce0296 Revert "ci: Only run functional tests on windows in master" (Hennadii Stepanov) Pull request description: This PR reverts commit aba4a5887b44bf7cbee9ea0a8e02bb92c1b4147b from https://github.com/bitcoin/bitcoin/pull/28567. The Windows-specific code received [quality](https://github.com/bitcoin/bitcoin/pull/28486) and [performance](https://github.com/bitcoin/bitcoin/pull/29045) improvements recently. So there are no reasons to skip functional tests in PRs anymore. In my own repo, I've run the GHA Windows job more than 100 times with no failure. ACKs for top commit: maflcko: lgtm ACK 7b22cd80e050b903b5765133b8116f4b17ce0296 TheCharlatan: ACK 7b22cd80e050b903b5765133b8116f4b17ce0296 Tree-SHA512: 1e8687e8efe12db506e7cd2d5df9e48b5acb98a339f84684dd0fd67280e22227e2a5a206f1108b09e49038fab7a3ca2ffbd985677f00048c0962b39b2b9a2ba5
2023-12-12Merge bitcoin/bitcoin#29055: tests, bench: Fix issue with ↵fanquake
CWallet::LoadWallet() being called in the wrong places bd7f5d33e3b9e01cd600afe1682b7afa935f68a3 wallet: Assert that the wallet is not initialized in LoadWallet (Andrew Chow) fb0b6ca4e5d981cf58bf23ae2993117f171608e8 tests, bench: Remove incorrect LoadWallet() calls (Andrew Chow) Pull request description: `CWallet::LoadWallet()` expects to be called after a `CWallet` is constructed, but before any of its member functions called. Doing so invalidates pointers which causes issues with some PRs and branches that I am working on. This was being used incorrectly in a few tests and benchmarks, resulting in segfaults. As a precaution for this kind of issue in the future, I've also added a few asserts to `LoadWallet()` so that developers will notice when it is used incorrectly. As similar issue was fixed in #27666 ACKs for top commit: S3RK: ACK bd7f5d33e3b9e01cd600afe1682b7afa935f68a3 furszy: ACK bd7f5d33 Tree-SHA512: 7664f12b8452994e7fc4d7d4f77697fb5f75edb0dba95ba99a4a23ec03d5b8e0ecbdcb7635547a0e8b4f89f708f98dcb5d039df0559e24b1ae411ed630e16e14
2023-12-12Merge bitcoin/bitcoin#29052: doc/reduce-traffic: update/clarify max outbound ↵fanquake
connection count d58f89d355faf46b68d0ff8095699d0aff41959c doc: update/clarify max outbound connection count (Marnix) Pull request description: closes https://github.com/bitcoin/bitcoin/issues/29046 ACKs for top commit: amitiuttarwar: ACK d58f89d355faf46b68d0ff8095699d0aff41959c brunoerg: ACK d58f89d355faf46b68d0ff8095699d0aff41959c Tree-SHA512: 4f6e8596ceea0b655338b3fc90a465e319d81e2a006f22a98cd9acada9316e7eb2340e2f66b1b4dde4e63ae94f57efddec50478715608e1be3212b8e2d1c3a1a
2023-12-12Merge bitcoin/bitcoin#29021: refactor: rpc: Pass CBlockIndex by reference ↵fanquake
instead of pointer fa5989d514d246e56977c528b2dd2abe6dc8efcc refactor: rpc: Pass CBlockIndex by reference instead of pointer (MarcoFalke) fa604eb6cfa7f70ce11c78c1060f0823884c745b refactor: Use reference instead of pointer in IsBlockPruned (MarcoFalke) Pull request description: Follow-up to https://github.com/bitcoin/bitcoin/pull/29003#issuecomment-1841435462 ACKs for top commit: TheCharlatan: ACK fa5989d514d246e56977c528b2dd2abe6dc8efcc pablomartin4btc: tACK fa5989d514d246e56977c528b2dd2abe6dc8efcc dergoegge: Code review ACK fa5989d514d246e56977c528b2dd2abe6dc8efcc Tree-SHA512: 7449de3e3bb435dcbf438df88df343bb70f6edc3228ee7c0078f912ffb415e951ba30f8ecad916765f8cf896f0d784fe30535c5cf997e303cf5af257ade69773
2023-12-12Revert "ci: Only run functional tests on windows in master"Hennadii Stepanov
This reverts commit aba4a5887b44bf7cbee9ea0a8e02bb92c1b4147b.
2023-12-11fuzz: disable BnB when SFFO is enabledfurszy
2023-12-11test: add coverage for BnB-SFFO restrictionfurszy
Verify the transaction creation process does not produce a BnB solution when SFFO is enabled. This is currently problematic because it could require a change output. And BnB is specialized on changeless solutions. Co-authored-by: Andrew Chow <achow101@gmail.com> Co-authored-by: Murch <murch@murch.one>
2023-12-11wallet: Assert that the wallet is not initialized in LoadWalletAndrew Chow
LoadWallet() cannot be run after the wallet has been initialized. So assert that to avoid making this mistake in the future.
2023-12-11tests, bench: Remove incorrect LoadWallet() callsAndrew Chow
LoadWallet() must only be called immediately after a CWallet is constructed, or not at all. Doing so after any other CWallet member functions have been called may cause pointers and other objects setup by other those functions to become invalidated. Since these tests and benchmarks are using completely new wallets with mock databases, it's not necessary to call LoadWallet() anyways, so these can be dropped.
2023-12-11doc: update/clarify max outbound connection countMarnix
2023-12-11doc: Update bitcoin-tx replaceable documentationKashif Smith
2023-12-11Merge bitcoin/bitcoin#28999: build: Enable -Wunreachable-codefanquake
fa8adbe7c17b16cf7ecd16eb9f3f1792e9da2876 build: Enable -Wunreachable-code (MarcoFalke) Pull request description: It seems a bit confusing to write code after a `return`. This can even lead to bugs, or incorrect code, such as https://github.com/bitcoin/bitcoin/pull/28830/files#r1415372320 . (Edit: The linked instance is not found by clang's `-Wunreachable-code`). Fix all issues by enabling `-Wunreachable-code`. This flag also enables `-Wunreachable-code-loop-increment`, according to https://clang.llvm.org/docs/DiagnosticsReference.html#wunreachable-code, so remove that. ACKs for top commit: ajtowns: > ACK [fa8adbe](https://github.com/bitcoin/bitcoin/commit/fa8adbe7c17b16cf7ecd16eb9f3f1792e9da2876) stickies-v: ACK fa8adbe7c17b16cf7ecd16eb9f3f1792e9da2876 jonatack: ACK fa8adbe7c17b16cf7ecd16eb9f3f1792e9da2876 tested with arm64 clang 17.0.6 Tree-SHA512: 12a2f74b69ae002e62ae08038f7458837090a12051a4c154d05ae4bb26fb19fc1fa76c63aedf2b3fbb36f048c593ca3b8c0efe03fe93cf07a0fd114fc84ce1e7
2023-12-11Merge bitcoin/bitcoin#25273: wallet: Pass through transaction locktime and ↵fanquake
preset input sequences and scripts to CreateTransaction 0295b44c257fe23f1ad344aff11372d67864f0db wallet: return CreatedTransactionResult from FundTransaction (Andrew Chow) 758501b71391136c33b525b1a0109b990d4f463e wallet: use optional for change position as an optional in CreateTransaction (Andrew Chow) 2d39db7aa128a948b6ad11242591ef26a342f5b1 wallet: Explicitly preserve scriptSig and scriptWitness in CreateTransaction (Andrew Chow) 14e50746f683361f4d511d384d6f1dc44ed2bf10 wallet: Explicitly preserve transaction version in CreateTransaction (Andrew Chow) 0fefcbb776063b7fcc03c28e544d830a2f540250 wallet: Explicitly preserve transaction locktime in CreateTransaction (Andrew Chow) 4d335bb1e00a414a4740007d5a192a73179b2262 wallet: Set preset input sequence through coin control (Andrew Chow) 596642c5a9f52dda2599b0bde424366bb22b3c6e wallet: Replace SelectExternal with SetTxOut (Andrew Chow) 5321786b9d90eaf35059bb07e6beaaa2cbb257ac coincontrol: Replace HasInputWeight with returning optional from Get (Andrew Chow) e1abfb5b2037ca4fe5a05aa578030c8016491c8b wallet: Introduce and use PreselectedInput class in CCoinControl (Andrew Chow) Pull request description: Currently `FundTransaction` handles transaction locktime and preset input data by extracting the selected inputs and change output from `CreateTransaction`'s results. This means that `CreateTransaction` is actually unaware of any user desired locktime or sequence numbers. This can have an effect on whether and how anti-fee-sniping works. This PR makes `CreateTransaction` aware of the locktime and preset input data by providing them to `CCoinControl`. `CreateTransasction` will then set the sequences, scriptSigs, scriptWItnesses, and locktime as appropriate if they are specified. This allows `FundTransaction` to actually use `CreateTransaction`'s result directly instead of having to extract the parts of it that it wants. Additionally `FundTransaction` will return a `CreateTransactionResult` as `CreateTransaction` does instead of having several output parameters. Lastly, instead of using `-1` as a magic number for the change output position, the change position is changed to be an optional with no value set indicating no desired change output position (when provided as an input parameter) or no change output present (in the result). ACKs for top commit: josibake: ACK https://github.com/bitcoin/bitcoin/commit/0295b44c257fe23f1ad344aff11372d67864f0db S3RK: Code review ACK 0295b44c257fe23f1ad344aff11372d67864f0db Tree-SHA512: 016be4d41cbf97e1938506e70959bb5335b87006162a1c1c62fa0adb637cbe7aefb76d342b8efad5f37dc693f270c8d0a0839e239fd1ac32c6941a8172f1a710
2023-12-11Merge bitcoin/bitcoin#29009: fuzz: p2p: Detect peer deadlocksfanquake
9f265d88253ed464413dea5614fa13dea0d8cfd5 fuzz: Detect deadlocks in process_message (dergoegge) fae1e7e012571201fd51c547ba4fb6044be9aeb5 fuzz: p2p: Detect peer deadlocks (MarcoFalke) Pull request description: It may be possible that a peer connection will deadlock, due to software bugs such as https://github.com/bitcoin/bitcoin/pull/18808. Fix this by detecting them in the fuzz target. Can be tested by introducing a bug such as: ```diff diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 1067341495..97495a13df 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -2436,3 +2436,3 @@ void PeerManagerImpl::ProcessGetData(CNode& pfrom, Peer& peer, const std::atomic if (it != peer.m_getdata_requests.end() && !pfrom.fPauseSend) { - const CInv &inv = *it++; + const CInv& inv = *it; if (inv.IsGenBlkMsg()) { ``` Using a fuzz input such as: ``` $ base64 ./timeout-ada0fecaba2b8c46c6e970cf637d9625b01bf7e5 kNptdNbW1tbWYghvXIpwb25vPQAA////////cwAjLv8AXAB2ZXJhY2sAQW5v/62tra3Pz/////// //////////////////////9c8GZpbHRlcmxvYWQAAAEAAwAAAABVYwC2XABmaWx0ZXJhZGQAAAAX Fxdn/////2V0F861tcqvEmAAACEAAABjYXB0dXJldmUAAH4AgAA1PNfX11x0Z2V0ZGF0YQBDACOw AQMAAAAGIm5GERoLWcqvEmBD61u/KMNPOl4zKh/HKLK3PPGIkQ9eE/////////8AAAAAAAAAAFtb WyjDTzpeMSofx7K3PNfX11x0Z2V0ZGF0YQBDACMwAQMAAAAGIm5GERoLWcqvEmBD61u/KMNPOl4z Kh/Hsrc88YiRD2/Nzc3Nzc3Nzc3NTc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3Nzc3N zWWj1NTUudTU1NTU1P///0j+P/9cdHR4AAAAAAAAy/4AAHR4AAAAAAAAP8v+AAD/+P////////// AX55bJl8HWnz/////wAgXGF0YVPxY2RkAAAA ``` And running the fuzz target: ``` $ FUZZ=process_messages ./src/test/fuzz/fuzz -runs=1 -timeout=18 ./timeout-ada0fecaba2b8c46c6e970cf637d9625b01bf7e5 INFO: Running with entropic power schedule (0xFF, 100). INFO: Seed: 3436516708 INFO: Loaded 1 modules (390807 inline 8-bit counters): 390807 [0x55d0d6221e80, 0x55d0d6281517), INFO: Loaded 1 PC tables (390807 PCs): 390807 [0x55d0d6281518,0x55d0d6877e88), ./src/test/fuzz/fuzz: Running 1 inputs 1 time(s) each. Running: ./timeout-ada0fecaba2b8c46c6e970cf637d9625b01bf7e5 ALARM: working on the last Unit for 19 seconds and the timeout value is 18 (use -timeout=N to change) ==375014== ERROR: libFuzzer: timeout after 19 seconds ``` ACKs for top commit: naumenkogs: ACK 9f265d88253ed464413dea5614fa13dea0d8cfd5 dergoegge: ACK 9f265d88253ed464413dea5614fa13dea0d8cfd5 brunoerg: ACK 9f265d88253ed464413dea5614fa13dea0d8cfd5 Tree-SHA512: da83ff90962bb679aae00e8e9dba639c180b7aaba544e0c4d0978d36e28a9ff1cd7a2e13009d8ab407ef57767656aca1ebc767a7d2f1bc880284f8f57c197a50
2023-12-11Merge bitcoin/bitcoin#29031: fuzz: Improve fuzzing stability for txorphan ↵fanquake
harness 15f5a0d0c8ce6b306cdeba6a4777334b848a76aa fuzz: Improve fuzzing stability for txorphan harness (dergoegge) Pull request description: The `txorphan` harness has low stability as eviction of orphan txs is entirely random at the moment. Fix this by passing the rng to `LimitOrphans`, which can be deterministic in tests. Also see #29018. ACKs for top commit: maflcko: lgtm ACK 15f5a0d0c8ce6b306cdeba6a4777334b848a76aa brunoerg: utACK 15f5a0d0c8ce6b306cdeba6a4777334b848a76aa Tree-SHA512: 854ec34b3a0f16f26db6dc419096c6e7a380e8400119534aa278d6b1d54c253b572aa2fad13c383c796c431d8ff4263956e6f60326e99f8bf6abd16d9a280e97
2023-12-11Merge bitcoin/bitcoin#29044: msvc: Define the same `QT_...` macros as in ↵fanquake
Autotools builds 1a5dae630df1eef9eac51557b2f1c5dba0924953 msvc: Define the same `QT_...` macros as in Autotools builds (Hennadii Stepanov) Pull request description: There are no reasons to have such a diversion. Also it fixes https://github.com/bitcoin/bitcoin/pull/28960#issuecomment-1847971114. ACKs for top commit: sipsorcery: tACK 1a5dae630df1eef9eac51557b2f1c5dba0924953. TheCharlatan: ACK 1a5dae630df1eef9eac51557b2f1c5dba0924953 Tree-SHA512: 75be5eabb8fec974b8d77a023c72323015a3d95fbc13b7fd85e5f25c250ae67850ddf0bcaef143828d75fe35a49e7c9b1966976b74f3ce7d14465174e6585ceb
2023-12-11Merge bitcoin/bitcoin#29041: test: fix intermittent error in rpc_net.py (#29030)fanquake
ea00f982d21aab51001d422225f00626a74db298 test: fix intermittent error in rpc_net.py (#29030) (Sebastian Falbesoner) Pull request description: Asserting for the debug log message "Added connection peer=" is insufficient for ensuring that this new connection will show up in a following getpeerinfo() call, as the debug message is written in the CNode ctor, which means it hasn't necessarily been added to CConnman.m_nodes at this point. Solve this by using the recently introduced `wait_for_new_peer` helper (see #29006, commit 00e0658e77f66103ebdeb29def99dc9f937c049d), which is more robust. Fixes #29030. ACKs for top commit: maflcko: lgtm ACK ea00f982d21aab51001d422225f00626a74db298 Tree-SHA512: dda307949a466fb3b24408a8c213d307e0af2155f2e8b4e52c836a22397f9d218bf9d8c54ca55bae62a96d7566f27167db9311dd8801785c327234783af5ed00
2023-12-11Merge bitcoin/bitcoin#29035: test: fix `addnode` functional test failure on ↵fanquake
OpenBSD fd0bde2793239bd6d60a2435fa28df915cedd7e6 test: fix `addnode` functional test failure on OpenBSD (Sebastian Falbesoner) Pull request description: This is the functional test counterpart of PR #28891 / commit 007d6f0e85bc329040bb405ef6016339518caa66 (unfortunately, I missed it back then and only ran the unit tests -- sorry for the noise). master branch on OpenBSD 7.4: ``` $ ./test/functional/rpc_net.py 2023-12-08T17:29:05.057000Z TestFramework (INFO): PRNG seed is: 6024296850131317403 2023-12-08T17:29:05.058000Z TestFramework (INFO): Initializing test directory /tmp/bitcoin_func_test_au3zchif 2023-12-08T17:29:05.618000Z TestFramework (INFO): Test getconnectioncount 2023-12-08T17:29:05.618000Z TestFramework (INFO): Test getpeerinfo 2023-12-08T17:29:06.643000Z TestFramework (INFO): Check getpeerinfo output before a version message was sent 2023-12-08T17:29:06.709000Z TestFramework (INFO): Test getnettotals 2023-12-08T17:29:06.773000Z TestFramework (INFO): Test getnetworkinfo 2023-12-08T17:29:06.978000Z TestFramework (INFO): Test addnode and getaddednodeinfo 2023-12-08T17:29:06.980000Z TestFramework (ERROR): Assertion failed Traceback (most recent call last): File "/home/thestack/bitcoin/test/functional/test_framework/test_framework.py", line 131, in main self.run_test() File "/home/thestack/bitcoin/./test/functional/rpc_net.py", line 65, in run_test self.test_addnode_getaddednodeinfo() File "/home/thestack/bitcoin/./test/functional/rpc_net.py", line 224, in test_addnode_getaddednodeinfo assert_raises_rpc_error(-23, "Node already added", self.nodes[0].addnode, node=ip_port2, command='add') File "/home/thestack/bitcoin/test/functional/test_framework/util.py", line 131, in assert_raises_rpc_error assert try_rpc(code, message, fun, *args, **kwds), "No exception raised" AssertionError: No exception raised ``` On the PR branch, the same call succeeds. ACKs for top commit: kevkevinpal: ACK [fd0bde2](https://github.com/bitcoin/bitcoin/pull/29035/commits/fd0bde2793239bd6d60a2435fa28df915cedd7e6) Tree-SHA512: ae20816fa4025c212e115ebd267b5e5784bfcdf0051219eb686faaade47ec4f91a3947af6d24258b159290000d2dcc3f6e65e788b83b8a9297282945dbdafbfb
2023-12-11Merge bitcoin/bitcoin#29045: msvc: Optimize "Release" buildsfanquake
6e0f1d2abbb700d4fd4b956a7d1f9505b653653c msvc: Optimize "Release" builds (Hennadii Stepanov) Pull request description: It is awkward not using optimization. In addition to the obvious benefits for Windows users, this PR reduces the duration of functional tests by an hour. Picked from https://github.com/bitcoin/bitcoin/pull/24773. ACKs for top commit: sipsorcery: tACK 6e0f1d2abbb700d4fd4b956a7d1f9505b653653c. Tree-SHA512: 5aa7fd38cb1a81d58ea3206756a8099891866c82a747d3b8079cab0b2afa1f40ba53adff2f32eb233efcd1227babee80ab175e35a83678fafa8a4f63c356e5ca
2023-12-11Merge bitcoin/bitcoin#29048: Add a note to msvc readme re building Qt for ↵fanquake
Bitcoin Core. d08e820abf5da2be09b8a84b5bd3450d1a55a039 Add a note to msvc readme re building Qt for Bitcoin Core. (Aaron Clauson) Pull request description: Updated the msvc readme with a note about avoiding path too long errors when building Qt with Bitcoin Core. Would have saved me half an hour if I'd remembered this from the last time I did the build. ACKs for top commit: hebasto: ACK d08e820abf5da2be09b8a84b5bd3450d1a55a039. TheCharlatan: ACK d08e820abf5da2be09b8a84b5bd3450d1a55a039 Tree-SHA512: f51017b15383dbcd39ad1e5e978bb255b9205dc23d72b5e3530c6aefcbbc2dc4ec3a85e5fc8c0019c8511173c298f80b837cb35f268deac424d19365b25fb335
2023-12-10Add a note to msvc readme re building Qt for Bitcoin Core.Aaron Clauson
2023-12-09msvc: Define the same `QT_...` macros as in Autotools buildsHennadii Stepanov
2023-12-09msvc: Optimize "Release" buildsHennadii Stepanov
It is awkward not using optimization.
2023-12-09test: fix intermittent error in rpc_net.py (#29030)Sebastian Falbesoner
Asserting for the debug log message "Added connection peer=" is insufficient for ensuring that this new connection will show up in a following getpeerinfo() call, as the debug message is written in the CNode ctor, which means it hasn't necessarily been added to CConnman.m_nodes at this point. Solve this by using the recently introduced `wait_for_new_peer` helper, which is more robust. Fixes #29030.
2023-12-08tests: Add unit tests for bitcoin-tx replaceable commandKashif Smith
2023-12-08wallet: return CreatedTransactionResult from FundTransactionAndrew Chow
Instead of using the output parameters, return CreatedTransactionResult from FundTransaction in the same way that CreateTransaction does. Additionally, instead of modifying the original CMutableTransaction, the result from CreateTransactionInternal is used.
2023-12-08wallet: use optional for change position as an optional in CreateTransactionAndrew Chow
Instead of making -1 a magic number meaning no change or random change position, use an optional to have that meaning.
2023-12-08wallet: Explicitly preserve scriptSig and scriptWitness in CreateTransactionAndrew Chow
When creating a transaction with preset inputs, also preserve the scriptSig and scriptWitness for those preset inputs if they are provided (e.g. in fundrawtransaction).
2023-12-08wallet: Explicitly preserve transaction version in CreateTransactionAndrew Chow
We provide the preset nVersion to CCoinControl so that CreateTransactionInternal can be aware of it and set it in the produced transaction.
2023-12-08wallet: Explicitly preserve transaction locktime in CreateTransactionAndrew Chow
We provide the preset nLockTime to CCoinControl so that CreateTransactionInternal can be aware of it and set it in the produced transaction.
generated by cgit v1.2.3 (git 2.26.2) at 2024-11-10 07:11:26 +0000