From be00c009884c62eb106653a17ed3b74a6fb2d2e4 Mon Sep 17 00:00:00 2001 From: Mirko von Leipzig <48352201+Mirko-von-Leipzig@users.noreply.github.com> Date: Fri, 18 Sep 2026 11:45:23 +0200 Subject: [PATCH 1/4] Collect batch fees into a single P2ID note --- bin/benchmark/README.md | 3 + bin/node/src/commands/block_producer.rs | 37 +++++++ bin/node/src/commands/fee_collector.rs | 4 +- bin/node/src/commands/mod.rs | 2 +- bin/node/src/commands/modes.rs | 16 ++- compose/node.yml | 2 + .../block-producer/src/batch_builder/mod.rs | 72 +++++++++++- crates/block-producer/src/domain/batch.rs | 53 ++++++++- crates/block-producer/src/errors.rs | 8 ++ crates/block-producer/src/fee_collector.rs | 13 +++ crates/block-producer/src/mempool/budget.rs | 52 +++++++-- .../block-producer/src/mempool/graph/batch.rs | 104 +++++++++++++++--- .../src/mempool/graph/transaction.rs | 6 +- crates/block-producer/src/mempool/mod.rs | 44 ++++---- crates/block-producer/src/mempool/tests.rs | 59 +++++++--- .../src/mempool/tests/add_transaction.rs | 5 +- .../src/mempool/tests/add_user_batch.rs | 61 +++++++++- crates/block-producer/src/server/mod.rs | 31 ++++-- crates/block-producer/src/server/tests.rs | 29 +++-- crates/block-producer/src/test_utils/batch.rs | 11 ++ crates/block-producer/src/validator/mod.rs | 2 +- .../src/network-operator/sequencer.md | 14 ++- scripts/bench-local.sh | 3 + scripts/run-node.sh | 3 + 24 files changed, 535 insertions(+), 99 deletions(-) diff --git a/bin/benchmark/README.md b/bin/benchmark/README.md index b4cd76f4c6..2dda08182b 100644 --- a/bin/benchmark/README.md +++ b/bin/benchmark/README.md @@ -215,11 +215,14 @@ nohup miden-remote-prover \ > logs/remote-prover.log 2>&1 & # The node runs store + block-producer + RPC in a single sequencer process. +# Send collected fees to an arbitrary non-existent account ID for now. +BATCH_BUILDER_WALLET_ACCOUNT_ID=0xcc0000000000dd010000ee000000ff nohup miden-node sequencer \ --data-directory "$DATA/node" \ --rpc.listen 127.0.0.1:57291 \ --validator.url http://127.0.0.1:50101 \ --ntx-builder.url http://127.0.0.1:50301 \ + --batch.builder.wallet-account-id "$BATCH_BUILDER_WALLET_ACCOUNT_ID" \ --batch.max-txs 1024 \ --block.max-batches 64 \ --block.interval 2s \ diff --git a/bin/node/src/commands/block_producer.rs b/bin/node/src/commands/block_producer.rs index 8993f6765e..752037a349 100644 --- a/bin/node/src/commands/block_producer.rs +++ b/bin/node/src/commands/block_producer.rs @@ -10,6 +10,7 @@ use miden_node_block_producer::{ DEFAULT_MAX_TXS_PER_BATCH, }; use miden_node_utils::clap::duration_to_human_readable_string; +use miden_protocol::account::AccountId; use url::Url; // BLOCK PRODUCTION @@ -17,6 +18,9 @@ use url::Url; #[derive(clap::Args, Clone, Debug)] pub struct BlockProducerOptions { + #[command(flatten)] + pub builder: BuilderOptions, + #[command(flatten)] pub batch: BatchOptions, @@ -50,6 +54,10 @@ impl BlockProducerOptions { ); } + if self.batch.max_txs.get() < 2 { + anyhow::bail!("batch.max-txs must be at least 2 to include the batch fee transaction"); + } + Ok(()) } } @@ -64,6 +72,7 @@ mod tests { BlockOptions, BlockProducerOptions, BlockProverOptions, + BuilderOptions, MempoolOptions, }; use crate::commands::block_producer::{ @@ -74,6 +83,12 @@ mod tests { fn options(max_batches: usize, max_txs: usize) -> BlockProducerOptions { BlockProducerOptions { + builder: BuilderOptions { + wallet_account_id: miden_protocol::account::AccountId::from_hex( + "0xcc0000000000dd010000ee000000ff", + ) + .unwrap(), + }, batch: BatchOptions { interval: DEFAULT_BATCH_INTERVAL, max_txs: NonZeroUsize::new(max_txs).unwrap(), @@ -124,6 +139,28 @@ mod tests { assert!(err.to_string().contains("batch.max-txs")); } + + #[test] + fn rejects_max_txs_without_room_for_a_user_transaction() { + let err = options(miden_protocol::MAX_BATCHES_PER_BLOCK, 1) + .validate() + .expect_err("the batch must include a user transaction"); + + assert!(err.to_string().contains("batch.max-txs")); + } +} + +#[derive(clap::Args, Clone, Debug)] +pub struct BuilderOptions { + /// Wallet account ID that receives the batch builder's fees. + #[arg( + long = "batch.builder.wallet-account-id", + env = "MIDEN_NODE_BATCH_BUILDER_WALLET_ACCOUNT_ID", + value_name = "ACCOUNT_ID", + value_parser = AccountId::from_hex, + help_heading = super::section::BLOCK_PRODUCTION_HELP_HEADING + )] + pub wallet_account_id: AccountId, } #[derive(clap::Args, Clone, Debug)] diff --git a/bin/node/src/commands/fee_collector.rs b/bin/node/src/commands/fee_collector.rs index 6aded03058..cfe4c8b72d 100644 --- a/bin/node/src/commands/fee_collector.rs +++ b/bin/node/src/commands/fee_collector.rs @@ -28,7 +28,7 @@ pub enum FeeCollectorCommand { /// Writes fee-collector.mac in the existing data directory. Refuses to overwrite an existing /// file. Creation is offline. Keep the file private because it contains the signing key. /// - /// Use `miden-node fee-collector deploy` to deploy this account before collecting batch fees. + /// Use `miden-node fee-collector deploy` to deploy this account before starting the sequencer. Create(CreateCommand), /// Deploy a fee collector account in a dedicated block. @@ -43,7 +43,7 @@ pub enum FeeCollectorCommand { /// and pays no transaction fee. If the matching account is already deployed, the command /// succeeds without creating another block. /// - /// Keep the account file and its signing key for fee collection. + /// After deployment, start the sequencer with the same account file. Deploy(Box), } diff --git a/bin/node/src/commands/mod.rs b/bin/node/src/commands/mod.rs index 6e0c6e1625..3e2af5b19c 100644 --- a/bin/node/src/commands/mod.rs +++ b/bin/node/src/commands/mod.rs @@ -45,7 +45,7 @@ pub enum Command { /// Create or deploy the sequencer's fee collector account. /// /// The immutable collector combines transaction fees into P2ID notes for the batch builder's - /// wallet. + /// wallet. Create and deploy a collector before starting the sequencer. #[command(subcommand)] FeeCollector(FeeCollectorCommand), diff --git a/bin/node/src/commands/modes.rs b/bin/node/src/commands/modes.rs index c3bc8b596a..636193aede 100644 --- a/bin/node/src/commands/modes.rs +++ b/bin/node/src/commands/modes.rs @@ -32,6 +32,7 @@ use tokio::net::TcpListener; use url::Url; use super::block_producer::BlockProducerOptions; +use super::fee_collector::FeeCollectorAccountOptions; use super::rpc::SyncOptions; use super::runtime::{RuntimeConfig, RuntimeOptions}; use super::store::StoreOptions; @@ -45,6 +46,9 @@ pub struct SequencerCommand { #[command(flatten)] pub runtime: RuntimeOptions, + #[command(flatten)] + pub fee_collector: FeeCollectorAccountOptions, + #[command(flatten)] pub external_services: SequencerExternalServiceOptions, @@ -73,10 +77,15 @@ pub struct SequencerCommand { } impl SequencerCommand { + #[expect( + clippy::too_many_lines, + reason = "Keep sequencer service startup and task supervision together" + )] pub async fn handle(self, shutdown: CancellationToken) -> anyhow::Result<()> { self.log_starting(); let runtime = self.runtime.runtime_config(&self.store); self.block_producer.validate()?; + let collection_account = self.fee_collector.read(&runtime.data_directory)?; let network_tx_auth = self.runtime.rpc.network_tx_auth()?; let (validator_clients, validator_monitors) = self.external_services.validator_clients_and_monitors()?; @@ -112,9 +121,12 @@ impl SequencerCommand { max_concurrent_proofs: self.block_producer.block.max_concurrent_proofs, mempool_tx_capacity: self.block_producer.mempool.tx_capacity, batch_workers: self.block_producer.batch.workers, + builder_account_id: self.block_producer.builder.wallet_account_id, + pass_through_account: collection_account, } - .spawn(shutdown.clone()) - .context("failed to spawn sequencer")?; + .start(shutdown.clone()) + .await + .context("failed to start sequencer")?; let block_producer = sequencer.api(); let rpc = Rpc { diff --git a/compose/node.yml b/compose/node.yml index f57ccef8dd..c53e151f4d 100644 --- a/compose/node.yml +++ b/compose/node.yml @@ -24,6 +24,8 @@ services: - --validator.url=http://validator-2:50101 - --validator.url=http://validator-3:50101 - --ntx-builder.url=http://ntx-builder:50301 + # Send collected fees to an arbitrary non-existent account ID for now. + - --batch.builder.wallet-account-id=${MIDEN_NODE_BATCH_BUILDER_WALLET_ACCOUNT_ID:-0xcc0000000000dd010000ee000000ff} - --rpc.network-tx-auth-header-value=secret_value environment: MIDEN_NODE_DISABLE_ACCOUNT_ALLOWLIST: "${MIDEN_NODE_DISABLE_ACCOUNT_ALLOWLIST:-true}" diff --git a/crates/block-producer/src/batch_builder/mod.rs b/crates/block-producer/src/batch_builder/mod.rs index 61c0395621..4d9388a40f 100644 --- a/crates/block-producer/src/batch_builder/mod.rs +++ b/crates/block-producer/src/batch_builder/mod.rs @@ -7,6 +7,7 @@ use std::time::Duration; use futures::TryFutureExt; use miden_node_proto::domain::sequencer::AuthenticatedTransaction; use miden_node_store::state::State; +use miden_node_tracing::spawn::spawn_blocking_in_current_span; use miden_node_tracing::{ ErrorSpanExt, Instrument, @@ -17,16 +18,20 @@ use miden_node_tracing::{ }; use miden_node_utils::shutdown::CancellationToken; use miden_protocol::MIN_PROOF_SECURITY_LEVEL; +use miden_protocol::account::{AccountFile, AccountId}; use miden_protocol::batch::{BatchId, ProposedBatch, ProvenBatch}; +use miden_protocol::block::BlockNumber; use miden_protocol::note::NoteId; use miden_protocol::transaction::TransactionId; use tokio::task::{JoinError, JoinSet}; use tokio::time::{Instant, MissedTickBehavior}; use url::Url; -use crate::domain::batch::SelectedBatch; +use crate::domain::batch::{SelectedBatch, SelectedBatchId}; use crate::errors::{BuildBatchError, StoreError}; +use crate::fee_collector::PassThroughTransactionBuilder; use crate::mempool::SharedMempool; +use crate::validator::BlockProducerValidatorClient; use crate::{COMPONENT, LOG_TARGET}; mod remote_prover; @@ -50,6 +55,8 @@ pub struct BatchBuilder { /// /// If not provided, a local batch prover is used. batch_prover: BatchProver, + pass_through: PassThroughTransactionBuilder, + validator: BlockProducerValidatorClient, state: Arc, } @@ -88,15 +95,22 @@ impl BatchBuilder { num_workers: NonZeroUsize, batch_prover_url: Option, intervals: BatchIntervals, + builder_account_id: AccountId, + pass_through_account: AccountFile, + validator: BlockProducerValidatorClient, ) -> anyhow::Result { let batch_prover = batch_prover_url.map_or(Ok(BatchProver::local()), BatchProver::remote)?; + let pass_through = + PassThroughTransactionBuilder::new(builder_account_id, pass_through_account)?; Ok(Self { active_jobs: JoinSet::new(), num_workers, intervals, batch_prover, + pass_through, + validator, state, }) } @@ -167,10 +181,12 @@ impl BatchBuilder { state: self.state.clone(), mempool, batch_prover: self.batch_prover.clone(), + pass_through: self.pass_through.clone(), + validator: self.validator.clone(), }; self.active_jobs.spawn( - async move { job.build_batch(batch).await } + async move { Box::pin(job.build_batch(batch)).await } .instrument(miden_node_tracing::Span::current()), ); } @@ -249,6 +265,8 @@ impl BatchBuilder { struct BatchJob { state: Arc, batch_prover: BatchProver, + pass_through: PassThroughTransactionBuilder, + validator: BlockProducerValidatorClient, mempool: SharedMempool, } @@ -298,6 +316,7 @@ impl BatchJob { &self, selected: SelectedBatch, ) -> Result { + let fee_notes = selected.collectible_fee_notes().to_vec(); let mut block_numbers: BTreeSet<_> = selected .transactions() .iter() @@ -335,12 +354,55 @@ impl BatchJob { .0 .expect("reference block header should exist"); - let transactions = selected + let protocol_config = view + .get_protocol_config(reference_block_header.protocol_config_commitment()) + .await + .map_err(StoreError::GetProtocolConfigFailed) + .map_err(BuildBatchError::FetchBatchInputsFailed)? + .expect("the reference block's protocol configuration should exist"); + let genesis = view + .get_block_header(Some(BlockNumber::GENESIS), false) + .await + .map_err(StoreError::GetBlockHeaderFailed) + .map_err(BuildBatchError::FetchBatchInputsFailed)? + .0 + .expect("the genesis block header should exist") + .commitment(); + + let mut transactions: Vec<_> = selected .into_transactions() .into_iter() .map(|tx| tx.proven_transaction()) .collect(); + let pass_through = self.pass_through.clone(); + let executed_pass_through_tx = pass_through + .execute( + fee_notes, + reference_block_header.clone(), + protocol_config, + partial_blockchain.clone(), + ) + .await + .map_err(BuildBatchError::BuildBatchFeeTransaction)?; + let inputs = executed_pass_through_tx.tx_inputs().clone(); + let pass_through_tx = spawn_blocking_in_current_span(move || { + PassThroughTransactionBuilder::prove(executed_pass_through_tx) + }) + .await + .map_err(BuildBatchError::JoinError)? + .map_err(BuildBatchError::BuildBatchFeeTransaction)?; + self.validator + .validate_transaction( + &pass_through_tx, + &inputs, + genesis, + reference_block_header.validator_config(), + ) + .await + .map_err(BuildBatchError::ValidateBatchFeeTransaction)?; + transactions.push(Arc::new(pass_through_tx)); + ProposedBatch::new( transactions, reference_block_header, @@ -367,7 +429,7 @@ impl BatchJob { target = COMPONENT, name = "batch_builder.rollback_batch", )] - fn rollback_batch(&self, batch_id: BatchId) -> Result<(), BuildBatchError> { + fn rollback_batch(&self, batch_id: SelectedBatchId) -> Result<(), BuildBatchError> { self.mempool .lock() .map_err(BuildBatchError::MempoolPoisoned)? @@ -409,7 +471,7 @@ impl SelectedBatch { }, ); SelectedBatchTelemetry { - batch_id: self.id(), + batch_id: self.id().as_batch_id(), transactions_count: self.transactions().len(), transaction_ids: tx_ids, input_notes_count, diff --git a/crates/block-producer/src/domain/batch.rs b/crates/block-producer/src/domain/batch.rs index 4a0e4fae15..0e2afe1905 100644 --- a/crates/block-producer/src/domain/batch.rs +++ b/crates/block-producer/src/domain/batch.rs @@ -1,4 +1,5 @@ use std::collections::{HashMap, HashSet}; +use std::fmt::{Display, Formatter}; use std::sync::Arc; use miden_node_proto::domain::sequencer::AuthenticatedTransaction; @@ -6,10 +7,36 @@ use miden_protocol::Word; use miden_protocol::account::AccountId; use miden_protocol::batch::BatchId; use miden_protocol::block::BlockNumber; +use miden_protocol::note::Note; +use miden_protocol::transaction::OutputNote; +use miden_standards::note::TxFeeNote; // SELECTED BATCH // ================================================================================================ +/// Identifies a transaction selection in the batch graph. +/// +/// A sequencer-built batch has a different [`BatchId`] after the batch builder appends the fee +/// transaction. A user-proven batch keeps the same ID. +#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] +pub(crate) struct SelectedBatchId(BatchId); + +impl SelectedBatchId { + pub(crate) fn from_batch_id(batch_id: BatchId) -> Self { + Self(batch_id) + } + + pub(crate) fn as_batch_id(self) -> BatchId { + self.0 + } +} + +impl Display for SelectedBatchId { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + self.0.fmt(f) + } +} + /// Parameters that define how the node builds a batch. #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub(crate) struct BatchParameters { @@ -33,10 +60,11 @@ impl BatchParameters { #[derive(Clone, Debug, PartialEq)] pub(crate) struct SelectedBatch { txs: Vec>, - id: BatchId, + id: SelectedBatchId, parameters: BatchParameters, account_updates: HashMap)>, unauthenticated_notes: HashSet, + collectible_fee_notes: Vec, } impl SelectedBatch { @@ -48,7 +76,7 @@ impl SelectedBatch { } } - pub(crate) fn id(&self) -> BatchId { + pub(crate) fn id(&self) -> SelectedBatchId { self.id } @@ -64,6 +92,10 @@ impl SelectedBatch { self.parameters } + pub(crate) fn collectible_fee_notes(&self) -> &[Note] { + &self.collectible_fee_notes + } + /// The aggregated list of account transitions this batch causes given as tuples of `(AccountId, /// initial commitment, final commitment, Option)`. /// @@ -141,7 +173,9 @@ not match the current commitment {}", /// Finalizes the batch selection. pub(crate) fn build(self) -> SelectedBatch { let Self { parameters, txs, account_updates } = self; - let id = BatchId::from_ids(txs.iter().map(|tx| (tx.id(), tx.account_id()))); + let id = SelectedBatchId::from_batch_id(BatchId::from_ids( + txs.iter().map(|tx| (tx.id(), tx.account_id())), + )); let mut unauthenticated_notes: HashSet<_> = txs.iter().flat_map(|tx| tx.unauthenticated_note_ids()).collect(); @@ -150,12 +184,25 @@ not match the current commitment {}", unauthenticated_notes.remove(&output_note); } + let fee_script_root = TxFeeNote::script_root(); + let collectible_fee_notes = txs + .iter() + .flat_map(|tx| tx.raw_proven_transaction().output_notes().iter()) + .filter_map(|note| match note { + OutputNote::Public(note) if note.recipient().script().root() == fee_script_root => { + Some(note.as_note().clone()) + }, + _ => None, + }) + .collect(); + SelectedBatch { txs, id, parameters, account_updates, unauthenticated_notes, + collectible_fee_notes, } } } diff --git a/crates/block-producer/src/errors.rs b/crates/block-producer/src/errors.rs index 6076801a6a..2a48221c05 100644 --- a/crates/block-producer/src/errors.rs +++ b/crates/block-producer/src/errors.rs @@ -152,6 +152,12 @@ pub enum BuildBatchError { #[error("failed to build proposed transaction batch")] ProposeBatchError(#[source] ProposedBatchError), + #[error("failed to build the batch fee transaction")] + BuildBatchFeeTransaction(#[source] anyhow::Error), + + #[error("failed to validate the batch fee transaction")] + ValidateBatchFeeTransaction(#[source] anyhow::Error), + #[error("failed to prove proposed transaction batch")] ProveBatchError(#[source] ProvenBatchError), @@ -229,6 +235,8 @@ pub enum StoreError { GetBlockInclusionProofsFailed(#[source] GetBlockInclusionProofsError), #[error("failed to get block header from store")] GetBlockHeaderFailed(#[source] GetBlockHeaderError), + #[error("failed to get protocol configuration from store")] + GetProtocolConfigFailed(#[source] DatabaseError), #[error("failed to get note inclusion proofs from store")] GetNoteInclusionProofsFailed(#[source] GetNoteInclusionProofsError), #[error("failed to apply block to store")] diff --git a/crates/block-producer/src/fee_collector.rs b/crates/block-producer/src/fee_collector.rs index 4d83fb720a..f70bc13917 100644 --- a/crates/block-producer/src/fee_collector.rs +++ b/crates/block-producer/src/fee_collector.rs @@ -121,6 +121,19 @@ pub async fn deploy_fee_collector( Ok(()) } +/// Checks the collector's committed state and loads its deployed nonce. +pub(crate) async fn load_deployed_collector( + state: &State, + account_file: &mut AccountFile, +) -> anyhow::Result<()> { + account_file.account.set_nonce(ONE)?; + anyhow::ensure!( + collector_is_deployed(state, &account_file.account).await?, + "fee collector is not deployed; use miden-node fee-collector deploy", + ); + Ok(()) +} + async fn collector_is_deployed(state: &State, account: &Account) -> anyhow::Result { let response = state .view() diff --git a/crates/block-producer/src/mempool/budget.rs b/crates/block-producer/src/mempool/budget.rs index 51b7c84ff7..39e41323e7 100644 --- a/crates/block-producer/src/mempool/budget.rs +++ b/crates/block-producer/src/mempool/budget.rs @@ -9,7 +9,7 @@ use miden_protocol::{ use crate::{DEFAULT_MAX_BATCHES_PER_BLOCK, DEFAULT_MAX_TXS_PER_BATCH}; /// Constraints placed on the batches proposed by the [`Mempool`](super::Mempool). -#[derive(Copy, Clone, Debug, PartialEq, Eq)] +#[derive(Clone, Debug, PartialEq, Eq)] pub struct BatchBudget { /// Maximum number of transactions allowed in a batch. pub transactions: usize, @@ -38,12 +38,7 @@ pub(crate) enum BudgetStatus { impl Default for BatchBudget { fn default() -> Self { - Self { - transactions: DEFAULT_MAX_TXS_PER_BATCH.get(), - input_notes: MAX_INPUT_NOTES_PER_BATCH, - output_notes: MAX_OUTPUT_NOTES_PER_BATCH, - accounts: MAX_ACCOUNTS_PER_BATCH, - } + Self::new(DEFAULT_MAX_TXS_PER_BATCH.get()) } } @@ -56,6 +51,16 @@ impl Default for BlockBudget { } impl BatchBudget { + /// Creates a standalone transaction budget and reserves room for one pass-through transaction. + pub fn new(max_transactions: usize) -> Self { + Self { + transactions: max_transactions.saturating_sub(1), + input_notes: MAX_INPUT_NOTES_PER_BATCH, + output_notes: MAX_OUTPUT_NOTES_PER_BATCH.saturating_sub(1), + accounts: MAX_ACCOUNTS_PER_BATCH.saturating_sub(1), + } + } + /// Returns `true` if no more transaction resources can be consumed from this budget. pub(crate) fn is_exhausted(&self) -> bool { self.transactions == 0 @@ -110,3 +115,36 @@ impl BlockBudget { } } } + +#[cfg(test)] +mod tests { + use miden_protocol::transaction::{OutputNote, PublicOutputNote}; + + use super::*; + use crate::test_utils::note::mock_fee_note; + use crate::test_utils::{MockAuthenticatedTxBuilder, MockProvenTxBuilder}; + + #[test] + fn batch_budget_reserves_pass_through_transaction_resources() { + let budget = BatchBudget::new(10); + + assert_eq!(budget.transactions, 9); + assert_eq!(budget.accounts, MAX_ACCOUNTS_PER_BATCH - 1); + assert_eq!(budget.output_notes, MAX_OUTPUT_NOTES_PER_BATCH - 1); + assert_eq!(budget.input_notes, MAX_INPUT_NOTES_PER_BATCH); + } + + #[test] + fn fee_notes_consume_the_output_note_budget() { + let fee_note = mock_fee_note(1); + let tx = MockProvenTxBuilder::with_account_index(1) + .output_notes(vec![OutputNote::Public(PublicOutputNote::new(fee_note).unwrap())]) + .build(); + let tx = MockAuthenticatedTxBuilder::new(tx).build(); + let mut budget = BatchBudget::new(10); + let initial_output_notes = budget.output_notes; + + assert_eq!(budget.check_then_subtract(&tx), BudgetStatus::WithinScope); + assert_eq!(budget.output_notes, initial_output_notes - 1); + } +} diff --git a/crates/block-producer/src/mempool/graph/batch.rs b/crates/block-producer/src/mempool/graph/batch.rs index 0bcb4623e3..bd9ba6e2e4 100644 --- a/crates/block-producer/src/mempool/graph/batch.rs +++ b/crates/block-producer/src/mempool/graph/batch.rs @@ -7,7 +7,7 @@ use miden_protocol::batch::{BatchId, ProvenBatch}; use miden_protocol::block::BlockNumber; use miden_protocol::note::Nullifier; -use crate::domain::batch::SelectedBatch; +use crate::domain::batch::{SelectedBatch, SelectedBatchId}; use crate::errors::StateConflict; use crate::mempool::BlockBudget; use crate::mempool::budget::BudgetStatus; @@ -18,7 +18,7 @@ use crate::mempool::graph::node::GraphNode; // ================================================================================================ impl GraphNode for SelectedBatch { - type Id = BatchId; + type Id = SelectedBatchId; fn nullifiers(&self) -> Box + '_> { Box::new(self.transactions().iter().flat_map(|tx| tx.nullifiers())) @@ -59,7 +59,8 @@ impl GraphNode for SelectedBatch { #[derive(Clone, Debug, PartialEq, Default)] pub struct BatchGraph { inner: Graph, - proven: HashMap>, + proven: HashMap>, + selected_by_proven: HashMap, } impl BatchGraph { @@ -73,12 +74,38 @@ impl BatchGraph { self.inner.append(batch) } + /// Inserts a user-proven batch into the dependency graph. + /// + /// # Errors + /// + /// Returns an error if the batch state conflicts with the current graph view. + pub fn append_user_batch( + &mut self, + batch: SelectedBatch, + proof: Arc, + ) -> Result<(), StateConflict> { + let selected_id = batch.id(); + assert_eq!( + selected_id.as_batch_id(), + proof.id(), + "a user proof must match its selected batch", + ); + + self.inner.append(batch)?; + self.insert_proof(selected_id, proof.id(), proof); + + Ok(()) + } + /// Reverts the given batch and _all_ its descendants _IFF_ it is present in the graph. /// /// This includes batches that have been marked as proven. /// /// Returns the reverted batches in the _reverse_ chronological order they were appended in. - pub fn revert_batch_and_descendants(&mut self, batch: BatchId) -> Vec { + pub fn revert_selected_batch_and_descendants( + &mut self, + batch: SelectedBatchId, + ) -> Vec { // We need this check because `inner.revert..` panics if the node is unknown. if !self.inner.contains(&batch) { return Vec::default(); @@ -86,12 +113,23 @@ impl BatchGraph { let reverted = self.inner.revert_node_and_descendants(batch); for batch in &reverted { - self.proven.remove(&batch.id()); + if let Some(proven) = self.proven.remove(&batch.id()) { + self.selected_by_proven.remove(&proven.id()); + } } reverted } + /// Reverts the proven batch and all its descendants if it is present in the graph. + pub fn revert_proven_batch_and_descendants(&mut self, batch: BatchId) -> Vec { + let Some(selected_id) = self.selected_by_proven.get(&batch).copied() else { + return Vec::new(); + }; + + self.revert_selected_batch_and_descendants(selected_id) + } + /// Reverts expired batches and their descendants. /// /// Only unselected batches are considered, the assumption being that selected batches @@ -106,7 +144,7 @@ impl BatchGraph { let mut reverted = Vec::with_capacity(to_revert.len()); for batch in to_revert { - reverted.extend_from_slice(&self.revert_batch_and_descendants(batch)); + reverted.extend_from_slice(&self.revert_selected_batch_and_descendants(batch)); } reverted @@ -115,13 +153,42 @@ impl BatchGraph { /// Marks the given batch as proven, making it available for selection in a block once it /// becomes a root. pub fn submit_proof(&mut self, proof: Arc) { - if self.inner.contains(&proof.id()) { - self.proven.insert(proof.id(), proof); + let proof_id = proof.id(); + let (_builder_transaction, selected_transactions) = proof + .transactions() + .as_slice() + .split_last() + .expect("a builder batch must contain a batch builder transaction"); + assert!( + !selected_transactions.is_empty(), + "a builder batch must contain at least one user transaction", + ); + let selected_id = SelectedBatchId::from_batch_id(BatchId::from_ids( + selected_transactions + .iter() + .map(|transaction| (transaction.id(), transaction.account_id())), + )); + + self.insert_proof(selected_id, proof_id, proof); + } + + fn insert_proof( + &mut self, + selected_id: SelectedBatchId, + proof_id: BatchId, + proof: Arc, + ) { + if self.inner.contains(&selected_id) { + if let Some(previous) = self.proven.get(&selected_id) { + self.selected_by_proven.remove(&previous.id()); + } + self.selected_by_proven.insert(proof_id, selected_id); + self.proven.insert(selected_id, proof); } } /// Returns `true` if the batch has been proven previously. - pub fn is_proven(&mut self, batch: &BatchId) -> bool { + pub fn is_proven(&mut self, batch: &SelectedBatchId) -> bool { self.proven.contains_key(batch) } @@ -135,14 +202,17 @@ impl BatchGraph { let mut selected = Vec::default(); // Only batches which are proven can be selected for inclusion in a block. - while let Some(candidate) = - self.inner.selection_candidates().iter().find_map(|(id, _)| self.proven.get(id)) + while let Some((selected_id, candidate)) = self + .inner + .selection_candidates() + .iter() + .find_map(|(id, _)| self.proven.get(id).map(|proof| (**id, proof))) { if budget.check_then_subtract(candidate) == BudgetStatus::Exceeded { break; } - self.inner.select_candidate(candidate.id()); + self.inner.select_candidate(selected_id); selected.push(Arc::clone(candidate)); } @@ -156,8 +226,14 @@ impl BatchGraph { /// Panics if the batch does not exist, or has existing ancestors in the batch /// graph. pub fn prune(&mut self, batch: BatchId) -> SelectedBatch { - self.proven.remove(&batch); - self.inner.prune(batch) + let selected_id = self + .selected_by_proven + .remove(&batch) + .expect("proven batch must map to a selected batch"); + if let Some(proven) = self.proven.remove(&selected_id) { + self.selected_by_proven.remove(&proven.id()); + } + self.inner.prune(selected_id) } pub fn proven_count(&self) -> usize { diff --git a/crates/block-producer/src/mempool/graph/transaction.rs b/crates/block-producer/src/mempool/graph/transaction.rs index 8c0fda9ae1..dfd924d70f 100644 --- a/crates/block-producer/src/mempool/graph/transaction.rs +++ b/crates/block-producer/src/mempool/graph/transaction.rs @@ -435,7 +435,7 @@ impl TransactionGraph { self.inner.prune(tx.id()); self.failures.remove(&tx.id()); } - self.user_batches.remove(&batch.id()); + self.user_batches.remove(&batch.id().as_batch_id()); } fn mark_committed_notes_authenticated_for_descendants( @@ -466,6 +466,10 @@ impl TransactionGraph { self.inner.node_count() } + pub fn contains(&self, transaction: &TransactionId) -> bool { + self.inner.contains(transaction) + } + pub fn accounts_count(&self) -> usize { self.inner.account_count() } diff --git a/crates/block-producer/src/mempool/mod.rs b/crates/block-producer/src/mempool/mod.rs index 705ba87811..15f2315da3 100644 --- a/crates/block-producer/src/mempool/mod.rs +++ b/crates/block-producer/src/mempool/mod.rs @@ -63,9 +63,8 @@ use miden_standards::note::TxFeeNote; use thiserror::Error; use crate::block_builder::SelectedBlock; -use crate::domain::batch::{BatchParameters, SelectedBatch}; +use crate::domain::batch::{BatchParameters, SelectedBatch, SelectedBatchId}; use crate::errors::{MempoolSubmissionError, StateConflict}; -use crate::mempool::budget::BudgetStatus; use crate::{ COMPONENT, DEFAULT_MEMPOOL_TX_CAPACITY, @@ -100,6 +99,9 @@ pub struct MempoolConfig { /// The constraints each proposed batch must adhere to. pub batch_budget: BatchBudget, + /// The maximum number of transactions allowed in a batch. + pub max_txs_per_batch: usize, + /// How close to the chain tip the mempool will allow submitted transactions and batches to /// expire. /// @@ -138,6 +140,7 @@ impl Default for MempoolConfig { Self { block_budget: BlockBudget::default(), batch_budget: BatchBudget::default(), + max_txs_per_batch: crate::DEFAULT_MAX_TXS_PER_BATCH.get(), expiration_slack: SERVER_MEMPOOL_EXPIRATION_SLACK, state_retention: SERVER_MEMPOOL_STATE_RETENTION, tx_capacity: DEFAULT_MEMPOOL_TX_CAPACITY, @@ -296,13 +299,8 @@ impl Mempool { return Err(MempoolSubmissionError::CapacityExceeded); } - // Ensure the batch doesn't exceed the mempool budget for batches. - let mut budget = self.config.batch_budget; - for tx in txs { - if budget.check_then_subtract(tx) == BudgetStatus::Exceeded { - // TODO: better error plox. - return Err(MempoolSubmissionError::CapacityExceeded); - } + if txs.len() > self.config.max_txs_per_batch { + return Err(MempoolSubmissionError::CapacityExceeded); } let batch_id = BatchId::from_transactions(txs.iter().map(|tx| tx.raw_proven_transaction())); @@ -313,6 +311,7 @@ impl Mempool { for tx in txs { self.authentication_staleness_check(tx.authentication_height())?; self.expiration_check(tx.expires_at())?; + self.fee_note_consumption_check(tx)?; } self.transactions @@ -353,7 +352,7 @@ impl Mempool { }; let batch = self .transactions - .select_any_internal_batch(self.config.batch_budget, parameters)?; + .select_any_internal_batch(self.config.batch_budget.clone(), parameters)?; let batch = self.append_selected_batch(batch); self.promote_user_batches(); let telemetry = self.telemetry(); @@ -384,7 +383,7 @@ impl Mempool { }; let batch = self .transactions - .select_full_internal_batch(self.config.batch_budget, parameters)?; + .select_full_internal_batch(self.config.batch_budget.clone(), parameters)?; let batch = self.append_selected_batch(batch); self.promote_user_batches(); let telemetry = self.telemetry(); @@ -410,8 +409,9 @@ impl Mempool { /// Moves selectable user-proven batches into the batch graph. fn promote_user_batches(&mut self) { while let Some((batch, proof)) = self.transactions.select_user_batch() { - self.append_selected_batch(batch); - self.batches.submit_proof(proof); + if let Err(err) = self.batches.append_user_batch(batch, proof) { + panic!("failed to append user batch to dependency graph: {}", err.as_report()); + } } } @@ -424,7 +424,7 @@ impl Mempool { target = COMPONENT, name = "mempool.rollback_batch", )] - pub fn rollback_batch(&mut self, batch: BatchId) { + pub(crate) fn rollback_batch(&mut self, batch: SelectedBatchId) { // Guards against bugs in the proof scheduler where a retry results in multiple results // coming back for the same batch. If the batch previously succeeded, then yanking it would // corrupt the mempool since the batch might be in a block. @@ -435,7 +435,7 @@ impl Mempool { return; } - let reverted_batches = self.batches.revert_batch_and_descendants(batch); + let reverted_batches = self.batches.revert_selected_batch_and_descendants(batch); for reverted in &reverted_batches { self.transactions.requeue_transactions(reverted); } @@ -591,7 +591,7 @@ impl Mempool { // // Transactions which have failed excessively are also reverted. for batch in &block.batches { - let reverted = self.batches.revert_batch_and_descendants(batch.id()); + let reverted = self.batches.revert_proven_batch_and_descendants(batch.id()); for batch in reverted { self.transactions.requeue_transactions(&batch); @@ -600,8 +600,11 @@ impl Mempool { let failed_txs = block .batches .iter() - .flat_map(|batch| batch.transactions().as_slice().iter().map(TransactionHeader::id)); - let evicted = self.transactions.increment_failure_count(failed_txs); + .flat_map(|batch| batch.transactions().as_slice()) + .map(TransactionHeader::id) + .filter(|transaction| self.transactions.contains(transaction)) + .collect::>(); + let evicted = self.transactions.increment_failure_count(failed_txs.into_iter()); let telemetry = self.telemetry(); miden_span_record!( mempool.transactions.uncommitted = telemetry.uncommitted_transactions, @@ -629,8 +632,9 @@ impl Mempool { .committed_blocks .iter() .flat_map(|block| block.batches.iter()) - .map(|batch| batch.transactions().as_slice().len()) - .sum::(); + .flat_map(|batch| batch.transactions().as_slice()) + .filter(|transaction| self.transactions.contains(&transaction.id())) + .count(); self.transactions .count() diff --git a/crates/block-producer/src/mempool/tests.rs b/crates/block-producer/src/mempool/tests.rs index 6b7490eb57..750bf6e827 100644 --- a/crates/block-producer/src/mempool/tests.rs +++ b/crates/block-producer/src/mempool/tests.rs @@ -4,12 +4,16 @@ use std::time::Duration; use assert_matches::assert_matches; use miden_protocol::Word; use miden_protocol::block::{BlockHeader, BlockNumber}; +use miden_protocol::transaction::TransactionHeader; use pretty_assertions::assert_eq; use serial_test::serial; use super::*; use crate::mempool::graph::{TransactionGraph, TransactionRemoval}; -use crate::test_utils::batch::TransactionBatchConstructor; +use crate::test_utils::batch::{ + TransactionBatchConstructor, + mock_proven_batch_with_builder_transaction, +}; use crate::test_utils::{MockAuthenticatedTxBuilder, MockProvenTxBuilder, mock_account_id}; mod add_transaction; @@ -72,7 +76,7 @@ fn retained_committed_transactions_do_not_consume_capacity() { uut.add_transaction(first.clone()).unwrap(); uut.select_any_batch().unwrap(); - uut.commit_batch(Arc::new(ProvenBatch::mocked_from_transactions([ + uut.commit_batch(Arc::new(mock_proven_batch_with_builder_transaction([ first.raw_proven_transaction() ]))); let block = uut.select_block(); @@ -237,7 +241,7 @@ fn children_of_failed_batches_are_ignored() { assert_eq!(uut, reference); let proven_batch = - Arc::new(ProvenBatch::mocked_from_transactions([txs[2].raw_proven_transaction()])); + Arc::new(mock_proven_batch_with_builder_transaction([txs[2].raw_proven_transaction()])); uut.commit_batch(proven_batch); assert_eq!(uut, reference); } @@ -286,7 +290,7 @@ fn block_commit_reverts_expired_txns() { // Force the tx into the next block by batching it. uut.add_transaction(tx_to_commit.clone()).unwrap(); uut.select_any_batch().unwrap(); - uut.commit_batch(Arc::new(ProvenBatch::mocked_from_transactions([ + uut.commit_batch(Arc::new(mock_proven_batch_with_builder_transaction([ tx_to_commit.raw_proven_transaction() ]))); @@ -305,7 +309,7 @@ fn block_commit_reverts_expired_txns() { // A reverted transaction behaves as if it never existed. reference.add_transaction(tx_to_commit.clone()).unwrap(); reference.select_any_batch().unwrap(); - reference.commit_batch(Arc::new(ProvenBatch::mocked_from_transactions([ + reference.commit_batch(Arc::new(mock_proven_batch_with_builder_transaction([ tx_to_commit.raw_proven_transaction() ]))); reference.select_block(); @@ -360,7 +364,7 @@ fn pruned_committed_notes_are_authenticated_for_inflight_descendants() { assert_eq!(parent_batch.transactions(), std::slice::from_ref(&parent)); uut.add_transaction(child.clone()).unwrap(); - uut.commit_batch(Arc::new(ProvenBatch::mocked_from_transactions([ + uut.commit_batch(Arc::new(mock_proven_batch_with_builder_transaction([ parent.raw_proven_transaction() ]))); @@ -409,7 +413,8 @@ fn rollbacks_of_already_proven_batches_are_ignored() { uut.add_transaction(txs[0].clone()).unwrap(); let batch = uut.select_any_batch().unwrap(); - let proof = Arc::new(ProvenBatch::mocked_from_transactions([txs[0].raw_proven_transaction()])); + let proof = + Arc::new(mock_proven_batch_with_builder_transaction([txs[0].raw_proven_transaction()])); uut.commit_batch(Arc::clone(&proof)); let reference = uut.clone(); @@ -418,6 +423,30 @@ fn rollbacks_of_already_proven_batches_are_ignored() { assert_eq!(uut, reference); } +#[test] +fn proven_batch_id_resolves_to_selected_batch_id() { + let (mut uut, _) = Mempool::for_tests(); + let user_tx = MockProvenTxBuilder::with_account_index(50).build(); + let user_tx = Arc::new(MockAuthenticatedTxBuilder::new(user_tx).build()); + + uut.add_transaction(user_tx.clone()).unwrap(); + let selected = uut.select_any_batch().unwrap(); + let synthetic_tx = MockProvenTxBuilder::with_account_index(51).build(); + let proof = Arc::new(ProvenBatch::mocked_from_transactions([ + user_tx.raw_proven_transaction(), + &synthetic_tx, + ])); + assert_ne!(selected.id().as_batch_id(), proof.id()); + + uut.commit_batch(Arc::clone(&proof)); + let block = uut.select_block(); + assert_eq!(block.batches.as_slice(), &[proof]); + + uut.rollback_block(block.block_number); + assert_eq!(uut.unbatched_transactions_count(), 1); + assert!(uut.select_any_batch().is_some()); +} + // BLOCK FAILED TESTS // ================================================================================================ @@ -429,7 +458,7 @@ fn block_failure_increments_tx_failures() { uut.add_transaction(reverted_txs[0].clone()).unwrap(); uut.select_any_batch().unwrap(); - uut.commit_batch(Arc::new(ProvenBatch::mocked_from_transactions([ + uut.commit_batch(Arc::new(mock_proven_batch_with_builder_transaction([ reverted_txs[0].raw_proven_transaction() ]))); @@ -449,12 +478,14 @@ fn block_failure_increments_tx_failures() { reference.add_transaction(reverted_txs[1].clone()).unwrap(); reference.add_transaction(reverted_txs[2].clone()).unwrap(); - reference.transactions.increment_failure_count( - block - .batches - .iter() - .flat_map(|batch| batch.transactions().as_slice().iter().map(TransactionHeader::id)), - ); + let failed_transactions = block + .batches + .iter() + .flat_map(|batch| batch.transactions().as_slice()) + .map(TransactionHeader::id) + .filter(|transaction| reference.transactions.contains(transaction)) + .collect::>(); + reference.transactions.increment_failure_count(failed_transactions.into_iter()); assert_eq!(uut, reference); } diff --git a/crates/block-producer/src/mempool/tests/add_transaction.rs b/crates/block-producer/src/mempool/tests/add_transaction.rs index ce7f4f6f80..a95c1700e5 100644 --- a/crates/block-producer/src/mempool/tests/add_transaction.rs +++ b/crates/block-producer/src/mempool/tests/add_transaction.rs @@ -3,13 +3,12 @@ use std::sync::Arc; use assert_matches::assert_matches; use miden_node_proto::domain::sequencer::AuthenticatedTransaction; use miden_protocol::Word; -use miden_protocol::batch::ProvenBatch; use miden_protocol::block::BlockHeader; use miden_protocol::transaction::{OutputNote, PublicOutputNote}; use crate::errors::{MempoolSubmissionError, StateConflict}; use crate::mempool::Mempool; -use crate::test_utils::batch::TransactionBatchConstructor; +use crate::test_utils::batch::mock_proven_batch_with_builder_transaction; use crate::test_utils::note::mock_fee_note; use crate::test_utils::{MockAuthenticatedTxBuilder, MockProvenTxBuilder, mock_account_id}; @@ -344,7 +343,7 @@ fn committed_fee_note_consumption_is_accepted() { uut.add_transaction(producer.clone()).unwrap(); uut.select_any_batch().unwrap(); - uut.commit_batch(Arc::new(ProvenBatch::mocked_from_transactions([ + uut.commit_batch(Arc::new(mock_proven_batch_with_builder_transaction([ producer.raw_proven_transaction() ]))); let block = uut.select_block(); diff --git a/crates/block-producer/src/mempool/tests/add_user_batch.rs b/crates/block-producer/src/mempool/tests/add_user_batch.rs index a695e00a55..45ca9e068d 100644 --- a/crates/block-producer/src/mempool/tests/add_user_batch.rs +++ b/crates/block-producer/src/mempool/tests/add_user_batch.rs @@ -5,12 +5,14 @@ use assert_matches::assert_matches; use miden_node_proto::domain::sequencer::AuthenticatedTransaction; use miden_protocol::batch::{BatchId, ProvenBatch}; use miden_protocol::block::BlockNumber; +use miden_protocol::transaction::{OutputNote, PublicOutputNote, TransactionHeader}; use pretty_assertions::assert_eq; use crate::domain::batch::BatchParameters; use crate::errors::{MempoolSubmissionError, StateConflict}; use crate::mempool::Mempool; use crate::test_utils::batch::TransactionBatchConstructor; +use crate::test_utils::note::mock_fee_note; use crate::test_utils::{MockAuthenticatedTxBuilder, MockProvenTxBuilder}; #[test] @@ -50,7 +52,7 @@ fn user_batch_bypasses_batch_proving() { #[test] fn user_batch_respects_batch_budget() { let (mut uut, _) = Mempool::for_tests(); - uut.config.batch_budget.transactions = 1; + uut.config.max_txs_per_batch = 1; let user_batch_txs = MockProvenTxBuilder::sequential(); let result = add_user_batch(&mut uut, &user_batch_txs[..2], BatchParameters::for_tests()); @@ -58,6 +60,19 @@ fn user_batch_respects_batch_budget() { assert_matches!(result, Err(MempoolSubmissionError::CapacityExceeded)); } +#[test] +fn user_batch_does_not_reserve_a_builder_transaction_slot() { + let (mut uut, _) = Mempool::for_tests(); + uut.config.batch_budget.transactions = 1; + uut.config.max_txs_per_batch = 2; + + let user_batch_txs = MockProvenTxBuilder::sequential(); + add_user_batch(&mut uut, &user_batch_txs[..2], BatchParameters::for_tests()).unwrap(); + + assert!(uut.select_any_batch().is_none()); + assert_eq!(uut.select_block().batches[0].transactions().as_slice().len(), 2); +} + #[test] fn user_batch_rejects_a_mismatched_proof() { let (mut uut, reference) = Mempool::for_tests(); @@ -92,7 +107,6 @@ fn user_batch_capacity_counts_batched_uncommitted_transactions() { #[test] fn user_batch_is_not_selected_for_proving() { let (mut uut, _) = Mempool::for_tests(); - uut.config.batch_budget.transactions = 3; let user_batch_txs = MockProvenTxBuilder::sequential(); add_user_batch(&mut uut, &user_batch_txs[..1], BatchParameters::for_tests()).unwrap(); @@ -123,6 +137,49 @@ fn user_batch_with_internal_state_conflicts_are_rejected() { assert_eq!(uut, reference); } +#[test] +fn user_batch_which_consumes_fee_note_is_accepted() { + let (mut uut, _) = Mempool::for_tests(); + let fee_note = mock_fee_note(30); + let producer = + build_tx(MockProvenTxBuilder::with_account_index(30).output_notes(vec![ + OutputNote::Public(PublicOutputNote::new(fee_note.clone()).unwrap()), + ])); + let consumer = + build_tx(MockProvenTxBuilder::with_account_index(31).unauthenticated_notes(vec![fee_note])); + + add_user_batch(&mut uut, &[producer.clone(), consumer.clone()], BatchParameters::for_tests()) + .unwrap(); + + let proof = &uut.select_block().batches[0]; + assert_eq!( + proof + .transactions() + .as_slice() + .iter() + .map(TransactionHeader::id) + .collect::>(), + vec![producer.id(), consumer.id()], + ); +} + +#[test] +fn user_batch_which_consumes_external_inflight_fee_note_is_rejected() { + let (mut uut, _) = Mempool::for_tests(); + let fee_note = mock_fee_note(32); + let producer = + build_tx(MockProvenTxBuilder::with_account_index(32).output_notes(vec![ + OutputNote::Public(PublicOutputNote::new(fee_note.clone()).unwrap()), + ])); + let consumer = + build_tx(MockProvenTxBuilder::with_account_index(33).unauthenticated_notes(vec![fee_note])); + uut.add_transaction(producer).unwrap(); + + let result = add_user_batch(&mut uut, &[consumer], BatchParameters::for_tests()); + + assert_matches!(result, Err(MempoolSubmissionError::ConsumesInflightFeeNotes { .. })); +} + #[test] fn user_batch_conflicts_with_existing_state_are_rejected() { let (mut uut, mut reference) = Mempool::for_tests(); diff --git a/crates/block-producer/src/server/mod.rs b/crates/block-producer/src/server/mod.rs index 635dce6f39..04ed92f61e 100644 --- a/crates/block-producer/src/server/mod.rs +++ b/crates/block-producer/src/server/mod.rs @@ -9,6 +9,7 @@ use miden_node_tracing::{debug, error, info, miden_instrument}; use miden_node_utils::formatting::{format_input_notes, format_output_notes}; use miden_node_utils::shutdown::CancellationToken; use miden_node_utils::tasks::Tasks; +use miden_protocol::account::{AccountFile, AccountId}; use miden_protocol::batch::{ProposedBatch, ProvenBatch}; use miden_protocol::block::BlockNumber; use miden_protocol::transaction::ProvenTransaction; @@ -53,10 +54,8 @@ impl Default for BlockProducerApiConfig { impl BlockProducerApiConfig { fn mempool_config(self) -> MempoolConfig { MempoolConfig { - batch_budget: BatchBudget { - transactions: self.max_txs_per_batch.get(), - ..BatchBudget::default() - }, + batch_budget: BatchBudget::new(self.max_txs_per_batch.get()), + max_txs_per_batch: self.max_txs_per_batch.get(), block_budget: BlockBudget { batches: self.max_batches_per_block.get(), }, @@ -100,26 +99,30 @@ pub struct Sequencer { /// The number of concurrent batch-builder workers. pub batch_workers: NonZeroUsize, + + /// The batch builder account that receives collected fees. + pub builder_account_id: AccountId, + + /// The deployed pass-through account and its signing key. + pub pass_through_account: AccountFile, } // BLOCK PRODUCER // ================================================================================================ impl Sequencer { - /// Spawns the sequencer tasks and returns its in-process API. - pub fn spawn(self, shutdown: CancellationToken) -> Result { + /// Checks the deployed collector, then starts the sequencer tasks and returns its API. + pub async fn start(mut self, shutdown: CancellationToken) -> Result { info!(target: LOG_TARGET, "Initializing sequencer"); let state = self.state; + crate::fee_collector::load_deployed_collector(&state, &mut self.pass_through_account) + .await?; let validator = BlockProducerValidatorClient::new(self.validator_urls.clone(), self.validator_timeout)?; - let chain_tip = state.committed_tip(); - - info!(target: LOG_TARGET, "Sequencer initialized"); - let block_builder = BlockBuilder::new( Arc::clone(&state), self.block_writer, - validator, + validator.clone(), self.block_interval, ); let batch_intervals = BatchIntervals::derive_from(self.block_interval, self.batch_interval); @@ -128,13 +131,16 @@ impl Sequencer { self.batch_workers, self.batch_prover_url, batch_intervals, + self.builder_account_id, + self.pass_through_account, + validator, )?; let api_config = BlockProducerApiConfig { max_txs_per_batch: self.max_txs_per_batch, max_batches_per_block: self.max_batches_per_block, mempool_tx_capacity: self.mempool_tx_capacity, }; - let mempool = Mempool::shared(chain_tip, api_config.mempool_config()); + let mempool = Mempool::shared(state.committed_tip(), api_config.mempool_config()); let api = BlockProducerApi::from_shared_mempool(mempool.clone(), state, shutdown.clone()); let block_prover = if let Some(url) = self.block_prover_url { Arc::new(BlockProver::remote(url)?) @@ -142,6 +148,7 @@ impl Sequencer { Arc::new(BlockProver::local()) }; let chain_tip_rx = api.state.subscribe_committed_tip(); + info!(target: LOG_TARGET, "Sequencer initialized"); // Spawn batch builder, block builder, and proof scheduler. The builders communicate // indirectly via a shared mempool. diff --git a/crates/block-producer/src/server/tests.rs b/crates/block-producer/src/server/tests.rs index 23d0def166..5d055ac553 100644 --- a/crates/block-producer/src/server/tests.rs +++ b/crates/block-producer/src/server/tests.rs @@ -5,14 +5,15 @@ use std::time::Duration; use miden_node_store::GenesisState; use miden_node_store::state::State; use miden_node_utils::fee::{test_fee_params, test_protocol_config}; -use miden_protocol::batch::ProvenBatch; +use miden_protocol::ONE; +use miden_protocol::account::Account; use miden_protocol::block::{BlockHeader, BlockNumber, ValidatorConfig}; use miden_protocol::testing::random_secret_key::random_secret_key; use url::Url; use crate::mempool::{Mempool, MempoolConfig}; use crate::server::MempoolStats; -use crate::test_utils::batch::TransactionBatchConstructor; +use crate::test_utils::batch::mock_proven_batch_with_builder_transaction; use crate::test_utils::{MockAuthenticatedTxBuilder, MockProvenTxBuilder}; use crate::{ DEFAULT_BATCH_WORKERS, @@ -47,7 +48,7 @@ fn mempool_stats_track_uncommitted_work_and_the_canonical_tip() { assert_eq!(stats.proposed_batches, 1); assert_eq!(stats.proven_batches, 0); - mempool.commit_batch(Arc::new(ProvenBatch::mocked_from_transactions([ + mempool.commit_batch(Arc::new(mock_proven_batch_with_builder_transaction([ tx.raw_proven_transaction() ]))); let stats = MempoolStats::from_mempool(&mempool); @@ -68,11 +69,15 @@ fn mempool_stats_track_uncommitted_work_and_the_canonical_tip() { assert_eq!(stats.proven_batches, 0); } -#[tokio::test] +#[tokio::test(flavor = "multi_thread")] async fn block_producer_starts_with_store_state() { let data_directory = tempfile::tempdir().expect("tempdir should be created"); - bootstrap_store(data_directory.path()); + let account_file = crate::test_utils::mock_collection_account(); + let mut deployed_account = account_file.account.clone(); + deployed_account.set_nonce(ONE).unwrap(); + bootstrap_store(data_directory.path(), deployed_account); let (state, block_writer, proof_writer) = State::for_tests(data_directory.path()).await; + let shutdown = miden_node_utils::shutdown::CancellationToken::new(); let block_producer = Sequencer { state, @@ -89,19 +94,27 @@ async fn block_producer_starts_with_store_state() { max_concurrent_proofs: DEFAULT_MAX_CONCURRENT_PROOFS, mempool_tx_capacity: NonZeroUsize::new(100).unwrap(), batch_workers: DEFAULT_BATCH_WORKERS, + pass_through_account: account_file, + builder_account_id: + miden_protocol::testing::account_id::ACCOUNT_ID_REGULAR_PRIVATE_ACCOUNT_UPDATABLE_CODE + .try_into() + .unwrap(), } - .spawn(miden_node_utils::shutdown::CancellationToken::new()) + .start(shutdown.clone()) + .await .unwrap(); let status = block_producer.api().status().await; assert_eq!(status.status, "connected"); assert_eq!(status.chain_tip, BlockNumber::GENESIS); + shutdown.cancel(); + block_producer.wait().await.unwrap(); } -fn bootstrap_store(path: &std::path::Path) { +fn bootstrap_store(path: &std::path::Path, account: Account) { let signer = random_secret_key(); let genesis_state = GenesisState::new( - vec![], + vec![account], test_fee_params(), 1, ValidatorConfig::new(vec![signer.public_key()], 1).unwrap(), diff --git a/crates/block-producer/src/test_utils/batch.rs b/crates/block-producer/src/test_utils/batch.rs index 62948bdf17..8683401f36 100644 --- a/crates/block-producer/src/test_utils/batch.rs +++ b/crates/block-producer/src/test_utils/batch.rs @@ -12,6 +12,17 @@ use miden_protocol::transaction::{ use crate::test_utils::MockProvenTxBuilder; +/// Builds a mocked proven batch with a final batch builder transaction. +pub fn mock_proven_batch_with_builder_transaction<'tx>( + txs: impl IntoIterator, +) -> ProvenBatch { + let builder_transaction = MockProvenTxBuilder::with_account_index(u32::MAX).build(); + let mut txs = txs.into_iter().collect::>(); + txs.push(&builder_transaction); + + ProvenBatch::mocked_from_transactions(txs) +} + pub trait TransactionBatchConstructor { /// Builds a **mocked** [`ProvenBatch`] from the given transactions, which most likely violates /// some of the rules of actual transaction batches. diff --git a/crates/block-producer/src/validator/mod.rs b/crates/block-producer/src/validator/mod.rs index dd621c9217..47fd3044cd 100644 --- a/crates/block-producer/src/validator/mod.rs +++ b/crates/block-producer/src/validator/mod.rs @@ -82,7 +82,7 @@ impl BlockProducerValidatorClient { genesis: Word, validators: &ValidatorConfig, ) -> anyhow::Result<()> { - let client = self.clients.first().context("collector deployment requires a validator")?; + let client = self.clients.first().context("transaction validation requires a validator")?; let key = (|| async { client.clone().get_transaction_encryption_key(()).await }) .retry(retry::exponential_bounded( Duration::from_millis(100), diff --git a/docs/external/src/network-operator/sequencer.md b/docs/external/src/network-operator/sequencer.md index 40c15e713b..e1e4b6aa63 100644 --- a/docs/external/src/network-operator/sequencer.md +++ b/docs/external/src/network-operator/sequencer.md @@ -10,14 +10,15 @@ produces blocks, serves public RPC, and connects to the validator and network tr ## Fee Collection -A dedicated immutable fee collector account combines transaction fees into a single P2ID note targeting the batch -builder's wallet account. +The sequencer uses a dedicated immutable fee collector account to send transaction fees to the batch builder's wallet +account. The fee collector transforms the fees into a single P2ID note targeting the wallet account. This process will need to change when we support fees paid in non-native tokens. For now, it provides a simple way to collect fees while avoiding race conditions on the receiving wallet account. -Use `miden-node fee-collector create` to create the account and `miden-node fee-collector deploy` to deploy it. -Deployment creates a dedicated block and therefore the validators must be running to sign this block. +The fee collector account must be created and deployed before the sequencer can start. Use +`miden-node fee-collector create` to create the account and `miden-node fee-collector deploy` to deploy it. Deployment +creates a dedicated block and therefore the validators must be running to sign this block. The collector account is fairly low-risk. It only needs to exist and is immutable once deployed. Keep the generated signing key to authorize transactions. A new collector can be trivially created and redeployed so backup isn't a strong @@ -33,12 +34,17 @@ miden-node sequencer \ --validator.url http://validator-2:50101 \ --validator.url http://validator-3:50101 \ --ntx-builder.url http://ntx-builder:50301 \ + --batch.builder.wallet-account-id \ --rpc.network-tx-auth-header-value ``` Only the public RPC listener should be externally reachable. The validator, NTX builder, and prover URLs are trusted internal services. +The wallet account receives batch-building fees. The sequencer needs only its ID, not its signing key. The sequencer +loads its deployed collection account from the account file. The wallet's P2ID notes remain unspent until a separate +service collects them. + The network transaction auth value is a shared secret used to authorize network transaction submissions. It must match the NTX builder's `--rpc.auth-header-value`; otherwise, the sequencer rejects network transactions from the builder. diff --git a/scripts/bench-local.sh b/scripts/bench-local.sh index a8d6736407..87b691698e 100755 --- a/scripts/bench-local.sh +++ b/scripts/bench-local.sh @@ -37,6 +37,8 @@ USE_REMOTE_PROVER="${USE_REMOTE_PROVER:-0}" CONCURRENCY="${CONCURRENCY:-8}" WAIT_BLOCKS="${WAIT_BLOCKS:-30}" RUN_DIR="${RUN_DIR:-./bench-local-run}" +# Send collected fees to an arbitrary non-existent account ID for now. +BATCH_BUILDER_WALLET_ACCOUNT_ID="${BATCH_BUILDER_WALLET_ACCOUNT_ID:-0xcc0000000000dd010000ee000000ff}" # Insecure, hard-coded local dev validator signing key and its public key (committed at # genesis). Generate a fresh pair with `miden-validator keygen`. VALIDATOR_SIGNING_KEY_HEX="${VALIDATOR_SIGNING_KEY_HEX:-0101010101010101010101010101010101010101010101010101010101010101}" @@ -184,6 +186,7 @@ start_bg node miden-node sequencer \ --rpc.listen "127.0.0.1:$RPC_PORT" \ --validator.url "http://127.0.0.1:$VALIDATOR_PORT" \ --ntx-builder.url "http://127.0.0.1:$NTX_PORT" \ + --batch.builder.wallet-account-id "$BATCH_BUILDER_WALLET_ACCOUNT_ID" \ --batch.max-txs 64 \ --block.max-batches 16 \ --block.interval 2s \ diff --git a/scripts/run-node.sh b/scripts/run-node.sh index b4d6293570..f1e820de58 100755 --- a/scripts/run-node.sh +++ b/scripts/run-node.sh @@ -5,6 +5,8 @@ set -euo pipefail SKIP_BOOTSTRAP="${SKIP_BOOTSTRAP:-false}" ENABLE_FULL_NODES="${ENABLE_FULL_NODES:-true}" EXTRA_ARGS="${EXTRA_ARGS:-}" +# Send collected fees to an arbitrary non-existent account ID for now. +BATCH_BUILDER_WALLET_ACCOUNT_ID="${BATCH_BUILDER_WALLET_ACCOUNT_ID:-0xcc0000000000dd010000ee000000ff}" # Shared secret authorizing the ntx-builder to submit network transactions to the sequencer's RPC. # Must match on both the sequencer (--rpc.network-tx-auth-header-value) and the ntx-builder # (--rpc.auth-header-value), otherwise network transactions are rejected with @@ -254,6 +256,7 @@ OTEL_RESOURCE_ATTRIBUTES="$(node_resource_attributes sequencer)" \ --validator.url "http://127.0.0.1:$VALIDATOR_1_PORT" \ --validator.url "http://127.0.0.1:$VALIDATOR_2_PORT" \ --ntx-builder.url "http://127.0.0.1:$NTX_BUILDER_PORT" \ + --batch.builder.wallet-account-id "$BATCH_BUILDER_WALLET_ACCOUNT_ID" \ --internal.listen "0.0.0.0:$SEQUENCER_INTERNAL_PORT" \ $EXTRA_ARGS & PIDS+=($!) From f27a0ce11f1999f1660e1d0484dd5f78d6eabdb8 Mon Sep 17 00:00:00 2001 From: Mirko von Leipzig <48352201+Mirko-von-Leipzig@users.noreply.github.com> Date: Fri, 18 Sep 2026 12:37:04 +0200 Subject: [PATCH 2/4] Build fee-free batches without a collector transaction --- .../block-producer/src/batch_builder/mod.rs | 138 +++++++++++++----- crates/block-producer/src/domain/batch.rs | 2 +- .../block-producer/src/mempool/graph/batch.rs | 25 +++- .../block-producer/src/mempool/graph/dag.rs | 4 + crates/block-producer/src/mempool/tests.rs | 83 +++++++---- .../src/mempool/tests/add_transaction.rs | 4 +- crates/block-producer/src/server/tests.rs | 4 +- crates/block-producer/src/test_utils/batch.rs | 13 +- 8 files changed, 194 insertions(+), 79 deletions(-) diff --git a/crates/block-producer/src/batch_builder/mod.rs b/crates/block-producer/src/batch_builder/mod.rs index 4d9388a40f..429ab592c3 100644 --- a/crates/block-producer/src/batch_builder/mod.rs +++ b/crates/block-producer/src/batch_builder/mod.rs @@ -354,54 +354,57 @@ impl BatchJob { .0 .expect("reference block header should exist"); - let protocol_config = view - .get_protocol_config(reference_block_header.protocol_config_commitment()) - .await - .map_err(StoreError::GetProtocolConfigFailed) - .map_err(BuildBatchError::FetchBatchInputsFailed)? - .expect("the reference block's protocol configuration should exist"); - let genesis = view - .get_block_header(Some(BlockNumber::GENESIS), false) - .await - .map_err(StoreError::GetBlockHeaderFailed) - .map_err(BuildBatchError::FetchBatchInputsFailed)? - .0 - .expect("the genesis block header should exist") - .commitment(); - let mut transactions: Vec<_> = selected .into_transactions() .into_iter() .map(|tx| tx.proven_transaction()) .collect(); - let pass_through = self.pass_through.clone(); - let executed_pass_through_tx = pass_through - .execute( - fee_notes, - reference_block_header.clone(), - protocol_config, - partial_blockchain.clone(), - ) + // A deployed collector must consume at least one note to prevent replay. + if !fee_notes.is_empty() { + let protocol_config = view + .get_protocol_config(reference_block_header.protocol_config_commitment()) + .await + .map_err(StoreError::GetProtocolConfigFailed) + .map_err(BuildBatchError::FetchBatchInputsFailed)? + .expect("the reference block's protocol configuration should exist"); + let genesis = view + .get_block_header(Some(BlockNumber::GENESIS), false) + .await + .map_err(StoreError::GetBlockHeaderFailed) + .map_err(BuildBatchError::FetchBatchInputsFailed)? + .0 + .expect("the genesis block header should exist") + .commitment(); + + let pass_through = self.pass_through.clone(); + let executed_pass_through_tx = pass_through + .execute( + fee_notes, + reference_block_header.clone(), + protocol_config, + partial_blockchain.clone(), + ) + .await + .map_err(BuildBatchError::BuildBatchFeeTransaction)?; + let inputs = executed_pass_through_tx.tx_inputs().clone(); + let pass_through_tx = spawn_blocking_in_current_span(move || { + PassThroughTransactionBuilder::prove(executed_pass_through_tx) + }) .await + .map_err(BuildBatchError::JoinError)? .map_err(BuildBatchError::BuildBatchFeeTransaction)?; - let inputs = executed_pass_through_tx.tx_inputs().clone(); - let pass_through_tx = spawn_blocking_in_current_span(move || { - PassThroughTransactionBuilder::prove(executed_pass_through_tx) - }) - .await - .map_err(BuildBatchError::JoinError)? - .map_err(BuildBatchError::BuildBatchFeeTransaction)?; - self.validator - .validate_transaction( - &pass_through_tx, - &inputs, - genesis, - reference_block_header.validator_config(), - ) - .await - .map_err(BuildBatchError::ValidateBatchFeeTransaction)?; - transactions.push(Arc::new(pass_through_tx)); + self.validator + .validate_transaction( + &pass_through_tx, + &inputs, + genesis, + reference_block_header.validator_config(), + ) + .await + .map_err(BuildBatchError::ValidateBatchFeeTransaction)?; + transactions.push(Arc::new(pass_through_tx)); + } ProposedBatch::new( transactions, @@ -502,7 +505,62 @@ mod tests { use std::future::pending; use std::time::Duration; + use miden_node_utils::genesis::GenesisBlock; + use miden_protocol::ONE; + use miden_protocol::block::{BlockSignatures, SignedBlock}; + use miden_testing::{Auth, MockChain}; + use miden_tx::LocalTransactionProver; + use super::*; + use crate::mempool::{Mempool, MempoolConfig}; + use crate::store::get_tx_inputs; + use crate::test_utils::mock_collection_account; + + #[tokio::test(flavor = "multi_thread")] + async fn builds_batches_without_fee_notes() -> anyhow::Result<()> { + let mut collector = mock_collection_account(); + collector.account.set_nonce(ONE)?; + let mut chain = MockChain::builder().verification_base_fee(0); + chain.add_account(collector.account.clone())?; + let wallet = chain.add_existing_wallet(Auth::basic_ecdsa())?; + let chain = chain.build()?; + let executed = chain.build_transaction(wallet.id()).build()?.execute().await?; + let transaction = LocalTransactionProver::default().prove(executed)?; + assert_eq!(transaction.output_notes().num_notes(), 0); + + let directory = tempfile::tempdir()?; + let genesis = chain.latest_block(); + let genesis = GenesisBlock::new( + SignedBlock::new( + genesis.header().clone(), + genesis.body().clone(), + BlockSignatures::new(Vec::new())?, + )?, + chain.protocol_config().clone(), + )?; + State::bootstrap(genesis, directory.path())?; + let (state, ..) = State::for_tests(directory.path()).await; + let inputs = get_tx_inputs(&state, &transaction).await?; + let transaction = + Arc::new(AuthenticatedTransaction::new_unchecked(Arc::new(transaction), inputs)?); + let mempool = Mempool::shared(BlockNumber::GENESIS, MempoolConfig::default()); + mempool.lock().unwrap().add_transaction(transaction)?; + let selected = mempool.lock().unwrap().select_any_batch().unwrap(); + let selected_id = selected.id().as_batch_id(); + let job = BatchJob { + state, + batch_prover: BatchProver::local(), + pass_through: PassThroughTransactionBuilder::new(wallet.id(), collector)?, + validator: BlockProducerValidatorClient::new(Vec::new(), Duration::from_secs(1))?, + mempool, + }; + + let proposed = Box::pin(job.get_batch_inputs(selected)).await?; + assert_eq!(proposed.id(), selected_id); + assert_eq!(proposed.transactions().len(), 1); + assert!(proposed.output_notes().is_empty()); + Ok(()) + } #[tokio::test] async fn abort_active_jobs_cancels_batch_jobs_without_waiting_for_completion() { diff --git a/crates/block-producer/src/domain/batch.rs b/crates/block-producer/src/domain/batch.rs index 0e2afe1905..7cdfbaaa5f 100644 --- a/crates/block-producer/src/domain/batch.rs +++ b/crates/block-producer/src/domain/batch.rs @@ -17,7 +17,7 @@ use miden_standards::note::TxFeeNote; /// Identifies a transaction selection in the batch graph. /// /// A sequencer-built batch has a different [`BatchId`] after the batch builder appends the fee -/// transaction. A user-proven batch keeps the same ID. +/// transaction. Batches without fee notes and user-proven batches keep the same ID. #[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] pub(crate) struct SelectedBatchId(BatchId); diff --git a/crates/block-producer/src/mempool/graph/batch.rs b/crates/block-producer/src/mempool/graph/batch.rs index bd9ba6e2e4..32c9841f08 100644 --- a/crates/block-producer/src/mempool/graph/batch.rs +++ b/crates/block-producer/src/mempool/graph/batch.rs @@ -154,22 +154,35 @@ impl BatchGraph { /// becomes a root. pub fn submit_proof(&mut self, proof: Arc) { let proof_id = proof.id(); + let selected_id = SelectedBatchId::from_batch_id(proof_id); + if self.inner.contains(&selected_id) { + self.insert_proof(selected_id, proof_id, proof); + return; + } + + // An appended fee transaction changes the batch ID. let (_builder_transaction, selected_transactions) = proof .transactions() .as_slice() .split_last() - .expect("a builder batch must contain a batch builder transaction"); - assert!( - !selected_transactions.is_empty(), - "a builder batch must contain at least one user transaction", - ); + .expect("a proven batch must contain a transaction"); + if selected_transactions.is_empty() { + return; + } let selected_id = SelectedBatchId::from_batch_id(BatchId::from_ids( selected_transactions .iter() .map(|transaction| (transaction.id(), transaction.account_id())), )); - self.insert_proof(selected_id, proof_id, proof); + // Do not match a late fee-free proof to a shorter selection. + if self + .inner + .get(&selected_id) + .is_some_and(|batch| !batch.collectible_fee_notes().is_empty()) + { + self.insert_proof(selected_id, proof_id, proof); + } } fn insert_proof( diff --git a/crates/block-producer/src/mempool/graph/dag.rs b/crates/block-producer/src/mempool/graph/dag.rs index a620af7151..8a23d8a925 100644 --- a/crates/block-producer/src/mempool/graph/dag.rs +++ b/crates/block-producer/src/mempool/graph/dag.rs @@ -275,6 +275,10 @@ where self.nodes.contains_key(node) } + pub(super) fn get(&self, node: &N::Id) -> Option<&N> { + self.nodes.get(node) + } + pub(super) fn get_mut(&mut self, node: &N::Id) -> Option<&mut N> { self.nodes.get_mut(node) } diff --git a/crates/block-producer/src/mempool/tests.rs b/crates/block-producer/src/mempool/tests.rs index 750bf6e827..0477cfb787 100644 --- a/crates/block-producer/src/mempool/tests.rs +++ b/crates/block-producer/src/mempool/tests.rs @@ -4,7 +4,7 @@ use std::time::Duration; use assert_matches::assert_matches; use miden_protocol::Word; use miden_protocol::block::{BlockHeader, BlockNumber}; -use miden_protocol::transaction::TransactionHeader; +use miden_protocol::transaction::{OutputNote, PublicOutputNote, TransactionHeader}; use pretty_assertions::assert_eq; use serial_test::serial; @@ -12,8 +12,9 @@ use super::*; use crate::mempool::graph::{TransactionGraph, TransactionRemoval}; use crate::test_utils::batch::{ TransactionBatchConstructor, - mock_proven_batch_with_builder_transaction, + mock_proven_batch_with_fee_collection, }; +use crate::test_utils::note::mock_fee_note; use crate::test_utils::{MockAuthenticatedTxBuilder, MockProvenTxBuilder, mock_account_id}; mod add_transaction; @@ -76,7 +77,7 @@ fn retained_committed_transactions_do_not_consume_capacity() { uut.add_transaction(first.clone()).unwrap(); uut.select_any_batch().unwrap(); - uut.commit_batch(Arc::new(mock_proven_batch_with_builder_transaction([ + uut.commit_batch(Arc::new(mock_proven_batch_with_fee_collection([ first.raw_proven_transaction() ]))); let block = uut.select_block(); @@ -241,7 +242,7 @@ fn children_of_failed_batches_are_ignored() { assert_eq!(uut, reference); let proven_batch = - Arc::new(mock_proven_batch_with_builder_transaction([txs[2].raw_proven_transaction()])); + Arc::new(mock_proven_batch_with_fee_collection([txs[2].raw_proven_transaction()])); uut.commit_batch(proven_batch); assert_eq!(uut, reference); } @@ -290,7 +291,7 @@ fn block_commit_reverts_expired_txns() { // Force the tx into the next block by batching it. uut.add_transaction(tx_to_commit.clone()).unwrap(); uut.select_any_batch().unwrap(); - uut.commit_batch(Arc::new(mock_proven_batch_with_builder_transaction([ + uut.commit_batch(Arc::new(mock_proven_batch_with_fee_collection([ tx_to_commit.raw_proven_transaction() ]))); @@ -309,7 +310,7 @@ fn block_commit_reverts_expired_txns() { // A reverted transaction behaves as if it never existed. reference.add_transaction(tx_to_commit.clone()).unwrap(); reference.select_any_batch().unwrap(); - reference.commit_batch(Arc::new(mock_proven_batch_with_builder_transaction([ + reference.commit_batch(Arc::new(mock_proven_batch_with_fee_collection([ tx_to_commit.raw_proven_transaction() ]))); reference.select_block(); @@ -364,7 +365,7 @@ fn pruned_committed_notes_are_authenticated_for_inflight_descendants() { assert_eq!(parent_batch.transactions(), std::slice::from_ref(&parent)); uut.add_transaction(child.clone()).unwrap(); - uut.commit_batch(Arc::new(mock_proven_batch_with_builder_transaction([ + uut.commit_batch(Arc::new(mock_proven_batch_with_fee_collection([ parent.raw_proven_transaction() ]))); @@ -413,8 +414,7 @@ fn rollbacks_of_already_proven_batches_are_ignored() { uut.add_transaction(txs[0].clone()).unwrap(); let batch = uut.select_any_batch().unwrap(); - let proof = - Arc::new(mock_proven_batch_with_builder_transaction([txs[0].raw_proven_transaction()])); + let proof = Arc::new(mock_proven_batch_with_fee_collection([txs[0].raw_proven_transaction()])); uut.commit_batch(Arc::clone(&proof)); let reference = uut.clone(); @@ -425,26 +425,59 @@ fn rollbacks_of_already_proven_batches_are_ignored() { #[test] fn proven_batch_id_resolves_to_selected_batch_id() { - let (mut uut, _) = Mempool::for_tests(); - let user_tx = MockProvenTxBuilder::with_account_index(50).build(); - let user_tx = Arc::new(MockAuthenticatedTxBuilder::new(user_tx).build()); + for append_fee_transaction in [false, true] { + let (mut uut, _) = Mempool::for_tests(); + let mut user_tx = MockProvenTxBuilder::with_account_index(50); + if append_fee_transaction { + user_tx = user_tx.output_notes(vec![OutputNote::Public( + PublicOutputNote::new(mock_fee_note(50)).unwrap(), + )]); + } + let user_tx = user_tx.build(); + let user_tx = Arc::new(MockAuthenticatedTxBuilder::new(user_tx).build()); + + uut.add_transaction(user_tx.clone()).unwrap(); + let selected = uut.select_any_batch().unwrap(); + let synthetic_tx = MockProvenTxBuilder::with_account_index(51).build(); + let mut transactions = vec![user_tx.raw_proven_transaction()]; + if append_fee_transaction { + transactions.push(&synthetic_tx); + } + let proof = Arc::new(ProvenBatch::mocked_from_transactions(transactions)); + assert_eq!(selected.id().as_batch_id() != proof.id(), append_fee_transaction); + + uut.commit_batch(Arc::clone(&proof)); + let block = uut.select_block(); + assert_eq!(block.batches.as_slice(), &[Arc::clone(&proof)]); - uut.add_transaction(user_tx.clone()).unwrap(); + uut.rollback_block(block.block_number); + assert_eq!(uut.unbatched_transactions_count(), 1); + + // A late proof must not restore a reverted batch. + uut.commit_batch(proof); + assert!(uut.select_block().batches.is_empty()); + assert!(uut.select_any_batch().is_some()); + } +} + +#[test] +fn late_fee_free_proof_does_not_match_a_shorter_selection() { + let (mut uut, _) = Mempool::for_tests(); + let txs = MockProvenTxBuilder::sequential(); + uut.add_transaction(Arc::clone(&txs[0])).unwrap(); + uut.add_transaction(Arc::clone(&txs[1])).unwrap(); let selected = uut.select_any_batch().unwrap(); - let synthetic_tx = MockProvenTxBuilder::with_account_index(51).build(); let proof = Arc::new(ProvenBatch::mocked_from_transactions([ - user_tx.raw_proven_transaction(), - &synthetic_tx, + txs[0].raw_proven_transaction(), + txs[1].raw_proven_transaction(), ])); - assert_ne!(selected.id().as_batch_id(), proof.id()); + uut.rollback_batch(selected.id()); - uut.commit_batch(Arc::clone(&proof)); - let block = uut.select_block(); - assert_eq!(block.batches.as_slice(), &[proof]); - - uut.rollback_block(block.block_number); - assert_eq!(uut.unbatched_transactions_count(), 1); - assert!(uut.select_any_batch().is_some()); + uut.config.batch_budget.transactions = 1; + let shorter = uut.select_any_batch().unwrap(); + assert_eq!(shorter.transactions(), &[Arc::clone(&txs[0])]); + uut.commit_batch(proof); + assert!(uut.select_block().batches.is_empty()); } // BLOCK FAILED TESTS @@ -458,7 +491,7 @@ fn block_failure_increments_tx_failures() { uut.add_transaction(reverted_txs[0].clone()).unwrap(); uut.select_any_batch().unwrap(); - uut.commit_batch(Arc::new(mock_proven_batch_with_builder_transaction([ + uut.commit_batch(Arc::new(mock_proven_batch_with_fee_collection([ reverted_txs[0].raw_proven_transaction() ]))); diff --git a/crates/block-producer/src/mempool/tests/add_transaction.rs b/crates/block-producer/src/mempool/tests/add_transaction.rs index a95c1700e5..944354b080 100644 --- a/crates/block-producer/src/mempool/tests/add_transaction.rs +++ b/crates/block-producer/src/mempool/tests/add_transaction.rs @@ -8,7 +8,7 @@ use miden_protocol::transaction::{OutputNote, PublicOutputNote}; use crate::errors::{MempoolSubmissionError, StateConflict}; use crate::mempool::Mempool; -use crate::test_utils::batch::mock_proven_batch_with_builder_transaction; +use crate::test_utils::batch::mock_proven_batch_with_fee_collection; use crate::test_utils::note::mock_fee_note; use crate::test_utils::{MockAuthenticatedTxBuilder, MockProvenTxBuilder, mock_account_id}; @@ -343,7 +343,7 @@ fn committed_fee_note_consumption_is_accepted() { uut.add_transaction(producer.clone()).unwrap(); uut.select_any_batch().unwrap(); - uut.commit_batch(Arc::new(mock_proven_batch_with_builder_transaction([ + uut.commit_batch(Arc::new(mock_proven_batch_with_fee_collection([ producer.raw_proven_transaction() ]))); let block = uut.select_block(); diff --git a/crates/block-producer/src/server/tests.rs b/crates/block-producer/src/server/tests.rs index 5d055ac553..dace9ceab0 100644 --- a/crates/block-producer/src/server/tests.rs +++ b/crates/block-producer/src/server/tests.rs @@ -13,7 +13,7 @@ use url::Url; use crate::mempool::{Mempool, MempoolConfig}; use crate::server::MempoolStats; -use crate::test_utils::batch::mock_proven_batch_with_builder_transaction; +use crate::test_utils::batch::mock_proven_batch_with_fee_collection; use crate::test_utils::{MockAuthenticatedTxBuilder, MockProvenTxBuilder}; use crate::{ DEFAULT_BATCH_WORKERS, @@ -48,7 +48,7 @@ fn mempool_stats_track_uncommitted_work_and_the_canonical_tip() { assert_eq!(stats.proposed_batches, 1); assert_eq!(stats.proven_batches, 0); - mempool.commit_batch(Arc::new(mock_proven_batch_with_builder_transaction([ + mempool.commit_batch(Arc::new(mock_proven_batch_with_fee_collection([ tx.raw_proven_transaction() ]))); let stats = MempoolStats::from_mempool(&mempool); diff --git a/crates/block-producer/src/test_utils/batch.rs b/crates/block-producer/src/test_utils/batch.rs index 8683401f36..76d2ac97ff 100644 --- a/crates/block-producer/src/test_utils/batch.rs +++ b/crates/block-producer/src/test_utils/batch.rs @@ -6,19 +6,26 @@ use miden_protocol::block::BlockNumber; use miden_protocol::transaction::{ InputNotes, OrderedTransactionHeaders, + OutputNote, ProvenTransaction, TransactionHeader, }; +use miden_standards::note::TxFeeNote; use crate::test_utils::MockProvenTxBuilder; -/// Builds a mocked proven batch with a final batch builder transaction. -pub fn mock_proven_batch_with_builder_transaction<'tx>( +/// Builds a mocked proven batch with a fee collection transaction if it contains fee notes. +pub fn mock_proven_batch_with_fee_collection<'tx>( txs: impl IntoIterator, ) -> ProvenBatch { let builder_transaction = MockProvenTxBuilder::with_account_index(u32::MAX).build(); let mut txs = txs.into_iter().collect::>(); - txs.push(&builder_transaction); + let fee_script_root = TxFeeNote::script_root(); + if txs.iter().flat_map(|tx| tx.output_notes().iter()).any( + |note| matches!(note, OutputNote::Public(note) if note.recipient().script().root() == fee_script_root), + ) { + txs.push(&builder_transaction); + } ProvenBatch::mocked_from_transactions(txs) } From 9cf42001e25ffffc57213c73eff97c2ae259b7d2 Mon Sep 17 00:00:00 2001 From: Mirko von Leipzig <48352201+Mirko-von-Leipzig@users.noreply.github.com> Date: Fri, 18 Sep 2026 14:34:07 +0200 Subject: [PATCH 3/4] Use the fee collector account name consistently --- bin/node/src/commands/modes.rs | 4 ++-- crates/block-producer/src/batch_builder/mod.rs | 4 ++-- crates/block-producer/src/server/mod.rs | 8 ++++---- crates/block-producer/src/server/tests.rs | 2 +- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/bin/node/src/commands/modes.rs b/bin/node/src/commands/modes.rs index 636193aede..3622abe4e2 100644 --- a/bin/node/src/commands/modes.rs +++ b/bin/node/src/commands/modes.rs @@ -85,7 +85,7 @@ impl SequencerCommand { self.log_starting(); let runtime = self.runtime.runtime_config(&self.store); self.block_producer.validate()?; - let collection_account = self.fee_collector.read(&runtime.data_directory)?; + let fee_collector_account = self.fee_collector.read(&runtime.data_directory)?; let network_tx_auth = self.runtime.rpc.network_tx_auth()?; let (validator_clients, validator_monitors) = self.external_services.validator_clients_and_monitors()?; @@ -122,7 +122,7 @@ impl SequencerCommand { mempool_tx_capacity: self.block_producer.mempool.tx_capacity, batch_workers: self.block_producer.batch.workers, builder_account_id: self.block_producer.builder.wallet_account_id, - pass_through_account: collection_account, + fee_collector_account, } .start(shutdown.clone()) .await diff --git a/crates/block-producer/src/batch_builder/mod.rs b/crates/block-producer/src/batch_builder/mod.rs index 429ab592c3..987151c986 100644 --- a/crates/block-producer/src/batch_builder/mod.rs +++ b/crates/block-producer/src/batch_builder/mod.rs @@ -96,13 +96,13 @@ impl BatchBuilder { batch_prover_url: Option, intervals: BatchIntervals, builder_account_id: AccountId, - pass_through_account: AccountFile, + fee_collector_account: AccountFile, validator: BlockProducerValidatorClient, ) -> anyhow::Result { let batch_prover = batch_prover_url.map_or(Ok(BatchProver::local()), BatchProver::remote)?; let pass_through = - PassThroughTransactionBuilder::new(builder_account_id, pass_through_account)?; + PassThroughTransactionBuilder::new(builder_account_id, fee_collector_account)?; Ok(Self { active_jobs: JoinSet::new(), diff --git a/crates/block-producer/src/server/mod.rs b/crates/block-producer/src/server/mod.rs index 04ed92f61e..c85d23b494 100644 --- a/crates/block-producer/src/server/mod.rs +++ b/crates/block-producer/src/server/mod.rs @@ -103,8 +103,8 @@ pub struct Sequencer { /// The batch builder account that receives collected fees. pub builder_account_id: AccountId, - /// The deployed pass-through account and its signing key. - pub pass_through_account: AccountFile, + /// The deployed fee collector account and its signing key. + pub fee_collector_account: AccountFile, } // BLOCK PRODUCER @@ -115,7 +115,7 @@ impl Sequencer { pub async fn start(mut self, shutdown: CancellationToken) -> Result { info!(target: LOG_TARGET, "Initializing sequencer"); let state = self.state; - crate::fee_collector::load_deployed_collector(&state, &mut self.pass_through_account) + crate::fee_collector::load_deployed_collector(&state, &mut self.fee_collector_account) .await?; let validator = BlockProducerValidatorClient::new(self.validator_urls.clone(), self.validator_timeout)?; @@ -132,7 +132,7 @@ impl Sequencer { self.batch_prover_url, batch_intervals, self.builder_account_id, - self.pass_through_account, + self.fee_collector_account, validator, )?; let api_config = BlockProducerApiConfig { diff --git a/crates/block-producer/src/server/tests.rs b/crates/block-producer/src/server/tests.rs index dace9ceab0..cdde9c1e25 100644 --- a/crates/block-producer/src/server/tests.rs +++ b/crates/block-producer/src/server/tests.rs @@ -94,7 +94,7 @@ async fn block_producer_starts_with_store_state() { max_concurrent_proofs: DEFAULT_MAX_CONCURRENT_PROOFS, mempool_tx_capacity: NonZeroUsize::new(100).unwrap(), batch_workers: DEFAULT_BATCH_WORKERS, - pass_through_account: account_file, + fee_collector_account: account_file, builder_account_id: miden_protocol::testing::account_id::ACCOUNT_ID_REGULAR_PRIVATE_ACCOUNT_UPDATABLE_CODE .try_into() From b85b39bdce78eafcb441885182076ccea4bdee3e Mon Sep 17 00:00:00 2001 From: Mirko von Leipzig <48352201+Mirko-von-Leipzig@users.noreply.github.com> Date: Fri, 18 Sep 2026 14:47:42 +0200 Subject: [PATCH 4/4] Use fee collector names during batch building --- .../block-producer/src/batch_builder/mod.rs | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/crates/block-producer/src/batch_builder/mod.rs b/crates/block-producer/src/batch_builder/mod.rs index 987151c986..e7949ccf79 100644 --- a/crates/block-producer/src/batch_builder/mod.rs +++ b/crates/block-producer/src/batch_builder/mod.rs @@ -29,7 +29,7 @@ use url::Url; use crate::domain::batch::{SelectedBatch, SelectedBatchId}; use crate::errors::{BuildBatchError, StoreError}; -use crate::fee_collector::PassThroughTransactionBuilder; +use crate::fee_collector::FeeCollectorTransactionBuilder; use crate::mempool::SharedMempool; use crate::validator::BlockProducerValidatorClient; use crate::{COMPONENT, LOG_TARGET}; @@ -55,7 +55,7 @@ pub struct BatchBuilder { /// /// If not provided, a local batch prover is used. batch_prover: BatchProver, - pass_through: PassThroughTransactionBuilder, + fee_collector: FeeCollectorTransactionBuilder, validator: BlockProducerValidatorClient, state: Arc, } @@ -101,15 +101,15 @@ impl BatchBuilder { ) -> anyhow::Result { let batch_prover = batch_prover_url.map_or(Ok(BatchProver::local()), BatchProver::remote)?; - let pass_through = - PassThroughTransactionBuilder::new(builder_account_id, fee_collector_account)?; + let fee_collector = + FeeCollectorTransactionBuilder::new(builder_account_id, fee_collector_account)?; Ok(Self { active_jobs: JoinSet::new(), num_workers, intervals, batch_prover, - pass_through, + fee_collector, validator, state, }) @@ -181,7 +181,7 @@ impl BatchBuilder { state: self.state.clone(), mempool, batch_prover: self.batch_prover.clone(), - pass_through: self.pass_through.clone(), + fee_collector: self.fee_collector.clone(), validator: self.validator.clone(), }; @@ -265,7 +265,7 @@ impl BatchBuilder { struct BatchJob { state: Arc, batch_prover: BatchProver, - pass_through: PassThroughTransactionBuilder, + fee_collector: FeeCollectorTransactionBuilder, validator: BlockProducerValidatorClient, mempool: SharedMempool, } @@ -377,8 +377,8 @@ impl BatchJob { .expect("the genesis block header should exist") .commitment(); - let pass_through = self.pass_through.clone(); - let executed_pass_through_tx = pass_through + let fee_collector = self.fee_collector.clone(); + let executed_fee_collection_tx = fee_collector .execute( fee_notes, reference_block_header.clone(), @@ -387,23 +387,23 @@ impl BatchJob { ) .await .map_err(BuildBatchError::BuildBatchFeeTransaction)?; - let inputs = executed_pass_through_tx.tx_inputs().clone(); - let pass_through_tx = spawn_blocking_in_current_span(move || { - PassThroughTransactionBuilder::prove(executed_pass_through_tx) + let inputs = executed_fee_collection_tx.tx_inputs().clone(); + let fee_collection_tx = spawn_blocking_in_current_span(move || { + FeeCollectorTransactionBuilder::prove(executed_fee_collection_tx) }) .await .map_err(BuildBatchError::JoinError)? .map_err(BuildBatchError::BuildBatchFeeTransaction)?; self.validator .validate_transaction( - &pass_through_tx, + &fee_collection_tx, &inputs, genesis, reference_block_header.validator_config(), ) .await .map_err(BuildBatchError::ValidateBatchFeeTransaction)?; - transactions.push(Arc::new(pass_through_tx)); + transactions.push(Arc::new(fee_collection_tx)); } ProposedBatch::new( @@ -550,7 +550,7 @@ mod tests { let job = BatchJob { state, batch_prover: BatchProver::local(), - pass_through: PassThroughTransactionBuilder::new(wallet.id(), collector)?, + fee_collector: FeeCollectorTransactionBuilder::new(wallet.id(), collector)?, validator: BlockProducerValidatorClient::new(Vec::new(), Duration::from_secs(1))?, mempool, };