Commit Graph

318 Commits

Author SHA1 Message Date
Ava Chow
1a2523e901 Merge bitcoin/bitcoin#34937: Fix startup failure with RLIM_INFINITY fd limits
735b25519a support: clamp RLIMIT_MEMLOCK to size_t (Sjors Provoost)
8ab4b9fc85 init: clamp fd limits to int (Sjors Provoost)
4afbabdcef Fix startup failure with RLIM_INFINITY fd limits (Sjors Provoost)

Pull request description:

  When setting the fd limit to unlimited, the node fails to start:

  ```sh
  ulimit -n unlimited
  build/bin/bitcoind
  Error: Not enough file descriptors available. -1 available, 160 required.
  ```

  This was caused by `RaiseFileDescriptorLimit()` (introduced in #2568) casting `limitFD.rlim_cur` to `int`, which for `RLIM_INFINITY` overflows to `-1`. Fix it by returning `std::numeric_limits<int>::max()` instead.

  Some platforms implement `RLIM_INFINITY` as the maximum uint64, others as int64 (-1). So simply changing the return type to `uint64_t` wouldn't work.

  Similarly, though unlikely to actually happen:

  ```sh
  ulimit -n 214748364
  build/bin/bitcoind
  Error: Not enough file descriptors available. -2147483648 available, 160 required.
  ```

  The second commit expands the fix by clamping all values above `std::numeric_limits<int>::max()` instead of letting them overflow.

  This PR also expands `test/functional/feature_init.py` to cover these, using `resource.setrlimit`. The check is skipped on environments with a hard limit below infinity (or that don't have the Python [Resource module](https://docs.python.org/3/library/resource.html)).

  macOS by default has a hard limit of infinity, but on e.g. Ubuntu the default hard limit is 524288.

  The third commit applies a similar fix to `PosixLockedPageAllocator::GetLimit()` for 32-bit systems, but without a test.

ACKs for top commit:
  winterrdog:
    Re-ACK 735b25519a
  achow101:
    ACK 735b25519a
  sedited:
    Re-ACK 735b25519a
  pinheadmz:
    ACK 735b25519a

Tree-SHA512: 0ce0292ecd61456bdec6943b06cbb9ecfc5180ee6dce850f8496ef54af22c1fae6ea473085202f5ba6f72e4dc51a29247620c9a0eae31e96658adc77b293129f
2026-06-19 15:57:59 -07:00
Sebastian Falbesoner
9fae7e9886 test: doc: remove --perf profiling from functional test framework
This feature was broken for more than a year and no developer apparently
even noticed, so one can conclude that it is largely unused; it seems
thus reasonable to remove it to reduce maintenance burden.
2026-06-13 22:59:23 +02:00
merge-script
47da4f9b71 Merge bitcoin/bitcoin#35410: net: use the proxy if overriden when doing v2->v1 reconnections
bf0d257c11 net: un-default the OpenNetworkConnection()'s proxy_override argument (Eugene Siegel)
5a3756d150 test: add a regression test for private broadcast v1 retries (Vasil Dimov)
ab35a028ed test: make reusable filling of a node's addrman (Vasil Dimov)
2333be9cbc test: make reusable starting a standalone P2P listener (Vasil Dimov)
2ffa81fac4 test: make reusable SOCKS5 server starting (Vasil Dimov)
32d072a49f doc: add release notes for #35319 (Vasil Dimov)
d01b461f71 net: ensure no direct private broadcast connections (Vasil Dimov)
fd230f942d net: use the proxy if overriden when doing v2->v1 reconnections (Vasil Dimov)

Pull request description:

  This PR includes https://github.com/bitcoin/bitcoin/pull/35319 and on top of that adds a regression functional test.

  The functional test exercises the relevant code paths without modifying non-test code. To do that it does:

  * Add a bunch of IPv4 addresses to the node's addrman (they will be added without P2P_V2 flag).
  * Get them to report P2P_V2 in their service flags and connect to each one, so that the flags
    in addrman are updated to contain P2P_V2.
  * Get one successful connection to a Tor peer (.onion) so that bitcoind assumes the configured
    Tor proxy works and is indeed a proxy to the Tor network. This will make it open private
    broadcast connections also to IPv4 addresses via that proxy.
  * Start some private broadcast connections.
  * Remember the destination IPv4 address of the first connection and get it to fail the v2
    transport.
  * Wait for a subsequent connection also through the Tor proxy to the same IPv4 and expect
    it to be v1, i.e. the v2->v1 downgrade retry.

  The test fails without the fix - the v1 retry never arrives to the Tor proxy. And passes with the fix. The fix is in the first commit here and in https://github.com/bitcoin/bitcoin/pull/35319, can remove it by `git show fd230f942d | git apply -R`.

ACKs for top commit:
  Crypt-iQ:
    reACK bf0d257c11
  andrewtoth:
    ACK bf0d257c11
  instagibbs:
    ACK bf0d257c11
  sedited:
    utACK bf0d257c11

Tree-SHA512: 11e89be36577199e0312e5e63efeac04e295faaba1cf1c13a30e683d35f473c8dbb419d1897b0333c2e993c10637adecafcf90fe08c812065c793cbc903744c9
2026-06-04 10:38:54 +01:00
Vasil Dimov
ab35a028ed test: make reusable filling of a node's addrman
Extract the part of `p2p_private_broadcast.py` that fills a given node's
addrman and put it into `test_framework/test_framework.py`.
2026-05-29 12:52:36 +02:00
rkrux
779f444680 test: move out JSONRPCException from authproxy to util
So that util is not dependent on authproxy at all and going
forward authproxy can use util methods.

Reviewing with --color-moved=dimmed-zebra option can be helpful.
2026-05-25 15:53:33 +05:30
Ryan Ofsky
ce2044a91d Merge bitcoin/bitcoin#33362: Run feature_bind_port_(discover|externalip).py in CI
75cf9708a0 ci: add one more routable address to the VMs (docker containers) (Vasil Dimov)
1b93983bf5 test: make feature_bind_port_(discover|externalip).py auto-detect the skip condition (Vasil Dimov)

Pull request description:

  `feature_bind_port_discover.py` and `feature_bind_port_externalip.py` require a routable address on the machine to run. Since that was not predictably available on CI, those tests required a manual setting up of IP addresses (e.g. using `ifconfig`) and then running the tests with a command line option telling them that the addresses are set up. The tests were not run in CI and [got rot](https://github.com/bitcoin/bitcoin/issues/31293#issuecomment-2497792487).

  Change that to auto-detect, from the tests, whether the needed IP addresses are present and if yes, run the test, otherwise skip it. Also change the CI to configure the needed addresses when running the functional tests. This way the tests will be run regularly on CI.

  Fixes: https://github.com/bitcoin/bitcoin/issues/31336

ACKs for top commit:
  willcl-ark:
    ACK 75cf9708a0
  frankomosh:
    Tested ACK 75cf9708a0. Built from source.
  ryanofsky:
    Code review ACK 75cf9708a0. Tested locally with and without the special addresses, and the detection seems to work well.

Tree-SHA512: 252911a37a06764f644a1a83c808f5255ac3bc74919426afa5d082c59e1ea924196354735f229d381cb5aff2340e001c2240bbadc8b5f27e5321fb4cfaef0fdb
2026-05-19 13:34:59 -04:00
Vasil Dimov
1b93983bf5 test: make feature_bind_port_(discover|externalip).py auto-detect the skip condition
Instead of requiring a run with an explicit `--ihave1111and2222`, detect
whether the routable addresses are set up and if not, then skip the test.

To detect whether the addresses are set use `bitcoind` - start it
and ask it to bind on them and see if it will error with
"Unable to bind". Since this is what the tests do anyway, just start
the nodes and see if an exception will be raised like
`FailedToStartError` / "Unable to bind".

This makes it possible for the CI to run
`feature_bind_port_discover.py` and
`feature_bind_port_externalip.py` by just setting up the
addresses, without having to explicitly provide `--ihave1111and2222`.

Co-authored-by: willcl-ark <will@256k1.dev>
2026-04-29 14:18:41 +02:00
Sjors Provoost
4afbabdcef Fix startup failure with RLIM_INFINITY fd limits
When setting the fd limit to unlimited, the node fails to start:

ulimit -n unlimited
build/bin/bitcoind
Error: Not enough file descriptors available. -1 available, 160 required.

This was caused by RaiseFileDescriptorLimit() casting limitFD.rlim_cur
to int, which for RLIM_INFINITY overflows to -1. Fix it by returning
std::numeric_limits<int>::max() instead.

This commit also adds a functional test, which is skipped on environments
with a hard limit below infinity.

Co-authored-by: Luke Dashjr <luke-jr+git@utopios.org>
Co-authored-by: winterrdog
2026-04-23 15:50:16 +02:00
merge-script
ac9ce25b5f Merge bitcoin/bitcoin#34425: test: Fix all races after a socket is closed gracefully
fae807ed25 test: Remove unused, confusing and brittle connect_nodes.wait_for_connect (MarcoFalke)
fab2772647 test: Fix all races after a socket is closed gracefully (MarcoFalke)
fa21edddb2 test: Stricter checks in rpc_setban.py (MarcoFalke)
faa404e119 test: Add is_connected_to helper (MarcoFalke)

Pull request description:

  Currently, functional tests may intermittently fail, due to a lack of synchronization after a node connection socket was closed gracefully: If node A is connected to node B, and node B closes the connection, node A *must* wait for the connection to be closed before continuing the test. Otherwise, subsequent re-connections may not work while a stale connection is still alive.

  This can be reproduced locally via something like:

  ```diff
  diff --git a/src/net.cpp b/src/net.cpp
  index 6b79e913e8..32bd061500 100644
  --- a/src/net.cpp
  +++ b/src/net.cpp
  @@ -2200,2 +2200,3 @@ void CConnman::SocketHandlerConnected(const std::vector<CNode*>& nodes,
                   }
  +            UninterruptibleSleep(599ms);
                   pnode->CloseSocketDisconnect();
  ```

  With this diff, the tests should fail on master, and pass after this fix.

  The fix is placed inside the `connect_nodes` helper.

ACKs for top commit:
  rkrux:
    ACK fae807ed25
  w0xlt:
    ACK fae807ed25
  davidgumberg:
    (re-ish) crACK fae807ed25
  sedited:
    ACK fae807ed25

Tree-SHA512: 053825bbd319d7c49e08810bbabbf9c3a43d89897d697c0ca9232fd8a88ae475b4f9fbd79dff549b4e0a8941cf926ca4567e7fd43dc1749bf023d8ee7dd49608
2026-04-19 11:06:17 +02:00
Lőrinc
dcd90fbe54 test: prep manual equality assert conversions
The later scripted diff only handles plain `assert x == y` lines.
Some remaining tests still use equality inside comprehensions, parenthesized asserts, and other shapes that the line-based rewrite would misread.
Rewrite those sites by hand first so the later mechanical conversion stays safe.
The commit also simplifies the dead `len(["errors"]) == 0` branch in `blocktools.py`, which can never be true.
2026-04-07 15:56:29 +03:00
MarcoFalke
fae807ed25 test: Remove unused, confusing and brittle connect_nodes.wait_for_connect
The option is unused since the last removals in:
* 4c40a923f0, and
* 81bf3ebff7

It was brittle and lead to intermittent test issues. Generally, it is
also confusing, because if a test wanted to connect nodes without
checking their connection, it can use `addnode`, like the rpc_setban.py
test.

So fix all issues by removing it.
2026-03-27 16:21:16 +01:00
MarcoFalke
fab2772647 test: Fix all races after a socket is closed gracefully
This waits for any disconnect (e.g. from a restart of one of the nodes)
to fully happen before the next connect.

Can be reviewed with the git option:

--color-moved=dimmed-zebra
2026-03-27 16:13:59 +01:00
David Gumberg
0d1301b47a test: functional: drop rmtree usage and add lint check
`shutil.rmtree` is dangerous because it recursively deletes. There are
not likely to be any issues with it's current uses, but it is possible
that some of the assumptions being made now won't always be true, e.g.
about what some of the variables being passed to `rmtree` represent.

For some remaining uses of rmtree that can't be avoided for now, use
`cleanup_dir` which asserts that the recursively deleted folder is a
child of the the `tmpdir` of the test run. Otherwise,
`tempfile.TemporaryDirectory` should be used which does it's own
deleting on being garbage collected, or old fashioned unlinking and
rmdir in the case of directories with known contents.
2026-03-24 16:06:35 -07:00
MarcoFalke
fa050da980 test: Move event loop creation to network thread
This should fix https://github.com/bitcoin/bitcoin/issues/34367

I am not familiar with Windows sockets thread-safety, but creating the
event loop on the main thread, and running it in the network thread
could lead to a fast abort in Python on Windows (without any stderr):

```
77/276 - wallet_txn_clone.py failed, Duration: 1 s

stdout:
2025-12-10T08:04:27.500134Z TestFramework (INFO): PRNG seed is: 4018092284830106117

stderr:

Combine the logs and print the last 99999999 lines ...
============
Combined log for D:\a\_temp/test_runner_₿_🏃_20251210_075632/wallet_txn_clone_196:
============
 test  2025-12-10T08:04:27.500134Z TestFramework (INFO): PRNG seed is: 4018092284830106117
 test  2025-12-10T08:04:27.500433Z TestFramework (DEBUG): Setting up network thread
```

Also, I couldn't find any docs that require the loop must be created on
the thread that runs them:

* https://docs.python.org/3/library/asyncio-eventloop.html#asyncio.new_event_loop
* https://docs.python.org/3/library/asyncio-eventloop.html#asyncio.loop.run_forever

However, the patch seems trivial to review, harmless, and easy to
revert, so it may be a good try to fix the intermittent Windows Python
crash.
2026-03-13 13:48:47 +01:00
Ryan Ofsky
403523127b Merge bitcoin/bitcoin#34608: test: Fix broken --valgrind handling after bitcoin wrapper
fa5d478853 test: valgrind --trace-children=yes for bitcoin wrapper (MarcoFalke)
fa29fb72cb test: Remove redundant warning about missing binaries (MarcoFalke)
fa03fbf7e3 test: Fix broken --valgrind handling after bitcoin wrapper (MarcoFalke)

Pull request description:

  Currently, tool_bitcoin.py is failing under `--valgrind`:

  ```sh
  $ ./bld-cmake/test/functional/tool_bitcoin.py --valgrind
  TestFramework (ERROR): Unexpected exception
  Traceback (most recent call last):
    File "./test/functional/test_framework/test_framework.py", line 138, in main
      self.setup()
      ~~~~~~~~~~^^
    File "./test/functional/test_framework/test_framework.py", line 269, in setup
      self.setup_network()
      ~~~~~~~~~~~~~~~~~~^^
    File "./test/functional/tool_bitcoin.py", line 38, in setup_network
      assert all(node.args[:len(node_argv)] == node_argv for node in self.nodes)
             ~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  AssertionError
  ```

  Fix this issue by running `bitcoin` under valgrind.

ACKs for top commit:
  achow101:
    ACK fa5d478853
  ryanofsky:
    Code review ACK fa5d478853 just squashing commits since last review (Thanks!)

Tree-SHA512: 503685ac69e1ca3046958655bed4fe6d0aee1525c5ea58ebf098efd0332c0e16f138540baffaf9af1263a8c42ac6b150ed8bca5a5371a3c49802e21957ec6632
2026-02-25 05:03:05 -05:00
MarcoFalke
fa5d478853 test: valgrind --trace-children=yes for bitcoin wrapper 2026-02-24 13:16:08 +01:00
MarcoFalke
fa29fb72cb test: Remove redundant warning about missing binaries
The error was added in commit 1ea7e45a1f,
because there was an additional confusing `AssertionError: [node 0]
Error: no RPC connection` instead of just a single `FileNotFoundError:
[Errno 2] No such file or directory`.

This is no longer needed on current master.

Also, the test is incomplete, because it was just checking bitcoind and
bitcoin-cli, not any other missing binaries.

Also, after the previous commit, it would not work in combination with
--valgrind.

Instead of trying to make it complete, and work in all combinations,
just remove it, because the already existing error will be clear in any
case.

This can be tested via:

```sh
 ./test/get_previous_releases.py

 mv releases releases_backup
 # Confirm the test is skipped due to missing releases
 ./bld-cmake/test/functional/wallet_migration.py
 # Confirm the test fails due to missing releases
 ./bld-cmake/test/functional/wallet_migration.py --previous-releases
 mv releases_backup releases

 mv ./releases/v28.2 ./releases/v28.2_backup
 # Confirm the test fails with a single FileNotFoundError
 ./bld-cmake/test/functional/wallet_migration.py
 mv ./releases/v28.2_backup ./releases/v28.2
 # Confirm the test runs and passes
 ./bld-cmake/test/functional/wallet_migration.py

 rm ./bld-cmake/bin/bitcoind
 # Confirm the test fails with a single "No such file or directory",
 # testing with and without --valgrind
 ./bld-cmake/test/functional/wallet_migration.py
 ./bld-cmake/test/functional/wallet_migration.py --valgrind
```
2026-02-24 13:15:58 +01:00
MarcoFalke
fa03fbf7e3 test: Fix broken --valgrind handling after bitcoin wrapper
Prior to this commit, tool_bitcoin.py was failing:

```sh
$ ./bld-cmake/test/functional/tool_bitcoin.py --valgrind
TestFramework (ERROR): Unexpected exception
Traceback (most recent call last):
  File "./test/functional/test_framework/test_framework.py", line 138, in main
    self.setup()
    ~~~~~~~~~~^^
  File "./test/functional/test_framework/test_framework.py", line 269, in setup
    self.setup_network()
    ~~~~~~~~~~~~~~~~~~^^
  File "./test/functional/tool_bitcoin.py", line 38, in setup_network
    assert all(node.args[:len(node_argv)] == node_argv for node in self.nodes)
           ~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError
```

This commit fixes this issue by running `bitcoin` under valgrind. Also,
it comes with other improvements:

* Drop the outdated valgrind 3.14 requirement, because there is no
  distro that ships a version that old anymore.
* Drop the VALGRIND_SUPPRESSIONS_FILE env var handling, because it was
  presumably never used since it was introduced. Also, the use-case
  seems limited.

Review note:

The set_cmd_args was ignoring the --valgrind test option.

In theory, this could be fixed by refactoring Binaries::node_argv() to
be used here. However, for now, just re-implement the node_argv logic in
set_cmd_args to prepend the valgrind cmd.
2026-02-24 13:15:57 +01:00
Ava Chow
4933d1fbba Merge bitcoin/bitcoin#28792: build: Embedded ASMap [3/3]: Build binary dump header file
24699fec84 doc: Add initial asmap data documentation (Fabian Jahr)
bab085d282 ci: Use without embedded asmap build option in one ci job (Fabian Jahr)
e53934422a doc: Expand documentation on asmap feature and tooling (Fabian Jahr)
6244212a55 init, net: Implement usage of binary-embedded asmap data (Fabian Jahr)
6202b50fb9 build: Generate ip_asn.dat.h during build process (Fabian Jahr)
634cd60dc8 build: Add embedded asmap data (Fabian Jahr)

Pull request description:

  This is the final in a series of PRs that implement the necessary changes for embedding of asmap data into the binary. This last part add the initial asmap data, implements the build changes and adds further documentation.

  Currently an asmap file needs to be acquired by there user from some location or the user needs to generate one themselves. Then they need to move the file to the right place in datadir or pass the path to the file as `-asmap=PATH` in order to use the asmap feature. The change here allows for builds to embed asmap data into the bitcoind binary which makes it possible to use the feature without handling of the asmap file by the user. If the user starts bitcoind with `-asmap` the embedded data will be used for bucketing of nodes.

  The data lives in the repository at `src/node/data/ip_asn.dat` and can be replaced with a new version at any time. The idea is that the data should be updated with every release. By default the data at that location is embedded into the binary but there is also a build option to prevent this (`-DWITH_EMBEDDED_ASMAP=OFF`). In this case the original behavior of the `-asmap` option is maintained.

ACKs for top commit:
  achow101:
    ACK 24699fec84
  sipa:
    ACK 24699fec84
  hodlinator:
    ACK 24699fec84

Tree-SHA512: c2e33dbeea387efdfd3d415432bf8fa64de80f272c1207015ea53b85bb77f5c29f1dae5644513a23c844a98fb0a4bb257bf765f38b15bfc4c41984f0315b4c6a
2026-02-18 16:43:34 -08:00
MarcoFalke
211111b804 test: Avoid empty errmsg in JSONRPCException
It is unclear why the fallback should be an empty message, when it is
better to include all rpc_error details that are available.

Also, include the http status.

This allows to revert commit 6354b4fd7f,
because it is no longer needed.
2026-02-13 18:55:53 +01:00
Fabian Jahr
6202b50fb9 build: Generate ip_asn.dat.h during build process
This can be disabled with -DWITH_EMBEDDED_ASMAP=OFF.
2026-02-03 18:03:37 +01:00
furszy
6354b4fd7f tests: log node JSON-RPC errors during test setup
Currently, if the node replies to any command with an error during
the test framework's setup(), we log the generic and not really useful
"Unexpected exception" from the BaseException catch, with no further
information.
This isn't helpful for diagnosing the issue. Fix it by explicitly handling
JSONRPCException and logging the response error message and http status
code.
2026-01-28 19:03:33 -05:00
MarcoFalke
fab055c907 test: Scale NetworkThread close timeout with timeout_factor 2026-01-21 16:18:07 +01:00
MarcoFalke
fa9fdbce79 test: Pass bench exe into test framework utils
This teaches the test framework about the bench executable, which is
required for the next commit.
2026-01-01 20:38:59 +01:00
merge-script
39ca015259 Merge bitcoin/bitcoin#33140: test: Avoid shutdown race in NetworkThread
fa6db79302 test: Avoid shutdown race in NetworkThread (MarcoFalke)

Pull request description:

  Locally, I am seeing rare intermittent exceptions in the network thread:

  ```
  stderr:
  Exception in thread NetworkThread:
  Traceback (most recent call last):
  File "/python3.10/threading.py", line 1016, in _bootstrap_inner
  self.run()
  File "./test/functional/test_framework/p2p.py", line 744, in run
  self.network_event_loop.run_forever()
  File "/python3.10/asyncio/base_events.py", line 603, in run_forever
  self._run_once()
  File "/python3.10/asyncio/base_events.py", line 1871, in _run_once
  event_list = self._selector.select(timeout)
  AttributeError: 'NoneType' object has no attribute 'select'
  ```

  I can reproduce this intermittently via `while ./bld-cmake/test/functional/test_runner.py $(for i in {1..400}; do echo -n "tool_rpcauth "; done)  -j 400 ; do true ; done`.

  I suspect this is a race where the shutdown starts the close of the network thread while it is starting.

  A different exception showing this race can be reproduced via:

  ```diff
  diff --git a/test/functional/test_framework/p2p.py b/test/functional/test_framework/p2p.py
  index 610aa4ccca..64561e157c 100755
  --- a/test/functional/test_framework/p2p.py
  +++ b/test/functional/test_framework/p2p.py
  @@ -741,6 +741,7 @@ class NetworkThread(threading.Thread):

       def run(self):
           """Start the network thread."""
  +        import time;time.sleep(.1)
           self.network_event_loop.run_forever()

       def close(self, *, timeout=10):
  ```

  It is trivial to reproduce via any test (e.g. `./bld-cmake/test/functional/tool_rpcauth.py`) and shows a similar traceback to the one above:

  ```
  Exception in thread NetworkThread:
  Traceback (most recent call last):
    File "/python3.10/threading.py", line 1016, in _bootstrap_inner
      self.run()
    File "./test/functional/test_framework/p2p.py", line 745, in run
      self.network_event_loop.run_forever()
    File "/python3.10/asyncio/base_events.py", line 591, in run_forever
      self._check_closed()
    File "/python3.10/asyncio/base_events.py", line 515, in _check_closed
      raise RuntimeError('Event loop is closed')
  RuntimeError: Event loop is closed
  ```

  So fix the second runtime error in hope of fixing the first one as well.

ACKs for top commit:
  brunoerg:
    code review ACK fa6db79302

Tree-SHA512: ca352ebf7929456ea2bbfcfe4f726adcbfcfb3dc0edeaddae7f6926f998888f0bd8b987ddef60308266eeab6bffa7ebdc32f5908db9de5404df95635dae4a8f6
2025-12-03 10:42:30 +00:00
MarcoFalke
fae612424b contrib: Remove confusing and redundant encoding from IO
The encoding arg is confusing, because it is not applied consistently
for all IO.

Also, it is useless, as the majority of files are ASCII encoded, which
are fine to encode and decode with any mode.

Moreover, UTF-8 is already required for most scripts to work properly,
so setting the encoding twice is redundant.

So remove the encoding from most IO. It would be fine to remove from all
IO, however I kept it for two files:

* contrib/asmap/asmap-tool.py: This specifically looks for utf-8
  encoding errors, so it makes sense to sepecify the utf-8 encoding
  explicitly.
* test/functional/test_framework/test_node.py: Reading the debug log in
  text mode specifically counts the utf-8 characters (not bytes), so it
  makes sense to specify the utf-8 encoding explicitly.
2025-11-26 11:31:16 +01:00
MarcoFalke
fa75ef4328 test: Move export_env_build_path to util.py 2025-10-15 14:25:58 +02:00
MarcoFalke
fa9f495308 test: Move get_binary_paths and Binaries to util.py
Can be reviewed with the git options
--color-moved=dimmed-zebra --color-moved-ws=ignore-all-space
2025-10-15 14:25:50 +02:00
MarcoFalke
fa6db79302 test: Avoid shutdown race in NetworkThread 2025-09-29 17:16:22 +02:00
Ava Chow
df67bb6fd8 test: Remove convert_to_json_for_cli 2025-09-23 12:58:00 -07:00
merge-script
dd61f08fd5 Merge bitcoin/bitcoin#33031: wallet: Set descriptor cache upgraded flag for migrated wallets
88b0647f02 wallet: Always write last hardened cache flag in migrated wallets (Ava Chow)
8a08eef645 tests: Check that the last hardened cache upgrade occurs (Ava Chow)

Pull request description:

  #32597 set the descriptor cache upgraded flag for newly created wallets, but migrated wallets still did not have the flag set when they are migrated. For consistency, and to avoid an unnecessary upgrade, we should be setting this flag for migrated wallets.

  The flag would end up being set anyways at the end of migration when the wallet is reloaded as it would perform the automatic upgrade at that time. However, this is unnecessary and we should just set it from the get go.

  This PR also adds a couple tests to verify that the flag is being set, and that the upgrade is being performed.

ACKs for top commit:
  cedwies:
    re-ACK 88b0647
  rkrux:
    lgtm ACK 88b0647f02
  pablomartin4btc:
    ACK 88b0647f02

Tree-SHA512: 7d0850db0ae38eedd1e6a3bfaa548c6c612182291059fb1a47279a4c4984ee7914ecd02d8c7e427ef67bf9f5e67cbc57a7ae4412fad539e1bf3e05c512a60d69
2025-09-23 15:27:17 -04:00
Pieter Wuille
8d2ee88fa2 tests: add functional tests for IPC interface
Co-Authored-By: ismaelsadeeq <ask4ismailsadiq@gmail.com>
Co-Authored-By: ryanofsky <ryan@ofsky.org>
Co-Authored-By: TheCharlatan <seb.kung@gmail.com>
Co-Authored-By: Sjors Provoost <sjors@sprovoost.nl>
2025-09-05 09:24:14 -04:00
Ryan Ofsky
3cc9a06c8d test: Add TestNode ipcbind option
With this change, tests can specify `self.extra_init = [{ipcbind: True}]` to
start a node listening on an IPC socket, instead of needing to choose which
node binary to invoke and what `self.extra_args=[["-ipcbind=..."]]` value to
pass to it.

The eliminates boilerplate code #30437 (interface_ipc_mining.py), #32297
(interface_ipc_cli.py), and #33201 (interface_ipc.py) previously needed in
their test setup.
2025-09-05 07:15:29 -04:00
Ryan Ofsky
3cceb60a71 test: Provide path to bitcoin binary
Set new `BitcoinTestFramework.binary_paths.bitcoin_bin` property with path to
the `bitcoin` wrapper binary. This allows new tests for `bitcoin-mine` in
#30437 and `bitcoin-cli` in #32297 to find the `bitcoin` binary and call
`bitcoin -m` to start nodes with IPC support. This way the new tests can run
whenever the ENABLE_IPC build option is enabled, instead of only running when
the `BITCOIN_CMD` environment variable is set to `bitcoin -m`
2025-09-05 07:15:29 -04:00
Ryan Ofsky
8c7f005629 test: add is_ipc_compiled() and skip_if_no_ipc() functions
Expose ENABLE_IPC build option to functional tests so new tests can test
IPC-only features.
2025-09-05 07:15:29 -04:00
Ava Chow
8a08eef645 tests: Check that the last hardened cache upgrade occurs
When loading an older wallet without the last hardened cache, an
automatic upgrade should be performed. Check this in
wallet_backwards_compatibility.py

When migrating a wallet, the migrated wallet should always have the last
hardened cache, so verify in wallet_migration.py
2025-07-31 10:29:32 -07:00
Brandon Odiwuor
8aed477c33 test: fix RPC coverage check 2025-07-26 10:21:41 +01:00
MarcoFalke
faa3e68411 test: Log KeyboardInterrupt as exception
log.exception is more verbose and useful to debug timeouts.

Also, log stderr for CalledProcessError to make debugging easier.
2025-07-18 07:32:39 +02:00
MarcoFalke
fa30b34026 test: Do not pass tests on unhandled exceptions
This adds a missing catch for BaseException (e.g. SystemExit), which
would otherwise be silently ignored.

Also, remove the redundant other catches, which are just calling
log.exception with a redundant log message.
2025-07-17 17:04:57 +02:00
Sjors Provoost
83bb414557 test: less ambiguous error if bitcoind is missing
Before this change, when a functional test is run without building
the source, the error message suggested that previous release binaries
were missing.

When no previous release version is set, make the error message more
specifically about bitcoind.
2025-07-08 13:19:48 +02:00
merge-script
6251949443 Merge bitcoin/bitcoin#32290: test: allow all functional tests to be run or skipped with --usecli
666016e56b ci: use --usecli in one of the CI jobs (Martin Zumsande)
7ea248a020 test: Disable several (sub)tests with cli (Martin Zumsande)
f420b6356b test: skip subtests that check for wrong types with cli (Martin Zumsande)
6530d0015b test: add function to convert to json for height_or_hash params (Martin Zumsande)
54d28722ba test: Don't send empty named args with cli (Martin Zumsande)
cca422060e test: convert tuple to json for cli (Martin Zumsande)
af34e98086 test: make rpc_psbt.py usable with --usecli (Martin Zumsande)
8f8ce9e174 test: rename .rpc to ._rpc and remove unnecessary uses (Martin Zumsande)
5b08885986 test: enable functional tests with large rpc args for cli (Martin Zumsande)
7d5352ac73 test: use -stdin for large rpc commands (Martin Zumsande)
6c364e0c10 test: Enable various tests for usage with cli (Martin Zumsande)

Pull request description:

  Fixes #32264

  I looked into all current failures listed in the issue, as well all tests that are already disabled for the cli with `self.supports_cli = False`. There are several reasons why existing tests fail  with `--usecli` on many systems, the most important ones are:

  - Most common reason is that the test executes a RPC call with a large arg that exceeds `MAX_ARG_STRLEN` of the OS, which is usually 128kb on linux: This is fixed by using `-stdin` for these large calls (idea by 0xB10C)
  - they test specifically the rpc interface - nothing to do there except disabling.
  - Some functional test submit wrong types to params on purpose to test the error message (which is different when using the cli) - deactivated these specific subtests locally for the cli when there is just one or two of them, deactivated the entire tests when there are more spots
  - When python sets `None` for an arg, the cli converts this to 'null' in `arg_to_cli`. This is fine e.g. for boolean args, but doesn't work for strings where it's interpreted as the string 'null'. Bypass this for named args by not including args in case the value is `None` for the cli is used (it's effectively the same as leaving the optional arg out).
  -  the `height_or_hash` param used in some RPC needs to be converted to a JSON (effectively adding full quotes).
  - Some tests were marked with `self.supports_cli = False` in the past but run fine on master today - enabled those.

  In total, this PR fixes all tests that fail on master and reduces the number of tests that are deactivated (`self.supports_cli = False`) from 40 to 21.
  It also adds `--usecli` to one CI job (multiprocess, i686, DEBUG) to detect regressions.

ACKs for top commit:
  maflcko:
    re-ACK 666016e56b 🔀
  pinheadmz:
    re-ACK 666016e56b

Tree-SHA512: 7a1efd212649ca100b236a1239294d40ecd36e2720e3b173a230b14545bb40b135111db7fed8a0d1448120f5387da146a03f1912e2028c8d03a0b6a3ca8761b0
2025-07-03 10:20:03 +01:00
Ava Chow
ce000c8ee0 Merge bitcoin/bitcoin#32219: test: enabling wallet migration functional test on windows
941b8f54c0 ci: run get_previous_releases as part of test cross win job (Max Edwards)
5e2182140b test: increment mocked time for migrating wallet backups (Max Edwards)
5174565802 ci: disable feature_unsupported_utxo_db functional test (Max Edwards)
3dc90d69a6 test: remove mempool.dat before copying (Max Edwards)
67a6b20d50 test: add windows support to get previous releases script (Max Edwards)
1a1b478ca3 scripted-diff: rename tarball to archive (Max Edwards)
4f06dc8484 test: remove building from source from get prev releases script (Max Edwards)

Pull request description:

  This PR updates the `test/get_previous_releases.py` script to also work on Windows by changing to be pure python rather than using unix tools such as `curl` and `tar`.

  This enables additional functional tests to run such as `wallet_migration.py`, `mempool_compatability.py` and `wallet_backwards_compatibility.py`.

  Unfortunately `feature_unsupported_utxo_db.py` _could_ run but this test requires Bitcoin `v0.14.3` which will not run under windows with emojis in the data directory (as the functional test runner has by default) . This test could be run as it's own step in the ci workflow file and would pass but as it's quite an old version / feature I have assumed it's not worth worrying about and best just to exclude.

  Two tests needed to be slightly modified to run under windows. Both were issues with trying to overwrite a file that already exists which windows seems to be more strict on than the unix based systems.

  Finally, building from source has been dropped from the `get_previous_releases.py` script. This had not been updated after the move to cmake and so it was assumed that nobody could have been using that feature.

ACKs for top commit:
  maflcko:
    re-ACK 941b8f54c0 🍪
  achow101:
    ACK 941b8f54c0
  hodlinator:
    re-ACK 941b8f54c0

Tree-SHA512: 22933d0ec278b9b0ffcd2a8e90026e1a3631b00186e7f78bd65be925049021e319367d488c36a82ab526a07b264bac18c2777f87ca1174b231ed49fed56d11cb
2025-07-01 13:58:54 -07:00
Max Edwards
4f06dc8484 test: remove building from source from get prev releases script
Using the get_previous_releases.py script to build from source only works for
releases prior to v29 due to removal of Autotools (in favor of CMake). It also
does not support building on Windows, and we are adding support for downloading
Windows release binaries in later commits of this PR.

As there were no complaints during review, it is assumed nobody uses this
functionality.
2025-06-25 20:56:22 +01:00
Martin Zumsande
6530d0015b test: add function to convert to json for height_or_hash params
This is necessary for bitcoin-cli because a string without quotes
is not a valid json.
2025-06-23 17:42:56 -04:00
Martin Zumsande
8f8ce9e174 test: rename .rpc to ._rpc and remove unnecessary uses
It is usually not necessary, and makes it impossible to use --usecli
2025-06-23 17:42:55 -04:00
MarcoFalke
fa955154c7 test: Add missing skip_if_no_bitcoin_tx 2025-06-13 13:03:48 +02:00
MarcoFalke
fac9db6eb0 test: Add missing tx util to Binaries 2025-06-13 09:48:38 +02:00
MarcoFalke
fa91835ec6 test: Use lowercase env var as attribute name
This is a refactor.
2025-06-13 09:48:32 +02:00
MarcoFalke
fac49094cd test: Remove duplicate ConfigParser
It is sufficient to parse once.
2025-06-13 09:48:28 +02:00
Martin Zumsande
ed179e0a65 test: apply microsecond precision to test framework logging
This makes it easier to compare test and bitcoind logs -
combine_logs.py orders by timestamp and will be more precise.
2025-06-03 14:02:01 -04:00