feat: Read foreign account inputs and witnesses from transaction inputs #2246
Conversation
…erad-tx-inputs-foreign-acc-inputs
bobbinth
left a comment
bobbinth
left a comment
There was a problem hiding this comment.
Looks good! Thank you! I left some comments inline - the main one is about parsing the account storage header.
To address it, we may need to add something like foreign_account_slot_names field to TransactionInputs and populate it after transaction execution.
bobbinth
left a comment
bobbinth
left a comment
There was a problem hiding this comment.
Looks good! Thank you! I left some more comments inline - but they are pretty minor.
sergerad
changed the title
| // HELPER FUNCTIONS | ||
| // ================================================================================================ | ||
|
|
||
| // TODO(sergerad): Move this fn to crypto SmtLeaf::try_from_elements. Panics will be replaced with |
There was a problem hiding this comment.
Will create an issue for this when PR is about to be merged
There was a problem hiding this comment.
bobbinth
left a comment
bobbinth
left a comment
There was a problem hiding this comment.
Looks good! Thank you! I left a few comments inline - all should be pretty easy to address.
bobbinth
left a comment
bobbinth
left a comment
There was a problem hiding this comment.
All looks good! Thank you!
| impl TryFrom<Felt> for StorageSlotType { | ||
| type Error = AccountError; | ||
|
|
||
| fn try_from(value: Felt) -> Result<Self, Self::Error> { | ||
| if value == ZERO { | ||
| Ok(StorageSlotType::Value) |
There was a problem hiding this comment.
Can we not use impl TryFrom<u8> for StorageSlotType instead?
I think the caller should first let byte = u8::try_from(felt) and then StorageSlotType::try_from(byte). Avoids having to test the impl twice, keeping it up to date in two places, etc.
| /// Returns the advice map key where: | ||
| /// - the seed for native accounts is stored. | ||
| /// - the account header for foreign accounts is stored. | ||
| pub fn account_id_map_key(id: AccountId) -> Word { |
There was a problem hiding this comment.
Can this remain private?
Context
The Validator needs to be able to read foreign account data when re-executing network transactions via its
TransactionInputsDataStorewhich implements theDataStoretrait.Blocks 0xMiden/miden-node#1493.
Changes
TransactionInputs::read_foreign_account_inputs()TransactionInputs::read_vault_asset_witnesses()TransactionInputs::read_storage_map_witness()AccountStorageHeader::from_elements()smt_leaf_from_elements(elements: &[Felt], leaf_index: LeafIndex<SMT_DEPTH>)with aTODOto move to cryptoSmtLeaf::try_from_elements.