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

AccountCode refactor from MerkleTree to Sequential hash for offset based storage access #763

Merged
merged 35 commits into from
Jul 30, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
cd0ef51
Rebased from next
phklive Jul 18, 2024
7d43073
Added rust memory & added prologue memory test for account procs
phklive Jul 18, 2024
58d7ee6
Fixed dropw error in authenticate_account_origin
phklive Jul 18, 2024
acc9969
Merge branch 'next' into phklive-account-code-refactor
phklive Jul 19, 2024
f634073
Cleanup MASM code
phklive Jul 19, 2024
c712835
Cleanup Rust code
phklive Jul 19, 2024
98b4137
Merge branch 'next' into phklive-account-code-refactor
phklive Jul 22, 2024
40ee7da
Remove procedure_commitment() changed value to reference
phklive Jul 22, 2024
392987e
Account code_root to code_commitment first pass
phklive Jul 22, 2024
285e404
replaced with any()
phklive Jul 23, 2024
711d14e
Added procedure_roots() method on AccountCode
phklive Jul 23, 2024
3bd86a7
Added TransactionHostError for AccountProcedureIndexMapError
phklive Jul 23, 2024
cc7303d
Account code_root to code_commitment second pass
phklive Jul 23, 2024
9cc6661
Added AccountProcedure and TryFrom [Felt; 8]
phklive Jul 23, 2024
3b31161
MASM improvements + AccountProcedure + AccountCode updates
phklive Jul 23, 2024
cfe49ea
Merge branch 'next' into phklive-account-code-refactor
phklive Jul 24, 2024
f164bbb
Added requested changes
phklive Jul 24, 2024
08ca435
Added errors in MASM and rust
phklive Jul 24, 2024
c20d28e
Optimized procedure
phklive Jul 24, 2024
acf352e
Added in-kernel test fix
phklive Jul 26, 2024
86728c0
Updated changelog.md
phklive Jul 26, 2024
4d9eaf0
Merge branch 'next' into phklive-account-code-refactor
phklive Jul 26, 2024
b44c6c8
Updated validate_account_procedures to use pipe_double_words_to_memory
phklive Jul 26, 2024
be55ddf
Improved comments
phklive Jul 26, 2024
95cc6ba
Fix cargo doc
phklive Jul 26, 2024
55558ef
docs: added comments to AccountProcedureInfo
bobbinth Jul 27, 2024
828d734
docs: update comments for AccountCode
bobbinth Jul 27, 2024
069b265
docs: improved comments in the prologue MASM
bobbinth Jul 27, 2024
8fd9ecf
Added check for MAX_NUM_PROCEDURES
phklive Jul 29, 2024
c93c547
Fixed check, added constants, removed testing account_procs
phklive Jul 29, 2024
d073035
Updated account_procs, changed pub from module to struct, split prolo…
phklive Jul 30, 2024
2eaf813
lint
phklive Jul 30, 2024
ff5e8a5
Added doc comment for helper functions in account code
phklive Jul 30, 2024
32dbaab
Move num_procs down to prevent index out of bounds error
phklive Jul 30, 2024
68c419b
Merge branch 'next' into phklive-account-code-refactor
phklive Jul 30, 2024
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
7 changes: 4 additions & 3 deletions miden-lib/asm/kernels/transaction/api.masm
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,11 @@ proc.authenticate_account_origin

# assert that the caller is from the user context
exec.account::authenticate_procedure
# => [CALLER, ...]
# => [storage_offset, ...]

# drop the caller
dropw
# TODO: Use the storage_offset for storage access
# drop the storage_offset
drop
# => [...]
end

Expand Down
54 changes: 30 additions & 24 deletions miden-lib/asm/miden/kernels/tx/account.masm
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ proc.set_item_raw
# => [OLD_VALUE]
end

#! Sets an item in the account storage. Panics if
#! Sets an item in the account storage. Panics if
#! - the index is out of bounds
#! - the slot type is not value
#!
Expand Down Expand Up @@ -469,33 +469,39 @@ export.set_map_item.3
# => [OLD_MAP_ROOT, OLD_VALUE, ...]
end

#! Verifies that the procedure root is part of the account code Merkle tree. Panics if the
#! procedure root is not part of the account code Merkle tree.
#! Returns the procedure information
#! procedure root is not part of the account code.
#!
#! Stack: [PROC_ROOT]
#! Output: [PROC_ROOT]
#! Stack: [index, ...]
#! Output: [PROC_ELEMENTS, storage_offset, ...]
bobbinth marked this conversation as resolved.
Show resolved Hide resolved
#!
#! - PROC_ROOT is the hash of the procedure to authenticate.
export.authenticate_procedure.1
# load the account code root onto the stack
exec.memory::get_acct_code_root swapw
# => [PROC_ROOT, CODE_ROOT]

