-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
small adjustments following network upgradability changes #3996
Conversation
@@ -130,6 +132,7 @@ func (d *Driver) ExecuteMessage(bs blockstore.Blockstore, preroot cid.Cid, epoch | |||
Syscalls: mkFakedSigSyscalls(vm.Syscalls(ffiwrapper.ProofVerifier)), // TODO always succeeds; need more flexibility. | |||
CircSupplyCalc: nil, | |||
BaseFee: BaseFee, | |||
NtwkVersion: sm.GetNtwkVersion, |
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.
Right now, GetNtwkVersion
is a pure function. If we expect it to actually depend on state, this will get more complicated.
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.
Any way we could construct an actual state manager? I guess this is just for testing so it's not too bad.
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.
NewStateManager
requires a ChainStore
, but the kind of vector that gets executed here (message-class vector) doesn't care about the chain. I guess we could create an empty ChainStore
, but the result at this stage would be the same.
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 guess we have GetNtwkVersion
as part of the StateManager
to be able to upgrade network versions conditionally, based on things like on-chain voting/signalling?
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.
For those cases, we would regardless need to provide a stub/test-controlled NtwkVersionGetter
, at least for this kind of vector.
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 guess we have GetNtwkVersion as part of the StateManager to be able to upgrade network versions conditionally, based on things like on-chain voting/signalling?
Correct
Refer to commit log for breakdown.