Merge bitcoin/bitcoin#35935: wallet: Avoid unnecessary wtxvariant rewrites

### src/wallet/export.cpp
@@ -165,7 +165,7 @@ util::Result<std::string> ExportWatchOnlyWallet(const CWallet& wallet, const fs:
                 if (!watchonly_wallet->LoadToWallet(std::move(copy_wtx))) {
                     return util::Error{strprintf(_("Error: Could not add tx %s to watchonly wallet"), txid.GetHex())};
                 }
-                watchonly_batch.WriteTx(watchonly_wallet->mapWallet.at(txid));
+                watchonly_batch.WriteFullTx(watchonly_wallet->mapWallet.at(txid));
             }
 
             // Copy address book

### src/wallet/transaction.cpp
@@ -3,6 +3,7 @@
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
 #include <wallet/transaction.h>
+#include <wallet/walletdb.h>
 
 #include <consensus/validation.h>
 #include <interfaces/chain.h>
@@ -61,16 +62,17 @@ void CWalletTx::updateState(interfaces::Chain& chain)
     if (!isConfirmed()) RecomputeCanonical();
 }
 
-bool CWalletTx::Update(CTransactionRef new_tx, const TxState& new_state)
+bool CWalletTx::Update(CTransactionRef new_tx, const TxState& new_state, WalletBatch& batch, bool metadata_changed)
 {
     Assert(new_tx);
     if (!Assume(GetHash() == new_tx->GetHash())) {
         return false;
     }
-    bool ret = false;
-    const auto& [tx_pair, inserted] = m_txs.emplace(new_tx->GetWitnessHash(), std::move(new_tx));
-    if (inserted) {
-        ret = true;
+    const auto& [tx_pair, new_variant] = m_txs.emplace(new_tx->GetWitnessHash(), std::move(new_tx));
+    if (new_variant) {
+        if (!batch.WriteWtxVariant(GetHash(), tx_pair->second)) {
+            throw std::ios_base::failure("Unable to write wtxvariant record");
+        }
     }
     const auto& [wtxid, tx] = *tx_pair;
 
@@ -79,7 +81,7 @@ bool CWalletTx::Update(CTransactionRef new_tx, const TxState& new_state)
         if (state<TxStateConfirmed>()) {
             m_canonical_wtxid = wtxid;
         }
-        ret = true;
+        metadata_changed = true;
     } else {
         assert(TxStateSerializedIndex(m_state) == TxStateSerializedIndex(new_state));
         assert(TxStateSerializedBlockHash(m_state) == TxStateSerializedBlockHash(new_state));
@@ -90,11 +92,17 @@ bool CWalletTx::Update(CTransactionRef new_tx, const TxState& new_state)
         const Wtxid prev_canonical = m_canonical_wtxid;
         RecomputeCanonical();
         if (m_canonical_wtxid != prev_canonical) {
-            ret = true;
+            metadata_changed = true;
+        }
+    }
+
+    if (metadata_changed) {
+        if (!batch.WriteTxMetadata(*this)) {
+            throw std::ios_base::failure("Unable to write tx record");
         }
     }
 
-    return ret;
+    return new_variant || metadata_changed;
 }
 
 void CWalletTx::RecomputeCanonical()

### src/wallet/transaction.h
@@ -28,6 +28,8 @@ class Chain;
 } // namespace interfaces
 
 namespace wallet {
+class WalletBatch;
+
 //! State of transaction confirmed in a block.
 struct TxStateConfirmed {
     uint256 confirmed_block_hash;
@@ -352,7 +354,7 @@ class CWalletTx
     // If the given transaction has a different wtxid, the transaction is stored if it has not been seen before.
     // The canonical wtxid is also updated. The tx that is confirmed becomes canonical. For unconfirmed txs,
     // those with witnesses are preferred, followed by least weight.
-    bool Update(CTransactionRef tx, const TxState& new_state);
+    bool Update(CTransactionRef tx, const TxState& new_state, WalletBatch& batch, bool metadata_changed);
 
     //! make sure balances are recalculated
     void MarkDirty()

### src/wallet/wallet.cpp
@@ -945,7 +945,7 @@ DBErrors CWallet::ReorderTransactions()
             nOrderPos = nOrderPosNext++;
             nOrderPosOffsets.push_back(nOrderPos);
 
-            if (!batch.WriteTx(*pwtx))
+            if (!batch.WriteTxMetadata(*pwtx))
                 return DBErrors::LOAD_FAIL;
         }
         else
@@ -963,7 +963,7 @@ DBErrors CWallet::ReorderTransactions()
                 continue;
 
             // Since we're changing the order, write it back
-            if (!batch.WriteTx(*pwtx))
+            if (!batch.WriteTxMetadata(*pwtx))
                 return DBErrors::LOAD_FAIL;
         }
     }
@@ -1015,7 +1015,7 @@ bool CWallet::MarkReplaced(const Txid& originalHash, const Txid& newHash)
     WalletBatch batch(GetDatabase());
 
     bool success = true;
