Skip to content

Update ERC 7730: update json-schema to 2020-12 - #1822

Merged
eip-review-bot merged 2 commits into
ethereum:masterfrom
melanciani:master
Jul 7, 2026
Merged

eip-review-bot merged 2 commits into
ethereum:masterfrom
melanciani:master

Conversation

@melanciani

Copy link
Copy Markdown
Contributor

following work on adding a new erc7730 registry using the assets/erc-7730/erc7730-v2.schema.json in ethereum/clear-signing-erc7730-registry#2595

TL;DR

The v2 schema declares draft-07 (line 2), but field uses unevaluatedProperties: false (a draft-2019-09+ keyword) to reject stray keys. Draft-07 ignores that keyword, so field accepts an extra $ref and overlaps the reference branch → any bare { "path": ..., "$ref": ... } field matches two oneOf branches and fails validation. Fix: either declare 2020-12, or add { "not": { "required": ["$ref"] } } to field.


Details

Symptom. In the https://github.com/ethereum/clear-signing-erc7730-registry repository, check-jsonschema rejects bare references like { "path": "amount", "$ref": "$.display.definitions.x" } with "valid under each of #/$display/reference, #/$format/field" (matches 2 of the fields[] oneOf branches).

Cause. Dialect mismatch: field's unevaluatedProperties: false (line ~640) is meant to reject unknown keys, but it only exists from draft 2019-09 — under the declared draft-07 it's silently ignored, so field tolerates $ref and overlaps reference. The same mismatch overlaps field/fieldGroup for { "path": ..., "fields": [...] } groups (e.g. registry/paraswap/calldata-AugustusSwapper-v6.2.json currently fails too).

Fix options:

  1. Root cause: set line 2 to https://json-schema.org/draft/2020-12/schema → activates the existing unevaluatedProperties, fixes both overlaps, keeps strictness. ⚠️ changes whole-file dialect (e.g. $ref-with-siblings, line 546) — needs full-registry re-validation.
  2. Targeted, draft-07-safe (applied): add { "not": { "required": ["$ref"] } } to field's allOf. Verified green on the affected file + kyberswap / erc721 / okx / 1inch / lido / threshold; no regression. Doesn't cover the fieldGroup overlap (would need an analogous { "not": { "required": ["fields"] } }).
  3. Permissive: oneOfanyOf (line 419) — one word, fixes both, but weakens validation.

Recommendation: option 1 long-term (the dialect is just mis-declared); option 2 short-term/low-risk.

Open to discussion

@eip-review-bot

eip-review-bot commented Jun 17, 2026

Copy link
Copy Markdown
Collaborator

✅ All reviewers have approved.

@melanciani melanciani changed the title Update ERC 7730: disable ref in field formatter Update ERC 7730: update json-schema to 2020-12 Jul 2, 2026
@melanciani

Copy link
Copy Markdown
Contributor Author

I instead moved to solution 1, as discussed

@kuzdogan

kuzdogan commented Jul 3, 2026

Copy link
Copy Markdown
Member

I guess we need to test this by changing the schema on https://github.com/ethereum/clear-signing-erc7730-registry too and running the validation tests against all descriptors @manuelwedler

@melanciani

Copy link
Copy Markdown
Contributor Author

@kuzdogan I opened ethereum/clear-signing-erc7730-registry#2624 to update the schema on the registry repo

note:

  • I've made Claude run a check on all current schemas in the registry repo and only a single schema fails, but because of a pre-existing unrelated issue : registry/paraswap/calldata-AugustusSwapper-v5.json
  • this can be checked by running check-jsonschema --schemafile specs/erc7730-v2.schema.json registry/paraswap/calldata-AugustusSwapper-v5.json in main currently

@eip-review-bot
eip-review-bot enabled auto-merge (squash) July 7, 2026 12:31

@eip-review-bot eip-review-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All Reviewers Have Approved; Performing Automatic Merge...

@eip-review-bot
eip-review-bot merged commit 179de12 into ethereum:master Jul 7, 2026
19 of 21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants