Merge pull request #998 from LedgerHQ/ref/eip712_amount_joins

EIP-712 amount-joins filtering refactoring

Author: apaillier-ledger

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

@@ -191,37 +191,31 @@ def encode_bytes_dyn(value: str, typesize: int) -> bytes:
 encoding_functions[EIP712FieldType.DYN_BYTES] = encode_bytes_dyn
 
 
-def send_filtering_token(token_idx: int):
-    assert token_idx < len(filtering_tokens)
-    if len(filtering_tokens[token_idx]) > 0:
-        token = filtering_tokens[token_idx]
-        if not token["sent"]:
-            response = app_client.provide_token_metadata(token["ticker"],
-                                                         bytes.fromhex(token["addr"][2:]),
-                                                         token["decimals"],
-                                                         token["chain_id"])
-            assert response.status == StatusWord.OK, \
-                f"Error sending token metadata for {token['ticker']}: {response.status}"
-            token["sent"] = True
+def send_all_filtering_tokens(tokens: list[dict]):
+    for token in tokens:
+        response = app_client.provide_token_metadata(token["ticker"],
+                                                     bytes.fromhex(token["addr"][2:]),
+                                                     token["decimals"],
+                                                     token["chain_id"])
+        assert response.status == StatusWord.OK, \
+            f"Error sending token metadata for {token['ticker']}: {response.status}"
 
 
 def send_filter(path: str, discarded: bool) -> Optional[Callable]:
     ret: Optional[Callable] = None
     assert path in filtering_paths.keys()
 
     if filtering_paths[path]["type"].startswith("amount_join_"):
-        if "token" in filtering_paths[path].keys():
-            token_idx = filtering_paths[path]["token"]
-            send_filtering_token(token_idx)
+        if "id" in filtering_paths[path].keys():
+            join_id = filtering_paths[path]["id"]
         else:
             # Permit (ERC-2612)
-            send_filtering_token(0)
-            token_idx = 0xff
+            join_id = 0xff
         if filtering_paths[path]["type"].endswith("_token"):
-            send_filtering_amount_join_token(path, token_idx, discarded)
+            send_filtering_amount_join_token(path, join_id, discarded)
         elif filtering_paths[path]["type"].endswith("_value"):
             send_filtering_amount_join_value(path,
-                                             token_idx,
+                                             join_id,
                                              filtering_paths[path]["name"],
                                              discarded)
     elif filtering_paths[path]["type"] == "datetime":
@@ -366,29 +360,29 @@ def send_filtering_message_info(display_name: str, filters_count: int):
         f"Error sending filtering message info for {display_name}: {response.status}"
 
 
-def send_filtering_amount_join_token(path: str, token_idx: int, discarded: bool):
+def send_filtering_amount_join_token(path: str, join_id: int, discarded: bool):
     to_sign = start_signature_payload(sig_ctx, 11)
     to_sign += path.encode()
-    to_sign.append(token_idx)
+    to_sign.append(join_id)
     sig = keychain.sign_data(keychain.Key.CAL, to_sign)
-    with app_client.eip712_filtering_amount_join_token(token_idx, sig, discarded):
+    with app_client.eip712_filtering_amount_join_token(join_id, sig, discarded):
         pass
     response = app_client.response()
     assert response.status == StatusWord.OK, \
-        f"Error sending filtering amount join token for {path} with token index {token_idx}: {response.status}"
+        f"Error sending filtering amount join token for {path} with token index {join_id}: {response.status}"
 
 
-def send_filtering_amount_join_value(path: str, token_idx: int, display_name: str, discarded: bool):
+def send_filtering_amount_join_value(path: str, join_id: int, display_name: str, discarded: bool):
     to_sign = start_signature_payload(sig_ctx, 22)
     to_sign += path.encode()
     to_sign += display_name.encode()
-    to_sign.append(token_idx)
+    to_sign.append(join_id)
     sig = keychain.sign_data(keychain.Key.CAL, to_sign)
