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

Add support for on-chain notes to block-producer #303

Closed
wants to merge 28 commits into from
Closed
Show file tree
Hide file tree
Changes from 27 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
7c09397
Getting notes by id works
phklive Apr 4, 2024
24c05ab
lint and test
phklive Apr 4, 2024
c52dafe
Improved comments + Added conversion from digest::Digest to NoteId
phklive Apr 4, 2024
626cb54
Added rpc validation for NoteId's
phklive Apr 4, 2024
0d80b8b
Added documentation to Readmes
phklive Apr 4, 2024
0335339
Lint
phklive Apr 4, 2024
1a4a829
Merge next + lint
phklive Apr 5, 2024
0c25238
Added details field to database
phklive Apr 5, 2024
e0572ba
Fixed ci problems
phklive Apr 5, 2024
9fd74e3
Merge branch 'phklive-get-notes-by-id' into phklive-db-onchain-note
phklive Apr 5, 2024
443795e
Added details to Note type
phklive Apr 5, 2024
a6bd2ea
Added test for endpoint + refactored sql query + improved documentation
phklive Apr 5, 2024
8ce4963
Merge branch 'phklive-get-notes-by-id' into phklive-db-onchain-note
phklive Apr 5, 2024
a32b312
Fixed lint
phklive Apr 5, 2024
2b037a2
Merge branch 'phklive-get-notes-by-id' into phklive-db-onchain-note
phklive Apr 5, 2024
895a7a2
Fixed ci with test problem
phklive Apr 5, 2024
7907be9
lint
phklive Apr 5, 2024
9daef27
Fix nit in migration
phklive Apr 5, 2024
fd34c43
Merge branch 'next' into phklive-db-onchain-note
phklive Apr 5, 2024
f9b4a0b
Order rpc and store endpoints alphabitically
phklive Apr 5, 2024
f0e7e1a
Change name to database to prevent gitignore problems
phklive Apr 5, 2024
544473d
Change name to database to prevent gitignore problems
phklive Apr 5, 2024
3c5a9fd
Merge branch 'next' into phklive-get-notes-by-id
phklive Apr 5, 2024
f4ec7e5
Merge branch 'phklive-get-notes-by-id' into phklive-db-onchain-note
phklive Apr 5, 2024
5ff0403
generated
phklive Apr 5, 2024
c1fdaaa
Updated block + added details to NoteCreated instead of NoteEnvelope
phklive Apr 5, 2024
d36c140
node runs, able to produce blocks, modified apply_block
phklive Apr 5, 2024
f0c0af1
Improved documentation with gh comments
phklive Apr 8, 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
2 changes: 1 addition & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,4 @@ jobs:
- name: Install cargo make
run: cargo install cargo-make
- name: cargo make - test
run: cargo make test
run: cargo make test-all
4 changes: 2 additions & 2 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ genesis.dat
*.sqlite3
*.sqlite3-shm
*.sqlite3-wal
db/
database/

# Configs
/genesis.toml
/miden.toml
/miden.toml
2 changes: 1 addition & 1 deletion Makefile.toml
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ dependencies = [
]

# testing
[tasks.test]
[tasks.test-all]
command = "cargo"
args = ["test", "--all-features", "--workspace", "--", "--nocapture"]

Expand Down
54 changes: 46 additions & 8 deletions block-producer/src/batch_builder/batch.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
use std::collections::BTreeMap;

use miden_node_proto::domain::accounts::UpdatedAccount;
use miden_node_proto::{domain::accounts::UpdatedAccount, generated::note::NoteCreated};
use miden_objects::{
accounts::AccountId,
batches::BatchNoteTree,
crypto::hash::blake::{Blake3Digest, Blake3_256},
notes::{NoteEnvelope, Nullifier},
utils::serde::Serializable,
Digest, MAX_NOTES_PER_BATCH,
};
use tracing::instrument;
Expand All @@ -26,9 +27,10 @@ pub struct TransactionBatch {
id: BatchId,
updated_accounts: BTreeMap<AccountId, AccountStates>,
produced_nullifiers: Vec<Nullifier>,
created_notes_smt: BatchNoteTree,
/// The notes stored `created_notes_smt`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems this comment is not in the right place now. And I think, it is confusing a bit, maybe it would be better to get rid of this comment at all?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is also what I thought, I did not know if I had to remove it.

Removed.

created_notes: Vec<NoteEnvelope>,
created_notes_smt: BatchNoteTree,
created_notes: Vec<NoteCreated>,
created_note_envelopes: Vec<NoteEnvelope>,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need these envelopes anywhere else?

}

