Tron: Add support for VoteWitnessContract (Voting)#6524
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds full support for Tron VoteWitnessContract: new protobuf messages (TronVote, TronVoteWitnessContract) and MessageType/TronRawContractType values; updates Python and Rust message mappings and tron parser to parse the new contract; extends sign_tx to detect VoteWitnessContract and call a new confirm_votes flow; implements confirm_tron_voting in multiple UI layouts; adds translations, QSTR entries, test fixtures for vote scenarios, and a changelog entry. Sequence Diagram(s)sequenceDiagram
participant Host as Host / Wallet API
participant Core as Core Signer (sign_tx)
participant Parser as Tron Parser
participant Layout as Layout.confirm_votes
participant UI as UI.confirm_tron_voting
participant User as User
Host->>Core: submit sign request (raw tx)
Core->>Parser: parse raw tx, detect VoteWitnessContract
Parser-->>Core: return TronVoteWitnessContract
Core->>Layout: confirm_votes(contract)
Layout->>UI: confirm_tron_voting(voting_list)
UI->>User: display votes & request confirmation
User-->>UI: accept/reject
UI-->>Layout: confirmation result
Layout-->>Core: proceed or raise DataError
Core-->>Host: signed tx or error
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
f880dd4 to
4f33166
Compare
4f33166 to
f9619f9
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
common/tests/fixtures/tron/sign_tx.json (1)
731-903: Add a boundary-failure fixture for candidate overflow.Great coverage for 1 and 9 candidates. Please also add a fixture for 10 candidates to lock in the “max 9” constraint and prevent regressions.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@common/tests/fixtures/tron/sign_tx.json` around lines 731 - 903, Add a new fixture named "Vote_10_candidates" mirroring the "Vote_9_candidates" entry but with the contract _message_type "TronVoteWitnessContract" containing a votes array of length 10 (i.e., one more candidate than the allowed max) so it exercises the boundary failure; keep address_n and tx fields consistent with the other vote fixtures, update raw/ raw_data_hex as appropriate for the 10-vote payload, and set the result to indicate a validation failure (e.g., no "signature" or an error field) so tests assert that submitting 10 candidates is rejected.core/src/trezor/ui/layouts/bolt/__init__.py (1)
1615-1617: Use translation-based labels for voting items instead of runtime punctuation.Line 1617 hardcodes the colon and newline at runtime:
f"{TR.words__votes}: {vote[0]}"andf"{vote[1]}\n". According to Trezor firmware patterns, colons and formatting should be part of the translation data itself, not injected programmatically. Consider creating a translation entry for the complete label (e.g.,words__votes_label: "Votes:") and using it directly, or restructure the items to pass label and value separately without embedded punctuation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/src/trezor/ui/layouts/bolt/__init__.py` around lines 1615 - 1617, Replace the runtime-punctuated labels in the items list by using a translation entry for the full label and avoid injecting punctuation programmatically: instead of f"{TR.words__votes}: {vote[0]}" and f"{vote[1]}\n", use a dedicated translation symbol (e.g., TR.words__votes_label) for the label and pass the label and value separately or format them using that translation entry so punctuation/newlines live in the translation data; update the items construction in __init__.py where voting_list is mapped (the list comprehension producing items) to reference TR.words__votes_label and supply the vote value without adding ":" or "\n" at runtime.core/src/trezor/ui/layouts/eckhart/__init__.py (1)
1667-1670: Avoid embedding layout whitespace in translated labels.Line 1670 prepends
\ntoTR.words__votes. Keep labels translation-driven and apply spacing via layout structure instead of string injection.Suggested adjustment
- item_list.append((f"\n{TR.words__votes}", f"{vote_count}", False)) + item_list.append((TR.words__votes, str(vote_count), False))Based on learnings: UI label punctuation/formatting should come from translation data, not be injected programmatically at runtime.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/src/trezor/ui/layouts/eckhart/__init__.py` around lines 1667 - 1670, The code injects a newline into the translated label TR.words__votes (in the loop building item_list from voting_list); remove the "\n" prefix and stop embedding layout whitespace in translation strings, and instead add spacing via the layout data you append to item_list (e.g. insert a dedicated spacer/row between the address and votes entries) so TR.words__votes remains a pure translation key and layout controls visual spacing.core/src/apps/tron/layout.py (1)
163-170: Add a defensive vote-count guard inconfirm_votes().The function forwards any
contract.voteswithout validation. Test fixtures demonstrate support for up to 9 candidates, but no code enforces this limit. A simple check (0 < len(voting_list) <= 9) would prevent silent UI regressions if upstream validation changes and make the design constraint explicit.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/src/apps/tron/layout.py` around lines 163 - 170, The confirm_votes function forwards contract.votes to the UI without validating the number of candidates; add a defensive guard in confirm_votes that builds voting_list (as currently done) then checks 0 < len(voting_list) <= 9 and raises a clear exception (e.g., ValueError with a descriptive message) if the check fails so the constraint is explicit and callers/CI will catch violations instead of silently breaking the UI.core/src/trezor/ui/layouts/delizia/__init__.py (1)
1570-1570: Minor style: Remove extra space before colon in type annotation.- item_list : list[StrPropertyType] = [] + item_list: list[StrPropertyType] = []🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/src/trezor/ui/layouts/delizia/__init__.py` at line 1570, The type annotation for the variable item_list has an extra space before the colon; update the declaration of item_list to use the standard annotation format by removing the space so it reads item_list: list[StrPropertyType] = [] (keep the same variable name and types).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@core/src/apps/tron/README.md`:
- Line 35: Update the README feature list to use the canonical contract name:
replace the `VoteWitnessAccount` entry with `VoteWitnessContract` so it matches
the implementation and other references in the PR (look for mentions of
VoteWitnessContract in the codebase to confirm exact naming).
In `@core/src/trezor/ui/layouts/caesar/__init__.py`:
- Around line 1676-1685: The call to trezorui_api.confirm_properties is not
correctly indented inside raise_if_not_confirmed which will cause a syntax/logic
error; fix by nesting the trezorui_api.confirm_properties(...) call as the first
argument to await raise_if_not_confirmed(...), i.e. ensure the parentheses of
raise_if_not_confirmed start before trezorui_api.confirm_properties and the
trezorui_api.confirm_properties(...) block (title=TR.words__review,
subtitle=TR.words__voting, items=[...], hold=True) is fully indented as that
argument, keeping the br_name="tron/vote" and br_code=ButtonRequestType.SignTx
kwargs at the same level as raise_if_not_confirmed's other parameters.
In `@core/src/trezor/ui/layouts/delizia/__init__.py`:
- Around line 1575-1584: The call to raise_if_not_confirmed is mis-indented so
trezorui_api.confirm_properties(...) and its br_name/br_code keyword args are
not passed correctly; fix by placing trezorui_api.confirm_properties(...) as the
first argument to await raise_if_not_confirmed(...) and ensure
br_name="tron/vote" and br_code=ButtonRequestType.SignTx are also passed inside
the same parentheses so the await, raise_if_not_confirmed,
trezorui_api.confirm_properties, br_name and br_code form a single, correctly
indented call.
---
Nitpick comments:
In `@common/tests/fixtures/tron/sign_tx.json`:
- Around line 731-903: Add a new fixture named "Vote_10_candidates" mirroring
the "Vote_9_candidates" entry but with the contract _message_type
"TronVoteWitnessContract" containing a votes array of length 10 (i.e., one more
candidate than the allowed max) so it exercises the boundary failure; keep
address_n and tx fields consistent with the other vote fixtures, update raw/
raw_data_hex as appropriate for the 10-vote payload, and set the result to
indicate a validation failure (e.g., no "signature" or an error field) so tests
assert that submitting 10 candidates is rejected.
In `@core/src/apps/tron/layout.py`:
- Around line 163-170: The confirm_votes function forwards contract.votes to the
UI without validating the number of candidates; add a defensive guard in
confirm_votes that builds voting_list (as currently done) then checks 0 <
len(voting_list) <= 9 and raises a clear exception (e.g., ValueError with a
descriptive message) if the check fails so the constraint is explicit and
callers/CI will catch violations instead of silently breaking the UI.
In `@core/src/trezor/ui/layouts/bolt/__init__.py`:
- Around line 1615-1617: Replace the runtime-punctuated labels in the items list
by using a translation entry for the full label and avoid injecting punctuation
programmatically: instead of f"{TR.words__votes}: {vote[0]}" and f"{vote[1]}\n",
use a dedicated translation symbol (e.g., TR.words__votes_label) for the label
and pass the label and value separately or format them using that translation
entry so punctuation/newlines live in the translation data; update the items
construction in __init__.py where voting_list is mapped (the list comprehension
producing items) to reference TR.words__votes_label and supply the vote value
without adding ":" or "\n" at runtime.
In `@core/src/trezor/ui/layouts/delizia/__init__.py`:
- Line 1570: The type annotation for the variable item_list has an extra space
before the colon; update the declaration of item_list to use the standard
annotation format by removing the space so it reads item_list:
list[StrPropertyType] = [] (keep the same variable name and types).
In `@core/src/trezor/ui/layouts/eckhart/__init__.py`:
- Around line 1667-1670: The code injects a newline into the translated label
TR.words__votes (in the loop building item_list from voting_list); remove the
"\n" prefix and stop embedding layout whitespace in translation strings, and
instead add spacing via the layout data you append to item_list (e.g. insert a
dedicated spacer/row between the address and votes entries) so TR.words__votes
remains a pure translation key and layout controls visual spacing.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
core/embed/rust/src/translations/generated/translated_string.rsis excluded by!**/generated/**rust/trezor-client/src/protos/generated/messages.rsis excluded by!**/generated/**rust/trezor-client/src/protos/generated/messages_tron.rsis excluded by!**/generated/**
📒 Files selected for processing (24)
common/protob/messages-tron.protocommon/protob/messages.protocommon/tests/fixtures/tron/sign_tx.jsoncore/.changelog.d/6524.addedcore/embed/rust/librust_qstr.hcore/mocks/trezortranslate_keys.pyicore/src/apps/tron/README.mdcore/src/apps/tron/consts.pycore/src/apps/tron/layout.pycore/src/apps/tron/sign_tx.pycore/src/trezor/enums/MessageType.pycore/src/trezor/enums/TronRawContractType.pycore/src/trezor/enums/__init__.pycore/src/trezor/messages.pycore/src/trezor/ui/layouts/bolt/__init__.pycore/src/trezor/ui/layouts/caesar/__init__.pycore/src/trezor/ui/layouts/delizia/__init__.pycore/src/trezor/ui/layouts/eckhart/__init__.pycore/translations/en.jsoncore/translations/order.jsoncore/translations/signatures.jsonpython/src/trezorlib/messages.pypython/src/trezorlib/tron.pyrust/trezor-client/src/messages/generated.rs
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
core/src/apps/tron/README.md (1)
35-35:⚠️ Potential issue | 🟡 MinorUse the canonical contract name in the feature list.
Line 35 still says
VoteWitnessAccount; the implemented contract type isVoteWitnessContract.Suggested fix
-- [X] `VoteWitnessAccount`: Vote using earned TRON Power. +- [X] `VoteWitnessContract`: Vote using earned TRON Power.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/src/apps/tron/README.md` at line 35, The README feature list uses the incorrect contract name "VoteWitnessAccount"; update the line to use the implemented canonical contract name "VoteWitnessContract" so documentation matches the code and contract type; locate the checklist item that currently references VoteWitnessAccount and change it to VoteWitnessContract (keeping the rest of the entry unchanged).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@core/src/apps/tron/sign_tx.py`:
- Around line 153-155: Add a bounds check for contract.votes before treating it
as a VoteWitnessContract: define a MAX_VOTES (per TRON spec) and verify
len(contract.votes) <= MAX_VOTES in the branch that identifies
messages.TronVoteWitnessContract; if it exceeds the limit, log/raise the same
style of error used elsewhere (e.g., mirror the _INT64_MAX handling) and abort
signing, otherwise set contract_type = TronRawContractType.VoteWitnessContract
and call await layout.confirm_votes(contract). Use the symbols
TronVoteWitnessContract, TronRawContractType.VoteWitnessContract,
layout.confirm_votes, and contract.votes to locate and implement the check.
In `@python/src/trezorlib/tron.py`:
- Around line 90-97: When rebuilding a VoteWitness contract, the code currently
reconstructs messages.TronVoteWitnessContract using only owner_address and votes
and drops raw_contract.support; update the reconstruction so it preserves
support by passing support=raw_contract.support into the
messages.TronVoteWitnessContract initializer (i.e., ensure the contract variable
includes support from raw_contract when creating the new
messages.TronVoteWitnessContract).
---
Duplicate comments:
In `@core/src/apps/tron/README.md`:
- Line 35: The README feature list uses the incorrect contract name
"VoteWitnessAccount"; update the line to use the implemented canonical contract
name "VoteWitnessContract" so documentation matches the code and contract type;
locate the checklist item that currently references VoteWitnessAccount and
change it to VoteWitnessContract (keeping the rest of the entry unchanged).
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
core/src/apps/tron/README.mdcore/src/apps/tron/consts.pycore/src/apps/tron/layout.pycore/src/apps/tron/sign_tx.pycore/src/trezor/ui/layouts/bolt/__init__.pycore/src/trezor/ui/layouts/caesar/__init__.pycore/src/trezor/ui/layouts/delizia/__init__.pycore/src/trezor/ui/layouts/eckhart/__init__.pycore/translations/signatures.jsonpython/src/trezorlib/tron.py
🚧 Files skipped from review as they are similar to previous changes (3)
- core/translations/signatures.json
- core/src/trezor/ui/layouts/caesar/init.py
- core/src/trezor/ui/layouts/bolt/init.py
There was a problem hiding this comment.
🧹 Nitpick comments (2)
core/src/apps/tron/sign_tx.py (2)
153-157: Extract the vote limit into a named constant.The new bound check is good, but Line 155 uses a protocol magic number. Please name it (e.g.,
_MAX_VOTE_WITNESSES) near other constraints to improve readability and future updates.♻️ Proposed refactor
_INT64_MAX = const(9_223_372_036_854_775_807) + _MAX_VOTE_WITNESSES = const(9) @@ elif messages.TronVoteWitnessContract.is_type_of(contract): contract_type = TronRawContractType.VoteWitnessContract - if len(contract.votes) > 9: + if len(contract.votes) > _MAX_VOTE_WITNESSES: raise DataError("Tron: too many votes") await layout.confirm_votes(contract)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/src/apps/tron/sign_tx.py` around lines 153 - 157, Replace the magic number 9 with a named constant (e.g., _MAX_VOTE_WITNESSES) defined alongside the other protocol constraints, then use that constant in the check inside the messages.TronVoteWitnessContract handling (where contract_type is set to TronRawContractType.VoteWitnessContract and DataError is raised); ensure the error message still triggers via raise DataError("Tron: too many votes") when len(contract.votes) > _MAX_VOTE_WITNESSES and keep the subsequent await layout.confirm_votes(contract) call unchanged.
59-59: Make the TODO actionable to prevent registry drift.Line 59 highlights a real maintenance risk. Please link this TODO to a tracking issue (or convert it to a short issue reference) so
consts.CONTRACT_TYPESand contract handlers don’t silently diverge over time.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/src/apps/tron/sign_tx.py` at line 59, Replace the vague TODO in sign_tx.py with an actionable item: either add a reference to a created tracking issue (e.g., "TRACK-123: verify consts.CONTRACT_TYPES stays in sync with handlers") or implement a lightweight assertion/test that compares consts.CONTRACT_TYPES against the actual contract handler registry (e.g., the dict or list named for contract handlers in this module or project) to fail CI if they diverge; reference the exact symbol consts.CONTRACT_TYPES and the contract handler registry symbol (e.g., CONTRACT_HANDLERS or the module that registers handlers) so future changes are caught automatically or traced by the issue.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@core/src/apps/tron/sign_tx.py`:
- Around line 153-157: Replace the magic number 9 with a named constant (e.g.,
_MAX_VOTE_WITNESSES) defined alongside the other protocol constraints, then use
that constant in the check inside the messages.TronVoteWitnessContract handling
(where contract_type is set to TronRawContractType.VoteWitnessContract and
DataError is raised); ensure the error message still triggers via raise
DataError("Tron: too many votes") when len(contract.votes) > _MAX_VOTE_WITNESSES
and keep the subsequent await layout.confirm_votes(contract) call unchanged.
- Line 59: Replace the vague TODO in sign_tx.py with an actionable item: either
add a reference to a created tracking issue (e.g., "TRACK-123: verify
consts.CONTRACT_TYPES stays in sync with handlers") or implement a lightweight
assertion/test that compares consts.CONTRACT_TYPES against the actual contract
handler registry (e.g., the dict or list named for contract handlers in this
module or project) to fail CI if they diverge; reference the exact symbol
consts.CONTRACT_TYPES and the contract handler registry symbol (e.g.,
CONTRACT_HANDLERS or the module that registers handlers) so future changes are
caught automatically or traced by the issue.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
common/tests/fixtures/tron/sign_tx.jsoncore/src/apps/tron/README.mdcore/src/apps/tron/sign_tx.py
🚧 Files skipped from review as they are similar to previous changes (2)
- core/src/apps/tron/README.md
- common/tests/fixtures/tron/sign_tx.json
There was a problem hiding this comment.
Pull request overview
Adds end-to-end support for TRON VoteWitnessContract (voting) across protobuf definitions, client libraries, firmware signing, UI confirmation flows, and test fixtures.
Changes:
- Introduces
TronVoteWitnessContract(+TronVote) message types and registers the new contract type/value in TRON enums across common protos, Rust, Python, and core firmware. - Implements firmware-side processing + UI confirmation for voting (including a >9-candidates rejection path).
- Updates translations and test fixtures (unit + UI golden hashes) to cover the new voting flow.
Reviewed changes
Copilot reviewed 25 out of 28 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
common/protob/messages-tron.proto |
Adds TronVoteWitnessContract message + VoteWitnessContract = 4 raw contract type. |
common/protob/messages.proto |
Registers new MessageType_TronVoteWitnessContract = 2210. |
core/src/apps/tron/sign_tx.py |
Handles VoteWitnessContract during signing and enforces max-candidates. |
core/src/apps/tron/layout.py |
Adds voting confirmation flow wiring into UI layouts. |
core/src/apps/tron/consts.py |
Includes vote contract in allowed contract message types and name mapping. |
core/src/apps/tron/README.md |
Marks voting support as implemented. |
core/src/trezor/ui/layouts/*/__init__.py |
Adds confirm_tron_voting UI flow in all layouts (bolt/caesar/delizia/eckhart). |
core/translations/order.json |
Adds new translation keys ordering for voting-related strings. |
core/translations/en.json |
Adds English strings for “Votes” and “Voting”. |
core/translations/signatures.json |
Updates translation merkle metadata for the new key set. |
core/mocks/trezortranslate_keys.pyi |
Adds typing stubs for new translation keys. |
core/embed/rust/src/translations/generated/translated_string.rs |
Regenerates embedded translation enum/lookup tables for new keys. |
core/embed/rust/librust_qstr.h |
Adds QSTRs for new translation keys. |
core/src/trezor/messages.py |
Adds generated core message stubs for new Tron messages. |
core/src/trezor/enums/__init__.py |
Adds new TRON raw contract type + message type. |
core/src/trezor/enums/TronRawContractType.py |
Adds VoteWitnessContract = 4. |
core/src/trezor/enums/MessageType.py |
Adds TronVoteWitnessContract = 2210. |
python/src/trezorlib/messages.py |
Adds new message + enum bindings for trezorlib. |
python/src/trezorlib/tron.py |
Parses VoteWitnessContract from raw tx payloads. |
rust/trezor-client/src/protos/generated/messages_tron.rs |
Regenerates Rust protobuf bindings for the new Tron messages/enum value. |
rust/trezor-client/src/protos/generated/messages.rs |
Regenerates Rust MessageType enum with the new value. |
rust/trezor-client/src/messages/generated.rs |
Hooks new Tron message type into Rust message dispatch. |
common/tests/fixtures/tron/sign_tx.json |
Adds sign_tx fixtures for voting (1/4/9 candidates + too-many error). |
tests/ui_tests/fixtures.json |
Adds UI golden hashes for voting tests across languages/models. |
core/.changelog.d/6524.added |
Adds changelog entry for the new Tron voting support. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
32a7665 to
52fcbe9
Compare
obrusvit
left a comment
There was a problem hiding this comment.
A few comments from me.
e073ce7 to
89a42ee
Compare
Tron Voting. Gen code.
- Main logic. Scaffolding in previous commit. - Supports up to 9 candidates. - Each layout needs some variation - Figma design available. [no changelog]
3cfa5c6 to
90438d2
Compare
|
Tested various flows of Tron using emulator build from Tested voting for a single and multiple candidates (up to 9, which is the maximum supported by Trezor). Verified that in case of more than 9 candidates Evaluated UI of Tron voting on all models. Example of casting votes: https://shasta.tronscan.org/#/transaction/ff2b90e26034a649c57c420f78383c7db569ecfaef2c92392dd1ce7b03a4d9a6 |




























































































































































Add support for TRON Voting.
Figma design available.
confirm_propertiesis missing subtitle use in Eckhart and Bolt, and page numbers in Delizia and Eckhart. This causes a deviation from the intended design. #6541 is created to fix these as it would affect a large number of flows.QA Notes
To get the
raw_hexto sign viatrezorctlSome input files for testing with varied votes and SR candidate addresses from Shasta testnet:
vote_allocations_single_1000.json
vote_allocations_small_3.json
vote_allocations_top_4_2k_20k.json
vote_allocations_top_9_random_big.json
vote_allocations_top_10_2k_20k.json
vote_allocations_top_18.json
vote_allocations_top_19_random_big.json