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: Track more information about a note's status and consumability #355

Merged
merged 39 commits into from
Jun 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
f889530
Add Consuming note status
tomyrd May 16, 2024
ff55be1
Infer Consuming status from store information
tomyrd May 16, 2024
bd17b34
Add some useful comments
tomyrd May 20, 2024
aa3ed6b
Add change to CHANGELOG
tomyrd May 20, 2024
19015a7
Add PR to CHANGELOG
tomyrd May 20, 2024
6973f80
Remove consuming notes from list consumable
tomyrd May 20, 2024
9a3dc69
Remove hardcoded note status names from sqlite store code
tomyrd May 20, 2024
68534e0
Remove `ConsumableNote` type
tomyrd May 29, 2024
d064cbc
Add function to get the consumability of a note
tomyrd May 29, 2024
a2add0f
Refactor `get_consumable_notes`
tomyrd May 29, 2024
7f3396d
Merge branch 'next' into tomyrd-new-note-status
tomyrd May 29, 2024
d2f32ad
Rename `Consuming` to `Processing`
tomyrd May 29, 2024
f39cfed
Fix clippy
tomyrd May 29, 2024
3b98420
Fix doc comments
tomyrd May 29, 2024
d6db780
Store `Processing` status in `SqliteStore`
tomyrd May 31, 2024
f8accf8
Override `get_unspent_input_note_nullifiers` function from `Store`
tomyrd May 31, 2024
ff640e7
Add note filter for `Processing` notes
tomyrd May 31, 2024
e2c4659
Fix comment
tomyrd May 31, 2024
6a27be3
Merge branch 'next' into tomyrd-new-note-status
tomyrd May 31, 2024
5388b3e
Remove `Processing` status from sql table
tomyrd Jun 4, 2024
a474689
Add additional info to `NoteStatus` enum
tomyrd Jun 5, 2024
d34ae2e
Refactor store/cli to use the new `NoteStatus`
tomyrd Jun 5, 2024
6b4834d
Remove `consumer_account_id` from note record
tomyrd Jun 5, 2024
20bea45
Add additional information to string
tomyrd Jun 5, 2024
e627c48
Add new fields to database
tomyrd Jun 5, 2024
92fabd1
Use local timezone for timestamp
tomyrd Jun 7, 2024
99332b8
Fix status display messages
tomyrd Jun 7, 2024
0481715
Merge branch 'next' into tomyrd-new-note-status
tomyrd Jun 7, 2024
8f304f8
Fix submitted typo
tomyrd Jun 10, 2024
5c0c6b0
Fix tests
tomyrd Jun 10, 2024
8be45ea
Improve doc comments for `NoteStatus`
tomyrd Jun 10, 2024
3ae7af8
Remove incorrect `unwrap_or`
tomyrd Jun 10, 2024
eff8862
Add `Pending` status to `SqliteStore`
tomyrd Jun 10, 2024
9b77e27
Merge branch 'next' into tomyrd-new-note-status
tomyrd Jun 14, 2024
098502c
Fix merge
tomyrd Jun 14, 2024
03085f4
Merge branch 'next' into tomyrd-new-note-status
tomyrd Jun 14, 2024
ef51a08
Merge branch 'next' into tomyrd-new-note-status
tomyrd Jun 18, 2024
f07079e
Fix sqlite_store errors
tomyrd Jun 18, 2024
a80bcc5
Address suggestions
tomyrd Jun 19, 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

* 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);
mFragaBA marked this conversation as resolved.
Show resolved Hide resolved

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);
}

mFragaBA marked this conversation as resolved.
Show resolved Hide resolved
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 @@ -169,7 +167,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 @@ -744,6 +744,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