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

igamigo · 2026-06-26T18:29:51Z

+                let account_patch = executed_tx.account_patch().clone();

                // Prove transaction.
                let tx_inputs: TransactionInputs = executed_tx.into();


nit: (I think) you can do executed_tx.into_parts() and avoid cloning here

igamigo · 2026-06-26T18:33:32Z

+            ProofKind::Batch => Self::Batch(LocalBatchProver::new()),
            ProofKind::Block => Self::Block(LocalBlockProver::new(MIN_PROOF_SECURITY_LEVEL)),


Mostly just curious but do we know why only the batch prover's security level parameter was removed?

igamigo · 2026-06-26T18:34:27Z

        let prover = self.clone();

        spawn_blocking_in_current_span(move || {
+            let executed_batch = BatchExecutor::new().execute(input).map_err(|e| {


Is there a lot of overhead in creating a BatchExecutor? If so, might make sense to do something about it

igamigo · 2026-06-26T19:06:42Z

+                    let executed_batch = BatchExecutor::new()
+                        .execute(proposed_batch)
+                        .map_err(BuildBatchError::ProveBatchError)?;


Same here about instantiating a BatchExecutor

igamigo · 2026-06-26T19:09:47Z

+                let executed_batch =
+                    BatchExecutor::new().execute(proposed_batch).map_err(|err| {


Same here (and below with LocalBatchProver)

igamigo · 2026-06-26T19:42:27Z

@@ -563,23 +601,8 @@

        let mut entries: Vec<(Word, Word)> = Vec::new();


We can type this better and make it Vec<(AssetVaultKey, Word)>

igamigo · 2026-06-26T19:43:38Z

-        let lineage = Self::vault_lineage_id(account_id);
-        let prev_tree =
-            self.forest.latest_version(lineage).map(|version| TreeId::new(lineage, version));
+        assert!(!vault_patch.is_empty(), "expected the patch not to be empty");


Let's add a sentence or section in the doc comments mentioning that this expects a non-empty patch and it panics otherwise

igamigo · 2026-06-26T19:43:57Z

-            self.forest.latest_version(lineage).map(|version| TreeId::new(lineage, version));
+        assert!(!vault_patch.is_empty(), "expected the patch not to be empty");

        let mut entries: Vec<(Word, Word)> = Vec::new();


Same here about typing this better

igamigo · 2026-06-26T19:46:45Z

        assert_eq!(prev_root, empty_smt_root(), "account should not be in the forest");
        assert!(
            self.forest.latest_version(lineage).is_none(),
            "account should not be in the forest"
        );


I would also mention the panics in teh docs of this function

igamigo · 2026-06-26T19:47:34Z


    /// Inserts asset vault data into the forest for the specified account. Assumes that asset vault
    /// for this account does not yet exist in the forest.
+    #[expect(clippy::unnecessary_wraps)]


I think we can remove Result<> from this function now, and this should propagate (I think) to some of the code that calls this

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

		ProofKind::Batch => Self::Batch(LocalBatchProver::new()),
		ProofKind::Block => Self::Block(LocalBlockProver::new(MIN_PROOF_SECURITY_LEVEL)),

		let executed_batch =
		BatchExecutor::new().execute(proposed_batch).map_err(\|err\| {

		@@ -563,23 +601,8 @@

		let mut entries: Vec<(Word, Word)> = Vec::new();

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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

juan518munoz Jun 24, 2026 •

edited

Loading