Enforce NEP-25 extended type validation #4187
Conversation
Jim8y
force-pushed
the
nep-25-extendedtype-validation
branch
from
256d956 to
1bce361
Compare
NEP-25 adds ExtendedType descriptors to contract manifests, but the branch currently accepts manifests that reference undefined named types, mismatch parameter metadata, or break the base NEP-25 rules. This commit adds ExtendedType validation to ContractAbi, ContractMethodDescriptor, and ContractParameterDefinition so contracts that violate NEP-25 are rejected before deployment.\n\nThe change keeps the detailed FormatException messages recently introduced in dev, ensuring operators get actionable errors while we enforce the extended-type invariants.
Jim8y
force-pushed
the
nep-25-extendedtype-validation
branch
from
1bce361 to
12da30f
Compare
| ValidateCore(expectedType: null, allowFields: true, knownNamedTypes, allowNamedTypeReference: true); | ||
| } | ||
|
|
||
| private void ValidateCore(ContractParameterType? expectedType, bool allowFields, ISet<string>? knownNamedTypes, bool allowNamedTypeReference) |
There was a problem hiding this comment.
allowNamedTypeReference is always true
| NamedType = json["namedtype"]?.GetString(), | ||
| }; | ||
| if (!Enum.IsDefined(typeof(ContractParameterType), type.Type)) throw new FormatException(); | ||
| if (type.Type == ContractParameterType.Void) throw Nep25Error("Void type is not allowed."); |
There was a problem hiding this comment.
Somewhat duplicating ValdateCore
|
|
||
| if (Type == ContractParameterType.Map && !Key.HasValue) | ||
| throw Nep25Error("key is required for Map type."); | ||
|
|
There was a problem hiding this comment.
Many of the checks are context-independent (can be performed on a single ExtendedType value), so can be done at ExtendedType.FromJSON/FromStackItem level. But that's just a suggestion, functionally ABI-level checks can be sufficient.
| ContractParameterType.String | ||
| }; | ||
|
|
||
| private static FormatException Nep25Error(string message) => new($"Invalid NEP-25 extended type: {message}"); |
There was a problem hiding this comment.
I dont like this approach.
There was a problem hiding this comment.
But why? And what's the alternative?
There was a problem hiding this comment.
There are many alternatives. But the simplest is to just use throw new FormatException($"Invalid NEP-25 extended type: {message}") instead of a function. But I would prefer to use an exception class with the name Nep25TypeException. The reason for this is when you throw Nep25Error(message) you dont know the exception at hand and can make it hard to track down when debugging. Makes you have to look at Call Stack...etc. It just harder to maintain.
interface/key/value/fields/namedtype usage, array element requirements, named type existence).
ContractEventDescriptor, ContractParameterDefinition) for both JSON and stack deserialization,
blocking mismatched parameter/return declarations.
types to ensure invalid manifests are rejected.