Skip to content

Commit

Permalink
[fix] rollback --submit-genesis argument changes
Browse files Browse the repository at this point in the history 
Signed-off-by: VAmuzing <amuzik95@gmail.com>
  • Loading branch information
@VAmuzing
VAmuzing committed Apr 3, 2024
1 parent ace1afa commit 0e57f34
Show file tree
Hide file tree
Showing 10 changed files with 185 additions and 39 deletions.
9 changes: 5 additions & 4 deletions cli/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use core::sync::atomic::{AtomicBool, Ordering};
use std::{fs, path::Path, sync::Arc};

use color_eyre::eyre::{eyre, Result, WrapErr};
use iroha_config::parameters::actual::Root as Config;
use iroha_config::parameters::{actual::Root as Config, user::CliContext};
use iroha_core::{
block_sync::{BlockSynchronizer, BlockSynchronizerHandle},
gossiper::{TransactionGossiper, TransactionGossiperHandle},
Expand Down Expand Up @@ -511,10 +511,11 @@ fn genesis_domain(public_key: PublicKey) -> Domain {
/// - If failed to build a genesis network
pub fn read_config_and_genesis<P: AsRef<Path>>(
path: Option<P>,
submit_genesis: bool,
) -> Result<(Config, Option<GenesisNetwork>)> {
use iroha_config::parameters::actual::Genesis;

let config = Config::load(path)
let config = Config::load(path, CliContext { submit_genesis })
.wrap_err("failed to load configuration")?;

let genesis = if let Genesis::Full { public_key: _, file } = &config.genesis {
Expand Down Expand Up @@ -618,7 +619,7 @@ mod tests {

// When

let (config, genesis) = read_config_and_genesis(Some(config_path))?;
let (config, genesis) = read_config_and_genesis(Some(config_path), true)?;

// Then

Expand Down Expand Up @@ -667,7 +668,7 @@ mod tests {

// When & Then

let report = read_config_and_genesis(Some(config_path)).unwrap_err();
let report = read_config_and_genesis(Some(config_path), false).unwrap_err();

assert_contains!(
format!("{report:#}"),
Expand Down
16 changes: 15 additions & 1 deletion cli/src/main.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,19 @@ struct Args {
num_args(0..=1),
)]
terminal_colors: bool,
/// Whether the current peer should submit the genesis block or not
///
/// Only one peer in the network should submit the genesis block.
///
/// This argument must be set alongside with `genesis.file` and `genesis.private_key`
/// configuration options. If not, Iroha will exit with an error.
///
/// In case when the network consists only of this one peer, i.e. the amount of trusted
/// peers in the configuration (`sumeragi.trusted_peers`) is less than 2, this peer must
/// submit the genesis, since there are no other peers who can provide it. In this case, Iroha
/// will exit with an error if `--submit-genesis` is not set.
#[arg(long)]
submit_genesis: bool,
}

#[tokio::main]
Expand All @@ -44,7 +57,7 @@ async fn main() -> Result<()> {
color_eyre::install()?;
}

let (config, genesis) = iroha::read_config_and_genesis(args.config)?;
let (config, genesis) = iroha::read_config_and_genesis(args.config, args.submit_genesis)?;
let logger = iroha_logger::init_global(&config.logger, args.terminal_colors)?;

iroha_logger::info!(
Expand Down Expand Up @@ -75,6 +88,7 @@ mod tests {
let args = Args::try_parse_from(["test"])?;

assert_eq!(args.terminal_colors, is_colouring_supported());
assert_eq!(args.submit_genesis, false);

Ok(())
}
Expand Down
6 changes: 4 additions & 2 deletions cli/src/samples.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use iroha_config::{
base::{HumanDuration, UnwrapPartial},
parameters::{
actual::Root as Config,
user::RootPartial as UserConfig,
user::{CliContext, RootPartial as UserConfig},
},
snapshot::Mode as SnapshotMode,
};
Expand Down Expand Up @@ -113,7 +113,9 @@ pub fn get_config(
get_user_config(trusted_peers, chain_id, key_pair)
.unwrap_partial()
.expect("config should build as all required fields were provided")
.parse()
.parse(CliContext {
submit_genesis: true,
})
.expect("config should finalize as the input is semantically valid (or there is a bug)")
}

Expand Down
11 changes: 5 additions & 6 deletions config/src/parameters/actual.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use crate::{
kura::InitMode,
parameters::{
defaults, user,
user::RootPartial,
user::{CliContext, RootPartial},
},
};

Expand Down Expand Up @@ -53,14 +53,14 @@ impl Root {
/// - unable to load config from a TOML file
/// - unable to parse config from envs
/// - the config is invalid
pub fn load<P: AsRef<Path>>(path: Option<P>) -> Result<Self, eyre::Report> {
pub fn load<P: AsRef<Path>>(path: Option<P>, cli: CliContext) -> Result<Self, eyre::Report> {
let from_file = path.map(RootPartial::from_toml).transpose()?;
let from_env = RootPartial::from_env(&StdEnv)?;
let merged = match from_file {
Some(x) => x.merge(from_env),
None => from_env,
};
let config = merged.unwrap_partial()?.parse()?;
let config = merged.unwrap_partial()?.parse(cli)?;
Ok(config)
}
}
Expand Down Expand Up @@ -91,10 +91,9 @@ pub enum Genesis {
},
/// The peer is responsible for submitting the genesis block
Full {
/// Genesis account key pair
// key_pair: KeyPair,
/// Genesis account public key
public_key: PublicKey,
/// Path to the [`RawGenesisBlock`]
/// Path to the signed genesis block
file: PathBuf,
},
}
Expand Down
68 changes: 57 additions & 11 deletions config/src/parameters/user.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ impl Root {
///
/// # Errors
/// If any invalidity found.
pub fn parse(self) -> Result<actual::Root, ErrorsCollection<Report>> {
pub fn parse(self, cli: CliContext) -> Result<actual::Root, ErrorsCollection<Report>> {
let mut emitter = Emitter::new();

let key_pair =
Expand All @@ -146,7 +146,14 @@ impl Root {
None
}, Some);

let genesis = self.genesis.parse();
let genesis = self.genesis.parse(cli).map_or_else(
|err| {
// FIXME
emitter.emit(eyre!("{err}"));
None
},
Some,
);

let kura = self.kura.parse();

Expand All @@ -158,6 +165,18 @@ impl Root {
Some,
);

if let Some(ref config) = sumeragi {
if !cli.submit_genesis && config.trusted_peers.len() == 0 {
emitter.emit(eyre!("\
The network consists from this one peer only (no `sumeragi.trusted_peers` provided). \
Since `--submit-genesis` is not set, there is no way to receive the genesis block. \
Either provide the genesis by setting `--submit-genesis` argument, `genesis.public_key`, \
and `genesis.file` configuration parameters, or increase the number of trusted peers in \
the network using `sumeragi.trusted_peers` configuration parameter.\
"));
}
}

let (p2p_address, block_sync, transaction_gossiper) = self.network.parse();

let logger = self.logger;
Expand Down Expand Up @@ -190,6 +209,7 @@ impl Root {
p2p_address,
};
let (telemetry, dev_telemetry) = telemetries.unwrap();
let genesis = genesis.unwrap();
let sumeragi = {
let mut x = sumeragi.unwrap();
x.trusted_peers.push(peer.peer_id());
Expand All @@ -215,6 +235,11 @@ impl Root {
}
}

#[derive(Copy, Clone)]
pub struct CliContext {
pub submit_genesis: bool,
}

pub(crate) fn private_key_from_env<E: Error>(
emitter: &mut Emitter<Report>,
env: &impl ReadEnv<E>,
Expand Down Expand Up @@ -287,22 +312,43 @@ pub struct Genesis {
}

impl Genesis {
fn parse(self) -> actual::Genesis {
match self.file {
None => actual::Genesis::Partial {
// fn parse(self, cli: CliContext) -> actual::Genesis {
// match self.file {
// None => actual::Genesis::Partial {
// public_key: self.public_key,
// },
// Some(file) => actual::Genesis::Full {
// public_key: self.public_key,
// file,
// },
// // (Some(_), Some(_), false) => Err(GenesisConfigError::GenesisWithoutSubmit),
// // (None, None, true) => Err(GenesisConfigError::SubmitWithoutGenesis),
// // _ => Err(GenesisConfigError::Inconsistent),
// }
// }
fn parse(self, cli: CliContext) -> Result<actual::Genesis, GenesisConfigError> {
match (self.file, cli.submit_genesis) {
(None, false) => Ok(actual::Genesis::Partial {
public_key: self.public_key,
},
Some(file) => actual::Genesis::Full {
}),
(Some(file), true) => Ok(actual::Genesis::Full {
public_key: self.public_key,
file,
},
// (Some(_), Some(_), false) => Err(GenesisConfigError::GenesisWithoutSubmit),
// (None, None, true) => Err(GenesisConfigError::SubmitWithoutGenesis),
// _ => Err(GenesisConfigError::Inconsistent),
}),
(Some(_), false) => Err(GenesisConfigError::GenesisWithoutSubmit),
(None, true) => Err(GenesisConfigError::SubmitWithoutGenesis),
}
}
}

#[derive(Debug, displaydoc::Display, thiserror::Error)]
pub enum GenesisConfigError {
/// `genesis.file` and `genesis.public_key` are presented, but `--submit-genesis` was not set
GenesisWithoutSubmit,
/// `--submit-genesis` was set, but `genesis.file` and `genesis.public_key` are not presented
SubmitWithoutGenesis,
}

#[derive(Debug)]
pub struct Kura {
pub init_mode: KuraInitMode,
Expand Down
40 changes: 32 additions & 8 deletions config/tests/fixtures.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,9 @@ fn test_env_from_file(p: impl AsRef<Path>) -> TestEnv {
fn minimal_config_snapshot() -> Result<()> {
let config = RootPartial::from_toml(fixtures_dir().join("minimal_with_trusted_peers.toml"))?
.unwrap_partial()?
.parse()?;
.parse(CliContext {
submit_genesis: false,
})?;

let expected = expect_test::expect![[r#"
Root {
Expand Down Expand Up @@ -174,16 +176,22 @@ fn minimal_config_snapshot() -> Result<()> {
fn config_with_genesis() -> Result<()> {
let _config = RootPartial::from_toml(fixtures_dir().join("minimal_alone_with_genesis.toml"))?
.unwrap_partial()?
.parse()?;
.parse(CliContext {
submit_genesis: true,
})?;

Ok(())
}

// TODO: Verify
#[test]
#[ignore = "--submit-genesis was removed, more in #4225"]
fn minimal_with_genesis_but_no_cli_arg_fails() -> Result<()> {
let error = RootPartial::from_toml(fixtures_dir().join("minimal_alone_with_genesis.toml"))?
.unwrap_partial()?
.parse()
.parse(CliContext {
submit_genesis: false,
})
.expect_err("should fail since `--submit-genesis=false`");

let expected = expect_test::expect![[r#"
Expand All @@ -199,7 +207,9 @@ fn minimal_with_genesis_but_no_cli_arg_fails() -> Result<()> {
fn minimal_without_genesis_but_with_submit_fails() -> Result<()> {
let error = RootPartial::from_toml(fixtures_dir().join("minimal_with_trusted_peers.toml"))?
.unwrap_partial()?
.parse()
.parse(CliContext {
submit_genesis: true,
})
.expect_err(
"should fail since there is no genesis in the config, but `--submit-genesis=true`",
);
Expand All @@ -214,7 +224,9 @@ fn minimal_without_genesis_but_with_submit_fails() -> Result<()> {
fn self_is_presented_in_trusted_peers() -> Result<()> {
let config = RootPartial::from_toml(fixtures_dir().join("minimal_alone_with_genesis.toml"))?
.unwrap_partial()?
.parse()?;
.parse(CliContext {
submit_genesis: true,
})?;

assert!(config
.sumeragi
Expand Down Expand Up @@ -257,7 +269,9 @@ fn inconsistent_genesis_config() -> Result<()> {
let error = RootPartial::from_toml(fixtures_dir().join("inconsistent_genesis.toml"))?
.unwrap_partial()
.expect("all fields are present")
.parse()
.parse(CliContext {
submit_genesis: false,
})
.expect_err("should fail with bad genesis config");

let expected = expect_test::expect![[r#"
Expand Down Expand Up @@ -433,7 +447,9 @@ fn config_from_file_and_env() -> Result<()> {
let _config = RootPartial::from_toml(fixtures_dir().join("minimal_file_and_env.toml"))?
.merge(RootPartial::from_env(&env)?)
.unwrap_partial()?
.parse()?;
.parse(CliContext {
submit_genesis: false,
})?;

Ok(())
}
Expand All @@ -443,7 +459,9 @@ fn fails_if_torii_address_and_p2p_address_are_equal() -> Result<()> {
let error = RootPartial::from_toml(fixtures_dir().join("bad.torii_addr_eq_p2p_addr.toml"))?
.unwrap_partial()
.expect("should not fail, all fields are present")
.parse()
.parse(CliContext {
submit_genesis: false,
})
.expect_err("should fail because of bad input");

let expected =
Expand Down Expand Up @@ -486,6 +504,9 @@ fn multiple_extends_works() -> Result<()> {
fn full_config_parses_fine() {
let _cfg = Root::load(
Some(fixtures_dir().join("full.toml")),
CliContext {
submit_genesis: true,
},
)
.expect("should be fine");
}
Expand All @@ -494,6 +515,9 @@ fn full_config_parses_fine() {
fn absolute_paths_are_preserved() {
let cfg = Root::load(
Some(fixtures_dir().join("absolute_paths.toml")),
CliContext {
submit_genesis: true,
},
)
.expect("should be fine");

Expand Down
4 changes: 4 additions & 0 deletions core/src/kiso.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -158,10 +158,14 @@ mod tests {
use super::*;

fn test_config() -> Root {
use iroha_config::parameters::user::CliContext;

Root::load(
// FIXME Specifying path here might break!
Some("../config/iroha_test_config.toml"),
CliContext {
submit_genesis: true,
},
)
.expect("test config should be valid, it is probably a bug")
}
Expand Down
6 changes: 4 additions & 2 deletions core/test_network/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -759,7 +759,7 @@ impl TestConfig for Config {
fn test() -> Self {
use iroha_config::{
base::{FromEnv as _, StdEnv, UnwrapPartial as _},
parameters::user::RootPartial,
parameters::user::{CliContext, RootPartial},
};

let mut layer = iroha::samples::get_user_config(
Expand All @@ -776,7 +776,9 @@ impl TestConfig for Config {
layer
.unwrap_partial()
.expect("should not fail as all fields are present")
.parse()
.parse(CliContext {
submit_genesis: true,
})
.expect("Test Iroha config failed to build. This is likely to be a bug.")
}

Expand Down

0 comments on commit 0e57f34

Please sign in to comment.