From 3747b966cad8ad0937a875bdbe0e2598700d33de Mon Sep 17 00:00:00 2001 From: Hugo Demeyere Date: Tue, 18 Aug 2026 20:14:25 +0900 Subject: [PATCH 1/2] fix(docgen): probe member converters to document their real JSON kind A member's CLR type does not determine its wire form. A field carrying an explicit [JsonConverter] can emit something other than the hex-quantity string its type implies - e.g. two `ulong` fields where one has a raw-number converter (struct-log gas vs tx gas). Instead of hard-coding the raw-number converter types, serialize a sample value through the member's converter and read back the JSON token: a Number/Boolean overrides the type-based label, while a String keeps its editorial flavour (hex data, hash, ...). Probing is scoped to explicit member converters; probing a type's default serialization is unsound for value-dependent unions (e.g. eth_syncing returns `false` or an object). Completes #12838 on top of #12868: trace_* result blockNumber and debug_* struct-log entries (gas/gasCost/pc/refund/step) now render as JSON integers, matching the wire format and Geth/OpenEthereum/Erigon. Co-Authored-By: Claude Opus 4.8 (1M context) --- tools/DocGen/JsonRpcGenerator.cs | 85 ++++++++++++++++++++++++++++---- 1 file changed, 76 insertions(+), 9 deletions(-) diff --git a/tools/DocGen/JsonRpcGenerator.cs b/tools/DocGen/JsonRpcGenerator.cs index cf5f198c7bd..bc5fc7dd0ba 100644 --- a/tools/DocGen/JsonRpcGenerator.cs +++ b/tools/DocGen/JsonRpcGenerator.cs @@ -13,6 +13,7 @@ using Nethermind.JsonRpc.Modules.Subscribe; using Nethermind.Serialization.Json; using Spectre.Console; +using System.Buffers; using System.Net; using System.Numerics; using System.Reflection; @@ -322,9 +323,9 @@ private static void WriteExpandedType(StreamWriter file, Type type, int indentat if (IsOpaqueJson(type)) return; - foreach ((string name, Type memberType) in GetSerializedMembers(type)) + foreach ((string name, Type memberType, Type? memberConverter) in GetSerializedMembers(type)) { - string memberJsonType = GetJsonTypeName(memberType); + string memberJsonType = GetJsonTypeName(memberType, memberConverter); file.WriteLine($"{Indent(indentation + 2)}- `{name}`: {memberJsonType}"); @@ -352,15 +353,28 @@ private static void WriteFromFile(StreamWriter file, string fileName) } } - private static string GetJsonTypeName(Type type) + private static string GetJsonTypeName(Type type, Type? converterType = null) { if (type.IsByRef && type.GetElementType() is { } elementType) type = elementType; - Type? underlyingType = Nullable.GetUnderlyingType(type); + type = Nullable.GetUnderlyingType(type) ?? type; + + // A member can pin its own converter, and the CLR type alone cannot tell a hex-quantity string + // from a raw JSON number (e.g. two `ulong` fields, one with a raw-number converter). Probing that + // converter's actual output is the only reliable signal, and it works for any converter, not a + // hard-coded list. This is scoped to explicit member converters: probing a type's default + // serialization is unsound for value-dependent unions (e.g. `eth_syncing` returns `false` or an + // object), so those keep the editorial mapping below. + if (converterType is not null && TryProbeScalarKind(type, converterType, out JsonTokenType token)) + { + if (token is JsonTokenType.Number) + return IsFloatingPoint(type) ? "_number_" : "_integer_"; + if (token is JsonTokenType.True or JsonTokenType.False) + return "_boolean_"; - if (underlyingType is not null) - return GetJsonTypeName(underlyingType); + // A string result keeps its flavour (hex data, hash, ...) from the editorial mapping below + } if (_knownTypeNames.TryGetValue(type, out string? knownName)) return knownName; @@ -389,6 +403,51 @@ private static string GetJsonTypeName(Type type) return _objectTypeName; } + private static bool IsFloatingPoint(Type type) => + type == typeof(double) || type == typeof(float) || type == typeof(decimal); + + // Serializes a sample value through the member's converter and reports the JSON token it produces, + // so number-vs-string-vs-boolean is read from the serializer rather than guessed from the CLR type. + private static bool TryProbeScalarKind(Type type, Type converterType, out JsonTokenType token) + { + token = JsonTokenType.None; + + try + { + object? sample = Activator.CreateInstance(type); + + if (sample is null) + return false; + + ArrayBufferWriter buffer = new(); + + using (Utf8JsonWriter writer = new(buffer)) + InvokeConverterWrite(converterType, type, sample, writer); + + Utf8JsonReader reader = new(buffer.WrittenSpan); + reader.Read(); + token = reader.TokenType; + + return token is JsonTokenType.Number or JsonTokenType.String or JsonTokenType.True or JsonTokenType.False; + } + // A converter that cannot serialize the sample (no default value, rejects zero, complex output) + // yields no usable token; the caller falls back to the editorial mapping. + catch (Exception) + { + return false; + } + } + + private static void InvokeConverterWrite(Type converterType, Type valueType, object sample, Utf8JsonWriter writer) + { + JsonConverter converter = (JsonConverter)Activator.CreateInstance(converterType)!; + MethodInfo write = converterType.GetMethod( + nameof(JsonConverter.Write), + [typeof(Utf8JsonWriter), valueType, typeof(JsonSerializerOptions)])!; + + write.Invoke(converter, [writer, sample, EthereumJsonSerializer.JsonOptions]); + } + private static Type GetReturnType(Type type) { Type returnType = type.IsGenericType @@ -417,14 +476,14 @@ private static bool IsOpaqueJson(Type type) => } } - private static IEnumerable<(string Name, Type Type)> GetSerializedMembers(Type type) + private static IEnumerable<(string Name, Type Type, Type? Converter)> GetSerializedMembers(Type type) { JsonTypeInfo? contract = GetContract(type); if (contract?.Kind is JsonTypeInfoKind.Object) return contract.Properties .Where(p => p.Get is not null) - .Select(p => (Name: p.Name, Type: p.PropertyType)) + .Select(p => (Name: p.Name, Type: p.PropertyType, Converter: MemberConverter(p.AttributeProvider))) .OrderBy(m => m.Name, StringComparer.Ordinal); // A hand-rolled converter exposes no contract members, leaving the CLR shape as the only guess @@ -436,10 +495,18 @@ private static bool IsOpaqueJson(Type type) => return type.GetProperties(memberFlags).Select(p => (Member: (MemberInfo)p, Type: p.PropertyType)) .Concat(type.GetFields(memberFlags).Select(f => (Member: (MemberInfo)f, Type: f.FieldType))) .Where(m => m.Member.GetCustomAttribute()?.Condition is not JsonIgnoreCondition.Always) - .Select(m => (Name: GetFallbackName(m.Member), Type: m.Type)) + .Select(m => (Name: GetFallbackName(m.Member), Type: m.Type, Converter: MemberConverter(m.Member))) .OrderBy(m => m.Name, StringComparer.Ordinal); } + // A member may pin its own converter via [JsonConverter], overriding the serialization its CLR type + // would otherwise get (e.g. a raw-number converter on a block number). Probing through that converter + // is what lets two fields of the same type document different wire forms. + private static Type? MemberConverter(ICustomAttributeProvider? member) => + member?.GetCustomAttributes(typeof(JsonConverterAttribute), inherit: false) is [JsonConverterAttribute { ConverterType: { } converterType }] + ? converterType + : null; + private static string GetFallbackName(MemberInfo member) => member.GetCustomAttribute()?.Name ?? JsonNamingPolicy.CamelCase.ConvertName(member.Name); From 28116c96b361559b1cb56ac7f35c354b5db979de Mon Sep 17 00:00:00 2001 From: Hugo Demeyere Date: Wed, 19 Aug 2026 17:58:55 +0900 Subject: [PATCH 2/2] fix(docgen): derive probe value type from the converter, not the member Address review (wurdum): the sample write looked up `Write` by the member's CLR type after nullable unwrap, so a `JsonConverter` (e.g. NullableRawULongConverter on BlockForRpc.Step) matched only via binder widening, and a JsonConverterFactory - which declares no `Write` - would null-deref into the blanket catch and silently mislabel the field. Take the value type from the converter's own `JsonConverter` base instead: it makes the nullable case explicit and returns cleanly for factories. Output is unchanged (verified by regenerating and diffing). Co-Authored-By: Claude Opus 4.8 (1M context) --- tools/DocGen/JsonRpcGenerator.cs | 30 ++++++++++++++++++++++++------ 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/tools/DocGen/JsonRpcGenerator.cs b/tools/DocGen/JsonRpcGenerator.cs index bc5fc7dd0ba..9ccb8f0fa6e 100644 --- a/tools/DocGen/JsonRpcGenerator.cs +++ b/tools/DocGen/JsonRpcGenerator.cs @@ -366,7 +366,7 @@ private static string GetJsonTypeName(Type type, Type? converterType = null) // hard-coded list. This is scoped to explicit member converters: probing a type's default // serialization is unsound for value-dependent unions (e.g. `eth_syncing` returns `false` or an // object), so those keep the editorial mapping below. - if (converterType is not null && TryProbeScalarKind(type, converterType, out JsonTokenType token)) + if (converterType is not null && TryProbeScalarKind(converterType, out JsonTokenType token)) { if (token is JsonTokenType.Number) return IsFloatingPoint(type) ? "_number_" : "_integer_"; @@ -406,15 +406,24 @@ private static string GetJsonTypeName(Type type, Type? converterType = null) private static bool IsFloatingPoint(Type type) => type == typeof(double) || type == typeof(float) || type == typeof(decimal); - // Serializes a sample value through the member's converter and reports the JSON token it produces, - // so number-vs-string-vs-boolean is read from the serializer rather than guessed from the CLR type. - private static bool TryProbeScalarKind(Type type, Type converterType, out JsonTokenType token) + // Serializes a sample value through the converter and reports the JSON token it produces, so + // number-vs-string-vs-boolean is read from the serializer rather than guessed from the CLR type. + private static bool TryProbeScalarKind(Type converterType, out JsonTokenType token) { token = JsonTokenType.None; + // The converter's own JsonConverter base is the source of truth for its value type: a + // nullable-aware converter declares Write against `T?`, and a JsonConverterFactory declares + // no Write at all (null here), so both are handled without relying on member-type coincidence. + Type? valueType = ConverterValueType(converterType); + + if (valueType is null) + return false; + try { - object? sample = Activator.CreateInstance(type); + // A non-null sample of the underlying value so a nullable converter writes its value, not null + object? sample = Activator.CreateInstance(Nullable.GetUnderlyingType(valueType) ?? valueType); if (sample is null) return false; @@ -422,7 +431,7 @@ private static bool TryProbeScalarKind(Type type, Type converterType, out JsonTo ArrayBufferWriter buffer = new(); using (Utf8JsonWriter writer = new(buffer)) - InvokeConverterWrite(converterType, type, sample, writer); + InvokeConverterWrite(converterType, valueType, sample, writer); Utf8JsonReader reader = new(buffer.WrittenSpan); reader.Read(); @@ -438,6 +447,15 @@ private static bool TryProbeScalarKind(Type type, Type converterType, out JsonTo } } + private static Type? ConverterValueType(Type converterType) + { + for (Type? t = converterType; t is not null; t = t.BaseType) + if (t.IsGenericType && t.GetGenericTypeDefinition() == typeof(JsonConverter<>)) + return t.GetGenericArguments()[0]; + + return null; + } + private static void InvokeConverterWrite(Type converterType, Type valueType, object sample, Utf8JsonWriter writer) { JsonConverter converter = (JsonConverter)Activator.CreateInstance(converterType)!;