fix(clear_signing): threshold value should use non-strict inequality
What the developer wrote
Authored by PrisionMike
fix(clear_signing): threshold value should use non-strict inequality
- Long discussed fix in the spec wordings.
- To be fixed formally here: https://github.com/ethereum/ERCs/pull/2017
- Practically this was always the case but was fixed with data instead of logic.
[no changelog]
What changed, and why it matters
This commit fixes a tiny but meaningful off-by-one error in how Trezor displays Ethereum transaction amounts. Previously, an amount exactly equal to a 'threshold' value was shown as a normal number; now it is shown as a special 'above threshold' or 'unlimited' warning. The change makes the device behavior match the intended specification, reducing the chance that a user approves a transaction they mistake for small when it is actually at the danger threshold.
Treat this as a low-severity correctness fix. Review whether any past transactions with amount exactly equal to threshold were mis-displayed, and consider adding a regression test for the boundary value. Because the commit message notes the issue was previously handled via data workarounds, verify that all bundled ERC-7730 JSON descriptors are now consistent with the corrected logic.
Security signals we found
Off-by-one comparison in security-relevant display logic
Threshold display affects user understanding of transaction value
Spec alignment with ERC-7730 / clear signing standard
No changelog entry despite security-relevant behavioral change
Evidence from the diff
In core/src/apps/ethereum/clear_signing.py, two comparisons of amount > self.threshold were changed to amount >= self.threshold. These checks decide whether the formatted amount is replaced with an AboveThreshold sentinel (rendered as a threshold-specific message or the word ‘unlimited’). The change also imports StrPropertyType from trezorui_api instead of trezor.ui.layouts and adds a docstring. The commit message says this aligns with a long-discussed spec wording change tracked in ethereum/ERCs#2017 and that, in practice, the issue was previously worked around by adjusting data rather than logic.
Changed components
core/src/apps/ethereum/clear_signing.pyEthereum clear signing amount formattingAboveThreshold display path for native currency and token amountsInspect captured patch +4 / −3
### core/src/apps/ethereum/clear_signing.py
@@ -21,7 +21,7 @@
EthereumERC7730FieldInfo,
EthereumTokenInfo,
)
- from trezor.ui.layouts import StrPropertyType
+ from trezorui_api import StrPropertyType
from trezor.ui.layouts.properties import AboveThreshold
from typing_extensions import Self
@@ -346,6 +346,7 @@ async def format(
defs: Definitions,
path_walker: PathWalker,
) -> tuple[str | AboveThreshold | None, EthereumTokenInfo | None, AnyBytes | None]:
+ """Returns (formatted_value, token, token_address)"""
from trezor.ui.layouts.properties import AboveThreshold
from .tokens import UNKNOWN_TOKEN
@@ -370,7 +371,7 @@ async def format(
# TODO: Dead code. We don't pull this externally but we should.
if self.native_currency_address is not None:
if token_address in self.native_currency_address:
- if self.threshold is not None and amount > self.threshold:
+ if self.threshold is not None and amount >= self.threshold:
return (
AboveThreshold(self.threshold_message or TR.words__unlimited),
None,
@@ -394,7 +395,7 @@ async def format(
if received_definitions is not None:
token = received_definitions.get_token(token_address)
- if self.threshold is not None and amount > self.threshold:
+ if self.threshold is not None and amount >= self.threshold:
return (
AboveThreshold(self.threshold_message or TR.words__unlimited),
token,Why this scored 38/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.