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

feat: ssa fuzzer #7641

Open
wants to merge 57 commits into
base: master
Choose a base branch
from
Open

Conversation

jewelofchaos9
Copy link
Contributor

Description

Summary*

This PR introduces SSA fuzzer for testing ACIR and Brillig execution of ssa built from scratch.

Additional Context

Documentation*

Check one:

  • No documentation needed.
  • Documentation included in this PR.
  • [For Experimental Features] Documentation to be submitted in a separate PR.

PR Checklist*

  • I have tested the changes locally.
  • I have formatted the changes with Prettier and/or cargo fmt on default settings.

@Rumata888 Rumata888 self-requested a review March 17, 2025 17:34

/// Inserts an add instruction between two values
pub fn insert_add_instruction(&mut self, lhs: Id<Value>, rhs: Id<Value>) -> Id<Value> {
let result = self.builder.insert_binary(lhs, BinaryOp::Add { unchecked: false }, rhs);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Checked/Unchecked should be parameterized, too

pub mod compiler;
pub mod config;
pub mod helpers;
pub mod runner;
Copy link
Collaborator

Choose a reason for hiding this comment

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

COuld you please add tests checking the correctness of all simple instructions (additions, etc). Run in both modes and check results

}

/// Checks if two indices exist in both ACIR and Brillig witness vectors
fn both_indeces_presented(
Copy link
Collaborator

Choose a reason for hiding this comment

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

indices

Copy link
Collaborator

Choose a reason for hiding this comment

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

Rename to "index_exists_in_both_witness_vectors"

}

/// Checks if an index exists in both ACIR and Brillig witness vectors
fn index_presented(
Copy link
Collaborator

Choose a reason for hiding this comment

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

index_exists


/// Gets an element from an array at the given index
fn insert_array_get(&mut self, array_idx: u32, index: u32) {
if array_idx >= self.acir_arrays.len() as u32
Copy link
Collaborator

Choose a reason for hiding this comment

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

It is better to just use indices modulo lengths. Otherwise the majority of testcases will skip instructions instead of creating them

/// Sets an element in an array at the given index
fn insert_array_set(&mut self, array_idx: u32, index: u32, value: u32) {
if array_idx >= self.acir_arrays.len() as u32
|| array_idx >= self.brillig_arrays.len() as u32
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here

}

/// Checks if an index exists in both ACIR and Brillig witness vectors
fn index_presented(
Copy link
Collaborator

Choose a reason for hiding this comment

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

It seems like you are duplicating functionality from field.rs here

jewelofchaos9 and others added 6 commits March 18, 2025 11:20
Co-authored-by: Innokentii Sennovskii <isennovskiy@gmail.com>
Co-authored-by: Innokentii Sennovskii <isennovskiy@gmail.com>
Co-authored-by: Innokentii Sennovskii <isennovskiy@gmail.com>
@jewelofchaos9 jewelofchaos9 requested a review from Rumata888 March 18, 2025 12:41
@@ -11,7 +11,7 @@ pub use self::test::{TestStatus, run_test};

mod check;
mod compile;
mod execute;
pub mod execute;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
pub mod execute;
mod execute;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I need it here

Copy link
Member

Choose a reason for hiding this comment

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

We reexport that function a couple of lines up.

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

Successfully merging this pull request may close these issues.

3 participants