Merge pull request #446 from Cypherock/feat/verify-before-delete/PRF-…

…6558

Verify wallet before delete

common/interfaces/flash_interface/flash_api.c

@@ -619,41 +619,35 @@ bool is_wallet_locked(const uint8_t wallet_index) {
 }
 
 // TODO: return codes for illegal and all
-/**
- * @brief Update the card states for the wallet at specified index (on deletion
- * of the wallet from the given card number)
- *
- * @param index
- * @param card_number
- * @return int
- */
 int delete_from_kth_card_flash(const uint8_t index, const uint8_t card_number) {
   ASSERT(index < MAX_WALLETS_ALLOWED);
 
   get_flash_ram_instance();    // to load
-  flash_ram_instance.wallets[index].cards_states &= ~(1 << (card_number - 1));
+
+  // Reset card write state
+  RESET_Ith_BIT(flash_ram_instance.wallets[index].cards_states,
+                card_number - 1);
+  // Reset card write attempt state
+  RESET_Ith_BIT(flash_ram_instance.wallets[index].cards_states,
+                card_number - 1 + 4);
+
   flash_struct_save();
   return SUCCESS_;
 }
 
 // TODO: check for illegal arguments and all
-/**
- * @brief Tells if the wallet at specified index is already deleted from the
- * given card number
- *
- * @param index
- * @param card_number
- * @return true
- * @return false
- */
 bool card_already_deleted_flash(const uint8_t index,
                                 const uint8_t card_number) {
   ASSERT(index < MAX_WALLETS_ALLOWED);
 
   get_flash_ram_instance();    // to load
-  return !(
-      ((flash_ram_instance.wallets[index].cards_states) >> (card_number - 1)) &
-      1);
+
+  bool wallet_found_on_card = IS_Ith_BIT_SET(
+      flash_ram_instance.wallets[index].cards_states, card_number - 1);
+  bool write_attempted_on_card = IS_Ith_BIT_SET(
+      flash_ram_instance.wallets[index].cards_states, card_number - 1 + 4);
+
+  return !(wallet_found_on_card | write_attempted_on_card);
 }
 
 /**

common/interfaces/flash_interface/flash_api.h

@@ -281,8 +281,8 @@ int get_flash_wallet_share_by_name(const char *name, uint8_t *wallet_share);
 int get_flash_wallet_nonce_by_name(const char *name, uint8_t *wallet_nonce);
 
 /**
- * @brief Update the card states for the wallet at specified index (on deletion
- * of the wallet from the given card number)
+ * @brief Update the card states(write and attempt states) for the wallet at
+ * specified index (on deletion of the wallet from the given card number)
  *
  * @param index Wallet index in flash
  * @param card_number Card number to delete
@@ -294,6 +294,9 @@ int delete_from_kth_card_flash(uint8_t index, uint8_t card_number);
  * @brief Tells if the wallet at specified index is already deleted from the
  * given card number
  *
+ * @details This function checks the write state and attempt state of the wallet
+ * to be deleted.
+ *
  * @param index Wallet index in flash
  * @param card_number Card number to check
  * @return true, false

common/interfaces/flash_interface/flash_struct.h

@@ -93,8 +93,11 @@ typedef struct Flash_Wallet {
 
   uint8_t state;    // if DEFAULT_VALUE_IN_FLASH then not valid. If equal to
                     // VALID_WALLET then valid
-  uint8_t cards_states;        // ith from right ((cards_states>>i)&1) bit tells
-                               // whether card (i+1) has the share or not.
+  uint8_t cards_states;    // ith bit from right ((cards_states>>i)&1) bit tells
+                           // whether card (i+1) has the share or not.
+                           // Attempt state is also recorded on the left nibble,
+                           // recorded before card write operation is attempted
+                           // and cleared when successful.
   uint8_t is_wallet_locked;    // 1 if wallet if locked
   Flash_Pow challenge;
 } Flash_Wallet;

common/libraries/util/utils.h

@@ -60,6 +60,10 @@
 #define UTIL_OUT_OF_BOUNDS (0x22)
 #define UTIL_IN_BOUNDS (0xAA)
 
+#define IS_Ith_BIT_SET(x, i) (((x) & (1 << (i))) != 0)
+#define SET_Ith_BIT(x, i) ((x) |= (1 << (i)))
+#define RESET_Ith_BIT(x, i) ((x) &= ~(1 << (i)))
+
 /**
  * @brief Generic return codes for functions
  */

src/card_flows/card_flow_delete_wallet.c

@@ -61,6 +61,7 @@
  *****************************************************************************/
 #include "card_operations.h"
 #include "constant_texts.h"
+#include "core_error.h"
 #include "nfc.h"
 #include "ui_instruction.h"
 
