Skip to content

Versioning - #103

Open
obrusvit wants to merge 4 commits into
mainfrom
obrusvit/definitions-versioning
Open

Versioning#103
obrusvit wants to merge 4 commits into
mainfrom
obrusvit/definitions-versioning

Conversation

@obrusvit

@obrusvit obrusvit commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

Fixes #79
Fixes #45
Fixes #100

This PR reinterprets the definitions magic trzd1 as magic+version, i.e. trzd + 1.
In addition, it splits metadata from defintions-latest.json into a separate file definitions-latest-metadata-v1.json. This is to prepare the repo for generating both versions at once, which will be required in the transitional period.
Requires changes in trezorlib introduced here: trezor/trezor-firmware#7639

No new functionality was introduced for version 2. As of this PR, version 1 remains the only one valid.

- `definitions-latest.json` will now contain only the downloaded data
- `definitions-latest-metadata-v1.json` will contain the metadata for
version 1; later on, this will be extended by `-v2.json` file
@obrusvit
obrusvit requested review from PrisionMike and a lite review from Copilot and removed request for Copilot August 27, 2026 10:15
@coderabbitai

coderabbitai Bot commented Aug 27, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 9c601d71-5418-4434-a095-9ac8e2dc3822


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.

Copilot AI lite review requested due to automatic review settings August 29, 2026 15:27

Copilot AI 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.

🟡 Changes recommended

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR introduces explicit definitions format versioning (separating the previous trzd1 “magic” into magic=trzd + version=1) and splits version-specific metadata out of definitions-latest.json into per-version metadata files, preparing the repo to generate/sign multiple versions in parallel during a transition period.

Changes:

  • Split coin data (definitions-latest.json) from per-version metadata (definitions-latest-metadata-v<version>.json) and update load/store/sign flows accordingly.
  • Thread a version parameter through serialization/merkle-root computation and CLI commands (generate, sign, computed-merkle-root, current-merkle-root).
  • Add/adjust tests to validate version handling and the new metadata storage layout.
File summaries
File Description
README.md Updates signing/docs to reflect per-version metadata files.
do_sign.sh Stages metadata file alongside definitions-latest.json after signing.
cli.py Adds --version options and updates merkle-root commands for versioned metadata.
definitions/common.py Implements version helpers, new metadata pathing, updated load/store APIs, and versioned payload encoding.
definitions/serialize.py Threads version into merkle-root computation and serialization dispatch; adds version to metadata.
definitions/generate.py Adds --version support and propagates version into serialization and output naming.
definitions/sign.py Adds --version support and writes signatures into per-version metadata files.
definitions/download.py Stores coin data once and writes per-version metadata for active versions.
definitions/ethereum/load.py Stores coin data once and writes per-version metadata for active versions when updating display formats.
definitions/ethereum/serialize.py Propagates version into payload encoding for Ethereum definitions.
definitions/solana/serialize.py Propagates version into payload encoding for Solana token definitions.
definitions/test_common.py New tests covering version validation, metadata pathing, and load/store roundtrips.
definitions/ethereum/test_load.py Updates tests to reflect split metadata storage and versioned metadata generation.
definitions/ethereum/test_erc7730.py Updates serialization callsites to include the new version parameter.
definitions-latest_testing.json Test snapshot data included/updated in PR.
Review details

Suppressed comments (1)

definitions/ethereum/load.py:486

  • Same as download.py: when multiple ACTIVE_VERSIONS are enabled, calling make_metadata() separately per version without a shared now can produce different timestamps/merkle roots for the same definitions snapshot. Reuse a single now for the entire loop.
    store_definitions_data(definitions_data)
    for version in ACTIVE_VERSIONS:
        metadata = make_metadata(definitions_data, version=version)
        store_metadata(metadata)

  • Files reviewed: 15/15 changed files
  • Comments generated: 5
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread definitions/common.py
Comment on lines +208 to +212
meta_path = metadata_path(version)
if not meta_path.is_file():
raise click.ClickException(
f'File "{meta_path}" with definitions metadata does not exist.'
)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Maybe it's easier to keep the metadata in the original JSON?

For example, we can add a new "metadata_v2" key - so there will be only one file to work with.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Let's keep it separated - #103 (comment).

Please make sure the data & metadata files are read/written in a consistent way
(following #103 (comment) above).

Comment thread definitions/download.py
Comment on lines +356 to +360
# save results: coin sections once, per-version metadata for all active versions
store_definitions_data(definitions_data)
for version in ACTIVE_VERSIONS:
metadata = make_metadata(definitions_data, version=version)
store_metadata(metadata)
Comment thread do_sign.sh
Comment on lines 20 to 24
MERKLE_ROOT=$(python cli.py current-merkle-root)

python cli.py sign --verify "$1"
git add definitions-latest.json
git add definitions-latest.json definitions-latest-metadata-v1.json
git commit -m "Sign definitions for $MERKLE_ROOT"
Comment thread README.md
- the results should look something like this signing commit - https://github.com/trezor/definitions/commit/42d3093e83c85dade59af92a37fb3c33d3b047eb
- `definitions.tar.gz` file should also be created, containing signed definitions, ready for deployment

Metadata (merkle root, signature, format version) are version-specific and live in `definitions-latest-metadata-v<version>.json`, separate from the coin data in `definitions-latest.json`.
Comment on lines +148 to +153
"metadata": {
"commit_hash": "6c841b35854e24d099c92c5aca7eef2942ac3d2f",
"datetime": "2026-08-13T09:49:20.206215+00:00",
"merkle_root": "5f87d66a253c555716e922f25a9691e20d836c22e493ec40e774fdbdfe686f47",
"unix_timestamp": 1786614560
},

@PrisionMike PrisionMike left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

Comment thread definitions/common.py
Comment on lines +208 to +212
meta_path = metadata_path(version)
if not meta_path.is_file():
raise click.ClickException(
f'File "{meta_path}" with definitions metadata does not exist.'
)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Maybe it's easier to keep the metadata in the original JSON?

For example, we can add a new "metadata_v2" key - so there will be only one file to work with.

Comment thread definitions/serialize.py
definitions_data: DefinitionsData, now: datetime.datetime | None = None
definitions_data: DefinitionsData,
now: datetime.datetime | None = None,
version: int = 1,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit: can we drop the default value for version here?
(we already have a default in cli.py...)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Same for other invocations of resolve_default_version().

@romanz

romanz commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

Let's postpone merging it after 26.09 definitions' signatures are updated (should happen later today).

@obrusvit

obrusvit commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator Author

@romanz Thinking of the metadata placement and whether we should keep it in the original file (perhaps add metadata_v2). Both approaches have some merit.

My reasoning for splitting the files:

  • definitions_latest.json is already very large, I'm thinking of splitting it further in the future to solana/eth_tokens/etc.
  • metadata is more reachable
  • updating/signing one specific version can touch into the specific version
  • at the moment, data is the same for both version but that may change and we may need to introduce defintiions_latest_v3.json or something. In that case, we'd probably need to separate the metadata anyway.

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.

Improve the naming of the generated data blobs Introduce versioning metadata is no longer at the start nor end of definitions-latest.json

4 participants