From 19da25812bf067d2e382cecda378999714633460 Mon Sep 17 00:00:00 2001 From: Alex Kremer Date: Thu, 23 Apr 2026 17:21:01 +0100 Subject: [PATCH] fix: Remaining clang-tidy unchecked optionals (#6979) --- .clang-tidy | 2 +- .../scripts/levelization/results/ordering.txt | 1 + include/xrpl/protocol/PublicKey.h | 5 +-- include/xrpl/tx/ApplyContext.h | 8 +++-- include/xrpl/tx/paths/OfferStream.h | 3 +- include/xrpl/tx/paths/detail/StrandFlow.h | 3 ++ src/libxrpl/net/HTTPClient.cpp | 8 ++++- src/libxrpl/server/Manifest.cpp | 16 ++++++++-- src/libxrpl/shamap/SHAMapInnerNode.cpp | 13 +++++--- src/libxrpl/tx/ApplyContext.cpp | 9 +++--- src/libxrpl/tx/invariants/AMMInvariant.cpp | 6 ++++ src/libxrpl/tx/paths/BookStep.cpp | 10 +++++- src/libxrpl/tx/paths/DirectStep.cpp | 7 +++++ src/libxrpl/tx/paths/Flow.cpp | 2 +- src/libxrpl/tx/paths/MPTEndpointStep.cpp | 7 +++++ src/libxrpl/tx/paths/OfferStream.cpp | 3 ++ src/libxrpl/tx/transactors/dex/AMMBid.cpp | 2 ++ src/libxrpl/tx/transactors/dex/AMMVote.cpp | 3 ++ .../tx/transactors/payment/DepositPreauth.cpp | 1 + src/test/app/AMMMPT_test.cpp | 12 +++++++ src/test/app/AccountTxPaging_test.cpp | 2 -- src/test/app/GRPCServerTLS_test.cpp | 20 +++++++++--- src/test/core/Config_test.cpp | 1 + src/test/jtx/TrustedPublisherServer.h | 13 ++++++-- src/test/jtx/impl/TestHelpers.cpp | 18 +++++++++-- src/test/jtx/mpt.h | 2 +- src/test/protocol/Hooks_test.cpp | 3 +- src/xrpld/app/ledger/LedgerHistory.cpp | 3 +- src/xrpld/app/main/Application.cpp | 15 ++++++--- src/xrpld/app/misc/TxQ.h | 6 ++-- src/xrpld/app/misc/detail/TxQ.cpp | 11 ++++++- src/xrpld/app/misc/detail/ValidatorList.cpp | 12 ++++++- src/xrpld/app/misc/detail/ValidatorSite.cpp | 6 ++-- src/xrpld/consensus/Consensus.h | 25 +++++++++++++-- src/xrpld/consensus/LedgerTrie.h | 6 +++- src/xrpld/overlay/detail/OverlayImpl.cpp | 3 +- src/xrpld/overlay/detail/PeerImp.cpp | 2 ++ src/xrpld/perflog/detail/PerfLogImp.cpp | 1 - src/xrpld/rpc/detail/PathRequest.cpp | 31 +++++++++++++------ src/xrpld/rpc/detail/RPCLedgerHelpers.cpp | 2 ++ 40 files changed, 240 insertions(+), 63 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index 3a21eba6c0..ce12e552c4 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -66,7 +66,7 @@ Checks: "-*, bugprone-terminating-continue, bugprone-throw-keyword-missing, bugprone-too-small-loop-variable, - # bugprone-unchecked-optional-access, # see https://github.com/XRPLF/rippled/pull/6502 + bugprone-unchecked-optional-access, bugprone-undefined-memory-manipulation, bugprone-undelegated-constructor, bugprone-unhandled-exception-at-new, diff --git a/.github/scripts/levelization/results/ordering.txt b/.github/scripts/levelization/results/ordering.txt index 02a14a0077..d2a1894585 100644 --- a/.github/scripts/levelization/results/ordering.txt +++ b/.github/scripts/levelization/results/ordering.txt @@ -93,6 +93,7 @@ test.core > xrpl.basics test.core > xrpl.core test.core > xrpld.core test.core > xrpl.json +test.core > xrpl.protocol test.core > xrpl.rdb test.core > xrpl.server test.csf > xrpl.basics diff --git a/include/xrpl/protocol/PublicKey.h b/include/xrpl/protocol/PublicKey.h index 9003175f3d..8325d2b1d2 100644 --- a/include/xrpl/protocol/PublicKey.h +++ b/include/xrpl/protocol/PublicKey.h @@ -267,9 +267,10 @@ getOrThrow(Json::Value const& v, xrpl::SField const& field) { using namespace xrpl; std::string const b58 = getOrThrow(v, field); - if (auto pubKeyBlob = strUnHex(b58); publicKeyType(makeSlice(*pubKeyBlob))) + if (auto pubKeyBlob = strUnHex(b58); pubKeyBlob && publicKeyType(makeSlice(*pubKeyBlob))) { - return PublicKey{makeSlice(*pubKeyBlob)}; + return PublicKey{makeSlice( + *pubKeyBlob)}; // NOLINT(bugprone-unchecked-optional-access) checked in condition above } for (auto const tokenType : {TokenType::NodePublic, TokenType::AccountPublic}) { diff --git a/include/xrpl/tx/ApplyContext.h b/include/xrpl/tx/ApplyContext.h index 6341c0bcc5..1817969978 100644 --- a/include/xrpl/tx/ApplyContext.h +++ b/include/xrpl/tx/ApplyContext.h @@ -46,20 +46,20 @@ public: ApplyView& view() { - return *view_; + return *view_; // NOLINT(bugprone-unchecked-optional-access) view_ emplaced in constructor } ApplyView const& view() const { - return *view_; + return *view_; // NOLINT(bugprone-unchecked-optional-access) view_ emplaced in constructor } // VFALCO Unfortunately this is necessary RawView& rawView() { - return *view_; + return *view_; // NOLINT(bugprone-unchecked-optional-access) view_ emplaced in constructor } ApplyFlags const& @@ -72,6 +72,7 @@ public: void deliver(STAmount const& amount) { + // NOLINTNEXTLINE(bugprone-unchecked-optional-access) view_ emplaced in constructor view_->deliver(amount); } @@ -98,6 +99,7 @@ public: void destroyXRP(XRPAmount const& fee) { + // NOLINTNEXTLINE(bugprone-unchecked-optional-access) view_ emplaced in constructor view_->rawDestroyXRP(fee); } diff --git a/include/xrpl/tx/paths/OfferStream.h b/include/xrpl/tx/paths/OfferStream.h index e40387e3d2..84dbac9a60 100644 --- a/include/xrpl/tx/paths/OfferStream.h +++ b/include/xrpl/tx/paths/OfferStream.h @@ -103,7 +103,8 @@ public: TOut ownerFunds() const { - return *ownerFunds_; + return *ownerFunds_; // NOLINT(bugprone-unchecked-optional-access) always set after step() + // is called } }; diff --git a/include/xrpl/tx/paths/detail/StrandFlow.h b/include/xrpl/tx/paths/detail/StrandFlow.h index 52b7d94484..306a0b33e7 100644 --- a/include/xrpl/tx/paths/detail/StrandFlow.h +++ b/include/xrpl/tx/paths/detail/StrandFlow.h @@ -234,8 +234,11 @@ flow( } } + // NOLINTBEGIN(bugprone-unchecked-optional-access) cachedIn/Out set after strand is stepped + // above auto const strandIn = *strand.front()->cachedIn(); auto const strandOut = *strand.back()->cachedOut(); + // NOLINTEND(bugprone-unchecked-optional-access) #ifndef NDEBUG { diff --git a/src/libxrpl/net/HTTPClient.cpp b/src/libxrpl/net/HTTPClient.cpp index b39a605313..028da37fa7 100644 --- a/src/libxrpl/net/HTTPClient.cpp +++ b/src/libxrpl/net/HTTPClient.cpp @@ -65,7 +65,9 @@ public: unsigned short const port, std::size_t maxResponseSize, beast::Journal& j) - : mSocket(io_context, httpClientSSLContext->context()) + : mSocket( + io_context, + httpClientSSLContext->context()) // NOLINT(bugprone-unchecked-optional-access) , mResolver(io_context) , mHeader(maxClientHeaderBytes) , mPort(port) @@ -242,6 +244,8 @@ public: { mShutdown = ecResult ? ecResult + // httpClientSSLContext always initialized before use + // NOLINTNEXTLINE(bugprone-unchecked-optional-access) : httpClientSSLContext->preConnectVerify(mSocket.SSLSocket(), mDeqSites[0]); } @@ -278,6 +282,8 @@ public: { JLOG(j_.trace()) << "Connected."; + // httpClientSSLContext always initialized before use + // NOLINTNEXTLINE(bugprone-unchecked-optional-access) mShutdown = httpClientSSLContext->postConnectVerify(mSocket.SSLSocket(), mDeqSites[0]); if (mShutdown) diff --git a/src/libxrpl/server/Manifest.cpp b/src/libxrpl/server/Manifest.cpp index d2d7bd2e38..0108b78782 100644 --- a/src/libxrpl/server/Manifest.cpp +++ b/src/libxrpl/server/Manifest.cpp @@ -494,7 +494,11 @@ ManifestCache::applyManifest(Manifest m) logMftAct(stream, "AcceptedNew", m.masterKey, m.sequence); if (!revoked) - signingToMasterKeys_.emplace(*m.signingKey, m.masterKey); + { + signingToMasterKeys_.emplace( + *m.signingKey, m.masterKey); // NOLINT(bugprone-unchecked-optional-access) + // non-revoked manifest always has signingKey + } auto masterKey = m.masterKey; map_.emplace(std::move(masterKey), std::move(m)); @@ -510,10 +514,16 @@ ManifestCache::applyManifest(Manifest m) if (auto stream = j_.info()) logMftAct(stream, "AcceptedUpdate", m.masterKey, m.sequence, iter->second.sequence); - signingToMasterKeys_.erase(*iter->second.signingKey); + signingToMasterKeys_.erase( + *iter->second.signingKey); // NOLINT(bugprone-unchecked-optional-access) prewriteCheck + // ensures old manifest is not revoked if (!revoked) - signingToMasterKeys_.emplace(*m.signingKey, m.masterKey); + { + signingToMasterKeys_.emplace( + *m.signingKey, m.masterKey); // NOLINT(bugprone-unchecked-optional-access) non-revoked + // manifest always has signingKey + } iter->second = std::move(m); diff --git a/src/libxrpl/shamap/SHAMapInnerNode.cpp b/src/libxrpl/shamap/SHAMapInnerNode.cpp index e501561ee4..d3324e91d0 100644 --- a/src/libxrpl/shamap/SHAMapInnerNode.cpp +++ b/src/libxrpl/shamap/SHAMapInnerNode.cpp @@ -291,7 +291,8 @@ SHAMapInnerNode::setChild(int m, intr_ptr::SharedPtr child) if (child) { - auto const childIndex = *getChildIndex(m); + auto const childIndex = + *getChildIndex(m); // NOLINT(bugprone-unchecked-optional-access) isBranch_ set above auto [_, hashes, children] = hashesAndChildren_.getHashesAndChildren(); hashes[childIndex].zero(); children[childIndex] = std::move(child); @@ -315,6 +316,7 @@ SHAMapInnerNode::shareChild(int m, intr_ptr::SharedPtr const& ch XRPL_ASSERT(child.get() != this, "xrpl::SHAMapInnerNode::shareChild : valid child input"); XRPL_ASSERT(!isEmptyBranch(m), "xrpl::SHAMapInnerNode::shareChild : non-empty branch input"); + // NOLINTNEXTLINE(bugprone-unchecked-optional-access) assert above hashesAndChildren_.getChildren()[*getChildIndex(m)] = child; } @@ -327,7 +329,8 @@ SHAMapInnerNode::getChildPointer(int branch) XRPL_ASSERT( !isEmptyBranch(branch), "xrpl::SHAMapInnerNode::getChildPointer : non-empty branch input"); - auto const index = *getChildIndex(branch); + auto const index = + *getChildIndex(branch); // NOLINT(bugprone-unchecked-optional-access) assert above packed_spinlock sl(lock_, index); std::lock_guard const lock(sl); @@ -342,7 +345,8 @@ SHAMapInnerNode::getChild(int branch) "xrpl::SHAMapInnerNode::getChild : valid branch input"); XRPL_ASSERT(!isEmptyBranch(branch), "xrpl::SHAMapInnerNode::getChild : non-empty branch input"); - auto const index = *getChildIndex(branch); + auto const index = + *getChildIndex(branch); // NOLINT(bugprone-unchecked-optional-access) assert above packed_spinlock sl(lock_, index); std::lock_guard const lock(sl); @@ -370,7 +374,8 @@ SHAMapInnerNode::canonicalizeChild(int branch, intr_ptr::SharedPtrgetHash() == hashes[childIndex], diff --git a/src/libxrpl/tx/ApplyContext.cpp b/src/libxrpl/tx/ApplyContext.cpp index fa17574616..43facecb24 100644 --- a/src/libxrpl/tx/ApplyContext.cpp +++ b/src/libxrpl/tx/ApplyContext.cpp @@ -58,13 +58,14 @@ ApplyContext::discard() std::optional ApplyContext::apply(TER ter) { + // NOLINTNEXTLINE(bugprone-unchecked-optional-access) view_ emplaced in constructor return view_->apply(base_, tx, ter, parentBatchId_, (flags_ & tapDRY_RUN) != 0u, journal); } std::size_t ApplyContext::size() { - return view_->size(); + return view_->size(); // NOLINT(bugprone-unchecked-optional-access) } void @@ -75,7 +76,7 @@ ApplyContext::visit( std::shared_ptr const&, std::shared_ptr const&)> const& func) { - view_->visit(base_, func); + view_->visit(base_, func); // NOLINT(bugprone-unchecked-optional-access) } TER @@ -116,8 +117,8 @@ ApplyContext::checkInvariantsHelper( // short-circuits). While the logic is still correct, the log // message won't be. Every failed invariant should write to the log, // not just the first one. - std::array const finalizers{ - {std::get(checkers).finalize(tx, result, fee, *view_, journal)...}}; + std::array const finalizers{{std::get(checkers).finalize( + tx, result, fee, *view_, journal)...}}; // NOLINT(bugprone-unchecked-optional-access) // call each check's finalizer to see that it passes if (!std::all_of(finalizers.cbegin(), finalizers.cend(), [](auto const& b) { return b; })) diff --git a/src/libxrpl/tx/invariants/AMMInvariant.cpp b/src/libxrpl/tx/invariants/AMMInvariant.cpp index 3cc888dea2..30be382c91 100644 --- a/src/libxrpl/tx/invariants/AMMInvariant.cpp +++ b/src/libxrpl/tx/invariants/AMMInvariant.cpp @@ -152,6 +152,8 @@ ValidAMM::finalizeCreate( // Create invariant: // sqrt(amount * amount2) == LPTokens // all balances are greater than zero + // NOLINTBEGIN(bugprone-unchecked-optional-access) lptAMMBalanceAfter_ set with ammAccount_ + // in visitEntry if (!validBalances(amount, amount2, *lptAMMBalanceAfter_, ZeroAllowed::No) || ammLPTokens(amount, amount2, lptAMMBalanceAfter_->get()) != *lptAMMBalanceAfter_) { @@ -160,6 +162,7 @@ ValidAMM::finalizeCreate( if (enforce) return false; } + // NOLINTEND(bugprone-unchecked-optional-access) } return true; @@ -204,6 +207,8 @@ ValidAMM::generalInvariant( ZeroAllowed zeroAllowed, beast::Journal const& j) const { + // NOLINTBEGIN(bugprone-unchecked-optional-access) ammAccount_ and lptAMMBalanceAfter_ set + // together in visitEntry; callers only invoke this inside else-of-if(!ammAccount_) auto const [amount, amount2] = ammPoolHolds( view, *ammAccount_, tx[sfAsset], tx[sfAsset2], fhIGNORE_FREEZE, ahIGNORE_AUTH, j); // Deposit and Withdrawal invariant: @@ -230,6 +235,7 @@ ValidAMM::generalInvariant( : ((*lptAMMBalanceAfter_ - poolProductMean) / poolProductMean)); return false; } + // NOLINTEND(bugprone-unchecked-optional-access) return true; } diff --git a/src/libxrpl/tx/paths/BookStep.cpp b/src/libxrpl/tx/paths/BookStep.cpp index f05460df61..ceab379301 100644 --- a/src/libxrpl/tx/paths/BookStep.cpp +++ b/src/libxrpl/tx/paths/BookStep.cpp @@ -610,7 +610,13 @@ BookStep::getQualityFunc(ReadView const& v, DebtDirection p // CLOB Quality const q = static_cast(this)->adjustQualityWithFees( - v, *(res->quality()), prevStepDir, WaiveTransferFee::No, OfferType::CLOB, v.rules()); + v, + *(res->quality()), // NOLINT(bugprone-unchecked-optional-access) CLOB QualityFunction + // always has quality set + prevStepDir, + WaiveTransferFee::No, + OfferType::CLOB, + v.rules()); return {QualityFunction{q, QualityFunction::CLOBLikeTag{}}, dir}; } @@ -1285,6 +1291,7 @@ BookStep::validFwd( return {false, EitherAmount(TOut(beast::zero))}; } + // NOLINTBEGIN(bugprone-unchecked-optional-access) fwdImp sets cache_ on success if (!(checkNear(savCache.in, cache_->in) && checkNear(savCache.out, cache_->out))) { JLOG(j_.warn()) << "Strand re-execute check failed." @@ -1295,6 +1302,7 @@ BookStep::validFwd( return {false, EitherAmount(cache_->out)}; } return {true, EitherAmount(cache_->out)}; + // NOLINTEND(bugprone-unchecked-optional-access) } template diff --git a/src/libxrpl/tx/paths/DirectStep.cpp b/src/libxrpl/tx/paths/DirectStep.cpp index c4b2c51934..1692f50781 100644 --- a/src/libxrpl/tx/paths/DirectStep.cpp +++ b/src/libxrpl/tx/paths/DirectStep.cpp @@ -580,6 +580,8 @@ DirectStepI::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::setCacheLimiting( if (fwdOut < cache_->out) cache_->out = fwdOut; cache_->srcDebtDir = srcDebtDir; + // NOLINTEND(bugprone-unchecked-optional-access) }; template @@ -620,6 +623,7 @@ DirectStepI::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(this)->maxFlow(sb, cache_->srcToDst); @@ -676,6 +680,7 @@ DirectStepI::fwdImp( << " srcToDst: " << to_string(srcToDst) << " out: " << to_string(out); } return {cache_->in, cache_->out}; + // NOLINTEND(bugprone-unchecked-optional-access) } template @@ -706,6 +711,7 @@ DirectStepI::validFwd(PaymentSandbox& sb, ApplyView& afView, EitherAmo return {false, EitherAmount(IOUAmount(beast::zero))}; } + // 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::validFwd(PaymentSandbox& sb, ApplyView& afView, EitherAmo return {false, EitherAmount(cache_->out)}; } return {true, EitherAmount(cache_->out)}; + // NOLINTEND(bugprone-unchecked-optional-access) } // Returns srcQOut, dstQIn diff --git a/src/libxrpl/tx/paths/Flow.cpp b/src/libxrpl/tx/paths/Flow.cpp index 413c160c7f..32ca39ec22 100644 --- a/src/libxrpl/tx/paths/Flow.cpp +++ b/src/libxrpl/tx/paths/Flow.cpp @@ -27,7 +27,7 @@ finishFlow(PaymentSandbox& sb, Asset const& srcAsset, Asset const& dstAsset, Flo path::RippleCalc::Output result; if (isTesSuccess(f.ter)) { - f.sandbox->apply(sb); + f.sandbox->apply(sb); // NOLINT(bugprone-unchecked-optional-access) sandbox set on success } else { diff --git a/src/libxrpl/tx/paths/MPTEndpointStep.cpp b/src/libxrpl/tx/paths/MPTEndpointStep.cpp index 7b063a39d3..aa989b27e0 100644 --- a/src/libxrpl/tx/paths/MPTEndpointStep.cpp +++ b/src/libxrpl/tx/paths/MPTEndpointStep.cpp @@ -556,6 +556,8 @@ MPTEndpointStep::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::setCacheLimiting( if (fwdOut < cache_->out) cache_->out = fwdOut; cache_->srcDebtDir = srcDebtDir; + // NOLINTEND(bugprone-unchecked-optional-access) }; template @@ -596,6 +599,7 @@ MPTEndpointStep::fwdImp( MPTAmount const& in) { XRPL_ASSERT(cache_, "MPTEndpointStep::fwdImp : valid cache"); + // NOLINTBEGIN(bugprone-unchecked-optional-access) assert above auto const [maxSrcToDst, srcDebtDir] = static_cast(this)->maxPaymentFlow(sb); @@ -669,6 +673,7 @@ MPTEndpointStep::fwdImp( << " srcToDst: " << to_string(srcToDst) << " out: " << to_string(out); } return {cache_->in, cache_->out}; + // NOLINTEND(bugprone-unchecked-optional-access) } template @@ -698,6 +703,7 @@ MPTEndpointStep::validFwd(PaymentSandbox& sb, ApplyView& afView, Eithe return {false, EitherAmount(MPTAmount(beast::zero))}; } + // 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::validFwd(PaymentSandbox& sb, ApplyView& afView, Eithe return {false, EitherAmount(cache_->out)}; } return {true, EitherAmount(cache_->out)}; + // NOLINTEND(bugprone-unchecked-optional-access) } // Returns srcQOut, dstQIn diff --git a/src/libxrpl/tx/paths/OfferStream.cpp b/src/libxrpl/tx/paths/OfferStream.cpp index c7e81ba203..0d003c63f7 100644 --- a/src/libxrpl/tx/paths/OfferStream.cpp +++ b/src/libxrpl/tx/paths/OfferStream.cpp @@ -150,6 +150,9 @@ TOfferStreamBase::shouldRmSmallIncreasedQOffer() const return false; } + if (!ownerFunds_) + return false; + TAmounts const ofrAmts{ toAmount(offer_.amount().in), toAmount(offer_.amount().out)}; diff --git a/src/libxrpl/tx/transactors/dex/AMMBid.cpp b/src/libxrpl/tx/transactors/dex/AMMBid.cpp index e878585d4c..69fd6a651b 100644 --- a/src/libxrpl/tx/transactors/dex/AMMBid.cpp +++ b/src/libxrpl/tx/transactors/dex/AMMBid.cpp @@ -321,6 +321,7 @@ applyBid(ApplyContext& ctx_, Sandbox& sb, AccountID const& account_, beast::Jour // Price the slot was purchased at. STAmount const pricePurchased = auctionSlot[sfPrice]; XRPL_ASSERT(timeSlot, "xrpl::applyBid : timeSlot is set"); + // NOLINTBEGIN(bugprone-unchecked-optional-access) auto const fractionUsed = (Number(*timeSlot) + 1) / AUCTION_SLOT_TIME_INTERVALS; auto const fractionRemaining = Number(1) - fractionUsed; auto const computedPrice = [&]() -> Number { @@ -331,6 +332,7 @@ applyBid(ApplyContext& ctx_, Sandbox& sb, AccountID const& account_, beast::Jour // Other intervals slot price return pricePurchased * p1_05 * (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 1287c8c428..e411a3a53a 100644 --- a/src/libxrpl/tx/transactors/dex/AMMVote.cpp +++ b/src/libxrpl/tx/transactors/dex/AMMVote.cpp @@ -176,6 +176,8 @@ applyVote(ApplyContext& ctx_, Sandbox& sb, AccountID const& account_, beast::Jou // Add the entry if the account has more tokens than // the least token holder or same tokens and higher fee. } + // NOLINTBEGIN(bugprone-unchecked-optional-access) slots full means loop ran, minTokens is + // set else if (lpTokensNew > *minTokens || (lpTokensNew == *minTokens && feeNew > minFee)) { auto const entry = updatedVoteSlots.begin() + minPos; @@ -184,6 +186,7 @@ applyVote(ApplyContext& ctx_, Sandbox& sb, AccountID const& account_, beast::Jou 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/payment/DepositPreauth.cpp b/src/libxrpl/tx/transactors/payment/DepositPreauth.cpp index f8dca6bb58..f770b443f7 100644 --- a/src/libxrpl/tx/transactors/payment/DepositPreauth.cpp +++ b/src/libxrpl/tx/transactors/payment/DepositPreauth.cpp @@ -63,6 +63,7 @@ DepositPreauth::preflight(PreflightContext const& ctx) if (authPresent != 0) { // Make sure that the passed account is valid. + // NOLINTNEXTLINE(bugprone-unchecked-optional-access) authPresent != 0 guarantees one is set AccountID const& target(optAuth ? *optAuth : *optUnauth); if (!target) { diff --git a/src/test/app/AMMMPT_test.cpp b/src/test/app/AMMMPT_test.cpp index 9884645836..f453d6cf75 100644 --- a/src/test/app/AMMMPT_test.cpp +++ b/src/test/app/AMMMPT_test.cpp @@ -6354,8 +6354,11 @@ private: // CLOB and AMM, AMM is not selected if (i == 2) { + // NOLINTBEGIN(bugprone-unchecked-optional-access) i==2 implies amm is + // emplaced (i>0) BEAST_EXPECT(amm->expectBalances( USD(1'000'000'000), ETH(1'000'000'000), amm->tokens())); + // NOLINTEND(bugprone-unchecked-optional-access) } env.require(balance(bob, USD(2'100'000'000))); q[i] = Quality( @@ -6393,8 +6396,10 @@ private: // AMM is not selected if (i > 0) { + // NOLINTBEGIN(bugprone-unchecked-optional-access) emplaced when i > 0 BEAST_EXPECT( amm->expectBalances(USD(1'000'000'000), ETH(1'000'000'000), amm->tokens())); + // NOLINTEND(bugprone-unchecked-optional-access) } if (i == 0 || i == 2) { @@ -6438,8 +6443,10 @@ private: // AMM and CLOB are selected if (i > 0) { + // NOLINTBEGIN(bugprone-unchecked-optional-access) emplaced when i > 0 BEAST_EXPECT(!amm->expectBalances( USD(1'000'000'000), ETH(1'000'000'000), amm->tokens())); + // NOLINTEND(bugprone-unchecked-optional-access) } if (i == 2) @@ -6500,8 +6507,10 @@ private: // AMM is selected in both cases if (i > 0) { + // NOLINTBEGIN(bugprone-unchecked-optional-access) emplaced when i > 0 BEAST_EXPECT(!amm->expectBalances( USD(1'000'000'000), ETH(1'000'000'000), amm->tokens())); + // NOLINTEND(bugprone-unchecked-optional-access) } // Partially crosses, AMM is selected, CLOB fails // limitQuality @@ -6584,6 +6593,8 @@ private: if (i == 2) { + // NOLINTBEGIN(bugprone-unchecked-optional-access) i==2 implies amm is + // emplaced (i>0) if (rates.first == lowRate) { // Liquidity is consumed from AMM strand only @@ -6607,6 +6618,7 @@ private: USD(281'976'305), }}})); } + // NOLINTEND(bugprone-unchecked-optional-access) } q[i] = Quality( Amounts{ diff --git a/src/test/app/AccountTxPaging_test.cpp b/src/test/app/AccountTxPaging_test.cpp index d5c299b782..b5b419f9b1 100644 --- a/src/test/app/AccountTxPaging_test.cpp +++ b/src/test/app/AccountTxPaging_test.cpp @@ -10,8 +10,6 @@ #include #include -#include - namespace xrpl { class AccountTxPaging_test : public beast::unit_test::suite diff --git a/src/test/app/GRPCServerTLS_test.cpp b/src/test/app/GRPCServerTLS_test.cpp index a421f981f7..a3411682a2 100644 --- a/src/test/app/GRPCServerTLS_test.cpp +++ b/src/test/app/GRPCServerTLS_test.cpp @@ -370,10 +370,12 @@ public: // Verify the server actually started by checking the port auto const grpcPort = env.app().config()[SECTION_PORT_GRPC].get("port"); BEAST_EXPECT(grpcPort.has_value()); + // NOLINTBEGIN(bugprone-unchecked-optional-access) grpcPort.has_value() checked above BEAST_EXPECT(*grpcPort > 0); // Test 1: Plaintext client should connect successfully std::string const serverAddress = "localhost:" + std::to_string(*grpcPort); + // NOLINTEND(bugprone-unchecked-optional-access) auto plaintextStub = org::xrpl::rpc::v1::XRPLedgerAPIService::NewStub( grpc::CreateChannel(serverAddress, grpc::InsecureChannelCredentials())); BEAST_EXPECT(makeTestGRPCCall(plaintextStub)); @@ -394,9 +396,11 @@ public: // Verify the server actually started by checking the port auto const grpcPort = env.app().config()[SECTION_PORT_GRPC].get("port"); BEAST_EXPECT(grpcPort.has_value()); + // NOLINTBEGIN(bugprone-unchecked-optional-access) grpcPort.has_value() checked above BEAST_EXPECT(*grpcPort > 0); std::string const serverAddress = "localhost:" + std::to_string(*grpcPort); + // NOLINTEND(bugprone-unchecked-optional-access) // Test 1: Plaintext client should FAIL against TLS server auto plaintextStub = org::xrpl::rpc::v1::XRPLedgerAPIService::NewStub( @@ -429,9 +433,11 @@ public: // Verify the server actually started by checking the port auto const grpcPort = env.app().config()[SECTION_PORT_GRPC].get("port"); BEAST_EXPECT(grpcPort.has_value()); + // NOLINTBEGIN(bugprone-unchecked-optional-access) grpcPort.has_value() checked above BEAST_EXPECT(*grpcPort > 0); auto const serverAddress = "localhost:" + std::to_string(*grpcPort); + // NOLINTEND(bugprone-unchecked-optional-access) // Test 1: TLS client WITHOUT client certificate should FAIL (mTLS requires client cert) grpc::SslCredentialsOptions sslOptsNoClient; @@ -651,9 +657,11 @@ public: // Verify the server actually started by checking the port auto const grpcPort = env.app().config()[SECTION_PORT_GRPC].get("port"); BEAST_EXPECT(grpcPort.has_value()); + // NOLINTBEGIN(bugprone-unchecked-optional-access) grpcPort.has_value() checked above BEAST_EXPECT(*grpcPort > 0); auto const serverAddress = "localhost:" + std::to_string(*grpcPort); + // NOLINTEND(bugprone-unchecked-optional-access) // Test: TLS client should be able to connect (no client cert required) grpc::SslCredentialsOptions sslOpts; @@ -686,7 +694,7 @@ public: // Server should fail to start - verify port is 0 auto const grpcPort = env.app().config()[SECTION_PORT_GRPC].get("port"); BEAST_EXPECT(grpcPort.has_value()); - BEAST_EXPECT(*grpcPort == 0); // Server should not have started + BEAST_EXPECT(*grpcPort == 0); // NOLINT(bugprone-unchecked-optional-access) } void @@ -707,7 +715,7 @@ public: // Server should fail to start - verify port is 0 auto const grpcPort = env.app().config()[SECTION_PORT_GRPC].get("port"); BEAST_EXPECT(grpcPort.has_value()); - BEAST_EXPECT(*grpcPort == 0); // Server should not have started + BEAST_EXPECT(*grpcPort == 0); // NOLINT(bugprone-unchecked-optional-access) } void @@ -729,7 +737,7 @@ public: // Server should fail to start - verify port is 0 auto const grpcPort = env.app().config()[SECTION_PORT_GRPC].get("port"); BEAST_EXPECT(grpcPort.has_value()); - BEAST_EXPECT(*grpcPort == 0); // Server should not have started + BEAST_EXPECT(*grpcPort == 0); // NOLINT(bugprone-unchecked-optional-access) } void @@ -751,7 +759,7 @@ public: // Server should fail to start - verify port is 0 auto const grpcPort = env.app().config()[SECTION_PORT_GRPC].get("port"); BEAST_EXPECT(grpcPort.has_value()); - BEAST_EXPECT(*grpcPort == 0); // Server should not have started + BEAST_EXPECT(*grpcPort == 0); // NOLINT(bugprone-unchecked-optional-access) } void @@ -778,7 +786,7 @@ public: // Server should fail to start due to empty CA file auto const grpcPort = env.app().config()[SECTION_PORT_GRPC].get("port"); BEAST_EXPECT(grpcPort.has_value()); - BEAST_EXPECT(*grpcPort == 0); // Server should not have started + BEAST_EXPECT(*grpcPort == 0); // NOLINT(bugprone-unchecked-optional-access) } void @@ -803,9 +811,11 @@ public: // Verify the server started successfully auto const grpcPort = env.app().config()[SECTION_PORT_GRPC].get("port"); BEAST_EXPECT(grpcPort.has_value()); + // NOLINTBEGIN(bugprone-unchecked-optional-access) grpcPort.has_value() checked above BEAST_EXPECT(*grpcPort > 0); auto const serverAddress = "localhost:" + std::to_string(*grpcPort); + // NOLINTEND(bugprone-unchecked-optional-access) // Test 1: TLS client WITHOUT client certificate should FAIL (mTLS requires client cert) grpc::SslCredentialsOptions sslOptsNoClient; diff --git a/src/test/core/Config_test.cpp b/src/test/core/Config_test.cpp index a3137b8e69..569a31df60 100644 --- a/src/test/core/Config_test.cpp +++ b/src/test/core/Config_test.cpp @@ -7,6 +7,7 @@ #include #include #include +#include // IWYU pragma: keep #include #include diff --git a/src/test/jtx/TrustedPublisherServer.h b/src/test/jtx/TrustedPublisherServer.h index 097dae97bb..50045044d7 100644 --- a/src/test/jtx/TrustedPublisherServer.h +++ b/src/test/jtx/TrustedPublisherServer.h @@ -106,8 +106,11 @@ public: st[sfPublicKey] = pk; st[sfSigningPubKey] = spk; + // NOLINTBEGIN(bugprone-unchecked-optional-access) publicKeyType returns value for valid + // keys sign(st, HashPrefix::manifest, *publicKeyType(spk), ssk); sign(st, HashPrefix::manifest, *publicKeyType(pk), sk, sfMasterSignature); + // NOLINTEND(bugprone-unchecked-optional-access) Serializer s; st.add(s); @@ -509,7 +512,9 @@ private: { if (ssl) { - http::read(*ssl_stream, sb, req, ec); + http::read( + *ssl_stream, sb, req, ec); // NOLINT(bugprone-unchecked-optional-access) + // ssl_stream emplaced when ssl==true } else { @@ -658,7 +663,8 @@ private: if (ssl) { - write(*ssl_stream, res, ec); + write(*ssl_stream, res, ec); // NOLINT(bugprone-unchecked-optional-access) + // ssl_stream emplaced when ssl==true } else { @@ -671,7 +677,8 @@ private: // Perform the SSL shutdown if (ssl) - ssl_stream->shutdown(ec); + ssl_stream->shutdown(ec); // NOLINT(bugprone-unchecked-optional-access) ssl_stream + // emplaced when ssl==true } }; diff --git a/src/test/jtx/impl/TestHelpers.cpp b/src/test/jtx/impl/TestHelpers.cpp index 2495e151f3..f585e7b28f 100644 --- a/src/test/jtx/impl/TestHelpers.cpp +++ b/src/test/jtx/impl/TestHelpers.cpp @@ -22,6 +22,7 @@ #include #include #include +#include #include #include #include @@ -302,7 +303,8 @@ find_paths( Json::Value p; p["Paths"] = path[jss::paths_computed]; STParsedJSONObject po("generic", p); - paths = po.object->getFieldPathSet(sfPaths); + if (po.object) + paths = po.object->getFieldPathSet(sfPaths); } } } @@ -321,8 +323,20 @@ find_paths_by_element( std::optional const& srcIssuer, std::optional const& domain) { + // srcElement is optional but is expected to always be present + XRPL_ASSERT( + srcElement.has_value(), "xrpl::test::jtx::find_paths_by_element::srcElement : nullptr"); + return find_paths( - env, src, dst, saDstAmount, saSendMax, srcElement->getPathAsset(), srcIssuer, domain); + env, + src, + dst, + saDstAmount, + saSendMax, + // NOLINTNEXTLINE(bugprone-unchecked-optional-access) + srcElement->getPathAsset(), + srcIssuer, + domain); } /******************************************************************************/ diff --git a/src/test/jtx/mpt.h b/src/test/jtx/mpt.h index 20b8f03762..eb08de5caa 100644 --- a/src/test/jtx/mpt.h +++ b/src/test/jtx/mpt.h @@ -259,7 +259,7 @@ public: { if (!env_.test.BEAST_EXPECT(id_)) Throw("Uninitialized issuanceID"); - return *id_; + return *id_; // NOLINT(bugprone-unchecked-optional-access) } std::int64_t diff --git a/src/test/protocol/Hooks_test.cpp b/src/test/protocol/Hooks_test.cpp index c6254d16bb..bd6e0a1859 100644 --- a/src/test/protocol/Hooks_test.cpp +++ b/src/test/protocol/Hooks_test.cpp @@ -142,9 +142,10 @@ class Hooks_test : public beast::unit_test::suite } case STI_ACCOUNT: { - // NOLINTNEXTLINE(bugprone-unchecked-optional-access) + // NOLINTBEGIN(bugprone-unchecked-optional-access) AccountID const id = *parseBase58("rwfSjJNK2YQuN64bSWn7T2eY9FJAyAPYJT"); + // NOLINTEND(bugprone-unchecked-optional-access) dummy.setAccountID(f, id); BEAST_EXPECT(dummy.getAccountID(f) == id); BEAST_EXPECT(dummy.isFieldPresent(f)); diff --git a/src/xrpld/app/ledger/LedgerHistory.cpp b/src/xrpld/app/ledger/LedgerHistory.cpp index 8fd2e075c6..535acbf63e 100644 --- a/src/xrpld/app/ledger/LedgerHistory.cpp +++ b/src/xrpld/app/ledger/LedgerHistory.cpp @@ -485,7 +485,8 @@ LedgerHistory::validatedLedger( hash, entry->builtConsensusHash, consensusHash, - entry->consensus.value()); + entry->consensus.value()); // NOLINT(bugprone-unchecked-optional-access) consensus + // always emplaced with built } else { diff --git a/src/xrpld/app/main/Application.cpp b/src/xrpld/app/main/Application.cpp index c83b45f247..5867f77ba0 100644 --- a/src/xrpld/app/main/Application.cpp +++ b/src/xrpld/app/main/Application.cpp @@ -816,27 +816,29 @@ public: OpenLedger& getOpenLedger() override { - return *openLedger_; + return *openLedger_; // NOLINT(bugprone-unchecked-optional-access) emplaced during + // initialization before any caller } OpenLedger const& getOpenLedger() const override { - return *openLedger_; + return *openLedger_; // NOLINT(bugprone-unchecked-optional-access) emplaced during + // initialization before any caller } Overlay& getOverlay() override { XRPL_ASSERT(overlay_, "xrpl::ApplicationImp::overlay : non-null overlay"); - return *overlay_; + return *overlay_; // NOLINT(bugprone-unchecked-optional-access) assert above } TxQ& getTxQ() override { XRPL_ASSERT(txQ_, "xrpl::ApplicationImp::getTxQ : non-null transaction queue"); - return *txQ_; + return *txQ_; // NOLINT(bugprone-unchecked-optional-access) assert above } RelationalDatabase& @@ -845,7 +847,7 @@ public: XRPL_ASSERT( relationalDatabase_, "xrpl::ApplicationImp::getRelationalDatabase : non-null relational database"); - return *relationalDatabase_; + return *relationalDatabase_; // NOLINT(bugprone-unchecked-optional-access) assert above } DatabaseCon& @@ -995,6 +997,7 @@ public: { XRPL_ASSERT( relationalDatabase_, "xrpl::ApplicationImp::doSweep : non-null relational database"); + // NOLINTNEXTLINE(bugprone-unchecked-optional-access) assert above if (!config_->standalone() && !relationalDatabase_->transactionDbHasSpace(*config_)) { signalStop("Out of transaction DB space"); @@ -2081,6 +2084,8 @@ ApplicationImp::loadOldLedger( forceValidity(getHashRouter(), txID, Validity::SigGoodOnly); + // emplaced during initialization before any caller + // NOLINTNEXTLINE(bugprone-unchecked-optional-access) openLedger_->modify([&txID, &s](OpenView& view, beast::Journal j) { view.rawTxInsert(txID, std::move(s), nullptr); return true; diff --git a/src/xrpld/app/misc/TxQ.h b/src/xrpld/app/misc/TxQ.h index 621f3b8c60..0f734f7897 100644 --- a/src/xrpld/app/misc/TxQ.h +++ b/src/xrpld/app/misc/TxQ.h @@ -578,7 +578,8 @@ private: TxConsequences const& consequences() const { - return pfResult->consequences; + return pfResult->consequences; // NOLINT(bugprone-unchecked-optional-access) invariant: + // pfResult is never empty } /// Return a TxDetails based on contained information. @@ -593,7 +594,8 @@ private: seqProxy, txn, retriesRemaining, - pfResult->ter, + pfResult->ter, // NOLINT(bugprone-unchecked-optional-access) invariant: pfResult is + // never empty lastResult}; } }; diff --git a/src/xrpld/app/misc/detail/TxQ.cpp b/src/xrpld/app/misc/detail/TxQ.cpp index eb3b7a54be..dde0988b4a 100644 --- a/src/xrpld/app/misc/detail/TxQ.cpp +++ b/src/xrpld/app/misc/detail/TxQ.cpp @@ -277,7 +277,8 @@ TxQ::FeeMetrics::escalatedSeriesFeeLevel( auto const totalFeeLevel = mulDiv(multiplier, sumNlast.second - sumNcurrent.second, target * target); - return {totalFeeLevel.has_value(), *totalFeeLevel}; + return { + totalFeeLevel.has_value(), *totalFeeLevel}; // NOLINT(bugprone-unchecked-optional-access) } LedgerHash TxQ::MaybeTx::parentHashComp{}; @@ -306,6 +307,7 @@ TxQ::MaybeTx::apply(Application& app, OpenView& view, beast::Journal j) XRPL_ASSERT(pfResult, "xrpl::TxQ::MaybeTx::apply : preflight result is set"); NumberSO const stNumberSO{view.rules().enabled(fixUniversalNumber)}; + // NOLINTBEGIN(bugprone-unchecked-optional-access) assert above if (pfResult->rules != view.rules() || pfResult->flags != flags) { JLOG(j.debug()) << "Queued transaction " << txID @@ -316,6 +318,7 @@ TxQ::MaybeTx::apply(Application& app, OpenView& view, beast::Journal j) } auto pcresult = preclaim(*pfResult, app, view); + // NOLINTEND(bugprone-unchecked-optional-access) return doApply(pcresult, app, view); } @@ -833,6 +836,7 @@ TxQ::apply( << ". Account has other queued transactions."; return {telCAN_NOT_QUEUE_BLOCKS, false}; } + // NOLINTNEXTLINE(bugprone-unchecked-optional-access) acctTxCount == 1 implies txIter is set if (acctTxCount == 1 && (txSeqProx != txIter->first->first)) { // The blocker is not replacing the lone queued transaction. @@ -872,8 +876,10 @@ TxQ::apply( // // We only need to check if txIter->first is a blocker because we // require that a blocker be alone in the account's queue. + // NOLINTBEGIN(bugprone-unchecked-optional-access) acctTxCount == 1 implies txIter is set if (acctTxCount == 1 && txIter->first->second.consequences().isBlocker() && (txIter->first->first != txSeqProx)) + // NOLINTEND(bugprone-unchecked-optional-access) { return {telCAN_NOT_QUEUE_BLOCKED, false}; } @@ -988,6 +994,8 @@ TxQ::apply( // o The current first thing in the queue has a Ticket and // * The tx has a Ticket that precedes it or // * txSeqProx == acctSeqProx. + // NOLINTBEGIN(bugprone-unchecked-optional-access) acctTxCount > 0 in else branch + // implies txIter is set XRPL_ASSERT(prevIter != txIter->end, "xrpl::TxQ::apply : not end"); if (prevIter == txIter->end || txSeqProx < prevIter->first) { @@ -1040,6 +1048,7 @@ TxQ::apply( potentialSpend += pfResult.consequences.potentialSpend(); } } + // NOLINTEND(bugprone-unchecked-optional-access) /* Check if the total fees in flight are greater than the account's current balance, or the diff --git a/src/xrpld/app/misc/detail/ValidatorList.cpp b/src/xrpld/app/misc/detail/ValidatorList.cpp index a799ad4834..8a62a71972 100644 --- a/src/xrpld/app/misc/detail/ValidatorList.cpp +++ b/src/xrpld/app/misc/detail/ValidatorList.cpp @@ -948,8 +948,12 @@ ValidatorList::applyListsAndBroadcast( // in the config file (Note: Keys specified in the local config file are // stored in ValidatorList::localPublisherList data member). if (broadcast && result.status <= PublisherStatus::expired && result.publisherKey && + // NOLINTNEXTLINE(bugprone-unchecked-optional-access) publisherKey checked in condition + // above publisherLists_[*result.publisherKey].maxSequence) { + // NOLINTBEGIN(bugprone-unchecked-optional-access) publisherKey and maxSequence checked in + // condition above auto const& pubCollection = publisherLists_[*result.publisherKey]; broadcastBlobs( @@ -960,6 +964,7 @@ ValidatorList::applyListsAndBroadcast( overlay, hashRouter, j_); + // NOLINTEND(bugprone-unchecked-optional-access) } return result; @@ -1010,6 +1015,7 @@ ValidatorList::applyLists( // inconsistent if (result.publisherKey && publisherLists_.contains(*result.publisherKey)) { + // NOLINTBEGIN(bugprone-unchecked-optional-access) publisherKey checked in condition above auto& pubCollection = publisherLists_[*result.publisherKey]; auto& remaining = pubCollection.remaining; auto const& current = pubCollection.current; @@ -1035,6 +1041,7 @@ ValidatorList::applyLists( pubCollection.fullHash = sha512Half(pubCollection); result.sequence = *pubCollection.maxSequence; + // NOLINTEND(bugprone-unchecked-optional-access) } return result; @@ -1799,9 +1806,11 @@ ValidatorList::calculateQuorum( // Use quorum if specified via command line. if (minimumQuorum_ > 0) { + // NOLINTBEGIN(bugprone-unchecked-optional-access) minimumQuorum_ > 0 implies it has a value JLOG(j_.warn()) << "Using potentially unsafe quorum of " << *minimumQuorum_ << " as specified on the command line"; return *minimumQuorum_; + // NOLINTEND(bugprone-unchecked-optional-access) } if (!publisherLists_.empty()) @@ -2012,7 +2021,8 @@ ValidatorList::updateTrusted( { std::optional const signingKey = validatorManifests_.getSigningKey(k); XRPL_ASSERT(signingKey, "xrpl::ValidatorList::updateTrusted : found signing key"); - trustedSigningKeys_.insert(*signingKey); + trustedSigningKeys_.insert( + *signingKey); // NOLINT(bugprone-unchecked-optional-access) assert above } } diff --git a/src/xrpld/app/misc/detail/ValidatorSite.cpp b/src/xrpld/app/misc/detail/ValidatorSite.cpp index ca96e0fa37..557a4e4f25 100644 --- a/src/xrpld/app/misc/detail/ValidatorSite.cpp +++ b/src/xrpld/app/misc/detail/ValidatorSite.cpp @@ -279,7 +279,8 @@ ValidatorSite::makeRequest( sp = std::make_shared( resource->pUrl.domain, resource->pUrl.path, - std::to_string(*resource->pUrl.port), + std::to_string(*resource->pUrl.port), // NOLINT(bugprone-unchecked-optional-access) + // port defaulted at parse time app_.getIOContext(), j_, app_.config(), @@ -292,7 +293,8 @@ ValidatorSite::makeRequest( sp = std::make_shared( resource->pUrl.domain, resource->pUrl.path, - std::to_string(*resource->pUrl.port), + std::to_string(*resource->pUrl.port), // NOLINT(bugprone-unchecked-optional-access) + // port defaulted at parse time app_.getIOContext(), sites_[siteIdx].lastRequestEndpoint, sites_[siteIdx].lastRequestSuccessful, diff --git a/src/xrpld/consensus/Consensus.h b/src/xrpld/consensus/Consensus.h index 4337e8da01..68ea8fb403 100644 --- a/src/xrpld/consensus/Consensus.h +++ b/src/xrpld/consensus/Consensus.h @@ -914,12 +914,14 @@ Consensus::simulate( JLOG(j_.info()) << "Simulating consensus"; now_ = now; closeLedger({}); + // NOLINTBEGIN(bugprone-unchecked-optional-access) closeLedger sets result_ result_->roundTime.tick(consensusDelay.value_or(100ms)); result_->proposers = prevProposers_ = currPeerPositions_.size(); prevRoundTime_ = result_->roundTime.read(); phase_ = ConsensusPhase::accepted; adaptor_.onForceAccept( *result_, previousLedger_, closeResolution_, rawCloseTimes_, mode_.get(), getJson(true)); + // NOLINTEND(bugprone-unchecked-optional-access) JLOG(j_.info()) << "Simulation complete"; } @@ -1209,8 +1211,13 @@ Consensus::shouldPause(std::unique_ptr const& clog) vars << " consensuslog (working seq: " << previousLedger_.seq() << ", " << "validated seq: " << adaptor_.getValidLedgerIndex() << ", " << "am validator: " << adaptor_.validator() << ", " - << "have validated: " << adaptor_.haveValidated() << ", " - << "roundTime: " << result_->roundTime.read().count() << ", " + << "have validated: " << adaptor_.haveValidated() + << ", " + // NOLINTBEGIN(bugprone-unchecked-optional-access) result_ is always set when shouldPause + // is called (from phaseEstablish after assert) + << "roundTime: " << result_->roundTime.read().count() + << ", " + // NOLINTEND(bugprone-unchecked-optional-access) << "max consensus time: " << parms.ledgerMAX_CONSENSUS.count() << ", " << "validators: " << totalValidators << ", " << "laggards: " << laggards << ", " @@ -1218,7 +1225,9 @@ Consensus::shouldPause(std::unique_ptr const& clog) << "quorum: " << quorum << ")"; if ((ahead == 0u) || (laggards == 0u) || (totalValidators == 0u) || !adaptor_.validator() || - !adaptor_.haveValidated() || result_->roundTime.read() > parms.ledgerMAX_CONSENSUS) + !adaptor_.haveValidated() || + // NOLINTNEXTLINE(bugprone-unchecked-optional-access) result_ set as shouldPause called + result_->roundTime.read() > parms.ledgerMAX_CONSENSUS) { j_.debug() << "not pausing (early)" << vars.str(); CLOG(clog) << "Not pausing (early). "; @@ -1316,6 +1325,7 @@ Consensus::phaseEstablish(std::unique_ptr const& clo CLOG(clog) << "phaseEstablish. "; // can only establish consensus if we already took a stance XRPL_ASSERT(result_, "xrpl::Consensus::phaseEstablish : result is set"); + // NOLINTBEGIN(bugprone-unchecked-optional-access) assert above ++peerUnchangedCounter_; ++establishCounter_; @@ -1371,6 +1381,7 @@ Consensus::phaseEstablish(std::unique_ptr const& clo mode_.get(), getJson(true), adaptor_.validating()); + // NOLINTEND(bugprone-unchecked-optional-access) } template @@ -1435,6 +1446,7 @@ Consensus::updateOurPositions(std::unique_ptr const& { // We must have a position if we are updating it XRPL_ASSERT(result_, "xrpl::Consensus::updateOurPositions : result is set"); + // NOLINTBEGIN(bugprone-unchecked-optional-access) assert above ConsensusParms const& parms = adaptor_.parms(); // Compute a cutoff time @@ -1607,6 +1619,7 @@ Consensus::updateOurPositions(std::unique_ptr const& if (!result_->position.isBowOut() && (mode_.get() == ConsensusMode::proposing)) adaptor_.propose(result_->position); } + // NOLINTEND(bugprone-unchecked-optional-access) } template @@ -1615,6 +1628,7 @@ Consensus::haveConsensus(std::unique_ptr const& clog { // Must have a stance if we are checking for consensus XRPL_ASSERT(result_, "xrpl::Consensus::haveConsensus : has result"); + // NOLINTBEGIN(bugprone-unchecked-optional-access) assert above // CHECKME: should possibly count unacquired TX sets as disagreeing int agree = 0, disagree = 0; @@ -1715,6 +1729,7 @@ Consensus::haveConsensus(std::unique_ptr const& clog } CLOG(clog) << "Consensus has been reached. "; + // NOLINTEND(bugprone-unchecked-optional-access) return true; } @@ -1742,6 +1757,7 @@ Consensus::createDisputes(TxSet_t const& o, std::unique_ptrcompares.emplace(o.id()).second; @@ -1801,6 +1817,7 @@ Consensus::createDisputes(TxSet_t const& o, std::unique_ptr @@ -1809,6 +1826,7 @@ Consensus::updateDisputes(NodeID_t const& node, TxSet_t const& other) { // Cannot updateDisputes without our stance XRPL_ASSERT(result_, "xrpl::Consensus::updateDisputes : result is set"); + // NOLINTBEGIN(bugprone-unchecked-optional-access) assert above // Ensure we have created disputes against this set if we haven't seen // it before @@ -1821,6 +1839,7 @@ Consensus::updateDisputes(NodeID_t const& node, TxSet_t const& other) if (d.setVote(node, other.exists(d.tx().id()))) peerUnchangedCounter_ = 0; } + // NOLINTEND(bugprone-unchecked-optional-access) } template diff --git a/src/xrpld/consensus/LedgerTrie.h b/src/xrpld/consensus/LedgerTrie.h index 38ad0afbf9..99ddb530df 100644 --- a/src/xrpld/consensus/LedgerTrie.h +++ b/src/xrpld/consensus/LedgerTrie.h @@ -470,7 +470,7 @@ public: // Loc truncates to prefix and newNode is its child XRPL_ASSERT(prefix, "xrpl::LedgerTrie::insert : prefix is set"); - loc->span = *prefix; + loc->span = *prefix; // NOLINT(bugprone-unchecked-optional-access) assert above newNode->parent = loc; loc->children.emplace_back(std::move(newNode)); loc->tipSupport = 0; @@ -703,7 +703,11 @@ public: // We did not consume the entire span, so we have found the // preferred ledger if (nextSeq < curr->span.end()) + { + // nextSeq within span guarantees before() is set + // NOLINTNEXTLINE(bugprone-unchecked-optional-access) return curr->span.before(nextSeq)->tip(); + } } // We have reached the end of the current span, so we need to diff --git a/src/xrpld/overlay/detail/OverlayImpl.cpp b/src/xrpld/overlay/detail/OverlayImpl.cpp index 30fa2587e7..f29d97cb11 100644 --- a/src/xrpld/overlay/detail/OverlayImpl.cpp +++ b/src/xrpld/overlay/detail/OverlayImpl.cpp @@ -689,7 +689,7 @@ OverlayImpl::onManifests( mo, "xrpl::OverlayImpl::onManifests : manifest " "deserialization succeeded"); - + // NOLINTBEGIN(bugprone-unchecked-optional-access) assert above app_.getOPs().pubManifest(*mo); if (app_.getValidators().listed(mo->masterKey)) @@ -697,6 +697,7 @@ OverlayImpl::onManifests( auto db = app_.getWalletDB().checkoutDb(); addValidatorManifest(*db, serialized); } + // NOLINTEND(bugprone-unchecked-optional-access) } } else diff --git a/src/xrpld/overlay/detail/PeerImp.cpp b/src/xrpld/overlay/detail/PeerImp.cpp index 5bfc33021d..46a640ec5c 100644 --- a/src/xrpld/overlay/detail/PeerImp.cpp +++ b/src/xrpld/overlay/detail/PeerImp.cpp @@ -2254,6 +2254,7 @@ PeerImp::onValidatorListMessage( applyResult.publisherKey, "xrpl::PeerImp::onValidatorListMessage : publisher key is " "set"); + // NOLINTNEXTLINE(bugprone-unchecked-optional-access) assert above auto const& pubKey = *applyResult.publisherKey; #ifndef NDEBUG if (auto const iter = publisherListSequences_.find(pubKey); @@ -3463,6 +3464,7 @@ PeerImp::processLedgerRequest(std::shared_ptr const& m) try { + // NOLINTNEXTLINE(bugprone-unchecked-optional-access) nodeids checked in onGetLedger if (map->getNodeFat(*shaMapNodeId, data, fatLeaves, queryDepth)) { JLOG(p_journal_.trace()) diff --git a/src/xrpld/perflog/detail/PerfLogImp.cpp b/src/xrpld/perflog/detail/PerfLogImp.cpp index 57e7be5156..6b80d29cc6 100644 --- a/src/xrpld/perflog/detail/PerfLogImp.cpp +++ b/src/xrpld/perflog/detail/PerfLogImp.cpp @@ -18,7 +18,6 @@ #include #include -#include #include #include #include diff --git a/src/xrpld/rpc/detail/PathRequest.cpp b/src/xrpld/rpc/detail/PathRequest.cpp index b4ac252f10..1719567357 100644 --- a/src/xrpld/rpc/detail/PathRequest.cpp +++ b/src/xrpld/rpc/detail/PathRequest.cpp @@ -507,6 +507,7 @@ PathRequest::getPathFinder( auto i = pathasset_map.find(asset); if (i != pathasset_map.end()) return i->second; + // NOLINTBEGIN(bugprone-unchecked-optional-access) isValid() ensures both are set auto pathfinder = std::make_unique( cache, *raSrcAccount, @@ -517,6 +518,7 @@ PathRequest::getPathFinder( saSendMax, domain, app_); + // NOLINTEND(bugprone-unchecked-optional-access) if (pathfinder->findPaths(level, continueCallback)) { pathfinder->computePathRanks(max_paths_, continueCallback); @@ -542,8 +544,10 @@ PathRequest::findPaths( } if (sourceAssets.empty()) { + // 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( @@ -621,13 +625,15 @@ PathRequest::findPaths( auto sandbox = std::make_unique(&*cache->getLedger(), tapNONE); auto rc = path::RippleCalc::rippleCalculate( *sandbox, - saMaxAmount, // --> Amount to send is unlimited - // to get an estimate. - dst_amount, // --> Amount to deliver. + saMaxAmount, // --> Amount to send is unlimited + // to get an estimate. + dst_amount, // --> Amount to deliver. + // NOLINTBEGIN(bugprone-unchecked-optional-access) isValid() ensures both are set *raDstAccount, // --> Account to deliver to. *raSrcAccount, // --> Account sending from. - ps, // --> Path set. - domain, // --> Domain. + // NOLINTEND(bugprone-unchecked-optional-access) + ps, // --> Path set. + domain, // --> Domain. app_, &rcInput); @@ -640,13 +646,15 @@ PathRequest::findPaths( sandbox = std::make_unique(&*cache->getLedger(), tapNONE); rc = path::RippleCalc::rippleCalculate( *sandbox, - saMaxAmount, // --> Amount to send is unlimited - // to get an estimate. - dst_amount, // --> Amount to deliver. + saMaxAmount, // --> Amount to send is unlimited + // to get an estimate. + dst_amount, // --> Amount to deliver. + // NOLINTBEGIN(bugprone-unchecked-optional-access) isValid() ensures both are set *raDstAccount, // --> Account to deliver to. *raSrcAccount, // --> Account sending from. - ps, // --> Path set. - domain, // --> Domain. + // NOLINTEND(bugprone-unchecked-optional-access) + ps, // --> Path set. + domain, // --> Domain. app_); if (!isTesSuccess(rc.result())) @@ -718,13 +726,16 @@ PathRequest::doUpdate( { // Old ripple_path_find API gives destination_currencies auto& destAssets = (newStatus[jss::destination_currencies] = Json::arrayValue); + // NOLINTNEXTLINE(bugprone-unchecked-optional-access) isValid() ensures both are set auto const assets = accountDestAssets(*raDstAccount, cache, true); for (auto const& asset : assets) destAssets.append(to_string(asset)); } + // NOLINTBEGIN(bugprone-unchecked-optional-access) isValid() ensures both are set newStatus[jss::source_account] = toBase58(*raSrcAccount); newStatus[jss::destination_account] = toBase58(*raDstAccount); + // NOLINTEND(bugprone-unchecked-optional-access) newStatus[jss::destination_amount] = saDstAmount.getJson(JsonOptions::none); newStatus[jss::full_reply] = !fast; diff --git a/src/xrpld/rpc/detail/RPCLedgerHelpers.cpp b/src/xrpld/rpc/detail/RPCLedgerHelpers.cpp index 0934289226..b4c654e245 100644 --- a/src/xrpld/rpc/detail/RPCLedgerHelpers.cpp +++ b/src/xrpld/rpc/detail/RPCLedgerHelpers.cpp @@ -440,6 +440,7 @@ getOrAcquireLedger(RPC::JsonContext const& context) auto refHash = hashOfSeq(*ledger, refIndex, j); XRPL_ASSERT(refHash, "xrpl::RPC::getOrAcquireLedger : nonzero ledger hash"); + // NOLINTBEGIN(bugprone-unchecked-optional-access) assert above ledger = ledgerMaster.getLedgerByHash(*refHash); if (!ledger) { @@ -456,6 +457,7 @@ getOrAcquireLedger(RPC::JsonContext const& context) } if (auto il = context.app.getInboundLedgers().find(*refHash)) + // NOLINTEND(bugprone-unchecked-optional-access) { Json::Value jvResult = RPC::make_error( rpcLGR_NOT_FOUND, "acquiring ledger containing requested index");