feat(node): consistent genesis #1016

cryptoAtwill · 2024-05-30T16:24:58Z

This PR follows this discussion to address #1002.

The flow is simpler now with:

Addition of SealGenesis command that runs the previous genesis creation logic:
- Load builtin/custom actor and deploy ipc contracts using memory store. Implementation not changed, just shifted location.
- Write memory store to Car file.
InitChain just reads the car file bytes into the StateStore

This PR is just an implementation of the flow, it's a quick way to show if the flow actually works. Pending items to make it ready for review:

Integration testing. The current state of the PR might have missing steps (unlikely, but possible), will actually perform integration testing to ensure it actually works.
Unit tests.
Some further code clean up and simplification.
Fix broken CI due to this change.

To actually try it out, perform the following:

# generate the fendermint genesis first, then seal the genesis state
cargo run -p fendermint_app --release -- genesis --genesis-file test-network/genesis.json \
  ipc \
    seal-state \
    --builtin-actors-path <FULL PATH TO YOUR BUILT_IN BUNDLE> \
    --custom-actors-path  <FULL PATH TO YOUR CUSTOM BUNDLE> \
    --artifacts-path  <FULL PATH TO YOUR CONTRACT OUT FOLDER> \
    --output-path <FULL PATH TO YOUR SEALED STATE OUTPUT PATH>

# convert to cometbft genesis
cargo run -p fendermint_app --release -- \
  genesis --genesis-file test-network/genesis.json \
  into-tendermint --out ~/.cometbft/config/genesis.json --sealed  <FULL PATH TO YOUR SEALED STATE OUTPUT PATH>

raulk

Will do a deeper review tomorrow, but this is headed in the right direction!

In the meantime, can you work on the command to extract the state tree from Fluence's genesis?

@folex can give you an endpoint if you need one.

raulk · 2024-05-30T22:16:32Z

fendermint/app/options/src/genesis.rs

+    /// The built in actors bundle path
+    #[arg(long, short)]
+    pub builtin_actors_path: PathBuf,
+
+    /// The custom actors bundle path
+    #[arg(long, short)]
+    pub custom_actors_path: PathBuf,
+
+    /// The ipc artifacts output path. If you are using ipc-monorepo, it should be the `out` folder
+    /// of `make build`
+    #[arg(long, short)]
+    pub ipc_artifacts_path: PathBuf,


I wonder if we can embed these artifacts in the build, and load them from the target directory instead?

@cryptoAtwill, any thoughts here?

oops, missed this. I think we have discussed this before, if I recall correctly, it's because ipc_contracts can be updated and compile it will result in a different artifacts build. Using a path gives more flexibility on which artifacts should be imported.

raulk · 2024-05-30T22:17:01Z

fendermint/app/options/src/genesis.rs

+
+    /// The sealed, i.e. finalized genesis state CAR file dump path
+    #[arg(long, short)]
+    pub sealed_genesis_path: PathBuf,


Let's simply call this output_path.

raulk · 2024-05-30T22:18:53Z

fendermint/vm/genesis/src/lib.rs

@@ -7,6 +7,7 @@ use anyhow::anyhow;
 use fvm_shared::bigint::{BigInt, Integer};
 use serde::{Deserialize, Serialize};
 use serde_with::serde_as;
+use std::path::PathBuf;


removed alr

raulk · 2024-05-30T22:19:34Z

fendermint/vm/interpreter/src/genesis.rs

