refactor(core): Refactored acc to review suggestions

common/interfaces/flash_interface/flash_api.c

@@ -619,44 +619,37 @@ 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
-  uint8_t encoded_card_number = encode_card_number(card_number);
-  flash_ram_instance.wallets[index].cards_states &=
-      ~(encoded_card_number | encoded_card_number << 4);
+
+  // 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
-  uint8_t card_state =
-      (flash_ram_instance.wallets[index].cards_states & 0x0F) |
-      (flash_ram_instance.wallets[index].cards_states >> 4 & 0x0F);
-  return !(0x01 == (0x01 & (card_state >> (card_number - 1))));
+
+  // If both write state and attempt state for the card number are clear, then
+  // wallet is already deleted
+  bool is_already_deleted = IS_Ith_BIT_RESET(
+      flash_ram_instance.wallets[index].cards_states, card_number - 1);
+  is_already_deleted &= IS_Ith_BIT_RESET(
+      flash_ram_instance.wallets[index].cards_states, card_number - 1 + 4);
+
+  return is_already_deleted;
 }
 
 /**

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,11 @@
 #define UTIL_OUT_OF_BOUNDS (0x22)
 #define UTIL_IN_BOUNDS (0xAA)
 
+#define IS_Ith_BIT_SET(x, i) (((x) & (1 << (i))) != 0)
+#define IS_Ith_BIT_RESET(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_operations/card_write_share.c

@@ -139,13 +139,16 @@ static void write_card_pre_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();
-  uint8_t temp = encode_card_number(card_num) << 4;
+  uint8_t attempt_card_state = encode_card_number(card_num) << 4;
 
-  if (temp == (temp & wallet_for_flash->cards_states)) {
+  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 |= temp;
+  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);