Skip to content
This repository has been archived by the owner on Jan 11, 2024. It is now read-only.

FM-191: Execute BottomUpResolve #201

Merged
merged 10 commits into from
Aug 23, 2023
Merged

FM-191: Execute BottomUpResolve #201

merged 10 commits into from
Aug 23, 2023

Conversation

aakoshh
Copy link
Contributor

@aakoshh aakoshh commented Aug 17, 2023

Closes consensus-shipyard/ipc#313
Depends on #198

The PR looks bigger than it is because it merged main, and because after running cargo update there are lots of changes to Cargo.lock and ethers also generated different code for Solidity actors. We can rebase once the dependencies are merged.

  • Transform the BottomUpResolve to a message we can send to the FVM (does not include cross-messages, just for quorum certificate validation and reward tracking)
  • Pass the FVM message to the inner interpreters
  • Create an STM data structure for tracking background resolutions
  • Add the cross-messages' root CID to the background resolution pool

@aakoshh aakoshh changed the base branch from main to fm-190-check-bottom-up August 17, 2023 09:20
@aakoshh aakoshh force-pushed the fm-191-exec-bottom-up branch from f532117 to 9414f71 Compare August 18, 2023 08:49
@aakoshh aakoshh marked this pull request as ready for review August 18, 2023 13:49
Copy link
Contributor

@adlrocha adlrocha left a comment

Choose a reason for hiding this comment

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

The high-level idea LGTM and I like the abstractions we are using. My only concern is the same as in the previous PRs, while the protocol would work end-to-end if all peers where Fendermint-based, we may need to simplify it a bit to support mainnet interactions.

fendermint/vm/interpreter/src/chain.rs Outdated Show resolved Hide resolved
let smsg = relayed_bottom_up_ckpt_to_fvm(&msg)
.context("failed to syntesize FVM message")?;

// Let the FVM validate the checkpoint quorum certificate and take not of the relayer for rewards.
Copy link
Contributor

Choose a reason for hiding this comment

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

Let me see if I understand this. The idea is that:

  • There is a Resolve message committing the checkpoint which includes the Cid of the list of bottom-up messages to be executed.
  • If the commitment happens, the Cid of the messages are queue for resolution and a posterior proposal an execution?

I still think we may need to remove the resolution stage and directly include messages as part of the checkpoint in a first implementation. Message resolution may include additional complexity in the L2->root interaction :(

In any case, I feel this will make more sense as I move towards the implementation of the actual execution.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, you got that right, and as per our sync we will implement both behaviours depending on what kind of parent we'll be dealing with. For Fendermint<-Fendermint relationships we will use this, for Lotus<-Fendermint we won't.

@aakoshh aakoshh force-pushed the fm-190-check-bottom-up branch from 06e0dc4 to 1fdbde4 Compare August 23, 2023 13:41
Base automatically changed from fm-190-check-bottom-up to main August 23, 2023 13:41
@aakoshh aakoshh force-pushed the fm-191-exec-bottom-up branch from bd53e3f to 48d9336 Compare August 23, 2023 13:53
@aakoshh aakoshh merged commit 26d9361 into main Aug 23, 2023
@aakoshh aakoshh deleted the fm-191-exec-bottom-up branch August 23, 2023 13:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement ExecInterpreter for Ipc::BottomUpResolve
2 participants