fuzz: refactor fixture processing harnesses to use &mut self (#77)

Author: buffalojoec

harness/src/fuzz/check.rs

@@ -0,0 +1,218 @@
+//! Checks to run against a fixture when validating.
+
+use {
+    crate::result::{Check, InstructionResult},
+    solana_sdk::{
+        account::{Account, ReadableAccount},
+        pubkey::Pubkey,
+    },
+};
+
+/// Checks to run against a fixture when validating.
+///
+/// Similar to Mollusk's `result::Check`, this allows a developer to dictate
+/// the type of checks to run on the fixture's effects.
+///
+/// Keep in mind that validation of fixtures works slightly differently than
+/// typical Mollusk unit tests. In a Mollusk test, you can provide the value to
+/// compare a portion of the result against (ie. compute units). However, when
+/// comparing the result of a Mollusk invocation against a fixture, the value
+/// from the fixture itself is used.
+///
+/// For that reason, these are unary variants, and do not offer the developer a
+/// way to provide values to check against.
+pub enum FixtureCheck {
+    /// Validate compute units consumed.
+    ComputeUnits,
+    /// Validate the program result.
+    ProgramResult,
+    /// Validate the return data.
+    ReturnData,
+    /// Validate all resulting accounts.
+    AllResultingAccounts {
+        /// Whether or not to validate each account's data.
+        data: bool,
+        /// Whether or not to validate each account's lamports.
+        lamports: bool,
+        /// Whether or not to validate each account's owner.
+        owner: bool,
+        /// Whether or not to validate each account's space.
+        space: bool,
+    },
+    /// Validate the resulting accounts at certain addresses.
+    OnlyResultingAccounts {
+        /// The addresses on which to apply the validation.
+        addresses: Vec<Pubkey>,
+        /// Whether or not to validate each account's data.
+        data: bool,
+        /// Whether or not to validate each account's lamports.
+        lamports: bool,
+        /// Whether or not to validate each account's owner.
+        owner: bool,
+        /// Whether or not to validate each account's space.
+        space: bool,
+    },
+    /// Validate all of the resulting accounts _except_ the provided addresses.
+    AllResultingAccountsExcept {
+        /// The addresses on which to _not_ apply the validation.
+        ignore_addresses: Vec<Pubkey>,
+        /// On non-ignored accounts, whether or not to validate each account's
+        /// data.
+        data: bool,
+        /// On non-ignored accounts, whether or not to validate each account's
+        /// lamports.
+        lamports: bool,
+        /// On non-ignored accounts, whether or not to validate each account's
+        /// owner.
+        owner: bool,
+        /// On non-ignored accounts, whether or not to validate each account's
+        /// space.
+        space: bool,
+    },
+}
+
+impl FixtureCheck {
+    /// Validate all possible checks for all resulting accounts.
+    ///
+    /// Note: To omit certain checks, use the variant directly, ie.
+    /// `FixtureCheck::AllResultingAccounts { data: false, .. }`.
+    pub fn all_resulting_accounts() -> Self {
+        Self::AllResultingAccounts {
+            data: true,
+            lamports: true,
+            owner: true,
+            space: true,
+        }
+    }
+
+    /// Validate all possible checks for only the resulting accounts at certain
+    /// addresses.
+    ///
+    /// Note: To omit certain checks, use the variant directly, ie.
+    /// `FixtureCheck::OnlyResultingAccounts { data: false, .. }`.
+    pub fn only_resulting_accounts(addresses: &[Pubkey]) -> Self {
+        Self::OnlyResultingAccounts {
+            addresses: addresses.to_vec(),
+            data: true,
+            lamports: true,
+            owner: true,
+            space: true,
+        }
+    }
+
+    /// Validate all possible checks for all of the resulting accounts _except_
+    /// the provided addresses.
+    ///
+    /// Note: To omit certain checks, use the variant directly, ie.
+    /// `FixtureCheck::AllResultingAccountsExcept { data: false, .. }`.
+    pub fn all_resulting_accounts_except(ignore_addresses: &[Pubkey]) -> Self {
+        Self::AllResultingAccountsExcept {
+            ignore_addresses: ignore_addresses.to_vec(),
+            data: true,
+            lamports: true,
+            owner: true,
+            space: true,
+        }
+    }
+}
+
+fn add_account_checks<'a>(
+    checks: &mut Vec<Check<'a>>,
+    accounts: impl Iterator<Item = &'a (Pubkey, Account)>,
+    data: bool,
+    lamports: bool,
+    owner: bool,
+    space: bool,
+) {
+    for (pubkey, account) in accounts {
+        let mut builder = Check::account(pubkey);
+        if data {
+            builder = builder.data(account.data());
+        }
+        if lamports {
+            builder = builder.lamports(account.lamports());
+        }
+        if owner {
+            builder = builder.owner(account.owner());
+        }
+        if space {
+            builder = builder.space(account.data().len());
+        }
+        checks.push(builder.build());
+    }
+}
+
+pub(crate) fn evaluate_results_with_fixture_checks(
+    expected: &InstructionResult,
+    result: &InstructionResult,
+    fixture_checks: &[FixtureCheck],
+) {
+    let mut checks = vec![];
+
+    for fixture_check in fixture_checks {
+        match fixture_check {
+            FixtureCheck::ComputeUnits => {
+                checks.push(Check::compute_units(expected.compute_units_consumed))
+            }
+            FixtureCheck::ProgramResult => {
+                checks.push(Check::program_result(expected.program_result.clone()))
+            }
+            FixtureCheck::ReturnData => checks.push(Check::return_data(&expected.return_data)),
+            FixtureCheck::AllResultingAccounts {
+                data,
+                lamports,
+                owner,
+                space,
+            } => {
+                add_account_checks(
+                    &mut checks,
+                    expected.resulting_accounts.iter(),
+                    *data,
+                    *lamports,
+                    *owner,
+                    *space,
+                );
+            }
+            FixtureCheck::OnlyResultingAccounts {
+                addresses,
+                data,
+                lamports,
+                owner,
+                space,
+            } => {
+                add_account_checks(
+                    &mut checks,
+                    expected
+                        .resulting_accounts
+                        .iter()
+                        .filter(|(pubkey, _)| addresses.contains(pubkey)),
+                    *data,
+                    *lamports,
+                    *owner,
+                    *space,
+                );
+            }
+            FixtureCheck::AllResultingAccountsExcept {
+                ignore_addresses,
+                data,
+                lamports,
+                owner,
+                space,
+            } => {
+                add_account_checks(
+                    &mut checks,
+                    expected
+                        .resulting_accounts
+                        .iter()
+                        .filter(|(pubkey, _)| !ignore_addresses.contains(pubkey)),
+                    *data,
+                    *lamports,
+                    *owner,
+                    *space,
+                );
+            }
+        }
+    }
+
+    result.run_checks(&checks);
+}

