Skip to content
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

feat: forest-tool snapshot compute-state #3430

Merged
merged 193 commits into from
Sep 8, 2023
Merged

feat: forest-tool snapshot compute-state #3430

merged 193 commits into from
Sep 8, 2023

Conversation

aatifsyed
Copy link
Contributor

@aatifsyed aatifsyed commented Aug 30, 2023 •
edited

Rework of #3161

Changes

  • new subcommand
  • parsing and lotus-compatibile printing for ExecutionEvents, with the following bugs:
    • We don't have a "Duration" for each message
    • We don't serialize the "EventsRoot"
  • Add NetworkChain::from_genesis
  • Change signature of the popular CB: FnMut(...) -> anyhow::Result<()> to take a MessageCallbackCtx

Testing

Tested against forest_snapshot_calibnet_2023-06-20_height_664544.car - see below for obtaining reference from lotus.
Adding integration tests is tracked below

Follow-on work

@elmattic
Scaffolding command
d1ce733
@elmattic
Remove memory db
ff23be1
@elmattic
cargo fmt
0857cb9
@elmattic
Move code to state subcommand
9485a3e
@elmattic
Add state_compute rpc
50c8577
@elmattic
Move to a CarBackedBlockstore
9dcdf58
@elmattic
Clean up
5868195
@elmattic
Add print before CarBackedBlockstore constructor
ba634aa
@elmattic
Fix doc comment
bab5c8c
@elmattic
Print computed state root hash
f6e4a3b
@elmattic
Remove message flag
7ed786f
@elmattic
Merge branch 'main' into elmattic/compute-state-cli
78116e4
@elmattic
Untangle apply_blocks from StateManager (wip)
b33ea85
@elmattic
Add missing callbacks
d259a51
@elmattic
Remove passing of chainstore
27a2482
@elmattic
Add print of json trace (wip)
09e44a2
@elmattic
Merge branch 'main' into elmattic/compute-state-cli
bb118b2
@elmattic
Remove read_header
615eff9
@elmattic
Remove endpoint
4566e50
@elmattic
exec traces (wip)
b1987d2
@elmattic
cargo fmt
1e66d3a
@elmattic
Add comment
db3a037
@elmattic
Remove ExecutionEvent
9dabe0f
@elmattic
Use similar data structures than Lotus for exec trace
99bc952
@elmattic
Add strict_bytes
94b7d21
@elmattic
cleanup
7358f99
@elmattic
Move serialization to json modules (wip)
c050a5b
@elmattic
Add message return
6836aa0
@elmattic
Add subcalls
8e10dff
@elmattic
Add gas trace charges
db12aa5
This was referenced Sep 5, 2023
Closed
Open
@aatifsyed aatifsyed marked this pull request as ready for review September 5, 2023 11:57
@aatifsyed aatifsyed requested a review from a team as a code owner September 5, 2023 11:57
@aatifsyed aatifsyed requested review from ruseinov, jdjaustin and lemmih and removed request for a team September 5, 2023 11:57
lemmih
lemmih requested changes Sep 5, 2023
View reviewed changes
Copy link
Contributor

@lemmih lemmih left a comment

Choose a reason for hiding this comment

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

Generally okay. Some FIXMEs and BUGs that I do not understand. And fvm3 structures should not be used directly. Guillaume opened an issue to avoid the fvm3 he used. You could do the same, but I think it would be easier just to use the shims.

src/tool/subcommands/snapshot_cmd.rs Outdated Show resolved Hide resolved
src/shim/gas.rs Outdated Show resolved Hide resolved
src/tool/subcommands/snapshot_cmd.rs Outdated Show resolved Hide resolved
@aatifsyed aatifsyed mentioned this pull request Sep 6, 2023
Open
3 tasks
@aatifsyed aatifsyed changed the title feat: forest-tool compute state feat: forest-tool snapshot compute-state Sep 6, 2023
@aatifsyed aatifsyed added this pull request to the merge queue Sep 8, 2023
Merged via the queue into main with commit e0b5e87 Sep 8, 2023
23 checks passed
@aatifsyed aatifsyed deleted the aatifsyed/refactor-compute-state-cli branch September 8, 2023 10:11
@aatifsyed aatifsyed mentioned this pull request Oct 3, 2023
Closed
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ruseinov ruseinov ruseinov approved these changes

@lemmih lemmih lemmih approved these changes

@jdjaustin jdjaustin Awaiting requested review from jdjaustin jdjaustin is a code owner automatically assigned from ChainSafe/forest

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@aatifsyed @ruseinov @lemmih @elmattic