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

Rename ExportGenesisStateCommand to ExportGenesisHeadCommand and make it respect custom genesis block builders #2331

Merged
merged 53 commits into from
Dec 13, 2023
Merged
Show file tree
Hide file tree
Changes from 47 commits
Commits
Show all changes
53 commits
Select commit Hold shift + click to select a range
6fb2596
aura AAAaaa
Nov 14, 2023
89e825b
println and 🤵 emojis
Nov 14, 2023
238a7e0
move log line :facepalm:
Nov 14, 2023
59b2c8f
hack the build-state command
Nov 14, 2023
f58981e
try again
Nov 14, 2023
3caefa5
comment the old generate_genesis_block function
Nov 14, 2023
08365ff
debugging in command
Nov 14, 2023
d456cd0
duplicate error message (To test for async weirdness).
Nov 14, 2023
7c84b90
block backend
Nov 14, 2023
5e20abc
try to have debug output not conflict with actual exported head data
Nov 14, 2023
c72c161
another log
Nov 14, 2023
e4ac486
some more random debugging lines
Nov 15, 2023
ece683a
comment idea to make logs more useful
Nov 15, 2023
cca6d02
format previous head data as hex to better compare to relay state and…
Nov 16, 2023
1b593f7
Remove some logs
Nov 16, 2023
a8e8fb1
Remove some more logging
Nov 16, 2023
439a5d4
Remove panics
Nov 16, 2023
297e1bd
delete commented code
Nov 16, 2023
1f6a2f3
remove some more debugging and tracing
Nov 16, 2023
757dc51
spelling
Nov 16, 2023
85d39ac
fmt
Nov 16, 2023
c187e00
Rename command
Nov 16, 2023
22136dc
update polkadot-parachain (but don't modify it's public CLI)
Nov 16, 2023
35d9518
Update template CLI preserving backward compatability
Nov 16, 2023
01dd857
Do all of the non-cumulus ones that copied the name
Nov 16, 2023
68d858f
And the test service also copied the name
Nov 16, 2023
8f2f471
remove last remaining debugging lines
Nov 16, 2023
81f5dfd
lingering comments
Nov 16, 2023
554fcb3
fmt
Nov 17, 2023
e8d4327
REstore original function in test client to be used only for tests.
Nov 17, 2023
babcd27
Oops, there were still logs left
Nov 17, 2023
30a55f8
comment
Nov 17, 2023
e1566bc
more stray stuff
Nov 17, 2023
50f7f47
x
Nov 17, 2023
31f8a2c
x
Nov 17, 2023
fe5b9f4
fmt
Nov 17, 2023
df69620
fix invocation
Nov 17, 2023
f7c04de
and fix polkadot parachain
Nov 17, 2023
6bc796d
x
Nov 17, 2023
238c874
Doc comments
Nov 17, 2023
89511b6
Basti's suggestion to use clap alias
JoshOrndorff Nov 29, 2023
c41f8a4
fmt
JoshOrndorff Nov 29, 2023
c19c29d
Don't need Block Backend
JoshOrndorff Nov 29, 2023
58a75ea
Merge branch 'master' into polkadot-v1.2.0-hack
JoshOrndorff Nov 29, 2023
a37a3d4
update `command.rs`s to reflect aliases
JoshOrndorff Nov 29, 2023
aa3d14a
update version extraction
JoshOrndorff Dec 12, 2023
0c9dbc2
Merge branch 'master' into polkadot-v1.2.0-hack
JoshOrndorff Dec 12, 2023
3c39cc9
Remove the custom export genesis head command
bkchr Dec 12, 2023
8da60f6
Update cumulus/parachain-template/node/src/cli.rs
bkchr Dec 13, 2023
1e528ee
Remove the custom export genesis wasm command
bkchr Dec 13, 2023
4866c84
Adds prdoc
bkchr Dec 13, 2023
1d5795b
Merge branch 'master' into polkadot-v1.2.0-hack
bkchr Dec 13, 2023
0a21af5
FMT
bkchr Dec 13, 2023
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
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions cumulus/client/cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,4 @@ sc-chain-spec = { path = "../../../substrate/client/chain-spec" }
sc-service = { path = "../../../substrate/client/service" }
sp-core = { path = "../../../substrate/primitives/core" }
sp-runtime = { path = "../../../substrate/primitives/runtime" }
sp-blockchain = { path = "../../../substrate/primitives/blockchain" }
85 changes: 26 additions & 59 deletions cumulus/client/cli/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,20 +23,18 @@ use std::{
io::{self, Write},
net::SocketAddr,
path::PathBuf,
sync::Arc,
};

