From 45810057119394b22f6570f1a786703d069093d7 Mon Sep 17 00:00:00 2001 From: Alexis Grojean Date: Fri, 2 Aug 2024 12:49:23 +0200 Subject: [PATCH 1/3] Update address confirm UX + confirmation strings. --- src/getPublicKeys_ui.c | 2 +- src/signTxOutput_ui.c | 2 +- src/signTx_ui.c | 2 +- src/ui_nbgl.c | 93 +++++++++++++++++++++++++----------------- 4 files changed, 59 insertions(+), 40 deletions(-) diff --git a/src/getPublicKeys_ui.c b/src/getPublicKeys_ui.c index 038f11cf..be5ca08f 100644 --- a/src/getPublicKeys_ui.c +++ b/src/getPublicKeys_ui.c @@ -107,7 +107,7 @@ void getPublicKeys_respondOneKey_ui_runStep() if (ctx->currentPath == ctx->numPaths) { #ifdef HAVE_NBGL if (!ctx->silent_export) { - display_status("PUBLIC KEY\nEXPORTED"); + display_status("Public key\nexported"); } #endif // HAVE_NBGL keys_advanceStage(); diff --git a/src/signTxOutput_ui.c b/src/signTxOutput_ui.c index ff955d92..44010738 100644 --- a/src/signTxOutput_ui.c +++ b/src/signTxOutput_ui.c @@ -450,7 +450,7 @@ void signTxOutput_handleConfirm_ui_runStep() char msg[100] = {0}; snprintf(msg, SIZEOF(msg), "%s\n%s", subctx->ui_text1, subctx->ui_text2); ASSERT(strlen(msg) + 1 < SIZEOF(msg)); - display_confirmation(msg, "", "OUTPUT\nCONFIRMED", "Output\nrejected", this_fn, respond_with_user_reject); + display_confirmation(msg, "", "Output\nconfirmed", "Output\nrejected", this_fn, respond_with_user_reject); #endif // HAVE_BAGL } UI_STEP(HANDLE_CONFIRM_STEP_RESPOND) { diff --git a/src/signTx_ui.c b/src/signTx_ui.c index a500fcba..41b1c13d 100644 --- a/src/signTx_ui.c +++ b/src/signTx_ui.c @@ -1692,6 +1692,6 @@ void signTx_handleWitness_ui_runStep() void endTxStatus(void) { #ifdef HAVE_NBGL - display_status("TRANSACTION\nSIGNED"); + display_status("Transaction\nsigned"); #endif // HAVE_NBGL } diff --git a/src/ui_nbgl.c b/src/ui_nbgl.c index 70981cf8..826d41a8 100644 --- a/src/ui_nbgl.c +++ b/src/ui_nbgl.c @@ -32,8 +32,15 @@ enum { CONFIRMATION_STATUS_TOKEN, }; +typedef enum { + STATUS_TYPE_TRANSACTION, + STATUS_TYPE_ADDRESS, +} statusType_t; + typedef struct { - const char* confirmedStatus; // text displayed in confirmation page (after long press) + bool standardStatus; + statusType_t statusType; + const char* confirmedStatus; // text displayed in confirmation page (after long press) const char* rejectedStatus; // text displayed in rejection page (after reject confirmed) callback_t approvedCallback; callback_t rejectedCallback; @@ -123,6 +130,8 @@ static void reset_transaction_current_context(void) void nbgl_reset_transaction_full_context(void) { reset_transaction_current_context(); + uiContext.standardStatus = false; + uiContext.statusType = STATUS_TYPE_TRANSACTION; uiContext.pendingElement = 0; uiContext.lightConfirmation = false; uiContext.rejectedStatus = NULL; @@ -251,7 +260,14 @@ static void display_cancel_status(void) if (uiContext.rejectedStatus) { nbgl_useCaseStatus(uiContext.rejectedStatus, false, cancellation_status_callback); } else { - nbgl_useCaseStatus("Transaction rejected", false, cancellation_status_callback); + switch (uiContext.statusType) { + case STATUS_TYPE_TRANSACTION: + nbgl_useCaseReviewStatus(STATUS_TYPE_TRANSACTION_REJECTED, cancellation_status_callback); + break; + case STATUS_TYPE_ADDRESS: + nbgl_useCaseReviewStatus(STATUS_TYPE_ADDRESS_REJECTED, cancellation_status_callback); + break; + } } } @@ -347,6 +363,16 @@ static void confirmation_status_callback(void) if (uiContext.confirmedStatus) { nbgl_useCaseStatus(uiContext.confirmedStatus, true, ui_idle_flow); nbgl_reset_transaction_full_context(); + } else if(uiContext.standardStatus) { + switch (uiContext.statusType) { + case STATUS_TYPE_TRANSACTION: + nbgl_useCaseReviewStatus(STATUS_TYPE_TRANSACTION_SIGNED, ui_idle_flow); + break; + case STATUS_TYPE_ADDRESS: + nbgl_useCaseReviewStatus(STATUS_TYPE_ADDRESS_VERIFIED, ui_idle_flow); + break; + } + nbgl_reset_transaction_full_context(); } else { nbgl_useCaseSpinner("Processing"); } @@ -364,35 +390,6 @@ static void display_confirmation_status(void) } } -static void display_address_callback(void) -{ - uint8_t address_index = 0; - - // Address field is not displayed in pairList, so there is one element less. - uiContext.pairList.nbPairs = uiContext.currentElementCount - 1; - uiContext.pairList.pairs = tagValues; - - uiContext.confirmedStatus = "ADDRESS\nVERIFIED"; - uiContext.rejectedStatus = "Address verification\ncancelled"; - - for (uint8_t i = 0; i < uiContext.currentElementCount; i++) { - if (strcmp(uiContext.tagTitle[i], "Address")) { - tagValues[i].item = uiContext.tagTitle[i]; - tagValues[i].value = uiContext.tagContent[i]; - } else { - address_index = i; - } - } - - nbgl_useCaseAddressConfirmationExt(uiContext.tagContent[address_index], light_confirm_callback, &uiContext.pairList); - reset_transaction_current_context(); - - #ifdef HEADLESS - nbgl_refresh(); - trigger_callback(&display_confirmation_status); - #endif -} - static void trigger_callback(callback_t userAcceptCallback) { set_app_callback(userAcceptCallback); @@ -545,15 +542,37 @@ void display_choice(const char* text1, const char* text2, callback_t userAcceptC void display_address(callback_t userAcceptCallback, callback_t userRejectCallback) { TRACE("Displaying Address"); - uiContext.rejectedStatus = "Address verification\ncancelled"; - set_callbacks(userAcceptCallback, userRejectCallback); - nbgl_useCaseReviewStart(&C_cardano_64, "Verify Cardano\naddress", - NULL, "Cancel", display_address_callback, - display_cancel_status); + + uint8_t address_index = 0; + + // Address field is not displayed in pairList, so there is one element less. + uiContext.pairList.nbPairs = uiContext.currentElementCount - 1; + uiContext.pairList.pairs = tagValues; + + uiContext.standardStatus = true; + uiContext.statusType = STATUS_TYPE_ADDRESS; + + for (uint8_t i = 0; i < uiContext.currentElementCount; i++) { + if (strcmp(uiContext.tagTitle[i], "Address")) { + tagValues[i].item = uiContext.tagTitle[i]; + tagValues[i].value = uiContext.tagContent[i]; + } else { + address_index = i; + } + } + + nbgl_useCaseAddressReview(uiContext.tagContent[address_index], + &uiContext.pairList, + &C_cardano_64, + "Verify Cardano address", + NULL, + light_confirm_callback); + reset_transaction_current_context(); + #ifdef HEADLESS nbgl_refresh(); - trigger_callback(&display_address_callback); + trigger_callback(&display_confirmation_status); #endif } From 92bc9c93ecfc34d7b2e80c7d7faf8a766ef9426a Mon Sep 17 00:00:00 2001 From: Alexis Grojean Date: Fri, 2 Aug 2024 14:28:48 +0200 Subject: [PATCH 2/3] Update version. --- .github/workflows/ci.yml | 2 +- Makefile | 6 ++--- src/ui_nbgl.c | 50 ++++++++++++++++++++-------------------- 3 files changed, 29 insertions(+), 29 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 124d69e7..e203aa1a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -42,7 +42,7 @@ jobs: - name : Install yarn run: npm install --global yarn - name: Install Cardano JS Library (Ledger fork) - run: git clone https://github.com/LedgerHQ/ledgerjs-cardano-shelley.git + run: git clone https://github.com/vacuumlabs/ledgerjs-cardano-shelley.git - name : Run tests run: | BUILD_DEVICE_NAME="$(echo ${{ matrix.device }} | sed 's/nanosp/nanos2/')" diff --git a/Makefile b/Makefile index 559b95bc..e0025416 100644 --- a/Makefile +++ b/Makefile @@ -29,8 +29,8 @@ APPNAME = "Cardano ADA" # Application version APPVERSION_M = 7 -APPVERSION_N = 2 -APPVERSION_P = 0 +APPVERSION_N = 1 +APPVERSION_P = 2 APPVERSION = "$(APPVERSION_M).$(APPVERSION_N).$(APPVERSION_P)" # Application source files @@ -56,7 +56,7 @@ VARIANT_VALUES = cardano_ada DEFINES += RESET_ON_CRASH # Enabling DEBUG flag will enable PRINTF and disable optimizations -# DEVEL = 1 +DEVEL = 1 # DEFINES += HEADLESS # Enabling debug PRINTF diff --git a/src/ui_nbgl.c b/src/ui_nbgl.c index 826d41a8..79d63311 100644 --- a/src/ui_nbgl.c +++ b/src/ui_nbgl.c @@ -33,15 +33,15 @@ enum { }; typedef enum { - STATUS_TYPE_TRANSACTION, - STATUS_TYPE_ADDRESS, + STATUS_TYPE_TRANSACTION, + STATUS_TYPE_ADDRESS, } statusType_t; typedef struct { bool standardStatus; - statusType_t statusType; - const char* confirmedStatus; // text displayed in confirmation page (after long press) - const char* rejectedStatus; // text displayed in rejection page (after reject confirmed) + statusType_t statusType; + const char* confirmedStatus; // text displayed in confirmation page (after long press) + const char* rejectedStatus; // text displayed in rejection page (after reject confirmed) callback_t approvedCallback; callback_t rejectedCallback; callback_t pendingDisplayPageFn; @@ -130,8 +130,8 @@ static void reset_transaction_current_context(void) void nbgl_reset_transaction_full_context(void) { reset_transaction_current_context(); - uiContext.standardStatus = false; - uiContext.statusType = STATUS_TYPE_TRANSACTION; + uiContext.standardStatus = false; + uiContext.statusType = STATUS_TYPE_TRANSACTION; uiContext.pendingElement = 0; uiContext.lightConfirmation = false; uiContext.rejectedStatus = NULL; @@ -259,15 +259,18 @@ static void display_cancel_status(void) if (uiContext.rejectedStatus) { nbgl_useCaseStatus(uiContext.rejectedStatus, false, cancellation_status_callback); - } else { - switch (uiContext.statusType) { - case STATUS_TYPE_TRANSACTION: - nbgl_useCaseReviewStatus(STATUS_TYPE_TRANSACTION_REJECTED, cancellation_status_callback); - break; - case STATUS_TYPE_ADDRESS: - nbgl_useCaseReviewStatus(STATUS_TYPE_ADDRESS_REJECTED, cancellation_status_callback); - break; - } + } + else { + switch (uiContext.statusType) { + case STATUS_TYPE_TRANSACTION: + nbgl_useCaseReviewStatus(STATUS_TYPE_TRANSACTION_REJECTED, + cancellation_status_callback); + break; + case STATUS_TYPE_ADDRESS: + nbgl_useCaseReviewStatus(STATUS_TYPE_ADDRESS_REJECTED, + cancellation_status_callback); + break; + } } } @@ -544,14 +547,14 @@ void display_address(callback_t userAcceptCallback, callback_t userRejectCallbac TRACE("Displaying Address"); set_callbacks(userAcceptCallback, userRejectCallback); - uint8_t address_index = 0; + uint8_t address_index = 0; // Address field is not displayed in pairList, so there is one element less. uiContext.pairList.nbPairs = uiContext.currentElementCount - 1; uiContext.pairList.pairs = tagValues; - uiContext.standardStatus = true; - uiContext.statusType = STATUS_TYPE_ADDRESS; + uiContext.standardStatus = true; + uiContext.statusType = STATUS_TYPE_ADDRESS; for (uint8_t i = 0; i < uiContext.currentElementCount; i++) { if (strcmp(uiContext.tagTitle[i], "Address")) { @@ -562,12 +565,9 @@ void display_address(callback_t userAcceptCallback, callback_t userRejectCallbac } } - nbgl_useCaseAddressReview(uiContext.tagContent[address_index], - &uiContext.pairList, - &C_cardano_64, - "Verify Cardano address", - NULL, - light_confirm_callback); + nbgl_useCaseAddressReview(uiContext.tagContent[address_index], &uiContext.pairList, + &C_cardano_64, "Verify Cardano address", NULL, + light_confirm_callback); reset_transaction_current_context(); #ifdef HEADLESS From b3012cc5160309ead76a7bac104c3f01281c71e9 Mon Sep 17 00:00:00 2001 From: Alexis Grojean Date: Fri, 2 Aug 2024 15:49:57 +0200 Subject: [PATCH 3/3] PR review. --- .github/workflows/ci.yml | 2 +- Makefile | 2 +- src/ui_nbgl.c | 38 ++++++++++++++++++++++++++------------ 3 files changed, 28 insertions(+), 14 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e203aa1a..d86d0ab1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -41,7 +41,7 @@ jobs: run : apk add --update nodejs npm - name : Install yarn run: npm install --global yarn - - name: Install Cardano JS Library (Ledger fork) + - name: Install Cardano JS Library run: git clone https://github.com/vacuumlabs/ledgerjs-cardano-shelley.git - name : Run tests run: | diff --git a/Makefile b/Makefile index e0025416..8ba2715b 100644 --- a/Makefile +++ b/Makefile @@ -56,7 +56,7 @@ VARIANT_VALUES = cardano_ada DEFINES += RESET_ON_CRASH # Enabling DEBUG flag will enable PRINTF and disable optimizations -DEVEL = 1 +# DEVEL = 1 # DEFINES += HEADLESS # Enabling debug PRINTF diff --git a/src/ui_nbgl.c b/src/ui_nbgl.c index 79d63311..5a18fb3a 100644 --- a/src/ui_nbgl.c +++ b/src/ui_nbgl.c @@ -257,9 +257,13 @@ static void display_cancel_status(void) { ui_idle(); + // If rejectedStatus string is not NULL, then use it to display + // the status notification after the user has rejected the transaction. if (uiContext.rejectedStatus) { nbgl_useCaseStatus(uiContext.rejectedStatus, false, cancellation_status_callback); } + // Otherwise use the statusType to determine the status to display. + // The string is managed by the SDK. else { switch (uiContext.statusType) { case STATUS_TYPE_TRANSACTION: @@ -363,23 +367,31 @@ static void _display_choice(void) static void confirmation_status_callback(void) { + // If confirmedStatus string is not NULL, then use it to display + // the status notification after the user has confirmed the transaction. if (uiContext.confirmedStatus) { nbgl_useCaseStatus(uiContext.confirmedStatus, true, ui_idle_flow); nbgl_reset_transaction_full_context(); - } else if(uiContext.standardStatus) { - switch (uiContext.statusType) { - case STATUS_TYPE_TRANSACTION: - nbgl_useCaseReviewStatus(STATUS_TYPE_TRANSACTION_SIGNED, ui_idle_flow); - break; - case STATUS_TYPE_ADDRESS: - nbgl_useCaseReviewStatus(STATUS_TYPE_ADDRESS_VERIFIED, ui_idle_flow); - break; - } - nbgl_reset_transaction_full_context(); - } else { + // Otherwise, if the standardStatus flag is set, then use the statusType + // to determine the status to display (the string is managed by the SDK). + } + else if (uiContext.standardStatus) { + switch (uiContext.statusType) { + case STATUS_TYPE_TRANSACTION: + nbgl_useCaseReviewStatus(STATUS_TYPE_TRANSACTION_SIGNED, ui_idle_flow); + break; + case STATUS_TYPE_ADDRESS: + nbgl_useCaseReviewStatus(STATUS_TYPE_ADDRESS_VERIFIED, ui_idle_flow); + break; + } + nbgl_reset_transaction_full_context(); + // If neither of the above conditions are met, then the status notification + // display is delayed until the confirmation process is finished. + // A spinner is displayed in the meantime. + } + else { nbgl_useCaseSpinner("Processing"); } - } static void display_confirmation_status(void) @@ -446,6 +458,8 @@ void force_display(callback_t userAcceptCallback, callback_t userRejectCallback) } } +// Fill page content. If the content's number of lines exceeds the maximum number of lines per page, +// the page is displayed and the pending element is added. void fill_and_display_if_required(const char* line1, const char* line2, callback_t userAcceptCallback, callback_t userRejectCallback)