-
Notifications
You must be signed in to change notification settings - Fork 7
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
Extend x/evm module params with enabled_precompiles #55
Conversation
0539c29
to
b9a0598
Compare
dea0f38
to
5b6405e
Compare
5b6405e
to
f385a8d
Compare
d04de80
to
6e25d37
Compare
1552ed6
to
29122ea
Compare
556bfe6
to
7905fcf
Compare
7905fcf
to
30aaf56
Compare
0af7b85
to
c8f023e
Compare
43ad1cf
to
770e0ba
Compare
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.
just a small nit re: method names
great test coverage!
x/evm/types/params.go
Outdated
// IsLondon returns if london hardfork is enabled. | ||
func IsLondon(ethConfig *params.ChainConfig, height int64) bool { | ||
return ethConfig.IsLondon(big.NewInt(height)) | ||
} | ||
|
||
func CheckIfEnabledPrecompilesAreRegistered(registeredModules []precompile_modules.Module, enabledPrecompiles []string) error { |
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.
need comment on public method
also maybe a name like ValidateEnabledPrecompiles
or ValidatePrecompileRegistration
is more idiomatic (and future proof?)
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.
agree, ValidatePrecompileRegistration
sounds better
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.
Fixed: c2e188f
x/evm/types/params.go
Outdated
return nil | ||
} | ||
|
||
func checkIfSortedInBytesRepr(addrs []common.Address) error { |
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.
nit: validate
might be more consistent naming here and below than checkIf
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.
agree, validateSortingInBytesRepr
and validateUniquenessInBytesRepr
sounds better
also validateOrderInBytesRepr
sounds good, but I went with validateSortingInBytesRepr
Fixed: 27126aa
fe885c3
to
e749630
Compare
e749630
to
013442b
Compare
x/evm
params withenabled_precompiles
enabled_precompiles
inInitGenesis