Skip to content

Commit 9d64a07

Browse files
committed
ci(tokens): gate the firmware token-table generators
The comment justifying the firmware-unit removal claimed "No change in THIS repository can affect firmware C++". That is false. keepkey-firmware's lib/firmware/CMakeLists.txt builds ethereum_tokens.def and uniswap_tokens.def by running deps/python-keepkey/keepkeylib/eth/{ethereum,uniswap}_tokens.py, and kkfirmware depends on that target -- so a change here can break the firmware C++ build, and tokens[] is what unittests/firmware/coins.cpp reads. This PR changes both generators and adds token_policy.py. Add tests/test_token_table_generators.py as the equivalent gate: it runs both generators and asserts the emitted table is well-formed (every row parses as X(chain, 20-byte address, symbol, decimals)), fills its declared budget, and is deterministic. Fault-injected all four arms -- a crashing generator, a malformed row, a wrong-length address, and silent candidate loss are each caught; an intentional budget change is not. Wire it and the ABI encoder tests into the lint job, and correct the CircleCI comment to state the real coupling.
1 parent d469ea6 commit 9d64a07

3 files changed

Lines changed: 199 additions & 6 deletions

File tree

.circleci/config.yml

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -58,11 +58,24 @@ jobs:
5858
# Fail the job on this repo's OWN result.
5959
#
6060
# The firmware's C++ firmware-unit suite used to run here and gated
61-
# this job. No change in THIS repository can affect firmware C++, and
62-
# the firmware repo already runs that suite in its own CI, so all it
63-
# did was fail python-keepkey for reasons no python change caused: a
64-
# token-table change cannot go green here until the matching firmware
65-
# change reaches the branch this clones, which is a release away.
61+
# this job. It was dropped because it failed python-keepkey for
62+
# reasons no python change caused: a token-table change cannot go
63+
# green here until the matching firmware change reaches the branch
64+
# this clones, which is a release away. The firmware repo runs that
65+
# suite in its own CI.
66+
#
67+
# This repo IS in the firmware's build graph, though, so dropping
68+
# the suite is not free. keepkey-firmware's lib/firmware/CMakeLists.txt
69+
# generates ethereum_tokens.def and uniswap_tokens.def by running
70+
# deps/python-keepkey/keepkeylib/eth/{ethereum,uniswap}_tokens.py,
71+
# and kkfirmware depends on that target -- so a change here can
72+
# break the firmware C++ BUILD, and tokens[] is what
73+
# unittests/firmware/coins.cpp reads.
74+
#
75+
# tests/test_token_table_generators.py is the replacement gate for
76+
# exactly that coupling: it runs both generators and asserts the
77+
# emitted table is well-formed, budget-conforming and
78+
# deterministic. Do not remove it without restoring firmware-unit.
6679
#
6780
# Read the status file defensively -- it is written by the container,
6881
# and a crash before it exists must FAIL rather than silently pass an

.github/workflows/ci.yml

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@ jobs:
3434
runs-on: ubuntu-latest
3535
steps:
3636
- uses: actions/checkout@v4
37+
with:
38+
submodules: recursive
3739

3840
- uses: actions/setup-python@v5
3941
with:
@@ -44,7 +46,7 @@ jobs:
4446

