wallet: post-#35501 cleanup in CWalletTx

- Rename arg_state to new_state in Update() declaration to match implementation
- Replace RecomputeCanonical manual loop with std::ranges::min_element
- Add variant txid validation in the deserialize constructor
- Make Init() private and have it clear all members including m_txs

Co-authored-by: Anthony Towns <aj@erisian.com.au>

src/wallet/transaction.cpp

@@ -104,24 +104,8 @@ void CWalletTx::RecomputeCanonical()
     // the least weight.
     Assert(!m_txs.empty());
 
-    // Returns true if 'a' should be preferred over 'b'
-    auto is_better = [](const CTransactionRef& a, const CTransactionRef& b) {
-        // A witnessed variant always beats a witnessless one
-        if (a->HasWitness() != b->HasWitness()) return a->HasWitness();
-        // Otherwise the lighter one wins
-        return GetTransactionWeight(*a) < GetTransactionWeight(*b);
-    };
-
-    auto it = m_txs.begin();
-    auto best_wtxid = it->first;
-    const CTransactionRef* best = &it->second;
-    it = std::next(it);
-    for (; it != m_txs.end(); it = std::next(it)) {
-        if (is_better(it->second, *best)) {
-            best = &it->second;
-            best_wtxid = it->first;
-        }
-    }
-    m_canonical_wtxid = best_wtxid;
+    m_canonical_wtxid = std::ranges::min_element(m_txs, std::less{}, [](const auto& entry) {
+                            return std::make_pair(!entry.second->HasWitness(), GetTransactionWeight(*entry.second));
+                        })->first;
 }
 } // namespace wallet

src/wallet/transaction.h

@@ -239,27 +239,22 @@ public:
         Assert(tx);
         m_canonical_wtxid = tx->GetWitnessHash();
         m_txs.emplace(tx->GetWitnessHash(), std::move(tx));
-        Init();
+        SetDefaults();
     }
 
     template <typename Stream>
     CWalletTx(deserialize_type, Stream& s, const std::map<Wtxid, CTransactionRef>& variants) : m_state(TxStateInactive{})
     {
         Unserialize(s);
+        const Txid& canonical_txid = GetHash();
+        for (const auto& [wtxid, tx] : variants) {
+            if (tx->GetHash() != canonical_txid) throw std::runtime_error("variant txid does not match wallet txid");
+        }
         // Merge witness variants
         m_txs.insert(variants.begin(), variants.end());
         Assert(m_txs.contains(GetWitnessHash()));
     }
 
-    void Init()
-    {
-        nTimeReceived = 0;
-        nTimeSmart = 0;
-        fChangeCached = false;
-        nChangeCached = 0;
-        nOrderPos = -1;
-    }
-
     TxState m_state;
 
     // Set of mempool transactions that conflict
@@ -357,7 +352,7 @@ public:
     // 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& arg_state);
+    bool Update(CTransactionRef tx, const TxState& new_state);
 
     //! make sure balances are recalculated
     void MarkDirty()
@@ -405,6 +400,22 @@ public:
     CWalletTx(CWalletTx&&) = default;
 
 private:
+    void SetDefaults()
+    {
+        nTimeReceived = 0;
+        nTimeSmart = 0;
+        fChangeCached = false;
+        nChangeCached = 0;
+        nOrderPos = -1;
+    }
+
+    void Init()
+    {
+        m_txs.clear();
+        m_canonical_wtxid = Wtxid{};
+        SetDefaults();
+    }
+
     Wtxid m_canonical_wtxid;
     std::map<Wtxid, CTransactionRef> m_txs;