-    with app_client.eip712_filtering_amount_join_value(token_idx, display_name, sig, discarded):
+    with app_client.eip712_filtering_amount_join_value(join_id, display_name, sig, discarded):
         pass
     response = app_client.response()
     assert response.status == StatusWord.OK, \
-        f"Error sending filtering amount join value for {path} with token index {token_idx}: {response.status}"
+        f"Error sending filtering amount join value for {path} with token index {join_id}: {response.status}"
 
 
 def send_filtering_datetime(path: str, display_name: str, discarded: bool):
@@ -536,9 +530,6 @@ def prepare_filtering(data_json, filtr_data):
 
     if "tokens" in filtr_data:
         filtering_tokens = filtr_data["tokens"]
-        for token in filtering_tokens:
-            if len(token) > 0:
-                token["sent"] = False
     else:
         filtering_tokens = []
 
@@ -627,6 +618,7 @@ def process_data(aclient: EthAppClient,
         assert response.status == StatusWord.OK, \
             f"Error activating filtering: {response.status}"
         prepare_filtering(data_json, filters)
+        send_all_filtering_tokens(filtering_tokens)
 
     # Send ledgerPKI certificate
     app_client.pki_client.send_certificate(PKIPubKeyUsage.PUBKEY_USAGE_COIN_META)

doc/ethapp.adoc

@@ -850,18 +850,18 @@ The first byte is used as a magic number so that a signature of one type cannot
 
 The signature is computed on :
 
-11 || chain ID (BE) || contract address || schema hash || field path || token index
+11 || chain ID (BE) || contract address || schema hash || field path || join identifier
 
 ##### Amount-join value
 
 This command should come before the corresponding *SEND STRUCT IMPLEMENTATION* and are only usable for message fields (and not domain ones).
 The first byte is used as a magic number so that a signature of one type cannot be valid as another type.
 
-A token index of 0xFF indicates the token address is in the _verifyingContract_ field of the EIP712Domain so the app won't receive an amount-join token filtering APDU. This enables support for Permit (ERC-2612) messages.
+A join identifier of 0xFF indicates the token address is in the _verifyingContract_ field of the EIP712Domain so the app won't receive an amount-join token filtering APDU. This enables support for Permit (ERC-2612) messages.
 
 The signature is computed on :
 
-22 || chain ID (BE) || contract address || schema hash || field path || display name || token index
+22 || chain ID (BE) || contract address || schema hash || field path || display name || join identifier
 
 ##### Date / Time
 
@@ -1145,7 +1145,7 @@ enum CalldataParamPresence
 [width="80%"]
 |==========================================
 | *Description*         | *Length (byte)*
-| Token index           | 1
+| Join identifier       | 1
 | Signature length      | 1
 | Signature             | variable
 |==========================================
@@ -1157,7 +1157,7 @@ enum CalldataParamPresence
 | *Description*         | *Length (byte)*
 | Display name length   | 1
 | Display name          | variable
-| Token index           | 1
+| Join identifier       | 1
 | Signature length      | 1
 | Signature             | variable
 |==========================================

src/features/sign_message_eip712/filtering.c

@@ -145,20 +145,6 @@ static bool sig_verif_end(cx_sha256_t *hash_ctx, const uint8_t *sig, uint8_t sig
     return true;
 }
 
-/**
- * Check if the given token index is valid
- *
- * @param[in] idx token index
- * @return whether the index is valid or not
- */
-static bool check_token_index(uint8_t idx) {
-    if (idx >= MAX_ASSETS) {
-        PRINTF("Error: token index out of range (%u)\n", idx);
-        return false;
-    }
-    return true;
-}
-
 /**
  * Check if the current element's typename matches the expected one
  *
@@ -1038,7 +1024,7 @@ bool filtering_amount_join_token(const uint8_t *payload,
                                  uint8_t length,
                                  bool discarded,
                                  uint32_t *path_crc) {
-    uint8_t token_idx;
+    uint8_t join_id;
     uint8_t sig_len;
     const uint8_t *sig;
     uint8_t offset = 0;
@@ -1049,10 +1035,10 @@ bool filtering_amount_join_token(const uint8_t *payload,
     }
 
     // Parsing
-    if ((offset + sizeof(token_idx)) > length) {
+    if ((offset + sizeof(join_id)) > length) {
         return false;
     }
-    token_idx = payload[offset++];
+    join_id = payload[offset++];
     if ((offset + sizeof(sig_len)) > length) {
         return false;
     }
@@ -1068,17 +1054,17 @@ bool filtering_amount_join_token(const uint8_t *payload,
         return false;
     }
     hash_filtering_path((cx_hash_t *) &hash_ctx, discarded, path_crc);
-    hash_byte(token_idx, (cx_hash_t *) &hash_ctx);
+    hash_byte(join_id, (cx_hash_t *) &hash_ctx);
     if (!sig_verif_end(&hash_ctx, sig, sig_len)) {
         return false;
     }
 
     // Handling
-    if (!check_typename("address") || !check_token_index(token_idx)) {
+    if (!check_typename("address")) {
         return false;
     }
     ui_712_flag_field(false, false, true, false, false, false);
-    ui_712_token_join_prepare_addr_check(token_idx);
+    ui_712_token_join_prepare_addr_check(join_id);
     return true;
 }
 
@@ -1097,7 +1083,7 @@ bool filtering_amount_join_value(const uint8_t *payload,
                                  uint32_t *path_crc) {
     uint8_t name_len;
     const char *name;
-    uint8_t token_idx;
+    uint8_t join_id;
     uint8_t sig_len;
     const uint8_t *sig;
     uint8_t offset = 0;
@@ -1120,10 +1106,10 @@ bool filtering_amount_join_value(const uint8_t *payload,
     }
     name = (char *) &payload[offset];
     offset += name_len;
-    if ((offset + sizeof(token_idx)) > length) {
+    if ((offset + sizeof(join_id)) > length) {
         return false;
     }
-    token_idx = payload[offset++];
+    join_id = payload[offset++];
     if ((offset + sizeof(sig_len)) > length) {
         return false;
     }
@@ -1140,32 +1126,25 @@ bool filtering_amount_join_value(const uint8_t *payload,
     }
     hash_filtering_path((cx_hash_t *) &hash_ctx, discarded, path_crc);
     hash_nbytes((uint8_t *) name, sizeof(char) * name_len, (cx_hash_t *) &hash_ctx);
-    hash_byte(token_idx, (cx_hash_t *) &hash_ctx);
+    hash_byte(join_id, (cx_hash_t *) &hash_ctx);
     if (!sig_verif_end(&hash_ctx, sig, sig_len)) {
         return false;
     }
 
     // Handling
-    if (token_idx == TOKEN_IDX_ADDR_IN_DOMAIN) {
+    if (join_id == TOKEN_IDX_ADDR_IN_DOMAIN) {
         // Permit (ERC-2612)
-        int resolved_idx = get_asset_index_by_addr(eip712_context->contract_addr);
-
-        if (resolved_idx == -1) {
-            PRINTF("ERROR: Could not find asset info for verifyingContract address!\n");
-            return false;
-        }
-        token_idx = (uint8_t) resolved_idx;
+        ui_712_token_join_prepare_addr_check(join_id);
         // simulate as if we had received a token-join addr
-        ui_712_token_join_prepare_addr_check(token_idx);
-        if (!amount_join_set_token_received()) {
+        if (!ui_712_set_amount_join_token_addr(eip712_context->contract_addr)) {
             return false;
         }
     }
-    if (!check_typename("uint") || !check_token_index(token_idx)) {
+    if (!check_typename("uint")) {
         return false;
     }
     ui_712_flag_field(false, false, true, false, false, false);
-    return ui_712_token_join_prepare_amount(token_idx, name, name_len);
+    return ui_712_token_join_prepare_amount(join_id, name, name_len);
 }
 
 /**