+
+/// The output of genesis creation
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub struct FvmGenesisOutput {


Can drop Fvm from the name.

Co-authored-by: Eric Tu <6364934+ec2@users.noreply.github.com> Co-authored-by: Mikers <snissn@gmail.com> Co-authored-by: raulk <raul@protocol.ai> Co-authored-by: Shashank Trivedi <100513286+lordshashank@users.noreply.github.com>

…mple-genesis

…pc into simple-genesis

karlem · 2024-08-05T21:12:43Z

fendermint/app/src/cmd/genesis.rs

@@ -94,6 +95,8 @@ cmd! {
            set_ipc_gateway(&genesis_file, args),
        GenesisIpcCommands::FromParent(args) =>
            new_genesis_from_parent(&genesis_file, args).await,
+        GenesisIpcCommands::SealState(args) =>


I wonder if it makes sense to have this under IPC? I understand that it does deploy IPC-related contracts, but it also deploys built-in contracts that might be useful without IPC. Maybe it can be moved as a root genesis command, with FEVM contracts as an optional parameter?

Currently the separation btw ipc and non-ipc is blurred. I would actually argue we track this in another issue, because it's the downstream implementation that assumes an artifacts folder which makes it difficult to split upstream.

karlem · 2024-08-06T16:39:36Z

fendermint/vm/interpreter/src/genesis.rs

+    }
+
+    /// Deploy a library contract with a dynamic ID and no constructor.
+    pub fn deploy_library(


No need for it to be pub?

karlem · 2024-08-06T16:40:04Z

fendermint/vm/interpreter/src/genesis.rs

+    }
+
+    /// Construct the bytecode of a top-level contract and deploy it with some constructor parameters.
+    pub fn deploy_contract<T>(


No need for it to be pub?

karlem · 2024-08-06T16:40:10Z

fendermint/vm/interpreter/src/genesis.rs

+    }
+
+    /// Collect Facet Cuts for the diamond pattern, where the facet address comes from already deployed library facets.
+    pub fn facets(&self, contract_name: &str) -> anyhow::Result<Vec<FacetCut>> {


No need for it to be pub?

karlem · 2024-08-06T16:41:17Z

fendermint/vm/interpreter/src/genesis.rs

+        .fold(TokenAmount::zero(), |s, a| s + a.balance.clone())
+}
+//
+// #[cfg(test)]


We probably want to either re-write the test so that they work with the new implementation or just delete them at least?

I will remove them first, rewrite them later

karlem · 2024-08-06T16:43:07Z

fendermint/vm/interpreter/src/genesis.rs

+    /// The custom actors bundle path
+    custom_actors_path: PathBuf,
+    /// The CAR path to flush the sealed genesis state
+    sealed_out_path: PathBuf,


We don't really mention that it creates sealed genesis anywhere so I guess out_path would do?

fendermint/vm/interpreter/src/genesis.rs

…pc into simple-genesis

cryptoAtwill · 2024-08-07T16:30:06Z

@karlem I have updated based on the review feedback, please take another look. I will remove old implementation in a follow up PR.

…mple-genesis

raulk

Initial comments.

raulk · 2024-08-16T13:25:15Z

.github/workflows/fevm-contract-tests.yaml

@@ -71,8 +71,7 @@ jobs:
        id: docker-build
        working-directory: ipc/fendermint
        run: |
-          export PATH="$PATH:/home/runner/.config/.foundry/bin"
-          docker pull ghcr.io/consensus-shipyard/fendermint:latest
+          export PATH="$PATH:/home/runner/.config/.foundry/bin" && make docker-build


Isn't this gonna crazy expensive? IIRC we build a Docker image in another job, can't we reuse that Docker image?

raulk · 2024-08-16T13:36:05Z

fendermint/app/src/cmd/genesis.rs

+        args.output_path.clone(),
+    );
+
+    genesis_creator.create(genesis).await


These APIs are weird. We have a Genesis and the GenesisCreator. You would imagine the GenesisCreator creates a Genesis, but in reality the Genesis already exists and the GenesisCreator wants it as an input.

I think we need to adjust this a little.

We should have a GenesisBuilder.

The genesis_file is poorly named. These are genesis parameters, so let's name it genesis_params_path.

You can then have a GenesisBuilder::from_params to construct a new GenesisBuilder from the parameters file.

We then have a method for each artifact we need to take: with_builtin_actors, with_custom_actors, with_system_contracts.

We then have a write_into<W: io::Write>(w: &mut W) method that writes the genesis file to a writer.

The file is managed by seal_genesis.

In practice:

let mut output = // create the file let gen = GenesisBuilder::from_params(genesis_params_path) .with_builtin_actors(...) .with_custom_actors(...) .with_system_contracts(...) .write_into(&output)

This works for me. Just a side note that probably

.with_builtin_actors(...) .with_custom_actors(...) .with_system_contracts(...)

should be added to constructor as they are required parameters, otherwise it feels it's not really required.

