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: import NoteFile in client #375

Merged
merged 42 commits into from
Jun 28, 2024
Merged
Show file tree
Hide file tree
Changes from 33 commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
42b197a
Update import in cli to use `NoteFile`
tomyrd Jun 4, 2024
13baaf5
Update CHANGELOG
tomyrd Jun 4, 2024
12b5c64
Update cli export to use `NoteFile`
tomyrd Jun 6, 2024
4341075
Fix build errors with file tag
tomyrd Jun 11, 2024
db7db62
Rename `import_input_note` to `import_note`
tomyrd Jun 11, 2024
9208610
Implement `From<InputNoteRecord>` for `NoteFile`
tomyrd Jun 11, 2024
657ea67
Change `import_note` parameter to `NoteFile`
tomyrd Jun 11, 2024
576749a
Fix integration tests
tomyrd Jun 11, 2024
9b422c3
Implement `metadata` for `NoteDetails`
tomyrd Jun 11, 2024
f2d7ac8
Add `get_notes_without_block_header` method to store
tomyrd Jun 12, 2024
8d2087e
Add `ignored` and `imported_tag` to `InputNoteRecord`
tomyrd Jun 12, 2024
acc869a
Update `import_note` with new functionality
tomyrd Jun 12, 2024
40d02a9
Fix integration tests
tomyrd Jun 12, 2024
b12ebc2
Update `ignored` field on note tag add
tomyrd Jun 12, 2024
33109cf
Update `export` to accept type of note file
tomyrd Jun 13, 2024
9372741
Update MMR data for committed notes on sync
tomyrd Jun 13, 2024
7849dca
Update ignored notes on sync
tomyrd Jun 13, 2024
0942374
Fix after rebase
tomyrd Jun 14, 2024
24af38f
Update tracked note when importing `NoteFile::NoteId`
tomyrd Jun 14, 2024
cc4d153
Remove `NoteFile` conversion from `InputNoteRecord`
tomyrd Jun 18, 2024
a4beec0
Check ephemeral tags when importing
tomyrd Jun 18, 2024
be7dd84
Fix rebase
tomyrd Jun 19, 2024
e61858c
Fix integration tests
tomyrd Jun 19, 2024
d823791
Remove tag from export cli command
tomyrd Jun 19, 2024
367e387
Remove `--no-verify` flag from import cli command
tomyrd Jun 19, 2024
c832de3
Remove `InputNoteRecord` conversion on cli export
tomyrd Jun 24, 2024
3bb0dbc
Update doc comments for `import_note`
tomyrd Jun 24, 2024
6f710a0
Change suggestions
tomyrd Jun 24, 2024
4fe28c2
Add `maybe_await`
tomyrd Jun 24, 2024
b6cddd5
Update note when importing `NoteId` even if private
tomyrd Jun 24, 2024
f2f95d1
Only update ignored notes if specified in `sync` command
tomyrd Jun 25, 2024
f34090c
Change suggestions
tomyrd Jun 25, 2024
51d2a8b
Update `cli-reference`
tomyrd Jun 25, 2024
1e9b811
Change suggestions
tomyrd Jun 26, 2024
1446b37
Add tests for ignored notes
tomyrd Jun 26, 2024
85016c0
Improve doc for `get_notes_without_block_header`
tomyrd Jun 27, 2024
8bfc30a
Merge branch 'next' into tomyrd-import-notefile
tomyrd Jun 27, 2024
cfa4e4d
Merge branch 'next' into tomyrd-import-notefile
tomyrd Jun 27, 2024
48eca28
Change suggestions
tomyrd Jun 28, 2024
487fa62
Fix authentication hash calls for note
tomyrd Jun 28, 2024
e44e5eb
Temporarily change `NODE_BRANCH` in Makefile
tomyrd Jun 28, 2024
7526bd6
force CI reset
igamigo Jun 28, 2024
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
@@ -1,5 +1,6 @@
# Changelog

