-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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(op-challenger): Refactor out Global Config #11309
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.
Its been a while so not sure how well this lines up with my original comments, but looking at it with fresh eyes, my suggestion would be to simplify things this way: #11348
Then we maintain a single vm.Config
and just have a helper object that we plug in to calculate the arguments required for the oracle server execution which is really all we're trying to do.
} | ||
|
||
var _ VmConfig = (*KonaVmConfig)(nil) | ||
|
||
func NewKonaVmConfig(vmConfig Config) *KonaVmConfig { | ||
func NewKonaVmConfig(cfg *Config) *KonaVmConfig { |
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.
We've been pretty consistent in passing around vm.Config
directly instead of via a pointer - would be good to stick with that since it's a pretty small object. Will save adding &
in a lot of places in this PR.
func (s *OpProgramVmConfig) Cfg() Config { | ||
return s.Config | ||
} | ||
|
||
func (s *OpProgramVmConfig) FillHostCommand(args []string, dataDir string, inputs utils.LocalGameInputs) ([]string, error) { | ||
if args == nil { | ||
return nil, errors.New("args is nil") | ||
} | ||
|
||
args = append(args, | ||
"--", |
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'd still say that this should be added by the executor
not the program.
Description
Refactors the global
Config
type out of theVmConfig
. Although this duplicates a bit of Config fields, they will be consistent since theVmConfig
is constructed using theConfig
object.This is a minimal change so we don't need to further refactor the challenger's top-level
config.Config
object (adding new fields for variousVmConfig
s).