Separate protobuf decoding from verification - #3817
Conversation
kkovaacs
left a comment
kkovaacs
left a comment
There was a problem hiding this comment.
I like the decode/verify structure. I've added a few comments, plus I think that the protobuf message changes should be split off of this huge PR.
I think the verify errors add a lot of complexity to the code though (the VerificationError types are quite complex for non-trivial types). In the end we're effectively converting those to strings on the API, so I'm not 100% convinced that precise errors are worth it?
| #[test] | ||
| fn build_helper_and_derives_support_a_renamed_runtime_dependency() { | ||
| let manifest_dir = std::path::Path::new(env!("CARGO_MANIFEST_DIR")); | ||
| // Do not reuse the parent's target directory: Cargo holds its build lock while testing. | ||
| let target = manifest_dir.join("../../target/protobuf-renamed-consumer"); | ||
| let output = std::process::Command::new(env!("CARGO")) | ||
| .args(["test", "--offline", "--quiet", "--manifest-path"]) | ||
| .arg(manifest_dir.join("tests/fixtures/renamed/Cargo.toml")) | ||
| .arg("--target-dir") | ||
| .arg(target) | ||
| .env_remove("CARGO_TARGET_TMPDIR") | ||
| .output() | ||
| .expect("run renamed consumer fixture"); | ||
| assert!( | ||
| output.status.success(), | ||
| "renamed consumer failed:\n{}\n{}", | ||
| String::from_utf8_lossy(&output.stdout), | ||
| String::from_utf8_lossy(&output.stderr) | ||
| ); | ||
| } |
There was a problem hiding this comment.
What exactly is this test testing?
There was a problem hiding this comment.
A bug I had early on was that the macro hardcoded the miden-protobuf crate name paths ::miden-protobuf, but this fails if one renames or otherwise aliases miden-protobuf to something else.
So the example itself renames the dependency, this test just runs the example and ensures it works.
I'll add a comment; or perhaps just say ensure_example_works.
2 participants
Summary
Separate structural protobuf decoding from explicit domain verification. Prerequisite PR #3827 (
mirko/protobuf-schema) contains the schema changes and matching handwritten conversions; this PR builds on it and contains no further schema changes.Introduce two protocol-independent crates:
miden-protobufdefines decoding errors, field adapters,DecodeMessage, the construction traits, build configuration, and compile-time-checked infallible result extraction.miden-protobuf-derivegenerates schema-shaped records and oneof enums, recursive field conversion, and complete structural error paths, including repeated-field indexes. Prost enums become named enum values rather than unchecked integers.miden-objectsuses the generated records and handwritten verification implementations. Composite verifiers share a source-preservingVerificationError; direct domain errors and specific invariant errors remain available through the source chain. Infallible verification retainsInfallible. Encoding remains handwritten.Presence Policy
Message and oneof fields are required by default unless explicitly optional in the schema. Build configuration preserves the descriptor's optional-presence metadata that would otherwise be lost from the generated Rust fields. Proto3 scalar fields retain their normal default-value behavior.
Usage
After Prost decodes the wire bytes, call
message.decode_fields()?to recursively decode its fields.Decoded<P>is the decoded representation of the wire typeP, not its domain counterpart.Then explicitly choose an opt-in, handwritten construction capability:
decoded.verify()?checks self-contained domain invariants.decoded.verify_with(context)?uses caller-supplied context, for example a signed block verified against an already-trusted parent header.decoded.build_unchecked()?constructs a domain value while skipping the documented checks. Remaining construction invariants can still fail.For example,
wire_signed_block.decode_fields()?.verify_with(&trusted_parent)?checks header/body consistency and authenticates the block against the parent's validators. Those consistency checks run only once.Structural decoding errors receive generated field/index paths. Semantic verification errors preserve their sources but do not receive generated wire paths. No generated direct protobuf-to-domain conversion bypasses the explicit construction decision.
The standalone consumer example under
crates/miden-protobuf/examples/consumerdemonstrates build configuration, generated records and oneofs, payload parsing, and decoding errors outsidemiden-objects.Verification
make formatcargo test -p miden-objects -p miden-protobuf -p miden-protobuf-derive --all-features --locked --offline(215 tests, including the consumer example runner)cargo check -p miden-objects --no-default-features --locked --offlinecargo clippy -p miden-objects -p miden-protobuf -p miden-protobuf-derive --all-features --all-targets --locked --offline -- -D warnings