Skip to content

Commit

Permalink
refactor: simplify TxFormats common fields logic (XRPLF#4637)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
mvadari authored and ckeshava committed Sep 22, 2023
1 parent c0bbabb commit 19eb419
Showing 1 changed file with 36 additions and 50 deletions.
86 changes: 36 additions & 50 deletions src/ripple/protocol/impl/TxFormats.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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,
{
Expand All @@ -55,7 +72,6 @@ TxFormats::TxFormats()
{sfSetFlag, soeOPTIONAL},
{sfClearFlag, soeOPTIONAL},
{sfTickSize, soeOPTIONAL},
{sfTicketSequence, soeOPTIONAL},
{sfNFTokenMinter, soeOPTIONAL},
},
commonFields);
Expand All @@ -66,7 +82,6 @@ TxFormats::TxFormats()
{sfLimitAmount, soeOPTIONAL},
{sfQualityIn, soeOPTIONAL},
{sfQualityOut, soeOPTIONAL},
{sfTicketSequence, soeOPTIONAL},
},
commonFields);

Expand All @@ -77,7 +92,6 @@ TxFormats::TxFormats()
{sfTakerGets, soeREQUIRED},
{sfExpiration, soeOPTIONAL},
{sfOfferSequence, soeOPTIONAL},
{sfTicketSequence, soeOPTIONAL},
},
commonFields);

Expand All @@ -87,7 +101,6 @@ TxFormats::TxFormats()
{sfAmount, soeREQUIRED},
{sfAmount2, soeREQUIRED},
{sfTradingFee, soeREQUIRED},
{sfTicketSequence, soeOPTIONAL},
},
commonFields);

Expand All @@ -100,7 +113,6 @@ TxFormats::TxFormats()
{sfAmount2, soeOPTIONAL},
{sfEPrice, soeOPTIONAL},
{sfLPTokenOut, soeOPTIONAL},
{sfTicketSequence, soeOPTIONAL},
{sfTradingFee, soeOPTIONAL},
},
commonFields);
Expand All @@ -114,7 +126,6 @@ TxFormats::TxFormats()
{sfAmount2, soeOPTIONAL},
{sfEPrice, soeOPTIONAL},
{sfLPTokenIn, soeOPTIONAL},
{sfTicketSequence, soeOPTIONAL},
},
commonFields);

Expand All @@ -124,7 +135,6 @@ TxFormats::TxFormats()
{sfAsset, soeREQUIRED},
{sfAsset2, soeREQUIRED},
{sfTradingFee, soeREQUIRED},
{sfTicketSequence, soeOPTIONAL},
},
commonFields);

Expand All @@ -136,7 +146,6 @@ TxFormats::TxFormats()
{sfBidMin, soeOPTIONAL},
{sfBidMax, soeOPTIONAL},
{sfAuthAccounts, soeOPTIONAL},
{sfTicketSequence, soeOPTIONAL},
},
commonFields);

Expand All @@ -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);

Expand All @@ -175,7 +181,6 @@ TxFormats::TxFormats()
{sfInvoiceID, soeOPTIONAL},
{sfDestinationTag, soeOPTIONAL},
{sfDeliverMin, soeOPTIONAL},
{sfTicketSequence, soeOPTIONAL},
},
commonFields);

Expand All @@ -188,7 +193,6 @@ TxFormats::TxFormats()
{sfCancelAfter, soeOPTIONAL},
{sfFinishAfter, soeOPTIONAL},
{sfDestinationTag, soeOPTIONAL},
{sfTicketSequence, soeOPTIONAL},
},
commonFields);

Expand All @@ -199,7 +203,6 @@ TxFormats::TxFormats()
{sfOfferSequence, soeREQUIRED},
{sfFulfillment, soeOPTIONAL},
{sfCondition, soeOPTIONAL},
{sfTicketSequence, soeOPTIONAL},
},
commonFields);

Expand All @@ -208,7 +211,6 @@ TxFormats::TxFormats()
{
{sfOwner, soeREQUIRED},
{sfOfferSequence, soeREQUIRED},
{sfTicketSequence, soeOPTIONAL},
},
commonFields);

Expand All @@ -218,7 +220,7 @@ TxFormats::TxFormats()
{sfLedgerSequence, soeREQUIRED},
{sfAmendment, soeREQUIRED},
},
commonFields);
pseudoCommonFields);

add(jss::SetFee,
ttFEE,
Expand All @@ -234,7 +236,7 @@ TxFormats::TxFormats()
{sfReserveBaseDrops, soeOPTIONAL},
{sfReserveIncrementDrops, soeOPTIONAL},
},
commonFields);
pseudoCommonFields);

add(jss::UNLModify,
ttUNL_MODIFY,
Expand All @@ -243,13 +245,12 @@ TxFormats::TxFormats()
{sfLedgerSequence, soeREQUIRED},
{sfUNLModifyValidator, soeREQUIRED},
},
commonFields);
pseudoCommonFields);

add(jss::TicketCreate,
ttTICKET_CREATE,
{
{sfTicketCount, soeREQUIRED},
{sfTicketSequence, soeOPTIONAL},
},
commonFields);

Expand All @@ -260,7 +261,6 @@ TxFormats::TxFormats()
{
{sfSignerQuorum, soeREQUIRED},
{sfSignerEntries, soeOPTIONAL},
{sfTicketSequence, soeOPTIONAL},
},
commonFields);

Expand All @@ -273,7 +273,6 @@ TxFormats::TxFormats()
{sfPublicKey, soeREQUIRED},
{sfCancelAfter, soeOPTIONAL},
{sfDestinationTag, soeOPTIONAL},
{sfTicketSequence, soeOPTIONAL},
},
commonFields);

Expand All @@ -283,7 +282,6 @@ TxFormats::TxFormats()
{sfChannel, soeREQUIRED},
{sfAmount, soeREQUIRED},
{sfExpiration, soeOPTIONAL},
{sfTicketSequence, soeOPTIONAL},
},
commonFields);

Expand All @@ -295,7 +293,6 @@ TxFormats::TxFormats()
{sfBalance, soeOPTIONAL},
{sfSignature, soeOPTIONAL},
{sfPublicKey, soeOPTIONAL},
{sfTicketSequence, soeOPTIONAL},
},
commonFields);

Expand All @@ -307,7 +304,6 @@ TxFormats::TxFormats()
{sfExpiration, soeOPTIONAL},
{sfDestinationTag, soeOPTIONAL},
{sfInvoiceID, soeOPTIONAL},
{sfTicketSequence, soeOPTIONAL},
},
commonFields);

Expand All @@ -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);

Expand All @@ -334,7 +328,6 @@ TxFormats::TxFormats()
{
{sfDestination, soeREQUIRED},
{sfDestinationTag, soeOPTIONAL},
{sfTicketSequence, soeOPTIONAL},
},
commonFields);

Expand All @@ -343,7 +336,6 @@ TxFormats::TxFormats()
{
{sfAuthorize, soeOPTIONAL},
{sfUnauthorize, soeOPTIONAL},
{sfTicketSequence, soeOPTIONAL},
},
commonFields);

Expand All @@ -354,7 +346,6 @@ TxFormats::TxFormats()
{sfTransferFee, soeOPTIONAL},
{sfIssuer, soeOPTIONAL},
{sfURI, soeOPTIONAL},
{sfTicketSequence, soeOPTIONAL},
},
commonFields);

Expand All @@ -363,7 +354,6 @@ TxFormats::TxFormats()
{
{sfNFTokenID, soeREQUIRED},
{sfOwner, soeOPTIONAL},
{sfTicketSequence, soeOPTIONAL},
},
commonFields);

Expand All @@ -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);

Expand All @@ -393,15 +381,13 @@ TxFormats::TxFormats()
{sfNFTokenBuyOffer, soeOPTIONAL},
{sfNFTokenSellOffer, soeOPTIONAL},
{sfNFTokenBrokerFee, soeOPTIONAL},
{sfTicketSequence, soeOPTIONAL},
},
commonFields);

add(jss::Clawback,
ttCLAWBACK,
{
{sfAmount, soeREQUIRED},
{sfTicketSequence, soeOPTIONAL},
},
commonFields);
}
Expand Down

0 comments on commit 19eb419

Please sign in to comment.