Skip to content

Commit

Permalink
feat: Track more information about a note's status and consumability (#…
Browse files Browse the repository at this point in the history
…355)

* Add Consuming note status

* Infer Consuming status from store information

* Add some useful comments

* Add change to CHANGELOG

* Add PR to CHANGELOG

* Remove consuming notes from list consumable

* Remove hardcoded note status names from sqlite store code

* Remove `ConsumableNote` type

* Add function to get the consumability of a note

* Refactor `get_consumable_notes`

* Rename `Consuming` to `Processing`

* Fix clippy

* Fix doc comments

* Store `Processing` status in `SqliteStore`

* Override `get_unspent_input_note_nullifiers` function from `Store`

* Add note filter for `Processing` notes

* Fix comment

* Remove `Processing` status from sql table

* Add additional info to `NoteStatus` enum

* Refactor store/cli to use the new `NoteStatus`

* Remove `consumer_account_id` from note record

* Add additional information to string

* Add new fields to database

* Use local timezone for timestamp

* Fix status display messages

* Fix submitted typo

* Fix tests

* Improve doc comments for `NoteStatus`

* Remove incorrect `unwrap_or`

* Add `Pending` status to `SqliteStore`

* Fix merge

* Fix sqlite_store errors

* Address suggestions
  • Loading branch information
tomyrd authored Jun 19, 2024
1 parent 36bd0db commit bc163fc
Show file tree
Hide file tree
Showing 20 changed files with 340 additions and 192 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# Changelog

