Skip to content

Conversation

cabreraam
Copy link

@cabreraam cabreraam commented Jul 3, 2025

This PR addresses Issue #2522.

I use pre-existing fields of TypeAliasTopLevel to handle using read_storage on aliased ElementaryType instances, e.g.,

type MyUint64 is uint64;
contract C {
    struct S {
        MyUint64 y;
        MyUint64 z;
    }
    S s;
}

AFAICT, this doesn't cover user defined types, but it seems like the above reproducer can be addressed by attributes of the TypeAliasTopLevel class.

This is the output I observe when I run get_storage_layout():

INFO:Slither-read-storage:
Contract 'C'
C.s with type C.S is located at slot: 0

INFO:Slither-read-storage:
Name: s
Type: C.S
Slot: 0

INFO:Slither-read-storage:
Contract 'C'
C.s with type C.S is located at slot: 0

INFO:Slither-read-storage:
Struct Variable: y
Name: s.y
Type: MyUint64
Slot: 0

INFO:Slither-read-storage:
Contract 'C'
C.s with type C.S is located at slot: 0

INFO:Slither-read-storage:
Struct Variable: z
Name: s.z
Type: MyUint64
Slot: 0

I couldn't get the test I added to run on my local machine, so I'll wait for the CI to run/if anyone has suggestions for changing things.

cabreraam added 3 commits July 3, 2025 13:43
Signed-off-by: Anthony Cabrera <[email protected]>
Signed-off-by: Anthony Cabrera <[email protected]>
@cabreraam cabreraam requested a review from smonicas as a code owner July 3, 2025 19:33
@CLAassistant
Copy link

CLAassistant commented Jul 3, 2025

CLA assistant check
All committers have signed the CLA.

c = slither.get_contract_from_name("C")
srs = SlitherReadStorage(c, 20)
srs.get_all_storage_variables()
srs.get_storage_layout()
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 this perform some assertion?

Copy link

@qci-cabrera qci-cabrera Jul 29, 2025

Choose a reason for hiding this comment

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

Yes, for sure. I think when I was looking into this, it was just a skill issue, and I just wanted the test to run as some initial signal that was the changes I made didn't immediately break something.

I'd need to re-look at this, but absolutely, there needs to be some kind of assert to truly be meaningful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants