path: root/patches/monero/0002-store-crash-fix.patch

From 94cf21261079d6d4ceb848be3863613e98c3bc89 Mon Sep 17 00:00:00 2001
From: Czarek Nakamoto <cyjan@mrcyjanek.net>
Date: Sat, 11 May 2024 16:25:10 +0200
Subject: [PATCH 02/20] store crash fix

Monero wallet crashes (sometimes) when it is syncing,
while the proper solution (that can be seen in feather)
is to not store wallet while it is being synced, this is not
acceptable for mobile wallets where OS can just come
and kill the wallet because it felt like it.

This patch depends on the background-sync patch, but
to use it as a standalone fix grabbing the definition for the
LOCK_REFRESH macro should be enough.

tobtoht suggested:
_say you want to store every 15 minutes during background sync. you stop the refresh every 15 minutes. then do something like this in the callback:_

```
// Make sure this doesn't run in the refresh thread
onRefreshed() {
  if (hasItBeen15MinutesSinceWeStored()) {
    store();
  }

  if (shouldWeContinueRefreshing()) {
    startRefresh();
  }
}
```

which works for crashes after the wallet is initially synced
but doesn't solve the issue for wallet that are syncing (it
would just wait for it to finish before actually storing).

Also imo store() functin should store the wallet, no matter
the current state.
---
 external/randomx          |  2 +-
 src/wallet/api/wallet.cpp | 53 +++++++++++++++++++--------------------
 src/wallet/api/wallet.h   |  1 -
 src/wallet/wallet2.cpp    | 11 +++++++-
 src/wallet/wallet2.h      |  3 +++
 5 files changed, 40 insertions(+), 30 deletions(-)

diff --git a/external/randomx b/external/randomx
index 5dfeeb30e..102f8acf9 160000
--- a/external/randomx
+++ b/external/randomx
@@ -1 +1 @@
-Subproject commit 5dfeeb30ec3446ec9d348153767abc324436c56c
+Subproject commit 102f8acf90a7649ada410de5499a7ec62e49e1da
diff --git a/src/wallet/api/wallet.cpp b/src/wallet/api/wallet.cpp
index 165b21c9f..c2f4176e2 100644
--- a/src/wallet/api/wallet.cpp
+++ b/src/wallet/api/wallet.cpp
@@ -55,8 +55,8 @@ using namespace cryptonote;
 #define MONERO_DEFAULT_LOG_CATEGORY "WalletAPI"
 
 #define LOCK_REFRESH() \
-    bool refresh_enabled = m_refreshEnabled; \
-    m_refreshEnabled = false; \
+    bool refresh_enabled = m_wallet->get_refresh_enabled(); \
+    m_wallet->set_refresh_enabled(false); \
     m_wallet->stop(); \
     m_refreshCV.notify_one(); \
     boost::mutex::scoped_lock lock(m_refreshMutex); \
