test: add asset lock/unlock validation fuzz targets

[thepastaclaw]

Summary

Add fuzz targets for asset lock/unlock validation — the L1↔L2 credit pool bridge (DIP-27). This covers CheckAssetLockTx() and GetAssetUnlockFee(), both consensus-critical functions that process untrusted transaction data.

Part of the Dash Core Fuzzing Initiative (Phase 3 — functional targets). Tracker: thepastaclaw/tracker#108.

Fuzz Targets

asset_lock_tx — Structured CheckAssetLockTx fuzzing

Uses FuzzedDataProvider to construct well-formed asset lock transactions, then systematically mutates them through 12 scenarios covering every validation branch:

• Wrong transaction type
• Non-empty OP_RETURN data
• OP_RETURN amount out of range (0 or >MAX_MONEY)
• Multiple OP_RETURN outputs
• Missing OP_RETURN output
• Corrupt/truncated payload
• Invalid payload version (0 or 255)
• Empty credit outputs
• Credit output amount out of range
• Non-P2PKH credit output scripts
• Credit amount mismatch with OP_RETURN value
• Valid transaction (happy path)

asset_lock_tx_raw — Raw deserialization CheckAssetLockTx fuzzing

Deserializes a CTransaction from raw fuzz bytes and passes it directly to CheckAssetLockTx(). Catches edge cases that structured fuzzing misses (e.g., unusual serialization states, version combinations).

asset_unlock_fee — Structured GetAssetUnlockFee fuzzing

Constructs CAssetUnlockPayload with fuzzed fields (version, index, fee, requestedHeight, quorumHash, quorumSig), then optionally corrupts the payload or zeroes the fee to exercise all validation paths.

Invariant Checks

All targets assert result == state.IsValid() — verifying that the validation functions maintain consistency between their return value and the validation state object.

Validation

• Build: Clean compilation with --enable-fuzz (clang++, no warnings)
• Targets registered: All 3 targets appear in PRINT_ALL_FUZZ_TARGETS_AND_ABORT output
• Fuzz runs: All 3 targets pass against 10 random 512-byte corpus inputs each (0 crashes)

  asset_lock_tx: succeeded against 10 files in 0s.
  asset_lock_tx_raw: succeeded against 10 files in 0s.
  asset_unlock_fee: succeeded against 10 files in 0s.
  

• Environment: macOS arm64, clang 17, --enable-fuzz --disable-wallet --with-gui=no

[github-actions]

⚠️ Potential Merge Conflicts Detected

This PR has potential conflicts with the following open PRs:

• test: add roundtrip serialize/deserialize fuzz targets for Dash types #7162 - test: add roundtrip serialize/deserialize fuzz targets for Dash types
• test: add Dash-specific process_message target #7171 - test: add Dash-specific process_message target
• test: add special transaction validation fuzz target #7169 - test: add special transaction validation fuzz target
• test: add CoinJoin validation fuzz targets #7174 - test: add CoinJoin validation fuzz targets
• test: add BLS operations and IES encryption fuzz targets #7167 - test: add BLS operations and IES encryption fuzz targets
• test: add llmq message handler fuzz target #7172 - test: add llmq message handler fuzz target
• test: add governance proposal validator fuzz target #7166 - test: add governance proposal validator fuzz target

Please coordinate with the authors of these PRs to avoid merge conflicts.

[coderabbitai]

Warning

Rate limit exceeded

@thepastaclaw has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 1 minutes and 14 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

Walkthrough

This pull request adds a new fuzz test source (src/test/fuzz/asset_lock_unlock.cpp) and registers it in the test Makefile. The new file provides helper generators and three fuzz targets—asset_lock_tx, asset_lock_tx_raw, and asset_unlock_fee—that construct, mutate, deserialize, and validate asset lock/unlock transactions and compute related fees using existing validation utilities (e.g., CheckAssetLockTx, GetAssetUnlockFee) under regtest parameters.

Sequence Diagram(s)

sequenceDiagram
    participant Fuzzer
    participant Builder as Tx Builder
    participant Deserializer as Tx Deserializer
    participant Validator as CheckAssetLockTx / GetAssetUnlockFee
    participant State as TxValidationState

    Fuzzer->>Builder: generate scripts, amounts, payloads, signatures, u256 values
    Builder->>Builder: construct asset-lock or asset-unlock transaction (various mutation cases)
    alt raw deserialization path
        Fuzzer->>Deserializer: provide fuzzed byte stream
        Deserializer->>Validator: parsed transaction
    else constructed transaction path
        Builder->>Validator: constructed transaction
    end
    Validator->>State: validate transaction / compute unlock fee
    State-->>Validator: validation result / fee value
    Validator-->>Fuzzer: return result (asserts or exits on invalid states)

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main change: adding asset lock/unlock validation fuzz targets, which directly matches the primary purpose of the changeset.
Description check	✅ Passed	The description provides comprehensive context about the three fuzz targets being added, their purposes, validation coverage, and testing results—directly aligned with the changeset contents.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

