chore: switch back protocol dep to git next by juan518munoz · Pull Request #2282 · 0xMiden/node

juan518munoz · 2026-06-24T19:50:59Z

Summary

Switch back miden protocol dependency from crates.io to github's next branch. Unsure if this is something to have addressed now, the main goal of the PR is to apply most recent changes of protocol as soon as possible on the miden-client.

Changelog

Most notable changes are Delta storages being replaced by Patch, and the removal of the fee field from transactions (to be replaced by batched fees).

[[entry]]
scope       = "protocol"
impact      = "breaking"
description = "Updated `miden-protocol` dependencies to use the `next` branch (v0.16). Block and transaction account updates now use the absolute `AccountPatch` representation instead of the relative `AccountDelta`, and the `miden-tx-batch-prover` crate was renamed to `miden-tx-batch`. The per-transaction fee was also removed following its removal from the protocol transaction kernel (https://github.com/0xMiden/protocol/pull/3108), so the `fee` field was dropped from the `TransactionHeader` gRPC message and from the store and validator transaction tables, and `ProvenTransaction` and `TransactionId` construction no longer take a fee."

juan518munoz · 2026-06-24T20:53:02Z

Should we make db changes like this (breaking)? Or rather remove throught a migration?

In the future we will have to do this via migrations, and we do have the framework in place for such a situation.

However, this one would be particularly painful since (iiuc) it would require running through every public account's delta and calculating the absolute value instead.

This might be good practice but its also a waste of time since we would never apply this migration to any active network.

So this would just be an additional breaking change entry e.g.

[[entry]] scope = "node" impact = "breaking" description = "Backwards incompatible database schema changes to align with new protocol."

Mirko-von-Leipzig · 2026-06-25T14:14:49Z

Thanks! Will begin reviewing. There are a handful of open PRs that I would like to squeeze in before merging this one.

igamigo · 2026-06-25T14:18:14Z

Thanks! Will begin reviewing. There are a handful of open PRs that I would like to squeeze in before merging this one.

No hurries, I'm also planning on taking a look soon. Thanks!

igamigo

Not a full review, but I got through most of the non-test code

…rotocol # Conflicts: # crates/rpc/src/server/api/submit_proven_tx_batch.rs

igamigo

LGTM, thanks! I left some comments, most of which are quite minor. I think the only real question I have is related to how we fetch (and potentially reconstruct to insert to cache) the account vault details.

igamigo · 2026-07-03T15:29:33Z

                .expect("wallet tx only writes existing value slots");
        }
-        let new_nonce = wallet.nonce() + account_delta.nonce_delta();
+        let new_nonce = account_patch.final_nonce().unwrap_or_else(|| wallet.nonce());


I think we could expect() here, instead of defaulting to the preivous nonce. If the transaction succeeded, the nonce should have been updated and if it's not, it'd be signaling a problem.

Marking as resolved as it was superseeded by the other comment

igamigo · 2026-07-03T15:30:01Z

Actually looking at this could we not do wallet.apply_patch() directly?

You are correct, addressed in 05e49d5

igamigo · 2026-07-03T15:37:09Z

    storage_update: Option<(BenchmarkStorageUpdate, usize)>,
-) -> AccountDelta {
-    let mut vault_delta = AccountVaultDelta::default();
+) -> AccountPatch {


I think we can do AccountPatch::try_from here

I'm not sure I'm following correctly, but building an AccountPatch from the received Account would imply converting the full account into a patch (note that it also requires cloning it), when in reality we only want to create a patch with a subset of its vault/storage, only the modified ones.

igamigo · 2026-07-03T15:37:57Z

    },
    #[error("transaction proof verification failed")]
-    ProofVerificationFailed(#[from] miden_tx::TransactionVerifierError),
+    ProofVerificationFailed(#[from] miden_protocol::errors::TransactionVerifierError),


nit: We can import the TransactionVerifierError

Added the error as an import in 05e49d5

igamigo · 2026-07-03T16:01:51Z

+#[expect(
+    clippy::large_enum_variant,
+    reason = "built per account update and consumed immediately"
+)]


Interesting that this starts showing up now

igamigo · 2026-07-03T16:09:31Z

+    for (slot_name, value_patch) in patch.values() {
+        value_updates.insert(slot_name, value_patch.value().unwrap_or(EMPTY_WORD));
    }


I think this is semantically the same as what was here before so maybe a question for @Mirko-von-Leipzig but should these removed values be deleted from the SQL table?

