fix(wallets): make WalletDB.deleteAccount atomic - #25219
Open
vezenovm wants to merge 6 commits into
Open
Conversation
Same defect as storeAccount (audit finding 12 vs 5): four parallel account entry deletes plus a separate alias cleanup, with no transaction. A failure partway through leaves the account entries gone but the alias behind, or any other partial mix. Wrap the whole body in store.transactionAsync() so the deletes commit all-or-nothing; the alias scan reads through the pending write transaction so its semantics are unchanged. Extends the crashingStore test helper to also inject failures on delete, and pins rollback: a failed alias delete must leave the account fully intact. Also documents that deleteAccount does not deregister the account from the PXE, per the audit's recommendation.
vezenovm
changed the title
Base automatically changed from
mv/f-548-wallet-db-atomic-store-account
to
merge-train/fairies
August 13, 2026 18:00
vezenovm
commented
Aug 13, 2026
vezenovm
marked this pull request as ready for review
August 13, 2026 18:00
The PXE exposes no account removal API, so telling callers to deregister separately prescribed an impossible action. State the true contract instead: deletion is local to this store and PXE state is unaffected.
…:AztecProtocol/aztec-packages into mv/f-548-wallet-db-atomic-delete-account
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #25216, fixing the same defect in
WalletDB.deleteAccount: four parallel account entry deletes plus a separate alias scan and delete, with no transaction. A failure partway through could leave any partial mix, e.g. the account entries gone with a stale alias left behind.The whole body now runs inside
store.transactionAsync(), so the deletes commit all-or-nothing.The red/green test reuses the
crashingStorehelper from #25216, generalized to inject failures ondeleteas well asset. It fails the alias delete, which runs after the four account entry deletes, then asserts the account is fully intact. Pre-fix, the account entries were deleted while the stale alias survived (retrieveAccountthrew "does not exist").deleteAccountnow carries a jsdoc note that deletion is local to this store and leaves PXE state unaffected.