Skip to content

Commit

Permalink
feat: Disallow empty transaction batches
Browse files Browse the repository at this point in the history
  • Loading branch information
PhilippGackstatter committed Feb 3, 2025
1 parent f67799d commit 23c70d0
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 2 deletions.
11 changes: 9 additions & 2 deletions crates/miden-objects/src/batch/proposed_batch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ impl ProposedBatch {
/// - The number of output notes exceeds [`MAX_OUTPUT_NOTES_PER_BATCH`].
/// - Note that output notes that are consumed in the same batch as unauthenticated input
/// notes do not count.
/// - Any note is consumed twice.
/// - Any note is consumed more than once.
/// - Any note is created more than once.
/// - The number of account updates exceeds [`MAX_ACCOUNTS_PER_BATCH`].
/// - Note that any number of transactions against the same account count as one update.
Expand All @@ -104,16 +104,23 @@ impl ProposedBatch {
/// the chain MMR.
/// - The transactions in the proposed batch which update the same account are not correctly
/// ordered.
/// - The provided list of transactions is empty. An empty batch is pointless and would
/// potentially result in the same [`BatchId`] for two empty batches which would mean batch
/// IDs are no longer unique.
/// - There are duplicate transactions.
pub fn new(
transactions: Vec<Arc<ProvenTransaction>>,
block_header: BlockHeader,
chain_mmr: ChainMmr,
unauthenticated_note_proofs: BTreeMap<NoteId, NoteInclusionProof>,
) -> Result<Self, BatchProposeError> {
// Check for duplicate transactions.
// Check for empty or duplicate transactions.
// --------------------------------------------------------------------------------------------

if transactions.is_empty() {
return Err(BatchProposeError::EmptyTransactionBatch);
}

let mut transaction_set = BTreeSet::new();
for tx in transactions.iter() {
if !transaction_set.insert(tx.id()) {
Expand Down
3 changes: 3 additions & 0 deletions crates/miden-objects/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -468,6 +468,9 @@ pub enum BatchProposeError {
)]
TooManyAccountUpdates(usize),

#[error("transaction batch must contain at least one transaction")]
EmptyTransactionBatch,

#[error("transaction {transaction_id} appears twice in the proposed batch input")]
DuplicateTransaction { transaction_id: TransactionId },

Expand Down
14 changes: 14 additions & 0 deletions crates/miden-tx-batch-prover/src/tests/proposed_batch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,20 @@ fn setup_chain() -> TestSetup {
TestSetup { chain, account1, account2 }
}

/// Tests that a note created and consumed in the same batch are erased from the input and
/// output note commitments.
#[test]
fn empty_transaction_batch() -> anyhow::Result<()> {
let TestSetup { chain, .. } = setup_chain();
let block1 = chain.block_header(1);

let error = ProposedBatch::new(vec![], block1, chain.chain(), BTreeMap::default()).unwrap_err();

assert_matches!(error, BatchProposeError::EmptyTransactionBatch);

Ok(())
}

/// Tests that a note created and consumed in the same batch are erased from the input and
/// output note commitments.
#[test]
Expand Down

0 comments on commit 23c70d0

Please sign in to comment.