aboutsummaryrefslogtreecommitdiff
path: root/test
AgeCommit message (Collapse)Author
2023-12-11Remove deprecated -rpcserialversionMarcoFalke
2023-12-11Merge bitcoin/bitcoin#29041: test: fix intermittent error in rpc_net.py (#29030)fanquake
ea00f982d21aab51001d422225f00626a74db298 test: fix intermittent error in rpc_net.py (#29030) (Sebastian Falbesoner) Pull request description: Asserting for the debug log message "Added connection peer=" is insufficient for ensuring that this new connection will show up in a following getpeerinfo() call, as the debug message is written in the CNode ctor, which means it hasn't necessarily been added to CConnman.m_nodes at this point. Solve this by using the recently introduced `wait_for_new_peer` helper (see #29006, commit 00e0658e77f66103ebdeb29def99dc9f937c049d), which is more robust. Fixes #29030. ACKs for top commit: maflcko: lgtm ACK ea00f982d21aab51001d422225f00626a74db298 Tree-SHA512: dda307949a466fb3b24408a8c213d307e0af2155f2e8b4e52c836a22397f9d218bf9d8c54ca55bae62a96d7566f27167db9311dd8801785c327234783af5ed00
2023-12-09test: fix intermittent error in rpc_net.py (#29030)Sebastian Falbesoner
Asserting for the debug log message "Added connection peer=" is insufficient for ensuring that this new connection will show up in a following getpeerinfo() call, as the debug message is written in the CNode ctor, which means it hasn't necessarily been added to CConnman.m_nodes at this point. Solve this by using the recently introduced `wait_for_new_peer` helper, which is more robust. Fixes #29030.
2023-12-08test: fix `addnode` functional test failure on OpenBSDSebastian Falbesoner
This is the functional test counterpart of PR #28891 / commit 007d6f0e85bc329040bb405ef6016339518caa66.
2023-12-08Merge bitcoin/bitcoin#29006: test: fix v2 transport intermittent test ↵fanquake
failure (#29002) 00e0658e77f66103ebdeb29def99dc9f937c049d test: fix v2 transport intermittent test failure (#29002) (Sebastian Falbesoner) Pull request description: This PR improves the following fragile construct for detection of a new connection to the node under test in `p2p_v2_transport.py`: https://github.com/bitcoin/bitcoin/blob/6d5790956f45e3de5c6c4ee6fda21878b0d1287b/test/functional/p2p_v2_transport.py#L154-L156 Only relying on the number of peers for that suffers from race conditions, as unrelated previous peers could disconnect at anytime in-between. In the test run in #29002, the following happens: - `getpeerinfo()` is called the first time -> assigned to `num_peers` - **previous peer disconnects**, the node's peer count is now `num_peers - 1` (in most test runs, this happens before the first getpeerinfo call) - new peer connects, the node's peer count is now `num_peers` - the condition that the node's peer count is `num_peers + 1` is never true, test fails Use the more robust approach of watching for an increased highest peer id instead (again using the `getpeerinfo` RPC call), with a newly introduced context manager method `TestNode.wait_for_new_peer()`. Note that for the opposite case of a disconnect, no new method is introduced; this is currently used only once in the test and is also simpler. Still happy to take suggestions for alternative solutions. Fixes #29002. ACKs for top commit: kevkevinpal: Concept ACK [00e0658](https://github.com/bitcoin/bitcoin/pull/29006/commits/00e0658e77f66103ebdeb29def99dc9f937c049d) maflcko: Ok, lgtm ACK 00e0658e77f66103ebdeb29def99dc9f937c049d stratospher: ACK 00e0658. Tree-SHA512: 0118b87f54ea5e6e080ff44f29d6af6674c757a588534b3add040da435f4359e71bf85bc0a5eb7170f99cc9956e1a03c35cce653d642d31eed41bbed1f94f44f
2023-12-08Merge bitcoin/bitcoin#28485: test: Extends MEMPOOL msg functional testfanquake
97c0dfa8942c7fd62c920deee03b4d0c59aba641 test: Extends MEMPOOL msg functional test (Sergi Delgado Segura) Pull request description: Currently, p2p_filter.py::test_msg_mempool is not testing much. This extends the tests so the interaction between sending `MEMPOOL` messages with a filter that does not include all transactions in the mempool reacts, plus how it interacts with `INV` messages, especially after the changes introduced by #27675 ACKs for top commit: instagibbs: ACK https://github.com/bitcoin/bitcoin/pull/28485/commits/97c0dfa8942c7fd62c920deee03b4d0c59aba641 theStack: re-ACK 97c0dfa8942c7fd62c920deee03b4d0c59aba641 Tree-SHA512: 746fdc867630f40509e6341f484a238dd855ae6d1be5eca121974491e4ca272dee88af4b90dda55ea9a5a19cbff198fa91ffa0c5bf1ddf0232b2c1b215b05b9a
2023-12-06test: Extends MEMPOOL msg functional testSergi Delgado Segura
Currently, p2p_filter.py::test_msg_mempool is not testing much. This extends the tests so the interaction between sending MEMPOOL messages with a filter that does not include all transactions in the mempool reacts, plus how it interacts with INV messages
2023-12-06Merge bitcoin/bitcoin#27581: net: Continuous ASMap health checkAndrew Chow
3ea54e5db7d53da5afa321e1800c29aa269dd3b3 net: Add continuous ASMap health check logging (Fabian Jahr) 28d7e55dff826a69f3f8e58139dbffb611cc5947 test: Add tests for unfiltered GetAddr usage (Fabian Jahr) b8843d37aed1276ff8527328c956c70c6e02ee13 fuzz: Let fuzzers use filter options in GetAddr/GetAddresses (Fabian Jahr) e16f420547fc72a5a2902927aa7138e43c0fb7c8 net: Optionally include terrible addresses in GetAddr results (Fabian Jahr) Pull request description: There are certain statistics we can collect by running all our known clearnet addresses against the ASMap file. This could show issues with a maliciously manipulated file or with an old file that has decayed with time. This is just a proof of concept for now. My idea currently is to run the analysis once per day and print the results to logs if an ASMap file is used. ACKs for top commit: achow101: ACK 3ea54e5db7d53da5afa321e1800c29aa269dd3b3 mzumsande: ACK 3ea54e5db7d53da5afa321e1800c29aa269dd3b3 brunoerg: crACK 3ea54e5db7d53da5afa321e1800c29aa269dd3b3 Tree-SHA512: 777acbfac43cc43ce4a0a3612434e4ddbc65f59ae8ffc9e24f21de09011bccb297f0599cbaa82bcf40ef68e5af582c4e98556379db7ceff7d9f97574a1cf8e09
2023-12-06test: fix v2 transport intermittent test failure (#29002)Sebastian Falbesoner
Only relying on the number of peers for detecting a new connection suffers from race conditions, as unrelated previous peers could disconnect at anytime in-between. Use the more robust approach of watching for an increased highest peer id instead (again using the `getpeerinfo` RPC call), with a newly introduced context manager method `TestNode.wait_for_new_peer()`. Fixes #29009.
2023-12-05test: add regression test for the getrawtransaction segfaultMartin Zumsande
This fails on master without the previous commit.
2023-12-04init: don't delete PID file if it was not generatedwillcl-ark
Previously, starting a second bitcoind using the same datadir would correctly fail to init and shutdown. However during shutdown the PID file belonging to the first instance would be erroneously removed by the second process shutting down. Fix this to only delete the PID file if we created it.
2023-12-02net: Add continuous ASMap health check loggingFabian Jahr
2023-12-01Merge bitcoin/bitcoin#28784: rpc: keep `.cookie` file if it was not generatedAndrew Chow
7cb9367157eb42ee06bc6fa024522cc14a80138d rpc: keep .cookie if it was not generated (Roman Zeyde) Pull request description: Otherwise, starting bitcoind twice may cause the `.cookie` file generated by the first instance to be deleted by the second instance shutdown (after failing to obtain a lock). ACKs for top commit: willcl-ark: re-ACK 7cb9367157eb42ee06bc6fa024522cc14a80138d achow101: ACK 7cb9367157eb42ee06bc6fa024522cc14a80138d kristapsk: re-ACK 7cb9367157eb42ee06bc6fa024522cc14a80138d stickies-v: ACK 7cb9367157eb42ee06bc6fa024522cc14a80138d Tree-SHA512: 0960dbc457975b0e0535f3d814824a879d7f85c9f1191537415b3fc253429a316a8e4badde56c8bc139778f132392983cec5fbe03891fb15ff61d3bc3f6e681b
2023-12-01Merge bitcoin/bitcoin#28848: bugfix, Change up submitpackage results to ↵Andrew Chow
return results for all transactions f23ba24aa079d68697d475789cd21bd7b5075550 test_submitpackage: only make a chain of 3 txns (Greg Sanders) e67a345162912ef7c1bfa3c89c7e7c629505f0a3 doc: submitpackage vsize results are sigops-adjusted (Greg Sanders) b67db52c399089e5d4c4202ebb905794dfd050d0 RPC submitpackage: change return format to allow partial errors (Greg Sanders) Pull request description: This was prompted by errors being returned that didn't "make any sense" to me, because it would for example return a "fee too low" error, when the "real" error was the child had something invalid, which disallowed CPFP evaluation. Rather than make judgment calls on what error is important(which is currently just return the "first"!), we simply return all errors and let the callers determine what's best. Added a top level `package_msg` for quick eye-balling of general success of the package. This PR also fixes a couple bugs: 1) Currently we don't actually broadcast a transaction, even if it was entered into our mempool, if a subsequent transaction causes `PKG_TX` failure. 2) "other-wtxid" is uncovered by tests, but IIUC was previously required to return "fees" and "vsize" results, but did not. I just make those results optional. ACKs for top commit: Sjors: Light re-utACK f23ba24aa079d68697d475789cd21bd7b5075550 achow101: ACK f23ba24aa079d68697d475789cd21bd7b5075550 glozow: utACK f23ba24aa079d68697d475789cd21bd7b5075550, thanks for taking the suggestions Tree-SHA512: ebfd716a4fed9e8c2dea3d2181ba6a6171b06718d29ac2324c67b7a30b374d199f7e1739f91ab5d036be172d0479de9bc89c32263ee62143c0338b9b622d0cca
2023-11-29test_submitpackage: only make a chain of 3 txnsGreg Sanders
2023-11-29RPC submitpackage: change return format to allow partial errorsGreg Sanders
Behavior prior to this commit allows some transactions to enter into the local mempool but not be reported to the user when encountering a PackageValidationResult::PCKG_TX result. This is further compounded with the fact that any transactions submitted to the mempool during this call would also not be relayed to peers, resulting in unexpected behavior. Fix this by, if encountering a package error, reporting all wtxids, along with a new error field, and broadcasting every transaction that was found in the mempool after submission. Note that this also changes fees and vsize to optional, which should also remove an issue with other-wtxid cases.
2023-11-29test: Add missing CBlockPolicyEstimator::processBlockTx suppressionMarcoFalke
2023-11-28Merge bitcoin/bitcoin#28554: bugfix: throw an error if an invalid parameter ↵Andrew Chow
is passed to getnetworkhashps RPC 9ac114e5cd9d8ade3a1d9f3d76a08ff59a3f1658 Throw error if invalid parameters passed to getnetworkhashps RPC endpoint (Jameson Lopp) Pull request description: When writing some scripts that iterated over many blocks to generate hashrate estimates I realized that my script was going out of range of the current chain tip height but was not encountering any errors. I believe that passing an invalid block height to this function but receiving the hashrate estimate for the chain tip instead should be considered unexpected behavior. ACKs for top commit: Sjors: re-utACK 9ac114e5cd9d8ade3a1d9f3d76a08ff59a3f1658 kevkevinpal: reACK [9ac114e](https://github.com/bitcoin/bitcoin/pull/28554/commits/9ac114e5cd9d8ade3a1d9f3d76a08ff59a3f1658) achow101: ACK 9ac114e5cd9d8ade3a1d9f3d76a08ff59a3f1658 Tree-SHA512: eefb465c2dd654fc48267f444e1809597ec5363cdd131ea9ec812458fed1e4bffbbbb0617d74687c9f7bb16274b598d8292f5eeb7953421e5d2a8dc2cc081f2b
2023-11-28Merge bitcoin/bitcoin#28805: test: Make existing functional tests compatible ↵Andrew Chow
with --v2transport 35fb9930adb3501b29d3ad20d2e74c0114f2bcbe test: enable v2 transport for p2p_timeouts.py (Martin Zumsande) 2c1669c37a9759e15ff5f4e340aeaa8778a81b9a test: enable v2 transport for rpc_net.py (Sebastian Falbesoner) cc961c26956859850202f56191981b0306a65fcf test: enable v2 transport for p2p_node_network_limited.py (Sebastian Falbesoner) 3598a1b5c932634dc7ccb991cc83df5e1a1dcaa9 test: enable --v2transport in combination with --usecli (Martin Zumsande) 68a90017519874793e34e3b439a63e5aa3a6f6a7 test: persist -v2transport over restarts and respect -v2transport=0 (Martin Zumsande) Pull request description: This makes the functional test suite compatible with BIP324, so that `python3 test_runner.py --v2transport` should succeed (currently, 12 tests fail for me on master). Includes two commits by TheStack I found in an old discussion https://github.com/bitcoin/bitcoin/pull/28331#discussion_r1326714164 Note that even though all tests should pass, the python `p2p.py` module will do v2 connections only after the merge of #24748, so that for now only connections between two full nodes will actually run v2. Some of the fixed tests were added with `--v2transport` to the test runner. Though after #24748 we might also want to consider running the entire suite with `--v2transport` in some CI. ACKs for top commit: sipa: utACK 35fb9930adb3501b29d3ad20d2e74c0114f2bcbe. Thanks for taking care of this. achow101: ACK 35fb9930adb3501b29d3ad20d2e74c0114f2bcbe theStack: ACK 35fb9930adb3501b29d3ad20d2e74c0114f2bcbe stratospher: ACK 35fb993. Tree-SHA512: 80dc0bf211fa525ff1d092043aea9f222f14c02e5832a548fb8b83b9ede1fcee03c5e8ade0d05c331bdaa492af9c1cf3d0f0b15b846673c6eacea82dd4cefbc3
2023-11-23test: Add gettransaction test for "coin-join" txMarcoFalke
2023-11-22Merge bitcoin/bitcoin#28862: lint: Report all lint errors instead of early exitfanquake
fa01f884d3ac128f09bfa57ac2648a19a19d854e ci: Add missing COPY for ./test/lint/test_runner (MarcoFalke) faff3e3b4604519375e122c103b156ec13eef80f lint: Report all lint errors instead of early exit (MarcoFalke) Pull request description: `all-lint.py` currently collects all failures. However, the `06_script.sh` does not, since July this year (https://github.com/bitcoin/bitcoin/pull/28103#discussion_r1268115806). Fix this by printing all failures before exiting. Can be tested by modifying (for example) two subtrees in the same commit and then running the linters. ACKs for top commit: kevkevinpal: ACK [fa01f88](https://github.com/bitcoin/bitcoin/pull/28862/commits/fa01f884d3ac128f09bfa57ac2648a19a19d854e) TheCharlatan: lgtm ACK fa01f884d3ac128f09bfa57ac2648a19a19d854e Tree-SHA512: c0f3110f2907d87e29c755e3b77a67dfae1f8a25833fe6ef8f2f2c58cfecf1aa46f1a20881576b62252b04930140a9e416c78b4edba0780d3c4fa7aaebabba81
2023-11-17Merge bitcoin/bitcoin#28725: test: refactor: use built-in collection types ↵fanquake
for type hints (Python 3.9 / PEP 585) 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
2023-11-16test: replace `Callable`/`Iterable` with their `collections.abc` alternative ↵stickies-v
(PEP 585)
2023-11-16Merge bitcoin/bitcoin#28605: Fix typosfanquake
43de4d3630274e1287179c86896ed4c2d8b9eff4 doc: fix typos (Sjors Provoost) Pull request description: This PR fixes typos found by lint-spelling.py using codespell 2.2.6. Our CI linter job uses codespell 2.2.5 and found fewer typos that I did locally. In any case it's happy now. ACKs for top commit: pablomartin4btc: re ACK 43de4d3630274e1287179c86896ed4c2d8b9eff4 Tree-SHA512: c032fe86cb49c924a468385653b31f309a9db68c478d70335bba3e65a1ff3826abe80284fe00a090ab5a509e1edbf17e476f6922fb15d055e50f1103dad2ccb0
2023-11-14test: migrate to some per-symbol ubsan suppressionsfanquake
Tested on aarch64 using the ASAN CI job. Currently unable to test on x86_64 due to AppArmor & podman issues.
2023-11-13lint: Report all lint errors instead of early exitMarcoFalke
2023-11-13Merge bitcoin/bitcoin#28207: mempool: Persist with XORAndrew Chow
fa6b053b5c964fb35935fa994cb782c0731a56f8 mempool: persist with XOR (MarcoFalke) Pull request description: Currently the `mempool.dat` file stores data received from remote peers as-is. This may be problematic when a program other than Bitcoin Core tries to interpret them by accident. For example, an anti-virus program or other program may scan the file and move it into quarantine, or delete it, or corrupt it. While the local wallet is expected to re-submit any pending transactions, unrelated transactions may be missing from the mempool after a restart. This may cause fee estimates to be off, or may cause block relay to be slower. Fix this, similar to https://github.com/bitcoin/bitcoin/pull/6650, by rolling a random XOR pattern over the dat file when writing or reading it. Obviously this can only protect against programs that accidentally and unintentionally are trying to mess with the dat file. Any program that intentionally wants to mess with the dat file can still trivially do so. ACKs for top commit: achow101: re-ACK fa6b053b5c964fb35935fa994cb782c0731a56f8 glozow: reACK fa6b053b5c964fb35935fa994cb782c0731a56f8 ismaelsadeeq: ACK fa6b053b5c964fb35935fa994cb782c0731a56f8 Tree-SHA512: ded2ce3d81bc944b828263534e3178a1e45a914fe8e024f4a14c6561a73e301820944ecc75dd704b3d4221a7a3a5c0597ccab79546250c1197609ee981fe324e
2023-11-13Merge bitcoin/bitcoin#28076: util: Replace std::filesystem with util/fs.hfanquake
bbbbdb0cd57d75a06357d2811363d30a498f4499 ci: Add filesystem lint check (MarcoFalke) fada2f91108a56cc5c447bd6b6fac411e4d5cdca refactor: Replace <filesystem> with <util/fs.h> (MarcoFalke) Pull request description: Using `std::filesystem` is problematic: * There is a `fs` namespace wrapper for it. So having two ways to achieve the same is confusing. * Not using the `fs` wrapper is dangerous and buggy, because it disables known bugs by deleting problematic functions. Fix all issues by removing use of it and adding a linter to avoid using it again in the future. ACKs for top commit: TheCharlatan: ACK bbbbdb0cd57d75a06357d2811363d30a498f4499 fanquake: ACK bbbbdb0cd57d75a06357d2811363d30a498f4499 🦀 Tree-SHA512: 0e2d49742b08eb2635e6fce41485277cb9c40fe20b81017c391d3472a43787db1278a236825714ca1e41c9d2f59913865cfb0c649e3c8ab1fb598c849f80c660
2023-11-13Merge bitcoin/bitcoin#28831: test: Avoid intermittent failures in feature_initfanquake
44445ae8f1123c3affdcc0dbd7b3830eff5548ef test: Avoid intermittent failures in feature_init (MarcoFalke) Pull request description: The code not only modifies block dat files, but also leveldb files, which may be of smaller size. Such corruption may not force leveldb to abort, according to the intermittent test failures. Fix the intermittent test failures by reverting https://github.com/bitcoin/bitcoin/commit/5ab6419f380cc0a8cde78b125f3eeee5fcba43ae . ACKs for top commit: kevkevinpal: lgtm ACK [44445ae](https://github.com/bitcoin/bitcoin/pull/28831/commits/44445ae8f1123c3affdcc0dbd7b3830eff5548ef) fjahr: ACK 44445ae8f1123c3affdcc0dbd7b3830eff5548ef theStack: ACK 44445ae8f1123c3affdcc0dbd7b3830eff5548ef Tree-SHA512: 8084e4aeb8a976c1706a1898d7854c55d0c4ec7b5a08f65f97ffc173c935f9b0e0c1caef7be1538a458e4c018f7bd1948173349ec76ca48bc4013a63f284bb0f
2023-11-10test: fix node index bug when comparing peerinfoKashif Smith
2023-11-10Merge bitcoin/bitcoin#28835: test: Check error details with assert_debug_log ↵fanquake
on the assumeutxo invalid hash dump - follow-up #28698 7de76853728b423339d17f39224cf20305da1832 test, assumeutxo: Use assert_debug_log for error details (pablomartin4btc) Pull request description: This is a follow-up on the invalid hash dump fix #28698, [suggested](https://github.com/bitcoin/bitcoin/pull/28698#pullrequestreview-1698178157) by theStack and agreed by Sjors and ryanofsky. ACKs for top commit: Sjors: ACK 7de76853728b423339d17f39224cf20305da1832 maflcko: lgtm ACK 7de76853728b423339d17f39224cf20305da1832 Tree-SHA512: 036b3cef3084e3ead8923e8dcabe4fa7ebe97fb514d223aa38bc38df10337e3fe3113e42322178b58fb03fcd4511af4b5b56bceecbb7ded5b9758842c70db3f2
2023-11-09test, assumeutxo: Use assert_debug_log for error detailspablomartin4btc
This is a follow-up on the invalid hash dump fix PR #28698. https://github.com/bitcoin/bitcoin/pull/28698#pullrequestreview-1698178157
2023-11-09mempool: persist with XORMarcoFalke
2023-11-09test: Avoid intermittent failures in feature_initMarcoFalke
2023-11-09Merge bitcoin/bitcoin#28822: test: Add missing wait for version to be sent ↵fanquake
in add_outbound_p2p_connection faa2ad88bc01bd434ce19fde19bcc5c78431702f test: Add missing wait for version to be sent in add_outbound_p2p_connection (MarcoFalke) Pull request description: Can be tested with: ```diff diff --git a/test/functional/test_framework/p2p.py b/test/functional/test_framework/p2p.py index b1ed97b794..eb4f72c6b6 100755 --- a/test/functional/test_framework/p2p.py +++ b/test/functional/test_framework/p2p.py @@ -205,6 +205,7 @@ class P2PConnection(asyncio.Protocol): assert not self._transport logger.debug("Connected & Listening: %s:%d" % (self.dstaddr, self.dstport)) self._transport = transport + import time;time.sleep(.1); if self.on_connection_send_msg: self.send_message(self.on_connection_send_msg) self.on_connection_send_msg = None # Never used again ``` Found and reported by mzumsande in https://github.com/bitcoin/bitcoin/pull/28782#pullrequestreview-1718560252 ACKs for top commit: mzumsande: ACK faa2ad88bc01bd434ce19fde19bcc5c78431702f Tree-SHA512: 863f06125dec40cccaa852d0d8ca2e2b9c0b74610205e9fd6c9c279bdf36801ff475e3d873fd1b18172eb8220e17b2caff60069ce63512e569934a43f27d03fd
2023-11-08test: enable v2 transport for p2p_timeouts.pyMartin Zumsande
by skipping the part where we send a non-version message before the version - this message would be interpreted as part of the v2 handshake.
2023-11-08test: enable v2 transport for rpc_net.pySebastian Falbesoner
- "transport_protocol_type" of inbound peer before version handshake is "detecting" on p2p v2 nodes (as opposed to "v1" for p2p v1) - size of a ping/pong message is 29 bytes (as opposed to 32 for p2p v1) - for the sendmsgtopeer RPC sub-test, enforce p2p v1 connection to have a peer id of zero
2023-11-08test: enable v2 transport for p2p_node_network_limited.pySebastian Falbesoner
2023-11-08test: enable --v2transport in combination with --usecliMartin Zumsande
By renaming the "command" send_cli arg. The old name was unsuitable because the "addnode" RPC has its own "command" arg, leading to ambiguity when included in kwargs. Can be tested with "python3 wallet_multiwallet.py --usecli --v2transport" which fails on master because of this (python throws a TypeError).
2023-11-08test: persist -v2transport over restarts and respect -v2transport=0Martin Zumsande
Before, a global -v2transport provided to the test would be dropped when restarting the node within a test and specifying any extra_args. Fix this by adding "v2transport=1" to args (not extra_args) based on the global parameter, and deciding for each (re)start of the node based on this default and test-specific extra_args (which take precedence over args) whether v2 should be used.
2023-11-08Merge bitcoin/bitcoin#28787: init: completely remove `-zapwallettxes` ↵Andrew Chow
(remaining hidden option) 5039c346ca87d6112ea1eb124bdc622ba9e9a513 init: completely remove `-zapwallettxes` (remaining hidden option) (Sebastian Falbesoner) Pull request description: The `-zapwallettxes` functionality has been removed in v0.21.0 (see commit 3340dbadd38f5624642cf0e14dddbe6f83a3863b / PR #19671), with the parameter being kept as hidden option, to inform users via an exit error that `abandontransaction` should be used instead. As any guides that still suggest to use `-zapwallettxes` would refer to a Bitcoin Core version that is EOL since many years (i.e. <= v0.20.x), it is highly unlikely that the error caused by the option is still relevant for any user, hence it seems fine to remove it now. ACKs for top commit: achow101: ACK 5039c346ca87d6112ea1eb124bdc622ba9e9a513 BrandonOdiwuor: ACK 5039c346ca87d6112ea1eb124bdc622ba9e9a513 fanquake: ACK 5039c346ca87d6112ea1eb124bdc622ba9e9a513 Tree-SHA512: e3ccc6918e0f8fa68dbd1a7ec4999cc2a44e28038711919fcddaf0727648c73a9ba0fb77674317147592a113fad20755d4e727f48176bc17b048fbdebad2d6c9
2023-11-08Merge bitcoin/bitcoin#28155: net: improves addnode / m_added_nodes logicglozow
0420f99f429ce2382057e101859067f40de47be0 Create net_peer_connection unit tests (Jon Atack) 4b834f649921aceb44d3e0b5a2ffd7847903f9f7 Allow unit tests to access additional CConnman members (Jon Atack) 34b9ef443bc2655a85c8802edc5d5d48d792a286 net/rpc: Makes CConnman::GetAddedNodeInfo able to return only non-connected address on request (Sergi Delgado Segura) 94e8882d820969ddc83f24f4cbe1515a886da4ea rpc: Prevents adding the same ip more than once when formatted differently (Sergi Delgado Segura) 2574b7e177ef045e64f1dd48cb000640ff5103d3 net/rpc: Check all resolved addresses in ConnectNode rather than just one (Sergi Delgado Segura) Pull request description: ## Rationale Currently, `addnode` has a couple of corner cases that allow it to either connect to the same peer more than once, hence wasting outbound connection slots, or add redundant information to `m_added_nodes`, hence making Bitcoin iterate through useless data on a regular basis. ### Connecting to the same node more than once In general, connecting to the same node more than once is something we should try to prevent. Currently, this is possible via `addnode` in two different ways: 1. Calling `addnode` more than once in a short time period, using two equivalent but distinct addresses 2. Calling `addnode add` using an IP, and `addnode onetry` after with an address that resolved to the same IP For the former, the issue boils down to `CConnman::ThreadOpenAddedConnections` calling `CConnman::GetAddedNodeInfo` once, and iterating over the result to open connections (`CConman::OpenNetworkConnection`) on the same loop for all addresses.`CConnman::ConnectNode` only checks a single address, at random, when resolving from a hostname, and uses it to check whether we are already connected to it. An example to test this would be calling: ``` bitcoin-cli addnode "127.0.0.1:port" add bitcoin-cli addnode "localhost:port" add ``` And check how it allows us to perform both connections some times, and some times it fails. The latter boils down to the same issue, but takes advantage of `onetry` bypassing the `CConnman::ThreadOpenAddedConnections` logic and calling `CConnman::OpenNetworkConnection` straightaway. A way to test this would be: ``` bitcoin-cli addnode "127.0.0.1:port" add bitcoin-cli addnode "localhost:port" onetry ``` ### Adding the same peer with two different, yet equivalent, addresses The current implementation of `addnode` is pretty naive when checking what data is added to `m_added_nodes`. Given the collection stores strings, the checks at `CConnman::AddNode()` basically check wether the exact provided string is already in the collection. If so, the data is rejected, otherwise, it is accepted. However, ips can be formatted in several ways that would bypass those checks. Two examples would be `127.0.0.1` being equal to `127.1` and `[::1]` being equal to `[0:0:0:0:0:0:0:1]`. Adding any pair of these will be allowed by the rpc command, and both will be reported as connected by `getaddednodeinfo`, given they map to the same `CService`. This is less severe than the previous issue, since even tough both nodes are reported as connected by `getaddednodeinfo`, there is only a single connection to them (as properly reported by `getpeerinfo`). However, this adds redundant data to `m_added_nodes`, which is undesirable. ### Parametrize `CConnman::GetAddedNodeInfo` Finally, this PR also parametrizes `CConnman::GetAddedNodeInfo` so it returns either all added nodes info, or only info about the nodes we are **not** connected to. This method is used both for `rpc`, in `getaddednodeinfo`, in which we are reporting all data to the user, so the former applies, and to check what nodes we are not connected to, in `CConnman::ThreadOpenAddedConnections`, in which we are currently returning more data than needed and then actively filtering using `CService.fConnected()` ACKs for top commit: jonatack: re-ACK 0420f99f429ce2382057e101859067f40de47be0 kashifs: > > tACK [0420f9](https://github.com/bitcoin/bitcoin/commit/0420f99f429ce2382057e101859067f40de47be0) sr-gi: > > > tACK [0420f9](https://github.com/bitcoin/bitcoin/commit/0420f99f429ce2382057e101859067f40de47be0) mzumsande: Tested ACK 0420f99f429ce2382057e101859067f40de47be0 Tree-SHA512: a3a10e748c12d98d439dfb193c75bc8d9486717cda5f41560f5c0ace1baef523d001d5e7eabac9fa466a9159a30bb925cc1327c2d6c4efb89dcaf54e176d1752
2023-11-08test: Add missing wait for version to be sent in add_outbound_p2p_connectionMarcoFalke
2023-11-08Merge bitcoin/bitcoin#28820: tests: Increase wallet_miniscript.py rpc ↵fanquake
timeout to 90 seconds 6559e4d27ac9e8344d1211239cffab427b94bc23 tests: Increase wallet_miniscript.py rpc timeout to 90 seconds (Andrew Chow) Pull request description: The signing test for the large miniscript can sometimes take longer than the 30 second timeout, depending on the load on my system. Increasing it to 90 seconds seems to be good enough. ACKs for top commit: kevkevinpal: but increasing seems fine ACK [6559e4d](https://github.com/bitcoin/bitcoin/pull/28820/commits/6559e4d27ac9e8344d1211239cffab427b94bc23) maflcko: lgtm ACK 6559e4d27ac9e8344d1211239cffab427b94bc23 Tree-SHA512: 1b7bf94c77f85a0deddb1384aacbeb934205d0a630fecc8e75a4a98d1946d77d9bca36692fb6c1ab8e9276392f617281aafc4c685c248a8d3b0c77f896cda624
2023-11-08Merge bitcoin/bitcoin#28782: test: Add missing sync on send_version in ↵fanquake
peer_connect fa025984698270d8c9a621aef80f1d788ea2ab2e test: Add missing sync on send_version in peer_connect (MarcoFalke) Pull request description: Without the sync, the logic will be racy. For example, `p2p_sendtxrcncl.py` is failing locally (and on CI occasionally), because non-version messages will be sent before the version message: ```py self.log.info('SENDTXRCNCL with version=0 triggers a disconnect') sendtxrcncl_low_version = create_sendtxrcncl_msg() sendtxrcncl_low_version.version = 0 peer = self.nodes[0].add_p2p_connection(PeerNoVerack(), send_version=True, wait_for_verack=False) with self.nodes[0].assert_debug_log(["txreconciliation protocol violation"]): peer.send_message(sendtxrcncl_low_version) peer.wait_for_disconnect() ``` ``` test 2023-11-02T08:15:19.620000Z TestFramework (INFO): SENDTXRCNCL with version=0 triggers a disconnect test 2023-11-02T08:15:19.621000Z TestFramework.p2p (DEBUG): Connecting to Bitcoin Node: 127.0.0.1:11312 test 2023-11-02T08:15:19.624000Z TestFramework.p2p (DEBUG): Connected & Listening: 127.0.0.1:11312 test 2023-11-02T08:15:19.798000Z TestFramework.p2p (DEBUG): Send message to 127.0.0.1:11312: msg_sendtxrcncl(version=0, salt=2) test 2023-11-02T08:15:19.799000Z TestFramework.p2p (DEBUG): Send message to 127.0.0.1:11312: msg_version(nVersion=70016 nServices=9 nTime=Thu Nov 2 08:15:19 2023 addrTo=CAddress(nServices=1 net=IPv4 addr=127.0.0.1 port=11312) addrFrom=CAddress(nServices=1 net=IPv4 addr=0.0.0.0 port=0) nNonce=0x369AC031CDA96022 strSubVer=/python-p2p-tester:0.0.3/ nStartingHeight=-1 relay=1) node0 2023-11-02T08:15:19.804409Z [net] [net.cpp:3676] [CNode] [net] Added connection peer=0 node0 2023-11-02T08:15:19.805256Z [net] [net.cpp:1825] [CreateNodeFromAcceptedSocket] [net] connection from 127.0.0.1:55964 accepted node0 2023-11-02T08:15:19.809861Z [msghand] [net_processing.cpp:3356] [ProcessMessage] [net] received: sendtxrcncl (12 bytes) peer=0 node0 2023-11-02T08:15:19.810297Z [msghand] [net_processing.cpp:3582] [ProcessMessage] [net] non-version message before version handshake. Message "sendtxrcncl" from peer=0 node0 2023-11-02T08:15:19.810928Z [msghand] [net_processing.cpp:3356] [ProcessMessage] [net] received: version (111 bytes) peer=0 ... test 2023-11-02T09:35:20.166000Z TestFramework.utils (ERROR): wait_until() failed. Predicate: '''' def test_function(): if check_connected: assert self.is_connected return test_function_in() ''' test 2023-11-02T09:35:20.187000Z TestFramework (ERROR): Assertion failed Traceback (most recent call last): File "/ci_container_base/ci/scratch/build/bitcoin-s390x-linux-gnu/test/functional/test_framework/test_framework.py", line 132, in main self.run_test() File "/ci_container_base/ci/scratch/build/bitcoin-s390x-linux-gnu/test/functional/p2p_sendtxrcncl.py", line 188, in run_test peer.wait_for_disconnect() File "/ci_container_base/ci/scratch/build/bitcoin-s390x-linux-gnu/test/functional/test_framework/p2p.py", line 478, in wait_for_disconnect self.wait_until(test_function, timeout=timeout, check_connected=False) File "/ci_container_base/ci/scratch/build/bitcoin-s390x-linux-gnu/test/functional/test_framework/p2p.py", line 470, in wait_until wait_until_helper_internal(test_function, timeout=timeout, lock=p2p_lock, timeout_factor=self.timeout_factor) File "/ci_container_base/ci/scratch/build/bitcoin-s390x-linux-gnu/test/functional/test_framework/util.py", line 275, in wait_until_helper_internal raise AssertionError("Predicate {} not true after {} seconds".format(predicate_source, timeout)) AssertionError: Predicate '''' def test_function(): if check_connected: assert self.is_connected return test_function_in() ''' not true after 4800.0 seconds ACKs for top commit: mzumsande: ACK fa025984698270d8c9a621aef80f1d788ea2ab2e Tree-SHA512: 78871f603d387e2df8c0acbdfa95441fa186f80e94593021bb219bbf1bc9dc7efc4e266bd254b5cc41114c38227ff3b7f6172335d9bb828427f0a2acffde752d
2023-11-08Merge bitcoin/bitcoin#28814: test: symbolizer improvementsfanquake
49d953281df5618430728c0a88471695207f086b fuzz: explicitly specify llvm-symbolizer path in runner (fanquake) Pull request description: It's not completely clear to me why this needs to be explicitly specified in some environments, and not in others, while at the same time that `llvm-symbolizer` is already in PATH, but this has fixed the 2 issues outlined in https://github.com/bitcoin/bitcoin/pull/28147. Use `LLVM_SYMBOLIZER_PATH` as the env var, as that is somewhat also used inside LLVM, but not consistently, i.e it's checked for in the asan_symbolize script, but not in in the ubsan_symbolize script, or from in compiler-rt. Alternative to #28804. ACKs for top commit: maflcko: lgtm ACK 49d953281df5618430728c0a88471695207f086b Tree-SHA512: c3d5bf1c3629793b342c70754a419b3c7a3cd39f800b9aa69ce3395cc2bf83b4d46f2b329974337b94b99573cd0b8600d3f147ed5c21387bf3812316570d1ee3
2023-11-07tests: Increase wallet_miniscript.py rpc timeout to 90 secondsAndrew Chow
The signing test for the large miniscript can sometimes take longer than the 30 second timeout, depending on the load on my system. Increasing it to 90 seconds seems to be good enough.
2023-11-07Merge bitcoin/bitcoin#28374: test: python cryptography required for BIP 324 ↵Andrew Chow
functional tests c534c0871038ded72dc9078cc91e030ceb746196 [test/crypto] Add FSChaCha20Poly1305 AEAD python implementation (stratospher) c2a458f1c2f54aca132da9959b3fca19fdf1f813 [test/crypto] Add FSChaCha20 python implementation (stratospher) c4ea5f628806ead16e21d2a1f639fc6acc0400f5 [test/crypto] Add RFC 8439's ChaCha20Poly1305 AEAD (stratospher) 9fc6e0355e5ab52e76e647e5dad7571f962b4d4c [test/crypto] Add Poly1305 python implementation (stratospher) fec2ca6c9a8a8e44b6e4d51c4ef7fa6eaca6e446 [test/crypto] Use chacha20_block function in `data_to_num3072` (stratospher) 0cde60da3a16a27697f1f9014fcd33ea92eb639b [test/crypto] Add ChaCha20 python implementation (stratospher) 69d3f50ab645ad0ade1d67c66d2058b48bcf943b [test/crypto] Add HMAC-based Key Derivation Function (HKDF) (stratospher) 08a4a56cbcfa54366c2c0bb52bb147fc2740edc5 [test] Move test framework crypto functions to crypto/ (stratospher) Pull request description: split off from #24748 to keep commits related to cryptography and functional test framework changes separate. This PR adds python implementation and unit tests for HKDF, ChaCha20, Poly1305, ChaCha20Poly1305 AEAD, FSChaCha20 and FSChaCha20Poly1305 AEAD. They're based on https://github.com/bitcoin/bips/blob/cc177ab7bc5abcdcdf9c956ee88afd1052053328/bip-0324/reference.py for easy review. ACKs for top commit: sipa: utACK c534c0871038ded72dc9078cc91e030ceb746196 achow101: ACK c534c0871038ded72dc9078cc91e030ceb746196 theStack: re-ACK c534c0871038ded72dc9078cc91e030ceb746196 Tree-SHA512: 08a0a422d2937eadcf0edfede37e535e6bc4c2e4b192441bbf9bc26dd3f03fa3388effd22f0527c55af173933d0b50e5b2b3d36f2b62d0aca3098728ef06970e
2023-11-07Throw error if invalid parameters passed to getnetworkhashps RPC endpointJameson Lopp
2023-11-07fuzz: explicitly specify llvm-symbolizer path in runnerfanquake
It's not completely clear to me why this needs to be explicitly specified in some environments, and not in others, while at the same time that `llvm-symbolizer` is already in PATH, but this has fixed the 2 issues outlined in #28147. Use `LLVM_SYMBOLIZER_PATH` as the env var, as that is somewhat also used inside LLVM, but not consistently, i.e it's checked for in the asan_symbolize script, but not in in the ubsan_symbolize script, or from in compiler-rt.
generated by cgit v1.2.3 (git 2.26.2) at 2024-09-14 08:26:21 +0000