harness/src/fuzz/firedancer.rs

@@ -7,7 +7,7 @@
 use {
     crate::{
         accounts::{compile_accounts, CompiledAccounts},
-        result::{Check, InstructionResult},
+        result::InstructionResult,
         Mollusk, DEFAULT_LOADER_KEY,
     },
     mollusk_svm_fuzz_fixture_firedancer::{
@@ -22,6 +22,7 @@ use {
     solana_compute_budget::compute_budget::ComputeBudget,
     solana_sdk::{
         account::Account,
+        feature_set::FeatureSet,
         instruction::{AccountMeta, Instruction, InstructionError},
         pubkey::Pubkey,
     },
@@ -66,17 +67,12 @@ fn num_to_instr_err(num: i32, custom_code: u32) -> InstructionError {
 }
 
 fn build_fixture_context(
-    mollusk: &Mollusk,
-    instruction: &Instruction,
     accounts: &[(Pubkey, Account)],
+    compute_budget: &ComputeBudget,
+    feature_set: &FeatureSet,
+    instruction: &Instruction,
+    slot: u64,
 ) -> FuzzContext {
-    let Mollusk {
-        compute_budget,
-        feature_set,
-        slot, // FD-Fuzz feature only.
-        ..
-    } = mollusk;
-
     let loader_key = if BUILTIN_PROGRAM_IDS.contains(&instruction.program_id) {
         solana_sdk::native_loader::id()
     } else {
@@ -100,14 +96,22 @@ fn build_fixture_context(
         instruction_accounts,
         instruction_data: instruction.data.clone(),
         compute_units_available: compute_budget.compute_unit_limit,
-        slot_context: FuzzSlotContext { slot: *slot },
+        slot_context: FuzzSlotContext { slot },
         epoch_context: FuzzEpochContext {
             feature_set: feature_set.clone(),
         },
     }
 }
 
-fn parse_fixture_context(context: &FuzzContext) -> (Mollusk, Instruction, Vec<(Pubkey, Account)>) {
+pub struct ParsedFixtureContext {
+    pub accounts: Vec<(Pubkey, Account)>,
+    pub compute_budget: ComputeBudget,
+    pub feature_set: FeatureSet,
+    pub instruction: Instruction,
+    pub slot: u64,
+}
+
+pub(crate) fn parse_fixture_context(context: &FuzzContext) -> ParsedFixtureContext {
     let FuzzContext {
         program_id,
         accounts,
@@ -128,13 +132,6 @@ fn parse_fixture_context(context: &FuzzContext) -> (Mollusk, Instruction, Vec<(P
         .map(|(key, acct, _)| (*key, acct.clone()))
         .collect::<Vec<_>>();
 
-    let mollusk = Mollusk {
-        compute_budget,
-        feature_set: epoch_context.feature_set.clone(),
-        slot: slot_context.slot,
-        ..Default::default()
-    };
-
     let metas = instruction_accounts
         .iter()
         .map(|ia| {
@@ -152,7 +149,13 @@ fn parse_fixture_context(context: &FuzzContext) -> (Mollusk, Instruction, Vec<(P
 
     let instruction = Instruction::new_with_bytes(*program_id, instruction_data, metas);
 
-    (mollusk, instruction, accounts)
+    ParsedFixtureContext {
+        accounts,
+        compute_budget,
+        feature_set: epoch_context.feature_set.clone(),
+        instruction,
+        slot: slot_context.slot,
+    }
 }
 
 fn build_fixture_effects(context: &FuzzContext, result: &InstructionResult) -> FuzzEffects {
@@ -195,9 +198,9 @@ fn build_fixture_effects(context: &FuzzContext, result: &InstructionResult) -> F
     }
 }
 
-fn parse_fixture_effects(
-    mollusk: &Mollusk,
+pub(crate) fn parse_fixture_effects(
     accounts: &[(Pubkey, Account)],
+    compute_unit_limit: u64,
     effects: &FuzzEffects,
 ) -> InstructionResult {
     let raw_result = if effects.program_result == 0 {
@@ -229,10 +232,7 @@ fn parse_fixture_effects(
         program_result,
         raw_result,
         execution_time: 0, // TODO: Omitted for now.
-        compute_units_consumed: mollusk
-            .compute_budget
-            .compute_unit_limit
-            .saturating_sub(effects.compute_units_available),
+        compute_units_consumed: compute_unit_limit.saturating_sub(effects.compute_units_available),
         return_data,
         resulting_accounts,
     }
@@ -250,9 +250,14 @@ pub fn build_fixture_from_mollusk_test(
     instruction: &Instruction,
     accounts: &[(Pubkey, Account)],
     result: &InstructionResult,
-    _checks: &[Check],
 ) -> FuzzFixture {
-    let input = build_fixture_context(mollusk, instruction, accounts);
+    let input = build_fixture_context(
+        accounts,
+        &mollusk.compute_budget,
+        &mollusk.feature_set,
+        instruction,
+        mollusk.slot, // FD-fuzz feature only.
+    );
     // This should probably be built from the checks, but there's currently no
     // mechanism to enforce full check coverage on a result.
     let output = build_fixture_effects(&input, result);
@@ -265,15 +270,14 @@ pub fn build_fixture_from_mollusk_test(
 
 pub fn load_firedancer_fixture(
     fixture: &mollusk_svm_fuzz_fixture_firedancer::Fixture,
-) -> (
-    Mollusk,
-    Instruction,
-    Vec<(Pubkey, Account)>,
-    InstructionResult,
-) {
-    let (mollusk, instruction, accounts) = parse_fixture_context(&fixture.input);
-    let result = parse_fixture_effects(&mollusk, &accounts, &fixture.output);
-    (mollusk, instruction, accounts, result)
+) -> (ParsedFixtureContext, InstructionResult) {
+    let parsed = parse_fixture_context(&fixture.input);
+    let result = parse_fixture_effects(
+        &parsed.accounts,
+        parsed.compute_budget.compute_unit_limit,
+        &fixture.output,
+    );
+    (parsed, result)
 }
 
 #[test]