Skip to content

fuzz: Mock CMainSignals in process_message(s) targets#26872

Closed
dergoegge wants to merge 3 commits intobitcoin:masterfrom
dergoegge:2023-01-fuzz-mock-signals
Closed

fuzz: Mock CMainSignals in process_message(s) targets#26872
dergoegge wants to merge 3 commits intobitcoin:masterfrom
dergoegge:2023-01-fuzz-mock-signals

Conversation

@dergoegge
Copy link
Copy Markdown
Member

Mocking CMainSignals gives us the ability to control when the CValidationInterface callbacks are called on PeerManager in the process_message(s) targets. It thereby makes these targets more deterministic/reproducable and slightly faster (per eyeball measurements 5%-10% more execs/s on my machine).

Mostly looking for conceptual review on this because there is an argument to be made about fuzzing with the real CMainSignals in combination with TSan, so maybe preserving that behavior in another target could also make sense.

@dergoegge
Copy link
Copy Markdown
Member Author

@MarcoFalke curious about your opinion on this (maybe this has been discussed before?)

@DrahtBot
Copy link
Copy Markdown
Contributor

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

Reviews

See the guideline for information on the review process.
A summary of reviews will appear here.

@maflcko
Copy link
Copy Markdown
Member

maflcko commented Jan 11, 2023

For those targets, none of them should be ever hit, practically speaking. It might be better to add a new validationinterface fuzz target

@dergoegge
Copy link
Copy Markdown
Member Author

For those targets, none of them should be ever hit, practically speaking.

Ugh, I assumed it's possible because we add those easily spendable coinbase outputs and call SyncWithValidationInterfaceQueue, but yeah looking at the coverage there is no coverage for any of the validation callbacks. The SyncWithValidationInterfaceQueue does slow down the targets, so it's a little annoying to have that when it's practically never needed.

I have been working on targets that more specifically fuzz certain protocol flows (e.g. the version handshake, block/tx relay). These targets mostly submit valid headers/blocks/txs to net_processing, so I guess what I am suggesting here makes more sense for them.

@dergoegge dergoegge closed this Jan 11, 2023
@bitcoin bitcoin locked and limited conversation to collaborators Jan 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants