Skip to content

fuzz: Avoid utxo_total_supply timeout (take 2) #28789

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Nov 7, 2023
Merged

fuzz: Avoid utxo_total_supply timeout (take 2) #28789

merged 1 commit into from
Nov 7, 2023

Conversation

maflcko
Copy link
Member

@maflcko maflcko commented Nov 3, 2023

Looks like this still may take a long time to run large fuzz inputs. Thus, reduce it further, but still allow it to catch the regression, if re-introduced:

diff --git a/src/consensus/tx_check.cpp b/src/consensus/tx_check.cpp
index f949655909..4bdd15c5ee 100644
--- a/src/consensus/tx_check.cpp
+++ b/src/consensus/tx_check.cpp
@@ -40,7 +40,7 @@ bool CheckTransaction(const CTransaction& tx, TxValidationState& state)
     std::set<COutPoint> vInOutPoints;
     for (const auto& txin : tx.vin) {
         if (!vInOutPoints.insert(txin.prevout).second)
-            return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-txns-inputs-duplicate");
+            {}//return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-txns-inputs-duplicate");
     }
 
     if (tx.IsCoinBase())

This is the second take, see #27780. If in the future it still times out, I think the fuzz test can just be removed.

Example input:

JREROy5pcnAgQyw7IC4ODg4ODg4ODg4O0dEODg4ODg4ZDg4ODg4ODg4ODg7RDg4ODg4ODg4O0dEODg4ODg4ODg4ODg7R0Q4ODg4ODg4ODtHRDg4ODtHR0dEODg4O0dEODg7R0Q4ODg4ODg4ODtHRDg4ODg4ODg4ODg4O0dEODg4ODg4ODg7R0Q4ODg7R0Q4O0dEODg4ODg4ODg4ODg7R0Q4ODg4ODtHRDg4ODtHR

@DrahtBot
Copy link
Contributor

DrahtBot commented Nov 3, 2023 •
edited
Loading

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

Type Reviewers
ACK dergoegge, brunoerg

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

@DrahtBot DrahtBot added the Tests label Nov 3, 2023
dergoegge
dergoegge approved these changes Nov 6, 2023
View reviewed changes
Copy link
Member

@dergoegge dergoegge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK fa7ba92

brunoerg
brunoerg approved these changes Nov 7, 2023
View reviewed changes
Copy link
Contributor

@brunoerg brunoerg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK fa7ba92

@fanquake fanquake merged commit 2b3f43b into bitcoin:master Nov 7, 2023
@maflcko maflcko deleted the 2311-fuzz-timeout- branch November 7, 2023 12:27
@bitcoin bitcoin locked and limited conversation to collaborators Nov 6, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
2 more reviewers

@dergoegge dergoegge dergoegge approved these changes

@brunoerg brunoerg brunoerg approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
Tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@maflcko @DrahtBot @dergoegge @brunoerg @fanquake