From 87cc4f21504b980e377a5d943f525abe80439e78 Mon Sep 17 00:00:00 2001 From: Francois Beutin Date: Mon, 7 Oct 2024 13:24:06 +0200 Subject: [PATCH 1/3] Fix Thorswap for ERC20 tokens by increasing CROSSCHAIN internal plugin priority (cherry picked from commit 942a7ba20892c92e79ce6d5935320e3146c5b402) --- src/eth_plugin_handler.c | 24 ++++++++++++++---------- src/handle_swap_sign_transaction.c | 5 +++++ src_features/signTx/cmd_signTx.c | 9 ++++++++- src_features/signTx/logic_signTx.c | 3 ++- 4 files changed, 29 insertions(+), 12 deletions(-) diff --git a/src/eth_plugin_handler.c b/src/eth_plugin_handler.c index 8150eb730..2a0253510 100644 --- 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 index 7ac6d6f36..5af5bd7ed 100644 --- 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 @@ -138,6 +139,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 index f4ecbada9..cd21300bc 100644 --- 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 index 55690e5fc..7dfef1afb 100644 --- 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); From 52db6e8720e9adac2f52174cf699103d6458bf0c Mon Sep 17 00:00:00 2001 From: Francois Beutin Date: Fri, 11 Oct 2024 17:03:30 +0200 Subject: [PATCH 2/3] When swapping ERC20 tokens with thorswap, ensure amount is 0 (cherry picked from commit f115468a25a5d3b95e5d6ec5306327935aeffd1d) --- src/handle_swap_sign_transaction.c | 113 +++++++++++++++++------------ 1 file changed, 68 insertions(+), 45 deletions(-) diff --git a/src/handle_swap_sign_transaction.c b/src/handle_swap_sign_transaction.c index 5af5bd7ed..8faecbb9f 100644 --- a/src/handle_swap_sign_transaction.c +++ b/src/handle_swap_sign_transaction.c @@ -38,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)); @@ -50,6 +54,7 @@ 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, @@ -57,74 +62,92 @@ bool copy_transaction_parameters(create_transaction_parameters_t* sign_transacti 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; } From d862fda3474e75e6af99fa78c3ffb077ded39953 Mon Sep 17 00:00:00 2001 From: Alexandre Paillier Date: Wed, 23 Oct 2024 15:35:30 +0200 Subject: [PATCH 3/3] Bumped version number to 1.12.2 --- CHANGELOG.md | 6 ++++++ Makefile | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 180407fbb..2f020aeda 100644 --- 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 index b4a85d381..781d05d37 100644 --- 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