feat(CLI): Generalize new account command #728

igamigo · 2025-02-12T13:12:49Z

Closes #695

This can be tested with something like this:

miden init

miden new-account --account-type fungible-faucet -c basic-fungible-faucet -i init_data.toml

Succesfully created new account. 
To view account details execute `miden account -s 0x8318a196a267aea0000006b51e173a`

Where init_data.toml is:

token_metadata.decimals=10
token_metadata.max_supply=10000000
token_metadata.ticker="TST"

If no TOML is provided, it looks like this:

miden new-account --account-type fungible-faucet -c fungible_faucet

Enter value for 'token_metadata.decimals' - Number of decimal places (type: u8): 3
Enter value for 'token_metadata.max_supply' - Maximum supply of the token in base units (type: felt): 3
Enter value for 'token_metadata.ticker' - Token symbol of the faucet's asset, limited to 4 characters. (type: tokensymbol): 3
Error: cli::account_error

  × account error: error instantiating component from template
  ├─▶ failed to create account component
  ├─▶ error converting value into expected type:
  ╰─▶ failed to parse input `3` as `tokensymbol`

TODO:

Refactor and deduplicate file name constants (and decide on a naming scheme).
Address TODOs in these changes (removing unwraps)
Fix integration tests using the InitStorageData::from_toml function
(probably on another PR) Decide how to further generalize this. For example, this still assumes the basic auth component always goes into the first slot. This in turn interacts with things such as import/export functionality, where we assume the public key is in a specific place. With the work related to detecting account features in miden-base, we could do all of this better.

tomyrd

Did some tests with the CLI and it seems to work correctly 👍 We should update the integration CLI tests with the new new-account command

bin/miden-cli/build.rs

tomyrd · 2025-02-13T12:48:41Z

bin/miden-cli/build.rs

+    // TODO: Do we want to add this?
+    output_filename.push(".mct");


I think there's value in having different extensions. Just so we can differentiate templates from account files and exported notes

tomyrd · 2025-02-13T12:55:57Z

bin/miden-cli/src/commands/new_account.rs

-            std::io::stdin().read_line(&mut input_value)?;
-            let input_value = input_value.trim();
+#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, ValueEnum)]
+pub enum CliAccountType {


We should add a small description mentioning that this is a redefinition of the AccountType struct.

tomyrd · 2025-02-13T14:05:48Z

bin/miden-cli/src/commands/new_account.rs

-            AccountType::RegularAccountImmutableCode
-        };
-
+        // Use the provided account type.


I think this comment is a little out of place here, we could add it in line 247

tomyrd · 2025-02-13T14:07:39Z

bin/miden-cli/templates/faucet.toml

@@ -0,0 +1,15 @@
+name = "fungible_faucet"
+description = "This is a test component"


Given that this template will be used in the CLI I would change this description.

bobbinth · 2025-02-15T23:12:44Z

bin/miden-cli/src/commands/new_account.rs

+const FAUCET_TEMPLATE_BYTES: &[u8] =
+    include_bytes!(concat!(env!("OUT_DIR"), "/templates/faucet.mct"));


This should probably be a bit more descriptive (both the constant name and the file name) - e.g., basic_fungible_faucet.mct.

bobbinth · 2025-02-15T23:14:25Z

bin/miden-cli/src/commands/new_account.rs

+/// 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(


nit: I would probably move this function to the end of the file into the "HELPER FUNCTIONS" section.

bobbinth · 2025-02-15T23:19:51Z

bin/miden-cli/src/commands/new_account.rs

+            print!(
+                "Enter value for placeholder '{placeholder_key}' - {description} (type: {}): ",
+                placeholder_type.r#type.to_string()
+            );


How does this print out? Maybe it should look more like:

Enter value for 'token_metadata.max_supply' - maximum supply of the token in base units (type: felt):

tomyrd

Code looks good and I did some local tests which worked! The integration tests seem to be failing because of the CLI tests which should be updated.

SantiagoPittella · 2025-02-21T20:47:57Z

bin/miden-cli/src/commands/init.rs

+        CliError::Config(
+            "failed to create account component templates directory".to_string().into(),
+            err.to_string(),
+        )


Should this be like:

Suggested change

CliError::Config(

"failed to create account component templates directory".to_string().into(),

err.to_string(),

)

CliError::Config(

err.into()

"failed to create account component templates directory".to_string()

)

bin/miden-cli/src/commands/new_account.rs

Co-authored-by: Santiago Pittella <87827390+SantiagoPittella@users.noreply.github.com>

…onMiden/miden-client into igamigo-general-new-acct

igamigo changed the title ~~feat: Generalize new account~~ feat(CLI): Generalize new account command Feb 12, 2025

igamigo force-pushed the igamigo-general-new-acct branch from dcbe31c to ee9baf4 Compare February 12, 2025 15:42

igamigo marked this pull request as ready for review February 12, 2025 18:37

igamigo mentioned this pull request Feb 12, 2025

Generalize NewWallet and NewFaucet #695

Open

tomyrd reviewed Feb 13, 2025

View reviewed changes

igamigo changed the base branch from next to igamigo-next-base February 13, 2025 20:00

bobbinth reviewed Feb 15, 2025

View reviewed changes

Base automatically changed from igamigo-next-base to next February 17, 2025 17:14

igamigo force-pushed the igamigo-general-new-acct branch 2 times, most recently from 140d424 to 380cace Compare February 20, 2025 21:43

feat: Generalize new account, implement InitStorageData changes

4a60d04

igamigo force-pushed the igamigo-general-new-acct branch from 380cace to 4a60d04 Compare February 20, 2025 21:46

tomyrd reviewed Feb 21, 2025

View reviewed changes

SantiagoPittella reviewed Feb 21, 2025

View reviewed changes

igamigo and others added 9 commits February 25, 2025 15:44

reviews: Doc comment

23cf528

Co-authored-by: Santiago Pittella <87827390+SantiagoPittella@users.noreply.github.com>

docs: Update docs

cadccd2

Merge branch 'igamigo-general-new-acct' of https://github.com/0xPolyg…

a1f4a76

…onMiden/miden-client into igamigo-general-new-acct

chore: remove unwrap

f3d6dce

chore: Merge next

19af238

chore: Bring new net

44ec58e

fix: Integration test

c279ea8

chore: Update upstream miden-base rev

0451a71

fix: Clippy

7944edc

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(CLI): Generalize new account command #728

feat(CLI): Generalize new account command #728

igamigo commented Feb 12, 2025 •

edited

Loading

tomyrd left a comment

tomyrd Feb 13, 2025

tomyrd Feb 13, 2025

tomyrd Feb 13, 2025

tomyrd Feb 13, 2025

bobbinth Feb 15, 2025

bobbinth Feb 15, 2025

bobbinth Feb 15, 2025

tomyrd left a comment

SantiagoPittella Feb 21, 2025

		// TODO: Do we want to add this?
		output_filename.push(".mct");

		@@ -0,0 +1,15 @@
		name = "fungible_faucet"
		description = "This is a test component"

		const FAUCET_TEMPLATE_BYTES: &[u8] =
		include_bytes!(concat!(env!("OUT_DIR"), "/templates/faucet.mct"));

feat(CLI): Generalize new account command #728

Are you sure you want to change the base?

feat(CLI): Generalize new account command #728

Conversation

igamigo commented Feb 12, 2025 • edited Loading

tomyrd left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tomyrd left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

igamigo commented Feb 12, 2025 •

edited

Loading