refactor(store): get account vault details from account state forest by kkovaacs · Pull Request #1981 · 0xMiden/node

kkovaacs · 2026-04-22T09:47:42Z

As outlined in #1978 we can avoid some DB queries since we have most account details in AccountStateForest. We were already doing that for specific storage map key requests, but account vault assets were still queried from the database even though we do have those in AccountStateForest as well.

This PR implements the vault_details lookup to use AccountStateForest instead of select_account_vault_at_block(). I added a historical vault enumeration API to AccountStateForest and switched the call site in State::fetch_public_account_details() to use it. The rest of the details path is unchanged.

GetAccount.details.vault_details now comes from AccountStateForest instead of select_account_vault_at_block(). I added a historical vault enumeration API to AccountStateForest and switched the call site in State::fetch_public_account_details() to use it. The rest of the details path is unchanged: header, storage header, code lookup, and storage map assembly still use the existing SQLite logic.

Copilot

Pull request overview

Refactors the store’s GetAccount path to reconstruct account vault assets from the in-memory AccountStateForest instead of querying SQLite, aligning with #1978’s performance goals by reducing DB round-trips.

Changes:

Switch State::fetch_public_account_details() vault-details lookup from select_account_vault_at_block() to AccountStateForest::get_vault_details().
Add AccountStateForest::get_vault_details(account_id, block_num) API to enumerate historical vault contents.
Add unit tests for latest/historical vault enumeration and MAX_RETURN_ENTRIES limit handling; update changelog entry.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
crates/store/src/state/mod.rs	Uses `AccountStateForest` for `vault_details` reconstruction (and updates tracing spans).
crates/store/src/account_state_forest/mod.rs	Adds `get_vault_details()` vault enumeration at a given block.
crates/store/src/account_state_forest/tests.rs	Adds tests covering historical/latest vault assets and limit-exceeded behavior.
CHANGELOG.md	Notes the `GetAccount` vault-assets optimization for the next release.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-22T09:53:07Z

+        let assets = entries
+            .map(|entry| Asset::from_key_value_words(entry.key, entry.value))
+            .collect::<Result<Vec<_>, _>>()?;
+


get_vault_details() collects all vault entries into a Vec before applying AccountVaultDetails::MAX_RETURN_ENTRIES. For large vaults this can allocate and parse far more than needed (and holds the forest read-lock longer), even though the response will be LimitExceeded. Consider iterating with an early cutoff (e.g., stop after MAX_RETURN_ENTRIES + 1 entries) and return LimitExceeded without collecting the full vault.

Suggested change

let assets = entries

.map(|entry| Asset::from_key_value_words(entry.key, entry.value))

.collect::<Result<Vec<_>, _>>()?;

let mut assets = Vec::new();

for entry in entries.take(AccountVaultDetails::MAX_RETURN_ENTRIES + 1) {

assets.push(Asset::from_key_value_words(entry.key, entry.value)?);

}

I think this is a valid concern and the way to deal with this is to first check how many entries a given tree has, and iterate over the entries only if the number of entries is under the threshold. Checking the number of entries used to be expensive, but with 0xMiden/crypto#916 it should now be much cheaper (though, I don't remember if 0xMiden/crypto#916 made it into v0.24 release of miden-crypto or not).

That optimization is in v0.24, but we're still on miden-crypto v0.23 with main AFAIU. Does that mean that the entry_count() call is too expensive to use as a pre-check here?

The v0.23 implementation iterates over all entries to get the count - so, using it would result in two passes over the entries here in most cases. We could try to take this hit now. We could also merge this code into main and when porting back into next update it to use the entry_count() which will become efficient once we update to the next version of miden-crypto (hopefully, in a couple of weeks).

Since we're pulling entries by using an iterator we can make sure we never fetch more than MAX_RETURN_ENTRIES + 1 entries from the tree. I've also added a TODO for the follow-up work.

Mirko-von-Leipzig

LGTM, maybe we do want that pre-check that co-pilot suggested?

bobbinth · 2026-04-22T18:27:30Z

-                let vault_assets =
-                    self.db.select_account_vault_at_block(account_id, block_num).await?;


Do we still need select_account_vault_at_block() function after this PR? Or could we get rid of entirely?

It's still used by tests checking that upsert_accounts() does the right thing. Since we're still keeping all vault data in SQLite I'd keep these tests and just move select_account_vault_at_block() into the tests module so that it can still be used by tests.

