add RegistryFactory and deprecate global action, auth and output registries in VMs #1624
Conversation
tsachiherman
requested review from
aaronbuchwald and
joshua-kim
as code owners
chain/dependencies.go
Outdated
| @@ -287,3 +287,6 @@ type AuthFactory interface { | |||
| MaxUnits() (bandwidth uint64, compute uint64) | |||
| Address() codec.Address | |||
| } | |||
|
|
|||
| // RegistryFactory is the factory function, provided to the VM initializer that provides the registries for teh actions, auth and output. | |||
| type RegistryFactory func() (actionRegistry ActionRegistry, authRegistry AuthRegistry, outputRegistry OutputRegistry) | |||
There was a problem hiding this comment.
Would it be better to have a struct here?
type Registry struct {
action ActionRegistry
auth AuthRegistry
output OutputRegistry
}| type RegistryFactory func() (actionRegistry ActionRegistry, authRegistry AuthRegistry, outputRegistry OutputRegistry) | |
| type RegistryFactory func() Registry |
I'm not sure if we plan on extending this, but I think backwards-compatible changes would be easier with a struct.
There was a problem hiding this comment.
sounds good to me.
chain/dependencies.go
Outdated
| type Registry struct { | ||
| Action ActionRegistry | ||
| Auth AuthRegistry | ||
| Output OutputRegistry | ||
| } | ||
|
|
||
| // RegistryFactory is the factory function, provided to the VM initializer that provides the registries for the actions, auth and output. | ||
| type RegistryFactory func() Registry |
There was a problem hiding this comment.
Can we change from an interface returning a struct of all of these types to an interface that supports ActionRegistry(), AuthRegistry(), and OutputRegistry() individually?
We can have a struct that implements that interface and takes the exact same fields, that way we can pass it around more easily and call a single function on the interface to get the value we care about.
This will also make it more ergonomic when we have a type that needs only the one of those values. The callee can then declare its own dependency and the caller can pass in the existing interface.
| type Registry struct { | ||
| actionRegistry chain.ActionRegistry | ||
| authRegistry chain.AuthRegistry | ||
| outputRegistry chain.OutputRegistry | ||
| } | ||
|
|
||
| func NewRegistry(action chain.ActionRegistry, auth chain.AuthRegistry, output chain.OutputRegistry) *Registry { |
There was a problem hiding this comment.
Could we define the struct implementing next to the interface in the chain package?
| Registry: *NewRegistry( | ||
| registry.ActionRegistry(), | ||
| registry.AuthRegistry(), | ||
| registry.OutputRegistry()), |
There was a problem hiding this comment.
Why create a registry from the existing registry? Can we just set the registry to the chain.Registry value that was passed in?
What ?
add RegistryFactory and deprecate global action, auth and output registries in VMs
Why ?
Avoid creation of global registries and pass these along the call stack as part of the server context.