Merge #311: Update firmware build scripts and tests to work with new firmwares

4a8f88e tests: Non-device tests do not need emulators compiled (Andrew Chow)
9b31a96 tests: Only send kill signal if process isn't already dead (Andrew Chow)
9ab7373 tests: Run each device's test suite individually (Andrew Chow)
3f82e60 test, trezor: report model T in tests correctly (Andrew Chow)
a741211 tests: move setUp and tearDown to DeviceTestCase (Andrew Chow)
e5808ac tests, ledger: Have speculos do button presses internally (Andrew Chow)
b8ba161 tests: use Travis' default xcode version (Andrew Chow)
4a4bf70 tests: Use new speculos cli args (Andrew Chow)
cb13862 tests: checkout Trezor T v2.2.0 (Andrew Chow)
83b60ad tests: coldcard simulator use --ms instead of -m (Andrew Chow)
cd0720f tests: Use p2sh-segwit address type for the p2sh-segwit import (Andrew Chow)
5355055 update Trezor 1 build script (Andrew Chow)
4b1c06f Update keepkey build script (Andrew Chow)

Pull request description:

  For the past 2 weeks, travis has been failing because hardware wallet vendors have been updating their firmware (and emulators) in a way that was incompatible with the HWI tests. I've finally gotten around to fixing that.

  Things that were updated:
  * Keepkey build script. They switched to Python 3 and require a newer version of nanopb. Some other build commands had to be modified.
  * Trezor 1 build script. `script/setup` needs to be run in the pipenv.
  * Trezor T master branch incompatibility. Trezor T has made some major change that makes HWI incompatible with their current master branch (and probably future firmware versions too). For now, we checkout the 2.2.0 (latest as of this PR) firmware for testing.
  * Speculos command line arguments. Speculos changed their command line argument names.
  * Speculos patched to just do the key presses internally. Instead of having us to run another thread to send the correct key presses at the right time, we modify speculos to do that itself.
  * Coldcard command line arguments. Coldcard changed some command line argument names.
  * Some ismine thing is "broken" in Core, added a workaround for that.
  * Each devices tests are run with their own test runner. Global emulators will only be run for their device's tests.
  * MacOS build will use travis' defaul xcode version so that we don't run into issues with it being outdated and no longer supported by travis

Top commit has no ACKs.

Tree-SHA512: 0a5b6eb759a57f05254814d3c90e786989e41474d15b95393c08ef68e515fa31847129cef0933e0c801b7dea882abf3674438c0483ca57824024e25e6a566c45

Author: achow101

.travis.yml

@@ -69,6 +69,9 @@ jobs:
             script: mypy hwilib/base58.py
         -   name: Run non-device tests only
             stage: test
+            install:
+                - pip install poetry
+                - poetry install
             script: cd test; poetry run ./run_tests.py
         -   name: With process_commands interface
             stage: test
@@ -112,7 +115,6 @@ jobs:
         -   name: macOS binary distribution (no tests)
             stage: test
             os: osx
-            osx_image: xcode7.3
             language: generic
             addons:
                 artifacts:

test/data/speculos-auto-button.patch

