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: ux fuzz invariant #4744

Merged
merged 58 commits into from
May 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
58 commits
Select commit Hold shift + click to select a range
8fd577a
Parse FuzzConfig from string (brief impl)
0xMelkor Apr 14, 2023
d8f91b8
ConfParser trait is able to extract configurations out of a structur…
0xMelkor Apr 14, 2023
94a1846
cargo +nightly fmt
0xMelkor Apr 14, 2023
efbaacb
FuzzConfig implements ConfParser trait
0xMelkor Apr 14, 2023
6e770e0
InvariantConfig implements ConfParser trait
0xMelkor Apr 14, 2023
b1db51f
Parsing logic optimized
0xMelkor Apr 15, 2023
b0a0bfc
Configurations can be parsed from project compilation output
0xMelkor Apr 16, 2023
1d38733
E2E tests for inline configuration load
0xMelkor Apr 16, 2023
7d540c0
- ConfParser: parse fn is now try_merge
0xMelkor Apr 18, 2023
f3b25fc
Since TestOptions is no more Copy => TEST_OPTS constant is now a func…
0xMelkor Apr 18, 2023
0868d67
Inline config matcher uses stripped file prefixes to identify contracts
0xMelkor Apr 18, 2023
9f8672e
TestOptionsBuilder docs
0xMelkor Apr 18, 2023
09ff7db
Inline fuzz configs are applied during fuzz test execution + E2E tests
0xMelkor Apr 19, 2023
9fc175c
Inline invariant configs are applied during fuzz test execution + E2E…
0xMelkor Apr 19, 2023
211be7a
typos
0xMelkor Apr 19, 2023
b424e8d
Docs typo
0xMelkor Apr 28, 2023
abcedb7
cargo +nightly fmt
0xMelkor Apr 28, 2023
2119106
Added test for block comments
0xMelkor Apr 29, 2023
a4dc0eb
Renamed ConfParser to InlineConfigParser
0xMelkor Apr 29, 2023
7e3f10a
Use NodeType enum to match condition
0xMelkor Apr 29, 2023
9bd4c36
Use helper type to describe the HashMap key
0xMelkor May 1, 2023
9b534c8
Misconfigured line number added to the error - Need UNIT TESTS
0xMelkor May 2, 2023
0614569
Added very descriptive context to the parse error + unit test
0xMelkor May 2, 2023
6e79594
Emphasis on the "Invalid" keyword
0xMelkor May 2, 2023
651cf78
Big refactoring. Design is cleaner and more appropriate.
0xMelkor May 3, 2023
e77920a
natspec unit tests
0xMelkor May 3, 2023
8e0ef3e
Refactor Unit tests InvariantConfig + FuzzConfig
0xMelkor May 3, 2023
13448df
Noisy comment test
0xMelkor May 3, 2023
6eef43e
Use meaningful names
0xMelkor May 3, 2023
35e52b8
Profile validation implemented + Unit tests
0xMelkor May 4, 2023
380375e
Given a natspec, extract current profile configs + Unit tests
0xMelkor May 4, 2023
efdbb09
TestOptions instantiated with new validation rules - NEED TESTS
0xMelkor May 4, 2023
20f023f
Integration tests working
0xMelkor May 4, 2023
2c69b6e
Integration tests docs and typos
0xMelkor May 4, 2023
f506d38
Utility function to get all available profiles in config - unit tests
0xMelkor May 4, 2023
b8d43ea
try update PR
0xMelkor May 4, 2023
9a04762
Punctuation in config/src/inline/conf_parser.rs
0xMelkor May 6, 2023
ccb368b
Punctuation in config/src/inline/conf_parser.rs
0xMelkor May 6, 2023
95212c9
review: docs in cli/src/cmd/forge/test/mod.rs
0xMelkor May 6, 2023
222dd31
review: naming convention in InlineConfigParser
0xMelkor May 6, 2023
187ea1b
review: test renaming suggestion
0xMelkor May 6, 2023
0d5d19d
review: test renaming suggestion
0xMelkor May 6, 2023
3ccaf97
review: test renaming suggestion
0xMelkor May 6, 2023
2472b37
review: docs punctuation
0xMelkor May 6, 2023
9afae06
review: docs
0xMelkor May 6, 2023
8682f26
review: function internal utils function renaming + docs
0xMelkor May 6, 2023
4184a1c
review: get_fn_docs unit tests
0xMelkor May 6, 2023
9d38aa7
review: test renaming suggestion
0xMelkor May 6, 2023
c61f98e
review: clarify intent
0xMelkor May 6, 2023
2be61d6
review: document functions
0xMelkor May 6, 2023
fa06e62
review: applied case typos
0xMelkor May 7, 2023
616d107
FIX CI: Available profiles fallback to vec![current_profile] in case …
0xMelkor May 7, 2023
fa97f9a
Merge branch 'foundry-rs:master' into feat/ux-fuzz-invariant
0xMelkor May 7, 2023
b016d3d
cargo +nightly fmt
0xMelkor May 8, 2023
2360a1c
review: case typo
0xMelkor May 8, 2023
ed94236
review: remove double quotes from src line
0xMelkor May 9, 2023
fcee8ec
review: removed duplicated error msg; removed row:col:len detail (it …
0xMelkor May 9, 2023
eef9149
fix CI
0xMelkor May 10, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

23 changes: 17 additions & 6 deletions cli/src/cmd/forge/test/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,14 @@ use forge::{
identifier::{EtherscanIdentifier, LocalTraceIdentifier, SignaturesIdentifier},
CallTraceDecoderBuilder, TraceKind,
},
MultiContractRunner, MultiContractRunnerBuilder, TestOptions,
MultiContractRunner, MultiContractRunnerBuilder, TestOptions, TestOptionsBuilder,
};
use foundry_common::{
compile::{self, ProjectCompiler},
evm::EvmArgs,
get_contract_name, get_file_name,
};
use foundry_config::{figment, Config};
use foundry_config::{figment, get_available_profiles, Config};
use regex::Regex;
use std::{collections::BTreeMap, path::PathBuf, sync::mpsc::channel, thread, time::Duration};
use tracing::trace;
Expand Down Expand Up @@ -123,8 +123,6 @@ impl TestArgs {
// Merge all configs
let (mut config, mut evm_opts) = self.load_config_and_evm_opts_emit_warnings()?;

let test_options = TestOptions { fuzz: config.fuzz, invariant: config.invariant };

let mut filter = self.filter(&config);

trace!(target: "forge::test", ?filter, "using filter");
Expand All @@ -150,6 +148,19 @@ impl TestArgs {
compiler.compile(&project)
}?;

// Create test options from general project settings
// and compiler output
let project_root = &project.paths.root;
0xMelkor marked this conversation as resolved.
Show resolved Hide resolved
let toml = config.get_config_path();
let profiles = get_available_profiles(toml)?;

let test_options: TestOptions = TestOptionsBuilder::default()
.fuzz(config.fuzz)
.invariant(config.invariant)
.compile_output(&output)
.profiles(profiles)
.build(project_root)?;

// Determine print verbosity and executor verbosity
let verbosity = evm_opts.verbosity;
if self.gas_report && evm_opts.verbosity < 3 {
Expand All @@ -167,8 +178,8 @@ impl TestArgs {
.sender(evm_opts.sender)
.with_fork(evm_opts.get_fork(&config, env.clone()))
.with_cheats_config(CheatsConfig::new(&config, &evm_opts))
.with_test_options(test_options)
.build(project.paths.root, output, env, evm_opts)?;
.with_test_options(test_options.clone())
.build(project_root, output, env, evm_opts)?;

if self.debug.is_some() {
filter.args_mut().test_pattern = self.debug;
Expand Down
1 change: 1 addition & 0 deletions config/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ figment = { version = "0.10", features = ["toml", "env"] }
number_prefix = "0.4.0"
serde = { version = "1.0", features = ["derive"] }
serde_regex = "1.1.0"
serde_json = "1.0.95"
toml = { version = "0.7", features = ["preserve_order"] }
toml_edit = "0.19"

Expand Down
86 changes: 86 additions & 0 deletions config/src/fuzz.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@
use ethers_core::types::U256;
use serde::{Deserialize, Serialize};

use crate::inline::{
parse_config_u32, InlineConfigParser, InlineConfigParserError, INLINE_CONFIG_FUZZ_KEY,
};

/// Contains for fuzz testing
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
pub struct FuzzConfig {
Expand Down Expand Up @@ -35,6 +39,34 @@ impl Default for FuzzConfig {
}
}

impl InlineConfigParser for FuzzConfig {
fn config_key() -> String {
INLINE_CONFIG_FUZZ_KEY.into()
}

fn try_merge(&self, configs: &[String]) -> Result<Option<Self>, InlineConfigParserError> {
let overrides: Vec<(String, String)> = Self::get_config_overrides(configs);

if overrides.is_empty() {
return Ok(None)
}

// self is Copy. We clone it with dereference.
let mut conf_clone = *self;
Comment on lines +54 to +55
Copy link
Member

Choose a reason for hiding this comment

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

ok so if i understand correctly this is a copy that we're later mutating and returning? can we do self.clone()? This doesn't seem too idiomatic


for pair in overrides {
let key = pair.0;
let value = pair.1;
match key.as_str() {
"runs" => conf_clone.runs = parse_config_u32(key, value)?,
"max-test-rejects" => conf_clone.max_test_rejects = parse_config_u32(key, value)?,
_ => Err(InlineConfigParserError::InvalidConfigProperty(key))?,
}
}
Ok(Some(conf_clone))
}
}

/// Contains for fuzz testing
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
pub struct FuzzDictionaryConfig {
Expand Down Expand Up @@ -70,3 +102,57 @@ impl Default for FuzzDictionaryConfig {
}
}
}

#[cfg(test)]
mod tests {
use crate::{inline::InlineConfigParser, FuzzConfig};

#[test]
fn unrecognized_property() {
let configs = &["forge-config: default.fuzz.unknownprop = 200".to_string()];
let base_config = FuzzConfig::default();
if let Err(e) = base_config.try_merge(configs) {
assert_eq!(e.to_string(), "'unknownprop' is an invalid config property");
} else {
assert!(false)
}
}

#[test]
fn successful_merge() {
let configs = &["forge-config: default.fuzz.runs = 42424242".to_string()];
let base_config = FuzzConfig::default();
let merged: FuzzConfig = base_config.try_merge(configs).expect("No errors").unwrap();
assert_eq!(merged.runs, 42424242);
}

#[test]
fn merge_is_none() {
let empty_config = &[];
let base_config = FuzzConfig::default();
let merged = base_config.try_merge(empty_config).expect("No errors");
assert!(merged.is_none());
}

#[test]
fn merge_is_none_unrelated_property() {
let unrelated_configs = &["forge-config: default.invariant.runs = 2".to_string()];
let base_config = FuzzConfig::default();
let merged = base_config.try_merge(unrelated_configs).expect("No errors");
assert!(merged.is_none());
}

#[test]
fn override_detection() {
let configs = &[
"forge-config: default.fuzz.runs = 42424242".to_string(),
"forge-config: ci.fuzz.runs = 666666".to_string(),
"forge-config: default.invariant.runs = 2".to_string(),
];
let variables = FuzzConfig::get_config_overrides(configs);
assert_eq!(
variables,
vec![("runs".into(), "42424242".into()), ("runs".into(), "666666".into())]
);
}
}
206 changes: 206 additions & 0 deletions config/src/inline/conf_parser.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,206 @@
use regex::Regex;

use crate::{InlineConfigError, NatSpec};

use super::{remove_whitespaces, INLINE_CONFIG_PREFIX};

/// Errors returned by the [`InlineConfigParser`] trait.
#[derive(Debug, Clone, PartialEq, Eq, thiserror::Error)]
pub enum InlineConfigParserError {
/// An invalid configuration property has been provided.
/// The property cannot be mapped to the configuration object
#[error("'{0}' is an invalid config property")]
InvalidConfigProperty(String),
/// An invalid profile has been provided
#[error("'{0}' specifies an invalid profile. Available profiles are: {1}")]
InvalidProfile(String, String),
/// An error occurred while trying to parse an integer configuration value
#[error("Invalid config value for key '{0}'. Unable to parse '{1}' into an integer value")]
ParseInt(String, String),
/// An error occurred while trying to parse a boolean configuration value
#[error("Invalid config value for key '{0}'. Unable to parse '{1}' into a boolean value")]
ParseBool(String, String),
}

/// This trait is intended to parse configurations from
/// structured text. Foundry users can annotate Solidity test functions,
/// providing special configs just for the execution of a specific test.
///
/// An example:
///
/// ```solidity
/// contract MyTest is Test {
/// /// forge-config: default.fuzz.runs = 100
/// /// forge-config: ci.fuzz.runs = 500
/// function test_SimpleFuzzTest(uint256 x) public {...}
///
/// /// forge-config: default.fuzz.runs = 500
/// /// forge-config: ci.fuzz.runs = 10000
/// function test_ImportantFuzzTest(uint256 x) public {...}
/// }
/// ```
pub trait InlineConfigParser
where
Self: Clone + Default + Sized + 'static,
{
/// Returns a config key that is common to all valid configuration lines
/// for the current impl. This helps to extract correct values out of a text.
///
/// An example key would be `fuzz` of `invariant`.
fn config_key() -> String;

/// Tries to override `self` properties with values specified in the `configs` parameter.
///
/// Returns
/// - `Some(Self)` in case some configurations are merged into self.
/// - `None` in case there are no configurations that can be applied to self.
/// - `Err(InlineConfigParserError)` in case of wrong configuration.
fn try_merge(&self, configs: &[String]) -> Result<Option<Self>, InlineConfigParserError>;

/// Validates all configurations contained in a natspec that apply
/// to the current configuration key.
///
/// i.e. Given the `invariant` config key and a natspec comment of the form,
/// ```solidity
/// /// forge-config: default.invariant.runs = 500
/// /// forge-config: default.invariant.depth = 500
/// /// forge-config: ci.invariant.depth = 500
/// /// forge-config: ci.fuzz.runs = 10
/// ```
/// would validate the whole `invariant` configuration.
fn validate_configs(natspec: &NatSpec) -> Result<(), InlineConfigError> {
let config_key = Self::config_key();

let configs = natspec
.config_lines()
.into_iter()
.filter(|l| l.contains(&config_key))
.collect::<Vec<String>>();

Self::default().try_merge(&configs).map_err(|e| {
let line = natspec.debug_context();
InlineConfigError { line, source: e }
})?;

Ok(())
}

/// Given a list of `config_lines, returns all available pairs (key, value)
/// matching the current config key
///
/// i.e. Given the `invariant` config key and a vector of config lines
/// ```rust
/// let _config_lines = vec![
/// "forge-config: default.invariant.runs = 500",
/// "forge-config: default.invariant.depth = 500",
/// "forge-config: ci.invariant.depth = 500",
/// "forge-config: ci.fuzz.runs = 10"
/// ];
/// ```
/// would return the whole set of `invariant` configs.
/// ```rust
/// let _result = vec![
/// ("runs", "500"),
/// ("depth", "500"),
/// ("depth", "500"),
/// ];
/// ```
fn get_config_overrides(config_lines: &[String]) -> Vec<(String, String)> {
let mut result: Vec<(String, String)> = vec![];
let config_key = Self::config_key();
let profile = ".*";
let prefix = format!("^{INLINE_CONFIG_PREFIX}:{profile}{config_key}\\.");
let re = Regex::new(&prefix).unwrap();

config_lines
.iter()
.map(|l| remove_whitespaces(l))
.filter(|l| re.is_match(l))
.map(|l| re.replace(&l, "").to_string())
.for_each(|line| {
let key_value = line.split('=').collect::<Vec<&str>>(); // i.e. "['runs', '500']"
if let Some(key) = key_value.first() {
if let Some(value) = key_value.last() {
result.push((key.to_string(), value.to_string()));
}
}
});

result
}
}

/// Checks if all configuration lines specified in `natspec` use a valid profile.
///
/// i.e. Given available profiles
/// ```rust
/// let _profiles = vec!["ci", "default"];
/// ```
/// A configuration like `forge-config: ciii.invariant.depth = 1` would result
/// in an error.
pub fn validate_profiles(natspec: &NatSpec, profiles: &[String]) -> Result<(), InlineConfigError> {
for config in natspec.config_lines() {
if !profiles.iter().any(|p| config.starts_with(&format!("{INLINE_CONFIG_PREFIX}:{p}."))) {
let err_line: String = natspec.debug_context();
let profiles = format!("{profiles:?}");
Err(InlineConfigError {
source: InlineConfigParserError::InvalidProfile(config, profiles),
line: err_line,
})?
}
}
Ok(())
}

/// Tries to parse a `u32` from `value`. The `key` argument is used to give details
/// in the case of an error.
pub fn parse_config_u32(key: String, value: String) -> Result<u32, InlineConfigParserError> {
value.parse().map_err(|_| InlineConfigParserError::ParseInt(key, value))
}

/// Tries to parse a `bool` from `value`. The `key` argument is used to give details
/// in the case of an error.
pub fn parse_config_bool(key: String, value: String) -> Result<bool, InlineConfigParserError> {
value.parse().map_err(|_| InlineConfigParserError::ParseBool(key, value))
}

#[cfg(test)]
mod tests {
use crate::{inline::conf_parser::validate_profiles, NatSpec};

#[test]
fn can_reject_invalid_profiles() {
let profiles = ["ci".to_string(), "default".to_string()];
let natspec = NatSpec {
contract: Default::default(),
function: Default::default(),
line: Default::default(),
docs: r#"
forge-config: ciii.invariant.depth = 1
forge-config: default.invariant.depth = 1
"#
.into(),
};

let result = validate_profiles(&natspec, &profiles);
assert!(result.is_err());
}

#[test]
fn can_accept_valid_profiles() {
let profiles = ["ci".to_string(), "default".to_string()];
let natspec = NatSpec {
contract: Default::default(),
function: Default::default(),
line: Default::default(),
docs: r#"
forge-config: ci.invariant.depth = 1
forge-config: default.invariant.depth = 1
"#
.into(),
};

let result = validate_profiles(&natspec, &profiles);
assert!(result.is_ok());
}
}
Loading