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: Use AccountComponentTemplate #680

Merged
merged 8 commits into from
Jan 27, 2025
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,10 @@
* Created functions for creating standard notes and note scripts easily on the web client (#686).
* [BREAKING] Renamed plural modules to singular (#687).
* [BREAKING] Made `idxdb` only usable on WASM targets (#685).
* Added fixed seed option for web client generation (#688)
* Added fixed seed option for web client generation (#688).
* [BREAKING] Updated `init` command in the CLI to receive a `--network` flag (#690).
* Improved CLI error messages (#682).
* Added account creation from component templates (#680).

### Fixes

Expand Down
12 changes: 6 additions & 6 deletions Cargo.lock

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

8 changes: 2 additions & 6 deletions bin/miden-cli/src/commands/account.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,12 +188,8 @@ pub async fn show_account<R: FeltRng>(

println!("Storage: \n");

let mut table = create_dynamic_table(&[
"Item Slot Index",
"Item Slot Type",
"Value Arity",
"Value/Commitment",
]);
let mut table =
create_dynamic_table(&["Item Slot Index", "Item Slot Type", "Value/Commitment"]);

for (idx, entry) in account_storage.slots().iter().enumerate() {
let item = account_storage
Expand Down
6 changes: 3 additions & 3 deletions bin/miden-cli/src/commands/init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,19 +102,19 @@ impl InitCmd {
};

let config_as_toml_string = toml::to_string_pretty(&cli_config).map_err(|err| {
CliError::Config("Failed to serialize config".to_string().into(), err.to_string())
CliError::Config("failed to serialize config".to_string().into(), err.to_string())
})?;

let mut file_handle = File::options()
.write(true)
.create_new(true)
.open(&config_file_path)
.map_err(|err| {
CliError::Config("Failed to create config file".to_string().into(), err.to_string())
CliError::Config("failed to create config file".to_string().into(), err.to_string())
})?;

file_handle.write(config_as_toml_string.as_bytes()).map_err(|err| {
CliError::Config("Failed to write config file".to_string().into(), err.to_string())
CliError::Config("failed to write config file".to_string().into(), err.to_string())
})?;

println!("Config file successfully created at: {:?}", config_file_path);
Expand Down
157 changes: 142 additions & 15 deletions bin/miden-cli/src/commands/new_account.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,21 @@
use std::{collections::BTreeMap, fs, io::Write, path::PathBuf};

use clap::{Parser, ValueEnum};
use miden_client::{
account::{
AccountBuilder, AccountStorageMode, AccountType, BasicFungibleFaucetComponent,
BasicWalletComponent, RpoFalcon512Component,
component::{
AccountComponent, AccountComponentTemplate, BasicFungibleFaucet, BasicWallet,
InitStorageData, MapRepresentation, PlaceholderType, RpoFalcon512, StorageValue,
},
AccountBuilder, AccountStorageMode, AccountType,
},
assets::TokenSymbol,
auth::AuthSecretKey,
crypto::{FeltRng, SecretKey},
Client, Felt,
utils::Deserializable,
Client, Felt, Word,
};
use miden_lib::utils::parse_hex_string_as_word;

use crate::{
commands::account::maybe_set_default_account, errors::CliError, utils::load_config_file,
Expand All @@ -30,6 +37,81 @@ impl From<CliAccountStorageMode> for AccountStorageMode {
}
}

/// Helper function to process extra component templates.
/// It reads user input for each placeholder in a component template.
// TODO: this could take a TOML file with key-values
fn process_component_templates(
extra_components: &[AccountComponentTemplate],
) -> Result<Vec<AccountComponent>, CliError> {
let mut account_components = vec![];
for component_template in extra_components {
let mut init_storage_data = BTreeMap::new();
for (placeholder_key, placeholder_type) in
component_template.metadata().get_unique_storage_placeholders()
{
print!(
"Enter hex value for placeholder '{}' (type: {}): ",
placeholder_key, placeholder_type
);
std::io::stdout().flush()?;

let mut input_value = String::new();
std::io::stdin().read_line(&mut input_value)?;
let input_value = input_value.trim();
Comment on lines +40 to +60
Copy link
Contributor

Choose a reason for hiding this comment

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

This approach is fine for this PR, but it is actually different from how I originally thought about it. Specifically, I was imagining that we'd prompt the user to provide data for specific fields and so we'd show a filed name and a field description to the user (rather than showing placeholder and type).

This may require refactoring the structure in miden-base - maybe to move away from placeholder-based approach and more toward specifying something like a "type" or a "kind" for a given storage slot. Let's create an issue for this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I see. I think we could show the storage slot metadata here (name and description), but not specifically for the values themselves although the placeholder itself does not carry this data (so it would have to be searched in the template metadata). Is this what you meant? Or did you mean more specific to the value?

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 ideally we'd iterate over all slots that require user input, show slot name/description, and allow user to enter the required values (there may be more than one value needed per slot). What we now call placeholder may need to be treated more like a "type" of the data. For example, we may have 3 slots each expecting Falcon public keys and so all 3 would have type auth::rpo_falcon512 (or something like this) - but the user should be able to provide 3 different public keys.

There are a few details to think through here - so, probably warrants a separate issue.


match placeholder_type {
PlaceholderType::Felt => {
let value = input_value
.strip_prefix("0x")
.ok_or("error parsing input: Missing 0x prefix".to_string())
.map(|hex| {
u64::from_str_radix(hex, 16).map_err(|e| {
CliError::Parse(e.into(), "failed to parse hex from input".into())
})
})
.map_err(|e| {
CliError::Parse(e.into(), "failed to parse hex from input".into())
})??;

init_storage_data
.insert(placeholder_key.clone(), StorageValue::Felt(Felt::new(value)));
},
PlaceholderType::Map => {
// TODO: Test this case further
let map: MapRepresentation = toml::from_str(input_value).map_err(|e| {
CliError::Parse(e.into(), "failed to parse map from input".into())
})?;
let map = map.try_build_map(&Default::default()).map_err(|e| {
CliError::Parse(e.into(), "failed to parse map from input".into())
})?;

init_storage_data.insert(placeholder_key.clone(), StorageValue::Map(map));
},
PlaceholderType::Word => {
let word: Word = parse_hex_string_as_word(input_value).map_err(|e| {
CliError::Parse(e.into(), "failed to parse hex from input".into())
})?;

init_storage_data.insert(placeholder_key.clone(), StorageValue::Word(word));
},
}
}
let component = AccountComponent::from_template(
component_template,
&InitStorageData::new(init_storage_data),
)
.map_err(|e| CliError::Account(e, "error instantiating component from template".into()))?
.with_supports_all_types();

account_components.push(component);
}

Ok(account_components)
}

// NEW FAUCET
// ================================================================================================

#[derive(Debug, Parser, Clone)]
/// Create a new faucet account.
pub struct NewFaucetCmd {
bobbinth marked this conversation as resolved.
Show resolved Hide resolved
Expand All @@ -47,6 +129,9 @@ pub struct NewFaucetCmd {
decimals: Option<u8>,
#[clap(short, long)]
max_supply: Option<u64>,
#[clap(short, long)]
/// List of additional component template files to include.
template_files: Vec<PathBuf>,
}

impl NewFaucetCmd {
Expand All @@ -62,6 +147,18 @@ impl NewFaucetCmd {
));
}

let mut extra_components = Vec::new();
for path in &self.template_files {
let bytes = fs::read(path)?;
let template = AccountComponentTemplate::read_from_bytes(&bytes).map_err(|e| {
CliError::AccountComponentError(
Box::new(e),
"failed to read account component template".into(),
)
})?;
extra_components.push(template);
}

let decimals = self.decimals.expect("decimals must be provided");
let token_symbol = self.token_symbol.clone().expect("token symbol must be provided");

Expand All @@ -78,18 +175,23 @@ impl NewFaucetCmd {

let anchor_block = client.get_latest_epoch_block().await?;

let (new_account, seed) = AccountBuilder::new(init_seed)
let mut builder = AccountBuilder::new(init_seed)
.anchor((&anchor_block).try_into().expect("anchor block should be valid"))
.account_type(AccountType::FungibleFaucet)
.storage_mode(self.storage_mode.into())
.with_component(RpoFalcon512Component::new(key_pair.public_key()))
.with_component(
BasicFungibleFaucetComponent::new(symbol, decimals, max_supply).map_err(|err| {
CliError::Account(err, "Failed to create a faucet".to_string())
})?,
)
.with_component(RpoFalcon512::new(key_pair.public_key()))
.with_component(BasicFungibleFaucet::new(symbol, decimals, max_supply).map_err(
|err| CliError::Account(err, "failed to create a faucet component".to_string()),
)?);

//add any extra component templates
for component in process_component_templates(&extra_components)? {
builder = builder.with_component(component);
}

let (new_account, seed) = builder
.build()
.map_err(|err| CliError::Account(err, "Failed to create a faucet".to_string()))?;
.map_err(|err| CliError::Account(err, "error building account".into()))?;

client
.add_account(&new_account, Some(seed), &AuthSecretKey::RpoFalcon512(key_pair), false)
Expand All @@ -105,6 +207,9 @@ impl NewFaucetCmd {
}
}

// NEW WALLET
// ================================================================================================

#[derive(Debug, Parser, Clone)]
/// Create a new wallet account.
pub struct NewWalletCmd {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same nit as above re putting comments above attributes.

Copy link
Contributor

Choose a reason for hiding this comment

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

I actually meant docs/attributes on lines 213 - 214 (not for the internal fields). Re-arranging them for the fields is fine too - but let's make sure the descriptions still show up in the CLI correctly.

Expand All @@ -114,10 +219,25 @@ pub struct NewWalletCmd {
#[clap(short, long)]
/// Defines if the account code is mutable (by default it isn't mutable).
pub mutable: bool,
#[clap(short, long)]
/// Optional list of additional component package files to include.
pub template_files: Vec<PathBuf>,
bobbinth marked this conversation as resolved.
Show resolved Hide resolved
}

impl NewWalletCmd {
pub async fn execute(&self, mut client: Client<impl FeltRng>) -> Result<(), CliError> {
let mut extra_components = Vec::new();
for path in &self.template_files {
let bytes = fs::read(path)?;
let template = AccountComponentTemplate::read_from_bytes(&bytes).map_err(|e| {
CliError::AccountComponentError(
Box::new(e),
"failed to read account component template".into(),
)
})?;
extra_components.push(template);
}

let key_pair = SecretKey::with_rng(client.rng());

let mut init_seed = [0u8; 32];
Expand All @@ -131,14 +251,21 @@ impl NewWalletCmd {

let anchor_block = client.get_latest_epoch_block().await?;

let (new_account, seed) = AccountBuilder::new(init_seed)
let mut builder = AccountBuilder::new(init_seed)
.anchor((&anchor_block).try_into().expect("anchor block should be valid"))
.account_type(account_type)
.storage_mode(self.storage_mode.into())
.with_component(RpoFalcon512Component::new(key_pair.public_key()))
.with_component(BasicWalletComponent)
.with_component(RpoFalcon512::new(key_pair.public_key()))
.with_component(BasicWallet);

//add any extra component templates
for component in process_component_templates(&extra_components)? {
builder = builder.with_component(component);
}

let (new_account, seed) = builder
.build()
.map_err(|err| CliError::Account(err, "Failed to create a wallet".to_string()))?;
.map_err(|err| CliError::Account(err, "failed to create a wallet".to_string()))?;

client
.add_account(&new_account, Some(seed), &AuthSecretKey::RpoFalcon512(key_pair), false)
Expand Down
11 changes: 8 additions & 3 deletions bin/miden-cli/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,16 @@ use miden_objects::{AccountError, AccountIdError, AssetError};
use miette::Diagnostic;
use thiserror::Error;

type SourceError = Box<dyn StdError + Send + Sync>;

#[derive(Debug, Diagnostic, Error)]
pub enum CliError {
#[error("account error: {1}")]
#[diagnostic(code(cli::account_error))]
Account(#[source] AccountError, String),
#[error("account component error: {1}")]
#[diagnostic(code(cli::account_error))]
AccountComponentError(#[source] SourceError, String),
#[error("account id error: {1}")]
#[diagnostic(code(cli::accountid_error), help("Check the account ID format."))]
AccountId(#[source] AccountIdError, String),
Expand All @@ -24,7 +29,7 @@ pub enum CliError {
code(cli::config_error),
help("Check if the configuration file exists and is well-formed.")
)]
Config(#[source] Box<dyn StdError + Send + Sync>, String),
Config(#[source] SourceError, String),
#[error("export error: {0}")]
#[diagnostic(code(cli::export_error), help("Check the ID."))]
Export(String),
Expand All @@ -45,10 +50,10 @@ pub enum CliError {
MissingFlag(String),
#[error("parse error: {1}")]
#[diagnostic(code(cli::parse_error), help("Check the inputs."))]
Parse(#[source] Box<dyn StdError + Send + Sync>, String),
Parse(#[source] SourceError, String),
#[error("transaction error: {1}")]
#[diagnostic(code(cli::transaction_error))]
Transaction(#[source] Box<dyn StdError + Send + Sync>, String),
Transaction(#[source] SourceError, String),
}

impl From<CliError> for String {
Expand Down
1 change: 0 additions & 1 deletion bin/miden-cli/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@ impl Cli {

// Define whether we want to use the executor's debug mode based on the env var and
// the flag override

let in_debug_mode = match env::var("MIDEN_DEBUG") {
Ok(value) if value.to_lowercase() == "true" => true,
_ => self.debug,
Expand Down
8 changes: 4 additions & 4 deletions bin/miden-cli/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ use config::RpcConfig;
use miden_client::{
self,
account::{
AccountBuilder, AccountId, AccountStorageMode, AccountType, BasicWalletComponent,
RpoFalcon512Component,
component::{BasicWallet, RpoFalcon512},
AccountBuilder, AccountId, AccountStorageMode, AccountType,
},
auth::AuthSecretKey,
crypto::{FeltRng, RpoRandomCoin, SecretKey},
Expand Down Expand Up @@ -124,8 +124,8 @@ async fn test_mint_with_untracked_account() {
.anchor((&anchor_block).try_into().unwrap())
.account_type(AccountType::RegularAccountImmutableCode)
.storage_mode(AccountStorageMode::Private)
.with_component(RpoFalcon512Component::new(key_pair.public_key()))
.with_component(BasicWalletComponent)
.with_component(RpoFalcon512::new(key_pair.public_key()))
.with_component(BasicWallet)
.build()
.unwrap();

Expand Down
Loading
Loading