-
Notifications
You must be signed in to change notification settings - Fork 144
feat: batch kernel skeleton #2904
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
Changes from all commits
1cbd5f9
0411259
ee2df53
c508740
9ed96f7
52adb85
ee3f559
e3d15c5
ba9af23
0d20126
11f0760
7a4afcb
26c044b
7c72675
b84ea28
5e25d94
d812d4d
7f3a5df
fc7d855
79165e9
e9c34d3
763a6a7
9c91b70
727ee9b
eaa71d4
473d20a
5d1b7d7
30fb74a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| # MAIN | ||
| # ================================================================================================= | ||
|
|
||
| #! Batch kernel program (skeleton). | ||
| #! | ||
| #! A transaction batch groups a set of independently-proven transactions so they can later be | ||
| #! aggregated into a block by the block kernel. This program defines the public input/output | ||
| #! contract that the batch kernel will eventually verify, but currently does not yet perform | ||
| #! any verification: it drops its inputs and exits, leaving the all-zero word output region as the | ||
| #! stack's initial padding zeros. | ||
| #! | ||
| #! Inputs: [ | ||
| #! BLOCK_COMMITMENT, | ||
| #! BATCH_ID, | ||
| #! pad(8), | ||
| #! ] | ||
|
Comment on lines
+12
to
+16
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if this should also include:
Also, I would maybe put the
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. AFAIU, In that case I'd also wait until we start working on the block kernel.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess the suggestion for So, it will probably make sense to also do this for batches, but we can probably also do this when we get there. |
||
| #! | ||
| #! Outputs: [ | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the original intention here was for the batch kernel to output the root of the This way, the block kernel would have to do much less work and could aggregate the individual I think the reason we don't currently do this at block level, is because we decided to also have note erasure at block level, and so if the block erases on of the batch's notes, the tree root would become stale and would have to be updated. Doing this update is probably still better than recomputing the entire tree from scratch, but we may want to think about that. To start simple, I see two routes:
I think the first route is a bit closer to the end goal, so I'd go with that one. Just a suggestion, though. I think I'm skeptical of the usefulness of note erasure at block level, given that batches are already incentivized to do as much note erasure as possible, and it just adds extra complexity.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Agreed, my guess is that this will rarely if ever be used in practice. I'd be in favor of dropping it. Created #3008
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yes I know, I should have maybe mentioned this in the comments that the first version would just have a commitment to all output notes instead. Mostly because, AFAICS, the So what I'd suggest is:
WDYT? If so I'll create an issue for 2.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Does "first version" mean the version after this (or some subsequent) PR or the version we go to mainnet with? If it's the former, then I'd already setup the outputs to reference what we want it to be, i.e. Your second step will basically imply removing note erasure support at the block level, but I think that's fine to do for now.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would have Also, agree with the above conclusion that it is fine to disable not erasure at the block level for now. We may find a way to make it work in the future (assuming we want to) with batch kernel outputting
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
One of the follow-up PRs. Mainnet version should have
ATM with this PR these are all just stack comment names anyway. As mentioned in another comment, we're not wiring anything up in this PR, all of which will be done in a follow-up. So sure, I can change the name of the stack comment to say Anyway as mentioned above, we don't even have the construction of
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Makes sense. One correction though: we do already have
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes but it's not constructed anywhere meaningfully as part of building a batch
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (unless I missed it)
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. renamed here: 763a6a7 |
||
| #! INPUT_NOTES_COMMITMENT, | ||
| #! BATCH_NOTE_TREE_ROOT, | ||
| #! batch_expiration_block_num, | ||
| #! pad(7), | ||
| #! ] | ||
|
Comment on lines
+17
to
+23
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A couple of potential modifications here:
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I would keep things simple for now, especially since we don't have the block kernel even in its skeleton form. Once that starts taking shape, we'd be in a better position to know exactly what the batch kernel should output. |
||
| #! | ||
| #! Where: | ||
| #! - BLOCK_COMMITMENT is the commitment of the batch's reference block. | ||
| #! - BATCH_ID is the batch's `BatchId`, the commitment to its transactions. | ||
| #! - INPUT_NOTES_COMMITMENT will be the sequential hash over every transaction's input note | ||
| #! commitments. In this skeleton it is the empty word. | ||
| #! - BATCH_NOTE_TREE_ROOT will be the root of the batch note tree built over every transaction's | ||
| #! output notes. In this skeleton it is the empty word. | ||
| #! - batch_expiration_block_num will be the minimum of every transaction's | ||
| #! `expiration_block_num`. In this skeleton it is zero. | ||
| #! | ||
| proc main | ||
| dropw dropw | ||
| end | ||
|
mmagician marked this conversation as resolved.
|
||
|
|
||
| begin | ||
| exec.main | ||
| end | ||
|
Comment on lines
+35
to
+41
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I know this is meant to be an initial skeleton - but I wonder if we should push it one step further and instead of just using empty words everywhere, set things to real inputs and real outputs (even if the output just come from the advice stack for now).
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will be all wired up in a follow-up PR which is already in draft. |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,109 @@ | ||
| use alloc::vec::Vec; | ||
|
|
||
| use miden_core::program::Kernel; | ||
|
|
||
| use crate::batch::{BatchId, ProposedBatch}; | ||
| use crate::block::BlockNumber; | ||
| use crate::utils::serde::Deserializable; | ||
| use crate::utils::sync::LazyLock; | ||
| use crate::vm::{AdviceInputs, Program, ProgramInfo, StackInputs, StackOutputs}; | ||
| use crate::{Felt, Word}; | ||
|
|
||
| // CONSTANTS | ||
| // ================================================================================================ | ||
|
|
||
| static KERNEL_MAIN: LazyLock<Program> = LazyLock::new(|| { | ||
| let bytes = include_bytes!(concat!(env!("OUT_DIR"), "/assets/kernels/batch_kernel.masb")); | ||
| Program::read_from_bytes(bytes).expect("failed to deserialize batch kernel runtime") | ||
| }); | ||
|
|
||
| // BATCH KERNEL | ||
| // ================================================================================================ | ||
|
|
||
| /// The batch kernel program: an executable Miden program that proves a batch of transactions. | ||
| /// | ||
| /// The kernel takes `[BLOCK_COMMITMENT, BATCH_ID]` as public inputs and emits | ||
| /// `[INPUT_NOTES_COMMITMENT, BATCH_NOTE_TREE_ROOT, batch_expiration_block_num]`. See | ||
| /// `asm/kernels/batch/main.masm` for the input/output contract. | ||
| pub struct BatchKernel; | ||
|
|
||
| impl BatchKernel { | ||
| // KERNEL SOURCE CODE | ||
| // -------------------------------------------------------------------------------------------- | ||
|
|
||
| /// Returns the executable batch kernel program loaded from the build's `OUT_DIR`. | ||
| pub fn main() -> Program { | ||
| KERNEL_MAIN.clone() | ||
| } | ||
|
|
||
| /// Returns [`ProgramInfo`] for the batch kernel program. | ||
| /// | ||
| /// The batch kernel does not expose syscalls, so the associated [`Kernel`] is empty. | ||
| pub fn program_info() -> ProgramInfo { | ||
| ProgramInfo::new(Self::main().hash(), Kernel::default()) | ||
| } | ||
|
|
||
| // INPUT BUILDERS | ||
| // -------------------------------------------------------------------------------------------- | ||
|
|
||
| /// Transforms the provided [`ProposedBatch`] into the stack and advice inputs needed to | ||
| /// execute the batch kernel. | ||
| pub fn prepare_inputs(proposed_batch: &ProposedBatch) -> (StackInputs, AdviceInputs) { | ||
| let block_commitment = proposed_batch.reference_block_header().commitment(); | ||
| let batch_id = proposed_batch.id(); | ||
|
|
||
| let stack_inputs = Self::build_input_stack(block_commitment, batch_id); | ||
| let advice_inputs = Self::build_advice_inputs(proposed_batch); | ||
|
|
||
| (stack_inputs, advice_inputs) | ||
| } | ||
|
|
||
| /// Returns the stack with the public inputs required by the batch kernel. | ||
| /// | ||
| /// The initial stack is: | ||
| /// | ||
| /// ```text | ||
| /// [BLOCK_COMMITMENT, BATCH_ID, pad(8)] | ||
| /// ``` | ||
| /// | ||
| /// Where: | ||
| /// - `BLOCK_COMMITMENT` is the commitment of the batch's reference block. | ||
| /// - `BATCH_ID` is the batch's [`BatchId`]. | ||
| pub fn build_input_stack(block_commitment: Word, batch_id: BatchId) -> StackInputs { | ||
| let mut inputs: Vec<Felt> = Vec::with_capacity(8); | ||
| inputs.extend_from_slice(block_commitment.as_elements()); | ||
| inputs.extend_from_slice(batch_id.as_word().as_elements()); | ||
|
|
||
| StackInputs::new(&inputs).expect("number of stack inputs should be <= 16") | ||
| } | ||
|
|
||
| /// Builds the stack with the expected batch kernel outputs. | ||
| /// | ||
| /// The output stack is defined as: | ||
| /// | ||
| /// ```text | ||
| /// [INPUT_NOTES_COMMITMENT, BATCH_NOTE_TREE_ROOT, batch_expiration_block_num] | ||
| /// ``` | ||
| pub fn build_output_stack( | ||
| input_notes_commitment: Word, | ||
| batch_note_tree_root: Word, | ||
| batch_expiration_block_num: BlockNumber, | ||
| ) -> StackOutputs { | ||
|
Comment on lines
+84
to
+91
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Docs and variable names might need an update if we change to |
||
| let mut outputs: Vec<Felt> = Vec::with_capacity(9); | ||
| outputs.extend_from_slice(input_notes_commitment.as_elements()); | ||
| outputs.extend_from_slice(batch_note_tree_root.as_elements()); | ||
| outputs.push(Felt::from(batch_expiration_block_num)); | ||
|
|
||
| StackOutputs::new(&outputs).expect("number of stack outputs should be <= 16") | ||
| } | ||
|
|
||
| // ADVICE BUILDER | ||
| // -------------------------------------------------------------------------------------------- | ||
|
|
||
| /// Builds the advice inputs (map + stack) consumed by the batch kernel. | ||
| /// | ||
| /// The skeleton kernel ignores its advice inputs, so this returns the default empty value. | ||
| fn build_advice_inputs(_proposed_batch: &ProposedBatch) -> AdviceInputs { | ||
| AdviceInputs::default() | ||
| } | ||
|
mmagician marked this conversation as resolved.
|
||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.