diff --git a/.clang-tidy b/.clang-tidy index 26c7995631..d8a7cb1b6f 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -8,12 +8,14 @@ Checks: "-*, bugprone-chained-comparison, bugprone-compare-pointer-to-member-virtual-function, bugprone-copy-constructor-init, + bugprone-crtp-constructor-accessibility, bugprone-dangling-handle, bugprone-dynamic-static-initializers, bugprone-empty-catch, bugprone-fold-init-type, bugprone-forward-declaration-namespace, bugprone-inaccurate-erase, + bugprone-inc-dec-in-conditions, bugprone-incorrect-enable-if, bugprone-incorrect-roundings, bugprone-infinite-loop, @@ -30,9 +32,12 @@ Checks: "-*, bugprone-multiple-statement-macro, bugprone-no-escape, bugprone-non-zero-enum-to-bool-conversion, + bugprone-optional-value-conversion, bugprone-parent-virtual-call, + bugprone-pointer-arithmetic-on-polymorphic-object, bugprone-posix-return, bugprone-redundant-branch-condition, + bugprone-reserved-identifier, bugprone-return-const-ref-from-parameter, bugprone-shared-ptr-array-mismatch, bugprone-signal-handler, @@ -49,19 +54,35 @@ Checks: "-*, bugprone-suspicious-include, bugprone-suspicious-memory-comparison, bugprone-suspicious-memset-usage, + bugprone-suspicious-missing-comma, bugprone-suspicious-realloc-usage, bugprone-suspicious-semicolon, bugprone-suspicious-string-compare, + bugprone-suspicious-stringview-data-usage, bugprone-swapped-arguments, + bugprone-switch-missing-default-case, 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-undefined-memory-manipulation, bugprone-undelegated-constructor, bugprone-unhandled-exception-at-new, + bugprone-unhandled-self-assignment, bugprone-unique-ptr-array-mismatch, bugprone-unsafe-functions, + bugprone-use-after-move, + bugprone-unused-raii, + bugprone-unused-return-value, + bugprone-unused-local-non-trivial-variable, bugprone-virtual-near-miss, + cppcoreguidelines-init-variables, + cppcoreguidelines-misleading-capture-default-by-value, cppcoreguidelines-no-suspend-with-lock, + cppcoreguidelines-pro-type-member-init, + cppcoreguidelines-pro-type-static-cast-downcast, + cppcoreguidelines-rvalue-reference-param-not-moved, + cppcoreguidelines-use-default-member-init, cppcoreguidelines-virtual-class-destructor, hicpp-ignored-remove-result, misc-definitions-in-headers, @@ -71,71 +92,48 @@ Checks: "-*, misc-throw-by-value-catch-by-reference, misc-unused-alias-decls, misc-unused-using-decls, - readability-duplicate-include, - readability-enum-initial-value, - readability-misleading-indentation, - readability-non-const-parameter, - readability-redundant-declaration, - readability-reference-to-constructed-temporary, modernize-deprecated-headers, modernize-make-shared, modernize-make-unique, performance-implicit-conversion-in-loop, performance-move-constructor-init, - performance-trivially-destructible + performance-trivially-destructible, + readability-avoid-nested-conditional-operator, + readability-avoid-return-with-void-value, + readability-braces-around-statements, + readability-const-return-type, + readability-container-contains, + readability-container-size-empty, + readability-duplicate-include, + readability-else-after-return, + readability-enum-initial-value, + readability-make-member-function-const, + readability-misleading-indentation, + readability-non-const-parameter, + readability-redundant-casting, + readability-redundant-declaration, + readability-redundant-inline-specifier, + readability-redundant-member-init, + readability-redundant-string-init, + readability-reference-to-constructed-temporary, + readability-static-definition-in-anonymous-namespace, + readability-use-std-min-max " # --- -# more checks that have some issues that need to be resolved: -# -# bugprone-crtp-constructor-accessibility, -# bugprone-inc-dec-in-conditions, -# bugprone-reserved-identifier, -# bugprone-move-forwarding-reference, -# bugprone-unused-local-non-trivial-variable, -# bugprone-switch-missing-default-case, -# bugprone-suspicious-stringview-data-usage, -# bugprone-suspicious-missing-comma, -# bugprone-pointer-arithmetic-on-polymorphic-object, -# bugprone-optional-value-conversion, -# bugprone-too-small-loop-variable, -# bugprone-unused-return-value, -# bugprone-use-after-move, -# bugprone-unhandled-self-assignment, -# bugprone-unused-raii, -# -# cppcoreguidelines-misleading-capture-default-by-value, -# cppcoreguidelines-init-variables, -# cppcoreguidelines-pro-type-member-init, -# cppcoreguidelines-pro-type-static-cast-downcast, -# cppcoreguidelines-use-default-member-init, -# cppcoreguidelines-rvalue-reference-param-not-moved, +# checks that have some issues that need to be resolved: # # llvm-namespace-comment, # misc-const-correctness, # misc-include-cleaner, # misc-redundant-expression, # -# readability-avoid-nested-conditional-operator, -# readability-avoid-return-with-void-value, -# readability-braces-around-statements, -# readability-container-contains, -# readability-container-size-empty, # readability-convert-member-functions-to-static, -# readability-const-return-type, -# readability-else-after-return, # readability-implicit-bool-conversion, # readability-inconsistent-declaration-parameter-name, # readability-identifier-naming, -# readability-make-member-function-const, # readability-math-missing-parentheses, -# readability-redundant-inline-specifier, -# readability-redundant-member-init, -# readability-redundant-casting, -# readability-redundant-string-init, # readability-simplify-boolean-expr, -# readability-static-definition-in-anonymous-namespace, # readability-suspicious-call-argument, -# readability-use-std-min-max, # readability-static-accessed-through-instance, # # modernize-concat-nested-namespaces, @@ -159,7 +157,7 @@ Checks: "-*, # --- # CheckOptions: - # readability-braces-around-statements.ShortStatementLines: 2 + readability-braces-around-statements.ShortStatementLines: 2 # readability-identifier-naming.MacroDefinitionCase: UPPER_CASE # readability-identifier-naming.ClassCase: CamelCase # readability-identifier-naming.StructCase: CamelCase @@ -194,7 +192,7 @@ CheckOptions: # readability-identifier-naming.PublicMemberSuffix: "" # readability-identifier-naming.FunctionIgnoredRegexp: ".*tag_invoke.*" bugprone-unsafe-functions.ReportMoreUnsafeFunctions: true -# bugprone-unused-return-value.CheckedReturnTypes: ::std::error_code;::std::error_condition;::std::errc + bugprone-unused-return-value.CheckedReturnTypes: ::std::error_code;::std::error_condition;::std::errc # misc-include-cleaner.IgnoreHeaders: '.*/(detail|impl)/.*;.*(expected|unexpected).*;.*ranges_lower_bound\.h;time.h;stdlib.h;__chrono/.*;fmt/chrono.h;boost/uuid/uuid_hash.hpp' # # HeaderFilterRegex: '^.*/(src|tests)/.*\.(h|hpp)$' diff --git a/.gersemi/definitions.cmake b/.gersemi/definitions.cmake index 13061629a4..a16e330ffa 100644 --- a/.gersemi/definitions.cmake +++ b/.gersemi/definitions.cmake @@ -51,6 +51,9 @@ endfunction() function(add_module parent name) endfunction() +function(setup_protocol_autogen) +endfunction() + function(target_link_modules parent scope) endfunction() diff --git a/.git-blame-ignore-revs b/.git-blame-ignore-revs index cf50d48f95..0cf704b051 100644 --- a/.git-blame-ignore-revs +++ b/.git-blame-ignore-revs @@ -1,16 +1,79 @@ # This feature requires Git >= 2.24 # To use it by default in git blame: # git config blame.ignoreRevsFile .git-blame-ignore-revs -50760c693510894ca368e90369b0cc2dabfd07f3 -e2384885f5f630c8f0ffe4bf21a169b433a16858 -241b9ddde9e11beb7480600fd5ed90e1ef109b21 -760f16f56835663d9286bd29294d074de26a7ba6 -0eebe6a5f4246fced516d52b83ec4e7f47373edd -2189cc950c0cebb89e4e2fa3b2d8817205bf7cef -b9d007813378ad0ff45660dc07285b823c7e9855 -fe9a5365b8a52d4acc42eb27369247e6f238a4f9 -9a93577314e6a8d4b4a8368cc9d2b15a5d8303e8 -552377c76f55b403a1c876df873a23d780fcc81c -97f0747e103f13e26e45b731731059b32f7679ac -b13370ac0d207217354f1fc1c29aef87769fb8a1 + +# This file is sorted in reverse chronological order, with the most recent commits at the top. +# The commits listed here are ignored by git blame, which is useful for formatting-only commits that would otherwise obscure the history of changes to a file. + +# refactor: Enable remaining clang-tidy `cppcoreguidelines` checks (#6538) +72f4cb097f626b08b02fc3efcb4aa11cb2e7adb8 +# refactor: Rename system name from 'ripple' to 'xrpld' (#6347) +ffea3977f0b771fe8e43a8f74e4d393d63a7afd8 +# refactor: Update transaction folder structure (#6483) +5865bd017f777491b4a956f9210be0c4161f5442 +# chore: Use gersemi instead of ancient cmake-format (#6486) +0c74270b055133a57a497b5c9fc5a75f7647b1f4 +# chore: Apply clang-format width 100 (#6387) +2c1fad102353e11293e3edde1c043224e7d3e983 +# chore: Set clang-format width to 100 in config file (#6387) +25cca465538a56cce501477f9e5e2c1c7ea2d84c +# chore: Set cmake-format width to 100 (#6386) +469ce9f291a4480c38d4ee3baca5136b2f053cd0 +# refactor: Modularize app/tx (#6228) +0976b2b68b64972af8e6e7c497900b5bce9fe22f +# chore: Update clang-format to 21.1.8 (#6352) +958d8f375453d80bb1aa4c293b5102c045a3e4b4 +# refactor: Replace include guards by '#pragma once' (#6322) +34ef577604782ca8d6e1c17df8bd7470990a52ff +# chore: Format all cmake files without comments (#6294) +fe9c8d568fcf6ac21483024e01f58962dd5c8260 +# chore: Add cmake-format pre-commit hook (#6279) +a0e09187b9370805d027c611a7e9ff5a0125282a +# chore: Set ColumnLimit to 120 in clang-format (#6288) +5f638f55536def0d88b970d1018a465a238e55f4 +# refactor: Fix typos in comments, configure cspell (#6164) +3c9f5b62525cb1d6ca1153eeb10433db7d7379fd +# refactor: Rename `rippled.cfg` to `xrpld.cfg` (#6098) +3d1b3a49b3601a0a7037fa0b19d5df7b5e0e2fc1 +# refactor: Rename `ripple` namespace to `xrpl` (#5982) +1eb0fdac6543706b4b9ddca57fd4102928a1f871 +# refactor: Rename `rippled` binary to `xrpld` (#5983) +9eb84a561ef8bb066d89f098bd9b4ac71baed67c +# refactor: Replaces secp256k1 source by Conan package (#6089) +813bc4d9491b078bb950f8255f93b02f71320478 +# refactor: Remove unnecessary copyright notices already covered by LICENSE.md (#5929) +1d42c4f6de6bf01d1286fc7459b17a37a5189e88 +# refactor: Rename `RIPPLE_` and `RIPPLED_` definitions to `XRPL_` (#5821) +ada83564d894829424b0f4d922b0e737e07abbf7 +# refactor: Modularize shamap and nodestore (#5668) +8eb233c2ea8ad5a159be73b77f0f5e1496d547ac +# refactor: Modularise ledger (#5493) +dc8b37a52448b005153c13a7f046ad494128cf94 +# chore: Update clang-format and prettier with pre-commit (#5709) +c14ce956adeabe476ad73c18d73103f347c9c613 +# chore: Fix file formatting (#5718) 896b8c3b54a22b0497cb0d1ce95e1095f9a227ce +# chore: Reverts formatting changes to external files, adds formatting changes to proto files (#5711) +b13370ac0d207217354f1fc1c29aef87769fb8a1 +# chore: Run prettier on all files (#5657) +97f0747e103f13e26e45b731731059b32f7679ac +# Reformat code with clang-format-18 +552377c76f55b403a1c876df873a23d780fcc81c +# Recompute loops (#4997) +d028005aa6319338b0adae1aebf8abe113162960 +# Rewrite includes (#4997) +1d23148e6dd53957fcb6205c07a5c6cd7b64d50c +# Rearrange sources (#4997) +e416ee72ca26fa0c09d2aee1b68bdfb2b7046eed +# Move CMake directory (#4997) +2e902dee53aab2a8f27f32971047bb81e022f94f +# Rewrite includes +0eebe6a5f4246fced516d52b83ec4e7f47373edd +# Format formerly .hpp files +760f16f56835663d9286bd29294d074de26a7ba6 +# Rename .hpp to .h +241b9ddde9e11beb7480600fd5ed90e1ef109b21 +# Consolidate external libraries +e2384885f5f630c8f0ffe4bf21a169b433a16858 +# Format first-party source according to .clang-format +50760c693510894ca368e90369b0cc2dabfd07f3 diff --git a/.github/actions/generate-version/action.yml b/.github/actions/generate-version/action.yml index 6b84aac2f3..8edb7920c6 100644 --- a/.github/actions/generate-version/action.yml +++ b/.github/actions/generate-version/action.yml @@ -11,7 +11,7 @@ runs: steps: # When a tag is pushed, the version is used as-is. - name: Generate version for tag event - if: ${{ github.event_name == 'tag' }} + if: ${{ startsWith(github.ref, 'refs/tags/') }} shell: bash env: VERSION: ${{ github.ref_name }} @@ -22,7 +22,7 @@ runs: # We use a plus sign instead of a hyphen because Conan recipe versions do # not support two hyphens. - name: Generate version for non-tag event - if: ${{ github.event_name != 'tag' }} + if: ${{ !startsWith(github.ref, 'refs/tags/') }} shell: bash run: | echo 'Extracting version from BuildInfo.cpp.' diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 3ab3a38807..f1f7aa18f7 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -29,22 +29,6 @@ If a refactor, how is this better than the previous implementation? If there is a spec or design document for this feature, please link it here. --> -### Type of Change - - - -- [ ] Bug fix (non-breaking change which fixes an issue) -- [ ] New feature (non-breaking change which adds functionality) -- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) -- [ ] Refactor (non-breaking change that only restructures code) -- [ ] Performance (increase or change in throughput and/or latency) -- [ ] Tests (you added tests for code that already exists, or your new feature included in this PR) -- [ ] Documentation update -- [ ] Chore (no impact to binary, e.g. `.gitignore`, formatting, dropping support for older tooling) -- [ ] Release - ### API Impact Domain. app_.logs()); - if (rc.result() != tesSUCCESS) + if (!isTesSuccess(rc.result())) { JLOG(m_journal.warn()) << iIdentifier << " Failed with covering path " << transHuman(rc.result()); @@ -652,9 +654,13 @@ PathRequest::doUpdate( { // first pass if (loaded || fast) + { iLevel = app_.config().PATH_SEARCH_FAST; + } else + { iLevel = app_.config().PATH_SEARCH; + } } else if ((iLevel == app_.config().PATH_SEARCH_FAST) && !fast) { diff --git a/src/xrpld/app/paths/PathRequest.h b/src/xrpld/app/paths/PathRequest.h index fde499a312..0ffc6c6e2c 100644 --- a/src/xrpld/app/paths/PathRequest.h +++ b/src/xrpld/app/paths/PathRequest.h @@ -139,7 +139,7 @@ private: std::optional domain; - bool convert_all_; + bool convert_all_{}; std::recursive_mutex mIndexLock; LedgerIndex mLastIndex; diff --git a/src/xrpld/app/paths/PathRequests.cpp b/src/xrpld/app/paths/PathRequests.cpp index 35a6c7aa48..61db1e58ef 100644 --- a/src/xrpld/app/paths/PathRequests.cpp +++ b/src/xrpld/app/paths/PathRequests.cpp @@ -95,7 +95,9 @@ PathRequests::updateAll(std::shared_ptr const& inLedger) return (bool)getSubscriber(request); }; if (!request->needsUpdate(newRequests, cache->getLedger()->seq())) + { remove = false; + } else { if (auto ipSub = getSubscriber(request)) diff --git a/src/xrpld/app/paths/Pathfinder.cpp b/src/xrpld/app/paths/Pathfinder.cpp index 254db35ea5..c777fcb2f7 100644 --- a/src/xrpld/app/paths/Pathfinder.cpp +++ b/src/xrpld/app/paths/Pathfinder.cpp @@ -95,7 +95,7 @@ struct PathCost }; using PathCostList = std::vector; -static PathTable mPathTable; +PathTable mPathTable; std::string pathTypeToString(Pathfinder::PathType const& type) @@ -260,7 +260,7 @@ Pathfinder::findPaths(int searchLevel, std::function const& continue // Now compute the payment type from the types of the source and destination // currencies. - PaymentType paymentType; + PaymentType paymentType = pt_XRP_to_XRP; if (bSrcXrp && bDstXrp) { // XRP -> XRP @@ -348,7 +348,7 @@ Pathfinder::getPathLiquidity( app_.logs(), &rcInput); // If we can't get even the minimum liquidity requested, we're done. - if (rc.result() != tesSUCCESS) + if (!isTesSuccess(rc.result())) return rc.result(); qualityOut = getRate(rc.actualAmountOut, rc.actualAmountIn); @@ -491,10 +491,10 @@ Pathfinder::rankPaths( if (!currentPath.empty()) { STAmount liquidity; - uint64_t uQuality; + uint64_t uQuality = 0; auto const resultCode = getPathLiquidity(currentPath, saMinDstAmount, liquidity, uQuality); - if (resultCode != tesSUCCESS) + if (!isTesSuccess(resultCode)) { JLOG(j_.debug()) << "findPaths: dropping : " << transToken(resultCode) << ": " << currentPath.getJson(JsonOptions::none); @@ -574,17 +574,29 @@ Pathfinder::getBestPaths( bool useExtraPath = false; if (pathsIterator == mPathRanks.end()) + { useExtraPath = true; + } else if (extraPathsIterator == extraPathRanks.end()) + { usePath = true; + } else if (extraPathsIterator->quality < pathsIterator->quality) + { useExtraPath = true; + } else if (extraPathsIterator->quality > pathsIterator->quality) + { usePath = true; + } else if (extraPathsIterator->liquidity > pathsIterator->liquidity) + { useExtraPath = true; + } else if (extraPathsIterator->liquidity < pathsIterator->liquidity) + { usePath = true; + } else { // Risk is high they have identical liquidity @@ -1020,9 +1032,13 @@ Pathfinder::addLink( int count = candidates.size(); // allow more paths from source if ((count > 10) && (uEndAccount != mSrcAccount)) + { count = 10; + } else if (count > 50) + { count = 50; + } auto it = candidates.begin(); while (count-- != 0) @@ -1090,7 +1106,9 @@ Pathfinder::addLink( addUniquePath(mCompletePaths, newPath); } else + { incompletePaths.push_back(newPath); + } } else if (!currentPath.hasSeen( book.out.account, book.out.currency, book.out.account)) @@ -1158,6 +1176,7 @@ makePath(char const* string) while (true) { + // NOLINTNEXTLINE(bugprone-switch-missing-default-case) switch (*string++) { case 's': // source diff --git a/src/xrpld/app/paths/RippleLineCache.cpp b/src/xrpld/app/paths/RippleLineCache.cpp index 9916facdc2..ac3e28e579 100644 --- a/src/xrpld/app/paths/RippleLineCache.cpp +++ b/src/xrpld/app/paths/RippleLineCache.cpp @@ -76,7 +76,7 @@ RippleLineCache::getRippleLines(AccountID const& accountID, LineDirection direct { XRPL_ASSERT(it->second == nullptr, "xrpl::RippleLineCache::getRippleLines : null lines"); auto lines = PathFindTrustLine::getItems(accountID, *ledger_, direction); - if (lines.size()) + if (!lines.empty()) { it->second = std::make_shared>(std::move(lines)); totalLineCount_ += it->second->size(); diff --git a/src/xrpld/app/paths/detail/AMMLiquidity.cpp b/src/xrpld/app/paths/detail/AMMLiquidity.cpp index 3ffc86d1ba..72ee8eb261 100644 --- a/src/xrpld/app/paths/detail/AMMLiquidity.cpp +++ b/src/xrpld/app/paths/detail/AMMLiquidity.cpp @@ -80,11 +80,17 @@ constexpr T maxAmount() { if constexpr (std::is_same_v) + { return XRPAmount(STAmount::cMaxNative); + } else if constexpr (std::is_same_v) + { return IOUAmount(STAmount::cMaxValue / 2, STAmount::cMaxOffset); + } else if constexpr (std::is_same_v) + { return STAmount(STAmount::cMaxValue / 2, STAmount::cMaxOffset); + } } template @@ -108,14 +114,12 @@ AMMLiquidity::maxOffer(TAmounts const& balances, Rules con balances, Quality{balances}); } - else - { - auto const out = maxOut(balances.out, issueOut()); - if (out <= TOut{0} || out >= balances.out) - return std::nullopt; - return AMMOffer( - *this, {swapAssetOut(balances, out, tradingFee_), out}, balances, Quality{balances}); - } + + auto const out = maxOut(balances.out, issueOut()); + if (out <= TOut{0} || out >= balances.out) + return std::nullopt; + return AMMOffer( + *this, {swapAssetOut(balances, out, tradingFee_), out}, balances, Quality{balances}); } template @@ -166,7 +170,7 @@ AMMLiquidity::getOffer(ReadView const& view, std::optional c return std::nullopt; return AMMOffer(*this, amounts, balances, Quality{amounts}); } - else if (!clobQuality) + if (!clobQuality) { // If there is no CLOB to compare against, return the largest // amount, which doesn't overflow. The size is going to be @@ -175,13 +179,12 @@ AMMLiquidity::getOffer(ReadView const& view, std::optional c // nullopt if the pool is small. return maxOffer(balances, view.rules()); } - else if ( - auto const amounts = + if (auto const amounts = changeSpotPriceQuality(balances, *clobQuality, tradingFee_, view.rules(), j_)) { return AMMOffer(*this, *amounts, balances, Quality{*amounts}); } - else if (view.rules().enabled(fixAMMv1_2)) + if (view.rules().enabled(fixAMMv1_2)) { if (auto const maxAMMOffer = maxOffer(balances, view.rules()); maxAMMOffer && Quality{maxAMMOffer->amount()} > *clobQuality) @@ -192,9 +195,11 @@ AMMLiquidity::getOffer(ReadView const& view, std::optional c { JLOG(j_.error()) << "AMMLiquidity::getOffer overflow " << e.what(); if (!view.rules().enabled(fixAMMOverflowOffer)) + { return maxOffer(balances, view.rules()); - else - return std::nullopt; + } + + return std::nullopt; } catch (std::exception const& e) { diff --git a/src/xrpld/app/paths/detail/BookStep.cpp b/src/xrpld/app/paths/detail/BookStep.cpp index dca38f8df5..ae0c371e3e 100644 --- a/src/xrpld/app/paths/detail/BookStep.cpp +++ b/src/xrpld/app/paths/detail/BookStep.cpp @@ -63,7 +63,7 @@ protected: std::optional cache_; -public: +private: BookStep(StrandContext const& ctx, Issue const& in, Issue const& out) : book_(in, out, ctx.domainID) , strandSrc_(ctx.strandSrc) @@ -74,6 +74,7 @@ public: { if (auto const ammSle = ctx.view.read(keylet::amm(in, out)); ammSle && ammSle->getFieldAmount(sfLPTokenBalance) != beast::zero) + { ammLiquidity_.emplace( ctx.view, (*ammSle)[sfAccount], @@ -82,8 +83,10 @@ public: out, ctx.ammContext, ctx.j); + } } +public: Book const& book() const { @@ -223,6 +226,8 @@ private: // whichever is a better quality. std::optional tipOfferQualityF(ReadView const& view) const; + + friend TDerived; }; //------------------------------------------------------------------------------ @@ -240,7 +245,11 @@ class BookPaymentStep : public BookStep> public: explicit BookPaymentStep() = default; - using BookStep>::BookStep; + BookPaymentStep(StrandContext const& ctx, Issue const& in, Issue const& out) + : BookStep>(ctx, in, out) + { + } + using BookStep>::qualityUpperBound; using typename BookStep>::OfferType; @@ -484,11 +493,14 @@ public: // when calculating the upper bound quality and the quality function // because single path AMM's offer quality is not constant. if (!rules.enabled(fixAMMv1_1)) + { return ofrQ; - else if ( - offerType == OfferType::CLOB || + } + if (offerType == OfferType::CLOB || (this->ammLiquidity_ && this->ammLiquidity_->multiPath())) + { return ofrQ; + } auto rate = [&](AccountID const& id) { if (isXRP(id) || id == this->strandDst_) @@ -667,9 +679,13 @@ BookStep::forEachOffer( // Note that offer.quality() returns a (non-optional) Quality. So // ofrQ is always safe to use below this point in the lambda. if (!ofrQ) + { ofrQ = offer.quality(); + } else if (*ofrQ != offer.quality()) + { return false; + } if (static_cast(this)->limitSelfCrossQuality( strandSrc_, strandDst_, offer, ofrQ, offers, offerAttempted)) @@ -697,8 +713,10 @@ BookStep::forEachOffer( if (auto const key = offer.key()) offers.permRmOffer(*key); if (!offerAttempted) + { // Change quality only if no previous offers were tried. ofrQ = std::nullopt; + } // Returning true causes offers.step() to delete the offer. return true; } @@ -804,7 +822,7 @@ BookStep::consumeOffer( { auto const dr = offer.send(sb, book_.in.account, offer.owner(), toSTAmount(ofrAmt.in, book_.in), j_); - if (dr != tesSUCCESS) + if (!isTesSuccess(dr)) Throw(dr); } @@ -813,7 +831,7 @@ BookStep::consumeOffer( { auto const cr = offer.send(sb, offer.owner(), book_.out.account, toSTAmount(ownerGives, book_.out), j_); - if (cr != tesSUCCESS) + if (!isTesSuccess(cr)) Throw(cr); } @@ -874,24 +892,34 @@ auto BookStep::tipOfferQuality(ReadView const& view) const -> std::optional> { - if (auto const res = tip(view); !res) + auto const res = tip(view); + if (!res) + { return std::nullopt; - else if (auto const q = std::get_if(&(*res))) + } + if (auto const q = std::get_if(&(*res))) + { return std::make_pair(*q, OfferType::CLOB); - else - return std::make_pair(std::get>(*res).quality(), OfferType::AMM); + } + + return std::make_pair(std::get>(*res).quality(), OfferType::AMM); } template std::optional BookStep::tipOfferQualityF(ReadView const& view) const { - if (auto const res = tip(view); !res) + auto const res = tip(view); + if (!res) + { return std::nullopt; - else if (auto const q = std::get_if(&(*res))) + } + if (auto const q = std::get_if(&(*res))) + { return QualityFunction{*q, QualityFunction::CLOBLikeTag{}}; - else - return std::get>(*res).getQualityFunc(); + } + + return std::get>(*res).getQualityFunc(); } template @@ -947,33 +975,31 @@ BookStep::revImp( // we need to consume the offer return true; } - else - { - auto ofrAdjAmt = ofrAmt; - auto stpAdjAmt = stpAmt; - auto ownerGivesAdj = ownerGives; - limitStepOut( - offer, - ofrAdjAmt, - stpAdjAmt, - ownerGivesAdj, - transferRateIn, - transferRateOut, - remainingOut); - remainingOut = beast::zero; - savedIns.insert(stpAdjAmt.in); - savedOuts.insert(remainingOut); - result.in = sum(savedIns); - result.out = out; - this->consumeOffer(sb, offer, ofrAdjAmt, stpAdjAmt, ownerGivesAdj); - // Explicitly check whether the offer is funded. Given that we have - // (stpAmt.out > remainingOut), it's natural to assume the offer - // will still be funded after consuming remainingOut but that is - // not always the case. If the mantissas of two IOU amounts differ - // by less than ten, then subtracting them leaves a zero. - return offer.fully_consumed(); - } + auto ofrAdjAmt = ofrAmt; + auto stpAdjAmt = stpAmt; + auto ownerGivesAdj = ownerGives; + limitStepOut( + offer, + ofrAdjAmt, + stpAdjAmt, + ownerGivesAdj, + transferRateIn, + transferRateOut, + remainingOut); + remainingOut = beast::zero; + savedIns.insert(stpAdjAmt.in); + savedOuts.insert(remainingOut); + result.in = sum(savedIns); + result.out = out; + this->consumeOffer(sb, offer, ofrAdjAmt, stpAdjAmt, ownerGivesAdj); + + // Explicitly check whether the offer is funded. Given that we have + // (stpAmt.out > remainingOut), it's natural to assume the offer + // will still be funded after consuming remainingOut but that is + // not always the case. If the mantissas of two IOU amounts differ + // by less than ten, then subtracting them leaves a zero. + return offer.fully_consumed(); }; { @@ -1329,7 +1355,7 @@ make_BookStepHelper(StrandContext const& ctx, Issue const& in, Issue const& out) ter = paymentStep->check(ctx); r = std::move(paymentStep); } - if (ter != tesSUCCESS) + if (!isTesSuccess(ter)) return {ter, nullptr}; return {tesSUCCESS, std::move(r)}; diff --git a/src/xrpld/app/paths/detail/DirectStep.cpp b/src/xrpld/app/paths/detail/DirectStep.cpp index 09ad2fcd67..a07e5824d6 100644 --- a/src/xrpld/app/paths/detail/DirectStep.cpp +++ b/src/xrpld/app/paths/detail/DirectStep.cpp @@ -66,7 +66,7 @@ protected: std::pair qualities(ReadView const& sb, DebtDirection srcDebtDir, StrandDirection strandDir) const; -public: +private: DirectStepI( StrandContext const& ctx, AccountID const& src, @@ -81,6 +81,7 @@ public: { } +public: AccountID const& src() const { @@ -195,6 +196,8 @@ private: } return false; } + + friend TDerived; }; //------------------------------------------------------------------------------ @@ -209,7 +212,15 @@ private: class DirectIPaymentStep : public DirectStepI { public: - using DirectStepI::DirectStepI; + DirectIPaymentStep( + StrandContext const& ctx, + AccountID const& src, + AccountID const& dst, + Currency const& c) + : DirectStepI(ctx, src, dst, c) + { + } + using DirectStepI::check; bool @@ -252,7 +263,15 @@ public: class DirectIOfferCrossingStep : public DirectStepI { public: - using DirectStepI::DirectStepI; + DirectIOfferCrossingStep( + StrandContext const& ctx, + AccountID const& src, + AccountID const& dst, + Currency const& c) + : DirectStepI(ctx, src, dst, c) + { + } + using DirectStepI::check; bool @@ -316,18 +335,20 @@ DirectIPaymentStep::quality(ReadView const& sb, QualityDirection qDir) const { // compute dst quality in if (this->dst_ < this->src_) + { return sfLowQualityIn; - else - return sfHighQualityIn; + } + + return sfHighQualityIn; } - else + + // compute src quality out + if (this->src_ < this->dst_) { - // compute src quality out - if (this->src_ < this->dst_) - return sfLowQualityOut; - else - return sfHighQualityOut; + return sfLowQualityOut; } + + return sfHighQualityOut; }(); if (!sle->isFieldPresent(field)) @@ -737,15 +758,13 @@ DirectStepI::qualities( { return qualitiesSrcRedeems(sb); } - else - { - auto const prevStepDebtDirection = [&] { - if (prevStep_) - return prevStep_->debtDirection(sb, strandDir); - return DebtDirection::issues; - }(); - return qualitiesSrcIssues(sb, prevStepDebtDirection); - } + + auto const prevStepDebtDirection = [&] { + if (prevStep_) + return prevStep_->debtDirection(sb, strandDir); + return DebtDirection::issues; + }(); + return qualitiesSrcIssues(sb, prevStepDebtDirection); } template @@ -803,7 +822,7 @@ DirectStepI::check(StrandContext const& ctx) const if (!(ctx.isLast && ctx.isFirst)) { auto const ter = checkFreeze(ctx.view, src_, dst_, currency_); - if (ter != tesSUCCESS) + if (!isTesSuccess(ter)) return ter; } @@ -814,7 +833,7 @@ DirectStepI::check(StrandContext const& ctx) const if (auto prevSrc = ctx.prevStep->directStepSrcAcct()) { auto const ter = checkNoRipple(ctx.view, *prevSrc, src_, dst_, currency_, j_); - if (ter != tesSUCCESS) + if (!isTesSuccess(ter)) return ter; } } @@ -897,7 +916,7 @@ make_DirectStepI( ter = paymentStep->check(ctx); r = std::move(paymentStep); } - if (ter != tesSUCCESS) + if (!isTesSuccess(ter)) return {ter, nullptr}; return {tesSUCCESS, std::move(r)}; diff --git a/src/xrpld/app/paths/detail/PaySteps.cpp b/src/xrpld/app/paths/detail/PaySteps.cpp index 7bbcb2a72c..9087647aeb 100644 --- a/src/xrpld/app/paths/detail/PaySteps.cpp +++ b/src/xrpld/app/paths/detail/PaySteps.cpp @@ -264,9 +264,13 @@ toStrand( auto const next = &normPath[i + 1]; if (cur->isAccount()) + { curIssue.account = cur->getAccountID(); + } else if (cur->hasIssuer()) + { curIssue.account = cur->getIssuerID(); + } if (cur->hasCurrency()) { @@ -283,7 +287,7 @@ toStrand( JLOG(j.trace()) << "Inserting implied account"; auto msr = make_DirectStepI( ctx(), cur->getAccountID(), curIssue.account, curIssue.currency); - if (msr.first != tesSUCCESS) + if (!isTesSuccess(msr.first)) return {msr.first, Strand{}}; result.push_back(std::move(msr.second)); impliedPE.emplace( @@ -298,7 +302,7 @@ toStrand( JLOG(j.trace()) << "Inserting implied account before offer"; auto msr = make_DirectStepI( ctx(), cur->getAccountID(), curIssue.account, curIssue.currency); - if (msr.first != tesSUCCESS) + if (!isTesSuccess(msr.first)) return {msr.first, Strand{}}; result.push_back(std::move(msr.second)); impliedPE.emplace( @@ -314,21 +318,19 @@ toStrand( { if (i != normPath.size() - 2) return {temBAD_PATH, Strand{}}; - else - { - // Last step. insert xrp endpoint step - auto msr = make_XRPEndpointStep(ctx(), next->getAccountID()); - if (msr.first != tesSUCCESS) - return {msr.first, Strand{}}; - result.push_back(std::move(msr.second)); - } + + // Last step. insert xrp endpoint step + auto msr = make_XRPEndpointStep(ctx(), next->getAccountID()); + if (!isTesSuccess(msr.first)) + return {msr.first, Strand{}}; + result.push_back(std::move(msr.second)); } else { JLOG(j.trace()) << "Inserting implied account after offer"; auto msr = make_DirectStepI( ctx(), curIssue.account, next->getAccountID(), curIssue.currency); - if (msr.first != tesSUCCESS) + if (!isTesSuccess(msr.first)) return {msr.first, Strand{}}; result.push_back(std::move(msr.second)); } @@ -346,8 +348,10 @@ toStrand( } auto s = toStep(ctx(/*isLast*/ i == normPath.size() - 2), cur, next, curIssue); - if (s.first == tesSUCCESS) + if (isTesSuccess(s.first)) + { result.emplace_back(std::move(s.second)); + } else { JLOG(j.debug()) << "toStep failed: " << s.first; @@ -457,7 +461,7 @@ toStrands( auto const ter = sp.first; auto& strand = sp.second; - if (ter != tesSUCCESS) + if (!isTesSuccess(ter)) { JLOG(j.trace()) << "failed to add default path"; if (isTemMalformed(ter) || paths.empty()) @@ -501,7 +505,7 @@ toStrands( auto ter = sp.first; auto& strand = sp.second; - if (ter != tesSUCCESS) + if (!isTesSuccess(ter)) { lastFailTer = ter; JLOG(j.trace()) << "failed to add path: ter: " << ter diff --git a/src/xrpld/app/paths/detail/XRPEndpointStep.cpp b/src/xrpld/app/paths/detail/XRPEndpointStep.cpp index ceb166d6ec..6eaa0d04c4 100644 --- a/src/xrpld/app/paths/detail/XRPEndpointStep.cpp +++ b/src/xrpld/app/paths/detail/XRPEndpointStep.cpp @@ -37,12 +37,12 @@ private: return EitherAmount(*cache_); } -public: XRPEndpointStep(StrandContext const& ctx, AccountID const& acc) : acc_(acc), isLast_(ctx.isLast), j_(ctx.j) { } +public: AccountID const& acc() const { @@ -135,6 +135,8 @@ private: } return false; } + + friend TDerived; }; //------------------------------------------------------------------------------ @@ -149,7 +151,10 @@ private: class XRPEndpointPaymentStep : public XRPEndpointStep { public: - using XRPEndpointStep::XRPEndpointStep; + XRPEndpointPaymentStep(StrandContext const& ctx, AccountID const& acc) + : XRPEndpointStep(ctx, acc) + { + } XRPAmount xrpLiquid(ReadView& sb) const @@ -238,7 +243,7 @@ XRPEndpointStep::revImp( auto& sender = isLast_ ? xrpAccount() : acc_; auto& receiver = isLast_ ? acc_ : xrpAccount(); auto ter = accountSend(sb, sender, receiver, toSTAmount(result), j_); - if (ter != tesSUCCESS) + if (!isTesSuccess(ter)) return {XRPAmount{beast::zero}, XRPAmount{beast::zero}}; cache_.emplace(result); @@ -261,7 +266,7 @@ XRPEndpointStep::fwdImp( auto& sender = isLast_ ? xrpAccount() : acc_; auto& receiver = isLast_ ? acc_ : xrpAccount(); auto ter = accountSend(sb, sender, receiver, toSTAmount(result), j_); - if (ter != tesSUCCESS) + if (!isTesSuccess(ter)) return {XRPAmount{beast::zero}, XRPAmount{beast::zero}}; cache_.emplace(result); @@ -327,7 +332,7 @@ XRPEndpointStep::check(StrandContext const& ctx) const auto& src = isLast_ ? xrpAccount() : acc_; auto& dst = isLast_ ? acc_ : xrpAccount(); auto ter = checkFreeze(ctx.view, src, dst, xrpCurrency()); - if (ter != tesSUCCESS) + if (!isTesSuccess(ter)) return ter; auto const issuesIndex = isLast_ ? 0 : 1; @@ -375,7 +380,7 @@ make_XRPEndpointStep(StrandContext const& ctx, AccountID const& acc) ter = paymentStep->check(ctx); r = std::move(paymentStep); } - if (ter != tesSUCCESS) + if (!isTesSuccess(ter)) return {ter, nullptr}; return {tesSUCCESS, std::move(r)}; diff --git a/src/xrpld/app/rdb/backend/detail/Node.cpp b/src/xrpld/app/rdb/backend/detail/Node.cpp index 9425cd9159..09b4ebc241 100644 --- a/src/xrpld/app/rdb/backend/detail/Node.cpp +++ b/src/xrpld/app/rdb/backend/detail/Node.cpp @@ -74,8 +74,8 @@ makeLedgerDBs( { // Check if AccountTransactions has primary key std::string cid, name, type; - std::size_t notnull, dflt_value, pk; - soci::indicator ind; + std::size_t notnull = 0, dflt_value = 0, pk = 0; + soci::indicator ind = soci::i_null; soci::statement st = (tx->getSession().prepare << ("PRAGMA table_info(AccountTransactions);"), soci::into(cid), @@ -97,8 +97,8 @@ makeLedgerDBs( return {std::move(lgr), std::move(tx), true}; } - else - return {std::move(lgr), {}, true}; + + return {std::move(lgr), {}, true}; } std::optional @@ -136,7 +136,7 @@ deleteBeforeLedgerSeq(soci::session& session, TableType type, LedgerIndex ledger std::size_t getRows(soci::session& session, TableType type) { - std::size_t rows; + std::size_t rows = 0; session << "SELECT COUNT(*) AS rows " "FROM " << to_string(type) << ";", @@ -148,7 +148,7 @@ getRows(soci::session& session, TableType type) RelationalDatabase::CountMinMax getRowsMinMax(soci::session& session, TableType type) { - RelationalDatabase::CountMinMax res; + RelationalDatabase::CountMinMax res{}; session << "SELECT COUNT(*) AS rows, " "MIN(LedgerSeq) AS first, " "MAX(LedgerSeq) AS last " @@ -282,7 +282,9 @@ saveValidatedLedger( for (auto const& account : accts) { if (!first) + { sql += ", ('"; + } else { sql += "('"; @@ -543,7 +545,7 @@ getHashesByIndex(soci::session& session, LedgerIndex minSeq, LedgerIndex maxSeq, sql.append(std::to_string(maxSeq)); sql.append(";"); - std::uint64_t ls; + std::uint64_t ls = 0; std::string lh; // SOCI requires boost::optional (not std::optional) as the parameter. boost::optional ph; @@ -587,7 +589,7 @@ getTxHistory(soci::session& session, Application& app, LedgerIndex startIndex, i boost::optional ledgerSeq; boost::optional status; soci::blob sociRawTxnBlob(session); - soci::indicator rti; + soci::indicator rti = soci::i_null; Blob rawTxn; soci::statement st = @@ -600,9 +602,13 @@ getTxHistory(soci::session& session, Application& app, LedgerIndex startIndex, i while (st.fetch()) { if (soci::i_ok == rti) + { convert(sociRawTxnBlob, rawTxn); + } else + { rawTxn.clear(); + } if (auto trans = Transaction::transactionFromSQL(ledgerSeq, status, rawTxn, app)) { @@ -645,7 +651,7 @@ transactionsSQL( constexpr std::uint32_t NONBINARY_PAGE_LENGTH = 200; constexpr std::uint32_t BINARY_PAGE_LENGTH = 500; - std::uint32_t numberOfResults; + std::uint32_t numberOfResults = 0; if (count) { @@ -665,8 +671,8 @@ transactionsSQL( numberOfResults = options.limit; } - std::string maxClause = ""; - std::string minClause = ""; + std::string maxClause; + std::string minClause; if (options.maxLedger) { @@ -683,13 +689,16 @@ transactionsSQL( std::string sql; if (count) + { sql = boost::str( boost::format( "SELECT %s FROM AccountTransactions " "WHERE Account = '%s' %s %s LIMIT %u, %u;") % selection % toBase58(options.account) % maxClause % minClause % options.offset % numberOfResults); + } else + { sql = boost::str( boost::format( "SELECT %s FROM " @@ -702,6 +711,7 @@ transactionsSQL( selection % toBase58(options.account) % maxClause % minClause % (descending ? "DESC" : "ASC") % (descending ? "DESC" : "ASC") % (descending ? "DESC" : "ASC") % options.offset % numberOfResults); + } JLOG(j.trace()) << "txSQL query: " << sql; return sql; } @@ -746,7 +756,7 @@ getAccountTxs( false, false, j); - if (sql == "") + if (sql.empty()) return {ret, 0}; int total = 0; @@ -755,7 +765,7 @@ getAccountTxs( boost::optional ledgerSeq; boost::optional status; soci::blob sociTxnBlob(session), sociTxnMetaBlob(session); - soci::indicator rti, tmi; + soci::indicator rti = soci::i_null, tmi = soci::i_null; Blob rawTxn, txnMeta; soci::statement st = @@ -769,14 +779,22 @@ getAccountTxs( while (st.fetch()) { if (soci::i_ok == rti) + { convert(sociTxnBlob, rawTxn); + } else + { rawTxn.clear(); + } if (soci::i_ok == tmi) + { convert(sociTxnMetaBlob, txnMeta); + } else + { txnMeta.clear(); + } auto txn = Transaction::transactionFromSQL(ledgerSeq, status, rawTxn, app); @@ -862,7 +880,7 @@ getAccountTxsB( true /*binary*/, false, j); - if (sql == "") + if (sql.empty()) return {ret, 0}; int total = 0; @@ -872,7 +890,7 @@ getAccountTxsB( boost::optional ledgerSeq; boost::optional status; soci::blob sociTxnBlob(session), sociTxnMetaBlob(session); - soci::indicator rti, tmi; + soci::indicator rti = soci::i_null, tmi = soci::i_null; soci::statement st = (session.prepare << sql, @@ -953,13 +971,17 @@ accountTxPage( bool lookingForMarker = options.marker.has_value(); - std::uint32_t numberOfResults; + std::uint32_t numberOfResults = 0; if (options.limit == 0 || options.limit == UINT32_MAX || (options.limit > page_length && !options.bAdmin)) + { numberOfResults = page_length; + } else + { numberOfResults = options.limit; + } // As an account can have many thousands of transactions, there is a limit // placed on the amount of transactions returned. If the limit is reached @@ -1041,7 +1063,7 @@ accountTxPage( boost::optional status; soci::blob txnData(session); soci::blob txnMeta(session); - soci::indicator dataPresent, metaPresent; + soci::indicator dataPresent = soci::i_null, metaPresent = soci::i_null; soci::statement st = (session.prepare << sql, @@ -1062,7 +1084,9 @@ accountTxPage( lookingForMarker = false; } else + { continue; + } } else if (numberOfResults == 0) { @@ -1072,17 +1096,25 @@ accountTxPage( } if (dataPresent == soci::i_ok) + { convert(txnData, rawData); + } else + { rawData.clear(); + } if (metaPresent == soci::i_ok) + { convert(txnMeta, rawMeta); + } else + { rawMeta.clear(); + } // Work around a bug that could leave the metadata missing - if (rawMeta.size() == 0) + if (rawMeta.empty()) onUnsavedLedger(ledgerSeq.value_or(0)); // `rawData` and `rawMeta` will be used after they are moved. @@ -1151,7 +1183,7 @@ getTransaction( Blob rawTxn, rawMeta; { soci::blob sociRawTxnBlob(session), sociRawMetaBlob(session); - soci::indicator txn, meta; + soci::indicator txn = soci::i_null, meta = soci::i_null; session << sql, soci::into(ledgerSeq), soci::into(status), soci::into(sociRawTxnBlob, txn), soci::into(sociRawMetaBlob, meta); @@ -1164,7 +1196,7 @@ getTransaction( if (!got_data) { uint64_t count = 0; - soci::indicator rti; + soci::indicator rti = soci::i_null; session << "SELECT COUNT(DISTINCT LedgerSeq) FROM Transactions WHERE " "LedgerSeq BETWEEN " @@ -1230,16 +1262,16 @@ dbHasSpace(soci::session& session, Config const& config, beast::Journal j) } static auto const pageSize = [&] { - std::uint32_t ps; + std::uint32_t ps = 0; session << "PRAGMA page_size;", soci::into(ps); return ps; }(); static auto const maxPages = [&] { - std::uint32_t mp; + std::uint32_t mp = 0; session << "PRAGMA max_page_count;", soci::into(mp); return mp; }(); - std::uint32_t pageCount; + std::uint32_t pageCount = 0; session << "PRAGMA page_count;", soci::into(pageCount); std::uint32_t freePages = maxPages - pageCount; std::uint64_t freeSpace = safe_cast(freePages) * pageSize; diff --git a/src/xrpld/app/rdb/backend/detail/SQLiteDatabase.cpp b/src/xrpld/app/rdb/backend/detail/SQLiteDatabase.cpp index 1b15fcf52e..4f5365ea01 100644 --- a/src/xrpld/app/rdb/backend/detail/SQLiteDatabase.cpp +++ b/src/xrpld/app/rdb/backend/detail/SQLiteDatabase.cpp @@ -400,7 +400,8 @@ SQLiteDatabase::oldestAccountTxPage(AccountTxPageOptions const& options) auto onTransaction = [&ret, &app = registry_.app()]( std::uint32_t ledger_index, std::string const& status, Blob&& rawTxn, Blob&& rawMeta) { - convertBlobsToTxResult(ret, ledger_index, status, rawTxn, rawMeta, app); + convertBlobsToTxResult( + ret, ledger_index, status, std::move(rawTxn), std::move(rawMeta), app); }; if (existsTransaction()) @@ -428,7 +429,8 @@ SQLiteDatabase::newestAccountTxPage(AccountTxPageOptions const& options) auto onTransaction = [&ret, &app = registry_.app()]( std::uint32_t ledger_index, std::string const& status, Blob&& rawTxn, Blob&& rawMeta) { - convertBlobsToTxResult(ret, ledger_index, status, rawTxn, rawMeta, app); + convertBlobsToTxResult( + ret, ledger_index, status, std::move(rawTxn), std::move(rawMeta), app); }; if (existsTransaction()) diff --git a/src/xrpld/app/rdb/detail/PeerFinder.cpp b/src/xrpld/app/rdb/detail/PeerFinder.cpp index cfab6f764a..d619d4bb85 100644 --- a/src/xrpld/app/rdb/detail/PeerFinder.cpp +++ b/src/xrpld/app/rdb/detail/PeerFinder.cpp @@ -83,7 +83,7 @@ updatePeerFinderDB(soci::session& session, int currentSchemaVersion, beast::Jour " PeerFinder_BootstrapCache_Next " " ( address ); "; - std::size_t count; + std::size_t count = 0; session << "SELECT COUNT(*) FROM PeerFinder_BootstrapCache;", soci::into(count); std::vector list; @@ -91,7 +91,7 @@ updatePeerFinderDB(soci::session& session, int currentSchemaVersion, beast::Jour { list.reserve(count); std::string s; - int valence; + int valence = 0; soci::statement st = (session.prepare << "SELECT " " address, " @@ -187,7 +187,7 @@ void readPeerFinderDB(soci::session& session, std::function const& func) { std::string s; - int valence; + int valence = 0; soci::statement st = (session.prepare << "SELECT " " address, " diff --git a/src/xrpld/core/detail/Config.cpp b/src/xrpld/core/detail/Config.cpp index d1937fa83e..52186ca9da 100644 --- a/src/xrpld/core/detail/Config.cpp +++ b/src/xrpld/core/detail/Config.cpp @@ -51,7 +51,7 @@ namespace detail { [[nodiscard]] std::uint64_t getMemorySize() { - if (struct sysinfo si; sysinfo(&si) == 0) + if (struct sysinfo si{}; sysinfo(&si) == 0) return static_cast(si.totalram) * si.mem_unit; return 0; @@ -154,7 +154,7 @@ parseIniFile(std::string const& strInput, bool const bTrim) boost::algorithm::split(vLines, strData, boost::algorithm::is_any_of("\n")); // Set the default Section name. - std::string strSection = SECTION_DEFAULT_NAME; + std::string strSection = SECTION_DEFAULT_NAME; // NOLINT(readability-redundant-string-init) // Initialize the default Section. secResult[strSection] = IniFileSections::mapped_type(); @@ -364,9 +364,13 @@ Config::setup(std::string const& strConf, bool bQuiet, bool bSilent, bool bStand // load() may have set a new value for the dataDir std::string const dbPath(legacy("database_path")); if (!dbPath.empty()) + { dataDir = boost::filesystem::path(dbPath); + } else if (RUN_STANDALONE) + { dataDir.clear(); + } } if (!dataDir.empty()) @@ -386,7 +390,6 @@ Config::setup(std::string const& strConf, bool bQuiet, bool bSilent, bool bStand if (RUN_STANDALONE) LEDGER_HISTORY = 0; - std::string ledgerTxDbType; Section ledgerTxTablesSection = section("ledger_tx_tables"); get_if_exists(ledgerTxTablesSection, "use_tx_tables", USE_TX_TABLES); @@ -494,13 +497,21 @@ Config::loadFromString(std::string const& fileContents) if (getSingleSection(secConfig, SECTION_NETWORK_ID, strTemp, j_)) { if (strTemp == "main") + { NETWORK_ID = 0; + } else if (strTemp == "testnet") + { NETWORK_ID = 1; + } else if (strTemp == "devnet") + { NETWORK_ID = 2; + } else + { NETWORK_ID = beast::lexicalCastThrow(strTemp); + } } if (getSingleSection(secConfig, SECTION_PEER_PRIVATE, strTemp, j_)) @@ -517,8 +528,10 @@ Config::loadFromString(std::string const& fileContents) { peers_in_max = beast::lexicalCastThrow(strTemp); if (*peers_in_max > 1000) + { Throw("Invalid value specified in [" SECTION_PEERS_IN_MAX "] section; the value must be less or equal than 1000"); + } } std::optional peers_out_max{}; @@ -526,15 +539,19 @@ Config::loadFromString(std::string const& fileContents) { peers_out_max = beast::lexicalCastThrow(strTemp); if (*peers_out_max < 10 || *peers_out_max > 1000) + { Throw("Invalid value specified in [" SECTION_PEERS_OUT_MAX "] section; the value must be in range 10-1000"); + } } // if one section is configured then the other must be configured too if ((peers_in_max && !peers_out_max) || (peers_out_max && !peers_in_max)) + { Throw("Both sections [" SECTION_PEERS_IN_MAX "]" "and [" SECTION_PEERS_OUT_MAX "] must be configured"); + } if (peers_in_max && peers_out_max) { @@ -546,17 +563,29 @@ Config::loadFromString(std::string const& fileContents) if (getSingleSection(secConfig, SECTION_NODE_SIZE, strTemp, j_)) { if (boost::iequals(strTemp, "tiny")) + { NODE_SIZE = 0; + } else if (boost::iequals(strTemp, "small")) + { NODE_SIZE = 1; + } else if (boost::iequals(strTemp, "medium")) + { NODE_SIZE = 2; + } else if (boost::iequals(strTemp, "large")) + { NODE_SIZE = 3; + } else if (boost::iequals(strTemp, "huge")) + { NODE_SIZE = 4; + } else + { NODE_SIZE = std::min(4, beast::lexicalCastThrow(strTemp)); + } } if (getSingleSection(secConfig, SECTION_SIGNING_SUPPORT, strTemp, j_)) @@ -574,32 +603,50 @@ Config::loadFromString(std::string const& fileContents) if (getSingleSection(secConfig, SECTION_RELAY_VALIDATIONS, strTemp, j_)) { if (boost::iequals(strTemp, "all")) + { RELAY_UNTRUSTED_VALIDATIONS = 1; + } else if (boost::iequals(strTemp, "trusted")) + { RELAY_UNTRUSTED_VALIDATIONS = 0; + } else if (boost::iequals(strTemp, "drop_untrusted")) + { RELAY_UNTRUSTED_VALIDATIONS = -1; + } else + { Throw("Invalid value specified in [" SECTION_RELAY_VALIDATIONS "] section"); + } } if (getSingleSection(secConfig, SECTION_RELAY_PROPOSALS, strTemp, j_)) { if (boost::iequals(strTemp, "all")) + { RELAY_UNTRUSTED_PROPOSALS = 1; + } else if (boost::iequals(strTemp, "trusted")) + { RELAY_UNTRUSTED_PROPOSALS = 0; + } else if (boost::iequals(strTemp, "drop_untrusted")) + { RELAY_UNTRUSTED_PROPOSALS = -1; + } else + { Throw("Invalid value specified in [" SECTION_RELAY_PROPOSALS "] section"); + } } if (exists(SECTION_VALIDATION_SEED) && exists(SECTION_VALIDATOR_TOKEN)) + { Throw("Cannot have both [" SECTION_VALIDATION_SEED "] and [" SECTION_VALIDATOR_TOKEN "] config sections"); + } if (getSingleSection(secConfig, SECTION_NETWORK_QUORUM, strTemp, j_)) NETWORK_QUORUM = beast::lexicalCastThrow(strTemp); @@ -614,24 +661,35 @@ Config::loadFromString(std::string const& fileContents) if (getSingleSection(secConfig, SECTION_LEDGER_HISTORY, strTemp, j_)) { if (boost::iequals(strTemp, "full")) + { LEDGER_HISTORY = std::numeric_limits::max(); + } else if (boost::iequals(strTemp, "none")) + { LEDGER_HISTORY = 0; + } else + { LEDGER_HISTORY = beast::lexicalCastThrow(strTemp); + } } if (getSingleSection(secConfig, SECTION_FETCH_DEPTH, strTemp, j_)) { if (boost::iequals(strTemp, "none")) + { FETCH_DEPTH = 0; + } else if (boost::iequals(strTemp, "full")) + { FETCH_DEPTH = std::numeric_limits::max(); + } else + { FETCH_DEPTH = beast::lexicalCastThrow(strTemp); + } - if (FETCH_DEPTH < 10) - FETCH_DEPTH = 10; + FETCH_DEPTH = std::max(FETCH_DEPTH, 10); } // By default, validators don't have pathfinding enabled, unless it is @@ -656,8 +714,10 @@ Config::loadFromString(std::string const& fileContents) SWEEP_INTERVAL = beast::lexicalCastThrow(strTemp); if (SWEEP_INTERVAL < 10 || SWEEP_INTERVAL > 600) + { Throw("Invalid " SECTION_SWEEP_INTERVAL ": must be between 10 and 600 inclusive"); + } } if (getSingleSection(secConfig, SECTION_WORKERS, strTemp, j_)) @@ -665,8 +725,10 @@ Config::loadFromString(std::string const& fileContents) WORKERS = beast::lexicalCastThrow(strTemp); if (WORKERS < 1 || WORKERS > 1024) + { Throw("Invalid " SECTION_WORKERS ": must be between 1 and 1024 inclusive."); + } } if (getSingleSection(secConfig, SECTION_IO_WORKERS, strTemp, j_)) @@ -674,8 +736,10 @@ Config::loadFromString(std::string const& fileContents) IO_WORKERS = beast::lexicalCastThrow(strTemp); if (IO_WORKERS < 1 || IO_WORKERS > 1024) + { Throw("Invalid " SECTION_IO_WORKERS ": must be between 1 and 1024 inclusive."); + } } if (getSingleSection(secConfig, SECTION_PREFETCH_WORKERS, strTemp, j_)) @@ -683,8 +747,10 @@ Config::loadFromString(std::string const& fileContents) PREFETCH_WORKERS = beast::lexicalCastThrow(strTemp); if (PREFETCH_WORKERS < 1 || PREFETCH_WORKERS > 1024) + { Throw("Invalid " SECTION_PREFETCH_WORKERS ": must be between 1 and 1024 inclusive."); + } } if (getSingleSection(secConfig, SECTION_COMPRESSION, strTemp, j_)) @@ -704,18 +770,26 @@ Config::loadFromString(std::string const& fileContents) // VP_REDUCE_RELAY_BASE_SQUELCH_ENABLE = // // sec.value_or("vp_base_squelch_enable", true); // if (sec.exists("vp_base_squelch_enable") && sec.exists("vp_enable")) + { Throw("Invalid " SECTION_REDUCE_RELAY " cannot specify both vp_base_squelch_enable and vp_enable " "options. " "vp_enable was deprecated and replaced by " "vp_base_squelch_enable"); + } if (sec.exists("vp_base_squelch_enable")) + { VP_REDUCE_RELAY_BASE_SQUELCH_ENABLE = sec.value_or("vp_base_squelch_enable", false); + } else if (sec.exists("vp_enable")) + { VP_REDUCE_RELAY_BASE_SQUELCH_ENABLE = sec.value_or("vp_enable", false); + } else + { VP_REDUCE_RELAY_BASE_SQUELCH_ENABLE = false; + } ///////////////// !!END OF TEMPORARY CODE BLOCK!! ///////////////////// ///////////////////// !!TEMPORARY CODE BLOCK!! /////////////////////// @@ -725,9 +799,11 @@ Config::loadFromString(std::string const& fileContents) VP_REDUCE_RELAY_SQUELCH_MAX_SELECTED_PEERS = sec.value_or("vp_base_squelch_max_selected_peers", 5); if (VP_REDUCE_RELAY_SQUELCH_MAX_SELECTED_PEERS < 3) + { Throw("Invalid " SECTION_REDUCE_RELAY " vp_base_squelch_max_selected_peers must be " "greater than or equal to 3"); + } ///////////////// !!END OF TEMPORARY CODE BLOCK!! ///////////////////// TX_REDUCE_RELAY_ENABLE = sec.value_or("tx_enable", false); @@ -735,10 +811,12 @@ Config::loadFromString(std::string const& fileContents) TX_REDUCE_RELAY_MIN_PEERS = sec.value_or("tx_min_peers", 20); TX_RELAY_PERCENTAGE = sec.value_or("tx_relay_percentage", 25); if (TX_RELAY_PERCENTAGE < 10 || TX_RELAY_PERCENTAGE > 100 || TX_REDUCE_RELAY_MIN_PEERS < 10) + { Throw("Invalid " SECTION_REDUCE_RELAY ", tx_min_peers must be greater than or equal to 10" ", tx_relay_percentage must be greater than or equal to 10 " "and less than or equal to 100"); + } } if (getSingleSection(secConfig, SECTION_MAX_TRANSACTIONS, strTemp, j_)) @@ -777,9 +855,11 @@ Config::loadFromString(std::string const& fileContents) } if (MAX_UNKNOWN_TIME < seconds{300} || MAX_UNKNOWN_TIME > seconds{1800}) + { Throw( "Invalid value 'max_unknown_time' in " SECTION_OVERLAY ": the time must be between 300 and 1800 seconds, inclusive."); + } try { @@ -805,24 +885,36 @@ Config::loadFromString(std::string const& fileContents) boost::regex const re("^\\s*(\\d+)\\s*(minutes|hours|days|weeks)\\s*(\\s+.*)?$"); boost::smatch match; if (!boost::regex_match(strTemp, match, re)) + { Throw("Invalid " SECTION_AMENDMENT_MAJORITY_TIME ", must be: [0-9]+ [minutes|hours|days|weeks]"); + } std::uint32_t duration = beast::lexicalCastThrow(match[1].str()); if (boost::iequals(match[2], "minutes")) + { AMENDMENT_MAJORITY_TIME = minutes(duration); + } else if (boost::iequals(match[2], "hours")) + { AMENDMENT_MAJORITY_TIME = hours(duration); + } else if (boost::iequals(match[2], "days")) + { AMENDMENT_MAJORITY_TIME = days(duration); + } else if (boost::iequals(match[2], "weeks")) + { AMENDMENT_MAJORITY_TIME = weeks(duration); + } if (AMENDMENT_MAJORITY_TIME < minutes(15)) + { Throw("Invalid " SECTION_AMENDMENT_MAJORITY_TIME ", the minimum amount of time an amendment must hold a " "majority is 15 minutes"); + } } if (getSingleSection(secConfig, SECTION_BETA_RPC_API, strTemp, j_)) @@ -846,25 +938,30 @@ Config::loadFromString(std::string const& fileContents) validatorsFile = strTemp; if (validatorsFile.empty()) + { Throw("Invalid path specified in [" SECTION_VALIDATORS_FILE "]"); + } if (!validatorsFile.is_absolute() && !CONFIG_DIR.empty()) validatorsFile = CONFIG_DIR / validatorsFile; if (!boost::filesystem::exists(validatorsFile)) + { Throw( "The file specified in [" SECTION_VALIDATORS_FILE "] " "does not exist: " + validatorsFile.string()); - + } else if ( !boost::filesystem::is_regular_file(validatorsFile) && !boost::filesystem::is_symlink(validatorsFile)) + { Throw( "Invalid file specified in [" SECTION_VALIDATORS_FILE "]: " + validatorsFile.string()); + } } else if (!CONFIG_DIR.empty()) { @@ -873,11 +970,15 @@ Config::loadFromString(std::string const& fileContents) if (!validatorsFile.empty()) { if (!boost::filesystem::exists(validatorsFile)) + { validatorsFile.clear(); + } else if ( !boost::filesystem::is_regular_file(validatorsFile) && !boost::filesystem::is_symlink(validatorsFile)) + { validatorsFile.clear(); + } } } @@ -922,6 +1023,7 @@ Config::loadFromString(std::string const& fileContents) section(SECTION_VALIDATOR_LIST_THRESHOLD).append(*valListThreshold); if (!entries && !valKeyEntries && !valListKeys) + { Throw( "The file specified in [" SECTION_VALIDATORS_FILE "] " @@ -933,19 +1035,24 @@ Config::loadFromString(std::string const& fileContents) "]" " section: " + validatorsFile.string()); + } } VALIDATOR_LIST_THRESHOLD = [&]() -> std::optional { auto const& listThreshold = section(SECTION_VALIDATOR_LIST_THRESHOLD); if (listThreshold.lines().empty()) + { return std::nullopt; - else if (listThreshold.values().size() == 1) + } + if (listThreshold.values().size() == 1) { auto strTemp = listThreshold.values()[0]; auto const listThreshold = beast::lexicalCastThrow(strTemp); if (listThreshold == 0) + { return std::nullopt; // NOTE: Explicitly ask for computed - else if (listThreshold > section(SECTION_VALIDATOR_LIST_KEYS).values().size()) + } + if (listThreshold > section(SECTION_VALIDATOR_LIST_KEYS).values().size()) { Throw( "Value in config section " @@ -954,12 +1061,10 @@ Config::loadFromString(std::string const& fileContents) } return listThreshold; } - else - { - Throw( - "Config section " - "[" SECTION_VALIDATOR_LIST_THRESHOLD "] should contain single value only"); - } + + Throw( + "Config section " + "[" SECTION_VALIDATOR_LIST_THRESHOLD "] should contain single value only"); }(); // Consolidate [validator_keys] and [validators] @@ -978,9 +1083,13 @@ Config::loadFromString(std::string const& fileContents) for (auto const& s : part.values()) { if (auto const f = getRegisteredFeature(s)) + { features.insert(*f); + } else + { Throw("Unknown feature: " + s + " in config file."); + } } } @@ -1051,13 +1160,13 @@ setup_FeeVote(Section const& section) { FeeSetup setup; { - std::uint64_t temp; + std::uint64_t temp = 0; if (set(temp, "reference_fee", section) && temp <= std::numeric_limits::max()) setup.reference_fee = temp; } { - std::uint32_t temp; + std::uint32_t temp = 0; if (set(temp, "account_reserve", section)) setup.account_reserve = temp; if (set(temp, "owner_reserve", section)) diff --git a/src/xrpld/overlay/detail/ConnectAttempt.cpp b/src/xrpld/overlay/detail/ConnectAttempt.cpp index ac0743e936..78aee006f1 100644 --- a/src/xrpld/overlay/detail/ConnectAttempt.cpp +++ b/src/xrpld/overlay/detail/ConnectAttempt.cpp @@ -51,7 +51,10 @@ void ConnectAttempt::stop() { if (!strand_.running_in_this_thread()) - return boost::asio::post(strand_, std::bind(&ConnectAttempt::stop, shared_from_this())); + { + boost::asio::post(strand_, std::bind(&ConnectAttempt::stop, shared_from_this())); + return; + } if (!socket_.is_open()) return; @@ -65,7 +68,10 @@ void ConnectAttempt::run() { if (!strand_.running_in_this_thread()) - return boost::asio::post(strand_, std::bind(&ConnectAttempt::run, shared_from_this())); + { + boost::asio::post(strand_, std::bind(&ConnectAttempt::run, shared_from_this())); + return; + } JLOG(journal_.debug()) << "run: connecting to " << remote_endpoint_; @@ -115,9 +121,10 @@ ConnectAttempt::tryAsyncShutdown() if (currentStep_ != ConnectionStep::TcpConnect && currentStep_ != ConnectionStep::TlsHandshake) { setTimer(ConnectionStep::ShutdownStarted); - return stream_.async_shutdown(bind_executor( + stream_.async_shutdown(bind_executor( strand_, std::bind(&ConnectAttempt::onShutdown, shared_from_this(), std::placeholders::_1))); + return; } close(); @@ -160,7 +167,7 @@ ConnectAttempt::close() cancelTimer(); error_code ec; - socket_.close(ec); + socket_.close(ec); // NOLINT(bugprone-unused-return-value) } void @@ -197,7 +204,8 @@ ConnectAttempt::setTimer(ConnectionStep step) catch (std::exception const& ex) { JLOG(journal_.error()) << "setTimer (global): " << ex.what(); - return close(); + close(); + return; } } @@ -240,7 +248,8 @@ ConnectAttempt::setTimer(ConnectionStep step) catch (std::exception const& ex) { JLOG(journal_.error()) << "setTimer (step " << stepToString(step) << "): " << ex.what(); - return close(); + close(); + return; } } @@ -272,7 +281,8 @@ ConnectAttempt::onTimer(error_code ec) // This should never happen JLOG(journal_.error()) << "onTimer: " << ec.message(); - return close(); + close(); + return; } // Determine which timer expired by checking their expiry times @@ -304,9 +314,13 @@ ConnectAttempt::onConnect(error_code ec) if (ec) { if (ec == boost::asio::error::operation_aborted) - return tryAsyncShutdown(); + { + tryAsyncShutdown(); + return; + } - return fail("onConnect", ec); + fail("onConnect", ec); + return; } if (!socket_.is_open()) @@ -315,10 +329,16 @@ ConnectAttempt::onConnect(error_code ec) // check if connection has really been established socket_.local_endpoint(ec); if (ec) - return fail("onConnect", ec); + { + fail("onConnect", ec); + return; + } if (shutdown_) - return tryAsyncShutdown(); + { + tryAsyncShutdown(); + return; + } ioPending_ = true; @@ -340,25 +360,38 @@ ConnectAttempt::onHandshake(error_code ec) if (ec) { if (ec == boost::asio::error::operation_aborted) - return tryAsyncShutdown(); + { + tryAsyncShutdown(); + return; + } - return fail("onHandshake", ec); + fail("onHandshake", ec); + return; } auto const local_endpoint = socket_.local_endpoint(ec); if (ec) - return fail("onHandshake", ec); + { + fail("onHandshake", ec); + return; + } setTimer(ConnectionStep::HttpWrite); // check if we connected to ourselves if (!overlay_.peerFinder().onConnected( slot_, beast::IPAddressConversion::from_asio(local_endpoint))) - return fail("Self connection"); + { + fail("Self connection"); + return; + } auto const sharedValue = makeSharedValue(*stream_ptr_, journal_); if (!sharedValue) - return shutdown(); // makeSharedValue logs + { + shutdown(); + return; // makeSharedValue logs + } req_ = makeRequest( !overlay_.peerFinder().config().peerPrivate, @@ -376,7 +409,10 @@ ConnectAttempt::onHandshake(error_code ec) app_); if (shutdown_) - return tryAsyncShutdown(); + { + tryAsyncShutdown(); + return; + } ioPending_ = true; @@ -396,13 +432,20 @@ ConnectAttempt::onWrite(error_code ec) if (ec) { if (ec == boost::asio::error::operation_aborted) - return tryAsyncShutdown(); + { + tryAsyncShutdown(); + return; + } - return fail("onWrite", ec); + fail("onWrite", ec); + return; } if (shutdown_) - return tryAsyncShutdown(); + { + tryAsyncShutdown(); + return; + } ioPending_ = true; @@ -429,17 +472,25 @@ ConnectAttempt::onRead(error_code ec) if (ec == boost::asio::error::eof) { JLOG(journal_.debug()) << "EOF"; - return shutdown(); + shutdown(); + return; } if (ec == boost::asio::error::operation_aborted) - return tryAsyncShutdown(); + { + tryAsyncShutdown(); + return; + } - return fail("onRead", ec); + fail("onRead", ec); + return; } if (shutdown_) - return tryAsyncShutdown(); + { + tryAsyncShutdown(); + return; + } processResponse(); } @@ -457,7 +508,8 @@ ConnectAttempt::processResponse() { JLOG(journal_.warn()) << "Unable to upgrade to peer protocol: " << response_.result() << " (" << response_.reason() << ")"; - return shutdown(); + shutdown(); + return; } // Parse response body to determine if this is a redirect or other @@ -465,8 +517,10 @@ ConnectAttempt::processResponse() std::string responseBody; responseBody.reserve(boost::asio::buffer_size(response_.body().data())); for (auto const buffer : response_.body().data()) + { responseBody.append( static_cast(buffer.data()), boost::asio::buffer_size(buffer)); + } Json::Value json; Json::Reader reader; @@ -481,12 +535,16 @@ ConnectAttempt::processResponse() << " failed to upgrade to peer protocol: " << response_.result() << " (" << response_.reason() << ")"; - return shutdown(); + shutdown(); + return; } Json::Value const& peerIps = json["peer-ips"]; if (!peerIps.isArray()) - return fail("processResponse: invalid peer-ips format"); + { + fail("processResponse: invalid peer-ips format"); + return; + } // Extract and validate peer endpoints std::vector redirectEndpoints; @@ -506,7 +564,8 @@ ConnectAttempt::processResponse() // Notify PeerFinder about the redirect redirectEndpoints may be empty overlay_.peerFinder().onRedirects(remote_endpoint_, redirectEndpoints); - return fail("processResponse: failed to connect to peer: redirected"); + fail("processResponse: failed to connect to peer: redirected"); + return; } // Just because our peer selected a particular protocol version doesn't @@ -520,12 +579,18 @@ ConnectAttempt::processResponse() negotiatedProtocol = pvs[0]; if (!negotiatedProtocol) - return fail("processResponse: Unable to negotiate protocol version"); + { + fail("processResponse: Unable to negotiate protocol version"); + return; + } } auto const sharedValue = makeSharedValue(*stream_ptr_, journal_); if (!sharedValue) - return shutdown(); // makeSharedValue logs + { + shutdown(); + return; // makeSharedValue logs + } try { @@ -553,14 +618,18 @@ ConnectAttempt::processResponse() { std::stringstream ss; ss << "Outbound Connect Attempt " << remote_endpoint_ << " " << to_string(result); - return fail(ss.str()); + fail(ss.str()); + return; } if (!socket_.is_open()) return; if (shutdown_) - return tryAsyncShutdown(); + { + tryAsyncShutdown(); + return; + } auto const peer = std::make_shared( app_, @@ -578,7 +647,8 @@ ConnectAttempt::processResponse() } catch (std::exception const& e) { - return fail(std::string("Handshake failure (") + e.what() + ")"); + fail(std::string("Handshake failure (") + e.what() + ")"); + return; } } diff --git a/src/xrpld/overlay/detail/Handshake.cpp b/src/xrpld/overlay/detail/Handshake.cpp index c463a31257..e9ad25bcc4 100644 --- a/src/xrpld/overlay/detail/Handshake.cpp +++ b/src/xrpld/overlay/detail/Handshake.cpp @@ -211,7 +211,7 @@ verifyHandshake( if (auto const iter = headers.find("Network-ID"); iter != headers.end()) { - std::uint32_t nid; + std::uint32_t nid = 0; if (!beast::lexicalCastChecked(nid, iter->value())) throw std::runtime_error("Invalid peer network identifier"); @@ -223,7 +223,7 @@ verifyHandshake( if (auto const iter = headers.find("Network-Time"); iter != headers.end()) { auto const netTime = [str = iter->value()]() -> TimeKeeper::time_point { - TimeKeeper::duration::rep val; + TimeKeeper::duration::rep val = 0; if (beast::lexicalCastChecked(val, str)) return TimeKeeper::time_point{TimeKeeper::duration{val}}; @@ -300,9 +300,11 @@ verifyHandshake( throw std::runtime_error("Invalid Local-IP"); if (beast::IP::is_public(remote) && remote != local_ip) + { throw std::runtime_error( "Incorrect Local-IP: " + remote.to_string() + " instead of " + local_ip.to_string()); + } } if (auto const iter = headers.find("Remote-IP"); iter != headers.end()) @@ -318,9 +320,11 @@ verifyHandshake( // We know our public IP and peer reports our connection came // from some other IP. if (remote_ip != public_ip) + { throw std::runtime_error( "Incorrect Remote-IP: " + public_ip.to_string() + " instead of " + remote_ip.to_string()); + } } } diff --git a/src/xrpld/overlay/detail/Message.cpp b/src/xrpld/overlay/detail/Message.cpp index 510bc24fe2..754545f04a 100644 --- a/src/xrpld/overlay/detail/Message.cpp +++ b/src/xrpld/overlay/detail/Message.cpp @@ -58,6 +58,8 @@ Message::compress() bool const compressible = [&] { if (messageBytes <= 70) return false; + + // NOLINTNEXTLINE(bugprone-switch-missing-default-case) switch (type) { case protocol::mtMANIFESTS: @@ -104,7 +106,9 @@ Message::compress() setHeader(bufferCompressed_.data(), compressedSize, type, Algorithm::LZ4, messageBytes); } else + { bufferCompressed_.resize(0); + } } } @@ -186,10 +190,12 @@ Message::getBuffer(Compressed tryCompressed) std::call_once(once_flag_, &Message::compress, this); - if (bufferCompressed_.size() > 0) + if (!bufferCompressed_.empty()) + { return bufferCompressed_; - else - return buffer_; + } + + return buffer_; } int diff --git a/src/xrpld/overlay/detail/OverlayImpl.cpp b/src/xrpld/overlay/detail/OverlayImpl.cpp index 55f45e727e..d9077686ec 100644 --- a/src/xrpld/overlay/detail/OverlayImpl.cpp +++ b/src/xrpld/overlay/detail/OverlayImpl.cpp @@ -24,6 +24,8 @@ #include #include +#include + namespace xrpl { namespace CrawlOptions { @@ -479,9 +481,13 @@ OverlayImpl::start() for (auto const& addr : addresses) { if (addr.port() == 0) + { ips.push_back(to_string(addr.at_port(DEFAULT_PEER_PORT))); + } else + { ips.push_back(to_string(addr)); + } } std::string const base("config: "); @@ -501,9 +507,13 @@ OverlayImpl::start() for (auto& addr : addresses) { if (addr.port() == 0) + { ips.emplace_back(addr.address(), DEFAULT_PEER_PORT); + } else + { ips.emplace_back(addr); + } } if (!ips.empty()) @@ -634,8 +644,10 @@ OverlayImpl::onManifests( } if (!relay.list().empty()) + { for_each([m2 = std::make_shared(relay, protocol::mtMANIFESTS)]( - std::shared_ptr&& p) { p->send(m2); }); + std::shared_ptr const& p) { p->send(m2); }); + } } void @@ -667,16 +679,16 @@ OverlayImpl::limit() } Json::Value -OverlayImpl::getOverlayInfo() +OverlayImpl::getOverlayInfo() const { using namespace std::chrono; Json::Value jv; - auto& av = jv["active"] = Json::Value(Json::arrayValue); + auto& av = jv[jss::active] = Json::Value(Json::arrayValue); - for_each([&](std::shared_ptr&& sp) { + for_each([&](std::shared_ptr const& sp) { auto& pv = av.append(Json::Value(Json::objectValue)); pv[jss::public_key] = base64_encode(sp->getNodePublic().data(), sp->getNodePublic().size()); - pv[jss::type] = sp->slot()->inbound() ? "in" : "out"; + pv[jss::type] = sp->slot()->inbound() ? jss::in : jss::out; pv[jss::uptime] = static_cast(duration_cast(sp->uptime()).count()); if (sp->crawl()) { @@ -688,18 +700,20 @@ OverlayImpl::getOverlayInfo() } else { - pv[jss::port] = std::to_string(sp->getRemoteAddress().port()); + pv[jss::port] = sp->getRemoteAddress().port(); } } { auto version{sp->getVersion()}; if (!version.empty()) + { // Could move here if Json::value supported moving from strings pv[jss::version] = std::string{version}; + } } - std::uint32_t minSeq, maxSeq; + std::uint32_t minSeq = 0, maxSeq = 0; sp->ledgerRange(minSeq, maxSeq); if (minSeq != 0 || maxSeq != 0) pv[jss::complete_ledgers] = std::to_string(minSeq) + "-" + std::to_string(maxSeq); @@ -825,7 +839,7 @@ OverlayImpl::processValidatorList(http_request_type const& req, Handoff& handoff // return the most recent validator list for that key. constexpr std::string_view prefix("/vl/"); - if (!req.target().starts_with(prefix.data()) || !setup_.vlEnabled) + if (!req.target().starts_with(prefix) || !setup_.vlEnabled) return false; std::uint32_t version = 1; @@ -868,20 +882,18 @@ OverlayImpl::processValidatorList(http_request_type const& req, Handoff& handoff // 404 not found return fail(boost::beast::http::status::not_found); } - else if (!*vl) + if (!*vl) { return fail(boost::beast::http::status::bad_request); } - else - { - msg.result(boost::beast::http::status::ok); - msg.body() = *vl; + msg.result(boost::beast::http::status::ok); - msg.prepare_payload(); - handoff.response = std::make_shared(msg); - return true; - } + msg.body() = *vl; + + msg.prepare_payload(); + handoff.response = std::make_shared(msg); + return true; } bool @@ -907,36 +919,41 @@ OverlayImpl::processHealth(http_request_type const& req, Handoff& handoff) std::string server_state = info[jss::server_state].asString(); auto load_factor = info[jss::load_factor_server].asDouble() / info[jss::load_base].asDouble(); - enum { healthy, warning, critical }; - int health = healthy; - auto set_health = [&health](int state) { - if (health < state) - health = state; - }; + enum class HealthState { healthy, warning, critical }; + auto health = HealthState::healthy; + auto set_health = [&health](HealthState state) { health = std::max(health, state); }; msg.body()[jss::info] = Json::objectValue; if (last_validated_ledger_age >= 7 || last_validated_ledger_age < 0) { msg.body()[jss::info][jss::validated_ledger] = last_validated_ledger_age; if (last_validated_ledger_age < 20) - set_health(warning); + { + set_health(HealthState::warning); + } else - set_health(critical); + { + set_health(HealthState::critical); + } } if (amendment_blocked) { msg.body()[jss::info][jss::amendment_blocked] = true; - set_health(critical); + set_health(HealthState::critical); } if (number_peers <= 7) { msg.body()[jss::info][jss::peers] = number_peers; if (number_peers != 0) - set_health(warning); + { + set_health(HealthState::warning); + } else - set_health(critical); + { + set_health(HealthState::critical); + } } if (!(server_state == "full" || server_state == "validating" || server_state == "proposing")) @@ -944,30 +961,36 @@ OverlayImpl::processHealth(http_request_type const& req, Handoff& handoff) msg.body()[jss::info][jss::server_state] = server_state; if (server_state == "syncing" || server_state == "tracking" || server_state == "connected") { - set_health(warning); + set_health(HealthState::warning); } else - set_health(critical); + { + set_health(HealthState::critical); + } } if (load_factor > 100) { msg.body()[jss::info][jss::load_factor] = load_factor; if (load_factor < 1000) - set_health(warning); + { + set_health(HealthState::warning); + } else - set_health(critical); + { + set_health(HealthState::critical); + } } switch (health) { - case healthy: + case HealthState::healthy: msg.result(boost::beast::http::status::ok); break; - case warning: + case HealthState::warning: msg.result(boost::beast::http::status::service_unavailable); break; - case critical: + case HealthState::critical: msg.result(boost::beast::http::status::internal_server_error); break; } @@ -991,7 +1014,7 @@ OverlayImpl::getActivePeers() const Overlay::PeerSequence ret; ret.reserve(size()); - for_each([&ret](std::shared_ptr&& sp) { ret.emplace_back(std::move(sp)); }); + for_each([&ret](std::shared_ptr const& sp) { ret.emplace_back(std::move(sp)); }); return ret; } @@ -1021,10 +1044,14 @@ OverlayImpl::getActivePeers( if (!reduceRelayEnabled) ++disabled; - if (toSkip.count(id) == 0) + if (!toSkip.contains(id)) + { ret.emplace_back(std::move(p)); + } else if (reduceRelayEnabled) + { ++enabledInSkip; + } } } @@ -1034,7 +1061,7 @@ OverlayImpl::getActivePeers( void OverlayImpl::checkTracking(std::uint32_t index) { - for_each([index](std::shared_ptr&& sp) { sp->checkTracking(index); }); + for_each([index](std::shared_ptr const& sp) { sp->checkTracking(index); }); } std::shared_ptr @@ -1070,7 +1097,7 @@ void OverlayImpl::broadcast(protocol::TMProposeSet& m) { auto const sm = std::make_shared(m, protocol::mtPROPOSE_LEDGER); - for_each([&](std::shared_ptr&& p) { p->send(sm); }); + for_each([&](std::shared_ptr const& p) { p->send(sm); }); } std::set @@ -1079,8 +1106,8 @@ OverlayImpl::relay(protocol::TMProposeSet& m, uint256 const& uid, PublicKey cons if (auto const toSkip = app_.getHashRouter().shouldRelay(uid)) { auto const sm = std::make_shared(m, protocol::mtPROPOSE_LEDGER, validator); - for_each([&](std::shared_ptr&& p) { - if (toSkip->find(p->id()) == toSkip->end()) + for_each([&](std::shared_ptr const& p) { + if (!toSkip->contains(p->id())) p->send(sm); }); return *toSkip; @@ -1092,7 +1119,7 @@ void OverlayImpl::broadcast(protocol::TMValidation& m) { auto const sm = std::make_shared(m, protocol::mtVALIDATION); - for_each([sm](std::shared_ptr&& p) { p->send(sm); }); + for_each([sm](std::shared_ptr const& p) { p->send(sm); }); } std::set @@ -1101,8 +1128,8 @@ OverlayImpl::relay(protocol::TMValidation& m, uint256 const& uid, PublicKey cons if (auto const toSkip = app_.getHashRouter().shouldRelay(uid)) { auto const sm = std::make_shared(m, protocol::mtVALIDATION, validator); - for_each([&](std::shared_ptr&& p) { - if (toSkip->find(p->id()) == toSkip->end()) + for_each([&](std::shared_ptr const& p) { + if (!toSkip->contains(p->id())) p->send(sm); }); return *toSkip; @@ -1297,7 +1324,7 @@ OverlayImpl::sendEndpoints() } void -OverlayImpl::sendTxQueue() +OverlayImpl::sendTxQueue() const { for_each([](auto const& p) { if (p->txReduceRelayEnabled()) @@ -1347,17 +1374,23 @@ OverlayImpl::updateSlotAndSquelch( return; if (!strand_.running_in_this_thread()) - return post( + { + post( strand_, // Must capture copies of reference parameters (i.e. key, validator) [this, key = key, validator = validator, peers = std::move(peers), type]() mutable { updateSlotAndSquelch(key, validator, std::move(peers), type); }); + return; + } + for (auto id : peers) + { slots_.updateSlotAndSquelch(key, validator, id, type, [&]() { reportInboundTraffic(TrafficCount::squelch_ignored, 0); }); + } } void @@ -1371,12 +1404,17 @@ OverlayImpl::updateSlotAndSquelch( return; if (!strand_.running_in_this_thread()) - return post( - strand_, - // Must capture copies of reference parameters (i.e. key, validator) - [this, key = key, validator = validator, peer, type]() { - updateSlotAndSquelch(key, validator, peer, type); - }); + { + { + post( + strand_, + // Must capture copies of reference parameters (i.e. key, validator) + [this, key = key, validator = validator, peer, type]() { + updateSlotAndSquelch(key, validator, peer, type); + }); + } + return; + } slots_.updateSlotAndSquelch(key, validator, peer, type, [&]() { reportInboundTraffic(TrafficCount::squelch_ignored, 0); @@ -1387,7 +1425,10 @@ void OverlayImpl::deletePeer(Peer::id_t id) { if (!strand_.running_in_this_thread()) - return post(strand_, std::bind(&OverlayImpl::deletePeer, this, id)); + { + post(strand_, std::bind(&OverlayImpl::deletePeer, this, id)); + return; + } slots_.deletePeer(id, true); } @@ -1396,7 +1437,10 @@ void OverlayImpl::deleteIdlePeers() { if (!strand_.running_in_this_thread()) - return post(strand_, std::bind(&OverlayImpl::deleteIdlePeers, this)); + { + post(strand_, std::bind(&OverlayImpl::deleteIdlePeers, this)); + return; + } slots_.deleteIdlePeers(); } diff --git a/src/xrpld/overlay/detail/OverlayImpl.h b/src/xrpld/overlay/detail/OverlayImpl.h index 9e891e6e8a..b77b4e69aa 100644 --- a/src/xrpld/overlay/detail/OverlayImpl.h +++ b/src/xrpld/overlay/detail/OverlayImpl.h @@ -474,7 +474,7 @@ private: Controlled through the config section [crawl] overlay=[0|1] */ Json::Value - getOverlayInfo(); + getOverlayInfo() const; /** Returns information about the local server. Reported through the /crawl API @@ -522,7 +522,7 @@ private: /** Send once a second transactions' hashes aggregated by peers. */ void - sendTxQueue(); + sendTxQueue() const; /** Check if peers stopped relaying messages * and if slots stopped receiving messages from the validator */ diff --git a/src/xrpld/overlay/detail/PeerImp.cpp b/src/xrpld/overlay/detail/PeerImp.cpp index dc7c350697..7ed8c45453 100644 --- a/src/xrpld/overlay/detail/PeerImp.cpp +++ b/src/xrpld/overlay/detail/PeerImp.cpp @@ -134,7 +134,10 @@ void PeerImp::run() { if (!strand_.running_in_this_thread()) - return post(strand_, std::bind(&PeerImp::run, shared_from_this())); + { + post(strand_, std::bind(&PeerImp::run, shared_from_this())); + return; + } auto parseLedgerHash = [](std::string_view value) -> std::optional { if (uint256 ret; ret.parseHex(value)) @@ -177,9 +180,13 @@ PeerImp::run() } if (inbound_) + { doAccept(); + } else + { doProtocolStart(); + } // Anything else that needs to be done with the connection should be // done in doProtocolStart @@ -189,7 +196,10 @@ void PeerImp::stop() { if (!strand_.running_in_this_thread()) - return post(strand_, std::bind(&PeerImp::stop, shared_from_this())); + { + post(strand_, std::bind(&PeerImp::stop, shared_from_this())); + return; + } if (!socket_.is_open()) return; @@ -209,14 +219,20 @@ void PeerImp::send(std::shared_ptr const& m) { if (!strand_.running_in_this_thread()) - return post(strand_, std::bind(&PeerImp::send, shared_from_this(), m)); + { + post(strand_, std::bind(&PeerImp::send, shared_from_this(), m)); + return; + } if (!socket_.is_open()) return; // we are in progress of closing the connection if (shutdown_) - return tryAsyncShutdown(); + { + tryAsyncShutdown(); + return; + } auto validator = m->getValidatorKey(); if (validator && !squelch_.expireSquelch(*validator)) @@ -273,7 +289,10 @@ void PeerImp::sendTxQueue() { if (!strand_.running_in_this_thread()) - return post(strand_, std::bind(&PeerImp::sendTxQueue, shared_from_this())); + { + post(strand_, std::bind(&PeerImp::sendTxQueue, shared_from_this())); + return; + } if (!txQueue_.empty()) { @@ -291,7 +310,10 @@ void PeerImp::addTxQueue(uint256 const& hash) { if (!strand_.running_in_this_thread()) - return post(strand_, std::bind(&PeerImp::addTxQueue, shared_from_this(), hash)); + { + post(strand_, std::bind(&PeerImp::addTxQueue, shared_from_this(), hash)); + return; + } if (txQueue_.size() == reduce_relay::MAX_TX_QUEUE_SIZE) { @@ -307,7 +329,10 @@ void PeerImp::removeTxQueue(uint256 const& hash) { if (!strand_.running_in_this_thread()) - return post(strand_, std::bind(&PeerImp::removeTxQueue, shared_from_this(), hash)); + { + post(strand_, std::bind(&PeerImp::removeTxQueue, shared_from_this(), hash)); + return; + } auto removed = txQueue_.erase(hash); JLOG(p_journal_.trace()) << "removeTxQueue " << removed; @@ -366,8 +391,10 @@ PeerImp::json() ret[jss::cluster] = true; if (auto const n = name(); !n.empty()) + { // Could move here if Json::Value supported moving from a string ret[jss::name] = n; + } } if (auto const d = domain(); !d.empty()) @@ -392,7 +419,7 @@ PeerImp::json() ret[jss::uptime] = static_cast(std::chrono::duration_cast(uptime()).count()); - std::uint32_t minSeq, maxSeq; + std::uint32_t minSeq = 0, maxSeq = 0; ledgerRange(minSeq, maxSeq); if ((minSeq != 0) || (maxSeq != 0)) @@ -545,10 +572,13 @@ void PeerImp::fail(std::string const& reason) { if (!strand_.running_in_this_thread()) - return post( + { + post( strand_, std::bind( (void (Peer::*)(std::string const&))&PeerImp::fail, shared_from_this(), reason)); + return; + } if (!socket_.is_open()) return; @@ -635,7 +665,7 @@ PeerImp::close() cancelTimer(); error_code ec; - socket_.close(ec); + socket_.close(ec); // NOLINT(bugprone-unused-return-value) overlay_.incPeerDisconnect(); @@ -658,7 +688,8 @@ PeerImp::setTimer(std::chrono::seconds interval) catch (std::exception const& ex) { JLOG(journal_.error()) << "setTimer: " << ex.what(); - return shutdown(); + shutdown(); + return; } timer_.async_wait(bind_executor( @@ -691,7 +722,8 @@ PeerImp::onTimer(error_code const& ec) // This should never happen JLOG(journal_.error()) << "onTimer: " << ec.message(); - return close(); + close(); + return; } // the timer expired before the shutdown completed @@ -699,11 +731,15 @@ PeerImp::onTimer(error_code const& ec) if (shutdown_) { JLOG(journal_.debug()) << "onTimer: shutdown timer expired"; - return close(); + close(); + return; } if (large_sendq_++ >= Tuning::sendqIntervals) - return fail("Large send queue"); + { + fail("Large send queue"); + return; + } if (auto const t = tracking_.load(); !inbound_ && t != Tracking::converged) { @@ -718,13 +754,17 @@ PeerImp::onTimer(error_code const& ec) (t == Tracking::unknown && (duration > app_.config().MAX_UNKNOWN_TIME))) { overlay_.peerFinder().on_failure(slot_); - return fail("Not useful"); + fail("Not useful"); + return; } } // Already waiting for PONG if (lastPingSeq_) - return fail("Ping Timeout"); + { + fail("Ping Timeout"); + return; + } lastPingTime_ = clock_type::now(); lastPingSeq_ = rand_int(); @@ -761,14 +801,20 @@ PeerImp::doAccept() // a shutdown was initiated before the handshake, there is nothing to do if (shutdown_) - return tryAsyncShutdown(); + { + tryAsyncShutdown(); + return; + } auto const sharedValue = makeSharedValue(*stream_ptr_, journal_); // This shouldn't fail since we already computed // the shared value successfully in OverlayImpl if (!sharedValue) - return fail("makeSharedValue: Unexpected failure"); + { + fail("makeSharedValue: Unexpected failure"); + return; + } JLOG(journal_.debug()) << "Protocol: " << to_string(protocol_); @@ -811,12 +857,22 @@ PeerImp::doAccept() if (!socket_.is_open()) return; if (ec == boost::asio::error::operation_aborted) - return tryAsyncShutdown(); + { + tryAsyncShutdown(); + return; + } if (ec) - return fail("onWriteResponse", ec); + { + fail("onWriteResponse", ec); + return; + } if (write_buffer->size() == bytes_transferred) - return doProtocolStart(); - return fail("Failed to write header"); + { + doProtocolStart(); + return; + } + fail("Failed to write header"); + return; })); } @@ -842,7 +898,10 @@ PeerImp::doProtocolStart() { // a shutdown was initiated before the handshare, there is nothing to do if (shutdown_) - return tryAsyncShutdown(); + { + tryAsyncShutdown(); + return; + } onReadMessage(error_code(), 0); @@ -895,17 +954,25 @@ PeerImp::onReadMessage(error_code ec, std::size_t bytes_transferred) if (ec == boost::asio::error::eof) { JLOG(journal_.debug()) << "EOF"; - return shutdown(); + shutdown(); + return; } if (ec == boost::asio::error::operation_aborted) - return tryAsyncShutdown(); + { + tryAsyncShutdown(); + return; + } - return fail("onReadMessage", ec); + fail("onReadMessage", ec); + return; } // we started shutdown, no reason to process further data if (shutdown_) - return tryAsyncShutdown(); + { + tryAsyncShutdown(); + return; + } if (auto stream = journal_.trace()) { @@ -921,7 +988,7 @@ PeerImp::onReadMessage(error_code ec, std::size_t bytes_transferred) while (read_buffer_.size() > 0) { - std::size_t bytes_consumed; + std::size_t bytes_consumed = 0; using namespace std::chrono_literals; std::tie(bytes_consumed, ec) = perf::measureDurationAndLog( @@ -936,7 +1003,10 @@ PeerImp::onReadMessage(error_code ec, std::size_t bytes_transferred) // the error_code is produced by invokeProtocolMessage // it could be due to a bad message if (ec) - return fail("onReadMessage", ec); + { + fail("onReadMessage", ec); + return; + } if (bytes_consumed == 0) break; @@ -946,7 +1016,10 @@ PeerImp::onReadMessage(error_code ec, std::size_t bytes_transferred) // check if a shutdown was initiated while processing messages if (shutdown_) - return tryAsyncShutdown(); + { + tryAsyncShutdown(); + return; + } readPending_ = true; @@ -978,9 +1051,13 @@ PeerImp::onWriteMessage(error_code ec, std::size_t bytes_transferred) if (ec) { if (ec == boost::asio::error::operation_aborted) - return tryAsyncShutdown(); + { + tryAsyncShutdown(); + return; + } - return fail("onWriteMessage", ec); + fail("onWriteMessage", ec); + return; } if (auto stream = journal_.trace()) @@ -995,7 +1072,10 @@ PeerImp::onWriteMessage(error_code ec, std::size_t bytes_transferred) send_queue_.pop(); if (shutdown_) - return tryAsyncShutdown(); + { + tryAsyncShutdown(); + return; + } if (!send_queue_.empty()) { @@ -1003,7 +1083,7 @@ PeerImp::onWriteMessage(error_code ec, std::size_t bytes_transferred) XRPL_ASSERT(!shutdownStarted_, "xrpl::PeerImp::onWriteMessage : shutdown started"); // Timeout on writes only - return boost::asio::async_write( + boost::asio::async_write( stream_, boost::asio::buffer(send_queue_.front()->getBuffer(compressionEnabled_)), bind_executor( @@ -1013,6 +1093,7 @@ PeerImp::onWriteMessage(error_code ec, std::size_t bytes_transferred) shared_from_this(), std::placeholders::_1, std::placeholders::_2))); + return; } } @@ -1122,9 +1203,13 @@ PeerImp::onMessage(std::shared_ptr const& m) std::lock_guard sl(recentLock_); if (latency_) + { latency_ = (*latency_ * 7 + rtt) / 8; + } else + { latency_ = rtt; + } } return; @@ -1315,7 +1400,7 @@ PeerImp::handleTransaction( } // LCOV_EXCL_STOP - HashRouterFlags flags; + HashRouterFlags flags = HashRouterFlags::UNDEFINED; constexpr std::chrono::seconds tx_interval = 10s; if (!app_.getHashRouter().shouldProcess(txID, id_, flags, tx_interval)) @@ -1330,7 +1415,9 @@ PeerImp::handleTransaction( // Erase only if the server has seen this tx. If the server has not // seen this tx then the tx could not has been queued for this peer. else if (eraseTxQueue && txReduceRelayEnabled()) + { removeTxQueue(txID); + } overlay_.reportInboundTraffic( TrafficCount::category::transaction_duplicate, Message::messageSize(*m)); @@ -1402,7 +1489,10 @@ PeerImp::onMessage(std::shared_ptr const& m) // Verify ledger info type if (itype < protocol::liBASE || itype > protocol::liTS_CANDIDATE) - return badData("Invalid ledger info type"); + { + badData("Invalid ledger info type"); + return; + } auto const ltype = [&m]() -> std::optional<::protocol::TMLedgerType> { if (m->has_ltype()) @@ -1413,21 +1503,31 @@ PeerImp::onMessage(std::shared_ptr const& m) if (itype == protocol::liTS_CANDIDATE) { if (!m->has_ledgerhash()) - return badData("Invalid TX candidate set, missing TX set hash"); + { + badData("Invalid TX candidate set, missing TX set hash"); + return; + } } else if ( !m->has_ledgerhash() && !m->has_ledgerseq() && !(ltype && *ltype == protocol::ltCLOSED)) { - return badData("Invalid request"); + badData("Invalid request"); + return; } // Verify ledger type if (ltype && (*ltype < protocol::ltACCEPTED || *ltype > protocol::ltCLOSED)) - return badData("Invalid ledger type"); + { + badData("Invalid ledger type"); + return; + } // Verify ledger hash if (m->has_ledgerhash() && !stringIsUint256Sized(m->ledgerhash())) - return badData("Invalid ledger hash"); + { + badData("Invalid ledger hash"); + return; + } // Verify ledger sequence if (m->has_ledgerseq()) @@ -1439,7 +1539,8 @@ PeerImp::onMessage(std::shared_ptr const& m) if (app_.getLedgerMaster().getValidatedLedgerAge() <= 10s && ledgerSeq > app_.getLedgerMaster().getValidLedgerIndex() + 10) { - return badData("Invalid ledger sequence " + std::to_string(ledgerSeq)); + badData("Invalid ledger sequence " + std::to_string(ledgerSeq)); + return; } } @@ -1447,25 +1548,35 @@ PeerImp::onMessage(std::shared_ptr const& m) if (itype != protocol::liBASE) { if (m->nodeids_size() <= 0) - return badData("Invalid ledger node IDs"); + { + badData("Invalid ledger node IDs"); + return; + } for (auto const& nodeId : m->nodeids()) { if (deserializeSHAMapNodeID(nodeId) == std::nullopt) - return badData("Invalid SHAMap node ID"); + { + badData("Invalid SHAMap node ID"); + return; + } } } // Verify query type if (m->has_querytype() && m->querytype() != protocol::qtINDIRECT) - return badData("Invalid query type"); + { + badData("Invalid query type"); + return; + } // Verify query depth if (m->has_querydepth()) { if (m->querydepth() > Tuning::maxQueryDepth || itype == protocol::liBASE) { - return badData("Invalid query depth"); + badData("Invalid query depth"); + return; } } @@ -1496,9 +1607,13 @@ PeerImp::onMessage(std::shared_ptr const& m) if (reply.has_error()) { if (reply.error() == protocol::TMReplyError::reBAD_REQUEST) + { peer->charge(Resource::feeMalformedRequest, "proof_path_request"); + } else + { peer->charge(Resource::feeRequestNoReply, "proof_path_request"); + } } else { @@ -1542,9 +1657,13 @@ PeerImp::onMessage(std::shared_ptr const& m) if (reply.has_error()) { if (reply.error() == protocol::TMReplyError::reBAD_REQUEST) + { peer->charge(Resource::feeMalformedRequest, "replay_delta_request"); + } else + { peer->charge(Resource::feeRequestNoReply, "replay_delta_request"); + } } else { @@ -1579,7 +1698,10 @@ PeerImp::onMessage(std::shared_ptr const& m) // Verify ledger hash if (!stringIsUint256Sized(m->ledgerhash())) - return badData("Invalid ledger hash"); + { + badData("Invalid ledger hash"); + return; + } // Verify ledger sequence { @@ -1588,7 +1710,8 @@ PeerImp::onMessage(std::shared_ptr const& m) { if (ledgerSeq != 0) { - return badData("Invalid ledger sequence " + std::to_string(ledgerSeq)); + badData("Invalid ledger sequence " + std::to_string(ledgerSeq)); + return; } } else @@ -1598,26 +1721,32 @@ PeerImp::onMessage(std::shared_ptr const& m) if (app_.getLedgerMaster().getValidatedLedgerAge() <= 10s && ledgerSeq > app_.getLedgerMaster().getValidLedgerIndex() + 10) { - return badData("Invalid ledger sequence " + std::to_string(ledgerSeq)); + badData("Invalid ledger sequence " + std::to_string(ledgerSeq)); + return; } } } // Verify ledger info type if (m->type() < protocol::liBASE || m->type() > protocol::liTS_CANDIDATE) - return badData("Invalid ledger info type"); + { + badData("Invalid ledger info type"); + return; + } // Verify reply error if (m->has_error() && (m->error() < protocol::reNO_LEDGER || m->error() > protocol::reBAD_REQUEST)) { - return badData("Invalid reply error"); + badData("Invalid reply error"); + return; } // Verify ledger nodes. if (m->nodes_size() <= 0 || m->nodes_size() > Tuning::hardMaxReplyNodes) { - return badData("Invalid Ledger/TXset nodes " + std::to_string(m->nodes_size())); + badData("Invalid Ledger/TXset nodes " + std::to_string(m->nodes_size())); + return; } // If there is a request cookie, attempt to relay the message @@ -1770,7 +1899,9 @@ PeerImp::onMessage(std::shared_ptr const& m) { std::lock_guard sl(recentLock_); if (!last_status_.has_newstatus() || m->has_newstatus()) + { last_status_ = *m; + } else { // preserve old status @@ -1856,7 +1987,7 @@ PeerImp::onMessage(std::shared_ptr const& m) checkTracking(m->ledgerseq(), app_.getLedgerMaster().getValidLedgerIndex()); } - app_.getOPs().pubPeerStatus([=, this]() -> Json::Value { + app_.getOPs().pubPeerStatus([m, this]() -> Json::Value { Json::Value j = Json::objectValue; if (m->has_newstatus()) @@ -1933,7 +2064,7 @@ PeerImp::onMessage(std::shared_ptr const& m) void PeerImp::checkTracking(std::uint32_t validationSeq) { - std::uint32_t serverSeq; + std::uint32_t serverSeq = 0; { // Extract the sequence number of the highest // ledger this peer has @@ -2227,7 +2358,7 @@ PeerImp::onMessage(std::shared_ptr const& m fee_.update(Resource::feeUselessData, "unsupported peer"); return; } - else if (m->version() < 2) + if (m->version() < 2) { JLOG(p_journal_.debug()) << "ValidatorListCollection: received invalid validator list " @@ -2312,8 +2443,10 @@ PeerImp::onMessage(std::shared_ptr const& m) // peer receives within IDLED seconds since the message has been // relayed. if (relayed && (stopwatch().now() - *relayed) < reduce_relay::IDLED) + { overlay_.updateSlotAndSquelch( key, val->getSignerPublic(), id_, protocol::mtVALIDATION); + } // increase duplicate validations received overlay_.reportInboundTraffic( @@ -2483,7 +2616,9 @@ PeerImp::onMessage(std::shared_ptr const& m) JLOG(p_journal_.debug()) << "GetObj: Late fetch pack for " << pLSeq; } else + { progress = true; + } } } @@ -2584,11 +2719,13 @@ PeerImp::onMessage(std::shared_ptr const& m) overlay_.addTxMetrics(m->transactions_size()); for (std::uint32_t i = 0; i < m->transactions_size(); ++i) + { handleTransaction( std::shared_ptr( m->mutable_transactions(i), [](protocol::TMTransaction*) {}), false, true); + } } void @@ -2596,7 +2733,10 @@ PeerImp::onMessage(std::shared_ptr const& m) { using on_message_fn = void (PeerImp::*)(std::shared_ptr const&); if (!strand_.running_in_this_thread()) - return post(strand_, std::bind((on_message_fn)&PeerImp::onMessage, shared_from_this(), m)); + { + post(strand_, std::bind((on_message_fn)&PeerImp::onMessage, shared_from_this(), m)); + return; + } if (!m->has_validatorpubkey()) { @@ -2621,9 +2761,13 @@ PeerImp::onMessage(std::shared_ptr const& m) std::uint32_t duration = m->has_squelchduration() ? m->squelchduration() : 0; if (!m->squelch()) + { squelch_.removeSquelch(key); + } else if (!squelch_.addSquelch(key, std::chrono::seconds{duration})) + { fee_.update(Resource::feeInvalidData, "squelch duration"); + } JLOG(p_journal_.debug()) << "onMessage: TMSquelch " << slice << " " << id() << " " << duration; } @@ -2879,12 +3023,16 @@ PeerImp::checkPropose( return; } - bool relay; + bool relay = false; if (isTrusted) + { relay = app_.getOPs().processTrustedProposal(peerPos); + } else + { relay = app_.config().RELAY_UNTRUSTED_PROPOSALS == 1 || cluster(); + } if (relay) { @@ -2895,11 +3043,13 @@ PeerImp::checkPropose( auto haveMessage = app_.overlay().relay(*packet, peerPos.suppressionID(), peerPos.publicKey()); if (!haveMessage.empty()) + { overlay_.updateSlotAndSquelch( peerPos.suppressionID(), peerPos.publicKey(), std::move(haveMessage), protocol::mtPROPOSE_LEDGER); + } } } @@ -3235,7 +3385,8 @@ PeerImp::processLedgerRequest(std::shared_ptr const& m) // Add requested node data to reply if (m->nodeids_size() > 0) { - auto const queryDepth{m->has_querydepth() ? m->querydepth() : (isHighLatency() ? 2 : 1)}; + std::uint32_t const defaultDepth = isHighLatency() ? 2 : 1; + auto const queryDepth{m->has_querydepth() ? m->querydepth() : defaultDepth}; std::vector> data; @@ -3347,9 +3498,13 @@ PeerImp::getScore(bool haveItem) const } if (latency) + { score -= latency->count() * spLatency; + } else + { score -= spNoLatency; + } return score; } diff --git a/src/xrpld/overlay/detail/ProtocolVersion.cpp b/src/xrpld/overlay/detail/ProtocolVersion.cpp index 3163ba55e0..29bff996a9 100644 --- a/src/xrpld/overlay/detail/ProtocolVersion.cpp +++ b/src/xrpld/overlay/detail/ProtocolVersion.cpp @@ -81,8 +81,8 @@ parseProtocolVersions(boost::beast::string_view const& value) if (boost::regex_match(s, m, re)) { - std::uint16_t major; - std::uint16_t minor; + std::uint16_t major = 0; + std::uint16_t minor = 0; if (!beast::lexicalCastChecked(major, std::string(m[1]))) continue; diff --git a/src/xrpld/overlay/detail/TrafficCount.cpp b/src/xrpld/overlay/detail/TrafficCount.cpp index f782d7cf0a..2ce32b4468 100644 --- a/src/xrpld/overlay/detail/TrafficCount.cpp +++ b/src/xrpld/overlay/detail/TrafficCount.cpp @@ -36,16 +36,22 @@ TrafficCount::categorize( if (auto msg = dynamic_cast(&message)) { if (msg->type() == protocol::liTS_CANDIDATE) + { return (inbound && !msg->has_requestcookie()) ? TrafficCount::category::ld_tsc_get : TrafficCount::category::ld_tsc_share; + } if (msg->type() == protocol::liTX_NODE) + { return (inbound && !msg->has_requestcookie()) ? TrafficCount::category::ld_txn_get : TrafficCount::category::ld_txn_share; + } if (msg->type() == protocol::liAS_NODE) + { return (inbound && !msg->has_requestcookie()) ? TrafficCount::category::ld_asn_get : TrafficCount::category::ld_asn_share; + } return (inbound && !msg->has_requestcookie()) ? TrafficCount::category::ld_get : TrafficCount::category::ld_share; @@ -54,16 +60,22 @@ TrafficCount::categorize( if (auto msg = dynamic_cast(&message)) { if (msg->itype() == protocol::liTS_CANDIDATE) + { return (inbound || msg->has_requestcookie()) ? TrafficCount::category::gl_tsc_share : TrafficCount::category::gl_tsc_get; + } if (msg->itype() == protocol::liTX_NODE) + { return (inbound || msg->has_requestcookie()) ? TrafficCount::category::gl_txn_share : TrafficCount::category::gl_txn_get; + } if (msg->itype() == protocol::liAS_NODE) + { return (inbound || msg->has_requestcookie()) ? TrafficCount::category::gl_asn_share : TrafficCount::category::gl_asn_get; + } return (inbound || msg->has_requestcookie()) ? TrafficCount::category::gl_share : TrafficCount::category::gl_get; @@ -72,28 +84,40 @@ TrafficCount::categorize( if (auto msg = dynamic_cast(&message)) { if (msg->type() == protocol::TMGetObjectByHash::otLEDGER) + { return (msg->query() == inbound) ? TrafficCount::category::share_hash_ledger : TrafficCount::category::get_hash_ledger; + } if (msg->type() == protocol::TMGetObjectByHash::otTRANSACTION) + { return (msg->query() == inbound) ? TrafficCount::category::share_hash_tx : TrafficCount::category::get_hash_tx; + } if (msg->type() == protocol::TMGetObjectByHash::otTRANSACTION_NODE) + { return (msg->query() == inbound) ? TrafficCount::category::share_hash_txnode : TrafficCount::category::get_hash_txnode; + } if (msg->type() == protocol::TMGetObjectByHash::otSTATE_NODE) + { return (msg->query() == inbound) ? TrafficCount::category::share_hash_asnode : TrafficCount::category::get_hash_asnode; + } if (msg->type() == protocol::TMGetObjectByHash::otCAS_OBJECT) + { return (msg->query() == inbound) ? TrafficCount::category::share_cas_object : TrafficCount::category::get_cas_object; + } if (msg->type() == protocol::TMGetObjectByHash::otFETCH_PACK) + { return (msg->query() == inbound) ? TrafficCount::category::share_fetch_pack : TrafficCount::category::get_fetch_pack; + } if (msg->type() == protocol::TMGetObjectByHash::otTRANSACTIONS) return TrafficCount::category::get_transactions; diff --git a/src/xrpld/peerfinder/PeerfinderManager.h b/src/xrpld/peerfinder/PeerfinderManager.h index ab53068fca..b850a10975 100644 --- a/src/xrpld/peerfinder/PeerfinderManager.h +++ b/src/xrpld/peerfinder/PeerfinderManager.h @@ -74,7 +74,7 @@ struct Config /** Write the configuration into a property stream */ void - onWrite(beast::PropertyStream::Map& map); + onWrite(beast::PropertyStream::Map& map) const; /** Make PeerFinder::Config from configuration parameters * @param config server's configuration diff --git a/src/xrpld/peerfinder/detail/Bootcache.cpp b/src/xrpld/peerfinder/detail/Bootcache.cpp index af9a1ad57b..d07ec444a4 100644 --- a/src/xrpld/peerfinder/detail/Bootcache.cpp +++ b/src/xrpld/peerfinder/detail/Bootcache.cpp @@ -4,6 +4,8 @@ #include +#include + namespace xrpl { namespace PeerFinder { @@ -131,8 +133,7 @@ Bootcache::on_success(beast::IP::Endpoint const& endpoint) else { Entry entry(result.first->right); - if (entry.valence() < 0) - entry.valence() = 0; + entry.valence() = std::max(entry.valence(), 0); ++entry.valence(); m_map.erase(result.first); result = m_map.insert(value_type(endpoint, entry)); @@ -156,8 +157,7 @@ Bootcache::on_failure(beast::IP::Endpoint const& endpoint) else { Entry entry(result.first->right); - if (entry.valence() > 0) - entry.valence() = 0; + entry.valence() = std::min(entry.valence(), 0); --entry.valence(); m_map.erase(result.first); result = m_map.insert(value_type(endpoint, entry)); diff --git a/src/xrpld/peerfinder/detail/PeerfinderConfig.cpp b/src/xrpld/peerfinder/detail/PeerfinderConfig.cpp index 58246cd198..cc74018fc0 100644 --- a/src/xrpld/peerfinder/detail/PeerfinderConfig.cpp +++ b/src/xrpld/peerfinder/detail/PeerfinderConfig.cpp @@ -1,6 +1,8 @@ #include #include +#include + namespace xrpl { namespace PeerFinder { @@ -51,7 +53,7 @@ Config::applyTuning() } void -Config::onWrite(beast::PropertyStream::Map& map) +Config::onWrite(beast::PropertyStream::Map& map) const { map["max_peers"] = maxPeers; map["out_peers"] = outPeers; @@ -81,8 +83,7 @@ Config::makeConfig( if (cfg.PEERS_MAX != 0) config.maxPeers = cfg.PEERS_MAX; - if (config.maxPeers < Tuning::minOutCount) - config.maxPeers = Tuning::minOutCount; + config.maxPeers = std::max(config.maxPeers, Tuning::minOutCount); config.outPeers = config.calcOutPeers(); // Calculate the number of outbound peers we want. If we dont want @@ -93,9 +94,13 @@ Config::makeConfig( // Calculate the largest number of inbound connections we could // take. if (config.maxPeers >= config.outPeers) + { config.inPeers = config.maxPeers - config.outPeers; + } else + { config.inPeers = 0; + } } else { diff --git a/src/xrpld/peerfinder/detail/PeerfinderManager.cpp b/src/xrpld/peerfinder/detail/PeerfinderManager.cpp index d04e09441c..70f082c0d5 100644 --- a/src/xrpld/peerfinder/detail/PeerfinderManager.cpp +++ b/src/xrpld/peerfinder/detail/PeerfinderManager.cpp @@ -33,8 +33,7 @@ public: beast::Journal journal, BasicConfig const& config, beast::insight::Collector::ptr const& collector) - : Manager() - , io_context_(io_context) + : io_context_(io_context) , work_(std::in_place, boost::asio::make_work_guard(io_context_)) , m_clock(clock) , m_journal(journal) diff --git a/src/xrpld/perflog/detail/PerfLogImp.cpp b/src/xrpld/perflog/detail/PerfLogImp.cpp index d1267953e4..fc9ef70830 100644 --- a/src/xrpld/perflog/detail/PerfLogImp.cpp +++ b/src/xrpld/perflog/detail/PerfLogImp.cpp @@ -257,8 +257,10 @@ void PerfLogImp::report() { if (!logFile_) + { // If logFile_ is not writable do no further work. return; + } auto const present = system_clock::now(); if (present < lastLog_ + setup_.logInterval) @@ -345,9 +347,13 @@ PerfLogImp::rpcEnd(std::string const& method, std::uint64_t const requestId, boo } std::lock_guard lock(counter->second.mutex); if (finish) + { ++counter->second.value.finished; + } else + { ++counter->second.value.errored; + } counter->second.value.duration += std::chrono::duration_cast(steady_clock::now() - startTime); } @@ -437,7 +443,7 @@ PerfLogImp::rotate() void PerfLogImp::start() { - if (setup_.perfLog.size()) + if (!setup_.perfLog.empty()) thread_ = std::thread(&PerfLogImp::run, this); } @@ -463,7 +469,7 @@ setup_PerfLog(Section const& section, boost::filesystem::path const& configDir) PerfLog::Setup setup; std::string perfLog; set(perfLog, "perf_log", section); - if (perfLog.size()) + if (!perfLog.empty()) { setup.perfLog = boost::filesystem::path(perfLog); if (setup.perfLog.is_relative()) @@ -472,7 +478,7 @@ setup_PerfLog(Section const& section, boost::filesystem::path const& configDir) } } - std::uint64_t logInterval; + std::uint64_t logInterval = 0; if (get_if_exists(section, "log_interval", logInterval)) setup.logInterval = std::chrono::seconds(logInterval); return setup; diff --git a/src/xrpld/rpc/ServerHandler.h b/src/xrpld/rpc/ServerHandler.h index af7ec4bf1c..74f59756b0 100644 --- a/src/xrpld/rpc/ServerHandler.h +++ b/src/xrpld/rpc/ServerHandler.h @@ -182,7 +182,7 @@ private: Port const& port, std::string const& request, beast::IP::Endpoint const& remoteIPAddress, - Output&&, + Output const&, std::shared_ptr coro, std::string_view forwardedFor, std::string_view user); @@ -192,7 +192,7 @@ private: }; ServerHandler::Setup -setup_ServerHandler(Config const& c, std::ostream&& log); +setup_ServerHandler(Config const& c, std::ostream& log); std::unique_ptr make_ServerHandler( diff --git a/src/xrpld/rpc/detail/DeliveredAmount.cpp b/src/xrpld/rpc/detail/DeliveredAmount.cpp index 7cb90eaa98..3e89edc1f9 100644 --- a/src/xrpld/rpc/detail/DeliveredAmount.cpp +++ b/src/xrpld/rpc/detail/DeliveredAmount.cpp @@ -34,7 +34,7 @@ getDeliveredAmount( if (auto const& deliveredAmount = transactionMeta.getDeliveredAmount(); deliveredAmount.has_value()) { - return *deliveredAmount; + return deliveredAmount; } if (serializedTx->isFieldPresent(sfAmount)) diff --git a/src/xrpld/rpc/detail/Handler.cpp b/src/xrpld/rpc/detail/Handler.cpp index ec970b107e..04cf0420b7 100644 --- a/src/xrpld/rpc/detail/Handler.cpp +++ b/src/xrpld/rpc/detail/Handler.cpp @@ -42,9 +42,13 @@ handle(JsonContext& context, Object& object) auto status = handler.check(); if (status) + { status.inject(object); + } else + { handler.writeResult(object); + } return status; } @@ -172,9 +176,11 @@ private: { if (overlappingApiVersion( table_.equal_range(entry.name_), entry.minApiVer_, entry.maxApiVer_)) + { LogicError( std::string("Handler for ") + entry.name_ + " overlaps with an existing handler"); + } table_.insert({entry.name_, entry}); } @@ -232,9 +238,11 @@ private: table_.equal_range(HandlerImpl::name), HandlerImpl::minApiVer, HandlerImpl::maxApiVer)) + { LogicError( std::string("Handler for ") + HandlerImpl::name + " overlaps with an existing handler"); + } table_.insert({HandlerImpl::name, handlerFrom()}); } diff --git a/src/xrpld/rpc/detail/MPTokenIssuanceID.cpp b/src/xrpld/rpc/detail/MPTokenIssuanceID.cpp index f818c72b59..ef8bc448ee 100644 --- a/src/xrpld/rpc/detail/MPTokenIssuanceID.cpp +++ b/src/xrpld/rpc/detail/MPTokenIssuanceID.cpp @@ -17,7 +17,7 @@ canHaveMPTokenIssuanceID( return false; // if the transaction failed nothing could have been delivered. - if (transactionMeta.getResultTER() != tesSUCCESS) + if (!isTesSuccess(transactionMeta.getResultTER())) return false; return true; diff --git a/src/xrpld/rpc/detail/RPCCall.cpp b/src/xrpld/rpc/detail/RPCCall.cpp index 134cbb34f8..cd36208294 100644 --- a/src/xrpld/rpc/detail/RPCCall.cpp +++ b/src/xrpld/rpc/detail/RPCCall.cpp @@ -116,7 +116,7 @@ private: jvResult[jss::currency] = strCurrency; - if (strIssuer.length()) + if (!strIssuer.empty()) { // Could confirm issuer is a valid Ripple address. jvResult[jss::issuer] = strIssuer; @@ -124,11 +124,9 @@ private: return jvResult; } - else - { - return RPC::make_param_error( - std::string("Invalid currency/issuer '") + strCurrencyIssuer + "'"); - } + + return RPC::make_param_error( + std::string("Invalid currency/issuer '") + strCurrencyIssuer + "'"); } static bool @@ -212,6 +210,7 @@ private: // account_tx accountID [ledger_min [ledger_max [limit [offset]]]] [binary] // [count] [descending] Json::Value + // NOLINTNEXTLINE(readability-make-member-function-const) parseAccountTransactions(Json::Value const& jvParams) { Json::Value jvRequest(Json::objectValue); @@ -298,19 +297,15 @@ private: { return jvTakerPays; } - else - { - jvRequest[jss::taker_pays] = jvTakerPays; - } + + jvRequest[jss::taker_pays] = jvTakerPays; if (isRpcError(jvTakerGets)) { return jvTakerGets; } - else - { - jvRequest[jss::taker_gets] = jvTakerGets; - } + + jvRequest[jss::taker_gets] = jvTakerGets; if (jvParams.size() >= 3) { @@ -366,9 +361,13 @@ private: std::string input = jvParams[0u].asString(); if (input.find_first_not_of("0123456789") == std::string::npos) + { jvRequest["can_delete"] = jvParams[0u].asUInt(); + } else + { jvRequest["can_delete"] = input; + } return jvRequest; } @@ -447,11 +446,17 @@ private: // determines whether an amendment is vetoed - so "reject" means // that jss::vetoed is true. if (boost::iequals(action, "reject")) + { jvRequest[jss::vetoed] = Json::Value(true); + } else if (boost::iequals(action, "accept")) + { jvRequest[jss::vetoed] = Json::Value(false); + } else + { return rpcError(rpcINVALID_PARAMS); + } } return jvRequest; @@ -967,8 +972,7 @@ private: return jvRequest; } - else if ( - (jvParams.size() >= 2 || bOffline) && reader.parse(jvParams[1u].asString(), txJSON)) + if ((jvParams.size() >= 2 || bOffline) && reader.parse(jvParams[1u].asString(), txJSON)) { // Signing or submitting tx_json. Json::Value jvRequest{Json::objectValue}; @@ -1055,9 +1059,13 @@ private: } if (jvParams[0u].asString().length() == 16) + { jvRequest[jss::ctid] = jvParams[0u].asString(); + } else + { jvRequest[jss::transaction] = jvParams[0u].asString(); + } return jvRequest; } @@ -1123,9 +1131,13 @@ private: if (param[0] != 'r') { if (param.size() == 64) + { jvRequest[jss::ledger_hash] = param; + } else + { jvRequest[jss::ledger_index] = param; + } if (size <= index) return RPC::make_param_error("Invalid hotwallet"); @@ -1355,10 +1367,12 @@ struct RPCCallImp // Receive reply if (strData.empty()) + { Throw( "no response from server. Please " "ensure that the rippled server is running in another " "process."); + } // Parse reply JLOG(j.debug()) << "RPC reply: " << strData << std::endl; @@ -1435,9 +1449,13 @@ rpcCmdToJson( }; if (jvRequest.isObject()) + { insert_api_version(jvRequest); + } else if (jvRequest.isArray()) + { std::for_each(jvRequest.begin(), jvRequest.end(), insert_api_version); + } JLOG(j.trace()) << "RPC Request: " << jvRequest << std::endl; return jvRequest; @@ -1476,8 +1494,8 @@ rpcClient( xrpl::ServerHandler::Setup setup; try { - setup = setup_ServerHandler( - config, beast::logstream{logs.journal("HTTPClient").warn()}); + beast::logstream rpcCallLog{logs.journal("HTTPClient").warn()}; + setup = setup_ServerHandler(config, rpcCallLog); } catch (std::exception const&) // NOLINT(bugprone-empty-catch) { @@ -1500,7 +1518,9 @@ rpcClient( jvRequest["admin_password"] = setup.client.admin_password; if (jvRequest.isObject()) + { jvParams.append(jvRequest); + } else if (jvRequest.isArray()) { for (Json::UInt i = 0; i < jvRequest.size(); ++i) @@ -1516,9 +1536,12 @@ rpcClient( setup.client.user, setup.client.password, "", - jvRequest.isMember(jss::method) // Allow parser to rewrite method. - ? jvRequest[jss::method].asString() - : jvRequest.isArray() ? "batch" : args[0], + // Allow parser to rewrite method. + [&]() -> std::string { + if (jvRequest.isMember(jss::method)) + return jvRequest[jss::method].asString(); + return jvRequest.isArray() ? "batch" : args[0]; + }(), jvParams, // Parsed, execute. setup.client.secure != 0, // Use SSL config.quiet(), @@ -1557,11 +1580,17 @@ rpcClient( { jvOutput[jss::status] = "error"; if (jvOutput.isMember(jss::error_code)) + { nRet = std::stoi(jvOutput[jss::error_code].asString()); + } else if (jvOutput[jss::error].isMember(jss::error_code)) + { nRet = std::stoi(jvOutput[jss::error][jss::error_code].asString()); + } else + { nRet = rpcBAD_SYNTAX; + } } // YYY We could have a command line flag for single line output for diff --git a/src/xrpld/rpc/detail/RPCHandler.cpp b/src/xrpld/rpc/detail/RPCHandler.cpp index 2066c8a5e9..840ebd5946 100644 --- a/src/xrpld/rpc/detail/RPCHandler.cpp +++ b/src/xrpld/rpc/detail/RPCHandler.cpp @@ -215,11 +215,9 @@ doCommand(RPC::JsonContext& context, Json::Value& result) return ret; } - else - { - auto ret = callMethod(context, method, handler->name_, result); - return ret; - } + + auto ret = callMethod(context, method, handler->name_, result); + return ret; } return rpcUNKNOWN_COMMAND; diff --git a/src/xrpld/rpc/detail/RPCHelpers.cpp b/src/xrpld/rpc/detail/RPCHelpers.cpp index 385bc7f4af..832ff778e1 100644 --- a/src/xrpld/rpc/detail/RPCHelpers.cpp +++ b/src/xrpld/rpc/detail/RPCHelpers.cpp @@ -24,9 +24,13 @@ getStartHint(std::shared_ptr const& sle, AccountID const& accountID) if (sle->getType() == ltRIPPLE_STATE) { if (sle->getFieldAmount(sfLowLimit).getIssuer() == accountID) + { return sle->getFieldU64(sfLowNode); - else if (sle->getFieldAmount(sfHighLimit).getIssuer() == accountID) + } + if (sle->getFieldAmount(sfHighLimit).getIssuer() == accountID) + { return sle->getFieldU64(sfHighNode); + } } if (!sle->isFieldPresent(sfOwnerNode)) @@ -46,7 +50,7 @@ isRelatedToAccount( return (sle->getFieldAmount(sfLowLimit).getIssuer() == accountID) || (sle->getFieldAmount(sfHighLimit).getIssuer() == accountID); } - else if (sle->isFieldPresent(sfAccount)) + if (sle->isFieldPresent(sfAccount)) { // If there's an sfAccount present, also test the sfDestination, if // present. This will match objects such as Escrows (ltESCROW), Payment @@ -57,12 +61,12 @@ isRelatedToAccount( return sle->getAccountID(sfAccount) == accountID || (sle->isFieldPresent(sfDestination) && sle->getAccountID(sfDestination) == accountID); } - else if (sle->getType() == ltSIGNER_LIST) + if (sle->getType() == ltSIGNER_LIST) { Keylet const accountSignerList = keylet::signers(accountID); return sle->key() == accountSignerList.key; } - else if (sle->getType() == ltNFTOKEN_OFFER) + if (sle->getType() == ltNFTOKEN_OFFER) { // Do not check the sfDestination field. NFToken Offers are NOT added to // the Destination account's directory. @@ -234,9 +238,13 @@ keypairForSignature(Json::Value const& params, Json::Value& error, unsigned int if (!keyType) { if (apiVersion > 1u) + { error = RPC::make_error(rpcBAD_KEY_TYPE); + } else + { error = RPC::invalid_field_error(jss::key_type); + } return {}; } @@ -279,7 +287,9 @@ keypairForSignature(Json::Value const& params, Json::Value& error, unsigned int if (!seed) { if (has_key_type) + { seed = getSeedFromRPC(params, error); + } else { if (!params[jss::secret].isString()) diff --git a/src/xrpld/rpc/detail/RPCLedgerHelpers.cpp b/src/xrpld/rpc/detail/RPCLedgerHelpers.cpp index ddf78425bd..54ff515894 100644 --- a/src/xrpld/rpc/detail/RPCLedgerHelpers.cpp +++ b/src/xrpld/rpc/detail/RPCLedgerHelpers.cpp @@ -55,7 +55,7 @@ ledgerFromIndex( if (index == "closed") return getLedger(ledger, LedgerShortcut::Closed, context); - std::uint32_t iVal; + std::uint32_t iVal = 0; if (!beast::lexicalCastChecked(iVal, index)) return {rpcINVALID_PARAMS, expected_field_message(fieldName, "string or number")}; @@ -78,10 +78,12 @@ ledgerFromRequest(T& ledger, JsonContext const& context) // while `ledger` is still supported, it is deprecated // and therefore shouldn't be mentioned in the error message if (hasLedger) + { return { rpcINVALID_PARAMS, "Exactly one of 'ledger', 'ledger_hash', or " "'ledger_index' can be specified."}; + } return { rpcINVALID_PARAMS, "Exactly one of 'ledger_hash' or " @@ -97,9 +99,11 @@ ledgerFromRequest(T& ledger, JsonContext const& context) return {rpcINVALID_PARAMS, expected_field_message(jss::ledger, "string or number")}; } if (legacyLedger.isString() && legacyLedger.asString().size() == 64) + { return ledgerFromHash(ledger, legacyLedger, context, jss::ledger); - else - return ledgerFromIndex(ledger, legacyLedger, context, jss::ledger); + } + + return ledgerFromIndex(ledger, legacyLedger, context, jss::ledger); } if (hasHash) @@ -182,17 +186,15 @@ ledgerFromSpecifier( { return getLedger(ledger, LedgerShortcut::Validated, context); } - else + + if (shortcut == org::xrpl::rpc::v1::LedgerSpecifier::SHORTCUT_CURRENT || + shortcut == org::xrpl::rpc::v1::LedgerSpecifier::SHORTCUT_UNSPECIFIED) { - if (shortcut == org::xrpl::rpc::v1::LedgerSpecifier::SHORTCUT_CURRENT || - shortcut == org::xrpl::rpc::v1::LedgerSpecifier::SHORTCUT_UNSPECIFIED) - { - return getLedger(ledger, LedgerShortcut::Current, context); - } - else if (shortcut == org::xrpl::rpc::v1::LedgerSpecifier::SHORTCUT_CLOSED) - { - return getLedger(ledger, LedgerShortcut::Closed, context); - } + return getLedger(ledger, LedgerShortcut::Current, context); + } + if (shortcut == org::xrpl::rpc::v1::LedgerSpecifier::SHORTCUT_CLOSED) + { + return getLedger(ledger, LedgerShortcut::Closed, context); } } } diff --git a/src/xrpld/rpc/detail/RPCSub.cpp b/src/xrpld/rpc/detail/RPCSub.cpp index a0e29f68e2..233981001a 100644 --- a/src/xrpld/rpc/detail/RPCSub.cpp +++ b/src/xrpld/rpc/detail/RPCSub.cpp @@ -26,26 +26,37 @@ public: , m_io_context(io_context) , m_jobQueue(jobQueue) , mUrl(strUrl) - , mSSL(false) , mUsername(strUsername) , mPassword(strPassword) - , mSending(false) , j_(logs.journal("RPCSub")) , logs_(logs) { parsedURL pUrl; if (!parseUrl(pUrl, strUrl)) + { Throw("Failed to parse url."); + } else if (pUrl.scheme == "https") + { mSSL = true; + } else if (pUrl.scheme != "http") + { Throw("Only http and https is supported."); + } mSeq = 1; mIp = pUrl.domain; - mPort = (!pUrl.port) ? (mSSL ? 443 : 80) : *pUrl.port; + if (!pUrl.port) + { + mPort = mSSL ? 443 : 80; + } + else + { + mPort = *pUrl.port; + } mPath = pUrl.path; JLOG(j_.info()) << "RPCCall::fromNetwork sub: ip=" << mIp << " port=" << mPort @@ -97,7 +108,7 @@ private: sendThread() { Json::Value jvEvent; - bool bSend; + bool bSend = false; do { @@ -159,14 +170,14 @@ private: std::string mUrl; std::string mIp; std::uint16_t mPort; - bool mSSL; + bool mSSL{false}; std::string mUsername; std::string mPassword; std::string mPath; int mSeq; // Next id to allocate. - bool mSending; // Sending thread is active. + bool mSending{false}; // Sending thread is active. std::deque> mDeque; diff --git a/src/xrpld/rpc/detail/Role.cpp b/src/xrpld/rpc/detail/Role.cpp index e040a55c69..499787cece 100644 --- a/src/xrpld/rpc/detail/Role.cpp +++ b/src/xrpld/rpc/detail/Role.cpp @@ -83,7 +83,7 @@ requestRole( if (ipAllowed(remoteIp.address(), port.secure_gateway_nets_v4, port.secure_gateway_nets_v6)) { - if (user.size()) + if (!user.empty()) return Role::IDENTIFIED; return Role::PROXY; } @@ -137,9 +137,11 @@ extractIpAddrFromField(std::string_view field) { std::size_t const firstNonSpace = ret.find_first_not_of(' '); if (firstNonSpace == std::string_view::npos) + { // We know there's at least one leading space. So if we got // npos, then it must be all spaces. Return empty string_view. return {}; + } ret = ret.substr(firstNonSpace); } @@ -151,9 +153,11 @@ extractIpAddrFromField(std::string_view field) { std::size_t const lastNonSpace = ret.find_last_not_of(" \r\n"); if (lastNonSpace == std::string_view::npos) + { // We know there's at least one leading space. So if we // got npos, then it must be all spaces. return {}; + } ret = ret.substr(0, lastNonSpace + 1); } diff --git a/src/xrpld/rpc/detail/ServerHandler.cpp b/src/xrpld/rpc/detail/ServerHandler.cpp index 5be41a5a23..f5187d6285 100644 --- a/src/xrpld/rpc/detail/ServerHandler.cpp +++ b/src/xrpld/rpc/detail/ServerHandler.cpp @@ -363,9 +363,13 @@ void logDuration(Json::Value const& request, T const& duration, beast::Journal& journal) { using namespace std::chrono_literals; - auto const level = (duration >= 10s) ? journal.error() - : (duration >= 1s) ? journal.warn() - : journal.debug(); + auto const level = [&]() { + if (duration >= 10s) + return journal.error(); + if (duration >= 1s) + return journal.warn(); + return journal.debug(); + }(); JLOG(level) << "RPC request processing duration = " << std::chrono::duration_cast(duration).count() @@ -535,9 +539,13 @@ ServerHandler::processSession( }()); if (beast::rfc2616::is_keep_alive(session->request())) + { session->complete(); + } else + { session->close(true); + } } static Json::Value @@ -561,7 +569,7 @@ ServerHandler::processRequest( Port const& port, std::string const& request, beast::IP::Endpoint const& remoteIPAddress, - Output&& output, + Output const& output, std::shared_ptr coro, std::string_view forwardedFor, std::string_view user) @@ -643,8 +651,10 @@ ServerHandler::processRequest( auto role = Role::FORBID; auto required = Role::FORBID; if (jsonRPC.isMember(jss::method) && jsonRPC[jss::method].isString()) + { required = RPC::roleRequired( apiVersion, app_.config().BETA_RPC_API, jsonRPC[jss::method].asString()); + } if (jsonRPC.isMember(jss::params) && jsonRPC[jss::params].isArray() && jsonRPC[jss::params].size() > 0 && jsonRPC[jss::params][Json::UInt(0)].isObjectOrNull()) @@ -749,8 +759,9 @@ ServerHandler::processRequest( { params = jsonRPC[jss::params]; if (!params) + { params = Json::Value(Json::objectValue); - + } else if (!params.isArray() || params.size() != 1) { usage.charge(Resource::feeMalformedRPC); @@ -909,9 +920,13 @@ ServerHandler::processRequest( if (params.isMember(jss::id)) r[jss::id] = params[jss::id]; if (batch) + { reply.append(std::move(r)); + } else + { reply = std::move(r); + } if (reply.isMember(jss::result) && reply[jss::result].isMember(jss::result)) { @@ -957,9 +972,13 @@ ServerHandler::processRequest( { static int const maxSize = 10000; if (response.size() <= maxSize) + { stream << "Reply: " << response; + } else + { stream << "Reply: " << response.substr(0, maxSize); + } } HTTPReply(httpStatus, response, output, rpcJ); @@ -981,10 +1000,9 @@ ServerHandler::statusResponse(http_request_type const& request) const if (app_.serverOkay(reason)) { msg.result(boost::beast::http::status::ok); - msg.body() = "" + systemName() + - " Test page for rippled

