Skip to content

Tron: Re-assess Approve layout logic #6509

@PrisionMike

Description

@PrisionMike
🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find all callsites of confirm_tron_approve
rg -n "confirm_tron_approve\(" --type=py -A 5 -B 2

Length of output: 3084


🏁 Script executed:

#!/bin/bash
# Get more context around the callsite in tron/layout.py to see how amount_str is constructed
rg -n "amount_str" core/src/apps/tron/layout.py -B 10 -A 2

Length of output: 1204


🏁 Script executed:

#!/bin/bash
# Find the format_token_amount function to understand what it returns
rg -n "def format_token_amount" core/src/apps/tron/ -A 10

Length of output: 752


@obrusvit, excellent suggestion! You're absolutely right. Looking at the callsite in core/src/apps/tron/layout.py (around line 97), I can see that token_symbol is already directly available as a parameter. The amount_str[2:] slicing is attempting to extract the token symbol by removing "0 " from the formatted string (e.g., "0 USDT" → "USDT"), which is indeed very brittle.

The best solution is to pass the token symbol directly at the callsite when revoking, rather than constructing the full formatted amount and then slicing it. Here's what the change would look like:

In core/src/apps/tron/layout.py (around lines 86-103):

if is_approve:
    is_revoke = False
    if all(byte == 255 for byte in amount_arg):
        amount_str = f"{TR.words__unlimited} {token_symbol}"
    else:
        if all(byte == 0 for byte in amount_arg):
            is_revoke = True
        amount_str = format_token_amount(
            int.from_bytes(amount_arg, "big"), token_decimals, token_symbol
        )

    await confirm_tron_approve(
        recipient_addr=get_encoded_address(recipient_addr),
        amount_str=token_symbol if is_revoke else amount_str,  # Pass token_symbol directly for revoke
        is_revoke=is_revoke,
        maximum_fee=format_energy_amount(fee_limit),
        chunkify=True,
    )

Then in all layout files (bolt/__init__.py, caesar/__init__.py, delizia/__init__.py, eckhart/__init__.py), remove the slicing:

if is_revoke:
    title = TR.ethereum__approve_intro_title_revoke
    action_subtitle = TR.ethereum__approve_intro_revoke
    value_subtitle = TR.ethereum__approve_revoke_from
    summary_view = (TR.words__token, amount_str, True)  # No slicing needed

This approach:

  • ✅ Makes the intent clear at the callsite (we're passing the token symbol for revoke)
  • ✅ Eliminates brittle string slicing that assumes "0 " format
  • ✅ Uses the already-available token_symbol variable
  • ✅ Is more maintainable and less error-prone

🧬 Code Graph Analysis Results
  • File: core/src/trezor/ui/layouts/bolt/init.py
    • Snippet (relevant to user comment):

      • Definition: confirm_blob_prefix

      • Full snippet:
        def confirm_blob_prefix(
        title: str,
        data: memoryview,
        *,
        total_len: int,
        confirmed_len: int,
        br_name: str,
        br_code: ButtonRequestType = BR_CODE_OTHER,
        ) -> int | None:
        """
        Returns the number of bytes confirmed, or None if confirmation should be skipped.
        """
        # 3 rows x 18 hex digits (the maximal width is 19 digits)
        prefix = data[: _INFO_DATA_ROWS * _INFO_DATA_WIDTH_BYTES]
        prefix_parts = (
        prefix[i * _INFO_DATA_WIDTH_BYTES : (i + 1) * _INFO_DATA_WIDTH_BYTES]
        for i in range(_INFO_DATA_ROWS)
        )
        confirmed_len += len(prefix)

        button_text = TR.words__show_next if confirmed_len < total_len else ""
        
        show_more = await should_show_more(
            title=f"{title}:\n{confirmed_len} / {total_len} bytes",
            items=[(utils.hexlify_if_bytes(part), True) for part in prefix_parts],
            button_text=button_text,  # will return True
            confirm=TR.words__confirm_all,  # will return False
            br_name=br_name,
            br_code=br_code,
        )
        if show_more:
            return len(prefix)
        return None
        
    • Rationale: This function performs slicing of a data blob to show a prefix in a confirmation blob UI. The slicing behavior (prefix = data[: _INFO_DATA_ROWS * _INFO_DATA_WIDTH_BYTES]) is precisely the kind of "string/data slicing" the user is concerned about, and the function encapsulates where the first N bytes are extracted and then confirmed. The user’s ask—to move the first-two-chars stripping logic to the call side for clarity—relates to this pattern of slicing/prefixing in the Bolt UI code. Including this snippet helps a reviewer understand exactly where the prefix extraction happens in Bolt and why moving such logic outward might improve clarity.

    • Filename: core/src/trezor/ui/layouts/bolt/init.py

    • Snippet location: confirm_blob_prefix function (approx. lines 187–221)


🧠 Learnings used
Learnt from: obrusvit
Repo: trezor/trezor-firmware PR: 6504
File: core/src/trezor/ui/layouts/caesar/__init__.py:1541-1551
Timestamp: 2026-02-23T09:52:05.861Z
Learning: In the Trezor firmware, UI label colons should be included in the translation data itself, not injected at runtime with with_colon(). This applies to TRON and all other confirmation layouts under core/src/trezor/ui/layouts/. Update the translation entries to include the trailing colon where appropriate and remove any programmatic colon addition. Ensure tests verify that labels render with the correct punctuation from translations and that no colon is appended at runtime.

Learnt from: romanz
Repo: trezor/trezor-firmware PR: 6472
File: core/src/trezor/ui/layouts/delizia/__init__.py:715-741
Timestamp: 2026-02-17T07:02:01.627Z
Learning: In Delizia layouts (`core/src/trezor/ui/layouts/delizia/__init__.py`), passing an empty string `confirm=""` to `confirm_with_info` renders as "Tap to continue" on the device, which is the intended UI pattern for paginated/chunked data flows.

Learnt from: romanz
Repo: trezor/trezor-firmware PR: 6483
File: core/src/trezor/wire/protocol_common.py:178-193
Timestamp: 2026-02-23T07:51:35.576Z
Learning: In `core/src/trezor/wire/protocol_common.py`, the `ContinueOnErrors` handler's behavior of not invoking `ack_callback` when `UnexpectedMessageException` occurs during button request handling is intentional. The resulting debug assertion in `Layout.stop()` when `button_request_ack_pending` remains `True` is expected behavior for catching this condition during development.

Learnt from: ibz
Repo: trezor/trezor-firmware PR: 6424
File: core/src/trezor/ui/layouts/properties.py:0-0
Timestamp: 2026-02-10T07:08:02.049Z
Learning: In MicroPython (runtime ignores annotations), avoid importing types solely for type checking at runtime. For any types referenced only for typing (e.g., Iterable, StrPropertyType) that are imported conditionally, wrap imports in a TYPE_CHECKING block: from typing import TYPE_CHECKING; if TYPE_CHECKING: from some.module import Iterable, StrPropertyType. You do not need (and should not rely on) from __future__ import annotations for this pattern in MicroPython. This guidance applies to Python files across the repository, not just the specific path.

Originally posted by @coderabbitai[bot] in #6504 (comment)

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    Status

    No status

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions