Merge pull request #639 from LedgerHQ/fix/apa/eip712_amount_join_unkn…

…own_token

EIP-712 amount-join support with unknown token

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

@@ -17,7 +17,8 @@
 # global variables
 app_client: EthAppClient = None
 filtering_paths: dict = {}
-current_path: list[str] = list()
+filtering_tokens: list[dict] = []
+current_path: list[str] = []
 sig_ctx: dict[str, Any] = {}
 
 
@@ -194,18 +195,36 @@ 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"]:
+            app_client.provide_token_metadata(token["ticker"],
+                                              bytes.fromhex(token["addr"][2:]),
+                                              token["decimals"],
+                                              token["chain_id"])
+            token["sent"] = True
+
+
 def send_filter(path: str, discarded: bool):
     assert path in filtering_paths.keys()
 
-    if filtering_paths[path]["type"] == "amount_join_token":
-        send_filtering_amount_join_token(path, filtering_paths[path]["token"], discarded)
-    elif filtering_paths[path]["type"] == "amount_join_value":
+    if filtering_paths[path]["type"].startswith("amount_join_"):
         if "token" in filtering_paths[path].keys():
-            token = filtering_paths[path]["token"]
+            token_idx = filtering_paths[path]["token"]
+            send_filtering_token(token_idx)
         else:
             # Permit (ERC-2612)
-            token = 0xff
-        send_filtering_amount_join_value(path, token, filtering_paths[path]["name"], discarded)
+            send_filtering_token(0)
+            token_idx = 0xff
+        if filtering_paths[path]["type"].endswith("_token"):
+            send_filtering_amount_join_token(path, token_idx, discarded)
+        elif filtering_paths[path]["type"].endswith("_value"):
+            send_filtering_amount_join_value(path,
+                                             token_idx,
+                                             filtering_paths[path]["name"],
+                                             discarded)
     elif filtering_paths[path]["type"] == "datetime":
         send_filtering_datetime(path, filtering_paths[path]["name"], discarded)
     elif filtering_paths[path]["type"] == "raw":
@@ -353,17 +372,20 @@ def send_filtering_raw(path: str, display_name: str, discarded: bool):
 
 def prepare_filtering(filtr_data, message):
     global filtering_paths
+    global filtering_tokens
 
     if "fields" in filtr_data:
         filtering_paths = filtr_data["fields"]
     else:
         filtering_paths = {}
+
     if "tokens" in filtr_data:
-        for token in filtr_data["tokens"]:
-            app_client.provide_token_metadata(token["ticker"],
-                                              bytes.fromhex(token["addr"][2:]),
-                                              token["decimals"],
-                                              token["chain_id"])
+        filtering_tokens = filtr_data["tokens"]
+        for token in filtering_tokens:
+            if len(token) > 0:
+                token["sent"] = False
+    else:
+        filtering_tokens = []
 
 
 def handle_optional_domain_values(domain):

src_features/signMessageEIP712/filtering.c

@@ -139,10 +139,6 @@ static bool check_token_index(uint8_t idx) {
         PRINTF("Error: token index out of range (%u)\n", idx);
         return false;
     }
-    if (!tmpCtx.transactionContext.assetSet[idx]) {
-        PRINTF("Error: token not set (%u)\n", idx);
-        return false;
-    }
     return true;
 }
 

src_features/signMessageEIP712/ui_logic.c

