feat: implement serialization for `AccountPatch` and `AccountVaultPatch` and `apply_patch` by PhilippGackstatter · Pull Request #3071 · 0xMiden/protocol

PhilippGackstatter · 2026-06-09T12:08:11Z

Implements:

serialization for AccountPatch and AccountVaultPatch. The latter's serialization for fungible assets is optimized to save space, as suggested by @bobbinth.
Account::apply_patch API.
impl TryFrom<&AccountPatch> for Account
Adds a missing ID check in apply_delta (ensuring the delta is applied to the right account).

Needed for including AccountPatch in ExecutedTransaction and replacing AccountDelta, so part of #2630.

…o pgackst-unified-asset-delta

…-delta

bobbinth

Looks good! Thank you! I left just a few comments inline - the main one is about serialization.

bobbinth · 2026-06-10T21:53:32Z

+    pub fn is_full_state(&self) -> bool {
+        // TODO(code_upgrades): Change this to another detection mechanism once we have code upgrade
+        // support, at which point the presence of code may not be enough of an indication that a
+        // patch can be converted to a full account.
+        self.code.is_some() && self.final_nonce.is_some()
+    }


Could this not be just self.final_nonce == 1? Basically, if the final nonce is one, this patch should contain all required info to create a new account.

Basically, if the final nonce is one, this patch should contain all required info to create a new account.

Yes, but I think the condition is larger: If we merge patches from multiple transactions, the nonce may have advanced past 1 but still contain the code, which is necessary to convert into an Account. So writing the condition this way is less strict.

could we potentially disallow merging patches with final_nonce == 1?

Ah - I see. In that case, wouldn't be just self.code.is_some() be sufficient? Basically, a combination of self.code.is_some() and self.final_nonce.is_none() shouldn't be possible by construction, right?

Per review feedback, serialize removed entries as bare `AssetVaultKey` and added entries as `Asset`, letting `Asset` own the fungible vs. non-fungible split instead of duplicating that logic in the patch. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

PhilippGackstatter · 2026-06-11T11:49:36Z

Pushed a small additional commit to implement AccountPatch::try_from(account) as it fits here conceptually and is just a few dozen lines of code.

…-delta

…zation

mmagician

partial review for now, LGTM so far

mmagician · 2026-06-12T13:46:21Z

-        // Code must be provided for new accounts (nonce = 1) to be able to reconstruct the full
-        // Account.
+        // Code must be provided for new accounts to be able to reconstruct the full Account.
+        // New accounts are usually defined with nonce 0, but here we have the post-creation


Suggested change

// New accounts are usually defined with nonce 0, but here we have the post-creation

// New accounts are defined with nonce 0, but here we have the post-creation

mmagician · 2026-06-12T13:49:34Z

+    pub fn is_full_state(&self) -> bool {
+        // TODO(code_upgrades): Change this to another detection mechanism once we have code upgrade
+        // support, at which point the presence of code may not be enough of an indication that a
+        // patch can be converted to a full account.
+        self.code.is_some() && self.final_nonce.is_some()
+    }


could we potentially disallow merging patches with final_nonce == 1?

mmagician · 2026-06-12T14:01:08Z

+    fn try_from(patch: &AccountPatch) -> Result<Self, Self::Error> {
+        if !patch.is_full_state() {
+            return Err(AccountError::PartialStatePatchToAccount);
+        }
+
+        let Some(code) = patch.code().cloned() else {
+            return Err(AccountError::PartialStatePatchToAccount);
+        };
+        let Some(nonce) = patch.final_nonce() else {
+            return Err(AccountError::PartialStatePatchToAccount);
+        };


nit: IDK how common returning one of these errors would be, but we could potentially have more granular errors, e.g. PartialStatePatchToAccountMissingNonce

mmagician · 2026-06-12T14:02:14Z

+    #[test]
+    fn account_try_from_partial_patch_fails() -> anyhow::Result<()> {
+        let account_id = AccountId::try_from(ACCOUNT_ID_PRIVATE_SENDER)?;
+
+        // Missing code.
+        let no_code = AccountPatch::new(
+            account_id,
+            AccountStoragePatch::new(),
+            AccountVaultPatch::default(),
+            None,
+            Some(Felt::from(2_u32)),
+        )?;
+        assert_matches!(
+            Account::try_from(&no_code).unwrap_err(),
+            AccountError::PartialStatePatchToAccount
+        );
+
+        // Missing final nonce (empty patch).
+        let empty = AccountPatch::new(
+            account_id,
+            AccountStoragePatch::new(),
+            AccountVaultPatch::default(),
+            None,
+            None,
+        )?;


nit: could use rstest here to cover the two cases

mmagician

The rest LGTM!

bobbinth

Looks good! Thank you! I left just one comment inline.

bobbinth · 2026-06-12T19:48:53Z

+    pub fn is_full_state(&self) -> bool {
+        // TODO(code_upgrades): Change this to another detection mechanism once we have code upgrade
+        // support, at which point the presence of code may not be enough of an indication that a
+        // patch can be converted to a full account.
+        self.code.is_some() && self.final_nonce.is_some()
+    }


Ah - I see. In that case, wouldn't be just self.code.is_some() be sufficient? Basically, a combination of self.code.is_some() and self.final_nonce.is_none() shouldn't be possible by construction, right?

PhilippGackstatter added 21 commits June 8, 2026 16:50

feat: implement asset::{merge, lt}, combine_asset_delta

7e4f1b5

feat: implement update_asset_delta

6308f4e

chore: add asset::split, match rust to_elements

b33a6d0

chore: optimize away branch in combine_asset_delta

0021659

chore: remove previous asset deltas

5ba6abd

chore: improve documentation in delta

92caa4b

chore: add changelog

2d991e2

fix: added_assets/removed_assets behind testing

9760368

chore: use absorb_double_words_from_memory for removed assets

0c9f956

chore: update AccountDelta::to_commitment ambiguity docs

b52f967

chore: remove unused fungible asset math

0a8db63

feat: track (initial, final) asset values in kernel delta

389b6bc

feat: compute delta from patch entries

82f6c78

chore: unify asset add/remove event handlers

6223ffc

fix: overwrite delta on repeated calls

0094f05

chore: use AssetDeltaOp in delta computation

09b9867

Merge branch 'next' into pgackst-unified-asset-delta

489b9b0

chore: improve docs, naming

9943f23

chore: reset delta on recomputation

45adc5b

Merge remote-tracking branch 'origin/pgackst-unified-asset-delta' int…

c89db48

…o pgackst-unified-asset-delta

chore: after -> on_asset_delta_computation

03e1c20

PhilippGackstatter added the pr-from-maintainers PRs that come from internal contributors or integration partners. They should be given priority label Jun 9, 2026

PhilippGackstatter force-pushed the pgackst-patch-serialization branch 2 times, most recently from 9d69cc7 to 0a0228d Compare June 9, 2026 13:48

PhilippGackstatter changed the title ~~feat: implement serialization for AccountPatch and AccountVaultPatch~~ feat: implement serialization for AccountPatch and AccountVaultPatch and apply_patch Jun 9, 2026

PhilippGackstatter force-pushed the pgackst-patch-serialization branch 2 times, most recently from fa79ae2 to 1c6c264 Compare June 9, 2026 14:41

PhilippGackstatter marked this pull request as ready for review June 9, 2026 14:42

PhilippGackstatter requested review from Fumuran and mmagician June 9, 2026 14:42

PhilippGackstatter requested a review from bobbinth June 9, 2026 14:42

PhilippGackstatter mentioned this pull request Jun 10, 2026

feat: implement unified asset delta #3038

Merged

PhilippGackstatter added 10 commits June 10, 2026 12:41

Merge remote-tracking branch 'origin/next' into pgackst-unified-asset…

298f36c

…-delta

chore: simplify computation of delta_op

d395761

feat: simplify AccountVaultDelta by tracking map in tracker

e4dce55

fix: typo

d940e61

feat: implement vault patch serialization

a42b083

feat: implement serialization for account patch

945fbaa

feat: implement Account::apply_patch

5dd4bde

chore: reject ID mismatch in apply_delta

4ffa7d3

chore: add changelog

e7d5d68

feat: implement TryFrom<&AccountPatch> for Account

985197b

PhilippGackstatter force-pushed the pgackst-patch-serialization branch from 1c6c264 to 985197b Compare June 10, 2026 11:45

PhilippGackstatter added 2 commits June 10, 2026 15:55

chore: add benchmarks against 5320982

9787c90

chore: update benchmarks to current branch

28a4c6c

bobbinth reviewed Jun 10, 2026

View reviewed changes

PhilippGackstatter and others added 2 commits June 11, 2026 11:44

chore: use set_nonce in increment_nonce

4302895

PhilippGackstatter requested a review from bobbinth June 11, 2026 10:03

PhilippGackstatter added 2 commits June 11, 2026 12:57

fix: apply_patch_rejects_non_increasing_nonce test

2d5bda9

feat: impl AccountPatch::try_from(account)

8427108

PhilippGackstatter added 3 commits June 12, 2026 13:54

Merge remote-tracking branch 'origin/next' into pgackst-unified-asset…

bd9eece

…-delta

chore: address review comments

75e97ea

Merge branch 'pgackst-unified-asset-delta' into pgackst-patch-seriali…

a5ef6f2

…zation

Base automatically changed from pgackst-unified-asset-delta to next June 12, 2026 12:36

Merge remote-tracking branch 'origin/next' into pgackst-patch-seriali…

a90f4e6

…zation

mmagician reviewed Jun 12, 2026

View reviewed changes

mmagician approved these changes Jun 12, 2026

View reviewed changes

bobbinth approved these changes Jun 12, 2026

View reviewed changes

	// New accounts are usually defined with nonce 0, but here we have the post-creation
	// New accounts are defined with nonce 0, but here we have the post-creation

Conversation

PhilippGackstatter commented Jun 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bobbinth left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

PhilippGackstatter commented Jun 11, 2026

Uh oh!

mmagician left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mmagician left a comment

Choose a reason for hiding this comment

Uh oh!

bobbinth left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

PhilippGackstatter commented Jun 9, 2026 •

edited

Loading