-
Notifications
You must be signed in to change notification settings - Fork 4
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
custom precompile capabilities #36
base: main
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.
generally lgtm just a few repo level questions of where the gitignore files and such are coming form
precompile/README.md
Outdated
|
||
4. Implement the precompile in Go at `./contracts/<precompile>/<precompile.go>`. | ||
- The struct should implement the `StatefulPrecompiledContract` interface | ||
- You must methods defined in the Solidity interface |
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 there is a missing word here
caller common.Address, | ||
value *uint256.Int, | ||
suppliedGas uint64, | ||
readOnly bool, |
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.
what is the instance in which this would be true?
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.
it's true if the precompile is called with the STATICCALL
opcode. State is not allowed to change during a STATICCALL
op
precompile/bindings/.gitignore
Outdated
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.
why do we need the .gitignore
here?
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.
it's here to ensure this dir exists but nothing in it can be tracked. the precompile/gen.sh
scripts needs it to exist. Probably cleaner to just have that script ensure it exists and ignore the dir in a .gitignore
|
||
// return precompiles that are enabled at height | ||
func PrecompileConfig(chainConfig *params.ChainConfig, height uint64, timestamp uint64) precompile.PrecompileMap { | ||
return NullPrecompiles |
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.
should we add a commented out line of code here on what enabling would look like?
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.
it is outlined in the readme but here probably doesn't hurt
Summary
Adds a precompile manager that makes it easy to add new precompiles to the evm by writing an implementation in go which corresponds to a defined solidity interface. These precompiles can optionally have state in the evm state db.
Background
This updates a PR from the old astria
go-ethereum
repo to the latest changes in the newerastria-geth
repo.The precompile manager is heavily utilized by Forma and potentially useful for other custom evm rollups. @itamarreif may have some ideas of things he might like to do with it.
There are no custom precompiles enabled by default.
Changes from old PR