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(cast/forge): add label addresses in foundry config #6680

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 9 additions & 1 deletion crates/cheatcodes/src/config.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
use super::Result;
use crate::Vm::Rpc;
use alloy_primitives::Address;
use foundry_common::fs::normalize_path;
use foundry_compilers::{utils::canonicalize, ProjectPathsConfig};
use foundry_config::{
cache::StorageCachingConfig, fs_permissions::FsAccessKind, Config, FsPermissions,
ResolvedRpcEndpoints,
};
use foundry_evm_core::opts::EvmOpts;
use std::path::{Path, PathBuf};
use std::{
collections::HashMap,
path::{Path, PathBuf},
};

/// Additional, configurable context the `Cheatcodes` inspector has access to
///
Expand All @@ -30,6 +34,8 @@ pub struct CheatsConfig {
pub allowed_paths: Vec<PathBuf>,
/// How the evm was configured by the user
pub evm_opts: EvmOpts,
/// Address labels from config
pub labels: HashMap<Address, String>,
}

impl CheatsConfig {
Expand All @@ -51,6 +57,7 @@ impl CheatsConfig {
root: config.__root.0.clone(),
allowed_paths,
evm_opts,
labels: config.labels.clone(),
}
}

Expand Down Expand Up @@ -164,6 +171,7 @@ impl Default for CheatsConfig {
root: Default::default(),
allowed_paths: vec![],
evm_opts: Default::default(),
labels: Default::default(),
}
}
}
Expand Down
39 changes: 20 additions & 19 deletions crates/cheatcodes/src/inspector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,8 @@ impl Cheatcodes {
/// Creates a new `Cheatcodes` with the given settings.
#[inline]
pub fn new(config: Arc<CheatsConfig>) -> Self {
Self { config, fs_commit: true, ..Default::default() }
let labels = config.labels.clone();
Self { config, fs_commit: true, labels, ..Default::default() }
}

fn apply_cheatcode<DB: DatabaseExt>(
Expand Down Expand Up @@ -262,7 +263,7 @@ impl Cheatcodes {
if data.journaled_state.depth > 1 && !data.db.has_cheatcode_access(inputs.caller) {
// we only grant cheat code access for new contracts if the caller also has
// cheatcode access and the new contract is created in top most call
return created_address
return created_address;
}

data.db.allow_cheatcode_access(created_address);
Expand All @@ -279,12 +280,12 @@ impl Cheatcodes {

// Delay revert clean up until expected revert is handled, if set.
if self.expected_revert.is_some() {
return
return;
}

// we only want to apply cleanup top level
if data.journaled_state.depth() > 0 {
return
return;
}

// Roll back all previously applied deals
Expand Down Expand Up @@ -722,11 +723,11 @@ impl<DB: DatabaseExt> Inspector<DB> for Cheatcodes {
return match self.apply_cheatcode(data, call) {
Ok(retdata) => (InstructionResult::Return, gas, retdata.into()),
Err(err) => (InstructionResult::Revert, gas, err.abi_encode().into()),
}
};
}

if call.contract == HARDHAT_CONSOLE_ADDRESS {
return (InstructionResult::Continue, gas, Bytes::new())
return (InstructionResult::Continue, gas, Bytes::new());
}

// Handle expected calls
Expand Down Expand Up @@ -769,7 +770,7 @@ impl<DB: DatabaseExt> Inspector<DB> for Cheatcodes {
})
.map(|(_, v)| v)
}) {
return (return_data.ret_type, gas, return_data.data.clone())
return (return_data.ret_type, gas, return_data.data.clone());
}
}

Expand Down Expand Up @@ -826,7 +827,7 @@ impl<DB: DatabaseExt> Inspector<DB> for Cheatcodes {
if let Err(err) =
data.journaled_state.load_account(broadcast.new_origin, data.db)
{
return (InstructionResult::Revert, gas, Error::encode(err))
return (InstructionResult::Revert, gas, Error::encode(err));
}

let is_fixed_gas_limit = check_if_fixed_gas_limit(data, call.gas_limit);
Expand Down Expand Up @@ -857,7 +858,7 @@ impl<DB: DatabaseExt> Inspector<DB> for Cheatcodes {
debug!(target: "cheatcodes", address=%broadcast.new_origin, nonce=prev+1, prev, "incremented nonce");
} else if broadcast.single_call {
let msg = "`staticcall`s are not allowed after `broadcast`; use `startBroadcast` instead";
return (InstructionResult::Revert, Gas::new(0), Error::encode(msg))
return (InstructionResult::Revert, Gas::new(0), Error::encode(msg));
}
}
}
Expand Down Expand Up @@ -920,15 +921,15 @@ impl<DB: DatabaseExt> Inspector<DB> for Cheatcodes {
retdata: Bytes,
) -> (InstructionResult, Gas, Bytes) {
if call.contract == CHEATCODE_ADDRESS || call.contract == HARDHAT_CONSOLE_ADDRESS {
return (status, remaining_gas, retdata)
return (status, remaining_gas, retdata);
}

if data.journaled_state.depth() == 0 && self.skip {
return (
InstructionResult::Revert,
remaining_gas,
super::Error::from(MAGIC_SKIP).abi_encode().into(),
)
);
}

