test: add Dash-specific process_message target#7171
test: add Dash-specific process_message target#7171thepastaclaw wants to merge 4 commits intodashpay:developfrom
Conversation
|
|
No actionable comments were generated in the recent review. 🎉 WalkthroughAdds a new fuzzing harness and test registration for Dash protocol message processing. A Makefile test include was updated to add the new source, and src/test/fuzz/process_message_dash.cpp implements a FUZZ_TARGET that initializes a REGTEST test node, mines blocks, creates and registers a mock peer, builds randomized CSerializedNetMsg instances from 41 Dash message types with random payloads, repeatedly calls ProcessMessagesOnce while handling exceptions, flushes/send buffers, synchronizes validation-interface callbacks, and shuts down the test node. Sequence Diagram(s)sequenceDiagram
participant FuzzInput as Fuzz Input
participant FDP as FuzzDataProvider
participant Harness as Fuzz Harness
participant Node as Test Node (REGTEST)
participant Peer as Mock Peer (CNode)
participant Proc as ProcessMessagesOnce
participant VI as ValidationInterface
FuzzInput->>FDP: provide bytes
FDP->>Harness: produce message type + payload
Harness->>Node: initialize, mine blocks, sync VI
Harness->>Peer: create & attach peer
Harness->>Peer: enqueue CSerializedNetMsg
Peer->>Proc: trigger ProcessMessagesOnce
Proc->>Node: validate/process message
Node->>VI: emit validation callbacks / state changes
loop until input exhausted
FDP->>Harness: more message data
Harness->>Peer: enqueue next message
Peer->>Proc: ProcessMessagesOnce
end
Harness->>Node: synchronize VI queue & shutdown
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/test/fuzz/process_message_dash.cpp (1)
25-67: Optional: usestd::to_arrayto eliminate the hardcoded count41Although the compiler enforces the count,
std::to_array(C++20, already required by the project) removes it entirely, making additions/removals to the list self-maintaining.♻️ Proposed refactor
-const std::array<const char*, 41> DASH_MESSAGE_TYPES{ +constexpr auto DASH_MESSAGE_TYPES = std::to_array<const char*>({ NetMsgType::SPORK, ... NetMsgType::PLATFORMBAN, -}; +});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/fuzz/process_message_dash.cpp` around lines 25 - 67, Replace the fixed-size std::array declaration for DASH_MESSAGE_TYPES with a std::to_array-based initialization so the element count is inferred and maintained automatically; locate the DASH_MESSAGE_TYPES variable and change its definition from std::array<const char*, 41> DASH_MESSAGE_TYPES{ ... } to an auto (or explicitly typed) variable initialized via std::to_array({ NetMsgType::SPORK, NetMsgType::GETSPORKS, ... }), ensuring the element type matches (const char* or appropriate NetMsgType string type) so additions/removals no longer require updating a hardcoded count.
🤖 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/process_message_dash.cpp`:
- Around line 1-3: Add the new Dash-specific fuzz test file
src/test/fuzz/process_message_dash.cpp to the exclusion list used by
clang-format by appending its relative path to
test/util/data/non-backported.txt; locate the new file name
process_message_dash.cpp and ensure an exact matching entry is added to the
non-backported.txt list so CI recognizes it as exempt from backport formatting
checks.
---
Nitpick comments:
In `@src/test/fuzz/process_message_dash.cpp`:
- Around line 25-67: Replace the fixed-size std::array declaration for
DASH_MESSAGE_TYPES with a std::to_array-based initialization so the element
count is inferred and maintained automatically; locate the DASH_MESSAGE_TYPES
variable and change its definition from std::array<const char*, 41>
DASH_MESSAGE_TYPES{ ... } to an auto (or explicitly typed) variable initialized
via std::to_array({ NetMsgType::SPORK, NetMsgType::GETSPORKS, ... }), ensuring
the element type matches (const char* or appropriate NetMsgType string type) so
additions/removals no longer require updating a hardcoded count.
Required for clang-format CI enforcement on Dash-specific files.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/util/data/non-backported.txt (1)
53-53: LGTM – correct registration of the new Dash-specific fuzz target.
src/test/fuzz/process_message_dash.cppis a.cppfile and is properly added here so clang-format CI will enforce style on it. Based on learnings, this matches the requirement that all new non-backported.cpp/.hfiles be listed in this file.One optional nit: alphabetically within the
src/test/block the entry (f) lands beforebls_tests.cpp(b). The file's ordering is already loose (e.g.,governance*.cppappears afterutil/llmq_tests.h), so this is a pre-existing pattern and not a blocker, but placing it afterbls_tests.cppwould be more consistent.♻️ Suggested re-ordering within the
src/test/blocksrc/test/block_reward_reallocation_tests.cpp +src/test/bls_tests.cpp +src/test/coinjoin_*.cpp +src/test/dip0020opcodes_tests.cpp +src/test/dynamic_activation*.cpp +src/test/evo*.cpp +src/test/fuzz/process_message_dash.cpp -src/test/bls_tests.cpp -src/test/coinjoin_*.cpp -src/test/dip0020opcodes_tests.cpp -src/test/dynamic_activation*.cpp -src/test/evo*.cpp🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/util/data/non-backported.txt` at line 53, The non-backported file list correctly includes src/test/fuzz/process_message_dash.cpp but its alphabetical placement in the src/test/ block is inconsistent; move the entry "src/test/fuzz/process_message_dash.cpp" so it appears after "bls_tests.cpp" within the src/test/ block of non-backported.txt to keep ordering consistent (no code changes required, just reorder the line).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@test/util/data/non-backported.txt`:
- Line 53: The non-backported file list correctly includes
src/test/fuzz/process_message_dash.cpp but its alphabetical placement in the
src/test/ block is inconsistent; move the entry
"src/test/fuzz/process_message_dash.cpp" so it appears after "bls_tests.cpp"
within the src/test/ block of non-backported.txt to keep ordering consistent (no
code changes required, just reorder the line).
|
Superseded by consolidated fuzz branch in #7173 to avoid merge conflicts. |
Summary
Adds a dedicated Dash-specific P2P fuzz target,
process_message_dash, to cover Dash net message handling paths with structured random message payloads.This target mirrors the existing
process_messageharness (TestingSetup+ConnmanTestMsg+ProcessMessagesOnce) but constrains message selection to Dash message types only (CoinJoin, governance, quorum/LLMQ, mnauth, etc.).Changes
src/test/fuzz/process_message_dash.cppNetMsgTypecommandsReceiveMsgFromsrc/Makefile.test.includeValidation
What was tested
make -j16 -C src test/fuzz/fuzzPRINT_ALL_FUZZ_TARGETS_AND_ABORT=1 src/test/fuzz/fuzz | rg '^process_message_dash$'tmpfile=$(mktemp); head -c 4096 /dev/urandom > "$tmpfile"; timeout 20s env FUZZ=process_message_dash src/test/fuzz/fuzz "$tmpfile"; rc=$?; rm -f "$tmpfile"; exit $rcResults
process_message_dashlisted)process_message_dash: succeeded against 1 files in 0s.)Environment
~/Projects/dash/worktrees/tracker-108Related tracker epic: thepastaclaw/tracker#108