…t-account-vault-asset-queries

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-23T06:56:07Z

+                .map_err(|err| {
+                    DatabaseError::DataCorrupted(format!(
+                        "failed to reconstruct vault for account {account_id} at block {block_num}: {err}"
+                    ))


get_vault_details() errors are all being flattened into DatabaseError::DataCorrupted(...), which loses the original error kind (e.g. Merkle vs asset parsing vs missing root) and makes troubleshooting harder. Consider matching on WitnessError and mapping each variant to the closest DatabaseError variant (e.g. MerkleError/AssetError/StorageMapError), using DataCorrupted only for the truly invariant-violating cases (like RootNotFound).

Suggested change

.map_err(|err| {

DatabaseError::DataCorrupted(format!(

"failed to reconstruct vault for account {account_id} at block {block_num}: {err}"

))

.map_err(|err| match err {

WitnessError::MerkleError(err) => DatabaseError::MerkleError(err),

WitnessError::AssetError(err) => DatabaseError::AssetError(err),

WitnessError::StorageMapError(err) => DatabaseError::StorageMapError(err),

WitnessError::RootNotFound(root) => DatabaseError::DataCorrupted(format!(

"failed to reconstruct vault for account {account_id} at block {block_num}: missing root {root}"

)),

kkovaacs added 2 commits April 22, 2026 10:10

chore: add changelog

86b95f7

kkovaacs requested review from Mirko-von-Leipzig, bobbinth, Copilot and sergerad April 22, 2026 09:47

Copilot started reviewing on behalf of kkovaacs April 22, 2026 09:48 View session

Copilot AI reviewed Apr 22, 2026

View reviewed changes

kkovaacs marked this pull request as ready for review April 22, 2026 09:55

Mirko-von-Leipzig approved these changes Apr 22, 2026

View reviewed changes

bobbinth reviewed Apr 22, 2026

View reviewed changes

kkovaacs and others added 3 commits April 22, 2026 22:42

refactor(store/db): move select_account_vault_at_block() to tests

98d53a7

Merge remote-tracking branch 'origin/main' into krisztian/optimize-ge…

d8e3fa2

…t-account-vault-asset-queries

fix(store/account_state_forest): avoid pulling all assets from tree

e9091dd

kkovaacs requested a review from Copilot April 23, 2026 06:49

Copilot started reviewing on behalf of kkovaacs April 23, 2026 06:50 View session

Copilot AI reviewed Apr 23, 2026

View reviewed changes

chore: fix changelog

6f3a859

sergerad approved these changes Apr 23, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(store): get account vault details from account state forest#1981

refactor(store): get account vault details from account state forest#1981
kkovaacs wants to merge 6 commits intomainfrom
krisztian/optimize-get-account-vault-asset-queries

kkovaacs commented Apr 22, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 22, 2026

Uh oh!

bobbinth Apr 22, 2026

Uh oh!

kkovaacs Apr 22, 2026

Uh oh!

bobbinth Apr 22, 2026

Uh oh!

kkovaacs Apr 23, 2026

Uh oh!

Mirko-von-Leipzig left a comment

Uh oh!

bobbinth Apr 22, 2026

Uh oh!

kkovaacs Apr 22, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 23, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

		let vault_assets =
		self.db.select_account_vault_at_block(account_id, block_num).await?;

-                .map_err(|err| {
-                    DatabaseError::DataCorrupted(format!(
-                        "failed to reconstruct vault for account {account_id} at block {block_num}: {err}"
-                    ))
+                .map_err(|err| match err {
+                    WitnessError::MerkleError(err) => DatabaseError::MerkleError(err),
+                    WitnessError::AssetError(err) => DatabaseError::AssetError(err),
+                    WitnessError::StorageMapError(err) => DatabaseError::StorageMapError(err),
+                    WitnessError::RootNotFound(root) => DatabaseError::DataCorrupted(format!(
+                        "failed to reconstruct vault for account {account_id} at block {block_num}: missing root {root}"
+                    )),

Conversation

kkovaacs commented Apr 22, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

bobbinth Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

kkovaacs Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

bobbinth Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

kkovaacs Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

Mirko-von-Leipzig left a comment

Choose a reason for hiding this comment

Uh oh!

bobbinth Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

kkovaacs Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants