Move batch building parts in the node to miden-base

[PhilippGackstatter]

What should be done?

As a first step for creating the batch kernel in miden-base, we can move the existing Rust parts of batch building from miden-node to miden-base.

How should it be done?

This would be TransactionBatch and some other types. As part of this, we could rename this to ProvenBatch. I have roughly this in mind:

#[derive(Debug, Clone)]
pub enum BatchError { /* ... */ }

#[derive(Debug, Clone)]
pub struct ProposedBatch {
    transactions: Vec<ProvenTransaction>,
    // TODO: Add NoteAuthenticationInfo.
}

impl ProposedBatch {
    pub fn new(txs: Vec<ProvenTransaction>) -> Result<Self, BatchError> {
        Ok(Self { transactions: txs })
    }
}

#[derive(Debug, Clone, PartialEq, Eq)]
pub struct ProvenBatch {
    id: BatchId,
    updated_accounts: BTreeMap<AccountId, AccountUpdate>,
    input_notes: Vec<InputNoteCommitment>,
    output_notes_smt: BatchNoteTree,
    output_notes: Vec<OutputNote>,
}

impl ProvenBatch {
    pub fn prove(proposed_batch: ProposedBatch) -> Result<Self, BatchError> {
        todo!()
    }
}

This means moving BatchId, OutputNoteTracker and miden_node_block_producer::batch_builder::batch::AccountUpdate to miden-base, but that doesn't seem like an issue. We might also need a copy of NoteAuthenticationInfo in miden-base, which is a type defined in a proto file in the node.

When is this task done?

When the above-mentioned types have been moved to miden-base.

Additional context

No response

[bobbinth]

Looks good! This largely aligns with how I was thinking about it as well. A few comments:

#[derive(Debug, Clone)]
pub struct ProposedBatch {
    transactions: Vec<ProvenTransaction>,
    // TODO: Add NoteAuthenticationInfo.
}

We may want to tract transactions as Vec<Arc<ProvenTransactions>> because cloning transactions may be expensive, and mempool in the node may not wan to transfer ownership of these transactions to the batch building.

#[derive(Debug, Clone, PartialEq, Eq)]
pub struct ProvenBatch {
    id: BatchId,
    updated_accounts: BTreeMap<AccountId, AccountUpdate>,
    input_notes: Vec<InputNoteCommitment>,
    output_notes_smt: BatchNoteTree,
    output_notes: Vec<OutputNote>,
}

We already have TxAccountUpdate and BlockAccountUpdate in miden-base - so, AccountUpdate may need to be BatchAccountUpdate. But I also wonder if there is a way to "normalize" these somehow to reduce the number of structs.

Also, and this may be a longer term goal, maybe there is a different way to track this info to retain more information about the transactions that went into the batch. Maybe something like:

pub struct ProvenBatch {
    id: BatchId,
    transactions: Vec<VerifiedTransaction>,
    output_notes_smt: BatchNoteTree,
}

Where VerifiedTransaction would contain the information needed to iterate over account updates and input/output notes of the batch.

impl ProvenBatch {
    pub fn prove(proposed_batch: ProposedBatch) -> Result<Self, BatchError> {
        todo!()
    }
}

I was thinking of having a dedicated struct for this (rather than putting prove() on ProvenTransaction - e.g., something like this:

impl LocalBatchBuilder {
    pub fn build(proposed_batch: ProposedBatch) -> Result<ProvenBatch, BatchError> {
        todo!()
    }
}

What's the benefit of going with the ProvenBatch::prove() approach?