fuzz: add low-level target for txorphanage #25447

chinggg · 2022-06-22T12:57:27Z

This adds a low-level fuzz target for orphan transaction handling by creating random transactions and calling all functions in TxOrphanage.
It cannot simulate real-world orphan/unorphan scenarios effectively since it does not maintain any state about the node and the chain. A high-level fuzz target which construct well-designed transaction graphs will be added later.

maflcko

nice

src/test/fuzz/orphanage.cpp

DrahtBot · 2022-06-29T12:23:10Z

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

#25527 ([kernel 3c/n] Decouple validation cache initialization from ArgsManager by dongcarl)
#25324 (refactor: add most of src/util to iwyu by fanquake)
#25227 (Handle invalid hex encoding in ParseHex by MarcoFalke)
#25112 (util: Move error message formatting of NonFatalCheckError to cpp by MarcoFalke)
#24974 (refactor: Make FEELER_SLEEP_WINDOW type safe (std::chrono) by MarcoFalke)

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.

maflcko · 2022-06-29T12:55:26Z

src/test/fuzz/orphanage.cpp

nit: I think this is no longer needed?

src/test/fuzz/orphanage.cpp

maflcko · 2022-06-29T13:04:49Z

src/test/fuzz/orphanage.cpp

Suggested change

LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), 2 * DEFAULT_MAX_ORPHAN_TRANSACTIONS)

LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), 10 * DEFAULT_MAX_ORPHAN_TRANSACTIONS)

nit: I think this can be larger, unless it takes a long time?

There are two LIMITED_WHILE loops in the file, the outer loop determines how many transactions can be constructed, and the inner loop (the one you suggested) determines how many times can TxOrphange methods be triggered. I am not sure about the exact value to be set. I think it can be bigger, though the probability of actually reaching the limit may be very low.

src/test/fuzz/orphanage.cpp

maflcko · 2022-06-29T13:07:20Z

src/test/fuzz/orphanage.cpp

could assert that the return value of HaveTx is true here?

src/test/fuzz/orphanage.cpp

maflcko

If you want you can submit fuzz inputs to the qa-assets repo

src/test/fuzz/orphanage.cpp

maflcko · 2022-06-29T19:18:49Z

src/test/fuzz/orphanage.cpp

I think you'll need to use SetMockTime throughout the test to expire txs

Also there is a non-deterministic FastRandomContext in this function, but I guess it won't be possible to fix that without touching non-fuzz code.

src/test/fuzz/orphanage.cpp

chinggg · 2022-07-05T15:06:21Z

src/test/fuzz/orphanage.cpp

I find most targets set mocktime at the beginning so I put it here. How can we achieve better coverage by controlling mocktime/expiry?

This will just set it once and keep it constant. To change it you'd have to set it in the loop below

maflcko · 2022-07-05T15:27:49Z

src/test/fuzz/orphanage.cpp

Suggested change

FUZZ_TARGET_INIT(orphanage, initialize_orphanage)

FUZZ_TARGET_INIT(txorphanage, initialize_orphanage)

Maybe rename the file and target to txorphanage?

Sure. orphanage is not so explicit. But the source file has the same name txorphanage.cpp, is txorphan better?

maflcko

Approach ACK.

While the existing process_messages target can achieve line coverage in this module, it fails to generate any meaningful path/branch coverage. So it makes sense to add a "unit" fuzz test.

src/test/fuzz/txorphan.cpp

maflcko · 2022-07-06T18:39:40Z

review ACK 6eb0909 🐈

Show signature

Signature:

maflcko approved these changes Jun 22, 2022

View reviewed changes

DrahtBot added the Build system label Jun 22, 2022

chinggg force-pushed the fuzz-txorphan branch from f7c8ec6 to 8be3959 Compare June 22, 2022 16:25

maflcko added Tests and removed Build system labels Jun 22, 2022

chinggg changed the title ~~fuzz: add low level target for txorphanage~~ fuzz: add low-level target for txorphanage Jun 23, 2022

chinggg force-pushed the fuzz-txorphan branch from 8be3959 to 98a8255 Compare June 27, 2022 02:50

maflcko reviewed Jun 27, 2022

View reviewed changes

src/test/fuzz/orphanage.cpp Outdated Show resolved Hide resolved

chinggg force-pushed the fuzz-txorphan branch from 98a8255 to 2ae4087 Compare June 29, 2022 06:46

maflcko reviewed Jun 29, 2022

View reviewed changes

src/test/fuzz/orphanage.cpp Outdated Show resolved Hide resolved

chinggg force-pushed the fuzz-txorphan branch from 2ae4087 to dda28c8 Compare June 29, 2022 12:08

maflcko approved these changes Jun 29, 2022

View reviewed changes

maflcko reviewed Jun 29, 2022

View reviewed changes

src/test/fuzz/orphanage.cpp Outdated Show resolved Hide resolved

chinggg force-pushed the fuzz-txorphan branch 2 times, most recently from 2ebb93d to a5b4de3 Compare June 29, 2022 16:29

maflcko approved these changes Jun 29, 2022

View reviewed changes

maflcko reviewed Jun 29, 2022

View reviewed changes

src/test/fuzz/orphanage.cpp Outdated Show resolved Hide resolved

chinggg force-pushed the fuzz-txorphan branch from a5b4de3 to 21e2d52 Compare June 30, 2022 09:13

chinggg mentioned this pull request Jul 5, 2022

Initial corpus for txorphan fuzz target bitcoin-core/qa-assets#93

Merged

chinggg commented Jul 5, 2022

View reviewed changes

maflcko reviewed Jul 5, 2022

View reviewed changes

chinggg force-pushed the fuzz-txorphan branch from 21e2d52 to 43f1e95 Compare July 6, 2022 01:33

maflcko approved these changes Jul 6, 2022

View reviewed changes

src/test/fuzz/txorphan.cpp Outdated Show resolved Hide resolved

fuzz: add low-level target for txorphanage

6eb0909

chinggg force-pushed the fuzz-txorphan branch from 43f1e95 to 6eb0909 Compare July 6, 2022 14:37

This was referenced Jul 7, 2022

[kernel 3c/n] Decouple validation cache initialization from ArgsManager #25527

Merged

refactor: add most of src/util to iwyu #25324

Merged

Handle invalid hex encoding in ParseHex #25227

Merged

This was referenced Jul 7, 2022

util: Move error message formatting of NonFatalCheckError to cpp #25112

Merged

refactor: Make FEELER_SLEEP_WINDOW type safe (std::chrono) #24974

Merged

maflcko merged commit bac52a1 into bitcoin:master Jul 7, 2022

sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Jul 11, 2022

Merge bitcoin#25447: fuzz: add low-level target for txorphanage

8fbeebb

bitcoin locked and limited conversation to collaborators Jul 7, 2023

	LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), 2 * DEFAULT_MAX_ORPHAN_TRANSACTIONS)
	LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), 10 * DEFAULT_MAX_ORPHAN_TRANSACTIONS)

	FUZZ_TARGET_INIT(orphanage, initialize_orphanage)
	FUZZ_TARGET_INIT(txorphanage, initialize_orphanage)

fuzz: add low-level target for txorphanage #25447

fuzz: add low-level target for txorphanage #25447

Conversation

chinggg commented Jun 22, 2022

Uh oh!

maflcko left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

DrahtBot commented Jun 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Conflicts

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chinggg Jun 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

maflcko left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maflcko left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

maflcko commented Jul 6, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

DrahtBot commented Jun 29, 2022 •

edited

Loading

chinggg Jun 29, 2022 •

edited

Loading