Use new go-state-types state accessors #1590
Conversation
arajasek
force-pushed
the
asr/refactor
branch
2 times, most recently
from
6ea3f69 to
2fb54df
Compare
|
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. |
|
@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. |
|
Thanks, what was really missing is filecoin-project/lotus#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). |
arajasek
force-pushed
the
asr/refactor
branch
3 times, most recently
from
8bee187 to
637436c
Compare
Codecov Report
@@ 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
left a comment
ZenGround0
left a comment
There was a problem hiding this comment.
Why is cbor gen twiddling lines of code around?
go.sum
Outdated
| github.com/filecoin-project/specs-actors/v5 v5.0.4/go.mod h1:5BAKRAMsOOlD8+qCw4UvT/lTLInCJ3JwOWZbX8Ipwq4= | ||
| github.com/filecoin-project/specs-actors/v6 v6.0.0 h1:i+16MFE8GScWWUF0kG7x2RZ5Hqpz0CeyBHTpnijCJ6I= | ||
| github.com/filecoin-project/specs-actors/v6 v6.0.0/go.mod h1:V1AYfi5GkHXipx1mnVivoICZh3wtwPxDVuds+fbfQtk= | ||
| github.com/filecoin-project/specs-actors/v5 v5.0.4-0.20220511204328-97fa6fffc45a h1:VBrBfi+7i+IjSDdt7ezdF18ViS5D/Bdc5hRUVOLcfhs= |
| numBlocks++ | ||
| totalCopySize += uint64(len(blk.RawData())) | ||
| return to.Put(blk) | ||
| return to.Put(context.TODO(), blk) |
There was a problem hiding this comment.
Because go-state-types pulls in the context blockstores (and I don't want to undo that, it should be on the latest thing).
| // ChainCommitRand abi.Randomness | ||
| //} | ||
| type SubmitWindowedPoStParams = miner0.SubmitWindowedPoStParams | ||
| type SubmitWindowedPoStParams = minertypes.SubmitWindowedPoStParams |
There was a problem hiding this comment.
Do these param substitutions need to happen here in the same way the proof types change needs to happen?
There was a problem hiding this comment.
Yes, because the runtime verification calls want them to be the same type.
|
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. |
|
Yeah, we can avoid the bump -- will just need to downgrade it in go-state-types and cut a new release of that. |
arajasek
force-pushed
the
asr/refactor
branch
3 times, most recently
from
7c2646e to
f549c51
Compare
| // Params []byte | ||
| //} | ||
| type ModVerifyParams = paych0.ModVerifyParams | ||
| type ModVerifyParams = paychtypes.ModVerifyParams |
There was a problem hiding this comment.
I take it runtime verification needs these to be the same too?
gen/gen.go
Outdated
| ); err != nil { | ||
| panic(err) | ||
| } | ||
| //if err := gen.WriteTupleEncodersToFile("./actors/runtime/proof/cbor_gen.go", "proof"); //proof.SectorInfo{}, |
|
Its worth a paranoid pass over all the cborgen of params moved to go-state-types to make sure nothing has changed. |
Needs filecoin-project/go-state-types#36