Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] SVM: Integration of SolFuzz-Agave #4384

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

buffalojoec
Copy link

@buffalojoec buffalojoec commented Jan 10, 2025

NOTE: This change is intended for the new SVM repository, not the Agave monorepo.
Its draft inclusion here is only for review & collaboration purposes.

Problem

Both the Anza and Firedancer teams have been using SolFuzz-Agave
for crucial testing of validator components as well as BPF programs. It's also a
potentially very useful tool for other developers in each domain.

There are actually multiple problems being addressed by these changes - which
include only the addition of new libraries and no changes to Agave's hot
path.

First and foremost, the Firedancer team maintains SolFuzz-Agave standalone, and
whenever changes are made to Agave's execution layer, they must cherry-pick and
update the harness to be compatible with the new APIs. It would be much easier
if we maintained changes to SolFuzz-Agave alongside any API-breaking PRs to Agave.

Secondly, Anza's developer tooling team has been working on both migrating
builtins to BPF programs as well as rewrites of SPL Token, SPL Token-2022 and
more. We use SolFuzz-Agave and Mollusk
to test BPF programs in many different ways, including conformance testing
between two versions of the same program.

However, the same internal entrypoint into Agave's program runtime is
reimplemented across both tools. It would be much easier - and safer - if this
entrypoint lived in one place, and was instead used by both tools. This would
also mean that it could be used for other future tooling, which would be a huge
benefit.

https://github.com/buffalojoec/mollusk/blob/c01523016195c315870bd9e4bfa3a64a6cffa659/harness/src/lib.rs#L182-L218

The above problems are the motivation for both the integration of SolFuzz-Agave
itself, as well as the structure of the libraries included in this PR.

Including SolFuzz-Agave in Agave's SVM maintenance allows a much more stable API
for testing and fuzzing validator execution-layer components, which helps
Firedancer as well as any new validator clients who wish to benefit from the
same tooling. It also allows us to build more robust BPF program tooling, which
can help the broader ecosystem in obvious ways.

Summary of Changes

The overall change is straightforward: integrate SolFuzz-Agave into the SVM
stack by creating a new set of standalone crates that comprise a
reimplementation of the original SolFuzz-Agave.

I've refactored a lot of the original code to make it easier to maintain and
change, and I've included testing from the original repository to ensure maximum
compatibility.

NOTE: This change is only handling the instr (instruction) entrypoint. The
others (transaction, VM) will come later, and follow a similar pattern.

I've also decided to break the tooling up into multiple crates. They are
summarized as follows:

  • -fixture: The library for working with fixtures created by Firedancer's
    Protosol protobuf definitions. It
    offers conversions between prost-generated protobuf types and Agave SDK
    types.
  • -fixture-fs: An extension crate for working with fixtures (above) on the
    local filesytem (ie. load_from_blob_file).
  • -instr-entrypoint: The main entrypoint into Agave's program runtime. This
    library has nothing to do with fixtures, and can be used standalone to build
    tooling (ie. Mollusk).
  • -instr: The actual SolFuzz-Agave instruction harness itself, which depends
    on the above libraries to offer the same fuzzing and testing entrypoint as the
    original tool, including any custom setup or checks that are not imposed on
    the base entrypoint.

@buffalojoec buffalojoec added the noCI Suppress CI on this Pull Request label Jan 10, 2025
@buffalojoec buffalojoec force-pushed the sf-agave-integration-wip branch from 9aaede3 to fc30727 Compare January 10, 2025 06:20
@buffalojoec buffalojoec force-pushed the sf-agave-integration-wip branch from fc30727 to d5fab2d Compare January 10, 2025 12:12
Comment on lines +25 to +26
find dump/test-vectors/instr/fixtures -type f -name '*.fix' | xargs -P 32 -I {} ../target/release/test_exec_instr {} > $LOG_PATH/test_exec_instr.log 2>&1
# Other tests will be included here...

Choose a reason for hiding this comment

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

Thank you so much for organizing this.

Is this working for all instruction test vectors? If so, we can delete my conformance code from here.

Are you planning to add conformance testing for transactions as well?

Copy link
Author

Choose a reason for hiding this comment

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

Not working for all just yet. Over 20,000 are passing, and about 140 are failing still. I think I'm just missing one or two very small things.

Will push an update and comment here when it's finally 100%.

Are you planning to add conformance testing for transactions as well?

Yep, we will add the rest of the harness and then this library can be used to build out the coverage for SVM from VM up to tx.

@mjain-jump
Copy link

This is awesome @buffalojoec thank you so much for this. Please let us know if you run into any issues with some of the failing tests. It would also be great to define a spec for this so that other validator clients can implement their own harnesses as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
noCI Suppress CI on this Pull Request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants