Skip to content
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

feat: checks on upper bounds of contract storage sizes #169

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

@TilakMaddy TilakMaddy marked this pull request as ready for review December 14, 2024 14:34
@TilakMaddy TilakMaddy marked this pull request as draft December 14, 2024 15:21
@TilakMaddy TilakMaddy marked this pull request as ready for review December 14, 2024 15:23
@TilakMaddy TilakMaddy changed the title feat: pre-checks for upper bound of contracts' sizes feat: checks on the upper bound of contract size Dec 14, 2024
@TilakMaddy TilakMaddy changed the title feat: checks on the upper bound of contract size feat: checks on upper bounds of contract storage sizes Dec 14, 2024
crates/sema/src/typeck/mod.rs Outdated Show resolved Hide resolved
crates/sema/src/typeck/mod.rs Outdated Show resolved Hide resolved
crates/sema/src/typeck/mod.rs Outdated Show resolved Hide resolved
crates/sema/src/typeck/mod.rs Outdated Show resolved Hide resolved
@TilakMaddy TilakMaddy requested a review from DaniPopes December 15, 2024 09:35
}
}

if gcx.sess.storage_sz_ub {
Copy link
Member

Choose a reason for hiding this comment

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

let's call it like "print-contract-storage-sizes"

Copy link
Contributor Author

@TilakMaddy TilakMaddy Dec 15, 2024

Choose a reason for hiding this comment

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

We only check the upper bound of storage size, so I'm afraid that wouldn't be accurate depiction of what users are looking for.

for example:
If there's

contract C {
  int128 a;
  int128 b;
}

Currently, this totals to "2", which is less than 2^256 so passes the check. (this check is pre-condition to make sure the slot allocation algorithm will work properly later)

Whereas someone wanting to print storage size would expect 128 + 128 = 256 bits so 1 slot. (because of how solidity allocates the slots)

If we go with this, do we make another flag for the actual contract storage size then?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@DaniPopes isn't this more like "print-contract-max-storage-sizes" ?

Copy link
Member

Choose a reason for hiding this comment

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

@DaniPopes isn't this more like "print-contract-max-storage-sizes" ?

sure

Copy link
Member

Choose a reason for hiding this comment

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

If we go with this, do we make another flag for the actual contract storage size then?

This probably will go with the actual storage implementation later on, the "storage-layout" output

crates/cli/src/lib.rs Show resolved Hide resolved
@@ -138,6 +138,9 @@ pub struct UnstableFeatures {
#[cfg(test)]
#[arg(long)]
test_value: Option<usize>,

#[arg(long)]
pub storage_sz_ub: bool,
Copy link
Member

Choose a reason for hiding this comment

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

please move to above help

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.

2 participants