Skip to content

Commit

Permalink
refactor: split input output note register structs (#199)
Browse files Browse the repository at this point in the history
* split input notes

* make enum for notes tables

* rename recipients variable and add more NoteTable uses

* run fmt

* change to_string for to_hex

* correct nullable fields and reorder fields

* remove unnecesarry note filter

* remove unnecessary commit_height field

* move related rows into metadata row for notes

* remove dbg statements

* switch to_string for to_hex

* Revert "move related rows into metadata row for notes"

This reverts commit bf46c0a.

* restore filtering by pending notes

* store grouped json fields

* add json fields with serialization/deserialization

I still have pending polishing the changes (abstracting some deserialization and removing debugs, etc.) and we should also add some kind of validation as well as having the proper NULL / NOT NULL columns (now everything is nullable)

* remove dbg calls and set nullable/non-nullable cols accordingly

* apply clippy suggestion

* add helper functions to work with json values returned as strings

* add documentation for handling json with sqlite

* fix incorrect function use to fetch output notes

* ignore doc tests

* add json validity constraints

* use structs to represent json columns

* remove dbg statements

* use propper serialization

* use existing NoteMetadata instead of NoteRecordMetadata

* also remove NoteRecordInclusionProof and use NoteInclusionProof instead

* add implementations for output notes

* change input note record struct

TODO: fix tests

* Fix test failures

There were two different kind of errors:

* When we queried for less fields than what parse_input_notes_columns expected (this happened on the function to get peding input note nullifiers)
* When we serialized the note status. Apparently serde serializes the note as `\"Variant\"` instead of `variant`.

* update doc comments

* cleanup struct

* remove dbg statements

* fix serialization for output notes

* change from to tryfrom for notestatus

* move note record structs to its own module

* remove todo comment

* remove quotes from note status
  • Loading branch information
mFragaBA authored Mar 15, 2024
1 parent a15be6a commit f05a3f8
Show file tree
Hide file tree
Showing 16 changed files with 720 additions and 254 deletions.
96 changes: 52 additions & 44 deletions src/cli/input_notes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,10 @@ use miden_client::{
client::rpc::NodeRpcClient,
store::{InputNoteRecord, NoteFilter as ClientNoteFilter, Store},
};
use miden_objects::{notes::NoteId, Digest};
use miden_objects::{
notes::{NoteId, NoteInputs, NoteScript},
Digest,
};
use miden_tx::utils::{Deserializable, Serializable};

use super::{Client, Parser};
Expand Down Expand Up @@ -119,7 +122,7 @@ fn list_input_notes<N: NodeRpcClient, S: Store>(
filter: ClientNoteFilter,
) -> Result<(), String> {
let notes = client.get_input_notes(filter)?;
print_notes_summary(&notes);
print_notes_summary(&notes)?;
Ok(())
}

Expand Down Expand Up @@ -164,7 +167,7 @@ pub fn import_note<N: NodeRpcClient, S: Store>(
let input_note_record =
InputNoteRecord::read_from_bytes(&contents).map_err(|err| err.to_string())?;

let note_id = input_note_record.note().id();
let note_id = input_note_record.id();
client.import_input_note(input_note_record)?;

Ok(note_id)
Expand All @@ -183,7 +186,7 @@ fn show_input_note<N: NodeRpcClient, S: Store>(
get_note_with_id_prefix(&client, &note_id).map_err(|err| err.to_string())?;

// print note summary
print_notes_summary(core::iter::once(&input_note_record));
print_notes_summary(core::iter::once(&input_note_record))?;

let mut table = Table::new();
table
Expand All @@ -192,14 +195,17 @@ fn show_input_note<N: NodeRpcClient, S: Store>(

// print note script
if show_script {
let script = NoteScript::read_from_bytes(input_note_record.details().script())
.map_err(|err| format!("Failed to parse the note record's program AST: {}", err))?;

table
.add_row(vec![
Cell::new("Note Script hash").add_attribute(Attribute::Bold),
Cell::new(input_note_record.note().script().hash()),
Cell::new(script.hash()),
])
.add_row(vec![
Cell::new("Note Script code").add_attribute(Attribute::Bold),
Cell::new(input_note_record.note().script().code()),
Cell::new(script.code()),
]);
};

Expand All @@ -208,32 +214,29 @@ fn show_input_note<N: NodeRpcClient, S: Store>(
table
.add_row(vec![
Cell::new("Note Vault hash").add_attribute(Attribute::Bold),
Cell::new(input_note_record.note().assets().commitment()),
Cell::new(input_note_record.assets().commitment()),
])
.add_row(vec![Cell::new("Note Vault").add_attribute(Attribute::Bold)]);

input_note_record.note().assets().iter().for_each(|asset| {
input_note_record.assets().iter().for_each(|asset| {
table.add_row(vec![Cell::new(format!("{:?}", asset))]);
})
};

if show_inputs {
let inputs = NoteInputs::read_from_bytes(input_note_record.details().inputs())
.map_err(|err| format!("Failed to parse the note record's inputs: {}", err))?;

table
.add_row(vec![
Cell::new("Note Inputs hash").add_attribute(Attribute::Bold),
Cell::new(input_note_record.note().inputs().commitment()),
Cell::new(inputs.commitment()),
])
.add_row(vec![Cell::new("Note Inputs").add_attribute(Attribute::Bold)]);
input_note_record
.note()
.inputs()
.values()
.iter()
.enumerate()
.for_each(|(idx, input)| {
table
.add_row(vec![Cell::new(idx).add_attribute(Attribute::Bold), Cell::new(input)]);
});

inputs.values().iter().enumerate().for_each(|(idx, input)| {
table.add_row(vec![Cell::new(idx).add_attribute(Attribute::Bold), Cell::new(input)]);
});
};

println!("{table}");
Expand All @@ -242,7 +245,7 @@ fn show_input_note<N: NodeRpcClient, S: Store>(

// HELPERS
// ================================================================================================
fn print_notes_summary<'a, I>(notes: I)
fn print_notes_summary<'a, I>(notes: I) -> Result<(), String>
where
I: IntoIterator<Item = &'a InputNoteRecord>,
{
Expand All @@ -255,22 +258,31 @@ where
"Commit Height",
]);

notes.into_iter().for_each(|input_note_record| {
for input_note_record in notes {
let commit_height = input_note_record
.inclusion_proof()
.map(|proof| proof.origin().block_num.to_string())
.unwrap_or("-".to_string());

let script = NoteScript::read_from_bytes(input_note_record.details().script())
.map_err(|err| format!("Failed to parse the note record's program AST: {}", err))?;

let inputs = NoteInputs::read_from_bytes(input_note_record.details().inputs())
.map_err(|err| format!("Failed to parse the note record's inputs: {}", err))?;

table.add_row(vec![
input_note_record.note().id().inner().to_string(),
input_note_record.note().script().hash().to_string(),
input_note_record.note().assets().commitment().to_string(),
input_note_record.note().inputs().commitment().to_string(),
Digest::new(input_note_record.note().serial_num()).to_string(),
input_note_record.id().inner().to_string(),
script.hash().to_string(),
input_note_record.assets().commitment().to_string(),
inputs.commitment().to_string(),
Digest::new(input_note_record.details().serial_num()).to_string(),
commit_height,
]);
});
}

println!("{table}");

Ok(())
}

// TESTS
Expand Down Expand Up @@ -319,7 +331,7 @@ mod tests {
let (_, commited_notes, _, _) = mock_full_chain_mmr_and_notes(consumed_notes);

let committed_note: InputNoteRecord = commited_notes.first().unwrap().clone().into();
let pending_note = InputNoteRecord::new(created_notes.first().unwrap().clone(), None);
let pending_note = InputNoteRecord::from(created_notes.first().unwrap().clone());

client.import_input_note(committed_note.clone()).unwrap();
client.import_input_note(pending_note.clone()).unwrap();
Expand All @@ -332,18 +344,14 @@ mod tests {
let mut filename_path_pending = temp_dir();
filename_path_pending.push("test_import_pending");

export_note(
&client,
&committed_note.note_id().inner().to_string(),
Some(filename_path.clone()),
)
.unwrap();
export_note(&client, &committed_note.id().inner().to_string(), Some(filename_path.clone()))
.unwrap();

assert!(filename_path.exists());

export_note(
&client,
&pending_note.note_id().inner().to_string(),
&pending_note.id().inner().to_string(),
Some(filename_path_pending.clone()),
)
.unwrap();
Expand All @@ -368,14 +376,14 @@ mod tests {

import_note(&mut client, filename_path).unwrap();
let imported_note_record: InputNoteRecord =
client.get_input_note(committed_note.note().id()).unwrap();
client.get_input_note(committed_note.id()).unwrap();

assert_eq!(committed_note.note().id(), imported_note_record.note().id());
assert_eq!(committed_note.id(), imported_note_record.id());

import_note(&mut client, filename_path_pending).unwrap();
let imported_pending_note_record = client.get_input_note(pending_note.note().id()).unwrap();
let imported_pending_note_record = client.get_input_note(pending_note.id()).unwrap();

assert_eq!(imported_pending_note_record.note().id(), pending_note.note().id());
assert_eq!(imported_pending_note_record.id(), pending_note.id());
}

#[tokio::test]
Expand Down Expand Up @@ -410,19 +418,19 @@ mod tests {
let (_, notes, _, _) = mock_full_chain_mmr_and_notes(consumed_notes);

let committed_note: InputNoteRecord = notes.first().unwrap().clone().into();
let pending_note = InputNoteRecord::new(created_notes.first().unwrap().clone(), None);
let pending_note = InputNoteRecord::from(created_notes.first().unwrap().clone());

client.import_input_note(committed_note.clone()).unwrap();
client.import_input_note(pending_note.clone()).unwrap();
assert!(pending_note.inclusion_proof().is_none());
assert!(committed_note.inclusion_proof().is_some());

// Check that we can fetch Both notes
let note = get_note_with_id_prefix(&client, &committed_note.note_id().to_hex()).unwrap();
assert_eq!(note.note_id(), committed_note.note_id());
let note = get_note_with_id_prefix(&client, &committed_note.id().to_hex()).unwrap();
assert_eq!(note.id(), committed_note.id());

let note = get_note_with_id_prefix(&client, &pending_note.note_id().to_hex()).unwrap();
assert_eq!(note.note_id(), pending_note.note_id());
let note = get_note_with_id_prefix(&client, &pending_note.id().to_hex()).unwrap();
assert_eq!(note.id(), pending_note.id());

// Check that we get an error if many match
let note_id_with_many_matches = "0x";
Expand Down
4 changes: 2 additions & 2 deletions src/cli/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ pub(crate) fn get_note_with_id_prefix<N: NodeRpcClient, S: Store>(
NoteIdPrefixFetchError::NoMatch(note_id_prefix.to_string())
})?
.into_iter()
.filter(|note_record| note_record.note_id().to_hex().starts_with(note_id_prefix))
.filter(|note_record| note_record.id().to_hex().starts_with(note_id_prefix))
.collect::<Vec<_>>();

if input_note_records.is_empty() {
Expand All @@ -160,7 +160,7 @@ pub(crate) fn get_note_with_id_prefix<N: NodeRpcClient, S: Store>(
if input_note_records.len() > 1 {
let input_note_record_ids = input_note_records
.iter()
.map(|input_note_record| input_note_record.note_id())
.map(|input_note_record| input_note_record.id())
.collect::<Vec<_>>();
tracing::error!(
"Multiple notes found for the prefix {}: {:?}",
Expand Down
2 changes: 1 addition & 1 deletion src/cli/transactions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ fn build_transaction_template<N: NodeRpcClient, S: Store>(
.iter()
.map(|note_id| {
get_note_with_id_prefix(client, note_id)
.map(|note_record| note_record.note_id())
.map(|note_record| note_record.id())
.map_err(|err| err.to_string())
})
.collect::<Result<Vec<NoteId>, _>>()?;
Expand Down
4 changes: 2 additions & 2 deletions src/client/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -206,14 +206,14 @@ impl<N: NodeRpcClient, S: Store> Client<N, S> {
.store
.get_input_notes(NoteFilter::Pending)?
.iter()
.map(|n| n.note().id())
.map(|n| n.id())
.collect();

let pending_output_notes: Vec<NoteId> = self
.store
.get_output_notes(NoteFilter::Pending)?
.iter()
.map(|n| n.note().id())
.map(|n| n.id())
.collect();

let mut pending_notes = [pending_input_notes, pending_output_notes].concat();
Expand Down
2 changes: 1 addition & 1 deletion src/store/data_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ impl<S: Store> DataStore for ClientDataStore<S> {
.store
.get_input_notes(NoteFilter::Committed)?
.iter()
.map(|note_record| note_record.note_id())
.map(|note_record| note_record.id())
.collect::<Vec<_>>();

for note_id in notes {
Expand Down
Loading

0 comments on commit f05a3f8

Please sign in to comment.