4547
- name: Install contract-test dependencies
4648
run: |
47-
pip install "protobuf>=3.20,<4" mnemonic ecdsa pytest
49+
pip install "protobuf>=3.20,<4" mnemonic ecdsa pytest requests
4850
4951
- name: Run deterministic Zcash PCZT contract tests
5052
env:
@@ -54,6 +56,19 @@ jobs:
5456
tests/test_msg_zcash_sign_pczt.py \
5557
tests/test_zcash_seed_fingerprint_helper.py
5658
59+
# keepkey-firmware GENERATES its token table by running this repo's
60+
# generators (lib/firmware/CMakeLists.txt -> ethereum_tokens.def), so a
61+
# change here can break the firmware C++ build. .circleci/config.yml no
62+
# longer runs the firmware's own unit suite; this is the gate that
63+
# replaced it.
64+
- name: Run ABI encoder and token-table generator contract tests
65+
env:
66+
PROTOCOL_BUFFERS_PYTHON_IMPLEMENTATION: python
67+
run: |
68+
python -m pytest -q \
69+
tests/test_clearsign_abi.py \
70+
tests/test_token_table_generators.py
71+
5772
- name: Lint summary
5873
run: |
5974
echo "## 🔑 KeepKey python-keepkey — Lint" >> "$GITHUB_STEP_SUMMARY"
@@ -62,6 +77,8 @@ jobs:
6277
echo "|-------|--------|" >> "$GITHUB_STEP_SUMMARY"
6378
echo "| Syntax | ✅ PASS |" >> "$GITHUB_STEP_SUMMARY"
6479
echo "| Zcash PCZT contract | ✅ PASS |" >> "$GITHUB_STEP_SUMMARY"
80+
echo "| ABI encoder | ✅ PASS |" >> "$GITHUB_STEP_SUMMARY"
81+
echo "| Token-table generators | ✅ PASS |" >> "$GITHUB_STEP_SUMMARY"
6582
6683
# ═══════════════════════════════════════════════════════════
6784
# STAGE 2: TEST — pull published emulator, run pytest
Lines changed: 163 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,163 @@
1+
"""The firmware token table is generated BY THIS REPOSITORY.
2+
3+
lib/firmware/CMakeLists.txt in keepkey-firmware builds `ethereum_tokens.def`
4+
and `uniswap_tokens.def` by running
5+
6+
python3 deps/python-keepkey/keepkeylib/eth/ethereum_tokens.py <out>.def
7+
python3 deps/python-keepkey/keepkeylib/eth/uniswap_tokens.py <out>.def
8+
9+
and `kkfirmware` depends on that target. So a change in this repository CAN
10+
break the firmware C++ build: if either generator crashes, emits a malformed
11+
X(...) row, or emits an address that is not 20 bytes, the firmware does not
12+
compile -- and `tokens[]` is what unittests/firmware/coins.cpp reads.
13+
14+
.circleci/config.yml used to run the firmware's own C++ suite here and gated
15+
this job on it. That gate was removed. This module is the replacement Copilot
16+
asked for on that change: an equivalent generator/firmware contract gate that
17+
lives where the change originates, runs in seconds, and does not fail this
18+
repository for unrelated firmware C++ churn.
19+
20+
It deliberately asserts the BUILD contract (the generators run and emit a
21+
well-formed, budget-conforming table), not the token SELECTION, which is
22+
policy and moves.
23+
"""
24+
25+
import ast
26+
import os
27+
import re
28+
import subprocess
29+
import sys
30+
import tempfile
31+
import unittest
32+
33+
_HERE = os.path.dirname(os.path.abspath(__file__))
34+
_PYKEEPKEY = os.path.dirname(_HERE)
35+
_ETH = os.path.join(_PYKEEPKEY, 'keepkeylib', 'eth')
36+
37+
# X(chain_id, "<20 escaped bytes>", " SYMBOL", decimals) // comment
38+
_ROW = re.compile(r'^X\((\d+),\s*"((?:[^"\\]|\\.)*)",\s*"\s*([^"]+)",\s*(\d+)\)')
39+
40+
GENERATORS = (
41+
('ethereum_tokens.py', 'BUDGET_ETHEREUM_LISTS'),
42+
('uniswap_tokens.py', 'BUDGET_UNISWAP_LIST'),
43+
)
44+
45+
46+
def _vetted_source_present():
47+
"""The ethereum-lists submodule must be checked out for the eth generator."""
48+
return os.path.isdir(os.path.join(_ETH, 'ethereum-lists', 'src', 'tokens'))
49+
50+
51+
class TestTokenTableGenerators(unittest.TestCase):
52+
53+
def _run(self, script):
54+
"""Run one generator into a temp file and return its rows."""
55+
with tempfile.TemporaryDirectory() as tmp:
56+
out = os.path.join(tmp, 'out.def')
57+
proc = subprocess.run(
58+
[sys.executable, os.path.join(_ETH, script), out],
59+
capture_output=True, text=True, cwd=_PYKEEPKEY)
60+
self.assertEqual(
61+
proc.returncode, 0,
62+
'%s exited %d -- the firmware build runs this exact command '
63+
'and would fail here.\nstdout: %s\nstderr: %s'
64+
% (script, proc.returncode, proc.stdout[-2000:],
65+
proc.stderr[-2000:]))
66+
self.assertTrue(os.path.isfile(out),
67+
'%s produced no output file' % script)
68+
with open(out) as f:
69+
text = f.read()
70+
return text, (proc.stdout or '') + (proc.stderr or '')
71+
72+
def _rows(self, text, script):
73+
rows = []
74+
for line in text.splitlines():
75+
line = line.strip()
76+
if not line or line.startswith('#'):
77+
continue
78+
match = _ROW.match(line)
79+
self.assertIsNotNone(
80+
match,
81+
'%s emitted a line the firmware preprocessor cannot consume: %r'
82+
% (script, line[:120]))
83+
chain_id, address, symbol, decimals = match.groups()
84+
rows.append((int(chain_id), address, symbol.strip(), int(decimals)))
85+
return rows
86+
87+
def _check(self, script, budget_name):
88+
if script == 'ethereum_tokens.py' and not _vetted_source_present():
89+
self.skipTest('keepkeylib/eth/ethereum-lists submodule not checked out')
90+
from keepkeylib.eth import token_policy
91+
92+
text, log = self._run(script)
93+
rows = self._rows(text, script)
94+
self.assertTrue(rows, '%s emitted an empty table' % script)
95+
96+
budget = getattr(token_policy, budget_name)
97+
self.assertTrue(
98+
len(rows) <= budget,
99+
'%s emitted %d rows, over its %s budget of %d -- this symbol is '
100+
'the largest read-only object in the ARM image'
101+
% (script, len(rows), budget_name, budget))
102+
103+
# The generator reports 'N of M kept (budget B)'. Checking N against
104+
# min(M, B) catches SHRINKAGE too: a change that silently drops most
105+
# candidates still respects the ceiling, so `<= budget` alone would
106+
# let the device's token table quietly collapse.
107+
kept = re.search(r'(\d+) of (\d+) kept \(budget (\d+)\)', log)
108+
self.assertIsNotNone(
109+
kept, '%s no longer reports its keep/candidate counts: %r'
110+
% (script, log[-500:]))
111+
n_kept, n_candidates, reported_budget = (int(g) for g in kept.groups())
112+
self.assertEqual(reported_budget, budget,
113+
'%s reports a budget that is not %s' % (script, budget_name))
114+
self.assertEqual(
115+
n_kept, min(n_candidates, budget),
116+
'%s kept %d of %d candidates against a budget of %d -- it must '
117+
'fill the budget when the source has the entries'
118+
% (script, n_kept, n_candidates, budget))
119+
self.assertEqual(
120+
len(rows), n_kept,
121+
'%s reported %d kept but emitted %d rows'
122+
% (script, n_kept, len(rows)))
123+
124+
for chain_id, address, symbol, decimals in rows:
125+
# The C string is 20 raw bytes; anything else silently shifts the
126+
# packed token struct the firmware reads.
127+
raw = ast.literal_eval('b"%s"' % address)
128+
self.assertEqual(
129+
len(raw), 20,
130+
'%s: %s on chain %d has a %d-byte address, not 20'
131+
% (script, symbol, chain_id, len(raw)))
132+
self.assertTrue(
133+
0 <= decimals <= 32,
134+
'%s: %s has implausible decimals %d' % (script, symbol, decimals))
135+
self.assertTrue(
136+
symbol and '"' not in symbol,
137+
'%s: unusable symbol %r' % (script, symbol))
138+
139+
def test_ethereum_tokens_generator_builds_a_valid_table(self):
140+
self._check('ethereum_tokens.py', 'BUDGET_ETHEREUM_LISTS')
141+
142+
def test_ethereum_tokens_closes_the_x_macro(self):
143+
"""ethereum_tokens.def is #included after a #define X; leaving the
144+
macro defined leaks it into the next translation unit."""
145+
if not _vetted_source_present():
146+
self.skipTest('keepkeylib/eth/ethereum-lists submodule not checked out')
147+
self.assertIn('#undef X', self._run('ethereum_tokens.py')[0])
148+
149+
def test_uniswap_tokens_generator_builds_a_valid_table(self):
150+
self._check('uniswap_tokens.py', 'BUDGET_UNISWAP_LIST')
151+
152+
def test_generators_are_deterministic(self):
153+
"""The firmware build compares digests to decide whether to rewrite the
154+
.def; a non-deterministic generator would churn the table every build."""
155+
for script, _ in GENERATORS:
156+
if script == 'ethereum_tokens.py' and not _vetted_source_present():
157+
continue
158+
self.assertEqual(self._run(script)[0], self._run(script)[0],
159+
'%s is not deterministic' % script)
160+
161+
162+
if __name__ == '__main__':
163+
unittest.main()

0 commit comments

Comments
 (0)