msggen: Move overrides into the model itself

We were using per-type overrides which caused some asymmetries, where
conversions could end up dropping fields as we went along. Essentially
each conversion would need to override a superset of the previous one,
which then caused issues when attempting to close the loop. By
overriding on the model level we ensure that all representations are
equivalent and convertible into one another, at the expense of
overriding a bit more aggressively, which should be fine anyway.
This commit is contained in:
Christian Decker
2023-05-03 13:41:18 +02:00
committed by Rusty Russell
parent 1e614dfb8a
commit db843159ea
6 changed files with 136 additions and 69 deletions

View File

@@ -21,3 +21,22 @@ digraph {
"Rust JSON-RPC structs" -> "cln-rpc";
}
```
`msggen` will load the schemas in `doc/schemas` into memory, and then
use `Patches` to enrich the model before using it to generate the
bindings for the various languages as well as the converters from one
format to another. These patches can be found in `msggen/patch.py` and
perform a variety of operations:
- Annotate the model with additional data from external sources, such
as the `.msggen.json` file at the repository root to track details
that can be derived but should remain constant (grpc field
numbering and versioning information)
- Aggregate common types with type overrides and omit fields that we
can't map currently.
- Infer optionality based on the versions a field was added or
deprecated, and the currently supported range of versions.
If there is a field that is currently missing in the model, that is in
the schemas it is most likely because it has been marked as omitted in
the patch.

View File

@@ -8,7 +8,7 @@ from msggen.gen.rust import RustGenerator
from msggen.gen.generator import GeneratorChain
from msggen.utils import load_jsonrpc_service
import logging
from msggen.patch import VersionAnnotationPatch, OptionalPatch
from msggen.patch import VersionAnnotationPatch, OptionalPatch, OverridePatch
from msggen.checks import VersioningCheck
@@ -73,6 +73,10 @@ def run(rootdir: Path):
p.apply(service)
OptionalPatch().apply(service)
# Mark some fields we can't map as omitted, and for complex types
# we manually mapped, use those types instead.
OverridePatch().apply(service)
# Run the checks here, we should eventually split that out to a
# separate subcommand
VersioningCheck().check(service)

View File

@@ -33,30 +33,9 @@ typemap = {
}
# Manual overrides for some of the auto-generated types for paths
overrides = {
# Truncate the tree here, it's a complex structure with identitcal
# types
'ListPeers.peers[].channels[].state_changes[]': None,
'ListPeers.peers[].channels[].htlcs[].state': None,
'ListPeers.peers[].channels[].opener': "ChannelSide",
'ListPeers.peers[].channels[].closer': "ChannelSide",
'ListPeers.peers[].channels[].features[]': "string",
'ListPeerChannels.channels[].state_changes[]': None,
'ListPeerChannels.channels[].htlcs[].state': None,
'ListPeerChannels.channels[].opener': "ChannelSide",
'ListPeerChannels.channels[].closer': "ChannelSide",
'ListPeerChannels.channels[].features[]': None,
'ListPeerChannels.channels[].channel_type': None,
'ListFunds.channels[].state': 'ChannelState',
'ListTransactions.transactions[].type[]': None,
'ListClosedChannels.closedchannels[].closer': "ChannelSide",
'ListClosedChannels.closedchannels[].opener': "ChannelSide",
'ListClosedChannels.closedchannels[].channel_type': None,
}
# GRPC builds a stub with the methods declared in the protobuf file,
# but it also comes with its own methods, e.g., `connect` which can
# clash with the generated ones. So rename the ones we know clash.
method_name_overrides = {
"Connect": "ConnectPeer",
}
@@ -188,7 +167,7 @@ class GrpcGenerator(IGenerator):
self.write(f"""{prefix}}}\n""", False)
def generate_message(self, message: CompositeField):
if overrides.get(message.path, "") is None:
if message.omit():
return
self.write(f"""
@@ -197,33 +176,26 @@ class GrpcGenerator(IGenerator):
# Declare enums inline so they are scoped correctly in C++
for _, f in enumerate(message.fields):
if isinstance(f, EnumField) and f.path not in overrides.keys():
if isinstance(f, EnumField) and not f.override():
self.generate_enum(f, indent=1)
for i, f in self.enumerate_fields(message.typename, message.fields):
if overrides.get(f.path, "") is None:
if f.omit():
continue
opt = "optional " if f.optional else ""
if isinstance(f, ArrayField):
typename = typemap.get(f.itemtype.typename, f.itemtype.typename)
if f.path in overrides:
typename = overrides[f.path]
typename = f.override(typemap.get(f.itemtype.typename, f.itemtype.typename))
self.write(f"\trepeated {typename} {f.normalized()} = {i};\n", False)
elif isinstance(f, PrimitiveField):
typename = typemap.get(f.typename, f.typename)
if f.path in overrides:
typename = overrides[f.path]
typename = f.override(typemap.get(f.typename, f.typename))
self.write(f"\t{opt}{typename} {f.normalized()} = {i};\n", False)
elif isinstance(f, EnumField):
typename = f.typename
if f.path in overrides:
typename = overrides[f.path]
typename = f.override(f.typename)
self.write(f"\t{opt}{typename} {f.normalized()} = {i};\n", False)
elif isinstance(f, CompositeField):
typename = f.typename
if f.path in overrides:
typename = overrides[f.path]
typename = f.override(f.typename)
self.write(f"\t{opt}{typename} {f.normalized()} = {i};\n", False)
self.write(f"""}}
@@ -263,7 +235,7 @@ class GrpcConverterGenerator(IGenerator):
def generate_composite(self, prefix, field: CompositeField):
"""Generates the conversions from JSON-RPC to GRPC.
"""
if overrides.get(field.path, "") is None:
if field.omit():
return
# First pass: generate any sub-fields before we generate the
@@ -284,7 +256,7 @@ class GrpcConverterGenerator(IGenerator):
""")
for f in field.fields:
if overrides.get(f.path, "") is None:
if f.omit():
continue
name = f.normalized()
@@ -423,7 +395,7 @@ class GrpcUnconverterGenerator(GrpcConverterGenerator):
def generate_composite(self, prefix, field: CompositeField) -> None:
# First pass: generate any sub-fields before we generate the
# top-level field itself.
if overrides.get(field.path, "") is None:
if field.omit():
return
for f in field.fields:
@@ -443,7 +415,7 @@ class GrpcUnconverterGenerator(GrpcConverterGenerator):
for f in field.fields:
name = f.normalized()
if overrides.get(f.path, "") is None:
if f.omit():
continue
if isinstance(f, ArrayField):

View File

@@ -15,25 +15,6 @@ logger = logging.getLogger(__name__)
# built-in keywords.
keywords = ["in", "type"]
# Manual overrides for some of the auto-generated types for paths
# Manual overrides for some of the auto-generated types for paths
overrides = {
'ListPeers.peers[].channels[].state_changes[].old_state': "ChannelState",
'ListPeers.peers[].channels[].state_changes[].new_state': "ChannelState",
'ListPeers.peers[].channels[].state_changes[].cause': "ChannelStateChangeCause",
'ListPeers.peers[].channels[].htlcs[].state': None,
'ListPeers.peers[].channels[].opener': "ChannelSide",
'ListPeers.peers[].channels[].closer': "ChannelSide",
'ListPeers.peers[].channels[].features[]': "string",
'ListFunds.channels[].state': 'ChannelState',
'ListTransactions.transactions[].type[]': None,
'Invoice.exposeprivatechannels': None,
'ListClosedChannels.closedchannels[].closer': "ChannelSide",
'ListClosedChannels.closedchannels[].opener': "ChannelSide",
'ListClosedChannels.closedchannels[].channel_type': None,
}
# A map of schema type to rust primitive types.
typemap = {
'boolean': 'bool',
@@ -44,7 +25,7 @@ typemap = {
'number': 'f64',
'pubkey': 'PublicKey',
'short_channel_id': 'ShortChannelId',
'signature': 'String',
'signature': 'Vec<u8>',
'string': 'String',
'txid': 'String',
'float': 'f32',
@@ -79,6 +60,8 @@ def normalize_varname(field):
def gen_field(field):
if field.omit():
return ("", "")
if isinstance(field, CompositeField):
return gen_composite(field)
elif isinstance(field, EnumField):
@@ -94,7 +77,7 @@ def gen_field(field):
def gen_enum(e):
defi, decl = "", ""
if e.path in overrides and overrides[e.path] is None:
if e.omit():
return "", ""
if e.description != "":
@@ -132,9 +115,9 @@ def gen_enum(e):
typename = e.typename
if e.path in overrides:
if e.override() is not None:
decl = "" # No declaration if we have an override
typename = overrides[e.path]
typename = e.override()
if not e.optional:
defi = f" // Path `{e.path}`\n"
@@ -176,9 +159,9 @@ def gen_array(a):
logger.debug(f"Generating array field {a.name} -> {name} ({a.path})")
_, decl = gen_field(a.itemtype)
if a.path in overrides:
if a.override():
decl = "" # No declaration if we have an override
itemtype = overrides[a.path]
itemtype = a.override()
elif isinstance(a.itemtype, PrimitiveField):
itemtype = a.itemtype.typename
elif isinstance(a.itemtype, CompositeField):

View File

@@ -40,6 +40,14 @@ class Field:
self.deprecated = deprecated
self.required = False
# Are we going to omit this field when generating bindings?
# This usually means that the field either doesn't make sense
# to convert or that msggen cannot handle converting this
# field and its children yet.
self.omitted = False
self.type_override: Optional[str] = None
@property
def name(self):
return FieldName(self.path.split(".")[-1])
@@ -53,6 +61,37 @@ class Field:
def normalized(self):
return self.name.normalized()
def capitalized(self):
return self.name.capitalized()
def omit(self):
"""Returns true if we should not consider this field in our model.
This can be either because the field is redundant, or because
msggen cannot currently handle it. The field (and it's type if
it's composite) will not be materialized in the generated
bindings and converters.
It is mainly switched on and off in the OverridePatch which is
the central location where we manage overrides and omissions.
"""
return self.omitted
def override(self, default: Optional[str] = None) -> Optional[str]:
"""Provide a type that should be used instead of the inferred one.
This is useful if for shared types that we don't want to
generate multiple times, and for enums that can result in
naming clashes in the grpc model (enum variantss must be
uniquely name in the top-level scope...).
It is mainly switched on and off in the OverridePatch which is
the central location where we manage overrides and omissions.
"""
return self.type_override if self.type_override else default
class Service:
"""Top level class that wraps all the RPC methods.

View File

@@ -133,3 +133,53 @@ class OptionalPatch(Patch):
if f.deprecated and self.versions.index(f.deprecated) < idx[1]:
f.optional = True
class OverridePatch(Patch):
"""Allows omitting some fields and overriding the type of fields based on configuration.
"""
omit = [
'Decode.invoice_paths[]',
'Decode.invoice_paths[].payinfo',
'Decode.offer_paths[].path[]',
'Decode.offer_recurrence',
'Decode.routes[][]',
'Decode.unknown_invoice_request_tlvs[]',
'Decode.unknown_invoice_tlvs[]',
'Decode.unknown_offer_tlvs[]',
'DecodePay.routes[][]',
'DecodeRoutes.routes',
'Invoice.exposeprivatechannels',
'ListClosedChannels.closedchannels[].channel_type',
'ListPeerChannels.channels[].channel_type',
'ListPeerChannels.channels[].features[]',
'ListPeerChannels.channels[].htlcs[].state',
'ListPeerChannels.channels[].state_changes[]',
'ListPeers.peers[].channels[].htlcs[].state',
'ListPeers.peers[].channels[].state_changes[]',
'ListTransactions.transactions[].type[]',
]
# Handcoded types to use instead of generating the types from the
# schema. Useful for repeated types, and types that have
# redundancies.
overrides = {
'ListClosedChannels.closedchannels[].closer': "ChannelSide",
'ListClosedChannels.closedchannels[].opener': "ChannelSide",
'ListFunds.channels[].state': 'ChannelState',
'ListPeerChannels.channels[].closer': "ChannelSide",
'ListPeerChannels.channels[].opener': "ChannelSide",
'ListPeers.peers[].channels[].closer': "ChannelSide",
'ListPeers.peers[].channels[].features[]': "string",
'ListPeers.peers[].channels[].opener': "ChannelSide",
'ListPeers.peers[].channels[].state_changes[].cause': "ChannelStateChangeCause",
'ListPeers.peers[].channels[].state_changes[].old_state': "ChannelState",
'ListPeers.peers[].channels[].state_changes[].old_state': "ChannelState",
}
def visit(self, f: model.Field) -> None:
"""For now just skips the fields we can't convert.
"""
f.omitted = f.path in self.omit
f.type_override = self.overrides.get(f.path, None)