-
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
Initialize precompiles in UpdateParams #58
Conversation
62432f9
to
b38c516
Compare
357e30a
to
208eaa5
Compare
208eaa5
to
f7e532b
Compare
if err := k.SetParams(ctx, req.Params); err != nil { | ||
return nil, err | ||
} | ||
|
||
return &types.MsgUpdateParamsResponse{}, nil | ||
} | ||
|
||
func (k *Keeper) updatePrecompiles(ctx sdk.Context, newEnabledPrecompiles []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.
Should we return an error if the precompile contract isn't registered in the geth precompile modules (RegisteredModules())?
Also small optional thing: I think this could be moved to a separate file, just feels it could be separate from the msg_server
if err := k.updatePrecompiles(ctx, req.Params.EnabledPrecompiles); err != nil { | ||
return nil, err | ||
} | ||
|
||
if err := k.SetParams(ctx, req.Params); err != nil { | ||
return nil, 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.
updatePrecompiles()
naming a bit vague in usage here since it's also set in state in SetParams() - could optionally rename to something more specific / different from just updating/setting like initializePreocmpiles
a0d627e
to
b64b3fd
Compare
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days-before-close if no further activity occurs. |
No description provided.