Skip to content

Add Ocarina ERC-7730 descriptors - #2598

Open
shukudaidayo wants to merge 7 commits into
ethereum:masterfrom
shukudaidayo:ocarina-trade
Open

Add Ocarina ERC-7730 descriptors#2598
shukudaidayo wants to merge 7 commits into
ethereum:masterfrom
shukudaidayo:ocarina-trade

Conversation

@shukudaidayo

@shukudaidayo shukudaidayo commented May 31, 2026

Copy link
Copy Markdown

Summary

Adds ERC-7730 descriptors for Ocarina's OTCRegistry contract, covering:

  • registerOrder calldata signing
  • OrderRegistration EIP-712 signing
  • TipAuthorization EIP-712 signing

Includes test fixtures for real mainnet Ocarina payloads plus edge cases for multi-item offers and multi-tip authorizations.

Testing

  • erc7730 lint registry/ocarina-trade/calldata-OTCRegistry.json registry/ocarina-trade/eip712-OTCRegistry.json
  • erc7730 format registry/ocarina-trade
  • check-jsonschema --schemafile specs/erc7730-tests.schema.json registry/ocarina-trade/tests/calldata-OTCRegistry.tests.json registry/ocarina-trade/tests/eip712-OTCRegistry.tests.json

Notes

A direct check-jsonschema run against the descriptor files currently reports known schema limitations around:

  • deeply nested calldata tuple format keys, e.g. registerOrder(...)
  • field-group oneOf ambiguity where grouped display objects can match both field and fieldGroup

Comparable direct-schema issues are present in existing registry descriptors; the Ocarina descriptors pass erc7730 lint, and test fixture schema validation passes.

Update: after rebasing onto latest master, validate JSON schemas now passes. So the schema regex / fieldGroup ambiguity notes appear resolved by the latest upstream changes. The remaining discussion is around the implementation test results for visible.mustBe and hidden fields inside bundled groups.

@github-actions github-actions Bot added the descriptors Changes to clear signing descriptors label May 31, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🧪 Clear Signing Tests

Waiting for maintainer approval to run tests.

This PR is from a fork. A maintainer needs to add the run-tests label to trigger the clear signing tests.

Once approved, the tests will run automatically and post screenshots here.

@shukudaidayo

Copy link
Copy Markdown
Author

The validate JSON schemas failure comes from two limitations in erc7730-v2.schema.json itself, not the descriptors:

  1. Tuple nesting depth. The function-signature regex caps tuple nesting at two levels ((?:[^()]|\([^()]*\))*). The Seaport order passed to registerOrder is three deep (reg( components( offer(...)[] ) )), so a valid signature can't match the pattern.

  2. fieldGroup / field ambiguity. Under display.formats, the items oneOf requires exactly one match, but a bundled field group validates as both #/$display/fieldGroup and #/$format/field. field does declare unevaluatedProperties: false, which would normally exclude a group's fields/iteration keys. However, the schema declares "$schema": "http://json-schema.org/draft-07/schema#", and unevaluatedProperties doesn't exist in draft-07, so the guard is silently ignored and the two subschemas overlap.

This isn't specific to these files. Several currently-merged descriptors fail validate JSON schemas for these schema-side reasons rather than because of malformed data: e.g. registry/morpho/calldata-MorphoBlue.json, both registry/flare/calldata-PollingFoundation-*.json, registry/paraswap/calldata-AugustusSwapper-*.json, several registry/1inch/calldata-AggregationRouter*.json, and registry/okx/*. They stay green only because this job runs on changed files (CHANGED_FILES), so they're never re-validated.

I'd prefer to keep the descriptors as-is, for two reasons:

  • Flattening the order struct isn't possible without breaking selector matching. The format key is the canonical function signature; the selector and calldata matching derive from that exact nested shape. A shallower rewrite would describe a different function.
  • Removing the bundled field groups degrades clear-signing UX. Multi-item orders would render as "offer token 1, offer token 2, offer id 1, offer id 2…" rather than coherent per-item groups.

