4ba1d0b553 fuzz: Add coverage for client_maxfeerate (Greg Sanders)
91d7d8f22a AcceptMultipleTransactions: Fix workspace client_maxfeerate (Greg Sanders)
f3aa5bd5eb fill_mempool: assertions and docsctring update (Greg Sanders)
a3da63e8fe Move fill_mempool to util function (Greg Sanders)
73b68bd8b4 fill_mempool: remove subtest-specific comment (Greg Sanders)
Pull request description:
Bug causes an `Assume()` failure due to the expectation that the individual result should be invalid when done over `submitpackage` via rpc.
Bug introduced by https://github.com/bitcoin/bitcoin/pull/28950 , and I discovered it rebasing https://github.com/bitcoin/bitcoin/pull/28984 since it's easier to hit in that test scenario.
Tests in place were only checking `AcceptSingleTransaction`-level checks due to package evaluation only triggering when minfee is too high for the parent transaction.
Added test along with fix, moving the fill_mempool utility into a common area for re-use.
ACKs for top commit:
glozow:
reACK 4ba1d0b553
theStack:
ACK 4ba1d0b553
ismaelsadeeq:
re-ACK 4ba1d0b553 via [diff](4fe7d150eb..4ba1d0b553)
Tree-SHA512: 3729bdf7f25d04e232f173ccee04ddbb2afdaafa3d04292a01cecf58fb11b3b2bc133e8490277f1a67622b62d17929c242dc980f9bb647896beea4332ee35306
95c594f4e9 depends: remove no longer needed patch for Boost::Process (Sebastian Falbesoner)
Pull request description:
As Boost::Process has been replaced by cpp-subprocess (PR #28981), this patch touches an unused code part and is hence not needed anymore.
ACKs for top commit:
hebasto:
ACK 95c594f4e9, I have reviewed the code and it looks OK.
fanquake:
ACK 95c594f4e9
Tree-SHA512: 0309b826f8c260e4180624f17302e51329fc4bd7a5431997d6d27d468dd5f7dbcd9db6a742efaba33ba30dbe361830eb1446fdbec927505ccf42412f9211934e
d5a715536e build: remove boost::process dependency for building external signer support (Sebastian Falbesoner)
70434b1c44 external_signer: replace boost::process with cpp-subprocess (Sebastian Falbesoner)
cc8b9875b1 Add `cpp-subprocess` header-only library (Hennadii Stepanov)
Pull request description:
Closes https://github.com/bitcoin/bitcoin/issues/24907.
This PR is based on **theStack**'s [work](https://github.com/bitcoin/bitcoin/issues/24907#issuecomment-1466087049).
The `subprocess.hpp` header has been sourced from the [upstream repo](https://github.com/arun11299/cpp-subprocess) with the only modification being the removal of convenience functions, which are not utilized in our codebase.
Windows-related changes will be addressed in subsequent follow-ups.
ACKs for top commit:
achow101:
reACK d5a715536e
Sjors:
re-tACK d5a715536e
theStack:
Light re-ACK d5a715536e
fanquake:
ACK d5a715536e - with the expectation that this code is going to be maintained as our own. Next PRs should:
Tree-SHA512: d7fb6fecc3f5792496204190afb7d85b3e207b858fb1a75efe483c05260843b81b27d14b299323bb667c990e87a07197059afea3796cf218ed8b614086bd3611
a71eadf66b Change MAC_OSX macro to __APPLE__ in crypto package (Lőrinc)
Pull request description:
Split out from https://github.com/bitcoin/bitcoin/pull/29450#issuecomment-2044405345 to avoid the uncertainties and simplify review.
ACKs for top commit:
theuni:
ACK a71eadf66b
fanquake:
ACK a71eadf66b
Tree-SHA512: b6a7bd7ca95585dd9110cefe7c1213f4a1a72bdfc88670abf4a0d9a8bbc12e093544524adce46aa9ca714c472d417f74ca4a678af682ed2488053059434eaa02
a8203e9412 refactor: Simplify `extra_txn` to be a vec of CTransactionRef instead of a vec of pair<Wtxid, CTransactionRef> (AngusP)
c3c18433ae refactor: Use typesafe Wtxid in compact block encoding message, instead of ambiguous uint256. (AngusP)
Pull request description:
The first commit replaces `uint256` with typesafe `Wtxid` (or `Txid`) types introduced in #28107.
The second commit then simplifies the extra tx `vector` to just be of `CTransactionRef`s instead of a `std::pair<Wtxid, CTransactionRef>`, as it's easy to get a `Wtxid` from a transaction ref.
ACKs for top commit:
glozow:
ACK a8203e9412
dergoegge:
ACK a8203e9412
Tree-SHA512: b4ba1423a8059f9fc118782bd8a668213d229c822f22b01267de74d6ea97fe4f2aad46b5da7c0178ecc9905293e9a0eafba1d75330297c055e27fd53c8c8ebfd
47cedee776 fuzz: Introduce `BITCOINFUZZ` environment variable (Hennadii Stepanov)
1573e9a11e fuzz, refactor: Deduplicate fuzz binary path creation (Hennadii Stepanov)
Pull request description:
These changes are split from https://github.com/bitcoin/bitcoin/pull/29774 and can be beneficial on their own.
The new `BITCOINFUZZ` environment variable complements the already existing set of variables used by tests: b5d21182e5/test/functional/test_framework/test_framework.py (L238-L243)
ACKs for top commit:
maflcko:
lgtm ACK 47cedee776
davidgumberg:
utACK 47cedee776
Tree-SHA512: 45809cfd13dc4a45c44cc433184352e84726cb95bea80fd8f581c59a0b8b0a5495260ff66922f9c57c38adbdbdd102439238f370fd49d6ea27a241a5e6249895
03b87a3e64 Drop Windows Socket dependency for `randomenv.cpp` (Hennadii Stepanov)
Pull request description:
This change drops a dependency on the ws2_32 library for our libbitcoinkernel by switching to [`GetComputerName`](https://learn.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-getcomputernamew) function.
ACKs for top commit:
sipsorcery:
utACK 03b87a3e64.
laanwj:
Code review ACK 03b87a3e64.
fanquake:
ACK 03b87a3e64
Tree-SHA512: a4abd5499176634d5f3fbf4e794a7504c40232fb73bd7f41955fbfb2cc7c44bc7ea4518c5203836e52f552c30414c6c3e1b24f0922641dbf1c8377981c0ffaf0
d4e36ae80d test: Update --tmpdir doc string to say directory must not exist (kevkevin)
Pull request description:
The error message given if passing an existing dir to --tmpdir is confusing so this makes it clear that the directory must not already exist
This change is motivated by this comment https://github.com/bitcoin/bitcoin/pull/29335#issuecomment-1960913020
ACKs for top commit:
maflcko:
lgtm ACK d4e36ae80d
davidgumberg:
ACK d4e36ae80d
Tree-SHA512: fb31fd079767abbf94076615817943f35f5c9262fc97e65c631a18d33b3a343fe6a2d151613256e632d2b372ab2de0435f4712309b4a77ed3c663fd93a7dcdd1
5efebc0edb depends: add the new LLVM debug macro (fanquake)
Pull request description:
`LIBCXX_HARDENING_MODE` is the new macro, the previous one was removed in LLVM 18.
See https://libcxx.llvm.org/Hardening.html.
Required before https://github.com/google/oss-fuzz/pull/11725 will do anything (with the bump to 18.x).
Seems reasonable to do now that almost all our test infra is using LLVM 18.
ACKs for top commit:
theuni:
ACK 5efebc0edb
Tree-SHA512: 43078eeb5940c55ef4f95c72682f8a372dcd3eb97956b3114149c16d9f59b067a999b2aab7f34ffb57eab191524514408e2bba154ff4a6ea0cd6ec4d119c5d18
4f273ab436 Change Luke Dashjr seed to dashjr-list-of-p2p-nodes.us (Luke Dashjr)
Pull request description:
To avoid issues with DNS blacklisting, I've setup a separate domain for my DNS seed.
(This time, without a potentially alarming name)
ACKs for top commit:
kevkevinpal:
Concept ACK [4f273ab](4f273ab436), name looks good to me
petertodd:
ACK 4f273ab436
mzumsande:
ACK 4f273ab436
fanquake:
ACK 4f273ab436
Tree-SHA512: 689698e3c735df3ed0c2756a9d4adb5644bb9d8a6954e23d66bfa9d94ee10954f77fb241d9593f750054d731aa1532368a0fc8277884f6c2a98ac47cd0bdeeb7
78407b99ed [clang-tidy] Enable the misc-no-recursion check (dergoegge)
Pull request description:
Recursion is a frequent source of stack overflow bugs. Secondly, introduction of recursion can be non-obvious.
This PR proposes to use the clang-tidy `misc-no-recursion` check to make introduction of new recursion obvious. We don't make use of recursion a lot in our code base but there are a few places that need suppressions anyway (mostly the descriptor and univalue/rpc code).
ACKs for top commit:
stickies-v:
ACK 78407b99ed
TheCharlatan:
Re-ACK 78407b99ed
fanquake:
ACK 78407b99ed
Tree-SHA512: 34126d704c46086fe7371906ca852c25ced1dbd5fcfd85bf623810cd171a797569a92a33c7e26b8dc01c30c7bbf81aa326718926e8354585091411989a4edb14
93fae5ae7c test: remove immediate tx relay workaround in wallet_groups.py (Sebastian Falbesoner)
Pull request description:
Reverts commit ab4efad51b (PR #26970). This workaround is not needed anymore, as since #27114 the test sets the noban permission for both in- and outbound connections via the `noban_tx_relay` setting, and we don't have to rely on this topology hack anymore. See commit c985eb854c (kudos to brunoerg!).
Can be tested by executing `$ time ./test/functional/wallet_groups.py` both on master and PR and verifying that the execution time is roughly equal.
ACKs for top commit:
maflcko:
lgtm ACK 93fae5ae7c
brunoerg:
utACK 93fae5ae7c
Tree-SHA512: b949fd05b4308815ba02d0ee4d1318f642b930288dd03223f46db7db745177af1c070bc7058743ac27963c5ad90564089867cc12f31fee94812a16919c353bab
61641e2466 ci: remove --with-asm usage (secp256k1) (fanquake)
c7efee591a ci: use LLVM 18.1.3 in MSAN jobs (fanquake)
Pull request description:
Bumps LLVM to `18.1.3`:
* Includes https://github.com/llvm/llvm-project/pull/86201, which is useful as it removes the need to (possibly) apply a work around when running the CI locally.
Drops `--with-asm=no` (only being passed to secp256k1) from the MSAN CI. New MSAN annotations were pulled in as part of #29803.
ACKs for top commit:
maflcko:
lgtm ACK 61641e2466
hebasto:
ACK 61641e2466.
Tree-SHA512: da51c9f08a9aacb9dd936c47ef47777a8c84234e4df5b9776647ac94ebe88084b5e7b8182af90cfa01ae183072f6ce5915b73825f66b2567214ab270b2ff7837
49c0b8b228 test: Bump timeouts in feature_index_prune and wallet_importdescriptors (Christopher Bergqvist)
Pull request description:
Timeout issues where encountered when running functional tests with `--jobs=16 --extended`.
Note that running `--extended` without `--jobs=16` does not trigger the issues.
Tested under NixOS on a Xeon CPU with 16 logical cores.
(A few tests are skipped locally as I haven't enabled BPF and a few other things).
## Measurements
Line in `feature_index_prune.py` took 101.6s, 96.6s, 103.0s across 3 runs on my machine.
Default limit is 60, suggested to increase limit to 150 seconds.
Line in the `wallet_importdescriptors.py --descriptors` took 5.4s, 5.7s, 6.0s across 3 runs.
Suggested to increase from 5 to 10 seconds.
## Logs
Output slightly modified by separate change that lets code run past given timeouts and the provides more information - "Took 101.6 seconds to complete, 69.4% over the given limit.".
<details>
<summary>
Click to expand.
</summary>
### feature_index_prune.py
```
52/305 - feature_index_prune.py failed, Duration: 250 s
stdout:
2024-04-01T22:25:24.010000Z TestFramework (INFO): PRNG seed is: 990421162716295219
2024-04-01T22:25:24.014000Z TestFramework (INFO): Initializing test directory /mnt/tmp/test_runner_₿_🏃_20240402_002516/feature_index_prune_302
2024-04-01T22:25:24.913000Z TestFramework (INFO): check if we can access blockfilters and coinstats when pruning is enabled but no blocks are actually pruned
2024-04-01T22:26:48.417000Z TestFramework (INFO): prune some blocks
2024-04-01T22:26:48.460000Z TestFramework (INFO): check if we can access the tips blockfilter and coinstats when we have pruned some blocks
2024-04-01T22:26:48.483000Z TestFramework (INFO): check if we can access the blockfilter and coinstats of a pruned block
2024-04-01T22:26:59.175000Z TestFramework (INFO): make sure trying to access the indices throws errors
2024-04-01T22:27:50.422000Z TestFramework (INFO): prune exactly up to the indices best blocks while the indices are disabled
2024-04-01T22:27:52.596000Z TestFramework (INFO): make sure that we can continue with the partially synced indices after having pruned up to the index height
2024-04-01T22:29:34.242000Z TestFramework.utils (ERROR): wait_until() failed. Predicate: '''
self.wait_until(lambda: self.nodes[1].getindexinfo() == expected_stats)#, timeout=150)
'''
2024-04-01T22:29:34.244000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
File "/home/chris/Documents/Code/bitcoin-core/test/functional/test_framework/test_framework.py", line 132, in main
self.run_test()
File "/home/chris/Documents/Code/bitcoin-core/test/functional/feature_index_prune.py", line 117, in run_test
self.sync_index(height=1500)
File "/home/chris/Documents/Code/bitcoin-core/test/functional/feature_index_prune.py", line 34, in sync_index
self.wait_until(lambda: self.nodes[1].getindexinfo() == expected_stats)#, timeout=150)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/chris/Documents/Code/bitcoin-core/test/functional/test_framework/test_framework.py", line 780, in wait_until
return wait_until_helper_internal(test_function, timeout=timeout, timeout_factor=self.options.timeout_factor)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/chris/Documents/Code/bitcoin-core/test/functional/test_framework/util.py", line 305, in wait_until_helper_internal
raise AssertionError(m)
AssertionError: Predicate '''
self.wait_until(lambda: self.nodes[1].getindexinfo() == expected_stats)#, timeout=150)
''' not true after 60 seconds. Took 101.6 seconds to complete, 69.4% over the given limit.
2024-04-01T22:29:34.298000Z TestFramework (INFO): Stopping nodes
2024-04-01T22:29:34.511000Z TestFramework (WARNING): Not cleaning up dir /mnt/tmp/test_runner_₿_🏃_20240402_002516/feature_index_prune_302
2024-04-01T22:29:34.511000Z TestFramework (ERROR): Test failed. Test logging available at /mnt/tmp/test_runner_₿_🏃_20240402_002516/feature_index_prune_302/test_framework.log
2024-04-01T22:29:34.511000Z TestFramework (ERROR):
2024-04-01T22:29:34.512000Z TestFramework (ERROR): Hint: Call /home/chris/Documents/Code/bitcoin-core/test/functional/combine_logs.py '/mnt/tmp/test_runner_₿_🏃_20240402_002516/feature_index_prune_302' to consolidate all logs
2024-04-01T22:29:34.512000Z TestFramework (ERROR):
2024-04-01T22:29:34.512000Z TestFramework (ERROR): If this failure happened unexpectedly or intermittently, please file a bug and provide a link or upload of the combined log.
2024-04-01T22:29:34.512000Z TestFramework (ERROR): https://github.com/bitcoin/bitcoin/issues
2024-04-01T22:29:34.512000Z TestFramework (ERROR):
stderr:
53/305 - p2p_blockfilters.py passed, Duration: 130 s
```
### wallet_importdescriptors.py --descriptors
```
297/305 - wallet_importdescriptors.py --descriptors failed, Duration: 76 s
stdout:
2024-04-01T22:48:27.663000Z TestFramework (INFO): PRNG seed is: 8528678505617325332
2024-04-01T22:48:27.664000Z TestFramework (INFO): Initializing test directory /mnt/tmp/test_runner_₿_🏃_20240402_004231/wallet_importdescriptors_98
2024-04-01T22:48:28.021000Z TestFramework (INFO): Setting up wallets
2024-04-01T22:48:28.100000Z TestFramework (INFO): Mining coins
2024-04-01T22:48:29.714000Z TestFramework (INFO): Import should fail if a descriptor is not provided
2024-04-01T22:48:29.725000Z TestFramework (INFO): Should import a p2pkh descriptor
2024-04-01T22:48:29.740000Z TestFramework (INFO): Test can import same descriptor with public key twice
2024-04-01T22:48:29.760000Z TestFramework (INFO): Test can update descriptor label
2024-04-01T22:48:29.785000Z TestFramework (INFO): Internal addresses cannot have labels
2024-04-01T22:48:29.788000Z TestFramework (INFO): Internal addresses should be detected as such
2024-04-01T22:48:29.854000Z TestFramework (INFO): Should not import a p2sh-p2wpkh descriptor without checksum
2024-04-01T22:48:29.855000Z TestFramework (INFO): Should not import a p2sh-p2wpkh descriptor that has range specified
2024-04-01T22:48:29.858000Z TestFramework (INFO): Should not import a p2sh-p2wpkh descriptor and have it set to active
2024-04-01T22:48:29.860000Z TestFramework (INFO): Should import a (non-active) p2sh-p2wpkh descriptor
2024-04-01T22:48:29.984000Z TestFramework (INFO): Should import a 1-of-2 bare multisig from descriptor
2024-04-01T22:48:30.002000Z TestFramework (INFO): Should not treat individual keys from the imported bare multisig as watchonly
2024-04-01T22:48:30.005000Z TestFramework (INFO): Ranged descriptors cannot have labels
2024-04-01T22:48:30.014000Z TestFramework (INFO): Private keys required for private keys enabled wallet
2024-04-01T22:48:30.027000Z TestFramework (INFO): Ranged descriptor import should warn without a specified range
2024-04-01T22:48:30.065000Z TestFramework (INFO): Should not import a ranged descriptor that includes xpriv into a watch-only wallet
2024-04-01T22:48:30.070000Z TestFramework (INFO): Should not import a descriptor with hardened derivations when private keys are disabled
2024-04-01T22:48:30.108000Z TestFramework (INFO): Verify we can only extend descriptor's range
2024-04-01T22:48:30.364000Z TestFramework (INFO): Check we can change descriptor internal flag
2024-04-01T22:48:30.536000Z TestFramework (INFO): Key ranges should be imported in order
2024-04-01T22:48:30.708000Z TestFramework (INFO): Check we can change next_index
2024-04-01T22:48:30.838000Z TestFramework (INFO): Check imported descriptors are not active by default
2024-04-01T22:48:30.870000Z TestFramework (INFO): Check can activate inactive descriptor
2024-04-01T22:48:30.903000Z TestFramework (INFO): Check can deactivate active descriptor
2024-04-01T22:48:30.924000Z TestFramework (INFO): Verify activation state is persistent
2024-04-01T22:48:30.973000Z TestFramework (INFO): Should import a descriptor with a WIF private key as spendable
2024-04-01T22:48:30.987000Z TestFramework (INFO): Test can import same descriptor with private key twice
2024-04-01T22:48:32.173000Z TestFramework (INFO): Test that multisigs can be imported, signed for, and getnewaddress'd
2024-04-01T22:48:43.803000Z TestFramework (INFO): Multisig with distributed keys
2024-04-01T22:48:48.895000Z TestFramework (INFO): We can create and use a huge multisig under P2WSH
2024-04-01T22:49:05.628000Z TestFramework (INFO): Under P2SH, multisig are standard with up to 15 compressed keys
2024-04-01T22:49:20.258000Z TestFramework (INFO): Amending multisig with new private keys
2024-04-01T22:49:23.306000Z TestFramework (INFO): Combo descriptors cannot be active
2024-04-01T22:49:23.313000Z TestFramework (INFO): Descriptors with no type cannot be active
2024-04-01T22:49:23.348000Z TestFramework (INFO): Test importing a descriptor to an encrypted wallet
2024-04-01T22:49:43.957000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
File "/home/chris/Documents/Code/bitcoin-core/test/functional/test_framework/test_framework.py", line 132, in main
self.run_test()
File "/home/chris/Documents/Code/bitcoin-core/test/functional/wallet_importdescriptors.py", line 691, in run_test
with self.nodes[0].assert_debug_log(expected_msgs=["Rescan started from block 0f9188f13cb7b2c71f2a335e3a4fc328bf5beb436012afca590b1a11466e2206... (slow variant inspecting all blocks)"], timeout=5):#10):
File "/nix/store/rac8pxbi1vapwrlqzbrkycbyg521djzw-python3-3.11.6/lib/python3.11/contextlib.py", line 144, in __exit__
next(self.gen)
File "/home/chris/Documents/Code/bitcoin-core/test/functional/test_framework/test_node.py", line 493, in assert_debug_log
self._raise_assertion_error(f'Expected messages "{expected_msgs}" found too late, took {now - start:.1f} seconds, {((now - start) / (time_end - start)) - 1:.1%} over the given limit. Log:\n\n{print_log}\n\n')
File "/home/chris/Documents/Code/bitcoin-core/test/functional/test_framework/test_node.py", line 188, in _raise_assertion_error
raise AssertionError(self._node_msg(msg))
AssertionError: [node 0] Expected messages "['Rescan started from block 0f9188f13cb7b2c71f2a335e3a4fc328bf5beb436012afca590b1a11466e2206... (slow variant inspecting all blocks)']" found too late, took 5.4 seconds, 8.9% over the given limit. Log:
- 2024-04-01T22:49:33.066512Z [http] [httpserver.cpp:306] [http_request_cb] [http] Received a POST request for /wallet/encrypted_wallet from 127.0.0.1:47658
- 2024-04-01T22:49:33.066668Z [httpworker.0] [rpc/request.cpp:187] [parse] [rpc] ThreadRPCServer method=importdescriptors user=__cookie__
- 2024-04-01T22:49:33.070999Z [httpworker.0] [wallet/sqlite.cpp:57] [TraceSqlCallback] [/mnt/tmp/test_runner_₿_🏃_20240402_004231/wallet_importdescriptors_98/node0/regtest/wallets/encrypted_wallet/wallet.dat] SQLite Statement: INSERT INTO main VALUES(?, ?)
- 2024-04-01T22:49:33.071061Z [httpworker.0] [wallet/sqlite.cpp:57] [TraceSqlCallback] [/mnt/tmp/test_runner_₿_🏃_20240402_004231/wallet_importdescriptors_98/node0/regtest/wallets/encrypted_wallet/wallet.dat] SQLite Statement: DELETE FROM main WHERE key = ?
- 2024-04-01T22:49:33.071137Z [httpworker.0] [wallet/sqlite.cpp:57] [TraceSqlCallback] [/mnt/tmp/test_runner_₿_🏃_20240402_004231/wallet_importdescriptors_98/node0/regtest/wallets/encrypted_wallet/wallet.dat] SQLite Statement: BEGIN TRANSACTION
- 2024-04-01T22:49:33.074190Z [httpworker.0] [wallet/sqlite.cpp:57] [TraceSqlCallback] [/mnt/tmp/test_runner_₿_🏃_20240402_004231/wallet_importdescriptors_98/node0/regtest/wallets/encrypted_wallet/wallet.dat] SQLite Statement: INSERT or REPLACE into main values(?, ?)
- 2024-04-01T22:49:33.075564Z [httpworker.0] [wallet/sqlite.cpp:57] [TraceSqlCallback] [/mnt/tmp/test_runner_₿_🏃_20240402_004231/wallet_importdescriptors_98/node0/regtest/wallets/encrypted_wallet/wallet.dat] SQLite Statement: INSERT or REPLACE into main values(?, ?)
...<thousands of almost identical lines>...
- 2024-04-01T22:49:38.416139Z [httpworker.0] [wallet/sqlite.cpp:57] [TraceSqlCallback] [/mnt/tmp/test_runner_₿_🏃_20240402_004231/wallet_importdescriptors_98/node0/regtest/wallets/encrypted_wallet/wallet.dat] SQLite Statement: INSERT or REPLACE into main values(?, ?)
- 2024-04-01T22:49:38.416528Z [httpworker.0] [wallet/sqlite.cpp:57] [TraceSqlCallback] [/mnt/tmp/test_runner_₿_🏃_20240402_004231/wallet_importdescriptors_98/node0/regtest/wallets/encrypted_wallet/wallet.dat] SQLite Statement: INSERT or REPLACE into main values(?, ?)
- 2024-04-01T22:49:38.427946Z [httpworker.0] [wallet/sqlite.cpp:57] [TraceSqlCallback] [/mnt/tmp/test_runner_₿_🏃_20240402_004231/wallet_importdescriptors_98/node0/regtest/wallets/encrypted_wallet/wallet.dat] SQLite Statement: COMMIT TRANSACTION
- 2024-04-01T22:49:38.429778Z [httpworker.0] [wallet/sqlite.cpp:57] [TraceSqlCallback] [/mnt/tmp/test_runner_₿_🏃_20240402_004231/wallet_importdescriptors_98/node0/regtest/wallets/encrypted_wallet/wallet.dat] SQLite Statement: INSERT or REPLACE into main values(?, ?)
- 2024-04-01T22:49:38.429916Z [httpworker.0] [wallet/sqlite.cpp:57] [TraceSqlCallback] [/mnt/tmp/test_runner_₿_🏃_20240402_004231/wallet_importdescriptors_98/node0/regtest/wallets/encrypted_wallet/wallet.dat] SQLite Statement: INSERT or REPLACE into main values(?, ?)
- 2024-04-01T22:49:38.430001Z [httpworker.0] [wallet/wallet.h:933] [WalletLogPrintf] [encrypted_wallet] Setting spkMan to active: id = c6149b35399517457b0b1d8ccdd7efda25a2f20fc7f8167adda8e79b10e260b7, type = legacy, internal = false
- 2024-04-01T22:49:38.430134Z [httpworker.0] [wallet/wallet.h:933] [WalletLogPrintf] [encrypted_wallet] RescanFromTime: Rescanning last 329 blocks
- 2024-04-01T22:49:38.430170Z [httpworker.0] [wallet/wallet.h:933] [WalletLogPrintf] [encrypted_wallet] Rescan started from block 0f9188f13cb7b2c71f2a335e3a4fc328bf5beb436012afca590b1a11466e2206... (slow variant inspecting all blocks)
- 2024-04-01T22:49:38.441914Z [httpworker.0] [wallet/scriptpubkeyman.h:258] [WalletLogPrintf] [encrypted_wallet] MarkUnusedAddresses: Detected a used keypool item at index 4000, mark all keypool items up to this item as used
2024-04-01T22:49:44.029000Z TestFramework (INFO): Stopping nodes
2024-04-01T22:49:44.132000Z TestFramework (WARNING): Not cleaning up dir /mnt/tmp/test_runner_₿_🏃_20240402_004231/wallet_importdescriptors_98
2024-04-01T22:49:44.132000Z TestFramework (ERROR): Test failed. Test logging available at /mnt/tmp/test_runner_₿_🏃_20240402_004231/wallet_importdescriptors_98/test_framework.log
2024-04-01T22:49:44.132000Z TestFramework (ERROR):
2024-04-01T22:49:44.133000Z TestFramework (ERROR): Hint: Call /home/chris/Documents/Code/bitcoin-core/test/functional/combine_logs.py '/mnt/tmp/test_runner_₿_🏃_20240402_004231/wallet_importdescriptors_98' to consolidate all logs
2024-04-01T22:49:44.133000Z TestFramework (ERROR):
2024-04-01T22:49:44.133000Z TestFramework (ERROR): If this failure happened unexpectedly or intermittently, please file a bug and provide a link or upload of the combined log.
2024-04-01T22:49:44.133000Z TestFramework (ERROR): https://github.com/bitcoin/bitcoin/issues
2024-04-01T22:49:44.133000Z TestFramework (ERROR):
stderr:
Remaining jobs: [feature_pruning.py, feature_dbcrash.py, feature_assumeutxo.py, rpc_scantxoutset.py, feature_coinstatsindex.py, p2p_node_network_limited.py --v1transport, p2p_node_network_limited.py --v2transport, feature_config_args.py]
298/305 - p2p_node_network_limited.py --v1transport passed, Duration: 24 s
```
</details>
## Related
Almost identical timeout in `feature_index_prune.py` in #27091 on MacOS, and for `wallet_importdescriptors.py --descriptors` in #27282 on Alpine & CI.
ACKs for top commit:
maflcko:
lgtm ACK 49c0b8b228
tdb3:
ACK for 49c0b8b228
itornaza:
approach ACK 49c0b8b228
BrandonOdiwuor:
crACK 49c0b8b228
Tree-SHA512: f62ade74701588d76bfe838b7e7bbda1db38fd98688fd5d13c2c008064027add2ee9d053dee602d84919fab4c9bf53183c31819d94a6174066f237d0f6a62086
2d1819455c crypto: chacha20: always use our fallback timingsafe_bcmp rather than libc's (Cory Fields)
Pull request description:
Looking at libc sources, apple and openbsd implementations match our naive fallback. Only FreeBSD (and only x86_64) seems to [implement an optimized version](https://github.com/freebsd/freebsd-src/blob/main/lib/libc/amd64/string/timingsafe_bcmp.S).
It's not worth the hassle of using a platform-specific function for such little gain.
Additionally, as mentioned below, this is the only case outside of sha2 that requires an autoconf check, and I have upcoming PRs to remove the sha2 ones.
Apple's [impl is unoptimized](https://opensource.apple.com/source/Libc/Libc-1244.1.7/string/FreeBSD/timingsafe_bcmp.c.auto.html).
As-is [OpenBSD's impl](https://github.com/openbsd/src/blob/master/lib/libc/string/timingsafe_bcmp.c).
Relevant IRC conversation with sipa:
> \<cfields\> sipa: chacha20poly1305.cpp uses libc's timingsafe_bcmp when possible. But looking around at apple/freebsd/openbsd, I don't see any impl that doesn't use the naive implementation that matches our fallback...
> \<cfields\> is there any reason to belive there's an optimized impl somewhere that we're actually hitting?
> \<cfields\> asking because after cleaning up sha2, timingsafe_bcmp is the last autoconf check that remains in all of crypto. It'd make life easy if we could just always use our internal one.
> \<cfields\> *all of crypto/
> \<sipa\> cfields: let's get rid of the dependency then
> \<sipa\> it's a trivial function
> \<sipa\> and if we need it for some platforms, no real reason not to use it on all
After the above discusstion, I did end up finding the x86_64-optimized FreeBSD impl, but I don't think that's all that significant.
ACKs for top commit:
sipa:
utACK 2d1819455c
fanquake:
ACK 2d1819455c
TheCharlatan:
ACK 2d1819455c
theStack:
ACK 2d1819455c
Tree-SHA512: b9583e19ac2f77c5d572aa5b95bc4b53669d5717e5708babef930644980de7c5d06a9c7decd5c2b559d70b8597328ecfe513375e3d8c3ef523db80012dfe9266
All `CTransactionRef` have `.GetWitnessHash()` that returns a cached `const Wtxid` (since fac1223a56),
so we don't need to pass transaction refs around with their IDs as they're easy to get from a ref.
Reverts commit ab4efad51b (PR #26970).
This workaround is not needed anymore, as since #27114 the test sets
the noban permission for both in- and outbound connections via the
`noban_tx_relay` setting, and we don't have to rely on these topology
hacks anymore. See commit c985eb854c.
The `BITCOINFUZZ` environment variable allows to override the default
path to the fuzz binary.
It complements the already existing set of variables used by tests:
- BITCOIND
- BITCOINCLI
- BITCOINUTIL
- BITCOINWALLET
561a650e0f test: Fix debug recommendation in argsman_tests (Fabian Jahr)
Pull request description:
There are recommendations in the `argsman_tests` comments on how to re-run and debug a test failure to see if it reflects an expected or unexpected change. The command tries to run a test in `util_tests` but this is in `argsman_tests` so the command doesn't work with just copy+paste. I didn't investigate further but I suspect that these tests were moved between files.
ACKs for top commit:
fanquake:
ACK 561a650e0f
Tree-SHA512: b3bb94ba1635c9455149b455f2b30ee37a8067a6242339531ab54d428177a288da29a4a10702652305eb34aa7638f51dad35fa6b0e7b74617e445327b8c4c053
a3485af67d ci: Drop duplicated compiler flags (Hennadii Stepanov)
Pull request description:
On the master branch @ 0d509bab45, it is easy to check the _"Options used to compile and link"_ section in the `configure` script output and observe duplicated compiler flags.
This PR cleans such cases up.
ACKs for top commit:
maflcko:
re-ACK a3485af67d
fanquake:
ACK a3485af67d - no-longer a change in behaviour.
Tree-SHA512: 7e644fcfad7be48af3b18edd2994c0c78a21ac3f9fff497724be80f74c9e859d156de15ca4024c5c50d1080435576ce63402b48aba5c2fd556e2ed7e318e0e34
fa9f36baba build: Remove HAVE_GMTIME_R (MarcoFalke)
fa72dcbfa5 refactor: FormatISO8601* without gmtime* (MarcoFalke)
fa2c486afc Revert "time: add runtime sanity check" (MarcoFalke)
Pull request description:
Now that the `ChronoSanityCheck` has passed for everyone with C++17 and is guaranteed by C++20 to always pass, remove it.
Also, remove `gmtime_r` and `gmtime_s` and replace them with `year_month_day`+`hh_mm_ss` from C++20.
ACKs for top commit:
sipa:
utACK fa9f36baba
fanquake:
ACK fa9f36baba - more std lib & even less stuff to port.
Tree-SHA512: a9e7e805b757b7dade0bcc3f95273a7dc4f68622630d74838339789dd203ad7542d36b2e090a93b2bc5a7ecc383207dd7ec82c68147108bdac7ce44f088c8c9a
Looking at apple/freebsd/openbsd sources, their implementations match our naive
fallback. It's not worth the hassle of using a platform-specific function for
no gain.
007ea322a6 depends: switch to building libqrencode with CMake (fanquake)
884330c0a5 guix: make cmake-minimal a global requirement (fanquake)
Pull request description:
Switch to building libqrencode with CMake. Note that upstream (https://github.com/fukuchi/libqrencode) hasn't seen any activity for ~4 years, so the odds of getting anything upstream seems low, but I've made two minor changes to the source here, which I will PR in any case.
From an initial look I couldn't find any significant difference between the Autotools and CMake produced libs. As part of this change we move cmake-minimal in Guix into the global package set.
ACKs for top commit:
TheCharlatan:
ACK 007ea322a6
Tree-SHA512: c784f790ddea958082c8ae96d3744bdf99331a8799765f9d44f00861b8e2cfcab1a88a3d64af5b10e51a8d5938d55eb6a3d271790b565e50492a39d00dc0e30f
bbe82c116eFix#29767, set m_synced = true after Commit() (nanlour)
Pull request description:
I think this problem https://github.com/bitcoin/bitcoin/issues/29767#issue-2216373048 is because of
in BaseIndex::Sync
61de64df67/src/index/base.cpp (L163-L168)
Setup m_synced = true; before Commit();
So this may cause a race condition window to BaseIndex::BlockConnected
61de64df67/src/index/base.cpp (L271-L274)
So i try to fix it with move m_synced = true after Commit().
Also see comment of Sync():
61de64df67/src/index/base.h (L151-L156)
I am a newcomer interested in Bitcoin, trying to become a member of the Bitcoin Core development team. Please give me some feedback if you could, as I may be doing something wrong. Thank you!
ACKs for top commit:
fjahr:
Code review ACK bbe82c116e
ryanofsky:
Code review ACK bbe82c116e
Tree-SHA512: 89a09498a232c87ef1e083d4cc4ed9bb15f045ad0624d5d150a87187b2b8a48a41137974dbc7ea5c37f73da90742c43259f5aa7f84b4179eb8d62033e44fa479
dergoegge