Skip to content

Commit

Permalink
feat: add stack validation for TransactionKernel::parse_output_stack (
Browse files Browse the repository at this point in the history
  • Loading branch information
Fumuran authored Jun 1, 2024
1 parent f457f40 commit 0a4aa8d
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 4 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
* [BREAKING] Split `Account` struct constructor into `new()` and `from_parts()` (#699).
* [BREAKING] Changed the encoding of inputs notes in the advice map for consumed notes. Now the data
is prefixed by its length, and the input and output notes encoding match (#707).
* Added validation for the output stack to make sure it was properly cleaned (#717).

## 0.3.0 (2024-05-14)

Expand Down
37 changes: 33 additions & 4 deletions miden-lib/src/transaction/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use miden_objects::{
transaction::{OutputNote, OutputNotes, TransactionOutputs},
utils::{group_slice_elements, serde::DeserializationError},
vm::{AdviceMap, ProgramInfo, StackInputs, StackOutputs},
Digest, Felt, TransactionOutputError, Word,
Digest, Felt, TransactionOutputError, Word, ZERO,
};
use miden_stdlib::StdLibrary;

Expand All @@ -30,6 +30,11 @@ pub use errors::{
TransactionEventParsingError, TransactionKernelError, TransactionTraceParsingError,
};

// CONSTANTS
// ================================================================================================

const ZERO_WORD: [Felt; 4] = [ZERO, ZERO, ZERO, ZERO];

// TRANSACTION KERNEL
// ================================================================================================

Expand Down Expand Up @@ -135,7 +140,14 @@ impl TransactionKernel {
/// - CNC is the commitment to the notes created by the transaction.
/// - FAH is the final account hash of the account that the transaction is being
/// executed against.
pub fn parse_output_stack(stack: &StackOutputs) -> (Digest, Digest) {
///
/// # Errors
/// Returns an error if:
/// - Words 3 and 4 on the stack are not ZERO.
/// - Overflow addresses are not empty.
pub fn parse_output_stack(
stack: &StackOutputs,
) -> Result<(Digest, Digest), TransactionOutputError> {
let output_notes_hash = stack
.get_stack_word(OUTPUT_NOTES_COMMITMENT_WORD_IDX * 4)
.expect("first word missing")
Expand All @@ -145,7 +157,24 @@ impl TransactionKernel {
.expect("second word missing")
.into();

(final_account_hash, output_notes_hash)
// make sure that the stack has been properly cleaned
if stack.get_stack_word(8).expect("third word missing") != ZERO_WORD {
return Err(TransactionOutputError::OutputStackInvalid(
"Third word on output stack should consist only of ZEROs".into(),
));
}
if stack.get_stack_word(12).expect("fourth word missing") != ZERO_WORD {
return Err(TransactionOutputError::OutputStackInvalid(
"Fourth word on output stack should consist only of ZEROs".into(),
));
}
if stack.has_overflow() {
return Err(TransactionOutputError::OutputStackInvalid(
"Output stack should not have overflow addresses".into(),
));
}

Ok((final_account_hash, output_notes_hash))
}

// TRANSACTION OUTPUT PARSER
Expand All @@ -169,7 +198,7 @@ impl TransactionKernel {
adv_map: &AdviceMap,
output_notes: Vec<OutputNote>,
) -> Result<TransactionOutputs, TransactionOutputError> {
let (final_acct_hash, output_notes_hash) = Self::parse_output_stack(stack);
let (final_acct_hash, output_notes_hash) = Self::parse_output_stack(stack)?;

// parse final account state
let final_account_data: &[Word] = group_slice_elements(
Expand Down
1 change: 1 addition & 0 deletions objects/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,7 @@ pub enum TransactionOutputError {
OutputNoteDataNotFound,
OutputNoteDataInvalid(NoteError),
OutputNotesCommitmentInconsistent(Digest, Digest),
OutputStackInvalid(String),
TooManyOutputNotes { max: usize, actual: usize },
}

Expand Down

0 comments on commit 0a4aa8d

Please sign in to comment.