From e93d82c1045606ba9306930985f967cb3ff7c7d4 Mon Sep 17 00:00:00 2001 From: Alexandre Paillier Date: Thu, 8 Sep 2022 18:41:49 +0200 Subject: [PATCH] Better separation between EIP-191 UI & business logic --- src_bagl/common_ui.c | 6 +- src_bagl/ui_flow.h | 2 - src_bagl/ui_flow_signMessage.c | 53 ++++++++++-- src_bagl/ui_flow_signMessage.h | 3 + src_features/signMessage/cmd_signMessage.c | 99 +++++++--------------- src_features/signMessage/sign_message.h | 4 - 6 files changed, 79 insertions(+), 88 deletions(-) diff --git a/src_bagl/common_ui.c b/src_bagl/common_ui.c index 57be492..e1449f0 100644 --- a/src_bagl/common_ui.c +++ b/src_bagl/common_ui.c @@ -32,10 +32,6 @@ void ui_display_public_key(void) { ux_flow_init(0, ux_display_public_flow, NULL); } -void ui_display_sign(void) { - ux_flow_init(0, ux_sign_flow, NULL); -} - void ui_sign_712_v0(void) { ux_flow_init(0, ux_sign_712_v0_flow, NULL); } @@ -74,4 +70,4 @@ void ui_confirm_parameter(void) { ux_flow_init(0, ux_confirm_parameter_flow, NULL); } -#endif // HAVE_BAGL \ No newline at end of file +#endif // HAVE_BAGL diff --git a/src_bagl/ui_flow.h b/src_bagl/ui_flow.h index 7b18286..184036b 100644 --- a/src_bagl/ui_flow.h +++ b/src_bagl/ui_flow.h @@ -20,8 +20,6 @@ extern const ux_flow_step_t* const ux_confirm_parameter_flow[]; extern const ux_flow_step_t* const ux_approval_allowance_flow[]; -extern const ux_flow_step_t* const ux_sign_flow[]; - extern const ux_flow_step_t* const ux_sign_712_v0_flow[]; extern const ux_flow_step_t* const ux_display_public_eth2_flow[]; diff --git a/src_bagl/ui_flow_signMessage.c b/src_bagl/ui_flow_signMessage.c index ec08ad0..b620b56 100644 --- a/src_bagl/ui_flow_signMessage.c +++ b/src_bagl/ui_flow_signMessage.c @@ -1,7 +1,36 @@ #include "shared_context.h" #include "ui_callbacks.h" +#include "ui_flow_signMessage.h" #include "sign_message.h" +static uint8_t ui_pos; + + +static void dummy_pre_cb(void) +{ + if (ui_pos == UI_191_POS_REVIEW) + { + question_switcher(); + } + else + { + ux_flow_prev(); + ui_pos = UI_191_POS_REVIEW; + } +} + +static void dummy_post_cb(void) +{ + if (ui_pos == UI_191_POS_QUESTION) + { + continue_displaying_message(); + } + else // UI_191_END + { + ui_191_switch_to_message_end(); + } +} + // clang-format off UX_STEP_NOCB( @@ -29,7 +58,7 @@ UX_STEP_INIT( UX_STEP_CB( ux_191_step_theres_more, bn, - theres_more_click_cb(), + skip_rest_of_message(), { "More to see!", "Double-click to skip" @@ -61,7 +90,7 @@ UX_STEP_CB( }); // clang-format on -UX_FLOW(ux_sign_flow, +UX_FLOW(ux_191_flow, &ux_191_step_review, &ux_191_step_message, &ux_191_step_dummy_pre, @@ -71,23 +100,33 @@ UX_FLOW(ux_sign_flow, &ux_191_step_cancel); +void ui_191_start(void) +{ + ux_flow_init(0, ux_191_flow, NULL); + ui_pos = UI_191_POS_REVIEW; +} + void ui_191_switch_to_message(void) { - ux_flow_init(0, ux_sign_flow, &ux_191_step_message); + ux_flow_init(0, ux_191_flow, &ux_191_step_message); + ui_pos = UI_191_POS_REVIEW; } void ui_191_switch_to_message_end(void) { - ux_flow_init(0, ux_sign_flow, &ux_191_step_dummy_pre); - // with pos != REVIEW, automatically goes back to previous step + // Force it to a value that will make it automatically do a prev() + ui_pos = UI_191_POS_QUESTION; + ux_flow_init(0, ux_191_flow, &ux_191_step_dummy_pre); } void ui_191_switch_to_sign(void) { - ux_flow_init(0, ux_sign_flow, &ux_191_step_sign); + ux_flow_init(0, ux_191_flow, &ux_191_step_sign); + ui_pos = UI_191_POS_END; } void ui_191_switch_to_question(void) { - ux_flow_init(0, ux_sign_flow, &ux_191_step_theres_more); + ux_flow_init(0, ux_191_flow, &ux_191_step_theres_more); + ui_pos = UI_191_POS_QUESTION; } diff --git a/src_bagl/ui_flow_signMessage.h b/src_bagl/ui_flow_signMessage.h index dd5e342..69e047b 100644 --- a/src_bagl/ui_flow_signMessage.h +++ b/src_bagl/ui_flow_signMessage.h @@ -1,6 +1,9 @@ #ifndef UI_FLOW_SIGNMESSAGE_H_ #define UI_FLOW_SIGNMESSAGE_H_ +typedef enum { UI_191_POS_REVIEW, UI_191_POS_QUESTION, UI_191_POS_END } e_ui_191_position; + +void ui_191_start(void); void ui_191_switch_to_message(void); void ui_191_switch_to_message_end(void); void ui_191_switch_to_sign(void); diff --git a/src_features/signMessage/cmd_signMessage.c b/src_features/signMessage/cmd_signMessage.c index 1dd76f7..6c85e2c 100644 --- a/src_features/signMessage/cmd_signMessage.c +++ b/src_features/signMessage/cmd_signMessage.c @@ -12,7 +12,6 @@ static struct { sign_message_state sign_state : 1; bool ui_started : 1; - ui_191_position ui_pos : 2; } states; static const char SIGN_MAGIC[] = @@ -81,42 +80,6 @@ static void reset_ui_buffer(void) UI_191_BUFFER[0] = '\0'; } -/** - * Switch to the beginning of the Message UI page - */ -static void switch_to_message(void) -{ - ui_191_switch_to_message(); - states.ui_pos = UI_191_REVIEW; -} - -/** - * Switch to the end of the Message UI page - */ -static void switch_to_message_end(void) -{ - ui_191_switch_to_message_end(); - states.ui_pos = UI_191_REVIEW; -} - -/** - * Switch to the Sign UI page - */ -static void switch_to_sign(void) -{ - ui_191_switch_to_sign(); - states.ui_pos = UI_191_END; -} - -/** - * Switch to the interactive question UI page - */ -static void switch_to_question(void) -{ - ui_191_switch_to_question(); - states.ui_pos = UI_191_QUESTION; -} - /** * Handle the data specific to the first APDU of an EIP-191 signature * @@ -165,7 +128,6 @@ static const uint8_t *first_apdu_data(const uint8_t *data, uint16_t *length) reset_ui_buffer(); states.sign_state = STATE_191_HASH_DISPLAY; states.ui_started = false; - states.ui_pos = UI_191_REVIEW; return data; } @@ -241,12 +203,12 @@ static void feed_display(void) { if (!states.ui_started) { - ui_display_sign(); + ui_191_start(); states.ui_started = true; } else { - switch_to_message(); + ui_191_switch_to_message(); } } @@ -301,7 +263,11 @@ bool handleSignPersonalMessage(uint8_t p1, { if (tmpCtx.messageSigningContext.remainingLength == 0) { - switch_to_sign(); +#ifdef NO_CONSENT + io_seproxyhal_touch_signMessage_ok(); +#else + ui_191_switch_to_sign(); +#endif } else { @@ -312,30 +278,28 @@ bool handleSignPersonalMessage(uint8_t p1, return true; } -void dummy_pre_cb(void) +/** + * Decide whether to show the question to show more of the message or not + */ +void question_switcher(void) { - if (states.ui_pos == UI_191_REVIEW) + if ((states.sign_state == STATE_191_HASH_DISPLAY) + && ((tmpCtx.messageSigningContext.remainingLength > 0) + || (unprocessed_length() > 0))) { - if ((states.sign_state == STATE_191_HASH_DISPLAY) - && ((tmpCtx.messageSigningContext.remainingLength > 0) - || (unprocessed_length() > 0))) - { - switch_to_question(); - } - else - { - // Go to Sign / Cancel - switch_to_sign(); - } + ui_191_switch_to_question(); } else { - ux_flow_prev(); - states.ui_pos = UI_191_REVIEW; + // Go to Sign / Cancel + ui_191_switch_to_sign(); } } -void theres_more_click_cb(void) +/** + * The user has decided to skip the rest of the message + */ +void skip_rest_of_message(void) { states.sign_state = STATE_191_HASH_ONLY; if (tmpCtx.messageSigningContext.remainingLength > 0) @@ -345,23 +309,18 @@ void theres_more_click_cb(void) } else { - switch_to_sign(); + ui_191_switch_to_sign(); } } -void dummy_post_cb(void) +/** + * The user has decided to see the next chunk of the message + */ +void continue_displaying_message(void) { - if (states.ui_pos == UI_191_QUESTION) + reset_ui_buffer(); + if (unprocessed_length() > 0) { - reset_ui_buffer(); - if (unprocessed_length() > 0) - { - feed_display(); - } - // TODO: respond to apdu ? - } - else // UI_191_END - { - switch_to_message_end(); + feed_display(); } } diff --git a/src_features/signMessage/sign_message.h b/src_features/signMessage/sign_message.h index 28348cd..bb369b5 100644 --- a/src_features/signMessage/sign_message.h +++ b/src_features/signMessage/sign_message.h @@ -2,7 +2,6 @@ #define SIGN_MESSAGE_H_ typedef enum { STATE_191_HASH_DISPLAY = 0, STATE_191_HASH_ONLY } sign_message_state; -typedef enum { UI_191_REVIEW = 0, UI_191_QUESTION, UI_191_END } ui_191_position; #define UI_191_BUFFER strings.tmp.tmp @@ -10,8 +9,5 @@ typedef enum { UI_191_REVIEW = 0, UI_191_QUESTION, UI_191_END } ui_191_position; void question_switcher(void); void skip_rest_of_message(void); void continue_displaying_message(void); -void dummy_pre_cb(void); -void theres_more_click_cb(void); -void dummy_post_cb(void); #endif // SIGN_MESSAGE_H_