raulk · 2024-08-16T13:36:43Z

fendermint/testing/materializer/src/docker/node.rs

                    --out /cometbft/config/genesis.json \
+                    --app-state /cometbft/config/sealed.json \


Why both an out and an app-state?

The app-state is practically the genesis state, but the terminology cometbft is using is app-state, just took that term here.

raulk · 2024-08-16T13:37:25Z

fendermint/vm/interpreter/src/genesis.rs

@@ -0,0 +1,795 @@
+// Copyright 2022-2024 Protocol Labs


Good question.

…pc into simple-genesis

Co-authored-by: cryptoAtwill <willes.lau@protocol.ai>

raulk · 2024-08-16T15:20:20Z

fendermint/app/options/src/genesis.rs

+    /// The solidity artifacts output path. If you are using ipc-monorepo, it should be the `out` folder
+    /// of `make build`
+    #[arg(long, short)]
+    pub artifacts_path: Option<PathBuf>,


Why is this an Option? It does not add up with the comments here? Are we loading them as embedded resources?

raulk · 2024-08-16T16:31:36Z

fendermint/app/src/app.rs

-            .context("failed to init from genesis")?;
-
-        let state_root = state.commit().context("failed to commit genesis state")?;
+        let (validators, state_params) = read_genesis_car(genesis_bytes, &self.state_store).await?;


Hm, ideally we should be able to get validators by calling a getter on the gateway, so that we don't have to carry them separately. But the problem is that the Gateway doesn't store pubkeys; we should extend it for that.

raulk · 2024-08-23T14:59:40Z

fendermint/vm/interpreter/src/genesis.rs