# load the index of the procedure root onto the advice stack, and move it to the operand stack
emit.ACCOUNT_PUSH_PROCEDURE_INDEX_EVENT adv_push.1 movdn.4
# => [PROC_ROOT, index, CODE_ROOT]

# push the depth of the code Merkle tree onto the stack
push.ACCOUNT_CODE_TREE_DEPTH movdn.4
# => [PROC_ROOT, depth, index, CODE_ROOT]
#! - PROC_ELEMENTS are field elements representing the procedure
#! - storage_offset is the procedure storage offset
export.get_procedure_info
# get procedure section ptr
push.2 mul exec.memory::get_account_procedures_section_offset add dup push.1 add swap
# => [proc_ptr, offset_ptr]

# verify the procedure exists in the account code Merkle tree
mtree_verify
# => [PROC_ROOT, depth, index, CODE_ROOT]
# load procedure information from memory
padw movup.4 mem_loadw movup.4 mem_load movdn.4
# => [PROC_ELEMENTS, storage_offset]
end

# drop accessory variables
movup.4 drop movup.4 drop swapw dropw
# => [PROC_ROOT]
#! Stack: [PROC_ROOT]
#! Output: [storage_offset]
export.authenticate_procedure
# load the index of the procedure onto the advice stack and move it to the operand stack
emit.ACCOUNT_PUSH_PROCEDURE_INDEX_EVENT adv_push.1
# => [index, PROC_ROOT]

# get the procedure info (ROOT, storage_offset) of the account procedure stored at the
# index we got above
exec.get_procedure_info
# => [PROC_ELEMENTS, storage_offset, PROC_ROOT]

# make sure stored proc root is the same as we got with the parameter
movup.4 movdn.8 assert_eqw.err=112
# => [storage_offset]
end

#! Validates that the account seed, provided via the advice map, satisfies the seed requirements.
Expand Down
13 changes: 13 additions & 0 deletions miden-lib/asm/miden/kernels/tx/memory.masm
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,9 @@ const.ACCT_CORE_DATA_SECTION_END_OFFSET=404
# The memory address at which the account storage slot type data beings
const.ACCT_STORAGE_SLOT_TYPE_DATA_OFFSET=405

# The memory address at which the account procedures section begins.
const.ACCT_PROCEDURES_SECTION_OFFSET=700
phklive marked this conversation as resolved.
Show resolved Hide resolved
bobbinth marked this conversation as resolved.
Show resolved Hide resolved

# INPUT NOTES DATA
# -------------------------------------------------------------------------------------------------

Expand Down Expand Up @@ -644,6 +647,16 @@ export.set_acct_nonce
drop movup.3 push.ACCT_ID_AND_NONCE_PTR mem_storew dropw
end

#! Returns the account procedures section offset.
#!
#! Stack: []
#! Output: [section_offset]
#!
#! - section_offset is the new account procedures section offset start.
export.get_account_procedures_section_offset
phklive marked this conversation as resolved.
Show resolved Hide resolved
push.ACCT_PROCEDURES_SECTION_OFFSET
end

#! Sets the code root of the account.
#!
#! Stack: [CODE_ROOT]
Expand Down
41 changes: 41 additions & 0 deletions miden-lib/asm/miden/kernels/tx/prologue.masm
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,46 @@ proc.validate_new_account
# => []
end

proc.validate_account_procedures
phklive marked this conversation as resolved.
Show resolved Hide resolved
# get code_root to query elements from AdviceMap
exec.memory::get_acct_code_root
# => [ACCT_CODE_ROOT]

# query elements from AdviceMap and get len
adv.push_mapval adv_push.1
# => [len, ACCT_CODE_ROOT]

# Should be done using a memory constant, loc at which piped elements will live
push.0 exec.memory::get_account_procedures_section_offset
# => [location, counter, len, ACCT_CODE_ROOT]

# start looping
padw padw padw push.1
# => [PAD, PAD, PAD, 1, location, counter, len, ACCT_CODE_ROOT]

while.true
# pipe elements from advice map and hash them
adv_pipe hperm
# => [HASH, HASH, HASH, location, counter, len, ACCT_CODE_ROOT]

# check if we piped all procedure elements
movup.13 add.1 dup movdn.14 dup.15 neq
# => [should_loop, HASH, HASH, HASH, location, counter, len, ACCT_CODE_ROOT]
end

