From d38f45701f2cf8938898dcbddd6c669913991ff1 Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Fri, 15 May 2026 17:35:14 -0400 Subject: [PATCH] clean up (roll back AI weirdness) --- include/xrpl/ledger/helpers/README.md | 2 +- include/xrpl/ledger/helpers/SLEBase.h | 1 + src/libxrpl/ledger/helpers/TokenHelpers.cpp | 10 +-- .../tx/transactors/account/AccountDelete.cpp | 4 +- .../tx/transactors/account/AccountSet.cpp | 76 +++++++++-------- .../tx/transactors/account/SetRegularKey.cpp | 16 ++-- .../tx/transactors/account/SignerListSet.cpp | 7 +- .../tx/transactors/bridge/XChainBridge.cpp | 12 +-- .../tx/transactors/check/CheckCash.cpp | 5 +- .../tx/transactors/check/CheckCreate.cpp | 6 +- .../tx/transactors/delegate/DelegateSet.cpp | 5 +- src/libxrpl/tx/transactors/dex/AMMBid.cpp | 1 + src/libxrpl/tx/transactors/dex/AMMVote.cpp | 45 ++++------- .../tx/transactors/dex/OfferCancel.cpp | 2 +- .../tx/transactors/dex/OfferCreate.cpp | 7 +- .../tx/transactors/escrow/EscrowCreate.cpp | 2 +- .../tx/transactors/lending/LoanPay.cpp | 2 +- .../tx/transactors/payment/Payment.cpp | 6 +- .../payment_channel/PaymentChannelCreate.cpp | 8 +- src/libxrpl/tx/transactors/system/Change.cpp | 4 +- src/libxrpl/tx/transactors/token/TrustSet.cpp | 15 ++-- src/test/app/AMMMPT_test.cpp | 2 +- src/test/app/Invariants_test.cpp | 70 ++++++++-------- src/xrpld/rpc/detail/PathRequest.cpp | 81 ++++++++++--------- src/xrpld/rpc/detail/Pathfinder.cpp | 35 +++++--- .../rpc/handlers/account/AccountInfo.cpp | 2 +- .../rpc/handlers/account/NoRippleCheck.cpp | 10 +-- src/xrpld/rpc/handlers/orderbook/AMMInfo.cpp | 11 ++- .../handlers/orderbook/DepositAuthorized.cpp | 2 +- 29 files changed, 221 insertions(+), 228 deletions(-) diff --git a/include/xrpl/ledger/helpers/README.md b/include/xrpl/ledger/helpers/README.md index 963502ed48..2a3ec4792b 100644 --- a/include/xrpl/ledger/helpers/README.md +++ b/include/xrpl/ledger/helpers/README.md @@ -38,7 +38,7 @@ Both specializations share all domain read methods. Write methods on `WAccountRo | File | Description | | ---------------------- | ---------------------------------------------------------------------------------------------------- | -| `SLEBase.h` | Template base class `SLEBase` and `ReadOnlySLE`/`WritableSLE` aliases | +| `SLEBase.h` | Template base class `SLEBase` for read-only and writable SLE wrappers | | `AccountRootHelpers.h` | `AccountRoot` wrapper (`RAccountRoot`, `WAccountRoot`) and free functions for pseudo-accounts | | `CredentialHelpers.h` | Free functions for Credential ledger entries | | `DirectoryHelpers.h` | Free functions for directory traversal (`dirFirst`, `dirNext`, `forEachItem`, etc.) | diff --git a/include/xrpl/ledger/helpers/SLEBase.h b/include/xrpl/ledger/helpers/SLEBase.h index ba12830b1d..470a67bed7 100644 --- a/include/xrpl/ledger/helpers/SLEBase.h +++ b/include/xrpl/ledger/helpers/SLEBase.h @@ -1,5 +1,6 @@ #pragma once +#include #include #include #include diff --git a/src/libxrpl/ledger/helpers/TokenHelpers.cpp b/src/libxrpl/ledger/helpers/TokenHelpers.cpp index 9f6a80f08a..ff129d5844 100644 --- a/src/libxrpl/ledger/helpers/TokenHelpers.cpp +++ b/src/libxrpl/ledger/helpers/TokenHelpers.cpp @@ -589,7 +589,7 @@ directSendNoFeeIOU( && sleRippleState->isFlag(senderNoRippleFlag) != AccountRoot(uSenderID, view, j)->isFlag(lsfDefaultRipple) && !sleRippleState->isFlag(senderFreezeFlag) && - !sleRippleState->getFieldAmount(!bSenderHigh ? sfLowLimit : sfHighLimit) + !sleRippleState->getFieldAmount(bSenderHigh ? sfHighLimit : sfLowLimit) // Sender trust limit is 0. && (sleRippleState->getFieldU32(bSenderHigh ? sfHighQualityIn : sfLowQualityIn) == 0u) // Sender quality in is 0. @@ -645,7 +645,7 @@ directSendNoFeeIOU( if (!wrappedAccount) return tefINTERNAL; // LCOV_EXCL_LINE - bool const noRipple = (wrappedAccount->getFlags() & lsfDefaultRipple) == 0; + bool const noRipple = !wrappedAccount->isFlag(lsfDefaultRipple); return trustCreate( view, @@ -666,7 +666,7 @@ directSendNoFeeIOU( } // Send regardless of limits. -// saAmount: Amount/currency/issuer to deliver to receiver. +// --> saAmount: Amount/currency/issuer to deliver to receiver. // <-- saActual: Amount actually cost. Sender pays fees. static TER directSendNoLimitIOU( @@ -717,7 +717,7 @@ directSendNoLimitIOU( } // Send regardless of limits. -// receivers: Amount/currency/issuer to deliver to receivers. +// --> receivers: Amount/currency/issuer to deliver to receivers. // <-- saActual: Amount actually cost to sender. Sender pays fees. static TER directSendNoLimitMultiIOU( @@ -1193,7 +1193,7 @@ directSendNoLimitMultiMPT( // Use uint64_t, not STAmount, to keep MaximumAmount comparisons in exact // integer arithmetic. STAmount implicitly converts to Number, whose // small-scale mantissa (~16 digits) can lose precision for values near - // kMAX_MP_TOKEN_AMOUNT (19 digits). + // kMaxMpTokenAmount (19 digits). std::uint64_t totalSendAmount{0}; std::uint64_t const maximumAmount = sle->at(~sfMaximumAmount).value_or(kMaxMpTokenAmount); std::uint64_t const outstandingAmount = sle->getFieldU64(sfOutstandingAmount); diff --git a/src/libxrpl/tx/transactors/account/AccountDelete.cpp b/src/libxrpl/tx/transactors/account/AccountDelete.cpp index 919a791e22..4896bd4953 100644 --- a/src/libxrpl/tx/transactors/account/AccountDelete.cpp +++ b/src/libxrpl/tx/transactors/account/AccountDelete.cpp @@ -232,7 +232,7 @@ AccountDelete::preclaim(PreclaimContext const& ctx) if (!acctDst) return tecNO_DST; - if (((acctDst->getFlags() & lsfRequireDestTag) != 0u) && !ctx.tx[~sfDestinationTag]) + if (acctDst->isFlag(lsfRequireDestTag) && !ctx.tx[~sfDestinationTag]) return tecDST_TAG_NEEDED; // If credentials are provided - check them anyway @@ -244,7 +244,7 @@ AccountDelete::preclaim(PreclaimContext const& ctx) if (!ctx.tx.isFieldPresent(sfCredentialIDs)) { // Check whether the destination account requires deposit authorization. - if ((acctDst->getFlags() & lsfDepositAuth) != 0u) + if (acctDst->isFlag(lsfDepositAuth)) { if (!ctx.view.exists(keylet::depositPreauth(dst, account))) return tecNO_PERMISSION; diff --git a/src/libxrpl/tx/transactors/account/AccountSet.cpp b/src/libxrpl/tx/transactors/account/AccountSet.cpp index 1f76dcf563..adb4c6849d 100644 --- a/src/libxrpl/tx/transactors/account/AccountSet.cpp +++ b/src/libxrpl/tx/transactors/account/AccountSet.cpp @@ -227,8 +227,6 @@ AccountSet::preclaim(PreclaimContext const& ctx) if (!acctRoot) return terNO_ACCOUNT; - std::uint32_t const uFlagsIn = acctRoot->getFieldU32(sfFlags); - std::uint32_t const uSetFlag = ctx.tx.getFieldU32(sfSetFlag); // legacy AccountSet flags @@ -237,7 +235,7 @@ AccountSet::preclaim(PreclaimContext const& ctx) // // RequireAuth // - if (bSetRequireAuth && ((uFlagsIn & lsfRequireAuth) == 0u)) + if (bSetRequireAuth && !acctRoot->isFlag(lsfRequireAuth)) { if (!dirIsEmpty(ctx.view, keylet::ownerDir(id))) { @@ -253,7 +251,7 @@ AccountSet::preclaim(PreclaimContext const& ctx) { if (uSetFlag == asfAllowTrustLineClawback) { - if ((uFlagsIn & lsfNoFreeze) != 0u) + if (acctRoot->isFlag(lsfNoFreeze)) { JLOG(ctx.j.trace()) << "Can't set Clawback if NoFreeze is set"; return tecNO_PERMISSION; @@ -268,7 +266,7 @@ AccountSet::preclaim(PreclaimContext const& ctx) else if (uSetFlag == asfNoFreeze) { // Cannot set NoFreeze if clawback is enabled - if ((uFlagsIn & lsfAllowTrustLineClawback) != 0u) + if (acctRoot->isFlag(lsfAllowTrustLineClawback)) { JLOG(ctx.j.trace()) << "Can't set NoFreeze if clawback is enabled"; return tecNO_PERMISSION; @@ -282,11 +280,10 @@ AccountSet::preclaim(PreclaimContext const& ctx) TER AccountSet::doApply() { - WAccountRoot acct(accountID_, view(), j_); - if (!acct) + if (!account_) return tefINTERNAL; // LCOV_EXCL_LINE - std::uint32_t const uFlagsIn = acct->getFieldU32(sfFlags); + std::uint32_t const uFlagsIn = account_->getFieldU32(sfFlags); std::uint32_t uFlagsOut = uFlagsIn; STTx const& tx{ctx_.tx}; @@ -317,13 +314,13 @@ AccountSet::doApply() // // RequireAuth // - if (bSetRequireAuth && ((uFlagsIn & lsfRequireAuth) == 0u)) + if (bSetRequireAuth && !account_->isFlag(lsfRequireAuth)) { JLOG(j_.trace()) << "Set RequireAuth."; uFlagsOut |= lsfRequireAuth; } - if (bClearRequireAuth && ((uFlagsIn & lsfRequireAuth) != 0u)) + if (bClearRequireAuth && account_->isFlag(lsfRequireAuth)) { JLOG(j_.trace()) << "Clear RequireAuth."; uFlagsOut &= ~lsfRequireAuth; @@ -332,13 +329,13 @@ AccountSet::doApply() // // RequireDestTag // - if (bSetRequireDest && ((uFlagsIn & lsfRequireDestTag) == 0u)) + if (bSetRequireDest && !account_->isFlag(lsfRequireDestTag)) { JLOG(j_.trace()) << "Set lsfRequireDestTag."; uFlagsOut |= lsfRequireDestTag; } - if (bClearRequireDest && ((uFlagsIn & lsfRequireDestTag) != 0u)) + if (bClearRequireDest && account_->isFlag(lsfRequireDestTag)) { JLOG(j_.trace()) << "Clear lsfRequireDestTag."; uFlagsOut &= ~lsfRequireDestTag; @@ -347,13 +344,13 @@ AccountSet::doApply() // // DisallowXRP // - if (bSetDisallowXRP && ((uFlagsIn & lsfDisallowXRP) == 0u)) + if (bSetDisallowXRP && !account_->isFlag(lsfDisallowXRP)) { JLOG(j_.trace()) << "Set lsfDisallowXRP."; uFlagsOut |= lsfDisallowXRP; } - if (bClearDisallowXRP && ((uFlagsIn & lsfDisallowXRP) != 0u)) + if (bClearDisallowXRP && account_->isFlag(lsfDisallowXRP)) { JLOG(j_.trace()) << "Clear lsfDisallowXRP."; uFlagsOut &= ~lsfDisallowXRP; @@ -362,7 +359,7 @@ AccountSet::doApply() // // DisableMaster // - if ((uSetFlag == asfDisableMaster) && ((uFlagsIn & lsfDisableMaster) == 0u)) + if ((uSetFlag == asfDisableMaster) && !account_->isFlag(lsfDisableMaster)) { if (!sigWithMaster) { @@ -370,7 +367,8 @@ AccountSet::doApply() return tecNEED_MASTER_KEY; } - if ((!acct->isFieldPresent(sfRegularKey)) && (!view().peek(keylet::signers(accountID_)))) + if ((!account_->isFieldPresent(sfRegularKey)) && + (!view().peek(keylet::signers(accountID_)))) { // Account has no regular key or multi-signer signer list. return tecNO_ALTERNATIVE_KEY; @@ -380,7 +378,7 @@ AccountSet::doApply() uFlagsOut |= lsfDisableMaster; } - if ((uClearFlag == asfDisableMaster) && ((uFlagsIn & lsfDisableMaster) != 0u)) + if ((uClearFlag == asfDisableMaster) && account_->isFlag(lsfDisableMaster)) { JLOG(j_.trace()) << "Clear lsfDisableMaster."; uFlagsOut &= ~lsfDisableMaster; @@ -405,7 +403,7 @@ AccountSet::doApply() // if (uSetFlag == asfNoFreeze) { - if (!sigWithMaster && ((uFlagsIn & lsfDisableMaster) == 0u)) + if (!sigWithMaster && !account_->isFlag(lsfDisableMaster)) { JLOG(j_.trace()) << "Must use master key to set NoFreeze."; return tecNEED_MASTER_KEY; @@ -435,16 +433,16 @@ AccountSet::doApply() // // Track transaction IDs signed by this account in its root // - if ((uSetFlag == asfAccountTxnID) && !acct->isFieldPresent(sfAccountTxnID)) + if ((uSetFlag == asfAccountTxnID) && !account_->isFieldPresent(sfAccountTxnID)) { JLOG(j_.trace()) << "Set AccountTxnID."; - acct->makeFieldPresent(sfAccountTxnID); + account_->makeFieldPresent(sfAccountTxnID); } - if ((uClearFlag == asfAccountTxnID) && acct->isFieldPresent(sfAccountTxnID)) + if ((uClearFlag == asfAccountTxnID) && account_->isFieldPresent(sfAccountTxnID)) { JLOG(j_.trace()) << "Clear AccountTxnID."; - acct->makeFieldAbsent(sfAccountTxnID); + account_->makeFieldAbsent(sfAccountTxnID); } // @@ -471,12 +469,12 @@ AccountSet::doApply() if (!uHash) { JLOG(j_.trace()) << "unset email hash"; - acct->makeFieldAbsent(sfEmailHash); + account_->makeFieldAbsent(sfEmailHash); } else { JLOG(j_.trace()) << "set email hash"; - acct->setFieldH128(sfEmailHash, uHash); + account_->setFieldH128(sfEmailHash, uHash); } } @@ -490,12 +488,12 @@ AccountSet::doApply() if (!uHash) { JLOG(j_.trace()) << "unset wallet locator"; - acct->makeFieldAbsent(sfWalletLocator); + account_->makeFieldAbsent(sfWalletLocator); } else { JLOG(j_.trace()) << "set wallet locator"; - acct->setFieldH256(sfWalletLocator, uHash); + account_->setFieldH256(sfWalletLocator, uHash); } } @@ -509,12 +507,12 @@ AccountSet::doApply() if (messageKey.empty()) { JLOG(j_.debug()) << "clear message key"; - acct->makeFieldAbsent(sfMessageKey); + account_->makeFieldAbsent(sfMessageKey); } else { JLOG(j_.debug()) << "set message key"; - acct->setFieldVL(sfMessageKey, messageKey); + account_->setFieldVL(sfMessageKey, messageKey); } } @@ -528,12 +526,12 @@ AccountSet::doApply() if (domain.empty()) { JLOG(j_.trace()) << "unset domain"; - acct->makeFieldAbsent(sfDomain); + account_->makeFieldAbsent(sfDomain); } else { JLOG(j_.trace()) << "set domain"; - acct->setFieldVL(sfDomain, domain); + account_->setFieldVL(sfDomain, domain); } } @@ -547,12 +545,12 @@ AccountSet::doApply() if (uRate == 0 || uRate == QUALITY_ONE) { JLOG(j_.trace()) << "unset transfer rate"; - acct->makeFieldAbsent(sfTransferRate); + account_->makeFieldAbsent(sfTransferRate); } else { JLOG(j_.trace()) << "set transfer rate"; - acct->setFieldU32(sfTransferRate, uRate); + account_->setFieldU32(sfTransferRate, uRate); } } @@ -565,21 +563,21 @@ AccountSet::doApply() if ((uTickSize == 0) || (uTickSize == Quality::kMaxTickSize)) { JLOG(j_.trace()) << "unset tick size"; - acct->makeFieldAbsent(sfTickSize); + account_->makeFieldAbsent(sfTickSize); } else { JLOG(j_.trace()) << "set tick size"; - acct->setFieldU8(sfTickSize, uTickSize); + account_->setFieldU8(sfTickSize, uTickSize); } } // Configure authorized minting account: if (uSetFlag == asfAuthorizedNFTokenMinter) - acct->setAccountID(sfNFTokenMinter, ctx_.tx[sfNFTokenMinter]); + account_->setAccountID(sfNFTokenMinter, ctx_.tx[sfNFTokenMinter]); - if (uClearFlag == asfAuthorizedNFTokenMinter && acct->isFieldPresent(sfNFTokenMinter)) - acct->makeFieldAbsent(sfNFTokenMinter); + if (uClearFlag == asfAuthorizedNFTokenMinter && account_->isFieldPresent(sfNFTokenMinter)) + account_->makeFieldAbsent(sfNFTokenMinter); if (uSetFlag == asfDisallowIncomingNFTokenOffer) { @@ -638,9 +636,9 @@ AccountSet::doApply() } if (uFlagsIn != uFlagsOut) - acct->setFieldU32(sfFlags, uFlagsOut); + account_->setFieldU32(sfFlags, uFlagsOut); - acct.update(); + account_.update(); return tesSUCCESS; } diff --git a/src/libxrpl/tx/transactors/account/SetRegularKey.cpp b/src/libxrpl/tx/transactors/account/SetRegularKey.cpp index 9657bb090f..d308d6a058 100644 --- a/src/libxrpl/tx/transactors/account/SetRegularKey.cpp +++ b/src/libxrpl/tx/transactors/account/SetRegularKey.cpp @@ -29,8 +29,7 @@ SetRegularKey::calculateBaseFee(ReadView const& view, STTx const& tx) if (calcAccountID(PublicKey(makeSlice(spk))) == id) { AccountRoot const acct(id, view); - - if (acct && ((acct->getFlags() & lsfPasswordSpent) == 0u)) + if (acct && !acct->isFlag(lsfPasswordSpent)) { // flag is armed and they signed with the right account return XRPAmount{0}; @@ -56,27 +55,26 @@ SetRegularKey::preflight(PreflightContext const& ctx) TER SetRegularKey::doApply() { - WAccountRoot acct(accountID_, view(), j_); - if (!acct) + if (!account_) return tefINTERNAL; // LCOV_EXCL_LINE if (!minimumFee(ctx_.registry, ctx_.baseFee, view().fees(), view().flags())) - acct->setFlag(lsfPasswordSpent); + account_->setFlag(lsfPasswordSpent); if (ctx_.tx.isFieldPresent(sfRegularKey)) { - acct->setAccountID(sfRegularKey, ctx_.tx.getAccountID(sfRegularKey)); + account_->setAccountID(sfRegularKey, ctx_.tx.getAccountID(sfRegularKey)); } else { // Account has disabled master key and no multi-signer signer list. - if (acct->isFlag(lsfDisableMaster) && !view().peek(keylet::signers(accountID_))) + if (account_->isFlag(lsfDisableMaster) && !view().peek(keylet::signers(accountID_))) return tecNO_ALTERNATIVE_KEY; - acct->makeFieldAbsent(sfRegularKey); + account_->makeFieldAbsent(sfRegularKey); } - acct.update(); + account_.update(); return tesSUCCESS; } diff --git a/src/libxrpl/tx/transactors/account/SignerListSet.cpp b/src/libxrpl/tx/transactors/account/SignerListSet.cpp index 1986435dea..806f867350 100644 --- a/src/libxrpl/tx/transactors/account/SignerListSet.cpp +++ b/src/libxrpl/tx/transactors/account/SignerListSet.cpp @@ -194,7 +194,7 @@ removeSignersFromLedger( return tesSUCCESS; // There are two different ways that the OwnerCount could be managed. - // If the lsfOneOwnerCount bit is Operation::Set then remove just one owner count. + // If the lsfOneOwnerCount bit is set then remove just one owner count. // Otherwise use the pre-MultiSignReserve amendment calculation. int removeFromOwnerCount = -1; if (!signers->isFlag(lsfOneOwnerCount)) @@ -252,9 +252,6 @@ SignerListSet::validateQuorumAndSignerEntries( } // Make sure there are no duplicate signers. - // SignerEntry only defines operator< and operator==, not the full - // std::totally_ordered Operation::Set required by std::ranges::less, so the - // ranges version does not compile. NOLINTNEXTLINE(modernize-use-ranges) XRPL_ASSERT( std::ranges::is_sorted(signers), "xrpl::SignerListSet::validateQuorumAndSignerEntries : sorted " @@ -378,7 +375,7 @@ SignerListSet::writeSignersToSLE(SLE::pointer const& ledgerEntry, std::uint32_t STArray toLedger(signers_.size()); for (auto const& entry : signers_) { - toLedger.push_back(STObject::makeInnerObject(sfSignerEntry)); + toLedger.pushBack(STObject::makeInnerObject(sfSignerEntry)); STObject& obj = toLedger.back(); obj.reserve(2); obj[sfAccount] = entry.account; diff --git a/src/libxrpl/tx/transactors/bridge/XChainBridge.cpp b/src/libxrpl/tx/transactors/bridge/XChainBridge.cpp index 680ed5e07d..525adfde4f 100644 --- a/src/libxrpl/tx/transactors/bridge/XChainBridge.cpp +++ b/src/libxrpl/tx/transactors/bridge/XChainBridge.cpp @@ -129,7 +129,7 @@ checkAttestationPublicKey( if (accountFromPK == attestationSignerAccount) { // master key - if ((acctSigner->getFieldU32(sfFlags) & lsfDisableMaster) != 0u) + if (acctSigner->isFlag(lsfDisableMaster)) { JLOG(j.trace()) << "Attempt to add an attestation with " "disabled master key."; @@ -406,7 +406,7 @@ transferHelper( { // Check dst tag and deposit auth - if (((acctDst->getFlags() & lsfRequireDestTag) != 0u) && !dstTag) + if (acctDst->isFlag(lsfRequireDestTag) && !dstTag) return tecDST_TAG_NEEDED; // If the destination is the claim owner, and this is a claim @@ -415,7 +415,7 @@ transferHelper( bool const canBypassDepositAuth = dst == claimOwner && depositAuthPolicy == DepositAuthPolicy::DstCanBypass; - if (!canBypassDepositAuth && ((acctDst->getFlags() & lsfDepositAuth) != 0u) && + if (!canBypassDepositAuth && acctDst->isFlag(lsfDepositAuth) && !psb.exists(keylet::depositPreauth(dst, src))) { return tecNO_PERMISSION; @@ -1422,7 +1422,7 @@ XChainCreateBridge::preclaim(PreclaimContext const& ctx) // Allowing clawing back funds would break the bridge's invariant that // wrapped funds are always backed by locked funds - if ((acctIssuer->getFlags() & lsfAllowTrustLineClawback) != 0u) + if (acctIssuer->isFlag(lsfAllowTrustLineClawback)) return tecNO_PERMISSION; } @@ -1433,7 +1433,7 @@ XChainCreateBridge::preclaim(PreclaimContext const& ctx) return terNO_ACCOUNT; auto const balance = acctSrc->at(sfBalance); - auto const reserve = ctx.view.fees().accountReserve(acctSrc->getFieldU32(sfOwnerCount) + 1); + auto const reserve = ctx.view.fees().accountReserve(acctSrc->at(sfOwnerCount) + 1); if (balance < reserve) return tecINSUFFICIENT_RESERVE; @@ -1976,7 +1976,7 @@ XChainCreateClaimID::preclaim(PreclaimContext const& ctx) return terNO_ACCOUNT; auto const balance = acctSrc->at(sfBalance); - auto const reserve = ctx.view.fees().accountReserve(acctSrc->getFieldU32(sfOwnerCount) + 1); + auto const reserve = ctx.view.fees().accountReserve(acctSrc->at(sfOwnerCount) + 1); if (balance < reserve) return tecINSUFFICIENT_RESERVE; diff --git a/src/libxrpl/tx/transactors/check/CheckCash.cpp b/src/libxrpl/tx/transactors/check/CheckCash.cpp index 79beb071eb..dcc4f5ae48 100644 --- a/src/libxrpl/tx/transactors/check/CheckCash.cpp +++ b/src/libxrpl/tx/transactors/check/CheckCash.cpp @@ -115,8 +115,7 @@ CheckCash::preclaim(PreclaimContext const& ctx) return tecNO_ENTRY; } - if (((acctDst->getFlags() & lsfRequireDestTag) != 0u) && - !sleCheck->isFieldPresent(sfDestinationTag)) + if (acctDst->isFlag(lsfRequireDestTag) && !sleCheck->isFieldPresent(sfDestinationTag)) { // The tag is basically account-specific information we don't // understand, but we can require someone to fill it in. @@ -200,7 +199,7 @@ CheckCash::preclaim(PreclaimContext const& ctx) return tecNO_ISSUER; } - if ((issuer->at(sfFlags) & lsfRequireAuth) != 0u) + if (issuer->isFlag(lsfRequireAuth)) { if (!sleTrustLine) { diff --git a/src/libxrpl/tx/transactors/check/CheckCreate.cpp b/src/libxrpl/tx/transactors/check/CheckCreate.cpp index 1f1bedc50c..f081d9312e 100644 --- a/src/libxrpl/tx/transactors/check/CheckCreate.cpp +++ b/src/libxrpl/tx/transactors/check/CheckCreate.cpp @@ -86,10 +86,8 @@ CheckCreate::preclaim(PreclaimContext const& ctx) return tecNO_DST; } - auto const flags = acctDst->getFlags(); - // Check if the destination has disallowed incoming checks - if ((flags & lsfDisallowIncomingCheck) != 0u) + if (acctDst->isFlag(lsfDisallowIncomingCheck)) return tecNO_PERMISSION; // Pseudo-accounts cannot cash checks. Note, this is not amendment-gated @@ -99,7 +97,7 @@ CheckCreate::preclaim(PreclaimContext const& ctx) if (acctDst.isPseudoAccount()) return tecNO_PERMISSION; - if (((flags & lsfRequireDestTag) != 0u) && !ctx.tx.isFieldPresent(sfDestinationTag)) + if (acctDst->isFlag(lsfRequireDestTag) && !ctx.tx.isFieldPresent(sfDestinationTag)) { // The tag is basically account-specific information we don't // understand, but we can require someone to fill it in. diff --git a/src/libxrpl/tx/transactors/delegate/DelegateSet.cpp b/src/libxrpl/tx/transactors/delegate/DelegateSet.cpp index 23707bf490..715dfe467a 100644 --- a/src/libxrpl/tx/transactors/delegate/DelegateSet.cpp +++ b/src/libxrpl/tx/transactors/delegate/DelegateSet.cpp @@ -115,7 +115,7 @@ DelegateSet::doApply() (*sle)[sfOwnerNode] = *page; // Add to authorized account's owner directory so AccountDelete can find - // and clean up inbound delegations. + // and clean up inbound delegations when the authorized account is deleted. auto const destPage = ctx_.view().dirInsert( keylet::ownerDir(authAccount), delegateKey, describeOwnerDir(authAccount)); @@ -139,6 +139,7 @@ DelegateSet::deleteDelegate(ApplyView& view, std::shared_ptr const& sle, be auto const delegator = (*sle)[sfAccount]; auto const delegatee = (*sle)[sfAuthorize]; + // Remove from delegating account's owner directory if (!view.dirRemove(keylet::ownerDir(delegator), (*sle)[sfOwnerNode], sle->key(), false)) { // LCOV_EXCL_START @@ -147,6 +148,7 @@ DelegateSet::deleteDelegate(ApplyView& view, std::shared_ptr const& sle, be // LCOV_EXCL_STOP } + // Remove from authorized account's owner directory, if present if (auto const optPage = (*sle)[~sfDestinationNode]) { if (!view.dirRemove(keylet::ownerDir(delegatee), *optPage, sle->key(), false)) @@ -158,6 +160,7 @@ DelegateSet::deleteDelegate(ApplyView& view, std::shared_ptr const& sle, be } } + // Only the delegating account's owner count was incremented on creation WAccountRoot wrappedOwner(delegator, view, j); if (!wrappedOwner) return tecINTERNAL; // LCOV_EXCL_LINE diff --git a/src/libxrpl/tx/transactors/dex/AMMBid.cpp b/src/libxrpl/tx/transactors/dex/AMMBid.cpp index e54a67936c..d8ea99b3e9 100644 --- a/src/libxrpl/tx/transactors/dex/AMMBid.cpp +++ b/src/libxrpl/tx/transactors/dex/AMMBid.cpp @@ -333,6 +333,7 @@ applyBid(ApplyContext& ctx, Sandbox& sb, AccountID const& accountId, beast::Jour // Other intervals slot price return pricePurchased * p105 * (1 - power(fractionUsed, 60)) + minSlotPrice; }(); + // NOLINTEND(bugprone-unchecked-optional-access) auto const payPrice = getPayPrice(computedPrice); diff --git a/src/libxrpl/tx/transactors/dex/AMMVote.cpp b/src/libxrpl/tx/transactors/dex/AMMVote.cpp index ff7c0c2d17..7ef0c22e2d 100644 --- a/src/libxrpl/tx/transactors/dex/AMMVote.cpp +++ b/src/libxrpl/tx/transactors/dex/AMMVote.cpp @@ -108,7 +108,7 @@ applyVote(ApplyContext& ctx, Sandbox& sb, AccountID const& accountId, beast::Jou auto lpTokens = ammLPHolds(sb, *ammSle, entryAccount, ctx.journal); if (lpTokens == beast::kZero) { - JLOG(j.debug()) << "AMMVote::applyVote, account " << entryAccount << " is not LP"; + JLOG(j.debug()) << "AMMVote::applyVote, accountId " << entryAccount << " is not LP"; continue; } auto feeVal = entry[sfTradingFee]; @@ -132,24 +132,17 @@ applyVote(ApplyContext& ctx, Sandbox& sb, AccountID const& accountId, beast::Jou // Find an entry with the least tokens/fee. Make the order deterministic // if the tokens/fees are equal. - if (!minTokens) + if (!minTokens || + (lpTokens < *minTokens || + (lpTokens == *minTokens && + (feeVal < minFee || (feeVal == minFee && entryAccount < minAccount))))) { minTokens = lpTokens; minPos = updatedVoteSlots.size(); minAccount = entryAccount; minFee = feeVal; } - else if ( - auto const& minTokensValue = *minTokens; lpTokens < minTokensValue || - (lpTokens == minTokensValue && - (feeVal < minFee || (feeVal == minFee && entryAccount < minAccount)))) - { - minTokens = lpTokens; - minPos = updatedVoteSlots.size(); - minAccount = entryAccount; - minFee = feeVal; - } - updatedVoteSlots.push_back(std::move(newEntry)); + updatedVoteSlots.pushBack(std::move(newEntry)); } // The account doesn't have the vote entry. @@ -172,7 +165,7 @@ applyVote(ApplyContext& ctx, Sandbox& sb, AccountID const& accountId, beast::Jou } else { - updatedVoteSlots.push_back(std::move(newEntry)); + updatedVoteSlots.pushBack(std::move(newEntry)); } }; // Add new entry if the number of the vote entries @@ -183,23 +176,17 @@ applyVote(ApplyContext& ctx, Sandbox& sb, AccountID const& accountId, beast::Jou // Add the entry if the account has more tokens than // the least token holder or same tokens and higher fee. } - else if (minTokens) + // NOLINTBEGIN(bugprone-unchecked-optional-access) slots full means loop ran, minTokens is + // set + else if (lpTokensNew > *minTokens || (lpTokensNew == *minTokens && feeNew > minFee)) { - auto const& minTokensValue = *minTokens; - if (lpTokensNew > minTokensValue || (lpTokensNew == minTokensValue && feeNew > minFee)) - { - auto const entry = updatedVoteSlots.begin() + minPos; - // Remove the least token vote entry. - num -= Number((*entry)[~sfTradingFee].valueOr(0)) * minTokensValue; - den -= minTokensValue; - update(minPos); - } - else - { - JLOG(j.debug()) << "AMMVote::applyVote, insufficient tokens to " - "override other votes"; - } + auto const entry = updatedVoteSlots.begin() + minPos; + // Remove the least token vote entry. + num -= Number((*entry)[~sfTradingFee].valueOr(0)) * *minTokens; + den -= *minTokens; + update(minPos); } + // NOLINTEND(bugprone-unchecked-optional-access) // All slots are full and the account does not hold more LPTokens. // Update anyway to refresh the slots. else diff --git a/src/libxrpl/tx/transactors/dex/OfferCancel.cpp b/src/libxrpl/tx/transactors/dex/OfferCancel.cpp index 8236d94516..695b2c1fb0 100644 --- a/src/libxrpl/tx/transactors/dex/OfferCancel.cpp +++ b/src/libxrpl/tx/transactors/dex/OfferCancel.cpp @@ -40,7 +40,7 @@ OfferCancel::preclaim(PreclaimContext const& ctx) if (!acct) return terNO_ACCOUNT; - if (acct->getFieldU32(sfSequence) <= offerSequence) + if (acct->at(sfSequence) <= offerSequence) { JLOG(ctx.j.trace()) << "Malformed transaction: " << "Sequence " << offerSequence << " is invalid."; diff --git a/src/libxrpl/tx/transactors/dex/OfferCreate.cpp b/src/libxrpl/tx/transactors/dex/OfferCreate.cpp index f1c554881b..f9c953ad95 100644 --- a/src/libxrpl/tx/transactors/dex/OfferCreate.cpp +++ b/src/libxrpl/tx/transactors/dex/OfferCreate.cpp @@ -577,7 +577,7 @@ OfferCreate::applyHybrid( auto bookInfo = STObject::makeInnerObject(sfBook); bookInfo.setFieldH256(sfBookDirectory, dir.key); bookInfo.setFieldU64(sfBookNode, *bookNode); - bookArr.push_back(std::move(bookInfo)); + bookArr.pushBack(std::move(bookInfo)); if (!bookExists) ctx_.registry.get().getOrderBookDB().addOrderBook(book); @@ -640,7 +640,6 @@ OfferCreate::applyGuts(Sandbox& sb, Sandbox& sbCancel) return {tecEXPIRED, true}; } - bool const bOpenLedger = sb.open(); bool crossed = false; if (isTesSuccess(result)) @@ -684,7 +683,7 @@ OfferCreate::applyGuts(Sandbox& sb, Sandbox& sbCancel) } if (!saTakerGets || !saTakerPays) { - JLOG(j_.debug()) << "Offer rounded to beast::kZERO"; + JLOG(j_.debug()) << "Offer rounded to zero"; return {result, true}; } @@ -729,7 +728,7 @@ OfferCreate::applyGuts(Sandbox& sb, Sandbox& sbCancel) stream << " out: " << formatAmount(placeOffer.out); } - if (result == tecFAILED_PROCESSING && bOpenLedger) + if (result == tecFAILED_PROCESSING && sb.open()) result = telFAILED_PROCESSING; if (!isTesSuccess(result)) diff --git a/src/libxrpl/tx/transactors/escrow/EscrowCreate.cpp b/src/libxrpl/tx/transactors/escrow/EscrowCreate.cpp index 72b613f16a..9f04f5023c 100644 --- a/src/libxrpl/tx/transactors/escrow/EscrowCreate.cpp +++ b/src/libxrpl/tx/transactors/escrow/EscrowCreate.cpp @@ -441,7 +441,7 @@ EscrowCreate::doApply() AccountRoot const acctDest(ctx_.tx[sfDestination], ctx_.view()); if (!acctDest) return tecNO_DST; // LCOV_EXCL_LINE - if (((acctDest->getFlags() & lsfRequireDestTag) != 0u) && !ctx_.tx[~sfDestinationTag]) + if (acctDest->isFlag(lsfRequireDestTag) && !ctx_.tx[~sfDestinationTag]) return tecDST_TAG_NEEDED; } diff --git a/src/libxrpl/tx/transactors/lending/LoanPay.cpp b/src/libxrpl/tx/transactors/lending/LoanPay.cpp index da224caffa..4b143ca87d 100644 --- a/src/libxrpl/tx/transactors/lending/LoanPay.cpp +++ b/src/libxrpl/tx/transactors/lending/LoanPay.cpp @@ -100,7 +100,7 @@ LoanPay::calculateBaseFee(ReadView const& view, STTx const& tx) if (loanSle->at(sfPaymentRemaining) <= kLoanPaymentsPerFeeIncrement) { - // If there are fewer than kLOAN_PAYMENTS_PER_FEE_INCREMENT payments left to + // If there are fewer than kLoanPaymentsPerFeeIncrement payments left to // pay, we can skip the computations. return normalCost; } diff --git a/src/libxrpl/tx/transactors/payment/Payment.cpp b/src/libxrpl/tx/transactors/payment/Payment.cpp index 2f3eec5ac5..306d348ee8 100644 --- a/src/libxrpl/tx/transactors/payment/Payment.cpp +++ b/src/libxrpl/tx/transactors/payment/Payment.cpp @@ -353,9 +353,7 @@ Payment::preclaim(PreclaimContext const& ctx) return tecNO_DST_INSUF_XRP; } } - else if ( - ((dstAcct->getFlags() & lsfRequireDestTag) != 0u) && - !ctx.tx.isFieldPresent(sfDestinationTag)) + else if (dstAcct->isFlag(lsfRequireDestTag) && !ctx.tx.isFieldPresent(sfDestinationTag)) { // The tag is basically account-specific information we don't // understand, but we can require someone to fill it in. @@ -663,7 +661,7 @@ Payment::doApply() dst->setFieldAmount(sfBalance, dst->getFieldAmount(sfBalance) + dstAmount); // Re-arm the password change fee if we can and need to. - if ((dst->getFlags() & lsfPasswordSpent) != 0u) + if (dst->isFlag(lsfPasswordSpent)) dst->clearFlag(lsfPasswordSpent); return tesSUCCESS; diff --git a/src/libxrpl/tx/transactors/payment_channel/PaymentChannelCreate.cpp b/src/libxrpl/tx/transactors/payment_channel/PaymentChannelCreate.cpp index afa332f9d6..241b7a5708 100644 --- a/src/libxrpl/tx/transactors/payment_channel/PaymentChannelCreate.cpp +++ b/src/libxrpl/tx/transactors/payment_channel/PaymentChannelCreate.cpp @@ -79,7 +79,7 @@ PaymentChannelCreate::preclaim(PreclaimContext const& ctx) // Check reserve and funds availability { auto const balance = acctSrc->at(sfBalance); - auto const reserve = ctx.view.fees().accountReserve(acctSrc->getFieldU32(sfOwnerCount) + 1); + auto const reserve = ctx.view.fees().accountReserve(acctSrc->at(sfOwnerCount) + 1); if (balance < reserve) return tecINSUFFICIENT_RESERVE; @@ -96,13 +96,11 @@ PaymentChannelCreate::preclaim(PreclaimContext const& ctx) if (!acctDst) return tecNO_DST; - auto const flags = acctDst->getFlags(); - // Check if they have disallowed incoming payment channels - if ((flags & lsfDisallowIncomingPayChan) != 0u) + if (acctDst->isFlag(lsfDisallowIncomingPayChan)) return tecNO_PERMISSION; - if (((flags & lsfRequireDestTag) != 0u) && !ctx.tx[~sfDestinationTag]) + if (acctDst->isFlag(lsfRequireDestTag) && !ctx.tx[~sfDestinationTag]) return tecDST_TAG_NEEDED; // Pseudo-accounts cannot receive payment channels, other than native diff --git a/src/libxrpl/tx/transactors/system/Change.cpp b/src/libxrpl/tx/transactors/system/Change.cpp index 1c772697e0..92a06fd807 100644 --- a/src/libxrpl/tx/transactors/system/Change.cpp +++ b/src/libxrpl/tx/transactors/system/Change.cpp @@ -200,7 +200,7 @@ Change::applyAmendment() else { // pass through - newMajorities.push_back(majority); + newMajorities.pushBack(majority); } } } @@ -211,7 +211,7 @@ Change::applyAmendment() if (gotMajority) { // This amendment now has a majority - newMajorities.push_back(STObject::makeInnerObject(sfMajority)); + newMajorities.pushBack(STObject::makeInnerObject(sfMajority)); auto& entry = newMajorities.back(); entry[sfAmendment] = amendment; entry[sfCloseTime] = view().parentCloseTime().time_since_epoch().count(); diff --git a/src/libxrpl/tx/transactors/token/TrustSet.cpp b/src/libxrpl/tx/transactors/token/TrustSet.cpp index 849b1a9378..30b03b2154 100644 --- a/src/libxrpl/tx/transactors/token/TrustSet.cpp +++ b/src/libxrpl/tx/transactors/token/TrustSet.cpp @@ -193,10 +193,9 @@ TrustSet::preclaim(PreclaimContext const& ctx) if (!acct) return terNO_ACCOUNT; - std::uint32_t const uTxFlags = ctx.tx.getFlags(); - bool const bSetAuth = (uTxFlags & tfSetfAuth) != 0u; + bool const bSetAuth = ctx.tx.isFlag(tfSetfAuth); - if (bSetAuth && ((acct->getFieldU32(sfFlags) & lsfRequireAuth) == 0u)) + if (bSetAuth && !acct->isFlag(lsfRequireAuth)) { JLOG(ctx.j.trace()) << "Retry: Auth not required."; return tefNO_AUTH_REQUIRED; @@ -218,7 +217,7 @@ TrustSet::preclaim(PreclaimContext const& ctx) // If the destination has opted to disallow incoming trustlines // then honour that flag - if ((acctDst->getFlags() & lsfDisallowIncomingTrustline) != 0u) + if (acctDst->isFlag(lsfDisallowIncomingTrustline)) { // The original implementation of featureDisallowIncoming was // too restrictive. If @@ -283,8 +282,8 @@ TrustSet::preclaim(PreclaimContext const& ctx) if (ctx.view.rules().enabled(featureDeepFreeze)) { bool const bNoFreeze = acct->isFlag(lsfNoFreeze); - bool const bSetFreeze = (uTxFlags & tfSetFreeze) != 0u; - bool const bSetDeepFreeze = (uTxFlags & tfSetDeepFreeze) != 0u; + bool const bSetFreeze = ctx.tx.isFlag(tfSetFreeze); + bool const bSetDeepFreeze = ctx.tx.isFlag(tfSetDeepFreeze); if (bNoFreeze && (bSetFreeze || bSetDeepFreeze)) { @@ -530,8 +529,8 @@ TrustSet::doApply() if (QUALITY_ONE == uHighQualityOut) uHighQualityOut = 0; - bool const bLowDefRipple = (lowAcct->getFlags() & lsfDefaultRipple) != 0u; - bool const bHighDefRipple = (highAcct->getFlags() & lsfDefaultRipple) != 0u; + bool const bLowDefRipple = lowAcct->isFlag(lsfDefaultRipple); + bool const bHighDefRipple = highAcct->isFlag(lsfDefaultRipple); bool const bLowReserveSet = (uLowQualityIn != 0u) || (uLowQualityOut != 0u) || ((uFlagsOut & lsfLowNoRipple) == 0) != bLowDefRipple || diff --git a/src/test/app/AMMMPT_test.cpp b/src/test/app/AMMMPT_test.cpp index 6e94ee5887..eba388e5fd 100644 --- a/src/test/app/AMMMPT_test.cpp +++ b/src/test/app/AMMMPT_test.cpp @@ -7037,7 +7037,7 @@ private: } // This test validates both invariant changes work together for - // the specific case of MPT/MPT pools with > kMAX_DELETABLE_AMM_TRUST_LINES. + // the specific case of MPT/MPT pools with > kMaxDeletableAmmTrustLines. { Env env( *this, diff --git a/src/test/app/Invariants_test.cpp b/src/test/app/Invariants_test.cpp index 719d414882..07e47e658e 100644 --- a/src/test/app/Invariants_test.cpp +++ b/src/test/app/Invariants_test.cpp @@ -94,8 +94,7 @@ class Invariants_test : public beast::unit_test::Suite static FeatureBitset defaultAmendments() { - return xrpl::test::jtx::testableAmendments() | featureSingleAssetVault | fixCleanup3_1_3 | - fixCleanup3_2_0; + return xrpl::test::jtx::testableAmendments() | fixCleanup3_1_3 | fixCleanup3_2_0; } /** Run a specific test case to put the ledger into a state that will be @@ -181,9 +180,9 @@ class Invariants_test : public beast::unit_test::Suite beast::Journal const jlog{sink}; ApplyContext ac{env.app(), ov, tx, tesSUCCESS, env.current()->fees().base, TapNone, jlog}; - // Invariants normally run in Transactor::operator(), which installs - // the current ledger rules for rule-aware protocol helpers. - CurrentTransactionRulesGuard const rulesGuard(ov.rules()); + // Invariants normally run in the Transaction's "apply" (operator()) context, and can always + // access global Rules. + CurrentTransactionRulesGuard const rg(ov.rules()); BEAST_EXPECT(precheck(a1, a2, ac)); @@ -303,10 +302,11 @@ class Invariants_test : public beast::unit_test::Suite doInvariantCheck( {{"account deletion left behind a non-zero balance"}}, - [&](Account const& a1, Account const& a2, ApplyContext& ac) { + // NOLINTNEXTLINE(readability-identifier-naming) + [&](Account const& A1, Account const& A2, ApplyContext& ac) { // A1 has a balance. Delete A1 - auto const a1ID = a1.id(); - auto const sleA1 = ac.view().peek(keylet::account(a1ID)); + auto const a1 = A1.id(); + auto const sleA1 = ac.view().peek(keylet::account(a1)); if (!sleA1) return false; if (!BEAST_EXPECT(*sleA1->at(sfBalance) != beast::kZero)) @@ -321,10 +321,11 @@ class Invariants_test : public beast::unit_test::Suite doInvariantCheck( {{"account deletion left behind a non-zero owner count"}}, - [&](Account const& a1, Account const& a2, ApplyContext& ac) { + // NOLINTNEXTLINE(readability-identifier-naming) + [&](Account const& A1, Account const& A2, ApplyContext& ac) { // Increment A1's owner count, then delete A1 - auto const a1ID = a1.id(); - WAccountRoot wrappedA1(a1ID, ac.view()); + auto const a1 = A1.id(); + WAccountRoot wrappedA1(a1, ac.view(), ac.journal); if (!wrappedA1) return false; // Clear the balance so the "account deletion left behind a @@ -356,15 +357,16 @@ class Invariants_test : public beast::unit_test::Suite doInvariantCheck( {{"account deletion left behind a "s + type.cStr() + " object"}}, - [&](Account const& a1, Account const& a2, ApplyContext& ac) { + // NOLINTNEXTLINE(readability-identifier-naming) + [&](Account const& A1, Account const& A2, ApplyContext& ac) { // Add an object to the ledger for account A1, then delete // A1 - auto const a1ID = a1.id(); - auto sleA1 = ac.view().peek(keylet::account(a1ID)); + auto const a1 = A1.id(); + auto sleA1 = ac.view().peek(keylet::account(a1)); if (!sleA1) return false; - auto const key = std::invoke(keyletfunc, a1ID); + auto const key = std::invoke(keyletfunc, a1); auto const newSLE = std::make_shared(key); ac.view().insert(newSLE); // Clear the balance so the "account deletion left behind a @@ -939,7 +941,7 @@ class Invariants_test : public beast::unit_test::Suite return false; auto sleNew = std::make_shared(keylet::escrow(a1, (*sle)[sfSequence] + 2)); - MPTIssue const mpt{MPTIssue{makeMptID(1, AccountID(0x4985601))}}; + MPTIssue const mpt{makeMptID(1, AccountID(0x4985601))}; STAmount const amt(mpt, -1); sleNew->setFieldAmount(sfAmount, amt); ac.view().insert(sleNew); @@ -955,7 +957,7 @@ class Invariants_test : public beast::unit_test::Suite if (!sle) return false; - MPTIssue const mpt{MPTIssue{makeMptID(1, AccountID(0x4985601))}}; + MPTIssue const mpt{makeMptID(1, AccountID(0x4985601))}; auto sleNew = std::make_shared(keylet::mptIssuance(mpt.getMptID())); sleNew->setFieldU64(sfOutstandingAmount, -1); ac.view().insert(sleNew); @@ -971,7 +973,7 @@ class Invariants_test : public beast::unit_test::Suite if (!sle) return false; - MPTIssue const mpt{MPTIssue{makeMptID(1, AccountID(0x4985601))}}; + MPTIssue const mpt{makeMptID(1, AccountID(0x4985601))}; auto sleNew = std::make_shared(keylet::mptIssuance(mpt.getMptID())); sleNew->setFieldU64(sfLockedAmount, -1); ac.view().insert(sleNew); @@ -987,7 +989,7 @@ class Invariants_test : public beast::unit_test::Suite if (!sle) return false; - MPTIssue const mpt{MPTIssue{makeMptID(1, AccountID(0x4985601))}}; + MPTIssue const mpt{makeMptID(1, AccountID(0x4985601))}; auto sleNew = std::make_shared(keylet::mptIssuance(mpt.getMptID())); sleNew->setFieldU64(sfOutstandingAmount, 1); sleNew->setFieldU64(sfLockedAmount, 10); @@ -1004,7 +1006,7 @@ class Invariants_test : public beast::unit_test::Suite if (!sle) return false; - MPTIssue const mpt{MPTIssue{makeMptID(1, AccountID(0x4985601))}}; + MPTIssue const mpt{makeMptID(1, AccountID(0x4985601))}; auto sleNew = std::make_shared(keylet::mptoken(mpt.getMptID(), a1)); sleNew->setFieldU64(sfMPTAmount, -1); ac.view().insert(sleNew); @@ -1020,7 +1022,7 @@ class Invariants_test : public beast::unit_test::Suite if (!sle) return false; - MPTIssue const mpt{MPTIssue{makeMptID(1, AccountID(0x4985601))}}; + MPTIssue const mpt{makeMptID(1, AccountID(0x4985601))}; auto sleNew = std::make_shared(keylet::mptoken(mpt.getMptID(), a1)); sleNew->setFieldU64(sfLockedAmount, -1); ac.view().insert(sleNew); @@ -1162,7 +1164,7 @@ class Invariants_test : public beast::unit_test::Suite STObject newNFToken(*nfTokenTemplate, sfNFToken, [&nftID](STObject& object) { object.setFieldH256(sfNFTokenID, nftID); }); - ret.push_back(std::move(newNFToken)); + ret.pushBack(std::move(newNFToken)); ++nftID; } return ret; @@ -1298,7 +1300,7 @@ class Invariants_test : public beast::unit_test::Suite cred.setAccountID(sfIssuer, a2); auto credType = "cred_type" + std::to_string(n); cred.setFieldVL(sfCredentialType, Slice(credType.c_str(), credType.size())); - credentials.push_back(std::move(cred)); + credentials.pushBack(std::move(cred)); } sle->setFieldArray(sfAcceptedCredentials, credentials); } @@ -1355,7 +1357,7 @@ class Invariants_test : public beast::unit_test::Suite cred.setAccountID(sfIssuer, a2); auto credType = std::string("cred_type") + std::to_string(9 - n); cred.setFieldVL(sfCredentialType, Slice(credType.c_str(), credType.size())); - credentials.push_back(std::move(cred)); + credentials.pushBack(std::move(cred)); } slePd->setFieldArray(sfAcceptedCredentials, credentials); ac.view().update(slePd); @@ -1378,7 +1380,7 @@ class Invariants_test : public beast::unit_test::Suite auto cred = STObject::makeInnerObject(sfCredential); cred.setAccountID(sfIssuer, a2); cred.setFieldVL(sfCredentialType, Slice("cred_type", 9)); - credentials.push_back(std::move(cred)); + credentials.pushBack(std::move(cred)); } slePd->setFieldArray(sfAcceptedCredentials, credentials); ac.view().update(slePd); @@ -1427,7 +1429,7 @@ class Invariants_test : public beast::unit_test::Suite cred.setAccountID(sfIssuer, a2); auto credType = "cred_type2" + std::to_string(n); cred.setFieldVL(sfCredentialType, Slice(credType.c_str(), credType.size())); - credentials.push_back(std::move(cred)); + credentials.pushBack(std::move(cred)); } slePd->setFieldArray(sfAcceptedCredentials, credentials); @@ -1457,7 +1459,7 @@ class Invariants_test : public beast::unit_test::Suite cred.setAccountID(sfIssuer, a2); auto credType = std::string("cred_type2") + std::to_string(9 - n); cred.setFieldVL(sfCredentialType, Slice(credType.c_str(), credType.size())); - credentials.push_back(std::move(cred)); + credentials.pushBack(std::move(cred)); } slePd->setFieldArray(sfAcceptedCredentials, credentials); @@ -1486,7 +1488,7 @@ class Invariants_test : public beast::unit_test::Suite auto cred = STObject::makeInnerObject(sfCredential); cred.setAccountID(sfIssuer, a2); cred.setFieldVL(sfCredentialType, Slice("cred_type", 9)); - credentials.push_back(std::move(cred)); + credentials.pushBack(std::move(cred)); } slePd->setFieldArray(sfAcceptedCredentials, credentials); ac.view().update(slePd); @@ -1841,7 +1843,7 @@ class Invariants_test : public beast::unit_test::Suite sleOffer->setFlag(lsfHybrid); STArray bookArr; - bookArr.push_back(STObject::makeInnerObject(sfBook)); + bookArr.pushBack(STObject::makeInnerObject(sfBook)); sleOffer->setFieldArray(sfAdditionalBooks, bookArr); ac.view().insert(sleOffer); return true; @@ -1877,8 +1879,8 @@ class Invariants_test : public beast::unit_test::Suite sleOffer->setFieldH256(sfDomainID, pd1); STArray bookArr; - bookArr.push_back(STObject::makeInnerObject(sfBook)); - bookArr.push_back(STObject::makeInnerObject(sfBook)); + bookArr.pushBack(STObject::makeInnerObject(sfBook)); + bookArr.pushBack(STObject::makeInnerObject(sfBook)); sleOffer->setFieldArray(sfAdditionalBooks, bookArr); ac.view().insert(sleOffer); return true; @@ -3970,7 +3972,7 @@ class Invariants_test : public beast::unit_test::Suite if (!sle) return false; - MPTIssue const mpt{MPTIssue{makeMptID(sle->getFieldU32(sfSequence), a1)}}; + MPTIssue const mpt{makeMptID(sle->getFieldU32(sfSequence), a1)}; auto sleNew = std::make_shared(keylet::mptIssuance(mpt.getMptID())); sleNew->setFieldU64(sfOutstandingAmount, 110); sleNew->setFieldU64(sfMaximumAmount, 100); @@ -3987,7 +3989,7 @@ class Invariants_test : public beast::unit_test::Suite if (!sle) return false; - MPTIssue const mpt{MPTIssue{makeMptID(sle->getFieldU32(sfSequence), a1)}}; + MPTIssue const mpt{makeMptID(sle->getFieldU32(sfSequence), a1)}; auto sleNew = std::make_shared(keylet::mptIssuance(mpt.getMptID())); sleNew->setFieldU64(sfOutstandingAmount, 100); sleNew->setFieldU64(sfMaximumAmount, 100); @@ -4058,7 +4060,7 @@ class Invariants_test : public beast::unit_test::Suite auto seq = sle->getFieldU32(sfSequence); for (int i = 0; i < nTokens; ++i) { - MPTIssue const mpt{MPTIssue{makeMptID(seq + i, a1)}}; + MPTIssue const mpt{makeMptID(seq + i, a1)}; auto sleNew = std::make_shared(keylet::mptIssuance(mpt.getMptID())); ac.view().insert(sleNew); diff --git a/src/xrpld/rpc/detail/PathRequest.cpp b/src/xrpld/rpc/detail/PathRequest.cpp index 9d115ea655..b1407193d3 100644 --- a/src/xrpld/rpc/detail/PathRequest.cpp +++ b/src/xrpld/rpc/detail/PathRequest.cpp @@ -11,7 +11,6 @@ #include #include #include -#include #include #include #include @@ -44,7 +43,6 @@ #include #include #include -#include #include #include #include @@ -182,8 +180,6 @@ PathRequest::isValid(std::shared_ptr const& crCache) { if (!raSrcAccount_ || !raDstAccount_) return false; - auto const& srcAccount = *raSrcAccount_; - auto const& dstAccount = *raDstAccount_; if (!convert_all_ && (saSendMax_ || saDstAmount_ <= beast::kZero)) { @@ -194,14 +190,14 @@ PathRequest::isValid(std::shared_ptr const& crCache) auto const& lrLedger = crCache->getLedger(); - if (!lrLedger->exists(keylet::account(srcAccount))) + if (!lrLedger->exists(keylet::account(*raSrcAccount_))) { // Source account does not exist. jvStatus_ = rpcError(RpcSrcActNotFound); return false; } - AccountRoot const acctDest(dstAccount, *lrLedger); + AccountRoot const acctDest(*raDstAccount_, *lrLedger); json::Value& jvDestCur = (jvStatus_[jss::destination_currencies] = json::ValueType::Array); @@ -224,14 +220,14 @@ PathRequest::isValid(std::shared_ptr const& crCache) } else { - bool const disallowXRP((acctDest->getFlags() & lsfDisallowXRP) != 0u); + bool const disallowXRP(acctDest->isFlag(lsfDisallowXRP)); - auto const destAssets = accountDestAssets(dstAccount, crCache, !disallowXRP); + auto const destAssets = accountDestAssets(*raDstAccount_, crCache, !disallowXRP); for (auto const& asset : destAssets) jvDestCur.append(to_string(asset)); - jvStatus_[jss::destination_tag] = (acctDest->getFlags() & lsfRequireDestTag); + jvStatus_[jss::destination_tag] = acctDest->isFlag(lsfRequireDestTag); } jvStatus_[jss::ledger_hash] = to_string(lrLedger->header().hash); @@ -264,7 +260,6 @@ PathRequest::doCreate(std::shared_ptr const& cache, json::Value cons { if (valid) { - // NOLINTNEXTLINE(bugprone-unchecked-optional-access) valid is from isValid() stream << iIdentifier_ << " valid: " << toBase58(*raSrcAccount_); stream << iIdentifier_ << " deliver: " << saDstAmount_.getFullText(); } @@ -506,21 +501,27 @@ PathRequest::doAborting() const std::unique_ptr const& PathRequest::getPathFinder( std::shared_ptr const& cache, - hash_map>& pathassetMap, - PathAsset const& asset, + hash_map>& currencyMap, + PathAsset const& currency, STAmount const& dstAmount, int const level, std::function const& continueCallback) { - auto i = pathassetMap.find(asset); - if (i != pathassetMap.end()) + auto i = currencyMap.find(currency); + if (i != currencyMap.end()) return i->second; - if (!raSrcAccount_ || !raDstAccount_) - Throw("PathRequest::getPathFinder: missing accounts"); - auto const& srcAccount = *raSrcAccount_; - auto const& dstAccount = *raDstAccount_; + // NOLINTBEGIN(bugprone-unchecked-optional-access) isValid() ensures both are set auto pathfinder = std::make_unique( - cache, srcAccount, dstAccount, asset, std::nullopt, dstAmount, saSendMax_, domain_, app_); + cache, + *raSrcAccount_, + *raDstAccount_, + currency, + std::nullopt, + dstAmount, + saSendMax_, + domain_, + app_); + // NOLINTEND(bugprone-unchecked-optional-access) if (pathfinder->findPaths(level, continueCallback)) { pathfinder->computePathRanks(kMaxPaths, continueCallback); @@ -529,7 +530,7 @@ PathRequest::getPathFinder( { pathfinder.reset(); // It's a bad request - clear it. } - return pathassetMap[asset] = std::move(pathfinder); + return currencyMap[currency] = std::move(pathfinder); } bool @@ -539,10 +540,6 @@ PathRequest::findPaths( json::Value& jvArray, std::function const& continueCallback) { - if (!raSrcAccount_ || !raDstAccount_) - Throw("PathRequest::findPaths: missing accounts"); - auto const& srcAccount = *raSrcAccount_; - auto const& dstAccount = *raDstAccount_; auto sourceAssets = sciSourceAssets_; if (sourceAssets.empty() && saSendMax_) { @@ -550,8 +547,10 @@ PathRequest::findPaths( } if (sourceAssets.empty()) { - auto assets = accountSourceAssets(srcAccount, cache, true); - bool const sameAccount = srcAccount == dstAccount; + // NOLINTBEGIN(bugprone-unchecked-optional-access) isValid() ensures both are set + auto assets = accountSourceAssets(*raSrcAccount_, cache, true); + bool const sameAccount = *raSrcAccount_ == *raDstAccount_; + // NOLINTEND(bugprone-unchecked-optional-access) for (auto const& asset : assets) { if (!std::visit( @@ -563,7 +562,7 @@ PathRequest::findPaths( if constexpr (std::is_same_v) { sourceAssets.insert( - Issue{a, a.isZero() ? xrpAccount() : srcAccount}); + Issue{a, a.isZero() ? xrpAccount() : *raSrcAccount_}); } else { @@ -580,7 +579,7 @@ PathRequest::findPaths( } auto const dstAmount = convertAmount(saDstAmount_, convert_all_); - hash_map> pathassetMap; + hash_map> currencyMap; for (auto const& asset : sourceAssets) { if (continueCallback && !continueCallback()) @@ -589,7 +588,7 @@ PathRequest::findPaths( << " Trying to find paths: " << STAmount(asset, 1).getFullText(); auto& pathfinder = - getPathFinder(cache, pathassetMap, asset, dstAmount, level, continueCallback); + getPathFinder(cache, currencyMap, PathAsset(asset), dstAmount, level, continueCallback); if (!pathfinder) { JLOG(journal_.debug()) << iIdentifier_ << " No paths found"; @@ -608,7 +607,7 @@ PathRequest::findPaths( if (isXRP(asset)) return xrpAccount(); - return srcAccount; + return *raSrcAccount_; }(); STAmount const saMaxAmount = [&]() { @@ -632,10 +631,12 @@ PathRequest::findPaths( saMaxAmount, // --> Amount to send is unlimited // to get an estimate. dstAmount, // --> Amount to deliver. - dstAccount, // --> Account to deliver to. - srcAccount, // --> Account sending from. - ps, // --> Path set. - domain_, // --> Domain. + // NOLINTBEGIN(bugprone-unchecked-optional-access) isValid() ensures both are set + *raDstAccount_, // --> Account to deliver to. + *raSrcAccount_, // --> Account sending from. + // NOLINTEND(bugprone-unchecked-optional-access) + ps, // --> Path set. + domain_, // --> Domain. app_, &rcInput); @@ -651,10 +652,12 @@ PathRequest::findPaths( saMaxAmount, // --> Amount to send is unlimited // to get an estimate. dstAmount, // --> Amount to deliver. - dstAccount, // --> Account to deliver to. - srcAccount, // --> Account sending from. - ps, // --> Path set. - domain_, // --> Domain. + // NOLINTBEGIN(bugprone-unchecked-optional-access) isValid() ensures both are set + *raDstAccount_, // --> Account to deliver to. + *raSrcAccount_, // --> Account sending from. + // NOLINTEND(bugprone-unchecked-optional-access) + ps, // --> Path set. + domain_, // --> Domain. app_); if (!isTesSuccess(rc.result())) @@ -738,8 +741,8 @@ PathRequest::doUpdate( // NOLINTBEGIN(bugprone-unchecked-optional-access) isValid() ensures both are set newStatus[jss::source_account] = toBase58(*raSrcAccount_); newStatus[jss::destination_account] = toBase58(*raDstAccount_); - newStatus[jss::destination_amount] = saDstAmount_.getJson(JsonOptions::Values::None); // NOLINTEND(bugprone-unchecked-optional-access) + newStatus[jss::destination_amount] = saDstAmount_.getJson(JsonOptions::Values::None); newStatus[jss::full_reply] = !fast; if (jvId_) diff --git a/src/xrpld/rpc/detail/Pathfinder.cpp b/src/xrpld/rpc/detail/Pathfinder.cpp index 15a5836414..d22e5b5e58 100644 --- a/src/xrpld/rpc/detail/Pathfinder.cpp +++ b/src/xrpld/rpc/detail/Pathfinder.cpp @@ -8,6 +8,7 @@ #include #include +#include #include #include #include @@ -39,9 +40,23 @@ #include #include #include +#include #include #include +namespace xrpl { +static std::ostream& +operator<<(std::ostream& os, Pathfinder::NodeType t) +{ + return os << static_cast(t); +} +static std::ostream& +operator<<(std::ostream& os, Pathfinder::PaymentType t) +{ + return os << static_cast(t); +} +} // namespace xrpl + /* Core Pathfinding Engine @@ -128,7 +143,7 @@ struct PathCost }; using PathCostList = std::vector; -PathTable gMPathTable; +PathTable gPathTable; std::string pathTypeToString(Pathfinder::PathType const& type) @@ -343,14 +358,14 @@ Pathfinder::findPaths(int searchLevel, std::function const& continue } // Now iterate over all paths for that paymentType. - for (auto const& costedPath : gMPathTable[paymentType]) + for (auto const& costedPath : gPathTable[paymentType]) { if (continueCallback && !continueCallback()) return false; // Only use paths with at most the current search level. if (costedPath.searchLevel <= searchLevel) { - JLOG(j_.trace()) << "findPaths trying payment type " << static_cast(paymentType); + JLOG(j_.trace()) << "findPaths trying payment type " << paymentType; addPathsForType(costedPath.type, continueCallback); if (completePaths_.size() > kPathfinderMaxCompletePaths) @@ -748,20 +763,19 @@ Pathfinder::getPathsOut( if (!inserted) return it->second; - AccountRoot const acctRoot(account, *ledger_); + RAccountRoot const acctRoot(account, *ledger_); if (!acctRoot) return 0; - auto const aFlags = acctRoot->getFieldU32(sfFlags); bool const bAuthRequired = [&]() { if (pathAsset.holds()) - return (aFlags & lsfRequireAuth) != 0; + return acctRoot->isFlag(lsfRequireAuth); return !isTesSuccess(requireAuth(*ledger_, asset.get(), account)); }(); bool const bFrozen = [&]() { if (pathAsset.holds()) - return (aFlags & lsfGlobalFreeze) != 0; + return acctRoot.isGlobalFrozen(); return isGlobalFrozen(*ledger_, asset.get()); }(); @@ -860,7 +874,7 @@ Pathfinder::addPathsForType( PathType const& pathType, std::function const& continueCallback) { - JLOG(j_.debug()) << "addPathsForType " << pathTypeToString(pathType); + JLOG(j_.debug()) << "addPathsForType " << CollectionAndDelimiter(pathType, ", "); // See if the set of paths for this type already exists. auto it = paths_.find(pathType); if (it != paths_.end()) @@ -1162,7 +1176,6 @@ Pathfinder::addLink( { std::ranges::sort( candidates, - std::bind( compareAccountCandidate, ledger_->seq(), @@ -1357,7 +1370,7 @@ makePath(char const* string) void fillPaths(Pathfinder::PaymentType type, PathCostList const& costs) { - auto& list = gMPathTable[type]; + auto& list = gPathTable[type]; XRPL_ASSERT(list.empty(), "xrpl::fillPaths : empty paths"); for (auto& cost : costs) list.push_back({cost.cost, makePath(cost.path)}); @@ -1377,7 +1390,7 @@ Pathfinder::initPathTable() { // CAUTION: Do not include rules that build default paths - gMPathTable.clear(); + gPathTable.clear(); fillPaths(PaymentType::XrpToXrp, {}); /* cspell: disable */ diff --git a/src/xrpld/rpc/handlers/account/AccountInfo.cpp b/src/xrpld/rpc/handlers/account/AccountInfo.cpp index 6376ac7e8c..931a476d88 100644 --- a/src/xrpld/rpc/handlers/account/AccountInfo.cpp +++ b/src/xrpld/rpc/handlers/account/AccountInfo.cpp @@ -38,7 +38,7 @@ namespace xrpl { * @brief Injects JSON describing a ledger entry. * * @param jv The JSON value to populate. - * @param sle The ledger entry to describe. + * @param account The ledger entry to describe. * * @details * Populates the provided JSON value with the description of the specified diff --git a/src/xrpld/rpc/handlers/account/NoRippleCheck.cpp b/src/xrpld/rpc/handlers/account/NoRippleCheck.cpp index b3fb169e84..b43398d9e7 100644 --- a/src/xrpld/rpc/handlers/account/NoRippleCheck.cpp +++ b/src/xrpld/rpc/handlers/account/NoRippleCheck.cpp @@ -116,16 +116,16 @@ doNoRippleCheck(RPC::JsonContext& context) json::Value& problems = (result["problems"] = json::ValueType::Array); - bool const bDefaultRipple = (acct->getFieldU32(sfFlags) & lsfDefaultRipple) != 0u; + bool const bDefaultRipple = acct->isFlag(lsfDefaultRipple); - if ((static_cast(bDefaultRipple) & static_cast(!roleGateway)) != 0) + if (bDefaultRipple && !roleGateway) { problems.append( "You appear to have set your default ripple flag even though you " "are not a gateway. This is not recommended unless you are " "experimenting"); } - else if ((static_cast(roleGateway) & static_cast(!bDefaultRipple)) != 0) + else if (roleGateway && !bDefaultRipple) { problems.append("You should immediately set your default ripple flag"); if (transactions) @@ -147,12 +147,12 @@ doNoRippleCheck(RPC::JsonContext& context) std::string problem; bool needFix = false; - if (bNoRipple & roleGateway) + if (bNoRipple && roleGateway) { problem = "You should clear the no ripple flag on your "; needFix = true; } - else if (!roleGateway & !bNoRipple) + else if (!roleGateway && !bNoRipple) { problem = "You should probably set the no ripple flag on your "; needFix = true; diff --git a/src/xrpld/rpc/handlers/orderbook/AMMInfo.cpp b/src/xrpld/rpc/handlers/orderbook/AMMInfo.cpp index ecab891c6b..d75d33bf50 100644 --- a/src/xrpld/rpc/handlers/orderbook/AMMInfo.cpp +++ b/src/xrpld/rpc/handlers/orderbook/AMMInfo.cpp @@ -13,7 +13,6 @@ #include #include #include -#include #include #include #include @@ -37,7 +36,7 @@ namespace xrpl { Expected -getJsonAsset(json::Value const& v, beast::Journal j) +getAsset(json::Value const& v, beast::Journal j) { try { @@ -51,7 +50,7 @@ getJsonAsset(json::Value const& v, beast::Journal j) } std::string -to_iso8601(NetClock::time_point tp) +toIso8601(NetClock::time_point tp) { // 2000-01-01 00:00:00 UTC is 946684800s from 1970-01-01 00:00:00 UTC using namespace std::chrono; @@ -97,7 +96,7 @@ doAMMInfo(RPC::JsonContext& context) if (params.isMember(jss::asset)) { - if (auto const i = getJsonAsset(params[jss::asset], context.j)) + if (auto const i = getAsset(params[jss::asset], context.j)) { asset1 = *i; } @@ -109,7 +108,7 @@ doAMMInfo(RPC::JsonContext& context) if (params.isMember(jss::asset2)) { - if (auto const i = getJsonAsset(params[jss::asset2], context.j)) + if (auto const i = getAsset(params[jss::asset2], context.j)) { asset2 = *i; } @@ -225,7 +224,7 @@ doAMMInfo(RPC::JsonContext& context) auction[jss::discounted_fee] = auctionSlot[sfDiscountedFee]; auction[jss::account] = to_string(auctionSlot.getAccountID(sfAccount)); auction[jss::expiration] = - to_iso8601(NetClock::time_point{NetClock::duration{auctionSlot[sfExpiration]}}); + toIso8601(NetClock::time_point{NetClock::duration{auctionSlot[sfExpiration]}}); if (auctionSlot.isFieldPresent(sfAuthAccounts)) { json::Value auth; diff --git a/src/xrpld/rpc/handlers/orderbook/DepositAuthorized.cpp b/src/xrpld/rpc/handlers/orderbook/DepositAuthorized.cpp index 98bde5ec3c..cf8594c72d 100644 --- a/src/xrpld/rpc/handlers/orderbook/DepositAuthorized.cpp +++ b/src/xrpld/rpc/handlers/orderbook/DepositAuthorized.cpp @@ -87,7 +87,7 @@ doDepositAuthorized(RPC::JsonContext& context) return result; } - bool const reqAuth = ((acctDest->getFlags() & lsfDepositAuth) != 0u) && (srcAcct != dstAcct); + bool const reqAuth = acctDest->isFlag(lsfDepositAuth) && (srcAcct != dstAcct); bool const credentialsPresent = params.isMember(jss::credentials); std::set> sorted;