Merge pull request #668 from LedgerHQ/fix/apa/token_swap_with_calldata

apaillier-ledger · web-flow · commit 65031ce4b94e · 2024-10-24T18:20:07.000+02:00
[hotfix] Token swap with calldata
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](http://keepachangelog.com/)
 and this project adheres to [Semantic Versioning](http://semver.org/).
 
+## [1.12.2](https://github.com/ledgerhq/app-ethereum/compare/1.12.1...1.12.2) - 2024-10-24
+
+### Fixed
+
+- Token swap with calldata
+
 ## [1.12.1](https://github.com/ledgerhq/app-ethereum/compare/1.12.0...1.12.1) - 2024-10-02
 
 ### Fixed
diff --git a/Makefile b/Makefile
@@ -37,7 +37,7 @@ include ./makefile_conf/chain/$(CHAIN).mk
 
 APPVERSION_M = 1
 APPVERSION_N = 12
-APPVERSION_P = 1
+APPVERSION_P = 2
 APPVERSION = $(APPVERSION_M).$(APPVERSION_N).$(APPVERSION_P)
 
 # Application source files
diff --git a/src/eth_plugin_handler.c b/src/eth_plugin_handler.c
@@ -140,34 +140,38 @@ eth_plugin_result_t eth_plugin_perform_init(uint8_t *contractAddress,
         case ERC721:
 #endif  // HAVE_NFT_SUPPORT
         case EXTERNAL:
+            PRINTF("eth_plugin_perform_init_default\n");
             eth_plugin_perform_init_default(contractAddress, init);
             contractAddress = NULL;
             break;
         case OLD_INTERNAL:
+            PRINTF("eth_plugin_perform_init_old_internal\n");
             if (eth_plugin_perform_init_old_internal(contractAddress, init)) {
                 contractAddress = NULL;
             }
             break;
+        case SWAP_WITH_CALLDATA:
+            PRINTF("contractAddress == %.*H\n", 20, contractAddress);
+            PRINTF("Fallback on swap_with_calldata plugin\n");
+            contractAddress = NULL;
+            dataContext.tokenContext.pluginStatus = ETH_PLUGIN_RESULT_OK;
+            break;
         default:
             PRINTF("Unsupported pluginType %d\n", pluginType);
             os_sched_exit(0);
             break;
     }
 
-    if (G_called_from_swap && (contractAddress != NULL)) {
-        PRINTF("contractAddress == %.*H\n", 20, contractAddress);
-        PRINTF("selector == %.*H\n", 20, contractAddress);
-        PRINTF("Fallback on swap_with_calldata plugin\n");
-        set_swap_with_calldata_plugin_type();
-        dataContext.tokenContext.pluginStatus = ETH_PLUGIN_RESULT_OK;
-        contractAddress = NULL;
-    }
-
     eth_plugin_result_t status = ETH_PLUGIN_RESULT_UNAVAILABLE;
 
     if (contractAddress != NULL) {
         PRINTF("No plugin available for %.*H\n", 20, contractAddress);
-        return status;
+        if (G_called_from_swap) {
+            PRINTF("Not supported in swap mode\n");
+            return ETH_PLUGIN_RESULT_ERROR;
+        } else {
+            return status;
+        }
     }
 
     PRINTF("eth_plugin_init\n");
diff --git a/src/handle_swap_sign_transaction.c b/src/handle_swap_sign_transaction.c
@@ -6,6 +6,7 @@
 #include "shared_context.h"
 #include "common_utils.h"
 #include "network.h"
+#include "cmd_setPlugin.h"
 #ifdef HAVE_NBGL
 #include "nbgl_use_case.h"
 #endif  // HAVE_NBGL
@@ -37,10 +38,14 @@ bool copy_transaction_parameters(create_transaction_parameters_t* sign_transacti
     // We need this "trick" as the input data position can overlap with app-ethereum globals
     txStringProperties_t stack_data;
     uint8_t destination_address_extra_data[CX_SHA256_SIZE + 1];
+    uint8_t swap_crosschain_hash[sizeof(G_swap_crosschain_hash)];
+    swap_mode_t swap_mode;
 
-    memset(&stack_data, 0, sizeof(stack_data));
-    memset(destination_address_extra_data, 0, sizeof(destination_address_extra_data));
+    explicit_bzero(&stack_data, sizeof(stack_data));
+    explicit_bzero(destination_address_extra_data, sizeof(destination_address_extra_data));
+    explicit_bzero(swap_crosschain_hash, sizeof(swap_crosschain_hash));
 
+    // Set destination address
     strlcpy(stack_data.toAddress,
             sign_transaction_params->destination_address,
             sizeof(stack_data.toAddress));
@@ -49,81 +54,100 @@ bool copy_transaction_parameters(create_transaction_parameters_t* sign_transacti
         (sign_transaction_params->fee_amount_length > 8)) {
         return false;
     }
+    PRINTF("Expecting destination_address %s\n", stack_data.toAddress);
 
     if (sign_transaction_params->destination_address_extra_id != NULL) {
         memcpy(destination_address_extra_data,
                sign_transaction_params->destination_address_extra_id,
                sizeof(destination_address_extra_data));
     }
 
-    char ticker[MAX_TICKER_LEN];
-    uint8_t decimals;
+    // if destination_address_extra_id is given, we use the first byte to determine if we use the
+    // normal swap protocol, or the one for cross-chain swaps
+    switch (destination_address_extra_data[0]) {
+        case EXTRA_ID_TYPE_NATIVE:
+            // we don't use the payin_extra_id field in this mode
+            swap_mode = SWAP_MODE_STANDARD;
+            PRINTF("Standard swap\n");
+            break;
+        case EXTRA_ID_TYPE_EVM_CALLDATA:
+            swap_mode = SWAP_MODE_CROSSCHAIN_PENDING_CHECK;
+
+            memcpy(swap_crosschain_hash,
+                   destination_address_extra_data + 1,
+                   sizeof(swap_crosschain_hash));
+
+            PRINTF("Crosschain swap with hash: %.*H\n", CX_SHA256_SIZE, swap_crosschain_hash);
+            break;
+        default:
+            // We can't return errors from here, we remember that we have an issue to report later
+            PRINTF("Invalid or unknown swap protocol\n");
+            swap_mode = SWAP_MODE_ERROR;
+    }
+
+    char asset_ticker[MAX_TICKER_LEN];
+    uint8_t asset_decimals;
     uint64_t chain_id = 0;
 
     if (!parse_swap_config(sign_transaction_params->coin_configuration,
                            sign_transaction_params->coin_configuration_length,
-                           ticker,
-                           &decimals,
+                           asset_ticker,
+                           &asset_decimals,
                            &chain_id)) {
         PRINTF("Error while parsing config\n");
         return false;
     }
-    if (!amountToString(sign_transaction_params->amount,
-                        sign_transaction_params->amount_length,
-                        decimals,
-                        ticker,
-                        stack_data.fullAmount,
-                        sizeof(stack_data.fullAmount))) {
-        return false;
-    }
 
     // fallback mechanism in the absence of chain ID in swap config
     if (chain_id == 0) {
         chain_id = config->chainId;
     }
-    // If the amount is a fee, its value is nominated in ETH even if we're doing an ERC20 swap
-    strlcpy(ticker, get_displayable_ticker(&chain_id, config), sizeof(ticker));
-    decimals = WEI_TO_ETHER;
+    PRINTF("chain_id = %d\n", (uint32_t) chain_id);
+
+    // If the amount is a fee, its value is nominated in NATIVE even if we're doing an ERC20 swap
+    const char* native_ticker = get_displayable_ticker(&chain_id, config);
+    uint8_t native_decimals = WEI_TO_ETHER;
     if (!amountToString(sign_transaction_params->fee_amount,
                         sign_transaction_params->fee_amount_length,
-                        decimals,
-                        ticker,
+                        native_decimals,
+                        native_ticker,
                         stack_data.maxFee,
                         sizeof(stack_data.maxFee))) {
         return false;
     }
+    PRINTF("Expecting fees %s\n", stack_data.maxFee);
+
+    if (swap_mode == SWAP_MODE_CROSSCHAIN_PENDING_CHECK &&
+        strcmp(native_ticker, asset_ticker) != 0) {
+        // Special case: crosschain swap of non native assets (tokens)
+        uint8_t zero_amount = 0;
+        if (!amountToString(&zero_amount,
+                            1,
+                            native_decimals,
+                            native_ticker,
+                            stack_data.fullAmount,
+                            sizeof(stack_data.fullAmount))) {
+            return false;
+        }
+    } else {
+        if (!amountToString(sign_transaction_params->amount,
+                            sign_transaction_params->amount_length,
+                            asset_decimals,
+                            asset_ticker,
+                            stack_data.fullAmount,
+                            sizeof(stack_data.fullAmount))) {
+            return false;
+        }
+    }
+    PRINTF("Expecting amount %s\n", stack_data.fullAmount);
 
     // Full reset the global variables
     os_explicit_zero_BSS_segment();
     // Keep the address at which we'll reply the signing status
     G_swap_sign_return_value_address = &sign_transaction_params->result;
     // Commit the values read from exchange to the clean global space
-
-    // if destination_address_extra_id is given, we use the first byte to determine if we use the
-    // normal swap protocol, or the one for cross-chain swaps
-    switch (destination_address_extra_data[0]) {
-        case EXTRA_ID_TYPE_NATIVE:
-            G_swap_mode = SWAP_MODE_STANDARD;
-            PRINTF("Standard swap\n");
-
-            // we don't use the payin_extra_id field in this mode
-            explicit_bzero(G_swap_crosschain_hash, sizeof(G_swap_crosschain_hash));
-            break;
-        case EXTRA_ID_TYPE_EVM_CALLDATA:
-            G_swap_mode = SWAP_MODE_CROSSCHAIN_PENDING_CHECK;
-
-            memcpy(G_swap_crosschain_hash,
-                   destination_address_extra_data + 1,
-                   sizeof(G_swap_crosschain_hash));
-
-            PRINTF("Crosschain swap with hash: %.*H\n", CX_SHA256_SIZE, G_swap_crosschain_hash);
-            break;
-        default:
-            // We can't return errors from here, we remember that we have an issue to report later
-            PRINTF("Invalid or unknown swap protocol\n");
-            G_swap_mode = SWAP_MODE_ERROR;
-    }
-
+    G_swap_mode = swap_mode;
+    memcpy(G_swap_crosschain_hash, swap_crosschain_hash, sizeof(G_swap_crosschain_hash));
     memcpy(&strings.common, &stack_data, sizeof(stack_data));
     return true;
 }
@@ -138,6 +162,10 @@ void __attribute__((noreturn)) handle_swap_sign_transaction(const chain_config_t
     reset_app_context();
     G_called_from_swap = true;
     G_swap_response_ready = false;
+    // If we are in crosschain context, automatically register the CROSSCHAIN plugin
+    if (G_swap_mode == SWAP_MODE_CROSSCHAIN_PENDING_CHECK) {
+        set_swap_with_calldata_plugin_type();
+    }
 
     common_app_init();
 
diff --git a/src_features/signTx/cmd_signTx.c b/src_features/signTx/cmd_signTx.c
@@ -83,7 +83,14 @@ uint16_t handleSign(uint8_t p1,
         case USTREAM_PROCESSING:
             return APDU_RESPONSE_OK;
         case USTREAM_FAULT:
-            return APDU_RESPONSE_INVALID_DATA;
+            if (G_called_from_swap) {
+                // We have encountered an error while trying to sign a SWAP type transaction
+                // Return dedicated error code and flag an early exit back to Exchange
+                G_swap_response_ready = true;
+                return APDU_RESPONSE_MODE_CHECK_FAILED;
+            } else {
+                return APDU_RESPONSE_INVALID_DATA;
+            }
         default:
             PRINTF("Unexpected parser status\n");
             return APDU_RESPONSE_INVALID_DATA;
diff --git a/src_features/signTx/logic_signTx.c b/src_features/signTx/logic_signTx.c
@@ -57,7 +57,8 @@ customStatus_e customProcessor(txContext_t *context) {
             dataContext.tokenContext.pluginStatus = ETH_PLUGIN_RESULT_UNAVAILABLE;
             // If contract debugging mode is activated, do not go through the plugin activation
             // as they wouldn't be displayed if the plugin consumes all data but fallbacks
-            if (!N_storage.contractDetails) {
+            // Still go through plugin activation in Swap context
+            if (!N_storage.contractDetails || G_called_from_swap) {
                 eth_plugin_prepare_init(&pluginInit,
                                         context->workBuffer,
                                         context->currentFieldLength);