igamigo · 2026-07-03T16:14:47Z


 /// Process wallet assets and return them as a fungible asset delta. Track the negative adjustments
 /// for the respective faucets.
 fn prepare_fungible_asset_update(


I could be wrong but it seems some of this setup could potentially be simplified with the delta to patch migration

igamigo · 2026-07-03T16:18:40Z

+        self.forest
+            .write()
+            .await
+            .cache_vault_keys(assets.iter().map(miden_protocol::asset::Asset::vault_key));


I think we may only want to cache here if the details are not in the "too many entries" category (this is what the storage map flow does, AFAIK)

Overall though, I may be missing something but these functions (both the new one and the previously-existing one related to storage maps) seem to be doing unnecessary work: they may be reading the whole set of key/values to just return LimitExceeded. cc @Mirko-von-Leipzig

igamigo · 2026-07-03T16:21:33Z

-            };
-            entries.push((key, value));
-        }
+        let mut entries: Vec<(AssetVaultKey, Word)> = Vec::new();


nit: We can do Vec::with_capacity() here

Done in 05e49d5

SantiagoPittella

It's looking good in general. Added some nits. Also lets ensure addressing Ignacio's comments + a search of "delta" throughout the repository since I think there are a couple places that still reference the old name

SantiagoPittella · 2026-07-03T17:50:37Z

+        for (slot_name, value) in account_patch.storage().values() {
            storage
-                .set_item(slot_name, *value)
+                .set_item(slot_name, value.value().unwrap_or_else(Word::empty))


Considering that the value shouldn't be removed and that StorageValuePatch::Remove is the only variant that makes this return None, lets replace this unwrap with an .expect("the value is always increased") or something like that

Superseeded by other comment

SantiagoPittella · 2026-07-03T17:52:33Z

Suggested change

/// Decides the actor's next mode after a coordinator notification, patching the in-memory

/// account when the actor's own transaction lands.

SantiagoPittella · 2026-07-03T17:54:00Z

Suggested change

/// - Upserts each touched network account: new full-state path insert, partial patches apply to

chore: switch back protocol dep to git next

a490cf1

juan518munoz commented Jun 24, 2026

View reviewed changes

docs: fix CHANGELOG entry PR link

257f79b

juan518munoz marked this pull request as ready for review June 24, 2026 22:00

juan518munoz added 2 commits June 26, 2026 15:56

chore: bump protocol version

f592f17

chore: bump protocol version

1c67505

igamigo reviewed Jun 26, 2026

View reviewed changes

juan518munoz and others added 13 commits June 29, 2026 12:15

chore: address PR comments

3acc5b5

Merge remote-tracking branch 'origin/next' into jmunoz-update-miden-p…

f645dee

…rotocol # Conflicts: # crates/rpc/src/server/api/submit_proven_tx_batch.rs

Merge branch 'next' into jmunoz-update-miden-protocol

ece4890

chore: bump protocol version to 96878f9f

490b783

Merge branch 'next' into jmunoz-update-miden-protocol

e310eb2

fix: linting

d98c2cc

fix: debug failing parallel proving

b77301f

ci: fix benchmark check

82f06c4

fix: cache rebuild on proto file changes

1f52956

Merge branch 'next' into jmunoz-update-miden-protocol

ac22874

test: reuse single RPC server

05ba698

Merge branch 'next' into jmunoz-update-miden-protocol

248be9b

Merge branch 'next' into jmunoz-update-miden-protocol

b245d48

igamigo requested a review from SantiagoPittella July 3, 2026 15:30

igamigo approved these changes Jul 3, 2026

View reviewed changes

igamigo mentioned this pull request Jul 3, 2026

NTX-Builder: tx expiration delta doesn't work with NetworkAuth component #2170

Open

SantiagoPittella approved these changes Jul 3, 2026

View reviewed changes

chore: address PR comments

05e49d5


	/// Decides the actor's next mode after a coordinator notification, patching the in-memory
	/// account when the actor's own transaction lands.


	/// - Upserts each touched network account: new full-state path insert, partial patches apply to

Uh oh!

Conversation

juan518munoz commented Jun 24, 2026

Summary

Changelog

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mirko-von-Leipzig commented Jun 25, 2026

Uh oh!

igamigo commented Jun 25, 2026

Uh oh!

igamigo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

igamigo 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!

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!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SantiagoPittella 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

juan518munoz Jun 24, 2026 •

edited

Loading