" + systemName() + - " Test

This page shows rippled http(s) " - "connectivity is working.

"; + msg.body() = "Test page for " + systemName() + + "

Test

This page shows " + systemName() + + " http(s) connectivity is working.

"; } else { @@ -1010,10 +1028,14 @@ ServerHandler::Setup::makeContexts() if (p.secure()) { if (p.ssl_key.empty() && p.ssl_cert.empty() && p.ssl_chain.empty()) + { p.context = make_SSLContext(p.ssl_ciphers); + } else + { p.context = make_SSLContextAuthed(p.ssl_key, p.ssl_cert, p.ssl_chain, p.ssl_ciphers); + } } else { @@ -1114,9 +1136,13 @@ parse_Ports(Config const& config, std::ostream& log) // Remove the peer protocol, and if that would // leave the port empty, remove the port as well if (p.erase("peer") && p.empty()) + { it = result.erase(it); + } else + { ++it; + } } } else @@ -1144,15 +1170,22 @@ setup_Client(ServerHandler::Setup& setup) { decltype(setup.ports)::const_iterator iter; for (iter = setup.ports.cbegin(); iter != setup.ports.cend(); ++iter) + { if (iter->protocol.count("http") > 0 || iter->protocol.count("https") > 0) break; + } if (iter == setup.ports.cend()) return; setup.client.secure = iter->protocol.count("https") > 0; - setup.client.ip = beast::IP::is_unspecified(iter->ip) ? - // VFALCO HACK! to make localhost work - (iter->ip.is_v6() ? "::1" : "127.0.0.1") - : iter->ip.to_string(); + if (beast::IP::is_unspecified(iter->ip)) + { + // VFALCO HACK! to make localhost work + setup.client.ip = iter->ip.is_v6() ? "::1" : "127.0.0.1"; + } + else + { + setup.client.ip = iter->ip.to_string(); + } setup.client.port = iter->port; setup.client.user = iter->user; setup.client.password = iter->password; @@ -1176,7 +1209,7 @@ setup_Overlay(ServerHandler::Setup& setup) } ServerHandler::Setup -setup_ServerHandler(Config const& config, std::ostream&& log) +setup_ServerHandler(Config const& config, std::ostream& log) { ServerHandler::Setup setup; setup.ports = parse_Ports(config, log); diff --git a/src/xrpld/rpc/detail/TransactionSign.cpp b/src/xrpld/rpc/detail/TransactionSign.cpp index 8afe3612fe..80c608dbbf 100644 --- a/src/xrpld/rpc/detail/TransactionSign.cpp +++ b/src/xrpld/rpc/detail/TransactionSign.cpp @@ -72,7 +72,7 @@ public: bool validMultiSign() const { - return isMultiSigning() && multiSignPublicKey_ && multiSignature_.size(); + return isMultiSigning() && multiSignPublicKey_ && !multiSignature_.empty(); } // Don't call this method unless isMultiSigning() returns true. @@ -179,11 +179,15 @@ checkPayment( if (tx_json.isMember(jss::Amount)) { if (tx_json[jss::DeliverMax] != tx_json[jss::Amount]) + { return RPC::make_error( rpcINVALID_PARAMS, "Cannot specify differing 'Amount' and 'DeliverMax'"); + } } else + { tx_json[jss::Amount] = tx_json[jss::DeliverMax]; + } tx_json.removeMember(jss::DeliverMax); } @@ -204,12 +208,16 @@ checkPayment( return RPC::invalid_field_error("tx_json.Destination"); if (params.isMember(jss::build_path) && ((doPath == false) || amount.holds())) + { return RPC::make_error( rpcINVALID_PARAMS, "Field 'build_path' not allowed in this context."); + } if (tx_json.isMember(jss::Paths) && params.isMember(jss::build_path)) + { return RPC::make_error( rpcINVALID_PARAMS, "Cannot specify both 'tx_json.Paths' and 'build_path'"); + } std::optional domain; if (tx_json.isMember(sfDomainID.jsonName)) @@ -220,10 +228,8 @@ checkPayment( { return RPC::make_error(rpcDOMAIN_MALFORMED, "Unable to parse 'DomainID'."); } - else - { - domain = num; - } + + domain = num; } if (!tx_json.isMember(jss::Paths) && params.isMember(jss::build_path)) @@ -338,9 +344,13 @@ checkTxJsonFields( if (verify && !config.standalone() && (validatedLedgerAge > Tuning::maxValidatedLedgerAge)) { if (apiVersion == 1) + { ret.first = rpcError(rpcNO_CURRENT); + } else + { ret.first = rpcError(rpcNOT_SYNCED); + } return ret; } @@ -525,8 +535,10 @@ transactionPreProcessImpl( if (verify) { if (!sle) + { // XXX Ignore transactions for accounts not created. return rpcError(rpcSRC_ACT_NOT_FOUND); + } JLOG(j.trace()) << "verify: " << toBase58(calcAccountID(pk)) << " : " << toBase58(srcAddressID); @@ -592,8 +604,10 @@ transactionPreProcessImpl( { // If the target object doesn't exist, make one. if (!parsed.object->isFieldPresent(*signatureTarget)) + { parsed.object->setFieldObject( *signatureTarget, STObject{*signatureTemplate, *signatureTarget}); + } sigObject = &parsed.object->peekFieldObject(*signatureTarget); } sigObject->setFieldVL( @@ -666,8 +680,10 @@ transactionConstructImpl( // Check the signature if that's called for. auto sttxNew = std::make_shared(sit); if (!app.checkSigs()) + { forceValidity( app.getHashRouter(), sttxNew->getTransactionID(), Validity::SigGoodOnly); + } if (checkValidity(app.getHashRouter(), *sttxNew, rules).first != Validity::Valid) { ret.first = RPC::make_error(rpcINTERNAL, "Invalid signature."); @@ -714,7 +730,9 @@ transactionFormatResultImpl(Transaction::pointer tpTrans, unsigned apiVersion) jvResult[jss::hash] = to_string(tpTrans->getID()); } else + { jvResult[jss::tx_json] = tpTrans->getJson(JsonOptions::none); + } RPC::insertDeliverMax( jvResult[jss::tx_json], tpTrans->getSTransaction()->getTxnType(), apiVersion); @@ -880,9 +898,11 @@ checkFee( { mult = request[jss::fee_mult_max].asInt(); if (mult < 0) + { return RPC::make_error( rpcINVALID_PARAMS, RPC::expected_field_message(jss::fee_mult_max, "a positive integer")); + } } else { @@ -896,9 +916,11 @@ checkFee( { div = request[jss::fee_div_max].asInt(); if (div <= 0) + { return RPC::make_error( rpcINVALID_PARAMS, RPC::expected_field_message(jss::fee_div_max, "a positive integer")); + } } else { @@ -1024,8 +1046,10 @@ checkMultiSignFields(Json::Value const& jvRequest) // Account. if (!jvRequest.isMember(jss::signature_target) && !tx_json[sfSigningPubKey.getJsonName()].asString().empty()) + { return RPC::make_error( rpcINVALID_PARAMS, "When multi-signing 'tx_json.SigningPubKey' must be empty."); + } return Json::Value(); } diff --git a/src/xrpld/rpc/handlers/AMMInfo.cpp b/src/xrpld/rpc/handlers/AMMInfo.cpp index 4e91c9c308..42e44f004d 100644 --- a/src/xrpld/rpc/handlers/AMMInfo.cpp +++ b/src/xrpld/rpc/handlers/AMMInfo.cpp @@ -2,6 +2,7 @@ #include #include +#include #include #include #include @@ -74,17 +75,25 @@ doAMMInfo(RPC::JsonContext& context) if (params.isMember(jss::asset)) { if (auto const i = getIssue(params[jss::asset], context.j)) + { issue1 = *i; + } else + { return Unexpected(i.error()); + } } if (params.isMember(jss::asset2)) { if (auto const i = getIssue(params[jss::asset2], context.j)) + { issue2 = *i; + } else + { return Unexpected(i.error()); + } } if (params.isMember(jss::amm_account)) @@ -175,7 +184,7 @@ doAMMInfo(RPC::JsonContext& context) "xrpl::doAMMInfo : auction slot is set"); if (amm->isFieldPresent(sfAuctionSlot)) { - auto const& auctionSlot = static_cast(amm->peekAtField(sfAuctionSlot)); + auto const& auctionSlot = safe_downcast(amm->peekAtField(sfAuctionSlot)); if (auctionSlot.isFieldPresent(sfAccount)) { Json::Value auction; @@ -203,11 +212,15 @@ doAMMInfo(RPC::JsonContext& context) } if (!isXRP(asset1Balance)) + { ammResult[jss::asset_frozen] = isFrozen(*ledger, ammAccountID, issue1.currency, issue1.account); + } if (!isXRP(asset2Balance)) + { ammResult[jss::asset2_frozen] = isFrozen(*ledger, ammAccountID, issue2.currency, issue2.account); + } result[jss::amm] = std::move(ammResult); if (!result.isMember(jss::ledger_index) && !result.isMember(jss::ledger_hash)) diff --git a/src/xrpld/rpc/handlers/AccountChannels.cpp b/src/xrpld/rpc/handlers/AccountChannels.cpp index 062a2b69a0..d2c3d4546d 100644 --- a/src/xrpld/rpc/handlers/AccountChannels.cpp +++ b/src/xrpld/rpc/handlers/AccountChannels.cpp @@ -81,7 +81,7 @@ doAccountChannels(RPC::JsonContext& context) if (!strDst.empty() && !raDstAccount) return rpcError(rpcACT_MALFORMED); - unsigned int limit; + unsigned int limit = 0; if (auto err = readLimitField(limit, RPC::Tuning::accountChannels, context)) return *err; diff --git a/src/xrpld/rpc/handlers/AccountInfo.cpp b/src/xrpld/rpc/handlers/AccountInfo.cpp index e88da097f7..f144c934ec 100644 --- a/src/xrpld/rpc/handlers/AccountInfo.cpp +++ b/src/xrpld/rpc/handlers/AccountInfo.cpp @@ -84,7 +84,9 @@ doAccountInfo(RPC::JsonContext& context) strIdent = params[jss::ident].asString(); } else + { return RPC::missing_field_error(jss::account); + } std::shared_ptr ledger; auto result = RPC::lookupLedger(ledger, context); @@ -150,12 +152,16 @@ doAccountInfo(RPC::JsonContext& context) acctFlags[lsf.first.data()] = sleAccepted->isFlag(lsf.second); if (ledger->rules().enabled(featureClawback)) + { acctFlags[allowTrustLineClawbackFlag.first.data()] = sleAccepted->isFlag(allowTrustLineClawbackFlag.second); + } if (ledger->rules().enabled(featureTokenEscrow)) + { acctFlags[allowTrustLineLockingFlag.first.data()] = sleAccepted->isFlag(allowTrustLineLockingFlag.second); + } result[jss::account_flags] = std::move(acctFlags); @@ -300,7 +306,9 @@ doAccountInfo(RPC::JsonContext& context) jvQueueData[jss::max_spend_drops_total] = to_string(totalSpend); } else + { jvQueueData[jss::txn_count] = 0u; + } result[jss::queue_data] = std::move(jvQueueData); } diff --git a/src/xrpld/rpc/handlers/AccountLines.cpp b/src/xrpld/rpc/handlers/AccountLines.cpp index bb61e6f6c0..952141fb8d 100644 --- a/src/xrpld/rpc/handlers/AccountLines.cpp +++ b/src/xrpld/rpc/handlers/AccountLines.cpp @@ -98,7 +98,7 @@ doAccountLines(RPC::JsonContext& context) return result; } - unsigned int limit; + unsigned int limit = 0; if (auto err = readLimitField(limit, RPC::Tuning::accountLines, context)) return *err; @@ -191,9 +191,13 @@ doAccountLines(RPC::JsonContext& context) if (visitData.ignoreDefault) { if (sleCur->getFieldAmount(sfLowLimit).getIssuer() == visitData.accountID) + { ignore = !(sleCur->getFieldU32(sfFlags) & lsfLowReserve); + } else + { ignore = !(sleCur->getFieldU32(sfFlags) & lsfHighReserve); + } } if (!ignore && count <= limit) diff --git a/src/xrpld/rpc/handlers/AccountObjects.cpp b/src/xrpld/rpc/handlers/AccountObjects.cpp index e01822e901..cd67391da8 100644 --- a/src/xrpld/rpc/handlers/AccountObjects.cpp +++ b/src/xrpld/rpc/handlers/AccountObjects.cpp @@ -53,7 +53,7 @@ doAccountNFTs(RPC::JsonContext& context) if (!ledger->exists(keylet::account(accountID))) return rpcError(rpcACT_NOT_FOUND); - unsigned int limit; + unsigned int limit = 0; if (auto err = readLimitField(limit, RPC::Tuning::accountNFTokens, context)) return *err; @@ -144,9 +144,13 @@ doAccountNFTs(RPC::JsonContext& context) } if (auto npm = (*cp)[~sfNextPageMin]) + { cp = ledger->read(Keylet(ltNFTOKEN_PAGE, *npm)); + } else + { cp = nullptr; + } } if (markerSet && !markerFound) @@ -229,9 +233,13 @@ getAccountObjects( jvObjects.append(cp->getJson(JsonOptions::none)); auto const npm = (*cp)[~sfNextPageMin]; if (npm) + { cp = ledger.read(Keylet(ltNFTOKEN_PAGE, *npm)); + } else + { cp = nullptr; + } if (--mlimit == 0) { @@ -429,13 +437,13 @@ doAccountObjects(RPC::JsonContext& context) rpcStatus.inject(result); return result; } - else if (type != ltANY) + if (type != ltANY) { typeFilter = std::vector({type}); } } - unsigned int limit; + unsigned int limit = 0; if (auto err = readLimitField(limit, RPC::Tuning::accountObjects, context)) return *err; diff --git a/src/xrpld/rpc/handlers/AccountOffers.cpp b/src/xrpld/rpc/handlers/AccountOffers.cpp index 130decad04..842cac71eb 100644 --- a/src/xrpld/rpc/handlers/AccountOffers.cpp +++ b/src/xrpld/rpc/handlers/AccountOffers.cpp @@ -63,7 +63,7 @@ doAccountOffers(RPC::JsonContext& context) if (!ledger->exists(keylet::account(accountID))) return rpcError(rpcACT_NOT_FOUND); - unsigned int limit; + unsigned int limit = 0; if (auto err = readLimitField(limit, RPC::Tuning::accountOffers, context)) return *err; diff --git a/src/xrpld/rpc/handlers/AccountTx.cpp b/src/xrpld/rpc/handlers/AccountTx.cpp index a6dcbb7b9d..d7bffe5d95 100644 --- a/src/xrpld/rpc/handlers/AccountTx.cpp +++ b/src/xrpld/rpc/handlers/AccountTx.cpp @@ -59,7 +59,7 @@ parseLedgerArgs(RPC::Context& context, Json::Value const& params) return LedgerRange{min, max}; } - else if (params.isMember(jss::ledger_hash)) + if (params.isMember(jss::ledger_hash)) { auto& hashValue = params[jss::ledger_hash]; if (!hashValue.isString()) @@ -78,21 +78,29 @@ parseLedgerArgs(RPC::Context& context, Json::Value const& params) } return hash; } - else if (params.isMember(jss::ledger_index)) + if (params.isMember(jss::ledger_index)) { LedgerSpecifier ledger; if (params[jss::ledger_index].isNumeric()) + { ledger = params[jss::ledger_index].asUInt(); + } else { std::string ledgerStr = params[jss::ledger_index].asString(); if (ledgerStr == "current" || ledgerStr.empty()) + { ledger = LedgerShortcut::Current; + } else if (ledgerStr == "closed") + { ledger = LedgerShortcut::Closed; + } else if (ledgerStr == "validated") + { ledger = LedgerShortcut::Validated; + } else { RPC::Status status{rpcINVALID_PARAMS, "ledger_index string malformed"}; @@ -108,8 +116,8 @@ parseLedgerArgs(RPC::Context& context, Json::Value const& params) std::variant getLedgerRange(RPC::Context& context, std::optional const& ledgerSpecifier) { - std::uint32_t uValidatedMin; - std::uint32_t uValidatedMax; + std::uint32_t uValidatedMin = 0; + std::uint32_t uValidatedMax = 0; bool bValidated = context.ledgerMaster.getValidatedRange(uValidatedMin, uValidatedMax); if (!bValidated) @@ -298,7 +306,9 @@ populateJsonResponse( jvObj[jss::close_time_iso] = to_string_iso(*closeTime); } else + { jvObj[json_tx] = txn->getJson(JsonOptions::include_date); + } auto const& sttx = txn->getSTransaction(); RPC::insertDeliverMax(jvObj[json_tx], sttx->getTxnType(), context.apiVersion); @@ -404,10 +414,8 @@ doAccountTxJson(RPC::JsonContext& context) { return *jv; } - else - { - args.ledger = std::get>(parseRes); - } + + args.ledger = std::get>(parseRes); if (params.isMember(jss::marker)) { diff --git a/src/xrpld/rpc/handlers/BlackList.cpp b/src/xrpld/rpc/handlers/BlackList.cpp index 102041b8d0..86abe53686 100644 --- a/src/xrpld/rpc/handlers/BlackList.cpp +++ b/src/xrpld/rpc/handlers/BlackList.cpp @@ -11,9 +11,11 @@ doBlackList(RPC::JsonContext& context) { auto& rm = context.app.getResourceManager(); if (context.params.isMember(jss::threshold)) + { return rm.getJson(context.params[jss::threshold].asInt()); - else - return rm.getJson(); + } + + return rm.getJson(); } } // namespace xrpl diff --git a/src/xrpld/rpc/handlers/BookOffers.cpp b/src/xrpld/rpc/handlers/BookOffers.cpp index 070d7988de..ec53473821 100644 --- a/src/xrpld/rpc/handlers/BookOffers.cpp +++ b/src/xrpld/rpc/handlers/BookOffers.cpp @@ -83,12 +83,16 @@ doBookOffers(RPC::JsonContext& context) return RPC::expected_field_error("taker_pays.issuer", "string"); if (!to_issuer(pay_issuer, taker_pays[jss::issuer].asString())) + { return RPC::make_error( rpcSRC_ISR_MALFORMED, "Invalid field 'taker_pays.issuer', bad issuer."); + } if (pay_issuer == noAccount()) + { return RPC::make_error( rpcSRC_ISR_MALFORMED, "Invalid field 'taker_pays.issuer', bad issuer account one."); + } } else { @@ -96,14 +100,18 @@ doBookOffers(RPC::JsonContext& context) } if (isXRP(pay_currency) && !isXRP(pay_issuer)) + { return RPC::make_error( rpcSRC_ISR_MALFORMED, "Unneeded field 'taker_pays.issuer' for " "XRP currency specification."); + } if (!isXRP(pay_currency) && isXRP(pay_issuer)) + { return RPC::make_error( rpcSRC_ISR_MALFORMED, "Invalid field 'taker_pays.issuer', expected non-XRP issuer."); + } AccountID get_issuer; @@ -113,12 +121,16 @@ doBookOffers(RPC::JsonContext& context) return RPC::expected_field_error("taker_gets.issuer", "string"); if (!to_issuer(get_issuer, taker_gets[jss::issuer].asString())) + { return RPC::make_error( rpcDST_ISR_MALFORMED, "Invalid field 'taker_gets.issuer', bad issuer."); + } if (get_issuer == noAccount()) + { return RPC::make_error( rpcDST_ISR_MALFORMED, "Invalid field 'taker_gets.issuer', bad issuer account one."); + } } else { @@ -126,14 +138,18 @@ doBookOffers(RPC::JsonContext& context) } if (isXRP(get_currency) && !isXRP(get_issuer)) + { return RPC::make_error( rpcDST_ISR_MALFORMED, "Unneeded field 'taker_gets.issuer' for " "XRP currency specification."); + } if (!isXRP(get_currency) && isXRP(get_issuer)) + { return RPC::make_error( rpcDST_ISR_MALFORMED, "Invalid field 'taker_gets.issuer', expected non-XRP issuer."); + } std::optional takerID; if (context.params.isMember(jss::taker)) @@ -155,10 +171,8 @@ doBookOffers(RPC::JsonContext& context) { return RPC::make_error(rpcDOMAIN_MALFORMED, "Unable to parse domain."); } - else - { - domain = num; - } + + domain = num; } if (pay_currency == get_currency && pay_issuer == get_issuer) @@ -167,7 +181,7 @@ doBookOffers(RPC::JsonContext& context) return RPC::make_error(rpcBAD_MARKET); } - unsigned int limit; + unsigned int limit = 0; if (auto err = readLimitField(limit, RPC::Tuning::bookOffers, context)) return *err; diff --git a/src/xrpld/rpc/handlers/Connect.cpp b/src/xrpld/rpc/handlers/Connect.cpp index c76ebed546..292cd50cbb 100644 --- a/src/xrpld/rpc/handlers/Connect.cpp +++ b/src/xrpld/rpc/handlers/Connect.cpp @@ -33,12 +33,16 @@ doConnect(RPC::JsonContext& context) return rpcError(rpcINVALID_PARAMS); } - int iPort; + int iPort = 0; if (context.params.isMember(jss::port)) + { iPort = context.params[jss::port].asInt(); + } else + { iPort = DEFAULT_PEER_PORT; + } auto const ip_str = context.params[jss::ip].asString(); auto ip = beast::IP::Endpoint::from_string(ip_str); diff --git a/src/xrpld/rpc/handlers/DepositAuthorized.cpp b/src/xrpld/rpc/handlers/DepositAuthorized.cpp index 7bad908c51..7ab7e30c8a 100644 --- a/src/xrpld/rpc/handlers/DepositAuthorized.cpp +++ b/src/xrpld/rpc/handlers/DepositAuthorized.cpp @@ -27,8 +27,10 @@ doDepositAuthorized(RPC::JsonContext& context) if (!params.isMember(jss::source_account)) return RPC::missing_field_error(jss::source_account); if (!params[jss::source_account].isString()) + { return RPC::make_error( rpcINVALID_PARAMS, RPC::expected_field_message(jss::source_account, "a string")); + } auto srcID = parseBase58(params[jss::source_account].asString()); if (!srcID) @@ -39,8 +41,10 @@ doDepositAuthorized(RPC::JsonContext& context) if (!params.isMember(jss::destination_account)) return RPC::missing_field_error(jss::destination_account); if (!params[jss::destination_account].isString()) + { return RPC::make_error( rpcINVALID_PARAMS, RPC::expected_field_message(jss::destination_account, "a string")); + } auto dstID = parseBase58(params[jss::destination_account].asString()); if (!dstID) @@ -84,7 +88,7 @@ doDepositAuthorized(RPC::JsonContext& context) RPC::expected_field_message( jss::credentials, "is non-empty array of CredentialID(hash256)")); } - else if (creds.size() > maxCredentialsArraySize) + if (creds.size() > maxCredentialsArraySize) { return RPC::make_error( rpcINVALID_PARAMS, RPC::expected_field_message(jss::credentials, "array too long")); @@ -151,8 +155,10 @@ doDepositAuthorized(RPC::JsonContext& context) // not set, then the deposit should be fine. bool depositAuthorized = true; if (reqAuth) + { depositAuthorized = ledger->exists(keylet::depositPreauth(dstAcct, srcAcct)) || (credentialsPresent && ledger->exists(keylet::depositPreauth(dstAcct, sorted))); + } result[jss::source_account] = params[jss::source_account].asString(); result[jss::destination_account] = params[jss::destination_account].asString(); diff --git a/src/xrpld/rpc/handlers/Feature1.cpp b/src/xrpld/rpc/handlers/Feature1.cpp index bd1e501506..24ff0d62b8 100644 --- a/src/xrpld/rpc/handlers/Feature1.cpp +++ b/src/xrpld/rpc/handlers/Feature1.cpp @@ -61,9 +61,13 @@ doFeature(RPC::JsonContext& context) return rpcError(rpcNO_PERMISSION); if (context.params[jss::vetoed].asBool()) + { table.veto(feature); + } else + { table.unVeto(feature); + } } Json::Value jvReply = table.getJson(feature, isAdmin); diff --git a/src/xrpld/rpc/handlers/GatewayBalances.cpp b/src/xrpld/rpc/handlers/GatewayBalances.cpp index a2176ab388..60e031c812 100644 --- a/src/xrpld/rpc/handlers/GatewayBalances.cpp +++ b/src/xrpld/rpc/handlers/GatewayBalances.cpp @@ -173,7 +173,7 @@ doGatewayBalances(RPC::JsonContext& context) // A positive balance means the cold wallet has an asset // (unusual) - if (hotWallets.count(peer) > 0) + if (hotWallets.contains(peer)) { // This is a specified hot wallet hotBalances[peer].push_back(-rs->getBalance()); diff --git a/src/xrpld/rpc/handlers/GetAggregatePrice.cpp b/src/xrpld/rpc/handlers/GetAggregatePrice.cpp index eaeff767cd..4fc8e360fc 100644 --- a/src/xrpld/rpc/handlers/GetAggregatePrice.cpp +++ b/src/xrpld/rpc/handlers/GetAggregatePrice.cpp @@ -3,6 +3,7 @@ #include #include +#include #include #include #include @@ -25,7 +26,7 @@ static void iteratePriceData( RPC::JsonContext& context, std::shared_ptr const& sle, - std::function&& f) + std::function const& f) { using Meta = std::shared_ptr; constexpr std::uint8_t maxHistory = 3; @@ -87,8 +88,8 @@ iteratePriceData( if (isNew && history == 1) return; - oracle = isNew ? &static_cast(node.peekAtField(sfNewFields)) - : &static_cast(node.peekAtField(sfFinalFields)); + oracle = isNew ? &safe_downcast(node.peekAtField(sfNewFields)) + : &safe_downcast(node.peekAtField(sfFinalFields)); break; } } @@ -148,7 +149,7 @@ doGetAggregatePrice(RPC::JsonContext& context) // support positive int, uint, and a number represented as a string auto validUInt = [](Json::Value const& params, Json::StaticString const& field) { auto const& jv = params[field]; - std::uint32_t v; + std::uint32_t v = 0; return jv.isUInt() || (jv.isInt() && jv.asInt() >= 0) || (jv.isString() && beast::lexicalCastChecked(v, jv.asString())); }; diff --git a/src/xrpld/rpc/handlers/LedgerData.cpp b/src/xrpld/rpc/handlers/LedgerData.cpp index 308f4b4436..733d1c99c6 100644 --- a/src/xrpld/rpc/handlers/LedgerData.cpp +++ b/src/xrpld/rpc/handlers/LedgerData.cpp @@ -140,14 +140,14 @@ doLedgerDataGrpc(RPC::GRPCContext& con { startKey = *key; } - else if (request.marker().size() != 0) + else if (!request.marker().empty()) { grpc::Status errorStatus{grpc::StatusCode::INVALID_ARGUMENT, "marker malformed"}; return {response, errorStatus}; } auto e = ledger->sles.end(); - if (request.end_marker().size() != 0) + if (!request.end_marker().empty()) { auto const key = uint256::fromVoidChecked(request.end_marker()); diff --git a/src/xrpld/rpc/handlers/LedgerEntry.cpp b/src/xrpld/rpc/handlers/LedgerEntry.cpp index e4e8f52fd7..8e3b7f214d 100644 --- a/src/xrpld/rpc/handlers/LedgerEntry.cpp +++ b/src/xrpld/rpc/handlers/LedgerEntry.cpp @@ -70,9 +70,11 @@ parseIndex(Json::Value const& params, Json::StaticString const fieldName, unsign if (index == jss::nunl) return keylet::negativeUNL().key; if (index == jss::hashes) + { // Note this only finds the "short" skip list. Use "hashes":index to // get the long list. return keylet::skip().key; + } } return parseObjectID(params, fieldName, "hex string"); } @@ -532,8 +534,10 @@ parseMPTokenIssuance( { auto const mptIssuanceID = LedgerEntryHelpers::parse(params); if (!mptIssuanceID) + { return LedgerEntryHelpers::invalidFieldError( "malformedMPTokenIssuance", fieldName, "Hash192"); + } return keylet::mptIssuance(*mptIssuanceID).key; } @@ -901,8 +905,8 @@ doLedgerEntry(RPC::JsonContext& context) // this exception return an invalidParam error. return RPC::make_error(rpcINVALID_PARAMS); } - else - throw; + + throw; } // Return the computed index regardless of whether the node exists. diff --git a/src/xrpld/rpc/handlers/LedgerHandler.cpp b/src/xrpld/rpc/handlers/LedgerHandler.cpp index 6d188f8520..43c9780bb0 100644 --- a/src/xrpld/rpc/handlers/LedgerHandler.cpp +++ b/src/xrpld/rpc/handlers/LedgerHandler.cpp @@ -215,11 +215,17 @@ doLedgerGrpc(RPC::GRPCContext& context) obj->set_data(inDesired->data(), inDesired->size()); } if (inBase && inDesired) + { obj->set_mod_type(org::xrpl::rpc::v1::RawLedgerObject::MODIFIED); + } else if (inBase && !inDesired) + { obj->set_mod_type(org::xrpl::rpc::v1::RawLedgerObject::DELETED); + } else + { obj->set_mod_type(org::xrpl::rpc::v1::RawLedgerObject::CREATED); + } auto const blob = inDesired ? inDesired->slice() : inBase->slice(); auto const objectType = static_cast(blob[1] << 8 | blob[2]); diff --git a/src/xrpld/rpc/handlers/LogLevel.cpp b/src/xrpld/rpc/handlers/LogLevel.cpp index 2596cd6c70..2bc1beb7d4 100644 --- a/src/xrpld/rpc/handlers/LogLevel.cpp +++ b/src/xrpld/rpc/handlers/LogLevel.cpp @@ -52,9 +52,13 @@ doLogLevel(RPC::JsonContext& context) std::string partition(context.params[jss::partition].asString()); if (boost::iequals(partition, "base")) + { context.app.logs().threshold(severity); + } else + { context.app.logs().get(partition).threshold(severity); + } return Json::objectValue; } diff --git a/src/xrpld/rpc/handlers/NFTOffers.cpp b/src/xrpld/rpc/handlers/NFTOffers.cpp index 51f75bfe4c..3af7c28f9e 100644 --- a/src/xrpld/rpc/handlers/NFTOffers.cpp +++ b/src/xrpld/rpc/handlers/NFTOffers.cpp @@ -44,7 +44,7 @@ appendNftOfferJson( static Json::Value enumerateNFTOffers(RPC::JsonContext& context, uint256 const& nftId, Keylet const& directory) { - unsigned int limit; + unsigned int limit = 0; if (auto err = readLimitField(limit, RPC::Tuning::nftOffers, context)) return *err; diff --git a/src/xrpld/rpc/handlers/NoRippleCheck.cpp b/src/xrpld/rpc/handlers/NoRippleCheck.cpp index e17a437efc..f21a67a31d 100644 --- a/src/xrpld/rpc/handlers/NoRippleCheck.cpp +++ b/src/xrpld/rpc/handlers/NoRippleCheck.cpp @@ -55,12 +55,16 @@ doNoRippleCheck(RPC::JsonContext& context) { std::string const role = params["role"].asString(); if (role == "gateway") + { roleGateway = true; + } else if (role != "user") + { return RPC::invalid_field_error("role"); + } } - unsigned int limit; + unsigned int limit = 0; if (auto err = readLimitField(limit, RPC::Tuning::noRippleCheck, context)) return *err; diff --git a/src/xrpld/rpc/handlers/PayChanClaim.cpp b/src/xrpld/rpc/handlers/PayChanClaim.cpp index 44312ee336..b24a241147 100644 --- a/src/xrpld/rpc/handlers/PayChanClaim.cpp +++ b/src/xrpld/rpc/handlers/PayChanClaim.cpp @@ -29,8 +29,10 @@ doChannelAuthorize(RPC::JsonContext& context) auto const& params(context.params); for (auto const& p : {jss::channel_id, jss::amount}) + { if (!params.isMember(p)) return RPC::missing_field_error(p); + } // Compatibility if a key type isn't specified. If it is, the // keypairForSignature code will validate parameters and return @@ -92,8 +94,10 @@ doChannelVerify(RPC::JsonContext& context) { auto const& params(context.params); for (auto const& p : {jss::public_key, jss::channel_id, jss::amount, jss::signature}) + { if (!params.isMember(p)) return RPC::missing_field_error(p); + } std::optional pk; { @@ -125,7 +129,7 @@ doChannelVerify(RPC::JsonContext& context) std::uint64_t const drops = *optDrops; auto sig = strUnHex(params[jss::signature].asString()); - if (!sig || !sig->size()) + if (!sig || sig->empty()) return rpcError(rpcINVALID_PARAMS); Serializer msg; diff --git a/src/xrpld/rpc/handlers/Peers.cpp b/src/xrpld/rpc/handlers/Peers.cpp index 1be136a5f8..b21efc01fb 100644 --- a/src/xrpld/rpc/handlers/Peers.cpp +++ b/src/xrpld/rpc/handlers/Peers.cpp @@ -27,9 +27,13 @@ doPeers(RPC::JsonContext& context) auto const s = p[jss::track].asString(); if (s == "diverged") + { p["sanity"] = "insane"; + } else if (s == "unknown") + { p["sanity"] = "unknown"; + } } } } @@ -53,8 +57,10 @@ doPeers(RPC::JsonContext& context) json[jss::fee] = static_cast(node.getLoadFee()) / ref; if (node.getReportTime() != NetClock::time_point{}) + { json[jss::age] = (node.getReportTime() >= now) ? 0 : (now - node.getReportTime()).count(); + } }); return jvResult; diff --git a/src/xrpld/rpc/handlers/Ping.cpp b/src/xrpld/rpc/handlers/Ping.cpp index cc3c558cd9..4e9b18c4c9 100644 --- a/src/xrpld/rpc/handlers/Ping.cpp +++ b/src/xrpld/rpc/handlers/Ping.cpp @@ -22,7 +22,7 @@ doPing(RPC::JsonContext& context) case Role::IDENTIFIED: ret[jss::role] = "identified"; ret[jss::username] = std::string{context.headers.user}; - if (context.headers.forwardedFor.size()) + if (!context.headers.forwardedFor.empty()) ret[jss::ip] = std::string{context.headers.forwardedFor}; break; case Role::PROXY: diff --git a/src/xrpld/rpc/handlers/ServerDefinitions.cpp b/src/xrpld/rpc/handlers/ServerDefinitions.cpp index e153065ea9..24abd69a02 100644 --- a/src/xrpld/rpc/handlers/ServerDefinitions.cpp +++ b/src/xrpld/rpc/handlers/ServerDefinitions.cpp @@ -63,9 +63,11 @@ ServerDefinitions::translate(std::string const& inp) { if (contains("512") || contains("384") || contains("256") || contains("192") || contains("160") || contains("128")) + { return replace("UINT", "Hash"); - else - return replace("UINT", "UInt"); + } + + return replace("UINT", "UInt"); } static std::unordered_map const replacements{ @@ -102,7 +104,9 @@ ServerDefinitions::translate(std::string const& inp) out += token; } else + { out += token; + } if (pos == inpToProcess.size()) break; inpToProcess = inpToProcess.substr(pos + 1); @@ -217,7 +221,7 @@ ServerDefinitions::ServerDefinitions() : defs_{Json::objectValue} for (auto const& [code, field] : xrpl::SField::getKnownCodeToField()) { - if (field->fieldName == "") + if (field->fieldName.empty()) continue; Json::Value innerObj = Json::objectValue; diff --git a/src/xrpld/rpc/handlers/Simulate.cpp b/src/xrpld/rpc/handlers/Simulate.cpp index 5703597beb..1d46f72425 100644 --- a/src/xrpld/rpc/handlers/Simulate.cpp +++ b/src/xrpld/rpc/handlers/Simulate.cpp @@ -126,7 +126,7 @@ autofillTx(Json::Value& tx_json, RPC::JsonContext& context) } if (auto error = autofillSignature(tx_json)) - return *error; + return error; if (!tx_json.isMember(jss::Sequence)) { diff --git a/src/xrpld/rpc/handlers/Submit.cpp b/src/xrpld/rpc/handlers/Submit.cpp index b8cca3670a..cac7259a00 100644 --- a/src/xrpld/rpc/handlers/Submit.cpp +++ b/src/xrpld/rpc/handlers/Submit.cpp @@ -55,7 +55,7 @@ doSubmit(RPC::JsonContext& context) auto ret = strUnHex(context.params[jss::tx_blob].asString()); - if (!ret || !ret->size()) + if (!ret || ret->empty()) return rpcError(rpcINVALID_PARAMS); SerialIter sitTrans(makeSlice(*ret)); @@ -76,8 +76,10 @@ doSubmit(RPC::JsonContext& context) { if (!context.app.checkSigs()) + { forceValidity( context.app.getHashRouter(), stTx->getTransactionID(), Validity::SigGoodOnly); + } auto [validity, reason] = checkValidity( context.app.getHashRouter(), *stTx, context.ledgerMaster.getCurrentLedger()->rules()); if (validity != Validity::Valid) diff --git a/src/xrpld/rpc/handlers/Subscribe.cpp b/src/xrpld/rpc/handlers/Subscribe.cpp index 8fbf5a917e..6e8d9dbaa3 100644 --- a/src/xrpld/rpc/handlers/Subscribe.cpp +++ b/src/xrpld/rpc/handlers/Subscribe.cpp @@ -281,10 +281,8 @@ doSubscribe(RPC::JsonContext& context) { return rpcError(rpcDOMAIN_MALFORMED); } - else - { - book.domain = domain; - } + + book.domain = domain; } if (!isConsistent(book)) diff --git a/src/xrpld/rpc/handlers/TransactionEntry.cpp b/src/xrpld/rpc/handlers/TransactionEntry.cpp index 8cd1120aad..36f53130fa 100644 --- a/src/xrpld/rpc/handlers/TransactionEntry.cpp +++ b/src/xrpld/rpc/handlers/TransactionEntry.cpp @@ -59,8 +59,10 @@ doTransactionEntry(RPC::JsonContext& context) jvResult[jss::hash] = to_string(sttx->getTransactionID()); if (!lpLedger->open()) + { jvResult[jss::ledger_hash] = to_string(context.ledgerMaster.getHashBySeq(lpLedger->seq())); + } bool const validated = context.ledgerMaster.isValidated(*lpLedger); @@ -73,7 +75,9 @@ doTransactionEntry(RPC::JsonContext& context) } } else + { jvResult[jss::tx_json] = sttx->getJson(JsonOptions::none); + } RPC::insertDeliverMax(jvResult[jss::tx_json], sttx->getTxnType(), context.apiVersion); diff --git a/src/xrpld/rpc/handlers/Tx.cpp b/src/xrpld/rpc/handlers/Tx.cpp index 84f2a6c618..482b7e3bf1 100644 --- a/src/xrpld/rpc/handlers/Tx.cpp +++ b/src/xrpld/rpc/handlers/Tx.cpp @@ -42,7 +42,7 @@ struct TxResult std::optional ctid; std::optional closeTime; std::optional ledgerHash; - TxSearched searchedAll; + TxSearched searchedAll = TxSearched::unknown; }; struct TxArgs @@ -153,7 +153,7 @@ doTxHelp(RPC::Context& context, TxArgs args) uint32_t netID = context.app.getNetworkIDService().getNetworkID(); if (txnIdx <= 0xFFFFU && netID < 0xFFFFU && lgrSeq < 0x0FFF'FFFFUL) - result.ctid = RPC::encodeCTID(lgrSeq, (uint32_t)txnIdx, (uint32_t)netID); + result.ctid = RPC::encodeCTID(lgrSeq, txnIdx, netID); } } @@ -192,7 +192,9 @@ populateJsonResponse( constexpr auto optionsJson = JsonOptions::include_date | JsonOptions::disable_API_prior_V2; if (args.binary) + { response[jss::tx_blob] = result.txn->getJson(optionsJson, true); + } else { response[jss::tx_json] = result.txn->getJson(optionsJson); @@ -258,8 +260,10 @@ doTxJson(RPC::JsonContext& context) TxArgs args; if (context.params.isMember(jss::transaction) && context.params.isMember(jss::ctid)) + { // specifying both is ambiguous return rpcError(rpcINVALID_PARAMS); + } if (context.params.isMember(jss::transaction)) { @@ -286,7 +290,9 @@ doTxJson(RPC::JsonContext& context) args.ctid = {lgr_seq, txn_idx}; } else + { return rpcError(rpcINVALID_PARAMS); + } args.binary = context.params.isMember(jss::binary) && context.params[jss::binary].asBool(); diff --git a/src/xrpld/rpc/handlers/Unsubscribe.cpp b/src/xrpld/rpc/handlers/Unsubscribe.cpp index 15da0d1d6a..824d57203c 100644 --- a/src/xrpld/rpc/handlers/Unsubscribe.cpp +++ b/src/xrpld/rpc/handlers/Unsubscribe.cpp @@ -165,8 +165,7 @@ doUnsubscribe(RPC::JsonContext& context) return rpcError(rpcSRC_CUR_MALFORMED); } // Parse optional issuer. - else if ( - ((taker_pays.isMember(jss::issuer)) && + if (((taker_pays.isMember(jss::issuer)) && (!taker_pays[jss::issuer].isString() || !to_issuer(book.in.account, taker_pays[jss::issuer].asString()))) // Don't allow illegal issuers. @@ -186,8 +185,7 @@ doUnsubscribe(RPC::JsonContext& context) return rpcError(rpcDST_AMT_MALFORMED); } // Parse optional issuer. - else if ( - ((taker_gets.isMember(jss::issuer)) && + if (((taker_gets.isMember(jss::issuer)) && (!taker_gets[jss::issuer].isString() || !to_issuer(book.out.account, taker_gets[jss::issuer].asString()))) // Don't allow illegal issuers. @@ -211,10 +209,8 @@ doUnsubscribe(RPC::JsonContext& context) { return rpcError(rpcDOMAIN_MALFORMED); } - else - { - book.domain = domain; - } + + book.domain = domain; } context.netOps.unsubBook(ispSub->getSeq(), book); diff --git a/src/xrpld/rpc/handlers/VaultInfo.cpp b/src/xrpld/rpc/handlers/VaultInfo.cpp index b17a16cc4a..4a704e0b0b 100644 --- a/src/xrpld/rpc/handlers/VaultInfo.cpp +++ b/src/xrpld/rpc/handlers/VaultInfo.cpp @@ -35,8 +35,7 @@ parseVault(Json::Value const& params, Json::Value& jvResult) RPC::inject_error(rpcACT_MALFORMED, jvResult); return std::nullopt; } - else if ( - !(params[jss::seq].isInt() || params[jss::seq].isUInt()) || + if (!(params[jss::seq].isInt() || params[jss::seq].isUInt()) || params[jss::seq].asDouble() <= 0.0 || params[jss::seq].asDouble() > double(Json::Value::maxUInt)) { diff --git a/src/xrpld/rpc/handlers/WalletPropose.cpp b/src/xrpld/rpc/handlers/WalletPropose.cpp index 273d829670..a5005ce701 100644 --- a/src/xrpld/rpc/handlers/WalletPropose.cpp +++ b/src/xrpld/rpc/handlers/WalletPropose.cpp @@ -76,9 +76,13 @@ walletPropose(Json::Value const& params) // to detect such keys to avoid user confusion. { if (params.isMember(jss::passphrase)) + { seed = RPC::parseRippleLibSeed(params[jss::passphrase]); + } else if (params.isMember(jss::seed)) + { seed = RPC::parseRippleLibSeed(params[jss::seed]); + } if (seed) { @@ -142,15 +146,19 @@ walletPropose(Json::Value const& params) // 80 bits of entropy isn't bad, but it's better to // err on the side of caution and be conservative. if (estimate_entropy(passphrase) < 80.0) + { obj[jss::warning] = "This wallet was generated using a user-supplied " "passphrase that has low entropy and is vulnerable " "to brute-force attacks."; + } else + { obj[jss::warning] = "This wallet was generated using a user-supplied " "passphrase. It may be vulnerable to brute-force " "attacks."; + } } }