Keep the "-upnp" option as a hidden arg for one major version in order
to show a more user friendly error to people who had this option set in
their config file.
ec317bc44b0cb089419d809b5fef38ecb9423051 test: update satoshi_round function (naiyoma)
Pull request description:
This PR refactors `satoshi_round` to accept different rounding modes and make rounding a required argument.
Continuation of https://github.com/bitcoin/bitcoin/pull/23225
ACKs for top commit:
maflcko:
review ACK ec317bc44b0cb089419d809b5fef38ecb9423051
achow101:
ACK ec317bc44b0cb089419d809b5fef38ecb9423051
AngusP:
ACK ec317bc44b0cb089419d809b5fef38ecb9423051
Tree-SHA512: 070f0aa6f66e58bff7412cae6b71f5f4ab8c718c7b5eeba4bb604fe22c6416a1ced0474294f504e1c28943ddc073104466b5944b43bae27e42bee5ca85afa468
Refactor satoshi_round function to accept different rounding modes.
Updated call site to use the revised `satoshi_round` function.
Co-authored-by: Kate Salazar <52637275+katesalazar@users.noreply.github.com>
bca346a97056748f1e3fb899f336d56d9fd45a64 net: require P2P binds to succeed (Vasil Dimov)
af552534ab83c572d3bc3f93ccaee5c1961ccab5 net: report an error if unable to bind on the Tor listening addr:port (Vasil Dimov)
9a7e5f4d68dadc64a675f32d1e91199d6b1aa095 net: don't extra bind for Tor if binds are restricted (Vasil Dimov)
Pull request description:
Make it possible to disable the Tor binding on `127.0.0.1:8334` and stop startup if any P2P bind fails instead of "if all P2P binds fail".
Fixes https://github.com/bitcoin/bitcoin/issues/22726
Fixes https://github.com/bitcoin/bitcoin/issues/22727
ACKs for top commit:
achow101:
ACK bca346a97056748f1e3fb899f336d56d9fd45a64
cbergqvist:
ACK bca346a97056748f1e3fb899f336d56d9fd45a64
pinheadmz:
ACK bca346a97056748f1e3fb899f336d56d9fd45a64
Tree-SHA512: fabef89a957191eea4f3e3b6109d2b8389f27ecc74440a920b0c10f31fff00a85bcfd1eb3c91826c7169c618f4de8a8d0a260e2caf40fd854f07ea9a980d8603
c9dacd958d7c1e98b08a7083c299d981e4c11193 test: Check that non empty version packet is ignored and no disconnection happens (stratospher)
997cc00b950a7d1b7f2a3971282685f4e81d87d2 test: Check that disconnection happens when AAD isn't filled (stratospher)
b5e6238fdbba5c777a5adfa4477dac51a82f4448 test: Check that disconnection happens when garbage sent/received are different (stratospher)
ad1482d5a20e6b155184a43d0724d2dcd950ce52 test: Check that disconnection happens when wrong garbage terminator is sent (stratospher)
e351576862471fc77b1e798a16833439e23ff0b4 test: Check that disconnection happens when >4095 garbage bytes is sent (stratospher)
e075fd131d668d9d1ba3c8566624481c4a57032d test: Introduce test types and modify v2 handshake function accordingly (stratospher)
7d07daa62311bdb0e2ce23d0b55f711f5088bd28 log: Add V2 handshake timeout (stratospher)
d4a1da8543522a213ac75761131d878eedfd4a5b test: Make global TRANSPORT_VERSION variable an instance variable (stratospher)
c642b08c4e45cb3a625a867ebd66c0ae51bde212 test: Log when the garbage is actually sent to transport layer (stratospher)
86cca2cba230c10324c6aedd12ae9655b83b2856 test: Support disconnect waiting for add_p2p_connection (stratospher)
bf9669af9ccc33dcade09bceb27d6745e9d9a75a test: Rename early key response test and move random_bitflip to util (stratospher)
Pull request description:
Add tests for the following v2 handshake scenarios:
1. Disconnection happens when > `MAX_GARBAGE_LEN` bytes garbage is sent
2. Disconnection happens when incorrect garbage terminator is sent
3. Disconnection happens when garbage bytes are tampered with
4. Disconnection happens when AAD of first encrypted packet after the garbage terminator is not filled
5. bitcoind ignores non-empty version packet and no disconnection happens
All these tests require a modified v2 P2P class (different from `EncryptedP2PState` used in `v2_p2p.py`) to implement our custom handshake behaviour based on different scenarios and have been kept in a single test file (`test/functional/p2p_v2_misbehaving.py`). Shifted the test in `test/functional/p2p_v2_earlykeyresponse.py` which is of the same pattern to this file too.
ACKs for top commit:
achow101:
ACK c9dacd958d7c1e98b08a7083c299d981e4c11193
mzumsande:
ACK c9dacd958d7c1e98b08a7083c299d981e4c11193
theStack:
Code-review ACK c9dacd958d7c1e98b08a7083c299d981e4c11193
Tree-SHA512: 90df81f0c7f4ecf0a47762d290a618ded92cde9f83d3ef3cc70e1b005ecb16125ec39a9d80ce95f99e695d29abd63443240cb5490aa57c5bc8fa2e52149a0672
In the Tor case, this prevents us from telling the Tor daemon to send
our incoming connections from the Tor network to an address where we
do not listen (we tried to listen but failed probably because another
application is already listening).
In the other cases (IPv4/IPv6 binds) this also prevents unpleasant
surprises caused by continuing operations even on bind failure. For
example, another application may be listening on portX, bitcoind tries
to bind on portX and portY, only succeeds with portY and continues
operation leaving the user thinking that his bitcoind is listening on
portX whereas another application is listening (the error message in
the log could easily be missed).
Avoid having the functional testing framework start multiple `bitcoind`s
that try to listen on the same `127.0.0.1:18445` (Tor listen for
regtest) if `bind_to_localhost_only` is set to `False`.
Also fix a typo in `test-shell.md` related to `bind_to_localhost_only`.
Fixes https://github.com/bitcoin/bitcoin/issues/22727
Early key response test is a special kind of test which requires
modified v2 handshake functions. More such tests can be added
where v2 handshake functions send incorrect garbage terminator,
excess garbage bytes etc.. Hence, rename p2p_v2_earlykey.py to a
general test file name - p2p_v2_misbehaving.py.
random_bitflip function (used in signature tests prior to this
commit) can be used in p2p_v2_misbehaving test to generate wrong
garbage terminator, wrong garbage bytes etc..
So, move the function to util.
In the functional tests, we often compare dicts with assert_equal, but the
output makes it very hard to tell exactly which entry in the dicts don't
match when there are a lot of entries and only minor differences. Change
the output to make it clearer.
a478c817b2f62b7334b36e331a2e37fe8380c754 test: replace `Callable`/`Iterable` with their `collections.abc` alternative (PEP 585) (stickies-v)
4b9afb18e6b9e16d7b299820f3a1382986a451d4 scripted-diff: use PEP 585 built-in collection types for verify-binary script (Sebastian Falbesoner)
d516cf83ed2da86dfefb395cd46f8a894907b88e test: use built-in collection types for type hints (Python 3.9 / PEP 585) (Sebastian Falbesoner)
Pull request description:
With Python 3.9 / [PEP 585](https://peps.python.org/pep-0585/), [type hinting has become a little less awkward](https://docs.python.org/3.9/whatsnew/3.9.html#type-hinting-generics-in-standard-collections), as for collection types one doesn't need to import the corresponding capitalized types (`Dict`, `List`, `Set`, `Tuple`, ...) anymore, but can use the built-in types directly (see https://peps.python.org/pep-0585/#implementation for the full list).
This PR applies the replacement for all Python scripts (i.e. in the contrib and test folders) for the basic types, i.e.:
- typing.Dict -> dict
- typing.List -> list
- typing.Set -> set
- typing.Tuple -> tuple
For an additional check, I ran mypy 1.6.1 on both master and the PR branch via
```
$ mypy --ignore-missing-imports --explicit-package-bases $(git ls-files "*.py")
```
and verified that the output is identical -- (from the 22 identified problems, most look like false-positives, it's probably worth it to go deeper here and address them in a follow-up though).
ACKs for top commit:
stickies-v:
ACK a478c817b2f62b7334b36e331a2e37fe8380c754
fanquake:
ACK a478c817b2f62b7334b36e331a2e37fe8380c754
Tree-SHA512: 6948c905f6abd644d84f09fcb3661d7edb2742e8f2b28560008697d251d77a61a1146ab4b070e65b0d27acede7a5256703da7bf6eb1c7c3a897755478c76c6e8
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
Since Python 3.9, type hinting has become a little less awkward, as for
collection types one doesn't need to import the corresponding
capitalized types (`Dict`, `List`, `Set`, `Tuple`, ...) anymore, but can
use the built-in types directly. [1] [2]
This commit applies the replacement for all Python scripts (i.e. in the
contrib and test folders) for the basic types:
- typing.Dict -> dict
- typing.List -> list
- typing.Set -> set
- typing.Tuple -> tuple
[1] https://docs.python.org/3.9/whatsnew/3.9.html#type-hinting-generics-in-standard-collections
[2] https://peps.python.org/pep-0585/#implementation for a list of type
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
Fix warnings for these files when ./test/lint/lint-python.py is run using
mypy 0.991 (released 11/2022) and later:
$ test/lint/lint-python.py
test/functional/test_framework/coverage.py:23: error: Incompatible default for argument "coverage_logfile" (default has type "None", argument has type "str") [assignment]
test/functional/test_framework/coverage.py:23: note: PEP 484 prohibits implicit Optional. Accordingly, mypy has changed its default to no_implicit_optional=True
test/functional/test_framework/util.py:318: error: Incompatible default for argument "timeout" (default has type "None", argument has type "int") [assignment]
test/functional/test_framework/util.py:318: note: PEP 484 prohibits implicit Optional. Accordingly, mypy has changed its default to no_implicit_optional=True
test/functional/test_framework/util.py:318: error: Incompatible default for argument "coveragedir" (default has type "None", argument has type "str") [assignment]
test/functional/interface_rest.py:67: error: Incompatible default for argument "query_params" (default has type "None", argument has type "dict[str, Any]") [assignment]
test/functional/interface_rest.py:67: note: PEP 484 prohibits implicit Optional. Accordingly, mypy has changed its default to no_implicit_optional=True
Verified using https://github.com/hauntsaninja/no_implicit_optional
For details, see:
https://mypy-lang.blogspot.com/2022/11/mypy-0990-released.html
eefe56967b4eb4b5144325cde4f40fc1cbde3e65 bugfix: Fix incorrect debug.log config file path (Ryan Ofsky)
3746f00be1b732a04976fc70cbb0661f97bbbd99 init: Error if ignored bitcoin.conf file is found (Ryan Ofsky)
398c3719b02197ad92fded20f6ff83b364747297 lint: Fix lint-format-strings false positives when format specifiers have argument positions (Ryan Ofsky)
Pull request description:
Show an error on startup if a bitcoin datadir that is being used contains a `bitcoin.conf` file that is ignored. There are two cases where this could happen:
- One case reported in [#27246 (comment)](https://github.com/bitcoin/bitcoin/issues/27246#issuecomment-1470006043) happens when a `bitcoin.conf` file in the default datadir (e.g. `$HOME/.bitcoin/bitcoin.conf`) has a `datadir=/path` line that sets different datadir containing a second `bitcoin.conf` file. Currently the second `bitcoin.conf` file is ignored with no warning.
- Another way this could happen is if a `-conf=` command line argument points to a configuration file with a `datadir=/path` line and that path contains a `bitcoin.conf` file, which is currently ignored.
This change only adds an error message and doesn't change anything about way settings are applied. It also doesn't trigger errors if there are redundant `-datadir` or `-conf` settings pointing at the same configuration file, only if they are pointing at different files and one file is being ignored.
ACKs for top commit:
pinheadmz:
re-ACK eefe56967b4eb4b5144325cde4f40fc1cbde3e65
willcl-ark:
re-ACK eefe56967b
TheCharlatan:
ACK eefe56967b4eb4b5144325cde4f40fc1cbde3e65
Tree-SHA512: 939a98a4b271b5263d64a2df3054c56fcde94784edf6f010d78693a371c38aa03138ae9cebb026b6164bbd898d8fd0845a61a454fd996e328fd7bcf51c580c2b
Show an error on startup if a bitcoin datadir that is being used contains a
`bitcoin.conf` file that is ignored. There are two cases where this could
happen:
- One case reported in
https://github.com/bitcoin/bitcoin/issues/27246#issuecomment-1470006043
happens when a bitcoin.conf file in the default datadir (e.g.
$HOME/.bitcoin/bitcoin.conf) has a "datadir=/path" line that sets different
datadir containing a second bitcoin.conf file. Currently the second
bitcoin.conf file is ignored with no warning.
- Another way this could happen is if a -conf= command line argument points
to a configuration file with a "datadir=/path" line and that specified path
contains a bitcoin.conf file, which is currently ignored.
This change only adds an error message and doesn't change anything about way
settings are applied. It also doesn't trigger errors if there are redundant
-datadir or -conf settings pointing at the same configuration file, only if
they are pointing at different files and one file is being ignored.
Also, move the burden of checking for a timeout to the client and
disable the timeout on the server. This should avoid intermittent issues
in slow tests (for example mining_getblocktemplate_longpoll.py, or
feature_dbcrash.py), or possibly when the server is running slow (for
example in valgrind). There shouldn't be any downside in tests caused
by a high rpcservertimeout.
Transactions with more than one datacarrier (OP_RETURN) output
are never considered standard, i.e. this change is necessary in
order to to get rid of the `acceptnonstdtxn` option for some
tests.
Confirmed UTXOs in functional tests can simply be created by using
MiniWallet's `send_self_transfer_multi` method with a subsequent
`generate` call to mine a block.
b167e536d0f5ae4c86d0c3da4a63337f9f0448ba test: refactor: use `create_lots_of_big_transactions` to dedup where possible (Sebastian Falbesoner)
8973eeb4124112a8cb7bb3d486780ac668b3e7bd test: use MiniWallet for mining_prioritisetransaction.py (Sebastian Falbesoner)
Pull request description:
This PR enables one more of the non-wallet functional tests (mining_prioritisetransaction.py) to be run even with the Bitcoin Code wallet by using the MiniWallet instead, as proposed in #20078. Note that the adapted helper function `create_lots_of_big_transactions` is currently only used in this test, i.e. there was no need to change any others.
ACKs for top commit:
ayush933:
tACK b167e53
danielabrozzoni:
tACK b167e536d0f5ae4c86d0c3da4a63337f9f0448ba
kouloumos:
ACK b167e536d0f5ae4c86d0c3da4a63337f9f0448ba
furszy:
ACK b167e536
Tree-SHA512: ccae20d7d414a720efdeea9c2ae399aa53a3a0e7db72bff8d0cb75d90621a7ae7c019ba68d24f9d06f7b111f87ff33bb9d8e5aa08b763e606cf10268780e205c