@@ -144,7 +145,22 @@ card_error_type_e card_flow_delete_wallet(Wallet *selected_wallet) {
   card_delete_share_cfg_t cfg = {.wallet = selected_wallet, .card_number = 0};
   card_error_type_e error_code = 0;
 
+  card_fetch_share_config_t configuration = {0};
+  card_fetch_share_response_t response = {0};
+  char heading[MAX_HEADING_LEN] = "";
+  configuration.xcor = 0;
+  configuration.operation.expected_family_id = get_family_id();
+  configuration.frontend.msg = ui_text_place_card_below;
+  configuration.frontend.heading = heading;
+  configuration.frontend.unexpected_card_error = ui_text_wrong_card_sequence;
+  configuration.operation.skip_card_removal = true;
+  configuration.operation.buzzer_on_success = false;
+  response.card_info.tapped_family_id = NULL;
+
   for (int i = 1; i <= 4; i++) {
+    snprintf(heading, MAX_HEADING_LEN, UI_TEXT_TAP_CARD, i);
+    configuration.operation.acceptable_cards = encode_card_number(i);
+
     error_code = CARD_OPERATION_DEFAULT_INVALID;
     cfg.card_number = i;
 
@@ -154,6 +170,19 @@ card_error_type_e card_flow_delete_wallet(Wallet *selected_wallet) {
       continue;
     }
 
+    error_code = card_fetch_share(&configuration, &response);
+
+    if (CARD_OPERATION_SUCCESS != error_code) {
+      if (CARD_OPERATION_ABORT_OPERATION == error_code &&
+          SW_RECORD_NOT_FOUND == response.card_info.status) {
+        // In case wallet is not found on card, consider it as a success case as
+        // wallet is already deleted or not created on the card.
+        clear_core_error_screen();
+      } else {
+        break;
+      }
+    }
+
     // Operation to delete wallet from card and update card state on flash
     error_code = card_delete_share(&cfg);
 

src/card_flows/card_flow_delete_wallet.h

@@ -33,8 +33,8 @@
  * GLOBAL FUNCTION PROTOTYPES
  *****************************************************************************/
 /**
- * @brief This functions executes a sequential card flow to delete wallet on
- * each of the 4 X1 cards
+ * @brief This functions executes a sequential card flow to fetch(for
+ * verification) and delete wallet on each of the 4 X1 cards
  *
  * @return true If the flow completed successfully and wallet share was written
  * and read back from all 4 cards

src/card_flows/card_flow_reconstruct_wallet.c

@@ -110,7 +110,9 @@ card_error_type_e card_flow_reconstruct_wallet(uint8_t threshold,
   configuration.operation.expected_family_id = get_family_id();
   configuration.frontend.heading = ui_text_tap_1_2_cards;
   configuration.frontend.msg = ui_text_place_card_below;
+  configuration.frontend.unexpected_card_error = ui_text_tap_another_card;
   configuration.operation.skip_card_removal = false;
+  configuration.operation.buzzer_on_success = true;
 
   card_fetch_share_response_t response = {0};
   response.card_info.tapped_family_id = NULL;

src/card_operations/card_delete_share.c

@@ -130,8 +130,6 @@ card_error_type_e card_delete_share(card_delete_share_cfg_t *delete_config) {
 
   snprintf(
       heading, sizeof(heading), UI_TEXT_TAP_CARD, delete_config->card_number);
-  instruction_scr_init(ui_text_place_card_below, heading);
-
   card_data.error_type = CARD_OPERATION_DEFAULT_INVALID;
   card_data.nfc_data.retries = 5;
 

src/card_operations/card_fetch_share.c

@@ -235,7 +235,9 @@ card_error_type_e card_fetch_share(const card_fetch_share_config_t *config,
           break;
         }
 
-        buzzer_start(BUZZER_DURATION);
+        if (config->operation.buzzer_on_success) {
+          buzzer_start(BUZZER_DURATION);
+        }
 
         if (false == config->operation.skip_card_removal) {
           wait_for_card_removal();
@@ -257,7 +259,7 @@ card_error_type_e card_fetch_share(const card_fetch_share_config_t *config,
        * sequence"
        */
       if (SW_CONDITIONS_NOT_SATISFIED == card_data.nfc_data.status) {
-        error_msg = ui_text_tap_another_card;
+        error_msg = config->frontend.unexpected_card_error;
       }
 
       if (CARD_OPERATION_SUCCESS == indicate_card_error(error_msg)) {

src/card_operations/card_operation_typedefs.h

@@ -54,11 +54,13 @@ typedef struct {
   uint8_t acceptable_cards;
   bool skip_card_removal;
   const uint8_t *expected_family_id;
+  bool buzzer_on_success;
 } card_operation_config_t;
 
 typedef struct {
   const char *heading;
   const char *msg;
+  const char *unexpected_card_error;
 } card_operation_frontend_t;
 
 typedef struct {

src/card_operations/card_write_share.c

@@ -91,6 +91,15 @@ extern Wallet_shamir_data wallet_shamir_data;
  */
 static void write_card_pre_process(uint8_t card_num);
 
+/**
+ * @brief The function records a card write attempt on the flash memory and
+ * updates the state of the flash wallet accordingly.
+ *
+ * @param card_num represents the number of the card on which write operation is
+ * being attempted.
+ */
+static void record_card_write_attempt_on_flash(uint8_t card_num);
+
 /**
  * @brief Handles post-processing required during the wallet share write
  * operation on the X1 card.
@@ -128,16 +137,34 @@ static void write_card_pre_process(uint8_t card_num) {
   return;
 }
 
-static void write_card_share_post_process(uint8_t card_num) {
+static void record_card_write_attempt_on_flash(uint8_t card_num) {
   Flash_Wallet *wallet_for_flash = get_flash_wallet();
-  wallet_for_flash->cards_states = (1 << card_num) - 1;
+  uint8_t attempt_card_state = encode_card_number(card_num) << 4;
+
+  if (attempt_card_state ==
+      (attempt_card_state & wallet_for_flash->cards_states)) {
+    // Attempt card state already recorded on flash, no need to write to flash
+    // again. Reached here probably due to retry attempt on same card.
+    return;
+  }
 
+  wallet_for_flash->cards_states |= attempt_card_state;
   if (card_num == 1) {
     wallet_for_flash->state = UNVERIFIED_VALID_WALLET;
     add_wallet_to_flash(wallet_for_flash, &wallet_index);
   } else {
     put_wallet_flash(wallet_index, wallet_for_flash);
   }
+  return;
+}
+
+static void write_card_share_post_process(uint8_t card_num) {
+  Flash_Wallet *wallet_for_flash = get_flash_wallet();
+
+  wallet_for_flash->cards_states &= 0x0F;
+  wallet_for_flash->cards_states |= encode_card_number(card_num);
+
+  put_wallet_flash(wallet_index, wallet_for_flash);
 
   if (WALLET_IS_ARBITRARY_DATA(wallet.wallet_info))
     memset(((uint8_t *)wallet_shamir_data.arbitrary_data_shares) +
@@ -153,6 +180,7 @@ static void write_card_share_post_process(uint8_t card_num) {
              (card_num - 1) * wallet.arbitrary_data_size,
          0,
          wallet.arbitrary_data_size);
+  return;
 }
 
 /*****************************************************************************
@@ -180,6 +208,7 @@ bool write_card_share(uint8_t card_num, const char *heading, const char *msg) {
     card_initialize_applet(&card_data);
 
     if (CARD_OPERATION_SUCCESS == card_data.error_type) {
+      record_card_write_attempt_on_flash(card_num);
       card_data.nfc_data.status = nfc_add_wallet(&wallet);
 
       if (card_data.nfc_data.status == SW_NO_ERROR) {

src/settings/factory_reset.c

@@ -294,7 +294,7 @@ static bool safe_to_delete_wallet_share(wallet_list_t *wallets_in_vault) {
  *****************************************************************************/
 void factory_reset(void) {
   wallet_list_t wallets_in_vault = {0};
-  uint8_t valid_wallets = get_valid_wallet_meta_data_list(&wallets_in_vault);
+  uint8_t valid_wallets = get_filled_wallet_meta_data_list(&wallets_in_vault);
 
   if (0 < valid_wallets &&
       !core_scroll_page(NULL, ui_text_factory_reset_instruction, NULL)) {
@@ -329,7 +329,7 @@ void factory_reset(void) {
 
 void clear_device_data(void) {
   wallet_list_t wallets_in_vault = {0};
-  uint8_t valid_wallets = get_valid_wallet_meta_data_list(&wallets_in_vault);
+  uint8_t valid_wallets = get_filled_wallet_meta_data_list(&wallets_in_vault);
 
   if (0 < valid_wallets &&
       !core_scroll_page(NULL, ui_text_clear_device_data_instruction, NULL)) {

src/wallet/wallet_list.c

@@ -118,15 +118,16 @@ uint8_t get_wallet_list(const char *wallet_list[]) {
   return num_wallets;
 }
 
-uint8_t get_valid_wallet_meta_data_list(wallet_list_t *list) {
+uint8_t get_filled_wallet_meta_data_list(wallet_list_t *list) {
   uint8_t count = 0;
   if (NULL == list) {
     return count;
   }
 
   for (uint8_t wallet_idx = 0; wallet_idx < MAX_WALLETS_ALLOWED; wallet_idx++) {
     wallet_state state = INVALID_WALLET;
-    if (!wallet_is_filled(wallet_idx, &state) || VALID_WALLET != state) {
+    if (!wallet_is_filled(wallet_idx, &state) ||
+        VALID_WALLET_WITHOUT_DEVICE_SHARE == state) {
       continue;
     }
 

src/wallet/wallet_list.h

@@ -58,12 +58,12 @@ uint8_t get_wallet_list(const char *wallet_list[]);
 
 /**
  * @brief This API fills metadata for all the wallets present on X1 vault and
- * are in VALID_WALLET state.
+ * are not in VALID_WALLET_WITHOUT_DEVICE_SHARE state.
  *
  * @param wallet_list Refernce to buffer which will be filled by this function
  * @return uint8_t The number of wallets returned
  */
-uint8_t get_valid_wallet_meta_data_list(wallet_list_t *wallet_list);
+uint8_t get_filled_wallet_meta_data_list(wallet_list_t *wallet_list);
 
 /**
  * @brief This API searches for wallet on the flash using wallet_id as key and