* Note importing in client now uses the `NoteFile` type (#375).
* Receive executed transaction info (id, commit height, account_id) from sync state RPC endpoint (#387).
* Rename "pending" notes to "expected" notes (#373).
* New note status added to reflect more possible states (#355).
Expand Down
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ test: ## Run tests

.PHONY: integration-test
integration-test: ## Run integration tests
cargo nextest run --no-capture --release --test=integration --features $(FEATURES_INTEGRATION_TESTING) --no-default-features
cargo nextest run --release --test=integration --features $(FEATURES_INTEGRATION_TESTING) --no-default-features

.PHONY: integration-test-full
integration-test-full: ## Run the integration test binary with ignored tests included
Expand Down
28 changes: 22 additions & 6 deletions docs/cli-reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ Note that the debug flag overrides the `MIDEN_DEBUG` environment variable.

## Commands

### `init`
### `init`

Creates a configuration file for the client in the current directory.

Expand Down Expand Up @@ -133,6 +133,10 @@ miden notes --show 0x70b7ec

Sync the client with the latest state of the Miden network. Shows a brief summary at the end.

| Flags | Description | Short Flag |
|-------------------|-------------------------------------------------------------|------------|
|`--update-ignored` | Update ignored notes in sync | `-u` |

### `tags`

View and add tags.
Expand Down Expand Up @@ -233,12 +237,24 @@ Continue with proving and submission? Changes will be irreversible once the proo

This confirmation can be skipped in non-interactive environments by providing the `--force` flag (`miden send --force ...`):

### `import`
### Importing and exporting

Import entities managed by the client, such as accounts and notes. The type of entitie is inferred.
#### `export`

Export input note data to a binary file .

When importing notes the CLI verifies that they exist on chain. The user can add an optional flag `--no-verify` that skips this verification.
| Flag | Description | Aliases |
|--------------------------------|------------------------------------------------|---------|
| `--filename <FILENAME>` | Desired filename for the binary file. | `-l` |
| ` --export-type <EXPORT_TYPE>` | Exported note type. | `-a` |
mFragaBA marked this conversation as resolved.
Show resolved Hide resolved

### `export`
##### Export type

Export input note data to a binary file .
The user needs to specify how the note should be exported via the `--export-type` flag. The following options are available:
- `id`: Only the note ID is exported. When importing, the note should be already committed on chan and all of extra information will be fetched. If the note ID is already tracked, the note will be updated with the new information.
mFragaBA marked this conversation as resolved.
Show resolved Hide resolved
- `full`: The note is exported with all of its information (metadata and inclusion proof). When importing, the note is considered committed.
mFragaBA marked this conversation as resolved.
Show resolved Hide resolved
- `partial`: The note is exported with minimal information and may be imported even if the note is not committed on chain. When importing, the note will be considered to be "expected" and will be updated after a sync when the original note is committed.

#### `import`

Import entities managed by the client, such as accounts and notes. The type of entitie is inferred.
mFragaBA marked this conversation as resolved.
Show resolved Hide resolved
41 changes: 29 additions & 12 deletions src/cli/export.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,7 @@
use std::{fs::File, io::Write, path::PathBuf};

use miden_client::{
rpc::NodeRpcClient,
store::{InputNoteRecord, Store},
Client,
};
use miden_objects::crypto::rand::FeltRng;
use miden_client::{rpc::NodeRpcClient, store::Store, Client};
use miden_objects::{crypto::rand::FeltRng, notes::NoteFile};
use miden_tx::{auth::TransactionAuthenticator, utils::Serializable};
use tracing::info;

Expand All @@ -22,14 +18,25 @@ pub struct ExportCmd {
/// Desired filename for the binary file. Defaults to the note ID if not provided
#[clap(short, long)]
filename: Option<PathBuf>,

/// Exported note type
#[clap(short, long, value_enum)]
export_type: ExportType,
}

#[derive(clap::ValueEnum, Clone, Debug)]
pub enum ExportType {
Id,
Full,
Partial,
}

impl ExportCmd {
pub fn execute<N: NodeRpcClient, R: FeltRng, S: Store, A: TransactionAuthenticator>(
&self,
client: Client<N, R, S, A>,
) -> Result<(), String> {
export_note(&client, self.id.as_str(), self.filename.clone())?;
export_note(&client, self.id.as_str(), self.filename.clone(), self.export_type.clone())?;
Ok(())
}
}
Expand All @@ -41,6 +48,7 @@ pub fn export_note<N: NodeRpcClient, R: FeltRng, S: Store, A: TransactionAuthent
client: &Client<N, R, S, A>,
note_id: &str,
filename: Option<PathBuf>,
export_type: ExportType,
) -> Result<File, String> {
let note_id = get_output_note_with_id_prefix(client, note_id)
.map_err(|err| err.to_string())?
Expand All @@ -51,10 +59,19 @@ pub fn export_note<N: NodeRpcClient, R: FeltRng, S: Store, A: TransactionAuthent
.pop()
.expect("should have an output note");

// Convert output note into InputNoteRecord before exporting
let input_note: InputNoteRecord = output_note
.try_into()
.map_err(|_err| format!("Can't export note with ID {}", note_id.to_hex()))?;
let note_file = match export_type {
ExportType::Id => NoteFile::NoteId(output_note.id()),
ExportType::Full => match output_note.inclusion_proof() {
Some(inclusion_proof) => {
NoteFile::NoteWithProof(output_note.clone().try_into()?, inclusion_proof.clone())
},
None => return Err("Note does not have inclusion proofx".to_string()),
},
ExportType::Partial => NoteFile::NoteDetails(
output_note.clone().try_into()?,
Some(output_note.metadata().tag()),
),
};

let file_path = if let Some(filename) = filename {
filename
Expand All @@ -65,7 +82,7 @@ pub fn export_note<N: NodeRpcClient, R: FeltRng, S: Store, A: TransactionAuthent

info!("Writing file to {}", file_path.to_string_lossy());
let mut file = File::create(file_path).map_err(|err| err.to_string())?;
file.write_all(&input_note.to_bytes()).map_err(|err| err.to_string())?;
file.write_all(&note_file.to_bytes()).map_err(|err| err.to_string())?;

println!("Succesfully exported note {}", note_id);
Ok(file)
Expand Down
46 changes: 16 additions & 30 deletions src/cli/import.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,11 @@ use std::{
path::PathBuf,
};

use miden_client::{
rpc::NodeRpcClient,
store::{InputNoteRecord, Store},
Client,
};
use miden_client::{rpc::NodeRpcClient, store::Store, Client};
use miden_objects::{
accounts::{AccountData, AccountId},
crypto::rand::FeltRng,
notes::NoteId,
notes::{NoteFile, NoteId},
utils::Deserializable,
};
use miden_tx::auth::TransactionAuthenticator;
Expand All @@ -27,9 +23,6 @@ pub struct ImportCmd {
/// Paths to the files that contains the account/note data
#[arg()]
filenames: Vec<PathBuf>,
/// Skip verification of note's existence in the chain (Only when importing notes)
#[clap(short, long, default_value = "false")]
no_verify: bool,
}

impl ImportCmd {
Expand All @@ -40,17 +33,18 @@ impl ImportCmd {
validate_paths(&self.filenames)?;
let (mut current_config, _) = load_config_file()?;
for filename in &self.filenames {
let note_id = import_note(&mut client, filename.clone(), !self.no_verify).await;
if note_id.is_ok() {
println!("Succesfully imported note {}", note_id.unwrap().inner());
continue;
}
let account_id = import_account(&mut client, filename)
.map_err(|_| format!("Failed to parse file {}", filename.to_string_lossy()))?;
println!("Succesfully imported account {}", account_id);

if account_id.is_regular_account() {
maybe_set_default_account(&mut current_config, account_id)?;
let note_id = import_note(&mut client, filename.clone()).await;

if let Ok(note_id) = note_id {
println!("Succesfully imported note {}", note_id.inner());
} else {
let account_id = import_account(&mut client, filename)
.map_err(|_| format!("Failed to parse file {}", filename.to_string_lossy()))?;
println!("Succesfully imported account {}", account_id);

if account_id.is_regular_account() {
maybe_set_default_account(&mut current_config, account_id)?;
}
}
}
Ok(())
Expand Down Expand Up @@ -84,23 +78,15 @@ fn import_account<N: NodeRpcClient, R: FeltRng, S: Store, A: TransactionAuthenti
pub async fn import_note<N: NodeRpcClient, R: FeltRng, S: Store, A: TransactionAuthenticator>(
client: &mut Client<N, R, S, A>,
filename: PathBuf,
verify: bool,
) -> Result<NoteId, String> {
let mut contents = vec![];
let mut _file = File::open(filename)
.and_then(|mut f| f.read_to_end(&mut contents))
.map_err(|err| err.to_string());

let input_note_record =
InputNoteRecord::read_from_bytes(&contents).map_err(|err| err.to_string())?;

let note_id = input_note_record.id();
client
.import_input_note(input_note_record, verify)
.await
.map_err(|err| err.to_string())?;
let note_file = NoteFile::read_from_bytes(&contents).map_err(|err| err.to_string())?;

Ok(note_id)
client.import_note(note_file).await.map_err(|err| err.to_string())
}

// HELPERS
Expand Down
6 changes: 3 additions & 3 deletions src/cli/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ use miden_objects::{
};
use miden_tx::auth::TransactionAuthenticator;
use rand::Rng;
use sync::SyncCmd;
use tracing::info;
use transactions::TransactionCmd;

Expand Down Expand Up @@ -76,8 +77,7 @@ pub enum Command {
Export(ExportCmd),
Init(InitCmd),
Notes(NotesCmd),
/// Sync this client with the latest state of the Miden network.
Sync,
Sync(SyncCmd),
/// View a summary of the current client state
Info,
Tags(TagsCmd),
Expand Down Expand Up @@ -139,7 +139,7 @@ impl Cli {
Command::Init(_) => Ok(()),
Command::Info => info::print_client_info(&client, &cli_config),
Command::Notes(notes) => notes.execute(client).await,
Command::Sync => sync::sync_state(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),
Expand Down
38 changes: 27 additions & 11 deletions src/cli/sync.rs
Original file line number Diff line number Diff line change
@@ -1,16 +1,32 @@
use clap::Parser;
use miden_client::{rpc::NodeRpcClient, store::Store, Client};
use miden_objects::crypto::rand::FeltRng;
use miden_tx::auth::TransactionAuthenticator;

pub async fn sync_state<N: NodeRpcClient, R: FeltRng, S: Store, A: TransactionAuthenticator>(
mut client: Client<N, R, S, A>,
) -> Result<(), String> {
let new_details = client.sync_state().await?;
println!("State synced to block {}", new_details.block_num);
println!("New public notes: {}", new_details.new_notes);
println!("Tracked notes updated: {}", new_details.new_inclusion_proofs);
println!("Tracked notes consumed: {}", new_details.new_nullifiers);
println!("Tracked accounts updated: {}", new_details.updated_onchain_accounts);
println!("Commited transactions: {}", new_details.commited_transactions);
Ok(())
#[derive(Debug, Parser, Clone)]
#[clap(about = "Sync this client with the latest state of the Miden network.")]
pub struct SyncCmd {
/// If enabled, the ignored notes will also be updated by fetching them directly from the node.
#[clap(short, long)]
update_ignored: bool,
}
Comment on lines +6 to +12
Copy link
Collaborator

Choose a reason for hiding this comment

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

For @bobbinth: It was not entirely clear from the issue discussion (unless I missed something) whether we wanted something like this flag or not, but considering whether we are OK with the CLI being more opinionated I think it's fine.

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 don't think this flag was specifically mentioned in the issue but I felt like maybe updating ignored notes with each sync maybe defeated the purpose. I can remove it and always call client.update_ignored_notes for the sync command if we want.

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 being more opinionated on the CLI side is fine - so, no issues with keeping this param.


impl SyncCmd {
pub async fn execute<N: NodeRpcClient, R: FeltRng, S: Store, A: TransactionAuthenticator>(
&self,
mut client: Client<N, R, S, A>,
) -> Result<(), String> {
let mut new_details = client.sync_state().await?;
if self.update_ignored {
new_details.combine_with(&client.update_ignored_notes().await?);
}

println!("State synced to block {}", new_details.block_num);
println!("New public notes: {}", new_details.new_notes);
println!("Tracked notes updated: {}", new_details.new_inclusion_proofs);
println!("Tracked notes consumed: {}", new_details.new_nullifiers);
println!("Tracked accounts updated: {}", new_details.updated_onchain_accounts);
println!("Commited transactions: {}", new_details.commited_transactions);
Ok(())
}
}
Loading