backport: trivial 2024 10 23 pr1 #6345
Merged
Conversation
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
PastaPastaPasta
changed the title
kwvg
previously approved these changes
Oct 24, 2024
UdjinM6
requested changes
Oct 24, 2024
| @@ -25,6 +25,8 @@ def pad16(x): | |||
|
|
|||
| def aead_chacha20_poly1305_encrypt(key, nonce, aad, plaintext): | |||
| """Encrypt a plaintext using ChaCha20Poly1305.""" | |||
| if plaintext is None: | |||
There was a problem hiding this comment.
29390: also a part of #6330 and it probably belongs there and should be dropped here
PastaPastaPasta
force-pushed
the
develop-trivial-2024-10-23-pr1
branch
from
62c3997 to
054edd3
Compare
|
This pull request has conflicts, please rebase. |
…ddSingle` e064487 addrman, refactor: improve stochastic test in `AddSingle` (brunoerg) Pull request description: This PR changes this algorithm to be O(1) instead of O(n). Also, in the current implementation, if `pinfo->nRefCount` is 0, we created an unnecessary variable (`nFactor`), this changes it. the change is relatively simple and does not cause conflicts. ACKs for top commit: achow101: ACK e064487 amitiuttarwar: ACK e064487 stratospher: ACK e064487. simple use of << instead of a loop, didn't observe any behaviour difference before and after. Tree-SHA512: ff0a65155e47f65d2ce3cb5a3fd7a86efef1861181143df13a9d8e59cb16aee9be2f8801457bba8478b17fac47b015bff5cc656f6fac2ccc071ee7178a38d291
5ca9b24 test: Add makefile target for running unit tests (TheCharlatan) Pull request description: `make check` runs a bunch of other subtree tests that exercise code that is hardly ever changed and have a comparatively long runtime. There seems to be no target for running just the unit tests, so add one. Alternatively the secp256k1 tests could be removed from the `check-local` target, reducing its runtime. This was rejected before though in bitcoin#20264. ACKs for top commit: delta1: utACK 5ca9b24 edilmedeiros: Tested ACK 5ca9b24 achow101: ACK 5ca9b24 ryanofsky: Tested ACK 5ca9b24. Tree-SHA512: 470969d44585d7cc33ad038a16e791db9e2be8469f52ddf122c46f20776fad34e6a48f988861a132c42540158fed05f3cf66fcc3bea05708253daaa35af54339
fa0ceae test: Fix utxo set hash serialisation 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/kernel/coinstats.cpp#L54 Large positive values for the block height are too difficult to hit in tests, but it still seems fine to fix this. The bug was introduced when the code was written in 6ccc8fc. (Lowercase `i` means signed, see https://docs.python.org/3/library/struct.html#format-characters) ACKs for top commit: epiccurious: Tested ACK fa0ceae. fjahr: utACK fa0ceae Tree-SHA512: ab4405c74fb191fff8520b456d3a800cd084d616bb9ddca27d56b8e5c8969bd537490f6e204c1870dbb09a3e130b03b22a27b6644252a024059c200bbd9004e7
…store.py 44d1153 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 bitcoin#29392 (comment) 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 bitcoin#29392 ACKs for top commit: maflcko: lgtm ACK 44d1153 Tree-SHA512: c08699e5ae348d4c0626022b519449d052f511d3f44601bcd8dac836a130a3f67fca149532e1e3690367ebfdcbcdd32e527170d039209c1f599ce861136ae29f
…telist{bind}Permissions::TryParse`
864e2e9 fuzz: increase length of string used for `NetWhitelist{bind}Permissions::TryParse` (brunoerg)
Pull request description:
The string `s` represents the value from `-whitelist`/`-whitebind` (e.g. "bloom,forcerelay,noban@1.2.3.4:32") and it is used in `NetWhitelistPermissions::TryParse` and `NetWhitebindPermissions::TryParse`. However, a max length of 32 is not enough to cover a lot of cases. Even disconsidering the permissions, 32 would not be enough to cover a lot of addresses. This PR fixes it.
ACKs for top commit:
maflcko:
lgtm ACK 864e2e9
epiccurious:
utACK 864e2e9.
vasild:
ACK 864e2e9
Tree-SHA512: 2b89031b9f2ea92d636f05fd167b1e5ac726742a7e7c1af8ddaeaf90236e659731aaa6b7c23f65ec16ce52ac1b9e68e7b16e23c59e355312d057e001976d172a
0fbf051 depends: fix BDB compilation on OpenBSD (Sebastian Falbesoner) Pull request description: Compiling C++ code with `-D_XOPEN_SOURCE=600` causes problems on OpenBSD. If that define is set, the C++ standard header detection routine in BDB's configure script fails due to a missing type name for `locale_t` (see https://gist.github.com/theStack/b41884e31ebc5cdca3220bcaa674cb70 for the relevant config.log part). This results in `HAVE_CXX_STDHEADERS` not being defined, which then it turn leads to the inclusion of `<iostream.h>` (rather than `<iostream>`), which doesn't exist, as described in bitcoin#28963. According to a mailing list post discussing a similar problem [1], "OpenBSD provides the POSIX APIs by default", so we don't need this define anyway and can remove it. This fixes the BDB build problem as described in issue bitcoin#28963. See also google/flatbuffers@f87e75a for a similar fix for google's flatbuffer project. Tested on OpenBSD 7.4 with clang 13.0.0. Fixes bitcoin#28963. [1] https://www.mail-archive.com/tech@openbsd.org/msg63386.html ACKs for top commit: fanquake: ACK 0fbf051 Tree-SHA512: 02139e9081ed855e067bfba8c81b54c657417576e553cc1035a916ada9be049358f5e14d756d5f234c5226bd7e943f61c6ae8990c1b152f9125681b7b777c9b3
fa3a410 fuzz: Set -rss_limit_mb=8000 for generate as well (MarcoFalke) fa4e396 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] bitcoin#20789 (comment) [1] bitcoin#27888 (comment) 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 (bitcoin#20752 (comment)). 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 fa3a410 dergoegge: utACK fa3a410 brunoerg: light ACK fa3a410 Tree-SHA512: bfd04a76ca09aec612397bae5f3f263a608faa7087697169bd4c506c8195c4d2dd84ddc7fcd3ebbc75771eab618fad840af819114968ca3668fc730092376768
6fa61e3 doc: Fix Broken Links (Justin Dhillon) Pull request description: ### Summery Here is what I have fixed: http://voorloopnul.com/blog/a-python-netstat-in-less-than-100-lines-of-code/ --> https://web.archive.org/web/20190424172231/http://voorloopnul.com/blog/a-python-netstat-in-less-than-100-lines-of-code/ ### Support my work These links were found with [link-inspector](https://github.com/justindhillon/link-inspector). If you find this PR useful, give the repo a ⭐ ACKs for top commit: fjahr: ACK 6fa61e3 Tree-SHA512: ba83badfc8a89f33813801f749bcd7ad41d4c9c817ece76f3bb1b60f24c28e99cfccc485a0ba059ec2c1134e8ffb5fa37fdc9835e553229ee5b1167c9b2e8d1f
5d45552 Squashed 'src/crc32c/' changes from 0bac72c455..b60d2b7334 (fanquake) Pull request description: Update the crc32c subtree. Includes: * bitcoin-core/crc32c-subtree#6 Which fixes bitcoin#29178. ACKs for top commit: hebasto: ACK 359a8d9. theuni: ACK 359a8d9 dergoegge: ACK 359a8d9 Tree-SHA512: 2cec81a34ad26bbbc298aea5daffa41e56114d31cc2eb5fe486f46a77c3467bba22bdeca1c52ae97220e119d98818304272fc6337442af55282accabcd4c5833
312f338 fuzz: restrict fopencookie usage to Linux & FreeBSD (fanquake) Pull request description: Should fix the GCC compilation portion of bitcoin#29517 (comment). See also: https://www.gnu.org/software/gnulib/manual/html_node/fopencookie.html. ACKs for top commit: m3dwards: ACK bitcoin@312f338 TheCharlatan: utACK 312f338 Tree-SHA512: aa8ff20c3fa735415d05a93b8855877035c300f4d2dfd82f65fd9ed5b5c96ab619b4d84eef114ed0013dc5ff0800cb628ed3801e1efde0cfb0d426930d1673d5
…de when no tests are found to run 33268a8 test: exit with code 1 when no fn tests are found (Max Edwards) Pull request description: As discussed in the following PR comment: bitcoin#29535 (comment) Prevents the test_runner from exiting silently with code 0 when no tests were found which has recently happened after a GHA runner update such as in this run: https://github.com/bitcoin/bitcoin/actions/runs/8131828989/job/22239779585#step:27:63 ACKs for top commit: TheCharlatan: ACK 33268a8 theStack: lgtm ACK 33268a8 Tree-SHA512: d389e9f5e4da7ce1627fb2fad9b33baf0b04e75dbdbfc0dbf5f1e3e2b0ae1e79721476c5668476055b0f7de29723ed02c8d7e420081a555030cb784886e240fc
…etching harness list 738a537 [fuzz] Apply fuzz env (suppressions, etc.) when fetching harness list (dergoegge) Pull request description: The fuzz test runner does not add the UBSan suppressions when fetching the harness list. We can observe this in CI as lots of UBSan errors prior to the harnesses actually executing: https://api.cirrus-ci.com/v1/task/5678606140047360/logs/ci.log ``` + test/fuzz/test_runner.py -j10 -l DEBUG /ci_container_base/ci/scratch/qa-assets/fuzz_seed_corpus/ --empty_min_time=60 /usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/string_view:578:38: runtime error: unsigned integer overflow: 12 - 23 cannot be represented in type 'size_type' (aka 'unsigned long') SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/string_view:578:38 in /usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/string_view:578:33: runtime error: implicit conversion from type 'size_type' (aka 'unsigned long') of value 18446744073709551605 (64-bit, unsigned) to type 'const difference_type' (aka 'const long') changed the value to -11 (64-bit, signed) SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/string_view:578:33 in crypto/sha256.cpp:75:57: runtime error: left shift of 1359893119 by 26 places cannot be represented in type 'uint32_t' (aka 'unsigned int') SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior crypto/sha256.cpp:75:57 in crypto/sha256.cpp:75:79: runtime error: left shift of 1359893119 by 21 places cannot be represented in type 'uint32_t' (aka 'unsigned int') SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior crypto/sha256.cpp:75:79 in crypto/sha256.cpp:75:101: runtime error: left shift of 1359893119 by 7 places cannot be represented in type 'uint32_t' (aka 'unsigned int') SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior crypto/sha256.cpp:75:101 in crypto/sha256.cpp:82:47: runtime error: unsigned integer overflow: 2968370640 + 2483695512 cannot be represented in type 'uint32_t' (aka 'unsigned int') SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior crypto/sha256.cpp:82:47 in crypto/sha256.cpp:74:57: runtime error: left shift of 1779033703 by 30 places cannot be represented in type 'uint32_t' (aka 'unsigned int') SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior crypto/sha256.cpp:74:57 in crypto/sha256.cpp:74:79: runtime error: left shift of 1779033703 by 19 places cannot be represented in type 'uint32_t' (aka 'unsigned int') SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior crypto/sha256.cpp:74:79 in crypto/sha256.cpp:74:101: runtime error: left shift of 1779033703 by 10 places cannot be represented in type 'uint32_t' (aka 'unsigned int') SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior crypto/sha256.cpp:74:101 in crypto/sha256.cpp:83:29: runtime error: unsigned integer overflow: 3458249854 + 980412007 cannot be represented in type 'uint32_t' (aka 'unsigned int') SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior crypto/sha256.cpp:83:29 in crypto/sha256.cpp:82:21: runtime error: unsigned integer overflow: 528734635 + 4228187651 cannot be represented in type 'uint32_t' (aka 'unsigned int') SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior crypto/sha256.cpp:82:21 in crypto/sha256.cpp:84:7: runtime error: unsigned integer overflow: 1013904242 + 3720769133 cannot be represented in type 'uint32_t' (aka 'unsigned int') SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior crypto/sha256.cpp:84:7 in crypto/sha256.cpp:85:12: runtime error: unsigned integer overflow: 3720769133 + 2654153126 cannot be represented in type 'uint32_t' (aka 'unsigned int') SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior crypto/sha256.cpp:85:12 in crypto/sha256.cpp:82:33: runtime error: unsigned integer overflow: 4165002546 + 1259303586 cannot be represented in type 'uint32_t' (aka 'unsigned int') SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior crypto/sha256.cpp:82:33 in crypto/sha256.cpp:125:50: runtime error: unsigned integer overflow: 3835390401 + 1367343104 cannot be represented in type 'unsigned int' SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior crypto/sha256.cpp:125:50 in crypto/sha256.cpp:77:58: runtime error: left shift of 1367343104 by 15 places cannot be represented in type 'uint32_t' (aka 'unsigned int') ... ``` To fix this we simply apply the usual fuzz env variables (that apply the suppressions) when fetching the harness list as well. ACKs for top commit: ismaelsadeeq: Tested ACK 738a537 fanquake: ACK 738a537 Tree-SHA512: befebaeb4ee5f2eddca67fc6dc69e997c6a250ea54844e5e6e93d1f6a13be49364a3ace31eaa942b02dcf73612af29ec4ace86c9eb7567b92f6f5dc3ea14dc11
4f1753d doc: Wrap flags with code in developer-notes.md (spicyzboss) Pull request description: Before wrap code block <img width="1077" alt="image" src="https://github.com/bitcoin/bitcoin/assets/73651621/93f39960-311d-411d-acaf-dce69ad36ca0"> After wrap code block <img width="1073" alt="image" src="https://github.com/bitcoin/bitcoin/assets/73651621/fe6a5cd2-e981-45b1-a150-5a55fab81ae6"> ACKs for top commit: fanquake: ACK 4f1753d Tree-SHA512: 15a123f3a13c62d9cfdf62d5df351b8c2b3f9f666fba5a2722325600d802a29da61773ad32fb9f8483a915f59dd42731ba724b81b7874d39cb9627f0266b5713
… port 5b358cd 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 5b358cd epiccurious: re-ACK 5b358cd. achow101: ACK 5b358cd kristapsk: re-ACK 5b358cd vasild: ACK 5b358cd Tree-SHA512: 027245afa771c9295fff0bfd17c251dca4a9f4c739e5773922de3c030a65ef05d96291edcbdeeaa50ba3add61f75f28d8c00be503e03fc33d3491d1956fc549f
a3badf7 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 a3badf7 vasild: ACK a3badf7 brunoerg: utACK a3badf7 josibake: ACK bitcoin@a3badf7 BrandonOdiwuor: Code Review ACK a3badf7 Tree-SHA512: 1d4b4a3b2e2e28ab09f10b41b04b52b37f64e0d8a54e2306f37de0c3eb3299a7ad4ba225b9efa67057a75e90d008a17385c810a32c9b212d240be280c2dcf2e5
…versions 6f2f4a4 Reserve memory for ToLower/ToUpper conversions (Lőrinc) Pull request description: Similarly to bitcoin#29458, we're preallocating the result string based on the input string's length. The methods were already [covered by tests](https://github.com/bitcoin/bitcoin/blob/master/src/test/util_tests.cpp#L1250-L1276). ACKs for top commit: tdb3: ACK for 6f2f4a4 maflcko: lgtm ACK 6f2f4a4 achow101: ACK 6f2f4a4 Empact: Code Review ACK bitcoin@6f2f4a4 stickies-v: ACK 6f2f4a4 Tree-SHA512: e3ba7af77decdc73272d804c94fef0b11028a85f3c0ea1ed6386672611b1c35fce151f02e64f5bb5acb5ba506aaa54577719b07925b9cc745143cf5c7e5eb262
0831b54 test: simplify test_runner.py (tdb3) Pull request description: Implements the simplifications to test_runner.py proposed by sipa in PR bitcoin#23995. Remove the num_running variable as it can be implied by the length of the jobs list. Remove the i variable as it can be implied by the length of the test_results list. Instead of counting results to determine if finished, make the queue object itself responsible (by looking at running jobs and jobs left). ACKs for top commit: mzumsande: re-ACK 0831b54 davidgumberg: reACK bitcoin@0831b54 marcofleon: re-ACK 0831b54 Tree-SHA512: e5473e68d49cd779b29d97635329283ae7195412cb1e92461675715ca7eedb6519a1a93ba28d40ca6f015d270f7bcd3e77cef279d9cd655155ab7805b49638f1
…t test 626f8e3 fuzz: actually test garbage >64b in p2p transport test (Pieter Wuille) Pull request description: This fixes an oversight from bitcoin#28196: in the `p2p_transport_bidirectional_v2` fuzz test, when the desired garbage length is over 64 bytes, the code would actually use garbage length 0. Fix this. ACKs for top commit: instagibbs: ACK bitcoin@626f8e3 brunoerg: crACK 626f8e3 Tree-SHA512: f6346367adb10464b6c9d20aef43625531d2a4d8110887ad03214b8c1907b83560f2dd5b5415e2180a40b4cd276d51881b32b60c740471b5c6bb218aa19848d8
PastaPastaPasta
force-pushed
the
develop-trivial-2024-10-23-pr1
branch
from
054edd3 to
b70e091
Compare
|
Rebased on latest develop to fix merge ff warning |
knst
reviewed
Oct 24, 2024
| @@ -13,7 +13,6 @@ $(package)_cflags+=-Wno-error=implicit-function-declaration -Wno-error=format-se | |||
| $(package)_cxxflags+=-std=c++17 | |||
| $(package)_cppflags_freebsd=-D_XOPEN_SOURCE=600 -D__BSD_VISIBLE=1 | |||
| $(package)_cppflags_netbsd=-D_XOPEN_SOURCE=600 | |||
There was a problem hiding this comment.
should bump version here: doc/build-openbsd.md
see bitcoin#29481 as an useful follow-up.
There was a problem hiding this comment.
Our openbsd docs are missing backports, unable to merge this PR relatively trivially atm
UdjinM6
approved these changes
Oct 24, 2024
knst
approved these changes
Oct 24, 2024
| @@ -15,7 +15,8 @@ that runs all of the unit tests. The main source file for the test library is fo | |||
| Unit tests will be automatically compiled if dependencies were met in `./configure` | |||
| and tests weren't explicitly disabled. | |||
|
|
|||
| After configuring, they can be run with `make check`. | |||
| After configuring, they can be run with `make check`, which includes unit tests from | |||
| subtrees, or `make && make -C src check-unit` for just the unit tests. | |||
There was a problem hiding this comment.
finally we have it! =) Unit tests run QUADRUPLE fast now!
Used to be:
$ time make check -j20
real 2m1.358s
user 4m53.064s
sys 0m19.916s
became:
$ time make -C src check-unit -j20
real 0m34.330s
user 2m37.573s
sys 0m17.590s
There was a problem hiding this comment.
Ahh, interesting!
There was a problem hiding this comment.
maybe actually add test_dash_qt_check to README.md ? but with or without it's fine
-subtrees, or `make && make -C src check-unit` for just the unit tests.
+subtrees, or `make && make -C src check-unit test_dash_qt_check` for just the unit tests.
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
Batch of trivial backports
What was done?
See commits
How Has This Been Tested?
built locally; large combined merge passed tests locally
Breaking Changes
Should be none
Checklist:
Go over all the following points, and put an
xin all the boxes that apply.