@@ -0,0 +1,64 @@
+From bd251085fa85ad20e90fa055e7487b5c73d3dc7d Mon Sep 17 00:00:00 2001
+From: Andrew Chow <achow101-github@achow101.com>
+Date: Thu, 12 Mar 2020 17:25:09 -0400
+Subject: [PATCH] Do button presses within seproxyhal
+
+---
+ mcu/seproxyhal.py | 34 ++++++++++++++++++++++++++++++++++
+ 1 file changed, 34 insertions(+)
+
+diff --git a/mcu/seproxyhal.py b/mcu/seproxyhal.py
+index 8ed3fec..1a6bfb6 100644
+--- a/mcu/seproxyhal.py
++++ b/mcu/seproxyhal.py
+@@ -2,6 +2,7 @@ import binascii
+ import logging
+ import os
+ import select
++import struct
+ import sys
+ import time
+ import threading
+@@ -144,6 +145,39 @@ class SeProxyHal:
+                 #self.logger.debug(f"DISPLAY_STATUS {data!r}")
+                 screen.display_status(data)
+                 self._queue_event_packet(SephTag.DISPLAY_PROCESSED_EVENT)
++                data_type, _, x, y, = struct.unpack('bbhh', data[:6])
++                if data_type == 7:
++                    if y == 12 or y ==28:
++                        self.line = data[28:].decode()
++                    elif y == 26:
++                        self.line += data[28:].decode()
++
++                    if y == 26 or y == 28:
++                        if self.line.startswith('Address') or self.line.startswith('Message hash') or self.line.startswith('Reviewoutput') or self.line.startswith('Amount') or self.line.startswith('Fees') or self.line.startswith('Confirmtransaction'):
++                            self.handle_button(2, True)
++                            self.handle_button(2, False)
++                            if self.line.startswith('Message hash'):
++                                self.seen_msg_hash = True
++                        elif self.line == 'Approve' or self.line.startswith('Accept'):
++                            self.handle_button(1, True)
++                            self.handle_button(2, True)
++                            self.handle_button(1, False)
++                            self.handle_button(2, False)
++                        elif self.line == 'Signmessage':
++                            if self.seen_msg_hash:
++                                self.handle_button(1, True)
++                                self.handle_button(2, True)
++                                self.handle_button(1, False)
++                                self.handle_button(2, False)
++                                self.seen_msg_hash = False
++                            else:
++                                self.handle_button(2, True)
++                                self.handle_button(2, False)
++                                self.handle_button(2, True)
++                                self.handle_button(2, False)
++                        elif self.line == 'Cancel' or self.line == 'Reject':
++                            self.handle_button(1, True)
++                            self.handle_button(1, False)
+ 
+             elif tag == SephTag.SCREEN_DISPLAY_RAW_STATUS:
+                 self.logger.debug("SephTag.SCREEN_DISPLAY_RAW_STATUS")
+-- 
+2.25.2
+

test/data/speculos-screen-text.patch

@@ -56,13 +56,15 @@
 args = parser.parse_args()
 
 # Run tests
+success = True
 suite = unittest.TestSuite()
 suite.addTests(unittest.defaultTestLoader.loadTestsFromTestCase(TestDescriptor))
 suite.addTests(unittest.defaultTestLoader.loadTestsFromTestCase(TestSegwitAddress))
 suite.addTests(unittest.defaultTestLoader.loadTestsFromTestCase(TestPSBT))
 suite.addTests(unittest.defaultTestLoader.loadTestsFromTestCase(TestBase58))
 if sys.platform.startswith("linux"):
     suite.addTests(unittest.defaultTestLoader.loadTestsFromTestCase(TestUdevRulesInstaller))
+success = unittest.TextTestRunner(stream=sys.stdout, verbosity=2).run(suite).wasSuccessful()
 
 if args.all:
     # Default all true unless overridden
@@ -86,17 +88,16 @@
     rpc, userpass = start_bitcoind(args.bitcoind)
 
     if args.bitbox:
-        suite.addTest(digitalbitbox_test_suite(args.bitbox_path, rpc, userpass, args.interface))
+        success &= digitalbitbox_test_suite(args.bitbox_path, rpc, userpass, args.interface)
     if args.coldcard:
-        suite.addTest(coldcard_test_suite(args.coldcard_path, rpc, userpass, args.interface))
+        success &= coldcard_test_suite(args.coldcard_path, rpc, userpass, args.interface)
     if args.trezor:
-        suite.addTest(trezor_test_suite(args.trezor_path, rpc, userpass, args.interface))
+        success &= trezor_test_suite(args.trezor_path, rpc, userpass, args.interface)
     if args.trezor_t:
-        suite.addTest(trezor_test_suite(args.trezor_t_path, rpc, userpass, args.interface, True))
+        success &= trezor_test_suite(args.trezor_t_path, rpc, userpass, args.interface, True)
     if args.keepkey:
-        suite.addTest(keepkey_test_suite(args.keepkey_path, rpc, userpass, args.interface))
+        success &= keepkey_test_suite(args.keepkey_path, rpc, userpass, args.interface)
     if args.ledger:
