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: Implement NoteMetadata protobuf message and NoteType enum #338

Merged
merged 14 commits into from
May 3, 2024
3 changes: 0 additions & 3 deletions crates/proto/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,9 @@ fn main() -> miette::Result<()> {
// Compute the directory of the `proto` definitions
let cwd: PathBuf = env::current_dir().into_diagnostic()?;
let proto_dir: PathBuf = cwd.join("proto");

igamigo marked this conversation as resolved.
Show resolved Hide resolved
// Compute the compiler's target file path.
let out = env::var("OUT_DIR").into_diagnostic()?;
let file_descriptor_path = PathBuf::from(out).join("file_descriptor_set.bin");

// Compile the proto file for all servers APIs
let protos = &[
proto_dir.join("block_producer.proto"),
Expand All @@ -21,7 +19,6 @@ fn main() -> miette::Result<()> {
let includes = &[proto_dir];
let file_descriptors = protox::compile(protos, includes)?;
fs::write(&file_descriptor_path, file_descriptors.encode_to_vec()).into_diagnostic()?;

let mut prost_config = prost_build::Config::new();
prost_config.skip_debug(["AccountId", "Digest"]);

Expand Down
27 changes: 18 additions & 9 deletions crates/proto/proto/note.proto
Original file line number Diff line number Diff line change
Expand Up @@ -5,24 +5,33 @@ import "digest.proto";
import "merkle.proto";
import "account.proto";

enum NoteType {
PUBLIC = 0;
OFF_CHAIN = 1;
ENCRYPTED = 2;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Should these values be aligned with values here. If so, I wonder if there is a good way to keep them consistent (maybe using enum here was not such a good idea?).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, not sure about this. There cannot be protobuf enums that don't start with 0 so it's either creating a phantom variant, creating a custom mapping function (from protobuf's note type to base note type), or just removing it altogether and using a fixed32. For making sure they stay synced I can add a test that asserts this.

Copy link
Collaborator Author

@igamigo igamigo May 3, 2024

Choose a reason for hiding this comment

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

On second thought it's not worth the added complexity if we can just convert the note type number. Removed in favor of a fixed32.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed! I came to the same conclusion. The only question - should we use int32 instead? (this way, encoding would take 1 byte instead of 4, but otherwise should be the same).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Changed to uint32. Seemed similar in terms of encoding low values but marginally more correct for the use case.


message NoteMetadata {
account.AccountId sender = 1;
NoteType note_type = 2;
fixed32 tag = 3;
fixed64 aux = 4;
}

message Note {
fixed32 block_num = 1;
uint32 note_index = 2;
digest.Digest note_id = 3;
account.AccountId sender = 4;
fixed32 tag = 5;
uint32 note_type = 6;
merkle.MerklePath merkle_path = 7;
NoteMetadata metadata = 4;
merkle.MerklePath merkle_path = 5;
// This field will be present when the note is on-chain.
// details contain the `Note` in a serialized format.
optional bytes details = 8;
optional bytes details = 6;
}

message NoteSyncRecord {
uint32 note_index = 1;
digest.Digest note_id = 2;
account.AccountId sender = 3;
fixed32 tag = 4;
uint32 note_type = 5;
merkle.MerklePath merkle_path = 6;
NoteMetadata metadata = 3;
merkle.MerklePath merkle_path = 4;
}
1 change: 1 addition & 0 deletions crates/proto/src/domain/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ pub mod accounts;
pub mod blocks;
pub mod digest;
pub mod merkle;
pub mod notes;
pub mod nullifiers;

// UTILITIES
Expand Down
33 changes: 33 additions & 0 deletions crates/proto/src/domain/notes.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
use miden_objects::{
notes::{NoteMetadata, NoteTag, NoteType},
Felt,
};

use crate::errors::{ConversionError, MissingFieldHelper};

impl TryFrom<crate::generated::note::NoteMetadata> for NoteMetadata {
type Error = ConversionError;

fn try_from(value: crate::generated::note::NoteMetadata) -> Result<Self, Self::Error> {
let sender = value
.sender
.ok_or_else(|| crate::generated::note::NoteMetadata::missing_field("Sender"))?
.try_into()?;
let note_type = NoteType::try_from(value.note_type as u64)?;
let tag = NoteTag::from(value.tag);
let aux = Felt::new(value.aux);
igamigo marked this conversation as resolved.
Show resolved Hide resolved

Ok(NoteMetadata::new(sender, note_type, tag, aux)?)
}
}

impl From<NoteMetadata> for crate::generated::note::NoteMetadata {
fn from(val: NoteMetadata) -> Self {
let sender = Some(val.sender().into());
let note_type = val.note_type() as i32;
let tag = val.tag().into();
let aux = val.aux().into();

crate::generated::note::NoteMetadata { sender, note_type, tag, aux }
}
}
2 changes: 2 additions & 0 deletions crates/proto/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ pub enum ConversionError {
InsufficientData { expected: usize, got: usize },
#[error("Value is not in the range 0..MODULUS")]
NotAValidFelt,
#[error("Invalid note type value: {0}")]
NoteTypeError(#[from] miden_objects::NoteError),
#[error("Field `{field_name}` required to be filled in protobuf representation of {entity}")]
MissingFieldInProtobufRepresentation {
entity: &'static str,
Expand Down
61 changes: 48 additions & 13 deletions crates/proto/src/generated/note.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,19 @@
#[derive(Eq, PartialOrd, Ord, Hash)]
#[allow(clippy::derive_partial_eq_without_eq)]
#[derive(Clone, PartialEq, ::prost::Message)]
pub struct NoteMetadata {
#[prost(message, optional, tag = "1")]
pub sender: ::core::option::Option<super::account::AccountId>,
#[prost(enumeration = "NoteType", tag = "2")]
pub note_type: i32,
#[prost(fixed32, tag = "3")]
pub tag: u32,
#[prost(fixed64, tag = "4")]
pub aux: u64,
}
#[derive(Eq, PartialOrd, Ord, Hash)]
#[allow(clippy::derive_partial_eq_without_eq)]
#[derive(Clone, PartialEq, ::prost::Message)]
pub struct Note {
#[prost(fixed32, tag = "1")]
pub block_num: u32,
Expand All @@ -10,16 +23,12 @@ pub struct Note {
#[prost(message, optional, tag = "3")]
pub note_id: ::core::option::Option<super::digest::Digest>,
#[prost(message, optional, tag = "4")]
pub sender: ::core::option::Option<super::account::AccountId>,
#[prost(fixed32, tag = "5")]
pub tag: u32,
#[prost(uint32, tag = "6")]
pub note_type: u32,
#[prost(message, optional, tag = "7")]
pub metadata: ::core::option::Option<NoteMetadata>,
#[prost(message, optional, tag = "5")]
pub merkle_path: ::core::option::Option<super::merkle::MerklePath>,
/// This field will be present when the note is on-chain.
/// details contain the `Note` in a serialized format.
#[prost(bytes = "vec", optional, tag = "8")]
#[prost(bytes = "vec", optional, tag = "6")]
pub details: ::core::option::Option<::prost::alloc::vec::Vec<u8>>,
}
#[derive(Eq, PartialOrd, Ord, Hash)]
Expand All @@ -31,11 +40,37 @@ pub struct NoteSyncRecord {
#[prost(message, optional, tag = "2")]
pub note_id: ::core::option::Option<super::digest::Digest>,
#[prost(message, optional, tag = "3")]
pub sender: ::core::option::Option<super::account::AccountId>,
#[prost(fixed32, tag = "4")]
pub tag: u32,
#[prost(uint32, tag = "5")]
pub note_type: u32,
#[prost(message, optional, tag = "6")]
pub metadata: ::core::option::Option<NoteMetadata>,
#[prost(message, optional, tag = "4")]
pub merkle_path: ::core::option::Option<super::merkle::MerklePath>,
}
#[derive(Eq, PartialOrd, Ord, Hash)]
#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, PartialOrd, Ord, ::prost::Enumeration)]
igamigo marked this conversation as resolved.
Show resolved Hide resolved
#[repr(i32)]
pub enum NoteType {
Public = 0,
OffChain = 1,
Encrypted = 2,
}
impl NoteType {
/// String value of the enum field names used in the ProtoBuf definition.
///
/// The values are not transformed in any way and thus are considered stable
/// (if the ProtoBuf definition does not change) and safe for programmatic use.
pub fn as_str_name(&self) -> &'static str {
match self {
NoteType::Public => "PUBLIC",
NoteType::OffChain => "OFF_CHAIN",
NoteType::Encrypted => "ENCRYPTED",
}
}
/// Creates an enum from field names used in the ProtoBuf definition.
pub fn from_str_name(value: &str) -> ::core::option::Option<Self> {
match value {
"PUBLIC" => Some(Self::Public),
"OFF_CHAIN" => Some(Self::OffChain),
"ENCRYPTED" => Some(Self::Encrypted),
_ => None,
}
}
}
10 changes: 3 additions & 7 deletions crates/store/src/db/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use miden_objects::{
accounts::delta::AccountUpdateDetails,
block::{BlockAccountUpdate, BlockNoteIndex},
crypto::{hash::rpo::RpoDigest, merkle::MerklePath, utils::Deserializable},
notes::{NoteId, NoteType, Nullifier},
notes::{NoteId, NoteMetadata, Nullifier},
BlockHeader, GENESIS_BLOCK,
};
use rusqlite::vtab::array;
Expand Down Expand Up @@ -49,9 +49,7 @@ pub struct NoteRecord {
pub block_num: BlockNumber,
pub note_index: BlockNoteIndex,
pub note_id: RpoDigest,
pub note_type: NoteType,
pub sender: AccountId,
pub tag: u32,
pub metadata: NoteMetadata,
pub details: Option<Vec<u8>>,
pub merkle_path: MerklePath,
}
Expand All @@ -62,9 +60,7 @@ impl From<NoteRecord> for NotePb {
block_num: note.block_num,
note_index: note.note_index.to_absolute_index() as u32,
note_id: Some(note.note_id.into()),
sender: Some(note.sender.into()),
tag: note.tag,
note_type: note.note_type as u32,
metadata: Some(note.metadata.into()),
merkle_path: Some(note.merkle_path.into()),
details: note.details,
}
Expand Down
46 changes: 30 additions & 16 deletions crates/store/src/db/sql.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use miden_objects::{
accounts::{delta::AccountUpdateDetails, Account, AccountDelta},
block::{BlockAccountUpdate, BlockNoteIndex},
crypto::{hash::rpo::RpoDigest, merkle::MerklePath},
notes::{NoteId, Nullifier},
notes::{NoteId, NoteMetadata, NoteType, Nullifier},
utils::serde::{Deserializable, Serializable},
BlockHeader,
};
Expand Down Expand Up @@ -347,13 +347,18 @@ pub fn select_notes(conn: &mut Connection) -> Result<Vec<NoteRecord>> {
let details_data = row.get_ref(8)?.as_blob_or_null()?;
let details = details_data.map(<Vec<u8>>::read_from_bytes).transpose()?;

let note_type = row.get::<_, u8>(4)?.try_into()?;
let sender = column_value_as_u64(row, 5)?;
let tag: u32 = row.get(6)?;

let metadata =
NoteMetadata::new(sender.try_into()?, note_type, tag.into(), Default::default())?;

notes.push(NoteRecord {
block_num: row.get(0)?,
note_index: BlockNoteIndex::new(row.get(1)?, row.get(2)?),
note_id,
note_type: row.get::<_, u8>(4)?.try_into()?,
sender: column_value_as_u64(row, 5)?,
tag: row.get(6)?,
metadata,
details,
merkle_path,
})
Expand Down Expand Up @@ -396,15 +401,14 @@ pub fn insert_notes(transaction: &Transaction, notes: &[NoteRecord]) -> Result<u
let mut count = 0;
for note in notes.iter() {
let details = note.details.as_ref().map(|details| details.to_bytes());

count += stmt.execute(params![
note.block_num,
note.note_index.batch_idx(),
note.note_index.note_idx_in_batch(),
note.note_id.to_bytes(),
note.note_type as u8,
u64_to_value(note.sender),
note.tag,
note.metadata.note_type() as u8,
u64_to_value(note.metadata.sender().into()),
note.metadata.tag().inner(),
bobbinth marked this conversation as resolved.
Show resolved Hide resolved
note.merkle_path.to_bytes(),
details
])?;
Expand Down Expand Up @@ -475,22 +479,27 @@ pub fn select_notes_since_block_by_tag_and_sender(
let note_index = BlockNoteIndex::new(row.get(1)?, row.get(2)?);
let note_id_data = row.get_ref(3)?.as_blob()?;
let note_id = RpoDigest::read_from_bytes(note_id_data)?;
let note_type = row.get::<_, u8>(4)?.try_into()?;
let note_type = row.get::<_, u8>(4)?;
let sender = column_value_as_u64(row, 5)?;
let tag = row.get(6)?;
let tag: u32 = row.get(6)?;
let merkle_path_data = row.get_ref(7)?.as_blob()?;
let merkle_path = MerklePath::read_from_bytes(merkle_path_data)?;
let details_data = row.get_ref(8)?.as_blob_or_null()?;
let details = details_data.map(<Vec<u8>>::read_from_bytes).transpose()?;

let metadata = NoteMetadata::new(
sender.try_into()?,
NoteType::try_from(note_type)?,
tag.into(),
Default::default(),
)?;
igamigo marked this conversation as resolved.
Show resolved Hide resolved

let note = NoteRecord {
block_num,
note_index,
note_id,
note_type,
sender,
tag,
details,
metadata,
merkle_path,
};
res.push(note);
Expand Down Expand Up @@ -538,14 +547,19 @@ pub fn select_notes_by_id(conn: &mut Connection, note_ids: &[NoteId]) -> Result<
let details_data = row.get_ref(8)?.as_blob_or_null()?;
let details = details_data.map(<Vec<u8>>::read_from_bytes).transpose()?;

let note_type = row.get::<_, u8>(4)?.try_into()?;
let sender = column_value_as_u64(row, 5)?;
let tag: u32 = row.get(6)?;

let metadata =
NoteMetadata::new(sender.try_into()?, note_type, tag.into(), Default::default())?;
igamigo marked this conversation as resolved.
Show resolved Hide resolved

notes.push(NoteRecord {
block_num: row.get(0)?,
note_index: BlockNoteIndex::new(row.get(1)?, row.get(2)?),
details,
note_id: note_id.into(),
note_type: row.get::<_, u8>(4)?.try_into()?,
sender: column_value_as_u64(row, 5)?,
tag: row.get(6)?,
metadata,
merkle_path,
})
}
Expand Down
19 changes: 10 additions & 9 deletions crates/store/src/db/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,9 +139,13 @@ fn test_sql_select_notes() {
block_num,
note_index: BlockNoteIndex::new(0, i as usize),
note_id: num_to_rpo_digest(i as u64),
note_type: NoteType::Public,
sender: i as u64,
tag: i,
metadata: NoteMetadata::new(
ACCOUNT_ID_OFF_CHAIN_SENDER.try_into().unwrap(),
NoteType::Public,
i.into(),
Default::default(),
)
.unwrap(),
details: Some(vec![1, 2, 3]),
merkle_path: MerklePath::new(vec![]),
};
Expand Down Expand Up @@ -601,9 +605,8 @@ fn test_notes() {
block_num: block_num_1,
note_index,
note_id,
note_type: NoteType::Public,
sender: sender.into(),
tag,
metadata: NoteMetadata::new(sender, NoteType::Public, tag.into(), Default::default())
.unwrap(),
details,
merkle_path: merkle_path.clone(),
};
Expand Down Expand Up @@ -635,9 +638,7 @@ fn test_notes() {
block_num: block_num_2,
note_index: note.note_index,
note_id: num_to_rpo_digest(3),
note_type: NoteType::OffChain,
sender: note.sender,
tag: note.tag,
metadata: note.metadata,
details: None,
merkle_path,
};
Expand Down
4 changes: 1 addition & 3 deletions crates/store/src/server/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,10 +142,8 @@ impl api_server::Api for StoreApi {
.into_iter()
.map(|note| NoteSyncRecord {
note_index: note.note_index.to_absolute_index() as u32,
note_type: note.note_type as u32,
note_id: Some(note.note_id.into()),
sender: Some(note.sender.into()),
tag: note.tag,
metadata: Some(note.metadata.into()),
merkle_path: Some(note.merkle_path.into()),
})
.collect();
Expand Down
4 changes: 1 addition & 3 deletions crates/store/src/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -211,9 +211,7 @@ impl State {
block_num: block.header().block_num(),
note_index,
note_id: note.id().into(),
note_type: note.metadata().note_type(),
sender: note.metadata().sender().into(),
tag: note.metadata().tag().into(),
metadata: *note.metadata(),
details,
merkle_path,
})
Expand Down
Loading