Feature/schema comparisons #1836
Conversation
There was a problem hiding this comment.
This is basically generic versions of the BasicX, ScryptoX and ManifestX traits which are needed to simplify generic bounds (e.g. in schema_comparison) - where I basically need to require that a Schema is encodable/decodable in a given extension.
For example, that a ScryptoSchema = Schema<ScryptoCustomSchema> can be encoded with ValueKind<ScryptoCustomValueKind> (-- or indeed ValueKind<ManifestCustomValueKind> - because the scrypto schema is shared by both the ScryptoExtension and ManifestExtension).
|
Docker tags |
There was a problem hiding this comment.
We have quite a lot of changes in this file.
Mainly motivated by improving the invariants we can rely on in the code, and in particular, in the container_stack which has now officially become the ancestor_path which allows us to enforce that the current_child_index always exists.
But effectively the code now captures more clearly the fact it's a state machine, and the correctness is more obvious.
In particular, error cases are handled much better - and the state of the ancestor_path is now "correct" when outputting an error.
There was a problem hiding this comment.
I've also taken a look at the benchmarks:
The new benchmark for schema::validate_payload is:
schema::validate_payload
time: [392.59 µs 392.83 µs 393.17 µs]
Versus another PR:
schema::validate_payload
time: [381.06 µs 381.17 µs 381.29 µs]
So I think that’s acceptable. I imagine it’s either within fluctuation or due to my no longer caching the container width.
I can add back the container width cache on the ancestor item if we want - interested to hear thoughts form reviewers.
Benchmark for b1b3f4cClick to view benchmark
|
There was a problem hiding this comment.
Some of this type hell with the CustomExtensions is fixed in #1837
I'm interested if people feel I should split this file out into separate sub-files.
…t-1b refactor: Remove some type hell with SchemaTypeLinks
Summary
Part 1 of implementing schema comparisons REP (the underlying tooling). Part 2 with the macros and implementing on various types will follow in another PR (EDIT: #1848 )
PR splits into two parts:
scrypto_decode_with_nice_errorandmanifest_decode_with_nice_errorschema_comparisontoolingCustomSchema::CustomTypeKind<L>for explicitCustomSchema::CustomLocalTypeKindandCustomSchema::AggregatorLocalTypeKindwhich simplifies a lotCustomSchema::DefaultCustomExtensionwhich we can use for encoding/decoding the schemas, which allows us to remove lots of really ugly/confusing bounds around the specific extension which we use for schema comparison, allowing us to just talk in terms ofCustomSchema- because the schemas now know how to encode/decode themselves against the default extension. The default extension forScryptoCustomSchemais nowScryptoExtension(chosen overManifestExtensionfor sanity).I'd recommend review commit-by-commit.
Details
Like the REP, the Schema comparison tooling operates over:
SingleTypeSchemaor aNamedTypesSchema(for a composite schema) - representing supported versions of the type.SingleTypeSchemaorNamedTypesSchema.Nto schemaN + 1The backwards compatibility tooling ensures that the current schema is equal to the latest named version ; and ensures sequential labelled versions are backwards compatible. Any failure outputs nice errors when it detects incompatibilities.
In separate news, the new recommended aliases are:
Testing
The traversal changes are tested in
payload_validator.rsand the schema comparison changes are tested inschema_comparison.rsinsbor-tests.Update Recommendations
Where slowness on errors and larger compile size are okay, you can change from
scrypto_decodetoscrypto_decode_with_nice_errorwhich gives much clearer errors if there is a failure. This would be useful in e.g. the node.If you get a compile error, change:
CustomSchema::CustomTypeKind<LocalTypeId>toCustomSchema::CustomLocalTypeKindCustomSchema::CustomTypeKind<RustTypeId>toCustomSchema::AggregatorLocalTypeKindFor Internal Integrators
Hold off integrating the schema traversal changes until there are macros in place.