Skip to content

Commit

Permalink
feat: enable consumption of unauthenticated notes (#417)
Browse files Browse the repository at this point in the history
* derive Debug for `CommittedNote`

* feat: enable consumption of notes without proof

* fix: fetch both expected and processing notes for commitment check

now that a not can go to `Processing` status either from `Expected` or
`Committed` it can happen that a note that got committed was actually in
Processing state instead of from Expected only.

* fix: reorder db updates

we reorder the db updates so we first set the committed notes and
afterwards we set the consumed notes. This way, if during a single sync
state a note gets both committed and consumed we can update all the data
while ending up in the `NoteStatus::Consumed` state.

* add auto generated files after rebase

* address review comments

* feat: insert unauthenticated input note instead of assuming it exists in the store

* fix: fix compilation error from integration tests

* feat: add basic integration test

* fix: mentioning expected note consumption in docs

* fix typo

* feat: add validation so authenticated notes can't be consumed without a proof

* update CHANGELOG

* fix: fix wasm compilation adding missing maybe_await!

* tests: fix

* chore: Lints

* Alphabetize errors

* refactor: add methods to retrieve the authenticated and unauthenticated input notes from the transaction request

* refactor: rename error variant

* test: Fix original test

* chore: Lints

* Merge resolution

* Fix comments

---------

Co-authored-by: tomyrd <[email protected]>
Co-authored-by: Ignacio Amigo <[email protected]>
  • Loading branch information
3 people authored Jul 5, 2024
1 parent be5af12 commit 3441e19
Show file tree
Hide file tree
Showing 13 changed files with 268 additions and 79 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# Changelog

* [BREAKING] Added unauthenticated notes to `TransactionRequest` and necessary changes to consume unauthenticated notes with the client (#417).
* Added advice map to `TransactionRequest` and updated integration test with example using the advice map to provide more than a single `Word` as `NoteArgs` for a note (#422).
* Restructured the client crate module organization (#417).
* Moved CLI tests to the `miden-cli` crate (#413).
Expand Down
20 changes: 16 additions & 4 deletions crates/rust-client/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ pub use miden_tx::{
DataStoreError, TransactionExecutorError, TransactionProverError,
};

use crate::transactions::transaction_request::TransactionRequestError;

// CLIENT ERROR
// ================================================================================================

Expand All @@ -18,6 +20,7 @@ pub enum ClientError {
AccountError(AccountError),
AssetError(AssetError),
DataDeserializationError(DeserializationError),
ExistenceVerificationError(NoteId),
HexParseError(HexParseError),
ImportNewAccountWithoutSeed,
MissingOutputNotes(Vec<NoteId>),
Expand All @@ -30,17 +33,20 @@ pub enum ClientError {
StoreError(StoreError),
TransactionExecutorError(TransactionExecutorError),
TransactionProvingError(TransactionProverError),
ExistenceVerificationError(NoteId),
TransactionRequestError(TransactionRequestError),
}

impl fmt::Display for ClientError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
ClientError::AccountError(err) => write!(f, "account error: {err}"),
ClientError::AssetError(err) => write!(f, "asset error: {err}"),
ClientError::DataDeserializationError(err) => {
write!(f, "data deserialization error: {err}")
},
ClientError::AssetError(err) => write!(f, "asset error: {err}"),
ClientError::ExistenceVerificationError(note_id) => {
write!(f, "The note with ID {note_id} doesn't exist in the chain")
},
ClientError::HexParseError(err) => write!(f, "error turning array to Digest: {err}"),
ClientError::ImportNewAccountWithoutSeed => write!(
f,
Expand Down Expand Up @@ -68,8 +74,8 @@ impl fmt::Display for ClientError {
ClientError::TransactionProvingError(err) => {
write!(f, "transaction prover error: {err}")
},
ClientError::ExistenceVerificationError(note_id) => {
write!(f, "The note with ID {note_id} doesn't exist in the chain")
ClientError::TransactionRequestError(err) => {
write!(f, "transaction request error: {err}")
},
}
}
Expand Down Expand Up @@ -132,6 +138,12 @@ impl From<NoteScreenerError> for ClientError {
}
}

impl From<TransactionRequestError> for ClientError {
fn from(err: TransactionRequestError) -> Self {
Self::TransactionRequestError(err)
}
}

#[cfg(feature = "sqlite")]
impl From<rusqlite::Error> for ClientError {
fn from(err: rusqlite::Error) -> Self {
Expand Down
1 change: 1 addition & 0 deletions crates/rust-client/src/rpc/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@ pub struct NullifierUpdate {
// ================================================================================================

/// Represents a committed note, returned as part of a `SyncStateResponse`
#[derive(Debug)]
pub struct CommittedNote {
/// Note ID of the committed note
note_id: NoteId,
Expand Down
28 changes: 8 additions & 20 deletions crates/rust-client/src/store/data_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,17 +54,8 @@ 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::Expected { .. } => {
return Err(DataStoreError::InternalError(format!(
"The input note ID {} does not contain a note origin.",
note_id.to_hex()
)));
},
NoteStatus::Consumed { .. } => {
return Err(DataStoreError::NoteAlreadyConsumed(*note_id));
},
_ => {},
if let NoteStatus::Consumed { .. } = note_record.status() {
return Err(DataStoreError::NoteAlreadyConsumed(*note_id));
}
}

Expand All @@ -85,16 +76,13 @@ impl<S: Store> DataStore for ClientDataStore<S> {

list_of_notes.push(input_note.clone());

let note_block_num = input_note
.proof()
.ok_or(DataStoreError::InternalError(
"Input note doesn't have inclusion proof".to_string(),
))?
.origin()
.block_num;
// Include block if note is authenticated
if let Some(inclusion_proof) = input_note.proof() {
let note_block_num = inclusion_proof.origin().block_num;

if note_block_num != block_num {
notes_blocks.push(note_block_num);
if note_block_num != block_num {
notes_blocks.push(note_block_num);
}
}
}

Expand Down
22 changes: 18 additions & 4 deletions crates/rust-client/src/store/note_record/input_note_record.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,17 @@ impl InputNoteRecord {
pub fn imported_tag(&self) -> Option<NoteTag> {
self.imported_tag
}

/// Returns whether the note record contains a valid inclusion proof correlated with its
/// status
pub fn is_authenticated(&self) -> bool {
match self.status {
NoteStatus::Expected { .. } => false,
NoteStatus::Committed { .. }
| NoteStatus::Processing { .. }
| NoteStatus::Consumed { .. } => self.inclusion_proof.is_some(),
}
}
}

impl From<&NoteDetails> for InputNoteRecord {
Expand Down Expand Up @@ -240,10 +251,13 @@ impl TryInto<InputNote> for InputNoteRecord {
let note = Note::new(self.assets, metadata, note_recipient);
Ok(InputNote::authenticated(note, proof.clone()))
},

(None, _) => Err(ClientError::NoteRecordError(
"Input Note Record contains no inclusion proof".to_string(),
)),
(None, Some(metadata)) => {
let note_inputs = NoteInputs::new(self.details.inputs)?;
let note_recipient =
NoteRecipient::new(self.details.serial_num, self.details.script, note_inputs);
let note = Note::new(self.assets, metadata, note_recipient);
Ok(InputNote::unauthenticated(note))
},
(_, None) => Err(ClientError::NoteRecordError(
"Input Note Record contains no metadata".to_string(),
)),
Expand Down
48 changes: 24 additions & 24 deletions crates/rust-client/src/store/sqlite_store/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,30 +97,6 @@ impl SqliteStore {
const BLOCK_NUMBER_QUERY: &str = "UPDATE state_sync SET block_num = ?";
tx.execute(BLOCK_NUMBER_QUERY, params![block_header.block_num()])?;

// Update spent notes
for nullifier_update in nullifiers.iter() {
const SPENT_INPUT_NOTE_QUERY: &str =
"UPDATE input_notes SET status = ?, nullifier_height = ? WHERE json_extract(details, '$.nullifier') = ?";
let nullifier = nullifier_update.nullifier.to_hex();
let block_num = nullifier_update.block_num;
tx.execute(
SPENT_INPUT_NOTE_QUERY,
params![NOTE_STATUS_CONSUMED.to_string(), block_num, nullifier],
)?;

const SPENT_OUTPUT_NOTE_QUERY: &str =
"UPDATE output_notes SET status = ?, nullifier_height = ? WHERE json_extract(details, '$.nullifier') = ?";
tx.execute(
SPENT_OUTPUT_NOTE_QUERY,
params![NOTE_STATUS_CONSUMED.to_string(), block_num, nullifier],
)?;
}

Self::insert_block_header_tx(&tx, block_header, new_mmr_peaks, block_has_relevant_notes)?;

// Insert new authentication nodes (inner nodes of the PartialMmr)
Self::insert_chain_mmr_nodes_tx(&tx, &new_authentication_nodes)?;

// Update tracked output notes
for (note_id, inclusion_proof) in committed_notes.updated_output_notes().iter() {
let block_num = inclusion_proof.origin().block_num;
Expand Down Expand Up @@ -182,6 +158,30 @@ impl SqliteStore {
insert_input_note_tx(&tx, note.clone().into())?;
}

// Update spent notes
for nullifier_update in nullifiers.iter() {
const SPENT_INPUT_NOTE_QUERY: &str =
"UPDATE input_notes SET status = ?, nullifier_height = ? WHERE json_extract(details, '$.nullifier') = ?";
let nullifier = nullifier_update.nullifier.to_hex();
let block_num = nullifier_update.block_num;
tx.execute(
SPENT_INPUT_NOTE_QUERY,
params![NOTE_STATUS_CONSUMED.to_string(), block_num, nullifier],
)?;

const SPENT_OUTPUT_NOTE_QUERY: &str =
"UPDATE output_notes SET status = ?, nullifier_height = ? WHERE json_extract(details, '$.nullifier') = ?";
tx.execute(
SPENT_OUTPUT_NOTE_QUERY,
params![NOTE_STATUS_CONSUMED.to_string(), block_num, nullifier],
)?;
}

Self::insert_block_header_tx(&tx, block_header, new_mmr_peaks, block_has_relevant_notes)?;

// Insert new authentication nodes (inner nodes of the PartialMmr)
Self::insert_chain_mmr_nodes_tx(&tx, &new_authentication_nodes)?;

// Mark transactions as committed
Self::mark_transactions_as_committed(&tx, &committed_transactions)?;

Expand Down
16 changes: 14 additions & 2 deletions crates/rust-client/src/sync/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -399,18 +399,30 @@ impl<N: NodeRpcClient, R: FeltRng, S: Store, A: TransactionAuthenticator> Client
let mut tracked_input_notes = vec![];
let mut tracked_output_notes_proofs = vec![];

let expected_input_notes: BTreeMap<NoteId, InputNoteRecord> =
let mut expected_input_notes: BTreeMap<NoteId, InputNoteRecord> =
maybe_await!(self.store.get_input_notes(NoteFilter::Expected))?
.into_iter()
.map(|n| (n.id(), n))
.collect();

let expected_output_notes: BTreeSet<NoteId> =
expected_input_notes.extend(
maybe_await!(self.store.get_input_notes(NoteFilter::Processing))?
.into_iter()
.map(|input_note| (input_note.id(), input_note)),
);

let mut expected_output_notes: BTreeSet<NoteId> =
maybe_await!(self.store.get_output_notes(NoteFilter::Expected))?
.into_iter()
.map(|n| n.id())
.collect();

expected_output_notes.extend(
maybe_await!(self.store.get_output_notes(NoteFilter::Processing))?
.into_iter()
.map(|output_note| output_note.id()),
);

for committed_note in committed_notes {
if let Some(note_record) = expected_input_notes.get(committed_note.note_id()) {
// The note belongs to our locally tracked set of expected notes, build the inclusion proof
Expand Down
43 changes: 36 additions & 7 deletions crates/rust-client/src/transactions/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use miden_objects::{
};
use miden_tx::{auth::TransactionAuthenticator, ProvingOptions, TransactionProver};
use tracing::info;
use transaction_request::TransactionRequestError;
use winter_maybe_async::{maybe_async, maybe_await};

use self::transaction_request::{
Expand All @@ -20,7 +21,7 @@ use super::{rpc::NodeRpcClient, Client, FeltRng};
use crate::{
errors::ClientError,
notes::NoteScreener,
store::{InputNoteRecord, Store, TransactionFilter},
store::{InputNoteRecord, NoteFilter, Store, TransactionFilter},
};

pub mod transaction_request;
Expand All @@ -40,7 +41,7 @@ pub use transaction_request::known_script_roots;
/// `output_notes` that the client has to store as input notes, based on the NoteScreener
/// output from filtering the transaction's output notes or some partial note we expect to receive
/// in the future (you can check at swap notes for an example of this).
#[derive(Clone)]
#[derive(Clone, Debug)]
pub struct TransactionResult {
transaction: ExecutedTransaction,
relevant_notes: Vec<InputNoteRecord>,
Expand Down Expand Up @@ -200,12 +201,13 @@ impl<N: NodeRpcClient, R: FeltRng, S: Store, A: TransactionAuthenticator> Client
let tx_script = self.tx_executor.compile_tx_script(program_ast, vec![], vec![])?;
Ok(TransactionRequest::new(
account_id,
vec![],
notes,
vec![],
vec![],
Some(tx_script),
None,
))
)?)
},
TransactionTemplate::MintFungibleAsset(asset, target_account_id, note_type) => {
self.build_mint_tx_request(asset, target_account_id, note_type)
Expand Down Expand Up @@ -239,6 +241,30 @@ impl<N: NodeRpcClient, R: FeltRng, S: Store, A: TransactionAuthenticator> Client
maybe_await!(self.tx_executor.load_account(account_id))
.map_err(ClientError::TransactionExecutorError)?;

// Ensure authenticated notes have their inclusion proofs (a.k.a they're in a committed
// state). TODO: we should consider refactoring this in a way we can handle this in
// `get_transaction_inputs`
let authenticated_input_note_ids: Vec<NoteId> =
transaction_request.authenticated_input_note_ids().collect::<Vec<_>>();

let authenticated_note_records = maybe_await!(self
.store
.get_input_notes(NoteFilter::List(&authenticated_input_note_ids)))?;

for authenticated_note_record in authenticated_note_records {
if !authenticated_note_record.is_authenticated() {
return Err(ClientError::TransactionRequestError(
TransactionRequestError::InputNoteNotAuthenticated,
));
}
}

// If tx request contains unauthenticated_input_notes we should insert them
for unauthenticated_input_note in transaction_request.unauthenticated_input_notes() {
// TODO: run this as a single TX
maybe_await!(self.store.insert_input_note(unauthenticated_input_note.clone().into()))?;
}

let block_num = maybe_await!(self.store.get_sync_height())?;

let note_ids = transaction_request.get_input_note_ids();
Expand All @@ -254,7 +280,7 @@ impl<N: NodeRpcClient, R: FeltRng, S: Store, A: TransactionAuthenticator> Client
))?;

// Check that the expected output notes matches the transaction outcome.
// We comprare authentication hashes where possible since that involves note IDs + metadata
// We compare authentication hashes where possible since that involves note IDs + metadata
// (as opposed to just note ID which remains the same regardless of metadata)
// We also do the check for partial output notes
let tx_note_auth_hashes: BTreeSet<Digest> =
Expand Down Expand Up @@ -379,12 +405,13 @@ impl<N: NodeRpcClient, R: FeltRng, S: Store, A: TransactionAuthenticator> Client

Ok(TransactionRequest::new(
payment_data.account_id(),
vec![],
BTreeMap::new(),
vec![created_note],
vec![],
Some(tx_script),
None,
))
)?)
}

/// Helper to build a [TransactionRequest] for Swap-type transactions easily.
Expand Down Expand Up @@ -430,12 +457,13 @@ impl<N: NodeRpcClient, R: FeltRng, S: Store, A: TransactionAuthenticator> Client

Ok(TransactionRequest::new(
swap_data.account_id(),
vec![],
BTreeMap::new(),
vec![created_note],
vec![payback_note_details],
Some(tx_script),
None,
))
)?)
}

/// Helper to build a [TransactionRequest] for transaction to mint fungible tokens.
Expand Down Expand Up @@ -480,12 +508,13 @@ impl<N: NodeRpcClient, R: FeltRng, S: Store, A: TransactionAuthenticator> Client

Ok(TransactionRequest::new(
asset.faucet_id(),
vec![],
BTreeMap::new(),
vec![created_note],
vec![],
Some(tx_script),
None,
))
)?)
}
}

Expand Down
Loading

0 comments on commit 3441e19

Please sign in to comment.