API to set parameters through CLI

[mversic]

We should add support to set parameters through iroha CLI

[divyaranjan1905]

I would like to be assigned with this issue to get started with it. Also, can you give me some idea about what sort of paramters are required to be set through CLI?

[nxsaken]

@divyaranjan1905 hi, this is the instruction that's used for setting chain-wide parameters:

iroha/data_model/src/isi.rs

Line 262 in cacc805

pub struct SetParameter(pub Parameter);

[divyaranjan1905]

@nxsaken Tried looking into surrounding crates to get an idea. Can we make this available to CLI by setting:

#[cfg(feature = "transparent_api")]

On top of this, I guess we also need to import the struct SetParameter somewhere else to use it? Or, does the model_single! macro take care of it?

Let me know where I'm off 🙏

[nxsaken]

@divyaranjan1905 you need to add a subcommand to iroha_client_cli that executes SetParameter. Here's a PR that you might use as a reference: #4408 (or you can check the git history to see how others have approached it).

[divyaranjan1905]

@nxsaken Thank you for the reference. From what I understand from there, after adding Parameters to enum SubCommand I defined a mod param that would contain all the stuff.

What I'm doubtful of is that, do I need to create a struct that contains all the parameters defined in:

iroha/data_model/src/parameter.rs

Line 30 in d8e65f0

/// Id of a custom parameter

The commit you mentioned does this in a struct Burn where they take in the necessary id, key etc.

[nxsaken]

@mversic how should a parameter be parsed from a CLI command?

[mversic]

@a-zorina can you give us an idea on this?

[divyaranjan1905]

Can we try going further with this?

[nxsaken]

Let's consider this. The Parameter enum contains all possible parameter variants that can be set:

iroha/data_model/src/parameter.rs

Lines 280 to 287 in d8e65f0

	pub enum Parameter {
	Sumeragi(SumeragiParameter),
	Block(BlockParameter),
	Transaction(TransactionParameter),
	SmartContract(SmartContractParameter),
	Executor(SmartContractParameter),
	Custom(CustomParameter),
	}

Each of the variants can also subdivide into further variants, for example SumeragiParameter:

iroha/data_model/src/parameter.rs

Lines 98 to 101 in d8e65f0

	pub enum SumeragiParameter {
	BlockTimeMs(u64),
	CommitTimeMs(u64),
	}

There is also CustomParameter, which allows using user-defined parameters:

iroha/data_model/src/parameter.rs

Lines 214 to 222 in d8e65f0

	pub struct CustomParameter {
	/// Unique id of the parameter.
	pub id: CustomParameterId,
	/// Payload containing actual value.
	///
	/// It is JSON-encoded, and its structure must correspond to the structure of
	/// the type defined in [`crate::executor::ExecutorDataModel`].
	pub payload: JsonString,
	}

We could mirror this hierarchy as CLI subcommands and arguments:

iroha set sumeragi block_time_ms 2000
iroha set block max_transactions 512
iroha set custom --id my_param --payload "{\"key\":\"value\"}"

@mversic @a-zorina thoughts?

[mversic]

let's talk in terms of how using iroha cli would look like. We have to have both set and get parameter operations.

We could have something like:

iroha parameter set sumeragi block_time_ms 2000
iroha parameter get block max_transactions
iroha parameter set my_param "{\"key\":\"value\"}"
iroha parameter get all

I don't think that we should differentiate between built-in and custom parameters, but I'm not completely sure if that's technically feasible

[nxsaken]

I think it should be possible to define dynamic subcommands with Clap, the builder API may be helpful here, haven't tried that myself. If it's too messy, it's better to leave it and focus on the common case.

@divyaranjan1905 I think this should unblock you, don't hesitate to reach out if you get stuck or anything.

[divyaranjan1905]

We could mirror this hierarchy as CLI subcommands and arguments:

iroha set sumeragi block_time_ms 2000
iroha set block max_transactions 512
iroha set custom --id my_param --payload "{\"key\":\"value\"}"

@nxsaken I was thinking along similar lines. I had defined a enum (say, Set) that stores the particular parameter that has been provided by the user through CLI, that they want to set. The enum looks vaguely like this:

pub enum Set {
  Sumeragi(SumeragiParameter),
  Block(BlockParameter),
}

And then I can have an impl of this Set that can match for the particular parameter that has been selected by enum Set and do the necessary setting. And for that we use SetParameter from isi.rs:

iroha/data_model/src/isi.rs

Line 262 in cacc805

pub struct SetParameter(pub Parameter);

Does this look fine? Now that @mversic mentioned about also getting existing parameters, I guess this enum needs to be renamed and the operation might change a bit, but does the overall idea seem right?

Moreover, I had a question regarding the CLI usage:

