Skip to content
Closed
Show file tree
Hide file tree
Changes from 2 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
14 changes: 7 additions & 7 deletions Cargo.lock

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

12 changes: 6 additions & 6 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,12 @@ miden-node-validator = { path = "crates/validator", version = "0.13" }
miden-remote-prover-client = { path = "crates/remote-prover-client", version = "0.13" }

# miden-base aka protocol dependencies. These should be updated in sync.
miden-block-prover = { branch = "next", git = "https://github.com/0xMiden/miden-base.git" }
miden-protocol = { branch = "next", default-features = false, git = "https://github.com/0xMiden/miden-base.git" }
miden-standards = { branch = "next", git = "https://github.com/0xMiden/miden-base.git" }
miden-testing = { branch = "next", git = "https://github.com/0xMiden/miden-base.git" }
miden-tx = { branch = "next", default-features = false, git = "https://github.com/0xMiden/miden-base.git" }
miden-tx-batch-prover = { branch = "next", git = "https://github.com/0xMiden/miden-base.git" }
miden-block-prover = { branch = "sergerad-tx-inputs-foreign-acc-inputs", git = "https://github.com/0xMiden/miden-base.git" }
miden-protocol = { branch = "sergerad-tx-inputs-foreign-acc-inputs", default-features = false, git = "https://github.com/0xMiden/miden-base.git" }
miden-standards = { branch = "sergerad-tx-inputs-foreign-acc-inputs", git = "https://github.com/0xMiden/miden-base.git" }
miden-testing = { branch = "sergerad-tx-inputs-foreign-acc-inputs", git = "https://github.com/0xMiden/miden-base.git" }
miden-tx = { branch = "sergerad-tx-inputs-foreign-acc-inputs", default-features = false, git = "https://github.com/0xMiden/miden-base.git" }
miden-tx-batch-prover = { branch = "sergerad-tx-inputs-foreign-acc-inputs", git = "https://github.com/0xMiden/miden-base.git" }

# Other miden dependencies. These should align with those expected by miden-base.
miden-air = { features = ["std", "testing"], version = "0.20" }
Expand Down
12 changes: 11 additions & 1 deletion crates/ntx-builder/src/actor/execute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,17 @@ impl DataStore for NtxDataStore {
foreign_account_id: AccountId,
_ref_block: BlockNumber,
) -> impl FutureMaybeSend<Result<AccountInputs, DataStoreError>> {
async move { Err(DataStoreError::AccountNotFound(foreign_account_id)) }
async move {
if foreign_account_id == self.account.id() {
// todo(currentpr): proper error
return Err(DataStoreError::AccountNotFound(foreign_account_id));
}
// TODO(currentpr): We don't have tx inputs here so what do we do?
let foreign_inputs =
self.tx_inputs.read_foreign_account_inputs(foreign_account_id).unwrap(); // todo unwrap
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@bobbinth assuming we expect to get foreign account inputs from tx inputs, is it possible to do so here? Should we somehow retrieve the tx inputs through the store or something?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure we can get foreign account inputs from the original TX inputs - or at least it may not always work.

I think to get foreign account inputs, we'd need to make a request to the store and load the account from there. We may also want to cache the foreign account NtxDataStore so that if there are additional requests, we won't need to go to the store again for the same data.

There is one nuance here though: currently, we are able to fetch the full account from the store, but this functionality will go away after the refactoring @drahnr is working on. So, it may make sense to add a dedicated endpoint to the data store that would return the data sufficient to construct AccountInputs (i.e., the PartialAccount and AccountWitness structs).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Any advice on how to do this? Seems like we would have to get the full account from store state, convert that to partial account + get account witness somehow.

it may make sense to add a dedicated endpoint to the data store that would return the data sufficient to construct AccountInputs (i.e., the PartialAccount and AccountWitness structs).

Copy link
Contributor

Choose a reason for hiding this comment

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

While we have the ability to get the full account from the store, things should be pretty easy:

  • The get the partial account, we get the full account and then convert it to partial account (we have impl From<&Account> for PartialAccount implemented).
  • The witness we'd get from the AccountTree - that should be pretty straight-forward too.

I think both of these we should be able to get from the current GetAccountProof endpoint. But soon, the ability to get full account will go away.

After a series of @drahnr's PRs gets merged, GetAccountProof becomes GetAccount and it no longer would return the full account. But it would still return enough info to build a PartialAccount + account witness. For partial account, all we really need is account_id, vault root, storage header, code, and nonce - and I believe all of this should be available from the GetAccount endpoint.

Copy link
Contributor

@drahnr drahnr Jan 6, 2026

Choose a reason for hiding this comment

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

After a series of @drahnr's PRs gets merged, GetAccountProof becomes GetAccount and it no longer would return the full account. But it would still return enough info to build a PartialAccount + account witness. For partial account, all we really need is account_id, vault root, storage header, code, and nonce - and I believe all of this should be available from the GetAccount endpoint.

All of this will be available once #1385 is merged as GetAccount


Ok(foreign_inputs)
}
}

fn get_vault_asset_witnesses(
Expand Down
61 changes: 42 additions & 19 deletions crates/validator/src/tx_validation/data_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,17 @@ 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 {
if foreign_account_id == self.tx_inputs.account().id() {
// todo(currentpr): proper error
return Err(DataStoreError::AccountNotFound(foreign_account_id));
}

let foreign_inputs =
self.tx_inputs.read_foreign_account_inputs(foreign_account_id).unwrap(); // todo unwrap
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@bobbinth do we need the impl for TransactionInputsDataStore, rather than just NtxDataStore?

If yes, do you imagine the impl looking like this - in the sense that account inputs are derived from tx inputs. The read_foreign_account_inputs fn is a stub for now in a branch of mine, just trying to flesh out integration first.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think here we should be able to get all the data from the TransactionInputs - so, it would work differently from NtxDataStore. Basically, the main difference between the validator and NTX builder is:

  • NTX builder: executes a transaction from scratch and so it doesn't now what execution path will be taken during transaction execution. For example, it doesn't know which foreign accounts would need to be loaded ahead of time, and so, we need to load them dynamically (i.e., by fetching the requested accounts from the store).
  • Validator: re-executes transaction that was previously executed by the user/NTX builder. All foreign account data should already be loaded into the TransactionInputs. The main challenge here would be to figure out how to extract the relevant data from TransactionInputs as it is not too structured. I believe I left a comment somewhere sketching out how to extract this data (and maybe putting some logic for this in miden-base would simplify things).


Ok(foreign_inputs)
}
}

fn get_vault_asset_witnesses(
Expand All @@ -63,31 +73,21 @@ 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,
});
}

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()),
}),
}
}))
// Get asset witnessess from local or foreign account.
if self.tx_inputs.account().id() == account_id {
get_asset_witnesses_from_account(self.tx_inputs.account(), vault_keys)
} else {
let foreign_inputs =
self.tx_inputs.read_foreign_account_inputs(account_id).unwrap(); // todo unwrap
get_asset_witnesses_from_account(foreign_inputs.account(), vault_keys)
}
}
}

Expand Down Expand Up @@ -123,3 +123,26 @@ impl MastForestStore for TransactionInputsDataStore {
self.mast_store.get(procedure_hash)
}
}

// HELPERS
// ================================================================================================

fn get_asset_witnesses_from_account(
account: &PartialAccount,
vault_keys: BTreeSet<AssetVaultKey>,
) -> Result<Vec<AssetWitness>, DataStoreError> {
Result::<Vec<_>, _>::from_iter(vault_keys.into_iter().map(|vault_key| {
match 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()),
}),
}
}))
}
Loading