-        suite.addTest(ledger_test_suite(args.ledger_path, rpc, userpass, args.interface))
+        success &= ledger_test_suite(args.ledger_path, rpc, userpass, args.interface)
 
-result = unittest.TextTestRunner(stream=sys.stdout, verbosity=2).run(suite)
-sys.exit(not result.wasSuccessful())
+sys.exit(not success)

test/run_tests.py

@@ -12,6 +12,7 @@ trezor_setup_needed=false
 if [ ! -d "trezor-firmware" ]; then
     git clone --recursive https://github.com/trezor/trezor-firmware.git
     cd trezor-firmware
+    git checkout core/v2.2.0
     trezor_setup_needed=true
 else
     cd trezor-firmware
@@ -36,8 +37,8 @@ fi
 cd legacy
 export EMULATOR=1 TREZOR_TRANSPORT_V1=1 DEBUG_LINK=1 HEADLESS=1
 if [ "$trezor_setup_needed" == true ] ; then
-    script/setup
-    pipenv install
+    pipenv sync
+    pipenv run script/setup
 fi
 pipenv run script/cibuild
 # Delete any emulator.img file
@@ -147,18 +148,14 @@ fi
 
 # Build the simulator. This is cached, but it is also fast
 if [ "$keepkey_setup_needed" == true ] ; then
-    git clone https://github.com/nanopb/nanopb.git -b nanopb-0.2.9.2
+    git clone https://github.com/nanopb/nanopb.git -b nanopb-0.3.9.4
 fi
-# This needs py2, so make a pipenv
-export PIPENV_IGNORE_VIRTUALENVS=1
-pipenv --python 2.7
-pipenv install protobuf
 cd nanopb/generator/proto
-pipenv run make
+make
 cd ../../../
 export PATH=$PATH:`pwd`/nanopb/generator
-pipenv run cmake -C cmake/caches/emulator.cmake . -DNANOPB_DIR=nanopb/ -DKK_HAVE_STRLCAT=OFF -DKK_HAVE_STRLCPY=OFF
-pipenv run make -j$(nproc) kkemu
+cmake -C cmake/caches/emulator.cmake . -DNANOPB_DIR=nanopb/ -DPROTOC_BINARY=/usr/bin/protoc
+make -j$(nproc) kkemu
 # Delete any emulator.img file
 find . -name "emulator.img" -exec rm {} \;
 cd ..
@@ -187,7 +184,7 @@ else
     fi
 fi
 # Apply patch to get screen info
-git am ../../data/speculos-screen-text.patch
+git am ../../data/speculos-auto-button.patch
 
 # Build the simulator. This is cached, but it is also fast
 mkdir -p build

test/setup_environment.sh

@@ -5,6 +5,7 @@
 import os
 import signal
 import subprocess
+import sys
 import time
 import unittest
 
@@ -13,7 +14,7 @@
 
 def coldcard_test_suite(simulator, rpc, userpass, interface):
     # Start the Coldcard simulator
-    coldcard_proc = subprocess.Popen(['python3', os.path.basename(simulator), '-m'], cwd=os.path.dirname(simulator), stdout=subprocess.DEVNULL, preexec_fn=os.setsid)
+    coldcard_proc = subprocess.Popen(['python3', os.path.basename(simulator), '--ms'], cwd=os.path.dirname(simulator), stdout=subprocess.DEVNULL, preexec_fn=os.setsid)
     # Wait for simulator to be up
     while True:
         try:
@@ -31,8 +32,9 @@ def coldcard_test_suite(simulator, rpc, userpass, interface):
     # Cleanup
 
     def cleanup_simulator():
-        os.killpg(os.getpgid(coldcard_proc.pid), signal.SIGTERM)
-        os.waitpid(os.getpgid(coldcard_proc.pid), 0)
+        if coldcard_proc.poll() is None:
+            os.killpg(os.getpgid(coldcard_proc.pid), signal.SIGTERM)
+            os.waitpid(os.getpgid(coldcard_proc.pid), 0)
     atexit.register(cleanup_simulator)
 
     # Coldcard specific management command tests
