wallet: guard and alert about a wallet invalid state during chain sync

-Context:
If `AddToWallet` db write fails, the method returns a wtx nullptr without
removing the recently added transaction from the wallet's map.

-Problem:
When a db write error occurs, `AddToWalletIfInvolvingMe` return false even
when the tx is on the wallet's map already --> which makes `SyncTransaction`
skip the `MarkInputsDirty` call --> which leads to a wallet invalid state
where the inputs of this new transaction are not marked dirty, while the
transaction that spends them still exist on the in-memory wallet tx map.

Plus, as we only store arriving transaction inside `AddToWalletIfInvolvingMe`
when we synchronize/scan blocks from the chain and nowhere else, it makes sense
to treat the tx db write error as a runtime error to notify the user about the
problem. Otherwise, the user will lose all the not stored transactions after a
wallet shutdown (without be able to recover them automatically on the next
startup because the chain sync would be above the block where the txs arrived).

2 files changed, 11 insertions, 1 deletions

diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 475627c76c..eb072c2c0c 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -1130,7 +1130,13 @@ bool CWallet::AddToWalletIfInvolvingMe(const CTransactionRef& ptx, const SyncTxS
             // Block disconnection override an abandoned tx as unconfirmed
             // which means user may have to call abandontransaction again
             TxState tx_state = std::visit([](auto&& s) -> TxState { return s; }, state);
-            return AddToWallet(MakeTransactionRef(tx), tx_state, /*update_wtx=*/nullptr, /*fFlushOnClose=*/false, rescanning_old_block);
+            CWalletTx* wtx = AddToWallet(MakeTransactionRef(tx), tx_state, /*update_wtx=*/nullptr, /*fFlushOnClose=*/false, rescanning_old_block);
+            if (!wtx) {
+                // Can only be nullptr if there was a db write error (missing db, read-only db or a db engine internal writing error).
+                // As we only store arriving transaction in this process, and we don't want an inconsistent state, let's throw an error.
+                throw std::runtime_error("DB error adding transaction to wallet, write failed");
+            }
+            return true;
         }
     }
     return false;
diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h
index e2c3d76438..3ebd26987b 100644
--- a/src/wallet/wallet.h
+++ b/src/wallet/wallet.h
@@ -505,6 +505,10 @@ public:
     //! @return true if wtx is changed and needs to be saved to disk, otherwise false
     using UpdateWalletTxFn = std::function<bool(CWalletTx& wtx, bool new_tx)>;
 
+    /**
+     * Add the transaction to the wallet, wrapping it up inside a CWalletTx
+     * @return the recently added wtx pointer or nullptr if there was a db write error.
+     */
     CWalletTx* AddToWallet(CTransactionRef tx, const TxState& state, const UpdateWalletTxFn& update_wtx=nullptr, bool fFlushOnClose=true, bool rescanning_old_block = false);
     bool LoadToWallet(const uint256& hash, const UpdateWalletTxFn& fill_wtx) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
     void transactionAddedToMempool(const CTransactionRef& tx, uint64_t mempool_sequence) override;