impl TransactionBatch {
Expand Down Expand Up @@ -62,23 +64,52 @@ impl TransactionBatch {
let produced_nullifiers =
txs.iter().flat_map(|tx| tx.input_notes().iter()).cloned().collect();

let (created_notes, created_notes_smt) = {
let created_notes: Vec<NoteEnvelope> =
let (created_notes, created_notes_smt, created_note_envelopes) = {
let created_note_envelopes: Vec<NoteEnvelope> =
txs.iter().flat_map(|tx| tx.output_notes().iter()).cloned().collect();

let created_notes: Vec<NoteCreated> = txs
.iter()
.flat_map(|tx| {
tx.output_notes().iter().map(|note| {
if let Some(note_with_details) = tx.get_output_note_details(&note.note_id())
{
NoteCreated {
batch_index: 0,
note_index: 0,
note_id: Some(note.note_id().into()),
sender: Some(note.metadata().sender().into()),
tag: note.metadata().tag().into(),
details: Some(note_with_details.to_bytes()),
}
} else {
NoteCreated {
batch_index: 0,
note_index: 0,
note_id: Some(note.note_id().into()),
sender: Some(note.metadata().sender().into()),
tag: note.metadata().tag().into(),
details: None,
}
}
})
})
.collect();

Comment on lines +71 to +98
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will likely have to change after 0xPolygonMiden/miden-base#572. Maybe it's best to point at that branch for now to not have to redo this soonish.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This implementation missing note_index field within the batch.
I would write something like this:

            let created_notes: Vec<NoteCreated> = txs
                .iter()
                .flat_map(|tx| {
                    tx.output_notes().iter().map(|note| {
                        (note, tx.get_output_note_details(&note.note_id())
                    })
                })
                .enumerate()
                .map(|(note_index, (note, details))| NoteCreated {
                    batch_index: 0,
                    note_index: note_index as u32,
                    note_id: Some(note.note_id().into()),
                    sender: Some(note.metadata().sender().into()),
                    tag: note.metadata().tag().into(),
                    details: details.map(Note::to_bytes),
                })
                .collect();

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the heads up, fixed the implementation.

if created_notes.len() > MAX_NOTES_PER_BATCH {
return Err(BuildBatchError::TooManyNotesCreated(created_notes.len(), txs));
}

// TODO: document under what circumstances SMT creating can fail
(
created_notes.clone(),
created_notes,
BatchNoteTree::with_contiguous_leaves(
created_notes
created_note_envelopes
.iter()
.map(|note_envelope| (note_envelope.note_id(), note_envelope.metadata())),
)
.map_err(|e| BuildBatchError::NotesSmtError(e, txs))?,
created_note_envelopes,
)
};

Expand All @@ -88,6 +119,7 @@ impl TransactionBatch {
produced_nullifiers,
created_notes_smt,
created_notes,
created_note_envelopes,
})
}

Expand Down Expand Up @@ -125,10 +157,16 @@ impl TransactionBatch {
}

/// Returns an iterator over created notes.
pub fn created_notes(&self) -> impl Iterator<Item = &NoteEnvelope> + '_ {
pub fn created_notes(&self) -> impl Iterator<Item = &NoteCreated> + '_ {
self.created_notes.iter()
}

#[allow(dead_code)]
/// Returns an iterator over created note envelopes.
pub fn created_notes_envelopes(&self) -> impl Iterator<Item = &NoteEnvelope> + '_ {
self.created_note_envelopes.iter()
}

