feat: Validate and prove blocks by sergerad · Pull Request #1381 · 0xMiden/node

sergerad · 2025-11-25T21:46:26Z

Context

We have updated base types to support deferred block proving. The node needs to be integrated with the new types.

The deferred proving flow is being worked towards. Eventually the store will make the proving request that the block producer is making currently. In the meantime, we can iterate on the current block production flow by adding block validation via the validator component.

Relates to #1316.

Changes

Update base dependancies.
Add block validation endpoint to validator component.
Add validator RPC client.
Integrate validator client to block producer.
Update block producer to validate blocks through the validator rpc.

crates/block-producer/src/block_builder/mod.rs

crates/proto/src/domain/proof_request.rs

sergerad · 2025-11-27T02:48:19Z

crates/block-producer/src/block_builder/mod.rs

+        };
+        tracing::debug!(target: COMPONENT, ?message);
+        let request = tonic::Request::new(message);
+        let _response = self.validator.clone().validate_block(request).await.unwrap(); // todo: rm unwrap


@bobbinth would the block producer do this here?

Yes - though I was imagining the flow a bit differently:

We get the proposed block and send it to the validator [as we do now].

The validator builds the block and returns the block header and body. As a part of this, the validator would sign the blog header (this part still needs to be implemented in miden-base).

The block producer checks them to make sure they are consistent.

