feat(core): Start displaying Ethereum ABI in a generalized way

Step toward solving issue #69

Improved Ethereum address display and changed test cases to use addresses including letter to see the upper/lower case formatting.

Added test for known ERC-20 token (DAI)

[no changelog]

common/tests/fixtures/ethereum/sign_tx.json

@@ -5,7 +5,7 @@
     },
     "tests": [
         {
-            "name": "erc20_token",
+            "name": "erc20_transfer_unknown_token",
             "parameters": {
                 "comment": "Sending 291 Grzegorz Brzęczyszczykiewicz tokens to address 0x574bbb36871ba6b78e27f4b4dcfb76ea0091880b",
                 "data": "a9059cbb000000000000000000000000574bbb36871ba6b78e27f4b4dcfb76ea0091880b0000000000000000000000000000000000000000000000000000000000000123",
@@ -24,11 +24,31 @@
                 "sig_s": "633a74429eb6d3aeec4ed797542236a85daab3cab15e37736b87a45697541d7a"
             }
         },
+        {
+            "name": "erc20_transfer_dai",
+            "parameters": {
+                "comment": "Sending 291 Grzegorz Brzęczyszczykiewicz tokens to address 0x574bbb36871ba6b78e27f4b4dcfb76ea0091880b",
+                "data": "a9059cbb000000000000000000000000574bbb36871ba6b78e27f4b4dcfb76ea0091880b0000000000000000000000000000000000000000000000000000000000000123",
+                "path": "m/44'/60'/0'/0/1",
+                "to_address": "0x6b175474e89094c44da98b954eedeac495271d0f",
+                "chain_id": 1,
+                "nonce": "0x0",
+                "gas_price": "0x14",
+                "gas_limit": "0x14",
+                "tx_type": null,
+                "value": "0x0"
+            },
+            "result": {
+                "sig_v": 37,
+                "sig_r": "72118d439b1784d235656d8b60a0b810ed68946b30901a42f85f8f19c14a655c",
+                "sig_s": "0804894c9e693faf144b4d2aaa16b05d08daf8215bf7272ce4135c7302766f56"
+            }
+        },
         {
             "name": "erc20_approve",
             "parameters": {
                 "comment": "Calling approve function for ERC 20 token",
-                "data": "095ea7b300000000000000000000000012345678901234567890123456789012345678900000000000000000000000000000000000000000000000056bc75e2d63100000",
+                "data": "095ea7b3000000000000000000000000c460622c115537f05137c407ad17b06bb115be8b0000000000000000000000000000000000000000000000056bc75e2d63100000",
                 "path": "m/44'/60'/0'/0/1",
                 "to_address": "0xfc6b5d6af8a13258f7cbd0d39e11b35e01a32f93",
                 "chain_id": 1,
@@ -40,8 +60,8 @@
             },
             "result": {
                 "sig_v": 38,
-                "sig_r": "44386ce8c59780b2c7674f347fc891bcb22bd5eddf5b17bf9bc3fc71f06e0561",
+                "sig_r": "42beac9ee8fd2391c48cb4ad8d8852843772a675693d5146a9eebb5907296f52",
-                "sig_s": "1467aa6e6887f4bef4278caad3499f06c1f8730967d988f760a4f536db7796f3"
+                "sig_s": "36680d5acffd5620b71a60815c6daae4b322db7a7e987482be13e67e5610bbab"
             }
         },
         {

core/src/apps/ethereum/layout.py

@@ -1,5 +1,4 @@
 from typing import TYPE_CHECKING
-from ubinascii import hexlify
 
 from trezor import ui
 from trezor.enums import ButtonRequestType
@@ -13,7 +12,7 @@ from trezor.ui.layouts import (
     should_show_more,
 )
 
-from .helpers import decode_typed_data
+from .helpers import address_from_bytes, decode_typed_data
 
 if TYPE_CHECKING:
     from typing import Any, Awaitable, Iterable
@@ -118,10 +117,12 @@ async def require_confirm_eip1559_fee(
         )
 
 
-def require_confirm_unknown_token(address_bytes: bytes) -> Awaitable[None]:
+def require_confirm_unknown_token(
+    address_bytes: bytes, network: EthereumNetworkInfo
+) -> Awaitable[None]:
     from trezor.ui.layouts import confirm_address
 
-    contract_address_hex = "0x" + hexlify(address_bytes).decode()
+    contract_address_hex = address_from_bytes(address_bytes, network)
     return confirm_address(
         "Unknown token",
         contract_address_hex,
@@ -131,10 +132,12 @@ def require_confirm_unknown_token(address_bytes: bytes) -> Awaitable[None]:
     )
 
 
-def require_confirm_address(address_bytes: bytes) -> Awaitable[None]:
+def require_confirm_address(
+    address_bytes: bytes, network: EthereumNetworkInfo
+) -> Awaitable[None]:
     from trezor.ui.layouts import confirm_address
 
-    address_hex = "0x" + hexlify(address_bytes).decode()
+    address_hex = address_from_bytes(address_bytes, network)
     return confirm_address(
         "Signing address",
         address_hex,
@@ -169,7 +172,7 @@ async def require_confirm_approve_tx(
     network: EthereumNetworkInfo,
     token: EthereumTokenInfo,
 ) -> None:
-    address_hex = "0x" + hexlify(address_bytes).decode()
+    address_hex = address_from_bytes(address_bytes, network)
 
     await confirm_action(
         "confirm_approve",

core/src/apps/ethereum/sign_tx.py

@@ -1,15 +1,18 @@
 from typing import TYPE_CHECKING
 
 from trezor.crypto import rlp
-from trezor.messages import EthereumTxRequest
+from trezor.messages import EthereumNetworkInfo, EthereumTxRequest
 from trezor.wire import DataError
 
-from .helpers import bytes_from_address
+from .helpers import address_from_bytes, bytes_from_address
 from .keychain import with_keychain_from_chain_id
 
 if TYPE_CHECKING:
+    from typing import Any
+
     from apps.common.keychain import Keychain
     from trezor.messages import EthereumSignTx, EthereumTxAck, EthereumTokenInfo
+    from trezor.ui.layouts.common import PropertyType
 
     from .definitions import Definitions
     from .keychain import MsgInSignTx
@@ -93,50 +96,81 @@ async def sign_tx_inner(
 ) -> tuple[EthereumTokenInfo | None, bytes, bytes, int | None]:
     from . import tokens
     from .layout import (
-        require_confirm_approve_tx,
         require_confirm_data,
         require_confirm_tx,
         require_confirm_unknown_token,
     )
+    from trezor.ui.layouts import confirm_properties
+    from trezor.enums import ButtonRequestType
     from apps.common import paths
+    from ubinascii import hexlify
 
     check_common_fields(msg)
     await paths.validate_path(keychain, msg.address_n)
 
-    data_initial_chunk = msg.data_initial_chunk  # local_cache_attribute
+    data_initial_chunk = memoryview(msg.data_initial_chunk)  # local_cache_attribute
     token = None
     address_bytes = recipient = bytes_from_address(msg.to)
     value = int.from_bytes(msg.value, "big")
+
+    FUNCTION_HASH_LENGTH = (
+        4  # ETH function hashes are truncated to 4 bytes in calldata for some reason.
+    )
     if (
         len(msg.to) in (40, 42)
         and len(msg.value) == 0
-        and msg.data_length == 68
+        and msg.data_length >= FUNCTION_HASH_LENGTH
-        and len(data_initial_chunk) == 68
+        and len(data_initial_chunk) >= FUNCTION_HASH_LENGTH
     ):
-        initial_prefix = data_initial_chunk[:16]
+        function_hash = int.from_bytes(data_initial_chunk[:FUNCTION_HASH_LENGTH], "big")
-        # See https://github.com/ethereum-lists/4bytes/blob/master/signatures/a9059cbb etc
-        ETH_ERC20_TRANSFER = (
-            b"\xa9\x05\x9c\xbb\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
-        )
-        ETH_ERC20_APPROVE = (
-            b"\x09\x5e\xa7\xb3\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
-        )
 
-        if initial_prefix == ETH_ERC20_TRANSFER:
+        if value:
-            token = defs.get_token(address_bytes)
+            raise DataError(
-            recipient = data_initial_chunk[16:36]
+                "Expecting zero or None as native transaction value when calling smart contract functions"
-            value = int.from_bytes(data_initial_chunk[36:68], "big")
+            )
+        # Force 0 value to None to signify that we are not spending regular ethereum in later dialogs
+        value = None
 
-            if token is tokens.UNKNOWN_TOKEN:
+        confirm_props: list[PropertyType] = []
-                await require_confirm_unknown_token(address_bytes)
-        elif initial_prefix == ETH_ERC20_APPROVE:
-            token = defs.get_token(address_bytes)
-            recipient = data_initial_chunk[16:36]
-            # We are not transferring any value, just setting an allowance.
-            value = None
-            allowance = int.from_bytes(data_initial_chunk[36:68], "big")
 
-            await require_confirm_approve_tx(recipient, allowance, defs.network, token)
+        # Bother the user with confirming unknown tokens if we are indeed calling a function and not just transferring value.
+        token = defs.get_token(address_bytes)
+        if token is tokens.UNKNOWN_TOKEN:
+            await require_confirm_unknown_token(address_bytes, defs.network)
+        else:
+            confirm_props.append(("Token:", token.name))
+
+        if defs.network:
+            confirm_props.append(("Network:", defs.network.name))
+
+        if abi := _fetch_function_abi(function_hash):
+            confirm_props.extend(
+                _fetch_abi_input_properties(
+                    abi["inputs"],
+                    data_initial_chunk[FUNCTION_HASH_LENGTH:],
+                    defs.network,
+                )
+            )
+
+            await confirm_properties(
+                "confirm_call",
+                abi["description"],
+                confirm_props,
+                hold=True,
+                br_code=ButtonRequestType.SignTx,
+            )
+        elif token:
+            confirm_props.append(
+                ("Raw calldata:", f"0x{hexlify(data_initial_chunk).decode()}")
+            )
+
+            await confirm_properties(
+                "confirm_unknown_abi",
+                "Confirm calling function",
+                confirm_props,
+                hold=True,
+                br_code=ButtonRequestType.SignTx,
+            )
 
     if value is not None:
         await require_confirm_tx(recipient, value, defs.network, token)
@@ -147,6 +181,108 @@ async def sign_tx_inner(
     return token, address_bytes, recipient, value
 
 
+def _fetch_function_abi(function_hash: int) -> dict[str, Any] | None:
+    # Current limitations:
+    # * Only contains ERC-20 functions
+    # * Does not include outputs, not shown in UI
+    # In the future this lookup could be provided by the host, signed with SL keys.
+    ABI_DB: dict[int, dict[str, Any]] = {
+        0xDD62ED3E: {
+            "name": "allowance",
+            "description": "ERC 20 Fetch allowance amount",
+            "inputs": [
+                {"name": "owner", "type": "address"},
+                {"name": "spender", "type": "address"},
+            ],
+        },
+        0x095EA7B3: {
+            "name": "approve",
+            "description": "ERC 20 Approve allowance",
+            "inputs": [
+                {
+                    "name": "spender",
+                    "type": "address",
+                },
+                {
+                    "name": "value",
+                    "type": "uint256",
+                },
+            ],
+        },
+        0xA9059CBB: {
+            "name": "transfer",
+            "description": "ERC 20 Transfer",
+            "inputs": [
+                {
+                    "name": "to",
+                    "type": "address",
+                },
+                {
+                    "name": "value",
+                    "type": "uint256",
+                },
+            ],
+        },
+        0x23B872DD: {
+            "name": "transferFrom",
+            "description": "ERC 20 Transfer from",
+            "inputs": [
+                {
+                    "name": "from",
+                    "type": "address",
+                },
+                {
+                    "name": "to",
+                    "type": "address",
+                },
+                {
+                    "name": "value",
+                    "type": "uint256",
+                },
+            ],
+        },
+    }
+    return ABI_DB.get(function_hash)
+
+
+def _fetch_abi_input_properties(
+    input_defs: list[dict[str, str]], calldata: memoryview, network: EthereumNetworkInfo
+) -> list[PropertyType]:
+    expected_data_length = len(input_defs) * 32
+    if len(calldata) != expected_data_length:
+        raise DataError(
+            f"Function definition requires more parameter data than provided in calldata ({len(calldata)} vs {expected_data_length})."
+        )
+
+    input_props: list[PropertyType] = []
+
+    for index, input_def in enumerate(input_defs):
+        input_type = input_def["type"]
+        input_name = input_def["name"]
+
+        if input_type == "address":
+            # We skip over the first 12 bytes of zero padding in the address
+            input_address = calldata[12 + (index * 32) : 32 + (index * 32)]
+            input_props.append(
+                (
+                    f"{input_name} ({input_type}):",
+                    address_from_bytes(input_address, network),
+                )
+            )
+        elif input_type == "uint256":
+            input_uint = int.from_bytes(
+                calldata[(index * 32) : 32 + (index * 32)], "big"
+            )
+            input_props.append((f"{input_name} ({input_type}):", str(input_uint)))
+        else:
+            raise DataError(
+                "Currently unsupported type returned by _fetch_function_abi: "
+                + input_type
+            )
+
+    return input_props
+
+
 def _get_total_length(msg: EthereumSignTx, data_total: int) -> int:
     length = 0
     if msg.tx_type is not None:

core/src/apps/ethereum/sign_typed_data.py

@@ -39,7 +39,7 @@ async def sign_typed_data(
     address_bytes: bytes = node.ethereum_pubkeyhash()
 
     # Display address so user can validate it
-    await require_confirm_address(address_bytes)
+    await require_confirm_address(address_bytes, defs.network)
 
     data_hash = await _generate_typed_data_hash(
         msg.primary_type, msg.metamask_v4_compat

tests/device_tests/ethereum/test_signtx.py

@@ -412,6 +412,6 @@ def test_signtx_erc20_approve(client: Client):
             value=0x0,
             tx_type=None,
             data=bytes.fromhex(
-                "095ea7b300000000000000000000000012345678901234567890123456789012345678900000000000000000000000000000000000000000000000056bc75e2d63100000"
+                "095ea7b3000000000000000000000000c460622c115537f05137c407ad17b06bb115be8b0000000000000000000000000000000000000000000000056bc75e2d63100000"
             ),
         )

tests/input_flows.py

@@ -1324,27 +1324,43 @@ class InputFlowErc20Approve(InputFlowBase):
 
     def input_flow_tt(self) -> GeneratorType:
         br = yield
-        assert br.code == messages.ButtonRequestType.Other
+        assert br.code == B.SignTx
-        assert "CONFIRM APPROVE" == self.layout().title()
+        assert "UNKNOWN TOKEN" == self.layout().title()
         assert (
-            "Allow 0x123456789012345- 67890123456789012- 34567890 to withdraw up to 100,000,000,000,0"
+            "Contract: 0xFc6B5d6af8A13258f 7CbD0D39E11b35e01a3 2F93"
+            in self.layout().text_content()
+        )
+        self.debug.press_yes()
+
+        yield
+        assert br.code == B.SignTx
+        assert "ERC 20 APPROVE ALLOWANCE" == self.layout().title()
+        assert "Network: Ethereum" in self.layout().text_content()
+        self.debug.swipe_up()
+
+        assert br.code == B.SignTx
+        assert "ERC 20 APPROVE ALLOWANCE" == self.layout().title()
+        assert (
+            "spender (address): 0xC460622c115537f0- 5137C407Ad17b06bb1- 15bE8b"
             in self.layout().text_content()
         )
         self.debug.swipe_up()
 
-        assert br.code == messages.ButtonRequestType.Other
+        assert br.code == B.SignTx
-        assert "CONFIRM APPROVE" == self.layout().title()
+        assert "ERC 20 APPROVE ALLOWANCE" == self.layout().title()
-        assert "00,000,000 Wei UNKN" in self.layout().text_content()
+        assert (
+            "value (uint256): 100000000000000- 000000" in self.layout().text_content()
+        )
         self.debug.press_yes()
 
         yield
-        assert br.code == messages.ButtonRequestType.Other
+        assert br.code == B.SignTx
         assert "CONFIRM FEE" == self.layout().title()
         assert "Gas price: 20 Wei ETH" in self.layout().text_content()
         self.debug.press_yes()
 
         yield
-        assert br.code == messages.ButtonRequestType.Other
+        assert br.code == B.SignTx
         assert "TOTAL" == self.layout().title()
         assert "Maximum fee: 400 Wei ETH" in self.layout().text_content()
         self.debug.press_yes()

tests/ui_tests/fixtures.json

@@ -453,7 +453,7 @@
 "T1_ethereum-test_signtx.py::test_signtx[Unknown_chain_id_testnet_path]": "4552f9a68c62613e1ef7df68c1f47f7d4888c4505f0586012b71c07acba1fbcb",
 "T1_ethereum-test_signtx.py::test_signtx[data_1]": "8b432aba21bc4344814cceaf693e114b9d3e3d6ceb83c3a6af7c3ed0f9b37449",
 "T1_ethereum-test_signtx.py::test_signtx[data_2_bigdata]": "445286b7501ca67dd16dafd7ea09c57cc4a37a642ae50f0c812d74353c37c017",
-"T1_ethereum-test_signtx.py::test_signtx[erc20_token]": "d797c970429f1e974a69b9f5bd814a4f13f15b1b5d6ab1c117e65b919a434cf9",
+"T1_ethereum-test_signtx.py::test_signtx[erc20_transfer]": "d797c970429f1e974a69b9f5bd814a4f13f15b1b5d6ab1c117e65b919a434cf9",
 "T1_ethereum-test_signtx.py::test_signtx[max_chain_id]": "4552f9a68c62613e1ef7df68c1f47f7d4888c4505f0586012b71c07acba1fbcb",
 "T1_ethereum-test_signtx.py::test_signtx[max_chain_plus_one]": "4552f9a68c62613e1ef7df68c1f47f7d4888c4505f0586012b71c07acba1fbcb",
 "T1_ethereum-test_signtx.py::test_signtx[max_uint64]": "a6e6d63cba839c897e80dc0b7cf5c2263be8ff64a5281a43fca992380cca872b",
@@ -2756,8 +2756,9 @@
 "TT_ethereum-test_signtx.py::test_signtx[Unknown_chain_id_testnet_path]": "3723ceb0436e8d7f4669edcf36e72cb1fe65d891f910886fa79e9e406c5e9479",
 "TT_ethereum-test_signtx.py::test_signtx[data_1]": "863146c0494b76d4d8a9773a406994c3a35fb8c0cb989f372214aaa80b63b73c",
 "TT_ethereum-test_signtx.py::test_signtx[data_2_bigdata]": "05aa081d00a78c3a432904b19c21df1512c7660f4be3edaec75c9bc1aad83ae8",
-"TT_ethereum-test_signtx.py::test_signtx[erc20_token]": "d51cf4119a784bac447971e8e80be44985c28a9e5f14917efb7c2064d7f21752",
+"TT_ethereum-test_signtx.py::test_signtx[erc20_transfer_unknown_token]": "577e31164a06244975ce24f985f60ff18a12abc52202565bbabedb0fe8470499",
-"TT_ethereum-test_signtx.py::test_signtx[erc20_approve]": "52b9f649ead0dd8a8fbcc8c6ee215299310b40c8837be323f6acf9d4789c010d",
+"TT_ethereum-test_signtx.py::test_signtx[erc20_transfer_dai]": "b1fcfc5c4d4a8fc025f7f8c3d4bd53d87893eb74655aa19f3365663a96bbdcb7",
+"TT_ethereum-test_signtx.py::test_signtx[erc20_approve]": "c6dee05e3fdec48023936f576915527d61eb90abdaedeb2546bb1f2104f83158",
 "TT_ethereum-test_signtx.py::test_signtx[max_chain_id]": "3723ceb0436e8d7f4669edcf36e72cb1fe65d891f910886fa79e9e406c5e9479",
 "TT_ethereum-test_signtx.py::test_signtx[max_chain_plus_one]": "3723ceb0436e8d7f4669edcf36e72cb1fe65d891f910886fa79e9e406c5e9479",
 "TT_ethereum-test_signtx.py::test_signtx[max_uint64]": "3723ceb0436e8d7f4669edcf36e72cb1fe65d891f910886fa79e9e406c5e9479",
@@ -2768,11 +2769,11 @@
 "TT_ethereum-test_signtx.py::test_signtx_data_pagination[input_flow_go_back]": "8de2c846062d9b028ba23c02d9a53c54ef2fc55d377f084a43bc5d77be56e458",
 "TT_ethereum-test_signtx.py::test_signtx_data_pagination[input_flow_scroll_down]": "ed6c9e088a12ea2e673dbcc99944aa5d6fd6e02e579442d908a3f3e245214145",
 "TT_ethereum-test_signtx.py::test_signtx_data_pagination[input_flow_skip]": "d3c5c78e3b92dc3e32fd8d0c1a4b02ed9d826caf2d0e140c604c50ba39158082",
-"TT_ethereum-test_signtx.py::test_signtx_erc20_approve": "56e917fc22a2d2b817ddf12af634e59076553d161b4ac403a822adf4a672fb70",
+"TT_ethereum-test_signtx.py::test_signtx_erc20_approve": "f4eac59fbc64bfa3d2ada6c0d1bb4300709f7fed918fc61f8a5f50446926b713",
 "TT_ethereum-test_signtx.py::test_signtx_eip1559[Ledger Live legacy path]": "6dafe7c724f262a4fff5448b475ce848d3a74f76cb0ebaf60a5eb83bd20abb82",
 "TT_ethereum-test_signtx.py::test_signtx_eip1559[data_1]": "2eb0129b9fd3c9123920b8f47d3150b0023f4455aa171b76c5e82c17f2747cc9",
 "TT_ethereum-test_signtx.py::test_signtx_eip1559[data_2_bigdata]": "f9fff4dd81fbe5d950a7e699afbb58a8b2e8a87423ea10e25906f21d1c769dd4",
-"TT_ethereum-test_signtx.py::test_signtx_eip1559[erc20]": "6248ae4760cb69b7bb072ab21122e76b6ffe2b62ff5c44f2a61b3dae65937228",
+"TT_ethereum-test_signtx.py::test_signtx_eip1559[erc20]": "abfcba6a2a98041f41de4ec669c5d3f09b99b69df4b1be8e50af4318ee950227",
 "TT_ethereum-test_signtx.py::test_signtx_eip1559[large_chainid]": "2b928f2a7ab544012ec6d5b74aa42898e38b149df7892b7631818401f65abaff",
 "TT_ethereum-test_signtx.py::test_signtx_eip1559[long_fees]": "f3f4f91ae6c9b265b36ef83887c4250a8056302b56830ae3a083eb8ecc893bcb",
 "TT_ethereum-test_signtx.py::test_signtx_eip1559[nodata]": "6dafe7c724f262a4fff5448b475ce848d3a74f76cb0ebaf60a5eb83bd20abb82",