src/test/fuzz/asset_lock_unlock.cpp (1)

60-70: ConsumeUInt256: consider using std::copy for clarity.

The manual iterator loop works correctly but could be simplified.

♻️ Optional simplification

 uint256 ConsumeUInt256(FuzzedDataProvider& fuzzed_data_provider)
 {
     uint256 value;
-    auto it = value.begin();
     const std::vector<uint8_t> bytes = fuzzed_data_provider.ConsumeBytes<uint8_t>(32);
-    for (uint8_t b : bytes) {
-        *it = b;
-        ++it;
-    }
+    std::copy(bytes.begin(), bytes.end(), value.begin());
     return value;
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/test/fuzz/asset_lock_unlock.cpp` around lines 60 - 70, The manual loop in
ConsumeUInt256 can be simplified by copying the consumed bytes into the uint256
buffer; replace the for-loop that assigns via iterator with a single std::copy
from bytes.begin()/bytes.end() to value.begin() (or use std::memcpy) to make the
intent clearer and reduce boilerplate—update the function ConsumeUInt256 (types:
uint256, FuzzedDataProvider, and std::vector<uint8_t> bytes) accordingly and
ensure <algorithm> is included if using std::copy.

🤖 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/Makefile.test.include`:
- Around line 280-282: The file list in src/Makefile.test.include is out of
alphabetical order: test/fuzz/asset_lock_unlock.cpp is placed between
test/fuzz/asmap.cpp and test/fuzz/asmap_direct.cpp; move the entry for
asset_lock_unlock.cpp so that the sequence is alphabetically sorted (i.e., place
test/fuzz/asset_lock_unlock.cpp after test/fuzz/asmap_direct.cpp) ensuring the
fuzz source list maintains correct lexicographic order.

---

Nitpick comments:
In `@src/test/fuzz/asset_lock_unlock.cpp`:
- Around line 60-70: The manual loop in ConsumeUInt256 can be simplified by
copying the consumed bytes into the uint256 buffer; replace the for-loop that
assigns via iterator with a single std::copy from bytes.begin()/bytes.end() to
value.begin() (or use std::memcpy) to make the intent clearer and reduce
boilerplate—update the function ConsumeUInt256 (types: uint256,
FuzzedDataProvider, and std::vector<uint8_t> bytes) accordingly and ensure
<algorithm> is included if using std::copy.

[coderabbitai]

🤖 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/Makefile.test.include`:
- Around line 280-283: The alphabetical ordering issue has been resolved: ensure
the entries in test_fuzz_fuzz_SOURCES keep lexicographic order by verifying
asmap_direct.cpp appears before asset_lock_unlock.cpp (and maintain the overall
ordering of test/fuzz/asmap.cpp, test/fuzz/asmap_direct.cpp,
test/fuzz/asset_lock_unlock.cpp, test/fuzz/autofile.cpp); no code changes
required other than preserving this ordering in future edits.

[PastaPastaPasta]

missing newlines

[coderabbitai]

🧹 Nitpick comments (1)

test/util/data/non-backported.txt (1)

52-54: src/test/fuzz/asset_lock_unlock.cpp entry is out of alphabetical order.

The entry on Line 53 is inserted between src/test/block_reward_reallocation_tests.cpp and src/test/bls_tests.cpp, but fuzz/ sorts after all b-prefixed filenames. It should sit after Line 54 (src/test/bls_tests.cpp).

♻️ Proposed reorder

 src/test/block_reward_reallocation_tests.cpp
-src/test/fuzz/asset_lock_unlock.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/asset_lock_unlock.cpp
 src/test/llmq*.cpp

Based on learnings, test/util/data/non-backported.txt is used to run clang-format on C/C++ files; the .cpp extension is correct and the entry is required for this new Dash-specific file. Only the placement is off.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/util/data/non-backported.txt` around lines 52 - 54, The entry
"src/test/fuzz/asset_lock_unlock.cpp" in test/util/data/non-backported.txt is
out of alphabetical order; move that line so it appears after
"src/test/bls_tests.cpp" (i.e., place the fuzz/ entry after the b-prefixed
entries) to restore correct alphabetical ordering within the file.

🤖 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`:
- Around line 52-54: The entry "src/test/fuzz/asset_lock_unlock.cpp" in
test/util/data/non-backported.txt is out of alphabetical order; move that line
so it appears after "src/test/bls_tests.cpp" (i.e., place the fuzz/ entry after
the b-prefixed entries) to restore correct alphabetical ordering within the
file.

[thepastaclaw]

Re: missing newlines — fixed in 0fcd955, pushed right around when you commented.