aboutsummaryrefslogtreecommitdiff
path: root/test/functional/test_framework
AgeCommit message (Collapse)Author
2024-05-09Merge bitcoin/bitcoin#30006: test: use sleepy wait-for-log in reindex readonlyAva Chow
fd6a7d3a13d89d74e161095b0e9bd3570210a40c test: use sleepy wait-for-log in reindex readonly (Matthew Zipkin) Pull request description: Also rename the busy wait-for-log method to prevent recurrence. See https://github.com/bitcoin/bitcoin/pull/27039#discussion_r1532578152 ACKs for top commit: maflcko: utACK fd6a7d3a13d89d74e161095b0e9bd3570210a40c achow101: ACK fd6a7d3a13d89d74e161095b0e9bd3570210a40c tdb3: ACK for fd6a7d3a13d89d74e161095b0e9bd3570210a40c rkrux: ACK [fd6a7d3](https://github.com/bitcoin/bitcoin/pull/30006/commits/fd6a7d3a13d89d74e161095b0e9bd3570210a40c) Tree-SHA512: 7ff0574833df1ec843159b35ee88b8bb345a513ac13ed0b72abd1bf330c454a3f9df4d927871b9e3d37bfcc07542b06ef63acef8e822cd18499adae8cbb0cda8
2024-05-05test: use tagged ephemeral MiniWallet instance in fill_mempoolSebastian Falbesoner
2024-05-05test: add MiniWallet tagging support to avoid UTXO mixingSebastian Falbesoner
Note that this commit doesn't change behaviour yet, as tagging isn't used in any MiniWallet instance.
2024-05-05test: refactor: eliminate COINBASE_MATURITY magic number in fill_mempoolSebastian Falbesoner
2024-05-05test: refactor: move fill_mempool to new module mempool_utilSebastian Falbesoner
This is needed to avoid circular dependencies in later commits. Can be reviewed via `--color-moved=dimmed-zebra`.
2024-05-03[test]: remove duplicate WITNESS_SCALE_FACTORismaelsadeeq
2024-04-30test: use sleepy wait-for-log in reindex readonlyMatthew Zipkin
Also rename the busy wait-for-log method to prevent recurrence
2024-04-25Merge bitcoin/bitcoin#29615: test: fix accurate multisig sigop count ↵Ava Chow
(BIP16), add unit test 3e9c736a26724ffe3b70b387995fbf48c06300e2 test: fix accurate multisig sigop count (BIP16), add unit test (Sebastian Falbesoner) Pull request description: In the course of reviewing #29589 I noticed the following buggy call-site of `CScriptOp.decode_op_n` in the CScript's `GetSigOpCount` method: https://github.com/bitcoin/bitcoin/blob/4cc99df44aec4d104590aee46cf18318e22a8568/test/functional/test_framework/script.py#L591-L593 This should be `lastOpcode` rather than `opcode`. The latter is either OP_CHECKMULTISIG or OP_CHECKMULTISIGVERIFY at this point, so `decode_op_n` would result in an error. Also, in `CScript.raw_iter`, we have to return the op as `CScriptOp` type instead of a bare integer, otherwise we can't call the decode method on it. To prevent this in the future, add some simple unit tests for `GetSigOpCount`. Note that this was unnoticed, as the code part was never hit so far in the test framework. ACKs for top commit: achow101: ACK 3e9c736a26724ffe3b70b387995fbf48c06300e2 Christewart: ACK 3e9c736a26724ffe3b70b387995fbf48c06300e2 rkrux: tACK [3e9c736](https://github.com/bitcoin/bitcoin/pull/29615/commits/3e9c736a26724ffe3b70b387995fbf48c06300e2) hernanmarino: tACK 3e9c736a26724ffe3b70b387995fbf48c06300e2 Tree-SHA512: 51647bb6d462fbd101effd851afdbd6ad198c0567888cd4fdcac389a9fb4bd3d7e648095c6944fd8875d36272107ebaabdc62d0e2423289055588c12294d05a7
2024-04-25Merge bitcoin/bitcoin#29736: test: Extends wait_for_getheaders so a specific ↵Ava Chow
block hash can be checked c4f857cc301d856f3c60acbe6271d3fe19441c7a test: Extends wait_for_getheaders so a specific block hash can be checked (Sergi Delgado Segura) Pull request description: Fixes https://github.com/bitcoin/bitcoin/issues/18614 Previously, `wait_for_getheaders` would check whether a node had received **any** getheaders message. This implied that, if a test needed to check for a specific block hash within a headers message, it had to make sure that it was checking the desired message. This normally involved having to manually clear `last_message`. This method, apart from being too verbose, was error-prone, given an undesired `getheaders` would make tests pass. This adds the ability to check for a specific block_hash within the last `getheaders` message. ACKs for top commit: achow101: ACK c4f857cc301d856f3c60acbe6271d3fe19441c7a BrandonOdiwuor: crACK c4f857cc301d856f3c60acbe6271d3fe19441c7a cbergqvist: ACK c4f857cc301d856f3c60acbe6271d3fe19441c7a stratospher: tested ACK c4f857c. went through all getheaders messages sent in the tests and checked that it's the one we want. Tree-SHA512: afc9a31673344dfaaefcf692ec2ab65958c3d4c005f5f3af525e9960f0622d8246d5311e59aba06cfd5c9e0ef9eb90a7fc8e210f030bfbe67b897c061efdeed1
2024-04-25Fix typos in description.md and wallet_util.pyhanmz
Signed-off-by: hanmz <hanmzarsenal@gmail.com>
2024-04-22Merge bitcoin/bitcoin#29777: test: refactor: introduce and use ↵Ava Chow
`calculate_input_weight` helper 6d91cb781c30966963f28e7577c7aa3829fa9390 test: add unit tests for `calculate_input_weight` (Sebastian Falbesoner) f81fad5e0f3be1f7aed59f9da00396c75c2a6406 test: introduce and use `calculate_input_weight` helper (Sebastian Falbesoner) Pull request description: Rather than manually estimating an input's weight by adding up all the involved components (fixed-size skeleton, compact-serialized lengths, and the actual scriptSig / witness stack items) we can simply take use of the serialization classes `CTxIn` / `CTxInWitness` instead, to achieve the same with significantly less code. The new helper is used in the functional tests rpc_psbt.py and wallet_send.py, where the previous manual estimation code was duplicated. Unit tests are added in the second commit. ACKs for top commit: kevkevinpal: tACK [6d91cb7](https://github.com/bitcoin/bitcoin/pull/29777/commits/6d91cb781c30966963f28e7577c7aa3829fa9390) QureshiFaisal: tACK [6d91cb7](https://github.com/bitcoin/bitcoin/pull/29777/commits/6d91cb781c30966963f28e7577c7aa3829fa9390) achow101: ACK 6d91cb781c30966963f28e7577c7aa3829fa9390 AngusP: tACK 6d91cb781c30966963f28e7577c7aa3829fa9390 rkrux: tACK [6d91cb7](https://github.com/bitcoin/bitcoin/pull/29777/commits/6d91cb781c30966963f28e7577c7aa3829fa9390) Tree-SHA512: 04424e4d94d0e13745a9c11df2dd3697c98552bbb0e792c4af67ecbb66060adc3cc0cefc202cdee2d9db0baf85b8bedf2eb339ac4b316d986b5f10f6b70c5a33
2024-04-15fixup: get all utxos up front in fill_mempool, discourage wallet mixingglozow
Co-authored-by: Sebastian Falbesoner <sebastian.falbesoner@gmail.com>
2024-04-11Merge bitcoin/bitcoin#29735: AcceptMultipleTransactions: Fix workspace not ↵glozow
being set as client_maxfeerate failure 4ba1d0b55339c3ea90e2bcd64662a06f0f90dd46 fuzz: Add coverage for client_maxfeerate (Greg Sanders) 91d7d8f22a1c528db14fa743c66cd861ea00e84b AcceptMultipleTransactions: Fix workspace client_maxfeerate (Greg Sanders) f3aa5bd5eb6d1088f98a4dc7daaab0e17a7d5529 fill_mempool: assertions and docsctring update (Greg Sanders) a3da63e8febe475f2250f6432bca237d31fa9107 Move fill_mempool to util function (Greg Sanders) 73b68bd8b4f9447e30091c7f8c3dc91a086bd93b fill_mempool: remove subtest-specific comment (Greg Sanders) Pull request description: Bug causes an `Assume()` failure due to the expectation that the individual result should be invalid when done over `submitpackage` via rpc. Bug introduced by https://github.com/bitcoin/bitcoin/pull/28950 , and I discovered it rebasing https://github.com/bitcoin/bitcoin/pull/28984 since it's easier to hit in that test scenario. Tests in place were only checking `AcceptSingleTransaction`-level checks due to package evaluation only triggering when minfee is too high for the parent transaction. Added test along with fix, moving the fill_mempool utility into a common area for re-use. ACKs for top commit: glozow: reACK 4ba1d0b55339c3ea90e2bcd64662a06f0f90dd46 theStack: ACK 4ba1d0b55339c3ea90e2bcd64662a06f0f90dd46 ismaelsadeeq: re-ACK https://github.com/bitcoin/bitcoin/commit/4ba1d0b55339c3ea90e2bcd64662a06f0f90dd46 via [diff](https://github.com/bitcoin/bitcoin/compare/4fe7d150eb3c85a6597d8fc910fe1490358197ad..4ba1d0b55339c3ea90e2bcd64662a06f0f90dd46) Tree-SHA512: 3729bdf7f25d04e232f173ccee04ddbb2afdaafa3d04292a01cecf58fb11b3b2bc133e8490277f1a67622b62d17929c242dc980f9bb647896beea4332ee35306
2024-04-09fill_mempool: assertions and docsctring updateGreg Sanders
2024-04-09Move fill_mempool to util functionGreg Sanders
2024-04-09Merge bitcoin/bitcoin#29498: test: Update --tmpdir doc string to say ↵fanquake
directory must not exist d4e36ae80d4b3f03647fd9057461edf7ecd794a3 test: Update --tmpdir doc string to say directory must not exist (kevkevin) Pull request description: The error message given if passing an existing dir to --tmpdir is confusing so this makes it clear that the directory must not already exist This change is motivated by this comment https://github.com/bitcoin/bitcoin/pull/29335#issuecomment-1960913020 ACKs for top commit: maflcko: lgtm ACK d4e36ae80d4b3f03647fd9057461edf7ecd794a3 davidgumberg: ACK https://github.com/bitcoin/bitcoin/pull/29498/commits/d4e36ae80d4b3f03647fd9057461edf7ecd794a3 Tree-SHA512: fb31fd079767abbf94076615817943f35f5c9262fc97e65c631a18d33b3a343fe6a2d151613256e632d2b372ab2de0435f4712309b4a77ed3c663fd93a7dcdd1
2024-04-05test: add unit tests for `calculate_input_weight`Sebastian Falbesoner
2024-04-05test: introduce and use `calculate_input_weight` helperSebastian Falbesoner
Rather than manually estimating an input's weight by adding up all the involved components (fixed-size skeleton, compact-serialized lengths, and the actual scriptSig / witness stack items) we can simply take use of the serialization classes `CTxIn` / `CTxInWitness` instead, to achieve the same with significantly less code. The new helper is used in the functional tests rpc_psbt.py and wallet_send.py, where the previous manual estimation code was duplicated.
2024-04-04test: Extends wait_for_getheaders so a specific block hash can be checkedSergi Delgado Segura
Previously, `wait_for_getheaders` would check whether a node had received **any** getheaders message. This implied that, if a test needed to check for a specific block hash within a headers message, it had to make sure that it was checking the desired message. This normally involved having to manually clear `last_message`. This method, apart from being too verbose, was error prone, given an undesired `getheaders` would make tests pass. This adds the ability to check for a specific block_hash within the last `getheaders` message.
2024-03-27test: makes timeout a forced named argument in tests methods that use itSergi Delgado Segura
This makes calls to such methods more explicit and less error prone
2024-03-19Merge bitcoin/bitcoin#29279: test: p2p: check disconnect due to lack of ↵glozow
desirable service flags 2f23987849758537f76df7374d85a7e87b578b61 test: p2p: check limited peers desirability (depending on best block depth) (Sebastian Falbesoner) c4a67d396d0aa99f658cafe381e39622859eb0be test: p2p: check disconnect due to lack of desirable service flags (Sebastian Falbesoner) 405ac819af1eb0f6cf6d1805cb668f4e8ab4a6f3 test: p2p: support disconnect waiting for `add_outbound_p2p_connection` (Sebastian Falbesoner) Pull request description: This PR adds missing test coverage for disconnecting peers which don't offer the desirable service flags in their VERSION message: https://github.com/bitcoin/bitcoin/blob/5f3a0574c45477288bc678b15f24940486084576/src/net_processing.cpp#L3384-L3389 This check is relevant for the connection types "outbound-full-relay", "block-relay-only" and "addr-fetch" (see `CNode::ExpectServicesFromConn(...)`). Feeler connections always disconnect, which is also tested here. In lack of finding a proper file where this test would fit in, I created a new one. Happy to take suggestions there. ACKs for top commit: davidgumberg: reACK https://github.com/bitcoin/bitcoin/commit/2f23987849758537f76df7374d85a7e87b578b61 itornaza: tested ACK 2f23987849758537f76df7374d85a7e87b578b61 fjahr: re-utACK 2f23987849758537f76df7374d85a7e87b578b61 cbergqvist: re ACK 2f23987849758537f76df7374d85a7e87b578b61 stratospher: tested ACK 2f23987. 🚀 Tree-SHA512: cf75d9d4379d0f34fa1e13152e6a8d93cd51b9573466ab3a2fec32dc3e1ac49b174bd1063cae558bc736b111c8a6e7058b1b57a496df56255221bf367d29eb5d
2024-03-13Merge bitcoin/bitcoin#27375: net: support unix domain sockets for -proxy and ↵Ava Chow
-onion 567cec9a05e1261e955535f734826b12341684b6 doc: add release notes and help text for unix sockets (Matthew Zipkin) bfe51928911daf484ae07deb52a7ff0bcb2526ae test: cover UNIX sockets in feature_proxy.py (Matthew Zipkin) c65c0d01630b44fa71321ea7ad68d5f9fbb7aefb init: allow UNIX socket path for -proxy and -onion (Matthew Zipkin) c3bd43142eba77dcf1acd4984e437759f65e237a gui: accomodate unix socket Proxy in updateDefaultProxyNets() (Matthew Zipkin) a88bf9dedd1d8c1db0a9c8b663dab3e3c2f0f030 i2p: construct Session with Proxy instead of CService (Matthew Zipkin) d9318a37ec09fe0b002815a7e48710e530620ae2 net: split ConnectToSocket() from ConnectDirectly() for unix sockets (Matthew Zipkin) ac2ecf3182fb5ad9bcd41540b19382376114d6ee proxy: rename randomize_credentials to m_randomize_credentials (Matthew Zipkin) a89c3f59dc44eaf4f59912c1accfc0ce5d61933a netbase: extend Proxy class to wrap UNIX socket as well as TCP (Matthew Zipkin) 3a7d6548effa6cd9a4a5413b690c2fd85da4ef65 net: move CreateSock() calls from ConnectNode() to netbase methods (Matthew Zipkin) 74f568cb6fd5c74b7b9bf0ce69876430746a53b1 netbase: allow CreateSock() to create UNIX sockets if supported (Matthew Zipkin) bae86c8d318d06818aa75a9ebe3db864197f0bc6 netbase: refactor CreateSock() to accept sa_family_t (Matthew Zipkin) adb3a3e51de205cc69b1a58647c65c04fa6c6362 configure: test for unix domain sockets (Matthew Zipkin) Pull request description: Closes https://github.com/bitcoin/bitcoin/issues/27252 UNIX domain sockets are a mechanism for inter-process communication that are faster than local TCP ports (because there is no need for TCP overhead) and potentially more secure because access is managed by the filesystem instead of serving an open port on the system. There has been work on [unix domain sockets before](https://github.com/bitcoin/bitcoin/pull/9979) but for now I just wanted to start on this single use-case which is enabling unix sockets from the client side, specifically connecting to a local Tor proxy (Tor can listen on unix sockets and even enforces strict curent-user-only access permission before binding) configured by `-onion=` or `-proxy=` I copied the prefix `unix:` usage from Tor. With this patch built locally you can test with your own filesystem path (example): `tor --SocksPort unix:/Users/matthewzipkin/torsocket/x` `bitcoind -proxy=unix:/Users/matthewzipkin/torsocket/x` Prep work for this feature includes: - Moving where and how we create `sockaddr` and `Sock` to accommodate `AF_UNIX` without disturbing `CService` - Expanding `Proxy` class to represent either a `CService` or a UNIX socket (by its file path) Future work: - Enable UNIX sockets for ZMQ (https://github.com/bitcoin/bitcoin/pull/27679) - Enable UNIX sockets for I2P SAM proxy (some code is included in this PR but not tested or exposed to user options yet) - Enable UNIX sockets on windows where supported - Update Network Proxies dialog in GUI to support UNIX sockets ACKs for top commit: Sjors: re-ACK 567cec9a05e1261e955535f734826b12341684b6 tdb3: re ACK for 567cec9a05e1261e955535f734826b12341684b6. achow101: ACK 567cec9a05e1261e955535f734826b12341684b6 vasild: ACK 567cec9a05e1261e955535f734826b12341684b6 Tree-SHA512: de81860e56d5de83217a18df4c35297732b4ad491e293a0153d2d02a0bde1d022700a1131279b187ef219651487537354b9d06d10fde56225500c7e257df92c1
2024-03-12Merge bitcoin/bitcoin#27114: p2p: Allow whitelisting manual connectionsAva Chow
0a533613fb44207053796fd01a9f4b523a3153d4 docs: add release notes for #27114 (brunoerg) e6b8f19de9a6d1c477d0bbda18d17794cd81a6f4 test: add coverage for whitelisting manual connections (brunoerg) c985eb854cc86deb747caea5283c17cf51b6a983 test: add option to speed up tx relay/mempool sync (brunoerg) 66bc6e2d1749f43d7b314aa2784a06af78440170 Accept "in" and "out" flags to -whitelist to allow whitelisting manual connections (Luke Dashjr) 8e06be347c5e14cbe75256eba170e0867f95f360 net_processing: Move extra service flag into InitializeNode (Luke Dashjr) 9133fd69a5cc9a0ab1a06a60d09f1b7e1039018e net: Move `NetPermissionFlags::Implicit` verification to `AddWhitelistPermissionFlags` (Luke Dashjr) 2863d7dddb62d987b3e1c3b8bfad7083f0f774b2 net: store `-whitelist{force}relay` values in `CConnman` (brunoerg) Pull request description: Revives #17167. It allows whitelisting manual connections. Fixes #9923 Since there are some PRs/issues around this topic, I'll list some motivations/comments for whitelisting outbound connections from them: - Speed-up tx relay/mempool sync for testing purposes (my personal motivation for this) - In #26970, theStack pointed out that we whitelist peers to speed up tx relay for fast mempool synchronization, however, since it applies only for inbound connections and considering the topology `node0 <--- node1 <---- node2 <--- ... <-- nodeN`, if a tx is submitted from any node other than node0, the mempool synchronization can take quite long. - https://github.com/bitcoin/bitcoin/pull/29058#issuecomment-1865155764 - "Before enabling -v2transport by default (which I'd image may happen after https://github.com/bitcoin/bitcoin/pull/24748) we could consider a way to force manual connections to be only-v1 or even only-v2 (disabling reconnect-with-v1). A possibility could be through a net permission flag, if https://github.com/bitcoin/bitcoin/pull/27114 makes it in." - https://github.com/bitcoin/bitcoin/pull/17167#issuecomment-1168606032 - "This would allow us to use https://github.com/bitcoin/bitcoin/pull/25355 when making outgoing connections to all nodes, except to whitelisted ones for which we would use our persistent I2P address." - Force-relay/mempool permissions for a node you intentionally connected to. ACKs for top commit: achow101: ACK 0a533613fb44207053796fd01a9f4b523a3153d4 sr-gi: re-ACK [0a53361](https://github.com/bitcoin/bitcoin/pull/27114/commits/0a533613fb44207053796fd01a9f4b523a3153d4) pinheadmz: ACK 0a533613fb44207053796fd01a9f4b523a3153d4 Tree-SHA512: 97a79bb854110da04540897d2619eda409d829016aafdf1825ab5515334b0b42ef82f33cd41587af235b3af6ddcec3f2905ca038b5ab22e4c8a03d34f27aebe1
2024-03-11Merge bitcoin/bitcoin#29007: test: create deterministic addrman in the ↵Ava Chow
functional tests 2cc8ca19f4185490f30a49516c890b2289fbab71 [test] Use deterministic addrman in addrman info tests (stratospher) a8978661093500df8d8dcf2a9c90837afacd0aab [test] Restart a node with empty addrman (stratospher) 71c19915c0c716d6f8a539dd92b8ad41e8c447ee [test] Use deterministic addrman in addpeeraddress test (stratospher) 7b868e6b678502e86571976d696c0e3cb72c0884 Revert "test: avoid non-determinism in asmap-addrman test" (stratospher) 69e091f3e1f65b12cf1804e7d39651f55a01827a [init] Create deterministic addrman in tests using -test=addrman (stratospher) be25ac3092b7755e26e1ec6c33a27cd0e3dd9eac [init] Remove -addrmantest command line arg (stratospher) 802e6e128bba5ffa6d4ec53ff45acccb7cb28f21 [init] Add new command line arg for use only in functional tests (stratospher) Pull request description: An address is placed in a `[bucket,position]` in the addrman table (new table or tried table) using the `addpeeraddress` RPC. This `[bucket,position]` is calculated using `nKey`(and other metrics) for the addrman which is chosen randomly during every run. Supposing there are 2 addresses to be placed in an addrman table. During every test run, a different `[bucket,position]` would be calculated for each address.These calculated `[bucket,position]` could even be the same for the 2 addresses in some test runs and result in collisions in the addrman. We wouldn't be able to predict when the collisions are going to happen because we can't predict the `nKey` value which is chosen at random. This can cause flaky tests. Because of these non deterministic collisions, we are limited in what we can do to test addrman functionality. Currently in our tests don't add a second address to prevent these collisions from happening - we only keep 1 address in the new table and 1 address in the tried table. See https://github.com/bitcoin/bitcoin/pull/26988#discussion_r1091145647, https://github.com/bitcoin/bitcoin/pull/23084, [#22831(comment)](https://github.com/bitcoin/bitcoin/pull/22831/files#r708302639). This PR lets us create a deterministic addrman with fixed `nKey` so that we can know the `[bucket,position]` collisions beforehand, safely add more addresses in an addrman table and write more extensive tests. ACKs for top commit: amitiuttarwar: ACK 2cc8ca19f4185490f30a49516c890b2289fbab71 achow101: ACK 2cc8ca19f4185490f30a49516c890b2289fbab71 0xB10C: ACK 2cc8ca19f4185490f30a49516c890b2289fbab71 mzumsande: Code Review ACK 2cc8ca19f4185490f30a49516c890b2289fbab71 Tree-SHA512: 8acd9bdfe7de1eb44d22373bf13533d8ecf602df966fdd5b8b78afcd8cc35a286c95d2712f67a89473a0d68dded7d38f5599f6e4bf95a6589475444545bfb189
2024-03-11test: p2p: check disconnect due to lack of desirable service flagsSebastian Falbesoner
2024-03-11test: p2p: support disconnect waiting for `add_outbound_p2p_connection`Sebastian Falbesoner
Adds a new boolean parameter `wait_for_disconnect` to the `add_outbound_p2p_connection` method. If set, the node under test is checked to disconnect immediately after receiving the version message (same logic as for feeler connections).
2024-03-11Merge bitcoin/bitcoin#29483: test, ci: add --v1transport option, add ↵Ava Chow
--v2transport to a CI task ecc036c5d63fb4bdff5caeede88baeb85bf62b05 ci: add --v2transport to an existing CI job (Martin Zumsande) 3a25a575f0c455b50abf95d85c0ba8de3cf53a87 test: ignore --v2transport for older versions instead of asserting (Martin Zumsande) 547aacff08a2a5d786cb0fa6c7bae022ea651f8c test: add -v1transport option and use it in test_runner (Martin Zumsande) Pull request description: This suggests a strategy to run the functional tests with both v1 and v2 transport in the CI. **Status Quo:** There is both the global `--v2transport` option for the `test_runner.py` (not enabled by default), plus the possibility to specify `--v2transport` for particular tests, which is used for a handful of tests. Currently, when running `test_runner.py --v2transport`, these tests are run twice with the same `--v2transport` configuration, as has been noted in https://github.com/bitcoin/bitcoin/pull/29358#discussion_r1485626063, which is wasteful. **Suggested Change:** Fix this by adding a `--v1transport` option and using it in `test_runner.py`, so that irrespective of the global `--v2transport` flag, the tests that run twice use v1 in one run and v2 in the other. Also add `--v2transport` to one CI task (`multiprocess, i686, DEBUG`). This means, that for each CI task, the majority of functional tests will run once using the global `--v2transport` option if provided, while a few selected tests will always run two times, once with `v1` and once with `v2`. **Rationale:** A simpler alternative would have been to remove all test-specific `--v2transport` commands from `test_runner.py` and just enable `--v2transport` option for a few CI tasks. I didn't do that because it would have meant that v2 would never be running in the CI for some platforms, and also be run a lot less locally by users and devs (who would have to actively enable the `--v2transport` option). ACKs for top commit: tdb3: ACK for ecc036c5d63fb4bdff5caeede88baeb85bf62b05. achow101: ACK ecc036c5d63fb4bdff5caeede88baeb85bf62b05 stratospher: ACK ecc036c. vasild: ACK ecc036c5d63fb4bdff5caeede88baeb85bf62b05 Tree-SHA512: 375b2293d49991f2fbd8e1bb646c0034004a09cee36063bc32996b721323eb19a43d7b2f36b3f9a3fdca846d74f48d8f1387565c03ef5d34b3481d2a0fe1d328
2024-03-11Merge bitcoin/bitcoin#29514: tests: Provide more helpful assert_equal errorsAva Chow
a3badf75f6fd88d465e59f46f0336a0c1eacb7de tests: Provide more helpful assert_equal errors (Anthony Towns) Pull request description: 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. ACKs for top commit: achow101: ACK a3badf75f6fd88d465e59f46f0336a0c1eacb7de vasild: ACK a3badf75f6fd88d465e59f46f0336a0c1eacb7de brunoerg: utACK a3badf75f6fd88d465e59f46f0336a0c1eacb7de josibake: ACK https://github.com/bitcoin/bitcoin/pull/29514/commits/a3badf75f6fd88d465e59f46f0336a0c1eacb7de BrandonOdiwuor: Code Review ACK a3badf75f6fd88d465e59f46f0336a0c1eacb7de Tree-SHA512: 1d4b4a3b2e2e28ab09f10b41b04b52b37f64e0d8a54e2306f37de0c3eb3299a7ad4ba225b9efa67057a75e90d008a17385c810a32c9b212d240be280c2dcf2e5
2024-03-10test: fix accurate multisig sigop count (BIP16), add unit testSebastian Falbesoner
2024-03-01test: cover UNIX sockets in feature_proxy.pyMatthew Zipkin
2024-02-29Merge bitcoin/bitcoin#29390: test: speedup bip324_cipher.py unit testAva Chow
a8c3454ba137dfac20b3c89bc558374de0524114 test: speedup bip324_cipher.py unit test (Sebastian Falbesoner) Pull request description: Executing the unit tests for the bip324_cipher.py module currently takes quite long (>60 seconds on my older notebook). Most time here is spent in empty plaintext/ciphertext encryption/decryption loops in `test_fschacha20poly1305aead`: https://github.com/bitcoin/bitcoin/blob/9eeee7caa3f95ee17a645e12d330261f8e3c2dbf/test/functional/test_framework/crypto/bip324_cipher.py#L193-L194 https://github.com/bitcoin/bitcoin/blob/9eeee7caa3f95ee17a645e12d330261f8e3c2dbf/test/functional/test_framework/crypto/bip324_cipher.py#L198-L199 Their sole purpose is increasing the FSChaCha20Poly1305 packet counter in order to trigger rekeying, i.e. the actual encryption/decryption is not relevant, as the result is thrown away. This commit speeds up the tests by supporting to pass "None" as plaintext/ciphertext, indicating to the routines that no actual encryption/decryption should be done. The approach here is a bit hacky, a cleaner alternative would probably be to introduce a special `seek`/`skip_packets` method and not touch the encrypt/decrypt routines, but that seemed overkill to me only for speeding up a unit test. Open for suggestions. master branch: ``` $ python3 -m unittest ./test/functional/test_framework/crypto/bip324_cipher.py .. ---------------------------------------------------------------------- Ran 2 tests in 64.658s ``` PR branch: ``` $ python3 -m unittest ./test/functional/test_framework/crypto/bip324_cipher.py .. ---------------------------------------------------------------------- Ran 2 tests in 0.822s ``` ACKs for top commit: delta1: Concept ACK a8c3454 epiccurious: Tested ACK a8c3454ba137dfac20b3c89bc558374de0524114. achow101: ACK a8c3454ba137dfac20b3c89bc558374de0524114 marcofleon: ACK a8c3454ba137dfac20b3c89bc558374de0524114. The comments at the top of `bip324_cipher.py` specify that this should only be used for testing, so I think this optimization makes sense in that context. cbergqvist: ACK a8c3454! stratospher: ACK a8c3454. I think it's worth it because of the significant speedup in the unit test. Tree-SHA512: 737dd805a850be6e035aa3c6d9e2c5b5b5e89ddc564f84a045c37e0238fef6419912de7c902139b64914abdd647c649fe02a694f1a5e1741d7d4459c041caccc
2024-02-29test: ignore --v2transport for older versions instead of assertingMartin Zumsande
Otherwise, a run with test_runner.py --v2transport=1 --previous-releases --extended would hit the removed assert for wallet_backwards_compatibility.py
2024-02-29test: add -v1transport option and use it in test_runnerMartin Zumsande
This option beats the --v2transport option and is meant to be used in test_runner.py. It applies these to a few tests that are particulary interesting in terms of the transport type. This ensures that these tests arei always run with both v1 and v2, irrespective of whether the global --v2transport test_runner option is set or not.
2024-02-29tests: Provide more helpful assert_equal errorsAnthony Towns
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.
2024-02-28test: add option to speed up tx relay/mempool syncbrunoerg
when `self.noban_tx_relay=True`, the following flag `-whitelist=noban,in,out@127.0.0.1`is added to `extra_args` to speed up tx relay/mempool sync.
2024-02-27test: Update --tmpdir doc string to say directory must not existkevkevin
The error message given if passing an existing dir to --tmpdir is confusing so this makes it clear that the directory must not already exist
2024-02-27doc: Fix Broken LinksJustin Dhillon
2024-02-27Merge bitcoin/bitcoin#29358: test: use v2 everywhere for P2PConnection if ↵fanquake
--v2transport is enabled bf5662c678455fb47c402f8520215726ddfe7a94 test: enable v2 for python p2p depending on global --v2transport flag (Martin Zumsande) 6e9e39da434f8dafacee4cba068daf499bdb4cc2 test: Don't use v2transport when it's too slow. (Martin Zumsande) 87549c8f89fe8c9f404b74c5a40b5ee54c5a966d test: enable p2p_invalid_messages.py with v2transport (Martin Zumsande) 5fc9db504b9ac784019e7e8c215c31abfccb62b6 test: enable p2p_sendtxrcncl.py with v2transport (Martin Zumsande) Pull request description: #24748 added v2 transport to the python `P2PConnection`, but so far each test that wants to make use of it needs to enable it on an individual basis. This PR changes it so that if the test suite is run with `--v2transport` option, v2 is used in each test by default, not only for connections between two bitcoind instances as before, but also wherever `P2PConnection` is used. Individual tests can override this global option. To do that, a few tests need to be adjusted. In addition, I added a commit to always use v1 in a few select subtests that send a large number of large messages (e.g. large reorgs). These tests don't have a fundamental problem with v2 but become very slow due to the unoptimised python ChaCha20 implementation (~30 minutes on my computer, so probably not suitable to be run in the CI). As a result, `python3 test_runner.py --v2transport` should succeed and use `v2` everywhere (unless v1 is chosen explicitly). [Edit]: To make the "test each commit" CI pass, several test fixes were squashed into the last commit, which actually enables v2 p2p for `P2PConnection`. I have an unsquashed version at https://github.com/mzumsande/bitcoin/tree/202401_bip324_alltests_unsquashed, in case that helps with review. ACKs for top commit: fjahr: tACK bf5662c678455fb47c402f8520215726ddfe7a94 vasild: ACK bf5662c678455fb47c402f8520215726ddfe7a94 stratospher: reACK bf5662c6. theStack: Tested ACK bf5662c678455fb47c402f8520215726ddfe7a94 Tree-SHA512: 4f5a08248ba8a755f7d0f48deb2b79bef03292345cacb7deef01be955481093800e4e56ff218ea56734eef5de1fb3ab0f04657447ea27d393bb536539d7b289d
2024-02-21Merge bitcoin/bitcoin#29400: test: Fix SegwitV0SignatureMsg nLockTime signednessAva Chow
fab15723b0518acbb1015e64df47dcac0187e92f test: Fix SegwitV0SignatureMsg nLockTime signedness (MarcoFalke) Pull request description: It is unsigned in Bitcoin Core, so the tests should match it: https://github.com/bitcoin/bitcoin/blob/5b8990a1f3c49b0b02b7383c69e95320acbda13e/src/script/interpreter.cpp#L1611 The bug was introduced when the code was written in 330b0f31ee5719d94f9e52dfc83c5d82168241f9. (Lowercase `i` means signed, see https://docs.python.org/3/library/struct.html#format-characters) ACKs for top commit: epiccurious: Tested ACK fab15723b0518acbb1015e64df47dcac0187e92f. Eunovo: Tested ACK https://github.com/bitcoin/bitcoin/pull/29400/commits/fab15723b0518acbb1015e64df47dcac0187e92f achow101: ACK fab15723b0518acbb1015e64df47dcac0187e92f Tree-SHA512: 68cb8582f6af22e6abb2fc9d6770277501baa0f9873e2e8d47699e87096fc5d4b9de45fa07199757b6e945c99d4c4ea95f01478322f2c093ef95cf5e0c78522b
2024-02-12test: enable v2 for python p2p depending on global --v2transport flagMartin Zumsande
This changes the default behavior, individual tests can overwrite this option. As a result, it is possible to run the entire test suite with --v2transport, and all connections to the python p2p will then use it. Also adjust several tests that are already running with --v2transport in the test runner (although they actually made v1 connection before this change). This is done in the same commit so that there isn't an intermediate commit in which the CI fails.
2024-02-09Merge bitcoin/bitcoin#28948: v3 transaction policy for anti-pinningAva Chow
29029df5c700e6940c712028303761d91ae15847 [doc] v3 signaling in mempool-replacements.md (glozow) e643ea795e4b6fea4a6bbb3d72870ee6a4c836b1 [fuzz] v3 transactions and sigop-adjusted vsize (glozow) 1fd16b5c62f54c7f4c60122acd65d852f63d1e8b [functional test] v3 transaction submission (glozow) 27c8786ba918a42c860e6a50eaee9fdf56d7c646 test framework: Add and use option for tx-version in MiniWallet methods (MarcoFalke) 9a1fea55b29fe025355b06b45e3d77d192acc635 [policy/validation] allow v3 transactions with certain restrictions (glozow) eb8d5a2e7d939dd3ee683486e98702079e0dfcc0 [policy] add v3 policy rules (glozow) 9a29d470fbb62bbb27d517efeafe46ff03c25f54 [rpc] return full string for package_msg and package-error (glozow) 158623b8e0726dff7eae4288138f1710e727db9c [refactor] change Workspace::m_conflicts and adjacent funcs/structs to use Txid (glozow) Pull request description: See #27463 for overall package relay tracking. Delving Bitcoin discussion thread: https://delvingbitcoin.org/t/v3-transaction-policy-for-anti-pinning/340 Delving Bitcoin discussion for LN usage: https://delvingbitcoin.org/t/lightning-transactions-with-v3-and-ephemeral-anchors/418 Rationale: - There are various pinning problems with RBF and our general ancestor/descendant limits. These policies help mitigate many pinning attacks and make package RBF feasible (see #28984 which implements package RBF on top of this). I would focus the most here on Rule 3 pinning. [1][2] - Switching to a cluster-based mempool (see #27677 and #28676) requires the removal of CPFP carve out, which applications depend on. V3 + package RBF + ephemeral anchors + 1-parent-1-child package relay provides an intermediate solution. V3 policy is for "Priority Transactions." [3][4] It allows users to opt in to more restrictive topological limits for shared transactions, in exchange for the more robust fee-bumping abilities that offers. Even though we don't have cluster limits, we are able to treat these transactions as having as having a maximum cluster size of 2. Immediate benefits: - You can presign a transaction with 0 fees (not just 1sat/vB!) and add a fee-bump later. - Rule 3 pinning is reduced by a significant amount, since the attacker can only attach a maximum of 1000vB to your shared transaction. This also enables some other cool things (again see #27463 for overall roadmap): - Ephemeral Anchors - Package RBF for these 1-parent-1-child packages. That means e.g. a commitment tx + child can replace another commitment tx using the child's fees. - We can transition to a "single anchor" universe without worrying about package limit pinning. So current users of CPFP carve out would have something else to use. - We can switch to a cluster-based mempool [5] (#27677 #28676), which removes CPFP carve out [6]. [1]: Original mailing list post and discussion about RBF pinning problems https://gist.github.com/glozow/25d9662c52453bd08b4b4b1d3783b9ff, https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2022-January/019817.html [2]: A FAQ is "we need this for cluster mempool, but is this still necessary afterwards?" There are some pinning issues that are fixed here and not fully fixed in cluster mempool, so we will still want this or something similar afterward. [3]: Mailing list post for v3 https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2022-September/020937.html [4]: Original PR #25038 also contains a lot of the discussion [5]: https://delvingbitcoin.org/t/an-overview-of-the-cluster-mempool-proposal/393/7 [6]: https://delvingbitcoin.org/t/an-overview-of-the-cluster-mempool-proposal/393#the-cpfp-carveout-rule-can-no-longer-be-supported-12 ACKs for top commit: sdaftuar: ACK 29029df5c700e6940c712028303761d91ae15847 achow101: ACK 29029df5c700e6940c712028303761d91ae15847 instagibbs: ACK 29029df5c700e6940c712028303761d91ae15847 modulo that Tree-SHA512: 9664b078890cfdca2a146439f8835c9d9ab483f43b30af8c7cd6962f09aa557fb1ce7689d5e130a2ec142235dbc8f21213881baa75241c5881660f9008d68450
2024-02-08test framework: Add and use option for tx-version in MiniWallet methodsMarcoFalke
2024-02-07Merge bitcoin/bitcoin#29363: test: Fix CPartialMerkleTree.nTransactions ↵fanquake
signedness facafa90f7a1eee452f9baf8a1b65a2edac0982b test: Fix CPartialMerkleTree.nTransactions signedness (MarcoFalke) Pull request description: It is unsigned in Bitcoin Core, so the tests should match it: https://github.com/bitcoin/bitcoin/blob/aa9231fafe45513134ec8953a217cda07446fae8/src/merkleblock.h#L59 Large positive values, or "negative" values, are rejected anyway, but it still seems fine to fix this. The bug was introduced when the code was written in d280617bf569f84457eaea546541dc74c67cd1e4. (Lowercase `i` means signed, see https://docs.python.org/3/library/struct.html#format-characters) ACKs for top commit: theStack: LGTM ACK facafa90f7a1eee452f9baf8a1b65a2edac0982b Empact: ACK facafa90f7a1eee452f9baf8a1b65a2edac0982b Tree-SHA512: 35ac11bb5382dffe132bfae6097efc343ef6c06b1b4b1545130ca27b228ca6894679004862fee921b095172abaddbef5972c24d9bc195ce970f35643bd4a0f09
2024-02-07test: Fix SegwitV0SignatureMsg nLockTime signednessMarcoFalke
2024-02-06test: enable p2p_sendtxrcncl.py with v2transportMartin Zumsande
By adding to the test framework a wait until the v2 handshake is completed, so that p2p_sendtxrcncl.py (which doesn't need to be changed itself) doesnt't send out any other messages before that.
2024-02-06Merge bitcoin/bitcoin#29356: test: make v2transport arg in addconnection ↵glozow
mandatory and few cleanups e7fd70f4b6b163f4ad5b25b4da7fa79899245235 [test] make v2transport arg in addconnection mandatory and few cleanups (stratospher) Pull request description: - make `v2transport` argument in `addconnection` regression-testing only RPC mandatory. https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1470738750 - previously it was an optional arg with default `false` value. - only place this RPC is used is in the [functional tests](https://github.com/bitcoin/bitcoin/blob/11b436a66af3ceaebb0f907878715f331516a0bc/test/functional/test_framework/test_node.py#L742) where we always pass the appropriate `v2transport` option to the RPC anyways. (and that too just for python dummy peer(`P2PInterface`) and bitcoind(`TestNode`) interactions) - rename `v2_handshake()` to `_on_data_v2_handshake()` https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1466958424 - more compact return statement in `wait_for_reconnect()` https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1466979708 - assertion to check that empty version packets are received from `TestNode`. ACKs for top commit: glozow: ACK e7fd70f4b6 theStack: Code-review ACK e7fd70f4b6b163f4ad5b25b4da7fa79899245235 mzumsande: Code Review ACK e7fd70f4b6b163f4ad5b25b4da7fa79899245235 Tree-SHA512: e66e29baccd91e1e4398b91f7d45c5fc7c2841d77d8a6178734586017bf2be63496721649da91848dec71da605ee31664352407d5bb896e624cc693767c61a1f
2024-02-06test: speedup bip324_cipher.py unit testSebastian Falbesoner
Executing the unit tests for the bip324_cipher.py module currently takes quite long (>60 seconds on my notebook). Most time here is spent in empty plaintext/ciphertext encryption/decryption loops: .... for _ in range(msg_idx): enc_aead.encrypt(b"", b"") ... for _ in range(msg_idx): enc_aead.decrypt(b"", bytes(16)) ... Their sole purpose is increasing the FSChaCha20Poly1305 packet counters in order to trigger rekeying, i.e. the actual encryption/decryption is not relevant, as the result is thrown away. This commit speeds up the tests by supporting to pass "None" as plaintext/ciphertext, indicating to the routines that no actual encryption/decryption should be done. master branch: $ python3 -m unittest ./test/functional/test_framework/crypto/bip324_cipher.py .. ---------------------------------------------------------------------- Ran 2 tests in 64.658s PR branch: $ python3 -m unittest ./test/functional/test_framework/crypto/bip324_cipher.py .. ---------------------------------------------------------------------- Ran 2 tests in 0.822s
2024-02-01test: p2p: adhere to typical VERSION message protocol flowSebastian Falbesoner
The test framework's p2p implementation currently sends out it's VERSION message immediately after an inbound connection (i.e. TestNode outbound connection) is made. This doesn't follow the usual protocol flow where the initiator sends a version first, and the responders processes that and only then responds with its own version message. Change that accordingly by only sending immediate VERSION message for outbound connections (or after v2 handshake for v2 connections, respectively), and sending out VERSION messages as response for incoming VERSION messages (i.e. in the function `on_version`) for inbound connections. Note that some of the overruled `on_version` methods in functional tests needed to be changed to send the version explicitly.
2024-02-01test: p2p: process post-v2-handshake data immediatelySebastian Falbesoner
In the course of executing the asyncio data reception callback during a v2 handshake, it's possible that the receive buffer already contains data for after the handshake (usually a VERSION message for inbound connections). If we don't process that data immediately, we would do so after the next message is received, but with the adapted protocol flow introduced in the next commit, there is no next message, as the TestNode wouldn't continue until we send back our own version in `on_version`. Fix this by calling `self._on_data` immediately if there's data left in the receive buffer after a completed v2 handshake.
2024-02-01test: p2p: introduce helper for sending prepared VERSION messageSebastian Falbesoner
This deduplicates code for sending out the VERSION message (if available and not sent yet), currently used at three different places: 1) in the `connection_made` asyncio callback (for v1 connections that are not v2 reconnects) 2) at the end of `v2_handshake`, if the v2 handshake succeeded 3) in the `on_version` callback, if a reconnection with v1 happens
generated by cgit v1.2.3 (git 2.26.2) at 2024-11-10 01:57:21 +0000