Repository navigation
grpc: freeze the gRPC wire format and enforce required fields up to v26.06 and more - #9678
Draft
daywalker90 wants to merge 7 commits into
Draft
daywalker90 wants to merge 7 commits into
daywalker90 wants to merge 7 commits into
Conversation
missing-grpc printed commands without gRPC coverage but always exited 0, so nothing enforced the mapping. Add an explicit grpc_method_blacklist with a reason per command, and fail when a command is neither mapped nor blacklisted, or when a blacklist entry is stale. Run it from check-source. Changelog-None
When a variant is removed from a JSON schema enum, msggen must
not reassign its proto3 integer value to another variant — that
is a breaking wire change for gRPC clients.
Teach enumerate_enum() to scan grpc-enum-map for entries that
were previously assigned a number but are no longer present in
the schema. Emit those entries with [deprecated = true] in the
generated proto3 definition, which satisfies three constraints:
1. Existing wire values are never reused.
2. proto3's requirement that the first enum value is 0 is met
even when the variant that held 0 is removed.
3. New code never produces the removed variant; old clients
that send it hit the TryFrom<i32> error arm in the Rust
model.
Changelog-None
Every field number that msggen emits is recorded in .msggen.json (grpc-wire-contract) together with its proto3 label and type. When a JSON schema change later implies a different label or type for a number that was already released, msggen keeps emitting the released one and records the difference so the Rust conversions adapt the value instead of changing the wire format. A change that cannot be bridged that way aborts generation. This pins down: - A field released as optional stays optional, so 0/""/false is never silently reinterpreted as "absent". Adding optional is allowed and does not change the encoding. - uint64 released fields that the schema now types as uint32 stay uint64 on the wire and are range-checked in the Rust conversion. - A double released field that the schema now types as an integer stays double. - A string released field that the schema now types as bytes/pubkey stays a string and is hex-decoded/encoded in the conversion. - Sat-denominated request fields (AmountSat/AmountSatOrAll) keep their released number as a deprecated msat field and move the sat value to a fresh *_sat field number. Setting both is rejected, and the old msat field must hold a whole number of satoshis. - Numbers that are no longer emitted are marked reserved. The cln-rpc Rust types follow the schema as before; only the gRPC wire format is frozen. Changelog-None
OptionalPatch.supported() moves from v24.11 to v26.06, so schema fields marked required and added up to v26.06 become non-optional. A field added after v26.06 stays optional. Changelog-Changed: JSON-RPC and cln-rpc: fields marked required in the schema and added up to v26.06 are required in the Rust types, e.g. `openchannel` hook `channel_type` and onion message `first_path_key`. `Option<T>` is now `T`.
Add check-grpc-breaking to check-source. It finds the newest release tag reachable from GRPC_BREAKING_BASE_REF and runs buf breaking against the protos at that tag, using the WIRE_JSON rules. The buf CLI is installed as a dev dependency via buf-bin. Changelog-None
Pass GRPC_BREAKING_BASE_REF so the check compares against the newest release reachable from the PR base. Changelog-None
WithdrawRequest.satoshi is the released msat field, and the sat value moved to satoshi_sat (see msggen: keep the generated proto wire format stable). Sending AmountSatOrAll to satoshi no longer type-checks, so the amount_sat_or_all validation case must target satoshi_sat. Changelog-None
This branch has not been deployed
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.
Makes the gRPC wire format stable across schema changes, and makes required fields non-optional in the Rust types up to v26.06 (supported version).
msggen now records the labels and types of every released gRPC field number in
.msggen.json(grpc-wire-contract). When a later schema change implies a different label or type for a released number, msggen keeps emitting the released one and bridges the difference in the Rust conversion. A change it cannot bridge aborts generation.make checknow fails if an RPC command has no gRPC mapping.record-wire-contract <rev>, which rebuilds the contract from a releasednode.proto. I ran this with the v26.06 tag.CI now runs
buf breakingto check if any grpc breaking changes were introduced.This returns node.proto to be compatible again with the v26.06 release. (Notably the Amount -> AmountSat change).