In parameters.rs there's a struct for multiple parameters like SumeragiParameters and an enum for single parameter like SumeragiParameter. The above enum Set only takes the latter, but which one are to be referred to? Or, both of them must be implemented?

[nxsaken]

Yeah, you'd probably need both Get and Set. They are almost the same, except Get has the Get::All option, while Set has a value argument for each parameter. Something like the following:

#[derive(clap::Subcommand)
enum Parameter {
    Get(parameter::Get),
    Set(iroha_data_model::Parameter),
}

mod parameter {
    #[derive(clap::Subcommand)
    enum Get {
        All,
        #[command(flatten)]
        Inner(iroha_data_model::ParameterNoDataFields), // this doesn't exist
    }
}

You should probably implement some clap-related parsing for iroha_data_model::Parameter to be compatible with the command to avoid code duplication. On a second thought, iroha_data_model::Parameter doesn't have a version without data fields, so it could make sense to repeat the whole hierarchy in the CLI – or add a data-less version of the hierarchy to iroha_data_model and reuse it here.

Ignore SumeragiParameters, it's not a Parameter itself.

[divyaranjan1905]

On a second thought, iroha_data_model::Parameter doesn't have a version without data fields

@nxsaken I don't think I understand this, iroha_data_model::Parameter is an enum with variants referring to other enums themselves that contain the parameter(s). Why would we need something without data fields (variants?)?

[nxsaken]

@divyaranjan1905 so that the user can specify which parameter they want to read (there is no value to provide)

[divyaranjan1905]

Okay, got it. I would need to look deeper into clap and probably try some traits to take care of the parsing.

[divyaranjan1905]

Here's something weird happening, that either of you might have a clue about @nxsaken @mversic. I am creating an enum labelling different subcommands and their arguments, as below:

 #[derive(Subcommand, Debug)]
 pub enum ParamCmd {
 #[command(about = "Setting Sumeragi Parameters.")]
     Sumeragi(SumeragiCmd),
 #[command(about = "Setting Block Parameters.")]
     Block(BlockCmd),
 #[command(about = "Setting Transaction Parameters.")]
     Transaction(TransactionCmd),
 #[command(about = "Setting Smart Contract Parameters.")]
     SmartContract(SmartContractCmd),
 #[command(about = "Setting Custom Parameters.")]
     Custom(CustomCmd),
 }

For each one of these, I have implemented another enum as below:

    #[derive(Subcommand, Debug)]
    enum SumeragiCmd {
	#[command(about = "Set block time in milliseconds.")]
	BlockTime {
	    #[arg(long = "block-time" )]
	    value: u64,
	},

	#[command(about = "Set commit time in milliseconds.")]
	CommitTime {
	    #[arg(long = "commit-time")]
	    value: u64,
	}
    }

With this we now have subcommands, for selecting sumeragi parameter and then choosing which parameter to get/set. The problem is that for some weird reason, I am getting the following error:

rustc [E0277]: error[E0277]: the trait bound `SumeragiCmd: clap::Args` is not satisfied
    --> client_cli/src/main.rs:1090:18
     |
1090 |         Sumeragi(SumeragiCmd),
     |                  ^^^^^^^^^^^ the trait `clap::Args` is not implemented for `SumeragiCmd`
     |
     = help: the following other types implement trait `clap::Args`:
               (dyn clap::Args + 'static)
               Box<T>
               Get
               GetKeyValue
               ListPermissions
               MetadataArgs
               MetadataValueArg
               ParamCli
             and 19 others

I was wondering if this would be a clap issue, but before reporting to them I copied the above into a cargo new one, but the error vanishes in the new crate. Is this to do with the internal types of client_cli that use clap::Args trait?

[divyaranjan1905]

Also, I get the following error when importing use clap::{Parser, Args, Subcommand}.

rustc [E0603]: the trait import `Args` is defined here...

[nxsaken]

In SumeragiCmd, I don't think you need the #[command] attribute on the variants. Or you can do something similar to MetadataArgs: derive clap::Args for it, then use it with #[command(flatten)].

Importing Args might clash with the Args structs present in main.rs.

If you still have the error, could you push the code, and maybe even open a draft PR? It would be easier to diagnose issues that way and exchange feedback.

[divyaranjan1905]

Okay, sure. I'll open a draft PR as soon as I can get the overall ideas implemented.

[divyaranjan1905]

Or you can do something similar to MetadataArgs: derive clap::Args for it, then use it with #[command(flatten)].

The thing is, clap::Args doesn't work with enums, only with structs.

[nxsaken]

I should've specified what "it" is. What I'm saying is make a struct ParameterValue { value: ... }, where ParameterValue is clap::Args, and value has #[arg], then reuse it across the enum variants. The enum variant itself gets #[command(flatten)]. Otherwise, when having the fields directly inside the variant, the variant doesn't need any attribute.