// Clean up pranks
Expand Down Expand Up @@ -970,7 +971,7 @@ impl<DB: DatabaseExt> Inspector<DB> for Cheatcodes {
(InstructionResult::Revert, remaining_gas, error.abi_encode().into())
}
Ok((_, retdata)) => (InstructionResult::Return, remaining_gas, retdata),
}
};
}
}

Expand Down Expand Up @@ -1039,7 +1040,7 @@ impl<DB: DatabaseExt> Inspector<DB> for Cheatcodes {
InstructionResult::Revert,
remaining_gas,
"log != expected log".abi_encode().into(),
)
);
} else {
// All emits were found, we're good.
// Clear the queue, as we expect the user to declare more events for the next call
Expand All @@ -1056,7 +1057,7 @@ impl<DB: DatabaseExt> Inspector<DB> for Cheatcodes {
// return a better error here
if status == InstructionResult::Revert {
if let Some(err) = diag {
return (status, remaining_gas, Error::encode(err.to_error_msg(&self.labels)))
return (status, remaining_gas, Error::encode(err.to_error_msg(&self.labels)));
}
}

Expand All @@ -1080,7 +1081,7 @@ impl<DB: DatabaseExt> Inspector<DB> for Cheatcodes {
// earlier error that happened first with unrelated information about
// another error when using cheatcodes.
if status == InstructionResult::Revert {
return (status, remaining_gas, retdata)
return (status, remaining_gas, retdata);
}

// If there's not a revert, we can continue on to run the last logic for expect*
Expand Down Expand Up @@ -1125,7 +1126,7 @@ impl<DB: DatabaseExt> Inspector<DB> for Cheatcodes {
"expected call to {address} with {expected_values} \
to be called {count} time{s}, but {but}"
);
return (InstructionResult::Revert, remaining_gas, Error::encode(msg))
return (InstructionResult::Revert, remaining_gas, Error::encode(msg));
}
}
}
Expand All @@ -1142,7 +1143,7 @@ impl<DB: DatabaseExt> Inspector<DB> for Cheatcodes {
"expected an emit, but the call reverted instead. \
ensure you're testing the happy path when using `expectEmit`"
};
return (InstructionResult::Revert, remaining_gas, Error::encode(msg))
return (InstructionResult::Revert, remaining_gas, Error::encode(msg));
}
}

Expand Down Expand Up @@ -1177,7 +1178,7 @@ impl<DB: DatabaseExt> Inspector<DB> for Cheatcodes {
call.caller == broadcast.original_caller
{
if let Err(err) = data.journaled_state.load_account(broadcast.new_origin, data.db) {
return (InstructionResult::Revert, None, gas, Error::encode(err))
return (InstructionResult::Revert, None, gas, Error::encode(err));
}

data.env.tx.caller = broadcast.new_origin;
Expand Down Expand Up @@ -1304,7 +1305,7 @@ impl<DB: DatabaseExt> Inspector<DB> for Cheatcodes {
Err(err) => {
(InstructionResult::Revert, None, remaining_gas, err.abi_encode().into())
}
}
};
}
}

Expand Down
6 changes: 5 additions & 1 deletion crates/cli/src/utils/cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -390,8 +390,12 @@ pub async fn handle_traces(
None
});

let labeled_addresses_in_config = config.labels.clone().into_iter();

let concatenated_addresses = labeled_addresses.chain(labeled_addresses_in_config);