Then, we send the block header, transaction batches, and block inputs to the prover (we currently send the block body to the prove as well, but I'm not sure why this is needed).

The prover returns the block proof.

We build the proven block.

Once we move the proving to the store, the last 3 steps will need to happen there. For this, instead of sending a proven block to the store, we'll need to send (block_header, block_body, tx_batches, block_inputs) to the store (probably organized into a struct). We could also skip sending block_inputs because the store should theoretically be able to build them - but it may be easier to just send them.

Once we move the proving to the store, the last 3 steps will need to happen there. For this, instead of sending a proven block to the store, we'll need to send (block_header, block_body, tx_batches, block_inputs) to the store (probably organized into a struct). We could also skip sending block_inputs because the store should theoretically be able to build them - but it may be easier to just send them.

Just to ensure I/we are on the same page. This is intended for a separate PR/future work correct? Because that would also entail changes on gRPC and client etc to allow for validated but as yet unproven blocks.

crates/block-producer/src/block_builder/mod.rs

Mirko-von-Leipzig

I think the large strokes are correct modulo the comments from @bobbinth and the step separation. We can also follow these up in a separate PR to unblock other PRs waiting on the protocol changes

sergerad · 2025-11-27T20:39:01Z

crates/block-producer/src/block_builder/mod.rs

+            .await
+            .map_err(BuildBlockError::ValidateBlockFailed)?;
+
+        // TODO: Check that the returned header and body match the proposed block.


@bobbinth what checks should we perform here?

Not for this PR, but I think here we'd want to check two things:

That the block_header returned from the validator is consistent with the proposed_block.

That the block_body returned from the validator is consistent with the block_header and proposed_block.

This is protection against the validator becoming malicious/inconsistent and returning an invalid block which will fail to prove later on.

The way to do this is probably by converting proposed_block into block_header and block_body in this function, and checking these against the data we get from the validator (excluding the signature). Since this is a non-negligible operation in terms of performance (would be good to benchmark how long it actually would take at some point), we should do this in parallel with the request to the validator. Meaning:

We make the request to the validator but don't await on it.

We build the block header and body locally.

We await on the validator request - and once it comes back do the comparison.

…lidation-endpoints

Mirko-von-Leipzig · 2025-11-28T06:51:09Z

crates/block-producer/src/block_builder/mod.rs

+        body: BlockBody,
+        block_proof: BlockProof,
+    ) -> Result<ProvenBlock, BuildBlockError> {
+        let proven_block = ProvenBlock::new_unchecked(header, body, block_proof);


Could you add a // SAFETY comment explaining why whatever invariants new_unchecked requires are upheld?

Mirko-von-Leipzig · 2025-11-28T06:52:03Z

crates/block-producer/src/block_builder/mod.rs

+/// passed in the proposed block.
+///
+/// This expects that transactions from the proposed block and proven block are in the same
+/// order, as define by [`OrderedTransactionHeaders`].


Suggested change

/// order, as define by [`OrderedTransactionHeaders`].

/// order, as defined by [`OrderedTransactionHeaders`].

Mirko-von-Leipzig · 2025-11-28T06:54:42Z

crates/block-producer/src/block_builder/mod.rs

+    if proposed_txs.as_slice().len() != proven_block.body().transactions().as_slice().len() {
+        return Err(BuildBlockError::other(format!(
+            "remote prover returned {} transaction headers but {} transactions were passed as part of the proposed block",
+            proven_block.body().transactions().as_slice().len(),
+            proposed_txs.as_slice().len()
+        )));
+    }


fyi we could also use itertools::equal to compare the iterators including length.

Up to you; your impl does give more info

Mirko-von-Leipzig · 2025-11-28T07:04:01Z

crates/block-producer/src/validator/mod.rs

+        let response = response.into_inner();
+
+        // Extract header from response (should always be present).
+        let header_proto = response.header.expect("validator always returns a header");


I wouldn't rely on this expectation, this should be a deserialization error.

In the proto crate we use https://github.com/0xMiden/miden-node/blob/8d76576ef63b1e06402b0c729e836a149507c456/crates/proto/src/errors/mod.rs#L64-L75

which gets uses something like https://github.com/0xMiden/miden-node/blob/8d76576ef63b1e06402b0c729e836a149507c456/crates/proto/src/domain/account.rs#L121-L123

Having the client here is fine for now but I would make these errors instead (and same below).

Mirko-von-Leipzig · 2025-11-28T07:04:10Z

crates/block-producer/src/validator/mod.rs

+// VALIDATOR CLIENT
+// ================================================================================================
+
+/// Interface to the validator's block-producer gRPC API.


Suggested change

/// Interface to the validator's block-producer gRPC API.

/// Interface to the validator's gRPC API.

bobbinth

Looks good! Thank you! I left some comments inline, but we can probably merge as is and address the comments in follow-up PRs.

bobbinth · 2025-11-28T09:10:48Z

proto/proto/validator.proto

+   // Validates a proposed block and returns the block header and body.
+   rpc ValidateBlock(blockchain.ProposedBlock) returns (ValidateBlockResponse) {}


Not for this PR, but I wonder if the naming here can be improved a bit:

Technically, we are not so much validating the block as constructing and signing it - so, maybe something like BuildBlock or SignBlock would be better?

Also, maybe the response should be SignedBlock? Technically, we are returning a fully constructed (and in the future) signed block from here.

I don't love the suggestions above either - maybe there are some better options.

bobbinth · 2025-11-28T09:18:41Z

bin/node/src/commands/block_producer.rs

        /// Url at which to serve the gRPC API.
        #[arg(env = ENV_BLOCK_PRODUCER_URL)]
        url: Url,


Question not related to this PR: are the valid values for this parameter only localhost:[port] and 0.0.0.0:[port] - or could we pass any URL into this?

bobbinth · 2025-11-28T09:23:45Z

bin/remote-prover/src/api/prover.rs

            .try_lock()
            .map_err(|_| Status::resource_exhausted("Server is busy handling another request"))?
-            .prove(proposed_block)
+            .prove(tx_batches, block_header.clone(), block_inputs)


nit: we clone the block_header here so that we compute block_id on line 182 below, right? If so, we could compute block_id after line 174 above and avoid the clone.

bobbinth · 2025-11-28T09:28:50Z

crates/block-producer/src/lib.rs

 pub mod store;
+pub mod validator;


Question: why do these need to be public?

bobbinth · 2025-11-28T09:37:16Z

crates/proto/src/domain/proof_request.rs

+pub struct BlockProofRequest {
+    pub tx_batches: OrderedBatches,
+    pub block_header: BlockHeader,
+    pub block_inputs: BlockInputs,
+}


Not for this PR, but it would be great to add doc comments to this struct explaining the purpose of its struct and its fields. For example, I think the block header is included here mostly because of the signature as everything else in the header could be derived from tx_batches and block_inputs.

Also, I wonder if eventually this struct (potentially named differently) should live in miden-base.

bobbinth · 2025-11-28T09:51:06Z

crates/store/src/state.rs


        // build note tree
-        let note_tree = block.build_output_note_tree();
+        let note_tree = block.body().compute_block_note_tree();


nit (and not for this PR): build_output_note_tree() was probably a better name than compute_block_note_tree(). Let's remember to update this when we work on follow-up changes in miden-base.

bobbinth · 2025-11-28T10:08:40Z

crates/block-producer/src/block_builder/mod.rs

+            .await
+            .map_err(BuildBlockError::ValidateBlockFailed)?;
+
+        // TODO: Check that the returned header and body match the proposed block.


Not for this PR, but I think here we'd want to check two things:

That the block_header returned from the validator is consistent with the proposed_block.

That the block_body returned from the validator is consistent with the block_header and proposed_block.

This is protection against the validator becoming malicious/inconsistent and returning an invalid block which will fail to prove later on.

The way to do this is probably by converting proposed_block into block_header and block_body in this function, and checking these against the data we get from the validator (excluding the signature). Since this is a non-negligible operation in terms of performance (would be good to benchmark how long it actually would take at some point), we should do this in parallel with the request to the validator. Meaning:

We make the request to the validator but don't await on it.

We build the block header and body locally.

We await on the validator request - and once it comes back do the comparison.

bobbinth · 2025-11-28T10:10:48Z

crates/block-producer/src/block_builder/mod.rs

        }
