Merge bitcoin/bitcoin#21206: refactor: Make CWalletTx sync state type-safe

d8ee8f3cd32bbfefec931724f5798cbb088ceb6f refactor: Make CWalletTx sync state type-safe (Russell Yanofsky)

Pull request description:

  Current `CWalletTx` state representation makes it possible to set inconsistent states that won't be handled correctly by wallet sync code or serialized & deserialized back into the same form.

  For example, it is possible to call `setConflicted` without setting a conflicting block hash, or `setConfirmed` with no transaction index. And it's possible update individual `m_confirm` and `fInMempool` data fields without setting an overall consistent state that can be serialized and handled correctly.

  Fix this without changing behavior by using `std::variant`, instead of an enum and collection of fields, to represent sync state, so state tracking code is safer and more legible.

  This is a first step to fixing state tracking bugs https://github.com/bitcoin-core/bitcoin-devwiki/wiki/Wallet-Transaction-Conflict-Tracking, by adding an extra margin of safety that can prevent new bugs from being introduced as existing bugs are fixed.

ACKs for top commit:
  laanwj:
    re-ACK d8ee8f3cd32bbfefec931724f5798cbb088ceb6f
  jonatack:
    Code review ACK d8ee8f3cd32bbfefec931724f5798cbb088ceb6f

Tree-SHA512: b9f15e9d99dbdbdd3ef7a76764e11f66949f50e6227e284126f209e4cb106af6d55e9a9e8c7d4aa216ddc92c6d5acc6f4aa4746f209bbd77f03831b51a2841c3

1 files changed, 5 insertions, 5 deletions

diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp
index e24d1111c7..c895d5af8c 100644
--- a/src/wallet/rpcwallet.cpp
+++ b/src/wallet/rpcwallet.cpp
@@ -166,13 +166,13 @@ static void WalletTxToJSON(const CWallet& wallet, const CWalletTx& wtx, UniValue
     entry.pushKV("confirmations", confirms);
     if (wtx.IsCoinBase())
         entry.pushKV("generated", true);
-    if (confirms > 0)
+    if (auto* conf = wtx.state<TxStateConfirmed>())
     {
-        entry.pushKV("blockhash", wtx.m_confirm.hashBlock.GetHex());
-        entry.pushKV("blockheight", wtx.m_confirm.block_height);
-        entry.pushKV("blockindex", wtx.m_confirm.nIndex);
+        entry.pushKV("blockhash", conf->confirmed_block_hash.GetHex());
+        entry.pushKV("blockheight", conf->confirmed_block_height);
+        entry.pushKV("blockindex", conf->position_in_block);
         int64_t block_time;
-        CHECK_NONFATAL(chain.findBlock(wtx.m_confirm.hashBlock, FoundBlock().time(block_time)));
+        CHECK_NONFATAL(chain.findBlock(conf->confirmed_block_hash, FoundBlock().time(block_time)));
         entry.pushKV("blocktime", block_time);
     } else {
         entry.pushKV("trusted", CachedTxIsTrusted(wallet, wtx));