From 273dd959378ea20c102339a525f84e04c24d633a Mon Sep 17 00:00:00 2001 From: Charles-Edouard de la Vergne Date: Mon, 11 Mar 2024 16:14:31 +0100 Subject: [PATCH] Add 'sender' field to review screens --- src/handle_swap_sign_transaction.c | 6 ++--- src/plugins.c | 8 +++---- src/shared_context.h | 3 ++- src_bagl/ui_flow_erc20_approval.c | 2 +- src_bagl/ui_flow_getPublicKey.c | 2 +- src_bagl/ui_flow_performPrivacyOperation.c | 2 +- src_bagl/ui_flow_signTx.c | 21 ++++++++++++---- src_features/getPublicKey/cmd_getPublicKey.c | 4 ++-- .../cmd_performPrivacyOperation.c | 4 ++-- src_features/signTx/logic_signTx.c | 24 +++++++++++++++---- src_nbgl/ui_approve_tx.c | 18 +++++++++----- src_nbgl/ui_display_privacy.c | 2 +- src_nbgl/ui_get_public_key.c | 2 +- 13 files changed, 66 insertions(+), 32 deletions(-) diff --git a/src/handle_swap_sign_transaction.c b/src/handle_swap_sign_transaction.c index 558650f..dd7d613 100644 --- a/src/handle_swap_sign_transaction.c +++ b/src/handle_swap_sign_transaction.c @@ -19,10 +19,10 @@ 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; memset(&stack_data, 0, sizeof(stack_data)); - strlcpy(stack_data.fullAddress, + strlcpy(stack_data.toAddress, sign_transaction_params->destination_address, - sizeof(stack_data.fullAddress)); - if ((stack_data.fullAddress[sizeof(stack_data.fullAddress) - 1] != '\0') || + sizeof(stack_data.toAddress)); + if ((stack_data.toAddress[sizeof(stack_data.toAddress) - 1] != '\0') || (sign_transaction_params->amount_length > 32) || (sign_transaction_params->fee_amount_length > 8)) { return false; diff --git a/src/plugins.c b/src/plugins.c index 0256979..ee5a2f7 100644 --- a/src/plugins.c +++ b/src/plugins.c @@ -4,8 +4,8 @@ void plugin_ui_get_id(void) { ethQueryContractID_t pluginQueryContractID; eth_plugin_prepare_query_contract_ID(&pluginQueryContractID, - strings.common.fullAddress, - sizeof(strings.common.fullAddress), + strings.common.toAddress, + sizeof(strings.common.toAddress), strings.common.fullAmount, sizeof(strings.common.fullAmount)); // Query the original contract for ID if it's not an internal alias @@ -33,8 +33,8 @@ void plugin_ui_get_item_internal(char *title_buffer, } void plugin_ui_get_item(void) { - plugin_ui_get_item_internal(strings.common.fullAddress, - sizeof(strings.common.fullAddress), + plugin_ui_get_item_internal(strings.common.toAddress, + sizeof(strings.common.toAddress), strings.common.fullAmount, sizeof(strings.common.fullAmount)); } diff --git a/src/shared_context.h b/src/shared_context.h index b22aff1..85a2296 100644 --- a/src/shared_context.h +++ b/src/shared_context.h @@ -127,7 +127,8 @@ typedef enum { #define NETWORK_STRING_MAX_SIZE 19 typedef struct txStringProperties_s { - char fullAddress[43]; + char fromAddress[43]; + char toAddress[43]; char fullAmount[79]; // 2^256 is 78 digits long char maxFee[50]; char nonce[8]; // 10M tx per account ought to be enough for everybody diff --git a/src_bagl/ui_flow_erc20_approval.c b/src_bagl/ui_flow_erc20_approval.c index 91e78f1..9c512b4 100644 --- a/src_bagl/ui_flow_erc20_approval.c +++ b/src_bagl/ui_flow_erc20_approval.c @@ -23,7 +23,7 @@ UX_STEP_NOCB( bnnn_paging, { .title = "Contract Name", - .text = strings.common.fullAddress, + .text = strings.common.toAddress, }); UX_STEP_NOCB( diff --git a/src_bagl/ui_flow_getPublicKey.c b/src_bagl/ui_flow_getPublicKey.c index 9bd6620..3ca3f77 100644 --- a/src_bagl/ui_flow_getPublicKey.c +++ b/src_bagl/ui_flow_getPublicKey.c @@ -15,7 +15,7 @@ UX_STEP_NOCB( bnnn_paging, { .title = "Address", - .text = strings.common.fullAddress, + .text = strings.common.toAddress, }); UX_STEP_CB( ux_display_public_flow_3_step, diff --git a/src_bagl/ui_flow_performPrivacyOperation.c b/src_bagl/ui_flow_performPrivacyOperation.c index 8c870c0..d1cb8d3 100644 --- a/src_bagl/ui_flow_performPrivacyOperation.c +++ b/src_bagl/ui_flow_performPrivacyOperation.c @@ -15,7 +15,7 @@ UX_STEP_NOCB( bnnn_paging, { .title = "Address", - .text = strings.common.fullAddress, + .text = strings.common.toAddress, }); UX_STEP_NOCB( ux_display_privacy_public_key_flow_3_step, diff --git a/src_bagl/ui_flow_signTx.c b/src_bagl/ui_flow_signTx.c index eabdf90..7725c84 100644 --- a/src_bagl/ui_flow_signTx.c +++ b/src_bagl/ui_flow_signTx.c @@ -109,12 +109,19 @@ UX_STEP_NOCB( .title = "Amount", .text = strings.common.fullAmount }); +UX_STEP_NOCB( + ux_approval_from_step, + bnnn_paging, + { + .title = "From", + .text = strings.common.fromAddress, + }); UX_STEP_NOCB( ux_approval_address_step, bnnn_paging, { - .title = "Address", - .text = strings.common.fullAddress, + .title = "To", + .text = strings.common.toAddress, }); UX_STEP_NOCB_INIT( @@ -122,7 +129,7 @@ UX_STEP_NOCB_INIT( bnnn_paging, plugin_ui_get_id(), { - .title = strings.common.fullAddress, + .title = strings.common.toAddress, .text = strings.common.fullAmount }); @@ -138,7 +145,7 @@ UX_FLOW_DEF_NOCB( ux_plugin_approval_display_step, bnnn_paging, { - .title = strings.common.fullAddress, + .title = strings.common.toAddress, .text = strings.common.fullAmount }); @@ -225,10 +232,16 @@ void ux_approve_tx(bool fromPlugin) { if (has_domain_name(&chain_id, tmpContent.txContent.destination)) { ux_approval_tx_flow[step++] = &ux_domain_name_step; if (N_storage.verbose_domain_name) { + if (strings.common.fromAddress[0] != 0) { + ux_approval_tx_flow[step++] = &ux_approval_from_step; + } ux_approval_tx_flow[step++] = &ux_approval_address_step; } } else { #endif // HAVE_DOMAIN_NAME + if (strings.common.fromAddress[0] != 0) { + ux_approval_tx_flow[step++] = &ux_approval_from_step; + } ux_approval_tx_flow[step++] = &ux_approval_address_step; #ifdef HAVE_DOMAIN_NAME } diff --git a/src_features/getPublicKey/cmd_getPublicKey.c b/src_features/getPublicKey/cmd_getPublicKey.c index 7fb8bae..66a4b4c 100644 --- a/src_features/getPublicKey/cmd_getPublicKey.c +++ b/src_features/getPublicKey/cmd_getPublicKey.c @@ -64,8 +64,8 @@ void handleGetPublicKey(uint8_t p1, *tx = set_result_get_publicKey(); THROW(APDU_RESPONSE_OK); } else { - snprintf(strings.common.fullAddress, - sizeof(strings.common.fullAddress), + snprintf(strings.common.toAddress, + sizeof(strings.common.toAddress), "0x%.*s", 40, tmpCtx.publicKeyContext.address); diff --git a/src_features/performPrivacyOperation/cmd_performPrivacyOperation.c b/src_features/performPrivacyOperation/cmd_performPrivacyOperation.c index ba8a102..e1a733f 100644 --- a/src_features/performPrivacyOperation/cmd_performPrivacyOperation.c +++ b/src_features/performPrivacyOperation/cmd_performPrivacyOperation.c @@ -94,8 +94,8 @@ void handlePerformPrivacyOperation(uint8_t p1, *tx = set_result_perform_privacy_operation(); THROW(0x9000); } else { - snprintf(strings.common.fullAddress, - sizeof(strings.common.fullAddress), + snprintf(strings.common.toAddress, + sizeof(strings.common.toAddress), "0x%.*s", 40, tmpCtx.publicKeyContext.address); diff --git a/src_features/signTx/logic_signTx.c b/src_features/signTx/logic_signTx.c index 1101bbd..8136792 100644 --- a/src_features/signTx/logic_signTx.c +++ b/src_features/signTx/logic_signTx.c @@ -346,12 +346,13 @@ __attribute__((noinline)) static bool finalize_parsing_helper(bool direct, bool tmpCtx.transactionContext.hash, 32)); + uint8_t msg_sender[ADDRESS_LENGTH] = {0}; + get_public_key(msg_sender, sizeof(msg_sender)); + // Finalize the plugin handling if (dataContext.tokenContext.pluginStatus >= ETH_PLUGIN_RESULT_SUCCESSFUL) { eth_plugin_prepare_finalize(&pluginFinalize); - uint8_t msg_sender[ADDRESS_LENGTH] = {0}; - get_public_key(msg_sender, sizeof(msg_sender)); pluginFinalize.address = msg_sender; if (!eth_plugin_call(ETH_PLUGIN_FINALIZE, (void *) &pluginFinalize)) { @@ -465,14 +466,14 @@ __attribute__((noinline)) static bool finalize_parsing_helper(bool direct, bool chainConfig->chainId); if (G_called_from_swap) { // Ensure the values are the same that the ones that have been previously validated - if (strcasecmp_workaround(strings.common.fullAddress, displayBuffer) != 0) { + if (strcasecmp_workaround(strings.common.toAddress, displayBuffer) != 0) { PRINTF("ERR_SILENT_MODE_CHECK_FAILED, address check failed\n"); THROW(ERR_SILENT_MODE_CHECK_FAILED); } } else { - strlcpy(strings.common.fullAddress, displayBuffer, sizeof(strings.common.fullAddress)); + strlcpy(strings.common.toAddress, displayBuffer, sizeof(strings.common.toAddress)); } - PRINTF("Address displayed: %s\n", strings.common.fullAddress); + PRINTF("Address displayed: %s\n", strings.common.toAddress); // Format the amount in a temporary buffer, if in swap case compare it with validated // amount, else commit it @@ -498,6 +499,19 @@ __attribute__((noinline)) static bool finalize_parsing_helper(bool direct, bool strlcpy(strings.common.fullAmount, displayBuffer, sizeof(strings.common.fullAmount)); } PRINTF("Amount displayed: %s\n", strings.common.fullAmount); + + if (G_called_from_swap) { + // Transaction parameters are managed by the Exchange caller app! + explicit_bzero(strings.common.fromAddress, sizeof(strings.common.fromAddress)); + } else { + // Format the from address in a temporary buffer + address_to_string(msg_sender, + ADDRESS_LENGTH, + strings.common.fromAddress, + sizeof(strings.common.fromAddress), + chainConfig->chainId); + PRINTF("FROM Address displayed: %s\n", strings.common.fromAddress); + } } // Compute the max fee in a temporary buffer, if in swap case compare it with validated max fee, diff --git a/src_nbgl/ui_approve_tx.c b/src_nbgl/ui_approve_tx.c index 7a0b9a0..7aed4d1 100644 --- a/src_nbgl/ui_approve_tx.c +++ b/src_nbgl/ui_approve_tx.c @@ -57,8 +57,8 @@ static const nbgl_icon_details_t *get_tx_icon(void) { if (tx_approval_context.fromPlugin && (pluginType == EXTERNAL)) { if (caller_app && caller_app->name) { - if ((strlen(strings.common.fullAddress) == strlen(caller_app->name)) && - (strcmp(strings.common.fullAddress, caller_app->name) == 0)) { + if ((strlen(strings.common.toAddress) == strlen(caller_app->name)) && + (strcmp(strings.common.toAddress, caller_app->name) == 0)) { icon = get_app_icon(true); } } @@ -120,6 +120,12 @@ static uint8_t setTagValuePairs(void) { pairs[nbPairs].value = strings.common.fullAmount; nbPairs++; + if (strings.common.fromAddress[0] != 0) { + pairs[nbPairs].item = "From"; + pairs[nbPairs].value = strings.common.fromAddress; + nbPairs++; + } + #ifdef HAVE_DOMAIN_NAME uint64_t chain_id = get_tx_chain_id(); tx_approval_context.domain_name_match = @@ -131,8 +137,8 @@ static uint8_t setTagValuePairs(void) { } if (!tx_approval_context.domain_name_match || N_storage.verbose_domain_name) { #endif - pairs[nbPairs].item = "Address"; - pairs[nbPairs].value = strings.common.fullAddress; + pairs[nbPairs].item = "To"; + pairs[nbPairs].value = strings.common.toAddress; nbPairs++; #ifdef HAVE_DOMAIN_NAME } @@ -172,14 +178,14 @@ static void reviewCommon(void) { "Review transaction\nto %s\n%s%s", op_name, (pluginType == EXTERNAL ? "on " : ""), - strings.common.fullAddress); + strings.common.toAddress); // Finish text: replace "Review" by "Sign" and add questionmark snprintf(g_stax_shared_buffer + buf_size, buf_size, "Sign transaction\nto %s\n%s%s", op_name, (pluginType == EXTERNAL ? "on " : ""), - strings.common.fullAddress); + strings.common.toAddress); nbgl_useCaseReview(TYPE_TRANSACTION, &pairsList, diff --git a/src_nbgl/ui_display_privacy.c b/src_nbgl/ui_display_privacy.c index 1d5eb8b..59ff20c 100644 --- a/src_nbgl/ui_display_privacy.c +++ b/src_nbgl/ui_display_privacy.c @@ -28,7 +28,7 @@ static void buildFirstPage(const char *review_string) { uint8_t nbPairs = 0; pairs[nbPairs].item = "Address"; - pairs[nbPairs].value = strings.common.fullAddress; + pairs[nbPairs].value = strings.common.toAddress; nbPairs++; pairs[nbPairs].item = "Key"; pairs[nbPairs].value = strings.common.fullAmount; diff --git a/src_nbgl/ui_get_public_key.c b/src_nbgl/ui_get_public_key.c index 9695061..bc00ab6 100644 --- a/src_nbgl/ui_get_public_key.c +++ b/src_nbgl/ui_get_public_key.c @@ -41,7 +41,7 @@ void ui_display_public_key(const uint64_t *chain_id) { icon = get_app_icon(false); } strlcat(g_stax_shared_buffer, "address", sizeof(g_stax_shared_buffer)); - nbgl_useCaseAddressReview(strings.common.fullAddress, + nbgl_useCaseAddressReview(strings.common.toAddress, NULL, icon, g_stax_shared_buffer,