let mut decoder = CallTraceDecoderBuilder::new()
.with_labels(labeled_addresses)
.with_labels(concatenated_addresses)
.with_signature_identifier(SignaturesIdentifier::new(
Config::foundry_cache_dir(),
config.offline,
Expand Down
39 changes: 37 additions & 2 deletions crates/config/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,9 @@ pub struct Config {
/// Should be removed once EvmVersion Cancun is supported by solc
pub cancun: bool,

/// Address labels
pub labels: HashMap<Address, String>,

/// The root path where the config detection started from, `Config::with_root`
#[doc(hidden)]
// We're skipping serialization here, so it won't be included in the [`Config::to_string()`]
Expand Down Expand Up @@ -411,7 +414,7 @@ impl Config {

/// Standalone sections in the config which get integrated into the selected profile
pub const STANDALONE_SECTIONS: &'static [&'static str] =
&["rpc_endpoints", "etherscan", "fmt", "doc", "fuzz", "invariant"];
&["rpc_endpoints", "etherscan", "fmt", "doc", "fuzz", "invariant", "labels"];

/// File name of config toml file
pub const FILE_NAME: &'static str = "foundry.toml";
Expand Down Expand Up @@ -1831,6 +1834,7 @@ impl Default for Config {
build_info_path: None,
fmt: Default::default(),
doc: Default::default(),
labels: Default::default(),
__non_exhaustive: (),
__warnings: vec![],
}
Expand Down Expand Up @@ -4432,7 +4436,7 @@ mod tests {
"foundry.toml",
r"
[default]
[profile.default.optimizer_details]
[profile.default.optimizer_details]
",
)?;

Expand All @@ -4442,4 +4446,35 @@ mod tests {
Ok(())
});
}

#[test]
fn test_parse_labels() {
figment::Jail::expect_with(|jail| {
jail.create_file(
"foundry.toml",
r#"
[labels]
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't the labels in config be chain specific though?

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 thought that most address labels for evm chains can be safely reused because contract addresses created using CREATE2 and externally owned accounts created from same private key or seed phrase share the same address over all chains. Tagging Uniswap V3: Factory over all chains would be cumbersome. I think it would be much better if there is chain-specific label section and globally shared label section to keep chain-specific labels from universally used addresses, but I just kept this PR's scope into global labels only.

0x1F98431c8aD98523631AE4a59f267346ea31F984 = "Uniswap V3: Factory"
0xC36442b4a4522E871399CD717aBDD847Ab11FE88 = "Uniswap V3: Positions NFT"
"#,
)?;

let config = Config::load();
assert_eq!(
config.labels,
HashMap::from_iter(vec![
(
Address::from_str("0x1F98431c8aD98523631AE4a59f267346ea31F984").unwrap(),
"Uniswap V3: Factory".to_string()
),
(
Address::from_str("0xC36442b4a4522E871399CD717aBDD847Ab11FE88").unwrap(),
"Uniswap V3: Positions NFT".to_string()
),
])
);

Ok(())
});
}
}
14 changes: 7 additions & 7 deletions crates/evm/evm/src/inspectors/stack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,7 @@ impl InspectorStack {
if new_status != status ||
(new_status == InstructionResult::Revert && new_retdata != retdata)
{
return (new_status, new_gas, new_retdata)
return (new_status, new_gas, new_retdata);
}
}
);
Expand All @@ -371,7 +371,7 @@ impl<DB: DatabaseExt> Inspector<DB> for InspectorStack {
// Allow inspectors to exit early
if interpreter.instruction_result != res {
#[allow(clippy::needless_return)]
return
return;
}
}
);
Expand All @@ -395,7 +395,7 @@ impl<DB: DatabaseExt> Inspector<DB> for InspectorStack {
// Allow inspectors to exit early
if interpreter.instruction_result != res {
#[allow(clippy::needless_return)]
return
return;
}
}
);
Expand Down Expand Up @@ -433,7 +433,7 @@ impl<DB: DatabaseExt> Inspector<DB> for InspectorStack {
// Allow inspectors to exit early
if interpreter.instruction_result != res {
#[allow(clippy::needless_return)]
return
return;
}
}
);
Expand All @@ -460,7 +460,7 @@ impl<DB: DatabaseExt> Inspector<DB> for InspectorStack {
// Allow inspectors to exit early
#[allow(clippy::needless_return)]
if status != InstructionResult::Continue {
return (status, gas, retdata)
return (status, gas, retdata);
}
}
);
Expand Down Expand Up @@ -509,7 +509,7 @@ impl<DB: DatabaseExt> Inspector<DB> for InspectorStack {

// Allow inspectors to exit early
if status != InstructionResult::Continue {
return (status, addr, gas, retdata)
return (status, addr, gas, retdata);
}
}
);
Expand Down Expand Up @@ -546,7 +546,7 @@ impl<DB: DatabaseExt> Inspector<DB> for InspectorStack {
);

if new_status != status {
return (new_status, new_address, new_gas, new_retdata)
return (new_status, new_address, new_gas, new_retdata);
}
}
);
Expand Down
1 change: 1 addition & 0 deletions crates/forge/tests/cli/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ forgetest!(can_extract_config_values, |prj, cmd| {
fmt: Default::default(),
doc: Default::default(),
fs_permissions: Default::default(),
labels: Default::default(),
cancun: true,
__non_exhaustive: (),
__warnings: vec![],
Expand Down
Loading