forked from bitcoin/bitcoin
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
backport: trivial 2024 06 05 #6047
Merged
Merged
Conversation
This file contains 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
5202bd1 test: Bump shellcheck version to 0.8.0 (Hennadii Stepanov) Pull request description: Among [added](https://github.com/koalaman/shellcheck/blob/master/CHANGELOG.md#v080---2021-11-06) rules, SC2295 could be [useful](bitcoin#23506 (comment)) for us. ACKs for top commit: dongcarl: Code Review ACK 5202bd1 fanquake: ACK 5202bd1 - would have rather this just been a part of bitcoin#23506 to avoid another PR and pointless rebasing. Tree-SHA512: fd7ff801c71af03c5a5b2823b7daba25a430b3ead5e5e50a3663961ee2223e55d322aec91d79999814cd35bd7ed6e9415a0b797718ceb8c0b1dbdbb40c336b82
UdjinM6
requested changes
Jun 5, 2024
fa72dd3 fuzz: Move ISO8601 to one place (MarcoFalke) Pull request description: Seems confusing to split this to two places. Also fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=42178 ACKs for top commit: fanquake: ACK fa72dd3 Tree-SHA512: 637b0671078848ea417fdf66b92715602040fad34d4ca5f7b843a519a1cfeebe5d992a79a399deba39926905125681d66ab0dc05f66f79a26f3bf555e12fb0ba
0a1b6fa test: fix intermittent timeouts in p2p_timeouts.py (Martin Zumsande) Pull request description: Fixes bitcoin#23800 by making sure that all peers are connected (i.e. `m_connected` is set) before the mocktime is bumped. We can't wait for verack here, but we can wait for a debug log entry ("Added connection peer=2") instead. In the failed CI runs (e.g. https://cirrus-ci.com/task/5600553806856192?logs=ci#L7469) different peers were added at different mocktimes. ACKs for top commit: naumenkogs: ACK 0a1b6fa theStack: Concept and approach ACK 0a1b6fa Tree-SHA512: 1a3c8a9a79339d4adc6ecb1731eb0d0eadb2e5024ad3c6779b4696691f85d6c3304ef8689746d0332150a4cf04489ca4b2ff3eeb0bb76feec28c1e4bb9dbca19
1268532 test: add functional test for -startupnotify (Bruno Garcia) Pull request description: This PR adds a functional test for -startupnotify. It basically starts the node passing a command on -startupnotify to create a file on tmp and then, we check if the file has been successfully created. ACKs for top commit: theStack: Tested ACK 1268532 kristapsk: re-ACK 1268532 Tree-SHA512: 5bf3e46124ee5c9d609c9993e6465d5a71a8d2275dcf07c8ce0549f013f7f8863d483b46b7164152f566468a689371ccb87f01cf118c3c9cac5b2be673b61a5c
…le in startupnotify test 96eb009 test: wait rather than assert presence of file in startupnotify test (fanquake) Pull request description: Should fix bitcoin#23967. ACKs for top commit: brunoerg: crACK 96eb009 kristapsk: utACK 96eb009 Tree-SHA512: 9107970e45c027cfc6c6cbfcfd5a7d9f5956259bbbb11f5b180c3947126e42e62c0f8ffd69cf7b39b51c9c5b4fedbb753839d59aebe876be68c1484bb6065819
…the correct size ac617cc wallettool: Check that the dumpfile checksum is the correct size (Andrew Chow) Pull request description: After parsing the checksum, make sure that it is the size that we expect it to be. This issue was reported by Pedro Baptista. ACKs for top commit: laanwj: Code review ACK ac617cc Tree-SHA512: 8135b3fb1f4f6b6c91cfbac7d1d3421f1f6c664a742c92940f68eae857f92ce49d042cc3aa5c2df6ef182825271483d65efc7543ec7a8ff047fd7c08666c8899
86a4a15 Highlight DNS request part (Prayank) Pull request description: _What?_ Highlight DNS requests part in Proxy section _Why?_ 1. DNS requests are very important while considering privacy 2. Lot of users might skip reading it because of the way it is mixed with everything else in the doc right now 3. I have seen lot of users ignoring DNS requests or unaware of such things while using privacy tools _How?_ Initially I had tried keeping these lines separate from code block but [Jonatack didn't agree with the changes](bitcoin#21157 (comment)). Harding suggested using [bold/italic in `<pre></pre>`](bitcoin#21157 (comment)). I have used the suggestions from previous PR and added `---` This is a part of alternative described in bitcoin#22316 ACKs for top commit: jonatack: ACK 86a4a15 Rspigler: ACK 86a4a15 achow101: ACK 86a4a15 RiccardoMasutti: ACK 86a4a15 lsilva01: ACK bitcoin@86a4a15 kristapsk: ACK 86a4a15 theStack: ACK 86a4a15 Tree-SHA512: a4fe0e8c08df330e5ca78ce19ce74be7034c653f4374469d928908847a6debf385283e3a6da66de600566c7bab6290ccd35df26864aef94cbb3f294123391437
…ionStr() helper 6f2593d gui, refactor: use std::chrono for formatDurationStr() helper (Jon Atack) Pull request description: Updates `formatDurationStr()` to use the `chrono` standard lib. No change in behavior. ACKs for top commit: RandyMcMillan: tACK 6f2593d shaavan: ACK 6f2593d w0xlt: tACK 6f2593d on Ubuntu 21.10 Qt 5.15.2 promag: Code review ACK 6f2593d. Tree-SHA512: 61e9afdb1db779150df338e6af08727c34f69639add465c2f7003ff775d97dce3e78e78d325bc6dea5bc13f0fce9ef1c3506d13f1661a5e083e52bba8a32ba44
f59bee3 fuzz: execute each file in dir without fuzz engine (Anthony Towns) Pull request description: Phony fuzzing (phuzzing)! Run the fuzz testing code against known inputs to detect errors. Advantage is you can easily test using the existing qa-assets datasets without having to compile with fuzzing enabled; disadvantage is that it doesn't do any actual fuzzing. Example usage: ``` $ for a in ${QA_ASSETS}/fuzz_seed_corpus/*; do echo ${a##*/}; done | xargs -P8 -I {} /bin/sh -c "FUZZ={} test/fuzz/fuzz ${QA_ASSETS}/fuzz_seed_corpus/{}" No fuzzer for address_deserialize. No fuzzer for addrdb. No fuzzer for banentry_deserialize. addition_overflow: succeeded against 848 files in 0s. asmap: succeeded against 981 files in 0s. checkqueue: succeeded against 211 files in 0s. ... ``` (`-P8` says run 8 of the tasks in parallel) If there are failures, the first one will be reported and the program will abort with output like: ``` fuzz: test/fuzz/versionbits.cpp:336: void (anonymous namespace)::versionbits_fuzz_target(FuzzBufferType): Assertion `exp_state != ThresholdState::FAILED' failed. Error processing seed "corpus/versionbits/35345ae8e722234095810b1117a29b63af7621af" ``` Rebase of bitcoin#22763, which was a rebase of bitcoin#21496, but also reports the name of the fuzzer and the time taken. Fixes bitcoin#21461 Top commit has no ACKs. Tree-SHA512: d8d046d4a309652eb13de42116276bf992480bc887ad3535a8ff18b354cb24826bc562b06af63802ec945c637f046563b6a5601d6321b46a5543127daafea09b
…x raises an error d6bc232 test: -peerblockfilters without -blockfilterindex raises an error (brunoerg) Pull request description: This PR adds test coverage for the following init error: https://github.com/bitcoin/bitcoin/blob/2a3e8fb3592e42300ec96c9f6724e15346e30ea7/src/init.cpp#L850 Setting -peerblockfilters without -blockfilterindex should raise an error when initializing. ACKs for top commit: ccdle12: Tested ACK bitcoin@d6bc232 Tree-SHA512: e740c2ccde6bb1bb8381bb676a6d01bd5746cf9ce0c8dadd62067a6b9b380027bfe8b8cdeae9846a0ab18385f3dc5dff607fe5274cb55107d47470db00015fb2
9d65ad3 Clear vTxHashes when mapTx is cleared (Peter Bushnell) Pull request description: vTxHashes is a vector of all entries in mapTx, if you clear one you should clear the other, lest someone try to use the txiter in vTxHashes which would result in a segfault. ACKs for top commit: laanwj: Code review ACK 9d65ad3 Tree-SHA512: 6832755e43ab7f528b46817aeadcb6ffdc965b97f59ab96bb053dedbb7e68155ba3db52286355dca33b509237f80eda249760b26db493762bc50d8e2cef16d8f
…ng returned 0cea7b1 print `(none)` if no warnings in -getinfo (/dev/fd0) Pull request description: Adds `(none)` in warnings when no warnings returned by -getinfo Reviewers can test this by making the following change in `/src/warnings.cpp`: ```diff bilingual_str GetWarnings(bool verbose) { bilingual_str warnings_concise; std::vector<bilingual_str> warnings_verbose; LOCK(g_warnings_mutex); // Pre-release build warning if (!CLIENT_VERSION_IS_RELEASE) { - warnings_concise = _("This is a pre-release test build - use at your own risk - do not use for mining or merchant applications");; + warnings_concise = _("");; ``` Before this pull request: ``` $ bitcoin-cli -getinfo Chain: regtest Blocks: 0 Headers: 0 Verification progress: 100.0000% Difficulty: 4.656542373906925e-10 Network: in 0, out 0, total 0 Version: 239900 Time offset (s): 0 Proxies: n/a Min tx relay fee rate (BTC/kvB): 0.00001000 Warnings: ``` After this pull request: ```diff $ bitcoin-cli -getinfo Chain: regtest Blocks: 0 Headers: 0 Verification progress: 100.0000% Difficulty: 4.656542373906925e-10 Network: in 0, out 0, total 0 Version: 239900 Time offset (s): 0 Proxies: n/a Min tx relay fee rate (BTC/kvB): 0.00001000 Warnings: (none) ``` ACKs for top commit: jonatack: ACK 0cea7b1 laanwj: Tested ACK 0cea7b1 Tree-SHA512: a12499d11ff84bc954db354f968eb1f5ee4999d8b80581fe0bdf604732b2e2f608cb5c35c4ca8cb5a430f3991954a6207f0758302618662e6b9505044cf2dc95
3ae7791 refactor: use Span in random.* (pasta) Pull request description: ~This PR does two things~ 1. use a Span<unsigned char> for GetRandBytes and GetStrongRandBytes ~2. make GetRand a template for which any integral type can be used, where the default behavior is to return a random integral up to the max of the integral unless a max is provided. This simplifies a lot of code from `GetRand(std::numeric_limits<uint64_t>::max()` -> `GetRand<uint64_t>()`~ MarcoFalke this was inspired by your comment here: bitcoin#24185 (comment) about using Span, so hopefully I'll be able to get this PR done and merged 😂 ~Also, if requested I could revert the `GetRand(std::numeric_limits<uint64_t>::max()` -> `GetRand<uint64_t>()` related changes if it ends up causing too many conflicts~ ACKs for top commit: laanwj: Thank you! Code review re-ACK 3ae7791 Tree-SHA512: 12375a83b68b288916ba0de81cfcab4aac14389a66a36811ae850427435eb67dd55e47df9ac3ec47db4e214f4330139e548bec815fff8a3f571484ea558dca79
…rtions.py 172c233 Porting lint-assertions.sh to lint-assertions.py (hiago) Pull request description: This PR is converting `test/lint/lint-assertions.sh` to `test/lint/lint-assertions.py`. It's an item of bitcoin#24783. ACKs for top commit: laanwj: Tested ACK 172c233 Tree-SHA512: 94d5b03acfeaf2303fad95d489d6c3aa7bd655889ddaa807cc97e0613b8eb8f5ef094feee2a98d974606890deb554e76490a5c523d64eb5bc55afa6a43221aae
4637bbe rpc: Explain active and internal in listdescriptors (Andrew Chow) Pull request description: The current help text for active and internal in listdescriptors is not particularly helpful. They require the reader to already know what those terms mean. This help text is updated to actually explain the definitions of those words in context of a descriptor wallet. ACKs for top commit: S3RK: ACK 4637bbe jarolrod: ACK 4637bbe w0xlt: ACK bitcoin@4637bbe Tree-SHA512: 0af2c04f3b9920799cf616ad618bde9248eb9f74cc28f443b5b0f6646deba76e9b1415aca0865ad3bcc24aa6af0e9d07ad7b7cd80f0fe80838cf847f1b944426
88044a1 Guard `#include <config/bitcoin-config.h>` (Hennadii Stepanov) Pull request description: A fix for builds when the `HAVE_CONFIG_H` macro is not defined. ACKs for top commit: Empact: Code Review ACK bitcoin@88044a1 Tree-SHA512: f2bf1693c7671d7113dccaf66ae34a84719d86cb3271fa18b36611deab93a48d787b3ccfbd735d3b763017d709971cb1151d8d7f30390720009e6e2a6275b5b0
…n::SweepBanned()` ab75388 refactor: Remove redundant scope in `BanMan::SweepBanned()` (Hennadii Stepanov) 52c0b3e refactor: Add thread safety annotation to `BanMan::SweepBanned()` (Hennadii Stepanov) 3919059 refactor: Move code from ctor into private `BanMan::LoadBanlist()` (Hennadii Stepanov) Pull request description: This PR adds a proper thread safety annotation to `BanMan::SweepBanned()`. Also a simple refactoring applied. ACKs for top commit: ajtowns: ACK ab75388 w0xlt: ACK bitcoin@ab75388 theStack: Code-review ACK ab75388 Tree-SHA512: 8699079c308454f3ffe72be2e77f0935214156bd509f9338b1104f8d128bfdd02ee06ee8c1c99b2eefdf317a00edd555d52990caaeb1ed4540eedc1e3c9d1faf
PastaPastaPasta
force-pushed
the
develop-trivial-2024-06-05
branch
from
June 7, 2024 03:58
f69b1c5
to
76279c1
Compare
UdjinM6
approved these changes
Jun 7, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK
PastaPastaPasta
closed this pull request by merging all changes
into
dashpay:develop
in
Jun 7, 2024
d7413ff
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Issue being fixed or feature implemented
Trivial batch of backports
What was done?
trivial backports
How Has This Been Tested?
Unit tests ran; waiting on CI
Breaking Changes
Checklist:
Go over all the following points, and put an
x
in all the boxes that apply.