Skip to content

Commit

Permalink
feat(core): Start displaying Ethereum ABI in a generalized way
Browse files Browse the repository at this point in the history 
Step toward solving issue trezor#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]
  • Loading branch information
@cbergqvist
cbergqvist committed Jul 26, 2023
1 parent ea86110 commit 0a1514e
Show file tree
Hide file tree
Showing 7 changed files with 233 additions and 57 deletions.
28 changes: 24 additions & 4 deletions common/tests/fixtures/ethereum/sign_tx.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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,
Expand All @@ -40,8 +60,8 @@
},
"result": {
"sig_v": 38,
"sig_r": "44386ce8c59780b2c7674f347fc891bcb22bd5eddf5b17bf9bc3fc71f06e0561",
"sig_s": "1467aa6e6887f4bef4278caad3499f06c1f8730967d988f760a4f536db7796f3"
"sig_r": "42beac9ee8fd2391c48cb4ad8d8852843772a675693d5146a9eebb5907296f52",
"sig_s": "36680d5acffd5620b71a60815c6daae4b322db7a7e987482be13e67e5610bbab"
}
},
{
Expand Down
17 changes: 10 additions & 7 deletions core/src/apps/ethereum/layout.py
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
from typing import TYPE_CHECKING
from ubinascii import hexlify

from trezor import ui
from trezor.enums import ButtonRequestType
Expand All @@ -13,7 +12,7 @@
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
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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",
Expand Down
198 changes: 167 additions & 31 deletions core/src/apps/ethereum/sign_tx.py
View file
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -91,52 +94,83 @@ async def sign_tx_inner(
keychain: Keychain,
defs: Definitions,
) -> 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 . import tokens
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 len(data_initial_chunk) == 68
and msg.data_length >= FUNCTION_HASH_LENGTH
and len(data_initial_chunk) >= FUNCTION_HASH_LENGTH
):
initial_prefix = data_initial_chunk[:16]
# 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:
token = defs.get_token(address_bytes)
recipient = data_initial_chunk[16:36]
value = int.from_bytes(data_initial_chunk[36:68], "big")

if token is tokens.UNKNOWN_TOKEN:
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)
function_hash = int.from_bytes(data_initial_chunk[:FUNCTION_HASH_LENGTH], "big")

if value:
raise DataError(
"Expecting zero or None as native transaction value when calling smart contract functions"
)
# Force 0 value to None to signify that we are not spending regular ethereum in later dialogs
value = None

confirm_props: list[PropertyType] = []

# 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)
Expand All @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion core/src/apps/ethereum/sign_typed_data.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion tests/device_tests/ethereum/test_signtx.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -412,6 +412,6 @@ def test_signtx_erc20_approve(client: Client):
value=0x0,
tx_type=None,
data=bytes.fromhex(
"095ea7b300000000000000000000000012345678901234567890123456789012345678900000000000000000000000000000000000000000000000056bc75e2d63100000"
"095ea7b3000000000000000000000000c460622c115537f05137c407ad17b06bb115be8b0000000000000000000000000000000000000000000000056bc75e2d63100000"
),
)
Loading

0 comments on commit 0a1514e

Please sign in to comment.