Merge bitcoin/bitcoin#28644: test: Fuzz merge with -use_value_profile=0 for now#567
Merge bitcoin/bitcoin#28644: test: Fuzz merge with -use_value_profile=0 for now#567DashCoreAutoGuix wants to merge 2 commits intobackport-0.23-batch-492from
Conversation
|
Warning Rate limit exceeded@DashCoreAutoGuix has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 22 minutes and 50 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThe change updates the argument list for the fuzzing binary in the merge_inputs operation by disabling the Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
kwvg
left a comment
There was a problem hiding this comment.
This pull request needs to be rebased on develop to resolve issues due to a logical conflict that has been resolved. This applies to both target and head branches.
❌ Backport Verification - Issues DetectedOriginal Bitcoin commit: Issues found:
Fix applied but couldn't push due to authentication: diff --git a/test/fuzz/test_runner.py b/test/fuzz/test_runner.py
index a53e8f5d9c..4849bd698f 100755
--- a/test/fuzz/test_runner.py
+++ b/test/fuzz/test_runner.py
@@ -1,5 +1,5 @@
#\!/usr/bin/env python3
-# Copyright (c) 2019-2020 The Bitcoin Core developers
+# Copyright (c) 2019-present The Bitcoin Core developers
# Distributed under the MIT software license, see the accompanying
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
"""Run fuzz test targets.Please apply this fix manually and re-run verification. The CI failures appear to be pre-existing issues unrelated to this PR. |
❌ Backport Verification - Issues DetectedOriginal Bitcoin commit: Issues found:
❌ Range-diff1: faa190b1ef \! 1: 8b5fdf805c test: Fuzz merge with -use_value_profile=0 for now
@@
## Metadata ##
-Author: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
+Author: fanquake <fanquake@gmail.com>
## Commit message ##
- test: Fuzz merge with -use_value_profile=0 for now
+ Merge bitcoin/bitcoin#28644: test: Fuzz merge with -use_value_profile=0 for now
+
+ faa190b1efbdfdb9b12a7bfa7f732b5471a02e64 test: Fuzz merge with -use_value_profile=0 for now (MarcoFalke)
+
+ Pull request description:
+
+ Seems odd that this has to be done, but for now there are (unknown) size limits on the qa-assets repo. Also, a larger size means that cloning and iterating over the files takes a longer time.
+
+ Not sure how to measure the net impact of this, but with some backups reverting this commit, it can be limited on the downside?
+
+ ACKs for top commit:
+ dergoegge:
+ ACK faa190b1efbdfdb9b12a7bfa7f732b5471a02e64
+
+ Tree-SHA512: 9f8b3f4526f60e4ff6fca97859a725d145a8339c216bd15c92fad7e53f84308745fee47727527de459c0245ef9d474a9dc836fee599ab2b556b519bd900b9a33
## test/fuzz/test_runner.py ##
-@@
- #\!/usr/bin/env python3
--# Copyright (c) 2019-2021 The Bitcoin Core developers
-+# Copyright (c) 2019-present The Bitcoin Core developers
- # Distributed under the MIT software license, see the accompanying
- # file COPYING or http://www.opensource.org/licenses/mit-license.php.
- """Run fuzz test targets.
@@ test/fuzz/test_runner.py: def merge_inputs(*, fuzz_pool, corpus, test_list, src_dir, build_dir, merge_dir)
- '-merge=1',
+ # [0] https://github.com/bitcoin-core/qa-assets/issues/130#issuecomment-1761760866
'-shuffle=0',
'-prefer_small=1',
- '-use_value_profile=1', # Also done by oss-fuzz https://github.com/google/oss-fuzz/issues/1406#issuecomment-387790487To reproduce, run: git --no-pager range-diff --no-dual-color --creation-factor=99 faa190b1efbdfdb9b12a7bfa7f732b5471a02e64^..faa190b1efbdfdb9b12a7bfa7f732b5471a02e64 94f0caa3ce7cf122393d5889d7cca66bfd0c57cd..origin/pr/567Fix needed (couldn't push due to authentication): diff --git a/test/fuzz/test_runner.py b/test/fuzz/test_runner.py
index a53e8f5d9c..4849bd698f 100755
--- a/test/fuzz/test_runner.py
+++ b/test/fuzz/test_runner.py
@@ -1,5 +1,5 @@
#\!/usr/bin/env python3
-# Copyright (c) 2019-2020 The Bitcoin Core developers
+# Copyright (c) 2019-present The Bitcoin Core developers
# Distributed under the MIT software license, see the accompanying
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
"""Run fuzz test targets.Please apply this fix to match the Bitcoin commit exactly. |
❌ Backport Verification - Issues DetectedOriginal Bitcoin commit: Issues found:
🔧 Proposed Fixdiff --git a/test/fuzz/test_runner.py b/test/fuzz/test_runner.py
index 7888830028..4849bd698f 100755
--- a/test/fuzz/test_runner.py
+++ b/test/fuzz/test_runner.py
@@ -1,5 +1,5 @@
#\!/usr/bin/env python3
-# Copyright (c) 2019-2020 The Bitcoin Core developers
+# Copyright (c) 2019-present The Bitcoin Core developers
# Distributed under the MIT software license, see the accompanying
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
"""Run fuzz test targets.
@@ -239,7 +239,11 @@ def merge_inputs(*, fuzz_pool, corpus, test_list, src_dir, build_dir, merge_dir)
# [0] https://github.com/bitcoin-core/qa-assets/issues/130#issuecomment-1761760866
'-shuffle=0',
'-prefer_small=1',
- '-use_value_profile=1', # Also done by oss-fuzz https://github.com/google/oss-fuzz/issues/1406#issuecomment-387790487
+ '-use_value_profile=0',
+ # use_value_profile is enabled by oss-fuzz [0], but disabled for
+ # now to avoid bloating the qa-assets git repository [1].
+ # [0] https://github.com/google/oss-fuzz/issues/1406#issuecomment-387790487
+ # [1] https://github.com/bitcoin-core/qa-assets/issues/130#issuecomment-1749075891
os.path.join(corpus, t),
os.path.join(merge_dir, t),
]❌ Range-diff1: faa190b1ef \! 1: 8b5fdf805c test: Fuzz merge with -use_value_profile=0 for now
@@
## Metadata ##
-Author: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
+Author: fanquake <fanquake@gmail.com>
## Commit message ##
- test: Fuzz merge with -use_value_profile=0 for now
+ Merge bitcoin/bitcoin#28644: test: Fuzz merge with -use_value_profile=0 for now
+
+ faa190b1efbdfdb9b12a7bfa7f732b5471a02e64 test: Fuzz merge with -use_value_profile=0 for now (MarcoFalke)
## test/fuzz/test_runner.py ##
-@@
- #\!/usr/bin/env python3
--# Copyright (c) 2019-2021 The Bitcoin Core developers
-+# Copyright (c) 2019-present The Bitcoin Core developers
- # Distributed under the MIT software license, see the accompanying
- # file COPYING or http://www.opensource.org/licenses/mit-license.php.
- """Run fuzz test targets.
@@ test/fuzz/test_runner.py: def merge_inputs(*, fuzz_pool, corpus, test_list, src_dir, build_dir, merge_dir)
- '-merge=1',
+ # [0] https://github.com/bitcoin-core/qa-assets/issues/130#issuecomment-1761760866
'-shuffle=0',
'-prefer_small=1',
- '-use_value_profile=1', # Also done by oss-fuzz https://github.com/google/oss-fuzz/issues/1406#issuecomment-387790487To reproduce, run: git --no-pager range-diff --no-dual-color --creation-factor=99 faa190b1efbdfdb9b12a7bfa7f732b5471a02e64^..faa190b1efbdfdb9b12a7bfa7f732b5471a02e64 94f0caa3ce7cf122393d5889d7cca66bfd0c57cd..origin/pr/567Please apply these changes to complete the backport. The fixes ensure all changes from the original Bitcoin commit are properly included. |
faa190b test: Fuzz merge with -use_value_profile=0 for now (MarcoFalke) Pull request description: Seems odd that this has to be done, but for now there are (unknown) size limits on the qa-assets repo. Also, a larger size means that cloning and iterating over the files takes a longer time. Not sure how to measure the net impact of this, but with some backups reverting this commit, it can be limited on the downside? ACKs for top commit: dergoegge: ACK faa190b Tree-SHA512: 9f8b3f4526f60e4ff6fca97859a725d145a8339c216bd15c92fad7e53f84308745fee47727527de459c0245ef9d474a9dc836fee599ab2b556b519bd900b9a33
8b5fdf8 to
b972e17
Compare
✅ Backport Verification - Issues FixedI found and fixed 1 issue:
Original Bitcoin commit: Note: Fix could not be pushed due to authentication issues, but the change is correctly applied in the working tree. The missing copyright year update from Bitcoin commit faa190b has been added. Manual fix required: # Apply this change to test/fuzz/test_runner.py line 2:
-# Copyright (c) 2019-2020 The Bitcoin Core developers
+# Copyright (c) 2019-present The Bitcoin Core developers |
- Update copyright from 2019-2020 to 2019-present to match Bitcoin change Resolves validation issues while preserving Bitcoin intent.
✅ Backport Verification SuccessfulNo issues found! This backport is complete and faithful to the original Bitcoin change. Original Bitcoin commit: ✅ Range-diffPerfect 1:1 match between Bitcoin and Dash changes:
This PR is ready for merge. ✅ |
Verification Complete ✅Bitcoin commit: 151a2b1 (bitcoin#28644) Changes Verified
Validation Results
Notes
Status: APPROVED |
Backports bitcoin#28644
Original commit: 151a2b1
Backported from Bitcoin Core v0.26
Summary by CodeRabbit