-
-        self.simulate_proving().await;
+        validate_tx_headers(&proven_block, &ordered_batches.to_transactions())?;


I'm not sure this is actually needed here - but maybe for now it is fine. Once we have the actual block proving, this would be replaced with verifying the proof returned from the prover against the block header. Maybe worth adding a comment here about this.

sergerad added 2 commits November 26, 2025 10:27

Upgrade next and compile

5ac0da6

refactor prove interface and return block proof

170ec58

sergerad marked this pull request as draft November 25, 2025 22:25

sergerad added 4 commits November 26, 2025 12:01

Map proposed block error

e981dc5

Fix up prove_block

59c9ef5

Move proof request type

d66eb83

Reinstate validate

c84242f

sergerad mentioned this pull request Nov 26, 2025

chore: Add serialization for BlockInputs 0xMiden/protocol#2112

Merged

sergerad added 2 commits November 26, 2025 17:32

RM comment and add missing file

60ccf93

toml

0beea32

sergerad commented Nov 26, 2025

View reviewed changes

crates/block-producer/src/block_builder/mod.rs Outdated Show resolved Hide resolved

sergerad commented Nov 26, 2025

View reviewed changes

crates/proto/src/domain/proof_request.rs Show resolved Hide resolved

sergerad added 2 commits November 27, 2025 13:08

Update dep and finish serde

005c4cc

Add validation endpoint, client, and integrate

26362a6

sergerad commented Nov 27, 2025

View reviewed changes

bobbinth reviewed Nov 27, 2025

View reviewed changes

crates/block-producer/src/block_builder/mod.rs Outdated Show resolved Hide resolved

drahnr mentioned this pull request Nov 27, 2025

feat: integrate NullifierTree<Memory..> #1364

Merged

Mirko-von-Leipzig approved these changes Nov 27, 2025

View reviewed changes

sergerad added 10 commits November 28, 2025 08:09

Fix build

b25a1a1

Split validate and prove

4906ea6

Build block in validator

c8a82ea

Validator builds blocks

f2ad156

Expect some header and body

de4c081

Update .env

d68f6ea

RM unusued variant

326ba8d

Nit

42b17d6

Pass batches instead

cf410aa

todo comment

e052275

sergerad requested a review from bobbinth November 27, 2025 20:38

sergerad commented Nov 27, 2025

View reviewed changes

juan518munoz mentioned this pull request Nov 27, 2025

chore: update miden-base to latest version #1390

Closed

Merge branch 'next' of github.com:0xMiden/miden-node into sergerad-va…

d8f80bf

…lidation-endpoints

sergerad marked this pull request as ready for review November 27, 2025 22:24

Update changelog

e5aa48a

Mirko-von-Leipzig approved these changes Nov 28, 2025

View reviewed changes

chore: minor import reshuffling

0604057

bobbinth approved these changes Nov 28, 2025

View reviewed changes

drahnr added 3 commits November 28, 2025 11:16

hack: enable test

f67b806

toml lint

e575488

taplo is giving up on comments extending line length

8311cbb

drahnr mentioned this pull request Nov 28, 2025

follow up to validate and prove blocks #1381 #1395

Closed

drahnr merged commit db22698 into next Nov 28, 2025
6 checks passed

drahnr deleted the sergerad-validation-endpoints branch November 28, 2025 10:57

This was referenced Dec 1, 2025

chore: Misc followups for block validation #1402

Merged

Guardrails: Tx and Block Validation #1316

Open

	/// order, as define by [`OrderedTransactionHeaders`].
	/// order, as defined by [`OrderedTransactionHeaders`].

	/// Interface to the validator's block-producer gRPC API.
	/// Interface to the validator's gRPC API.

		// Validates a proposed block and returns the block header and body.
		rpc ValidateBlock(blockchain.ProposedBlock) returns (ValidateBlockResponse) {}

Conversation

sergerad commented Nov 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Context

Changes

Uh oh!

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!

Uh oh!

Mirko-von-Leipzig 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!

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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

sergerad commented Nov 25, 2025 •

edited

Loading