wallet: Add TxStateString function for debugging and logging #28544

I found this useful while debugging silent conflict between #10102 and #27469 recently

<!--e57a25ab6845829454e8d69fc972939a-->

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

<!--006a51241073e994b41acfe9ec718e94-->

Code Coverage

For detailed information about the code coverage, see the test coverage report.

<!--021abf342d371248e50ceaed478a90ca-->

Reviews

See the guideline for information on the review process.

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

<!--174a7506f384e20aa4161008e828411d-->

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #27307 (wallet: track mempool conflicts with wallet transactions by ishaanam)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

ACK dc1520b1

I'm slightly more inclined to encapsulate the string representation into a toString() per struct so we don't have to touch different functions when adding a new tx state. But.. that could be just me. Not blocking at all.

<details>

diff --git a/src/wallet/transaction.h b/src/wallet/transaction.h
--- a/src/wallet/transaction.h	(revision 42056e6a2944b3f61aed376b3317551ca1241971)
+++ b/src/wallet/transaction.h	(date 1695826678557)
@@ -29,10 +29,13 @@
     int position_in_block;
 
     explicit TxStateConfirmed(const uint256& block_hash, int height, int index) : confirmed_block_hash(block_hash), confirmed_block_height(height), position_in_block(index) {}
+
+    std::string toString() const { return strprintf("Confirmed (block=%s, index=%i)", confirmed_block_hash.ToString(), position_in_block); }
 };
 
 //! State of transaction added to mempool.
 struct TxStateInMempool {
+    std::string toString() const { return strprintf("InMempool"); }
 };
 
 //! State of rejected transaction that conflicts with a confirmed block.
@@ -41,6 +44,8 @@
     int conflicting_block_height;
 
     explicit TxStateConflicted(const uint256& block_hash, int height) : conflicting_block_hash(block_hash), conflicting_block_height(height) {}
+
+    std::string toString() const { return strprintf("Conflicted (block=%s)", conflicting_block_hash.ToString()); }
 };
 
 //! State of transaction not confirmed or conflicting with a known block and
@@ -51,6 +56,8 @@
     bool abandoned;
 
     explicit TxStateInactive(bool abandoned = false) : abandoned(abandoned) {}
+
+    std::string toString() const { return strprintf("Inactive (abandoned=%i)", abandoned); }
 };
 
 //! State of transaction loaded in an unrecognized state with unexpected hash or
@@ -62,6 +69,8 @@
     int index;
 
     TxStateUnrecognized(const uint256& block_hash, int index) : block_hash(block_hash), index(index) {}
+
+    std::string toString() const { return strprintf("Unrecognized (block=%s, index=%i)", block_hash.ToString(), index); }
 };
 
 //! All possible CWalletTx states
@@ -109,6 +118,11 @@
     }, state);
 }
 
+//! Return TxState as a string for logging or debugging.
+static inline std::string TxStateString(const TxState& state)
+{
+    return std::visit([](const auto& s) { return s.toString(); }, state);
+}
 
 /**
  * Cachable amount subdivided into watchonly and spendable parts.

</details>

re: #28544#pullrequestreview-1646937471

I'm slightly more inclined to encapsulate the string representation into a toString() per struct so we don't have to touch different functions when adding a new tx state. But.. that could be just me. Not blocking at all.

Agree this is nicer. It also lets the function work with SyncTxState variants, not just TxState.

Updated dc1520b14ea38f80ed7d4c4598beb54d86667f24 -> 8a553c94098c96cb3679468c2b460be145a0eabf (pr/statestr.1 -> pr/statestr.2, compare) with suggestion

Code ACK 8a553c9

utACK 8a553c94098c96cb3679468c2b460be145a0eabf

ACK 8a553c94098c96cb3679468c2b460be145a0eabf