The descriptors pass erc7730 lint and test-fixture schema validation (see the Testing section above).

I'm happy to open a fix PR upstream against assets/erc-7730/erc7730-v2.schema.json (deeper tuple nesting in the signature regex + disambiguating field/fieldGroup). For reference, no open ethereum/ERCs PR addresses either limitation.

Given the fix lives upstream, could this PR be merged as-is in the meantime? Or would you prefer I land the schema PR first and have you re-sync specs?

@manuelwedler

Copy link
Copy Markdown
Collaborator

Hey there :)
We've been recently adapting the testing CI to run tests against the TypeScript and Rust libraries at the same time. For this, we needed to change the test file format. Could you please do the following:

  • Rebase this PR on latest master
  • Have a look at the updated README for documentation on the new test format
  • Add test files in the v2 format
  • Remove the test files in the old format

Thank you! We will have a look at the PR afterwards.

@github-actions

github-actions Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Clear Signing Tests

Tested 2b890fc · started 2026-08-26 11:45 UTC · finished 2026-08-26 11:46 UTC

Entity Descriptor Case @ethereum-sourcify/clear-signing@0.2.2 llbartekll/clear-signing@0.1.0
ocarina-trade calldata-OTCRegistry Publish a multi-item OTCRegistry offer on Base ⚠️
ocarina-trade calldata-OTCRegistry Publish a real mainnet Ocarina offer payload ⚠️
ocarina-trade calldata-OTCRegistry Publish a real mainnet Ocarina NFT-for-USDS offer payload ⚠️
ocarina-trade eip712-OTCRegistry Authorize a real mainnet Ocarina offer registration
ocarina-trade eip712-OTCRegistry Authorize a real mainnet Ocarina support tip ⚠️
ocarina-trade eip712-OTCRegistry Authorize multiple support tips with native ETH and USDC ⚠️

✅ pass · ❌ fail · ⚠️ error · ⏭️ skipped · — not run

Details

⚠️ ocarina-trade/calldata-OTCRegistry · Publish a multi-item OTCRegistry offer on Base · `llbartekll/clear-signing@0.1.0`

format_calldata: InvalidDescriptor { detail: "field 'Offer type' (path '#.reg.components.offer.[].itemType') failed visible.mustMatch", retryable: false }

⚠️ ocarina-trade/calldata-OTCRegistry · Publish a real mainnet Ocarina offer payload · `llbartekll/clear-signing@0.1.0`

format_calldata: InvalidDescriptor { detail: "field 'Offer type' (path '#.reg.components.offer.[].itemType') failed visible.mustMatch", retryable: false }

⚠️ ocarina-trade/calldata-OTCRegistry · Publish a real mainnet Ocarina NFT-for-USDS offer payload · `llbartekll/clear-signing@0.1.0`

format_calldata: InvalidDescriptor { detail: "field 'Offer type' (path '#.reg.components.offer.[].itemType') failed visible.mustMatch", retryable: false }

⚠️ ocarina-trade/eip712-OTCRegistry · Authorize a real mainnet Ocarina support tip · `llbartekll/clear-signing@0.1.0`

format_typed_data: InvalidDescriptor { detail: "bundled groups require all array-expanded fields to have the same length", retryable: false }

⚠️ ocarina-trade/eip712-OTCRegistry · Authorize multiple support tips with native ETH and USDC · `llbartekll/clear-signing@0.1.0`

format_typed_data: InvalidDescriptor { detail: "bundled groups require all array-expanded fields to have the same length", retryable: false }

📋 View test details

@shukudaidayo

shukudaidayo commented Jul 9, 2026

Copy link
Copy Markdown
Author

@manuelwedler Thanks! I've updated this PR as requested, rebasing and migrating to testsv2.

The schema validation continues to fail due to the aforementioned tuple nesting depth and fieldGroup / field ambiguity issues with the schema. Resolved!

