Add metadata.maps support by melanciani · Pull Request #67 · sourcifyeth/clear-signing

melanciani · 2026-08-24T19:02:17Z

EIP-7730 defines metadata.maps but this is currently not supported in the clear-signing sourcifyeth lib. This PR suggests to add this feature. Examples of its use can be seen in ethereum/clear-signing-erc7730-registry#2595, where a contract needs to display a tokenAmount from an underlying token

Adds ERC-7730 metadata.maps support: a descriptor can resolve a constant parameter (e.g. a token address) from a lookup table keyed on transaction context.

params.token, params.chainId and the other constant params the schema marks map-capable accept a { map, keyPath } reference.
Fixes a crash: an object-valued token param previously reached isAddressString() and threw s.startsWith is not a function.
Map keys are matched case-insensitively, so checksummed address keys resolve against a lowercased @.to.
On a lookup miss the whole format is abandoned with a DESCRIPTOR_NOT_APPLICABLE warning and the raw-calldata fallback — per ERC-7730 the descriptor does not describe the transaction. It does not degrade to a raw field.
New DescriptorMapReference / DescriptorMetadataMap types; metadata.maps is no longer Record<string, unknown>.

Note on cross-implementation consistency: the Rust implementation
(https://github.com/llbartekll/clear-signing#) already has a maps implementation, but of a
different shape — it names the lookup bag entries rather than the spec's
values, puts keyPath on the map definition (the pre-2026-04 spec form)
rather than on the reference, and exposes a non-spec params.mapReference
that substitutes a field's display value instead of feeding constant params.
It also rejects the whole descriptor on the spec form, since params.token
is typed as a plain string. This PR follows the current
spec shape, but an ERC clarification would help both work to converge.

Open question: on a lookup miss this degrades to raw with
FORMAT_PARAM_RESOLUTION_ERROR, consistent with other unresolvable params.
The spec says a wallet MUST treat the descriptor as invalid for the
transaction — should we follow it strictly ?

kuzdogan · 2026-08-26T13:00:39Z

+  "display": {
+    "formats": {
+      "wrap(address to, uint256 amount)": {
+        "$id": "wrap",


This key has a space after the comma. The ERC-7730 spec does not allow a space after a comma in a format key. The parser in this library removes the space, so the tests pass. But the registry lint tool will reject this key. Please change it to "wrap(address to,uint256 amount)". The same key is in calldata-ConfidentialWrapper.json.

_{Posted with Claude Code}

good catch !

kuzdogan · 2026-08-26T13:00:39Z

+    return {
+      warnings: [
+        warn(
+          "DESCRIPTOR_NOT_APPLICABLE",


The PR description does not match the code. The description says: when a map lookup fails, the field falls back to raw with FORMAT_PARAM_RESOLUTION_ERROR. The code does something different. When a map lookup fails, the code emits DESCRIPTOR_NOT_APPLICABLE and drops the whole format. The test "abandons the whole format" checks this. I think the code is correct. The spec says the descriptor is invalid for that transaction. Please update the PR description to match the code.

_{Posted with Claude Code}

indeed, updated the PR description

kuzdogan · 2026-08-26T13:12:26Z

Thank you. The implementation is clean. All map references are replaced in one pass in processSingleField. The format handlers do not need to know about maps. A lookup miss follows the spec rule.

I added two small comments inline.

One more request. AGENTS.md describes the descriptor concepts and the warning codes. Please add a short section about metadata.maps. It can list three things: the shape of a map reference, where the substitution happens, and the DESCRIPTOR_NOT_APPLICABLE warning.

About the Rust implementation: I agree. An ERC clarification is the way to make the two implementations agree. This PR follows the current spec text, and that is correct.

_{Posted with Claude Code}

kuzdogan

lgtm but defer to @manuelwedler for the approval next week when he's back, as he's more versed in this codebase

manuelwedler

Thanks for this. The implementation works against the spec's example-maps.json (I ran it locally: chain 1 and 137 both pick their own share token), and the pre-pass in processSingleField is the right design. My comments are about making that pre-pass the single mechanism and about consistency with the rest of the code:

Map resolution belongs in fields.ts only. Drop the handler-level branches and guards in formatters.ts and move the helpers there as module-private code. Share the key lookup with enum through one helper in descriptor.ts.
Iterated fields: a keyPath with .[] is never expanded, so every element misses.
Use INVALID_DESCRIPTOR on a miss instead of a new warning code.
Tests: leave the registry-case copies untouched and test through the two ERC-7730 example files, as issue #7 asks.
The param type union is wider than the registry schema.

I will implement these changes myself on top of this branch, so no action is needed from your side. Thanks again for the PR.

manuelwedler · 2026-09-24T14:20:03Z

+  if (direct !== undefined) return direct;
+
+  const lowered = key.toLowerCase();
+  for (const [candidate, mapped] of Object.entries(values)) {


This is the same lookup as resolveEnumLabel: resolve a $.metadata.* pointer to an object, try the exact key, then sweep case-insensitively. Please extract one shared helper (for example lookupMetadataEntry(metadata, pointer, key) in descriptor.ts next to resolveMetadataValue) and use it from both places.

On the broader question of whether maps should resolve like enum: the lookup should be shared, but the call site is right as it is. enum maps the field's own value to a label at render time, so it is a format. A map reference replaces a constant param that any format may consume, so substituting once before rendering is the correct place. Resolving lazily inside every handler is exactly what creates the three-way inconsistency noted above.

manuelwedler · 2026-09-24T14:20:04Z

+
+- **Shape.** `params.token: { "map": "$.metadata.maps.underlying", "keyPath": "@.to" }`, where `metadata.maps.underlying.values` maps a key to the constant. `keyPath` is resolved like any other path; its value is reduced to a canonical string (`mapKeyFromResolved`) and matched **case-insensitively** against the map keys, so a checksummed address key matches a lowercased `@.to`. Guarded by `isMapReference`; typed as `DescriptorMapReference` / `DescriptorMetadataMap` in `types.ts`.
+- **Where substitution happens.** `resolveParamMapReferences` (in `formatters.ts`) runs once per field in `processSingleField`, **before** rendering, replacing every map reference in the field's params with its resolved constant. Format handlers therefore never see a map reference; the `asConstant` guard in `formatters.ts` keeps them correct if driven directly. Values keep their JSON type, so a map yielding an integer still satisfies a numeric param like `decimals`.
+- **Miss → `DESCRIPTOR_NOT_APPLICABLE`.** If no map key matches, the descriptor does not describe this transaction. `processSingleField` returns this warning and the whole format is abandoned (raw-calldata fallback), rather than degrading the single field — per the ERC-7730 spec, unrelated constants in the same descriptor may be equally out of date. This is distinct from `INVALID_DESCRIPTOR` (a malformed descriptor); the descriptor here is well-formed but out of scope.


The Testing list further down (the zama.spec.ts entry, and a new example-maps.spec.ts entry once the spec examples are added) also needs updating. And if the helpers move to descriptor.ts, the module responsibilities section should say so.

manuelwedler · 2026-09-24T14:33:56Z

+  // is rendered. Per ERC-7730 a lookup miss means this descriptor does not
+  // describe the transaction, so the whole format is abandoned rather than
+  // rendered with a missing constant.
+  const mapParams = resolveParamMapReferences(


Coverage gap in iterated fields: expandParamArrayIndex only rewrites string-valued params, so a keyPath inside a map reference object keeps its .[] iterator. Every element then misses and the whole format is abandoned with DESCRIPTOR_NOT_APPLICABLE.

I reproduced it with applyFieldFormats directly:

{ "path": "amounts.[]", "label": "Amount", "format": "tokenAmount", "params": { "token": { "map": "$.metadata.maps.deco", "keyPath": "tokens.[]" } } }

Result: No metadata.maps entry matches for param 'token' of field 'Amount', although both tokens.[0] and tokens.[1] have entries in the map.

Please rewrite keyPath in expandParamArrayIndex (and expandFieldForIndex) the same way as tokenPath / calleePath are rewritten, and add a case for it under the array-iteration tests in test/fields.spec.ts.

For the record, the other places I checked are covered: $ref definitions (merged before the pre-pass), group patterns 1 and 3 (the scoped resolvePath handles a relative keyPath), nested calldata (recursive format()), and EIP-712 (@.chainId resolves to a uint). Two minor cases are not handled and the spec is silent on them: a map reference as an element of an array-valued param (nativeCurrencyAddress, senderAddress), and a map reference in a field's value. Fine to leave out, but a note in AGENTS.md would help.

manuelwedler · 2026-09-24T14:44:13Z

+    return {
+      warnings: [
+        warn(
+          "DESCRIPTOR_NOT_APPLICABLE",


Please use INVALID_DESCRIPTOR here instead of a new DESCRIPTOR_NOT_APPLICABLE code.

The spec's own wording is that the wallet "MUST consider the 7730 file invalid for the transaction".

The return path is the same as the other INVALID_DESCRIPTOR returns in this function (missing value, format or label): the whole format is abandoned and the model carries rawCalldataFallback.

A wallet has no different UI for a map miss than for any other unusable descriptor, so a new code only adds an entry to the union, a GUIDE.md paragraph and a case in every wallet's switch.

Keep the cause visible in the message instead: name the param, the map and the resolved key, for example No entry for key '0x…' in map '$.metadata.maps.underlying' (param 'token' of field 'Amount'). Then drop the new WarningCode entry and the "Miss →" paragraph in AGENTS.md.

DEPLOYMENT_MISMATCH would be the closest alternative in meaning, but it belongs to the chain/address binding check in the calldata path and would be wrong for an EIP-712 miss.

One more case: resolveMapReference also returns undefined when the keyPath itself does not resolve, for example @.chainId absent from an EIP-712 domain. The branch a few lines below reports a missing @. path as CONTAINER_MISSING_REQUIRED_PATH. Please handle a missing container key path the same way, so it is not reported as a descriptor problem.

manuelwedler · 2026-09-24T14:52:17Z

-  "metadata": { "contractName": "ConfidentialWrapper" },
+  "metadata": {
+    "contractName": "ConfidentialWrapper",
+    "maps": {


Please leave the files under test/registry-cases/ untouched. They are copies of the registry files, and this one no longer matches the merged registry version (the registry has no metadata.maps, and its wrap format is raw). The same goes for the new calldata-MapMissWrapper.json and the additions in zama.spec.ts: please revert them.

The maps feature should be tested through the ERC-7730 example files instead, as issue #7 asks: add example-maps.json and example-maps-pools.json verbatim from https://eips.ethereum.org/assets/eip-7730/ to test/erc7730-test-cases/ (prettier formatting only, like the other examples there), each with its own example-*.spec.ts. The registry-case changes add no extra value, because they only exercise params.token, which the spec examples cover in the same way.

One note for those tests: the underlyingToken values in example-maps.json are 21 bytes long, so that field renders with FORMAT_PARAM_RESOLUTION_ERROR. The test should assert that as is. We do not edit spec files.

manuelwedler · 2026-09-24T15:00:43Z

+  // Context-dependent constant via metadata.maps. Normally already substituted
+  // by resolveParamMapReferences before rendering; handled here too so the
+  // helper is correct when called directly.
+  if (isMapReference(token)) {


Map resolution should not live in formatters.ts at all. The pre-pass in processSingleField (resolveParamMapReferences) is the right design: it walks the merged params, replaces each { map, keyPath } with its constant, and stops the format on a miss. Because it runs after mergeDefinitions and before resolveFieldValue, no handler ever sees a reference. Please make it the single mechanism:

Drop this branch and the one in resolveChainId, and the metadata parameter threaded through formatTokenAmount.

Drop the asConstant guards (or, if the union type needs narrowing, apply one narrowing uniformly across all handlers rather than three different treatments: bespoke branch here, asConstant in six helpers, nothing in isNativeCurrencyAddress / isSenderAddress / tokenAmountMessage).

Move isMapReference, mapKeyFromResolved, resolveMapReference and resolveParamMapReferences into fields.ts as module-private functions. fields.ts is their only consumer, and AGENTS.md asks to keep internal helpers module-private. Test them through applyFieldFormats in test/fields.spec.ts, as the other pipeline behaviours are tested. The resolveTokenAddress with a map reference unit tests go away with the branch.

The only shared piece is the key lookup, which resolveEnumLabel also needs. Put that one helper in descriptor.ts next to resolveMetadataValue (see the comment below).

manuelwedler · 2026-09-24T15:08:52Z

-  nativeCurrencyAddress?: string | string[];
-  threshold?: string | number;
+  token?: string | DescriptorMapReference;
+  nativeCurrencyAddress?: string | string[] | DescriptorMapReference;


Please align these types with the registry schema (erc7730-v2.schema.json). It allows a mapReference only on the params that take "a path reference or a constant value": token, chainId (tokenAmount and tokenTicker), collection, callee, selector, amount and spender. Keep | DescriptorMapReference on those seven and remove it from nativeCurrencyAddress, threshold, base, decimals and senderAddress. The pre-pass in fields.ts can stay generic; the types are what document where a reference is allowed.

manuelwedler · 2026-09-24T15:08:54Z

+  });
+
+  it("substitutes a map reference on any constant param", () => {
+    // decimals is one of the params the schema newly declares map-capable.


decimals is not map-capable in the schema (see the comment in types.ts). Please use one of the schema's params for the second reference, for example chainId, so the test does not exercise a shape the types no longer allow.

- Move map resolution into fields.ts as module-private code; format handlers no longer see map references - Share the exact-then-case-insensitive key lookup with the enum format via resolveMetadataEntry in descriptor.ts - Expand .[] in a map reference's keyPath for iterated fields - Report a lookup miss as INVALID_DESCRIPTOR and a missing container key path as CONTAINER_MISSING_REQUIRED_PATH; drop DESCRIPTOR_NOT_APPLICABLE - Allow map references only on the params the ERC-7730 schema marks map-capable - Test via the spec examples example-maps.json and example-maps-pools.json and the field pipeline suite; restore the Zama registry copy

melanciani mentioned this pull request Aug 24, 2026

Add Zama confidential wrappers ethereum/clear-signing-erc7730-registry#2595

Merged

kuzdogan reviewed Aug 26, 2026

View reviewed changes

melanciani requested a review from kuzdogan August 28, 2026 21:13

kuzdogan approved these changes Aug 31, 2026

View reviewed changes

manuelwedler self-assigned this Sep 7, 2026

manuelwedler added this to Sourcify Public Sep 7, 2026

manuelwedler moved this to Sprint - Needs Review in Sourcify Public Sep 7, 2026

manuelwedler linked an issue Sep 24, 2026 that may be closed by this pull request

Add map support #7

Closed

manuelwedler reviewed Sep 24, 2026

View reviewed changes

melanciani and others added 5 commits September 24, 2026 17:45

Add metadata.maps support

8ebe2be

Align with suggested spec

c222126

remove key spaces and update agents.md

8bd5a09

Move map support into its own section in fields.ts

fcfa875

manuelwedler force-pushed the melanciani/add_maps_support branch from 6a2b51b to fcfa875 Compare September 24, 2026 15:46

manuelwedler merged commit 0db92af into sourcifyeth:main Sep 24, 2026
2 checks passed

github-project-automation Bot moved this from Sprint - Needs Review to Sprint - Done in Sourcify Public Sep 24, 2026

Conversation

melanciani commented Aug 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kuzdogan commented Aug 26, 2026

Uh oh!

kuzdogan left a comment

Choose a reason for hiding this comment

Uh oh!

manuelwedler left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

melanciani commented Aug 24, 2026 •

edited

Loading