fix clang-tidy

This commit is contained in:
Mayukha Vadari
2026-05-14 18:36:48 -04:00
parent 554c0baa0a
commit 144835ca5d
5 changed files with 76 additions and 42 deletions

View File

@@ -48,6 +48,7 @@ public:
operator=(SLEBase const&) = delete;
SLEBase&
operator=(SLEBase&&) = delete;
SLEBase() = delete;
// --- Common interface (always available) ---
@@ -176,8 +177,6 @@ public:
}
protected:
SLEBase() = delete;
/** Constructor for read-only context */
explicit SLEBase(
SLE::const_pointer sle,

View File

@@ -580,6 +580,8 @@ DirectStepI<TDerived>::setCacheLimiting(
IOUAmount const& fwdOut,
DebtDirection srcDebtDir)
{
// NOLINTBEGIN(bugprone-unchecked-optional-access) cache_ always set before setCacheLimiting is
// called
if (cache_->in < fwdIn)
{
IOUAmount const smallDiff(1, -9);
@@ -609,6 +611,7 @@ DirectStepI<TDerived>::setCacheLimiting(
if (fwdOut < cache_->out)
cache_->out = fwdOut;
cache_->srcDebtDir = srcDebtDir;
// NOLINTEND(bugprone-unchecked-optional-access)
};
template <class TDerived>
@@ -620,6 +623,7 @@ DirectStepI<TDerived>::fwdImp(
IOUAmount const& in)
{
XRPL_ASSERT(cache_, "xrpl::DirectStepI::fwdImp : cache is set");
// NOLINTBEGIN(bugprone-unchecked-optional-access) assert above
auto const [maxSrcToDst, srcDebtDir] =
static_cast<TDerived const*>(this)->maxFlow(sb, cache_->srcToDst);
@@ -676,6 +680,7 @@ DirectStepI<TDerived>::fwdImp(
<< " srcToDst: " << to_string(srcToDst) << " out: " << to_string(out);
}
return {cache_->in, cache_->out};
// NOLINTEND(bugprone-unchecked-optional-access)
}
template <class TDerived>
@@ -706,6 +711,7 @@ DirectStepI<TDerived>::validFwd(PaymentSandbox& sb, ApplyView& afView, EitherAmo
return {false, EitherAmount(IOUAmount(beast::kZERO))};
}
// NOLINTBEGIN(bugprone-unchecked-optional-access) fwdImp sets cache_ on success
if (maxSrcToDst < cache_->srcToDst)
{
JLOG(j_.warn()) << "DirectStepI: Strand re-execute check failed."
@@ -725,6 +731,7 @@ DirectStepI<TDerived>::validFwd(PaymentSandbox& sb, ApplyView& afView, EitherAmo
return {false, EitherAmount(cache_->out)};
}
return {true, EitherAmount(cache_->out)};
// NOLINTEND(bugprone-unchecked-optional-access)
}
// Returns srcQOut, dstQIn

View File

@@ -556,6 +556,8 @@ MPTEndpointStep<TDerived>::setCacheLimiting(
MPTAmount const& fwdOut,
DebtDirection srcDebtDir)
{
// NOLINTBEGIN(bugprone-unchecked-optional-access) cache_ always set before setCacheLimiting is
// called
if (cache_->in < fwdIn)
{
MPTAmount const smallDiff(1);
@@ -585,6 +587,7 @@ MPTEndpointStep<TDerived>::setCacheLimiting(
if (fwdOut < cache_->out)
cache_->out = fwdOut;
cache_->srcDebtDir = srcDebtDir;
// NOLINTEND(bugprone-unchecked-optional-access)
};
template <class TDerived>
@@ -596,6 +599,7 @@ MPTEndpointStep<TDerived>::fwdImp(
MPTAmount const& in)
{
XRPL_ASSERT(cache_, "MPTEndpointStep<TDerived>::fwdImp : valid cache");
// NOLINTBEGIN(bugprone-unchecked-optional-access) assert above
auto const [maxSrcToDst, srcDebtDir] = static_cast<TDerived const*>(this)->maxPaymentFlow(sb);
@@ -669,6 +673,7 @@ MPTEndpointStep<TDerived>::fwdImp(
<< " srcToDst: " << to_string(srcToDst) << " out: " << to_string(out);
}
return {cache_->in, cache_->out};
// NOLINTEND(bugprone-unchecked-optional-access)
}
template <class TDerived>
@@ -698,6 +703,7 @@ MPTEndpointStep<TDerived>::validFwd(PaymentSandbox& sb, ApplyView& afView, Eithe
return {false, EitherAmount(MPTAmount(beast::kZERO))};
}
// NOLINTBEGIN(bugprone-unchecked-optional-access) fwdImp sets cache_ on success
if (maxSrcToDst < cache_->srcToDst)
{
JLOG(j_.warn()) << "MPTEndpointStep: Strand re-execute check failed."
@@ -717,6 +723,7 @@ MPTEndpointStep<TDerived>::validFwd(PaymentSandbox& sb, ApplyView& afView, Eithe
return {false, EitherAmount(cache_->out)};
}
return {true, EitherAmount(cache_->out)};
// NOLINTEND(bugprone-unchecked-optional-access)
}
// Returns srcQOut, dstQIn

View File

@@ -133,10 +133,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 ||
(lpTokens < *minTokens ||
(lpTokens == *minTokens &&
(feeVal < minFee || (feeVal == minFee && account < minAccount)))))
if (!minTokens)
{
minTokens = lpTokens;
minPos = updatedVoteSlots.size();
minAccount = account;
minFee = feeVal;
}
else if (
auto const& minTokensValue = *minTokens; lpTokens < minTokensValue ||
(lpTokens == minTokensValue &&
(feeVal < minFee || (feeVal == minFee && account < minAccount))))
{
minTokens = lpTokens;
minPos = updatedVoteSlots.size();
@@ -177,13 +184,22 @@ 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 (lpTokensNew > *minTokens || (lpTokensNew == *minTokens && feeNew > minFee))
else if (minTokens)
{
auto const entry = updatedVoteSlots.begin() + minPos;
// Remove the least token vote entry.
num -= Number((*entry)[~sfTradingFee].valueOr(0)) * *minTokens;
den -= *minTokens;
update(minPos);
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";
}
}
// All slots are full and the account does not hold more LPTokens.
// Update anyway to refresh the slots.

View File

@@ -11,6 +11,7 @@
#include <xrpl/basics/Log.h>
#include <xrpl/basics/UnorderedContainers.h>
#include <xrpl/basics/base_uint.h>
#include <xrpl/basics/contract.h>
#include <xrpl/beast/utility/Journal.h>
#include <xrpl/beast/utility/Zero.h>
#include <xrpl/beast/utility/instrumentation.h>
@@ -43,6 +44,7 @@
#include <memory>
#include <mutex>
#include <optional>
#include <stdexcept>
#include <string>
#include <utility>
#include <variant>
@@ -180,6 +182,8 @@ PathRequest::isValid(std::shared_ptr<AssetCache> const& crCache)
{
if (!raSrcAccount_ || !raDstAccount_)
return false;
auto const& srcAccount = *raSrcAccount_;
auto const& dstAccount = *raDstAccount_;
if (!convert_all_ && (saSendMax_ || saDstAmount_ <= beast::kZERO))
{
@@ -190,14 +194,14 @@ PathRequest::isValid(std::shared_ptr<AssetCache> const& crCache)
auto const& lrLedger = crCache->getLedger();
if (!lrLedger->exists(keylet::account(*raSrcAccount_)))
if (!lrLedger->exists(keylet::account(srcAccount)))
{
// Source account does not exist.
jvStatus_ = rpcError(RpcSrcActNotFound);
return false;
}
AccountRoot const acctDest(*raDstAccount_, *lrLedger);
AccountRoot const acctDest(dstAccount, *lrLedger);
json::Value& jvDestCur = (jvStatus_[jss::destination_currencies] = json::ValueType::Array);
@@ -222,7 +226,7 @@ PathRequest::isValid(std::shared_ptr<AssetCache> const& crCache)
{
bool const disallowXRP((acctDest->getFlags() & lsfDisallowXRP) != 0u);
auto const destAssets = accountDestAssets(*raDstAccount_, crCache, !disallowXRP);
auto const destAssets = accountDestAssets(dstAccount, crCache, !disallowXRP);
for (auto const& asset : destAssets)
jvDestCur.append(to_string(asset));
@@ -260,6 +264,7 @@ PathRequest::doCreate(std::shared_ptr<AssetCache> 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();
}
@@ -510,16 +515,12 @@ PathRequest::getPathFinder(
auto i = pathassetMap.find(asset);
if (i != pathassetMap.end())
return i->second;
if (!raSrcAccount_ || !raDstAccount_)
Throw<std::logic_error>("PathRequest::getPathFinder: missing accounts");
auto const& srcAccount = *raSrcAccount_;
auto const& dstAccount = *raDstAccount_;
auto pathfinder = std::make_unique<Pathfinder>(
cache,
*raSrcAccount_,
*raDstAccount_,
asset,
std::nullopt,
dstAmount,
saSendMax_,
domain_,
app_);
cache, srcAccount, dstAccount, asset, std::nullopt, dstAmount, saSendMax_, domain_, app_);
if (pathfinder->findPaths(level, continueCallback))
{
pathfinder->computePathRanks(kMAX_PATHS, continueCallback);
@@ -538,6 +539,10 @@ PathRequest::findPaths(
json::Value& jvArray,
std::function<bool(void)> const& continueCallback)
{
if (!raSrcAccount_ || !raDstAccount_)
Throw<std::logic_error>("PathRequest::findPaths: missing accounts");
auto const& srcAccount = *raSrcAccount_;
auto const& dstAccount = *raDstAccount_;
auto sourceAssets = sciSourceAssets_;
if (sourceAssets.empty() && saSendMax_)
{
@@ -545,8 +550,8 @@ PathRequest::findPaths(
}
if (sourceAssets.empty())
{
auto assets = accountSourceAssets(*raSrcAccount_, cache, true);
bool const sameAccount = *raSrcAccount_ == *raDstAccount_;
auto assets = accountSourceAssets(srcAccount, cache, true);
bool const sameAccount = srcAccount == dstAccount;
for (auto const& asset : assets)
{
if (!std::visit(
@@ -558,7 +563,7 @@ PathRequest::findPaths(
if constexpr (std::is_same_v<TAsset, Currency>)
{
sourceAssets.insert(
Issue{a, a.isZero() ? xrpAccount() : *raSrcAccount_});
Issue{a, a.isZero() ? xrpAccount() : srcAccount});
}
else
{
@@ -603,7 +608,7 @@ PathRequest::findPaths(
if (isXRP(asset))
return xrpAccount();
return *raSrcAccount_;
return srcAccount;
}();
STAmount const saMaxAmount = [&]() {
@@ -624,13 +629,13 @@ PathRequest::findPaths(
auto sandbox = std::make_unique<PaymentSandbox>(&*cache->getLedger(), TapNone);
auto rc = path::RippleCalc::rippleCalculate(
*sandbox,
saMaxAmount, // --> Amount to send is unlimited
// to get an estimate.
dstAmount, // --> Amount to deliver.
*raDstAccount_, // --> Account to deliver to.
*raSrcAccount_, // --> Account sending from.
ps, // --> Path set.
domain_, // --> Domain.
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.
app_,
&rcInput);
@@ -643,13 +648,13 @@ PathRequest::findPaths(
sandbox = std::make_unique<PaymentSandbox>(&*cache->getLedger(), TapNone);
rc = path::RippleCalc::rippleCalculate(
*sandbox,
saMaxAmount, // --> Amount to send is unlimited
// to get an estimate.
dstAmount, // --> Amount to deliver.
*raDstAccount_, // --> Account to deliver to.
*raSrcAccount_, // --> Account sending from.
ps, // --> Path set.
domain_, // --> Domain.
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.
app_);
if (!isTesSuccess(rc.result()))