Skip to content

feat: implement initial Lotus V2 State APIs #13027

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 8 commits into from
Apr 11, 2025
Merged

feat: implement initial Lotus V2 State APIs #13027

merged 8 commits into from
Apr 11, 2025

Conversation

masih
Copy link
Member

@masih masih commented Apr 10, 2025 •
edited
Loading

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.

Part of: #12990

@github-project-automation github-project-automation bot moved this to 📌 Triage in FilOz Apr 10, 2025
@masih masih changed the title Implement initial Lotus V2 State APIs feat: implement initial Lotus V2 State APIs Apr 10, 2025
@masih masih force-pushed the masih/v2api-state branch from 8783dfe to ec07ded Compare April 10, 2025 18:20
@masih masih self-assigned this Apr 10, 2025
@masih masih marked this pull request as ready for review April 10, 2025 18:50
@masih masih requested review from rvagg and Kubuxu April 10, 2025 18:50
@rvagg rvagg mentioned this pull request Apr 11, 2025
Open
@rvagg
Copy link
Member

rvagg commented Apr 11, 2025

This is fine, except for the StateCompute and StateSimulate, I'm very hesitant to support proliferation of these APIs.

Rather than pollute this issue I broke out a new issue to dump my current thoughts over @ #13028

I also followed up looking at how useful TipSetLimit is, and I don't think it's all that helpful in this case because our own uses of StateCompute have tipset==height, so we'd have to find external consumers for who this is useful I think: #13028 (comment)

So maybe we should hold off on the StateCompute and StateSimulate for now so we can do a more thoughtful job of replacing our current re-execution APIs.

BigLep
BigLep reviewed Apr 11, 2025
View reviewed changes
Copy link
Member

@BigLep BigLep left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Things that came to mind while following along. Nothing blocking here.

@BigLep
Copy link
Member

BigLep commented Apr 11, 2025

So maybe we should hold off on the StateCompute and StateSimulate for now so we can do a more thoughtful job of replacing our current re-execution APIs.

I was wondering something similar. Do we focus on shipping some clear cut wins initially (e.g., ETH API group and read APIs for Chain/State) before gettng bogged down in the more complicated cases.

@rvagg
Copy link
Member

rvagg commented Apr 11, 2025

Now that the topic has been opened up, I'm thinking that instead of modifying the /v1 API as I'm doing in #12822 maybe we should just start working on a /v2 replacement for these APIs. But it's a low priority and I don't think needs to be part of an initial release of this work, unless we're getting signal from elsewhere that this is an important API?

@masih
Copy link
Member Author

masih commented Apr 11, 2025 •
edited
Loading

But it's a low priority and I don't think needs to be part of an initial release of this work, unless we're getting signal from elsewhere that this is an important API?

StateCompute is the API that was mentioned by Zondax as "used a lot. Which is why I added it to the initial pass at State v2 APIs. The separation of Simulate and Compute is also motivated by their usage pattern as far as I can see in Zondax Rosetta repo, where they always call StateCompute via a wrapper that only takes a TipSet and uses the tipset height to then call StateCompute.

@masih
Copy link
Member Author

masih commented Apr 11, 2025

modifying the /v1 API as I'm doing in #12822 maybe we should just start working on a /v2 replacement for these APIs.

This is consistent with my take, in that if there is not a fire to put out in v1 APIs I would say the energy is best spent on getting v2 the best that it can be. I am also conscious that I don't know as much on the context of the work involved in modifying v1 APIs.

masih added 7 commits April 11, 2025 11:37
@masih
Implement initial Lotus V2 State APIs
9f36a9e 
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.
* `StateSimulate` - enhanced to accept tipset selector, and separated
   out of v1 `StateCompute` to focus solely on simulation of compute for
   future instances. Additionally, it accepts a limit either relative or
   absolute for how far to simulate.
* `StateCompute` - enhanced to accept tipset selector, and changed to
  purely apply state at given tipset based on a common usage pattern
  across exchanges. This function now implicitly computes for the height
  of the given tipset. For simulation of future heights, use
  `StateSimulate`.

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.
@masih
Change that log
e4d0a5c
@masih
gen that doc
83733ac
@masih
Re gen that doc
6375a04
@masih
Fix missing permission in proxy gen
73d2850
@masih
Undo changelog formatting
8fdb6ac
@masih
Remove StateCompute and StateSimulate to be approached in #13028
68e1d35 
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.
@masih masih force-pushed the masih/v2api-state branch from 60debd0 to 68e1d35 Compare April 11, 2025 11:25
@github-project-automation github-project-automation bot moved this from 📌 Triage to ✔️ Approved by reviewer in FilOz Apr 11, 2025
@masih
Copy link
Member Author

masih commented Apr 11, 2025

Updated the Filecoin v2 API docs to include the new APIs introduced here.

Merging and happy to address any further comments retrospectively.

@masih masih enabled auto-merge (squash) April 11, 2025 11:56
@masih masih merged commit fe65cf4 into master Apr 11, 2025
91 checks passed
@masih masih deleted the masih/v2api-state branch April 11, 2025 11:57
@github-project-automation github-project-automation bot moved this from ✔️ Approved by reviewer to 🎉 Done in FilOz Apr 11, 2025
Kubuxu
Kubuxu reviewed Apr 11, 2025
View reviewed changes
Copy link
Contributor

@Kubuxu Kubuxu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Late to the party but SGWM given what we discussed

@rjan90 rjan90 mentioned this pull request May 1, 2025
Merged
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BigLep BigLep BigLep left review comments

@rvagg rvagg rvagg approved these changes

@Kubuxu Kubuxu Kubuxu left review comments

Assignees

@masih masih

Labels
None yet
Projects
FilOz
Status: 🎉 Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@masih @rvagg @BigLep @Kubuxu