filecoin-project / specs-actors

DEPRECATED Specification of builtin actors, in the form of executable code.
Other
86 stars 102 forks source link

Use new go-state-types state accessors #1590

Closed arajasek closed 2 years ago

arajasek commented 2 years ago

Needs https://github.com/filecoin-project/go-state-types/pull/36

anorth commented 2 years ago

I think I am missing some context for this. I can't see the imported code in go-state-types.

If this is proposing to move the actor message and state types out of this repo, I'd like to discuss that first.

arajasek commented 2 years ago

@anorth Are you unable to see this PR?

The proposal is to (mostly) leave actors unchanged, but add the state types to go-state-types. Go-based clients like Lotus can then sever their dependency on specs-actors entirely for v8 and onwards, while still having the ability to de/serialize state, message params, etc.

anorth commented 2 years ago

Thanks, what was really missing is https://github.com/filecoin-project/lotus/issues/8607. That discussion has started multiple times in different places, thanks for giving it a canonical home (even if Lotus is an odd place for it).

codecov-commenter commented 2 years ago

Codecov Report

Merging #1590 (637436c) into release/v7 (a2b8075) will decrease coverage by 0.0%. The diff coverage is 66.6%.

@@             Coverage Diff              @@
##           release/v7   #1590     +/-   ##
============================================
- Coverage        69.0%   69.0%   -0.1%     
============================================
  Files              72      72             
  Lines            8791    8784      -7     
============================================
- Hits             6072    6065      -7     
  Misses           1858    1858             
  Partials          861     861             
ZenGround0 commented 2 years ago

Can we avoid updating cbor-gen? Doing this as part of a non-breaking change scares me a decent amount. If we need it to match go-state-types I'd prefer waiting until the next upgrade to update.

arajasek commented 2 years ago

Yeah, we can avoid the bump -- will just need to downgrade it in go-state-types and cut a new release of that.

ZenGround0 commented 2 years ago

Its worth a paranoid pass over all the cborgen of params moved to go-state-types to make sure nothing has changed.