# keep relevant hash
exec.native::state_to_digest
# => [HASH, location, counter, len, ACCT_CODE_ROOT]

# drop location, counter and len
movup.4 movup.5 movup.6 drop drop drop
# => [HASH, ACCT_CODE_ROOT]

# verify hashed values match root
assert_eqw
# => []
end

#! Saves the account data to memory and validates it.
#!
#! This procedure will:
Expand Down Expand Up @@ -476,6 +516,7 @@ proc.process_account_data
# code root updates
exec.memory::get_acct_code_root
exec.memory::set_new_acct_code_root dropw
exec.validate_account_procedures
# => []

# copy the initial account vault hash to the input vault hash to support transaction asset
Expand Down
7 changes: 5 additions & 2 deletions miden-lib/src/transaction/inputs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -245,8 +245,11 @@ fn add_account_to_advice_inputs(
// --- account code -------------------------------------------------------
let code = account.code();

// extend the merkle store with account code tree
inputs.extend_merkle_store(code.procedure_tree().inner_nodes());
// extend the advice_map with the account code data and procedure length
let num_procs = code.as_elements().len() / 8;
let mut procs = code.as_elements();
procs.insert(0, Felt::from(num_procs as u32));
inputs.extend_map([(*code.procedure_commitment(), procs)]);
bobbinth marked this conversation as resolved.
Show resolved Hide resolved

// --- account seed -------------------------------------------------------
if let Some(account_seed) = account_seed {
Expand Down
3 changes: 3 additions & 0 deletions miden-lib/src/transaction/memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,9 @@ pub const ACCT_NEW_CODE_ROOT_PTR: MemoryAddress =
/// The memory address at which the account storage slot type data beings
pub const ACCT_STORAGE_SLOT_TYPE_DATA_OFFSET: MemoryAddress = 405;

/// The memory address at which the account procedures section begins.
pub const ACCT_PROCEDURES_SECTION_OFFSET: MemoryAddress = 700;
bobbinth marked this conversation as resolved.
Show resolved Hide resolved

// NOTES DATA
// ================================================================================================

Expand Down
7 changes: 6 additions & 1 deletion miden-tx/src/compiler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,12 @@ impl TransactionCompiler {
) -> Result<AccountCode, TransactionCompilerError> {
let account_code = AccountCode::new(account_code, &self.assembler)
.map_err(TransactionCompilerError::LoadAccountFailed)?;
self.account_procedures.insert(account_id, account_code.procedures().to_vec());

self.account_procedures.insert(
account_id,
account_code.procedures().iter().map(|(p, _)| *p).collect::<Vec<Digest>>(),
bobbinth marked this conversation as resolved.
Show resolved Hide resolved
);

Ok(account_code)
}

Expand Down
2 changes: 1 addition & 1 deletion miden-tx/src/compiler/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ fn test_load_account() {

let acct_procs = [hex_to_bytes(ACCT_PROC_1), hex_to_bytes(ACCT_PROC_2)];
for proc in account_code.procedures() {
assert!(acct_procs.contains(&proc.as_bytes().to_vec()));
assert!(acct_procs.contains(&proc.0.as_bytes().to_vec()));
}
}

Expand Down
27 changes: 8 additions & 19 deletions miden-tx/src/host/account_procs.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use miden_lib::transaction::TransactionKernelError;
use miden_objects::accounts::AccountCode;

use super::{AdviceProvider, BTreeMap, Digest, NodeIndex, ProcessState};
use super::{AdviceProvider, BTreeMap, Digest, Felt, ProcessState};

// ACCOUNT PROCEDURE INDEX MAP
// ================================================================================================
Expand All @@ -13,27 +12,17 @@ impl AccountProcedureIndexMap {
/// Returns a new [AccountProcedureIndexMap] instantiated with account procedures present in
/// the provided advice provider.
///
/// This function assumes that the account procedure tree (or a part thereof) is loaded into the
/// Merkle store of the provided advice provider.
pub fn new<A: AdviceProvider>(account_code_root: Digest, adv_provider: &A) -> Self {
// get the Merkle store with the procedure tree from the advice provider
let proc_store = adv_provider.get_store_subset([account_code_root].iter());
// get the account procedures from the advice_map
let procs = adv_provider.get_mapped_values(&account_code_root).unwrap();
bobbinth marked this conversation as resolved.
Show resolved Hide resolved

// iterate over all possible procedure indexes
let mut result = BTreeMap::new();
for i in 0..AccountCode::MAX_NUM_PROCEDURES {
let index = NodeIndex::new(AccountCode::PROCEDURE_TREE_DEPTH, i as u64)
.expect("procedure tree index is valid");
// if the node at the current index does not exist, skip it and try the next node;this
// situation is valid if not all account procedures are loaded into the advice provider
if let Ok(proc_root) = proc_store.get_node(account_code_root, index) {
// if we got an empty digest, this means we got to the end of the procedure list
if proc_root == Digest::default() {
break;
}
result.insert(proc_root, i as u8);
}

for (proc_idx, proc_info) in procs[1..].chunks_exact(8).enumerate() {
let root: [Felt; 4] = proc_info[0..4].try_into().expect("Slice with incorrect len.");
result.insert(Digest::from(root), proc_idx.try_into().unwrap());
}

bobbinth marked this conversation as resolved.
Show resolved Hide resolved
Self(result)
}

Expand Down
4 changes: 2 additions & 2 deletions miden-tx/src/host/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ use miden_objects::{
Digest, Hasher,
};
use vm_processor::{
crypto::NodeIndex, AdviceExtractor, AdviceInjector, AdviceProvider, AdviceSource, ContextId,
ExecutionError, Felt, Host, HostResponse, ProcessState,
AdviceExtractor, AdviceInjector, AdviceProvider, AdviceSource, ContextId, ExecutionError, Felt,
Host, HostResponse, ProcessState,
};

mod account_delta_tracker;
Expand Down
27 changes: 8 additions & 19 deletions miden-tx/src/testing/account_procs.rs
bobbinth marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
use alloc::collections::BTreeMap;

use miden_lib::transaction::TransactionKernelError;
use miden_objects::accounts::AccountCode;
use vm_processor::{crypto::NodeIndex, AdviceProvider, Digest, ProcessState};
use vm_processor::{AdviceProvider, Digest, Felt, ProcessState};

// ACCOUNT PROCEDURE INDEX MAP
// ================================================================================================
Expand All @@ -14,27 +13,17 @@ impl AccountProcedureIndexMap {
/// Returns a new [AccountProcedureIndexMap] instantiated with account procedures present in
/// the provided advice provider.
///
/// This function assumes that the account procedure tree (or a part thereof) is loaded into the
/// Merkle store of the provided advice provider.
pub fn new<A: AdviceProvider>(account_code_root: Digest, adv_provider: &A) -> Self {
// get the Merkle store with the procedure tree from the advice provider
let proc_store = adv_provider.get_store_subset([account_code_root].iter());
// get the account procedures from the advice_map
let procs = adv_provider.get_mapped_values(&account_code_root).unwrap();

// iterate over all possible procedure indexes
let mut result = BTreeMap::new();
for i in 0..AccountCode::MAX_NUM_PROCEDURES {
let index = NodeIndex::new(AccountCode::PROCEDURE_TREE_DEPTH, i as u64)
.expect("procedure tree index is valid");
// if the node at the current index does not exist, skip it and try the next node;this
// situation is valid if not all account procedures are loaded into the advice provider
if let Ok(proc_root) = proc_store.get_node(account_code_root, index) {
// if we got an empty digest, this means we got to the end of the procedure list
if proc_root == Digest::default() {
break;
}
result.insert(proc_root, i as u8);
}

for (proc_idx, proc_info) in procs[1..].chunks_exact(8).enumerate() {
let root: [Felt; 4] = proc_info[0..4].try_into().expect("Slice with incorrect len.");
result.insert(Digest::from(root), proc_idx.try_into().unwrap());
}

Self(result)
}

Expand Down
12 changes: 5 additions & 7 deletions miden-tx/src/tests/kernel_tests/test_account.rs
Original file line number Diff line number Diff line change
Expand Up @@ -511,14 +511,12 @@ fn test_authenticate_procedure() {
.build();
let account = tx_context.tx_inputs().account();

let proc0_index = LeafIndex::new(0).unwrap();
let proc1_index = LeafIndex::new(1).unwrap();
let tc_0: [Felt; 4] = account.code().procedures()[0].0.as_elements().try_into().unwrap();
let tc_1: [Felt; 4] = account.code().procedures()[1].0.as_elements().try_into().unwrap();
let tc_2: [Felt; 4] = account.code().procedures()[2].0.as_elements().try_into().unwrap();

let test_cases = vec![
(account.code().procedure_tree().get_leaf(&proc0_index), true),
(account.code().procedure_tree().get_leaf(&proc1_index), true),
([ONE, ZERO, ONE, ZERO], false),
];
let test_cases =
vec![(tc_0, true), (tc_1, true), (tc_2, true), ([ONE, ZERO, ONE, ZERO], false)];

for (root, valid) in test_cases.into_iter() {
let tx_context = TransactionContextBuilder::with_standard_account(
Expand Down
26 changes: 17 additions & 9 deletions miden-tx/src/tests/kernel_tests/test_prologue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,15 @@ use alloc::{collections::BTreeMap, vec::Vec};
use miden_lib::transaction::{
memory::{
MemoryOffset, ACCT_CODE_ROOT_PTR, ACCT_DB_ROOT_PTR, ACCT_ID_AND_NONCE_PTR, ACCT_ID_PTR,
ACCT_STORAGE_ROOT_PTR, ACCT_STORAGE_SLOT_TYPE_DATA_OFFSET, ACCT_VAULT_ROOT_PTR,
BLK_HASH_PTR, BLOCK_METADATA_PTR, BLOCK_NUMBER_IDX, CHAIN_MMR_NUM_LEAVES_PTR,
CHAIN_MMR_PEAKS_PTR, CHAIN_ROOT_PTR, INIT_ACCT_HASH_PTR, INIT_NONCE_PTR,
INPUT_NOTES_COMMITMENT_PTR, INPUT_NOTE_ARGS_OFFSET, INPUT_NOTE_ASSETS_HASH_OFFSET,
INPUT_NOTE_ASSETS_OFFSET, INPUT_NOTE_ID_OFFSET, INPUT_NOTE_INPUTS_HASH_OFFSET,
INPUT_NOTE_METADATA_OFFSET, INPUT_NOTE_NUM_ASSETS_OFFSET, INPUT_NOTE_SCRIPT_ROOT_OFFSET,
INPUT_NOTE_SECTION_OFFSET, INPUT_NOTE_SERIAL_NUM_OFFSET, NOTE_ROOT_PTR,
NULLIFIER_DB_ROOT_PTR, PREV_BLOCK_HASH_PTR, PROOF_HASH_PTR, PROTOCOL_VERSION_IDX,
TIMESTAMP_IDX, TX_HASH_PTR, TX_SCRIPT_ROOT_PTR,
ACCT_PROCEDURES_SECTION_OFFSET, ACCT_STORAGE_ROOT_PTR, ACCT_STORAGE_SLOT_TYPE_DATA_OFFSET,
ACCT_VAULT_ROOT_PTR, BLK_HASH_PTR, BLOCK_METADATA_PTR, BLOCK_NUMBER_IDX,
CHAIN_MMR_NUM_LEAVES_PTR, CHAIN_MMR_PEAKS_PTR, CHAIN_ROOT_PTR, INIT_ACCT_HASH_PTR,
INIT_NONCE_PTR, INPUT_NOTES_COMMITMENT_PTR, INPUT_NOTE_ARGS_OFFSET,
INPUT_NOTE_ASSETS_HASH_OFFSET, INPUT_NOTE_ASSETS_OFFSET, INPUT_NOTE_ID_OFFSET,
INPUT_NOTE_INPUTS_HASH_OFFSET, INPUT_NOTE_METADATA_OFFSET, INPUT_NOTE_NUM_ASSETS_OFFSET,
INPUT_NOTE_SCRIPT_ROOT_OFFSET, INPUT_NOTE_SECTION_OFFSET, INPUT_NOTE_SERIAL_NUM_OFFSET,
NOTE_ROOT_PTR, NULLIFIER_DB_ROOT_PTR, PREV_BLOCK_HASH_PTR, PROOF_HASH_PTR,
PROTOCOL_VERSION_IDX, TIMESTAMP_IDX, TX_HASH_PTR, TX_SCRIPT_ROOT_PTR,
},
TransactionKernel,
};
Expand Down Expand Up @@ -257,6 +257,14 @@ fn account_data_memory_assertions(process: &Process<MockHost>, inputs: &Transact
"The account types data should be stored in (ACCT_STORAGE_SLOT_TYPE_DATA_OFFSET..ACCT_STORAGE_SLOT_TYPE_DATA_OFFSET + 64)"
);
}

for (i, elements) in inputs.account().code().as_elements().chunks(4).enumerate() {
assert_eq!(
read_root_mem_value(process, ACCT_PROCEDURES_SECTION_OFFSET + i as u32),
Word::try_from(elements).unwrap(),
"The account procedures and offsets should be stored at ACCT_PROCEDURES_SECTION_OFFSET"
);
}
}

fn input_notes_memory_assertions(
Expand Down
Loading
Loading