* New note status added to reflect more possible states (#355).
* [BREAKING] Library API reorganization (#367).
* Added `wasm` and `async` feature to make the code compatible with WASM-32 target (#378).
* Changed `cargo-make` usage for `make` and `Makefile.toml` for a regular `Makefile` (#359).
Expand Down
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ testing = ["miden-objects/testing", "miden-lib/testing"]
tonic = ["dep:tonic", "dep:miden-node-proto"]

[dependencies]
chrono = { version = "0.4", optional = false }
clap = { version = "4.3", features = ["derive"], optional = true }
comfy-table = { version = "7.1.0", optional = true }
figment = { version = "0.10", features = ["toml", "env"], optional = true }
Expand Down
2 changes: 1 addition & 1 deletion src/cli/new_transactions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ impl ConsumeNotesCmd {
info!("No input note IDs provided, getting all notes consumable by {}", account_id);
let consumable_notes = client.get_consumable_notes(Some(account_id))?;

list_of_notes.extend(consumable_notes.iter().map(|n| n.note.id()));
list_of_notes.extend(consumable_notes.iter().map(|(note, _)| note.id()));
}

if list_of_notes.is_empty() {
Expand Down
52 changes: 10 additions & 42 deletions src/cli/notes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@ use comfy_table::{presets, Attribute, Cell, ContentArrangement, Table};
use miden_client::{
errors::{ClientError, IdPrefixFetchError},
rpc::NodeRpcClient,
store::{InputNoteRecord, NoteFilter as ClientNoteFilter, NoteStatus, OutputNoteRecord, Store},
store::{InputNoteRecord, NoteFilter as ClientNoteFilter, OutputNoteRecord, Store},
transactions::transaction_request::known_script_roots::{P2ID, P2IDR, SWAP},
Client, ConsumableNote,
Client, NoteConsumability,
};
use miden_objects::{
accounts::AccountId,
Expand All @@ -27,6 +27,7 @@ pub enum NoteFilter {
Pending,
Committed,
Consumed,
Processing,
Consumable,
}

Expand All @@ -39,6 +40,7 @@ impl TryInto<ClientNoteFilter<'_>> for NoteFilter {
NoteFilter::Pending => Ok(ClientNoteFilter::Pending),
NoteFilter::Committed => Ok(ClientNoteFilter::Committed),
NoteFilter::Consumed => Ok(ClientNoteFilter::Consumed),
NoteFilter::Processing => Ok(ClientNoteFilter::Processing),
NoteFilter::Consumable => Err("Consumable filter is not supported".to_string()),
}
}
Expand Down Expand Up @@ -310,14 +312,14 @@ where

fn print_consumable_notes_summary<'a, I>(notes: I) -> Result<(), String>
where
I: IntoIterator<Item = &'a ConsumableNote>,
I: IntoIterator<Item = &'a (InputNoteRecord, Vec<NoteConsumability>)>,
{
let mut table = create_dynamic_table(&["Note ID", "Account ID", "Relevance"]);

for consumable_note in notes {
for relevance in &consumable_note.relevances {
for (note, relevances) in notes {
for relevance in relevances {
table.add_row(vec![
consumable_note.note.id().to_hex(),
note.id().to_hex(),
relevance.0.to_string(),
relevance.1.to_string(),
]);
Expand Down Expand Up @@ -351,21 +353,6 @@ fn note_summary(
.or(output_note_record.map(|record| record.id()))
.expect("One of the two records should be Some");

let commit_height = input_note_record
.map(|record| {
record
.inclusion_proof()
.map(|proof| proof.origin().block_num.to_string())
.unwrap_or("-".to_string())
})
.or(output_note_record.map(|record| {
record
.inclusion_proof()
.map(|proof| proof.origin().block_num.to_string())
.unwrap_or("-".to_string())
}))
.expect("One of the two records should be Some");

let assets_hash_str = input_note_record
.map(|record| record.assets().commitment().to_string())
.or(output_note_record.map(|record| record.assets().commitment().to_string()))
Expand Down Expand Up @@ -408,27 +395,8 @@ fn note_summary(
let status = input_note_record
.map(|record| record.status())
.or(output_note_record.map(|record| record.status()))
.expect("One of the two records should be Some");

let note_consumer = input_note_record
.map(|record| record.consumer_account_id())
.or(output_note_record.map(|record| record.consumer_account_id()))
.expect("One of the two records should be Some");

let status = match status {
NoteStatus::Committed => {
status.to_string() + format!(" (height {})", commit_height).as_str()
},
NoteStatus::Consumed => {
status.to_string()
+ format!(
" (by {})",
note_consumer.map(|id| id.to_string()).unwrap_or("?".to_string())
)
.as_str()
},
_ => status.to_string(),
};
.expect("One of the two records should be Some")
.to_string();

let note_metadata = input_note_record
.map(|record| record.metadata())
Expand Down
3 changes: 1 addition & 2 deletions src/client/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,7 @@ mod notes;
pub mod store_authenticator;
pub mod sync;
pub mod transactions;
pub use note_screener::{NoteRelevance, NoteScreener};
pub use notes::ConsumableNote;
pub use note_screener::{NoteConsumability, NoteRelevance, NoteScreener};

// MIDEN CLIENT
// ================================================================================================
Expand Down
15 changes: 10 additions & 5 deletions src/client/note_screener.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@ pub enum NoteRelevance {
After(u32),
}

/// Represents the consumability of a note by a specific account.
///
/// The tuple contains the account ID that may consume the note and the moment it will become relevant.
pub type NoteConsumability = (AccountId, NoteRelevance);

impl fmt::Display for NoteRelevance {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
Expand Down Expand Up @@ -46,7 +51,7 @@ impl<S: Store> NoteScreener<S> {
pub fn check_relevance(
&self,
note: &Note,
) -> Result<Vec<(AccountId, NoteRelevance)>, NoteScreenerError> {
) -> Result<Vec<NoteConsumability>, NoteScreenerError> {
let account_ids = BTreeSet::from_iter(maybe_await!(self.store.get_account_ids())?);

let script_hash = note.script().hash().to_string();
Expand All @@ -63,7 +68,7 @@ impl<S: Store> NoteScreener<S> {
fn check_p2id_relevance(
note: &Note,
account_ids: &BTreeSet<AccountId>,
) -> Result<Vec<(AccountId, NoteRelevance)>, NoteScreenerError> {
) -> Result<Vec<NoteConsumability>, NoteScreenerError> {
let mut note_inputs_iter = note.inputs().values().iter();
let account_id_felt = note_inputs_iter
.next()
Expand All @@ -85,7 +90,7 @@ impl<S: Store> NoteScreener<S> {
fn check_p2idr_relevance(
note: &Note,
account_ids: &BTreeSet<AccountId>,
) -> Result<Vec<(AccountId, NoteRelevance)>, NoteScreenerError> {
) -> Result<Vec<NoteConsumability>, NoteScreenerError> {
let mut note_inputs_iter = note.inputs().values().iter();
let account_id_felt = note_inputs_iter
.next()
Expand Down Expand Up @@ -129,7 +134,7 @@ impl<S: Store> NoteScreener<S> {
&self,
note: &Note,
account_ids: &BTreeSet<AccountId>,
) -> Result<Vec<(AccountId, NoteRelevance)>, NoteScreenerError> {
) -> Result<Vec<NoteConsumability>, NoteScreenerError> {
let note_inputs = note.inputs().values().to_vec();
if note_inputs.len() != 9 {
return Ok(Vec::new());
Expand Down Expand Up @@ -177,7 +182,7 @@ impl<S: Store> NoteScreener<S> {
&self,
_note: &Note,
account_ids: &BTreeSet<AccountId>,
) -> Result<Vec<(AccountId, NoteRelevance)>, NoteScreenerError> {
) -> Result<Vec<NoteConsumability>, NoteScreenerError> {
// TODO: try to execute the note script against relevant accounts; this will
// require querying data from the store
Ok(account_ids
Expand Down
43 changes: 20 additions & 23 deletions src/client/notes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,23 +8,13 @@ use miden_tx::{auth::TransactionAuthenticator, ScriptTarget};
use tracing::info;
use winter_maybe_async::{maybe_async, maybe_await};

use super::{note_screener::NoteRelevance, rpc::NodeRpcClient, Client};
use super::{note_screener::NoteConsumability, rpc::NodeRpcClient, Client};
use crate::{
client::NoteScreener,
errors::ClientError,
store::{InputNoteRecord, NoteFilter, OutputNoteRecord, Store},
};

// TYPES
// --------------------------------------------------------------------------------------------
/// Contains information about a note that can be consumed
pub struct ConsumableNote {
/// The consumable note
pub note: InputNoteRecord,
/// Stores which accounts can consume the note and it's relevance
pub relevances: Vec<(AccountId, NoteRelevance)>,
}

impl<N: NodeRpcClient, R: FeltRng, S: Store, A: TransactionAuthenticator> Client<N, R, S, A> {
// INPUT NOTE DATA RETRIEVAL
// --------------------------------------------------------------------------------------------
Expand All @@ -38,40 +28,48 @@ impl<N: NodeRpcClient, R: FeltRng, S: Store, A: TransactionAuthenticator> Client
maybe_await!(self.store.get_input_notes(filter)).map_err(|err| err.into())
}

/// Returns input notes that are able to be consumed by the account_id.
/// Returns the input notes and their consumability.
///
/// If account_id is None then all consumable input notes are returned.
#[maybe_async]
pub fn get_consumable_notes(
&self,
account_id: Option<AccountId>,
) -> Result<Vec<ConsumableNote>, ClientError> {
) -> Result<Vec<(InputNoteRecord, Vec<NoteConsumability>)>, ClientError> {
let commited_notes = maybe_await!(self.store.get_input_notes(NoteFilter::Committed))?;

let note_screener = NoteScreener::new(self.store.clone());

let mut relevant_notes = Vec::new();
for input_note in commited_notes {
let account_relevance =
let mut account_relevance =
maybe_await!(note_screener.check_relevance(&input_note.clone().try_into()?))?;

if let Some(account_id) = account_id {
account_relevance.retain(|(id, _)| *id == account_id);
}

if account_relevance.is_empty() {
continue;
}

relevant_notes.push(ConsumableNote {
note: input_note,
relevances: account_relevance,
});
}

if let Some(account_id) = account_id {
relevant_notes.retain(|note| note.relevances.iter().any(|(id, _)| *id == account_id));
relevant_notes.push((input_note, account_relevance));
}

Ok(relevant_notes)
}

/// Returns the consumability of the provided note.
#[maybe_async]
pub fn get_note_consumability(
&self,
note: InputNoteRecord,
) -> Result<Vec<NoteConsumability>, ClientError> {
let note_screener = NoteScreener::new(self.store.clone());
maybe_await!(note_screener.check_relevance(&note.clone().try_into()?))
.map_err(|err| err.into())
}

/// Returns the input note with the specified hash.
#[maybe_async]
pub fn get_input_note(&self, note_id: NoteId) -> Result<InputNoteRecord, ClientError> {
Expand Down Expand Up @@ -174,7 +172,6 @@ impl<N: NodeRpcClient, R: FeltRng, S: Store, A: TransactionAuthenticator> Client
note.metadata().copied(),
inclusion_proof,
note.details().clone(),
None,
);

maybe_await!(self.store.insert_input_note(&note)).map_err(|err| err.into())
Expand Down
4 changes: 4 additions & 0 deletions src/client/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -794,6 +794,10 @@ fn get_transactions_to_commit(
// account be included in a single block. If that happens, we'll need to rewrite
// this check.

// TODO: If all input notes were consumed but the account hashes don't match, it means that
// another transaction consumed them and that the current transaction failed. We should
// handle this case in the future and rollback the store accordingly.

t.input_note_nullifiers.iter().all(|n| nullifiers.contains(n))
&& t.output_notes.iter().all(|n| note_ids.contains(&n.id()))
&& account_hash_updates.iter().any(|(account_id, account_hash)| {
Expand Down
2 changes: 1 addition & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ extern crate alloc;
mod client;
pub use client::{
accounts::AccountTemplate, rpc, store_authenticator::StoreAuthenticator, sync::SyncSummary,
transactions, Client, ConsumableNote, NoteRelevance,
transactions, Client, NoteConsumability, NoteRelevance,
};

pub mod config;
Expand Down
4 changes: 2 additions & 2 deletions src/store/data_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,13 @@ impl<S: Store> DataStore for ClientDataStore<S> {
let note_record = input_note_records.get(note_id).expect("should have key");

match note_record.status() {
NoteStatus::Pending => {
NoteStatus::Pending { .. } => {
return Err(DataStoreError::InternalError(format!(
"The input note ID {} does not contain a note origin.",
note_id.to_hex()
)));
},
NoteStatus::Consumed => {
NoteStatus::Consumed { .. } => {
return Err(DataStoreError::NoteAlreadyConsumed(*note_id));
},
_ => {},
Expand Down
3 changes: 3 additions & 0 deletions src/store/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ pub trait Store {
fn get_unspent_input_note_nullifiers(&self) -> Result<Vec<Nullifier>, StoreError> {
let nullifiers = maybe_await!(self.get_input_notes(NoteFilter::Committed))?
.iter()
.chain(maybe_await!(self.get_input_notes(NoteFilter::Processing))?.iter())
.map(|input_note| Ok(Nullifier::from(Digest::try_from(input_note.nullifier())?)))
.collect::<Result<Vec<_>, _>>();

Expand Down Expand Up @@ -303,6 +304,8 @@ pub enum NoteFilter<'a> {
/// Return a list of pending notes ([InputNoteRecord] or [OutputNoteRecord]). These represent notes for which the store
/// does not have anchor data.
Pending,
/// Return a list of notes that are currently being processed.
Processing,
/// Return a list containing the note that matches with the provided [NoteId].
List(&'a [NoteId]),
/// Return a list containing the note that matches with the provided [NoteId].
Expand Down
Loading

0 comments on commit bc163fc

Please sign in to comment.