@@ -178,7 +178,7 @@ struct Wallet2CallbackImpl : public tools::i_wallet2_callback
     virtual void on_new_block(uint64_t height, const cryptonote::block& block)
     {
         // Don't flood the GUI with signals. On fast refresh - send signal every 1000th block
-        // get_refresh_from_block_height() returns the blockheight from when the wallet was 
+        // get_refresh_from_block_height() returns the blockheight from when the wallet was
         // created or the restore height specified when wallet was recovered
         if(height >= m_wallet->m_wallet->get_refresh_from_block_height() || height % 1000 == 0) {
             // LOG_PRINT_L3(__FUNCTION__ << ": new block. height: " << height);
@@ -379,7 +379,7 @@ bool Wallet::keyValid(const std::string &secret_key_string, const std::string &a
       error = tr("Failed to parse address");
       return false;
   }
-  
+
   cryptonote::blobdata key_data;
   if(!epee::string_tools::parse_hexstr_to_binbuff(secret_key_string, key_data) || key_data.size() != sizeof(crypto::secret_key))
   {
@@ -404,7 +404,7 @@ bool Wallet::keyValid(const std::string &secret_key_string, const std::string &a
       error = tr("key does not match address");
       return false;
   }
-  
+
   return true;
 }
 
@@ -466,7 +466,7 @@ WalletImpl::WalletImpl(NetworkType nettype, uint64_t kdf_rounds)
     m_wallet2Callback.reset(new Wallet2CallbackImpl(this));
     m_wallet->callback(m_wallet2Callback.get());
     m_refreshThreadDone = false;
-    m_refreshEnabled = false;
+    m_wallet->set_refresh_enabled(false);
     m_addressBook.reset(new AddressBookImpl(this));
     m_subaddress.reset(new SubaddressImpl(this));
     m_subaddressAccount.reset(new SubaddressAccountImpl(this));
@@ -487,7 +487,7 @@ WalletImpl::~WalletImpl()
     m_wallet->callback(NULL);
     // Pause refresh thread - prevents refresh from starting again
     WalletImpl::pauseRefresh(); // Call the method directly (not polymorphically) to protect against UB in destructor.
-    // Close wallet - stores cache and stops ongoing refresh operation 
+    // Close wallet - stores cache and stops ongoing refresh operation
     close(false); // do not store wallet as part of the closing activities
     // Stop refresh thread
     stopRefresh();
@@ -698,7 +698,7 @@ bool WalletImpl::recoverFromKeysWithPassword(const std::string &path,
            setSeedLanguage(language);
            LOG_PRINT_L1("Generated deterministic wallet from spend key with seed language: " + language);
         }
-        
+
     }
     catch (const std::exception& e) {
         setStatusError(string(tr("failed to generate new wallet: ")) + e.what());
@@ -962,6 +962,7 @@ void WalletImpl::stop()
 bool WalletImpl::store(const std::string &path)
 {
     clearStatus();
+    LOCK_REFRESH();
     try {
         if (path.empty()) {
             m_wallet->store();
@@ -1110,14 +1111,14 @@ uint64_t WalletImpl::daemonBlockChainTargetHeight() const
     } else {
         clearStatus();
     }
-    // Target height can be 0 when daemon is synced. Use blockchain height instead. 
+    // Target height can be 0 when daemon is synced. Use blockchain height instead.
     if(result == 0)
         result = daemonBlockChainHeight();
     return result;
 }
 
 bool WalletImpl::daemonSynced() const
-{   
+{
     if(connected() == Wallet::ConnectionStatus_Disconnected)
         return false;
     uint64_t blockChainHeight = daemonBlockChainHeight();
@@ -1189,14 +1190,14 @@ UnsignedTransaction *WalletImpl::loadUnsignedTx(const std::string &unsigned_file
 
     return transaction;
   }
-  
+
   // Check tx data and construct confirmation message
   std::string extra_message;
   if (!std::get<2>(transaction->m_unsigned_tx_set.transfers).empty())
     extra_message = (boost::format("%u outputs to import. ") % (unsigned)std::get<2>(transaction->m_unsigned_tx_set.transfers).size()).str();
   transaction->checkLoadedTx([&transaction](){return transaction->m_unsigned_tx_set.txes.size();}, [&transaction](size_t n)->const tools::wallet2::tx_construction_data&{return transaction->m_unsigned_tx_set.txes[n];}, extra_message);
   setStatus(transaction->status(), transaction->errorString());
-    
+
   return transaction;
 }
 
@@ -1211,7 +1212,7 @@ bool WalletImpl::submitTransaction(const string &fileName) {
     setStatus(Status_Ok, tr("Failed to load transaction from file"));
     return false;
   }
-  
+
   if(!transaction->commit()) {
     setStatusError(transaction->m_errorString);
     return false;
@@ -1220,7 +1221,7 @@ bool WalletImpl::submitTransaction(const string &fileName) {
   return true;
 }
 
-bool WalletImpl::exportKeyImages(const string &filename, bool all) 
+bool WalletImpl::exportKeyImages(const string &filename, bool all)
 {
   if (m_wallet->watch_only())
   {
@@ -1229,7 +1230,7 @@ bool WalletImpl::exportKeyImages(const string &filename, bool all)
   }
   if (checkBackgroundSync("cannot export key images"))
     return false;
-  
+
   try
   {
     if (!m_wallet->export_key_images(filename, all))
@@ -1664,7 +1665,7 @@ PendingTransaction *WalletImpl::createTransactionMultDest(const std::vector<stri
     clearStatus();
     // Pause refresh thread while creating transaction
     pauseRefresh();
-      
+
     cryptonote::address_parse_info info;
 
     uint32_t adjusted_priority = m_wallet->adjust_priority(static_cast<uint32_t>(priority));
@@ -2448,10 +2449,10 @@ void WalletImpl::refreshThreadFunc()
         }
 
         LOG_PRINT_L3(__FUNCTION__ << ": refresh lock acquired...");
-        LOG_PRINT_L3(__FUNCTION__ << ": m_refreshEnabled: " << m_refreshEnabled);
+        LOG_PRINT_L3(__FUNCTION__ << ": m_refreshEnabled: " << m_wallet->get_refresh_enabled());
         LOG_PRINT_L3(__FUNCTION__ << ": m_status: " << status());
         LOG_PRINT_L3(__FUNCTION__ << ": m_refreshShouldRescan: " << m_refreshShouldRescan);
-        if (m_refreshEnabled) {
+        if (m_wallet->get_refresh_enabled()) {
             LOG_PRINT_L3(__FUNCTION__ << ": refreshing...");
             doRefresh();
         }
@@ -2485,7 +2486,7 @@ void WalletImpl::doRefresh()
     } catch (const std::exception &e) {
         setStatusError(e.what());
         break;
-    }while(!rescan && (rescan=m_refreshShouldRescan.exchange(false))); // repeat if not rescanned and rescan was requested
+    }while(m_wallet->get_refresh_enabled() && !rescan && (rescan=m_refreshShouldRescan.exchange(false))); // repeat if not rescanned and rescan was requested
 
     if (m_wallet2Callback->getListener()) {
         m_wallet2Callback->getListener()->refreshed();
@@ -2495,9 +2496,9 @@ void WalletImpl::doRefresh()
 
 void WalletImpl::startRefresh()
 {
-    if (!m_refreshEnabled) {
+    if (!m_wallet->get_refresh_enabled()) {
         LOG_PRINT_L2(__FUNCTION__ << ": refresh started/resumed...");
-        m_refreshEnabled = true;
+        m_wallet->set_refresh_enabled(true);
         m_refreshCV.notify_one();
     }
 }
@@ -2507,7 +2508,7 @@ void WalletImpl::startRefresh()
 void WalletImpl::stopRefresh()
 {
     if (!m_refreshThreadDone) {
-        m_refreshEnabled = false;
+        m_wallet->set_refresh_enabled(false);
         m_refreshThreadDone = true;
         m_refreshCV.notify_one();
         m_refreshThread.join();
@@ -2518,9 +2519,7 @@ void WalletImpl::pauseRefresh()
 {
     LOG_PRINT_L2(__FUNCTION__ << ": refresh paused...");
     // TODO synchronize access
-    if (!m_refreshThreadDone) {
-        m_refreshEnabled = false;
-    }
+    m_wallet->set_refresh_enabled(false);
 }
 
 
@@ -2530,7 +2529,7 @@ bool WalletImpl::isNewWallet() const
     // it's the same case as if it created from scratch, i.e. we need "fast sync"
     // with the daemon (pull hashes instead of pull blocks).
     // If wallet cache is rebuilt, creation height stored in .keys is used.
-    // Watch only wallet is a copy of an existing wallet. 
+    // Watch only wallet is a copy of an existing wallet.
     return !(blockChainHeight() > 1 || m_recoveringFromSeed || m_recoveringFromDevice || m_rebuildWalletCache) && !watchOnly();
 }
 
@@ -2642,7 +2641,7 @@ void WalletImpl::hardForkInfo(uint8_t &version, uint64_t &earliest_height) const
     m_wallet->get_hard_fork_info(version, earliest_height);
 }
 
-bool WalletImpl::useForkRules(uint8_t version, int64_t early_blocks) const 
+bool WalletImpl::useForkRules(uint8_t version, int64_t early_blocks) const
 {
     return m_wallet->use_fork_rules(version,early_blocks);
 }
diff --git a/src/wallet/api/wallet.h b/src/wallet/api/wallet.h
index 1f199a72c..ac7ce2f6a 100644
--- a/src/wallet/api/wallet.h
+++ b/src/wallet/api/wallet.h
@@ -273,7 +273,6 @@ private:
     std::unique_ptr<SubaddressAccountImpl>  m_subaddressAccount;
 
     // multi-threaded refresh stuff
-    std::atomic<bool> m_refreshEnabled;
     std::atomic<bool> m_refreshThreadDone;
     std::atomic<int>  m_refreshIntervalMillis;
     std::atomic<bool> m_refreshShouldRescan;
diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp
index af1f03d2c..af876c9f3 100644
--- a/src/wallet/wallet2.cpp
+++ b/src/wallet/wallet2.cpp
@@ -1195,6 +1195,7 @@ wallet2::wallet2(network_type nettype, uint64_t kdf_rounds, bool unattended, std
   m_upper_transaction_weight_limit(0),
   m_run(true),
   m_callback(0),
+  m_refreshEnabled(false),
   m_trusted_daemon(false),
   m_nettype(nettype),
   m_multisig_rounds_passed(0),
@@ -1415,6 +1416,14 @@ bool wallet2::set_daemon(std::string daemon_address, boost::optional<epee::net_u
   return ret;
 }
 //----------------------------------------------------------------------------------------------------
+bool wallet2::get_refresh_enabled() {
+    return m_refreshEnabled;
+}
+//----------------------------------------------------------------------------------------------------
+void wallet2::set_refresh_enabled(bool val) {
+    m_refreshEnabled = val;
+}
+//----------------------------------------------------------------------------------------------------
 bool wallet2::set_proxy(const std::string &address)
 {
   return m_http_client->set_proxy(address);
@@ -4178,7 +4187,7 @@ void wallet2::refresh(bool trusted_daemon, uint64_t start_height, uint64_t & blo
   // infer when we get an incoming output
 
   bool first = true, last = false;
-  while(m_run.load(std::memory_order_relaxed) && blocks_fetched < max_blocks)
+  while(m_run.load(std::memory_order_relaxed) && blocks_fetched < max_blocks && m_refreshEnabled)
   {
     uint64_t next_blocks_start_height;
     std::vector<cryptonote::block_complete_entry> next_blocks;
diff --git a/src/wallet/wallet2.h b/src/wallet/wallet2.h
index a765dc475..92f735f96 100644
--- a/src/wallet/wallet2.h
+++ b/src/wallet/wallet2.h
@@ -1078,6 +1078,8 @@ private:
       epee::net_utils::ssl_options_t ssl_options = epee::net_utils::ssl_support_t::e_ssl_support_autodetect,
       const std::string &proxy = "");
     bool set_proxy(const std::string &address);
+    bool get_refresh_enabled();
+    void set_refresh_enabled(bool val);
 
     void stop() { m_run.store(false, std::memory_order_relaxed); m_message_store.stop(); }
 
@@ -1997,6 +1999,7 @@ private:
 
     boost::recursive_mutex m_daemon_rpc_mutex;
 
+    bool m_refreshEnabled;
     bool m_trusted_daemon;
     i_wallet2_callback* m_callback;
     hw::device::device_type m_key_device_type;
-- 
2.50.1 (Apple Git-155)