aboutsummaryrefslogtreecommitdiff
path: root/test
AgeCommit message (Collapse)Author
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-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#29586: wallet: default wallet migration, modify ↵Ava Chow
inconvenient backup filename a951dba3a9d7663f009701140d663338e6c526a4 wallet: default wallet migration, modify inconvenient backup filename (furszy) Pull request description: Fixes #29584 On default legacy wallets, the backup filename starts with an "-" due to the wallet name being empty. This is inconvenient for systems who treat what follows the initial "-" character as flags. Note: As the user can freely set the wallet name to anything, we could also guard the backup filename against other inconvenient characters in the future (we need to be careful here, because the wallet name could also be a path). ACKs for top commit: achow101: ACK a951dba3a9d7663f009701140d663338e6c526a4 brunoerg: utACK a951dba3a9d7663f009701140d663338e6c526a4 vasild: ACK a951dba3a9d7663f009701140d663338e6c526a4 Tree-SHA512: 6347bb12cfb50526a4baad96e4f1df9d82b493f79f0a0f7e0a1c8335a86a1e8e147c7b7f95cec6ede6f4507506a7eaf7972bd35131a2d5ed4cbbf38d94f0a9ca
2024-03-11Merge bitcoin/bitcoin#28120: p2p: make block download logic aware of limited ↵Ava Chow
peers threshold c5b5843d8f10d96f76ee6b95f2b1b1b4ce755f75 test: avoid requesting blocks beyond limited peer threshold (furszy) 2f6a05512fa86d086b2b976c401394571d88bd93 p2p: sync from limited peer, only request blocks below threshold (furszy) 73127722a2d2b5c8da4102284f9d0cf504a2e72d refactor: Make FindNextBlocks friendlier (furszy) Pull request description: Even when the node believes it has IBD completed, need to avoid requesting historical blocks from network-limited peers. Otherwise, the limited peer will disconnect right away. The simplest scenario could be a node that gets synced, drops connections, and stays inactive for a while. Then, once it re-connects (IBD stays completed), the node tries to fetch all the missing blocks from any peer, getting disconnected by the limited ones. Note: Can verify the behavior by cherry-picking the test commit alone on master. It will fail there. ACKs for top commit: achow101: ACK c5b5843d8f10d96f76ee6b95f2b1b1b4ce755f75 vasild: ACK c5b5843d8f10d96f76ee6b95f2b1b1b4ce755f75 mzumsande: Code Review ACK c5b5843d8f10d96f76ee6b95f2b1b1b4ce755f75 pinheadmz: ACK c5b5843d8f10d96f76ee6b95f2b1b1b4ce755f75 Tree-SHA512: 9e550698bc6e63cc587b2b988a87d0ab555a8fa188c91c3f33287f8201d77c28b373331845356ad86f17bb21c15950b6466bc1cafd0ce8139d70364cb71c2ad2
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-08Merge bitcoin/bitcoin#29393: i2p: log connection was refused due to ↵Ava Chow
arbitrary port 5b358cdd1a5f5d2fe87a9e41c638996eab2e2796 i2p: log connection was refused due to arbitrary port (brunoerg) Pull request description: For I2P, we do not try to connect if port is != 0. However, we do not have anything that indicates it or any error when trying to connect with port != 0. This PR adds a log for it. Also, it improves the functional test. With this log we can ensure the reason we won't connect is the port, in the current test, we cannot ensure it. ACKs for top commit: jonatack: ACK 5b358cdd1a5f5d2fe87a9e41c638996eab2e2796 epiccurious: re-ACK 5b358cdd1a5f5d2fe87a9e41c638996eab2e2796. achow101: ACK 5b358cdd1a5f5d2fe87a9e41c638996eab2e2796 kristapsk: re-ACK 5b358cdd1a5f5d2fe87a9e41c638996eab2e2796 vasild: ACK 5b358cdd1a5f5d2fe87a9e41c638996eab2e2796 Tree-SHA512: 027245afa771c9295fff0bfd17c251dca4a9f4c739e5773922de3c030a65ef05d96291edcbdeeaa50ba3add61f75f28d8c00be503e03fc33d3491d1956fc549f
2024-03-08wallet: default wallet migration, modify inconvenient backup filenamefurszy
On default legacy wallets, the backup filename starts with an "-" due to the wallet name being empty. This is inconvenient for systems who treat what follows the initial "-" character as flags.
2024-03-07[fuzz] Apply fuzz env (suppressions, etc.) when fetching harness listdergoegge
2024-03-06test: exit with code 1 when no fn tests are foundMax Edwards
Prevents the test_runner from exiting silently with code 0 when no tests were found.
2024-03-05Merge bitcoin/bitcoin#29541: test: remove file-wide interpreter.cpp ubsan ↵fanquake
suppression 217c0ce552a5d519b5cc702aba0c82514a1c449e test: remove file-wide interpreter.cpp ubsan suppression (fanquake) Pull request description: ACKs for top commit: Sjors: utACK 217c0ce552a5d519b5cc702aba0c82514a1c449e hebasto: ACK 217c0ce552a5d519b5cc702aba0c82514a1c449e. dergoegge: ACK 217c0ce552a5d519b5cc702aba0c82514a1c449e Tree-SHA512: ae0c2ff4531fdb7b0296709f66b71d4065fe3f32cbd39a44e45934a975b5cf6cf01c2f136f110753efee8e301636f7700278aed1d995b463fc025c07d586a8fa
2024-03-05Merge bitcoin/bitcoin#29567: doc: fix broken reference to CI setup in ↵fanquake
test/lint/README.md 53ffd5a410186109b9a56c5922768905e168acb3 docs: Fix broken reference to CI setup in test/lint/README.md (naiyoma) Pull request description: The current [reference](https://github.com/bitcoin/bitcoin/blob/master/test/ci/lint/04_install.sh ) for CI setup in /test/lint#readme returns a 404. ACKs for top commit: fanquake: ACK 53ffd5a410186109b9a56c5922768905e168acb3 Tree-SHA512: 813c19a145f09e7da11963598b70dc438acba784eb722e509d6af59dc3af8f5da97628c454bed2b03cc919689603e070796de2db8d784d9162ae34e7b85a77d9
2024-03-05docs: Fix broken reference to CI setup in test/lint/README.mdnaiyoma
2024-03-05Merge bitcoin/bitcoin#29502: test: modify weight estimate in functional testsfanquake
e67ab174c9c04bba7a10724b7e694dd57f732177 test: fix flaky wallet_send functional test (Max Edwards) 3c49e6967050cfc367b3c826c9eac86a48528af5 test: fix weight estimates in functional tests (Max Edwards) Pull request description: Fixes: https://github.com/bitcoin/bitcoin/issues/25164 The wallet_send functional test has been flaky due to a slightly overestimated weight calculation. This PR makes the weight calculation more accurate, although occasionally, due to how ECDSA signatures can be different lengths it might slightly over estimate. The assertion in the test can handle this slight variation and so should continue passing. Update: Because the signature can be shorter that is used in the weight estimation or the final transaction the estimate could be both slightly smaller or slightly larger. ACKs for top commit: achow101: ACK e67ab174c9c04bba7a10724b7e694dd57f732177 S3RK: Code review ACK e67ab174c9c04bba7a10724b7e694dd57f732177 Tree-SHA512: 3bf73b355309dce860fa1520afb8461e94268e4bcf0e92a8273c279b41b058c44472cf59daafa15a515529b50bd665b5d498bbe4d934f2315dbe810a05bc73f9
2024-03-04Merge bitcoin/bitcoin#29524: p2p: Don't consider blocks mutated if they ↵fanquake
don't connect to known prev block a1fbde0ef7cf6c94d4a5181f8ceb327096713160 p2p: Don't consider blocks mutated if they don't connect to known prev block (Greg Sanders) Pull request description: Followup to https://github.com/bitcoin/bitcoin/pull/29412 to revert some of the behavior change that was likely unintentional. Based on comments from https://github.com/bitcoin/bitcoin/pull/29412#discussion_r1507499192 ACKs for top commit: 0xB10C: utACK a1fbde0ef7cf6c94d4a5181f8ceb327096713160 dergoegge: Code review ACK a1fbde0ef7cf6c94d4a5181f8ceb327096713160 Sjors: ACK a1fbde0ef7cf6c94d4a5181f8ceb327096713160 sr-gi: tACK https://github.com/bitcoin/bitcoin/commit/a1fbde0ef7cf6c94d4a5181f8ceb327096713160 Tree-SHA512: be6204c8cc57b271d55c1d02a5c77d03a37738d91cb5ac164f483b0bab3991c24679c5ff02fbaa52bf37ee625874b63f4c9f7b39ad6fd5f3a25386567a0942e4
2024-03-02test: remove file-wide interpreter.cpp ubsan suppressionfanquake
2024-03-01test: fix flaky wallet_send functional testMax Edwards
Rather than asserting that the exact fees are the same, check the fee rate rounded to nearest interger. This will account for small differences in fees caused by variability in ECDSA signature lengths.
2024-03-01test: fix weight estimates in functional testsMax Edwards
Updates the weight estimate to be more accurate by removing byte buffers and calculating the length of the count of scriptWitnesses rather than just using the count itself.
2024-02-29p2p: Don't consider blocks mutated if they don't connect to known prev blockGreg Sanders
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-29Merge bitcoin/bitcoin#29510: wallet: `getrawchangeaddress` and ↵Ava Chow
`getnewaddress` failures should not affect keypools for descriptor wallets e073f1dfda7a2a2cb2be9fe2a1d576f122596021 test: make sure keypool sizes do not change on `getrawchangeaddress`/`getnewaddress` failures (UdjinM6) 367bb7a80cc71130995672c853d4a6e0134721d6 wallet: Avoid updating `ReserveDestination::nIndex` when `GetReservedDestination` fails (UdjinM6) Pull request description: I think the expected behaviour of `getrawchangeaddress` and `getnewaddress` RPCs is that their failure should not affect keypool in any way. At least that's how legacy wallets work, you can confirm this behaviour by running `wallet_keypool.py --legacy-wallet` on master with e073f1dfda7a2a2cb2be9fe2a1d576f122596021 applied on top. However running `wallet_keypool.py --descriptors` on the same commit results in the following failure: ``` File "/path/to/bitcoin/test/functional/test_framework/test_framework.py", line 131, in main self.run_test() File "/path/to/bitcoin/test/functional/wallet_keypool.py", line 114, in run_test assert_equal(kp_size_before, kp_size_after) File "/path/to/bitcoin/test/functional/test_framework/util.py", line 57, in assert_equal raise AssertionError("not(%s)" % " == ".join(str(arg) for arg in (thing1, thing2) + args)) AssertionError: not([18, 24] == [19, 24]) ``` This happens because we pass `nIndex` (which is a class member) into `GetReservedDestination` and since it's passed by reference we get an updated value back, so `nIndex` won't be equal `-1` anymore, no matter if the function failed or succeeded. This means that `ReturnDestination` (called by dtor of `ReserveDestination`) will try to return something we did not actually reserve. The fix is to simply use a temporary variable instead of a class member and only update `nIndex` when `op_address` actually has value, basically do it the same way we do for other class members (`address` and `fInternal`) already. ACKs for top commit: achow101: ACK e073f1dfda7a2a2cb2be9fe2a1d576f122596021 josibake: ACK https://github.com/bitcoin/bitcoin/pull/29510/commits/e073f1dfda7a2a2cb2be9fe2a1d576f122596021 Tree-SHA512: 1128288a60dd4d8f306ef6f7ac66cdfeae3c9cc35c66ecada2d78fa61ac759f2a757b70fc3976ba8b5081200942b58dfabc184c01ccf911af40ba8c145344651
2024-02-29Merge bitcoin/bitcoin#29511: test: Fix intermittent failure in rpc_net.py ↵Ava Chow
--v2transport 0487f91a2046c0d6f91ccaedeb00fbefa635c66d test: Fix intermittent failure in rpc_net.py --v2transport (stratospher) Pull request description: Fixes #29508. Make sure that v2 handshake is complete before comparing getpeerinfo outputs so that `transport_protocol_type` isn't stuck at 'detecting'. This is done by adding a wait_until statement till `transport_protocol_type = v2` so that bitcoind waits until the v2 handshake is complete. (on the python side, this is ensured by default since `wait_for_handshake = True` inside `add_p2p_connection()`) ACKs for top commit: Sjors: ACK 0487f91a2046c0d6f91ccaedeb00fbefa635c66d mzumsande: Code Review ACK 0487f91a2046c0d6f91ccaedeb00fbefa635c66d achow101: ACK 0487f91a2046c0d6f91ccaedeb00fbefa635c66d vasild: ACK 0487f91a2046c0d6f91ccaedeb00fbefa635c66d Tree-SHA512: 44dd646a61cd38da243f527df7321e22d1821c2b090be43673027746098caf450c6671708ed731ba257952df6b5886e64c9c2f9686a82f6ef0f25780b7a87d3d
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-29test: Fix intermittent failure in rpc_net.py --v2transportstratospher
Make sure that v2 handshake is complete before comparing getpeerinfo outputs so that `transport_protocol_type` isn't stuck at 'detecting'. - on the python side, this is ensured by default `wait_for_handshake = True` inside `add_p2p_connection()`. - on the c++ side, add a wait_until statement till `transport_protocol_type = v2` so that v2 handshake is complete. Co-Authored-By: Martin Zumsande <mzumsande@gmail.com>
2024-02-28Merge bitcoin/bitcoin#29412: p2p: Don't process mutated blocksAva Chow
d8087adc7ebd4ea05dd3843e5a92e8115fd7bbcc [test] IsBlockMutated unit tests (dergoegge) 1ed2c9829700054526156297552bb47230406098 Add transaction_identifier::size to allow Span conversion (dergoegge) 1ec6bbeb8d27d31647d1433ccb87b362f6d81f90 [validation] Cache merkle root and witness commitment checks (dergoegge) 5bf4f5ba32da4627f152b54d266df9b2aa930457 [test] Add regression test for #27608 (dergoegge) 49257c0304828a185c273fcb99742c54bbef0c8e [net processing] Don't process mutated blocks (dergoegge) 2d8495e0800f5332748cd50eaad801ff77671bba [validation] Merkle root malleation should be caught by IsBlockMutated (dergoegge) 66abce1d98115e41f394bc4f4f52341960ddc839 [validation] Introduce IsBlockMutated (dergoegge) e7669e1343aec4298fd30d539847963e6fa5619c [refactor] Cleanup merkle root checks (dergoegge) 95bddb930aa72edd40fdff52cf447202995b0dce [validation] Isolate merkle root checks (dergoegge) Pull request description: This PR proposes to check for mutated blocks early as a defense-in-depth mitigation against attacks leveraging mutated blocks. We introduce `IsBlockMutated` which catches all known forms of block malleation and use it to do an early mutation check whenever we receive a `block` message. We have observed attacks that abused mutated blocks in the past, which could have been prevented by simply not processing mutated blocks (e.g. https://github.com/bitcoin/bitcoin/pull/27608 for which a regression test is included in this PR). ACKs for top commit: achow101: ACK d8087adc7ebd4ea05dd3843e5a92e8115fd7bbcc maflcko: ACK d8087adc7ebd4ea05dd3843e5a92e8115fd7bbcc 🏄 fjahr: Code review ACK d8087adc7ebd4ea05dd3843e5a92e8115fd7bbcc sr-gi: Code review ACK https://github.com/bitcoin/bitcoin/commit/d8087adc7ebd4ea05dd3843e5a92e8115fd7bbcc Tree-SHA512: 618ff4ea7f168e10f07504d3651290efbb1bb2ab3b838ffff3527c028caf6c52dedad18d04d3dbc627977479710930e200f2dfae18a08f627efe7e64a57e535f
2024-02-28test: make sure keypool sizes do not change on ↵UdjinM6
`getrawchangeaddress`/`getnewaddress` failures
2024-02-27doc: Fix Broken LinksJustin Dhillon
2024-02-27[test] Add regression test for #27608dergoegge
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-27Merge bitcoin/bitcoin#28178: fuzz: Generate with random libFuzzer settingsfanquake
fa3a4102ef0ae06d8930d7a7b567759e2a5b5fde fuzz: Set -rss_limit_mb=8000 for generate as well (MarcoFalke) fa4e396e1da8e5b04a5f906b95017b969ea37bae fuzz: Generate with random libFuzzer settings (MarcoFalke) Pull request description: Sometimes a libFuzzer setting like `-use_value_profile=1` helps [0], sometimes it hurts [1]. [0] https://github.com/bitcoin/bitcoin/pull/20789#issuecomment-752961937 [1] https://github.com/bitcoin/bitcoin/pull/27888#issuecomment-1645976254 By picking a random value, it is ensured that at least some of the runs will have the beneficial configuration set. Also, set `-max_total_time` to prevent slow fuzz targets from getting a larger time share, or possibly peg to a single core for a long time and block the python script from exiting for a long time. This can be improved in the future. For example, the python script can exit after some time (https://github.com/bitcoin/bitcoin/pull/20752#discussion_r549248791). Alternatively, it can measure if coverage progress was made and run for less time if no progress has been made recently anyway, so that more time can be spent on targets that are new or still make progress. ACKs for top commit: murchandamus: utACK fa3a4102ef0ae06d8930d7a7b567759e2a5b5fde dergoegge: utACK fa3a4102ef0ae06d8930d7a7b567759e2a5b5fde brunoerg: light ACK fa3a4102ef0ae06d8930d7a7b567759e2a5b5fde Tree-SHA512: bfd04a76ca09aec612397bae5f3f263a608faa7087697169bd4c506c8195c4d2dd84ddc7fcd3ebbc75771eab618fad840af819114968ca3668fc730092376768
2024-02-26Merge bitcoin/bitcoin#29467: test: Fix intermittent issue in interface_rest.pyfanquake
faeed91c0be6e5dda4790522d0dc999afd869d11 test: Fix intermittent issue in interface_rest.py (MarcoFalke) Pull request description: Fixes: ``` test 2024-02-22T16:15:37.465000Z TestFramework (ERROR): Assertion failed Traceback (most recent call last): File "/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/test_framework.py", line 131, in main self.run_test() File "/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/interface_rest.py", line 340, in run_test assert_equal(json_obj, mempool_info) File "/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/util.py", line 57, in assert_equal raise AssertionError("not(%s)" % " == ".join(str(arg) for arg in (thing1, thing2) + args)) AssertionError: not({'loaded': True, 'size': 3, 'bytes': 312, 'usage': 3600, 'total_fee': Decimal('0.00093600'), 'maxmempool': 300000000, 'mempoolminfee': Decimal('0.00001000'), 'minrelaytxfee': Decimal('0.00001000'), 'incrementalrelayfee': Decimal('0.00001000'), 'unbroadcastcount': 1, 'fullrbf': False} == {'loaded': True, 'size': 3, 'bytes': 312, 'usage': 3600, 'total_fee': Decimal('0.00093600'), 'maxmempool': 300000000, 'mempoolminfee': Decimal('0.00001000'), 'minrelaytxfee': Decimal('0.00001000'), 'incrementalrelayfee': Decimal('0.00001000'), 'unbroadcastcount': 0, 'fullrbf': False}) ``` https://cirrus-ci.com/task/4852944378527744?logs=ci#L4436 ACKs for top commit: m3dwards: ACK https://github.com/bitcoin/bitcoin/pull/29467/commits/faeed91c0be6e5dda4790522d0dc999afd869d11 mzumsande: ACK faeed91c0be6e5dda4790522d0dc999afd869d11 Tree-SHA512: 513422229db45d2586c554b9a466e86848bfcf5280b0f000718cbfc44d93dd1af69e19a56f6ac578f5d7aada74ab0c90d4a9e09a324062b6f9ed239e5e34f540
2024-02-26Merge bitcoin/bitcoin#29470: test: Add option to skip python unit testsfanquake
5f240ab2e89fb20286fbaf9a1f00346bb1cad5a1 test: Add option to skip unit tests for the test runner (Martin Zumsande) Pull request description: In the python `test_runner`, it's possible to disable specific functional tests (or just enable a few specific ones), but the unit tests for the python test framework cannot be skipped. Add this option (`--skipunit` or `-u`), it would save some time for devs not interested in running those every time. ACKs for top commit: fjahr: re-ACK 5f240ab2e89fb20286fbaf9a1f00346bb1cad5a1 tdb3: Code review and tested ACK 5f240ab2e89fb20286fbaf9a1f00346bb1cad5a1 stratospher: tested ACK 5f240ab. Tree-SHA512: f7c9cfefc18a6510e24ca4601309b40fdf4180a4c5fe592be9cf7607be6541784b283c46c8d6e60740ff3eba83025dd5d0db36e55bf8bad1404b38120859e113
2024-02-26Merge bitcoin/bitcoin#29408: lint: Check for missing bitcoin-config.h includesfanquake
fa58ae74ea67485495dbc2d003adfbca68d6869b refactor: Add missing include for USE_BDB, USE_SQLITE to bench/wallet_ismine.cpp (MarcoFalke) fa31908ea848488ff842f1b9fce6235bb8855ec7 lint: Check for missing or redundant bitcoin-config.h includes (MarcoFalke) fa63b0e351dee4782ee19ad46603957ef8d020eb lint: Make lint error easier to spot in output (MarcoFalke) fa770fd368e32950dd727e111a5d66e1dbb93716 doc: Add missing RUST_BACKTRACE=1 (MarcoFalke) fa100512677587b4e84a8be2235cf6d49b6a0134 lint: Add get_subtrees() helper (MarcoFalke) Pull request description: Missing `bitcoin-config.h` includes are problematic, because the build could silently pass, but produce an unintended result. For example, a slower fallback algorithm could be picked, even though `bitcoin-config.h` indicates that a faster feature is available and should be used. As the build succeeds silently, this problem is not possible to detect with iwyu. Thus, fix this by using a linter based on grepping the source code. ACKs for top commit: theuni: Weak ACK fa58ae74ea67485495dbc2d003adfbca68d6869b. TheCharlatan: ACK fa58ae74ea67485495dbc2d003adfbca68d6869b hebasto: ACK fa58ae74ea67485495dbc2d003adfbca68d6869b, tested on Ubuntu 23.10 -- it catches bugs properly. I didn't review rust code changes. Tree-SHA512: cf4346f81ea5b8c215da6004cb2403d1aaf569589613c305d8ba00329b82b3841da94fe1a69815ce15f2edecbef9b031758ec9b6433564976190e3cf91ec8181
2024-02-23test: Add option to skip unit tests for the test runnerMartin Zumsande
2024-02-22test: Fix intermittent issue in interface_rest.pyMarcoFalke
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-20test: assert rpc error for addnode v2transport not enabledkevkevin
Added coverage for the addnode rpc when v2transport is not enabled but is set as true when calling addnode rpc
2024-02-20lint: Check for missing or redundant bitcoin-config.h includesMarcoFalke
2024-02-20lint: Make lint error easier to spot in outputMarcoFalke
2024-02-20doc: Add missing RUST_BACKTRACE=1MarcoFalke
This will print a backtrace when an internal code error happened.
2024-02-20lint: Add get_subtrees() helperMarcoFalke
This is needed for a future change.
2024-02-15rpc: Fixed signed integer overflow for large feeratesMarcoFalke
2024-02-13Merge bitcoin/bitcoin#29394: test, assumeutxo: Add test to ensure failure ↵fanquake
when mempool not empty 8d20602e555dfe026b421363ee32cfca17c674d8 test, assumeutxo: Add test to ensure failure when mempool not empty (Hernan Marino) Pull request description: Add a test to ensure that loadtxoutset fails when the node's mempool is not empty, as suggested by maflcko here: https://github.com/bitcoin/bitcoin/pull/27596#discussion_r1344713537 ACKs for top commit: maflcko: re-ACK 8d20602e555dfe026b421363ee32cfca17c674d8 BrandonOdiwuor: ACK 8d20602e555dfe026b421363ee32cfca17c674d8 Tree-SHA512: 97c9668c0f38897934bf0d326515d898d4e682ff219deba9d751b35125b5cf33d51c9df116a74117ecf0394f28995a3d0cae1266b1e5acb4365ff4f309ce3f6c
2024-02-13Merge bitcoin/bitcoin#29424: v3 followupsfanquake
6b161cb82a9766ef814f05e5b8f019f15d5ee14d [test] second child of a v3 tx can be replaced individually (glozow) 5c998a696c7a4ff2a91fe3d8c7177d2b806eb7ac [refactor] use MAX_PUBKEYS_PER_MULTISIG instead of magic numbers in test (glozow) a9346421db813ebb1233f6477fe924e2f7c562ad [test] PackageV3Checks with inheritance violation in mempool ancestor (glozow) 63b62e123e38cb92c2135e63eb1a5b760c11dd4e [doc] fix docs and comments from v3 (glozow) Pull request description: Addresses final comments from #28948: - thread at https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1483245289, using https://github.com/ismaelsadeeq/bitcoin/commit/87fc7f0a8d8ad1fc2af60ee6cc678df5e6fb01dd with some modifications - https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1483769698 - https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1483776227 - https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1484427635 - https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1484467280 - https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1484531064 - https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1484992098 - https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1484992336 - https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1484994642 ACKs for top commit: instagibbs: ACK 6b161cb82a9766ef814f05e5b8f019f15d5ee14d sdaftuar: utACK 6b161cb82a9766ef814f05e5b8f019f15d5ee14d Tree-SHA512: 584fce7810f4d704ee6ab51fdc7d42bab342140cae3d076f89b5e1966dd1dd8293cb25b3121e41a4dcd65f9d4a735102b9ab2e90f98aa770b84e21f4d35d63d3
2024-02-13Merge bitcoin/bitcoin#29425: test: fix intermittent failure in ↵fanquake
wallet_reorgrestore.py 44d11532f80705b790bc6e28df9a96ac54b25f9b test: fix intermittent failure in wallet_reorgrestore.py (Martin Zumsande) Pull request description: By adding a missing `sync_blocks` call. There was a race at `node2` between connecting the block produced by `node0`, and using `-generate` to create new blocks itself. In the failed run, block generation started before connecting the block, resulting in a final block height that was smaller by 1 than expected. See https://github.com/bitcoin/bitcoin/issues/29392#issuecomment-1939541603 for a more detailed analysis of the failed run. Can be reproduced by adding a sleep to [this spot](https://github.com/bitcoin/bitcoin/blob/6ff0aa089c01ff3e610ecb47814ed739d685a14c/src/validation.cpp#L4217) in `ChainstateManager::ProcessNewBlock()`: ``` if (util::ThreadGetInternalName() == "msghand") { std::this_thread::sleep_for(0.2s); } ``` which fails for me on master and succeeds with the fix. Fixes #29392 ACKs for top commit: maflcko: lgtm ACK 44d11532f80705b790bc6e28df9a96ac54b25f9b Tree-SHA512: c08699e5ae348d4c0626022b519449d052f511d3f44601bcd8dac836a130a3f67fca149532e1e3690367ebfdcbcdd32e527170d039209c1f599ce861136ae29f
2024-02-12test: fix intermittent failure in wallet_reorgrestore.pyMartin Zumsande
...by adding a missing sync_blocks call. There was a race at node2 between connecting the block produced by node 0, and using -generate to create new blocks itself. In the failed run, the latter happened first, resulting in a final block height that was smaller by 1 than expected.
2024-02-12Merge bitcoin/bitcoin#28987: wallet: simplify and batch zap wallet txes processAva Chow
9a3c5c8697659e34d0476103af942a4615818f4e scripted-diff: rename ZapSelectTx to RemoveTxs (furszy) 83b762845f5804f23b63526d403b2f327fe99632 wallet: batch and simplify ZapSelectTx process (furszy) 595d50a1032ad7ffa9945464c86aa57f16665e93 wallet: migration, remove extra NotifyTransactionChanged call (furszy) a2b071f9920c2f4893afcc43a152f593c03966bf wallet: ZapSelectTx, remove db rewrite code (furszy) Pull request description: Work decoupled from #28574. Brother of #28894. Includes two different, yet interconnected, performance and code improvements to the zap wallet transactions process. 1) As the goal of the `ZapSelectTx` function is to erase tx records that match any of the inputted hashes. There is no need to traverse the whole database record by record. We could just check if the tx exist, and remove it directly by calling `EraseTx()`. 2) Instead of performing single write operations per removed tx record, this PR batches them all within a single atomic db txn. Moreover, these changes will enable us to consolidate all individual write operations that take place during the wallet migration process into a single db txn in the future. ACKs for top commit: achow101: ACK 9a3c5c8697659e34d0476103af942a4615818f4e josibake: ACK https://github.com/bitcoin/bitcoin/pull/28987/commits/9a3c5c8697659e34d0476103af942a4615818f4e Tree-SHA512: fb2ecc48224c400ab3b1fbb32e174b5b13bf03794717727f80f01f55fb183883b067a68c0a127b2de8885564da15425d021a96541953bf38a72becc2e9929ccf
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-12[test] second child of a v3 tx can be replaced individuallyglozow
Co-authored-by: ismaelsadeeq <ask4ismailsadiq@gmail.com>
generated by cgit v1.2.3 (git 2.26.2) at 2024-09-13 11:20:51 +0000