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

refactor: move authentication outside the Client #718

Open
wants to merge 22 commits into
base: next
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
### Changes

* Add check for empty pay to id notes (#714).
* [BREAKING] Refactored authentication out of the `Client` and added new separate authenticators (#718).

## 0.7.0 (2025-01-28)

Expand Down
7 changes: 0 additions & 7 deletions Cargo.lock

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

2 changes: 1 addition & 1 deletion bin/miden-cli/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ This binary allows the user to interact with the Miden rollup via a simple comma

## Usage

Before you can use the Miden client, you'll need to make sure you have both [Rust](https://www.rust-lang.org/tools/install) and sqlite3 installed. Miden client requires rust version **1.84** or higher.
Before you can use the Miden client, you'll need to make sure you have both [Rust](https://www.rust-lang.org/tools/install) and SQLite3 installed. Miden client requires rust version **1.84** or higher.

### Running `miden-client`'s CLI

Expand Down
44 changes: 34 additions & 10 deletions bin/miden-cli/src/commands/export.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
use std::{fs::File, io::Write, path::PathBuf};

use miden_client::{
account::AccountData, crypto::FeltRng, store::NoteExportType, utils::Serializable, Client,
account::{Account, AccountData},
authenticator::keystore::{FilesystemKeyStore, KeyStore},
crypto::FeltRng,
store::NoteExportType,
utils::Serializable,
Client, Word,
};
use tracing::info;

Expand Down Expand Up @@ -49,9 +54,13 @@ impl From<&ExportType> for NoteExportType {
}

impl ExportCmd {
pub async fn execute(&self, mut client: Client<impl FeltRng>) -> Result<(), CliError> {
pub async fn execute(
&self,
mut client: Client<impl FeltRng>,
keystore: FilesystemKeyStore,
) -> Result<(), CliError> {
if self.account {
export_account(&client, self.id.as_str(), self.filename.clone()).await?;
export_account(&client, &keystore, self.id.as_str(), self.filename.clone()).await?;
} else if let Some(export_type) = &self.export_type {
export_note(&mut client, self.id.as_str(), self.filename.clone(), export_type).await?;
} else {
Expand All @@ -66,8 +75,9 @@ impl ExportCmd {
// EXPORT ACCOUNT
// ================================================================================================

async fn export_account<R: FeltRng>(
client: &Client<R>,
async fn export_account(
client: &Client<impl FeltRng>,
keystore: &FilesystemKeyStore,
account_id: &str,
filename: Option<PathBuf>,
) -> Result<File, CliError> {
Expand All @@ -79,12 +89,14 @@ async fn export_account<R: FeltRng>(
.ok_or(CliError::Export(format!("Account with ID {account_id} not found")))?;
let account_seed = account.seed().copied();

let auth = client
.get_account_auth(account_id)
.await?
.ok_or(CliError::Export(format!("Account with ID {account_id} not found")))?;
let account: Account = account.into();

let auth = keystore
.get_key(get_public_key_from_account(&account))
.map_err(CliError::KeyStore)?
.ok_or(CliError::Export("Auth not found for account".to_string()))?;

let account_data = AccountData::new(account.into(), account_seed, auth);
let account_data = AccountData::new(account, account_seed, auth);

let file_path = if let Some(filename) = filename {
filename
Expand Down Expand Up @@ -139,3 +151,15 @@ async fn export_note(
println!("Succesfully exported note {note_id}");
Ok(file)
}

/// Gets the public key from the storage of an account. This will only work if the account is
/// created by the CLI as it expects the public key to be stored in index 0 of the account storage
/// if it is a regular account, and in index 1 if it is a faucet account.
pub fn get_public_key_from_account(account: &Account) -> Word {
Word::from(
account
.storage()
.get_item(u8::from(account.is_faucet()))
.expect("Account should have the public key in storage"),
)
}
27 changes: 17 additions & 10 deletions bin/miden-cli/src/commands/import.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use std::{

use miden_client::{
account::{AccountData, AccountId},
authenticator::keystore::{FilesystemKeyStore, KeyStore},
crypto::FeltRng,
note::NoteFile,
utils::Deserializable,
Expand All @@ -29,7 +30,11 @@ pub struct ImportCmd {
}

impl ImportCmd {
pub async fn execute(&self, mut client: Client<impl FeltRng>) -> Result<(), CliError> {
pub async fn execute(
&self,
mut client: Client<impl FeltRng>,
keystore: FilesystemKeyStore,
) -> Result<(), CliError> {
validate_paths(&self.filenames)?;
let (mut current_config, _) = load_config_file()?;
for filename in &self.filenames {
Expand All @@ -45,9 +50,13 @@ impl ImportCmd {
);
let account_data_file_contents = fs::read(filename)?;

let account_id =
import_account(&mut client, &account_data_file_contents, self.overwrite)
.await?;
let account_id = import_account(
&mut client,
&keystore,
&account_data_file_contents,
self.overwrite,
)
.await?;

println!("Successfully imported account {account_id}");

Expand All @@ -65,20 +74,18 @@ impl ImportCmd {

async fn import_account(
client: &mut Client<impl FeltRng>,
keystore: &FilesystemKeyStore,
account_data_file_contents: &[u8],
overwrite: bool,
) -> Result<AccountId, CliError> {
let account_data = AccountData::read_from_bytes(account_data_file_contents)
.map_err(ClientError::DataDeserializationError)?;
let account_id = account_data.account.id();

keystore.add_key(&account_data.auth_secret_key).map_err(CliError::KeyStore)?;

client
.add_account(
&account_data.account,
account_data.account_seed,
&account_data.auth_secret_key,
overwrite,
)
.add_account(&account_data.account, account_data.account_seed, overwrite)
.await?;
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is fine for now but maybe we could add a TODO here to address potential cases where adding the secret key succeeds, but adding the account fails.

Alternatively, maybe in the future we want to have some sort of "keystore" command that adds/removes keys separately from accounts.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Created an issue for this


Ok(account_id)
Expand Down
27 changes: 19 additions & 8 deletions bin/miden-cli/src/commands/new_account.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use miden_client::{
},
asset::TokenSymbol,
auth::AuthSecretKey,
authenticator::keystore::{FilesystemKeyStore, KeyStore},
crypto::{FeltRng, SecretKey},
utils::Deserializable,
Client, Felt, Word,
Expand Down Expand Up @@ -135,7 +136,11 @@ pub struct NewFaucetCmd {
}

impl NewFaucetCmd {
pub async fn execute(&self, mut client: Client<impl FeltRng>) -> Result<(), CliError> {
pub async fn execute(
&self,
mut client: Client<impl FeltRng>,
keystore: FilesystemKeyStore,
) -> Result<(), CliError> {
if self.non_fungible {
todo!("Non-fungible faucets are not supported yet");
}
Expand Down Expand Up @@ -193,9 +198,10 @@ impl NewFaucetCmd {
.build()
.map_err(|err| CliError::Account(err, "error building account".into()))?;

client
.add_account(&new_account, Some(seed), &AuthSecretKey::RpoFalcon512(key_pair), false)
.await?;
keystore
.add_key(&AuthSecretKey::RpoFalcon512(key_pair))
.map_err(CliError::KeyStore)?;
client.add_account(&new_account, Some(seed), false).await?;

println!("Succesfully created new faucet.");
println!(
Expand Down Expand Up @@ -225,7 +231,11 @@ pub struct NewWalletCmd {
}

impl NewWalletCmd {
pub async fn execute(&self, mut client: Client<impl FeltRng>) -> Result<(), CliError> {
pub async fn execute(
&self,
mut client: Client<impl FeltRng>,
keystore: FilesystemKeyStore,
) -> Result<(), CliError> {
let mut extra_components = Vec::new();
for path in &self.extra_components {
let bytes = fs::read(path)?;
Expand Down Expand Up @@ -267,9 +277,10 @@ impl NewWalletCmd {
.build()
.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)
.await?;
keystore
.add_key(&AuthSecretKey::RpoFalcon512(key_pair))
.map_err(CliError::KeyStore)?;
client.add_account(&new_account, Some(seed), false).await?;

println!("Succesfully created new wallet.");
println!(
Expand Down
6 changes: 5 additions & 1 deletion bin/miden-cli/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,10 @@ const TOKEN_SYMBOL_MAP_FILEPATH: &str = "token_symbol_map.toml";
pub struct CliConfig {
/// Describes settings related to the RPC endpoint.
pub rpc: RpcConfig,
/// Path to the sqlite store file.
/// Path to the `SQLite` store file.
pub store_filepath: PathBuf,
/// Path to the directory that contains the secret key files.
pub secret_keys_directory: PathBuf,
/// Address of the Miden node to connect to.
pub default_account_id: Option<String>,
/// Path to the file containing the token symbol map.
Expand Down Expand Up @@ -49,13 +51,15 @@ impl Provider for CliConfig {
impl Default for CliConfig {
fn default() -> Self {
const STORE_FILENAME: &str = "store.sqlite3";
const KEYS_DIRECTORY: &str = "keys";

// Get current directory
let exec_dir = PathBuf::new();

Self {
rpc: RpcConfig::default(),
store_filepath: exec_dir.join(STORE_FILENAME),
secret_keys_directory: exec_dir.join(KEYS_DIRECTORY),
default_account_id: None,
token_symbol_map_filepath: Path::new(TOKEN_SYMBOL_MAP_FILEPATH).to_path_buf(),
remote_prover_endpoint: None,
Expand Down
5 changes: 4 additions & 1 deletion bin/miden-cli/src/errors.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::error::Error as StdError;

use miden_client::ClientError;
use miden_client::{authenticator::keystore::KeyStoreError, ClientError};
use miden_objects::{AccountError, AccountIdError, AssetError};
use miette::Diagnostic;
use thiserror::Error;
Expand Down Expand Up @@ -45,6 +45,9 @@ pub enum CliError {
#[error("io error")]
#[diagnostic(code(cli::io_error))]
IO(#[from] std::io::Error),
#[error("keystore error")]
#[diagnostic(code(cli::keystore_error))]
KeyStore(#[source] KeyStoreError),
#[error("missing flag: {0}")]
#[diagnostic(code(cli::config_error), help("Check the configuration file format."))]
MissingFlag(String),
Expand Down
18 changes: 9 additions & 9 deletions bin/miden-cli/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,10 @@ use comfy_table::{presets, Attribute, Cell, ContentArrangement, Table};
use errors::CliError;
use miden_client::{
account::AccountHeader,
authenticator::{keystore::FilesystemKeyStore, ClientAuthenticator},
crypto::{FeltRng, RpoRandomCoin},
rpc::TonicRpcClient,
store::{
sqlite_store::SqliteStore, NoteFilter as ClientNoteFilter, OutputNoteRecord, Store,
StoreAuthenticator,
},
store::{sqlite_store::SqliteStore, NoteFilter as ClientNoteFilter, OutputNoteRecord, Store},
Client, ClientError, Felt, IdPrefixFetchError,
};
use rand::Rng;
Expand Down Expand Up @@ -109,7 +107,9 @@ impl Cli {
let coin_seed: [u64; 4] = rng.gen();

let rng = RpoRandomCoin::new(coin_seed.map(Felt::new));
let authenticator = StoreAuthenticator::new_with_rng(store.clone() as Arc<dyn Store>, rng);
let keystore = FilesystemKeyStore::new(cli_config.secret_keys_directory.clone())
.map_err(CliError::KeyStore)?;
let authenticator = ClientAuthenticator::new(rng, keystore.clone());

let client = Client::new(
Box::new(TonicRpcClient::new(
Expand All @@ -125,16 +125,16 @@ impl Cli {
// Execute CLI command
match &self.action {
Command::Account(account) => account.execute(client).await,
Command::NewFaucet(new_faucet) => new_faucet.execute(client).await,
Command::NewWallet(new_wallet) => new_wallet.execute(client).await,
Command::Import(import) => import.execute(client).await,
Command::NewFaucet(new_faucet) => new_faucet.execute(client, keystore).await,
Command::NewWallet(new_wallet) => new_wallet.execute(client, keystore).await,
Command::Import(import) => import.execute(client, keystore).await,
Command::Init(_) => Ok(()),
Command::Info => info::print_client_info(&client, &cli_config).await,
Command::Notes(notes) => notes.execute(client).await,
Command::Sync(sync) => sync.execute(client).await,
Command::Tags(tags) => tags.execute(client).await,
Command::Transaction(transaction) => transaction.execute(client).await,
Command::Export(cmd) => cmd.execute(client).await,
Command::Export(cmd) => cmd.execute(client, keystore).await,
Command::Mint(mint) => mint.execute(client).await,
Command::Send(send) => send.execute(client).await,
Command::Swap(swap) => swap.execute(client).await,
Expand Down
Loading
Loading