-
Notifications
You must be signed in to change notification settings - Fork 2.1k
[WIP-RFC] Inspect EIP-7201 bucket Schema #11429
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thank you! I think this makes sense (not familiar with EIP-7201 but the regex matches in my initial pass through) left some comments, also could you please add an unit test with the sample you posted in description?
.collect() | ||
} | ||
|
||
fn short_hex(h: &str) -> String { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we should short this, is that a big issue if we display it entirely?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, it just distorts the table render.
This is the difference in outputs
tc@TCs-MacBook-Pro frxAccount-EIP7702 % /Users/tc/Documents/GitHub/foundry/target/debug/forge inspect src/FrxCommerce.sol:FrxCommerceAccount storageLayout
╭----------------+----------------------+---------------+--------+-------+---------------╮
| Name | Type | Slot | Offset | Bytes | Contract |
+========================================================================================+
| storage-bucket | struct EIP712Storage | 0xa16a46…d100 | 0 | 32 | EIP712Storage |
|----------------+----------------------+---------------+--------+-------+---------------|
| storage-bucket | struct NoncesStorage | 0x5ab42c…bb00 | 0 | 32 | NoncesStorage |
╰----------------+----------------------+---------------+--------+-------+---------------╯
tc@TCs-MacBook-Pro frxAccount-EIP7702 % /Users/tc/Documents/GitHub/foundry/target/debug/forge inspect src/FrxCommerce.sol:FrxCommerceAccount storageLayout
╭----------------+----------------------+--------------------------------------------------------------------+--------+-------+---------------╮
| Name | Type | Slot | Offset | Bytes | Contract |
+=============================================================================================================================================+
| storage-bucket | struct EIP712Storage | 0xa16a46d94261c7517cc8ff89f61c0ce93598e3c849801011dee649a6a557d100 | 0 | 32 | EIP712Storage |
|----------------+----------------------+--------------------------------------------------------------------+--------+-------+---------------|
| storage-bucket | struct NoncesStorage | 0x5ab42ced628888259c08ac98db1eb0cf702fc1501344311d8b100cd1bfe4bb00 | 0 | 32 | NoncesStorage |
╰----------------+----------------------+--------------------------------------------------------------------+--------+-------+---------------╯
Personally prefer the former but can change if you feel strongly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, then maybe we could reuse
foundry/crates/evm/core/src/decode.rs
Line 227 in e6098ac
fn trimmed_hex(s: &[u8]) -> String { |
@DaniPopes @zerosnacks wdyt?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comments less the short hex change should be implemented.
If you feel strongly about the non-short hex lmk and will change.
Otherwise the bytes32 makes the table a little wide which messes up the formatting if the terminal window is small.
.collect() | ||
} | ||
|
||
fn short_hex(h: &str) -> String { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, it just distorts the table render.
This is the difference in outputs
tc@TCs-MacBook-Pro frxAccount-EIP7702 % /Users/tc/Documents/GitHub/foundry/target/debug/forge inspect src/FrxCommerce.sol:FrxCommerceAccount storageLayout
╭----------------+----------------------+---------------+--------+-------+---------------╮
| Name | Type | Slot | Offset | Bytes | Contract |
+========================================================================================+
| storage-bucket | struct EIP712Storage | 0xa16a46…d100 | 0 | 32 | EIP712Storage |
|----------------+----------------------+---------------+--------+-------+---------------|
| storage-bucket | struct NoncesStorage | 0x5ab42c…bb00 | 0 | 32 | NoncesStorage |
╰----------------+----------------------+---------------+--------+-------+---------------╯
tc@TCs-MacBook-Pro frxAccount-EIP7702 % /Users/tc/Documents/GitHub/foundry/target/debug/forge inspect src/FrxCommerce.sol:FrxCommerceAccount storageLayout
╭----------------+----------------------+--------------------------------------------------------------------+--------+-------+---------------╮
| Name | Type | Slot | Offset | Bytes | Contract |
+=============================================================================================================================================+
| storage-bucket | struct EIP712Storage | 0xa16a46d94261c7517cc8ff89f61c0ce93598e3c849801011dee649a6a557d100 | 0 | 32 | EIP712Storage |
|----------------+----------------------+--------------------------------------------------------------------+--------+-------+---------------|
| storage-bucket | struct NoncesStorage | 0x5ab42ced628888259c08ac98db1eb0cf702fc1501344311d8b100cd1bfe4bb00 | 0 | 32 | NoncesStorage |
╰----------------+----------------------+--------------------------------------------------------------------+--------+-------+---------------╯
Personally prefer the former but can change if you feel strongly
.collect() | ||
} | ||
|
||
fn short_hex(h: &str) -> String { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, then maybe we could reuse
foundry/crates/evm/core/src/decode.rs
Line 227 in e6098ac
fn trimmed_hex(s: &[u8]) -> String { |
@DaniPopes @zerosnacks wdyt?
Motivation
Fixes #7662
Adds support for EIP-7201 storage bucket declarations support w/n
forge inspect PATH:Contract storageLayout
tableBefore
Usage buckets ought to be declared above the empty constructor argument for the implementation contract to be set for a given proxy eg:
Solution
Add support for w/n the foundry solc artifact parsing, to support these tags. This ensures that storage declared via name-spacing is visible to the end developer
PR Checklist
Disclaimers