Merge bitcoin/bitcoin#30212: rename TransactionError:ALREADY_IN_CHAIN

e9de0a7 doc: release note for 30212 (willcl-ark)
87b1880 rpc: clarify ALREADY_IN_CHAIN rpc errors (willcl-ark)

Pull request description:

  Closes: #19363

  Renaming this error improves clarity around the returned error both internally and externally when a transactions' outputs are already found in the utxo set (`TransactionError::ALREADY_IN_CHAIN -> TransactionError::ALREADY_IN_UTXO_SET`)

ACKs for top commit:
  tdb3:
    ACK e9de0a7
  ismaelsadeeq:
    ACK e9de0a7
  ryanofsky:
    Code review ACK e9de0a7.

Tree-SHA512: 7d2617200909790340951fe56a241448f9ce511900777cb2a712e8b9c0778a27d1f912b460f82335844224f1abb4322bc898ca076440959edade55c082a09237

Author: ryanofsky

doc/release-notes-30212.md

@@ -0,0 +1,8 @@
+RPC
+---
+
+- Previously when using the `sendrawtransaction` rpc and specifying outputs
+  that are already in the UXTO set an RPC error code `-27` with RPC error
+  text "Transaction already in block chain" was returned in response.
+  The help text has been updated to "Transaction outputs already in utxo set"
+  to more accurately describe the source of the issue.

src/common/messages.cpp

@@ -100,8 +100,8 @@ bilingual_str TransactionErrorString(const TransactionError err)
             return Untranslated("No error");
         case TransactionError::MISSING_INPUTS:
             return Untranslated("Inputs missing or spent");
-        case TransactionError::ALREADY_IN_CHAIN:
-            return Untranslated("Transaction already in block chain");
+        case TransactionError::ALREADY_IN_UTXO_SET:
+            return Untranslated("Transaction outputs already in utxo set");
         case TransactionError::MEMPOOL_REJECTED:
             return Untranslated("Transaction rejected by mempool");
         case TransactionError::MEMPOOL_ERROR:

src/node/transaction.cpp

@@ -55,7 +55,7 @@ TransactionError BroadcastTransaction(NodeContext& node, const CTransactionRef t
             const Coin& existingCoin = view.AccessCoin(COutPoint(txid, o));
             // IsSpent doesn't mean the coin is spent, it means the output doesn't exist.
             // So if the output does exist, then this transaction exists in the chain.
-            if (!existingCoin.IsSpent()) return TransactionError::ALREADY_IN_CHAIN;
+            if (!existingCoin.IsSpent()) return TransactionError::ALREADY_IN_UTXO_SET;
         }
 
         if (auto mempool_tx = node.mempool->get(txid); mempool_tx) {

src/node/types.h

@@ -19,7 +19,7 @@ namespace node {
 enum class TransactionError {
     OK, //!< No error
     MISSING_INPUTS,
-    ALREADY_IN_CHAIN,
+    ALREADY_IN_UTXO_SET,
     MEMPOOL_REJECTED,
     MEMPOOL_ERROR,
     MAX_FEE_EXCEEDED,

src/rpc/mempool.cpp

@@ -43,7 +43,7 @@ static RPCHelpMan sendrawtransaction()
         "\nThe transaction will be sent unconditionally to all peers, so using sendrawtransaction\n"
         "for manual rebroadcast may degrade privacy by leaking the transaction's origin, as\n"
         "nodes will normally not rebroadcast non-wallet transactions already in their mempool.\n"
-        "\nA specific exception, RPC_TRANSACTION_ALREADY_IN_CHAIN, may throw if the transaction cannot be added to the mempool.\n"
+        "\nA specific exception, RPC_TRANSACTION_ALREADY_IN_UTXO_SET, may throw if the transaction cannot be added to the mempool.\n"
         "\nRelated RPCs: createrawtransaction, signrawtransactionwithkey\n",
         {
             {"hexstring", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The hex string of the raw transaction"},

src/rpc/protocol.h

@@ -46,14 +46,13 @@ enum RPCErrorCode
     RPC_DESERIALIZATION_ERROR       = -22, //!< Error parsing or validating structure in raw format
     RPC_VERIFY_ERROR                = -25, //!< General error during transaction or block submission
     RPC_VERIFY_REJECTED             = -26, //!< Transaction or block was rejected by network rules
-    RPC_VERIFY_ALREADY_IN_CHAIN     = -27, //!< Transaction already in chain
+    RPC_VERIFY_ALREADY_IN_UTXO_SET  = -27, //!< Transaction already in utxo set
     RPC_IN_WARMUP                   = -28, //!< Client still warming up
     RPC_METHOD_DEPRECATED           = -32, //!< RPC method is deprecated
 
     //! Aliases for backward compatibility
     RPC_TRANSACTION_ERROR           = RPC_VERIFY_ERROR,
     RPC_TRANSACTION_REJECTED        = RPC_VERIFY_REJECTED,
-    RPC_TRANSACTION_ALREADY_IN_CHAIN= RPC_VERIFY_ALREADY_IN_CHAIN,
 
     //! P2P client errors
     RPC_CLIENT_NOT_CONNECTED        = -9,  //!< Bitcoin is not connected

src/rpc/util.cpp

@@ -399,8 +399,8 @@ RPCErrorCode RPCErrorFromTransactionError(TransactionError terr)
     switch (terr) {
         case TransactionError::MEMPOOL_REJECTED:
             return RPC_TRANSACTION_REJECTED;
-        case TransactionError::ALREADY_IN_CHAIN:
-            return RPC_TRANSACTION_ALREADY_IN_CHAIN;
+        case TransactionError::ALREADY_IN_UTXO_SET:
+            return RPC_VERIFY_ALREADY_IN_UTXO_SET;
         default: break;
     }
     return RPC_TRANSACTION_ERROR;

src/test/fuzz/kitchen_sink.cpp

@@ -23,7 +23,7 @@ using node::TransactionError;
 namespace {
 constexpr TransactionError ALL_TRANSACTION_ERROR[] = {
     TransactionError::MISSING_INPUTS,
-    TransactionError::ALREADY_IN_CHAIN,
+    TransactionError::ALREADY_IN_UTXO_SET,
     TransactionError::MEMPOOL_REJECTED,
     TransactionError::MEMPOOL_ERROR,
     TransactionError::MAX_FEE_EXCEEDED,

test/functional/rpc_rawtransaction.py

@@ -430,13 +430,13 @@ def sendrawtransaction_testmempoolaccept_tests(self):
         assert_equal(testres['allowed'], True)
         self.nodes[2].sendrawtransaction(hexstring=tx['hex'], maxfeerate='0.20000000')
 
-        self.log.info("Test sendrawtransaction/testmempoolaccept with tx already in the chain")
+        self.log.info("Test sendrawtransaction/testmempoolaccept with tx outputs already in the utxo set")
         self.generate(self.nodes[2], 1)
         for node in self.nodes:
             testres = node.testmempoolaccept([tx['hex']])[0]
             assert_equal(testres['allowed'], False)
             assert_equal(testres['reject-reason'], 'txn-already-known')
-            assert_raises_rpc_error(-27, 'Transaction already in block chain', node.sendrawtransaction, tx['hex'])
+            assert_raises_rpc_error(-27, 'Transaction outputs already in utxo set', node.sendrawtransaction, tx['hex'])
 
     def decoderawtransaction_tests(self):
         self.log.info("Test decoderawtransaction")