/// Returns the root hash of the created notes SMT.
pub fn created_notes_root(&self) -> Digest {
self.created_notes_smt.root()
Expand Down
6 changes: 3 additions & 3 deletions block-producer/src/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@ use std::collections::BTreeMap;
use miden_node_proto::{
domain::accounts::UpdatedAccount,
errors::{ConversionError, MissingFieldHelper},
generated::responses::GetBlockInputsResponse,
generated::{note::NoteCreated, responses::GetBlockInputsResponse},
AccountInputRecord, NullifierWitness,
};
use miden_objects::{
accounts::AccountId,
crypto::merkle::{MerklePath, MmrPeaks, SmtProof},
notes::{NoteEnvelope, Nullifier},
notes::Nullifier,
BlockHeader, Digest,
};

Expand All @@ -19,7 +19,7 @@ use crate::store::BlockInputsError;
pub struct Block {
pub header: BlockHeader,
pub updated_accounts: Vec<UpdatedAccount>,
pub created_notes: Vec<(usize, usize, NoteEnvelope)>,
pub created_notes: Vec<(usize, usize, NoteCreated)>,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we put NoteCreated here, we don't need to provide additionally batch_index and note_index here.

pub produced_nullifiers: Vec<Nullifier>,
// TODO:
// - full states for updated public accounts
Expand Down
12 changes: 6 additions & 6 deletions block-producer/src/block_builder/mod.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use std::sync::Arc;
use std::{sync::Arc, usize};

use async_trait::async_trait;
use miden_node_proto::generated::note::NoteCreated;
use miden_node_utils::formatting::{format_array, format_blake3_digest};
use miden_objects::notes::Nullifier;
use tracing::{debug, info, instrument};
Expand Down Expand Up @@ -79,14 +80,13 @@ where

let updated_accounts: Vec<_> =
batches.iter().flat_map(TransactionBatch::updated_accounts).collect();
let created_notes = batches
let created_notes: Vec<(usize, usize, NoteCreated)> = batches
.iter()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need these two additional usizes, because we already have batch_index and note_index fields in NoteCreated structure. The task is to fill them here. I would probably set note_index in the batch builder (as in my proposed piece of code) and batch_index here.

I think, the best solution would be to not to create NoteCreated in the batch builder, but just to add serialized details field in the TransactionBatch::created_note_envelopes vector:

created_note_envelopes_with_details: Vec<(NoteEnvelope, Option<Vec<u8>>)>,

And after that we would be able to produce NoteCreated here and init its batch_index and note_index fields during creation.

.enumerate()
.flat_map(|(batch_idx, batch)| {
batch
.created_notes()
.enumerate()
.map(move |(note_idx_in_batch, note)| (batch_idx, note_idx_in_batch, *note))
batch.created_notes().enumerate().map(move |(note_idx_in_batch, note)| {
(batch_idx, note_idx_in_batch, note.clone())
})
})
.collect();
let produced_nullifiers: Vec<Nullifier> =
Expand Down
29 changes: 22 additions & 7 deletions block-producer/src/test_utils/block.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use miden_node_proto::domain::accounts::UpdatedAccount;
use miden_node_proto::{domain::accounts::UpdatedAccount, generated::note::NoteCreated};
use miden_objects::{
block::BlockNoteTree,
crypto::merkle::{Mmr, SimpleSmt},
Expand Down Expand Up @@ -90,7 +90,8 @@ pub struct MockBlockBuilder {
last_block_header: BlockHeader,

updated_accounts: Option<Vec<UpdatedAccount>>,
created_notes: Option<Vec<(usize, usize, NoteEnvelope)>>,
created_notes: Option<Vec<(usize, usize, NoteCreated)>>,
created_note_envelopes: Option<Vec<(usize, usize, NoteEnvelope)>>,
produced_nullifiers: Option<Vec<Nullifier>>,
}

Expand All @@ -103,6 +104,7 @@ impl MockBlockBuilder {

updated_accounts: None,
created_notes: None,
created_note_envelopes: None,
produced_nullifiers: None,
}
}
Expand All @@ -123,13 +125,22 @@ impl MockBlockBuilder {

pub fn created_notes(
mut self,
created_notes: Vec<(usize, usize, NoteEnvelope)>,
created_notes: Vec<(usize, usize, NoteCreated)>,
) -> Self {
self.created_notes = Some(created_notes);

self
}

pub fn created_note_envelopes(
mut self,
created_note_envelopes: Vec<(usize, usize, NoteEnvelope)>,
) -> Self {
self.created_note_envelopes = Some(created_note_envelopes);

self
}

pub fn produced_nullifiers(
mut self,
produced_nullifiers: Vec<Nullifier>,
Expand All @@ -141,14 +152,15 @@ impl MockBlockBuilder {

pub fn build(self) -> Block {
let created_notes = self.created_notes.unwrap_or_default();
let created_note_envelopes = self.created_note_envelopes.unwrap_or_default();

let header = BlockHeader::new(
self.last_block_header.hash(),
self.last_block_header.block_num() + 1,
self.store_chain_mmr.peaks(self.store_chain_mmr.forest()).unwrap().hash_peaks(),
self.store_accounts.root(),
Digest::default(),
note_created_smt_from_envelopes(created_notes.iter().cloned()).root(),
note_created_smt_from_envelopes(created_note_envelopes.iter().cloned()).root(),
Digest::default(),
Digest::default(),
ZERO,
Expand Down Expand Up @@ -177,9 +189,12 @@ pub(crate) fn note_created_smt_from_batches<'a>(
batches: impl Iterator<Item = &'a TransactionBatch>
) -> BlockNoteTree {
let note_leaf_iterator = batches.enumerate().flat_map(|(batch_idx, batch)| {
batch.created_notes().enumerate().map(move |(note_idx_in_batch, note)| {
(batch_idx, note_idx_in_batch, (note.note_id().into(), *note.metadata()))
})
batch
.created_notes_envelopes()
.enumerate()
.map(move |(note_idx_in_batch, note)| {
(batch_idx, note_idx_in_batch, (note.note_id().into(), *note.metadata()))
})
});

BlockNoteTree::with_entries(note_leaf_iterator).unwrap()
Expand Down
2 changes: 1 addition & 1 deletion node/miden-node.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,5 @@ store_url = "http://localhost:28943"
[store]
# port defined as: sum(ord(c)**p for (p, c) in enumerate('miden-store', 1)) % 2**16
endpoint = { host = "localhost", port = 28943 }
database_filepath = "db/miden-store.sqlite3"
database_filepath = "database/miden-store.sqlite3"
genesis_filepath = "genesis.dat"
4 changes: 3 additions & 1 deletion proto/proto/note.proto
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ message Note {
account.AccountId sender = 4;
fixed64 tag = 5;
merkle.MerklePath merkle_path = 7;
optional bytes details = 8;
}

message NoteSyncRecord {
Expand All @@ -28,4 +29,5 @@ message NoteCreated {
digest.Digest note_id = 3;
account.AccountId sender = 4;
fixed64 tag = 5;
}
optional bytes details = 6;
}
5 changes: 5 additions & 0 deletions proto/proto/requests.proto
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,11 @@ message SubmitProvenTransactionRequest {
bytes transaction = 1;
}

message GetNotesByIdRequest {
// List of NoteId's to be queried from the database
repeated digest.Digest note_ids = 1;
}

message ListNullifiersRequest {}

message ListAccountsRequest {}
Expand Down
5 changes: 5 additions & 0 deletions proto/proto/responses.proto
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,11 @@ message GetTransactionInputsResponse {

message SubmitProvenTransactionResponse {}

message GetNotesByIdResponse {
// Lists Note's returned by the database
repeated note.Note notes = 1;
}

message ListNullifiersResponse {
// Lists all nullifiers of the current chain
repeated smt.SmtLeafEntry nullifiers = 1;
Expand Down
1 change: 1 addition & 0 deletions proto/proto/rpc.proto
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import "responses.proto";
service Api {
rpc CheckNullifiers(requests.CheckNullifiersRequest) returns (responses.CheckNullifiersResponse) {}
rpc GetBlockHeaderByNumber(requests.GetBlockHeaderByNumberRequest) returns (responses.GetBlockHeaderByNumberResponse) {}
rpc GetNotesById(requests.GetNotesByIdRequest) returns (responses.GetNotesByIdResponse) {}
rpc SyncState(requests.SyncStateRequest) returns (responses.SyncStateResponse) {}
rpc SubmitProvenTransaction(requests.SubmitProvenTransactionRequest) returns (responses.SubmitProvenTransactionResponse) {}
}
1 change: 1 addition & 0 deletions proto/proto/store.proto
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ service Api {
rpc CheckNullifiers(requests.CheckNullifiersRequest) returns (responses.CheckNullifiersResponse) {}
rpc GetBlockHeaderByNumber(requests.GetBlockHeaderByNumberRequest) returns (responses.GetBlockHeaderByNumberResponse) {}
rpc GetBlockInputs(requests.GetBlockInputsRequest) returns (responses.GetBlockInputsResponse) {}
rpc GetNotesById(requests.GetNotesByIdRequest) returns (responses.GetNotesByIdResponse) {}
rpc GetTransactionInputs(requests.GetTransactionInputsRequest) returns (responses.GetTransactionInputsResponse) {}
rpc SyncState(requests.SyncStateRequest) returns (responses.SyncStateResponse) {}
rpc ListNullifiers(requests.ListNullifiersRequest) returns (responses.ListNullifiersResponse) {}
Expand Down
15 changes: 7 additions & 8 deletions proto/src/domain/notes.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
use miden_objects::notes::NoteEnvelope;

use crate::generated::note;
use crate::generated::note::{self, NoteCreated};

// Note
// ================================================================================================
Expand All @@ -20,14 +18,15 @@ impl From<note::Note> for note::NoteSyncRecord {
// NoteCreated
// ================================================================================================

impl From<(usize, usize, NoteEnvelope)> for note::NoteCreated {
fn from((batch_idx, note_idx, note): (usize, usize, NoteEnvelope)) -> Self {
impl From<(usize, usize, NoteCreated)> for note::NoteCreated {
fn from((batch_idx, note_idx, note): (usize, usize, NoteCreated)) -> Self {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure we need such conversion.

Self {
batch_index: batch_idx as u32,
note_index: note_idx as u32,
note_id: Some(note.note_id().into()),
sender: Some(note.metadata().sender().into()),
tag: note.metadata().tag().into(),
note_id: note.note_id,
sender: note.sender,
tag: note.tag,
details: note.details,
}
}
}
4 changes: 4 additions & 0 deletions proto/src/generated/note.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ pub struct Note {
pub tag: u64,
#[prost(message, optional, tag = "7")]
pub merkle_path: ::core::option::Option<super::merkle::MerklePath>,
#[prost(bytes = "vec", optional, tag = "8")]
pub details: ::core::option::Option<::prost::alloc::vec::Vec<u8>>,
}
#[derive(Eq, PartialOrd, Ord, Hash)]
#[allow(clippy::derive_partial_eq_without_eq)]
Expand Down Expand Up @@ -44,4 +46,6 @@ pub struct NoteCreated {
pub sender: ::core::option::Option<super::account::AccountId>,
#[prost(fixed64, tag = "5")]
pub tag: u64,
#[prost(bytes = "vec", optional, tag = "6")]
pub details: ::core::option::Option<::prost::alloc::vec::Vec<u8>>,
}
8 changes: 8 additions & 0 deletions proto/src/generated/requests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,14 @@ pub struct SubmitProvenTransactionRequest {
#[derive(Eq, PartialOrd, Ord, Hash)]
#[allow(clippy::derive_partial_eq_without_eq)]
#[derive(Clone, PartialEq, ::prost::Message)]
pub struct GetNotesByIdRequest {
/// List of NoteId's to be queried from the database
#[prost(message, repeated, tag = "1")]
pub note_ids: ::prost::alloc::vec::Vec<super::digest::Digest>,
}
#[derive(Eq, PartialOrd, Ord, Hash)]
#[allow(clippy::derive_partial_eq_without_eq)]
#[derive(Clone, PartialEq, ::prost::Message)]
pub struct ListNullifiersRequest {}
#[derive(Eq, PartialOrd, Ord, Hash)]
#[allow(clippy::derive_partial_eq_without_eq)]
Expand Down
Loading
Loading