feat(cheatcodes): getArtifactPathByCode and getArtifactPathByDeployedCode
#8938
Conversation
yash-atreya
requested review from
DaniPopes,
klkvr and
mattsse
as code owners
|
this is great to have but doesn't really solve #7634 which requests an address (or bytecode) to be passed as param and then match the deployed bytecode > use bytecode_diff_score to identify the contract / it's path ( |
|
re failing test - we're at a point when a new cheatcode added breaks it so seed should be updated, pls see https://github.com/foundry-rs/foundry/pull/8882/files#diff-d6c9e2f7cec6e686d5693fd6c6bd6400e4f617e1dea2695df29cab51549ba797R163 |
+1. Added |
yeah, don't see a way to match by address, but looks like |
crates/cheatcodes/spec/src/vm.rs
Outdated
| /// Gets the artifact path from the creation code. | ||
| #[cheatcode(group = Filesystem)] | ||
| function getArtifactPath(bytes calldata creationCode) external view returns (string memory path); |
There was a problem hiding this comment.
imo it's not obvious what this function accepts as argument. should we make this more explicit, like getArtifactPathByCode or smth @grandizzy ?
There was a problem hiding this comment.
agree that getArtifactPathByCode is cleaner, also since for getCode we have getCode(artifactPath) and getDeployedCode(artifactPath) maybe we need to add a getArtifactPathByDeployedCode too? (where the only difference is that ContractsByArtifact.find_by_deployed_code instead find_by_creation_code is called)
@yash-atreya wdyt?
There was a problem hiding this comment.
+1 to both.
Makes sense to name them explicitly, also avoids signature conflicts.
crates/forge/tests/it/fuzz.rs
Outdated
| @@ -160,7 +160,7 @@ async fn test_scrape_bytecode() { | |||
| let filter = Filter::new(".*", ".*", ".*fuzz/FuzzScrapeBytecode.t.sol"); | |||
| let mut runner = TEST_DATA_DEFAULT.runner(); | |||
| runner.test_options.fuzz.runs = 2000; | |||
| runner.test_options.fuzz.seed = Some(U256::from(100u32)); | |||
| runner.test_options.fuzz.seed = Some(U256::from(120u32)); | |||
There was a problem hiding this comment.
Re: #8938 (comment)
cc @grandizzy
There was a problem hiding this comment.
when we fuzz we use test contract bytecode which changed with adding the new cheatcode so there are now different fuzzed inputs, had similar here: https://github.com/foundry-rs/foundry/pull/8882/files#diff-d6c9e2f7cec6e686d5693fd6c6bd6400e4f617e1dea2695df29cab51549ba797R163
ref #8115
There was a problem hiding this comment.
I don't see how changing an interface (Vm) change bytecode of an unrelated contract, where can I see this error? If this is actually a problem when adding cheatcodes, this should be hotfixed ASAP
There was a problem hiding this comment.
the issue can be reproduced in master by appending a line like function writeToml() external; in testdata/cheats/Vm.sol and cargo test test_scrape_bytecode will start failing. It can make it pass if bumping fuzz runs from 2000 to 5000. let me create a simpler repro to expose such
There was a problem hiding this comment.
I made some more tests with a simpler repro and figured out that happens because test has target contract in test contract, so they get inline, if they're split then issue is not reproducible and test pass without changing the seed, here's a draft PR #8953
@DaniPopes
There was a problem hiding this comment.
this change is no longer needed, the flaky test was fixed and PR updated
grandizzy
changed the title
cheatcodes): vm.getArtifactPathcheatcodes): vm.getArtifactPathByCode/getArtifactPathByDeployedCode
grandizzy
changed the title
cheatcodes): vm.getArtifactPathByCode/getArtifactPathByDeployedCodecheatcodes): getArtifactPathByCode and getArtifactPathByDeployedCode
Motivation
Closes #7634
Solution
Adds new cheatcodes
function getArtifactPathByCode(bytes calldata code) external returns (string memory path);function getArtifactPathByDeployedCode(bytes calldata deployedCode) external returns (string memory path);