Skip to content

fuzz: Rework rpc fuzz target - #35860

Merged
fanquake merged 1 commit into
bitcoin:masterfrom
maflcko:2608-fuzz-new-rpc
Aug 3, 2026
Merged

fuzz: Rework rpc fuzz target#35860
fanquake merged 1 commit into
bitcoin:masterfrom
maflcko:2608-fuzz-new-rpc

Conversation

@maflcko

@maflcko maflcko commented Aug 1, 2026

Copy link
Copy Markdown
Member

The rpc fuzz target constructs a vector of string args and passes that to RPCConvertValues.

This has many issues:

  • Each of those strings could represent an array itself. E.g. via range argument or via ConsumeArrayRPCArgument. However, those strings may not be converted to an array via RPCConvertValues and just be passed on as string argument. Having a call to ConsumeArrayRPCArgument that ends up with a plain json string is confusing.
  • The strings could only represent an object or json null, when a raw string represented such a serialized json and was also converted to one via RPCConvertValues. Having a call to ConsumeScalarRPCArgument that was intended to give a raw string but ends up with a arbitrary json object is confusing.

Fix those "stringly-typed" issues by making the fuzz target "type safe":

  • Rename ConsumeScalarRPCArgument to ConsumeBasicRPCArgument and return a proper UniValue from it.
  • The "consume string" case inside that function, which had a "double meaning" is turned into two type-safe cases: One that returns a json string and one that reads an arbitrary json from a string.
  • A new case for json null is added.
  • ConsumeRPCArgument is changed to cover both json arrays and json dicts properly.
  • Pass the resulting positional UniValue array directly to the RPC method, avoiding the need for RPCConvertValues.

Making the fuzz target "type safe" is also the first step in making it schema-aware.

@DrahtBot

DrahtBot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

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

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/35860.

Reviews

See the guideline and AI policy for information on the review process.

Type Reviewers
ACK dergoegge

If your review is incorrectly listed, please copy-paste <!--meta-tag:bot-skip--> into the comment that the bot should ignore.

@DrahtBot

DrahtBot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

🚧 At least one of the CI tasks failed.
Task tidy: https://github.com/bitcoin/bitcoin/actions/runs/30698617249/job/91365600298
LLM reason (✨ experimental): CI failed because clang-tidy reported misc-no-recursion errors (recursive call chain in test/fuzz/rpc.cpp, treated as warnings-as-errors).

Hints

Try to run the tests locally, according to the documentation. However, a CI failure may still
happen due to a number of reasons, for example:

  • Possibly due to a silent merge conflict (the changes in this pull request being
    incompatible with the current code in the target branch). If so, make sure to rebase on the latest
    commit of the target branch.

  • A sanitizer issue, which can only be found by compiling with the sanitizer and running the
    affected test.

  • An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

@maflcko
maflcko force-pushed the 2608-fuzz-new-rpc branch 2 times, most recently from faecf69 to 0000d72 Compare August 1, 2026 14:13

@dergoegge dergoegge left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK fa895bb

@fanquake
fanquake merged commit 30f6b05 into bitcoin:master Aug 3, 2026
29 checks passed
@maflcko
maflcko deleted the 2608-fuzz-new-rpc branch August 3, 2026 11:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants