-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Reduce state/tipset/msg execution/replay/compute APIs in /v2 #13028
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
Comments
Here's another fun factoid: it looks like all of our current uses of Line 81 in 6f09aca
Lines 91 to 97 in 6f09aca
Line 102 in 6f09aca
lotus/cmd/lotus-shed/miner-fees.go Line 195 in 6f09aca
lotus/itests/migration_test.go Line 403 in 6f09aca
lotus/cmd/lotus-shed/mismatches.go Line 40 in 6f09aca
In all of the above, it's only the last one where we care about the
|
Something else to consider here - |
This is consistent with what I observed in Zondax codebase and the rationale for removing height from |
OK, so it just becomes the same as a full-messages |
To unblock progress in making incremental progress at v2 apis I am removing the implementation of StateSimulate and StateCompute in light of discussions at #13028.
Implement the Lotus V2 APIs for : * `StateGetActor` - enhanced to accept tipset selector. * `StateGetID` - enhanced to accept tipset selector / renamed from `LookupID` in v1 for consistency. Tests are added to exercise the end-to-end API call through JSON rpc with the raw wire format of the JSON-RPC request. Additional factory functions are added for `TipSetLimit`, a new concept to specify a limit for tipsets either as absolute height or relative to some tipset selector. The future State message search APIs in v2 aim to use Limit in upcoming PRs. To unblock progress in making incremental progress at v2 apis I am removing the implementation of StateSimulate and StateCompute in light of discussions at #13028.
|
Why split implicit messages from messages? IMO, we should just let you pass arbitrary messages from any sender (ideally by CID OR By explicitly specifying a message to execute). TipSetMessageSelectionStrategy We also need a way to say "and include all messages from this same sender in the message pool". Or even include all messages from these specific senders? All messages to the recipient? It would be really nice to have a complex "message-inclusion-strategy" query here: (we can start with the simplest version, but I'd structure this as an object we can extend over time) Also: does "all" mean "including implicit messages"? We probably need to distinguish between "all messages" and "all all". debugging I'd also add a "debug actors" option that enables FVM debugging and returns the logs (embedded in the trace). |
I don't think "from mpool" is a good idea, it is inherently racy and non-reproducible. |
Note to implementer: Also see #13046 |
I was thinking about things like gas estimation. But maybe that'll use a separate API? I'd prefer to have one API. |
Breaking this out from #13027 where I was starting to ramble a bit too much and departing from the issue at hand.
Given we are working on /v2, there's an opportunity to consolidate our msg execution/replay APIs.
We have
StateReplay
andStateCall
andStateCompute
today and they all do roughly the same thing but with minor variations, and the internals are becoming very messy to support the variations while keeping backward compatibility, made more complicated by the needs of the Ethereum APIs which want to do similar things.@Stebalien and I were discussing these the other day trying to boil them down to a single method that we can overload with an options struct argument to support the variations we care about. Then we can push those options down into the core and clean things up a bit.
In #12822 I've replaced a use of
StateReplay
withStateCall
and I'm having to introduce third argument to it just to get my new option in, but in that case I'm introducing an optional struct argument (optional so it's backward compatible as JSONRPC) so I can get the new option in.callInternal
already illustrates the problem if you look both at the number of args, and the number of branches in it:lotus/chain/stmgr/call.go
Line 93 in 6f09aca
Then, if you look at how
StateReplay
works, you see that it's basically the same thing asStateCall
, but with a "execute everything up to this msg" strategy: https://github.com/filecoin-project/lotus/blob/6f09aca67ac53f65004751f55e6aa3c86b349153/chain/stmgr/call.go#L295-L311StateCompute
is something like aStateCall
but with an batch of messages and a "apply all messages, then apply these new ones" strategy but also return the new state root.Right now we're considering at adding a state root to
InvocResult
as part of solving #13022, which would makeComputeStateOutput
redundant.So, I think we could introduce something like:
The text was updated successfully, but these errors were encountered: