aboutsummaryrefslogtreecommitdiff
path: root/test
AgeCommit message (Collapse)Author
2024-09-12Merge bitcoin/bitcoin#30880: test: Wait for local services to update in ↵Ava Chow
feature_assumeutxo 19f4a7c95a99162122068d4badffeea240967a65 test: Wait for local services to update in feature_assumeutxo (Fabian Jahr) Pull request description: Closes #30878 It seems like there is a race where the block is stored locally and `getblock` does not error anymore, but `ActivateBestChain` has not finished yet, so the local services are not updated yet either. Fix this by waiting for the local services to update. Can be reproduced locally by adding the sleep here: ```cpp ──────────────────────────────────────────────────────────────────────────────────────────────────────────┐ src/validation.cpp:3567: bool Chainstate::ActivateBestChain(BlockValidationState& state, std::shared_ptr< │ ──────────────────────────────────────────────────────────────────────────────────────────────────────────┘ } if (WITH_LOCK(::cs_main, return m_disabled)) { std::this_thread::sleep_for(std::chrono::seconds(10)); // Background chainstate has reached the snapshot base block, so exit. // Restart indexes to resume indexing for all blocks unique to the snapshot ``` ACKs for top commit: maflcko: review-only ACK 19f4a7c95a99162122068d4badffeea240967a65 achow101: ACK 19f4a7c95a99162122068d4badffeea240967a65 pablomartin4btc: tACK 19f4a7c95a99162122068d4badffeea240967a65 furszy: Code review ACK [19f4a7c](https://github.com/bitcoin/bitcoin/pull/30880/commits/19f4a7c95a99162122068d4badffeea240967a65). Tree-SHA512: 70dad3795988956c5e20f2d2d895fb56c5e3ce257c7547d3fd729c88949f0e24cb34594da1537bce8794ad02b2db44e7e46e995aa32539cd4dd84c4f1d4bb1c4
2024-09-12Merge bitcoin/bitcoin#30546: util: Use consteval checked format string in ↵Ryan Ofsky
FatalErrorf, LogConnectFailure fa5bc450d5d4c1d2daf7b205f1678402c3c21678 util: Use compile-time check for LogConnectFailure (MarcoFalke) fa7087b896c0150c29d7a27c53e0533831a2bf3b util: Use compile-time check for FatalErrorf (MarcoFalke) faa62c0112f2b7ab69c80a5178f5b79217bed0a6 util: Add ConstevalFormatString (MarcoFalke) fae7b83eb58d22ed83878561603991131372cdd7 lint: Remove forbidden functions from lint-format-strings.py (MarcoFalke) Pull request description: The `test/lint/lint-format-strings.py` was designed to count the number of format specifiers and assert that they are equal to the number of parameters passed to the format function. The goal seems reasonable, but the implementation has many problems: * It is written in Python, meaning that C++ code can not be parsed correctly. Currently it relies on brittle regex and string parsing. * Apart from the parsing errors, there are also many logic errors. For example, `count_format_specifiers` allows a mix of positional specifiers and non-positional specifiers, which can lead to runtime format bugs. Also, `count_format_specifiers` silently skipped over "special" format specifiers, which are valid in tinyformat, which again can lead to runtime format bugs being undetected. * The brittle logic has a history of breaking in pull requests that are otherwise fine. This causes the CI to fail and the pull request being blocked from progress until the bug in the linter is fixed, or the code is rewritten to work around the bug. * It is only run in the CI, or when the developer invokes the script. It would be better if the developer got the error message at compile-time, directly when writing the code. Fix all issues by using a `consteval` checked format string in `FatalErrorf` and `LogConnectFailure`. This is the first step toward https://github.com/bitcoin/bitcoin/issues/30530 and a follow-up will apply the approach to the other places. ACKs for top commit: stickies-v: re-ACK fa5bc450d5d4c1d2daf7b205f1678402c3c21678 l0rinc: ACK fa5bc450d5d4c1d2daf7b205f1678402c3c21678 hodlinator: ACK fa5bc450d5d4c1d2daf7b205f1678402c3c21678 ryanofsky: Code review ACK fa5bc450d5d4c1d2daf7b205f1678402c3c21678 Tree-SHA512: d6189096b16083143687ed1b1559cf4f92f97dd87bc5d00673e44f4fb9fce7bb7b215cfdfc39b6e6a24f0b75a79a03ededce966639e554f7172e1fc22cf015ae
2024-09-12test: Wait for local services to update in feature_assumeutxoFabian Jahr
2024-09-12Merge bitcoin/bitcoin#30872: test: fix exclude parsing for functional runnermerge-script
72b46f28bf8d37a166961b5aa2a22302b932b756 test: fix exclude parsing for functional runner (Max Edwards) Pull request description: This restores previous behaviour of being able to exclude a test by name without having to specify .py extension. It was noticed in https://github.com/bitcoin/bitcoin/issues/30851 that tests were no longer being excluded. PR https://github.com/bitcoin/bitcoin/pull/30244 introduced being able to exclude a specific tests based on args (such as `--exclude "rpc_bind.py --ipv6`) but it made the wrong assumption that test names intended to be excluded would include the .py extension. The following https://github.com/bitcoin/bitcoin/pull/30244#issuecomment-2344009687 shows that this is not how the `--exclude` flag was used in CI. https://github.com/bitcoin/bitcoin/pull/30244#issuecomment-2344009687 gave three examples of `--exclude` being used in CI so I compared the number of tests that the runner would run for these three examples in three situations, before #30244 was introduced, in master today and with this PR applied. Example: `--previous-releases --coverage --extended --exclude feature_dbcrash` Test count: Before #30244 introduced: 314 Master: 315 With this PR: 314 Example: `--exclude feature_init,rpc_bind,feature_bind_extra` Test count: Before #30244 introduced: 306 Master 311 With this PR: 306 Example: `--exclude rpc_bind,feature_bind_extra` Before #30244 introduced: 307 Master 311 With this PR: 307 I've also tested that the functionality introduced with #30244 remains and we can still exclude specific tests by argument. ACKs for top commit: maflcko: review ACK 72b46f28bf8d37a166961b5aa2a22302b932b756 willcl-ark: ACK 72b46f28bf8d37a166961b5aa2a22302b932b756 Tree-SHA512: 37c0e3115f4e3efdf9705f4ff8cd86a5cc906aacc1ab26b0f767f5fb6a953034332b29b0667073f8382a48a2fe9d649b7e60493daf04061260adaa421419d8c8
2024-09-12util: Use compile-time check for LogConnectFailureMarcoFalke
2024-09-12util: Use compile-time check for FatalErrorfMarcoFalke
2024-09-12test: fix exclude parsing for functional runnerMax Edwards
This restores previous behaviour of being able to exclude a test by name without having to specify .py extension.
2024-09-12Merge bitcoin/bitcoin#30733: test: remove unused src_dir param from ↵merge-script
run_tests after CMake migration 2ad560139b80e149844f285cd1456bb7cc0eb1ee Remove unused src_dir param from run_tests (Lőrinc) Pull request description: The `src_dir` usage was removed in https://github.com/bitcoin/bitcoin/commit/a8a2e364acf55bbe18404ab21f852d52257bcb6d#diff-437d7f6e9f2229879b60aae574a8217f14c643bbf3cfa9225d8011d6d52df00cL598, making the parameter unused. Top commit has no ACKs. Tree-SHA512: 1fd8b93811b4ab467ba5a160a4fe204e9606e1bf237c7595ed6f8b7821cf59d2a776c0e1e154852a45b2a35e5bdbd8996314e4f63a9c750f21b9a17875cb636a
2024-09-11Merge bitcoin/bitcoin#30807: Fix peers abruptly disconnecting from ↵Ava Chow
AssumeUTXO nodes during IBD 992f83bb6f4b29b44f4eaace1d1a2c0001d43cac test: add coverage for assumeUTXO honest peers disconnection (furszy) 6d5812e5c852c233bd7ead2ceef051f8567619ed assumeUTXO: fix peers disconnection during sync (furszy) Pull request description: Because AssumeUTXO nodes prioritize tip synchronization, they relay their local address through the network before completing the background chain sync. This, combined with the advertising of full-node service (`NODE_NETWORK`), can result in an honest peer in IBD connecting to the AssumeUTXO node (while syncing) and requesting an historical block the node does not have. This behavior leads to an abrupt disconnection due to perceived unresponsiveness from the AssumeUTXO node. This lack of response occurs because nodes ignore `getdata` requests when they do not have the block data available (further discussion can be found in #30385). Fix this by refraining from signaling full-node service support while the background chain is being synced. During this period, the node will only signal `NODE_NETWORK_LIMITED` support. Then, full-node (`NODE_NETWORK`) support will be re-enabled once the background chain sync is completed. Thanks mzumsande for a post-#30385 convo too. Testing notes: Just cherry-pick the second commit (bb08c22) on master. It will fail there, due to the IBD node requesting historical blocks to the snapshot node - which is bad because the snapshot node will ignore the requests and stall + disconnect after some time. ACKs for top commit: achow101: ACK 992f83bb6f4b29b44f4eaace1d1a2c0001d43cac naumenkogs: ACK 992f83bb6f4b29b44f4eaace1d1a2c0001d43cac mzumsande: ACK 992f83bb6f4b29b44f4eaace1d1a2c0001d43cac Tree-SHA512: fef525d1cf3200c2dd89a346be9c82d77f2e28ddaaea1f490a435e180d1a47a371cadea508349777d740ab56e94be536ad8f7d61cc81f6550c58b609b3779ed3
2024-09-11lint: Remove forbidden functions from lint-format-strings.pyMarcoFalke
Given that all of them are forbidden by the test/lint/lint-locale-dependence.py check, they can be removed.
2024-09-10Merge bitcoin/bitcoin#30805: test: Add explicit onion bind to p2p_permissionsglozow
082779d6062c5b72f3497bb864e4dbb4373a3a4c test: Add explicit onion bind to p2p_permissions (Ava Chow) Pull request description: When the bind option is replaced in the bitcoin.conf, bitcoind will attempd to bind to the default tor listening port. If another bitcoind is running that is already bound to that port, the bind will fail which, since #22729, causes the test to fail. This failure can be avoided by explicitly binding the tor port when the bind is removed. ACKs for top commit: tdb3: ACK 082779d6062c5b72f3497bb864e4dbb4373a3a4c theStack: re-ACK 082779d6062c5b72f3497bb864e4dbb4373a3a4c glozow: ACK 082779d6062c5b72f3497bb864e4dbb4373a3a4c Tree-SHA512: 4acb69ea2e00aeacf9e7c9ab9595ceaf0e0d2adbd795602034b2184197d9bad54c7bc9f3da43ef9c52a71869fe96ba8c87fc5b7c37880f258f5a2aaab2b4046c
2024-09-10test: add coverage for assumeUTXO honest peers disconnectionfurszy
Exercising and verifying the following points: 1. An IBD node can sync headers from an AssumeUTXO node at any time. 2. IBD nodes do not request historical blocks from AssumeUTXO nodes while they are syncing the background-chain. 3. The assumeUTXO node dynamically adjusts the network services it offers according to its state. 4. IBD nodes can fully sync from AssumeUTXO nodes after they finish the background-chain sync.
2024-09-10assumeUTXO: fix peers disconnection during syncfurszy
Because AssumeUTXO nodes prioritize tip synchronization, they relay their local address through the network before completing the background chain sync. This, combined with the advertising of full-node service (NODE_NETWORK), can result in an honest peer in IBD connecting to the AssumeUTXO node (while syncing) and requesting an historical block the node does not have. This behavior leads to an abrupt disconnection due to perceived unresponsiveness (lack of response) from the AssumeUTXO node. This lack of response occurs because nodes ignore getdata requests when they do not have the block data available (further discussion can be found in PR 30385). Fix this by refraining from signaling full-node service support while the background chain is being synced. During this period, the node will only signal 'NODE_NETWORK_LIMITED' support. Then, full-node ('NODE_NETWORK') support will be re-enabled once the background chain sync is completed.
2024-09-10test: Add explicit onion bind to p2p_permissionsAva Chow
When the bind option is replaced in the bitcoin.conf, bitcoind will attempd to bind to the default tor listening port. If another bitcoind is running that is already bound to that port, the bind will fail which, since #22729, causes the test to fail. This failure can be avoided by explicitly binding the tor port when the bind is removed.
2024-09-09Merge bitcoin/bitcoin#30817: test: Add coverage for dumptxoutset failure ↵Ava Chow
robustness c2b779da4e7f1bf1a5c5d67ec94cba3027b42ee7 refactor: Manage dumptxoutset RAII classes with std::optional (Fabian Jahr) 4b5bf335adabd1586043caa72a98356a8255bc29 test: Add coverage for failing dumptxoutset behavior (Fabian Jahr) Pull request description: This adds a test that checks that network activity is not suspended if dumptxoutset fails in the middle of its process which is implemented with the `NetworkDisable` RAII class. I would have liked to add coverage for the `TemporaryRollback` RAII class but that seems a lot more tricky since the failure needs to happen at some point after the rollback and on the scale of our test chain here I couldn't find a way to do it yet. This was requested by pablomartin4btc here: https://github.com/bitcoin/bitcoin/pull/30808#pullrequestreview-2280450117. To test the test you can comment out the content of the destructor of `NetworkDisable`. It also addresses the feedback by ryanofsky to use `std::optional` instead of `std::unique_ptr` for the management of the RAII object: https://github.com/bitcoin/bitcoin/pull/30808#discussion_r1744149228 ACKs for top commit: achow101: ACK c2b779da4e7f1bf1a5c5d67ec94cba3027b42ee7 pablomartin4btc: cr & tACK c2b779da4e7f1bf1a5c5d67ec94cba3027b42ee7 tdb3: ACK c2b779da4e7f1bf1a5c5d67ec94cba3027b42ee7 BrandonOdiwuor: Code Review ACK c2b779da4e7f1bf1a5c5d67ec94cba3027b42ee7 theStack: ACK c2b779da4e7f1bf1a5c5d67ec94cba3027b42ee7 Tree-SHA512: 9556e75014a2599bb870b70faf887608b332f2312626333f771d4ec11c04f863a2cf17e223ec473d4e8b0c9e8008394a4e0c321561f7ef3a2eec713dcfaea58a
2024-09-09Merge bitcoin/bitcoin#30684: init: fix init fatal error on invalid negated ↵Ava Chow
option value ee47ca29d6ef55650a0af63bca817c5d494f31ef init: fix fatal error on '-wallet' negated option value (furszy) Pull request description: Currently, if users provide a double negated value such as '-nowallet=0' or a non-boolean convertible value to a negated option such as '-nowallet=not_a_boolean', the initialization process results in a fatal error, causing an unclean shutdown and displaying a poorly descriptive error message: "JSON value of type bool is not of expected type string." (On bitcoind. The GUI does not display any error msg - upcoming PR -). This PR fixes the issue by ensuring that only string values are returned in the the "wallet" settings list, failing otherwise. It also improves the clarity of the returned error message. Note: This bug was introduced in https://github.com/bitcoin/bitcoin/pull/22217. Where the `GetArgs("-wallet")` call was replaced by `GetSettingsList("-wallet")`. ACKs for top commit: achow101: ACK ee47ca29d6ef55650a0af63bca817c5d494f31ef ryanofsky: Code review ACK ee47ca29d6ef55650a0af63bca817c5d494f31ef, just adding the suggested test since last review TheCharlatan: ACK ee47ca29d6ef55650a0af63bca817c5d494f31ef ismaelsadeeq: Tested ACK ee47ca29d6ef55650a0af63bca817c5d494f31ef Tree-SHA512: 5f01076f74a048019bb70791160f0accc2db7a457d969cb23687bed81ccbbdec1dda68311e7c6e2dd56250e23e8d926d4066e5014b2a99a2fc202e24ed264fbd
2024-09-09Merge bitcoin/bitcoin#30401: fix: increase consistency of rpcauth parsingAva Chow
27c976d11a68d66db97d9a7a30c6a6a71c6ab586 fix: increase consistency of rpcauth parsing (tdb3) 2ad3689512a36eaff957df9ac28e65b2fedbc36c test: add norpcauth test (tdb3) 67df0dec1abe547773e532aa60aff0317e018123 test: blank rpcauth CLI interaction (tdb3) ecc98ccff25b7e758337e764e59d764076772fec test: add cases for blank rpcauth (tdb3) Pull request description: The current `rpcauth` parsing behavior is inconsistent and unintuitive (see https://github.com/bitcoin/bitcoin/pull/29141#issuecomment-1972085251 and additional details below). The current behavior inconsistently treats empty `rpcauth` as an error (or not) depending on the location within CLI/bitcoin.conf and the location of adjacent valid `rpcauth` params. Empty `rpcauth` is now consistently treated as an error and prevents bitcoind from starting. Continuation of the upforgrabs PR #29141. ### Additional details: Current `rpcauth` behavior is nonsensical: - If an empty `rpcauth` argument was specified as the last command line argument, it would cause all other `rpcauth` arguments to be ignored. - If an empty `rpcauth` argument was specified on the command line followed by any nonempty `rpcauth` argument, it would cause an error. - If an empty `rpcauth=` line was specified after non-empty rpcauth line in the config file it would cause an error. - If an empty `rpcauth=` line in a config file was first it would cause other rpcauth entries in the config file to be ignored, unless there were `-rpcauth` command line arguments and the last one was nonempty, in which case it would cause an error. New behavior is simple: - If an empty rpcauth config line or command line argument is used it will cause an error ACKs for top commit: naiyoma: Tested ACK [https://github.com/bitcoin/bitcoin/commit/27c976d11a68d66db97d9a7a30c6a6a71c6ab586](https://github.com/bitcoin/bitcoin/commit/27c976d11a68d66db97d9a7a30c6a6a71c6ab586) achow101: ACK 27c976d11a68d66db97d9a7a30c6a6a71c6ab586 ryanofsky: Code review ACK 27c976d11a68d66db97d9a7a30c6a6a71c6ab586. Since last review commit message was just tweaked to clarify previous behavior. Tree-SHA512: af2e9dd60d1ad030409ae2c3805ab139c7435327823d9f8bbeede815f376cb696a5929b08a6e8c8b5f7278ed49cfb231789f9041bd57f1f03ec96501b669da5b
2024-09-06cmake: add USE_SOURCE_PERMISSIONS to all configure_file usagefanquake
`USE_SOURCE_PERMISSIONS` is the default, so this should not change behaviour. However, being explicit makes it clear what we are doing. Related to #30815. See https://cmake.org/cmake/help/latest/command/configure_file.html#options.
2024-09-06Merge bitcoin/bitcoin#30790: bench: Remove redundant logging benchmarksmerge-script
fadbcd51fc77a3f4e877851463f3c7425fb751d2 bench: Remove redundant logging benchmarks (MarcoFalke) fa8dd952e279a87f6027ddd2e2119bf2ae2f9943 bench: Use LogInfo instead of the deprecated alias LogPrintf (MarcoFalke) Pull request description: `LogPrint*ThreadNames` is redundant with `LogWith(out)ThreadNames`, because they all measure toggling the thread names (and check that it has no effect on performance). Fix it by removing the redundant ones. This also allows to drop a deprecated logging alias. ACKs for top commit: stickies-v: ACK fadbcd51fc77a3f4e877851463f3c7425fb751d2 Tree-SHA512: 4fe137f374aa4ee1aa0e1da4a1f9839c0e52c23dbb93198ecafee98de39d311cc47304bba4191f3807aa00c51b1eae543e3f270f03d341c84910e5e341a1d475
2024-09-05lint: Check for release note snippets in the wrong folderMarcoFalke
2024-09-05test: Add coverage for failing dumptxoutset behaviorFabian Jahr
In case of a failure to create the dump, the node should not be left in an inconsistent state like deactivated network activity or an invalidated blockchain.
2024-09-05bench: Remove redundant logging benchmarksMarcoFalke
LogPrint*ThreadNames is redundant with LogWith(out)ThreadNames, because they all measure toggling the thread names (and check that it has no effect on performance). This also allows to remove unused and deprecated macros.
2024-09-04Merge bitcoin/bitcoin#30244: ci: parse TEST_RUNNER_EXTRA into an arrayAva Chow
8131bf7483c0ea10d3573c9f2e977d19d8569b7f ci: parse TEST_RUNNER_EXTRA into an array (Max Edwards) c4762b0aa06f2654d108bc7ca05887ffd88cf6f8 test: allow excluding func test by name and arg (Max Edwards) Pull request description: While working on CI I wanted to disable some functional tests so I used the `TEST_RUNNER_EXTRA` var. The problem I had was tests that have flags such as `rpc_bind.py --ipv6` must be passed in quotes otherwise the `--ipv6` portion will be considered an argument to `test_runner.py` rather than a test name. This change allows proper parsing of quotes and complex values such as: ```shell TEST_RUNNER_EXTRA='--exclude "rpc_bind.py --ipv6,feature_proxy.py"' ``` Update: While testing this it was noticed that `test_runner.py` when given `--exclude "rpc_bind.py --ipv6"` will exclude all `rpc_bind.py` tests so this PR has been updated to include a change to the test runner to only exclude the specific test if you pass an arg or exclude all tests of that name if you do not pass an arg. `--exclude rpc_bind.py` will exclude all three variants and `--exclude rpc_bind --ipv6` will only exclude the IPV6 variant. ACKs for top commit: maflcko: ACK 8131bf7483c0ea10d3573c9f2e977d19d8569b7f achow101: ACK 8131bf7483c0ea10d3573c9f2e977d19d8569b7f hebasto: ACK 8131bf7483c0ea10d3573c9f2e977d19d8569b7f, tested on Ubuntu 23.10 and Windows 11. Tree-SHA512: 82b73f12d627f533d8e5be4a518d455ef4427a755bbe03ccd11d0bb70c7ff3cee76220b0264fcfb236661c4cf5deba034cbfc2372b96d5861f3436c21eae8264
2024-09-04Merge bitcoin/bitcoin#30723: lint: Speed up and fix flake8 checksAva Chow
fafdb7df34507eee735893aa871da6ae529e6372 lint: Speed up flake8 checks (MarcoFalke) faf17df7fb88590d936d10c471a9ea6a2ce4454d lint: Document missing py_lint dependency (MarcoFalke) faebeb828f5f0ec68d90e7f76add66bc562f6fa3 lint: Remove python whitespace and shadowing lint rules (MarcoFalke) 77770478355ce6c1ab077dbc12ec898875ec5620 lint: Remove python lint rules that are SyntaxError (MarcoFalke) faaf3e53f09c73278e36674db0af14a262f0bd94 test: [refactor] Fix F841 flake8 (MarcoFalke) 444421db69539b74077306b6d0cb23e82afeb891 test: [refactor] Fix E714 pycodestyle (MarcoFalke) Pull request description: The checks have many issues: * Some checks that could in theory hide bugs are not applied -> Fix them and apply them going forward * Some checks are redundant Python 2 checks, or of low value -> Remove them * The checks are slow -> Speed them up from ~10 seconds to about ~20 milliseconds ACKs for top commit: davidgumberg: review and tested reACK https://github.com/bitcoin/bitcoin/commit/fafdb7df34507eee735893aa871da6ae529e6372 kevkevinpal: ACK [fafdb7d](https://github.com/bitcoin/bitcoin/pull/30723/commits/fafdb7df34507eee735893aa871da6ae529e6372) achow101: ACK fafdb7df34507eee735893aa871da6ae529e6372 Tree-SHA512: a0488b722cfaf7071bd6848cd3be002e0b6c38af80d8b5cbb08613c0b174ef63277289f960db8ac31adb09fe563a4973203b8fb10b83cbcfdc6f0ef39bd04410
2024-09-04Merge bitcoin/bitcoin#30148: cli: restrict multiple exclusive argument usage ↵Ava Chow
in bitcoin-cli c8e6771af002eaf15567794fcdc57fdb0e3fb140 test: restrict multiple CLI arguments (naiyoma) 8838c4f171f3c3e568d237b216167fddb521d0a7 common/args.h: automate check for multiple cli commands (naiyoma) Pull request description: This PR is a continuation of the validation suggested [here](https://github.com/bitcoin/bitcoin/pull/27815) to ensure that only one Request Handler can be specified at a time. ACKs for top commit: stratospher: reACK c8e6771. achow101: ACK c8e6771af002eaf15567794fcdc57fdb0e3fb140 tdb3: cr re ACK c8e6771af002eaf15567794fcdc57fdb0e3fb140 Tree-SHA512: f4fe036fee342a54f1a7ac702ac35c40bf3d420fde6ab16313a75327292d5ee5c8ece1be9f852a13fcf73da1148b143b37b4894e294052abdde6eefb2e8c6f3f
2024-09-04Merge bitcoin/bitcoin#29566: test: update satoshi_round functionAva Chow
ec317bc44b0cb089419d809b5fef38ecb9423051 test: update satoshi_round function (naiyoma) Pull request description: This PR refactors `satoshi_round` to accept different rounding modes and make rounding a required argument. Continuation of https://github.com/bitcoin/bitcoin/pull/23225 ACKs for top commit: maflcko: review ACK ec317bc44b0cb089419d809b5fef38ecb9423051 achow101: ACK ec317bc44b0cb089419d809b5fef38ecb9423051 AngusP: ACK ec317bc44b0cb089419d809b5fef38ecb9423051 Tree-SHA512: 070f0aa6f66e58bff7412cae6b71f5f4ab8c718c7b5eeba4bb604fe22c6416a1ced0474294f504e1c28943ddc073104466b5944b43bae27e42bee5ca85afa468
2024-09-04Merge bitcoin/bitcoin#28417: contrib/signet/miner updatesAva Chow
fb6d51eb25a2bb69a3ecdfc4796a88d4d1aacc65 signet/miner: Use argparse exclusive groups (Anthony Towns) 338a266a9a08e47bc6dd02175c8fa649f701515d signet/miner: add support for a poolnum/poolid tag in mined blocks (Anthony Towns) 409ab7d35b3cfa255a83e1004c55691515e4e3f5 signet/miner: add Generate.mine function (Anthony Towns) 7b3133237072a77231b38e59d619fd50fa769a6f signet/miner: add Generate.gbt function (Anthony Towns) 85c5c0bea9d45e93a9fb20988457480798d68637 signet/miner: add Generate.next_block_time function (Anthony Towns) 5540e6ca4930f99a1c0a1ee7b6e1c6ed75f95b55 signet/miner: move next_block_* functions into new Generator class (Anthony Towns) 35f46311969261f42727de4faac38dd9651f8d78 signet/miner: rename do_decode_psbt to decode_psbt (Anthony Towns) aac040b439fd917274eabfc81b89eb6ed2fee325 signet/miner: drop create_coinbase function (Anthony Towns) 16951f549eba8540311d52c4ee387714ea9f7d4c signet/miner: drop do_createpsbt function (Anthony Towns) 3aed0a4284d1d44144649140e947aef6943d2967 signet/miner: drop get_reward_address function (Anthony Towns) Pull request description: Refactors the code a bunch, and adds `--poolnum` / `--poolid` options so that signers can tag their coinbases in a way that explorers can recognise (see also https://github.com/bitcoin-data/mining-pools/pull/82 and https://github.com/mempool/mempool/issues/2903). The refactoring in particular helps enable the "try using inquisition's getblocktemplate, and if that doesn't work fall back to core's getblocktemplate" logic, as described/implemented in https://github.com/bitcoin-inquisition/bitcoin/pull/7 ACKs for top commit: achow101: ACK fb6d51eb25a2bb69a3ecdfc4796a88d4d1aacc65 danielabrozzoni: Code review ACK fb6d51eb25a2bb69a3ecdfc4796a88d4d1aacc65 Tree-SHA512: d84095c4045ab196685b847e04ce2cdaedf387bc2527430ede918318dc5b70bf3d87b754264016f895f506fac70d4fdea5ef3cd8c3c375fd586afeae01e045e5
2024-09-04Merge bitcoin/bitcoin#29605: net: Favor peers from addrman over fetching ↵Ava Chow
seednodes 6eeb188d40fe8f6c3b0a3be6dca519ea9a7b2358 test: adds seednode functional tests (Sergi Delgado Segura) 3270f0adad6ccbb8c004fb222f420e9b3ea32ea6 net: Favor peers from addrman over fetching seednodes (Sergi Delgado Segura) Pull request description: This is a follow-up of #28016 motivated by https://github.com/bitcoin/bitcoin/pull/28016#pullrequestreview-1913140932 and https://github.com/bitcoin/bitcoin/pull/28016#issuecomment-1984448937. The current behavior of seednode fetching is pretty eager: we do it as the first step under `ThreadOpenNetworkConnections` even if some peers may be queryable from our addrman. This poses two potential issues: - First, if permanently set (e.g. running with seednode in a config file) we'd be signaling such seed every time we restart our node - Second, we will be giving the seed node way too much influence over our addrman, populating the latter with data from the former even when unnecessary This changes the behavior to only add seednodes to `m_addr_fetch` if our addrman is empty, or little by little after we've spent some time trying addresses from our addrman. Also, seednodes are added to `m_addr_fetch` in random order, to avoid signaling the same node in case more than one seed is added and we happen to try them over multiple restarts ACKs for top commit: achow101: ACK 6eeb188d40fe8f6c3b0a3be6dca519ea9a7b2358 cbergqvist: ACK 6eeb188d40fe8f6c3b0a3be6dca519ea9a7b2358 itornaza: Tested ACK 6eeb188d40fe8f6c3b0a3be6dca519ea9a7b2358 tdb3: ACK 6eeb188d40fe8f6c3b0a3be6dca519ea9a7b2358 Tree-SHA512: b04445412f22018852d6bef4d3f1e88425ee6ddb434f61dcffa9e0c41b8e31f8c56f83858d5c7686289c86dc4c9476c437df15ea61a47082e2bb2e073cc62f15
2024-09-03Merge bitcoin/bitcoin#29553: assumeutxo: Add dumptxoutset height param, ↵Ava Chow
remove shell scripts 94b0adcc371540732453d70309c4083d4bd9cd6b rpc, refactor: Prevent potential race conditions in dumptxoutset (Fabian Jahr) e868a6e070a91c00555e72181f9b14bbf0373fdc doc: Improve assumeutxo guide and add more docs/comments (Fabian Jahr) b29c21fc92dcc3da95bd032ba41675a8b9a0a24b assumeutxo: Remove devtools/utxo_snapshot.sh (Fabian Jahr) 20a1c77aa7dec2449071187a439d17f7aeaee648 contrib: Remove test_utxo_snapshots.sh (Fabian Jahr) 842685035244e151f4a10019af2dfe0563f11a82 test: Test for dumptxoutset at specific height (Fabian Jahr) 993cafe7e45ab0af1e862c7def3de688f47c0443 RPC: Add type parameter to dumptxoutset (Fabian Jahr) fccf4f91d21c351d742943d35476f53d40963b8b RPC: Extract ReconsiderBlock helper (Fabian Jahr) 446ce51c21cd2466cb12fa0166fd069d42b603bf RPC: Extract InvalidateBlock helper (Fabian Jahr) Pull request description: This adds a height parameter to the `dumptxoutset` RPC. This internalizes the workflow that was previously done by scripts: roll back the chain to the height we actually want the snapshot from, create the snapshot, roll forward to the real tip again. The nice thing about internalizing this functionality is that we can write tests for the code and it gives us more options to make the functionality robust. The shell scripts we have so far will be more cumbersome to maintain in the long run, especially since we will only notice later when we have broken them. I think it's safe to remove these `test_utxo_snapshots.sh` as well when we have this option in `dumptxoutset` because we have also added some good additional functional test coverage for this functionality. ACKs for top commit: Sjors: re-utACK 94b0adcc371540732453d70309c4083d4bd9cd6b achow101: ACK 94b0adcc371540732453d70309c4083d4bd9cd6b mzumsande: ACK 94b0adcc371540732453d70309c4083d4bd9cd6b pablomartin4btc: re-ACK 94b0adcc371540732453d70309c4083d4bd9cd6b Tree-SHA512: a4c9af5f687d1ca7bfb579a36f363882823386b5fa80c05de531b05a2782b5da6ff5baf3ada4bca8f32f63975d86f1948175abed9affe51fc958472b5f838dab
2024-09-03tracing: cast block_connected duration to nanoseconds0xb10c
When the tracepoint was introduced in 8f37f5c2a562c38c83fc40234ade9c301fc4e685, the connect_block duration was passed in microseconds `µs`. By starting to use steady clock in fabf1cdb206e368a9433abf99a5ea2762a5ed2c0 this changed to nanoseconds `ns`. As the test only checked if the duration value is `> 0` as a plausibility check, this went unnoticed. I detected this when setting up monitoring for block validation time as part of the Great Consensus Cleanup Revival discussion. This change casts the duration explicitly to nanoseconds (as it has been nanoseconds for the last three releases; switching back now would 'break' the broken API again; there don't seem to be many users affected), updates the documentation and adds a check for an upper bound to the tracepoint interface tests. The upper bound is quite lax as mining the block takes much longer than connecting the empty test block. It's however able to detect incorrect duration units passed.
2024-09-02Merge bitcoin/bitcoin#30761: test: Avoid intermittent timeout in ↵merge-script
p2p_headers_sync_with_minchainwork.py fa247e6e8c7fddf9e3461c3e2e6f5fade0fe64cf test: Avoid intermittent timeout in p2p_headers_sync_with_minchainwork.py (MarcoFalke) Pull request description: Similar to https://github.com/bitcoin/bitcoin/pull/30705: The goal of this test case is to check that the sync works at all, not to check any timeout. On extremely slow hardware (for example qemu virtual hardware), downloading the 4110 BLOCKS_TO_MINE may take longer than the block download timeout. Fix it by pinning the time using mocktime temporarily, and advance it immediately after the sync. ACKs for top commit: stratospher: ACK fa247e6. Checked the timeout downloading block logs before/after using `setmocktime`. tdb3: ACK fa247e6e8c7fddf9e3461c3e2e6f5fade0fe64cf Tree-SHA512: f61632a8d9e484f1b888aafbf87f7adf71b8692387bd77f603cdbc0de49f30d42e654741d46ae1ff8b9706a5559ee0faabdb192ed0db7449010b68bfcdbaa42d
2024-09-02Merge bitcoin/bitcoin#30750: scripted-diff: LogPrint -> LogDebugmerge-script
fa09cb41f58d0483ffe134eb274b9048c5260faa refactor: Remove unused LogPrint (MarcoFalke) 333341589010b1d9b21b68ae6649992fd2653756 scripted-diff: LogPrint -> LogDebug (MarcoFalke) Pull request description: `LogPrint` has many issues: * It seems to indicate that something is being "printed", however config options such as `-printtoconsole` actually control what and where something is logged. * It does not mention the log severity (debug). * It is a deprecated alias for `LogDebug`, according to the dev notes. * It wastes review cycles, because reviewers sometimes point out that it is deprecated. * It makes the code inconsistent, when both are used, possibly even in lines right next to each other (like in `InitHTTPServer`) Fix all issues by removing the deprecated alias. I checked all conflicting pull requests and at the time of writing there are no conflicts, except in pull requests that are marked as draft, are yet unreviewed, or are blocked on feedback for other reasons. So I think it is fine to do now. ACKs for top commit: stickies-v: ACK fa09cb41f58d0483ffe134eb274b9048c5260faa danielabrozzoni: utACK fa09cb41f58d0483ffe134eb274b9048c5260faa TheCharlatan: ACK fa09cb41f58d0483ffe134eb274b9048c5260faa Tree-SHA512: 14270f4cfa3906025a0b994cbb5b2e3c8c2427c0beb19c717a505a2ccbfb1fd1ecf2fd03f6c52d22cde69a8d057e50d2207119fab2c2bc8228db3f10d4288d0f
2024-09-02Merge bitcoin/bitcoin#30664: build: Remove Autotools-based build systemmerge-script
faa382ae7642da0e436ea2c7f7eac67386280a7e ci, doc: Drop reference to `src/.bear-tidy-config` (Hennadii Stepanov) d71ac768424333b65a6d88c9752cc9c7fdb276f3 build: Remove Autotools-based build system (Hennadii Stepanov) e268b48419b802857c329a7ae27d3dbe4c1a9a4b doc: Adjust `doc/design/libraries.md` (Hennadii Stepanov) d209e4f1566f9240f105bb93ed61bda9b4bb272b doc: Drop mentions of `share/genbuild.sh` (Hennadii Stepanov) Pull request description: This PR deletes the Autotools-based build system. The MSVC build system is deleted in https://github.com/bitcoin/bitcoin/pull/30731. ACKs for top commit: maflcko: re-ACK faa382ae7642da0e436ea2c7f7eac67386280a7e 🍦 TheCharlatan: ACK faa382ae7642da0e436ea2c7f7eac67386280a7e fanquake: ACK faa382ae7642da0e436ea2c7f7eac67386280a7e Tree-SHA512: 53df977b5b199a1c38f7f61a042a62b24831c559ba65a461b4ac1c96a1a56e2dfd676df79f1358fd1cc1749ff27e7b548086157f337d4f596c1054cb3d2d5739
2024-09-01test: Test for dumptxoutset at specific heightFabian Jahr
2024-09-01RPC: Add type parameter to dumptxoutsetFabian Jahr
2024-08-30build: Remove Autotools-based build systemHennadii Stepanov
2024-08-30Remove unused src_dir param from run_testsLőrinc
2024-08-30test: Avoid intermittent timeout in p2p_headers_sync_with_minchainwork.pyMarcoFalke
2024-08-29Merge bitcoin/bitcoin#30714: test: fix `TestShell` initialization (late ↵glozow
follow-up for #30463) bd7ce05f9d9d21903163a2bd9dd2df3ed3990c3e test: fix `TestShell` initialization (late follow-up for #30463) (Sebastian Falbesoner) Pull request description: Creating a `TestShell` instance as stated in the [docs](https://github.com/bitcoin/bitcoin/blob/master/test/functional/test-shell.md) currently fails on master: ``` $ python3 Python 3.10.13 (main, Mar 15 2024, 07:36:23) [Clang 16.0.6 ] on openbsd7 Type "help", "copyright", "credits" or "license" for more information. >>> import sys >>> sys.path.insert(0, "/home/thestack/bitcoin/test/functional") >>> from test_framework.test_shell import TestShell >>> test = TestShell().setup(num_nodes=2, setup_clean_chain=True) Traceback (most recent call last): File "<stdin>", line 1, in <module> File "/home/thestack/bitcoin/test/functional/test_framework/test_shell.py", line 70, in __new__ TestShell.instance = TestShell.__TestShell() TypeError: BitcoinTestFramework.__init__() missing 1 required positional argument: 'test_file' ``` Since #30463, BitcoinTestFramework instances expect the path of the calling test at construction, in order to find shared data like the configuration (config.ini) and the cache. Note that in contrast to actual functional tests, we can't simply pass `__file__` here, as the test shell module sits within the `test_framework` subfolder, so we have to navigate up to the parent directory and append some dummy test file name. On the long-term we should probably add some TestShell instantation smoke-test to detect issues like this early. As I'm not too familiar with the CI I'm not sure what is a good way to achieve this (a functional test obviously can't be used, as that's already a BitcoinTestFramework test in itself), but happy to take suggestions. ACKs for top commit: ismaelsadeeq: Tested ACK bd7ce05f9d9d21903163a2bd9dd2df3ed3990c3e danielabrozzoni: tACK bd7ce05f9d9d21903163a2bd9dd2df3ed3990c3e brunoerg: ACK bd7ce05f9d9d21903163a2bd9dd2df3ed3990c3e Tree-SHA512: c3a2365e2cda48a233ee724673c490787981354914f33e10eadbbad9c68e8403d84c5551229a611401e743886539de380ba4bfcb77032b6c85731e3bbe962dc1
2024-08-29Merge bitcoin/bitcoin#30701: Use MiniWallet in functional test ↵glozow
rpc_signrawtransactionwithkey. a563f41232e2dd360ee8e76f6348dd10c7f4f2a3 Remove second node since only 1 is needed for the test (Martin Saposnic) 1f4cdb3d69926eedb6ed9376f30c4eefcf610a0c Replace custom funding tx creation with MiniWallet. (Martin Saposnic) Pull request description: In response to issue https://github.com/bitcoin/bitcoin/issues/30600, optimizations have been implemented to enhance test efficiency and readability: This PR refactors the `rpc_signrawtransactionwithkey.py` functional test to use MiniWallet for creating funding transactions. This simplifies the test code and improves performance by eliminating the need to mine new blocks for each funding transaction. Key changes: - Replaced custom `send_to_address` method with MiniWallet's `send_to` method - Removed unnecessary setup of a clean chain and second node - Simplified transaction creation and signing process ACKs for top commit: glozow: ACK a563f41232e2dd360ee8e76f6348dd10c7f4f2a3 ismaelsadeeq: code review ACK a563f41232e2dd360ee8e76f6348dd10c7f4f2a3 theStack: ACK a563f41232e2dd360ee8e76f6348dd10c7f4f2a3 Tree-SHA512: 318959f89702b169453d537dafb822f5ef1921db1088941d8bbdb3171dd7a6ecad590e57a3802bc37bcf8992267ed6ffa7f156b229d9817ebf812bd35df509b5
2024-08-29refactor: Remove unused LogPrintMarcoFalke
2024-08-29scripted-diff: LogPrint -> LogDebugMarcoFalke
-BEGIN VERIFY SCRIPT- sed -i 's/\<LogPrint\>/LogDebug/g' $( git grep -l '\<LogPrint\>' -- ./contrib/ ./src/ ./test/ ':(exclude)src/logging.h' ) -END VERIFY SCRIPT-
2024-08-29lint: Speed up flake8 checksMarcoFalke
Previously they may have taken more than 10 seconds. Now they should finish in less than one second. This also allows to drop one dependency to be installed.
2024-08-29lint: Document missing py_lint dependencyMarcoFalke
Also, change the linter name, needed for the next commit.
2024-08-29lint: Remove python whitespace and shadowing lint rulesMarcoFalke
The rules have many issues: * Most are redundant, because Python already has a built-in IndentationError, a subclass of SyntaxError, to enforce whitespace. * They are not enforced consistently anyway, see for examples [1][2] below. * They are stylistic rules where the author intentionally formatted the code to be easier to read. Starting to enforce them now would make the code harder to read and create frustration in the future. Fix all issues by removing them. [1]: test/functional/feature_cltv.py:63:35: E272 [*] Multiple spaces before keyword | 61 | # | Script to prepend to scriptSig | nSequence | nLockTime | 62 | # +-------------------------------------------------+------------+--------------+ 63 | [[OP_CHECKLOCKTIMEVERIFY], None, None], | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ E272 [2]: contrib/asmap/asmap.py:395:13: E306 [*] Expected 1 blank line before a nested definition, found 0 | 393 | prefix.pop() 394 | hole = not fill and (lhole or rhole) 395 | def candidate(ctx: Optional[int], res0: Optional[list[ASNEntry]], | ^^^ E306
2024-08-29lint: Remove python lint rules that are SyntaxErrorMarcoFalke
Any kind of syntax error is already reported, so there is no need to enumerate all possible types of syntax errors of ancient versions of Python 2 or 3.
2024-08-29test: [refactor] Fix F841 flake8MarcoFalke
2024-08-29test: [refactor] Fix E714 pycodestyleMarcoFalke
2024-08-28Update spelling.ignore-wordsLőrinc
Removed ba, inflight, keypair and warmup. Added incomin found in optionsdialog.ui:345 and re-use found in utxo_snapshot.cpp
2024-08-28Merge bitcoin/bitcoin#22838: descriptors: Be able to specify change and ↵glozow
receiving in a single descriptor string a0abcbd3822bd17a1d73c42ccd5b040a150b0501 doc: Mention multipath specifier (Ava Chow) 0019f61fc546b4d5f42eb4086f42560863fe0efb tests: Test importing of multipath descriptors (Ava Chow) f97d5c137d605ac48f1122a836c9aa5f834957ba wallet, rpc: Allow importdescriptors to import multipath descriptors (Ava Chow) 32dcbca3fb918bc899a0637f876db31c3419aafd rpc: Allow importmulti to import multipath descriptors correctly (Ava Chow) 64dfe3ce4bed9ac168d0b08def8af7485db94ef1 wallet: Move internal to be per key when importing (Ava Chow) 16922455253f47fae0466c4ec6c3adfadcfe9182 tests: Multipath descriptors for scantxoutset and deriveaddresses (Ava Chow) cddc0ba9a9dca3ca5873d768b3b504cdb2ab947b rpc: Have deriveaddresses derive receiving and change (Ava Chow) 360456cd221501fde3efe11bdba5c6d999dbb323 tests: Multipath descriptors for getdescriptorinfo (Ava Chow) a90eee444c965bbd7bcddf9656eca9cee14c3aec tests: Add unit tests for multipath descriptors (Ava Chow) 1bbf46e2dae4599d04c79aaacf7c5db00b2e707f descriptors: Change Parse to return vector of descriptors (Ava Chow) 0d640c6f02bc20e5c1be773443dd74d8806d953b descriptors: Have ParseKeypath handle multipath specifiers (Ava Chow) a5f39b103461a98689fd5d382e8da29037f55bea descriptors: Change ParseScript to return vector of descriptors (Ava Chow) 0d55deae157f4f8226b2419d55e7dc0dfb6e4aec descriptors: Add DescriptorImpl::Clone (Ava Chow) 7e86541f723d62c7ec6768f7f592c09ba2047d9e descriptors: Add PubkeyProvider::Clone (Ava Chow) Pull request description: It is convenient to have a descriptor which specifies both receiving and change addresses in a single string. However, as discussed in https://github.com/bitcoin/bitcoin/issues/17190#issuecomment-895515768, it is not feasible to use a generic multipath specification like BIP 88 due to combinatorial blow up and that it would result in unexpected descriptors. To resolve that problem, this PR proposes a targeted solution which allows only a single pair of 2 derivation indexes to be inserted in the place of a single derivation index. So instead of two descriptor `wpkh(xpub.../0/0/*)` and `wpkh(xpub.../0/1/*)` to represent receive and change addresses, this could be written as `wpkh(xpub.../0/<0;1>/*)`. The multipath specifier is of the form `<NUM;NUM>`. Each `NUM` can have its own hardened specifier, e.g. `<0;1h>` is valid. The multipath specifier can also only appear in one path index in the derivation path. This results in the parser returning two descriptors. The first descriptor uses the first `NUM` in all pairs present, and the second uses the second `NUM`. In our implementation, if a multipath descriptor is not provided, a pair is still returned, but the second element is just `nullptr`. The wallet will not output the multipath descriptors (yet). Furthermore, when a multipath descriptor is imported, it is expanded to the two descriptors and each imported on its own, with the second descriptor being implicitly for internal (change) addresses. There is no change to how the wallet stores or outputs descriptors (yet). Note that the path specifier is different from what was proposed. It uses angle brackets and the semicolon because these are unused characters available in the character set and I wanted to avoid conflicts with characters already in use in descriptors. Closes #17190 ACKs for top commit: darosior: re-ACK a0abcbd3822bd17a1d73c42ccd5b040a150b0501 mjdietzx: reACK a0abcbd3822bd17a1d73c42ccd5b040a150b0501 pythcoiner: reACK a0abcbd furszy: Code review ACK a0abcbd glozow: light code review ACK a0abcbd3822 Tree-SHA512: 84ea40b3fd1b762194acd021cae018c2f09b98e595f5e87de5c832c265cfe8a6d0bc4dae25785392fa90db0f6301ddf9aea787980a29c74f81d04b711ac446c2
generated by cgit v1.2.3 (git 2.26.2) at 2024-11-22 00:25:16 +0000