Skip to content
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

fix: Remove caller ID workaround for root contexts #857

Merged
merged 1 commit into from
Sep 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 6 additions & 26 deletions miden-lib/asm/kernels/transaction/lib/account.masm
Original file line number Diff line number Diff line change
Expand Up @@ -528,12 +528,6 @@ end
#! Panics if
#! - index is out of bounds
export.get_procedure_info
# TODO: Fix VM caller == [0,0,0,0] bug and remove this check
# check if index == 255
dup push.255 neq
# => [is_255, index]

if.true
# check that index < number of procedures contained in the account code
dup exec.memory::get_num_account_procedures lt assert.err=ERR_PROC_INDEX_OUT_OF_BOUNDS
# => [index]
Expand All @@ -545,7 +539,6 @@ export.get_procedure_info
# load procedure information from memory
mem_load swap padw movup.4 mem_loadw
# => [PROC_ROOT, storage_offset]
end
end

#! Verifies that the procedure root is part of the account code
Expand All @@ -563,26 +556,13 @@ export.authenticate_procedure
emit.ACCOUNT_PUSH_PROCEDURE_INDEX_EVENT adv_push.1
# => [index, PROC_ROOT]

# get procedure info (PROC_ELEMENTS, storage_offset) from memory stored at index
# get procedure info (PROC_ROOT, storage_offset) from memory stored at index
exec.get_procedure_info
# In production:
# => [PROC_ELEMENTS, storage_offset, PROC_ROOT]
# During testing:
# => [255, PROC_ROOT]

# TODO: Fix VM caller == [0,0,0,0] bug and remove this check
# check if get_procedure_info returned 255
dup push.255 neq
# => [is_255, index, PROC_ROOT]

if.true
# verify that PROC_ROOT exists in memory at index
movup.4 movdn.8 assert_eqw.err=ERR_PROC_NOT_PART_OF_ACCOUNT_CODE
# => [storage_offset]
else
# drop PROC_ROOT
movdn.4 dropw drop push.0
end
# => [PROC_ROOT, storage_offset, PROC_ROOT]

# verify that PROC_ROOT exists in memory at index
movup.4 movdn.8 assert_eqw.err=ERR_PROC_NOT_PART_OF_ACCOUNT_CODE
# => [storage_offset]
end

#! Validates that the account seed, provided via the advice map, satisfies the seed requirements.
Expand Down
17 changes: 16 additions & 1 deletion miden-lib/asm/kernels/transaction/lib/prologue.masm
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,8 @@ proc.validate_new_account

if.true
# assert the fungible faucet reserved slot is initialized correctly (EMPTY_WORD)
or or or assertz.err=ERR_PROLOGUE_NEW_FUNGIBLE_FAUCET_NON_EMPTY_RESERVED_SLOT
# TODO: Switch to standard library implementation when available (miden-vm/#1483)
exec.is_empty_word_dropped not assertz.err=ERR_PROLOGUE_NEW_FUNGIBLE_FAUCET_NON_EMPTY_RESERVED_SLOT
# => []

# get the faucet reserved storage data slot type and entry arity
Expand All @@ -368,6 +369,7 @@ proc.validate_new_account
# empty SMT)
exec.constants::get_empty_smt_root assert_eqw.err=ERR_PROLOGUE_NEW_NON_FUNGIBLE_FAUCET_INVALID_RESERVED_SLOT
# => []
push.1001 drop # TODO: remove line, see miden-vm/#1122
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need to add this here specifically? Did we have an error with decorators previously?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it's what Bobbin mentioned yesterday (and related to the error mentioned in the comment). Essentially for asserts with errors the MAST root would be the same and thus the error returned by the VM could be unrelated to the actual error happening in the execution. I do not know the inner workings but adding this push and drop fixes it and we can assert for the actual error in the test.


# get the faucet reserved storage data slot type and entry arity
exec.account::get_faucet_storage_data_slot exec.account::get_storage_slot_type_info
Expand All @@ -390,6 +392,19 @@ proc.validate_new_account
# => []
end

#! Returns whether the input word is equal to EMPTY_WORD as a boolean value.
#!
#! This procedure drops the input word.
#! Stack: [INPUT_WORD]
#! Output: [is_empty]
#!
#! Where:
#! - INPUT_WORD is the word is compared with EMPTY_WORD.
#! - is_empty is a boolean value that is 1 if INPUT_WORD is equal to EMPTY_WORD, and 0 otherwise.
proc.is_empty_word_dropped
eq.0 swap eq.0 and swap eq.0 and swap eq.0 and
end

#! Validates that account procedures match account code commitment and saves procedure information
#! into memory.
#!
Expand Down
1 change: 1 addition & 0 deletions miden-lib/src/transaction/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,7 @@ impl TransactionKernel {
pub fn testing_assembler_with_mock_account() -> Assembler {
let assembler = Self::testing_assembler();
let library = AccountCode::mock_library(assembler.clone());

assembler.with_library(library).expect("failed to add mock account code")
}
}
2 changes: 1 addition & 1 deletion miden-tx/src/auth/tx_authenticator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use rand::Rng;
use vm_processor::{Digest, Felt, Word};

use super::signatures::get_falcon_signature;
use crate::error::AuthenticationError;
use crate::errors::AuthenticationError;

// TRANSACTION AUTHENTICATOR
// ================================================================================================
Expand Down
Loading
Loading