Skip to content
Merged
Show file tree
Hide file tree
Changes from 13 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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
- Added `GetLimits` endpoint to the RPC server ([#1410](https://github.com/0xMiden/miden-node/pull/1410)).
- Added gRPC-Web probe support to the `miden-network-monitor` binary ([#1484](https://github.com/0xMiden/miden-node/pull/1484)).
- Add DB schema change check ([#1268](https://github.com/0xMiden/miden-node/pull/1485)).
- Add foreign account support to validator ([#1493](https://github.com/0xMiden/miden-node/pull/1493)).
- Improve DB query performance for account queries ([#1496](https://github.com/0xMiden/miden-node/pull/1496).
- Limit number of storage map keys in `GetAccount` requests ([#1517](https://github.com/0xMiden/miden-node/pull/1517)).
- The network monitor now marks the chain as unhealthy if it fails to create new blocks ([#1512](https://github.com/0xMiden/miden-node/pull/1512)).
Expand Down
4 changes: 2 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

71 changes: 35 additions & 36 deletions crates/validator/src/tx_validation/data_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,11 @@ impl DataStore for TransactionInputsDataStore {
foreign_account_id: AccountId,
_ref_block: BlockNumber,
) -> impl FutureMaybeSend<Result<AccountInputs, DataStoreError>> {
async move { Err(DataStoreError::AccountNotFound(foreign_account_id)) }
async move {
self.tx_inputs.read_foreign_account_inputs(foreign_account_id).map_err(|err| {
DataStoreError::other_with_source("failed to read foreign account inputs", err)
})
}
}

fn get_vault_asset_witnesses(
Expand All @@ -63,58 +67,53 @@ impl DataStore for TransactionInputsDataStore {
vault_keys: BTreeSet<AssetVaultKey>,
) -> impl FutureMaybeSend<Result<Vec<AssetWitness>, DataStoreError>> {
async move {
if self.tx_inputs.account().id() != account_id {
return Err(DataStoreError::AccountNotFound(account_id));
}

if self.tx_inputs.account().vault().root() != vault_root {
return Err(DataStoreError::Other {
error_msg: "vault root mismatch".into(),
source: None,
});
if self.tx_inputs.account().id() == account_id {
// Get asset witnessess from native account.
Result::<Vec<_>, _>::from_iter(vault_keys.into_iter().map(|vault_key| {
match self.tx_inputs.account().vault().open(vault_key) {
Copy link
Contributor

Choose a reason for hiding this comment

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

The native account's witnesses are handled just like the ones for foreign accounts, i.e. they are added to the advice inputs on lazy loading, but not to the PartialVault. Any asset witnesses that are present in the partial vault at the beginning are also added to the advice inputs.

So, the advice inputs contain both the pre-provided asset witnesses and those fetched during lazy loading, and so we should read from the advice inputs and not from the partial vault.

The need for this should go away with 0xMiden/miden-base#2298 completely, at which point this could be replaced with unimplemented!.

Because of these subtle differences, I still think it would be good to add a test for this, e.g. one that has an FPI call, reads a storage map item lazily and has input notes with some assets, so the pre-fetch is executed.

And then all of these annoying details should be abstracted away once we have a re-execution API exposed in base directly: 0xMiden/miden-base#2293.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

0xMiden/miden-base#2298 has been merged so I have moved this to unimplemented

Copy link
Contributor

Choose a reason for hiding this comment

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

0xMiden/miden-base#2298 is not a part of v0.13.0 release - it went into next. So, we can't make use of it in this PR.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated

Ok(vault_proof) => AssetWitness::new(vault_proof.into()).map_err(|err| {
DataStoreError::other_with_source(
"failed to open vault asset tree",
err,
)
}),
Err(err) => {
Err(DataStoreError::other_with_source("failed to open vault", err))
},
}
}))
} else {
// Get asset witnessess from foreign account.
self.tx_inputs
.read_vault_asset_witnesses(vault_root, vault_keys)
.map_err(|err| {
DataStoreError::other_with_source(
"failed to read vault asset witnesses",
err,
)
})
}

Result::<Vec<_>, _>::from_iter(vault_keys.into_iter().map(|vault_key| {
match self.tx_inputs.account().vault().open(vault_key) {
Ok(vault_proof) => {
AssetWitness::new(vault_proof.into()).map_err(|err| DataStoreError::Other {
error_msg: "failed to open vault asset tree".into(),
source: Some(err.into()),
})
},
Err(err) => Err(DataStoreError::Other {
error_msg: "failed to open vault".into(),
source: Some(err.into()),
}),
}
}))
}
}

fn get_storage_map_witness(
&self,
account_id: AccountId,
_account_id: AccountId,
_map_root: Word,
_map_key: Word,
) -> impl FutureMaybeSend<Result<StorageMapWitness, DataStoreError>> {
async move {
if self.tx_inputs.account().id() != account_id {
return Err(DataStoreError::AccountNotFound(account_id));
}

// For partial accounts, storage map witness is not available.
Err(DataStoreError::Other {
error_msg: "storage map witness not available with partial account state".into(),
source: None,
})
unimplemented!(
"get_storage_map_witness is not used during re-execution of transactions"
)
}
}

fn get_note_script(
&self,
_script_root: Word,
) -> impl FutureMaybeSend<Result<Option<NoteScript>, DataStoreError>> {
async move { Ok(None) }
async move { unimplemented!("get_note_script is not used during re-execution of transactions") }
}
}

Expand Down