-    if (!batch.WriteTx(wtx)) {
+    if (!batch.WriteTxMetadata(wtx)) {
         WalletLogPrintf("%s: Updating batch tx %s failed\n", __func__, wtx.GetHash().ToString());
         success = false;
     }
@@ -1091,11 +1091,20 @@ CWalletTx* CWallet::AddToWallet(CTransactionRef tx, const TxState& state, const
 
         // Update birth time when tx time is older than it.
         MaybeUpdateBirthTime(wtx.GetTxTime());
+
+        if (!batch.WriteFullTx(wtx)) {
+            return nullptr;
+        }
     }
 
     if (!fInsertedNew)
     {
-        fUpdated |= wtx.Update(tx, state);
+        try {
+            fUpdated |= wtx.Update(tx, state, batch, fUpdated);
+        } catch (const std::ios_base::failure& e) {
+            WalletLogPrintf("Error: Unable to write tx update, %s", e.what());
+            return nullptr;
+        }
     }
 
     // Mark inactive coinbase transactions and their descendants as abandoned
@@ -1110,7 +1119,7 @@ CWalletTx* CWallet::AddToWallet(CTransactionRef tx, const TxState& state, const
             desc_tx->m_state = inactive_state;
             // Break caches since we have changed the state
             desc_tx->MarkDirty();
-            batch.WriteTx(*desc_tx);
+            batch.WriteTxMetadata(*desc_tx);
             MarkInputsDirty(desc_tx->GetTx());
             for (unsigned int i = 0; i < desc_tx->GetTx()->vout.size(); ++i) {
                 COutPoint outpoint(desc_tx->GetHash(), i);
@@ -1132,11 +1141,6 @@ CWalletTx* CWallet::AddToWallet(CTransactionRef tx, const TxState& state, const
     }
     WalletLogPrintf("AddToWallet %s %s %s", hash.ToString(), status, TxStateString(state));
 
-    // Write to disk
-    if (fInsertedNew || fUpdated)
-        if (!batch.WriteTx(wtx))
-            return nullptr;
-
     // Break debit/credit balance caches:
     wtx.MarkDirty();
 
@@ -1398,7 +1402,7 @@ void CWallet::RecursiveUpdateTxState(WalletBatch* batch, const Txid& tx_hash, co
         TxUpdate update_state = try_updating_state(wtx);
         if (update_state != TxUpdate::UNCHANGED) {
             wtx.MarkDirty();
-            if (batch) batch->WriteTx(wtx);
+            if (batch) batch->WriteTxMetadata(wtx);
             // Iterate over all its outputs, and update those tx states as well (if applicable)
             for (unsigned int i = 0; i < wtx.GetTx()->vout.size(); ++i) {
                 std::pair<TxSpends::const_iterator, TxSpends::const_iterator> range = mapTxSpends.equal_range(COutPoint(now, i));
@@ -4048,7 +4052,7 @@ util::Result<void> CWallet::ApplyMigrationData(WalletBatch& local_wallet_batch,
                 if (!data.watchonly_wallet->LoadToWallet(std::move(copy_wtx))) {
                     return util::Error{strprintf(_("Error: Could not add watchonly tx %s to watchonly wallet"), wtx->GetHash().GetHex())};
                 }
-                watchonly_batch->WriteTx(data.watchonly_wallet->mapWallet.at(hash));
+                watchonly_batch->WriteFullTx(data.watchonly_wallet->mapWallet.at(hash));
                 // Mark as to remove from the migrated wallet only if it does not also belong to it
                 if (!is_mine) {
                     txids_to_delete.push_back(hash);
@@ -4061,7 +4065,7 @@ util::Result<void> CWallet::ApplyMigrationData(WalletBatch& local_wallet_batch,
             return util::Error{strprintf(_("Error: Transaction %s in wallet cannot be identified to belong to migrated wallets"), wtx->GetHash().GetHex())};
         }
         // Rewrite the transaction so that anything that may have changed about it in memory also persists to disk
-        local_wallet_batch.WriteTx(*wtx);
+        local_wallet_batch.WriteTxMetadata(*wtx);
     }
 
     // Do the removes

### src/wallet/walletdb.cpp
@@ -96,7 +96,7 @@ bool WalletBatch::ErasePurpose(const std::string& strAddress)
     return EraseIC(std::make_pair(DBKeys::PURPOSE, strAddress));
 }
 
-bool WalletBatch::WriteTx(const CWalletTx& wtx)
+bool WalletBatch::WriteFullTx(const CWalletTx& wtx)
 {
     const Txid txid = wtx.GetHash();
     // Persist all witness variants. Including the canonical one
@@ -118,6 +118,11 @@ bool WalletBatch::WriteWtxVariant(const Txid& txid, const CTransactionRef& tx)
     return WriteIC(std::make_pair(DBKeys::WTX_VARIANT, std::make_pair(txid, tx->GetWitnessHash())), TX_WITH_WITNESS(tx));
 }
 
+bool WalletBatch::WriteTxMetadata(const CWalletTx& wtx)
+{
+    return WriteIC(std::make_pair(DBKeys::TX, wtx.GetHash()), wtx);
+}
+
 bool WalletBatch::WriteKeyMetadata(const CKeyMetadata& meta, const CPubKey& pubkey, const bool overwrite)
 {
     return WriteIC(std::make_pair(DBKeys::KEYMETA, pubkey), meta, overwrite);

### src/wallet/walletdb.h
@@ -228,9 +228,13 @@ class WalletBatch
     bool WritePurpose(const std::string& strAddress, const std::string& purpose);
     bool ErasePurpose(const std::string& strAddress);
 
-    bool WriteTx(const CWalletTx& wtx);
+    // Write a CWalletTx and all variant witness txs (single tx record and multiple wtxvariant records)
+    bool WriteFullTx(const CWalletTx& wtx);
     bool EraseTx(Txid hash);
+    // Write a single witness variant of CWalletTx (single wtxvariant record)
     bool WriteWtxVariant(const Txid& txid, const CTransactionRef& tx);
+    // Write only the canonical witness tx and all of the tx metadata (single tx record)
+    bool WriteTxMetadata(const CWalletTx& wtx);
 
     bool WriteKeyMetadata(const CKeyMetadata& meta, const CPubKey& pubkey, bool overwrite);
     bool WriteKey(const CPubKey& vchPubKey, const CPrivKey& vchPrivKey, const CKeyMetadata &keyMeta);