Skip to content

refactor(proto): replace composite gRPC byte payloads with structured messages - #2471

Draft
kkovaacs wants to merge 8 commits into
nextfrom
krisztian/grpc-api-protobuf-messages
Draft

refactor(proto): replace composite gRPC byte payloads with structured messages#2471
kkovaacs wants to merge 8 commits into
nextfrom
krisztian/grpc-api-protobuf-messages

Conversation

@kkovaacs

Copy link
Copy Markdown
Collaborator

Summary

This PR addresses GitHub issue #1882 by replacing opaque, Miden-serialized byte payloads in the node’s gRPC APIs with fine-grained Protobuf messages for notes, blocks, transactions, batches, sequencer requests, and validator block proposals.

  • Replace opaque Miden-serialized gRPC payloads with structured Protobuf messages for notes, blocks, proven transactions, proposed/proven batches, sequencer requests, and validator block proposals.
  • Add fine-grained schemas for field elements, words, account patches, note details and attachments, block headers and bodies, partial blockchains, output notes, and transaction data.
  • Add Rust conversion and validation layers covering required fields, canonical encodings and ordering, collection limits, duplicate detection, commitments, account updates, Merkle witnesses, and MMR data.
  • Migrate RPC, sequencer, validator, block producer, store, monitoring, recovery, benchmark, and NTX-builder call sites to the new messages.
  • Keep cryptographic execution proofs, MAST forests, keys, signatures, ciphertext, and other primitive leaves in their canonical byte encodings.
  • The remote-prover API remains unchanged.

Breaking changes

This is a big-bang wire-format migration. Clients must regenerate bindings from the Protobuf definitions shipped with this node release; compatibility with the former composite bytes fields is intentionally not retained.

Tradeoffs

An important implementation compromise is the BatchAccountUpdate constructor workaround.

  • The pinned miden-protocol does not expose a validated parts constructor. Consequently, the production protobuf crate enables the protocol’s testing feature and calls BatchAccountUpdate::new_unchecked after locally reproducing its validation rules.
    • This exposes testing-only APIs in production builds.
    • Validation logic is duplicated and could drift when protocol invariants change.
    • The proper follow-up is an upstream validated constructor, after which the feature and local checks can be removed.

Other notable tradeoffs:

  • “Structured Protobuf” is not completely byte-free. Execution proofs remain canonical byte arrays in proto/proto/types/transaction.proto:87; MAST forests, keys, signatures, ciphertext, primitives, and full AccountDetails.details also remain opaque. The remote-prover API was deliberately left unchanged.
  • The standalone proven-batch decoder performs structural validation and deserializes the execution proof, but does not cryptographically verify it. It also reconstructs the BatchId because the ID is not transmitted. See crates/proto/src/domain/batch.rs:211. This preserves the existing internal trust boundary but means Protobuf conversion alone is not proof verification.
  • Several domain invariants are now manually mirrored in conversion code: ordering, limits, duplicate detection, account visibility, commitments, Merkle openings, and cross-field consistency. This gives better boundary errors but creates maintenance coupling to miden-protocol.
  • Maps are represented as canonically ordered repeated fields instead of native Protobuf maps. This preserves deterministic protocol ordering, but non-Rust clients must know how to sort account IDs, nullifiers, note IDs, and similar keys before submitting requests.
  • Structured messages are generally larger and more allocation-heavy than the former compact Miden serialization. They improve introspection and cross-language usability at the cost of additional wire overhead and conversion work.
  • Validator proposals require the block producer to retain and clone the original BlockInputs, because ProposedBlock does not preserve every original input needed to reproduce the structured request. See crates/block-producer/src/block_builder/mod.rs:267.
  • BlockProof is currently an empty presence-bearing envelope because the current domain proof has no structured fields. See proto/proto/types/blockchain.proto:108. It distinguishes “proof present” from “absent,” but will require schema evolution when block proofs gain content.
  • The tests exercise standalone non-empty batch conversion and empty proposed blocks through gRPC, but there is no single end-to-end validator gRPC test carrying a non-empty proposal with real account, nullifier, and note witnesses. The validator helper explicitly documents this limitation in bin/validator/src/server/validator_service/tests.rs:142.

Changelog

[[entry]]
scope       = "rpc"
impact      = "breaking"
description = "Replaced composite gRPC byte payloads with structured messages."

