feat(cast/storage): fetch address state if no slot is provided #3335
Conversation
mattsse
left a comment
mattsse
left a comment
There was a problem hiding this comment.
this is great! tysm
some early suggestions :)
| let match_code = |artifact: &ConfigurableContractArtifact| -> Option<bool> { | ||
| let bytes = | ||
| artifact.deployed_bytecode.as_ref()?.bytecode.as_ref()?.object.as_bytes()?; | ||
| Some(bytes == &address_code) |
There was a problem hiding this comment.
In the case of an external library, will the object be linked here? If it isn't then this match will always fail
| // Get deployed bytecode at given address | ||
| let address_code = provider.get_code(address, block).await?; | ||
| if address_code.is_empty() { | ||
| eyre::bail!("Provided address has no deployed code and thus no storage"); |
There was a problem hiding this comment.
Technically if the contract selfdestructed, then it could have no code but have storage. I don't think there would be any way to get the storage reliably though, geth does have a RPC endpoint debug_storageRangeAt but its not part of the official json rpc spec
There was a problem hiding this comment.
You're right, there's also parity_listStorageKeys, we could check if they are present and throw when they're not
|
I think this is almost done, just need a few things:
|
|
I've been using this branch to get the storage layout for deployed contracts, and it's been super helpful. Would love to get this PR out of draft and merged if possible! I think it's ok to to handle some of the above things in a future PR if they turn out to be a lot of work |
mattsse
left a comment
mattsse
left a comment
There was a problem hiding this comment.
this looks pretty good.
love the tests and example docs.
judging from the tests, this seems to be working.
smol nit, otherwise lgtm
| let value = value?.into_uint(); | ||
| let t = layout.types.get_mut(&slot.storage_type).expect("Bad storage"); | ||
| // TODO: Better format values according to their Solidity type | ||
| t.value = Some(format!("{value:?}")); |
There was a problem hiding this comment.
can we pretty format here or is can we remove the TODO?
There was a problem hiding this comment.
not sure how to format a H256 into solidity values. im assuming we already do this somewhere in forge when displaying traces
| let auto_detect = version < MIN_SOLC; | ||
|
|
||
| // Create a new temp project | ||
| // TODO: Cache instead of using a temp directory: metadata from Etherscan won't change |
There was a problem hiding this comment.
this isn't very important and we shouldn't cache this in .foundry imo
There was a problem hiding this comment.
yeah its more so you dont have to download+compile every time when making multiple queries on the same address
cli/tests/it/cast.rs
Outdated
| }); | ||
|
|
||
| // tests that the `cast storage` command works correctly | ||
| casttest!(cast_storage_succeeds, |_: TestProject, mut cmd: TestCommand| { |
There was a problem hiding this comment.
For some reason the etherscan key is not read here?
There was a problem hiding this comment.
from what I'm seeing in other tests, they only run when the api key/private key env var is set (https://github.com/foundry-rs/foundry/blob/master/cli/tests/it/verify.rs)
and they're set only in the live tests
There was a problem hiding this comment.
@mattsse let's bring the etherscan api key in scope here? Any reason not to?
There was a problem hiding this comment.
don't think so, but unsure if the required key secrets are set. will check and enable
There was a problem hiding this comment.
@mattsse when you get a chance, let's set the secrets here and merge
There was a problem hiding this comment.
I don't think I've access to set secrets.
This reverts commit 3d16d55. This was not the right fix as it triggered all integration tests to run.
5 participants
Motivation
closes: #3332
blocked by: gakonst/ethers-rs#1747 and #3395Solution
cast storage <address> <slot>cast storage <address>