carrot_impl: scan_key_image fails password cb once per batch and doesn't block other instances

src/wallet/wallet2.cpp

@@ -2380,7 +2380,8 @@ bool wallet2::verify_spend_authority_proof(const cryptonote::transaction &tx, co
 //----------------------------------------------------------------------------------------------------
 void wallet2::scan_key_image(const wallet::enote_view_incoming_scan_info_t &enote_scan_info,
   const bool pool,
-  std::optional<crypto::key_image> &ki_out)
+  std::optional<crypto::key_image> &ki_out,
+  bool &password_failure_inout)
 {
   ki_out = std::nullopt;
 
@@ -2390,18 +2391,19 @@ void wallet2::scan_key_image(const wallet::enote_view_incoming_scan_info_t &enot
   // if keys are encrypted, ask for password
   if (is_key_encryption_enabled())
   {
-    static critical_section password_lock;
+    boost::lock_guard password_failure_lock(m_refresh_mutex);
-    CRITICAL_REGION_LOCAL(password_lock);
     if (!m_encrypt_keys_after_refresh)
     {
       boost::optional<epee::wipeable_string> pwd;
-      if (m_callback)
+      if (m_callback && !password_failure_inout)
         pwd = m_callback->on_get_password(pool ? "output found in pool" : "output received");
+      password_failure_inout = true;
       THROW_WALLET_EXCEPTION_IF(!pwd, error::password_needed,
         tr("No password provided. Password is needed to compute key image for incoming enotes"));
       THROW_WALLET_EXCEPTION_IF(!verify_password(*pwd), error::password_needed,
         tr("Invalid password. Password is needed to compute key image for incoming enotes"));
       m_encrypt_keys_after_refresh.reset(new wallet_keys_unlocker(*this, &*pwd));
+      password_failure_inout = false;
     }
   }
 
@@ -2432,10 +2434,11 @@ void wallet2::process_new_transaction(
       this->m_subaddresses);
 
   // if view-incoming scan was successful, try deriving the key image
+  bool password_failure = false;
   std::vector<std::optional<crypto::key_image>> output_key_images(n_outputs);
   for (size_t i = 0; i < n_outputs; ++i)
     if (enote_scan_infos.at(i))
-      scan_key_image(*enote_scan_infos.at(i), pool, output_key_images[i]);
+      scan_key_image(*enote_scan_infos.at(i), pool, output_key_images[i], password_failure);
 
   // create output tracker cache from scratch
   // this is kind of slow, but in the cases where this function is called (i.e. not normal block syncing), it's okay
@@ -3211,8 +3214,8 @@ void wallet2::process_parsed_blocks(const uint64_t start_height, const std::vect
   // define view-incoming scan and key image derivation job
   std::vector<std::optional<wallet::enote_view_incoming_scan_info_t>> enote_scan_infos(num_tx_outputs);
   std::vector<std::optional<crypto::key_image>> output_key_images(num_tx_outputs);
-  std::atomic<bool> password_is_needed = false;
+  bool password_failure = false;
-  auto tx_scan_job = [this, &enote_scan_infos, &output_key_images, &password_is_needed]
+  auto tx_scan_job = [this, &enote_scan_infos, &output_key_images, &password_failure]
     (const cryptonote::transaction &tx, size_t tx_output_idx)
   {
     const size_t output_span_end = tx_output_idx + tx.vout.size();
@@ -3229,18 +3232,10 @@ void wallet2::process_parsed_blocks(const uint64_t start_height, const std::vect
     // if view-incoming scan was successful, try deriving the key image
     for (size_t i = tx_output_idx; i < output_span_end; ++i)
     {
-      if (enote_scan_infos.at(i))
+      if (!enote_scan_infos.at(i))
-      {
+        continue;
-        try
+
-        {
+      scan_key_image(*enote_scan_infos.at(i), /*pool=*/false, output_key_images[i], password_failure);
-          scan_key_image(*enote_scan_infos.at(i), /*pool=*/false, output_key_images[i]);
-        }
-        catch (const error::password_needed &e)
-        {
-          password_is_needed.store(true);
-          throw e;
-        }
-      }
     }
   }; //tx_scan_job
 
@@ -3264,10 +3259,11 @@ void wallet2::process_parsed_blocks(const uint64_t start_height, const std::vect
   }
   if (!scan_blocks_waiter.wait())
   {
-    THROW_WALLET_EXCEPTION_IF(password_is_needed.load(), error::password_needed);
+    THROW_WALLET_EXCEPTION_IF(password_failure, error::password_needed);
     THROW_WALLET_EXCEPTION(error::wallet_internal_error, "Unrecognized exception in enote scanning threadpool");
   }
 
+  // Start processing blockchain entries with scanned outputs
   size_t current_index = start_height;
   tx_output_idx = 0;
   for (size_t i = 0; i < blocks.size(); ++i)
@@ -3647,6 +3643,7 @@ void wallet2::update_pool_state_by_pool_query(std::vector<std::tuple<cryptonote:
   process_txs.clear();
 
   auto keys_reencryptor = epee::misc_utils::create_scope_leave_handler([&, this]() {
+    boost::lock_guard refresh_lock(m_refresh_mutex);
     m_encrypt_keys_after_refresh.reset();
   });
 
@@ -3723,6 +3720,7 @@ void wallet2::update_pool_state_from_pool_data(bool incremental, const std::vect
 {
   MTRACE("update_pool_state_from_pool_data start");
   auto keys_reencryptor = epee::misc_utils::create_scope_leave_handler([&, this]() {
+    boost::lock_guard refresh_lock(m_refresh_mutex);
     m_encrypt_keys_after_refresh.reset();
   });
 
@@ -3976,6 +3974,7 @@ void wallet2::refresh(bool trusted_daemon, uint64_t start_height, uint64_t & blo
   start_height = 0;
 
   auto keys_reencryptor = epee::misc_utils::create_scope_leave_handler([&, this]() {
+    boost::lock_guard refresh_lock(m_refresh_mutex);
     m_encrypt_keys_after_refresh.reset();
   });
 

src/wallet/wallet2.h

@@ -1894,7 +1894,25 @@ private:
     bool load_keys_buf(const std::string& keys_buf, const epee::wipeable_string& password);
     bool load_keys_buf(const std::string& keys_buf, const epee::wipeable_string& password, boost::optional<crypto::chacha_key>& keys_to_encrypt);
     void load_wallet_cache(const bool use_fs, const std::string& cache_buf = "");
-    void scan_key_image(const wallet::enote_view_incoming_scan_info_t &enote_scan_info, bool pool, std::optional<crypto::key_image> &ki_out);
+    /*!
+     * \brief Calculate key image for view-scanned enote, requesting password and decrypting spend privkey if applicable
+     * \param enote_scan_info view-scanned information for enote
+     * \param pool true iff enote was found in pool, only matters for password prompt
+     * \param[out] ki_out key image result, equal to std::nullopt when calculation failed or isn't possible
+     * \param[inout] password_failure_inout if false, then skips password request. Set to true when password callback fails
+     * \throw error::password_needed if password callback fails
+     *
+     * If the password callback is successful, then the wallet spend privkey is decrypted and the unlocker guard is
+     * stored in `m_encrypt_keys_after_refresh`. To re-encrypt the spend privkey, you will need to call
+     * `m_encrypt_keys_after_refresh.reset()`. The access to the `password_failure_inout` is thread-safe, and a batch
+     * of concurrent calls to `scan_key_image()` with the same referenced boolean in `password_failure_inout` will only
+     * call the password callback once in total on any individual failure.
+     */
+    void scan_key_image(const wallet::enote_view_incoming_scan_info_t &enote_scan_info,
+      const bool pool,
+      std::optional<crypto::key_image> &ki_out,
+      bool &password_failure_inout);
+
     void process_new_transaction(
         const crypto::hash &txid,
         const cryptonote::transaction& tx,
@@ -2174,6 +2192,8 @@ private:
     crypto::chacha_key m_cache_key;
     boost::optional<crypto::chacha_key> m_custom_background_key = boost::none;
     std::shared_ptr<wallet_keys_unlocker> m_encrypt_keys_after_refresh;
+    /// synchronizes access to m_encrypt_keys_after_refresh and password callback
+    boost::mutex m_refresh_mutex;
 
     bool m_unattended;
     bool m_devices_registered;