@@ -101,7 +103,11 @@ def test_getxpub(self):
     suite.addTest(DeviceTestCase.parameterize(TestDisplayAddress, rpc, userpass, 'coldcard', 'coldcard', '/tmp/ckcc-simulator.sock', '0f056943', 'tpubDDpWvmUrPZrhSPmUzCMBHffvC3HyMAPnWDSAQNBTnj1iZeJa7BZQEttFiP4DS4GCcXQHezdXhn86Hj6LHX5EDstXPWrMaSneRWM8yUf6NFd', interface=interface))
     suite.addTest(DeviceTestCase.parameterize(TestSignMessage, rpc, userpass, 'coldcard', 'coldcard', '/tmp/ckcc-simulator.sock', '0f056943', 'tpubDDpWvmUrPZrhSPmUzCMBHffvC3HyMAPnWDSAQNBTnj1iZeJa7BZQEttFiP4DS4GCcXQHezdXhn86Hj6LHX5EDstXPWrMaSneRWM8yUf6NFd', interface=interface))
     suite.addTest(DeviceTestCase.parameterize(TestSignTx, rpc, userpass, 'coldcard', 'coldcard', '/tmp/ckcc-simulator.sock', '0f056943', 'tpubDDpWvmUrPZrhSPmUzCMBHffvC3HyMAPnWDSAQNBTnj1iZeJa7BZQEttFiP4DS4GCcXQHezdXhn86Hj6LHX5EDstXPWrMaSneRWM8yUf6NFd', interface=interface))
-    return suite
+
+    result = unittest.TextTestRunner(stream=sys.stdout, verbosity=2).run(suite)
+    cleanup_simulator()
+    atexit.unregister(cleanup_simulator)
+    return result.wasSuccessful()
 
 if __name__ == '__main__':
     parser = argparse.ArgumentParser(description='Test Coldcard implementation')
@@ -113,5 +119,4 @@ def test_getxpub(self):
     # Start bitcoind
     rpc, userpass = start_bitcoind(args.bitcoind)
 
-    suite = coldcard_test_suite(args.simulator, rpc, userpass, args.interface)
-    unittest.TextTestRunner(verbosity=2).run(suite)
+    sys.exit(not coldcard_test_suite(args.simulator, rpc, userpass, args.interface))

test/test_coldcard.py

@@ -113,13 +113,13 @@ def __str__(self):
     def __repr__(self):
         return '{}: {}'.format(self.full_type, super().__repr__())
 
-class TestDeviceConnect(DeviceTestCase):
     def setUp(self):
         self.emulator.start()
 
     def tearDown(self):
         self.emulator.stop()
 
+class TestDeviceConnect(DeviceTestCase):
     def test_enumerate(self):
         enum_res = self.do_command(self.get_password_args() + ['enumerate'])
         found = False
@@ -175,9 +175,6 @@ def setUp(self):
             self.dev_args.append('--testnet')
         self.emulator.start()
 
-    def tearDown(self):
-        self.emulator.stop()
-
     def test_getkeypool_bad_args(self):
         result = self.do_command(self.dev_args + ['getkeypool', '--sh_wpkh', '--wpkh', '0', '20'])
         self.assertIn('error', result)
@@ -205,9 +202,9 @@ def test_getkeypool(self):
         import_result = self.wrpc.importmulti(keypool_desc)
         self.assertTrue(import_result[0]['success'])
         for i in range(0, 21):
-            addr_info = self.wrpc.getaddressinfo(self.wrpc.getnewaddress())
+            addr_info = self.wrpc.getaddressinfo(self.wrpc.getnewaddress('', 'p2sh-segwit'))
             self.assertEqual(addr_info['hdkeypath'], "m/49'/1'/0'/0/{}".format(i))
-            addr_info = self.wrpc.getaddressinfo(self.wrpc.getrawchangeaddress())
+            addr_info = self.wrpc.getaddressinfo(self.wrpc.getrawchangeaddress('p2sh-segwit'))
             self.assertEqual(addr_info['hdkeypath'], "m/49'/1'/0'/1/{}".format(i))
 
         keypool_desc = self.do_command(self.dev_args + ['getkeypool', '--wpkh', '0', '20'])
