refactor: simplify TxFormats common fields logic (XRPLF#4637)

Minor refactor to `TxFormats.cpp`: - Rename `commonFields` to `pseudoCommonFields` (since it is the common fields that all pseudo-transactions need) - Add a new static variable, `commonFields`, which represents all the common fields that non-pseudo transactions need (essentially everything that `pseudoCommonFields` contains, plus `sfTicketSequence`) This makes it harder to accidentally leave out `sfTicketSequence` in a new transaction.
sophiax851 · Jun 12, 2024 · 937354b · 937354b
1 parent c62303c
commit 937354b
Showing 1 changed file with 36 additions and 50 deletions.
diff --git a/src/ripple/protocol/impl/TxFormats.cpp b/src/ripple/protocol/impl/TxFormats.cpp
@@ -24,25 +24,42 @@ namespace ripple {
 
 TxFormats::TxFormats()
 {
-    // Fields shared by all 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 */  \
+        {sfNetworkID, 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:
     static const std::initializer_list<SOElement> commonFields{
-        {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
-        {sfNetworkID, soeOPTIONAL},
+        PSEUDO_TXN_COMMON_FIELDS,
+        {sfTicketSequence, soeOPTIONAL},
     };
 
+#pragma pop_macro("PSEUDO_TXN_COMMON_FIELDS")
+
     add(jss::AccountSet,
         ttACCOUNT_SET,
         {
@@ -55,7 +72,6 @@ TxFormats::TxFormats()
             {sfSetFlag, soeOPTIONAL},
             {sfClearFlag, soeOPTIONAL},
             {sfTickSize, soeOPTIONAL},
-            {sfTicketSequence, soeOPTIONAL},
             {sfNFTokenMinter, soeOPTIONAL},
         },
         commonFields);
@@ -66,7 +82,6 @@ TxFormats::TxFormats()
             {sfLimitAmount, soeOPTIONAL},
             {sfQualityIn, soeOPTIONAL},
             {sfQualityOut, soeOPTIONAL},
-            {sfTicketSequence, soeOPTIONAL},
         },
         commonFields);
 
@@ -77,7 +92,6 @@ TxFormats::TxFormats()
             {sfTakerGets, soeREQUIRED},
             {sfExpiration, soeOPTIONAL},
             {sfOfferSequence, soeOPTIONAL},
-            {sfTicketSequence, soeOPTIONAL},
         },
         commonFields);
 
@@ -87,7 +101,6 @@ TxFormats::TxFormats()
             {sfAmount, soeREQUIRED},
             {sfAmount2, soeREQUIRED},
             {sfTradingFee, soeREQUIRED},
-            {sfTicketSequence, soeOPTIONAL},
         },
         commonFields);
 
@@ -100,7 +113,6 @@ TxFormats::TxFormats()
             {sfAmount2, soeOPTIONAL},
             {sfEPrice, soeOPTIONAL},
             {sfLPTokenOut, soeOPTIONAL},
-            {sfTicketSequence, soeOPTIONAL},
             {sfTradingFee, soeOPTIONAL},
         },
         commonFields);
@@ -114,7 +126,6 @@ TxFormats::TxFormats()
             {sfAmount2, soeOPTIONAL},
             {sfEPrice, soeOPTIONAL},
             {sfLPTokenIn, soeOPTIONAL},
-            {sfTicketSequence, soeOPTIONAL},
         },
         commonFields);
 
@@ -124,7 +135,6 @@ TxFormats::TxFormats()
             {sfAsset, soeREQUIRED},
             {sfAsset2, soeREQUIRED},
             {sfTradingFee, soeREQUIRED},
-            {sfTicketSequence, soeOPTIONAL},
         },
         commonFields);
 
@@ -136,7 +146,6 @@ TxFormats::TxFormats()
             {sfBidMin, soeOPTIONAL},
             {sfBidMax, soeOPTIONAL},
             {sfAuthAccounts, soeOPTIONAL},
-            {sfTicketSequence, soeOPTIONAL},
         },
         commonFields);
 
@@ -145,23 +154,20 @@ TxFormats::TxFormats()
         {
             {sfAsset, soeREQUIRED},
             {sfAsset2, soeREQUIRED},
-            {sfTicketSequence, soeOPTIONAL},
         },
         commonFields);
 
     add(jss::OfferCancel,
         ttOFFER_CANCEL,
         {
             {sfOfferSequence, soeREQUIRED},
-            {sfTicketSequence, soeOPTIONAL},
         },
         commonFields);
 
     add(jss::SetRegularKey,
         ttREGULAR_KEY_SET,
         {
             {sfRegularKey, soeOPTIONAL},
-            {sfTicketSequence, soeOPTIONAL},
         },
         commonFields);
 
@@ -175,7 +181,6 @@ TxFormats::TxFormats()
             {sfInvoiceID, soeOPTIONAL},
             {sfDestinationTag, soeOPTIONAL},
             {sfDeliverMin, soeOPTIONAL},
-            {sfTicketSequence, soeOPTIONAL},
         },
         commonFields);
 
@@ -188,7 +193,6 @@ TxFormats::TxFormats()
             {sfCancelAfter, soeOPTIONAL},
             {sfFinishAfter, soeOPTIONAL},
             {sfDestinationTag, soeOPTIONAL},
-            {sfTicketSequence, soeOPTIONAL},
         },
         commonFields);
 
@@ -199,7 +203,6 @@ TxFormats::TxFormats()
             {sfOfferSequence, soeREQUIRED},
             {sfFulfillment, soeOPTIONAL},
             {sfCondition, soeOPTIONAL},
-            {sfTicketSequence, soeOPTIONAL},
         },
         commonFields);
 