Please advise on any next steps!

@shukudaidayo

Copy link
Copy Markdown
Author

Regarding the implementation test failures, my read is that these issues with the runners surrounding visible.mustBe and hidden fields inside bundled groups, rather than incorrect descriptors or test files.

  • Sourcify rejects Offer type / Tip type fields hidden via schema-valid visible.mustBe
  • Rust fails similarly; it aliases mustBe internally, but appears to compare calldata uints as ABI hex words rather than the descriptor's string literals (e.g. 0x0000000000000000000000000000000000000000000000000000000000000002 vs "2")
  • The Rust EIP-712 tests fail due to an equal-length check for bundled arrays, but the lengths are unequal because hidden fields aren't counted

@shukudaidayo

Copy link
Copy Markdown
Author

@manuelwedler Update: I've opened four companion PRs for the runner issues surfaced by this descriptor - see above

With those runner fixes applied locally, and assuming the registry CI pins are updated after they merge, the Ocarina testsv2 fixtures pass:

  • Sourcify runner: calldata 3/3, EIP-712 3/3
  • Rust runner: calldata 3/3, EIP-712 3/3
  • Ocarina descriptor schema validation: pass
  • Ocarina testsv2 schema validation: pass

@manuelwedler

Copy link
Copy Markdown
Collaborator

@shukudaidayo Thanks a lot for flagging these. Good to get the library tested with more complex descriptors. I will look into the PRs later.

@github-actions

Copy link
Copy Markdown
Contributor

Clear signing recommendations

These are suggestions. They do not block this pull request.

2 format(s) have no interpolatedIntent. A wallet prefers it over intent, because it puts the values of the transaction in the sentence that the signer reads.

  • registry/ocarina-trade/calldata-OTCRegistry.jsonregisterOrder(((address offerer,address zone,(uint8 itemType,address token,uint256 identifierOrCriteria,uint256 startAmount,uint256 endAmount)[] offer,(uint8 itemType,address token,uint256 identifierO
  • registry/ocarina-trade/eip712-OTCRegistry.jsonOrderRegistration(bytes32 orderHash,bytes seaportSignature,string memo)

2 deprecated field(s). Keep such a field only for backward compatibility. A new descriptor should use display.formats.

  • registry/ocarina-trade/calldata-OTCRegistry.jsoncontext.contract.abi
  • registry/ocarina-trade/eip712-OTCRegistry.jsoncontext.eip712.schemas

@shukudaidayo

Copy link
Copy Markdown
Author

I've updated the Ocarina descriptors from visible.mustBe to the current schema-standard visible.mustMatch. The validate JSON schemas check now passes.

The remaining validate descriptors failure comes from the erc7730 linter, which still models this property as mustBe and therefore rejects mustMatch. In a local diagnostic run with the linter's visibility model changed from mustBe to mustMatch, both descriptors linted successfully with no other findings.

This appears to be a linter/schema version mismatch rather than a descriptor validation issue.

@kuzdogan kuzdogan moved this from Triage to In Progress in Clear Signing PM Aug 26, 2026
@kuzdogan kuzdogan moved this from In Progress to Blocked in Clear Signing PM Sep 1, 2026
@kuzdogan

kuzdogan commented Sep 1, 2026

Copy link
Copy Markdown
Member

Why CI fails on this PR

This PR is the first in the registry to use visible: {"mustMatch": [...]} and "iteration": "bundled" — no merged descriptor uses either feature yet. It hits three separate problems, and only one of them is fixable inside this PR.

1. 🔎 validate descriptors — the Python lint does not know mustMatch

Every mustMatch object fails with:

Value "{'mustMatch': ['2']}" is not valid: Unable to extract tag using discriminator visibility_rules_discriminator()

The job runs erc7730 lint from the PyPI erc7730 package (LedgerHQ/python-erc7730). That library's discriminator for visible only recognizes the old key names (src/erc7730/model/input/v2/unions.py):

if has_any_property(v, "ifNotIn", "mustBe"):
    return "conditions"
return None

The upstream ERC renamed mustBemustMatch, and this registry synced the rename in ff03978 ("sync spec files from upstream ERCs", PR #2644). The Python library still expects mustBe. Writing mustBe here is not a way out: specs/erc7730-v2.schema.json allows only mustMatch (with additionalProperties: false), so the "validate JSON schemas" check would fail instead. The two checks currently contradict each other for this rule. This needs a fix in python-erc7730 — accept mustMatch, ideally with mustBe as an alias, the way the Rust runner already does (#[serde(rename = "mustMatch", alias = "mustBe")]).

2. Rust calldata tests — mustMatch values must be hex words, not decimal strings

All three calldata cases fail on the first mustMatch field:

field 'Offer type' (path '#.reg.components.offer.[].itemType') failed visible.mustMatch

The payload values are actually inside the allowed list (the Base test's offer items have itemType 2 and 3). The failure is mechanical. The runner (llbartekll/clear-signing, pinned at 10605ba) converts a decoded integer to JSON as a raw hex word (to_json_value in decoder.rs):

ArgumentValue::Uint(bytes) => {
    let hex_str = format!("0x{}", hex::encode(bytes));
    serde_json::Value::String(hex_str)
}

and then compares strings. So the decoded value is "0x0000…0002" and the descriptor's "2" never matches. The runner's own integration tests write mustMatch values as full 32-byte hex words. Two follow-ups hide behind this: the zone list uses checksummed addresses, but the runner emits lowercase hex, so that comparison would fail next; and after both fixes the calldata tests would still hit problem 3 below.

This one is fixable in the PR (write the values as lowercase hex words), but that makes the descriptor hard to read — a numeric comparison in the runner would be the better long-term fix.

3. Rust eip712 tests — hidden fields break bundled groups

Two eip712 cases fail with:

bundled groups require all array-expanded fields to have the same length

Here mustMatch itself works, because EIP-712 message values are already strings ("itemType": "1" matches "1"). The problem is what the runner does next. A matched mustMatch field is hidden, and hidden array elements are skipped (render_typed_group_field_kind in eip712.rs):

if !is_typed_field_visible(...).await? {
    continue;          // hidden element contributes nothing
}
bundles.push(rendered);

Then the bundled branch requires every field in the group to have rendered the same number of items. In the "Tips" group with one tip: "Tip type" (mustMatch), "Tip token" (never), and "Tip identifier" (mustMatch) each render 0 items; "Tip amount" and "Tip to" render 1. 0 ≠ 1 → error. So in this runner version, any non-empty bundled group that contains an always-hidden field errors. The calldata engine has the identical code.

Workaround inside the PR: move the mustMatch and never fields out of the bundled groups into sibling top-level fields. The per-element check still runs there, and the sequential path has no length constraint. The cleaner fix belongs in the runner: a checked-but-hidden field should not shorten the bundle count.

Summary

Check Cause Where the fix belongs
validate descriptors Python lib expects mustBe, spec says mustMatch python-erc7730
Rust calldata mustMatch compares decimal strings against hex words PR (hex values) or runner
Rust eip712 hidden fields empty out bundled groups PR (restructure) or runner

Posted with Claude Code

@kuzdogan

kuzdogan commented Sep 1, 2026

Copy link
Copy Markdown
Member

I feel like we need to move away from Ledger maintained linter and other libraries @manuelwedler

@manuelwedler

Copy link
Copy Markdown
Collaborator

@kuzdogan I'm taking a look this week and see how we can resolve these

@manuelwedler

Copy link
Copy Markdown
Collaborator

Regarding 1., I created this issue: #2971

Regarding 2.+ 3., I opened llbartekll/clear-signing#13

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

descriptors Changes to clear signing descriptors run-tests

Projects

Status: Blocked

Development

Successfully merging this pull request may close these issues.

3 participants