Skip to content
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

fuzz: reduce keypool size in scriptpubkeyman target #30494

Merged
merged 1 commit into from
Jul 22, 2024
Merged

fuzz: reduce keypool size in scriptpubkeyman target #30494

merged 1 commit into from
Jul 22, 2024

Conversation

brunoerg
Copy link
Contributor

@brunoerg brunoerg commented Jul 20, 2024 •
edited
Loading

Fixes #30476

This PR reduces keypool size in scriptpubkeyman fuzz target to avoid spend a lot of time in TopUp (which is obviously called by many spkm functions).

For reference:

This PR:

INFO: Running with entropic power schedule (0xFF, 100).
INFO: Seed: 1845055748
INFO: Loaded 1 modules   (1225616 inline 8-bit counters): 1225616 [0x106346fe0, 0x106472370), 
INFO: Loaded 1 PC tables (1225616 PCs): 1225616 [0x106472370,0x107725c70), 
./src/test/fuzz/fuzz: Running 1 inputs 10 time(s) each.
Running: ./qa-assets/fuzz_seed_corpus/scriptpubkeyman/c9b8928cecb1edc192fb2d5816b4b7878cdfcf50
Executed ./qa-assets/fuzz_seed_corpus/scriptpubkeyman/c9b8928cecb1edc192fb2d5816b4b7878cdfcf50 in 250 ms

Master:

INFO: Running with entropic power schedule (0xFF, 100).
INFO: Seed: 2004906948
INFO: Loaded 1 modules   (1225603 inline 8-bit counters): 1225603 [0x104196f80, 0x1042c2303), 
INFO: Loaded 1 PC tables (1225603 PCs): 1225603 [0x1042c2308,0x105575b38), 
./src/test/fuzz/fuzz: Running 1 inputs 10 time(s) each.
Running: ./qa-assets/fuzz_seed_corpus/scriptpubkeyman/c9b8928cecb1edc192fb2d5816b4b7878cdfcf50
Executed ./qa-assets/fuzz_seed_corpus/scriptpubkeyman/c9b8928cecb1edc192fb2d5816b4b7878cdfcf50 in 21016 ms

@DrahtBot
Copy link
Contributor

DrahtBot commented Jul 20, 2024 •
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 maflcko, dergoegge
Concept ACK Sjors

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #30221 (wallet: Ensure best block matches wallet scan state by achow101)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@DrahtBot DrahtBot added the Tests label Jul 20, 2024
@brunoerg brunoerg changed the title fuzz: reduce keypool size in scriptpubkeyman target fuzz: reduce keypool size in scriptpubkeyman target Jul 20, 2024
@brunoerg brunoerg mentioned this pull request Jul 20, 2024
Closed
@DrahtBot DrahtBot mentioned this pull request Jul 21, 2024
Open
@maflcko
Copy link
Member

maflcko commented Jul 22, 2024 •
edited
Loading

review ACK dcb4ec9

This implements my suggestion from #30476 (comment), but I haven't benchmarked whether it works.

Edit: According to the CI timing output, it works.

https://cirrus-ci.com/task/6392536002985984?logs=ci#L6480

https://cirrus-ci.com/task/6473792925466624?logs=ci#L7201

@Sjors
Copy link
Member

Sjors commented Jul 22, 2024

Concept ACK

@DrahtBot DrahtBot requested a review from Sjors July 22, 2024 13:07
@fanquake fanquake requested a review from dergoegge July 22, 2024 16:57
dergoegge
dergoegge approved these changes Jul 22, 2024
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.

utACK dcb4ec9

@fanquake fanquake merged commit 8754d05 into bitcoin:master Jul 22, 2024
16 checks passed
@brunoerg brunoerg deleted the 2024-07-fuzz-spkm-fix branch July 22, 2024 17:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dergoegge dergoegge dergoegge approved these changes

@Sjors Sjors Awaiting requested review from Sjors

Assignees
No one assigned
Labels
Tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

scriptpubkeyman fuzz target TopUp is slow
6 participants
@brunoerg @DrahtBot @maflcko @Sjors @dergoegge @fanquake