@@ -208,7 +211,6 @@ TxFormats::TxFormats()
         {
             {sfOwner, soeREQUIRED},
             {sfOfferSequence, soeREQUIRED},
-            {sfTicketSequence, soeOPTIONAL},
         },
         commonFields);
 
@@ -218,7 +220,7 @@ TxFormats::TxFormats()
             {sfLedgerSequence, soeREQUIRED},
             {sfAmendment, soeREQUIRED},
         },
-        commonFields);
+        pseudoCommonFields);
 
     add(jss::SetFee,
         ttFEE,
@@ -234,7 +236,7 @@ TxFormats::TxFormats()
             {sfReserveBaseDrops, soeOPTIONAL},
             {sfReserveIncrementDrops, soeOPTIONAL},
         },
-        commonFields);
+        pseudoCommonFields);
 
     add(jss::UNLModify,
         ttUNL_MODIFY,
@@ -243,13 +245,12 @@ TxFormats::TxFormats()
             {sfLedgerSequence, soeREQUIRED},
             {sfUNLModifyValidator, soeREQUIRED},
         },
-        commonFields);
+        pseudoCommonFields);
 
     add(jss::TicketCreate,
         ttTICKET_CREATE,
         {
             {sfTicketCount, soeREQUIRED},
-            {sfTicketSequence, soeOPTIONAL},
         },
         commonFields);
 
@@ -260,7 +261,6 @@ TxFormats::TxFormats()
         {
             {sfSignerQuorum, soeREQUIRED},
             {sfSignerEntries, soeOPTIONAL},
-            {sfTicketSequence, soeOPTIONAL},
         },
         commonFields);
 
@@ -273,7 +273,6 @@ TxFormats::TxFormats()
             {sfPublicKey, soeREQUIRED},
             {sfCancelAfter, soeOPTIONAL},
             {sfDestinationTag, soeOPTIONAL},
-            {sfTicketSequence, soeOPTIONAL},
         },
         commonFields);
 
@@ -283,7 +282,6 @@ TxFormats::TxFormats()
             {sfChannel, soeREQUIRED},
             {sfAmount, soeREQUIRED},
             {sfExpiration, soeOPTIONAL},
-            {sfTicketSequence, soeOPTIONAL},
         },
         commonFields);
 
@@ -295,7 +293,6 @@ TxFormats::TxFormats()
             {sfBalance, soeOPTIONAL},
             {sfSignature, soeOPTIONAL},
             {sfPublicKey, soeOPTIONAL},
-            {sfTicketSequence, soeOPTIONAL},
         },
         commonFields);
 
@@ -307,7 +304,6 @@ TxFormats::TxFormats()
             {sfExpiration, soeOPTIONAL},
             {sfDestinationTag, soeOPTIONAL},
             {sfInvoiceID, soeOPTIONAL},
-            {sfTicketSequence, soeOPTIONAL},
         },
         commonFields);
 
@@ -317,15 +313,13 @@ TxFormats::TxFormats()
             {sfCheckID, soeREQUIRED},
             {sfAmount, soeOPTIONAL},
             {sfDeliverMin, soeOPTIONAL},
-            {sfTicketSequence, soeOPTIONAL},
         },
         commonFields);
 
     add(jss::CheckCancel,
         ttCHECK_CANCEL,
         {
             {sfCheckID, soeREQUIRED},
-            {sfTicketSequence, soeOPTIONAL},
         },
         commonFields);
 
@@ -334,7 +328,6 @@ TxFormats::TxFormats()
         {
             {sfDestination, soeREQUIRED},
             {sfDestinationTag, soeOPTIONAL},
-            {sfTicketSequence, soeOPTIONAL},
         },
         commonFields);
 
@@ -343,7 +336,6 @@ TxFormats::TxFormats()
         {
             {sfAuthorize, soeOPTIONAL},
             {sfUnauthorize, soeOPTIONAL},
-            {sfTicketSequence, soeOPTIONAL},
         },
         commonFields);
 
@@ -354,7 +346,6 @@ TxFormats::TxFormats()
             {sfTransferFee, soeOPTIONAL},
             {sfIssuer, soeOPTIONAL},
             {sfURI, soeOPTIONAL},
-            {sfTicketSequence, soeOPTIONAL},
         },
         commonFields);
 
@@ -363,7 +354,6 @@ TxFormats::TxFormats()
         {
             {sfNFTokenID, soeREQUIRED},
             {sfOwner, soeOPTIONAL},
-            {sfTicketSequence, soeOPTIONAL},
         },
         commonFields);
 
@@ -375,15 +365,13 @@ TxFormats::TxFormats()
             {sfDestination, soeOPTIONAL},
             {sfOwner, soeOPTIONAL},
             {sfExpiration, soeOPTIONAL},
-            {sfTicketSequence, soeOPTIONAL},
         },
         commonFields);
 
     add(jss::NFTokenCancelOffer,
         ttNFTOKEN_CANCEL_OFFER,
         {
             {sfNFTokenOffers, soeREQUIRED},
-            {sfTicketSequence, soeOPTIONAL},
         },
         commonFields);
 
@@ -393,15 +381,13 @@ TxFormats::TxFormats()
             {sfNFTokenBuyOffer, soeOPTIONAL},
             {sfNFTokenSellOffer, soeOPTIONAL},
             {sfNFTokenBrokerFee, soeOPTIONAL},
-            {sfTicketSequence, soeOPTIONAL},
         },
         commonFields);
 
     add(jss::Clawback,
         ttCLAWBACK,
         {
             {sfAmount, soeREQUIRED},
-            {sfTicketSequence, soeOPTIONAL},
         },
         commonFields);
 }