fix: preserve unsigned metadata in canonical surface contracts - #248
Open
patrickleet wants to merge 2 commits into
Open
fix: preserve unsigned metadata in canonical surface contracts#248patrickleet wants to merge 2 commits into
patrickleet wants to merge 2 commits into
Conversation
Preserve typed nonnegative integer ranges through command derives, portable metadata, generated codecs and U64 projection proofs. Reject inexact browser numbers before optimism or dispatch while preserving native unsigned range and signed codecs. BREAKING CHANGE: manually constructed command/surface fields require unsigned_integer metadata; the protocol fingerprint changes and services and generated clients must be rebuilt together.
Refs [[unsigned-surface-metadata-breaks-canonical-typed-m]]
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Typed application manifests with unsigned command fields fail
NonCanonical("surface contract material"): the Surface emitter includesunsigned_integer, but manifest reconstruction dropped it. Preserve that optional metadata recursively for command inputs and outputs. Unrefined fields retain their existing JSON shape and fingerprints.The regression uses the same
Service::applicationpath as Forge, with intentionally unsorted derived fields, all unsigned widths, optional/list/nested types, ordinary signed/string fields, canonical byte round trips, and rejection of altered unsigned metadata. The unsigned-input documentation now describes canonical manifest retention.Validation:
NonCanonical("surface contract material").cargo test --all-features --lib: 1,075 passed, 4 ignored.cargo test --all-features --test application_composition --test application_plans: 35 passed.cargo test -p distributed_cli: 293 passed, 11 ignored.cargo test -p distributed_cli --test cli_manifest -- --ignored: 6 passed, including existing fingerprint assertions.rustfmt --check;git diff --checkpasses.This branch starts at #247 and targets its same base,
fix/aggregate-replay-event-version; the incremental fix is commit be009bd. The inherited #247 unsigned-input changes are breaking for manually constructed command type fields and require service/client regeneration. This follow-up adds no further API or wire-version change. No Forge source or dependency workaround is included.Separate finding: direct Module composition can reject unsorted authored command fields because its ownership comparison sees declaration order versus Surface order. Forge's Service path already calls
with_surface_binding, replacing those shapes with the canonical Surface versions, and the regression verifies that path with unsorted source fields. The direct-Module finding is recorded separately in GitKBunsigned-surface-metadata-breaks-canonical-typed-m; it is outside this fix.