fae405556d56f6f13ce57f69a06b9ec1e825422b scripted-diff: Rename CBlockTreeDB -> BlockTreeDB (MarcoFalke)
faf63039cce40f5cf8dea5a1d24945773c3433a1 Fixup style of moved code (MarcoFalke)
fa65111b99627289fd47dcfaa5197e0f09b8a50e move-only: Move CBlockTreeDB to node/blockstorage (MarcoFalke)
fa8685597e7302fc136f21b6dd3a4b187fa8e251 index: Drop legacy -txindex check (MarcoFalke)
fa69148a0a26c5054dbccdceeac8e117bf449275 scripted-diff: Use blocks_path where possible (MarcoFalke)
Pull request description:
The only reason for the check was to print a warning about an increase in storage use. Now that 22.x is EOL and everyone should have migrated (or decided to not care about storage use), remove the check.
Also, a move-only commit is included. (Rebased from https://github.com/bitcoin/bitcoin/pull/22242)
ACKs for top commit:
TheCharlatan:
ACK fae405556d56f6f13ce57f69a06b9ec1e825422b, though I lack historical context to really judge the second commit fa8685597e7302fc136f21b6dd3a4b187fa8e251.
stickies-v:
ACK fae405556d56f6f13ce57f69a06b9ec1e825422b
Tree-SHA512: 9da8f48767ae52d8e8e21c09a40c949cc0838794f1856cc5f58a91acd3f00a3bca818c8082242b3fdc9ca5badb09059570bb3870850d3807b75a8e23b5222da1
fbcacd4cf0dbe54e51f89cda05ff3db9e378ea12 test: remove fixed timeouts from feature_config_args (Martin Zumsande)
Pull request description:
Fixes#28290
These fixed timeouts aren't affected by the `timeout_factor` option and can therefore cause timeouts in slow environments.
They are also unnecessary for the test because they measure the wrong thing:
While there is an internal waiting time of 60s within `ThreadOpenConnections` (beginning only when that thread is started) for fixed seeds querying, the timeouts here don't measure that but the time from startup until a debug log message is encountered, during which many other things happen in init, so they don't make much sense to me in the first place.
ACKs for top commit:
MarcoFalke:
lgtm ACK fbcacd4cf0dbe54e51f89cda05ff3db9e378ea12
Tree-SHA512: 7bb3b7db2f9666b1929ffb7773c838ee98b0845569428e5d00ecf5234973d534c4f474e213896c71baabd6096a79347bd21b41a17b130053049714eb8a447c79
668aa6af8d5fbf047d43cf6f85f3335565637fb9 test: p2p: check that `getaddr` msgs are only responded once per connection (Sebastian Falbesoner)
Pull request description:
This simple PR adds missing test coverage for ignoring repeated `getaddr` requests (introduced in #7856, commit 66b07247a7a9e48e082502338176cc06edf61474):
6f03c45f6b/src/net_processing.cpp (L4642-L4648)
ACKs for top commit:
MarcoFalke:
lgtm ACK 668aa6af8d5fbf047d43cf6f85f3335565637fb9
brunoerg:
crACK 668aa6af8d5fbf047d43cf6f85f3335565637fb9
Tree-SHA512: edcdc6501c684fb41911e393f55ded9b044cd2f92918877eca152edd5a4287d1a9d57ae999f1cb42185eae00c3a0af411fcb9bcd5b990ef48849c3834b141584
They cannot be scaled by the timeout_factor option and can
therefore cause timeouts in slow environments.
They are also not necessary for the test, since they measure time
frome startup until a debug message is encountered, which
is not restricted to 1 minute by any internal logic within bitcoind.
b3a93b409e7fb33af77bd34a269a3eae71d3ba83 test: add functional test for deadlock situation (Martin Zumsande)
3557aa4d0ab59c18807661a49070b0e5cbfecde3 test: add basic tests for sendmsgtopeer to rpc_net.py (Martin Zumsande)
a9a1d69391596f57851f545fae12f8851149d2c3 rpc: add test-only sendmsgtopeer rpc (Martin Zumsande)
Pull request description:
This adds a `sendmsgtopeer` rpc (for testing only) that allows a node to send a message (provided in hex) to a peer.
While we would usually use a `p2p` object instead of a node for this in the test framework, that isn't possible in situations where this message needs to trigger an actual interaction of multiple nodes.
Use this rpc to add test coverage for the bug fixed in #27981 (that just got merged):
The test lets two nodes (almost) simultaneously send a single large (4MB) p2p message to each other, which would have caused a deadlock previously (making this test fail), but succeeds now.
As can be seen from the discussion in #27981, it was not easy to reproduce this bug without `sendmsgtopeer`. I would imagine that `sendmsgtopeer` could also be helpful in various other test constellations.
ACKs for top commit:
ajtowns:
ACK b3a93b409e7fb33af77bd34a269a3eae71d3ba83
sipa:
ACK b3a93b409e7fb33af77bd34a269a3eae71d3ba83
achow101:
ACK b3a93b409e7fb33af77bd34a269a3eae71d3ba83
Tree-SHA512: 6e22e72402f3c4dd70cddb9e96ea988444720f7a164031df159fbdd48056c8ac77ac53def045d9208a3ca07437c7c8e34f8b4ebc7066c0a84d81cd53f2f4fa5f
c4929cfa50ddb12943198a7f45723eedbd087d8f test: wallet_backup.py, fix intermittent failure in "restore using dumped wallet" (furszy)
Pull request description:
Aiming to fix#25652.
The failure arises because the test expects `init_wallet()` (the test framework function) to create a wallet with no keys. However, the function also imports the deterministic private key used to receive the coinbase coins.
This causes a race within the "restore using dumped wallet" case, where we intend to have a new wallet (with no existing keys) to test the 'importwallet()' RPC result.
The reason why this failure is intermittent is that it depends on other peers delivering the chain right after node2 startup and prior to the test 'node2.getbalance()' call and also the synchronization of the validation queue.
ACKs for top commit:
MarcoFalke:
lgtm ACK c4929cfa50ddb12943198a7f45723eedbd087d8f
Tree-SHA512: 80faa590439305576086a7d6e328f2550c97b218771fc5eba0567feff78732a2605d028a30a368d50944ae3d25fdbd6d321fb97321791a356416f2b790999613
fa5cc3ccfb15d05775ace9c1827b06203ce17116 test: Fix intermittent issue in mempool_reorg (MarcoFalke)
Pull request description:
Currently the test case may fail intermittently, see https://github.com/bitcoin/bitcoin/issues/28313
Fix this by changing a number and reducing the failure rate a bit.
ACKs for top commit:
glozow:
ACK fa5cc3ccfb15d05775ace9c1827b06203ce17116
Tree-SHA512: ff552111b434ca712c7dbdc5ba32a986a6fa4512cba5a756234eae69428063bf6ecfdc8f350ee84226ed4d3e4262b4639dbe49162a722e8da85f0d61e5690c51
1b09cc5959d4719ffad131b395f8185e9ab4b1a1 Make post-p2sh consensus rules mandatory for tx relay (Anthony Towns)
69c31bc748104407c596e84bcef893dc968fd758 doc, policy: Clarify comment on STANDARD_SCRIPT_VERIFY_FLAGS (Anthony Towns)
Pull request description:
The `MANDATORY_SCRIPT_VERIFY_FLAGS` constant was introduced in #3843 to distinguish between block consensus rules and relay standardness rules. However it was not actually used in the consensus code path: instead it only differentiates between the failure being reported as `TX_CONSENSUS` and `mandatory-script-verify-flag-failed` vs `TX_NOT_STANDARD` and `non-mandatory-script-verify-flag`.
This updates the list of mandatory flags to include the post-p2sh soft forks that are enforced as consensus rules via `GetBlockScriptFlags()`. The effect of this change is that validation.cpp will report `TX_CONSENSUS` failures for txs that fail dersig/csv/cltv/nulldummy/witness/taproot checks, instead of `TX_NOT_STANDARD`, which in turn adds `Misbehaving(100)` via `MaybePunishNodeForTx` in `net_processing`.
ACKs for top commit:
Sjors:
Code review ACK 1b09cc5959d4719ffad131b395f8185e9ab4b1a1
darosior:
ACK 1b09cc5959d4719ffad131b395f8185e9ab4b1a1
achow101:
ACK 1b09cc5959d4719ffad131b395f8185e9ab4b1a1
theStack:
Concept and code-review ACK 1b09cc5959d4719ffad131b395f8185e9ab4b1a1
Tree-SHA512: d3e5868e8cece478f2e934956ba0c231d8bb9c2daefd0df1f817774e292049902cfc1d0cd76dbd2e7722627a93eab2d7046ff678199aac70a2b01642e69349f1
The failure arises because the test expects 'init_wallet()' (the test
framework function) creating a wallet with no keys. However, the function
also imports the deterministic private key used to receive the coinbase coins.
This causes a race within the "restore using dumped wallet" case, where we
intend to have a new wallet (with no existing keys) to test the
'importwallet()' RPC result.
The reason behind the intermittent failures might be other peers delivering
the chain right after node2 startup (sync of the validation queue included)
and prior to the 'node2.getbalance()' check.
9eac5a0529f869075f0331e40d322c34fc8fc2af [functional test] transaction orphan handling (glozow)
61e77bb901bca038b46064dc4531e12c2ab50673 [test framework] make it easier to fast-forward setmocktime (glozow)
Pull request description:
I was doing some mutation testing (through reckless refactoring) locally and found some specific behaviors in orphan handling that weren't picked up by tests. Adding some of these test cases now can maybe help with reviewing refactors like #28031.
- Parent requests aren't sent immediately. A delay is added and the requests are filtered by AlreadyHaveTx before they are sent, which means you can't use fake orphans to probe precise arrival timing of a tx.
- Parent requests include all that are not AlreadyHaveTx. This means old confirmed parents may be requested.
- The node does not give up on orphans if the peer responds to a parent request with notfound. This means that if a parent is an old confirmed transaction (in which notfound is expected), the orphan should still be resolved.
- Rejected parents can cause an orphan to be dropped, but it depends on the reason and only based on txid.
- Rejected parents can cause an orphan to be rejected too, by both wtxid and txid.
- Requests for orphan parents should be de-duplicated with "regular" txrequest. If a missing parent has the same hash as an in-flight request, it shouldn't be requested.
- Multiple orphans with overlapping parents should not cause duplicated parent requests.
ACKs for top commit:
instagibbs:
reACK 9eac5a0529
dergoegge:
reACK 9eac5a0529f869075f0331e40d322c34fc8fc2af
achow101:
ACK 9eac5a0529f869075f0331e40d322c34fc8fc2af
fjahr:
Code review ACK 9eac5a0529f869075f0331e40d322c34fc8fc2af
Tree-SHA512: 85488dc6a3f62cf0c38e7dfe7839c01215b44b172d1755b18164d41d01038f3a749451241e4eba8b857fd344a445740b21d6382c45977234b21460e3f53b1b2a
452c094449de00f3640e6e0763366e7603375825 test: fix 'unknown named parameter' test in `wallet_basic` (brunoerg)
Pull request description:
This PR removes loop when testing an unknown named parameter. They don't have any effect.
ACKs for top commit:
jonatack:
ACK 452c094449de00f3640e6e0763366e7603375825
theStack:
re-ACK 452c094449de00f3640e6e0763366e7603375825
Tree-SHA512: cf1a37d738bb6fdf9817e7b1d33bc69643dae61e3dbfae5c1e9f26220c55db6f134018dd9a1c65c13869ee58bcb6f3337c5999aabf2614d3126fbc01270705e8
ee5a0369cc4305da7b3d26f37677de05ad797e51 test: ensure acceptstalefeeestimates is supported only on regtest chain (ismaelsadeeq)
22d5d4b2b2486feaef981e96f0321f020617f082 tx fees, policy: doc: update and delete unnecessary comment (ismaelsadeeq)
Pull request description:
This PR Follow up comments from [#27622](https://github.com/bitcoin/bitcoin/pull/27622)
It test that the new `regtest-only` option `acceptstalefeeestimates` is not supported on [main, signet and test chains](https://github.com/bitcoin/bitcoin/pull/27622/files#r1235218268), removes an unnecessary [comment](https://github.com/bitcoin/bitcoin/pull/27622/files#r1235204323), and update fee estimator `MAXFILEAGE` [description comment](https://github.com/bitcoin/bitcoin/pull/27622/files#r1233887314).
ACKs for top commit:
jonatack:
ACK ee5a0369cc4305da7b3d26f37677de05ad797e51
glozow:
utACK ee5a0369cc4305da7b3d26f37677de05ad797e51
Tree-SHA512: 4755f25b08db62f37614ea768272b12580ee0d481fb7fa339379901a6132c66828777c6747d3fe67490ceace3a6ff248bf13bdf65720f6e5ba8642eb762acd3c
5e3e83b005518659a69916c373b808da27e51791 RPC/Mining: Document template_request better for getblocktemplate (Luke Dashjr)
de319c61759952318364fbcb28c47f0959d89d0e RPC/rpcdoccheck: Error if a oneline_description has a quote for a non-string (Luke Dashjr)
7c61e9df90579ed42a30016e52355e437733b128 Bugfix: RPC: Remove quotes from non-string oneline descriptions (Luke Dashjr)
Pull request description:
Various JSON Object parameters had a `oneline_description` with quote characters. Fix those, and extend `rpcdoccheck` to detect them.
Also, slightly improve GBT's oneline description for template_request.
ACKs for top commit:
MarcoFalke:
review ACK 5e3e83b005518659a69916c373b808da27e51791
Tree-SHA512: 363d1669a661d0acfc19fddb57e777d781c7246f330cf62160e77dde10a6adcb0249db748127067da1afe1b7d17c71cf611d9fdc3664d6bf5b3f30105637769a
fa748c6f2ac2f9cac7ce42fd745ed3c3eae093b7 test: Fix intermittent issue in mining_getblocktemplate_longpoll.py (MarcoFalke)
Pull request description:
Fixes https://github.com/bitcoin/bitcoin/issues/26962
Wait for the thread to have started and the RPC to have reached the node before continuing. Otherwise the test may run into a race.
For example:
```
test 2023-06-23T13:10:29.245000Z TestFramework (INFO): Test that introducing a new transaction into the mempool will terminate the longpoll
node0 2023-06-23T13:10:29.245712Z [http] [httpserver.cpp:254] [http_request_cb] [http] Received a POST request for / from 127.0.0.1:43568
node0 2023-06-23T13:10:29.245915Z [httpworker.3] [rpc/request.cpp:181] [parse] [rpc] ThreadRPCServer method=getblocktemplate user=__cookie__
node0 2023-06-23T13:10:29.252594Z [http] [httpserver.cpp:254] [http_request_cb] [http] Received a POST request for / from 127.0.0.1:43568
node0 2023-06-23T13:10:29.254545Z [httpworker.2] [rpc/request.cpp:181] [parse] [rpc] ThreadRPCServer method=getblockchaininfo user=__cookie__
node0 2023-06-23T13:10:29.256530Z [http] [httpserver.cpp:254] [http_request_cb] [http] Received a POST request for / from 127.0.0.1:43568
node0 2023-06-23T13:10:29.256741Z [httpworker.1] [rpc/request.cpp:181] [parse] [rpc] ThreadRPCServer method=sendrawtransaction user=__cookie__
node0 2023-06-23T13:10:29.258033Z [httpworker.1] [validationinterface.cpp:213] [TransactionAddedToMempool] [validation] Enqueuing TransactionAddedToMempool: txid=38335600f2465c0f8bb2b86d5830a34851d86fa879800c0e1434ddfc78c42898 wtxid=c033cd3efd301c369d66cf759769159609471bd4f9efb3ee30e7209e57b74778
node0 2023-06-23T13:10:29.258263Z [httpworker.1] [txmempool.cpp:660] [check] [mempool] Checking mempool with 1 transactions and 1 inputs
node0 2023-06-23T13:10:29.258542Z [scheduler] [validationinterface.cpp:213] [operator()] [validation] TransactionAddedToMempool: txid=38335600f2465c0f8bb2b86d5830a34851d86fa879800c0e1434ddfc78c42898 wtxid=c033cd3efd301c369d66cf759769159609471bd4f9efb3ee30e7209e57b74778
node0 2023-06-23T13:10:29.259549Z [http] [httpserver.cpp:254] [http_request_cb] [http] Received a POST request for / from 127.0.0.1:43568
node0 2023-06-23T13:10:29.259745Z [httpworker.0] [rpc/request.cpp:181] [parse] [rpc] ThreadRPCServer method=decoderawtransaction user=__cookie__
node0 2023-06-23T13:10:29.261066Z [http] [httpserver.cpp:254] [http_request_cb] [http] Received a POST request for / from 127.0.0.1:52690
node0 2023-06-23T13:10:29.261803Z [http] [httpserver.cpp:254] [http_request_cb] [http] Received a POST request for / from 127.0.0.1:43568
node0 2023-06-23T13:10:29.262770Z [httpworker.2] [rpc/request.cpp:181] [parse] [rpc] ThreadRPCServer method=getblocktemplate user=__cookie__
```
(`sendrawtransaction` is called before `getblocktemplate`)
ACKs for top commit:
jamesob:
Github ACK fa748c6f2a
theStack:
ACK fa748c6f2ac2f9cac7ce42fd745ed3c3eae093b7
Tree-SHA512: c67d9ec7c56e8a22c1a26a3c3d4d4a4bcc17e4282cad0d66561ba2abd6e92240cb028369b4edc6077ea34e8736c0294f6066381979aee22a6166580cea43729a
fb02ba3c5f5bcd96b5e3622ef001b8e57ce63fc0 mempool_entry: improve struct packing (Anthony Towns)
1a118062fbc4ec8f645f4ec4298d869a869c3344 net_processing: Clean up INVENTORY_BROADCAST_MAX constants (Anthony Towns)
6fa49937e488d0924044786c76b42324b659f351 test: Check tx from disconnected block is immediately requestable (glozow)
e4ffabbffacc4b890d393aafcc8286916ef887d8 net_processing: don't add txids to m_tx_inventory_known_filter (Anthony Towns)
6ec1809d33bfc42b80cb6f35625dccd56be8d507 net_processing: drop m_recently_announced_invs bloom filter (Anthony Towns)
a70beafdb22564043dc24fc98133fdadbaf77d8a validation: when adding txs due to a block reorg, allow immediate relay (Anthony Towns)
1e9684f39fba909b3501e9402d5b61f4bf744ff2 mempool_entry: add mempool entry sequence number (Anthony Towns)
Pull request description:
This PR replaces the `m_recently_announced_invs` bloom filter with a simple sequence number tracking the mempool state when we last considered sending an INV message to a node. This saves 33kB per peer (or more if we raise the rate at which we relay transactions over the network, in which case we would need to increase the size of the bloom filter proportionally).
The philosophy here (compare with #18861 and #19109) is that we consider the rate limiting on INV messages to only be about saving bandwidth and not protecting privacy, and therefore after you receive an INV message, it's immediately fair game to request any transaction that was in the mempool at the time the INV message was sent. We likewise consider the BIP 133 feefilter and BIP 37 bloom filters to be bandwidth optimisations here, and treat transactions as requestable if they would have been announced without those filters. Given that philosophy, tracking the timestamp of the last INV message and comparing that against the mempool entry time allows removal of each of `m_recently_announced_invs`, `m_last_mempool_req` and `UNCONDITIONAL_RELAY_DELAY` and associated logic.
ACKs for top commit:
naumenkogs:
ACK fb02ba3c5f5bcd96b5e3622ef001b8e57ce63fc0
amitiuttarwar:
review ACK fb02ba3c5f5
glozow:
reACK fb02ba3c5f5bcd96b5e3622ef001b8e57ce63fc0
Tree-SHA512: cbba5ee04c86df26b6057f3654c00a2b45ec94d354f4f157a769cecdaa0b509edaac02b3128afba39b023e82473fc5e28c915a787f84457ffe66638c6ac9c2d4
769f5b15f207ce6d1067672ea5e195541c97de6b test: check backup from `migratewallet` can be successfully restored (brunoerg)
Pull request description:
`migratewallet` migrates the wallet to a descriptor one. During the process, it generates a backup file of the wallet in case of an incorrect migration. This PR adds test to check if the backup file can be successfully restored.
ACKs for top commit:
achow101:
ACK 769f5b15f207ce6d1067672ea5e195541c97de6b
MarcoFalke:
lgtm ACK 769f5b15f207ce6d1067672ea5e195541c97de6b
Tree-SHA512: 94c50b34fbd47c4d3cc34b94e9e7903bc233608c7f50f45c161669996fd5f5b7d8f9a4e6a3437b9151d66a76af833f3f1ca28e44ecb63b5a8f391f6d6be0e39f
fa776e61cd64a5ffd9a4be589ab8efeb5421861a Add importmempool RPC (MarcoFalke)
fa20d734a29ba50cd19b78cb4fe39a2d826131b7 refactor: Add and use kernel::ImportMempoolOptions (MarcoFalke)
fa8866990dba7817427977bfe834efdb17114d37 doc: Clarify the getmempoolinfo.loaded RPC field documentation (MarcoFalke)
6888886cecf6665da70b3dc3772b3c12ef06ad76 Remove Chainstate::LoadMempool (MarcoFalke)
Pull request description:
Currently it is possible to import a mempool by placing it in the datadir and starting the node. However this has many issues:
* Users aren't expected to fiddle with the datadir, possibly corrupting it
* An existing mempool file in the datadir may be overwritten
* The node needs to be restarted
* Importing an untrusted file this way is dangerous, because it can corrupt the mempool
Fix all issues by adding a new RPC.
ACKs for top commit:
ajtowns:
utACK fa776e61cd64a5ffd9a4be589ab8efeb5421861a
achow101:
ACK fa776e61cd64a5ffd9a4be589ab8efeb5421861a
glozow:
reACK fa776e61cd64a5ffd9a4be589ab8efeb5421861a
Tree-SHA512: fcb1a92d6460839283c546c47a2d930c363ac1013c4c50dc5215ddf9fe5e51921d23fe0abfae0a5a7631983cfc7e2fff3788b70f95937d0a989a203be4d67546
Have each TestNode keep track of the last timestamp it called
setmocktime with, and add a bumpmocktime() function to bump by a
number of seconds. Makes it easy to fast forward n seconds without
keeping track of what the last timestamp was.
6a7686b44618eabd2f8ee9f1d357cfeb1bce6d91 scripted-diff: Specify Python major version explicitly on Windows (Hennadii Stepanov)
Pull request description:
On Windows, it is the accepted practice to use `py.exe` launcher:
- https://learn.microsoft.com/en-us/windows/python/faqs#what-is-py-exe-
- https://docs.python.org/3/using/windows.html#python-launcher-for-windows
One of its features is the correct handling of shebang lines like the one we use: `#!/usr/bin/env python3`.
However, Windows OS app execution aliases might [interfere](https://learn.microsoft.com/en-us/windows/python/faqs#why-does-running-python-exe-open-the-microsoft-store-) with the launcher's behaviour. Such aliases are enabled on Windows 11 by default:
For example, on a fresh Windows 11 Pro installation with the Python installed from the [Chocolatey](https://community.chocolatey.org/packages/python/3.11.4) package manager, one will get the following error:
```
>py -3 test\functional\rpc_signer.py
2023-08-03T19:41:13.353000Z TestFramework (INFO): PRNG seed is: 2694758731106548661
2023-08-03T19:41:13.353000Z TestFramework (INFO): Initializing test directory C:\Users\hebasto\AppData\Local\Temp\bitcoin_func_test_mldbzzw3
2023-08-03T19:41:14.538000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
File "C:\Users\hebasto\bitcoin\test\functional\test_framework\util.py", line 140, in try_rpc
fun(*args, **kwds)
File "C:\Users\hebasto\bitcoin\test\functional\test_framework\coverage.py", line 50, in __call__
return_val = self.auth_service_proxy_instance.__call__(*args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "C:\Users\hebasto\bitcoin\test\functional\test_framework\authproxy.py", line 129, in __call__
raise JSONRPCException(response['error'], status)
test_framework.authproxy.JSONRPCException: RunCommandParseJSON error: process(py C:\Users\hebasto\bitcoin\test\functional\mocks\signer.py enumerate) returned 9009: Python was not found; run without arguments to install from the Microsoft Store, or disable this shortcut from Settings > Manage App Execution Aliases.
(-1)
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "C:\Users\hebasto\bitcoin\test\functional\test_framework\test_framework.py", line 131, in main
self.run_test()
File "C:\Users\hebasto\bitcoin\test\functional\rpc_signer.py", line 72, in run_test
assert_raises_rpc_error(-1, 'fingerprint not found',
File "C:\Users\hebasto\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"
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "C:\Users\hebasto\bitcoin\test\functional\test_framework\util.py", line 146, in try_rpc
raise AssertionError(
AssertionError: Expected substring not found in error message:
substring: 'fingerprint not found'
error message: 'RunCommandParseJSON error: process(py C:\Users\hebasto\bitcoin\test\functional\mocks\signer.py enumerate) returned 9009: Python was not found; run without arguments to install from the Microsoft Store, or disable this shortcut from Settings > Manage App Execution Aliases.
'.
2023-08-03T19:41:14.592000Z TestFramework (INFO): Stopping nodes
2023-08-03T19:41:14.799000Z TestFramework (WARNING): Not cleaning up dir C:\Users\hebasto\AppData\Local\Temp\bitcoin_func_test_mldbzzw3
2023-08-03T19:41:14.799000Z TestFramework (ERROR): Test failed. Test logging available at C:\Users\hebasto\AppData\Local\Temp\bitcoin_func_test_mldbzzw3/test_framework.log
2023-08-03T19:41:14.799000Z TestFramework (ERROR):
2023-08-03T19:41:14.799000Z TestFramework (ERROR): Hint: Call C:\Users\hebasto\bitcoin\test\functional\combine_logs.py 'C:\Users\hebasto\AppData\Local\Temp\bitcoin_func_test_mldbzzw3' to consolidate all logs
2023-08-03T19:41:14.799000Z TestFramework (ERROR):
2023-08-03T19:41:14.799000Z TestFramework (ERROR): If this failure happened unexpectedly or intermittently, please file a bug and provide a link or upload of the combined log.
2023-08-03T19:41:14.799000Z TestFramework (ERROR): https://github.com/bitcoin/bitcoin/issues
2023-08-03T19:41:14.799000Z TestFramework (ERROR):
```
This PR resolves this issue by explicitly specifying the Python major version and makes testing of self-compiled binaries more straightforward.
ACKs for top commit:
MarcoFalke:
lgtm ACK 6a7686b44618eabd2f8ee9f1d357cfeb1bce6d91
stickies-v:
utACK 6a7686b44618eabd2f8ee9f1d357cfeb1bce6d91
Tree-SHA512: 5681141e222bc833c6250cb79fe3a1c8e02255eb2c86010bc0f8239afcdfed784ed7788c8579209d931bd357f58d5655cf33ffeb2f46b1879f37cdc30e7a7c91
faafc35a779745d59fdb0e88698b579215f42b08 doc: Clarify that -datacarriersize applies to the full raw scriptPubKey, not the data push (MarcoFalke)
55550e7fe7e4ffe14637a901b568d1d7e1c716d4 test: Add -datacarriersize=2 tests (MarcoFalke)
Pull request description:
Clarify with a test that `-datacarriersize` applies to the serialized size of the scriptPubKey, not the size of the pushed data. So for example,
* `-datacarriersize=2` will reject a `raw(6a01aa)`, even though only one byte is pushed
* `-datacarriersize=0` (or `-datacarrier=0`) will reject a `raw(6a)`, even though no byte is pushed
* `-datacarriersize=0` (or `-datacarrier=0`) will reject a `raw(6a00)`, even though zero bytes are pushed
ACKs for top commit:
ajtowns:
ACK faafc35a779745d59fdb0e88698b579215f42b08
instagibbs:
ACK faafc35a77
Tree-SHA512: f01ace02798f596ac2a02461e9f2a6ef91b3b37c976ea0b3bc860e2d3efb0ace0fd8b779dd18249cee7f84ebbe5fd21d8506afd3a15edadc00b843ff3b4aacc7
Using `py.exe` launcher might by fragile depending on how Python was
installed. Specifying the Python version explicitly fixes test errors
like this:
```
RunCommandParseJSON error: process(py C:\Users\hebasto\bitcoin\test\functional\mocks\signer.py enumerate) returned 9009: Python was not found...
```
-BEGIN VERIFY SCRIPT-
sed -i 's|"py "|"py -3 "|g' $(git grep -l '"py "' -- test/functional)
-END VERIFY SCRIPT-
Connection object used as context manager only commits or rollbacks
transactions, so the connection object should be closed manually.
Fixes the following error on Windows:
```
PermissionError: [WinError 32] The process cannot access the file because it is being used by another process: ...
```
ba8ab4fc545800c4fb283a5ff0b19138a0451aba test: cover addrv2 support in anchors.dat with a TorV3 address (Matthew Zipkin)
b4bee4bbf45785fbbb355194ccb23c70acd19d27 test: add keep_alive option to socks5 proxy in test_framework (Matthew Zipkin)
5aaf988ccca210228c5a41ea0a18b0c85a85cf71 test: cover TorV3 address in p2p_addrv2_relay (Matthew Zipkin)
80f64a3d40779d342b740fbc57474e170b102678 test: add support for all networks in CAddress in messages.py (brunoerg)
Pull request description:
Closes https://github.com/bitcoin/bitcoin/issues/27140
Adds test coverage for https://github.com/bitcoin/bitcoin/pull/20516 to ensure that https://github.com/bitcoin/bitcoin/issues/20511 is completed and may be closed.
This PR adds a test case to `feature_anchors.py` where an onion v3 address is set as a blocks-only relay peer and then shutdown, ensuring that the address is saved to anchors.dat in addrv2 format. We then ensure that bitcoin attempts to reconnect to that anchor address on restart.
To compute the addrv2 serialization of the onion v3 address, I added logic to `CAddress` in `messages.py`. This new logic is covered by extending `p2p_addrv2_relay.py` to include an onion v3 address. Future work will be adding coverage for ipv6, torv2 and cjdns in these modules and also `feature_proxy.py`
Also includes de/serialization unit test for `CAddress` in test framework.
ACKs for top commit:
jonatack:
ACK ba8ab4fc545800c4fb283a5ff0b19138a0451aba
brunoerg:
crACK ba8ab4fc545800c4fb283a5ff0b19138a0451aba
willcl-ark:
ACK ba8ab4fc54
Tree-SHA512: 7220e30d7cb975903d9ac575a7215a08e8f784c24c5741561affcbde12fb92cbf8704cb42e66494b788ba6ed4bb255fb0cc327e4f2190fae50c0ed9f336c0ff0
2c0c6f44770403899bd8514ad7343356853bf38c test: dedup file hashing using `sha256sum_file` helper (Sebastian Falbesoner)
Pull request description:
Rather than doing the open/read/hash-steps manually in the affected functional tests, we can just use the `sha256sum_file` helper from the utils module instead.
Note that for the tool_wallet.py test, the used hash is changed from sha1 to sha256, but as the only purpose is to detect file content changes, this doesn't matter. Also, the optimization using `memoryview` is overkill here, as the opened file has only a size of 24KiB and determining the hash via the helper doesn't take longer than a few hundred micro-seconds on my machine.
ACKs for top commit:
kristapsk:
ACK 2c0c6f44770403899bd8514ad7343356853bf38c
Tree-SHA512: 64fe21650b56a50e9f1a95f6ef27d88d8bfbb621e5be456f327ef8dbb5596b529d03976c200f3fd68da48cc427de9f257b403f3228e38cf1df918006674fac68
8a20f765cce2fc0fadf1a2b66b843b67f2d2ae12 test: drop duplicate getaddrs from p2p_getaddr_caching (Martin Zumsande)
feb0096139e9e864632d2826d2e213b26146fff1 test: fix intermittent failure in p2p_getaddr_caching (Martin Zumsande)
Pull request description:
Fixes#28133
In the consistency check, it's not enough to check that our address/port is unique, only the combination of source and target must be unique. Otherwise, the OS may reuse ports for connections to different `-addrbind`, which was happening in the failed runs.
While at it, the second commit cleans up duplicate `getaddr` messages in `p2p_getaddr_caching.py` that do nothing but generate `Ignoring repeated "getaddr"` log messages (and cleans up some whitespace the python linter complains about).
ACKs for top commit:
vasild:
ACK 8a20f765cce2fc0fadf1a2b66b843b67f2d2ae12
Tree-SHA512: eabe4727d7887f729074076f6333a918bba8cb34b8e3baaa83f167b441b0daa24f7c4824abcf03a9538a2ef14b2d826ff19aeffcb93a6c20735253a9678aac9c
90c8f79e945863f3818748b86572948d1558aec3 test: remove redundant test values (Jon Atack)
c3f203387df60c596a1e416658d87d68b85adbf2 test: use common assert_signing_completed_successfully helper (Jon Atack)
647d95aae9720543c2c9c46c70191e6f9f775d66 test: add coverage for passing an invalid sighashtype (Jon Atack)
Pull request description:
Add test coverage for passing an invalid sighashtype to RPCs signrawtransactionwithwallet, signrawtransactionwithkey, walletprocesspsbt, and descriptorprocesspsbt.
ACKs for top commit:
MarcoFalke:
lgtm ACK 90c8f79e945863f3818748b86572948d1558aec3 🎥
brunoerg:
light crACK 90c8f79e945863f3818748b86572948d1558aec3
Tree-SHA512: 3861658487edd0d9a377390acf3d43f45c3dd9e324894f0fdb8f5312b618301a55479b1f70c61daee0b20785e768ffde6fa5abe6af190b73c0d0e017f3976704
bee2d57a655645dbfaf0242e85c5af034023a2fb script: update flake8 to 6.1.0 (Jon Atack)
38c3fd846bff163eb7c50bd77efcdcf8fcbc7f43 test: python E721 updates (Jon Atack)
Pull request description:
Update our functional tests per [E721](https://www.flake8rules.com/rules/E721.html) enforced by [flake8 6.1.0](https://flake8.pycqa.org/en/latest/release-notes/6.1.0.html), and update our CI lint task to use that release. This makes the following linter output on current master with flake8 6.1.0 green.
```
$ ./test/lint/lint-python.py ; ./test/lint/lint-spelling.py
test/functional/p2p_invalid_locator.py:35:16: E721 do not compare types, for exact checks use `is` / `is not`, for instance checks use `isinstance()`
test/functional/test_framework/siphash.py:34:12: E721 do not compare types, for exact checks use `is` / `is not`, for instance checks use `isinstance()`
test/functional/test_framework/siphash.py:64:12: E721 do not compare types, for exact checks use `is` / `is not`, for instance checks use `isinstance()`
src/test/fuzz/descriptor_parse.cpp:88: occurences ==> occurrences
^ Warning: codespell identified likely spelling errors. Any false positives? Add them to the list of ignored words in test/lint/spelling.ignore-words.txt
```
ACKs for top commit:
MarcoFalke:
lgtm ACK bee2d57a655645dbfaf0242e85c5af034023a2fb
Tree-SHA512: f3788a543ca98e44eeeba1d06c32f1b11eec95d4aef068aa1b6b5c401261adfa3fb6c6d6c769f3fe6839d78e74a310d5c926867e7c367d6513a53d580fd376f3
fafe43cb6c76a5f60194be128a40baf161d39920 scripted-diff: Use blocks_path where possible (MarcoFalke)
fa060c15fb5081e66ed1ebe05dca6e8026f32c4f test: Add blocks_path property to TestNode (MarcoFalke)
faba4fc3257c0e7d7bcb5146dab07ffe3193744b test: Drop 22.x node from TxindexCompatibilityTest (MarcoFalke)
fa7f65b0f81f3a2899c8eb540182c7d9a90fff00 test: Use clean chain in MempoolCompatibilityTest (MarcoFalke)
Pull request description:
The node in this test was never really needed, because the compatibility tests shouldn't be used to test previous releases. (The test suite of the previous release itself should be used for that). So remove it.
Also, other test changes. (See individual commits)
ACKs for top commit:
theStack:
Code-review ACK fafe43cb6c76a5f60194be128a40baf161d39920
Tree-SHA512: 289f54695bf5310663ab38ebf1aa457f53d0aafae56e6657be0e75bf96b303165bad417dc7eaf4c40f0639aa92ce139e5bacb318a2eabab1f8e23a811cabe0cc