use codec::Encode;
use sc_chain_spec::ChainSpec;
use sc_client_api::ExecutorProvider;
use sc_client_api::HeaderBackend;
use sc_service::{
config::{PrometheusConfig, TelemetryEndpoints},
BasePath, TransactionPoolOptions,
};
use sp_core::hexdisplay::HexDisplay;
use sp_runtime::{
traits::{Block as BlockT, Hash as HashT, Header as HeaderT, Zero},
StateVersion,
};
use sp_runtime::traits::{Block as BlockT, Zero};
use url::Url;

/// The `purge-chain` command used to remove the whole chain: the parachain and the relay chain.
Expand Down Expand Up @@ -129,9 +127,9 @@ impl sc_cli::CliConfiguration for PurgeChainCmd {
}
}

/// Command for exporting the genesis state of the parachain
/// Command for exporting the genesis head data of the parachain
#[derive(Debug, clap::Parser)]
pub struct ExportGenesisStateCommand {
pub struct ExportGenesisHeadCommand {
/// Output file name or stdout if unspecified.
#[arg()]
pub output: Option<PathBuf>,
Expand All @@ -145,24 +143,29 @@ pub struct ExportGenesisStateCommand {
pub shared_params: sc_cli::SharedParams,
}

impl ExportGenesisStateCommand {
/// Run the export-genesis-state command
pub fn run<Block: BlockT>(
&self,
chain_spec: &dyn ChainSpec,
client: &impl ExecutorProvider<Block>,
) -> sc_cli::Result<()> {
let state_version = sc_chain_spec::resolve_state_version_from_wasm(
&chain_spec.build_storage()?,
client.executor(),
)?;

let block: Block = generate_genesis_block(chain_spec, state_version)?;
let raw_header = block.header().encode();
impl ExportGenesisHeadCommand {
/// Run the export-genesis-head command
pub fn run<B, C>(&self, client: Arc<C>) -> sc_cli::Result<()>
where
B: BlockT,
C: HeaderBackend<B> + 'static,
{
let genesis_hash = client.hash(Zero::zero())?.ok_or(sc_cli::Error::Client(
sp_blockchain::Error::Backend(
"Failed to lookup genesis block hash when exporting genesis head data.".into(),
),
))?;
let genesis_header = client.header(genesis_hash)?.ok_or(sc_cli::Error::Client(
sp_blockchain::Error::Backend(
"Failed to lookup genesis header by hash when exporting genesis head data.".into(),
),
))?;

let raw_header = genesis_header.encode();
let output_buf = if self.raw {
raw_header
} else {
format!("0x{:?}", HexDisplay::from(&block.header().encode())).into_bytes()
format!("0x{:?}", HexDisplay::from(&genesis_header.encode())).into_bytes()
};

if let Some(output) = &self.output {
Expand All @@ -175,43 +178,7 @@ impl ExportGenesisStateCommand {
}
}

/// Generate the genesis block from a given ChainSpec.
pub fn generate_genesis_block<Block: BlockT>(
chain_spec: &dyn ChainSpec,
genesis_state_version: StateVersion,
) -> Result<Block, String> {
let storage = chain_spec.build_storage()?;

let child_roots = storage.children_default.iter().map(|(sk, child_content)| {
let state_root = <<<Block as BlockT>::Header as HeaderT>::Hashing as HashT>::trie_root(
child_content.data.clone().into_iter().collect(),
genesis_state_version,
);
(sk.clone(), state_root.encode())
});
let state_root = <<<Block as BlockT>::Header as HeaderT>::Hashing as HashT>::trie_root(
storage.top.clone().into_iter().chain(child_roots).collect(),
genesis_state_version,
);

let extrinsics_root = <<<Block as BlockT>::Header as HeaderT>::Hashing as HashT>::trie_root(
Vec::new(),
genesis_state_version,
);

Ok(Block::new(
<<Block as BlockT>::Header as HeaderT>::new(
Zero::zero(),
extrinsics_root,
state_root,
Default::default(),
Default::default(),
),
Default::default(),
))
}

impl sc_cli::CliConfiguration for ExportGenesisStateCommand {
impl sc_cli::CliConfiguration for ExportGenesisHeadCommand {
fn shared_params(&self) -> &sc_cli::SharedParams {
&self.shared_params
}
Expand Down
7 changes: 5 additions & 2 deletions cumulus/parachain-template/node/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,11 @@ pub enum Subcommand {
/// Remove the whole chain.
PurgeChain(cumulus_client_cli::PurgeChainCmd),

/// Export the genesis state of the parachain.
ExportGenesisState(cumulus_client_cli::ExportGenesisStateCommand),
/// Export the genesis head data of the parachain.
/// Head data is the encoded block header.
/// This is the same as the old, poorly named, ExportGenesisState command.
bkchr marked this conversation as resolved.
Show resolved Hide resolved
#[command(alias = "export-genesis-state")]
ExportGenesisHead(cumulus_client_cli::ExportGenesisHeadCommand),
JoshOrndorff marked this conversation as resolved.
Show resolved Hide resolved

/// Export the genesis wasm of the parachain.
ExportGenesisWasm(cumulus_client_cli::ExportGenesisWasmCommand),
Expand Down
4 changes: 2 additions & 2 deletions cumulus/parachain-template/node/src/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,12 +162,12 @@ pub fn run() -> Result<()> {
cmd.run(config, polkadot_config)
})
},
Some(Subcommand::ExportGenesisState(cmd)) => {
Some(Subcommand::ExportGenesisHead(cmd)) => {
let runner = cli.create_runner(cmd)?;
runner.sync_run(|config| {
let partials = new_partial(&config)?;

cmd.run(&*config.chain_spec, &*partials.client)
cmd.run(partials.client)
})
},
Some(Subcommand::ExportGenesisWasm(cmd)) => {
Expand Down
3 changes: 2 additions & 1 deletion cumulus/polkadot-parachain/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,8 @@ pub enum Subcommand {
PurgeChain(cumulus_client_cli::PurgeChainCmd),

/// Export the genesis state of the parachain.
ExportGenesisState(cumulus_client_cli::ExportGenesisStateCommand),
#[command(alias = "export-genesis-state")]
ExportGenesisHead(cumulus_client_cli::ExportGenesisHeadCommand),

/// Export the genesis wasm of the parachain.
ExportGenesisWasm(cumulus_client_cli::ExportGenesisWasmCommand),
Expand Down
4 changes: 2 additions & 2 deletions cumulus/polkadot-parachain/src/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -530,10 +530,10 @@ pub fn run() -> Result<()> {
cmd.run(config, polkadot_config)
})
},
Some(Subcommand::ExportGenesisState(cmd)) => {
Some(Subcommand::ExportGenesisHead(cmd)) => {
let runner = cli.create_runner(cmd)?;
runner.sync_run(|config| {
construct_partials!(config, |partials| cmd.run(&*config.chain_spec, &*partials.client))
construct_partials!(config, |partials| cmd.run(partials.client))
})
},
Some(Subcommand::ExportGenesisWasm(cmd)) => {
Expand Down
8 changes: 4 additions & 4 deletions cumulus/test/service/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,23 +60,23 @@ pub enum Subcommand {
BuildSpec(sc_cli::BuildSpecCmd),

/// Export the genesis state of the parachain.
ExportGenesisState(ExportGenesisStateCommand),
ExportGenesisState(ExportGenesisHeadCommand),

/// Export the genesis wasm of the parachain.
ExportGenesisWasm(ExportGenesisWasmCommand),
}

#[derive(Debug, clap::Parser)]
#[group(skip)]
pub struct ExportGenesisStateCommand {
pub struct ExportGenesisHeadCommand {
#[arg(default_value_t = 2000u32)]
pub parachain_id: u32,
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't remember why we have parachain_id here, it looks like it is unused. Since we are at it, you could clean this up if you like.

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 was already feeling a sloppy about doing both the rename and the fix in a single PR. I kind of prefer a separate issue for this.

I also hesitate a little bit because I'm not sure if removing this will break zombienet. And I also don't know the history of when it stopped being used.

Copy link
Contributor

@skunert skunert Nov 30, 2023

Choose a reason for hiding this comment

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

Okay, I will create an issue and can assign it to myself, will come back to this.

Edit: #2567


#[command(flatten)]
pub base: cumulus_client_cli::ExportGenesisStateCommand,
pub base: cumulus_client_cli::ExportGenesisHeadCommand,
}

impl CliConfiguration for ExportGenesisStateCommand {
impl CliConfiguration for ExportGenesisHeadCommand {
fn shared_params(&self) -> &SharedParams {
&self.base.shared_params
}
Expand Down
43 changes: 41 additions & 2 deletions cumulus/test/service/src/genesis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,50 @@
// along with Cumulus. If not, see <http://www.gnu.org/licenses/>.

use codec::Encode;
use cumulus_client_cli::generate_genesis_block;
use cumulus_primitives_core::ParaId;
use cumulus_test_runtime::Block;
use polkadot_primitives::HeadData;
use sp_runtime::traits::Block as BlockT;
use sc_chain_spec::ChainSpec;
use sp_runtime::{
traits::{Block as BlockT, Hash as HashT, Header as HeaderT, Zero},
StateVersion,
};

/// Generate a simple test genesis block from a given ChainSpec.
pub fn generate_genesis_block<Block: BlockT>(
Copy link
Contributor

Choose a reason for hiding this comment

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

DQ: Why do we need to preserve this logic for the test node?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is called in two places. I briefly considered how to eliminate it entirely, but I wasn't familiar enough with the test service. If you have an idea, I think it would be nice to eliminate this entirely. But this already fixes a real bug for me, so if this cleanup gets too complex, maybe we can do it in a followup.

chain_spec: &dyn ChainSpec,
genesis_state_version: StateVersion,
) -> Result<Block, String> {
let storage = chain_spec.build_storage()?;

let child_roots = storage.children_default.iter().map(|(sk, child_content)| {
let state_root = <<<Block as BlockT>::Header as HeaderT>::Hashing as HashT>::trie_root(
child_content.data.clone().into_iter().collect(),
genesis_state_version,
);
(sk.clone(), state_root.encode())
});
let state_root = <<<Block as BlockT>::Header as HeaderT>::Hashing as HashT>::trie_root(
storage.top.clone().into_iter().chain(child_roots).collect(),
genesis_state_version,
);

let extrinsics_root = <<<Block as BlockT>::Header as HeaderT>::Hashing as HashT>::trie_root(
Vec::new(),
genesis_state_version,
);

Ok(Block::new(
<<Block as BlockT>::Header as HeaderT>::new(
Zero::zero(),
extrinsics_root,
state_root,
Default::default(),
Default::default(),
),
Default::default(),
))
}

/// Returns the initial head data for a parachain ID.
pub fn initial_head_data(para_id: ParaId) -> HeadData {
Expand Down
4 changes: 3 additions & 1 deletion cumulus/test/service/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@
pub mod bench_utils;

pub mod chain_spec;
mod genesis;

/// Utilities for creating test genesis block and head data
pub mod genesis;

use runtime::AccountId;
use sc_executor::{HeapAllocStrategy, WasmExecutor, DEFAULT_HEAP_ALLOC_STRATEGY};
Expand Down
10 changes: 7 additions & 3 deletions cumulus/test/service/src/main.rs
Copy link
Contributor

Choose a reason for hiding this comment

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

Please replace the state version extraction by this, then we can merge for now 👍

let wasm_executor: WasmExecutor<sp_io::SubstrateHostFunctions> =WasmExecutor::builder().build();
let state_version = sc_chain_spec::resolve_state_version_from_wasm(
		&spec.build_storage()?,
		&wasm_executor,
)?;

Copy link
Member

Choose a reason for hiding this comment

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

Not sure why we have the custom export logic there any way. We can just call the default code for ExportGenesisState command.

@JoshOrndorff can you please do this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry for the delay. I've done it now.

Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,8 @@ mod cli;
use std::{io::Write, sync::Arc};

use cli::{RelayChainCli, Subcommand, TestCollatorCli};
use cumulus_client_cli::generate_genesis_block;
use cumulus_primitives_core::{relay_chain::CollatorPair, ParaId};
use cumulus_test_service::AnnounceBlockFn;
use cumulus_test_service::{genesis::generate_genesis_block, AnnounceBlockFn};
use polkadot_service::runtime_traits::AccountIdConversion;
use sc_cli::{CliConfiguration, SubstrateCli};
use sp_core::{hexdisplay::HexDisplay, Encode, Pair};
Expand Down Expand Up @@ -51,7 +50,12 @@ fn main() -> Result<(), sc_cli::Error> {

let spec =
cli.load_spec(&params.base.shared_params.chain.clone().unwrap_or_default())?;
let state_version = cumulus_test_service::runtime::VERSION.state_version();
let wasm_executor: WasmExecutor<sp_io::SubstrateHostFunctions> =
WasmExecutor::builder().build();
let state_version = sc_chain_spec::resolve_state_version_from_wasm(
&spec.build_storage()?,
&wasm_executor,
)?;

let block: parachains_common::Block = generate_genesis_block(&*spec, state_version)?;
let raw_header = block.header().encode();
Expand Down
6 changes: 3 additions & 3 deletions polkadot/parachain/test-parachains/adder/collator/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,16 @@ use sc_cli::SubstrateCli;
pub enum Subcommand {
/// Export the genesis state of the parachain.
#[command(name = "export-genesis-state")]
ExportGenesisState(ExportGenesisStateCommand),
ExportGenesisState(ExportGenesisHeadCommand),

/// Export the genesis wasm of the parachain.
#[command(name = "export-genesis-wasm")]
ExportGenesisWasm(ExportGenesisWasmCommand),
}

/// Command for exporting the genesis state of the parachain
/// Command for exporting the genesis head data of the parachain
#[derive(Debug, Parser)]
pub struct ExportGenesisStateCommand {}
pub struct ExportGenesisHeadCommand {}

/// Command for exporting the genesis wasm file.
#[derive(Debug, Parser)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,16 @@ use std::path::PathBuf;
pub enum Subcommand {
/// Export the genesis state of the parachain.
#[command(name = "export-genesis-state")]
ExportGenesisState(ExportGenesisStateCommand),
ExportGenesisState(ExportGenesisHeadCommand),

/// Export the genesis wasm of the parachain.
#[command(name = "export-genesis-wasm")]
ExportGenesisWasm(ExportGenesisWasmCommand),
}

/// Command for exporting the genesis state of the parachain
/// Command for exporting the genesis head data of the parachain
#[derive(Debug, Parser)]
pub struct ExportGenesisStateCommand {
pub struct ExportGenesisHeadCommand {
/// Output file name or stdout if unspecified.
#[arg()]
pub output: Option<PathBuf>,
Expand Down
Loading