update for lotus/actors network upgrade changes #260
Conversation
| FaultedSectors map[int][]uint64 | ||
| // FaultedSectors is a slice per-deadline faulty sectors. If the miner | ||
| // has no faulty sectors, this will be nil. | ||
| FaultedSectors [][]uint64 |
There was a problem hiding this comment.
Is this acceptable? I'm trying to avoid hard-coding the number of deadlines.
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| var mas miner.State |
There was a problem hiding this comment.
We didn't actually use this.
| } | ||
| // need to modify the VM invoker to add the puppet actor | ||
| chainValInvoker := vm.NewInvoker() | ||
| chainValInvoker.Register(puppet.PuppetActorCodeID, puppet.Actor{}, puppet.State{}) |
There was a problem hiding this comment.
This has been removed downstream. Did we need it?
There was a problem hiding this comment.
We need only the ChaosActor (which replaced the PuppetActor) AFAIK, and this is done at https://github.com/filecoin-project/lotus/blob/master/conformance/driver.go#L144
So I think this is fine, and we should actually remove tvx/lotus/driver.go as this has now been moved to Lotus - something we can do after this PR.
There was a problem hiding this comment.
Yes. I'm going to merge #246 which makes tvx use the lotus driver, and then we can erase the driver. I'll push to both PRs directly.
| @@ -51,15 +51,12 @@ func (d *Driver) ExecuteMessage(msg *types.Message, preroot cid.Cid, bs blocksto | |||
| Syscalls: mkFakedSigSyscalls(vm.Syscalls(ffiwrapper.ProofVerifier)), | |||
| CircSupplyCalc: nil, | |||
| BaseFee: BaseFee, | |||
| NtwkVersion: func(context.Context, abi.ChainEpoch) network.Version { | |||
| // TODO: Network upgrade. | |||
| return network.Version0 | |||
There was a problem hiding this comment.
For now, this is hard-coded at 0. But we'll probably want to add some form of lotus API call to get the actual version given a height.
There was a problem hiding this comment.
This driver is actually not used any longer. We incubated it here, but we didn't erase it when it was moved to Lotus. This change needs to be applied here: https://github.com/filecoin-project/lotus/blob/master/conformance/driver.go#L125-L133. Will take care of it.
OTOH, I guess a hardcoded value is OK here for now, but we will want to change this in the test vector. How can we resolve, given a chain epoch, what network version to return here? There must be a function on Lotus side for that? (we don't need it exposed over the RPC).
| @@ -45,9 +43,13 @@ func NewSurgeon(ctx context.Context, api api.FullNode, stores *Stores) *Surgeon | |||
| // compute a minimal state tree. In the future, thid method will dive into | |||
| // other system actors like the power actor and the market actor. | |||
| func (sg *Surgeon) GetMaskedStateTree(tsk types.TipSetKey, retain []address.Address) (cid.Cid, error) { | |||
| stateMap := adt.MakeEmptyMap(sg.stores.ADTStore) | |||
| // TODO: this will need to be parameterized on network version. | |||
| stateTree, err := state.NewStateTree(sg.stores.CBORStore, builtin.Version0) | |||
There was a problem hiding this comment.
Ditto. We'll need to get the version from the API.
| // TODO: The simplest approach would be to just add a "remove" method | ||
| // (as below) to the init actor abstraction. However, I'm not sure if we | ||
| // even _want_ this code. From what I can tell, lotus never removes | ||
| // address in the init actor, even if the underlying actor disappears. |
There was a problem hiding this comment.
Big TODO. This is a weird case because:
- We usually only read state using these abstractions (we rarely modify).
- We never delete actors from this map. So any function to do so would exist for oni (of course, maybe we should be deleting actors?).
There was a problem hiding this comment.
Alternatively, we could introduce per-version logic.
There was a problem hiding this comment.
We only use this to trim down the state tree when we extract a test vector from an on-chain message. This constructs a synthetic state tree, with only the entries that the vector effectively needs. For a given message M, we don't need the full address mapping inside the init actor.
There was a problem hiding this comment.
Ok, it turns out that init_.State is now an interface, and we can't freely manipulate the underlying struct. We're gonna have to type assert into a concrete struct.
|
Just a note on the background of this code - it originates from the Lotus CLI tools, and is used to record various state around proving deadlines, number of sectors per miner, etc. |
No description provided.