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(store): GetNoteAuthenticationInfo endpoint #421

Merged
merged 5 commits into from
Aug 23, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Expand Up @@ -17,6 +17,7 @@
- Added `SyncNotes` endpoint (#424).
- Cache sql statements (#427).
- Added `execution_hint` field to `Notes` table (#441).
- Implemented `GetNoteInclusionProofs` endpoint for both miden-store and miden-rpc (#421).

### Fixes

Expand Down
31 changes: 20 additions & 11 deletions crates/block-producer/src/batch_builder/batch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,11 @@ use std::{
mem,
};

use miden_node_store::state::NoteAuthenticationInfo;
use miden_objects::{
accounts::{delta::AccountUpdateDetails, AccountId},
batches::BatchNoteTree,
crypto::{
hash::blake::{Blake3Digest, Blake3_256},
merkle::MerklePath,
},
crypto::hash::blake::{Blake3Digest, Blake3_256},
notes::{NoteHeader, NoteId, Nullifier},
transaction::{InputNoteCommitment, OutputNote, TransactionId},
AccountDeltaError, Digest, MAX_NOTES_PER_BATCH,
Expand Down Expand Up @@ -88,7 +86,7 @@ impl TransactionBatch {
#[instrument(target = "miden-block-producer", name = "new_batch", skip_all, err)]
pub fn new(
txs: Vec<ProvenTransaction>,
found_unauthenticated_notes: BTreeMap<NoteId, MerklePath>,
found_unauthenticated_notes: NoteAuthenticationInfo,
) -> Result<Self, BuildBatchError> {
let id = Self::compute_id(&txs);

Expand Down Expand Up @@ -141,10 +139,11 @@ impl TransactionBatch {
// If an unauthenticated note was found in the store, transform it to an
// authenticated one (i.e. erase additional note details
// except the nullifier)
found_unauthenticated_notes
.get(&input_note_header.id())
.map(|_path| InputNoteCommitment::from(input_note.nullifier()))
.unwrap_or_else(|| input_note.clone())
if found_unauthenticated_notes.contains_note(&input_note_header.id()) {
InputNoteCommitment::from(input_note.nullifier())
} else {
input_note.clone()
}
Mirko-von-Leipzig marked this conversation as resolved.
Show resolved Hide resolved
},
None => input_note.clone(),
};
Expand Down Expand Up @@ -287,6 +286,9 @@ impl OutputNoteTracker {

#[cfg(test)]
mod tests {
use miden_objects::notes::NoteInclusionProof;
use miden_processor::crypto::MerklePath;

use super::*;
use crate::test_utils::{
mock_proven_tx,
Expand Down Expand Up @@ -401,8 +403,15 @@ mod tests {
#[test]
fn test_convert_unauthenticated_note_to_authenticated() {
let txs = mock_proven_txs();
let found_unauthenticated_notes =
BTreeMap::from_iter([(mock_note(5).id(), Default::default())]);
let found_unauthenticated_notes = BTreeMap::from_iter([(
mock_note(5).id(),
NoteInclusionProof::new(0, 0, MerklePath::default()).unwrap(),
)]);
let found_unauthenticated_notes = NoteAuthenticationInfo {
note_proofs: found_unauthenticated_notes,
block_proofs: Default::default(),
chain_length: Default::default(),
};
let batch = TransactionBatch::new(txs, found_unauthenticated_notes).unwrap();

let expected_input_notes =
Expand Down
2 changes: 1 addition & 1 deletion crates/block-producer/src/batch_builder/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ where
};
let missing_notes: Vec<_> = dangling_notes
.into_iter()
.filter(|note_id| !stored_notes.contains_key(note_id))
.filter(|note_id| !stored_notes.contains_note(note_id))
.collect();

if !missing_notes.is_empty() {
Expand Down
8 changes: 8 additions & 0 deletions crates/block-producer/src/batch_builder/tests/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use std::iter;

use miden_objects::{crypto::merkle::Mmr, Digest};
use tokio::sync::RwLock;

use super::*;
Expand Down Expand Up @@ -260,12 +261,19 @@ async fn test_block_builder_no_missing_notes() {
async fn test_block_builder_fails_if_notes_are_missing() {
let accounts: Vec<_> = (1..=4).map(MockPrivateAccount::<3>::from).collect();
let notes: Vec<_> = (1..=6).map(mock_note).collect();
// We require mmr for the note authentication to succeed.
//
// We also need two blocks worth of mmr because the mock store skips genesis.
let mut mmr = Mmr::new();
mmr.add(Digest::new([1u32.into(), 2u32.into(), 3u32.into(), 4u32.into()]));
mmr.add(Digest::new([1u32.into(), 2u32.into(), 3u32.into(), 4u32.into()]));
Mirko-von-Leipzig marked this conversation as resolved.
Show resolved Hide resolved

let store = Arc::new(
MockStoreSuccessBuilder::from_accounts(
accounts.iter().map(|account| (account.id, account.states[0])),
)
.initial_notes([vec![OutputNote::Full(notes[0].clone())]].iter())
.initial_chain_mmr(mmr)
.build(),
);
let block_builder = Arc::new(DefaultBlockBuilder::new(Arc::clone(&store), Arc::clone(&store)));
Expand Down
17 changes: 7 additions & 10 deletions crates/block-producer/src/block.rs
Original file line number Diff line number Diff line change
@@ -1,17 +1,15 @@
use std::collections::{BTreeMap, BTreeSet};
use std::collections::BTreeMap;

use miden_node_proto::{
errors::{ConversionError, MissingFieldHelper},
generated::responses::GetBlockInputsResponse,
AccountInputRecord, NullifierWitness,
};
use miden_node_store::state::NoteAuthenticationInfo;
use miden_objects::{
accounts::AccountId,
crypto::{
hash::rpo::RpoDigest,
merkle::{MerklePath, MmrPeaks, SmtProof},
},
notes::{NoteId, Nullifier},
crypto::merkle::{MerklePath, MmrPeaks, SmtProof},
notes::Nullifier,
BlockHeader, Digest,
};

Expand All @@ -36,7 +34,7 @@ pub struct BlockInputs {
pub nullifiers: BTreeMap<Nullifier, SmtProof>,

/// List of unauthenticated notes found in the store
pub found_unauthenticated_notes: BTreeSet<NoteId>,
pub found_unauthenticated_notes: NoteAuthenticationInfo,
}

#[derive(Clone, Debug, Default)]
Expand Down Expand Up @@ -95,9 +93,8 @@ impl TryFrom<GetBlockInputsResponse> for BlockInputs {

let found_unauthenticated_notes = response
.found_unauthenticated_notes
.into_iter()
.map(|digest| Ok(RpoDigest::try_from(digest)?.into()))
.collect::<Result<_, ConversionError>>()?;
.ok_or(GetBlockInputsResponse::missing_field("found_authenticated_notes"))?
.try_into()?;

Ok(Self {
block_header,
Expand Down
4 changes: 3 additions & 1 deletion crates/block-producer/src/block_builder/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,9 @@ where
.await?;

let missing_notes: Vec<_> = dangling_notes
.difference(&block_inputs.found_unauthenticated_notes)
.difference(
&block_inputs.found_unauthenticated_notes.note_proofs.keys().copied().collect(),
Mirko-von-Leipzig marked this conversation as resolved.
Show resolved Hide resolved
)
.copied()
.collect();
if !missing_notes.is_empty() {
Expand Down
40 changes: 14 additions & 26 deletions crates/block-producer/src/store/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,21 +9,21 @@ use itertools::Itertools;
use miden_node_proto::{
errors::{ConversionError, MissingFieldHelper},
generated::{
digest, note,
digest,
requests::{
ApplyBlockRequest, GetBlockInputsRequest, GetNotesByIdRequest,
ApplyBlockRequest, GetBlockInputsRequest, GetNoteInclusionProofsRequest,
GetTransactionInputsRequest,
},
responses::{GetTransactionInputsResponse, NullifierTransactionInputRecord},
store::api_client as store_client,
},
AccountState,
};
use miden_node_store::state::NoteAuthenticationInfo;
use miden_node_utils::formatting::format_opt;
use miden_objects::{
accounts::AccountId,
block::Block,
crypto::merkle::MerklePath,
notes::{NoteId, Nullifier},
utils::Serializable,
Digest,
Expand Down Expand Up @@ -56,15 +56,12 @@ pub trait Store: ApplyBlock {

/// Returns note authentication information for the set of specified notes.
///
/// If authentication info could for a note does not exist in the store, the note is omitted
/// If authentication info for a note does not exist in the store, the note is omitted
/// from the returned set of notes.
///
/// TODO: right now this return only Merkle paths per note, but this will need to be updated to
/// return full authentication info.
async fn get_note_authentication_info(
&self,
notes: impl Iterator<Item = &NoteId> + Send,
) -> Result<BTreeMap<NoteId, MerklePath>, NotePathsError>;
) -> Result<NoteAuthenticationInfo, NotePathsError>;
}

#[async_trait]
Expand Down Expand Up @@ -257,33 +254,24 @@ impl Store for DefaultStore {
async fn get_note_authentication_info(
&self,
notes: impl Iterator<Item = &NoteId> + Send,
) -> Result<BTreeMap<NoteId, MerklePath>, NotePathsError> {
let request = tonic::Request::new(GetNotesByIdRequest {
) -> Result<NoteAuthenticationInfo, NotePathsError> {
let request = tonic::Request::new(GetNoteInclusionProofsRequest {
note_ids: notes.map(digest::Digest::from).collect(),
});

let store_response = self
.store
.clone()
.get_notes_by_id(request)
.get_note_inclusion_proofs(request)
.await
.map_err(|err| NotePathsError::GrpcClientError(err.message().to_string()))?
.into_inner();

Ok(store_response
.notes
.into_iter()
.map(|note| {
Ok((
RpoDigest::try_from(
note.note_id.ok_or(note::Note::missing_field(stringify!(note_id)))?,
)?
.into(),
note.merkle_path
.ok_or(note::Note::missing_field(stringify!(merkle_path)))?
.try_into()?,
))
})
.collect::<Result<BTreeMap<_, _>, ConversionError>>()?)
let note_authentication_info = store_response
.proofs
.ok_or(GetTransactionInputsResponse::missing_field("proofs"))?
.try_into()?;

Ok(note_authentication_info)
}
}
18 changes: 16 additions & 2 deletions crates/block-producer/src/test_utils/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,14 @@ pub async fn build_expected_block_header(
store: &MockStoreSuccess,
batches: &[TransactionBatch],
) -> BlockHeader {
let last_block_header = *store.last_block_header.read().await;
let last_block_header = *store
.block_headers
.read()
.await
.iter()
.max_by_key(|(block_num, _)| *block_num)
.unwrap()
.1;
Mirko-von-Leipzig marked this conversation as resolved.
Show resolved Hide resolved

// Compute new account root
let updated_accounts: Vec<_> =
Expand Down Expand Up @@ -104,7 +111,14 @@ impl MockBlockBuilder {
Self {
store_accounts: store.accounts.read().await.clone(),
store_chain_mmr: store.chain_mmr.read().await.clone(),
last_block_header: *store.last_block_header.read().await,
last_block_header: *store
.block_headers
.read()
.await
.iter()
.max_by_key(|(block_num, _)| *block_num)
.unwrap()
.1,

updated_accounts: None,
created_notes: None,
Expand Down
Loading
Loading