+        }
+
+        match bytes[0] {
+            0 => {


We should make these versions/format identifiers constants somewhere.

raulk · 2024-08-29T09:39:49Z

fendermint/vm/interpreter/src/genesis.rs

+    pub fn compress_and_encode(&self) -> anyhow::Result<String> {
+        let bytes = match self {
+            GenesisAppState::V1(ref bytes) => {
+                let mut wtr = snap::write::FrameEncoder::new(vec![0]);


@cryptoAtwill this looks like a bug? We write the version discriminator (0) under snappy compression, but we expect to find it uncompressed below?

complete flow

19f93f5

cryptoAtwill requested a review from raulk May 30, 2024 16:25

This was referenced May 30, 2024

Including builtin and custom actor bundles in genesis #1009

Closed

Track ipc contract genesis artifacts #1015

Closed

cryptoAtwill marked this pull request as draft May 30, 2024 16:27

raulk reviewed May 30, 2024

View reviewed changes

cryptoAtwill and others added 7 commits June 3, 2024 11:02

convert fendermint to cometbft genesis

fcdc70f

remove deprecated fields

3d8f00f

fix cursor

8a8efd6

fix genesis app bytes

3890de4

fmt

14e6dea

minor changes

4dc0482

Fix cicd (#1023)

02b183f

cryptoAtwill marked this pull request as ready for review June 5, 2024 07:20

cryptoAtwill and others added 5 commits June 6, 2024 21:20

Fix cicd (#1025)

d88face

Fix cicd (#1027)

2a4d428

Co-authored-by: Eric Tu <6364934+ec2@users.noreply.github.com> Co-authored-by: Mikers <snissn@gmail.com> Co-authored-by: raulk <raul@protocol.ai> Co-authored-by: Shashank Trivedi <100513286+lordshashank@users.noreply.github.com>

Merge branch 'main' of protocol-github:consensus-shipyard/ipc into si…

0e31699

…mple-genesis

Merge branch 'simple-genesis' of protocol-github:consensus-shipyard/i…

0d1c55c

…pc into simple-genesis

Merge branch 'main' into simple-genesis

d551aed

cryptoAtwill requested a review from a team as a code owner August 5, 2024 12:18

cryptoAtwill changed the title ~~Consistent genesis~~ feat(node): Consistent genesis Aug 5, 2024

cryptoAtwill changed the title ~~feat(node): Consistent genesis~~ feat(node): consistent genesis Aug 5, 2024

karlem reviewed Aug 5, 2024

View reviewed changes

karlem reviewed Aug 6, 2024

View reviewed changes

fendermint/vm/interpreter/src/genesis.rs Outdated Show resolved Hide resolved

karlem reviewed Aug 6, 2024

View reviewed changes

fendermint/vm/interpreter/src/genesis.rs Outdated Show resolved Hide resolved

cryptoAtwill added 2 commits August 7, 2024 22:35

Merge branch 'simple-genesis' of protocol-github:consensus-shipyard/i…

c4b7e66

…pc into simple-genesis

add genesis app state schema

fa0f330

cryptoAtwill and others added 11 commits August 8, 2024 12:01

update infra and simplify genesis

b0038cc

fmt

22e5958

fix infra

0319141

Merge branch 'main' of protocol-github:consensus-shipyard/ipc into si…

7a86832

…mple-genesis

merge with main

79d1264

Update fevm-contract-tests.yaml

0ae95c8

Update fevm-contract-tests.yaml

fa372d6

Update testnode.toml

5332a31

skip fendermint build

ed66780

remove pull

272c29c

Merge branch 'main' into simple-genesis

0db23d2

raulk reviewed Aug 16, 2024

View reviewed changes

cryptoAtwill and others added 7 commits August 19, 2024 13:23

rename to genesis builder

ef2740c

Merge branch 'simple-genesis' of protocol-github:consensus-shipyard/i…

4ba0219

…pc into simple-genesis

fix(node): remove genesis interpreter (#1118)

47556e9

Co-authored-by: cryptoAtwill <willes.lau@protocol.ai>

Merge branch 'main' into simple-genesis

37a5522

Merge branch 'main' into simple-genesis

38fbeed

clean up read_genesis_car.

86d1cf9

size Vec with capacity.

707060b

raulk approved these changes Aug 26, 2024

View reviewed changes

raulk added 2 commits August 26, 2024 16:50

rust fmt.

0ebe60f

fix clippy.

29099f1

raulk reviewed Aug 29, 2024

View reviewed changes

raulk and others added 2 commits August 29, 2024 10:59

fix version; allocate enough vec capacity.

a3b3c4a

Merge branch 'main' into simple-genesis

501a256

raulk merged commit dd73ac2 into main Aug 29, 2024
15 checks passed

raulk deleted the simple-genesis branch August 29, 2024 10:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(node): consistent genesis #1016

feat(node): consistent genesis #1016

cryptoAtwill commented May 30, 2024 •

edited

Loading

raulk left a comment

raulk May 30, 2024

raulk Aug 16, 2024

cryptoAtwill Aug 19, 2024

raulk May 30, 2024

raulk May 30, 2024

cryptoAtwill Jun 3, 2024

raulk May 30, 2024

karlem Aug 5, 2024 •

edited

Loading

cryptoAtwill Aug 7, 2024

karlem Aug 6, 2024

karlem Aug 6, 2024

karlem Aug 6, 2024

karlem Aug 6, 2024

cryptoAtwill Aug 7, 2024 •

edited

Loading

karlem Aug 6, 2024 •

edited

Loading

cryptoAtwill commented Aug 7, 2024

raulk left a comment

raulk Aug 16, 2024

raulk Aug 16, 2024

cryptoAtwill Aug 19, 2024

raulk Aug 16, 2024

cryptoAtwill Aug 19, 2024

raulk Aug 16, 2024

raulk Aug 16, 2024

raulk Aug 16, 2024

raulk Aug 23, 2024

raulk Aug 29, 2024

		--out /cometbft/config/genesis.json \
		--app-state /cometbft/config/sealed.json \

feat(node): consistent genesis #1016

feat(node): consistent genesis #1016

Conversation

cryptoAtwill commented May 30, 2024 • edited Loading

raulk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

karlem Aug 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cryptoAtwill Aug 7, 2024 • edited Loading

Choose a reason for hiding this comment

karlem Aug 6, 2024 • edited Loading

Choose a reason for hiding this comment

cryptoAtwill commented Aug 7, 2024

raulk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cryptoAtwill commented May 30, 2024 •

edited

Loading

karlem Aug 5, 2024 •

edited

Loading

cryptoAtwill Aug 7, 2024 •

edited

Loading

karlem Aug 6, 2024 •

edited

Loading