Skip to content

refactor: Change way of providing code and inputs to executor #1229

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

Merged
merged 36 commits into from
Apr 22, 2025
Merged
Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
8c59e66
refactor: Change way of providing code and inputs to executor
igamigo Mar 19, 2025
0a576c0
test: Fix 3 tests
igamigo Mar 19, 2025
f0888df
chore: Merge next
igamigo Mar 19, 2025
4052596
chore: Merge next
igamigo Mar 31, 2025
017d8fd
Checkpoint
igamigo Apr 2, 2025
7df20d7
feat: Add ForeignTransactionInputs
igamigo Apr 7, 2025
1edbfe7
chore: Merge next
igamigo Apr 7, 2025
0ac3111
chore: Remove TODO
igamigo Apr 7, 2025
d88556f
chore: CHANGELOG
igamigo Apr 7, 2025
327fc3f
reviews: Address comment
igamigo Apr 10, 2025
4040442
Revert change
igamigo Apr 10, 2025
fc60697
refactor: Remove unused DataStore error variant
igamigo Apr 10, 2025
a34b940
chore: Clippy
igamigo Apr 10, 2025
2f7e499
refactor: TransactionArgs mutators
igamigo Apr 11, 2025
20c9d25
chore: Clippy
igamigo Apr 11, 2025
32a5296
refactor: MAST store
igamigo Apr 11, 2025
5965b5c
refactor: FPI inputs
igamigo Apr 11, 2025
0a680cf
reviews: propagate errors, add validations and tests
igamigo Apr 14, 2025
75e1fdc
test: Fix assembles
igamigo Apr 14, 2025
96c28ef
reviews: Move mast_store.rs
igamigo Apr 14, 2025
eee32d8
Merge branch 'next' into igamigo-refactor-executor
igamigo Apr 14, 2025
853ca80
fix: Remove accidentall std addition
igamigo Apr 14, 2025
26ae3a5
chore: Merge
igamigo Apr 14, 2025
124d03f
chore: Lints
igamigo Apr 14, 2025
e7a88e2
test: Fix doc test build
igamigo Apr 14, 2025
beb71f1
reviews: Change name, add execute_code variant and make function private
igamigo Apr 15, 2025
b4b2e1c
reviews: Box string in error
igamigo Apr 15, 2025
cd15d1f
reviews: Typo
igamigo Apr 15, 2025
9e3b4e9
chore: Merge next
igamigo Apr 21, 2025
635cf7f
chore: Merge conflicts
igamigo Apr 21, 2025
7b959d1
tests: refactor test to work around the new validations
igamigo Apr 21, 2025
bf97884
feat: Add is_valid()
igamigo Apr 21, 2025
f352972
chore: Clippy
igamigo Apr 21, 2025
5757967
style: CHANGELOG extra newline
igamigo Apr 21, 2025
6503c8b
chore: Renames and imports
igamigo Apr 21, 2025
65ea187
reviews: Remove unnecessary (crate) and add note
igamigo Apr 22, 2025
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 @@ -7,6 +7,7 @@
- Added getter for proof security level in `ProvenBatch` and `ProvenBlock` (#1259).
- [BREAKING] Replaced the `ProvenBatch::new_unchecked` with the `ProvenBatch::new` method to initialize the struct with validations (#1260).
- Added a retry strategy for worker's health check (#1255).
- [BREAKING] Refactored how foreign account inputs are passed to `TransactionExecutor`, and upgraded Rust version to 1.86 (#1229).

## 0.8.1 (2025-03-26) - `miden-objects` and `miden-tx` crates only.

Expand Down
14 changes: 7 additions & 7 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ members = [

[workspace.package]
edition = "2024"
rust-version = "1.85"
rust-version = "1.86"
license = "MIT"
authors = ["Miden contributors"]
homepage = "https://polygon.technology/polygon-miden"
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
[![LICENSE](https://img.shields.io/badge/license-MIT-blue.svg)](https://github.com/0xPolygonMiden/miden-base/blob/main/LICENSE)
[![test](https://github.com/0xPolygonMiden/miden-base/actions/workflows/test.yml/badge.svg)](https://github.com/0xPolygonMiden/miden-base/actions/workflows/test.yml)
[![build](https://github.com/0xPolygonMiden/miden-base/actions/workflows/build.yml/badge.svg)](https://github.com/0xPolygonMiden/miden-base/actions/workflows/build.yml)
[![RUST_VERSION](https://img.shields.io/badge/rustc-1.85+-lightgray.svg)](https://www.rust-lang.org/tools/install)
[![RUST_VERSION](https://img.shields.io/badge/rustc-1.86+-lightgray.svg)](https://www.rust-lang.org/tools/install)
[![GitHub Release](https://img.shields.io/github/release/0xPolygonMiden/miden-base)](https://github.com/0xPolygonMiden/miden-base/releases/)

Description and core structures for the Miden Rollup protocol.
Expand Down
31 changes: 11 additions & 20 deletions bin/bench-tx/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use std::{
fs::{File, read_to_string, write},
io::Write,
path::Path,
sync::Arc,
};

use miden_lib::{note::create_p2id_note, transaction::TransactionKernel};
Expand Down Expand Up @@ -67,17 +68,12 @@ pub fn benchmark_default_tx() -> Result<TransactionMeasurements, String> {
let account_id = tx_context.account().id();

let block_ref = tx_context.tx_inputs().block_header().block_num();
let note_ids = tx_context
.tx_inputs()
.input_notes()
.iter()
.map(|note| note.id())
.collect::<Vec<_>>();

let tx_args = tx_context.tx_args().clone();
let notes = tx_context.tx_inputs().input_notes().clone();
let executor: TransactionExecutor =
TransactionExecutor::new(tx_context.get_data_store(), None).with_tracing();
TransactionExecutor::new(Arc::new(tx_context), None).with_tracing();
let executed_transaction = executor
.execute_transaction(account_id, block_ref, &note_ids, tx_context.tx_args().clone())
.execute_transaction(account_id, block_ref, notes, tx_args)
.map_err(|e| e.to_string())?;

Ok(executed_transaction.into())
Expand Down Expand Up @@ -116,26 +112,21 @@ pub fn benchmark_p2id() -> Result<TransactionMeasurements, String> {
let tx_context = TransactionContextBuilder::new(target_account.clone())
.input_notes(vec![note.clone()])
.build();
let block_ref = tx_context.tx_inputs().block_header().block_num();

let executor = TransactionExecutor::new(tx_context.get_data_store(), Some(falcon_auth.clone()))
.with_tracing();
let notes = tx_context.tx_inputs().input_notes().clone();

let block_ref = tx_context.tx_inputs().block_header().block_num();
let note_ids = tx_context
.tx_inputs()
.input_notes()
.iter()
.map(|note| note.id())
.collect::<Vec<_>>();
let executor =
TransactionExecutor::new(Arc::new(tx_context), Some(falcon_auth.clone())).with_tracing();

let tx_script_target =
TransactionScript::compile(DEFAULT_AUTH_SCRIPT, [], TransactionKernel::assembler())
.unwrap();
let tx_args_target = TransactionArgs::with_tx_script(tx_script_target);
let tx_args_target = TransactionArgs::default().with_tx_script(tx_script_target);

// execute transaction
let executed_transaction = executor
.execute_transaction(target_account.id(), block_ref, &note_ids, tx_args_target)
.execute_transaction(target_account.id(), block_ref, notes, tx_args_target)
.unwrap();

Ok(executed_transaction.into())
Expand Down
23 changes: 17 additions & 6 deletions crates/miden-lib/src/transaction/inputs.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use alloc::vec::Vec;

use miden_objects::{
Digest, EMPTY_WORD, Felt, FieldElement, WORD_SIZE, Word, ZERO,
Digest, EMPTY_WORD, Felt, FieldElement, TransactionInputError, WORD_SIZE, Word, ZERO,
account::{Account, StorageSlot},
transaction::{ChainMmr, InputNote, TransactionArgs, TransactionInputs, TransactionScript},
vm::AdviceInputs,
Expand All @@ -22,7 +22,7 @@ pub(super) fn extend_advice_inputs(
tx_inputs: &TransactionInputs,
tx_args: &TransactionArgs,
advice_inputs: &mut AdviceInputs,
) {
) -> Result<(), TransactionInputError> {
// TODO: remove this value and use a user input instead
let kernel_version = 0;

Expand All @@ -32,8 +32,13 @@ pub(super) fn extend_advice_inputs(
add_kernel_commitments_to_advice_inputs(advice_inputs, kernel_version);
add_chain_mmr_to_advice_inputs(tx_inputs.block_chain(), advice_inputs);
add_account_to_advice_inputs(tx_inputs.account(), tx_inputs.account_seed(), advice_inputs);
add_input_notes_to_advice_inputs(tx_inputs, tx_args, advice_inputs);
add_input_notes_to_advice_inputs(tx_inputs, tx_args, advice_inputs)?;
for foreign_account in tx_args.foreign_accounts() {
TransactionKernel::extend_advice_inputs_for_account(advice_inputs, foreign_account)?;
}

advice_inputs.extend(tx_args.advice_inputs().clone());
Ok(())
}

// ADVICE STACK BUILDER
Expand Down Expand Up @@ -223,10 +228,10 @@ fn add_input_notes_to_advice_inputs(
tx_inputs: &TransactionInputs,
tx_args: &TransactionArgs,
inputs: &mut AdviceInputs,
) {
) -> Result<(), TransactionInputError> {
// if there are no input notes, nothing is added to the advice inputs
if tx_inputs.input_notes().is_empty() {
return;
return Ok(());
}

let mut note_data = Vec::new();
Expand Down Expand Up @@ -283,7 +288,12 @@ fn add_input_notes_to_advice_inputs(
proof.location().node_index_in_block().into(),
note.commitment(),
)
.unwrap(),
.map_err(|err| {
TransactionInputError::InvalidMerklePath(
format!("input note ID {}", note.id()),
err,
)
})?,
);
note_data.push(proof.location().block_num().into());
note_data.extend(note_block_header.sub_commitment());
Expand All @@ -300,6 +310,7 @@ fn add_input_notes_to_advice_inputs(

// NOTE: keep map in sync with the `prologue::process_input_notes_data` kernel procedure
inputs.extend_map([(tx_inputs.input_notes().commitment(), note_data)]);
Ok(())
}

// KERNEL COMMITMENTS INJECTOR
Expand Down
55 changes: 42 additions & 13 deletions crates/miden-lib/src/transaction/mod.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
use alloc::{string::ToString, sync::Arc, vec::Vec};

use miden_objects::{
Digest, EMPTY_WORD, Felt, TransactionOutputError, ZERO,
account::{AccountCode, AccountHeader, AccountId, AccountStorageHeader},
Digest, EMPTY_WORD, Felt, TransactionInputError, TransactionOutputError, ZERO,
account::{AccountCode, AccountId},
assembly::{Assembler, DefaultSourceManager, KernelLibrary},
block::BlockNumber,
crypto::merkle::{MerkleError, MerklePath},
transaction::{
OutputNote, OutputNotes, TransactionArgs, TransactionInputs, TransactionOutputs,
ForeignAccountInputs, OutputNote, OutputNotes, TransactionArgs, TransactionInputs,
TransactionOutputs,
},
utils::{serde::Deserializable, sync::LazyLock},
vm::{AdviceInputs, AdviceMap, Program, ProgramInfo, StackInputs, StackOutputs},
Expand Down Expand Up @@ -114,7 +114,7 @@ impl TransactionKernel {
tx_inputs: &TransactionInputs,
tx_args: &TransactionArgs,
init_advice_inputs: Option<AdviceInputs>,
) -> (StackInputs, AdviceInputs) {
) -> Result<(StackInputs, AdviceInputs), TransactionInputError> {
let account = tx_inputs.account();

let stack_inputs = TransactionKernel::build_input_stack(
Expand All @@ -126,9 +126,9 @@ impl TransactionKernel {
);

let mut advice_inputs = init_advice_inputs.unwrap_or_default();
inputs::extend_advice_inputs(tx_inputs, tx_args, &mut advice_inputs);
inputs::extend_advice_inputs(tx_inputs, tx_args, &mut advice_inputs)?;

(stack_inputs, advice_inputs)
Ok((stack_inputs, advice_inputs))
}

// ASSEMBLER CONSTRUCTOR
Expand Down Expand Up @@ -199,11 +199,14 @@ impl TransactionKernel {
/// account.
pub fn extend_advice_inputs_for_account(
advice_inputs: &mut AdviceInputs,
account_header: &AccountHeader,
account_code: &AccountCode,
storage_header: &AccountStorageHeader,
merkle_path: &MerklePath,
) -> Result<(), MerkleError> {
foreign_account_inputs: &ForeignAccountInputs,
) -> Result<(), TransactionInputError> {
let account_header = foreign_account_inputs.account_header();
let storage_header = foreign_account_inputs.storage_header();
let account_code = foreign_account_inputs.account_code();
let merkle_path = foreign_account_inputs.account_witness();
let storage_proofs = foreign_account_inputs.storage_map_proofs();

let account_id = account_header.id();
let storage_root = account_header.storage_commitment();
let code_root = account_header.code_commitment();
Expand All @@ -224,9 +227,35 @@ impl TransactionKernel {
// Extend the advice inputs with Merkle store data
advice_inputs.extend_merkle_store(
// The prefix is the index in the account tree.
merkle_path.inner_nodes(account_id.prefix().as_u64(), account_header.commitment())?,
merkle_path
.inner_nodes(account_id.prefix().as_u64(), account_header.commitment())
.map_err(|err| {
TransactionInputError::InvalidMerklePath(
format!("foreign account ID {}", account_id),
err,
)
})?,
);

// Load merkle nodes for storage maps
for proof in storage_proofs {
// Extend the merkle store and map with the storage maps
advice_inputs.extend_merkle_store(
proof
.path()
.inner_nodes(proof.leaf().index().value(), proof.leaf().hash())
.map_err(|err| {
TransactionInputError::InvalidMerklePath(
format!("foreign account ID {} storage proof", account_id),
err,
)
})?,
);
// Populate advice map with Sparse Merkle Tree leaf nodes
advice_inputs
.extend_map(core::iter::once((proof.leaf().hash(), proof.leaf().to_elements())));
}

Ok(())
}

Expand Down
6 changes: 5 additions & 1 deletion crates/miden-objects/src/batch/account_update.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use alloc::boxed::Box;

use crate::{
Digest,
account::{AccountId, delta::AccountUpdateDetails},
Expand Down Expand Up @@ -106,7 +108,9 @@ impl BatchAccountUpdate {
}

self.details = self.details.clone().merge(tx.account_update().details().clone()).map_err(
|source_err| BatchAccountUpdateError::TransactionUpdateMergeError(tx.id(), source_err),
|source_err| {
BatchAccountUpdateError::TransactionUpdateMergeError(tx.id(), Box::new(source_err))
},
)?;
self.final_state_commitment = tx.account_update().final_state_commitment();

Expand Down
3 changes: 2 additions & 1 deletion crates/miden-objects/src/block/proposed_block.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use alloc::{
boxed::Box,
collections::{BTreeMap, BTreeSet},
vec::Vec,
};
Expand Down Expand Up @@ -674,7 +675,7 @@ impl AccountUpdateAggregator {
details = Some(match details {
None => update_details,
Some(details) => details.merge(update_details).map_err(|source| {
ProposedBlockError::AccountUpdateError { account_id, source }
ProposedBlockError::AccountUpdateError { account_id, source: Box::new(source) }
})?,
});
}
Expand Down
6 changes: 4 additions & 2 deletions crates/miden-objects/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ pub enum BatchAccountUpdateError {
)]
AccountUpdateInitialStateMismatch(TransactionId),
#[error("failed to merge account delta from transaction {0}")]
TransactionUpdateMergeError(TransactionId, #[source] AccountDeltaError),
TransactionUpdateMergeError(TransactionId, #[source] Box<AccountDeltaError>),
}

// ASSET ERROR
Expand Down Expand Up @@ -450,6 +450,8 @@ pub enum TransactionInputError {
InputNoteNotInBlock(NoteId, BlockNumber),
#[error("account ID computed from seed is invalid")]
InvalidAccountIdSeed(#[source] AccountIdError),
#[error("merkle path for {0} is invalid")]
InvalidMerklePath(String, #[source] MerkleError),
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
InvalidMerklePath(String, #[source] MerkleError),
InvalidMerklePath(Box<str>, #[source] MerkleError),

Nit: We could box the string to save 8 bytes. This is the largest variant in the enum now and dictates its stack size. Just requires adding an .into() after the format! where it is used.

#[error(
"total number of input notes is {0} which exceeds the maximum of {MAX_INPUT_NOTES_PER_TX}"
)]
Expand Down Expand Up @@ -779,7 +781,7 @@ pub enum ProposedBlockError {
#[error("failed to merge transaction delta into account {account_id}")]
AccountUpdateError {
account_id: AccountId,
source: AccountDeltaError,
source: Box<AccountDeltaError>,
},
}

Expand Down
5 changes: 3 additions & 2 deletions crates/miden-objects/src/note/note_tag.rs
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,7 @@ mod tests {
ACCOUNT_ID_REGULAR_PUBLIC_ACCOUNT_IMMUTABLE_CODE,
ACCOUNT_ID_REGULAR_PUBLIC_ACCOUNT_IMMUTABLE_CODE_2,
ACCOUNT_ID_REGULAR_PUBLIC_ACCOUNT_UPDATABLE_CODE,
ACCOUNT_ID_REGULAR_PUBLIC_ACCOUNT_UPDATABLE_CODE_ON_2, ACCOUNT_ID_SENDER,
ACCOUNT_ID_REGULAR_PUBLIC_ACCOUNT_UPDATABLE_CODE_ON_CHAIN_2, ACCOUNT_ID_SENDER,
},
};

Expand All @@ -342,7 +342,8 @@ mod tests {
AccountId::try_from(ACCOUNT_ID_REGULAR_PUBLIC_ACCOUNT_IMMUTABLE_CODE).unwrap(),
AccountId::try_from(ACCOUNT_ID_REGULAR_PUBLIC_ACCOUNT_IMMUTABLE_CODE_2).unwrap(),
AccountId::try_from(ACCOUNT_ID_REGULAR_PUBLIC_ACCOUNT_UPDATABLE_CODE).unwrap(),
AccountId::try_from(ACCOUNT_ID_REGULAR_PUBLIC_ACCOUNT_UPDATABLE_CODE_ON_2).unwrap(),
AccountId::try_from(ACCOUNT_ID_REGULAR_PUBLIC_ACCOUNT_UPDATABLE_CODE_ON_CHAIN_2)
.unwrap(),
AccountId::try_from(ACCOUNT_ID_PUBLIC_FUNGIBLE_FAUCET).unwrap(),
AccountId::try_from(ACCOUNT_ID_PUBLIC_FUNGIBLE_FAUCET_1).unwrap(),
AccountId::try_from(ACCOUNT_ID_PUBLIC_FUNGIBLE_FAUCET_2).unwrap(),
Expand Down
2 changes: 1 addition & 1 deletion crates/miden-objects/src/testing/account_id.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ pub const ACCOUNT_ID_REGULAR_PUBLIC_ACCOUNT_UPDATABLE_CODE: u128 = account_id(
AccountStorageMode::Public,
0xacdd_eefc,
);
pub const ACCOUNT_ID_REGULAR_PUBLIC_ACCOUNT_UPDATABLE_CODE_ON_2: u128 = account_id(
pub const ACCOUNT_ID_REGULAR_PUBLIC_ACCOUNT_UPDATABLE_CODE_ON_CHAIN_2: u128 = account_id(
AccountType::RegularAccountUpdatableCode,
AccountStorageMode::Public,
0xeeff_ccdd,
Expand Down
Loading