From 0af9ac92cf105d89771a93c950dbeec469a7b49a Mon Sep 17 00:00:00 2001 From: Ajinkya Rajandekar <145996984+ajinkyaraj-23@users.noreply.github.com> Date: Tue, 24 Sep 2024 11:27:24 +0100 Subject: [PATCH] Address review comments 1.(test) add skip in review class (ragger) 2. add testing for unexpected states in blindsigning flow. --- app/src/apdu_sign.c | 14 +++++++------- app/src/ui_stream_nbgl.c | 2 ++ tests/integration/touch/test_basic.py | 1 - .../touch/test_blindsign_different_modes.py | 8 ++++---- tests/integration/touch/test_blindsign_too_deep.py | 4 ++-- tests/integration/touch/utils.py | 12 ++++++++++-- 6 files changed, 25 insertions(+), 16 deletions(-) diff --git a/app/src/apdu_sign.c b/app/src/apdu_sign.c index 307f72cd1..6819c0b1e 100644 --- a/app/src/apdu_sign.c +++ b/app/src/apdu_sign.c @@ -914,10 +914,10 @@ handle_data_apdu_clear(command_t *cmd) #define OPERATION_TYPE_STR_LENGTH 22 static void -get_blindsign_type(char *type) +get_blindsign_type(char *type, size_t type_size) { TZ_PREAMBLE(("type=%s", type)); - TZ_ASSERT(strlen(type) == OPERATION_TYPE_STR_LENGTH, EXC_WRONG_PARAM); + TZ_ASSERT(type_size >= OPERATION_TYPE_STR_LENGTH, EXC_MEMORY_ERROR); // clang-format off switch (global.keys.apdu.sign.tag) { case 0x01: @@ -1018,7 +1018,7 @@ getTagValuePair(uint8_t pairIndex) } char num_buffer[DECIMAL_SIZE] = {0}; - char type[OPERATION_TYPE_STR_LENGTH] = "Micheline\nexpression"; + char type[OPERATION_TYPE_STR_LENGTH] = "Unknown types"; char hash[TZ_BASE58_BUFFER_SIZE(sizeof(global.keys.apdu.hash.final_hash))]; @@ -1047,7 +1047,7 @@ getTagValuePair(uint8_t pairIndex) (char **)&(pair.value)); } break; case SUMMARY_INDEX_TYPE: { - get_blindsign_type(type); + get_blindsign_type(type, sizeof(type)); pair.item = "Type"; ui_strings_push(type, strlen(type), (char **)&(pair.value)); } @@ -1089,7 +1089,7 @@ continue_blindsign_cb(void) useCaseTagValueList.startIndex = 0; useCaseTagValueList.nbPairs = 5; } - PRINTF("[DEBUG] SIGN Stauts: %d, start_index %d Number of pairs:%d ", + PRINTF("[DEBUG] SIGN Status: %d, start_index %d Number of pairs:%d ", global.step, useCaseTagValueList.startIndex, useCaseTagValueList.nbPairs); useCaseTagValueList.smallCaseForValue = false; @@ -1117,8 +1117,8 @@ handle_data_apdu_blind(void) #ifdef HAVE_BAGL - char type[OPERATION_TYPE_STR_LENGTH] = "Michelline\nexpression"; - get_blindsign_type(type); + char type[OPERATION_TYPE_STR_LENGTH] = "Unknown type"; + get_blindsign_type(type, sizeof(type)); tz_ui_stream_push_all(TZ_UI_STREAM_CB_NOCB, "Sign Hash", type, TZ_UI_LAYOUT_BN, TZ_UI_ICON_NONE); diff --git a/app/src/ui_stream_nbgl.c b/app/src/ui_stream_nbgl.c index 5d65b9af8..140067e6b 100644 --- a/app/src/ui_stream_nbgl.c +++ b/app/src/ui_stream_nbgl.c @@ -95,6 +95,8 @@ blindsign_choice(bool confirm) } else { tz_ui_stream_t *s = &global.stream; + TZ_ASSERT(EXC_UNEXPECTED_STATE, + global.blindsign_reason != REASON_NONE); if (global.blindsign_reason == REASON_TOO_MANY_SCREENS) { s->cb(TZ_UI_STREAM_CB_SUMMARY); } else { diff --git a/tests/integration/touch/test_basic.py b/tests/integration/touch/test_basic.py index fb91fe8c8..de2437fe5 100755 --- a/tests/integration/touch/test_basic.py +++ b/tests/integration/touch/test_basic.py @@ -14,7 +14,6 @@ # limitations under the License. from utils import tezos_app, BlindsigningStatus -import os if __name__ == "__main__": app = tezos_app(__file__) diff --git a/tests/integration/touch/test_blindsign_different_modes.py b/tests/integration/touch/test_blindsign_different_modes.py index 8c28921cd..6b4e39eca 100755 --- a/tests/integration/touch/test_blindsign_different_modes.py +++ b/tests/integration/touch/test_blindsign_different_modes.py @@ -53,7 +53,7 @@ def blindsign_common(app: TezosAppScreen): def blindsign_review_sign(app: TezosAppScreen): app.assert_screen("blindsign_warning_ledger_1") - app.review.enable_blindsign.reject() + app.review.back_to_safety.reject() app.assert_screen("summary_review_transaction") app.review.next() app.assert_screen("tbdm_blind_review_1") @@ -105,7 +105,7 @@ def blindsign_review_sign(app: TezosAppScreen): app.assert_screen("tbdm_op_0_screen_8") app.review.next() app.assert_screen("blindsign_warning_too_many_screens") - app.review.enable_blindsign.reject() + app.review.back_to_safety.reject() app.expect_apdu_return("9000") app.send_apdu("800f81ff48000000096d6573736167653137000000096d6573736167653138000000096d65737361676531397000ffdd6102321bc251e4a5190ad5b12b251069d9b4c0843d0f0103ff80ade204") blindsign_review_sign(app) @@ -117,9 +117,9 @@ def blindsign_review_sign(app: TezosAppScreen): send_payload(app, "800f01ffeb0300000000000000000000000000000000000000000000000000000000000000006b00ffdd6102321bc251e4a5190ad5b12b251069d9b4c0843d0b0104020320182716513907b6bab33f905396d031931c07e01bddd780780c1a56b9c086da6c00ffdd6102321bc251e4a5190ad5b12b251069d9b480897a0c0107c08db701000278eb8b6ab9a768579cd5146b480789650c83f28effff0d7570646174655f636f6e6669670000000607070005030a6e00ffdd6102321bc251e4a5190ad5b12b251069d9b4c08db7010d0105ff01ee572f02e5be5d097ba17369789582882e8abb87c900ffdd6102321bc2") app.review.next() app.assert_screen("tbdm_blindsign_on_screen_1") - app.welcome.settings() # skip button is at the same position as settings button on Home screen + app.review.skip() app.assert_screen("skip_review") - app.review.enable_blindsign.confirm() + app.review.enable_skip.confirm() app.expect_apdu_return("9000") app.send_apdu("800f01ffeb51e4a5190ad5b12b251069d9b48092f4010e0106000000fa000000086d65737361676530000000086d65737361676531000000086d65737361676532000000086d65737361676533000000086d65737361676534000000086d65737361676535000000086d65737361676536000000086d65737361676537000000086d65737361676538000000086d65737361676539000000096d6573736167653130000000096d6573736167653131000000096d6573736167653132000000096d6573736167653133000000096d6573736167653134000000096d6573736167653135000000096d6573736167653136") app.expect_apdu_return("9000") diff --git a/tests/integration/touch/test_blindsign_too_deep.py b/tests/integration/touch/test_blindsign_too_deep.py index 07b6be295..09c412b8b 100755 --- a/tests/integration/touch/test_blindsign_too_deep.py +++ b/tests/integration/touch/test_blindsign_too_deep.py @@ -47,8 +47,8 @@ app.review.next() app.assert_screen("tbtd_review_blindsign_on_0") - app.welcome.settings() # skip button - app.review.enable_blindsign.confirm() + app.review.skip() # skip button + app.review.enable_skip.confirm() app.assert_screen("too_deep_blindsign_warning") with app.fading_screen("loading_operation"): app.review.enable_blindsign.reject() diff --git a/tests/integration/touch/utils.py b/tests/integration/touch/utils.py index c24861e2f..50778edf1 100644 --- a/tests/integration/touch/utils.py +++ b/tests/integration/touch/utils.py @@ -28,6 +28,7 @@ from ragger.firmware.touch.element import Center from ragger.firmware.touch.screen import MetaScreen from ragger.firmware.touch.use_cases import ( + UseCaseHome, UseCaseHomeExt, UseCaseSettings as OriginalUseCaseSettings, UseCaseAddressConfirmation as OriginalUseCaseAddressConfirmation, @@ -66,8 +67,10 @@ class UseCaseReview(OriginalUseCaseReview): reject_tx: UseCaseChoice enable_expert: UseCaseChoice enable_blindsign: UseCaseChoice + enable_skip: UseCaseChoice back_to_safety: UseCaseChoice details: UseCaseViewDetails + __skip_screen: UseCaseHome _center: Center MORE_POSITIONS = { @@ -81,8 +84,10 @@ def __init__(self, client: BackendInterface, firmware: Firmware): self.enable_expert = UseCaseChoice(client, firmware) self.enable_blindsign = UseCaseChoice(client, firmware) self.back_to_safety = UseCaseChoice(client, firmware) - self._center = Center(client, firmware) - self.details = UseCaseViewDetails(client, firmware) + self.enable_skip = UseCaseChoice(client, firmware) + self._center = Center(client, firmware) + self.details = UseCaseViewDetails(client, firmware) + self.__skip_screen = UseCaseHome(client, firmware) @property def more_position(self) -> Position: @@ -97,6 +102,9 @@ def show_more(self) -> None: """Tap to show more.""" self.client.finger_touch(*self.more_position) + def skip(self) -> None: + """Press the skip button.""" + self.__skip_screen.settings() class UseCaseAddressConfirmation(OriginalUseCaseAddressConfirmation): """Extension of UseCaseAddressConfirmation for our app."""