@@ -415,19 +415,21 @@ static bool ui_712_format_uint(const uint8_t *data, uint8_t length, bool first)
  * @return whether it was successful or not
  */
 static bool ui_712_format_amount_join(void) {
-    const tokenDefinition_t *token;
-    token = &tmpCtx.transactionContext.extraInfo[ui_ctx->amount.idx].token;
+    const tokenDefinition_t *token = NULL;
 
+    if (tmpCtx.transactionContext.assetSet[ui_ctx->amount.idx]) {
+        token = &tmpCtx.transactionContext.extraInfo[ui_ctx->amount.idx].token;
+    }
     if ((ui_ctx->amount.joins[ui_ctx->amount.idx].value_length == INT256_LENGTH) &&
         ismaxint(ui_ctx->amount.joins[ui_ctx->amount.idx].value,
                  ui_ctx->amount.joins[ui_ctx->amount.idx].value_length)) {
         strlcpy(strings.tmp.tmp, "Unlimited ", sizeof(strings.tmp.tmp));
-        strlcat(strings.tmp.tmp, token->ticker, sizeof(strings.tmp.tmp));
+        strlcat(strings.tmp.tmp, (token != NULL) ? token->ticker : "???", sizeof(strings.tmp.tmp));
     } else {
         if (!amountToString(ui_ctx->amount.joins[ui_ctx->amount.idx].value,
                             ui_ctx->amount.joins[ui_ctx->amount.idx].value_length,
-                            token->decimals,
-                            token->ticker,
+                            (token != NULL) ? token->decimals : 0,
+                            (token != NULL) ? token->ticker : "???",
                             strings.tmp.tmp,
                             sizeof(strings.tmp.tmp))) {
             return false;
@@ -456,13 +458,23 @@ void amount_join_set_token_received(void) {
  * @return whether it was successful or not
  */
 static bool update_amount_join(const uint8_t *data, uint8_t length) {
-    tokenDefinition_t *token;
+    const tokenDefinition_t *token = NULL;
 
-    token = &tmpCtx.transactionContext.extraInfo[ui_ctx->amount.idx].token;
+    if (tmpCtx.transactionContext.assetSet[ui_ctx->amount.idx]) {
+        token = &tmpCtx.transactionContext.extraInfo[ui_ctx->amount.idx].token;
+    } else {
+        if (tmpCtx.transactionContext.currentAssetIndex == ui_ctx->amount.idx) {
+            // So that the following amount-join find their tokens in the expected indices
+            tmpCtx.transactionContext.currentAssetIndex =
+                (tmpCtx.transactionContext.currentAssetIndex + 1) % MAX_ASSETS;
+        }
+    }
     switch (ui_ctx->amount.state) {
         case AMOUNT_JOIN_STATE_TOKEN:
-            if (memcmp(data, token->address, ADDRESS_LENGTH) != 0) {
-                return false;
+            if (token != NULL) {
+                if (memcmp(data, token->address, ADDRESS_LENGTH) != 0) {
+                    return false;
+                }
             }
             amount_join_set_token_received();
             break;

tests/ragger/test_eip712.py

@@ -242,14 +242,14 @@ def __init__(self, data: dict, filters: dict, suffix: str = ""):
             "name": "Advanced Filtering",
             "tokens": [
                 {
-                    "addr": "0x6b175474e89094c44da98b954eedeac495271d0f",
-                    "ticker": "DAI",
+                    "addr": "0xc02aaa39b223fe8d0a0e5c4f27ead9083c756cc2",
+                    "ticker": "WETH",
                     "decimals": 18,
                     "chain_id": 1,
                 },
                 {
-                    "addr": "0xc02aaa39b223fe8d0a0e5c4f27ead9083c756cc2",
-                    "ticker": "WETH",
+                    "addr": "0x6b175474e89094c44da98b954eedeac495271d0f",
+                    "ticker": "DAI",
                     "decimals": 18,
                     "chain_id": 1,
                 },
@@ -258,20 +258,20 @@ def __init__(self, data: dict, filters: dict, suffix: str = ""):
                 "value_send": {
                     "type": "amount_join_value",
                     "name": "Send",
-                    "token": 0,
+                    "token": 1,
                 },
                 "token_send": {
                     "type": "amount_join_token",
-                    "token": 0,
+                    "token": 1,
                 },
                 "value_recv": {
                     "type": "amount_join_value",
                     "name": "Receive",
-                    "token": 1,
+                    "token": 0,
                 },
                 "token_recv": {
                     "type": "amount_join_token",
-                    "token": 1,
+                    "token": 0,
                 },
                 "with": {
                     "type": "raw",
@@ -528,3 +528,114 @@ def test_eip712_filtering_empty_array(firmware: Firmware,
     # verify signature
     addr = recover_message(data, vrs)
     assert addr == get_wallet_addr(app_client)
+
+
+TOKENS = [
+    [
+        {
+            "addr": "0x1111111111111111111111111111111111111111",
+            "ticker": "SRC",
+            "decimals": 18,
+            "chain_id": 1,
+        },
+        {},
+    ],
+    [
+        {},
+        {
+            "addr": "0x2222222222222222222222222222222222222222",
+            "ticker": "DST",
+            "decimals": 18,
+            "chain_id": 1,
+        },
+    ]
+]
+
+
+@pytest.fixture(name="tokens", params=TOKENS)
+def tokens_fixture(request) -> list[dict]:
+    return request.param
+
+
+def test_eip712_advanced_missing_token(firmware: Firmware,
+                                       backend: BackendInterface,
+                                       navigator: Navigator,
+                                       default_screenshot_path: Path,
+                                       test_name: str,
+                                       tokens: list[dict],
+                                       golden_run: bool):
+    global SNAPS_CONFIG
+
+    test_name += "-%s-%s" % (len(tokens[0]) == 0, len(tokens[1]) == 0)
+    SNAPS_CONFIG = SnapshotsConfig(test_name)
+
+    app_client = EthAppClient(backend)
+    if firmware == Firmware.NANOS:
+        pytest.skip("Not supported on LNS")
+
+    data = {
+        "types": {
+            "EIP712Domain": [
+                {"name": "name", "type": "string"},
+                {"name": "version", "type": "string"},
+                {"name": "chainId", "type": "uint256"},
+                {"name": "verifyingContract", "type": "address"},
+            ],
+            "Root": [
+                {"name": "token_from", "type": "address"},
+                {"name": "value_from", "type": "uint256"},
+                {"name": "token_to", "type": "address"},
+                {"name": "value_to", "type": "uint256"},
+            ]
+        },
+        "primaryType": "Root",
+        "domain": {
+            "name": "test",
+            "version": "1",
+            "verifyingContract": "0x0000000000000000000000000000000000000000",
+            "chainId": 1,
+        },
+        "message": {
+            "token_from": "0x1111111111111111111111111111111111111111",
+            "value_from": web3.Web3.to_wei(3.65, "ether"),
+            "token_to": "0x2222222222222222222222222222222222222222",
+            "value_to": web3.Web3.to_wei(15.47, "ether"),
+        }
+    }
+
+    filters = {
+        "name": "Token not in CAL test",
+        "tokens": tokens,
+        "fields": {
+            "token_from": {
+                "type": "amount_join_token",
+                "token": 0,
+            },
+            "value_from": {
+                "type": "amount_join_value",
+                "name": "From",
+                "token": 0,
+            },
+            "token_to": {
+                "type": "amount_join_token",
+                "token": 1,
+            },
+            "value_to": {
+                "type": "amount_join_value",
+                "name": "To",
+                "token": 1,
+            },
+        }
+    }
+    vrs = eip712_new_common(firmware,
+                            navigator,
+                            default_screenshot_path,
+                            app_client,
+                            data,
+                            filters,
+                            False,
+                            golden_run)
+
+    # verify signature
+    addr = recover_message(data, vrs)
+    assert addr == get_wallet_addr(app_client)