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: added BlockNumber struct #1043

Merged
merged 12 commits into from
Jan 16, 2025
Merged
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
- Introduced `AccountComponentTemplate` with TOML serialization and templating (#1015, #1027).
- Introduce `AccountIdError` and make account ID byte representations (`u128`, `[u8; 15]`) consistent (#1055).
- Refactor `AccountId` and `AccountIdPrefix` into version wrappers (#1058).
- Add `BlockNumber` struct (#1043).

## 0.6.2 (2024-11-20)

Expand Down
4 changes: 2 additions & 2 deletions miden-lib/src/transaction/inputs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ fn build_advice_stack(
inputs.extend_stack(header.kernel_root());
inputs.extend_stack(header.proof_hash());
inputs.extend_stack([
header.block_num().into(),
header.block_num().as_u32().into(),
PhilippGackstatter marked this conversation as resolved.
Show resolved Hide resolved
header.version().into(),
header.timestamp().into(),
ZERO,
Expand Down Expand Up @@ -282,7 +282,7 @@ fn add_input_notes_to_advice_inputs(
.inner_nodes(proof.location().node_index_in_block().into(), note.hash())
.unwrap(),
);
note_data.push(proof.location().block_num().into());
note_data.push(proof.location().block_num().as_u32().into());
note_data.extend(note_block_header.sub_hash());
note_data.extend(note_block_header.note_root());
note_data.push(proof.location().node_index_in_block().into());
Expand Down
6 changes: 4 additions & 2 deletions miden-tx/src/executor/data_store.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
#[cfg(feature = "async")]
use alloc::boxed::Box;

use miden_objects::{accounts::AccountId, notes::NoteId, transaction::TransactionInputs};
use miden_objects::{
accounts::AccountId, block::BlockNumber, notes::NoteId, transaction::TransactionInputs,
};
use winter_maybe_async::*;

use crate::DataStoreError;
Expand Down Expand Up @@ -32,7 +34,7 @@ pub trait DataStore {
fn get_transaction_inputs(
&self,
account_id: AccountId,
block_ref: u32,
block_ref: BlockNumber,
notes: &[NoteId],
) -> Result<TransactionInputs, DataStoreError>;
}
3 changes: 2 additions & 1 deletion miden-tx/src/executor/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use miden_lib::transaction::TransactionKernel;
use miden_objects::{
accounts::{AccountCode, AccountId},
assembly::Library,
block::BlockNumber,
notes::NoteId,
transaction::{ExecutedTransaction, TransactionArgs, TransactionInputs},
vm::StackOutputs,
Expand Down Expand Up @@ -128,7 +129,7 @@ impl TransactionExecutor {
pub fn execute_transaction(
&self,
account_id: AccountId,
block_ref: u32,
block_ref: BlockNumber,
notes: &[NoteId],
tx_args: TransactionArgs,
) -> Result<ExecutedTransaction, TransactionExecutorError> {
Expand Down
20 changes: 10 additions & 10 deletions miden-tx/src/testing/mock_chain/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ use miden_objects::{
},
assets::{Asset, FungibleAsset, TokenSymbol},
block::{
block_num_from_epoch, compute_tx_hash, Block, BlockAccountUpdate, BlockNoteIndex,
BlockNoteTree, NoteBatch,
compute_tx_hash, Block, BlockAccountUpdate, BlockNoteIndex, BlockNoteTree, BlockNumber,
NoteBatch,
},
crypto::{
dsa::rpo_falcon512::SecretKey,
Expand Down Expand Up @@ -587,8 +587,8 @@ impl MockChain {
let note_block_num = input_note.location().unwrap().block_num();
if note_block_num != block.header().block_num() {
block_headers_map.insert(
note_block_num,
self.blocks.get(note_block_num as usize).unwrap().header(),
note_block_num.as_u32(),
self.blocks.get(note_block_num.as_u32() as usize).unwrap().header(),
);
}
input_notes.push(input_note);
Expand All @@ -597,13 +597,13 @@ impl MockChain {
// If the account is new, add the anchor block's header from which the account ID is derived
// to the MMR.
if account.is_new() {
let epoch_block_num = block_num_from_epoch(account.id().anchor_epoch());
let epoch_block_num = BlockNumber::from_epoch(account.id().anchor_epoch());
// The reference block of the transaction is added to the MMR in
// prologue::process_chain_data so we can skip adding it to the block headers here.
if epoch_block_num != block.header().block_num() {
block_headers_map.insert(
epoch_block_num,
self.blocks.get(epoch_block_num as usize).unwrap().header(),
epoch_block_num.as_u32(),
self.blocks.get(epoch_block_num.as_u32() as usize).unwrap().header(),
);
}
}
Expand Down Expand Up @@ -632,7 +632,7 @@ impl MockChain {
/// This will also make all the objects currently pending available for use.
/// If `block_num` is `Some(number)`, blocks will be generated up to `number`.
pub fn seal_block(&mut self, block_num: Option<u32>) -> Block {
let next_block_num = self.blocks.last().map_or(0, |b| b.header().block_num() + 1);
let next_block_num = self.blocks.last().map_or(0, |b| b.header().block_num().as_u32() + 1);
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd recommend adding a BlockNumber::next or similar method to increment by one.

let target_block_num = block_num.unwrap_or(next_block_num);

if target_block_num < next_block_num {
Expand Down Expand Up @@ -690,7 +690,7 @@ impl MockChain {
let header = BlockHeader::new(
version,
prev_hash,
current_block_num,
BlockNumber::from(current_block_num),
chain_root,
account_root,
nullifier_root,
Expand Down Expand Up @@ -719,7 +719,7 @@ impl MockChain {
BlockNoteIndex::new(batch_index, note_index).unwrap();
let note_path = notes_tree.get_note_path(block_note_index);
let note_inclusion_proof = NoteInclusionProof::new(
block.header().block_num(),
block.header().block_num().as_u32(),
block_note_index.leaf_index_value(),
note_path,
)
Expand Down
3 changes: 2 additions & 1 deletion miden-tx/src/testing/tx_context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use miden_lib::transaction::TransactionKernel;
use miden_objects::{
accounts::{Account, AccountCode, AccountId},
assembly::Assembler,
block::BlockNumber,
notes::{Note, NoteId},
transaction::{ExecutedTransaction, InputNote, InputNotes, TransactionArgs, TransactionInputs},
};
Expand Down Expand Up @@ -139,7 +140,7 @@ impl DataStore for TransactionInputs {
fn get_transaction_inputs(
&self,
account_id: AccountId,
block_num: u32,
block_num: BlockNumber,
notes: &[NoteId],
) -> Result<TransactionInputs, DataStoreError> {
assert_eq!(account_id, self.account().id());
Expand Down
3 changes: 2 additions & 1 deletion miden-tx/src/tests/kernel_tests/test_epilogue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,8 @@ fn test_block_expiration_height_monotonically_decreases() {

// Expiry block should be set to transaction's block + the stored expiration delta
// (which can only decrease, not increase)
let expected_expiry = v1.min(v2) + tx_context.tx_inputs().block_header().block_num() as u64;
let expected_expiry =
v1.min(v2) + tx_context.tx_inputs().block_header().block_num().as_u32() as u64;
assert_eq!(process.get_stack_item(8).as_int(), expected_expiry);
}
}
Expand Down
2 changes: 1 addition & 1 deletion miden-tx/src/tests/kernel_tests/test_prologue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ fn block_data_memory_assertions(process: &Process<MockHost>, inputs: &Transactio

assert_eq!(
read_root_mem_value(process, BLOCK_METADATA_PTR)[BLOCK_NUMBER_IDX],
inputs.tx_inputs().block_header().block_num().into(),
inputs.tx_inputs().block_header().block_num().as_u32().into(),
"The block number should be stored at BLOCK_METADATA_PTR[BLOCK_NUMBER_IDX]"
);

Expand Down
14 changes: 6 additions & 8 deletions objects/src/accounts/account_id/id_anchor.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
use crate::{
block::block_epoch_from_number, errors::AccountIdError, BlockHeader, Digest, EMPTY_WORD,
};
use crate::{block::BlockNumber, errors::AccountIdError, BlockHeader, Digest, EMPTY_WORD};

// ACCOUNT ID ANCHOR
// ================================================================================================
Expand All @@ -14,8 +12,8 @@ use crate::{
/// # Constraints
///
/// This type enforces the following constraints.
/// - The `anchor_block_number` % 2^[`BlockHeader::EPOCH_LENGTH_EXPONENT`] must be zero. In other
/// words, the block number must a multiple of 2^[`BlockHeader::EPOCH_LENGTH_EXPONENT`].
/// - The `anchor_block_number` % 2^[`BlockNumber::EPOCH_LENGTH_EXPONENT`] must be zero. In other
/// words, the block number must a multiple of 2^[`BlockNumber::EPOCH_LENGTH_EXPONENT`].
/// - The epoch derived from the `anchor_block_number` must be strictly less than [`u16::MAX`].
#[derive(Debug, Clone, Copy)]
pub struct AccountIdAnchor {
Expand Down Expand Up @@ -51,14 +49,14 @@ impl AccountIdAnchor {
/// Returns an error if any of the anchor constraints are not met. See the [type
/// documentation](AccountIdAnchor) for details.
pub fn new(
anchor_block_number: u32,
anchor_block_number: BlockNumber,
anchor_block_hash: Digest,
) -> Result<Self, AccountIdError> {
if anchor_block_number & 0x0000_ffff != 0 {
if anchor_block_number.as_u32() & 0x0000_ffff != 0 {
return Err(AccountIdError::AnchorBlockMustBeEpochBlock);
}

let anchor_epoch = block_epoch_from_number(anchor_block_number);
let anchor_epoch = anchor_block_number.block_epoch();

if anchor_epoch == u16::MAX {
return Err(AccountIdError::AnchorEpochMustNotBeU16Max);
Expand Down
4 changes: 2 additions & 2 deletions objects/src/accounts/account_id/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ use crate::{errors::AccountIdError, AccountError, ACCOUNT_TREE_DEPTH};
/// as the [`NoteMetadata`](crate::notes::NoteMetadata). In such cases, it can happen that all
/// bits of the encoded suffix would be one, so having the epoch constraint is important.
/// - The ID is dependent on the hash of an epoch block. This is a block whose number is a multiple
/// of 2^[`BlockHeader::EPOCH_LENGTH_EXPONENT`][epoch_len_exp], e.g. `0`, `65536`, `131072`, ...
/// of 2^[`BlockNumber::EPOCH_LENGTH_EXPONENT`][epoch_len_exp], e.g. `0`, `65536`, `131072`, ...
/// These are the first blocks of epoch 0, 1, 2, ... We call this dependence _anchoring_ because
/// the ID is anchored to that epoch block's hash. Anchoring makes it practically impossible for
/// an attacker to construct a rainbow table of account IDs whose epoch is X, if the block for
Expand All @@ -108,7 +108,7 @@ use crate::{errors::AccountIdError, AccountError, ACCOUNT_TREE_DEPTH};
/// hashes to the user's ID can claim the assets sent to the user's ID. Adding the anchor
/// block hash to ID generation process makes this attack practically impossible.
///
/// [epoch_len_exp]: crate::block::BlockHeader::EPOCH_LENGTH_EXPONENT
/// [epoch_len_exp]: crate::block::BlockNumber::EPOCH_LENGTH_EXPONENT
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum AccountId {
V0(AccountIdV0),
Expand Down
6 changes: 4 additions & 2 deletions objects/src/accounts/builder/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ mod tests {
use vm_core::FieldElement;

use super::*;
use crate::accounts::StorageSlot;
use crate::{accounts::StorageSlot, block::BlockNumber};

const CUSTOM_CODE1: &str = "
export.foo
Expand Down Expand Up @@ -317,7 +317,9 @@ mod tests {

let anchor_block_hash = Digest::new([Felt::new(42); 4]);
let anchor_block_number = 1 << 16;
let id_anchor = AccountIdAnchor::new(anchor_block_number, anchor_block_hash).unwrap();
let id_anchor =
AccountIdAnchor::new(BlockNumber::from(anchor_block_number), anchor_block_hash)
.unwrap();

let (account, seed) = Account::builder([5; 32])
.anchor(id_anchor)
Expand Down
87 changes: 64 additions & 23 deletions objects/src/block/header.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use alloc::vec::Vec;
use core::fmt;

use super::{Digest, Felt, Hasher, ZERO};
use crate::utils::serde::{
Expand Down Expand Up @@ -29,7 +30,7 @@ use crate::utils::serde::{
pub struct BlockHeader {
version: u32,
prev_hash: Digest,
block_num: u32,
block_num: BlockNumber,
chain_root: Digest,
account_root: Digest,
nullifier_root: Digest,
Expand All @@ -43,19 +44,12 @@ pub struct BlockHeader {
}

impl BlockHeader {
/// The length of an epoch expressed as a power of two. `2^(EPOCH_LENGTH_EXPONENT)` is the
/// number of blocks in an epoch.
///
/// The epoch of a block can be obtained by shifting the block number to the right by this
/// exponent.
pub const EPOCH_LENGTH_EXPONENT: u8 = 16;

/// Creates a new block header.
#[allow(clippy::too_many_arguments)]
pub fn new(
version: u32,
prev_hash: Digest,
block_num: u32,
block_num: BlockNumber,
chain_root: Digest,
account_root: Digest,
nullifier_root: Digest,
Expand All @@ -76,7 +70,7 @@ impl BlockHeader {
kernel_root,
proof_hash,
timestamp,
block_num,
block_num.as_u32(),
);

// The sub hash is merged with the note_root - hash(sub_hash, note_root) to produce the
Expand Down Expand Up @@ -129,15 +123,15 @@ impl BlockHeader {
}

/// Returns the block number.
pub fn block_num(&self) -> u32 {
pub fn block_num(&self) -> BlockNumber {
self.block_num
}

/// Returns the epoch to which this block belongs.
///
/// This is the block number shifted right by [`Self::EPOCH_LENGTH_EXPONENT`].
/// This is the block number shifted right by [`BlockNumber::EPOCH_LENGTH_EXPONENT`].
pub fn block_epoch(&self) -> u16 {
block_epoch_from_number(self.block_num)
self.block_num.block_epoch()
}

/// Returns the chain root.
Expand Down Expand Up @@ -187,8 +181,8 @@ impl BlockHeader {
}

/// Returns the block number of the epoch block to which this block belongs.
pub fn epoch_block_num(&self) -> u32 {
block_num_from_epoch(self.block_epoch())
pub fn epoch_block_num(&self) -> BlockNumber {
BlockNumber::from_epoch(self.block_epoch())
}

// HELPERS
Expand Down Expand Up @@ -275,17 +269,64 @@ impl Deserializable for BlockHeader {
}
}

// UTILITIES
// ================================================================================================
/// BLOCK NUMBER
PhilippGackstatter marked this conversation as resolved.
Show resolved Hide resolved

/// A convenience wrapper around a `u32` representing the number of a block.
///
/// Each block has a unique number and block numbers increase monotonically by `1`.
#[derive(Debug, Eq, PartialEq, Copy, Clone, PartialOrd, Ord, Hash)]
pub struct BlockNumber(u32);
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we not derive Default, and possible add some utility functions like next/previous or child/parent?

I'd also appreciate addition and checked subtraction implementations.

Copy link
Author

Choose a reason for hiding this comment

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

Using next or previous results in this warning from clippy:

method `next` can be confused for the standard trait method `std::iter::Iterator::next`

hence going with parent and child


impl Serializable for BlockNumber {
fn write_into<W: ByteWriter>(&self, target: &mut W) {
target.write_u32(self.0);
}

fn get_size_hint(&self) -> usize {
core::mem::size_of::<u32>()
}
}

impl Deserializable for BlockNumber {
fn read_from<R: ByteReader>(source: &mut R) -> Result<Self, DeserializationError> {
source.read::<u32>().map(BlockNumber::from)
}
}

impl From<u32> for BlockNumber {
fn from(value: u32) -> Self {
BlockNumber(value)
}
}

/// Returns the block number of the epoch block for the given `epoch`.
pub const fn block_num_from_epoch(epoch: u16) -> u32 {
(epoch as u32) << BlockHeader::EPOCH_LENGTH_EXPONENT
impl fmt::Display for BlockNumber {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "{}", self.0)
}
}

/// Returns the epoch of the given block number.
pub const fn block_epoch_from_number(block_number: u32) -> u16 {
(block_number >> BlockHeader::EPOCH_LENGTH_EXPONENT) as u16
impl BlockNumber {
/// The length of an epoch expressed as a power of two. `2^(EPOCH_LENGTH_EXPONENT)` is the
/// number of blocks in an epoch.
///
/// The epoch of a block can be obtained by shifting the block number to the right by this
/// exponent.
pub const EPOCH_LENGTH_EXPONENT: u8 = 16;

/// Creates the [`BlockNumber`] corresponding to the epoch block for the provided `epoch`.
pub const fn from_epoch(epoch: u16) -> BlockNumber {
BlockNumber((epoch as u32) << BlockNumber::EPOCH_LENGTH_EXPONENT)
}

/// Returns the epoch to which this block number belongs.
pub const fn block_epoch(&self) -> u16 {
(self.0 >> BlockNumber::EPOCH_LENGTH_EXPONENT) as u16
}

/// Returns the block number as a `u32`.
pub fn as_u32(&self) -> u32 {
self.0
}
}

#[cfg(test)]
Expand Down
2 changes: 1 addition & 1 deletion objects/src/block/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use super::{
};

mod header;
pub use header::{block_epoch_from_number, block_num_from_epoch, BlockHeader};
pub use header::{BlockHeader, BlockNumber};
mod note_tree;
pub use note_tree::{BlockNoteIndex, BlockNoteTree};

Expand Down
Loading
Loading