Merge pull request #559 from LedgerHQ/fix/apa/eip712_empty_array_handling

EIP-712 empty array handling fix

client/CHANGELOG.md

@@ -5,13 +5,17 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
-## [0.4.1] - 2024-04-05
+## [0.4.1] - 2024-04-15
 
 ### Added
 
 - Add new function `send_raw`, allowing to send a raw payload APDU
 - Add new error code definition
 
+### Fixed
+
+- Encoding of EIP-712 bytes elements
+
 ## [0.4.0] - 2024-04-03
 
 ### Added

client/src/ledger_app_clients/ethereum/eip712/InputData.py

@@ -4,7 +4,8 @@ import re
 import signal
 import sys
 import copy
-from typing import Any, Callable, Optional
+from typing import Any, Callable, Optional, Union
+import struct
 
 from client import keychain
 from client.client import EthAppClient, EIP712FieldType
@@ -118,69 +119,63 @@ def send_struct_def_field(typename, keyname):
     return (typename, type_enum, typesize, array_lvls)
 
 
-def encode_integer(value, typesize):
-    data = bytearray()
-
+def encode_integer(value: Union[str | int], typesize: int) -> bytes:
     # Some are already represented as integers in the JSON, but most as strings
     if isinstance(value, str):
-        base = 10
-        if value.startswith("0x"):
-            base = 16
-        value = int(value, base)
+        value = int(value, 0)
 
     if value == 0:
-        data.append(0)
+        data = b'\x00'
     else:
-        if value < 0:  # negative number, send it as unsigned
-            mask = 0
-            for i in range(typesize):  # make a mask as big as the typesize
-                mask = (mask << 8) | 0xff
-            value &= mask
-        while value > 0:
-            data.append(value & 0xff)
-            value >>= 8
-        data.reverse()
+        # biggest uint type accepted by struct.pack
+        uint64_mask = 0xffffffffffffffff
+        data = struct.pack(">QQQQ",
+                           (value >> 192) & uint64_mask,
+                           (value >> 128) & uint64_mask,
+                           (value >> 64) & uint64_mask,
+                           value & uint64_mask)
+        data = data[len(data) - typesize:]
+        data = data.lstrip(b'\x00')
     return data
 
 
-def encode_int(value, typesize):
+def encode_int(value: str, typesize: int) -> bytes:
     return encode_integer(value, typesize)
 
 
-def encode_uint(value, typesize):
+def encode_uint(value: str, typesize: int) -> bytes:
     return encode_integer(value, typesize)
 
 
-def encode_hex_string(value, size):
-    data = bytearray()
-    value = value[2:]  # skip 0x
-    byte_idx = 0
-    while byte_idx < size:
-        data.append(int(value[(byte_idx * 2):(byte_idx * 2 + 2)], 16))
-        byte_idx += 1
-    return data
+def encode_hex_string(value: str, size: int) -> bytes:
+    assert value.startswith("0x")
+    value = value[2:]
+    if len(value) < (size * 2):
+        value = value.rjust(size * 2, "0")
+    assert len(value) == (size * 2)
+    return bytes.fromhex(value)
 
 
-def encode_address(value, typesize):
+def encode_address(value: str, typesize: int) -> bytes:
     return encode_hex_string(value, 20)
 
 
-def encode_bool(value, typesize):
-    return encode_integer(value, typesize)
+def encode_bool(value: str, typesize: int) -> bytes:
+    return encode_integer(value, 1)
 
 
-def encode_string(value, typesize):
+def encode_string(value: str, typesize: int) -> bytes:
     data = bytearray()
     for char in value:
         data.append(ord(char))
     return data
 
 
-def encode_bytes_fix(value, typesize):
+def encode_bytes_fix(value: str, typesize: int) -> bytes:
     return encode_hex_string(value, typesize)
 
 
-def encode_bytes_dyn(value, typesize):
+def encode_bytes_dyn(value: str, typesize: int) -> bytes:
     # length of the value string
     # - the length of 0x (2)
     # / by the length of one byte in a hex string (2)

src_features/signMessageEIP712/field_hash.c

@@ -228,7 +228,7 @@ static bool field_hash_finalize(const void *const field_ptr,
             return false;
         }
     }
-    path_advance();
+    path_advance(true);
     fh->state = FHS_IDLE;
     ui_712_finalize_field();
     return true;

src_features/signMessageEIP712/path.c

@@ -255,6 +255,7 @@ static bool array_depth_list_push(uint8_t path_idx, uint8_t size) {
     arr = &path_struct->array_depths[path_struct->array_depth_count];
     arr->path_index = path_idx;
     arr->size = size;
+    arr->index = 0;
     path_struct->array_depth_count += 1;
     return true;
 }
@@ -285,11 +286,14 @@ static bool array_depth_list_pop(void) {
  * Updates the path so that it doesn't point to a struct-type field, but rather
  * only to actual fields.
  *
+ * @param[in] skip_if_array skip if path is already pointing at an array
+ * @param[in] stop_at_array stop at the first downstream array
  * @return whether the path update worked or not
  */
-static bool path_update(void) {
+static bool path_update(bool skip_if_array, bool stop_at_array) {
     uint8_t fields_count;
     const void *struct_ptr;
+    const void *starting_field_ptr;
     const void *field_ptr;
     const char *typename;
     uint8_t typename_len;
@@ -298,10 +302,20 @@ static bool path_update(void) {
     if (path_struct == NULL) {
         return false;
     }
-    if ((field_ptr = get_field(NULL)) == NULL) {
+    if ((starting_field_ptr = get_field(NULL)) == NULL) {
         return false;
     }
+    field_ptr = starting_field_ptr;
     while (struct_field_type(field_ptr) == TYPE_CUSTOM) {
+        // check if we meet one of the given conditions
+        if (((field_ptr == starting_field_ptr) && skip_if_array) ||
+            ((field_ptr != starting_field_ptr) && stop_at_array)) {
+            // only if it is the first iteration of that array depth
+            if ((path_struct->array_depths[path_struct->array_depth_count - 1].index == 0) &&
+                struct_field_is_array(field_ptr)) {
+                break;
+            }
+        }
         typename = get_struct_field_typename(field_ptr, &typename_len);
         if ((struct_ptr = get_structn(typename, typename_len)) == NULL) {
             return false;
@@ -313,11 +327,16 @@ static bool path_update(void) {
         if (push_new_hash_depth(true) == false) {
             return false;
         }
-        // get the struct typehash
-        if (type_hash(typename, typename_len, hash) == false) {
-            return false;
+
+        // The only times they are both at false is if we are traversing an empty array,
+        // don't do a typehash in that case
+        if ((skip_if_array != false) || (stop_at_array != false)) {
+            // get the struct typehash
+            if (type_hash(typename, typename_len, hash) == false) {
+                return false;
+            }
+            feed_last_hash_depth(hash);
         }
-        feed_last_hash_depth(hash);
 
         // TODO: Find a better way to show inner structs in verbose mode when it might be
         //       an empty array of structs in which case we don't want to show it but the
@@ -381,7 +400,7 @@ bool path_set_root(const char *const struct_name, uint8_t name_length) {
     struct_state = DEFINED;
 
     // because the first field could be a struct type
-    path_update();
+    path_update(true, true);
     return true;
 }
 
@@ -449,6 +468,9 @@ bool path_new_array_depth(const uint8_t *const data, uint8_t length) {
     }
 
     array_size = *data;
+    if (!path_update(false, array_size > 0)) {
+        return false;
+    }
     array_depth_count_bak = path_struct->array_depth_count;
     for (pidx = 0; pidx < path_struct->depth_count; ++pidx) {
         if ((field_ptr = get_nth_field(NULL, pidx + 1)) == NULL) {
@@ -492,8 +514,8 @@ bool path_new_array_depth(const uint8_t *const data, uint8_t length) {
     }
     if (array_size == 0) {
         do {
-            path_advance();
-        } while (path_struct->array_depth_count != array_depth_count_bak);
+            path_advance(false);
+        } while (path_struct->array_depth_count > array_depth_count_bak);
     }
 
     return true;
@@ -546,8 +568,8 @@ static bool path_advance_in_array(void) {
 
         if ((path_struct->array_depth_count > 0) &&
             (arr_depth->path_index == (path_struct->depth_count - 1))) {
-            if (arr_depth->size > 0) arr_depth->size -= 1;
-            if (arr_depth->size == 0) {
+            arr_depth->index += 1;
+            if (arr_depth->index == arr_depth->size) {
                 array_depth_list_pop();
                 end_reached = true;
             } else {
@@ -563,7 +585,7 @@ static bool path_advance_in_array(void) {
  *
  * @return whether the advancement was successful or not
  */
-bool path_advance(void) {
+bool path_advance(bool array_check) {
     bool end_reached;
 
     do {
@@ -573,8 +595,7 @@ bool path_advance(void) {
             end_reached = false;
         }
     } while (end_reached);
-    path_update();
-    return true;
+    return path_update(array_check, array_check);
 }
 
 /**

src_features/signMessageEIP712/path.h

@@ -12,6 +12,7 @@
 typedef struct {
     uint8_t path_index;
     uint8_t size;
+    uint8_t index;
 } s_array_depth;
 
 typedef enum { ROOT_DOMAIN, ROOT_MESSAGE } e_root_type;
@@ -27,7 +28,7 @@ typedef struct {
 
 bool path_set_root(const char *const struct_name, uint8_t length);
 const void *path_get_field(void);
-bool path_advance(void);
+bool path_advance(bool array_check);
 bool path_init(void);
 void path_deinit(void);
 bool path_new_array_depth(const uint8_t *const data, uint8_t length);

tests/ragger/eip712_input_files/00-simple_mail.ini

@@ -1,4 +0,0 @@
-[signature]
-v = 1b
-r = 23599abd6c4b631e42770c112b5955907fe91339f1ea1e102f7682262ca178b9
-s = 29fc94518588165114b4c4acb4d73e6d028dfb051d90e517b3b4746e04eb0f5f

tests/ragger/eip712_input_files/01-addresses_array_mail.ini

@@ -1,4 +0,0 @@
-[signature]
-v = 1c
-r = 3f084a471e6158bce792287500d62d40061acc1864180ed2da7a704bf3aced0f
-s = 3b799ced9e48cda152b4b9a4b7f45e3119dc7acdf16710a73800b4e336fa1b40

tests/ragger/eip712_input_files/02-recipients_array_mail-data.json

@@ -38,10 +38,6 @@
             { "name": "chainId", "type": "uint256" },
             { "name": "verifyingContract", "type": "address" }
         ],
-        "Group": [
-          { "name": "name", "type": "string" },
-          { "name": "members", "type": "Person[]" }
-        ],
         "Mail": [
             { "name": "from", "type": "Person" },
             { "name": "to", "type": "Person[]" },

tests/ragger/eip712_input_files/02-recipients_array_mail.ini

@@ -1,4 +0,0 @@
-[signature]
-v = 1b
-r = 49dd2aa96d7494e0cd9111f19f87ac50194e4bbc61ea9f4bb86d674da0ae7721
-s = 7a12ddd9083b4caaabd2fb80df6de1d5d926c0e8a73bf371a45e231d409d79d6

tests/ragger/eip712_input_files/03-long_string-data.json

@@ -33,10 +33,6 @@
             { "name": "chainId", "type": "uint256" },
             { "name": "verifyingContract", "type": "address" }
         ],
-        "Group": [
-          { "name": "name", "type": "string" },
-          { "name": "members", "type": "Person[]" }
-        ],
         "Mail": [
             { "name": "from", "type": "Person" },
             { "name": "to", "type": "Person[]" },

tests/ragger/eip712_input_files/03-long_string.ini

@@ -1,4 +0,0 @@
-[signature]
-v = 1c
-r = b23ffac2cb350fd6e7d06ec4b981fe016d33426d753c870e7e753797cc43bb1f
-s = 37948a656fa3403e21956ef10c8d3152f7ce22cc252d958c9f9249435090f426

tests/ragger/eip712_input_files/04-long_bytes-data.json

@@ -33,10 +33,6 @@
             { "name": "chainId", "type": "uint256" },
             { "name": "verifyingContract", "type": "address" }
         ],
-        "Group": [
-          { "name": "name", "type": "string" },
-          { "name": "members", "type": "Person[]" }
-        ],
         "Mail": [
             { "name": "from", "type": "Person" },
             { "name": "to", "type": "Person[]" },

tests/ragger/eip712_input_files/04-long_bytes.ini

@@ -1,4 +0,0 @@
-[signature]
-v = 1b
-r = db18ea1b9757773385138d0802fb2f8107c3e45882962b8e0c6789eccdbfab05
-s = 3d66d4dee47916fb7fec39a538ad8d5e94fbc92f99327410716180ab07591218

tests/ragger/eip712_input_files/05-signed_ints.ini

@@ -1,4 +0,0 @@
-[signature]
-v = 1c
-r = 50fb2861367daf3b5b73cac277e698b27bf7627a462fade1acb5a2ef285ba8ae
-s = 131a62515a0a5c4b35c5cb672b46f562151c45508d8efcdf78c4608bc14c5f30

tests/ragger/eip712_input_files/06-boolean.ini

@@ -1,4 +0,0 @@
-[signature]
-v = 1c
-r = 929681d77ed88cd1adef57185a0cd7b3a268aca5d4122b8c0acfd2ce4c0afb18
-s = 5afe8e3004c182b6b02fe7559c26f20f4133ad9b17223658ccd9061b33b021cf

tests/ragger/eip712_input_files/07-fixed_bytes.ini

@@ -1,4 +0,0 @@
-[signature]
-v = 1b
-r = e021d88afc50079b0341b01193c4687c47b85bcd6749fe69e0b87521d65a1847
-s = 5b7670d2a67c781a11164920403db0f7707161e81d88226cdbf91298390dfeda

tests/ragger/eip712_input_files/08-opensea.ini

@@ -1,4 +0,0 @@
-[signature]
-v = 1b
-r = 1539547ae7cf8ebcd3eabfb57cd2b1fb7775ce757c3f4a307c7425d35b7bfff7
-s = 47248cb61e554c1f90af6331d9c9e51cbb8655667514194f509abe097a032319

tests/ragger/eip712_input_files/09-rarible.ini

@@ -1,4 +0,0 @@
-[signature]
-v = 1c
-r = 341bca1c0dfd805d4befc21500084424dbe559c7aafd78d8fb461c0c76dfea1d
-s = 33ebb7b6fe0691961cd8b263faac20ecbbdcaef3febb57eb76614cad629080ea

tests/ragger/eip712_input_files/10-multidimensional_arrays.ini

@@ -1,4 +0,0 @@
-[signature]
-v = 1b
-r = d11a91bdf7836288818875d046452061d565cc6dc1bf3dd6216ab27ef9a2844f
-s = 4f6bda8ac4c39721aff7ae08989897ede9d573085a192d03ab0eb7735d2ef403

tests/ragger/eip712_input_files/11-complex_structs.ini

@@ -1,4 +0,0 @@
-[signature]
-v = 1c
-r = cce2e63aaac6a5f9a74684d8fdddcbc7f3b27aa17235bfab89226821ead933b6
-s = 3f3c93977abcc3f8cc9a3dc1ecc02dbca14aca1a6ecb2fb6ca3d7c713ace1ec4

tests/ragger/eip712_input_files/12-sign_in.ini

@@ -1,4 +0,0 @@
-[signature]
-v = 1b
-r = 7be1671577753c13bfd1da8b234b6df8484daf47351c2366637fd291dd4aa4d9
-s = 1a7ffbb01dc8a64e9ee97d19b8f154e9eecbe0b1bfb9dcfa781a65e474573963

tests/ragger/eip712_input_files/13-empty_arrays-data.json

@@ -6,19 +6,36 @@
         "version": "1"
     },
     "message": {
-        "list1": [],
-        "list2": [],
-        "list3": [
+        "test1": [],
+        "test2": [
             [
-                "1",
-                "2"
+                "one",
+                "two"
             ],
             [],
             [
-                "3",
-                "4"
+                "three",
+                "four"
             ]
-        ]
+        ],
+        "test3": [
+            {
+                "sub": [
+                    {
+                        "sub": [],
+                        "value": 3
+                    }
+                ],
+                "value": 1
+            }
+        ],
+        "test4": [
+            {
+                "sub": [],
+                "value": 2
+            }
+        ],
+        "test5": []
     },
     "primaryType": "Struct",
     "types": {
@@ -29,9 +46,22 @@
             { "name": "verifyingContract", "type": "address" }
         ],
         "Struct": [
-            { "name": "list1", "type": "EIP712Domain[]" },
-            { "name": "list2", "type": "uint8[]" },
-            { "name": "list3", "type": "string[][]" }
+            { "name": "test1", "type": "uint8[]" },
+            { "name": "test2", "type": "string[][]" },
+            { "name": "test3", "type": "Top[]" },
+            { "name": "test4", "type": "Top[]" },
+            { "name": "test5", "type": "Top[]" }
+        ],
+        "Bottom": [
+            { "name": "value", "type": "uint8" }
+        ],
+        "Mid": [
+            { "name": "sub", "type": "Bottom[]" },
+            { "name": "value", "type": "uint8" }
+        ],
+        "Top": [
+            { "name": "sub", "type": "Mid[]" },
+            { "name": "value", "type": "uint8" }
         ]
     }
 }

tests/ragger/eip712_input_files/13-empty_arrays.ini

@@ -1,4 +0,0 @@
-[signature]
-v = 1b
-r = 5d0635a868602e29366da6328f8fadf2d6a9b4e69ee7a65928e85ca56fb1b515
-s = 257364d6faaf5687edf90c3984f4240b0ce7b2dee55aa1f8f39c32d0d4d8c93d

tests/ragger/eip712_input_files/14-rabby_bug-data.json

@@ -0,0 +1,61 @@
+{
+    "types": {
+        "BatchSignedERC721Orders": [
+            { "type": "address", "name": "maker" },
+            { "type": "uint256", "name": "listingTime" },
+            { "type": "uint256", "name": "expiryTime" },
+            { "type": "uint256", "name": "startNonce" },
+            { "type": "address", "name": "erc20Token" },
+            { "type": "address", "name": "platformFeeRecipient" },
+            { "type": "BasicCollection[]", "name": "basicCollections" },
+            { "type": "Collection[]", "name": "collections" },
+            { "type": "uint256", "name": "hashNonce" }
+        ],
+        "BasicCollection": [
+            { "type": "address", "name": "nftAddress" },
+            { "type": "bytes32", "name": "fee" },
+            { "type": "bytes32[]", "name": "items" }
+        ],
+        "Collection": [
+            { "type": "address", "name": "nftAddress" },
+            { "type": "bytes32", "name": "fee" },
+            { "type": "OrderItem[]", "name": "items" }
+        ],
+        "OrderItem": [
+            { "type": "uint256", "name": "erc20TokenAmount" },
+            { "type": "uint256", "name": "nftId" }
+        ],
+        "EIP712Domain": [
+            { "name": "name", "type": "string" },
+            { "name": "version", "type": "string" },
+            { "name": "chainId", "type": "uint256" },
+            { "name": "verifyingContract", "type": "address" }
+        ]
+    },
+    "domain": {
+        "name": "ElementEx",
+        "version": "1.0.0",
+        "chainId": "5000",
+        "verifyingContract": "0x2fa13cf695ec51ded5b8e45ad0bef838ab17e2af"
+    },
+    "primaryType": "BatchSignedERC721Orders",
+    "message": {
+        "maker": "0x6d3b90747dbf5883bf88ff7eb5fcc86f408b5409",
+        "listingTime": "1706688449",
+        "expiryTime": "1709280466",
+        "startNonce": "7",
+        "erc20Token": "0x09bc4e0d864854c6afb6eb9a9cdf58ac190d0df9",
+        "platformFeeRecipient": "0x07538262ae993ca117a0e481f908209137a4626e",
+        "basicCollections": [
+            {
+                "nftAddress": "0xaaaea1fb9f3de3f70e89f37b69ab11b47eb9ce6f",
+                "fee": "0x000000000000000000c80000000000000000000000000000000000000000000",
+                "items": [
+                    "0x000000000000000020c8558000000000000000000000000000000000000005d"
+                ]
+            }
+        ],
+        "collections": [],
+        "hashNonce": "0"
+    }
+}

tests/ragger/test_eip712.py

@@ -1,12 +1,12 @@
 import fnmatch
 import os
 import time
-from configparser import ConfigParser
 from functools import partial
 from pathlib import Path
 import json
 from typing import Optional
 import pytest
+from eth_account.messages import encode_typed_data
 
 import client.response_parser as ResponseParser
 from client.utils import recover_message
@@ -29,7 +29,8 @@ class SnapshotsConfig:
 
 
 BIP32_PATH = "m/44'/60'/0'/0/0"
-snaps_config: Optional[SnapshotsConfig] = None
+SNAPS_CONFIG: Optional[SnapshotsConfig] = None
+WALLET_ADDR: Optional[bytes] = None
 
 
 def eip712_json_path() -> str:
@@ -59,33 +60,44 @@ def filtering_fixture(request) -> bool:
     return request.param
 
 
+def get_wallet_addr(client: EthAppClient) -> bytes:
+    global WALLET_ADDR
+
+    # don't ask again if we already have it
+    if WALLET_ADDR is None:
+        with client.get_public_addr(display=False):
+            pass
+        _, WALLET_ADDR, _ = ResponseParser.pk_addr(client.response().data)
+    return WALLET_ADDR
+
+
 def test_eip712_legacy(firmware: Firmware,
                        backend: BackendInterface,
                        navigator: Navigator):
     app_client = EthAppClient(backend)
-    with app_client.eip712_sign_legacy(
-            BIP32_PATH,
-            bytes.fromhex('6137beb405d9ff777172aa879e33edb34a1460e701802746c5ef96e741710e59'),
-            bytes.fromhex('eb4221181ff3f1a83ea7313993ca9218496e424604ba9492bb4052c03d5c3df8')):
-        moves = []
-        if firmware.device.startswith("nano"):
-            moves += [NavInsID.RIGHT_CLICK]
-            if firmware.device == "nanos":
-                screens_per_hash = 4
+
+    with open(input_files()[0]) as file:
+        data = json.load(file)
+        smsg = encode_typed_data(full_message=data)
+        with app_client.eip712_sign_legacy(BIP32_PATH, smsg.header, smsg.body):
+            moves = []
+            if firmware.device.startswith("nano"):
+                moves += [NavInsID.RIGHT_CLICK]
+                if firmware.device == "nanos":
+                    screens_per_hash = 4
+                else:
+                    screens_per_hash = 2
+                moves += [NavInsID.RIGHT_CLICK] * screens_per_hash * 2
+                moves += [NavInsID.BOTH_CLICK]
             else:
-                screens_per_hash = 2
-            moves += [NavInsID.RIGHT_CLICK] * screens_per_hash * 2
-            moves += [NavInsID.BOTH_CLICK]
-        else:
-            moves += [NavInsID.USE_CASE_REVIEW_TAP] * 2
-            moves += [NavInsID.USE_CASE_REVIEW_CONFIRM]
-        navigator.navigate(moves)
+                moves += [NavInsID.USE_CASE_REVIEW_TAP] * 2
+                moves += [NavInsID.USE_CASE_REVIEW_CONFIRM]
+            navigator.navigate(moves)
 
-    v, r, s = ResponseParser.signature(app_client.response().data)
+        vrs = ResponseParser.signature(app_client.response().data)
+        recovered_addr = recover_message(data, vrs)
 
-    assert v == bytes.fromhex("1c")
-    assert r == bytes.fromhex("ea66f747173762715751c889fea8722acac3fc35db2c226d37a2e58815398f64")
-    assert s == bytes.fromhex("52d8ba9153de9255da220ffd36762c0b027701a3b5110f0a765f94b16a9dfb55")
+    assert recovered_addr == get_wallet_addr(app_client)
 
 
 def autonext(firmware: Firmware, navigator: Navigator, default_screenshot_path: Path):
@@ -94,18 +106,18 @@ def autonext(firmware: Firmware, navigator: Navigator, default_screenshot_path:
         moves = [NavInsID.RIGHT_CLICK]
     else:
         moves = [NavInsID.USE_CASE_REVIEW_TAP]
-    if snaps_config is not None:
+    if SNAPS_CONFIG is not None:
         navigator.navigate_and_compare(default_screenshot_path,
-                                       snaps_config.test_name,
+                                       SNAPS_CONFIG.test_name,
                                        moves,
                                        screen_change_before_first_instruction=False,
                                        screen_change_after_last_instruction=False,
-                                       snap_start_idx=snaps_config.idx)
-        snaps_config.idx += 1
+                                       snap_start_idx=SNAPS_CONFIG.idx)
+        SNAPS_CONFIG.idx += 1
     else:
         navigator.navigate(moves,
-                     screen_change_before_first_instruction=False,
-                     screen_change_after_last_instruction=False)
+                           screen_change_before_first_instruction=False,
+                           screen_change_after_last_instruction=False)
 
 
 def eip712_new_common(firmware: Firmware,
@@ -132,12 +144,12 @@ def eip712_new_common(firmware: Firmware,
             if not verbose and filters is None:
                 moves += [NavInsID.USE_CASE_REVIEW_TAP]
             moves += [NavInsID.USE_CASE_REVIEW_CONFIRM]
-        if snaps_config is not None:
+        if SNAPS_CONFIG is not None:
             navigator.navigate_and_compare(default_screenshot_path,
-                                     snaps_config.test_name,
-                                     moves,
-                                     snap_start_idx=snaps_config.idx)
-            snaps_config.idx += 1
+                                           SNAPS_CONFIG.test_name,
+                                           moves,
+                                           snap_start_idx=SNAPS_CONFIG.idx)
+            SNAPS_CONFIG.idx += 1
         else:
             navigator.navigate(moves)
     return ResponseParser.signature(app_client.response().data)
@@ -155,7 +167,6 @@ def test_eip712_new(firmware: Firmware,
         pytest.skip("Not supported on LNS")
 
     test_path = f"{input_file.parent}/{'-'.join(input_file.stem.split('-')[:-1])}"
-    conf_file = f"{test_path}.ini"
 
     filters = None
     if filtering:
@@ -166,51 +177,39 @@ def test_eip712_new(firmware: Firmware,
         except (IOError, json.decoder.JSONDecodeError) as e:
             pytest.skip(f"{filterfile.name}: {e.strerror}")
 
-    config = ConfigParser()
-    config.read(conf_file)
-
-    # sanity check
-    assert "signature" in config.sections()
-    assert "v" in config["signature"]
-    assert "r" in config["signature"]
-    assert "s" in config["signature"]
-
     if verbose:
         settings_toggle(firmware, navigator, [SettingID.VERBOSE_EIP712])
 
     with open(input_file, encoding="utf-8") as file:
-        v, r, s = eip712_new_common(firmware,
-                                    navigator,
-                                    default_screenshot_path,
-                                    app_client,
-                                    json.load(file),
-                                    filters,
-                                    verbose)
+        data = json.load(file)
+        vrs = eip712_new_common(firmware,
+                                navigator,
+                                default_screenshot_path,
+                                app_client,
+                                data,
+                                filters,
+                                verbose)
 
-    assert v == bytes.fromhex(config["signature"]["v"])
-    assert r == bytes.fromhex(config["signature"]["r"])
-    assert s == bytes.fromhex(config["signature"]["s"])
+        recovered_addr = recover_message(data, vrs)
+
+    assert recovered_addr == get_wallet_addr(app_client)
 
 
 def test_eip712_address_substitution(firmware: Firmware,
                                      backend: BackendInterface,
                                      navigator: Navigator,
                                      default_screenshot_path: Path,
+                                     test_name: str,
                                      verbose: bool):
-    global snaps_config
+    global SNAPS_CONFIG
 
     app_client = EthAppClient(backend)
     if firmware.device == "nanos":
         pytest.skip("Not supported on LNS")
 
-    with app_client.get_public_addr(display=False):
-        pass
-    _, DEVICE_ADDR, _ = ResponseParser.pk_addr(app_client.response().data)
-
-    test_name = "eip712_address_substitution"
     if verbose:
         test_name += "_verbose"
-    snaps_config = SnapshotsConfig(test_name)
+    SNAPS_CONFIG = SnapshotsConfig(test_name)
     with open(f"{eip712_json_path()}/address_substitution.json", encoding="utf-8") as file:
         data = json.load(file)
 
@@ -247,4 +246,4 @@ def test_eip712_address_substitution(firmware: Firmware,
 
     # verify signature
     addr = recover_message(data, vrs)
-    assert addr == DEVICE_ADDR
+    assert addr == get_wallet_addr(app_client)