-
Notifications
You must be signed in to change notification settings - Fork 35.5k
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
fuzz: Faster wallet_notifications target #28933
Conversation
This avoids having to include both headers when assert and Assert are used at the same time.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code CoverageFor detailed information about the code coverage, see the test coverage report. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. |
fa092cc
to
fa4fc99
Compare
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.
Code review ACK fa4fc99
With the mocked database this shouldn't be going to disk at all now, right?
Yeah, I guess so. (A datadir is still created, though) |
Is that necessary or just a side effect from using |
Concept ACK will review soon! |
Both. For one, |
Empty dirs aren't of size zero and #28811 would still apply (given more timeouts/crashes). It's a bit of a shame if it's always empty to still create it, but not an issue for this PR in any case. |
Are there any timeouts left at this point? If yes, it may be good to report them. |
We'll see I guess, initial exec/s looked good but they have now deteriorated to 3 exec/s (per core). No timeouts so far though and CPU utilization is at 100% on all cores, so this is a definite improvement. |
Is there any benefit about using this |
Which function do you mean?
|
Sorry, edited that. It's |
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.
ACK fa4fc99
I've run into timeouts. e.g. this input takes >5s to execute on my machine.
Afl++ stability also dropped to ~70% which suggests that there is some non-determinism in the target. |
This is expected, because the wallet internally uses system random, similar to the p2p fuzz targets. This should be fixed in a follow-up for all fuzz targets. |
fa4fc99
to
fa15861
Compare
Thanks, fixed by reducing the number of new addresses that are generated each run |
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.
reACK fa15861
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.
reACK fa15861
Avoid read/write from storage to speed the target up.