From 0be2afdb9ccf1e122924fd8342228f5b205874f6 Mon Sep 17 00:00:00 2001
From: Rome Reginelli <rome@ripple.com>
Date: Wed, 4 Oct 2023 17:03:14 -0700
Subject: [PATCH] refactor: reunify transaction common fields: (#4715)

Make transactions and pseudo-transactions share the same commonFields
again. This regularizes the code in a nice way.

While this technically allows pseudo-transactions to have a
TicketSequence field, pseudo-transactions are only ever constructed by
code paths that don't add such a field, so this is not a transaction
processing change. It may be possible to add a separate check to ensure
TicketSequence (and other fields that don't make sense on
pseudo-transactions) are never added to pseudo-transactions, but that
should not be necessary. (TicketSequence is not the only common field
that can not and does not appear in pseudo-transactions.) Note:
TicketSequence is already documented as a common field.

Related: #4637

Fix #4714
---
 src/ripple/protocol/impl/TxFormats.cpp | 60 ++++++++++----------------
 1 file changed, 22 insertions(+), 38 deletions(-)

diff --git a/src/ripple/protocol/impl/TxFormats.cpp b/src/ripple/protocol/impl/TxFormats.cpp
index 3bd388640..e9e4acbe0 100644
--- a/src/ripple/protocol/impl/TxFormats.cpp
+++ b/src/ripple/protocol/impl/TxFormats.cpp
@@ -27,45 +27,29 @@ namespace ripple {
 
 TxFormats::TxFormats()
 {
-#pragma push_macro("PSEUDO_TXN_COMMON_FIELDS")
-
-    // clang-format off
-
-    #define PSEUDO_TXN_COMMON_FIELDS                        \
-        {sfTransactionType, soeREQUIRED},                   \
-        {sfFlags, soeOPTIONAL},                             \
-        {sfSourceTag, soeOPTIONAL},                         \
-        {sfAccount, soeREQUIRED},                           \
-        {sfSequence, soeREQUIRED},                          \
-        {sfPreviousTxnID, soeOPTIONAL}, /* emulate027 */    \
-        {sfLastLedgerSequence, soeOPTIONAL},                \
-        {sfAccountTxnID, soeOPTIONAL},                      \
-        {sfFee, soeREQUIRED},                               \
-        {sfOperationLimit, soeOPTIONAL},                    \
-        {sfMemos, soeOPTIONAL},                             \
-        {sfSigningPubKey, soeREQUIRED},                     \
-        {sfTxnSignature, soeOPTIONAL},                      \
-        {sfSigners, soeOPTIONAL}, /* submit_multisigned */  \
-        {sfEmitDetails, soeOPTIONAL},                       \
-        {sfFirstLedgerSequence, soeOPTIONAL},               \
-        {sfNetworkID, soeOPTIONAL},                         \
-        {sfHookParameters, soeOPTIONAL}
-
-    // clang-format on
-
-    // Fields shared by all pseudo-transaction txFormats:
-    static const std::initializer_list<SOElement> pseudoCommonFields{
-        PSEUDO_TXN_COMMON_FIELDS,
-    };
-
-    // Fields shared by all normal transaction txFormats:
+    // Fields shared by all txFormats:
     static const std::initializer_list<SOElement> commonFields{
-        PSEUDO_TXN_COMMON_FIELDS,
+        {sfTransactionType, soeREQUIRED},
+        {sfFlags, soeOPTIONAL},
+        {sfSourceTag, soeOPTIONAL},
+        {sfAccount, soeREQUIRED},
+        {sfSequence, soeREQUIRED},
+        {sfPreviousTxnID, soeOPTIONAL},  // emulate027
+        {sfLastLedgerSequence, soeOPTIONAL},
+        {sfAccountTxnID, soeOPTIONAL},
+        {sfFee, soeREQUIRED},
+        {sfOperationLimit, soeOPTIONAL},
+        {sfMemos, soeOPTIONAL},
+        {sfSigningPubKey, soeREQUIRED},
         {sfTicketSequence, soeOPTIONAL},
+        {sfTxnSignature, soeOPTIONAL},
+        {sfSigners, soeOPTIONAL},  // submit_multisigned
+        {sfEmitDetails, soeOPTIONAL},
+        {sfFirstLedgerSequence, soeOPTIONAL},
+        {sfNetworkID, soeOPTIONAL},
+        {sfHookParameters, soeOPTIONAL},
     };
 
-#pragma pop_macro("PSEUDO_TXN_COMMON_FIELDS")
-
     add(jss::AccountSet,
         ttACCOUNT_SET,
         {
@@ -244,7 +228,7 @@ TxFormats::TxFormats()
             {sfLedgerSequence, soeREQUIRED},
             {sfAmendment, soeREQUIRED},
         },
-        pseudoCommonFields);
+        commonFields);
 
     add(jss::EmitFailure,
         ttEMIT_FAILURE,
@@ -268,7 +252,7 @@ TxFormats::TxFormats()
             {sfReserveBaseDrops, soeOPTIONAL},
             {sfReserveIncrementDrops, soeOPTIONAL},
         },
-        pseudoCommonFields);
+        commonFields);
 
     add(jss::UNLModify,
         ttUNL_MODIFY,
@@ -277,7 +261,7 @@ TxFormats::TxFormats()
             {sfLedgerSequence, soeREQUIRED},
             {sfUNLModifyValidator, soeREQUIRED},
         },
-        pseudoCommonFields);
+        commonFields);
 
     add(jss::UNLReport,
         ttUNL_REPORT,