Merge 3da69c464f into merged_master (Bitcoin PR bitcoin/bitcoin#28546)

src/wallet/transaction.cpp

@@ -4,6 +4,10 @@
 
 #include <wallet/transaction.h>
 
+#include <interfaces/chain.h>
+
+using interfaces::FoundBlock;
+
 namespace wallet {
 bool CWalletTx::IsEquivalentTo(const CWalletTx& _tx) const
 {
@@ -25,6 +29,27 @@ int64_t CWalletTx::GetTxTime() const
     return n ? n : nTimeReceived;
 }
 
+void CWalletTx::updateState(interfaces::Chain& chain)
+{
+    bool active;
+    auto lookup_block = [&](const uint256& hash, int& height, TxState& state) {
+        // If tx block (or conflicting block) was reorged out of chain
+        // while the wallet was shutdown, change tx status to UNCONFIRMED
+        // and reset block height, hash, and index. ABANDONED tx don't have
+        // associated blocks and don't need to be updated. The case where a
+        // transaction was reorged out while online and then reconfirmed
+        // while offline is covered by the rescan logic.
+        if (!chain.findBlock(hash, FoundBlock().inActiveChain(active).height(height)) || !active) {
+            state = TxStateInactive{};
+        }
+    };
+    if (auto* conf = state<TxStateConfirmed>()) {
+        lookup_block(conf->confirmed_block_hash, conf->confirmed_block_height, m_state);
+    } else if (auto* conf = state<TxStateConflicted>()) {
+        lookup_block(conf->conflicting_block_hash, conf->conflicting_block_height, m_state);
+    }
+}
+
 void CWalletTx::CopyFrom(const CWalletTx& _tx)
 {
     *this = _tx;

src/wallet/transaction.h

@@ -23,6 +23,10 @@
 #include <variant>
 #include <vector>
 
+namespace interfaces {
+class Chain;
+} // namespace interfaces
+
 namespace wallet {
 // ELEMENTS: forward declarations
 class CWallet;
@@ -331,6 +335,10 @@ public:
     template<typename T> const T* state() const { return std::get_if<T>(&m_state); }
     template<typename T> T* state() { return std::get_if<T>(&m_state); }
 
+    //! Update transaction state when attaching to a chain, filling in heights
+    //! of conflicted and confirmed blocks
+    void updateState(interfaces::Chain& chain);
+
     bool isAbandoned() const { return state<TxStateInactive>() && state<TxStateInactive>()->abandoned; }
     bool isConflicted() const { return state<TxStateConflicted>(); }
     bool isInactive() const { return state<TxStateInactive>(); }

src/wallet/wallet.cpp

@@ -1193,23 +1193,7 @@ bool CWallet::LoadToWallet(const uint256& hash, const UpdateWalletTxFn& fill_wtx
     // If wallet doesn't have a chain (e.g when using bitcoin-wallet tool),
     // don't bother to update txn.
     if (HaveChain()) {
-        bool active;
-        auto lookup_block = [&](const uint256& hash, int& height, TxState& state) {
-            // If tx block (or conflicting block) was reorged out of chain
-            // while the wallet was shutdown, change tx status to UNCONFIRMED
-            // and reset block height, hash, and index. ABANDONED tx don't have
-            // associated blocks and don't need to be updated. The case where a
-            // transaction was reorged out while online and then reconfirmed
-            // while offline is covered by the rescan logic.
-            if (!chain().findBlock(hash, FoundBlock().inActiveChain(active).height(height)) || !active) {
-                state = TxStateInactive{};
-            }
-        };
-        if (auto* conf = wtx.state<TxStateConfirmed>()) {
-            lookup_block(conf->confirmed_block_hash, conf->confirmed_block_height, wtx.m_state);
-        } else if (auto* conf = wtx.state<TxStateConflicted>()) {
-            lookup_block(conf->conflicting_block_hash, conf->conflicting_block_height, wtx.m_state);
-        }
+      wtx.updateState(chain());
     }
     if (/* insertion took place */ ins.second) {
         wtx.m_it_wtxOrdered = wtxOrdered.insert(std::make_pair(wtx.nOrderPos, &wtx));
@@ -3743,8 +3727,10 @@ int CWallet::GetTxDepthInMainChain(const CWalletTx& wtx) const
 {
     AssertLockHeld(cs_wallet);
     if (auto* conf = wtx.state<TxStateConfirmed>()) {
+        assert(conf->confirmed_block_height >= 0);
         return GetLastBlockHeight() - conf->confirmed_block_height + 1;
     } else if (auto* conf = wtx.state<TxStateConflicted>()) {
+        assert(conf->conflicting_block_height >= 0);
         return -1 * (GetLastBlockHeight() - conf->conflicting_block_height + 1);
     } else {
         return 0;

src/wallet/wallet.h

@@ -564,6 +564,13 @@ public:
      * <0  : conflicts with a transaction this deep in the blockchain
      *  0  : in memory pool, waiting to be included in a block
      * >=1 : this many blocks deep in the main chain
+     *
+     * Preconditions: it is only valid to call this function when the wallet is
+     * online and the block index is loaded. So this cannot be called by
+     * bitcoin-wallet tool code or by wallet migration code. If this is called
+     * without the wallet being online, it won't be able able to determine the
+     * the height of the last block processed, or the heights of blocks
+     * referenced in transaction, and might cause assert failures.
      */
     int GetTxDepthInMainChain(const CWalletTx& wtx) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
     bool IsTxInMainChain(const CWalletTx& wtx) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet)