backport: Merge bitcoin#30703, 30024, 29773, 29707, 28336, 29335, 30063, 3006, 30161, 30160#7308
Draft
vijaydasmp wants to merge 10 commits intodashpay:developfrom
Draft
backport: Merge bitcoin#30703, 30024, 29773, 29707, 28336, 29335, 30063, 3006, 30161, 30160#7308vijaydasmp wants to merge 10 commits intodashpay:developfrom
vijaydasmp wants to merge 10 commits intodashpay:developfrom
Conversation
…releases.py fa5aeab test: Avoid duplicate curl call in get_previous_releases.py (MarcoFalke) Pull request description: Seems odd having to translate `404` to "Binary tag was not found". Also, it seems odd to write a for-loop over a list with one item. Fix both issues by just using a single call to `curl --fail ...`. Can be tested with: `test/get_previous_releases.py -b v99.99.99` Before: ``` Releases directory: releases Fetching: https://bitcoincore.org/bin/bitcoin-core-99.99.99/bitcoin-99.99.99-x86_64-linux-gnu.tar.gz % Total % Received % Xferd Average Speed Time Time Time Current Dload Upload Total Spent Left Speed 0 286k 0 0 0 0 0 0 --:--:-- 0:00:01 --:--:-- 0 Binary tag was not found ``` After: ``` Releases directory: releases Fetching: https://bitcoincore.org/bin/bitcoin-core-99.99.99/bitcoin-99.99.99-x86_64-linux-gnu.tar.gz % Total % Received % Xferd Average Speed Time Time Time Current Dload Upload Total Spent Left Speed 0 286k 0 0 0 0 0 0 --:--:-- 0:00:01 --:--:-- 0 curl: (22) The requested URL returned error: 404 ACKs for top commit: fanquake: ACK fa5aeab brunoerg: utACK fa5aeab tdb3: tested ACK fa5aeab Tree-SHA512: d5d31e0bccdd9de9b4a8ecf2e69348f4e8cee773050c8259b61db1ce5de73f6fbfffbe8c4d2571f7bef2de29cb42fd244573deebfbec614e487e76ef41681b9c
…_SCRIPT_ELEMENT_SIZE ffc6745 Replace remaining "520" magic numbers with MAX_SCRIPT_ELEMENT_SIZE (Jon Atack) Pull request description: Noticed these while reviewing BIPs yesterday. It would be clearer and more future-proof to refer to their constant name. ACKs for top commit: instagibbs: ACK ffc6745 sipa: ACK ffc6745 achow101: ACK ffc6745 glozow: ACK ffc6745, agree it's clearer for these comments to refer to the greppable name of the limit rather than the number Tree-SHA512: 462afc1c64543877ac58cb3acdb01d42c6d08abfb362802f29f3482d75401a2a8adadbc2facd222a9a9fefcaab6854865ea400f50ad60bec17831d29f7798afe
31a15f0 bench: Disable WalletCreate* benchmarks when building with MSVC (Hennadii Stepanov) 23dc0c1 msvc, bench: Add missing source files to bench_bitcoin project (Hennadii Stepanov) Pull request description: On the master branch, the `bench_bitcoin.vcxproj` MSVC project misses wallet-specific source files. This PR fixes this issue. Benchmark run on Windows: ``` > src\bench_bitcoin.exe -filter="CoinSelection|BnBExhaustion|Wallet.*" | ns/op | op/s | err% | total | benchmark |--------------------:|--------------------:|--------:|----------:|:---------- | 398,800.00 | 2,507.52 | 1.5% | 0.01 | `BnBExhaustion` | 584,450.00 | 1,711.01 | 1.5% | 0.01 | `CoinSelection` | 86,603,650.00 | 11.55 | 0.4% | 1.91 | `WalletAvailableCoins` | 7,604.00 | 131,509.73 | 0.9% | 0.01 | `WalletBalanceClean` | 124,028.57 | 8,062.66 | 2.6% | 0.01 | `WalletBalanceDirty` | 7,587.12 | 131,802.30 | 1.9% | 0.01 | `WalletBalanceMine` | 48.58 | 20,583,872.99 | 0.9% | 0.01 | `WalletBalanceWatch` | 2,371,060.00 | 421.75 | 1.3% | 0.13 | `WalletCreateTxUseOnlyPresetInputs` | 96,861,760.00 | 10.32 | 0.9% | 5.31 | `WalletCreateTxUsePresetInputsAndCoinSelection` | 280.71 | 3,562,424.13 | 1.5% | 0.01 | `WalletIsMineDescriptors` | 1,033.47 | 967,618.32 | 0.3% | 0.01 | `WalletIsMineLegacy` | 282.36 | 3,541,599.91 | 0.5% | 0.01 | `WalletIsMineMigratedDescriptors` | 484,547,300.00 | 2.06 | 1.0% | 2.43 | `WalletLoadingDescriptors` | 29,924,300.00 | 33.42 | 0.4% | 0.15 | `WalletLoadingLegacy` ``` ACKs for top commit: maflcko: lgtm ACK 31a15f0 Tree-SHA512: 0241af06126edf612489322cdce66ba43792066b5400b1719a8b9d1ec62030e8a9d497e2f01e38290e94c387db59ccf2a458f4b35d3dc8030a1a1413d89eb792
5195baa depends: fix miniupnpc snprintf usage on Windows (fanquake) 3c2d440 depends: switch miniupnpc to CMake (Cory Fields) f5618c7 depends: add upstream CMake patch to miniupnpc (fanquake) 6866b57 depends: miniupnpc 2.2.7 (fanquake) Pull request description: This picks up one of the changes from bitcoin#29232, which is a switch to building miniupnpc with CMake. It includes an update to the most recent version of miniupnpc (2.2.7), which means we can drop one patch from that commit, and includes a new patch for a change I've upstreamed miniupnp/miniupnp#721, as well as some suggestions from the previous PR. ACKs for top commit: theuni: ACK 5195baa. TheCharlatan: utACK 5195baa Tree-SHA512: 5b27e132cd5eed285e9be34c8b96893417d92a1ae55c99345c9a89e1c1c5e40e4bc840bc061b879758b2b11fcb520cd98c3da985c1e153f2e5380cf63efe2d69
…ific error messages 98570fe test: add coverage for parsing cryptographically invalid pubkeys (Sebastian Falbesoner) c740b15 rpc: use `HexToPubKey` helper for all legacy pubkey-parsing RPCs (Sebastian Falbesoner) 100e8a7 rpc: check and throw specific pubkey parsing errors in `HexToPubKey` (Sebastian Falbesoner) Pull request description: Parsing legacy public keys can fail for three reasons (in this order): - pubkey is not in hex - pubkey has an invalid length (not 33 or 65 bytes for compressed/uncompressed, respectively) - pubkey is crytographically invalid, i.e. is not on curve (`CPubKey.IsFullyValid()` check) Many RPCs currently perform these checks manually with different error messages, even though we already have a `HexToPubKey` helper. This PR puts all three checks in this helper (the length check was done on the call-sites before), adds specific error messages for each case, and consequently uses it for all RPCs that parse legacy pubkeys. This leads to deduplicated code and also to more consistent and detailed error messages for the user. Affected RPC calls are `createmultisig`, `addmultisigaddress`, `importpubkey`, `importmulti`, `fundrawtransaction`, `walletcreatefundedpsbt`, `send` and `sendall`. Note that the error code (-5 a.k.a. `RPC_INVALID_ADDRESS_OR_KEY`) doesn't change in any of the causes, so the changes are not breaking RPC API compatibility. Only the messages are more specific. The last commits adds test coverage for the cryptographically invalid (not-on-curve) pubkey case which wasn't exercised before. ACKs for top commit: stratospher: tested ACK 98570fe. davidgumberg: ACK bitcoin@98570fe Eunovo: Tested ACK bitcoin@98570fe achow101: ACK 98570fe Tree-SHA512: cfa474176e95b5b18f3a9da28fdd9e87195cd58994c1331198f2840925fff322fd323a6371feab74a1b32e4b9ea58a6dc732fa751b4cdd45402c1029af609ece
357ad11 test: Handle functional test disk-full error (Brandon Odiwuor) Pull request description: Fixes: bitcoin#23099 Handle disk-full more gracefully in functional tests ACKs for top commit: itornaza: re-ACK 357ad11 achow101: reACK 357ad11 cbergqvist: reACK 357ad11. Looks good! tdb3: re ACK for 357ad11 Tree-SHA512: 9bb0d3fbe84600c88873b9f55d4b5d1443f79ec303467680c301be2b4879201387f203d9d1984169461f321037189b5e10a6a4b9d61750de638f072d2f95d77e
… variable 189d0da build, test: Remove unused `TIMEOUT` environment variable (Hennadii Stepanov) Pull request description: Setting the `TIMEOUT` environment variable has been a noop in both cases since its introduction. It seems to have been inadvertently copy-pasted from existed code. For example, in commit d80e3cb, it was needlessly copied from a valid case a few lines above for the `qa/pull-tester/run-bitcoind-for-test.sh` script. ACKs for top commit: maflcko: utACK 189d0da edilmedeiros: ACK 189d0da Tree-SHA512: 61111eba30e0c82a0220bea48eba451cd9caa68785b48ec8a91059ca5aadfaff2f6d2ccdc5aa737c5cefa33579cb735431bb9e94bda8fa047825d7bd28d542fb
fd6a7d3 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 bitcoin#27039 (comment) ACKs for top commit: maflcko: utACK fd6a7d3 achow101: ACK fd6a7d3 tdb3: ACK for fd6a7d3 rkrux: ACK [fd6a7d3](bitcoin@fd6a7d3) Tree-SHA512: 7ff0574833df1ec843159b35ee88b8bb345a513ac13ed0b72abd1bf330c454a3f9df4d927871b9e3d37bfcc07542b06ef63acef8e822cd18499adae8cbb0cda8
7b8eea0 tests: add fuzz tests for VecDeque (Pieter Wuille) 62fd24a util: add VecDeque (Pieter Wuille) Pull request description: Extracted from bitcoin#30126. This adds a `VecDeque` data type, inspired by `std::deque`, but backed by a single allocated memory region used as a ring buffer instead of a linked list of arrays. This gives better memory locality and less allocation overhead, plus better guarantees (some C++ standard library implementations, though not libstdc++ and libc++, use a separate allocation per element in a deque). It is intended for the candidate set search queue in bitcoin#30126, but may be useful as a replacement for `std::deque` in other places too. It's not a full drop-in replacement, as I did not add iteration support which is unnecessary for the intended use case, but nothing prevents adding that if needed. Everything is tested through a simulation-based fuzz test that compares the behavior with normal `std::deque` equivalent operations, both for trivially-copyable/destructible types and others. ACKs for top commit: instagibbs: reACK bitcoin@7b8eea0 cbergqvist: re-ACK 7b8eea0 hebasto: re-ACK 7b8eea0, I've verified changes since my recent [review](bitcoin#30161 (review)) with glozow: ACK 7b8eea0 Tree-SHA512: 1b62f3ba1a43a1293d8c9de047e2399442e74c46de2df81406151fe27538716ce265f35fb6779ee56d77a39cddf8fb4b4e15bda8f04ebf3b149e2f05fa55cb21
vijaydasmp
changed the title
|
47f705b tests: add fuzz tests for BitSet (Pieter Wuille) 59a6df6 util: add BitSet (Pieter Wuille) Pull request description: Extracted from bitcoin#30126. This introduces the `BitSet` data structure, inspired by `std::bitset`, but with a few features that cannot be implemented on top without efficiency loss: * Finding the first set bit (`First`) * Finding the last set bit (`Last`) * Iterating over all set bits (`begin` and `end`). And a few other operators/member functions that help readability for bitcoin#30126: * `operator-` for set subtraction * `Overlaps()` for testing whether intersection is non-empty * `IsSupersetOf()` for testing (non-strict) supersetness * `IsSubsetOf()` for testing (non-strict) subsetness * `Fill()` to construct a set with all numbers from 0 to n-1, inclusive * `Singleton()` to construct a set with one specific element. Everything is tested through a simulation-based fuzz test that compares the behavior with normal `std::bitset` equivalent operations. ACKs for top commit: instagibbs: ACK bitcoin@47f705b achow101: ACK 47f705b cbergqvist: re-ACK 47f705b theStack: Code-review ACK 47f705b Tree-SHA512: e451bf4b801f193239ee434b6b614f5a2ac7bb49c70af5aba24c2ac0c54acbef4672556800e4ac799ae835632bdba716209c5ca8c37433a6883dab4eb7cd67c1
vijaydasmp
changed the title
Author
|
@codex review pr |
💡 Codex Reviewdash/test/functional/test_runner.py Lines 684 to 686 in a8e58ba  Exit loop without bypassing dangling-job cleanup
The no-space error branch calls ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
bitcoin backports