Refactored NoteOrigin and NoteInclusionProof structs
#810
Conversation
# Conflicts: # CHANGELOG.md
This is only breaking for 0xPolygonMiden/miden-node#421 |
objects/src/notes/location.rs
Outdated
|
|
||
| impl NoteInclusionProof { | ||
| /// Returns a new [NoteInclusionProof]. | ||
| pub fn new(block_num: u32, index: u64, note_path: MerklePath) -> Result<Self, NoteError> { |
There was a problem hiding this comment.
Should index be u32 instead of u64? I see that BlockNoteIndex::to_absolute_index() returns u64 as well but u32 should be enough there as well I think.
There was a problem hiding this comment.
Now maximum note index can be 2^12 * 2^8 - 1 = 2^20 - 1 so it fits to u32. Actually we're trying to reduce this index even to u16 by reducing tree depth. Anyway, this is good catch, I will try to make to_absolute_index return u32 as well.
…:leaf_index` return `u32`
polydez
force-pushed
the
polydez-incl-proofs-simplyfication
branch
from
a912557 to
47007c7
Compare
objects/src/notes/location.rs
Outdated
| /// Returns the index of the note in the note Merkle tree of the block the note was created in. | ||
| pub fn node_index(&self) -> u64 { | ||
| self.node_index_in_block as u64 | ||
| } |
There was a problem hiding this comment.
I would add to the doc comment a bit more info about the returned value. Specifically, we should say what the height of the tree is and what the maximum value is.
Once we reduce the height of the tree to 16, this will be less ambiguous as we can simply return u16 or LeafIndex<16> (this will be even cleaner, I think).
objects/src/constants.rs
Outdated
| /// The depth of the Merkle tree used to commit to notes produced in a block. | ||
| pub const NOTE_TREE_DEPTH: u8 = 20; | ||
| pub const OUTPUT_NOTE_TREE_DEPTH: u8 = 20; |
There was a problem hiding this comment.
I'm not sure we need to do this (and other related) renaming. The note tree in a block contains just notes - these are output notes at some point and input notes at another point. Also, there isn't really a concept of an "input note tree" - so, just "note tree" is unambiguous, I think.
Resolves: #781
Here I renamed
NoteOrigintoNoteLocation, simplified node index field tou32(haven't succeed with note trees depth reduction, will try these reductions next). Removed block's sub-hash and note root fromNoteInclusionProof.