FIP0031 refinements #8429
FIP0031 refinements #8429
Conversation
There was a problem hiding this comment.
Can we add instructions to https://github.com/filecoin-project/lotus/blob/d9100981ada8b3186d906a4f4140b83a819d2299/documentation/misc/actors_version_checklist.md for when we need the next version of actors (v9).
|
sure! |
|
|
||
| for _, a := range acts { | ||
| err = st.MutateActor(a, func(act *types.Actor) error { | ||
| name := actors.CanonicalName(builtin.ActorNameByCode(act.Code)) |
There was a problem hiding this comment.
Why aren't we just putting those actors with correct CIDs right away?
There was a problem hiding this comment.
the correct cids... thays a big conversation, but it is not workable to expect specs-actors to have baked in the correct CIDs.
There was a problem hiding this comment.
The manifest should allow us to remove the baked-in code cids from v8 specs-actors, no?
| // nv, err := m.FullNode.FullNode.StateNetworkVersion(ctx, types.EmptyTSK) | ||
| // require.NoError(n.t, err) | ||
|
|
||
| // TODO this doesn't currently work with the FVM -- we need to specify policy somehow | ||
| // proofType, err := miner.WindowPoStProofTypeFromSectorSize(m.options.sectorSize) | ||
| // require.NoError(n.t, err) | ||
| // so do this instead, which works: | ||
| proofType := abi.RegisteredPoStProof_StackedDrgWindow64GiBV1 |
There was a problem hiding this comment.
will be fixed once we have test bundles to load.
|
So the merge overwrote the specs-actors dep and results in a broken manifest loader; see filecoin-project/specs-actors#1587 |
|
fixed, updated filecoin-project/specs-actors#1587 |
|
Also, the forks_test ended broken; fixed. |
| ac := a.Code() | ||
| ar.actors[ac] = ai | ||
|
|
||
| // necessary to make stuff work |
There was a problem hiding this comment.
As far as I understand in v8 actors we only really need ActorRegistry to map actor method->param type. Ideally we'd move that logic to chain/actors/, and only use the ActorRegistry in LegacyVM (so this would have no method type mappings, and wouldn't need to worry about FVM).
There was a problem hiding this comment.
(This can be a followup PR)
| // required for genesis/testing | ||
| if nv := rt.NetworkVersion(); nv >= network.Version16 { | ||
| name, av, ok := actors.GetActorMetaByCode(act.Code) | ||
|
|
||
| if ok { | ||
| // lies, lies, lies | ||
| builder := cid.V1Builder{Codec: cid.Raw, MhType: mh.IDENTITY} | ||
| synthetic := fmt.Sprintf("fil/%d/%s", av, name) | ||
| syntheticCid, err := builder.Sum([]byte(synthetic)) | ||
| if err != nil { | ||
| panic(aerrors.Fatalf("failed to generate synthetic CID: %s", err)) | ||
| } | ||
|
|
||
| return syntheticCid, true | ||
| } | ||
| } |
There was a problem hiding this comment.
Really I think we should be trying to get rid of the id(fil/8/y) code cids from the codebase, otherwise we'll have to translate them back and forth everywhere, which will be really painful, especially with more network upgrades
There was a problem hiding this comment.
yes, I totally agree, but it's not as simple -- we'll have to do some surgery in specs-actors.
There was a problem hiding this comment.
Won't we remove most Go specs-actors logic, and mostly just keep the data-accessing bits? That should remove almost all dependencies on old code CIDs. (I'm assuming that builtin-actors don't use them - didn't look at them much yet)
There was a problem hiding this comment.
if we remove all specs-actors logic we will be fine; really, we should become fvm only, except for state.
| // required for genesis/testing | ||
| if nv := rt.NetworkVersion(); nv >= network.Version16 { | ||
| name, av, ok := actors.GetActorMetaByCode(act.Code) | ||
|
|
||
| if ok { | ||
| // lies, lies, lies | ||
| builder := cid.V1Builder{Codec: cid.Raw, MhType: mh.IDENTITY} | ||
| synthetic := fmt.Sprintf("fil/%d/%s", av, name) | ||
| syntheticCid, err := builder.Sum([]byte(synthetic)) | ||
| if err != nil { | ||
| panic(aerrors.Fatalf("failed to generate synthetic CID: %s", err)) | ||
| } | ||
|
|
||
| return syntheticCid, true | ||
| } | ||
| } |
There was a problem hiding this comment.
Won't we remove most Go specs-actors logic, and mostly just keep the data-accessing bits? That should remove almost all dependencies on old code CIDs. (I'm assuming that builtin-actors don't use them - didn't look at them much yet)
| @@ -580,6 +580,23 @@ func MakeGenesisBlock(ctx context.Context, j journal.Journal, bs bstore.Blocksto | |||
| return nil, xerrors.Errorf("setup miners failed: %w", err) | |||
| } | |||
|
|
|||
| if template.NetworkVersion >= network.Version16 { | |||
There was a problem hiding this comment.
Might not be easy, but we should make the genesis logic use the FVM. Definitely not needed now, but this is the only thing which depends on a big chunk of Go specs-actors logic
There was a problem hiding this comment.
(definitely not something we want to do in this PR / not needed for M1)
There was a problem hiding this comment.
yeah, agreed. The sooner specs-actors retires, the better.
|
yeah, i have no idea whats wrong with make gen. Itests need a policy adjusted bundle before they can be fixed, i have listed it as TODO follow up in the nv16 tracking pr -- basically improving our bundle situation and fixing itests is next on my pile. |
This embeds the bundle for v8; long run we want to autofetch from IPFS, but we discussed this with @raulk and decided it is fine for now, avoiding scope creep.
As it is it will load and embed the bundle from
build/builtin-actors/builtin-actors-v8.car.TBD:
Note: made it upload to estuary and fetch from ipfs instead.