Add canonical protobuf wrappers for Miden field elements and words. Implement owned and borrowed domain conversions with strict encoded-length and canonical-value validation, plus focused round-trip and malformed-input tests.
Replace serialized note attachment payloads with validated protobuf messages backed by canonical Word wrappers. Update note and RPC conversions, reserve the removed wire fields, add attachment boundary and consistency tests, and lower the GetNotesById limit to keep worst-case responses under 4 MiB.
Assert the public descriptor exposes structured note messages and reserves legacy fields. Document the breaking client regeneration requirement and record the completed migration in grpc.md.
Replace opaque BlockBody, SignedBlock, and BlockProof gRPC payloads with validated protobuf structures. Add shared account patch, output note, and transaction header conversions, migrate all in-repository consumers, retain serialization at persistence boundaries, and add descriptor and round-trip coverage.
Replace opaque serialized transaction and batch payloads with fine-grained protobuf messages across RPC, sequencer, validator, and remote prover APIs. Add structured execution-proof and partial-blockchain envelopes, strict domain conversions and validation, and update clients, services, tests, and migration documentation.
Replace the structured execution-proof envelope in transaction and batch messages with canonical Miden-serialized byte fields. Restore the remote prover's generic proof-type and byte-payload API across its server and clients, remove the VM proof schema and conversions, and update tests and migration documentation.
Comment on lines +7 to +20
// A field element encoded by miden_protocol::utils::serde::Serializable.
message Felt {
// Exactly eight bytes containing a canonical field element.
bytes encoded = 1;
}

// WORD
// ================================================================================================

// A word encoded by miden_protocol::utils::serde::Serializable.
message Word {
// Exactly 32 bytes containing four canonical field elements.
bytes encoded = 1;
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you consider a more precise integer/fields approach?

Suggested change
// A field element encoded by miden_protocol::utils::serde::Serializable.
message Felt {
// Exactly eight bytes containing a canonical field element.
bytes encoded = 1;
}
// WORD
// ================================================================================================
// A word encoded by miden_protocol::utils::serde::Serializable.
message Word {
// Exactly 32 bytes containing four canonical field elements.
bytes encoded = 1;
}
message Felt {
uint64 a = 1;
uint64 b = 2;
uint64 c = 3;
uint64 d = 4;
}
// WORD
// ================================================================================================
message Word {
Felt a = 1;
Felt b = 2;
Felt c = 3;
Felt d = 4;
}

Comment on lines +167 to +209
impl BatchAccountUpdateProjection {
fn into_domain(self) -> Result<BatchAccountUpdate, ConversionError> {
if self.details.get_size_hint() > ACCOUNT_UPDATE_MAX_SIZE as usize {
return Err(ConversionError::message("account update exceeds the size limit"));
}

match (&self.details, self.account_id.is_private()) {
(AccountUpdateDetails::Private, true) => {},
(AccountUpdateDetails::Public(_), true) => {
return Err(ConversionError::message(
"private account update must not reveal public details",
));
},
(AccountUpdateDetails::Private, false) => {
return Err(ConversionError::message(
"public account update must include public details",
));
},
(AccountUpdateDetails::Public(patch), false) => {
if patch.id() != self.account_id {
return Err(ConversionError::message(
"public account patch ID does not match account ID",
));
}
if self.initial_state_commitment.is_empty() {
let account = Account::try_from(patch).map_err(ConversionError::new)?;
if account.to_commitment() != self.final_state_commitment {
return Err(ConversionError::message(
"new public account commitment does not match its full-state patch",
));
}
}
},
}

Ok(BatchAccountUpdate::new_unchecked(
self.account_id,
self.initial_state_commitment,
self.final_state_commitment,
self.details,
))
}
}

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@PhilippGackstatter FYI: miden-protocol does not expose a validated parts constructor for BatchAccountUpdate. As a workaround we're enabling protocol’s testing feature and calling BatchAccountUpdate::new_unchecked after locally reproducing its validation rules.

The proper fix is an upstream validated constructor so that this hack can be removed.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you need this in 0.16 or is 0.17 sufficient?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the code already exists on our end here, I guess either is fine.

We should perhaps just call out that this is temporary so it doesn't pollute the PR review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants