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.

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

feat(node): consistent genesis #1016

Merged
merged 40 commits into from
Aug 29, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
19f93f5
complete flow
May 30, 2024
fcdc70f
convert fendermint to cometbft genesis
Jun 3, 2024
3d8f00f
remove deprecated fields
Jun 3, 2024
8a8efd6
fix cursor
Jun 3, 2024
3890de4
fix genesis app bytes
Jun 3, 2024
14e6dea
fmt
Jun 3, 2024
4dc0482
minor changes
Jun 4, 2024
02b183f
Fix cicd (#1023)
cryptoAtwill Jun 5, 2024
d88face
Fix cicd (#1025)
cryptoAtwill Jun 6, 2024
2a4d428
Fix cicd (#1027)
cryptoAtwill Jun 10, 2024
0e31699
Merge branch 'main' of protocol-github:consensus-shipyard/ipc into si…
Jun 10, 2024
0d1c55c
Merge branch 'simple-genesis' of protocol-github:consensus-shipyard/i…
Jun 10, 2024
d551aed
Merge branch 'main' into simple-genesis
cryptoAtwill Aug 5, 2024
22219f1
Update fendermint/vm/interpreter/src/genesis.rs
cryptoAtwill Aug 7, 2024
6a1b4dd
Update fendermint/vm/interpreter/src/genesis.rs
cryptoAtwill Aug 7, 2024
7c6276b
review feedbacks
Aug 7, 2024
c4b7e66
Merge branch 'simple-genesis' of protocol-github:consensus-shipyard/i…
Aug 7, 2024
fa0f330
add genesis app state schema
Aug 7, 2024
b0038cc
update infra and simplify genesis
Aug 8, 2024
22e5958
fmt
Aug 8, 2024
0319141
fix infra
Aug 8, 2024
7a86832
Merge branch 'main' of protocol-github:consensus-shipyard/ipc into si…
Aug 13, 2024
79d1264
merge with main
Aug 13, 2024
0ae95c8
Update fevm-contract-tests.yaml
cryptoAtwill Aug 14, 2024
fa372d6
Update fevm-contract-tests.yaml
cryptoAtwill Aug 14, 2024
5332a31
Update testnode.toml
cryptoAtwill Aug 14, 2024
ed66780
skip fendermint build
Aug 14, 2024
272c29c
remove pull
Aug 14, 2024
0db23d2
Merge branch 'main' into simple-genesis
raulk Aug 16, 2024
ef2740c
rename to genesis builder
Aug 19, 2024
4ba0219
Merge branch 'simple-genesis' of protocol-github:consensus-shipyard/i…
Aug 19, 2024
47556e9
fix(node): remove genesis interpreter (#1118)
cryptoAtwill Aug 21, 2024
37a5522
Merge branch 'main' into simple-genesis
raulk Aug 23, 2024
38fbeed
Merge branch 'main' into simple-genesis
raulk Aug 26, 2024
86d1cf9
clean up read_genesis_car.
raulk Aug 26, 2024
707060b
size Vec with capacity.
raulk Aug 26, 2024
0ebe60f
rust fmt.
raulk Aug 26, 2024
29099f1
fix clippy.
raulk Aug 27, 2024
a3b3c4a
fix version; allocate enough vec capacity.
raulk Aug 29, 2024
501a256
Merge branch 'main' into simple-genesis
cryptoAtwill Aug 29, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions fendermint/app/options/src/genesis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,28 @@ pub enum GenesisIpcCommands {
Gateway(GenesisIpcGatewayArgs),
/// Fetch the genesis parameters of a subnet from the parent.
FromParent(Box<GenesisFromParentArgs>),
/// Seal the genesis state from the genesis parameter file
SealState(SealGenesisArgs),
}

#[derive(Args, Debug, Clone)]
pub struct SealGenesisArgs {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
pub struct SealGenesisArgs {
pub struct SealGenesisStateArgs {

maybe?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is fine as it is.

/// 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,
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

@cryptoAtwill, any thoughts here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.


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

Choose a reason for hiding this comment

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

Let's simply call this output_path.

}

#[derive(Args, Debug, Clone)]
Expand Down
48 changes: 6 additions & 42 deletions fendermint/app/src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ use serde::{Deserialize, Serialize};
use tendermint::abci::request::CheckTxKind;
use tendermint::abci::{request, response};
use tracing::instrument;
use fendermint_vm_interpreter::genesis::read_genesis_car;

use crate::events::{NewBlock, ProposalProcessed};
use crate::AppExitCode;
Expand Down Expand Up @@ -136,8 +137,10 @@ where
/// Path to the Wasm bundle.
///
/// Only loaded once during genesis; later comes from the [`StateTree`].
#[deprecated]
builtin_actors_bundle: PathBuf,
/// Path to the custom actor WASM bundle.
#[deprecated]
custom_actors_bundle: PathBuf,
/// Block height where we should gracefully stop the node
halt_height: i64,
Expand Down Expand Up @@ -447,45 +450,15 @@ where

/// Called once upon genesis.
async fn init_chain(&self, request: request::InitChain) -> AbciResult<response::InitChain> {
let bundle = &self.builtin_actors_bundle;
let bundle = std::fs::read(bundle)
.map_err(|e| anyhow!("failed to load builtin bundle CAR from {bundle:?}: {e}"))?;

let custom_actors_bundle = &self.custom_actors_bundle;
let custom_actors_bundle = std::fs::read(custom_actors_bundle).map_err(|e| {
anyhow!("failed to load custom actor bundle CAR from {custom_actors_bundle:?}: {e}")
})?;

let state = FvmGenesisState::new(
self.state_store_clone(),
self.multi_engine.clone(),
&bundle,
&custom_actors_bundle,
)
.await
.context("failed to create genesis state")?;

tracing::info!(
manifest_root = format!("{}", state.manifest_data_cid),
"pre-genesis state created"
);

let genesis_bytes = request.app_state_bytes.to_vec();
let genesis_hash =
fendermint_vm_message::cid(&genesis_bytes).context("failed to compute genesis CID")?;

// Make it easy to spot any discrepancies between nodes.
tracing::info!(genesis_hash = genesis_hash.to_string(), "genesis");

let (state, out) = self
.interpreter
.init(state, genesis_bytes)
.await
.context("failed to init from genesis")?;

let state_root = state.commit().context("failed to commit genesis state")?;
let validators =
to_validator_updates(out.validators).context("failed to convert validators")?;
let (validators, state_params) = read_genesis_car(&genesis_bytes, &self.state_store).await?;
let validators = to_validator_updates(validators).context("failed to convert validators")?;

// Let's pretend that the genesis state is that of a fictive block at height 0.
// The record will be stored under height 1, and the record after the application
Expand All @@ -502,16 +475,7 @@ where
let app_state = AppState {
block_height: height,
oldest_state_height: height,
state_params: FvmStateParams {
state_root,
timestamp: out.timestamp,
network_version: out.network_version,
base_fee: out.base_fee,
circ_supply: out.circ_supply,
chain_id: out.chain_id.into(),
power_scale: out.power_scale,
app_version: 0,
},
state_params,
};

let response = response::InitChain {
Expand Down
16 changes: 16 additions & 0 deletions fendermint/app/src/cmd/genesis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use fendermint_vm_genesis::{
ipc, Account, Actor, ActorMeta, Collateral, Genesis, Multisig, PermissionMode, SignerAddr,
Validator, ValidatorKey,
};
use fendermint_vm_interpreter::genesis::GenesisCreator;

use crate::cmd;
use crate::options::genesis::*;
Expand Down Expand Up @@ -93,6 +94,8 @@ cmd! {
set_ipc_gateway(&genesis_file, args),
GenesisIpcCommands::FromParent(args) =>
new_genesis_from_parent(&genesis_file, args).await,
GenesisIpcCommands::SealState(args) =>
Copy link
Contributor

@karlem karlem Aug 5, 2024

Choose a reason for hiding this comment

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

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

seal_state(&genesis_file, args).await,
}
}
}
Expand Down Expand Up @@ -279,6 +282,19 @@ fn set_ipc_gateway(genesis_file: &PathBuf, args: &GenesisIpcGatewayArgs) -> anyh
})
}

async fn seal_state(genesis_file: &PathBuf, args: &SealGenesisArgs) -> anyhow::Result<()> {
let genesis = read_genesis(genesis_file)?;

let genesis_creator = GenesisCreator::new(
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe GenesisStateCreator?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think should be fine with Genesis, I renamed everything else to align with genesis and removed state.

args.builtin_actors_path.clone(),
args.custom_actors_path.clone(),
args.ipc_artifacts_path.clone(),
args.sealed_genesis_path.clone(),
);

genesis_creator.create(genesis).await
Copy link
Contributor

Choose a reason for hiding this comment

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

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)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

}

async fn new_genesis_from_parent(
genesis_file: &PathBuf,
args: &GenesisFromParentArgs,
Expand Down
2 changes: 2 additions & 0 deletions fendermint/app/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
extern crate core;

// Copyright 2022-2024 Protocol Labs
// SPDX-License-Identifier: Apache-2.0, MIT
mod app;
Expand Down
1 change: 1 addition & 0 deletions fendermint/vm/genesis/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Copy link
Contributor

Choose a reason for hiding this comment

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

Needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed alr


use fendermint_actor_eam::PermissionModeParams;
use fvm_shared::version::NetworkVersion;
Expand Down
11 changes: 11 additions & 0 deletions fendermint/vm/interpreter/src/fvm/state/genesis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,17 @@ where
}
}

/// Flush the data to the block store. Returns the state root cid and the underlying state store.
pub fn finalize(self) -> anyhow::Result<(Cid, DB)> {
Copy link
Contributor

@karlem karlem Aug 6, 2024

Choose a reason for hiding this comment

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

It might be worth to either explain the difference betweenfinalize and commit functions in comment or remove one a keep the other? Now it's a bit ambiguous.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will remove commit method once this PR is merged, currently the old implementation is still there.

match self.stage {
Stage::Tree(_) => Err(anyhow!("invalid finalize state")),
Stage::Exec(exec_state) => match exec_state.commit()? {
(_, _, true) => bail!("FVM parameters are not expected to be updated in genesis"),
(cid, _, _) => Ok((cid, self.store)),
},
}
}

/// Replaces the built in actor with custom actor. This assumes the system actor is already
/// created, else it would throw an error.
pub fn replace_builtin_actor(
Expand Down
4 changes: 2 additions & 2 deletions fendermint/vm/interpreter/src/fvm/state/snapshot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ where
}

#[pin_project::pin_project]
struct StateTreeStreamer<BS> {
pub(crate) struct StateTreeStreamer<BS> {
/// The list of cids to pull from the blockstore
#[pin]
dfs: VecDeque<Cid>,
Expand Down Expand Up @@ -286,7 +286,7 @@ fn walk_ipld_cids(ipld: Ipld, dfs: &mut VecDeque<Cid>) {
}
}

fn derive_cid<T: Serialize>(t: &T) -> anyhow::Result<(Cid, Vec<u8>)> {
pub(crate) fn derive_cid<T: Serialize>(t: &T) -> anyhow::Result<(Cid, Vec<u8>)> {
let bytes = fvm_ipld_encoding::to_vec(&t)?;
let cid = Cid::new_v1(DAG_CBOR, Code::Blake2b256.digest(&bytes));
Ok((cid, bytes))
Expand Down
Loading
Loading