From 8775a2016ba1bfa0c0142f168b118ca0b6b3eb84 Mon Sep 17 00:00:00 2001 From: Charles-Edouard de la Vergne Date: Thu, 5 Sep 2024 17:53:10 +0200 Subject: [PATCH] Change Confirm/Cancel notifications order --- src/plugins.c | 3 +++ src/ui_callbacks.h | 3 +++ src_bagl/common_ui.c | 15 ++++++++++++ src_bagl/ui_flow_erc20_approval.c | 4 ++-- src_bagl/ui_flow_getEth2PublicKey.c | 2 +- src_bagl/ui_flow_getPublicKey.c | 10 ++++++-- src_bagl/ui_flow_signMessage.c | 14 +++++++++-- src_bagl/ui_flow_signMessage712.c | 15 ++++++++++-- src_bagl/ui_flow_signMessage712_v0.c | 15 ++++++++++-- src_bagl/ui_flow_signTx.c | 24 +++++++++++++------ .../getPublicKey/ui_common_getPublicKey.c | 4 ++-- .../signMessage/ui_common_signMessage.c | 4 ++-- .../signMessageEIP712_common/common_712.c | 4 ++-- src_features/signTx/logic_signTx.c | 1 + src_features/signTx/ui_common_signTx.c | 4 +--- src_nbgl/ui_approve_tx.c | 18 +++++--------- src_nbgl/ui_get_eth2_public_key.c | 14 ++++------- src_nbgl/ui_get_public_key.c | 16 ++++--------- src_nbgl/ui_message_signing.c | 14 ++++------- src_nbgl/ui_sign_message.c | 14 ++++------- 20 files changed, 118 insertions(+), 80 deletions(-) diff --git a/src/plugins.c b/src/plugins.c index e0bbbb926..3271f3d90 100644 --- a/src/plugins.c +++ b/src/plugins.c @@ -1,5 +1,6 @@ #include "eth_plugin_handler.h" #include "ui_callbacks.h" +#include "common_ui.h" void plugin_ui_get_id(void) { ethQueryContractID_t pluginQueryContractID; @@ -11,6 +12,7 @@ void plugin_ui_get_id(void) { // Query the original contract for ID if it's not an internal alias if (!eth_plugin_call(ETH_PLUGIN_QUERY_CONTRACT_ID, (void *) &pluginQueryContractID)) { PRINTF("Plugin query contract ID call failed\n"); + ui_idle(); io_seproxyhal_touch_tx_cancel(); } } @@ -28,6 +30,7 @@ void plugin_ui_get_item_internal(char *title_buffer, msg_buffer_size); if (!eth_plugin_call(ETH_PLUGIN_QUERY_CONTRACT_UI, (void *) &pluginQueryContractUI)) { PRINTF("Plugin query contract UI call failed\n"); + ui_idle(); io_seproxyhal_touch_tx_cancel(); } } diff --git a/src/ui_callbacks.h b/src/ui_callbacks.h index c91796b51..52296356f 100644 --- a/src/ui_callbacks.h +++ b/src/ui_callbacks.h @@ -18,5 +18,8 @@ unsigned int io_seproxyhal_touch_data_cancel(void); unsigned int io_seproxyhal_touch_eth2_address_ok(void); unsigned int io_seproxyhal_touch_privacy_ok(void); unsigned int io_seproxyhal_touch_privacy_cancel(void); +unsigned int address_cancel_cb(void); +unsigned int tx_ok_cb(void); +unsigned int tx_cancel_cb(void); uint16_t io_seproxyhal_send_status(uint16_t sw, uint32_t tx, bool reset, bool idle); diff --git a/src_bagl/common_ui.c b/src_bagl/common_ui.c index 80645a360..96826f8e6 100644 --- a/src_bagl/common_ui.c +++ b/src_bagl/common_ui.c @@ -49,4 +49,19 @@ void ui_confirm_parameter(void) { ux_flow_init(0, ux_confirm_parameter_flow, NULL); } +unsigned int address_cancel_cb(void) { + ui_idle(); + return io_seproxyhal_touch_address_cancel(); +} + +unsigned int tx_ok_cb(void) { + ui_idle(); + return io_seproxyhal_touch_tx_ok(); +} + +unsigned int tx_cancel_cb(void) { + ui_idle(); + return io_seproxyhal_touch_tx_cancel(); +} + #endif // HAVE_BAGL diff --git a/src_bagl/ui_flow_erc20_approval.c b/src_bagl/ui_flow_erc20_approval.c index 2ff2e71af..28ed3f7bb 100644 --- a/src_bagl/ui_flow_erc20_approval.c +++ b/src_bagl/ui_flow_erc20_approval.c @@ -45,7 +45,7 @@ UX_STEP_NOCB( UX_STEP_CB( ux_approval_allowance_6_step, pbb, - io_seproxyhal_touch_tx_ok(), + tx_ok_cb(), { &C_icon_validate_14, "Accept", @@ -55,7 +55,7 @@ UX_STEP_CB( UX_STEP_CB( ux_approval_allowance_7_step, pb, - io_seproxyhal_touch_tx_cancel(), + tx_cancel_cb(), { &C_icon_crossmark, "Reject", diff --git a/src_bagl/ui_flow_getEth2PublicKey.c b/src_bagl/ui_flow_getEth2PublicKey.c index 2156e0b56..9d2122655 100644 --- a/src_bagl/ui_flow_getEth2PublicKey.c +++ b/src_bagl/ui_flow_getEth2PublicKey.c @@ -39,7 +39,7 @@ UX_STEP_CB( UX_STEP_CB( ux_display_public_eth2_flow_4_step, pb, - io_seproxyhal_touch_address_cancel(), + address_cancel_cb(), { &C_icon_crossmark, "Reject", diff --git a/src_bagl/ui_flow_getPublicKey.c b/src_bagl/ui_flow_getPublicKey.c index 52b968d3c..d70396d75 100644 --- a/src_bagl/ui_flow_getPublicKey.c +++ b/src_bagl/ui_flow_getPublicKey.c @@ -1,5 +1,11 @@ #include "shared_context.h" #include "ui_callbacks.h" +#include "common_ui.h" + +static unsigned int address_ok_cb(void) { + ui_idle(); + return io_seproxyhal_touch_address_ok(); +} // clang-format off UX_STEP_NOCB( @@ -20,7 +26,7 @@ UX_STEP_NOCB( UX_STEP_CB( ux_display_public_flow_3_step, pb, - io_seproxyhal_touch_address_ok(), + address_ok_cb(), { &C_icon_validate_14, "Approve", @@ -28,7 +34,7 @@ UX_STEP_CB( UX_STEP_CB( ux_display_public_flow_4_step, pb, - io_seproxyhal_touch_address_cancel(), + address_cancel_cb(), { &C_icon_crossmark, "Reject", diff --git a/src_bagl/ui_flow_signMessage.c b/src_bagl/ui_flow_signMessage.c index 60b3fcafb..561bb6cde 100644 --- a/src_bagl/ui_flow_signMessage.c +++ b/src_bagl/ui_flow_signMessage.c @@ -28,6 +28,16 @@ static void dummy_post_cb(void) { } } +static unsigned int signMessage_ok_cb(void) { + ui_idle(); + return io_seproxyhal_touch_signMessage_ok(); +} + +static unsigned int signMessage_cancel_cb(void) { + ui_idle(); + return io_seproxyhal_touch_signMessage_cancel(); +} + // clang-format off UX_STEP_NOCB( ux_191_step_review, @@ -79,7 +89,7 @@ UX_STEP_INIT( UX_STEP_CB( ux_191_step_sign, pbb, - io_seproxyhal_touch_signMessage_ok(), + signMessage_ok_cb(), { &C_icon_validate_14, "Sign", @@ -88,7 +98,7 @@ UX_STEP_CB( UX_STEP_CB( ux_191_step_cancel, pbb, - io_seproxyhal_touch_signMessage_cancel(), + signMessage_cancel_cb(), { &C_icon_crossmark, "Cancel", diff --git a/src_bagl/ui_flow_signMessage712.c b/src_bagl/ui_flow_signMessage712.c index 3ffb5b023..7a52baa4a 100644 --- a/src_bagl/ui_flow_signMessage712.c +++ b/src_bagl/ui_flow_signMessage712.c @@ -2,6 +2,7 @@ #include "ui_logic.h" #include "shared_context.h" // strings +#include "common_ui.h" enum { UI_712_POS_REVIEW, UI_712_POS_END }; static uint8_t ui_pos; @@ -28,6 +29,16 @@ static void dummy_cb(void) { } } +static unsigned int _approve_cb(void) { + ui_idle(); + return ui_712_approve(); +} + +static unsigned int _reject_cb(void) { + ui_idle(); + return ui_712_reject(); +} + // clang-format off UX_STEP_NOCB( ux_712_step_review, @@ -56,7 +67,7 @@ UX_STEP_INIT( UX_STEP_CB( ux_712_step_approve, pb, - ui_712_approve(), + _approve_cb(), { &C_icon_validate_14, "Approve", @@ -64,7 +75,7 @@ UX_STEP_CB( UX_STEP_CB( ux_712_step_reject, pb, - ui_712_reject(), + _reject_cb(), { &C_icon_crossmark, "Reject", diff --git a/src_bagl/ui_flow_signMessage712_v0.c b/src_bagl/ui_flow_signMessage712_v0.c index 72c643509..46cd039c8 100644 --- a/src_bagl/ui_flow_signMessage712_v0.c +++ b/src_bagl/ui_flow_signMessage712_v0.c @@ -2,6 +2,7 @@ #include "ui_callbacks.h" #include "common_712.h" #include "uint_common.h" +#include "common_ui.h" void prepare_domain_hash_v0() { array_bytes_string(strings.tmp.tmp, @@ -17,6 +18,16 @@ void prepare_message_hash_v0() { KECCAK256_HASH_BYTESIZE); } +static unsigned int _approve_cb(void) { + ui_idle(); + return ui_712_approve_cb(); +} + +static unsigned int _reject_cb(void) { + ui_idle(); + return ui_712_reject_cb(); +} + // clang-format off UX_STEP_NOCB( ux_sign_712_v0_flow_1_step, @@ -45,7 +56,7 @@ UX_STEP_NOCB_INIT( UX_STEP_CB( ux_sign_712_v0_flow_4_step, pbb, - ui_712_approve_cb(), + _approve_cb(), { &C_icon_validate_14, "Sign", @@ -54,7 +65,7 @@ UX_STEP_CB( UX_STEP_CB( ux_sign_712_v0_flow_5_step, pbb, - ui_712_reject_cb(), + _reject_cb(), { &C_icon_crossmark, "Cancel", diff --git a/src_bagl/ui_flow_signTx.c b/src_bagl/ui_flow_signTx.c index c473e4673..89ff9173f 100644 --- a/src_bagl/ui_flow_signTx.c +++ b/src_bagl/ui_flow_signTx.c @@ -11,6 +11,16 @@ #include "domain_name.h" #include "ui_domain_name.h" +static unsigned int data_ok_cb(void) { + ui_idle(); + return io_seproxyhal_touch_data_ok(); +} + +static unsigned int data_cancel_cb(void) { + ui_idle(); + return io_seproxyhal_touch_data_cancel(); +} + // clang-format off UX_STEP_NOCB( ux_confirm_selector_flow_1_step, @@ -31,7 +41,7 @@ UX_STEP_NOCB( UX_STEP_CB( ux_confirm_selector_flow_3_step, pb, - io_seproxyhal_touch_data_ok(), + data_ok_cb(), { &C_icon_validate_14, "Approve", @@ -39,7 +49,7 @@ UX_STEP_CB( UX_STEP_CB( ux_confirm_selector_flow_4_step, pb, - io_seproxyhal_touch_data_cancel(), + data_cancel_cb(), { &C_icon_crossmark, "Reject", @@ -72,7 +82,7 @@ UX_STEP_NOCB( UX_STEP_CB( ux_confirm_parameter_flow_3_step, pb, - io_seproxyhal_touch_data_ok(), + data_ok_cb(), { &C_icon_validate_14, "Approve", @@ -80,7 +90,7 @@ UX_STEP_CB( UX_STEP_CB( ux_confirm_parameter_flow_4_step, pb, - io_seproxyhal_touch_data_cancel(), + data_cancel_cb(), { &C_icon_crossmark, "Reject", @@ -176,7 +186,7 @@ UX_STEP_NOCB( UX_STEP_CB( ux_approval_accept_step, pbb, - io_seproxyhal_touch_tx_ok(), + tx_ok_cb(), { &C_icon_validate_14, "Accept", @@ -185,7 +195,7 @@ UX_STEP_CB( UX_STEP_CB( ux_approval_accept_blind_step, pbb, - io_seproxyhal_touch_tx_ok(), + tx_ok_cb(), { &C_icon_validate_14, "Accept risk", @@ -194,7 +204,7 @@ UX_STEP_CB( UX_STEP_CB( ux_approval_reject_step, pb, - io_seproxyhal_touch_tx_cancel(), + tx_cancel_cb(), { &C_icon_crossmark, "Reject", diff --git a/src_features/getPublicKey/ui_common_getPublicKey.c b/src_features/getPublicKey/ui_common_getPublicKey.c index 870509840..fe1a0bae0 100644 --- a/src_features/getPublicKey/ui_common_getPublicKey.c +++ b/src_features/getPublicKey/ui_common_getPublicKey.c @@ -5,9 +5,9 @@ unsigned int io_seproxyhal_touch_address_ok(void) { uint32_t tx = set_result_get_publicKey(); - return io_seproxyhal_send_status(APDU_RESPONSE_OK, tx, true, true); + return io_seproxyhal_send_status(APDU_RESPONSE_OK, tx, true, false); } unsigned int io_seproxyhal_touch_address_cancel(void) { - return io_seproxyhal_send_status(APDU_RESPONSE_CONDITION_NOT_SATISFIED, 0, true, true); + return io_seproxyhal_send_status(APDU_RESPONSE_CONDITION_NOT_SATISFIED, 0, true, false); } diff --git a/src_features/signMessage/ui_common_signMessage.c b/src_features/signMessage/ui_common_signMessage.c index 31bc78ded..047b21a37 100644 --- a/src_features/signMessage/ui_common_signMessage.c +++ b/src_features/signMessage/ui_common_signMessage.c @@ -22,9 +22,9 @@ unsigned int io_seproxyhal_touch_signMessage_ok(void) { if (info & CX_ECCINFO_xGTn) { G_io_apdu_buffer[0] += 2; } - return io_seproxyhal_send_status(APDU_RESPONSE_OK, 65, true, true); + return io_seproxyhal_send_status(APDU_RESPONSE_OK, 65, true, false); } unsigned int io_seproxyhal_touch_signMessage_cancel(void) { - return io_seproxyhal_send_status(APDU_RESPONSE_CONDITION_NOT_SATISFIED, 0, true, true); + return io_seproxyhal_send_status(APDU_RESPONSE_CONDITION_NOT_SATISFIED, 0, true, false); } diff --git a/src_features/signMessageEIP712_common/common_712.c b/src_features/signMessageEIP712_common/common_712.c index 53d4388f7..bdf64cbd1 100644 --- a/src_features/signMessageEIP712_common/common_712.c +++ b/src_features/signMessageEIP712_common/common_712.c @@ -53,9 +53,9 @@ unsigned int ui_712_approve_cb(void) { if (info & CX_ECCINFO_xGTn) { G_io_apdu_buffer[0] += 2; } - return io_seproxyhal_send_status(APDU_RESPONSE_OK, 65, true, true); + return io_seproxyhal_send_status(APDU_RESPONSE_OK, 65, true, false); } unsigned int ui_712_reject_cb(void) { - return io_seproxyhal_send_status(APDU_RESPONSE_CONDITION_NOT_SATISFIED, 0, true, true); + return io_seproxyhal_send_status(APDU_RESPONSE_CONDITION_NOT_SATISFIED, 0, true, false); } diff --git a/src_features/signTx/logic_signTx.c b/src_features/signTx/logic_signTx.c index 0d5ac24e7..6aba49f37 100644 --- a/src_features/signTx/logic_signTx.c +++ b/src_features/signTx/logic_signTx.c @@ -582,6 +582,7 @@ uint16_t finalizeParsing(void) { // If called from swap, the user has already validated a standard transaction // And we have already checked the fields of this transaction above if (G_called_from_swap && g_use_standard_ui) { + ui_idle(); io_seproxyhal_touch_tx_ok(); } else { // If blind-signing detected, start the warning flow beforehand diff --git a/src_features/signTx/ui_common_signTx.c b/src_features/signTx/ui_common_signTx.c index 74caf859c..c29f3672e 100644 --- a/src_features/signTx/ui_common_signTx.c +++ b/src_features/signTx/ui_common_signTx.c @@ -65,13 +65,11 @@ uint32_t io_seproxyhal_touch_tx_ok(void) { } } reset_app_context(); - // Display back the original UX - ui_idle(); return 0; // do not redraw the widget } unsigned int io_seproxyhal_touch_tx_cancel(void) { - return io_seproxyhal_send_status(APDU_RESPONSE_CONDITION_NOT_SATISFIED, 0, true, true); + return io_seproxyhal_send_status(APDU_RESPONSE_CONDITION_NOT_SATISFIED, 0, true, false); } unsigned int io_seproxyhal_touch_data_ok(void) { diff --git a/src_nbgl/ui_approve_tx.c b/src_nbgl/ui_approve_tx.c index fdbf19134..118be0a85 100644 --- a/src_nbgl/ui_approve_tx.c +++ b/src_nbgl/ui_approve_tx.c @@ -33,21 +33,15 @@ struct tx_approval_context_t { static struct tx_approval_context_t tx_approval_context; -static void reviewReject(void) { - io_seproxyhal_touch_tx_cancel(); - memset(&tx_approval_context, 0, sizeof(tx_approval_context)); -} - -static void confirmTransation(void) { - io_seproxyhal_touch_tx_ok(); - memset(&tx_approval_context, 0, sizeof(tx_approval_context)); -} - static void reviewChoice(bool confirm) { if (confirm) { - nbgl_useCaseReviewStatus(STATUS_TYPE_TRANSACTION_SIGNED, confirmTransation); + io_seproxyhal_touch_tx_ok(); + memset(&tx_approval_context, 0, sizeof(tx_approval_context)); + nbgl_useCaseReviewStatus(STATUS_TYPE_TRANSACTION_SIGNED, ui_idle); } else { - nbgl_useCaseReviewStatus(STATUS_TYPE_TRANSACTION_REJECTED, reviewReject); + io_seproxyhal_touch_tx_cancel(); + memset(&tx_approval_context, 0, sizeof(tx_approval_context)); + nbgl_useCaseReviewStatus(STATUS_TYPE_TRANSACTION_REJECTED, ui_idle); } } diff --git a/src_nbgl/ui_get_eth2_public_key.c b/src_nbgl/ui_get_eth2_public_key.c index 3be40aa33..e9b2ded0f 100644 --- a/src_nbgl/ui_get_eth2_public_key.c +++ b/src_nbgl/ui_get_eth2_public_key.c @@ -4,19 +4,13 @@ #include "ui_nbgl.h" #include "uint_common.h" -static void reviewReject(void) { - io_seproxyhal_touch_address_cancel(); -} - -static void confirmTransation(void) { - io_seproxyhal_touch_address_ok(); -} - static void reviewChoice(bool confirm) { if (confirm) { - nbgl_useCaseReviewStatus(STATUS_TYPE_ADDRESS_VERIFIED, confirmTransation); + io_seproxyhal_touch_address_ok(); + nbgl_useCaseReviewStatus(STATUS_TYPE_ADDRESS_VERIFIED, ui_idle); } else { - nbgl_useCaseReviewStatus(STATUS_TYPE_ADDRESS_REJECTED, reviewReject); + io_seproxyhal_touch_address_cancel(); + nbgl_useCaseReviewStatus(STATUS_TYPE_ADDRESS_REJECTED, ui_idle); } } diff --git a/src_nbgl/ui_get_public_key.c b/src_nbgl/ui_get_public_key.c index 855e9207c..bcaccfb78 100644 --- a/src_nbgl/ui_get_public_key.c +++ b/src_nbgl/ui_get_public_key.c @@ -1,23 +1,17 @@ -#include +#include "nbgl_use_case.h" #include "shared_context.h" #include "ui_callbacks.h" #include "ui_nbgl.h" #include "network.h" #include "network_icons.h" -static void cancel_send(void) { - io_seproxyhal_touch_address_cancel(); -} - -static void confirm_send(void) { - io_seproxyhal_touch_address_ok(); -} - static void review_choice(bool confirm) { if (confirm) { - nbgl_useCaseReviewStatus(STATUS_TYPE_ADDRESS_VERIFIED, confirm_send); + io_seproxyhal_touch_address_ok(); + nbgl_useCaseReviewStatus(STATUS_TYPE_ADDRESS_VERIFIED, ui_idle); } else { - nbgl_useCaseReviewStatus(STATUS_TYPE_ADDRESS_REJECTED, cancel_send); + io_seproxyhal_touch_address_cancel(); + nbgl_useCaseReviewStatus(STATUS_TYPE_ADDRESS_REJECTED, ui_idle); } } diff --git a/src_nbgl/ui_message_signing.c b/src_nbgl/ui_message_signing.c index 4fd171a19..ff18abb9f 100644 --- a/src_nbgl/ui_message_signing.c +++ b/src_nbgl/ui_message_signing.c @@ -1,18 +1,12 @@ #include "ui_nbgl.h" #include "ui_logic.h" -static void ui_message_712_approved(void) { - ui_712_approve(); -} - -static void ui_message_712_rejected(void) { - ui_712_reject(); -} - void ui_typed_message_review_choice(bool confirm) { if (confirm) { - nbgl_useCaseReviewStatus(STATUS_TYPE_MESSAGE_SIGNED, ui_message_712_approved); + ui_712_approve(); + nbgl_useCaseReviewStatus(STATUS_TYPE_MESSAGE_SIGNED, ui_idle); } else { - nbgl_useCaseReviewStatus(STATUS_TYPE_MESSAGE_REJECTED, ui_message_712_rejected); + ui_712_reject(); + nbgl_useCaseReviewStatus(STATUS_TYPE_MESSAGE_REJECTED, ui_idle); } } diff --git a/src_nbgl/ui_sign_message.c b/src_nbgl/ui_sign_message.c index 93312a064..efabd82d2 100644 --- a/src_nbgl/ui_sign_message.c +++ b/src_nbgl/ui_sign_message.c @@ -29,19 +29,13 @@ static bool g_skipped; static void ui_191_process_state(void); -static void reject_message(void) { - io_seproxyhal_touch_signMessage_cancel(); -} - -static void sign_message(void) { - io_seproxyhal_touch_signMessage_ok(); -} - static void ui_191_finish_cb(bool confirm) { if (confirm) { - nbgl_useCaseReviewStatus(STATUS_TYPE_MESSAGE_SIGNED, sign_message); + io_seproxyhal_touch_signMessage_ok(); + nbgl_useCaseReviewStatus(STATUS_TYPE_MESSAGE_SIGNED, ui_idle); } else { - nbgl_useCaseReviewStatus(STATUS_TYPE_MESSAGE_REJECTED, reject_message); + io_seproxyhal_touch_signMessage_cancel(); + nbgl_useCaseReviewStatus(STATUS_TYPE_MESSAGE_REJECTED, ui_idle); } }