Skip to content

Conversation

@b1u3h4t
Copy link

@b1u3h4t b1u3h4t commented Jul 27, 2022

Motivation

After deploying the contract, it is often necessary to check whether the parameters are set successfully, so here is a simple method to query the value of the slot corresponding to the contract variable.

Solution

Obtain the value of the specified slot in the contract through getStorageAt, and then print it in a developer-friendly way to facilitate the review of related variables.

TODO

  • fetch etherscan source and compile
  • check upgradeable contract and implementation contract bytecode
  • storage value for mapping, array and so on
  • more tests

Copy link
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

this is a good idea,

but I think this should be opt in

also I'd appreciate a simple forgetest! integration test for this, lmk if you need some pointers, have a look at the existing ones,

probably needs some setup up via

forgetest_async!(
    test_inspect,
    |prj: TestProject, cmd: TestCommand| async move {
        // this will launch anvil
        let (api, handle) = spawn(NodeConfig::test()).await;
        
        // deploy
        // inspect
}

@mattsse mattsse added the T-feature Type: feature label Jul 27, 2022
Comment on lines 158 to 165
];
if let Some(ref provider) = provider {
let location = TxHash::from_low_u64_be(slot.slot.parse::<u64>()?);
let value =
provider.get_storage_at(contract_addr, location, None).await?;
row.push(format!("{:?}", value));
}
table.add_row(row);
Copy link
Member

Choose a reason for hiding this comment

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

great!

@onbjerg
Copy link
Contributor

onbjerg commented Jul 28, 2022

Imo this should be in Cast where we also already have cast storage - could just add a flag or something to it to get this functionality

@b1u3h4t
Copy link
Author

b1u3h4t commented Jul 28, 2022

Imo this should be in Cast where we also already have cast storage - could just add a flag or something to it to get this functionality

It makes sense, maybe we need to expose more forge-related contract interfaces to cast, so that cast can expand more transaction-related details from a contract perspective.

@mds1
Copy link
Collaborator

mds1 commented Jul 29, 2022

I agree this is useful, but big +1 that this should not be part of forge. forge inspect's purpose is to simply make solc output such as storage layout more accessible and readable.

Right now cast storage <address> <slot> prints the the specified slot. Expanding on @onbjerg's suggestion, I'd propose cast storage <address> (i.e. when no slot is specified) to behave as follows:

  1. Fetch the bytecode of the provided address.
  2. If we're in a foundry project, check artifacts for matching bytecode + recompile if needed to get storage layout.
  3. Otherwise, fetch source from etherscan and compile it with the specified settings to get storage layout (this is also useful for debugging txs against a live network where we'd recompile to get source maps).
  4. Print variable names, slots, sizes, offsets, and decoded values in a table similar to the existing one that you get with forge inspect MyContract storage-layout --pretty.

Note this PR does not properly handle dynamic types. It will print the length of an array for array types, and zero for mappings because it doesn't know the keys. I think that's ok for the first version, but I'd love for the next iteration to figure out array lengths and all mapping keys to print the all storage. @banteg has a POC for doing this here: https://github.com/banteg/storage-layout

@tynes
Copy link
Contributor

tynes commented Aug 2, 2022

Definitely +1 for @mds1 cast storage <address> proposal, I think it would also be cool to have a --blocknumber param that lets you do historical queries so that you can write a script that can see how state changes in a contract over a range of blocks

@mds1
Copy link
Collaborator

mds1 commented Aug 3, 2022

Agreed, --block <blockNumber/earliest/latest/pending> is the syntax used elsewhere in cast which we should support here too

@gakonst
Copy link
Member

gakonst commented Aug 15, 2022

hey @b1u3h4t what are the best next steps here?

(we at a minimum want a unit test here)

@rkrasiuk rkrasiuk added C-forge Command: forge C-cast Command: cast and removed C-forge Command: forge labels Aug 23, 2022
@onbjerg
Copy link
Contributor

onbjerg commented Aug 23, 2022

Ping @b1u3h4t

@gakonst
Copy link
Member

gakonst commented Sep 19, 2022

On further thought I think we should go wiht @mds1 suggestion. Closing given stale, and we can revisit! Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-cast Command: cast T-feature Type: feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants