Skip to content

Clear signing: open the registry floodgates - #67

Merged
PrisionMike merged 14 commits into
mainfrom
prisionmike/registry-floodgates
Jul 31, 2026
Merged

Clear signing: open the registry floodgates#67
PrisionMike merged 14 commits into
mainfrom
prisionmike/registry-floodgates

Conversation

@PrisionMike

Copy link
Copy Markdown
Collaborator

Stacked on #66 (const_value re-land) — base will be retargeted to main once #66 merges.

What

Opens up ERC-7730 registry ingestion end-to-end:

  • Track latest registry. The submodule commit pin is removed from do_update.sh; the registry now follows upstream master like the other submodules (bumped to 5a67786 here).
  • Blocklist instead of allowlist. ENABLED_PROVIDERS is gone; a small BLOCKED_PROVIDERS set (uniswap, safe to start) excludes providers from parsing, everything else is emitted. The blocklist is printed at the top of definitions-latest.log.
  • Reinterpret instead of skip.
    • addressName on a uint leaf (1inch pools, maker order receivers) retypes the ABI leaf to ABI_ADDRESS — same static one-word layout, and a value with dirty high bytes raises DirtyAddress on-device, which falls back to blind signing. Same treatment for a tokenAmount tokenPath pointing at a uint (1inch order.takerAsset). addressName on a bytes leaf passes through (the firmware renders bytes as hex).
    • calldata fields are emitted as FORMATTER_RAW (embedded calldata shown as hex bytes); a faithful renderer needs a new proto formatter + firmware support first.
    • Constant handling extends Clear signing: metadata constant value path support (re-land of #61) #66's build_const_field: non-string values are stringified, $.metadata.constants.* as a field path also resolves, and each failure mode gets a distinct drop reason.
  • Bolder logging. Every accepted-but-modified field is recorded as an adjustment (new log section, grouped by kind and file). Every drop reason is a distinct feature tag (per-element-field-path for [].field extraction, array-slice-path, unknown-path-segment, …) and every log entry carries the full function signature, so it's always clear what was dropped or bent, where, and why.

Numbers

Emitted display formats grow 612 → 3274 (all serialize to proto); registry-wide only 41 files still have any dropped display format, down from 184 fully-skipped.

Requires

trezorlib with the const_value proto variant — on firmware main since trezor/trezor-firmware@9e5934473e; the fail-fast guard in common.py (from #66) checks for it.

🤖 Generated with Claude Code

@coderabbitai

coderabbitai Bot commented Jul 23, 2026

Copy link
Copy Markdown

Review Change Stack

Walkthrough

The ERC-7730 pipeline now supports sliced paths, calldata, enums, constants, nested fields, provider metadata, and structured adjustment reporting. The download command adds an ERC-7730-only refresh mode that preserves network and token data. Tests cover the expanded parser and refresh behavior. Supported asset definitions add Nong Wan and Vladhood, update four token identifiers, refresh metadata, and advance two repository references.

Estimated code review effort: 5 (Critical) | ~120 minutes

Sequence Diagram(s)

sequenceDiagram
  participant do_update_sh
  participant download
  participant ERC7730Registry
  participant load_display_formats
  participant definitions_file
  do_update_sh->>download: run --erc7730-only
  download->>ERC7730Registry: load registry descriptors
  ERC7730Registry->>load_display_formats: build display formats
  load_display_formats->>definitions_file: write refreshed formats
  download->>definitions_file: preserve network and token data
Loading

Suggested reviewers: mmilata, ibz, andrewkozlik

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 23.36% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: expanding end-to-end ERC-7730 registry ingestion.
Description check ✅ Passed The description directly explains the registry ingestion, parser, logging, tooling, and generated data changes.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch prisionmike/registry-floodgates

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 Checkov (3.3.8)
coins_details.json

Checkov skipped this file: it is too large to scan (9305410 bytes)

definitions-latest.json

Checkov skipped this file: it is too large to scan (10565765 bytes)

🔧 ast-grep (0.45.0)
coins_details.json

ast-grep skipped this file: it is too large to scan (9305410 bytes)

definitions-latest.json

ast-grep skipped this file: it is too large to scan (10565765 bytes)


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@PrisionMike
PrisionMike marked this pull request as draft July 23, 2026 23:29
@PrisionMike
PrisionMike force-pushed the prisionmike/registry-floodgates branch 6 times, most recently from f1031fc to 92e1f45 Compare July 29, 2026 16:25
PrisionMike and others added 14 commits July 31, 2026 17:28
…tations

Rework the descriptor parser for reviewability and fold in the reinterpret-
instead-of-skip semantics:

Structure (behavior-preserving):
* Module docstring states the pipeline and the two outcomes every field has:
  DROP (UnsupportedFeature, format dropped whole) or ADJUST (kept, bent,
  logged). Sections 1-5 follow the pipeline order.
* `_FormatContext` replaces six threaded parameters; `ctx.adjust()` records
  accepted-but-modified fields.
* `_build_one_format` returns (candidate, issues, adjustments) — the
  `nonlocal had_issue` closure flag is gone; "issues non-empty means drop"
  is now the visible contract.
* The 265-line build_field_dict is split: `_build_path_field` orchestrates,
  with `_check_kind_or_reinterpret`, `_apply_token_amount_params`,
  `_apply_threshold`, `_apply_unit_params`, `_apply_date_params` and
  `_build_non_path_field`/`_const_value_field` each owning one concern.
* Deployment expansion extracted to `_expand_deployments`.

Semantics (each logged as an adjustment or a distinct drop reason):
* addressName on a uint leaf retypes the ABI leaf to ABI_ADDRESS in place
  (`_abi_leaf_at` + `_retype_uint_leaf_as_address`; uintN and address share
  the one-word static layout, dirty values degrade to blind signing).
  Same for tokenAmount's tokenPath at a packed uint. addressName on bytes
  passes through (firmware renders hex).
* Constant fields extend build_const_field (which this replaces): non-string
  values stringify, $.metadata.constants.* works as a field path, and each
  failure mode has its own drop reason instead of a silent None.
* path_to_dict raises a distinct UnsupportedFeature per drop reason
  (per-element-field-path, array-slice-path, unknown-path-segment,
  iteration-over-non-array, unparseable-path, descriptor-path).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Reconcile with the prisionmike/clear-signing/bytesN-types and sliced-paths
firmware branches.

bytesN-types (ABI_BYTES20 / ABI_INT160 in the proto enum):
* _ABI_TYPE_MAP emits bytes20 and int160. bytes20 classifies as a bytes-like
  scalar; int160 as numeric (decodes to int on-device). The uint->address
  retype deliberately does NOT apply to the signed int160.
* Recovers the tBTC/threshold descriptors (all 7 remaining
  unrepresentable-params drops were bytes20).

sliced-paths (slice_start/slice_end sint32 on EthereumERC7730Path; the
walker views a sliced word as 32 big-endian bytes, DateFormatter accepts
sliced bytes as a big-endian timestamp):
* path_to_dict accepts one trailing slice on a scalar leaf. A statically
  20-byte slice (`token.[-20:]`, the 1inch packed-address pattern) classifies
  KIND_ADDRESS — addressName and tokenPath work with no retype and no
  adjustment; other slices classify KIND_BYTES.
* `date` over a sliced word is allowed (exact firmware semantics).
* Still dropped, each with a distinct reason: slices of arrays/tuples
  (selects elements, not bytes), non-trailing slices, slices after `.[]`
  (the firmware would slice the array rather than each element), and
  out-of-sint32 bounds.
* common.py carries the slice bounds on the data-path variant and the
  fail-fast proto guard now checks for slice_start.

1inch AggregationRouterV6 alone recovers 247 records (364 sliced fields);
its only remaining drops are `enum` formatters.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The calldata-formatter firmware branch renders a `calldata` field as the
nested call it embeds: it resolves the callee address from `callee_path`,
fetches that contract's display format, and expands the field into the
nested call's rows. `selector` (4 bytes) marks args-only embedded calldata.

* `calleePath` is required (the firmware raises without it) and resolves
  like tokenPath — address kind, or a packed uint retyped to address
  (logged as callee-address-in-numeric).
* Registry extras with no proto representation (amountPath, spenderPath)
  are dropped from the field and logged as calldata-params-ignored; the
  nested call still renders in full via the callee's display format.
* Distinct drop reasons: calldata-missing-callee, unresolvable-callee-path,
  invalid-calldata-selector.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The enum-support firmware branch adds FORMATTER_ENUM and an on-device
EnumFormatter: the decoded calldata value is a key into a descriptor-
supplied mapping (EthereumERC7730EnumEntry, uint32 key -> display string);
a key missing from the mapping fails clear signing into blind signing
rather than risk showing a wrong value.

* `params.$ref` resolves against `$.metadata.enums.<name>`; keys must fit
  uint32. The JSON-boolean variant (True/False keys over a bool value,
  flyingtulip) maps to 1/0 — on-device the decoded bool compares equal —
  logged as enum-bool-keys.
* Every accepted enum field is logged (enum-field adjustment) so use of the
  feature is visible while firmware support is fresh.
* Distinct drop reasons: enum-missing-ref, unresolvable-enum-ref,
  invalid-enum-entry.

Recovers the aave interest-rate-mode, okx swap-direction, celo governance
and flyingtulip enum fields (19 unsupported-formatter drops).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The registry's `visible` vocabulary is always / never / optional, plus the
spec's rule objects (ifNotIn / mustMatch).

`optional` means "wallets MAY display this field if possible or sensible"
(ERC-7730) — hiding it is spec-compliant, so it is skipped like `never`
and the choice is logged (optional-field-hidden adjustment).

The rule objects are true conditionals the firmware cannot honor either
way — mustMatch even carries a validation duty — so they drop the display
format with their own reason (conditional-visibility) instead of being
displayed or hidden unconditionally.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The firmware shows the descriptor's provider next to the rendered call
(EthereumDisplayFormatInfo.provider_name, externalise-provider-name).
`metadata.owner` is authoritative; ownerless descriptors (1inch, safe, …)
fall back to "<registry subdirectory>: <metadata.contractName>", or just
the subdirectory. Unset when nothing is known.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* do_update.sh keeps the clear-signing registry submodule pinned at a fixed
  commit (3ddfbc0 here) rather than tracking upstream master: we are happy
  with this registry state and want it held constant.
* ENABLED_PROVIDERS grows from 11 to 21 entries (1inch, aave, consensus-specs,
  corestake, ethena, igra, kiln, lido, lifi, lombard, morpho, opencover, p2p,
  poap, quickswap, sei, starkgate, tether, walletconnect, weth, yieldxyz).
  fellow-fund is dropped. The whole registry is still scanned so the log
  inventories what the other providers would cost, but only enabled providers
  feed the emitted records.
* definitions-latest.log gains a "Providers enabled:" header and a new
  Adjustments section (accepted-with-modification fields, grouped by kind and
  by file) alongside the unsupported-features and conflicts sections.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A full download pulls networks and tokens from CoinGecko/DeFiLlama, which is
slow and rate-limited. When iterating on the ERC-7730 registry parsing we only
care about the display formats. --erc7730-only reloads them from the registry
submodule, reuses the existing networks/tokens/Solana definitions verbatim, and
skips every CoinGecko/DeFiLlama call.

do_update.sh gains a matching --erc7730-only path that runs the download plus
the (offline) dev-signed tarball build, skipping the git-clean gate, submodule
update, CoinGecko-heavy coins details, and the auto-commit.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A nested field group scopes sub-`fields` under a common `path` prefix
(Morpho Blue's `#.marketParams`, paraswap's `#.swapData`, flare's
`#._settings`). Instead of dropping the display format, flatten the group:
join each sub-field's relative path — and its relative `*Path` params,
e.g. `tokenPath: srcToken` — onto the group's, then build the sub-fields
like top-level ones, preserving display order. Groups over arrays still
drop via per-element-field-path; group-level visibility is respected;
groups nest; absolute sub-paths pass through. Logged as
nested-fields-flattened adjustments.

Additionally, address consumers (addressName, tokenPath, calleePath)
accept statically 32-byte slices by narrowing the slice start by 12 to the
word's low 20 bytes ([292:324] -> [304:324], [-32:] -> [-20:]): paraswap's
Balancer descriptors slice the whole ABI-encoded word out of a bytes blob
(`tokenPath: #.data.[292:324]`) where 1inch slices the address precisely
(`token.[-20:]`). Mirrors what the firmware's parse_address does to a
static word; no firmware change needed. Logged as address-in-word-slice;
other slice lengths still drop.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- force renaming using:
- python cli.py download -v --sleep-duration 2.5 --really-apply-all-renames-without-confirmation
- python cli.py generate --dev-sign
- python coins_details/coins_details.py

Applies the one change the gated pass rejected: chain 1315
story-aeneid -> data-aeneid (Story Aeneid Testnet -> Data Network
Aeneid Testnet, tIP -> tDATA), following Story Protocol's rebrand to
Data Network. Its mainnet sibling (chain 1514) needed no force.

Display formats then regenerated (./do_update.sh --erc7730-only) so the
committed data matches the allowlist: 3686 -> 1675 records, spanning the
21 enabled registry directories (37 distinct metadata.owner names).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@PrisionMike
PrisionMike force-pushed the prisionmike/registry-floodgates branch from 92e1f45 to d9c79fb Compare July 31, 2026 15:30
@PrisionMike
PrisionMike requested a review from mmilata July 31, 2026 15:30
@PrisionMike
PrisionMike changed the base branch from prisionmike/const-value-path-main to main July 31, 2026 15:31
@PrisionMike
PrisionMike marked this pull request as ready for review July 31, 2026 15:31

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 7

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
eth_definitions/erc7730.py (1)

377-393: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Reject path segments that walk past a scalar leaf.

Two cases produce a silently wrong proto path instead of a DROP:

  • A PathField after a scalar leaf. current is only reassigned when sub_component.components is non-empty, so a.b over f(uint256 a, uint256 b) resolves b against the top-level inputs and emits path: [0, 1].
  • An ArrayElement on a non-array leaf. x.[0] over f(uint256 x) appends the index and emits path: [0, 0].

In both cases _abi_leaf_at later returns None ("shouldn't happen"), but the field is still emitted, so the firmware walks an index path that does not match the ABI tree. Track whether the current leaf is still a container and raise unknown-path-segment / iteration-over-non-array otherwise.

🛡️ Sketch of the guard
         if isinstance(element, PathField):
+            if indices and not sub_components_available:
+                raise UnsupportedFeature(
+                    "unknown-path-segment",
+                    f"{path_str} (segment {element.identifier!r} walks past a scalar leaf)",
+                )
             name_to_idx = {p.name: i for i, p in enumerate(current) if p.name}
@@
         elif isinstance(element, ArrayElement):
+            if leaf_array_depth <= 0:
+                raise UnsupportedFeature("iteration-over-non-array", path_str)
             indices.append(element.index)
-            if leaf_array_depth > 0:
-                leaf_array_depth -= 1  # indexing peels one array dimension
+            leaf_array_depth -= 1  # indexing peels one array dimension

sub_components_available is a flag you set in the PathField branch from bool(sub_component.components).

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@eth_definitions/erc7730.py` around lines 377 - 393, Update the
path-resolution logic around the PathField and ArrayElement branches to reject
traversal past scalar leaves. Track whether the current PathField target has
components, and raise UnsupportedFeature("unknown-path-segment", ...) when a
subsequent field is resolved without container components; before appending an
ArrayElement index, require a remaining array depth and raise
"iteration-over-non-array" otherwise. Preserve valid nested-field and array
traversal behavior.
🧹 Nitpick comments (5)
eth_definitions/test_download.py (1)

181-223: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

Good coverage of the section-preservation contract, but the fresh-vs-stale networks drift isn't tested.

test_update_display_formats_only_preserves_other_sections passes networks=[{"chain_id": 1}] matching old["networks"]'s single chain_id, so it can't catch the drift described in the _update_display_formats_only review comment (Line 636 in download.py): a chain_id present in the freshly-loaded networks argument but absent from old_defs["networks"]. Consider adding a case where networks contains an extra chain_id not present in old["networks"], and asserting that a display format for that chain is excluded from the persisted output (or that the network is added), to lock in the intended contract once the underlying function is fixed.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@eth_definitions/test_download.py` around lines 181 - 223, Extend
test_update_display_formats_only_preserves_other_sections with a freshly loaded
networks entry whose chain_id is absent from old["networks"], plus a display
format for that chain. Assert the intended contract after
_update_display_formats_only: the new network is added or its display format is
excluded from persisted data, matching the implementation’s corrected behavior.
eth_definitions/test_erc7730.py (2)

1599-1601: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Rename the unused unpacked variable.

Ruff reports RUF059 for rec. Keep the single-element unpacking, because it asserts exactly one record, and rename the binding.

♻️ Proposed change
-    [rec] = build_display_formats(desc, adjustments=adjustments)
+    [_rec] = build_display_formats(desc, adjustments=adjustments)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@eth_definitions/test_erc7730.py` around lines 1599 - 1601, Rename the unused
rec binding in the build_display_formats unpacking to an underscore-prefixed
variable while preserving single-element unpacking and its assertion that
exactly one record is returned.

Source: Linters/SAST tools


1804-1810: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Assert the round-trip of provider_name, not just a non-empty payload.

assert _serialize_eth_display_format(rec, 1234567890) passes for any non-empty bytes, so the test does not prove that provider_name reaches the protobuf message. The other serialization tests in this file build the message and read the field back. Do the same here.

💚 Proposed change
-def test_provider_name_serializes_to_proto():
-    from .common import _serialize_eth_display_format
-
-    desc = _descriptor(formats=_ADDR_FIELD)
-    desc["metadata"]["owner"] = "Kiln"
-    [rec] = build_display_formats(desc, source="kiln/f.json")
-    assert _serialize_eth_display_format(rec, 1234567890)
+def test_provider_name_serializes_to_proto():
+    from trezorlib.messages import EthereumDisplayFormatInfo
+
+    desc = _descriptor(formats=_ADDR_FIELD)
+    desc["metadata"]["owner"] = "Kiln"
+    [rec] = build_display_formats(desc, source="kiln/f.json")
+    info = EthereumDisplayFormatInfo(
+        chain_id=rec["chain_id"],
+        provider_name=rec.get("provider_name"),
+        address=bytes.fromhex(rec["address"][2:]),
+        func_sig=bytes.fromhex(rec["func_sig"][2:]),
+        intent=rec["intent"],
+        parameter_definitions=[],
+        field_definitions=[],
+    )
+    assert info.provider_name == "Kiln"
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@eth_definitions/test_erc7730.py` around lines 1804 - 1810, Update
test_provider_name_serializes_to_proto to deserialize the result of
_serialize_eth_display_format and assert that the protobuf message’s
provider_name field equals "Kiln", matching the round-trip pattern used by the
other serialization tests.
eth_definitions/erc7730.py (2)

1011-1016: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Consider restricting the address retype to uint widths that can hold an address.

str(node.get("atomic","")).startswith("ABI_UINT") matches every width, including ABI_UINT8 and ABI_UINT32. An address does not fit in those widths, so a descriptor that formats a narrow uint as addressName is a registry error, and retyping it silently emits a mostly zero address instead of recording a DROP with formatter-type-mismatch. Limiting the retype to ABI_UINT160 / ABI_UINT256 (the documented packed-address widths) keeps the reinterpretation targeted.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@eth_definitions/erc7730.py` around lines 1011 - 1016, Restrict the
uint-to-address retyping in the shown atomic-node handling to ABI_UINT160 and
ABI_UINT256 only. Preserve the existing ABI_ADDRESS path and return False for
narrower or unsupported uint widths so they produce the normal
formatter-type-mismatch DROP instead of being retyped.

643-645: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Consider restricting the enum bytes allowance to bool leaves.

KIND_BYTES covers bool, bytesN, bytes, and string. The comment states the allowance exists for the bool case only. An enum over a bytes32 or string leaf therefore passes validation and emits enum_values whose uint32 keys can never match on-device, so clear signing falls back to blind signing for that call. A dedicated KIND_BOOL kind, or a check of the leaf type in _apply_enum_params, keeps the drop reason explicit in the log.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@eth_definitions/erc7730.py` around lines 643 - 645, Restrict enum handling in
the enum validation path around _apply_enum_params to bool leaves only, rather
than accepting all KIND_BYTES types. Add or reuse a dedicated KIND_BOOL
classification (or inspect the leaf type there), reject bytesN, bytes, and
string enum leaves, and preserve explicit logging of the rejection reason.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@coins_details.json`:
- Around line 489866-489867: Update the timestamp generation in coins_details.py
so updated_at and updated_at_readable derive from the same timezone-aware
instant. Use UTC consistently, preferably with an ISO 8601 UTC representation
for the readable field, while preserving the epoch value in updated_at.
- Line 154196: Verify that the ethereum-lists/chains source revision maps the
Story network to the data key, then update all external consumers, balance/key
mappings, and migration or compatibility paths to use erc20:data consistently
while preserving existing holdings and preventing duplicate assets; focus on the
token ID migration surrounding the coins_details entries.

In `@do_update.sh`:
- Around line 9-26: Add a default case to the argument-parsing case statement in
do_update.sh so any unsupported argument exits immediately with a clear error
message. Preserve the existing handling of --erc7730-only and --show-added, and
ensure invalid arguments are not silently ignored or passed to python cli.py.

In `@eth_definitions/download.py`:
- Around line 621-670: Update _update_display_formats_only so display formats
are validated against the networks that will actually be persisted, not the
freshly loaded networks alone. Filter the networks passed to
_load_display_formats_from_repo, or otherwise constrain its known_chain_ids, to
chain IDs present in old_defs["networks"] (and preserve the main download flow’s
shortcut filtering if applicable); keep DefinitionsData’s persisted network list
unchanged.

In `@eth_definitions/erc7730.py`:
- Around line 1361-1372: Update the enum-key parsing in the entries loop to
retry with base 10 when int(ks, 0) fails and ks consists only of digits,
allowing zero-padded decimal keys such as “08” while preserving
UnsupportedFeature for other invalid keys.
- Around line 1500-1522: Bound the queue-processing loop in the field-flattening
logic by tracking pops per signature and stopping once a reasonable maximum is
reached. When the cap is exceeded, record an UnsupportedFeature issue with a
clear detail tied to sig_key so the format is dropped instead of expanding
indefinitely; preserve the existing malformed-entry, unresolvable-reference, and
normal flattening behavior below the limit.

In `@ethereum-lists/chains`:
- Line 1: Update the ethereum-lists/chains submodule reference from unreachable
commit 81d95d66cfc369fb231caa7baaaa19b9be7143d2 to a commit available from the
configured remote, preserving the submodule path and its intended contents.

---

Outside diff comments:
In `@eth_definitions/erc7730.py`:
- Around line 377-393: Update the path-resolution logic around the PathField and
ArrayElement branches to reject traversal past scalar leaves. Track whether the
current PathField target has components, and raise
UnsupportedFeature("unknown-path-segment", ...) when a subsequent field is
resolved without container components; before appending an ArrayElement index,
require a remaining array depth and raise "iteration-over-non-array" otherwise.
Preserve valid nested-field and array traversal behavior.

---

Nitpick comments:
In `@eth_definitions/erc7730.py`:
- Around line 1011-1016: Restrict the uint-to-address retyping in the shown
atomic-node handling to ABI_UINT160 and ABI_UINT256 only. Preserve the existing
ABI_ADDRESS path and return False for narrower or unsupported uint widths so
they produce the normal formatter-type-mismatch DROP instead of being retyped.
- Around line 643-645: Restrict enum handling in the enum validation path around
_apply_enum_params to bool leaves only, rather than accepting all KIND_BYTES
types. Add or reuse a dedicated KIND_BOOL classification (or inspect the leaf
type there), reject bytesN, bytes, and string enum leaves, and preserve explicit
logging of the rejection reason.

In `@eth_definitions/test_download.py`:
- Around line 181-223: Extend
test_update_display_formats_only_preserves_other_sections with a freshly loaded
networks entry whose chain_id is absent from old["networks"], plus a display
format for that chain. Assert the intended contract after
_update_display_formats_only: the new network is added or its display format is
excluded from persisted data, matching the implementation’s corrected behavior.

In `@eth_definitions/test_erc7730.py`:
- Around line 1599-1601: Rename the unused rec binding in the
build_display_formats unpacking to an underscore-prefixed variable while
preserving single-element unpacking and its assertion that exactly one record is
returned.
- Around line 1804-1810: Update test_provider_name_serializes_to_proto to
deserialize the result of _serialize_eth_display_format and assert that the
protobuf message’s provider_name field equals "Kiln", matching the round-trip
pattern used by the other serialization tests.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: a93bb1d6-f452-4f45-ac1d-bedf5ebd0fec

📥 Commits

Reviewing files that changed from the base of the PR and between 33e062d and d9c79fb.

📒 Files selected for processing (11)
  • coins_details.json
  • definitions-latest.json
  • do_update.sh
  • eth_definitions/common.py
  • eth_definitions/download.py
  • eth_definitions/erc7730.py
  • eth_definitions/test_download.py
  • eth_definitions/test_erc7730.py
  • ethereum-lists/chains
  • ethereum/clear-signing-erc7730-registry
  • supported_coins_list.txt

Comment thread coins_details.json
"evil-larry": {
"coingecko_id": "evil-larry",
"id": "erc20:sty:0x693c7acf65e52c71bafe555bc22d69cb7f8a78a2",
"id": "erc20:data:0x693c7acf65e52c71bafe555bc22d69cb7f8a78a2",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

for new in \
  'erc20:data:0x693c7acf65e52c71bafe555bc22d69cb7f8a78a2' \
  'erc20:data:0x543374350269cce6651358769512875faa4cccff' \
  'erc20:data:0xa4f5c615f72ddeb2220471694fff1c0c3de051e1' \
  'erc20:data:0x1514000000000000000000000000000000000000'
do
  old="${new/erc20:data/erc20:sty}"
  printf '\nOLD: %s\n' "$old"
  rg -n --hidden --glob '!.git' -F "$old" . || true
  printf 'NEW: %s\n' "$new"
  rg -n --hidden --glob '!.git' -F "$new" . || true
done

Repository: trezor/definitions

Length of output: 1004


🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

printf 'Files matching coins/chains/config names:\n'
git ls-files | rg -n '(^|/)(coins_details\.json|supported_coins_list\.txt|.*chains.*|.*chain.*|.*config.*|.*ethereum-lists.*|.*data.*chains.*)$' | head -200

printf '\nReferences to sty/data chain keys (excluding generated builds/dist likely irrelevant by name):\n'
rg -n --hidden --glob '!node_modules/**' --glob '!dist/**' --glob '!build/**' --glob '!*.min.js' --glob '!*.map' \
  'erc20:(sty|data):0x(693c7acf65e52c71bafe555bc22d69cb7f8a78a2|543374350269cce6651358769512875faa4cccff|a4f5c615f72ddeb2220471694fff1c0c3de051e1|1514000000000000000000000000000000000000)|"key":\s*"sty"|"key":\s*"data"|chainId|chain_key|coin_key' . | head -300

printf '\nRecord contexts in coins_details.json:\n'
sed -n '154188,154204p;215079,215095p;253457,253473p;470301,470317p' coins_details.json

Repository: trezor/definitions

Length of output: 3620


🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

printf 'ethereum-lists/chains tracked metadata:\n'
git ls-files ethereum-lists/chains | sed -n '1,120p'

printf '\nChain named data / story entries:\n'
rg -n '("id"|"chainId"|"slug"|"key"|"title"|"network_name"|"name"|"status"|"links"|"symbol"|"network_logo"|1514|377|0x1514|data|story)' ethereum-lists/chains -S | head -200

printf '\nChain files matching Data/Story addresses (brief):\n'
python3 - <<'PY'
import pathlib,re,json
pattern=re.compile(r'(Story|Story|Story|Story|Story|Story|story|Data|DATA|data|1514|377|0x1514|0x377)', re.I)
for p in pathlib.Path('ethereum-lists/chains').glob('*'):
    if p.is_file() and (p.name.endswith('.json') or p.name.endswith('.metadata.json')):
        try:
            text=p.read_text(encoding='utf-8', errors='ignore')
            if pattern.search(text):
                data=json.loads(text)
                print(f'\n--- {p} ---')
                for k in ['id','chainId','key','name','network_name','slug','status','network_logo']:
                    if k in data:
                        print(f'{k}: {data[k]!r}')
                snippets=' | '.join(text[max(0,text.find(s)-60):min(len(text),text.find(s)+120)] for s in ['id','chainId','key'] if s in text)
                print(snippets[:400])
        except Exception as e:
            print(f'--- {p}: parse error {type(e).__name__}: {e} ---')
PY

printf '\nCheck for direct chain identifier in definition metadata around migrated tokens:\n'
python3 - <<'PY'
import json
from pathlib import Path
p=Path('coins_details.json')
data=json.loads(p.read_text())
for k,v in data.items():
    if v.get('id') in (
        'erc20:data:0x693c7acf65e52c71bafe555bc22d69cb7f8a78a2',
        'erc20:data:0x543374350269cce6651358769512875faa4cccff',
        'erc20:data:0xa4f5c615f72ddeb2220471694fff1c0c3de051e1',
        'erc20:data:0x1514000000000000000000000000000000000000',
    ):
        print(k)
        for field in ['chain_key','chainKey','networks','id','token_contract','decimals']:
            if field in v:
                print(f'  {field}: {v[field]!r}')
PY

Repository: trezor/definitions

Length of output: 253


🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

printf 'Top-level ethereum-lists directories:\n'
find ethereum-lists -maxdepth 1 -mindepth 1 -type d -print | sort

printf '\nTracked files under ethereum-lists/chains:\n'
git ls-files ethereum-lists/chains | sort | tail -n 20 || true

printf '\nFile type / symlink for ethereum-lists/chains:\n'
file ethereum-lists/chains 2>/dev/null || true
ls -ld ethereum-lists/chains 2>/dev/null || true

printf '\nAll files in ethereum-lists/chains (first 200):\n'
find ethereum-lists/chains -maxdepth 3 -type f -print | sort | head -200 || true

printf '\nSearch for story/data/1514/377 across ethereum-lists:\n'
rg -n -i '\b(data|story|1514|377|0x1514)\b' ethereum-lists || true

Repository: trezor/definitions

Length of output: 527


Confirm erc20:data is the intended chain key and migrate external references.

The migrated tokens still list networks: ["story"], but their coins_details.js IDs now use erc20:data; confirm the ethereum-lists/chains source rev resolves to data as the story network key. If it does, update external consumers, balance/key mappings, and migration/compatibility paths so clients do not expose duplicate assets or lose existing holdings.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@coins_details.json` at line 154196, Verify that the ethereum-lists/chains
source revision maps the Story network to the data key, then update all external
consumers, balance/key mappings, and migration or compatibility paths to use
erc20:data consistently while preserving existing holdings and preventing
duplicate assets; focus on the token ID migration surrounding the coins_details
entries.

Comment thread coins_details.json
Comment on lines +489866 to +489867
"updated_at": 1785342745,
"updated_at_readable": "Wed Jul 29 18:32:25 2026"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

rg -n -C3 'updated_at(_readable)?' --hidden --glob '!.git' .

python - <<'PY'
from datetime import datetime, timezone

print(datetime.fromtimestamp(1785342745, timezone.utc).isoformat())
PY

Repository: trezor/definitions

Length of output: 1506


Generate readable and epoch timestamps from the same timezone.

coins_details.py writes updated_at=int(time.time()) and updated_at_readable=time.asctime(), so updated_at_readable represents local time while updated_at is UTC. Use a timezone-aware value such as an ISO 8601 UTC timestamp for both fields, or store an explicit timezone/offset with the readable time.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@coins_details.json` around lines 489866 - 489867, Update the timestamp
generation in coins_details.py so updated_at and updated_at_readable derive from
the same timezone-aware instant. Use UTC consistently, preferably with an ISO
8601 UTC representation for the readable field, while preserving the epoch value
in updated_at.

Comment thread do_update.sh
Comment on lines +9 to +26
ERC7730_ONLY=""
SHOW_ADDED=""
for arg in "$@"; do
case "$arg" in
--erc7730-only) ERC7730_ONLY="1" ;;
--show-added) SHOW_ADDED="--show-added" ;;
esac
done

# Fast local iteration on the ERC-7730 registry: refresh only the display
# formats and rebuild the signed tarball. Skips the git-clean gate, the
# submodule update, the CoinGecko-heavy coins details, and the auto-commit.
if [[ -n "$ERC7730_ONLY" ]]; then
python cli.py download -v --erc7730-only $SHOW_ADDED
python cli.py generate --dev-sign
exit 0
fi

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Inspect the full do_update.sh to verify argument handling consistency.
cat -n do_update.sh

Repository: trezor/definitions

Length of output: 1877


🏁 Script executed:

#!/bin/bash
# Check whether do_update.sh uses positional script arguments after the parsed flags.
rg -n '\$[0-9@#*?]|\$SHOW_ADDED|\$ERC7730_ONLY|--show-added|--erc7730-only' do_update.sh

Repository: trezor/definitions

Length of output: 439


Add explicit handling for unrecognized script arguments. The parsing loop captures --erc7730-only and --show-added, but unknown values such as a typo like --eth7730-only are silently ignored before being passed to python cli.py. Add a *) branch to exit with a clear error when an argument does not match a supported flag.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@do_update.sh` around lines 9 - 26, Add a default case to the argument-parsing
case statement in do_update.sh so any unsupported argument exits immediately
with a clear error message. Preserve the existing handling of --erc7730-only and
--show-added, and ensure invalid arguments are not silently ignored or passed to
python cli.py.

Comment on lines +621 to +670
def _update_display_formats_only(
networks: list[Network],
change_strategy: ChangeResolutionStrategy,
show_all: bool,
show_added: bool,
) -> None:
"""Refresh only the ERC-7730 display formats, leaving the networks, ERC-20 and
Solana token definitions untouched. No CoinGecko/DeFiLlama calls are made;
`networks` (loaded from the repo) only supplies the set of known chain ids.
"""
if not DEFINITIONS_PATH.exists():
raise click.ClickException(
f"{DEFINITIONS_PATH} not found — run a full download first."
)

display_formats = _load_display_formats_from_repo(networks)

old_defs = load_json_file(DEFINITIONS_PATH)

def callback():
DEFINITIONS_PATH.write_text(json.dumps(old_defs, indent=2) + "\n")

check_definitions_list(
old_defs=old_defs.get("erc20_display_formats", []),
new_defs=display_formats,
change_strategy=change_strategy,
show_all=show_all,
show_added=show_added,
update_callback=callback,
main_keys=("chain_id", "address", "func_sig"),
def_type="DISPLAY_FORMAT",
# A wrong display format is worse than a missing one: no tombstones —
# formats the parser stops emitting are removed and stop being signed.
only_mark_as_deleted=False,
)

display_formats.sort(key=lambda x: (x["chain_id"], x["address"], x["func_sig"]))

# Reuse the existing networks/tokens/Solana definitions verbatim.
definitions_data = DefinitionsData(
networks=old_defs["networks"],
erc20_tokens=old_defs["erc20_tokens"],
solana_tokens=old_defs["solana_tokens"],
erc20_display_formats=display_formats,
)

metadata = make_metadata(definitions_data)
store_definitions_data(metadata, definitions_data)


Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

known_chain_ids is computed from fresh repo networks but the persisted networks list is the stale old_defs["networks"].

_load_display_formats_from_repo(networks) at Line 636 uses networks (the CLI's freshly loaded, unfiltered repo networks) to build known_chain_ids inside _dedup_display_formats. The final DefinitionsData at Lines 660-664 instead persists old_defs["networks"] verbatim, untouched by this refresh.

If a chain_id is present in the fresh networks but absent from old_defs["networks"] — for example, a chain added to the ethereum-lists/chains submodule since the last full download, or a chain whose shortcut is empty and would be filtered out by the main download() flow's networks = [n for n in networks if n["shortcut"]] (a filter this function never applies) — a display format for that chain can pass the known_chain_ids check and get persisted, while no corresponding network entry exists in the output. Downstream consumers that key network metadata off erc20_display_formats[].chain_id would then encounter a display format for an unknown network.

Filter known_chain_ids down to chain_ids actually present in old_defs["networks"], or apply the same shortcut filter used in the main flow, so --erc7730-only never introduces a display format whose chain isn't represented in the persisted networks list.

🔧 Proposed fix
     if not DEFINITIONS_PATH.exists():
         raise click.ClickException(
             f"{DEFINITIONS_PATH} not found — run a full download first."
         )
 
-    display_formats = _load_display_formats_from_repo(networks)
-
     old_defs = load_json_file(DEFINITIONS_PATH)
+    known_chain_ids = {n["chain_id"] for n in old_defs["networks"]}
+    persisted_networks = [n for n in networks if n["chain_id"] in known_chain_ids]
+    display_formats = _load_display_formats_from_repo(persisted_networks)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def _update_display_formats_only(
networks: list[Network],
change_strategy: ChangeResolutionStrategy,
show_all: bool,
show_added: bool,
) -> None:
"""Refresh only the ERC-7730 display formats, leaving the networks, ERC-20 and
Solana token definitions untouched. No CoinGecko/DeFiLlama calls are made;
`networks` (loaded from the repo) only supplies the set of known chain ids.
"""
if not DEFINITIONS_PATH.exists():
raise click.ClickException(
f"{DEFINITIONS_PATH} not found — run a full download first."
)
display_formats = _load_display_formats_from_repo(networks)
old_defs = load_json_file(DEFINITIONS_PATH)
def callback():
DEFINITIONS_PATH.write_text(json.dumps(old_defs, indent=2) + "\n")
check_definitions_list(
old_defs=old_defs.get("erc20_display_formats", []),
new_defs=display_formats,
change_strategy=change_strategy,
show_all=show_all,
show_added=show_added,
update_callback=callback,
main_keys=("chain_id", "address", "func_sig"),
def_type="DISPLAY_FORMAT",
# A wrong display format is worse than a missing one: no tombstones —
# formats the parser stops emitting are removed and stop being signed.
only_mark_as_deleted=False,
)
display_formats.sort(key=lambda x: (x["chain_id"], x["address"], x["func_sig"]))
# Reuse the existing networks/tokens/Solana definitions verbatim.
definitions_data = DefinitionsData(
networks=old_defs["networks"],
erc20_tokens=old_defs["erc20_tokens"],
solana_tokens=old_defs["solana_tokens"],
erc20_display_formats=display_formats,
)
metadata = make_metadata(definitions_data)
store_definitions_data(metadata, definitions_data)
def _update_display_formats_only(
networks: list[Network],
change_strategy: ChangeResolutionStrategy,
show_all: bool,
show_added: bool,
) -> None:
"""Refresh only the ERC-7730 display formats, leaving the networks, ERC-20 and
Solana token definitions untouched. No CoinGecko/DeFiLlama calls are made;
`networks` (loaded from the repo) only supplies the set of known chain ids.
"""
if not DEFINITIONS_PATH.exists():
raise click.ClickException(
f"{DEFINITIONS_PATH} not found — run a full download first."
)
old_defs = load_json_file(DEFINITIONS_PATH)
known_chain_ids = {n["chain_id"] for n in old_defs["networks"]}
persisted_networks = [n for n in networks if n["chain_id"] in known_chain_ids]
display_formats = _load_display_formats_from_repo(persisted_networks)
def callback():
DEFINITIONS_PATH.write_text(json.dumps(old_defs, indent=2) + "\n")
check_definitions_list(
old_defs=old_defs.get("erc20_display_formats", []),
new_defs=display_formats,
change_strategy=change_strategy,
show_all=show_all,
show_added=show_added,
update_callback=callback,
main_keys=("chain_id", "address", "func_sig"),
def_type="DISPLAY_FORMAT",
# A wrong display format is worse than a missing one: no tombstones —
# formats the parser stops emitting are removed and stop being signed.
only_mark_as_deleted=False,
)
display_formats.sort(key=lambda x: (x["chain_id"], x["address"], x["func_sig"]))
# Reuse the existing networks/tokens/Solana definitions verbatim.
definitions_data = DefinitionsData(
networks=old_defs["networks"],
erc20_tokens=old_defs["erc20_tokens"],
solana_tokens=old_defs["solana_tokens"],
erc20_display_formats=display_formats,
)
metadata = make_metadata(definitions_data)
store_definitions_data(metadata, definitions_data)
🧰 Tools
🪛 ast-grep (0.45.0)

[info] 640-640: use jsonify instead of json.dumps for JSON output
Context: json.dumps(old_defs, indent=2)
Note: [CWE-116] Improper Encoding or Escaping of Output.

(use-jsonify)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@eth_definitions/download.py` around lines 621 - 670, Update
_update_display_formats_only so display formats are validated against the
networks that will actually be persisted, not the freshly loaded networks alone.
Filter the networks passed to _load_display_formats_from_repo, or otherwise
constrain its known_chain_ids, to chain IDs present in old_defs["networks"] (and
preserve the main download flow’s shortcut filtering if applicable); keep
DefinitionsData’s persisted network list unchanged.

Comment on lines +1361 to 1372
for k, v in entries.items():
ks = str(k)
if ks in ("True", "true", "False", "false"):
key = 1 if ks.lower() == "true" else 0
bool_keys = True
else:
try:
decimals = int(params["decimals"])
except (TypeError, ValueError):
key = int(ks, 0)
except ValueError:
raise UnsupportedFeature(
"invalid-decimals", f"{params['decimals']!r} (field {label!r})"
"invalid-enum-entry", f"key {k!r} in {ref!r} (field {label!r})"
) from None

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

int(ks, 0) rejects zero-padded decimal enum keys.

With base 0, Python rejects strings with a leading zero: int("08", 0) raises ValueError. An enum map written as {"00": "...", "01": "..."} therefore drops the whole display format with invalid-enum-entry, even though the intent is unambiguous. Fall back to base 10 when the base-0 parse fails and the string is all digits.

🐛 Proposed fix
             try:
                 key = int(ks, 0)
             except ValueError:
-                raise UnsupportedFeature(
-                    "invalid-enum-entry", f"key {k!r} in {ref!r} (field {label!r})"
-                ) from None
+                try:
+                    key = int(ks, 10)
+                except ValueError:
+                    raise UnsupportedFeature(
+                        "invalid-enum-entry", f"key {k!r} in {ref!r} (field {label!r})"
+                    ) from None
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
for k, v in entries.items():
ks = str(k)
if ks in ("True", "true", "False", "false"):
key = 1 if ks.lower() == "true" else 0
bool_keys = True
else:
try:
decimals = int(params["decimals"])
except (TypeError, ValueError):
key = int(ks, 0)
except ValueError:
raise UnsupportedFeature(
"invalid-decimals", f"{params['decimals']!r} (field {label!r})"
"invalid-enum-entry", f"key {k!r} in {ref!r} (field {label!r})"
) from None
for k, v in entries.items():
ks = str(k)
if ks in ("True", "true", "False", "false"):
key = 1 if ks.lower() == "true" else 0
bool_keys = True
else:
try:
key = int(ks, 0)
except ValueError:
try:
key = int(ks, 10)
except ValueError:
raise UnsupportedFeature(
"invalid-enum-entry", f"key {k!r} in {ref!r} (field {label!r})"
) from None
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@eth_definitions/erc7730.py` around lines 1361 - 1372, Update the enum-key
parsing in the entries loop to retry with base 10 when int(ks, 0) fails and ks
consists only of digits, allowing zero-padded decimal keys such as “08” while
preserving UnsupportedFeature for other invalid keys.

Comment on lines +1500 to +1522
# A queue rather than a plain loop: a nested field group flattens into its
# sub-fields, which are pushed back to the FRONT so display order holds.
queue: list[Any] = list(display_format.get("fields", []))
while queue:
f = queue.pop(0)
if not isinstance(f, dict):
issues.append(("malformed-field-entry", f"{sig_key}: {f!r}"))
continue
resolved = _resolve_ref(f, definitions)
if resolved is None:
issues.append(("unresolvable-ref", f"{sig_key}: {f.get('$ref')!r}"))
continue
f = resolved
if isinstance(f.get("fields"), list):
# A nested field group (a `path` scoping sub-`fields`, e.g.
# `#.marketParams` in Morpho Blue, `#.swapData` in paraswap):
# flatten it by joining each sub-field's relative path onto the
# group's, then build the sub-fields like top-level ones.
try:
queue[0:0] = _flatten_field_group(f, ctx)
except UnsupportedFeature as e:
issues.append((e.feature, f"{sig_key}: {e.detail}"))
continue

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Bound the field queue to prevent an unbounded loop on self-referential definitions.

The queue grows whenever a group flattens, and the flattened entries are re-queued. _resolve_ref merges $.display.definitions.* entries without any cycle detection, so a definition whose fields list references itself expands forever and the download never terminates. The descriptors come from the registry submodule, so this input is not fully under your control.

Add a simple cap, for example a maximum number of queue pops per signature, and record the overflow as an UnsupportedFeature so the format drops with a clear reason.

🛡️ Proposed guard
     queue: list[Any] = list(display_format.get("fields", []))
-    while queue:
+    steps = 0
+    while queue:
+        steps += 1
+        if steps > _MAX_FIELD_QUEUE_STEPS:
+            issues.append(("field-expansion-overflow", sig_key))
+            break
         f = queue.pop(0)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@eth_definitions/erc7730.py` around lines 1500 - 1522, Bound the
queue-processing loop in the field-flattening logic by tracking pops per
signature and stopping once a reasonable maximum is reached. When the cap is
exceeded, record an UnsupportedFeature issue with a clear detail tied to sig_key
so the format is dropped instead of expanding indefinitely; preserve the
existing malformed-entry, unresolvable-reference, and normal flattening behavior
below the limit.

Comment thread ethereum-lists/chains
@@ -1 +1 @@
Subproject commit 06c01af06080802e7e3c352fc2d1e92ceffa435a
Subproject commit 81d95d66cfc369fb231caa7baaaa19b9be7143d2

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

path='ethereum-lists/chains'
new_commit="$(git rev-parse HEAD:"$path")"
old_commit="$(git rev-parse HEAD^:"$path")"

git cat-file -e "${new_commit}^{commit}"
git submodule update --init "$path"
git -C "$path" diff --name-status "$old_commit" "$new_commit"
git -C "$path" diff --stat "$old_commit" "$new_commit"
git diff --submodule=log HEAD^ HEAD -- "$path"

Repository: trezor/definitions

Length of output: 332


🏁 Script executed:

#!/bin/bash
set -u

echo "Git version:"
git version

echo "Repository metadata relevant to submodule:"
if [ -d .git/modules/ethereum-list/chains ]; then
  echo "module config exists"
  sed -n '1,200p' .git/modules/ethereum-list/chains/config 2>/dev/null || true
else
  echo "no .git/modules/ethereum-list/chains or hidden directory"
fi

echo "Submodule entry:"
git ls-files -s ethereum-lists/chains || true
git submodule status ethereum-lists/chains 2>/dev/null || true

echo "Working tree .gitlink:"
if [ -e .gitmodules ]; then
  sed -n '1,160p' .gitmodules
fi

echo "HEAD previous commits and submodule hashes:"
git rev-parse --verify HEAD^ 2>/dev/null || true
git rev-parse HEAD 2>/dev/null || true
git rev-parse HEAD^:ethereum-lists/chains 2>/dev/null || true
git rev-parse HEAD:ethereum-lists/chains 2>/dev/null || true

echo "Commit object availability for claimed/new submodule hashes (HEAD only available in sandbox):"
for sha in 81d95d66cfc369fb231caa7baaaa19b9be7143d2 $(git ls-files -s ethereum-lists/chains 2>/dev/null | awk '{print $2}' || true); do
  [ -z "$sha" ] && continue
  status=$(git cat-file -e "${sha}^{commit}" 2>&1 && echo exists || echo missing)
  printf '%s %s\n' "$status" "$sha"
done

echo "Submodule URL and configured refs:"
git config -f .gitmodules --get submodule.ethereum-lists/chains.url || true
git config --get submodule.ethereum-lists/chains.update || true
git config --get remote.promisor.url || true
git config --get remote.origin.url || true
git config --get remote.origin.fsckObjects || true

Repository: trezor/definitions

Length of output: 1834


Reject the unreachable submodule reference.

Commit 81d95d66cfc369fb231caa7baaaa19b9be7143d2 is not available from the configured remote. Update ethereum-lists/chains to a reachable submodule commit before merging.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ethereum-lists/chains` at line 1, Update the ethereum-lists/chains submodule
reference from unreachable commit 81d95d66cfc369fb231caa7baaaa19b9be7143d2 to a
commit available from the configured remote, preserving the submodule path and
its intended contents.

@mmilata mmilata left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I've skimmed the changes and did not spot anything obviously malicious. I'm approving the PR with the disclaimer that

  • this is going to be shipped as experimental feature
  • I'm largely unfamiliar with Ethereum contracts and ERC7730
  • I don't have experience reviewing LLM-generated code

@PrisionMike
PrisionMike merged commit 6abf453 into main Jul 31, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants