From ec8626046bfb6e11145c4ab65149daba5518a812 Mon Sep 17 00:00:00 2001 From: Scott Determan Date: Thu, 5 Oct 2023 18:29:23 -0400 Subject: [PATCH 01/15] refactor: reduce boilerplate in applySteps: (#4710) When a new transactor is added, there are several places in applySteps that need to be modified. This patch refactors the code so only one function needs to be modified. --- src/ripple/app/tx/impl/applySteps.cpp | 624 +++++++------------------- 1 file changed, 170 insertions(+), 454 deletions(-) diff --git a/src/ripple/app/tx/impl/applySteps.cpp b/src/ripple/app/tx/impl/applySteps.cpp index d4d9e72a830..4c882f3fb8a 100644 --- a/src/ripple/app/tx/impl/applySteps.cpp +++ b/src/ripple/app/tx/impl/applySteps.cpp @@ -50,371 +50,240 @@ #include #include +#include + namespace ripple { -// Templates so preflight does the right thing with T::ConsequencesFactory. -// -// This could be done more easily using if constexpr, but Visual Studio -// 2017 doesn't handle if constexpr correctly. So once we're no longer -// building with Visual Studio 2017 we can consider replacing the four -// templates with a single template function that uses if constexpr. -// -// For Transactor::Normal -template < - class T, - std::enable_if_t = 0> -TxConsequences -consequences_helper(PreflightContext const& ctx) -{ - return TxConsequences(ctx.tx); -}; +namespace { -// For Transactor::Blocker -template < - class T, - std::enable_if_t = 0> -TxConsequences -consequences_helper(PreflightContext const& ctx) -{ - return TxConsequences(ctx.tx, TxConsequences::blocker); -}; - -// For Transactor::Custom -template < - class T, - std::enable_if_t = 0> -TxConsequences -consequences_helper(PreflightContext const& ctx) +struct UnknownTxnType : std::exception { - return T::makeTxConsequences(ctx); + TxType txnType; + UnknownTxnType(TxType t) : txnType{t} + { + } }; -template -std::pair -invoke_preflight_helper(PreflightContext const& ctx) +// Call a lambda with the concrete transaction type as a template parameter +// throw an "UnknownTxnType" exception on error +template +auto +with_txn_type(TxType txnType, F&& f) { - auto const tec = T::preflight(ctx); - return { - tec, - isTesSuccess(tec) ? consequences_helper(ctx) : TxConsequences{tec}}; -} - -static std::pair -invoke_preflight(PreflightContext const& ctx) -{ - switch (ctx.tx.getTxnType()) + switch (txnType) { case ttACCOUNT_DELETE: - return invoke_preflight_helper(ctx); + return f.template operator()(); case ttACCOUNT_SET: - return invoke_preflight_helper(ctx); + return f.template operator()(); case ttCHECK_CANCEL: - return invoke_preflight_helper(ctx); + return f.template operator()(); case ttCHECK_CASH: - return invoke_preflight_helper(ctx); + return f.template operator()(); case ttCHECK_CREATE: - return invoke_preflight_helper(ctx); + return f.template operator()(); case ttDEPOSIT_PREAUTH: - return invoke_preflight_helper(ctx); + return f.template operator()(); case ttOFFER_CANCEL: - return invoke_preflight_helper(ctx); + return f.template operator()(); case ttOFFER_CREATE: - return invoke_preflight_helper(ctx); + return f.template operator()(); case ttESCROW_CREATE: - return invoke_preflight_helper(ctx); + return f.template operator()(); case ttESCROW_FINISH: - return invoke_preflight_helper(ctx); + return f.template operator()(); case ttESCROW_CANCEL: - return invoke_preflight_helper(ctx); + return f.template operator()(); case ttPAYCHAN_CLAIM: - return invoke_preflight_helper(ctx); + return f.template operator()(); case ttPAYCHAN_CREATE: - return invoke_preflight_helper(ctx); + return f.template operator()(); case ttPAYCHAN_FUND: - return invoke_preflight_helper(ctx); + return f.template operator()(); case ttPAYMENT: - return invoke_preflight_helper(ctx); + return f.template operator()(); case ttREGULAR_KEY_SET: - return invoke_preflight_helper(ctx); + return f.template operator()(); case ttSIGNER_LIST_SET: - return invoke_preflight_helper(ctx); + return f.template operator()(); case ttTICKET_CREATE: - return invoke_preflight_helper(ctx); + return f.template operator()(); case ttTRUST_SET: - return invoke_preflight_helper(ctx); + return f.template operator()(); case ttAMENDMENT: case ttFEE: case ttUNL_MODIFY: - return invoke_preflight_helper(ctx); + return f.template operator()(); case ttNFTOKEN_MINT: - return invoke_preflight_helper(ctx); + return f.template operator()(); case ttNFTOKEN_BURN: - return invoke_preflight_helper(ctx); + return f.template operator()(); case ttNFTOKEN_CREATE_OFFER: - return invoke_preflight_helper(ctx); + return f.template operator()(); case ttNFTOKEN_CANCEL_OFFER: - return invoke_preflight_helper(ctx); + return f.template operator()(); case ttNFTOKEN_ACCEPT_OFFER: - return invoke_preflight_helper(ctx); + return f.template operator()(); case ttCLAWBACK: - return invoke_preflight_helper(ctx); + return f.template operator()(); case ttAMM_CREATE: - return invoke_preflight_helper(ctx); + return f.template operator()(); case ttAMM_DEPOSIT: - return invoke_preflight_helper(ctx); + return f.template operator()(); case ttAMM_WITHDRAW: - return invoke_preflight_helper(ctx); + return f.template operator()(); case ttAMM_VOTE: - return invoke_preflight_helper(ctx); + return f.template operator()(); case ttAMM_BID: - return invoke_preflight_helper(ctx); + return f.template operator()(); case ttAMM_DELETE: - return invoke_preflight_helper(ctx); + return f.template operator()(); case ttXCHAIN_CREATE_BRIDGE: - return invoke_preflight_helper(ctx); + return f.template operator()(); case ttXCHAIN_MODIFY_BRIDGE: - return invoke_preflight_helper(ctx); + return f.template operator()(); case ttXCHAIN_CREATE_CLAIM_ID: - return invoke_preflight_helper(ctx); + return f.template operator()(); case ttXCHAIN_COMMIT: - return invoke_preflight_helper(ctx); + return f.template operator()(); case ttXCHAIN_CLAIM: - return invoke_preflight_helper(ctx); + return f.template operator()(); case ttXCHAIN_ADD_CLAIM_ATTESTATION: - return invoke_preflight_helper(ctx); + return f.template operator()(); case ttXCHAIN_ADD_ACCOUNT_CREATE_ATTESTATION: - return invoke_preflight_helper( - ctx); + return f.template operator()(); case ttXCHAIN_ACCOUNT_CREATE_COMMIT: - return invoke_preflight_helper(ctx); + return f.template operator()(); default: - assert(false); - return {temUNKNOWN, TxConsequences{temUNKNOWN}}; + throw UnknownTxnType(txnType); } } +} // namespace -/* invoke_preclaim uses name hiding to accomplish - compile-time polymorphism of (presumably) static - class functions for Transactor and derived classes. -*/ +// Templates so preflight does the right thing with T::ConsequencesFactory. +// +// This could be done more easily using if constexpr, but Visual Studio +// 2017 doesn't handle if constexpr correctly. So once we're no longer +// building with Visual Studio 2017 we can consider replacing the four +// templates with a single template function that uses if constexpr. +// +// For Transactor::Normal +// + +// clang-format off +// Current formatter for rippled is based on clang-10, which does not handle `requires` clauses +template +requires(T::ConsequencesFactory == Transactor::Normal) +TxConsequences + consequences_helper(PreflightContext const& ctx) +{ + return TxConsequences(ctx.tx); +}; + +// For Transactor::Blocker +template +requires(T::ConsequencesFactory == Transactor::Blocker) +TxConsequences + consequences_helper(PreflightContext const& ctx) +{ + return TxConsequences(ctx.tx, TxConsequences::blocker); +}; + +// For Transactor::Custom template +requires(T::ConsequencesFactory == Transactor::Custom) +TxConsequences + consequences_helper(PreflightContext const& ctx) +{ + return T::makeTxConsequences(ctx); +}; +// clang-format on + +static std::pair +invoke_preflight(PreflightContext const& ctx) +{ + try + { + return with_txn_type(ctx.tx.getTxnType(), [&]() { + auto const tec = T::preflight(ctx); + return std::make_pair( + tec, + isTesSuccess(tec) ? consequences_helper(ctx) + : TxConsequences{tec}); + }); + } + catch (UnknownTxnType const& e) + { + // Should never happen + JLOG(ctx.j.fatal()) + << "Unknown transaction type in preflight: " << e.txnType; + assert(false); + return {temUNKNOWN, TxConsequences{temUNKNOWN}}; + } +} + static TER invoke_preclaim(PreclaimContext const& ctx) { - // If the transactor requires a valid account and the transaction doesn't - // list one, preflight will have already a flagged a failure. - auto const id = ctx.tx.getAccountID(sfAccount); - - if (id != beast::zero) + try { - TER result = T::checkSeqProxy(ctx.view, ctx.tx, ctx.j); + // use name hiding to accomplish compile-time polymorphism of static + // class functions for Transactor and derived classes. + return with_txn_type(ctx.tx.getTxnType(), [&]() { + // If the transactor requires a valid account and the transaction + // doesn't list one, preflight will have already a flagged a + // failure. + auto const id = ctx.tx.getAccountID(sfAccount); - if (result != tesSUCCESS) - return result; + if (id != beast::zero) + { + TER result = T::checkSeqProxy(ctx.view, ctx.tx, ctx.j); - result = T::checkPriorTxAndLastLedger(ctx); + if (result != tesSUCCESS) + return result; - if (result != tesSUCCESS) - return result; + result = T::checkPriorTxAndLastLedger(ctx); - result = T::checkFee(ctx, calculateBaseFee(ctx.view, ctx.tx)); + if (result != tesSUCCESS) + return result; - if (result != tesSUCCESS) - return result; + result = T::checkFee(ctx, calculateBaseFee(ctx.view, ctx.tx)); - result = T::checkSign(ctx); + if (result != tesSUCCESS) + return result; - if (result != tesSUCCESS) - return result; - } + result = T::checkSign(ctx); - return T::preclaim(ctx); -} + if (result != tesSUCCESS) + return result; + } -static TER -invoke_preclaim(PreclaimContext const& ctx) -{ - switch (ctx.tx.getTxnType()) + return T::preclaim(ctx); + }); + } + catch (UnknownTxnType const& e) { - case ttACCOUNT_DELETE: - return invoke_preclaim(ctx); - case ttACCOUNT_SET: - return invoke_preclaim(ctx); - case ttCHECK_CANCEL: - return invoke_preclaim(ctx); - case ttCHECK_CASH: - return invoke_preclaim(ctx); - case ttCHECK_CREATE: - return invoke_preclaim(ctx); - case ttDEPOSIT_PREAUTH: - return invoke_preclaim(ctx); - case ttOFFER_CANCEL: - return invoke_preclaim(ctx); - case ttOFFER_CREATE: - return invoke_preclaim(ctx); - case ttESCROW_CREATE: - return invoke_preclaim(ctx); - case ttESCROW_FINISH: - return invoke_preclaim(ctx); - case ttESCROW_CANCEL: - return invoke_preclaim(ctx); - case ttPAYCHAN_CLAIM: - return invoke_preclaim(ctx); - case ttPAYCHAN_CREATE: - return invoke_preclaim(ctx); - case ttPAYCHAN_FUND: - return invoke_preclaim(ctx); - case ttPAYMENT: - return invoke_preclaim(ctx); - case ttREGULAR_KEY_SET: - return invoke_preclaim(ctx); - case ttSIGNER_LIST_SET: - return invoke_preclaim(ctx); - case ttTICKET_CREATE: - return invoke_preclaim(ctx); - case ttTRUST_SET: - return invoke_preclaim(ctx); - case ttAMENDMENT: - case ttFEE: - case ttUNL_MODIFY: - return invoke_preclaim(ctx); - case ttNFTOKEN_MINT: - return invoke_preclaim(ctx); - case ttNFTOKEN_BURN: - return invoke_preclaim(ctx); - case ttNFTOKEN_CREATE_OFFER: - return invoke_preclaim(ctx); - case ttNFTOKEN_CANCEL_OFFER: - return invoke_preclaim(ctx); - case ttNFTOKEN_ACCEPT_OFFER: - return invoke_preclaim(ctx); - case ttCLAWBACK: - return invoke_preclaim(ctx); - case ttAMM_CREATE: - return invoke_preclaim(ctx); - case ttAMM_DEPOSIT: - return invoke_preclaim(ctx); - case ttAMM_WITHDRAW: - return invoke_preclaim(ctx); - case ttAMM_VOTE: - return invoke_preclaim(ctx); - case ttAMM_BID: - return invoke_preclaim(ctx); - case ttAMM_DELETE: - return invoke_preclaim(ctx); - case ttXCHAIN_CREATE_BRIDGE: - return invoke_preclaim(ctx); - case ttXCHAIN_MODIFY_BRIDGE: - return invoke_preclaim(ctx); - case ttXCHAIN_CREATE_CLAIM_ID: - return invoke_preclaim(ctx); - case ttXCHAIN_COMMIT: - return invoke_preclaim(ctx); - case ttXCHAIN_CLAIM: - return invoke_preclaim(ctx); - case ttXCHAIN_ACCOUNT_CREATE_COMMIT: - return invoke_preclaim(ctx); - case ttXCHAIN_ADD_CLAIM_ATTESTATION: - return invoke_preclaim(ctx); - case ttXCHAIN_ADD_ACCOUNT_CREATE_ATTESTATION: - return invoke_preclaim(ctx); - default: - assert(false); - return temUNKNOWN; + // Should never happen + JLOG(ctx.j.fatal()) + << "Unknown transaction type in preclaim: " << e.txnType; + assert(false); + return temUNKNOWN; } } static XRPAmount invoke_calculateBaseFee(ReadView const& view, STTx const& tx) { - switch (tx.getTxnType()) + try { - case ttACCOUNT_DELETE: - return DeleteAccount::calculateBaseFee(view, tx); - case ttACCOUNT_SET: - return SetAccount::calculateBaseFee(view, tx); - case ttCHECK_CANCEL: - return CancelCheck::calculateBaseFee(view, tx); - case ttCHECK_CASH: - return CashCheck::calculateBaseFee(view, tx); - case ttCHECK_CREATE: - return CreateCheck::calculateBaseFee(view, tx); - case ttDEPOSIT_PREAUTH: - return DepositPreauth::calculateBaseFee(view, tx); - case ttOFFER_CANCEL: - return CancelOffer::calculateBaseFee(view, tx); - case ttOFFER_CREATE: - return CreateOffer::calculateBaseFee(view, tx); - case ttESCROW_CREATE: - return EscrowCreate::calculateBaseFee(view, tx); - case ttESCROW_FINISH: - return EscrowFinish::calculateBaseFee(view, tx); - case ttESCROW_CANCEL: - return EscrowCancel::calculateBaseFee(view, tx); - case ttPAYCHAN_CLAIM: - return PayChanClaim::calculateBaseFee(view, tx); - case ttPAYCHAN_CREATE: - return PayChanCreate::calculateBaseFee(view, tx); - case ttPAYCHAN_FUND: - return PayChanFund::calculateBaseFee(view, tx); - case ttPAYMENT: - return Payment::calculateBaseFee(view, tx); - case ttREGULAR_KEY_SET: - return SetRegularKey::calculateBaseFee(view, tx); - case ttSIGNER_LIST_SET: - return SetSignerList::calculateBaseFee(view, tx); - case ttTICKET_CREATE: - return CreateTicket::calculateBaseFee(view, tx); - case ttTRUST_SET: - return SetTrust::calculateBaseFee(view, tx); - case ttAMENDMENT: - case ttFEE: - case ttUNL_MODIFY: - return Change::calculateBaseFee(view, tx); - case ttNFTOKEN_MINT: - return NFTokenMint::calculateBaseFee(view, tx); - case ttNFTOKEN_BURN: - return NFTokenBurn::calculateBaseFee(view, tx); - case ttNFTOKEN_CREATE_OFFER: - return NFTokenCreateOffer::calculateBaseFee(view, tx); - case ttNFTOKEN_CANCEL_OFFER: - return NFTokenCancelOffer::calculateBaseFee(view, tx); - case ttNFTOKEN_ACCEPT_OFFER: - return NFTokenAcceptOffer::calculateBaseFee(view, tx); - case ttCLAWBACK: - return Clawback::calculateBaseFee(view, tx); - case ttAMM_CREATE: - return AMMCreate::calculateBaseFee(view, tx); - case ttAMM_DEPOSIT: - return AMMDeposit::calculateBaseFee(view, tx); - case ttAMM_WITHDRAW: - return AMMWithdraw::calculateBaseFee(view, tx); - case ttAMM_VOTE: - return AMMVote::calculateBaseFee(view, tx); - case ttAMM_BID: - return AMMBid::calculateBaseFee(view, tx); - case ttAMM_DELETE: - return AMMDelete::calculateBaseFee(view, tx); - case ttXCHAIN_CREATE_BRIDGE: - return XChainCreateBridge::calculateBaseFee(view, tx); - case ttXCHAIN_MODIFY_BRIDGE: - return BridgeModify::calculateBaseFee(view, tx); - case ttXCHAIN_CREATE_CLAIM_ID: - return XChainCreateClaimID::calculateBaseFee(view, tx); - case ttXCHAIN_COMMIT: - return XChainCommit::calculateBaseFee(view, tx); - case ttXCHAIN_CLAIM: - return XChainClaim::calculateBaseFee(view, tx); - case ttXCHAIN_ADD_CLAIM_ATTESTATION: - return XChainAddClaimAttestation::calculateBaseFee(view, tx); - case ttXCHAIN_ADD_ACCOUNT_CREATE_ATTESTATION: - return XChainAddAccountCreateAttestation::calculateBaseFee( - view, tx); - case ttXCHAIN_ACCOUNT_CREATE_COMMIT: - return XChainCreateAccountCommit::calculateBaseFee(view, tx); - default: - assert(false); - return XRPAmount{0}; + return with_txn_type(tx.getTxnType(), [&]() { + return T::calculateBaseFee(view, tx); + }); + } + catch (UnknownTxnType const& e) + { + assert(false); + return XRPAmount{0}; } } @@ -460,173 +329,20 @@ TxConsequences::TxConsequences(STTx const& tx, std::uint32_t sequencesConsumed) static std::pair invoke_apply(ApplyContext& ctx) { - switch (ctx.tx.getTxnType()) + try { - case ttACCOUNT_DELETE: { - DeleteAccount p(ctx); - return p(); - } - case ttACCOUNT_SET: { - SetAccount p(ctx); - return p(); - } - case ttCHECK_CANCEL: { - CancelCheck p(ctx); - return p(); - } - case ttCHECK_CASH: { - CashCheck p(ctx); - return p(); - } - case ttCHECK_CREATE: { - CreateCheck p(ctx); - return p(); - } - case ttDEPOSIT_PREAUTH: { - DepositPreauth p(ctx); - return p(); - } - case ttOFFER_CANCEL: { - CancelOffer p(ctx); - return p(); - } - case ttOFFER_CREATE: { - CreateOffer p(ctx); - return p(); - } - case ttESCROW_CREATE: { - EscrowCreate p(ctx); - return p(); - } - case ttESCROW_FINISH: { - EscrowFinish p(ctx); - return p(); - } - case ttESCROW_CANCEL: { - EscrowCancel p(ctx); - return p(); - } - case ttPAYCHAN_CLAIM: { - PayChanClaim p(ctx); - return p(); - } - case ttPAYCHAN_CREATE: { - PayChanCreate p(ctx); - return p(); - } - case ttPAYCHAN_FUND: { - PayChanFund p(ctx); - return p(); - } - case ttPAYMENT: { - Payment p(ctx); - return p(); - } - case ttREGULAR_KEY_SET: { - SetRegularKey p(ctx); - return p(); - } - case ttSIGNER_LIST_SET: { - SetSignerList p(ctx); - return p(); - } - case ttTICKET_CREATE: { - CreateTicket p(ctx); - return p(); - } - case ttTRUST_SET: { - SetTrust p(ctx); + return with_txn_type(ctx.tx.getTxnType(), [&]() { + T p(ctx); return p(); - } - case ttAMENDMENT: - case ttFEE: - case ttUNL_MODIFY: { - Change p(ctx); - return p(); - } - case ttNFTOKEN_MINT: { - NFTokenMint p(ctx); - return p(); - } - case ttNFTOKEN_BURN: { - NFTokenBurn p(ctx); - return p(); - } - case ttNFTOKEN_CREATE_OFFER: { - NFTokenCreateOffer p(ctx); - return p(); - } - case ttNFTOKEN_CANCEL_OFFER: { - NFTokenCancelOffer p(ctx); - return p(); - } - case ttNFTOKEN_ACCEPT_OFFER: { - NFTokenAcceptOffer p(ctx); - return p(); - } - case ttCLAWBACK: { - Clawback p(ctx); - return p(); - } - case ttAMM_CREATE: { - AMMCreate p(ctx); - return p(); - } - case ttAMM_DEPOSIT: { - AMMDeposit p(ctx); - return p(); - } - case ttAMM_WITHDRAW: { - AMMWithdraw p(ctx); - return p(); - } - case ttAMM_VOTE: { - AMMVote p(ctx); - return p(); - } - case ttAMM_BID: { - AMMBid p(ctx); - return p(); - } - case ttAMM_DELETE: { - AMMDelete p(ctx); - return p(); - } - case ttXCHAIN_CREATE_BRIDGE: { - XChainCreateBridge p(ctx); - return p(); - } - case ttXCHAIN_MODIFY_BRIDGE: { - BridgeModify p(ctx); - return p(); - } - case ttXCHAIN_CREATE_CLAIM_ID: { - XChainCreateClaimID p(ctx); - return p(); - } - case ttXCHAIN_COMMIT: { - XChainCommit p(ctx); - return p(); - } - case ttXCHAIN_CLAIM: { - XChainClaim p(ctx); - return p(); - } - case ttXCHAIN_ADD_CLAIM_ATTESTATION: { - XChainAddClaimAttestation p(ctx); - return p(); - } - case ttXCHAIN_ADD_ACCOUNT_CREATE_ATTESTATION: { - XChainAddAccountCreateAttestation p(ctx); - return p(); - } - case ttXCHAIN_ACCOUNT_CREATE_COMMIT: { - XChainCreateAccountCommit p(ctx); - return p(); - } - default: - assert(false); - return {temUNKNOWN, false}; + }); + } + catch (UnknownTxnType const& e) + { + // Should never happen + JLOG(ctx.journal.fatal()) + << "Unknown transaction type in apply: " << e.txnType; + assert(false); + return {temUNKNOWN, false}; } } From 6ba9450c8988be3c74027a9fc53870e491fa1c6a Mon Sep 17 00:00:00 2001 From: Denis Angell Date: Fri, 6 Oct 2023 01:25:16 +0200 Subject: [PATCH 02/15] `fixDisallowIncomingV1`: allow issuers to authorize trust lines (#4721) Context: The `DisallowIncoming` amendment provides an option to block incoming trust lines from reaching your account. The asfDisallowIncomingTrustline AccountSet Flag, when enabled, prevents any incoming trust line from being created. However, it was too restrictive: it would block an issuer from authorizing a trust line, even if the trust line already exists. Consider: 1. Issuer sets asfRequireAuth on their account. 2. User sets asfDisallowIncomingTrustline on their account. 3. User submits tx to SetTrust to Issuer. At this point, without `fixDisallowIncomingV1` active, the issuer would not be able to authorize the trust line. The `fixDisallowIncomingV1` amendment, once activated, allows an issuer to authorize a trust line even after the user sets the asfDisallowIncomingTrustline flag, as long as the trust line already exists. --- src/ripple/app/tx/impl/SetTrust.cpp | 15 +++++++++- src/ripple/protocol/Feature.h | 3 +- src/ripple/protocol/impl/Feature.cpp | 1 + src/test/app/SetTrust_test.cpp | 42 ++++++++++++++++++++++++++++ 4 files changed, 59 insertions(+), 2 deletions(-) diff --git a/src/ripple/app/tx/impl/SetTrust.cpp b/src/ripple/app/tx/impl/SetTrust.cpp index 7869cc7027d..00a5165221e 100644 --- a/src/ripple/app/tx/impl/SetTrust.cpp +++ b/src/ripple/app/tx/impl/SetTrust.cpp @@ -140,7 +140,20 @@ SetTrust::preclaim(PreclaimContext const& ctx) return tecNO_DST; if (sleDst->getFlags() & lsfDisallowIncomingTrustline) - return tecNO_PERMISSION; + { + // The original implementation of featureDisallowIncoming was + // too restrictive. If + // o fixDisallowIncomingV1 is enabled and + // o The trust line already exists + // Then allow the TrustSet. + if (ctx.view.rules().enabled(fixDisallowIncomingV1) && + ctx.view.exists(keylet::line(id, uDstAccountID, currency))) + { + // pass + } + else + return tecNO_PERMISSION; + } } // If destination is AMM and the trustline doesn't exist then only diff --git a/src/ripple/protocol/Feature.h b/src/ripple/protocol/Feature.h index df0570d3a52..17aca813f71 100644 --- a/src/ripple/protocol/Feature.h +++ b/src/ripple/protocol/Feature.h @@ -74,7 +74,7 @@ namespace detail { // Feature.cpp. Because it's only used to reserve storage, and determine how // large to make the FeatureBitset, it MAY be larger. It MUST NOT be less than // the actual number of amendments. A LogicError on startup will verify this. -static constexpr std::size_t numFeatures = 62; +static constexpr std::size_t numFeatures = 63; /** Amendments that this server supports and the default voting behavior. Whether they are enabled depends on the Rules defined in the validated @@ -349,6 +349,7 @@ extern uint256 const fixNFTokenRemint; extern uint256 const fixReducedOffersV1; extern uint256 const featureClawback; extern uint256 const featureXChainBridge; +extern uint256 const fixDisallowIncomingV1; } // namespace ripple diff --git a/src/ripple/protocol/impl/Feature.cpp b/src/ripple/protocol/impl/Feature.cpp index aa41b7d5b6e..6a3430f4f50 100644 --- a/src/ripple/protocol/impl/Feature.cpp +++ b/src/ripple/protocol/impl/Feature.cpp @@ -456,6 +456,7 @@ REGISTER_FIX (fixReducedOffersV1, Supported::yes, VoteBehavior::De REGISTER_FEATURE(Clawback, Supported::yes, VoteBehavior::DefaultNo); REGISTER_FEATURE(AMM, Supported::yes, VoteBehavior::DefaultNo); REGISTER_FEATURE(XChainBridge, Supported::yes, VoteBehavior::DefaultNo); +REGISTER_FIX(fixDisallowIncomingV1, Supported::yes, VoteBehavior::DefaultNo); // The following amendments are obsolete, but must remain supported // because they could potentially get enabled. diff --git a/src/test/app/SetTrust_test.cpp b/src/test/app/SetTrust_test.cpp index fce9c4295c2..599ac1917f9 100644 --- a/src/test/app/SetTrust_test.cpp +++ b/src/test/app/SetTrust_test.cpp @@ -275,6 +275,48 @@ class SetTrust_test : public beast::unit_test::suite BEAST_EXPECT(!(flags & lsfDisallowIncomingTrustline)); } + // fixDisallowIncomingV1 + { + for (bool const withFix : {true, false}) + { + auto const amend = withFix + ? features | disallowIncoming + : (features | disallowIncoming) - fixDisallowIncomingV1; + + Env env{*this, amend}; + auto const dist = Account("dist"); + auto const gw = Account("gw"); + auto const USD = gw["USD"]; + auto const distUSD = dist["USD"]; + + env.fund(XRP(1000), gw, dist); + env.close(); + + env(fset(gw, asfRequireAuth)); + env.close(); + + env(fset(dist, asfDisallowIncomingTrustline)); + env.close(); + + env(trust(dist, USD(10000))); + env.close(); + + // withFix: can set trustline + // withOutFix: cannot set trustline + auto const trustResult = + withFix ? ter(tesSUCCESS) : ter(tecNO_PERMISSION); + env(trust(gw, distUSD(10000)), + txflags(tfSetfAuth), + trustResult); + env.close(); + + auto const txResult = + withFix ? ter(tesSUCCESS) : ter(tecPATH_DRY); + env(pay(gw, dist, USD(1000)), txResult); + env.close(); + } + } + Env env{*this, features | disallowIncoming}; auto const gw = Account{"gateway"}; From ced14ec1ab9736536160f959b50b3a6560187c03 Mon Sep 17 00:00:00 2001 From: Florent <36513774+florent-uzio@users.noreply.github.com> Date: Fri, 6 Oct 2023 19:23:30 +0400 Subject: [PATCH 03/15] docs(rippled-example.cfg): add P2P link compression (#4753) P2P link compression is a feature added in 1.6.0 by #3287. https://xrpl.org/enable-link-compression.html If the default changes in the future - for example, as currently proposed by #4387 - the comment will be updated at that time. Fix #4656 --- cfg/rippled-example.cfg | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/cfg/rippled-example.cfg b/cfg/rippled-example.cfg index e21197eed5a..a3bcf0056be 100644 --- a/cfg/rippled-example.cfg +++ b/cfg/rippled-example.cfg @@ -389,6 +389,21 @@ # # # +# [compression] +# +# true or false +# +# true - enables compression +# false - disables compression [default]. +# +# The rippled server can save bandwidth by compressing its peer-to-peer communications, +# at a cost of greater CPU usage. If you enable link compression, +# the server automatically compresses communications with peer servers +# that also have link compression enabled. +# https://xrpl.org/enable-link-compression.html +# +# +# # [ips] # # List of hostnames or ips where the Ripple protocol is served. A default From 053b69c63f3fbe94db9820c8021eff9fc38ac8fa Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Fri, 6 Oct 2023 14:11:39 -0400 Subject: [PATCH 04/15] docs(API-CHANGELOG): add `XRPFees` change (#4741) * Add a new API Changelog section for release 1.10. * Mark `jss::fee_ref` as deprecated. * Fix a copy-paste error in one of the unit tests. --- API-CHANGELOG.md | 11 +++++++++++ src/ripple/protocol/jss.h | 2 +- src/test/app/TxQ_test.cpp | 6 +++--- 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/API-CHANGELOG.md b/API-CHANGELOG.md index 7579a40b72c..5115eee0855 100644 --- a/API-CHANGELOG.md +++ b/API-CHANGELOG.md @@ -98,6 +98,17 @@ Additions are intended to be non-breaking (because they are purely additive). - Added `NFTokenPages` to the `account_objects` RPC. (https://github.com/XRPLF/rippled/pull/4352) - Fixed: `marker` returned from the `account_lines` command would not work on subsequent commands. (https://github.com/XRPLF/rippled/pull/4361) +## XRP Ledger version 1.10.0 + +[Version 1.10.0](https://github.com/XRPLF/rippled/releases/tag/1.10.0) +was released on Mar 14, 2023. + +### Breaking changes in 1.10 + +- If the `XRPFees` feature is enabled, the `fee_ref` field will be + removed from the [ledger subscription stream](https://xrpl.org/subscribe.html#ledger-stream), because it will no longer + have any meaning. + # In development Changes below this point are in development. diff --git a/src/ripple/protocol/jss.h b/src/ripple/protocol/jss.h index 9a3e315dd8e..e31a1cb3bf8 100644 --- a/src/ripple/protocol/jss.h +++ b/src/ripple/protocol/jss.h @@ -315,7 +315,7 @@ JSS(fee_base); // out: NetworkOPs JSS(fee_div_max); // in: TransactionSign JSS(fee_level); // out: AccountInfo JSS(fee_mult_max); // in: TransactionSign -JSS(fee_ref); // out: NetworkOPs +JSS(fee_ref); // out: NetworkOPs, DEPRECATED JSS(fetch_pack); // out: NetworkOPs JSS(first); // out: rpc/Version JSS(firstSequence); // out: NodeToShardStatus diff --git a/src/test/app/TxQ_test.cpp b/src/test/app/TxQ_test.cpp index 4bc0040f867..ac6bf56f06c 100644 --- a/src/test/app/TxQ_test.cpp +++ b/src/test/app/TxQ_test.cpp @@ -4843,13 +4843,13 @@ class TxQ1_test : public beast::unit_test::suite drops[jss::base_fee] == "0"); BEAST_EXPECT( drops.isMember(jss::median_fee) && - drops[jss::base_fee] == "0"); + drops[jss::median_fee] == "0"); BEAST_EXPECT( drops.isMember(jss::minimum_fee) && - drops[jss::base_fee] == "0"); + drops[jss::minimum_fee] == "0"); BEAST_EXPECT( drops.isMember(jss::open_ledger_fee) && - drops[jss::base_fee] == "0"); + drops[jss::open_ledger_fee] == "0"); } } From 3e08c390f5f18b308ace0dbdbb27fa02e49141fa Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Mon, 9 Oct 2023 18:51:22 -0400 Subject: [PATCH 05/15] ci: reenable Windows CI build with Artifactory support (#4596) Artifactory support was added to the `nix` builds with #4556. This extends that support to the Windows build. Now the Windows build works; CI will build and test a Windows release build. This only affects CI and does not change any C++ code. * Copy the remote setup step outcome fix from #4716 discussion * Allow the Windows job to succeed if tests fail: * Currently the tests do not always pass, even on a single threaded run on the GitHub runners. So we are using parallel runs and mark the test step as allowed to fail (continue-on-error). * At this point, it's more important that the build succeeds than that the tests succeed, because: * We've got plenty of test coverage on the other jobs. * Test failures are much rarer than build failures because of cross-platform issues. * Having a test failure locally doesn't interrupt a workflow nearly as much as a build failure. Note that Conan Center cannot hold the binaries we need. They do not build the configurations we need, and they will not add them. ## Future Tasks This introduces a new bottleneck since the build and test takes over an hour. Speed up the job by: * Making this job run on heavy Windows runners. * Increasing the number of hardware threads. --- .github/actions/build/action.yml | 6 +- .github/workflows/macos.yml | 4 ++ .github/workflows/nix.yml | 6 +- .github/workflows/windows.yml | 107 ++++++++++++++++--------------- 4 files changed, 64 insertions(+), 59 deletions(-) diff --git a/.github/actions/build/action.yml b/.github/actions/build/action.yml index 2c4135e0cf1..d7a92b3b409 100644 --- a/.github/actions/build/action.yml +++ b/.github/actions/build/action.yml @@ -10,16 +10,12 @@ inputs: runs: using: composite steps: - - name: dependencies - uses: ./.github/actions/dependencies - with: - configuration: ${{ inputs.configuration }} - name: configure shell: bash run: | cd ${build_dir} cmake \ - ${{ inputs.generator && format('-G {0}', inputs.generator) || '' }} \ + ${{ inputs.generator && format('-G "{0}"', inputs.generator) || '' }} \ -DCMAKE_TOOLCHAIN_FILE:FILEPATH=build/generators/conan_toolchain.cmake \ -DCMAKE_BUILD_TYPE=${{ inputs.configuration }} \ ${{ inputs.cmake-args }} \ diff --git a/.github/workflows/macos.yml b/.github/workflows/macos.yml index 1ebb9f690bb..120a6ec1782 100644 --- a/.github/workflows/macos.yml +++ b/.github/workflows/macos.yml @@ -37,6 +37,10 @@ jobs: run : | conan profile get env.CXXFLAGS default || true conan profile update 'conf.tools.build:cxxflags+=["-DBOOST_ASIO_DISABLE_CONCEPTS"]' default + - name: dependencies + uses: ./.github/actions/dependencies + with: + configuration: ${{ matrix.configuration }} - name: build uses: ./.github/actions/build with: diff --git a/.github/workflows/nix.yml b/.github/workflows/nix.yml index 4198369e78a..fb017d45bf7 100644 --- a/.github/workflows/nix.yml +++ b/.github/workflows/nix.yml @@ -88,7 +88,7 @@ jobs: # Print the list of dependencies that would need to be built locally. # A non-empty list means we have "failed" to cache binaries remotely. run: | - echo missing=$(conan info . --build missing --json 2>/dev/null | grep '^\[') | tee ${GITHUB_OUTPUT} + echo missing=$(conan info . --build missing --settings build_type=${{ matrix.configuration }} --json 2>/dev/null | grep '^\[') | tee ${GITHUB_OUTPUT} - name: build dependencies if: (steps.binaries.outputs.missing != '[]') uses: ./.github/actions/dependencies @@ -146,6 +146,10 @@ jobs: ls ~/.conan - name: checkout uses: actions/checkout@v3 + - name: dependencies + uses: ./.github/actions/dependencies + with: + configuration: ${{ matrix.configuration }} - name: build uses: ./.github/actions/build with: diff --git a/.github/workflows/windows.yml b/.github/workflows/windows.yml index 0f918a50959..4988e323e2b 100644 --- a/.github/workflows/windows.yml +++ b/.github/workflows/windows.yml @@ -1,19 +1,11 @@ name: windows -# We have disabled this workflow because it fails in our CI Windows -# environment, but we cannot replicate the failure in our personal Windows -# test environments, nor have we gone through the trouble of setting up an -# interactive CI Windows environment. -# We welcome contributions to diagnose or debug the problems on Windows. Until -# then, we leave this tombstone as a reminder that we have tried (but failed) -# to write a reliable test for Windows. -# on: [push, pull_request] -on: - workflow_dispatch: - push: - branches: - - 'action' - paths: - - '.github/workflow/windows.yml' + +on: [push, pull_request] + +# https://docs.github.com/en/actions/using-jobs/using-concurrency +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true jobs: @@ -25,6 +17,11 @@ jobs: - Visual Studio 16 2019 configuration: - Release + # Github hosted runners tend to hang when running Debug unit tests. + # Instead of trying to work around it, disable the Debug job until + # something beefier (i.e. a heavy self-hosted runner) becomes + # available. + # - Debug runs-on: windows-2019 env: build_dir: .build @@ -37,11 +34,12 @@ jobs: python-version: 3.9 - name: learn Python cache directory id: pip-cache + shell: bash run: | pip install --upgrade pip - echo "dir=$(pip cache dir)" >> $GITHUB_OUTPUT + echo "dir=$(pip cache dir)" | tee ${GITHUB_OUTPUT} - name: restore Python cache directory - uses: actions/cache@v2 + uses: actions/cache@v3 with: path: ${{ steps.pip-cache.outputs.dir }} key: ${{ runner.os }}-${{ hashFiles('.github/workflows/windows.yml') }} @@ -55,45 +53,48 @@ jobs: cmake --version dir env: - name: configure Conan + shell: bash + env: + CONAN_URL: http://18.143.149.228:8081/artifactory/api/conan/conan-non-prod run: | conan profile new default --detect - conan profile update settings.compiler.cppstd=20 default - conan profile update settings.compiler.runtime=MT default - conan profile update settings.compiler.toolset=v141 default - - name: learn Conan cache directory - id: conan-cache - run: | - echo "dir=$(conan config get storage.path)" >> $GITHUB_OUTPUT - - name: restore Conan cache directory - uses: actions/cache@v2 - with: - path: ${{ steps.conan-cache.outputs.dir }} - key: ${{ hashFiles('~/.conan/profiles/default', 'conanfile.py', 'external/rocksdb/*', '.github/workflows/windows.yml') }} - - name: export custom recipes + conan profile update settings.compiler.runtime=MT${{ matrix.configuration == 'Debug' && 'd' || '' }} default + # Do not quote the URL. An empty string will be accepted (with + # a non-fatal warning), but a missing argument will not. + conan remote add ripple ${{ env.CONAN_URL }} --insert 0 + - name: try to authenticate to ripple Conan remote + shell: bash + id: remote run: | - conan export external/snappy snappy/1.1.9@ - conan export external/soci soci/4.0.3@ - - name: install dependencies + echo outcome=$(conan user --remote ripple ${{ secrets.CONAN_USERNAME }} \ + --password ${{ secrets.CONAN_TOKEN }} >&2 && echo success || \ + echo failure) | tee ${GITHUB_OUTPUT} + - name: list missing binaries + id: binaries + shell: bash + # Print the list of dependencies that would need to be built locally. + # A non-empty list means we have "failed" to cache binaries remotely. run: | - mkdir $env:build_dir - cd $env:build_dir - conan install .. --build missing --settings build_type=${{ matrix.configuration }} - - name: configure - run: | - $env:build_dir - cd $env:build_dir - pwd - ls - cmake ` - -G "${{ matrix.generator }}" ` - -DCMAKE_TOOLCHAIN_FILE:FILEPATH=build/generators/conan_toolchain.cmake ` - -Dassert=ON ` - -Dreporting=OFF ` - -Dunity=OFF ` - .. + echo missing=$(conan info . --build missing --settings build_type=${{ matrix.configuration }} --json 2>/dev/null | grep '^\[') | tee ${GITHUB_OUTPUT} + - name: build dependencies + uses: ./.github/actions/dependencies + with: + configuration: ${{ matrix.configuration }} + - name: upload dependencies to remote + if: (steps.binaries.outputs.missing != '[]') && (steps.remote.outputs.outcome == 'success') + run: conan upload --remote ripple '*' --all --parallel --confirm - name: build + uses: ./.github/actions/build + with: + generator: '${{ matrix.generator }}' + configuration: ${{ matrix.configuration }} + # Hard code for now. Move to the matrix if varied options are needed + cmake-args: '-Dassert=ON -Dreporting=OFF -Dunity=OFF' + - name: test (permitted to silently fail) + shell: bash + # Github runners are resource limited, which causes unit tests to fail + # (e.g. OOM). To allow forward progress until self-hosted runners are + # up and running reliably, allow the job to succeed even if tests fail. + continue-on-error: true run: | - cmake --build $env:build_dir --target rippled --config ${{ matrix.configuration }} --parallel $env:NUMBER_OF_PROCESSORS - - name: test - run: | - & "$env:build_dir\${{ matrix.configuration }}\rippled.exe" --unittest + ${build_dir}/${{ matrix.configuration }}/rippled --unittest --unittest-jobs $(nproc) From 1151fba415a5949adfa1bca8bebf135355a0619c Mon Sep 17 00:00:00 2001 From: Chenna Keshava B S <21219765+ckeshava@users.noreply.github.com> Date: Wed, 11 Oct 2023 08:45:45 -0700 Subject: [PATCH 06/15] fix(CI): update workflow for uploading binaries to artifactory (#4746) Update the nix CI runner. This commit does not modify any source code files. The unix builds were successful, but the binaries were not uploaded to the internal artifactory. This PR borrows an idea from @ximinez to attempt to fix this issue. After successful authentication, the `outcome` variable contains a string. In the upload step, we are checking if outcome == 'success' as a prerequisite for uploading the binary. This commit updates the contents of the `outcome` variable. --- .github/workflows/nix.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/nix.yml b/.github/workflows/nix.yml index fb017d45bf7..e43ae1379c9 100644 --- a/.github/workflows/nix.yml +++ b/.github/workflows/nix.yml @@ -79,7 +79,7 @@ jobs: - name: try to authenticate to ripple Conan remote id: remote run: | - echo outcome=$(conan user --remote ripple ${{ secrets.CONAN_USERNAME }} --password ${{ secrets.CONAN_TOKEN }} && echo success || echo failure) | tee ${GITHUB_OUTPUT} + echo outcome=$(conan user --remote ripple ${{ secrets.CONAN_USERNAME }} --password ${{ secrets.CONAN_TOKEN }} >&2 && echo success || echo failure) | tee ${GITHUB_OUTPUT} - name: archive profile # Create this archive before dependencies are added to the local cache. run: tar -czf conan.tar -C ~/.conan . From 50cc1cf0c97f6f3cbf8deb6e532804d3196fa319 Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Thu, 12 Oct 2023 13:10:35 -0400 Subject: [PATCH 07/15] fix(PathRequest): remove incorrect assert (#4743) The assert is saying that the only reason `pathFinder` would be null is if the request was aborted (connection dropped, etc.). That's what `continueCallback()` checks. But that is very clearly not true if you look at `getPathFinder`, which calls `findPaths`, which can return false for many reasons. Fix #4744 --- src/ripple/app/paths/PathRequest.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/ripple/app/paths/PathRequest.cpp b/src/ripple/app/paths/PathRequest.cpp index 02b46c81e91..f2aa363f934 100644 --- a/src/ripple/app/paths/PathRequest.cpp +++ b/src/ripple/app/paths/PathRequest.cpp @@ -538,7 +538,6 @@ PathRequest::findPaths( continueCallback); if (!pathfinder) { - assert(continueCallback && !continueCallback()); JLOG(m_journal.debug()) << iIdentifier << " No paths found"; continue; } From c915984340e999cf8224e7d9377eeee8acbc0f2e Mon Sep 17 00:00:00 2001 From: Jackson Mills Date: Thu, 12 Oct 2023 10:12:17 -0700 Subject: [PATCH 08/15] docs(conan.md): fix broken link to conanfile.py (#4740) --- docs/build/conan.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/build/conan.md b/docs/build/conan.md index 01fb7150d38..210db18a5df 100644 --- a/docs/build/conan.md +++ b/docs/build/conan.md @@ -69,7 +69,7 @@ compiler and linker options for all dependencies _and_ this project. However, that is very tedious and error-prone, which is why we lean on tools like Conan. -We have written a Conan configuration file ([`conanfile.py`](./conanfile.py)) +We have written a Conan configuration file ([`conanfile.py`](../../conanfile.py)) so that Conan can be used to correctly download, configure, build, and install all of the dependencies for this project, using a single set of compiler and linker options for all of them. From 1fde585003c286841e3884012bef000d1ff5f4ee Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Mon, 16 Oct 2023 17:26:40 -0400 Subject: [PATCH 09/15] fix(CI): Call python to upgrade pip on Windows (#4768) In Windows, we need to call `python` in order for the `pip` upgrade command to work. This changes the GitHub Actions Windows CI job to use the correct command to upgrade PIP, fixing this error: ``` ERROR: To modify pip, please run the following command: C:\hostedtoolcache\windows\Python\3.9.13\x64\python.exe -m pip install --upgrade pip ``` A future task is to make job run on heavy Windows runners so that it doesn't take so long. Context: #4596 --- .github/workflows/windows.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/windows.yml b/.github/workflows/windows.yml index 4988e323e2b..98cbb9acc40 100644 --- a/.github/workflows/windows.yml +++ b/.github/workflows/windows.yml @@ -36,7 +36,7 @@ jobs: id: pip-cache shell: bash run: | - pip install --upgrade pip + python -m pip install --upgrade pip echo "dir=$(pip cache dir)" | tee ${GITHUB_OUTPUT} - name: restore Python cache directory uses: actions/cache@v3 From 1fc1eb9f68cb23d07a1858ed001bf5d57e304d81 Mon Sep 17 00:00:00 2001 From: Elliot Lee Date: Mon, 16 Oct 2023 17:59:19 -0700 Subject: [PATCH 10/15] Set version to 2.0.0-b3 --- src/ripple/protocol/impl/BuildInfo.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ripple/protocol/impl/BuildInfo.cpp b/src/ripple/protocol/impl/BuildInfo.cpp index a0f43df8b64..85659579de6 100644 --- a/src/ripple/protocol/impl/BuildInfo.cpp +++ b/src/ripple/protocol/impl/BuildInfo.cpp @@ -33,7 +33,7 @@ namespace BuildInfo { // and follow the format described at http://semver.org/ //------------------------------------------------------------------------------ // clang-format off -char const* const versionString = "2.0.0-b2" +char const* const versionString = "2.0.0-b3" // clang-format on #if defined(DEBUG) || defined(SANITIZER) From be6ac7e7a17bbe26a47de8c8ef9340c31c14fb46 Mon Sep 17 00:00:00 2001 From: Elliot Lee Date: Tue, 17 Oct 2023 21:27:03 -0700 Subject: [PATCH 11/15] docs(pull_request_template): add API Impact section (#4757) --- .github/pull_request_template.md | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index ca7367b8753..f3c5edd0b0b 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -33,10 +33,27 @@ Please check [x] relevant options, delete irrelevant ones. - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) - [ ] Refactor (non-breaking change that only restructures code) -- [ ] Tests (You added tests for code that already exists, or your new feature included in this PR) -- [ ] Documentation Updates +- [ ] Tests (you added tests for code that already exists, or your new feature included in this PR) +- [ ] Documentation update +- [ ] Chore (no impact to binary, e.g. `.gitignore`, formatting, dropping support for older tooling) - [ ] Release +### API Impact + + + +- [ ] Public API: New feature (new methods and/or new fields) +- [ ] Public API: Breaking change (in general, breaking changes should only impact the next api_version) +- [ ] `libxrpl` change (any change that may affect `libxrpl` or dependents of `libxrpl`) +- [ ] Peer protocol change (must be backward compatible or bump the peer protocol version) +