Skip to content

fix(store): query only latest vault rows when applying account delta#1789

Merged
drahnr merged 7 commits intonextfrom
tomasarrachea-vault-reconstruction-fix
Mar 13, 2026
Merged

fix(store): query only latest vault rows when applying account delta#1789
drahnr merged 7 commits intonextfrom
tomasarrachea-vault-reconstruction-fix

Conversation

@TomasArrachea
Copy link
Collaborator

While testing for 0xMiden/miden-client#1843, I made some changes to the node seed store binary and bumped into this bug.

When applying a partial account delta, the store reconstructed the AssetVault by querying all historical vault rows instead of only the current state. This caused DuplicateAsset error when the same vault key had entries across multiple blocks. i.e., after 2+ balance updates to the same faucet for an account. This is way the current test did not catch the error, I updated it to update the account 3 times and it fails.

The fix is to replace the historical range query with a new select_latest_vault_assets function that filters on is_latest = true, returning only the current vault state for reconstruction.

@TomasArrachea TomasArrachea marked this pull request as ready for review March 12, 2026 19:01
Copy link
Contributor

@drahnr drahnr left a comment

Choose a reason for hiding this comment

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

Thank you!

@Mirko-von-Leipzig
Copy link
Collaborator

@drahnr do we need this on main as well?

Comment on lines +213 to +219
let mut assets = Vec::new();
for (_vault_key_bytes, maybe_asset_bytes) in entries {
if let Some(asset_bytes) = maybe_asset_bytes {
assets.push(Asset::read_from_bytes(&asset_bytes)?);
}
}
Ok(assets)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
let mut assets = Vec::new();
for (_vault_key_bytes, maybe_asset_bytes) in entries {
if let Some(asset_bytes) = maybe_asset_bytes {
assets.push(Asset::read_from_bytes(&asset_bytes)?);
}
}
Ok(assets)
Result::from_iter(entries.into_iter().filter_map(|(_vault_key_bytes, maybe_asset_bytes)| { maybe_asset_bytes.map(Asset::read_from_bytes).transpose()}));

@drahnr
Copy link
Contributor

drahnr commented Mar 13, 2026

The code in question was changed in #1538 ( 965282b74 ), which is not present on main. So next should be sufficient.

@drahnr drahnr merged commit 3f7ee85 into next Mar 13, 2026
18 checks passed
@drahnr drahnr deleted the tomasarrachea-vault-reconstruction-fix branch March 13, 2026 20:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants