test: add deserialization fuzz targets for Dash-specific types#7161
test: add deserialization fuzz targets for Dash-specific types#7161thepastaclaw wants to merge 5 commits intodashpay:developfrom
Conversation
Add 42 new fuzz targets covering all Dash-specific serializable types that currently have zero fuzzing coverage (~24,000 lines of code). Targets cover: - evo/: ProRegTx, ProUpServTx, ProUpRegTx, ProUpRevTx, CAssetLockPayload, CAssetUnlockPayload, CCbTx, CCreditPool, CDeterministicMN, CDeterministicMNState, CDeterministicMNStateDiff, CSimplifiedMNListEntry, CSimplifiedMNListDiff, CMNAuth, MNHFTx - llmq/: CFinalCommitment, CDKGComplaint, CDKGJustification, CDKGPrematureCommitment, CRecoveredSig, CSigShare, CSigSesAnn, CSigSharesInv, CBatchedSigShares, CQuorumDataRequest, CGetQuorumRotationInfo, CQuorumSnapshot - governance/: Governance::Object, CGovernanceObject, CGovernanceVote, vote_instance_t, vote_rec_t, CGovernanceObjectVoteFile - bls/: CBLSIESEncryptedBlob, CBLSIESMultiRecipientBlobs - coinjoin/: CCoinJoinStatusUpdate, CCoinJoinAccept, CCoinJoinEntry, CCoinJoinQueue, CCoinJoinBroadcastTx Uses the same pattern as the existing deserialize.cpp for Bitcoin Core types (FUZZ_TARGET macro with CDataStream deserialization).
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a new fuzz test source file src/test/fuzz/deserialize_dash.cpp, registers it in the test/fuzz build list (Makefile include), and adds the path to test/util/data/non-backported.txt. The new file defines initialize_deserialize_dash(), dash_invalid_fuzzing_input_exception, a templated DashDeserializeFromFuzzingInput helper that builds a CDataStream (optional protocol handling) and deserializes into a target type, and a FUZZ_TARGET_DASH_DESERIALIZE wrapper. It declares many fuzz targets covering Dash evo, llmq, governance, BLS, and coinjoin types. No production public APIs are changed. Sequence Diagram(s)sequenceDiagram
participant Fuzzer
participant Init as Initialization
participant Harness as FUZZ_TARGET wrapper
participant Parser as DashDeserializeFromFuzzingInput
participant Stream as CDataStream
participant Type as TargetType
participant Except as dash_invalid_fuzzing_input_exception
Fuzzer->>Init: call initialize_deserialize_dash()
Fuzzer->>Harness: deliver raw fuzz buffer
Harness->>Parser: call DashDeserializeFromFuzzingInput(buffer, optional_proto)
Parser->>Stream: construct CDataStream from buffer
Stream->>Type: attempt to deserialize into target object
alt deserialize succeeds
Type-->>Harness: return deserialized object
else deserialize/IO/protocol error
Stream-->>Except: throw dash_invalid_fuzzing_input_exception
Except-->>Harness: caught and filtered
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/test/fuzz/deserialize_dash.cpp (2)
61-97: Movedash_invalid_fuzzing_input_exceptiondefinition above the macro that references it.The macro at line 66 catches
dash_invalid_fuzzing_input_exception, but the type isn't defined until line 72. This compiles correctly (macro expansion happens at call sites after line 97), but it's confusing to read top-to-bottom. Moving the anonymous namespace block before the macro definition would make the dependency order clear.♻️ Proposed reordering
+namespace { + +struct dash_invalid_fuzzing_input_exception : public std::exception { +}; + +template <typename T> +void DashDeserializeFromFuzzingInput(FuzzBufferType buffer, T& obj, const std::optional<int> protocol_version = std::nullopt, const int ser_type = SER_NETWORK) +{ + // ... (move full implementation here) +} + +} // namespace + `#define` FUZZ_TARGET_DASH_DESERIALIZE(name, code) \ FUZZ_TARGET(name, .init = initialize_deserialize_dash) \ { \ try { \ code \ } catch (const dash_invalid_fuzzing_input_exception&) { \ } \ } - -namespace { - -struct dash_invalid_fuzzing_input_exception : public std::exception { -}; - -template <typename T> -void DashDeserializeFromFuzzingInput(...) -{ ... } - -} // namespace🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/fuzz/deserialize_dash.cpp` around lines 61 - 97, Move the dash_invalid_fuzzing_input_exception type definition (and the anonymous namespace containing it and DashDeserializeFromFuzzingInput) so it appears before the FUZZ_TARGET_DASH_DESERIALIZE macro; specifically, ensure the dash_invalid_fuzzing_input_exception struct is defined prior to the FUZZ_TARGET_DASH_DESERIALIZE macro so the macro's catch(...) that references dash_invalid_fuzzing_input_exception sees the type declaration when reading top-to-bottom (adjust file order so DashDeserializeFromFuzzingInput and the exception live above the FUZZ_TARGET_DASH_DESERIALIZE macro).
49-49: Prefer<cstdint>over<stdint.h>in C++20 code.♻️ Proposed fix
-#include <stdint.h> +#include <cstdint>As per coding guidelines: "Dash Core implementation must be written in C++20."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/fuzz/deserialize_dash.cpp` at line 49, Replace the C header include '#include <stdint.h>' with the C++ header '<cstdint>' to comply with C++20 coding guidelines; update the include in deserialize_dash.cpp by removing the old include directive and adding the corresponding '<cstdint>' include so code uses the std::intXX types from the std namespace where applicable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/test/fuzz/deserialize_dash.cpp`:
- Around line 1-59: The new Dash-specific fuzz target file
src/test/fuzz/deserialize_dash.cpp was added but not listed in the repository's
non-backported list; update test/util/data/non-backported.txt to include the
exact relative path "src/test/fuzz/deserialize_dash.cpp" (one entry per line),
save the file, and commit so clang-format enforcement recognizes the new
non-backported file; verify by running the repo's style/format check or CI that
references non-backported.txt.
---
Nitpick comments:
In `@src/test/fuzz/deserialize_dash.cpp`:
- Around line 61-97: Move the dash_invalid_fuzzing_input_exception type
definition (and the anonymous namespace containing it and
DashDeserializeFromFuzzingInput) so it appears before the
FUZZ_TARGET_DASH_DESERIALIZE macro; specifically, ensure the
dash_invalid_fuzzing_input_exception struct is defined prior to the
FUZZ_TARGET_DASH_DESERIALIZE macro so the macro's catch(...) that references
dash_invalid_fuzzing_input_exception sees the type declaration when reading
top-to-bottom (adjust file order so DashDeserializeFromFuzzingInput and the
exception live above the FUZZ_TARGET_DASH_DESERIALIZE macro).
- Line 49: Replace the C header include '#include <stdint.h>' with the C++
header '<cstdint>' to comply with C++20 coding guidelines; update the include in
deserialize_dash.cpp by removing the old include directive and adding the
corresponding '<cstdint>' include so code uses the std::intXX types from the std
namespace where applicable.
CDeterministicMN has a deleted default constructor (must specify internalId). Construct with internalId=0 which gets overwritten during deserialization.
There was a problem hiding this comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/test/fuzz/deserialize_dash.cpp`:
- Around line 1-59: This new Dash-specific fuzz source deserialize_dash.cpp
needs to be registered in the repository's non-backported C++ file list: add the
filename "deserialize_dash.cpp" to non-backported.txt (the list of
non-backported .cpp/.h files) so clang-format checks treat it as a Dash-only
addition; ensure the entry is just the C++ filename and follows the existing
file formatting in non-backported.txt.
- Add deserialize_dash.cpp to non-backported.txt for clang-format enforcement - Reorder exception type and helper function above macro definition for readability
|
Addressed CodeRabbit feedback in 92341bf:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/test/fuzz/deserialize_dash.cpp (1)
49-49: Prefer<cstdint>over the C-style<stdint.h>.
<stdint.h>is not used directly in this file —FuzzBufferTypeand the byte-span types are already brought in via<test/fuzz/fuzz.h>. If the include is kept forintinds >> version, it is unnecessary sinceintis a fundamental type. Either remove it or replace with<cstdint>per C++20 convention.♻️ Proposed change
-#include <stdint.h> +// (remove — not required; FuzzBufferType types included via <test/fuzz/fuzz.h>)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/fuzz/deserialize_dash.cpp` at line 49, Replace the C-style include with the C++ header: remove or change `#include <stdint.h>` to `#include <cstdint>` (or drop it entirely since `FuzzBufferType` and the byte-span types come from `test/fuzz/fuzz.h` and `int` used in `ds >> version` doesn’t need this header); update only the include line near the top of `deserialize_dash.cpp` (references: `FuzzBufferType`, `ds >> version`) so the file uses the C++20 `<cstdint>` convention or no redundant include.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/test/fuzz/deserialize_dash.cpp`:
- Around line 58-59: The template function signature
DashDeserializeFromFuzzingInput is too long and must be reformatted to satisfy
clang-format (wrap template parameters and function parameters across multiple
lines) and the macro body around the block at lines 88–95 must remove the manual
trailing-space alignment and instead rely on clang-format-safe formatting (no
explicit padding/backslash column alignment). Run clang-format (or
contrib/devtools/clang-format-diff.py), apply its suggested changes to the
DashDeserializeFromFuzzingInput declaration and to the macro body so the file
matches project column limits and removes trailing-space padding, then re-run
CI.
---
Duplicate comments:
In `@test/util/data/non-backported.txt`:
- Line 73: The review confirms the missing non-backported test file has been
correctly added (src/test/fuzz/deserialize_dash.cpp) and no further changes are
required; leave the added file in test/util/data/non-backported.txt as-is and
proceed with merging since the issue is resolved.
---
Nitpick comments:
In `@src/test/fuzz/deserialize_dash.cpp`:
- Line 49: Replace the C-style include with the C++ header: remove or change
`#include <stdint.h>` to `#include <cstdint>` (or drop it entirely since
`FuzzBufferType` and the byte-span types come from `test/fuzz/fuzz.h` and `int`
used in `ds >> version` doesn’t need this header); update only the include line
near the top of `deserialize_dash.cpp` (references: `FuzzBufferType`, `ds >>
version`) so the file uses the C++20 `<cstdint>` convention or no redundant
include.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/test/fuzz/deserialize_dash.cpp (1)
46-49:<stdexcept>appears to be an unused include.
std::ios_base::failurecomes from<ios>, which is transitively pulled in by<streams.h>. No type from<stdexcept>(e.g.,std::runtime_error,std::logic_error) is directly used in this file. The include can be dropped.🧹 Proposed removal
`#include` <exception> `#include` <optional> -#include <stdexcept> `#include` <stdint.h>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/fuzz/deserialize_dash.cpp` around lines 46 - 49, Remove the unneeded `#include` <stdexcept> from the include list in this file: the code does not use any types from stdexcept (e.g., std::runtime_error) and the only exception type used (std::ios_base::failure) comes from <ios> (transitively via <streams.h>), so delete the <stdexcept> include to clean up unused headers.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/test/fuzz/deserialize_dash.cpp`:
- Around line 46-49: Remove the unneeded `#include` <stdexcept> from the include
list in this file: the code does not use any types from stdexcept (e.g.,
std::runtime_error) and the only exception type used (std::ios_base::failure)
comes from <ios> (transitively via <streams.h>), so delete the <stdexcept>
include to clean up unused headers.
|
Note: PR #7162 (roundtrip fuzz targets) is a companion to this PR but not dependent on it. Both add new fuzz target files independently. If one merges first, the other will need a trivial rebase of |
src/test/fuzz/deserialize_dash.cpp
Outdated
| #include <streams.h> | ||
| #include <version.h> | ||
|
|
||
| // evo/ types |
There was a problem hiding this comment.
remove these kinda unneeded comments
There was a problem hiding this comment.
Done — removed section comments and sorted includes alphabetically.
src/test/fuzz/deserialize_dash.cpp
Outdated
| const std::optional<int> protocol_version = std::nullopt, | ||
| const int ser_type = SER_NETWORK) | ||
| { | ||
| CDataStream ds(buffer, ser_type, INIT_PROTO_VERSION); |
There was a problem hiding this comment.
using INIT_PROTO_VERSION is so old; I think maybe using either MIN_PEER_PROTO_VERSION or PROTOCOL_VERSION or MIN_MASTERNODE_PROTO_VERSION would make more sense; unless I'm misunderstanding something
There was a problem hiding this comment.
Good call — changed to PROTOCOL_VERSION. These Dash-specific types only exist in modern protocol versions, so INIT_PROTO_VERSION (209) didn't make sense here.
… use PROTOCOL_VERSION - Remove section comments from includes, sort alphabetically - Use PROTOCOL_VERSION instead of INIT_PROTO_VERSION for CDataStream initialization since Dash-specific types only exist in modern protocol versions
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/test/fuzz/deserialize_dash.cpp (1)
195-206: Consider adding a fuzz target forCDKGContribution.
CDKGContributionis declared in<llmq/dkgsession.h>withSERIALIZE_METHODSand is a security-sensitive DKG message. The other three DKG message types (CDKGComplaint,CDKGJustification,CDKGPrematureCommitment) are all covered with dedicated fuzz targets, butCDKGContributionis currently missing.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/fuzz/deserialize_dash.cpp` around lines 195 - 206, Add a new fuzz target for CDKGContribution using the same pattern as the existing DKG targets: create a FUZZ_TARGET_DASH_DESERIALIZE entry named e.g. dash_dkg_contribution_deserialize that constructs llmq::CDKGContribution and calls DashDeserializeFromFuzzingInput(buffer, obj); place it alongside the other targets (near dash_dkg_complaint_deserialize, dash_dkg_justification_deserialize, dash_dkg_premature_commitment_deserialize) so CDKGContribution from <llmq/dkgsession.h> (which uses SERIALIZE_METHODS) is fuzzed too.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/test/fuzz/deserialize_dash.cpp`:
- Around line 195-206: Add a new fuzz target for CDKGContribution using the same
pattern as the existing DKG targets: create a FUZZ_TARGET_DASH_DESERIALIZE entry
named e.g. dash_dkg_contribution_deserialize that constructs
llmq::CDKGContribution and calls DashDeserializeFromFuzzingInput(buffer, obj);
place it alongside the other targets (near dash_dkg_complaint_deserialize,
dash_dkg_justification_deserialize, dash_dkg_premature_commitment_deserialize)
so CDKGContribution from <llmq/dkgsession.h> (which uses SERIALIZE_METHODS) is
fuzzed too.
2 participants
Summary
Add deserialization fuzz targets for all Dash-specific serializable types. This is Phase 1 of a broader fuzzing initiative for Dash-specific code.
Currently, Dash Core inherits ~106 fuzz targets from Bitcoin Core, but zero fuzz targets exist for any Dash-specific code — approximately 24,000 lines across
src/evo/,src/llmq/,src/coinjoin/,src/governance/, andsrc/bls/are completely unfuzzed.Changes
New file
src/test/fuzz/deserialize_dash.cppwith 42 deserialization fuzz targets covering:evo/ (16 targets) — Provider transactions, asset lock/unlock, coinbase payload, credit pool, deterministic MN state, simplified MN lists, MN auth, hard fork signaling:
CProRegTx,CProUpServTx,CProUpRegTx,CProUpRevTxCAssetLockPayload,CAssetUnlockPayloadCCbTx,CCreditPoolCDeterministicMN,CDeterministicMNState,CDeterministicMNStateDiffCSimplifiedMNListEntry,CGetSimplifiedMNListDiff,CSimplifiedMNListDiffCMNAuth,MNHFTx,MNHFTxPayloadllmq/ (12 targets) — Quorum commitments, DKG messages, signing, quorum rotation:
CFinalCommitment,CFinalCommitmentTxPayloadCDKGComplaint,CDKGJustification,CDKGPrematureCommitmentCRecoveredSig,CSigShare,CSigSesAnn,CSigSharesInv,CBatchedSigSharesCQuorumDataRequest,CGetQuorumRotationInfo,CQuorumSnapshotgovernance/ (6 targets) — Governance objects and voting:
Governance::Object,CGovernanceObjectCGovernanceVote,vote_instance_t,vote_rec_t,CGovernanceObjectVoteFilebls/ (2 targets) — BLS IES encryption:
CBLSIESEncryptedBlob,CBLSIESMultiRecipientBlobscoinjoin/ (5 targets) — CoinJoin protocol messages:
CCoinJoinStatusUpdate,CCoinJoinAccept,CCoinJoinEntry,CCoinJoinQueue,CCoinJoinBroadcastTxApproach
Uses the same pattern as the existing
src/test/fuzz/deserialize.cppfor Bitcoin Core types:FUZZ_TARGETmacro withBasicTestingSetupinitializationCDataStreamdeserialization from fuzzer-provided bufferstd::ios_base::failureexceptionsTesting
These targets should compile and run in the existing
linux64_fuzzCI configuration with no additional dependencies.Validation
What was tested:
./configure --enable-fuzz-binary --disable-tests --without-gui --disable-bench+make -C src/test/fuzz -j$(nproc)— compiled all 42 new fuzz targetsResults:
linux64-build / Build source— pass (CI)linux64-test / Test source— pass (CI)linux64_nowallet-build— pass;linux64_nowallet-test— passlinux64_sqlite-build— pass;linux64_sqlite-test— passlinux64_ubsan-build— pass;linux64_ubsan-test— passmac-build / Build source— passwin64-build / Build source— passEnvironment: Local macOS arm64 (fuzz build); GitHub Actions CI (linux64_fuzz + full matrix)