-
Notifications
You must be signed in to change notification settings - Fork 27
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
Add stylus programSize and programMemoryFootprint precompiles #120
Conversation
Codecov Report
@@ Coverage Diff @@
## stylus #120 +/- ##
==========================================
- Coverage 56.47% 56.44% -0.04%
==========================================
Files 276 276
Lines 42665 42684 +19
==========================================
- Hits 24094 24091 -3
- Misses 16111 16118 +7
- Partials 2460 2475 +15 |
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.
Nice work, just some minor things :)
system_tests/program_test.go
Outdated
programMemoryFootprint, err := arbWasm.ProgramMemoryFootprint(nil, programAddress) | ||
Require(t, err) | ||
if programMemoryFootprint != 1 { | ||
Fatal(t, "unexpected memory footprint", programMemoryFootprint) | ||
} |
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.
Lets move this to the memory test, which has an example that's known to be 120 pages :)
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.
👍
system_tests/program_test.go
Outdated
programSize, err := arbWasm.ProgramSize(nil, programAddress) | ||
Require(t, err) | ||
if programSize < 5000 || programSize > 20000 { | ||
Fatal(t, "unexpected size", programSize) | ||
} |
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 think we know the exact size and could check against it
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.
👍
arbos/programs/programs.go
Outdated
func (p Programs) CodehashSize(codeHash common.Hash) (uint32, error) { | ||
program, err := p.deserializeProgram(codeHash) | ||
// wasmSize represents the number of half kb units, return as bytes | ||
return uint32(program.wasmSize) * 512, err | ||
} | ||
|
||
func (p Programs) CodehashMemoryFootprint(codeHash common.Hash) (uint16, error) { | ||
program, err := p.deserializeProgram(codeHash) | ||
return program.footprint, err | ||
} | ||
|
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.
Let's name these ProgramSize
and ProgramMemoryFootprint
, but still have them take codehashes
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.
👍
LGTM |
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.
Looks good!
Requires