@@ -223,9 +220,9 @@ def test_getkeypool(self):
         import_result = self.wrpc.importmulti(keypool_desc)
         self.assertTrue(import_result[0]['success'])
         for i in range(0, 21):
-            addr_info = self.wrpc.getaddressinfo(self.wrpc.getnewaddress())
+            addr_info = self.wrpc.getaddressinfo(self.wrpc.getnewaddress('', 'p2sh-segwit'))
             self.assertEqual(addr_info['hdkeypath'], "m/49'/1'/3'/0/{}".format(i))
-            addr_info = self.wrpc.getaddressinfo(self.wrpc.getrawchangeaddress())
+            addr_info = self.wrpc.getaddressinfo(self.wrpc.getrawchangeaddress('p2sh-segwit'))
             self.assertEqual(addr_info['hdkeypath'], "m/49'/1'/3'/1/{}".format(i))
         keypool_desc = self.do_command(self.dev_args + ['getkeypool', '--wpkh', '--account', '3', '0', '20'])
         import_result = self.wrpc.importmulti(keypool_desc)
@@ -289,9 +286,6 @@ def setUp(self):
             self.dev_args.append('--testnet')
         self.emulator.start()
 
-    def tearDown(self):
-        self.emulator.stop()
-
     def _generate_and_finalize(self, unknown_inputs, psbt):
         if not unknown_inputs:
             # Just do the normal signing process to test "all inputs" case
@@ -464,12 +458,6 @@ def test_big_tx(self):
                 pass
 
 class TestDisplayAddress(DeviceTestCase):
-    def setUp(self):
-        self.emulator.start()
-
-    def tearDown(self):
-        self.emulator.stop()
-
     def test_display_address_bad_args(self):
         result = self.do_command(self.dev_args + ['displayaddress', '--sh_wpkh', '--wpkh', '--path', 'm/49h/1h/0h/0/0'])
         self.assertIn('error', result)
@@ -544,12 +532,6 @@ def test_display_address_descriptor(self):
         self.assertEqual(result['code'], -7)
 
 class TestSignMessage(DeviceTestCase):
-    def setUp(self):
-        self.emulator.start()
-
-    def tearDown(self):
-        self.emulator.stop()
-
     def test_sign_msg(self):
         self.do_command(self.dev_args + ['signmessage', '"Message signing test"', 'm/44h/1h/0h/0/0'])
 

test/test_device.py

@@ -5,6 +5,7 @@
 import json
 import os
 import subprocess
+import sys
 import time
 import unittest
 
@@ -147,7 +148,11 @@ def test_getxpub(self):
     suite.addTest(DeviceTestCase.parameterize(TestGetKeypool, rpc, userpass, type, full_type, path, fingerprint, master_xpub, '0000', interface=interface))
     suite.addTest(DeviceTestCase.parameterize(TestSignTx, rpc, userpass, type, full_type, path, fingerprint, master_xpub, '0000', interface=interface))
     suite.addTest(DeviceTestCase.parameterize(TestSignMessage, rpc, userpass, type, full_type, path, fingerprint, master_xpub, '0000', interface=interface))
-    return suite
+
+    result = unittest.TextTestRunner(stream=sys.stdout, verbosity=2).run(suite)
+    cleanup_simulator()
+    atexit.unregister(cleanup_simulator)
+    return result.wasSuccessful()
 
 if __name__ == '__main__':
     parser = argparse.ArgumentParser(description='Test Digital Bitbox implementation')
@@ -159,5 +164,4 @@ def test_getxpub(self):
     # Start bitcoind
     rpc, userpass = start_bitcoind(args.bitcoind)
 
-    suite = digitalbitbox_test_suite(args.simulator, rpc, userpass, args.interface)
-    unittest.TextTestRunner(verbosity=2).run(suite)
+    sys.exit(not digitalbitbox_test_suite(args.simulator, rpc, userpass, args.interface))