fe3ac3700d31a6329056fee983d91dd8e4c987c1 test: replace random_bytes with randbytes #28720 (ns-xvrn)
Pull request description:
With Python upgraded to 3.9 replaced the `random_bytes` function in util of functional tests and replaced it's usage with `random.randbytes`.
Closes#28720.
ACKs for top commit:
maflcko:
lgtm ACK fe3ac3700d31a6329056fee983d91dd8e4c987c1
BrandonOdiwuor:
ACK fe3ac3700d31a6329056fee983d91dd8e4c987c1
stickies-v:
ACK fe3ac3700d31a6329056fee983d91dd8e4c987c1, thanks for picking this up
kristapsk:
utACK fe3ac3700d31a6329056fee983d91dd8e4c987c1
Tree-SHA512: f65a75e73ebd840c2936eb133d42bccd552f25b717c8ca25c18d06e0593e12f292389cfcc0a0b0759004b67a46ea0c8ac237973ef90f246139778230be1e64e1
50d1ac120716ab17f32b28513c0ac9940001a783 test: remove unused `find_output` helper (Sebastian Falbesoner)
73a339abc3c864461c8b8830e139c8ec51570243 test: refactor: support sending funds with outpoint result (Sebastian Falbesoner)
Pull request description:
In wallet-related functional tests we often want to send funds to an address and use the resulting (non-change) UTXO directly after as input for another transaction. Doing that is currently tedious, as it involves finding the index part of the outpoint manually by calling helpers like `find_vout_for_address` or `find_output` first. This results in two different txid/vout variables which then again have to be combined to a single dictionary `{"txid": ..., "vout": ...}` in order to be specified as input for RPCs like `createrawtransaction` or `createpsbt`. For example:
```
txid1 = node1.sendtoaddress(addr1, value1)
vout1 = find_vout_for_address(node1, txid1, addr1)
txid2 = node2.sendtoaddress(addr2, value2)
vout2 = find_vout_for_address(node2, txid2, addr2)
node.createrawtransaction([{'txid': txid1, 'vout': vout1}, {'txid': txid2, 'vout': vout2}], .....)
```
This PR introduces a helper `create_outpoints` to immediately return the outpoint as
UTXO dictionary in the common format, making the tests more readable and avoiding unnecessary duplication:
```
utxo1 = self.create_outpoints(node1, outputs=[{addr1: value1}])[0]
utxo2 = self.create_outpoints(node2, outputs=[{addr2: value2}])[0]
node.createrawtransaction([utxo1, utxo2], .....)
```
Tests are switched to work with UTXO-objects rather than two individual txid/vout variables accordingly.
The `find_output` helper is removed, as it seems generally a bad idea to search for an outpoint only based on the output value. If that's really ever needed in the future, it makes probably more sense to add it as an additional parameter to `find_vout_of_address`. Note that `find_output` supported specifying a block-hash for where to look for the transaction (being passed on to the `getrawtransaction` RPC). This seems to be unneeded, as txids are always unique and for the only test that used that parameter (rpc_psbt.py) there was no observed difference in run-time, so it was not reintroduced in the new helper.
There are still some `find_vout_of_address` calls remaining, used for detecting change outputs or for whenever the sending happens via `sendrawtransaction` instead, so this PR tackles not all, but the most common case.
ACKs for top commit:
achow101:
ACK 50d1ac120716ab17f32b28513c0ac9940001a783
BrandonOdiwuor:
ACK 50d1ac120716ab17f32b28513c0ac9940001a783
maflcko:
ACK 50d1ac120716ab17f32b28513c0ac9940001a783 🖨
Tree-SHA512: af2bbf13a56cc840fefc1781390cf51625f1e41b3c030f07fc9abb1181b2d414ddbf795e887db029e119cbe45de14f7c987c0cba72ff0b8953080ee218a7915a
fa25e8b0a1610553014c786428f146ef9c694678 doc: Recommend lint image build on every call (MarcoFalke)
faf70c1f330a92612cf381d32c791e9ba445d3f2 Bump python minimum version to 3.9 (MarcoFalke)
fa8996b930886da712c09ffe4b58016b36c2ae5b ci: Bump i686_multiprocess.sh to latest Ubuntu LTS (MarcoFalke)
Pull request description:
All supported operating systems ship with python 3.9 (or later), so bumping the minimum should not cause any issues. A bump will allow new code to use new python 3.9 features.
For reference:
* https://packages.debian.org/bullseye/python3
* https://packages.ubuntu.com/focal/python3.9
* FreeBSD 12/13 also ships with 3.9
* CentOS-like 8/9 also ships with 3.9 (and 3.11)
* OpenSuse Leap also ships with 3.9 (and 3.11) https://software.opensuse.org/package/python311-base
This is for Bitcoin Core 27.0 in 2024 (next year), not the soon upcoming 26.0 next month.
ACKs for top commit:
Sjors:
ACK fa25e8b0a1610553014c786428f146ef9c694678
jamesob:
ACK fa25e8b0a1610553014c786428f146ef9c694678 ([`jamesob/ackr/28211.1.MarcoFalke.bump_python_minimum_supp`](https://github.com/jamesob/bitcoin/tree/ackr/28211.1.MarcoFalke.bump_python_minimum_supp))
Tree-SHA512: 86c9f6ac4b5ba94a62ee6a6062dd48a8295d8611a39cdb5829f4f0dbc77aaa1a51edccc7a99275bf699143ad3a6fe826de426d413e5a465e3b0e82b86d10c32e
This commit introduces a helper `create_outpoints` to execute the
`send` RPC and immediately return the target address outpoints as UTXO
dictionary in the common format, making the tests more readable and
avoiding unnecessary duplication.
004903ebade38ba47c5ddc17b756d605b963528e test: Add Wallet Unlock Context Manager (Brandon Odiwuor)
Pull request description:
Fixes#28601, see https://github.com/bitcoin/bitcoin/pull/28403#discussion_r1325426430
Add Context Manager to manage the locking and unlocking of locked wallets with a passphrase during testing.
ACKs for top commit:
kevkevinpal:
lgtm ACK [004903e](004903ebad)
maflcko:
lgtm ACK 004903ebade38ba47c5ddc17b756d605b963528e
Tree-SHA512: ab234c167e71531df0d974ff9a31d444f7ce2a1d05aba5ea868cc9452f139845eeb24ca058d88f058bc02482b762adf2d99e63a6640b872cc71a57a0068abfe8
Two recently added tests (PR #28625 / commit 2e31250027ac580a7a72221fe2ff505b30836175
and PR #28634 / commit 3bb51c29df596aab2c1fde184667cee435597715)
introduced a bug by wrongly using the `assert_debug_log` helper.
Instead of passing the expected debug string in a list as expected, it
was passed as bare string, which is then interpretered as a list of
characters, very likely leading the debug log assertion pass even if the
intended message is not appearing.
In order to avoid bugs like this in the future, enforce that the
`{un}expected_msgs` parameters are lists.
This is unnecessary and caused test failures. The backward
compatibility tests are meant to find regressions in the
current codebase, not to detect bugs in older releases.
0f83ab407ec5aa0591c54c03bcf408c7f2f0a192 test: display abrupt shutdown errors in console output (furszy)
Pull request description:
Making it easier to debug errors in the CI environment,
particularly in scenarios where it's not immediately clear
what happened nor which node crashed (or shutdown abruptly).
A bit of context:
Currently, the test framework redirects each node's stderr output
stream to a different temporary file inside each node's data directory.
While this is sufficient for storing the error, it isn't very helpful for
understanding what happened just by reading the CI console output.
Most of the time, reading the stderr file in the CI environment is not
possible, because people don't have access to it.
Testing Note:
The displayed error difference can be observed by cherry-picking this
commit 9cc5393c0f on top of this branch and running any
functional test.
ACKs for top commit:
maflcko:
lgtm ACK 0f83ab407ec5aa0591c54c03bcf408c7f2f0a192
theStack:
ACK 0f83ab407ec5aa0591c54c03bcf408c7f2f0a192
Tree-SHA512: 83ce4d21d5316e8cb16a17d3fbe77b8649fced9e09410861d9674c233f6e9c34bcf573504e387e4f439c2841b2ee9855d0d35607fa13aa89eafe0080c45ee82d
Making it easier to debug errors in the CI environment,
particularly in scenarios where it's not immediately clear
what happened nor which node crashed (or shutdown abruptly).
edbed31066e3674ba52b8c093ab235625527f383 chainparams: add signet assumeutxo param at height 160_000 (Sjors Provoost)
b8cafe38713cbf10d15459042f7f911bcc1b1e4e chainparams: add testnet assumeutxo param at height 2_500_000 (Sjors Provoost)
99839bbfa7110c7abf22e587ae2f72c9c57d3c85 doc: add note about confusing HaveTxsDownloaded name (James O'Beirne)
7ee46a755f1d57ce9d51975d3b54dc9ac3d08d52 contrib: add script to demo/test assumeutxo (James O'Beirne)
42cae39356fd20d521aaf99aff1ed85856f3c9f3 test: add feature_assumeutxo functional test (James O'Beirne)
0f64bac6030334d798ae205cd7af4bf248feddd9 rpc: add getchainstates (James O'Beirne)
bb0585779472962f40d9cdd9c6532132850d371c refuse to activate a UTXO snapshot if mempool not empty (James O'Beirne)
ce585a9a158476b0ad3296477b922e79f308e795 rpc: add loadtxoutset (James O'Beirne)
62ac519e718eb7a31dca1102a96ba219fbc7f95d validation: do not activate snapshot if behind active chain (James O'Beirne)
9511fb3616b7bbe1d0d2f54a45ea0a650ba0367b validation: assumeutxo: swap m_mempool on snapshot activation (James O'Beirne)
7fcd21544a333ffdf1910b65c573579860be6a36 blockstorage: segment normal/assumedvalid blockfiles (James O'Beirne)
4c3b8ca35c2e4a441264749bb312df2bd054b5b8 validation: populate nChainTx value for assumedvalid chainstates (James O'Beirne)
49ef778158c43859946a592e11ec34fe1b93a5b6 test: adjust chainstate tests to use recognized snapshot base (James O'Beirne)
1019c399825b0d512c1fd751c376d46fed4992b9 validation: pruning for multiple chainstates (James O'Beirne)
373cf91531b84bfdd06fdf8abf4dca228029ce6b validation: indexing changes for assumeutxo (James O'Beirne)
1fffdd76a1bca908f55d73b64983655b14cf7432 net_processing: validationinterface: ignore some events for bg chain (James O'Beirne)
fbe0a7d7ca680358237b6c2369b3fd2b43221113 wallet: validationinterface: only handle active chain notifications (James O'Beirne)
f073917a9e7ba423643dcae0339776470b628f65 validationinterface: only send zmq notifications for active (James O'Beirne)
4d8f4dcb450d31e4847804e62bf91545b949fa14 validation: pass ChainstateRole for validationinterface calls (James O'Beirne)
1e59acdf17309f567c370885f0cf02605e2baa58 validation: only call UpdatedBlockTip for active chainstate (James O'Beirne)
c6af23c5179cc383f8e6c275373af8d11e6a989f validation: add ChainstateRole (James O'Beirne)
9f2318c76cc6986d48e13831cf5bd8dab194fdf4 validation: MaybeRebalanceCaches when chain leaves IBD (James O'Beirne)
434495a8c1496ca23fe35b84499f3daf668d76b8 chainparams: add blockhash to AssumeutxoData (James O'Beirne)
c711ca186f8d8a28810be0beedcb615ddcf93163 assumeutxo: remove snapshot during -reindex{-chainstate} (James O'Beirne)
c93ef43e4fd4fbc1263cdc9e98ae5856830fe89e bugfix: correct is_snapshot_cs in VerifyDB (James O'Beirne)
b73d3bbd23220857bf17cbb6401275bf58013b72 net_processing: Request assumeutxo background chain blocks (Suhas Daftuar)
Pull request description:
- Background and FAQ: https://github.com/jamesob/assumeutxo-docs/tree/2019-04-proposal/proposal
- Prior progress/project: https://github.com/bitcoin/bitcoin/projects/11
- Replaces https://github.com/bitcoin/bitcoin/pull/15606, which was closed due to Github slowness. Original description and commentary can be found there.
---
This changeset finishes the first phase of the assumeutxo project. It makes UTXO snapshots loadable via RPC (`loadtxoutset`) and adds `assumeutxo` parameters to chainparams. It contains all the remaining changes necessary to both use an assumedvalid snapshot chainstate and do a full validation sync in the background.
This may look like a lot to review, but note that
- ~200 lines are a (non-essential) demo shell script
- Many lines are functional test, documentation, and relatively dilute RPC code.
So it shouldn't be as burdensome to review as the linecount might suggest.
- **P2P**: minor changes are made to `init.cpp` and `net_processing.cpp` to make simultaneous IBD across multiple chainstates work.
- **Pruning**: implement correct pruning behavior when using a background chainstate
- **Blockfile separation**: to prevent "fragmentation" in blockfile storage, have background chainstates use separate blockfiles from active snapshot chainstates to avoid interleaving heights and impairing pruning.
- **Indexing**: some `CValidationInterface` events are given with an additional parameter, ChainstateRole, and all indexers ignore events from ChainstateRole::ASSUMEDVALID so that indexation only happens sequentially.
- Have `-reindex` properly wipe snapshot chainstates.
- **RPC**: introduce RPC commands `loadtxoutset` and (hidden) `getchainstates`.
- **Release docs & first assumeutxo commitment**: add notes and a particular assumeutxo hash value for first AU-enabled release.
- This will complete the project and allow use of UTXO snapshots for faster node bootstrap.
The next phase, if it were to be pursued, would be coming up with a way to distribute the UTXO snapshots over the P2P network.
---
### UTXO snapshots
Create your own with `./contrib/devtools/utxo_snapshot.sh`, e.g.
```shell
./contrib/devtools/utxo_snapshot.sh 788000 utxo.dat ./src/bitcoin-cli -datadir=$(pwd)/testdata`)
```
or use the pre-generated ones listed below.
- Testnet: **2'500'000** (Sjors):
- torrent: `magnet:?xt=urn:btih:511e09f4bf853aefab00de5c070b1e031f0ecbe9&dn=utxo-testnet-2500000.dat&tr=udp%3A%2F%2Ftracker.bitcoin.sprovoost.nl%3A6969`
- sha256: `79db4b025448cc0ac388d8589a28eab02de53055d181e34eb47391717aa16388`
- Signet: **160'000** (Sjors):
- torrent: `magnet:?xt=urn:btih:9da986cb27b3980ea7fd06b21e199b148d486880&dn=utxo-signet-160000.dat&tr=udp%3A%2F%2Ftracker.bitcoin.sprovoost.nl%3A6969`
- sha256: `eeeca845385ba91e84ef58c09d38f98f246a24feadaad57fe1e5874f3f92ef8c`
- Mainnet: **800'000** (Sjors):
- Note: this needs the following commit cherry-picked in: 24deb2022b
- torrent: `magnet:?xt=urn:btih:50ee955bef37f5ec3e5b0df4cf0288af3d715a2e&dn=utxo-800000.dat&tr=udp%3A%2F%2Ftracker.bitcoin.sprovoost.nl%3A6969`
### Testing
#### For fun (~5min)
If you want to do a quick test, you can run `./contrib/devtools/test_utxo_snapshots.sh` and follow the instructions. This is mostly obviated by the functional tests, though.
#### For real (longer)
If you'd like to experience a real usage of assumeutxo, you can do that too.
I've cut a new snapshot at height 788'000 (http://img.jameso.be/utxo-788000.dat - but you can do it yourself with `./contrib/devtools/utxo_snapshot.sh` if you want). Download that, and then create a datadir for testing:
```sh
$ cd ~/src/bitcoin # or whatever
# get the snapshot
$ curl http://img.jameso.be/utxo-788000.dat > utxo-788000.dat
# you'll want to do this if you like copy/pasting
$ export AU_DATADIR=/home/${USER}/au-test # or wherever
$ mkdir ${AU_DATADIR}
$ vim ${AU_DATADIR}/bitcoin.conf
dbcache=8000 # or, you know, something high
blockfilterindex=1
coinstatsindex=1
prune=3000
logthreadnames=1
```
Obtain this branch, build it, and then start bitcoind:
```sh
$ git remote add jamesob https://github.com/jamesob/bitcoin
$ git fetch jamesob assumeutxo
$ git checkout jamesob/assumeutxo
$ ./configure $conf_args && make # (whatever you like to do here)
# start 'er up and watch the logs
$ ./src/bitcoind -datadir=${AU_DATADIR}
```
Then, in some other window, load the snapshot
```sh
$ ./src/bitcoin-cli -datadir=${AU_DATADIR} loadtxoutset $(pwd)/utxo-788000.dat
```
You'll see some log messages about headers retrieval and waiting to see the snapshot in the headers chain. Once you get the full headers chain, you'll spend a decent amount of time (~10min) loading the snapshot, checking it, and flushing it to disk. After all that happens, you should be syncing to tip in pretty short order, and you'll see the occasional `[background validation]` log message go by.
In yet another window, you can check out chainstate status with
```sh
$ ./src/bitcoin-cli -datadir=${AU_DATADIR} getchainstates
```
as well as usual favorites like `getblockchaininfo`.
ACKs for top commit:
achow101:
ACK edbed31066e3674ba52b8c093ab235625527f383
Tree-SHA512: 6086fb9a38dc7df85fedc76b30084dd8154617a2a91e89a84fb41326d34ef8e7d7ea593107afba01369093bf8cc91770621d98f0ea42a5b3b99db868d2f14dc2
380130d9d70f8f8d395949a51f43806f6e600efa test: add coverage to feature_addrman.py (kevkevin)
Pull request description:
I added two new tests that will cover the nNew and nTried tests which add coverage to the if block by checking values larger than our range since we only check for negative values now
adding coverage to these lines
https://github.com/bitcoin/bitcoin/blob/master/src/addrman.cpp#L273https://github.com/bitcoin/bitcoin/blob/master/src/addrman.cpp#L280
our test seem to only cover the `nTried < 0` and `nNew < 0` scenarios
ACKs for top commit:
ismaelsadeeq:
ACK 380130d9d70f8f8d395949a51f43806f6e600efa, code looks good to me 🍃 .
0xB10C:
Re-ACK 380130d9d70f8f8d395949a51f43806f6e600efa
Tree-SHA512: a063bd9ca4d2d536a27c8c22a28fb13759a96f19cd8ba6cb8879cf7f65046d4ff6e8f70df17feaffd0d0d08ef914cb18a11258d313a4841c811a7e7ae4df6d5b
I added two new tests that will cover the nNew and nTried tests which
add coverage to the if block by checking values larger than our range
since we only check for negative values now
Co-authored-by: ismaelsadeeq <ask4ismailsadiq@gmail.com>
96b3f2dbe4395ca55cfdd58c8f9f9bd7ca163983 test: add unit test coverage for Python ECDSA implementation (Sebastian Falbesoner)
Pull request description:
This PR adds missing unit test coverage for the Python ECDSA implementation, which should be useful for detecting potential problems early whenever changes in the test framework's Python implementation of secp256k1 are made (e.g. #26222). Note that right now we don't call `ECPubKey.verify_ecdsa` anywhere in our tests, so we wouldn't notice if it is broken at some point.
To keep it simple, the already existing unit test for Schnorr signatures is extended to also check ECDSA signatures. For that purpose, the dictionary storing private-key/public-key entries use their legacy types `ECKey/ECPubKey` instead of bare byte-arrays, and for Schnorr signing/verification the necessary conversions (ECKey -> bare private key, ECPubKey -> x-only pubkey) is done later when needed. To avoid code duplication, a helper function `random_bitflip` for damaging signatures is introduced.
The unit test can be run by either calling it for this single module:
`$ python3 -m unittest ./test/functional/test_framework/key.py`
or simply running `$ ./test/functional/test_runner.py` which calls all test framework module's unit tests at the start (see TEST_FRAMEWORK_MODULES list).
ACKs for top commit:
achow101:
ACK 96b3f2dbe4395ca55cfdd58c8f9f9bd7ca163983
sipa:
utACK 96b3f2dbe4395ca55cfdd58c8f9f9bd7ca163983
stratospher:
tested ACK 96b3f2d.
Tree-SHA512: b993f25b843fa047376addda4ce4b0f15750ffba926528b5cca4c5f99b9af456206f4e8af885d25a017dddddf382ddebf38765819b3d16a3f28810d03b010808
83d7cfd5429b0be7755bc48145032956f5f56dae test: refactor: deduplicate segwitv0 ECDSA signing for tx inputs (Sebastian Falbesoner)
Pull request description:
This PR is a simple follow-up for #28025. It introduces a `signing_input_segwitv0` helper in order to deduplicate the following steps needed to create a segwitv0 ECDSA signature:
1. calculate the `SegwitV0SignatureHash` with the desired sighash type
2. create the actual digital signature by calling ECKey.sign_ecdsa on the signature message hash calculated above
3. put the DER-encoded result (plus sighash byte) at the bottom of the witness stack
ACKs for top commit:
achow101:
ACK 83d7cfd5429b0be7755bc48145032956f5f56dae
pinheadmz:
code review ACK at 83d7cfd5429b0be7755bc48145032956f5f56dae
Tree-SHA512: b8e55409ddc9ddb14cfc06daeb4730d7750a4632f175f88dcac6ec4d216e71fd4a7eee325a64d6ebba3b33be50bcd30c2de7400f834c01abb67e52840d9823b6
32c1dd1ad65af0ad4d36a56d2ca32a8481237e68 [test] mempool coins disappearing mid-package evaluation (glozow)
a67f460c3fd1c7eb8070623666d887eefccff0d6 [refactor] split setup in mempool_limit test (glozow)
d08696120e3647b4c2cd0ae8d6e57dea12418b7c [test framework] add ability to spend only confirmed utxos (glozow)
3ea71feb11c261f002ed918f91f3434fd8a23589 [validation] don't LimitMempoolSize in any subpackage submissions (glozow)
d227b7234cd4cfd7c593ffcf8e2f24573d1ebea5 [validation] return correct result when already-in-mempool tx gets evicted (glozow)
9698b81828ff98820fa49c83ca364063233374c6 [refactor] back-fill results in AcceptPackage (glozow)
8ad7ad33929ee846a55a43c55732be0cb8973060 [validation] make PackageMempoolAcceptResult members mutable (glozow)
03b87c11ca0705e1d6147b90da33ce555f9f41c8 [validation] add AcceptSubPackage to delegate Accept* calls and clean up m_view (glozow)
3f01a3dab1c4ee37fd4093b6a0a3b622f53e231d [CCoinsViewMemPool] track non-base coins and allow Reset (glozow)
7d7f7a1189432b1b6245ba25df572229870567cb [policy] check for duplicate txids in package (glozow)
Pull request description:
While we are evaluating a package, we split it into "subpackages" for evaluation (currently subpackages all have size 1 except the last one). If a subpackage has size 1, we may add a tx to mempool and call `LimitMempoolSize()`, which evicts transactions if the mempool gets full. We handle the case where the just-submitted transaction is evicted immediately, but we don't handle the case in which a transaction from a previous subpackage (either just submitted or already in mempool) is evicted. Mainly, since the coins created by the evicted transaction are cached in `m_view`, we don't realize the UTXO has disappeared until `CheckInputsFromMempoolAndCache` asserts that they exist. Also, the returned `PackageMempoolAcceptResult` reports that the transaction is in mempool even though it isn't anymore.
Fix this by not calling `LimitMempoolSize()` until the very end, and editing the results map with "mempool full" if things fall out.
Pointed out by instagibbs in faeed687e5 on top of the v3 PR.
ACKs for top commit:
instagibbs:
reACK 32c1dd1ad6
Tree-SHA512: 61e7f69db4712e5e5bfa27d037ab66bdd97f1bf60a8d9ffb96adb1f0609af012c810d681102ee5c7baec7b5fe8cb7c304a60c63ccc445d00d86a2b7f0e7ddb90
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
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.
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
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
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
Only the combined addr:port of source and destination
must be unique. If the destination is different, the same addr:port
for the source may be used by the OS.
read() fails in text mode when the unicode hasn't been fully written
yet. Fixes: "wallet_importdescriptors.py: can't decode bytes in position
228861-228863: unexpected end of data"
(https://github.com/bitcoin/bitcoin/issues/28030)
20b49460b35268db59f7efcb02736b0e31191a74 test: remove race in the user-agent reception check (Vasil Dimov)
Pull request description:
In `add_p2p_connection()` we connect to `bitcoind` from the Python client and check that it has received our version string.
This check looked up the last/newest entry from `getpeerinfo` RPC, assuming that it must be the connection we have just opened. But this will not be the case if a new inbound or outbound connection is made to/from `bitcoind` in the meantime.
Instead of the last entry in `getpeerinfo`, check all and find the one which corresponds to our connection using our outgoing address:port tuple which is unique.
ACKs for top commit:
jonatack:
re-ACK 20b49460b35268db59f7efcb02736b0e31191a74
MarcoFalke:
Concept ACK 20b49460b35268db59f7efcb02736b0e31191a74
Tree-SHA512: 61fd3359ef11ea830021ede0e745497a7b60690c32d21c47cd12ff79f78907bb45e922c9d01e5d7ff614a8cd5e4560d39a3fc86b01b200429773a23ace3917e4
5cf44275c8ca8c32d238f37f717d78e9823f44c2 test: refactor: deduplicate legacy ECDSA signing for tx inputs (Sebastian Falbesoner)
Pull request description:
There are several instances in functional tests and the framework (MiniWallet, feature_block.py, p2p_segwit.py) where we create a legacy ECDSA signature for a certain transaction's input by doing the following steps:
1. calculate the `LegacySignatureHash` with the desired sighash type
2. create the actual digital signature by calling `ECKey.sign_ecdsa` on the signature message hash calculated above
3. put the DER-encoded result as CScript data push into tx input's scriptSig
Create a new helper `sign_input_legacy` which hides those details and takes only the necessary parameters (tx, input index, relevant scriptPubKey, private key, sighash type [SIGHASH_ALL by default]). For further convenience, the signature is prepended to already existing data-pushes in scriptSig, in order to avoid rehashing the transaction after calling the new signing function.
ACKs for top commit:
dimitaracev:
ACK `5cf4427`
achow101:
ACK 5cf44275c8ca8c32d238f37f717d78e9823f44c2
pinheadmz:
ACK 5cf44275c8ca8c32d238f37f717d78e9823f44c2
Tree-SHA512: 8f0e4fb2c3e0f84fac5dbc4dda87973276242b0f628034272a7f3e45434c1e17dd1b26a37edfb302dcaf380dbfe98b0417391ace5e0ac9720155d8fba702031e
faf902858d38150caa8991b0ab9d7cfee2905684 test: Check expected_stderr after stop (MarcoFalke)
Pull request description:
This fixes a bug where stderr wasn't checked for the shutdown sequence.
Fix that by waiting for the shutdown to finish and then check stderr.
ACKs for top commit:
theStack:
ACK faf902858d38150caa8991b0ab9d7cfee2905684
Tree-SHA512: a70cd1e6cda84d542782e41e8b59741dbcd472c0d0575bcef5cbfd1418473ce94efe921481d557bae3fbbdd78f1c49c09c48872883c052d87c5c9a9a51492692
The Socks5 server we use in the test framework would disconnect
by default immediately after the handshake and sometimes would
not register as a connected peer by bitcoind.
