From 4221dcf5681d96488fa23c5c76e352e53a74c621 Mon Sep 17 00:00:00 2001 From: tequ Date: Fri, 18 Apr 2025 08:34:44 +0900 Subject: [PATCH 01/12] Add tests for SetRemarks (#491) --- src/ripple/app/tx/impl/SetRemarks.cpp | 10 +++++----- src/test/app/SetRemarks_test.cpp | 6 ++---- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/src/ripple/app/tx/impl/SetRemarks.cpp b/src/ripple/app/tx/impl/SetRemarks.cpp index f45479bf5..0edc67121 100644 --- a/src/ripple/app/tx/impl/SetRemarks.cpp +++ b/src/ripple/app/tx/impl/SetRemarks.cpp @@ -25,6 +25,7 @@ #include #include #include +#include #include namespace ripple { @@ -313,22 +314,21 @@ SetRemarks::preclaim(PreclaimContext const& ctx) TER SetRemarks::doApply() { - auto j = ctx_.journal; Sandbox sb(&ctx_.view()); auto const sle = sb.read(keylet::account(account_)); if (!sle) - return terNO_ACCOUNT; + return tefINTERNAL; auto const objID = ctx_.tx[sfObjectID]; auto sleO = sb.peek(keylet::unchecked(objID)); if (!sleO) - return terNO_ACCOUNT; + return tefINTERNAL; std::optional issuer = getRemarksIssuer(sleO); if (!issuer || *issuer != account_) - return tecNO_PERMISSION; + return tefINTERNAL; auto const& remarksTxn = ctx_.tx.getFieldArray(sfRemarks); @@ -400,7 +400,7 @@ SetRemarks::doApply() } if (newRemarks.size() > 32) - return tecTOO_MANY_REMARKS; + return tefINTERNAL; if (newRemarks.empty() && sleO->isFieldPresent(sfRemarks)) sleO->makeFieldAbsent(sfRemarks); diff --git a/src/test/app/SetRemarks_test.cpp b/src/test/app/SetRemarks_test.cpp index 8948c4613..82b782035 100644 --- a/src/test/app/SetRemarks_test.cpp +++ b/src/test/app/SetRemarks_test.cpp @@ -377,10 +377,8 @@ struct SetRemarks_test : public beast::unit_test::suite //---------------------------------------------------------------------- // doApply - // terNO_ACCOUNT - // tecNO_TARGET - // tecNO_PERMISSION - // tecTOO_MANY_REMARKS + // All checks in doApply are done in preclaim. + BEAST_EXPECT(1); } void From b87120ebc6a0d331af42dcf7492d4093db762270 Mon Sep 17 00:00:00 2001 From: Denis Angell Date: Wed, 23 Apr 2025 15:56:13 +0200 Subject: [PATCH 02/12] update conan boost --- Builds/CMake/conan/Boost.cmake | 1 + 1 file changed, 1 insertion(+) diff --git a/Builds/CMake/conan/Boost.cmake b/Builds/CMake/conan/Boost.cmake index 161ab20c5..309fb5243 100644 --- a/Builds/CMake/conan/Boost.cmake +++ b/Builds/CMake/conan/Boost.cmake @@ -32,6 +32,7 @@ target_link_libraries(ripple_boost Boost::program_options Boost::regex Boost::system + Boost::iostreams Boost::thread) if(Boost_COMPILER) target_link_libraries(ripple_boost INTERFACE Boost::disable_autolinking) From 401395a2045421e43e70ccbe8f695eda34285e5b Mon Sep 17 00:00:00 2001 From: Denis Angell Date: Thu, 24 Apr 2025 08:36:57 +0200 Subject: [PATCH 03/12] patch remarks (#497) --- src/ripple/rpc/handlers/ServerDefinitions.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/ripple/rpc/handlers/ServerDefinitions.cpp b/src/ripple/rpc/handlers/ServerDefinitions.cpp index 0138266b5..09c0cebc6 100644 --- a/src/ripple/rpc/handlers/ServerDefinitions.cpp +++ b/src/ripple/rpc/handlers/ServerDefinitions.cpp @@ -396,6 +396,8 @@ private: return "SetRegularKey"; if (inp == "HookSet") return "SetHook"; + if (inp == "RemarksSet") + return "SetRemarks"; return inp; }; From 372f25d09b8de70ee850a58160053acc6da9f790 Mon Sep 17 00:00:00 2001 From: tequ Date: Thu, 24 Apr 2025 15:38:14 +0900 Subject: [PATCH 04/12] Remove #ifndef DEBUG guards and exception handling wrappers (#496) --- src/ripple/app/consensus/RCLConsensus.cpp | 5 ----- src/ripple/app/ledger/impl/BuildLedger.cpp | 4 ---- src/ripple/app/tx/impl/apply.cpp | 4 ---- src/ripple/app/tx/impl/applySteps.cpp | 12 ------------ 4 files changed, 25 deletions(-) diff --git a/src/ripple/app/consensus/RCLConsensus.cpp b/src/ripple/app/consensus/RCLConsensus.cpp index 681645606..4fb0c5275 100644 --- a/src/ripple/app/consensus/RCLConsensus.cpp +++ b/src/ripple/app/consensus/RCLConsensus.cpp @@ -498,15 +498,11 @@ RCLConsensus::Adaptor::doAccept( for (auto const& item : *result.txns.map_) { -#ifndef DEBUG try { -#endif retriableTxs.insert( std::make_shared(SerialIter{item.slice()})); JLOG(j_.debug()) << " Tx: " << item.key(); - -#ifndef DEBUG } catch (std::exception const& ex) { @@ -514,7 +510,6 @@ RCLConsensus::Adaptor::doAccept( JLOG(j_.warn()) << " Tx: " << item.key() << " throws: " << ex.what(); } -#endif } auto built = buildLCL( diff --git a/src/ripple/app/ledger/impl/BuildLedger.cpp b/src/ripple/app/ledger/impl/BuildLedger.cpp index fbdf9945f..56feda066 100644 --- a/src/ripple/app/ledger/impl/BuildLedger.cpp +++ b/src/ripple/app/ledger/impl/BuildLedger.cpp @@ -116,10 +116,8 @@ applyTransactions( { auto const txid = it->first.getTXID(); -#ifndef DEBUG try { -#endif if (pass == 0 && built->txExists(txid)) { it = txns.erase(it); @@ -142,7 +140,6 @@ applyTransactions( case ApplyResult::Retry: ++it; } -#ifndef DEBUG } catch (std::exception const& ex) { @@ -151,7 +148,6 @@ applyTransactions( failed.insert(txid); it = txns.erase(it); } -#endif } JLOG(j.debug()) << (certainRetry ? "Pass: " : "Final pass: ") << pass diff --git a/src/ripple/app/tx/impl/apply.cpp b/src/ripple/app/tx/impl/apply.cpp index 3e369f06a..a39249a4d 100644 --- a/src/ripple/app/tx/impl/apply.cpp +++ b/src/ripple/app/tx/impl/apply.cpp @@ -168,10 +168,8 @@ applyTransaction( JLOG(j.debug()) << "TXN " << txn.getTransactionID() << (retryAssured ? "/retry" : "/final"); -#ifndef DEBUG try { -#endif auto const result = apply(app, view, txn, flags, j); if (result.second) { @@ -191,14 +189,12 @@ applyTransaction( JLOG(j.debug()) << "Transaction retry: " << transHuman(result.first); return ApplyResult::Retry; -#ifndef DEBUG } catch (std::exception const& ex) { JLOG(j.warn()) << "Throws: " << ex.what(); return ApplyResult::Fail; } -#endif } } // namespace ripple diff --git a/src/ripple/app/tx/impl/applySteps.cpp b/src/ripple/app/tx/impl/applySteps.cpp index 4f29f4501..2ae333fdd 100644 --- a/src/ripple/app/tx/impl/applySteps.cpp +++ b/src/ripple/app/tx/impl/applySteps.cpp @@ -591,19 +591,15 @@ preflight( { PreflightContext const pfctx(app, tx, rules, flags, j); -#ifndef DEBUG try { -#endif return {pfctx, invoke_preflight(pfctx)}; -#ifndef DEBUG } catch (std::exception const& e) { JLOG(j.fatal()) << "apply: " << e.what(); return {pfctx, {tefEXCEPTION, TxConsequences{tx}}}; } -#endif } PreclaimResult @@ -640,21 +636,17 @@ preclaim( preflightResult.j); } -#ifndef DEBUG try { -#endif if (!isTesSuccess(ctx->preflightResult)) return {*ctx, ctx->preflightResult}; return {*ctx, invoke_preclaim(*ctx)}; -#ifndef DEBUG } catch (std::exception const& e) { JLOG(ctx->j.fatal()) << "apply: " << e.what(); return {*ctx, tefEXCEPTION}; } -#endif } XRPAmount @@ -678,10 +670,8 @@ doApply(PreclaimResult const& preclaimResult, Application& app, OpenView& view) // info to recover. return {tefEXCEPTION, false}; } -#ifndef DEBUG try { -#endif if (!preclaimResult.likelyToClaimFee) return {preclaimResult.ter, false}; @@ -694,14 +684,12 @@ doApply(PreclaimResult const& preclaimResult, Application& app, OpenView& view) preclaimResult.flags, preclaimResult.j); return invoke_apply(ctx); -#ifndef DEBUG } catch (std::exception const& e) { JLOG(preclaimResult.j.fatal()) << "apply: " << e.what(); return {tefEXCEPTION, false}; } -#endif } } // namespace ripple From 8cf7d485abb1703f457b47dd09f23482dcc72879 Mon Sep 17 00:00:00 2001 From: Denis Angell Date: Thu, 24 Apr 2025 08:45:01 +0200 Subject: [PATCH 05/12] fix: ledger_index (#498) --- src/ripple/app/misc/impl/AccountTxPaging.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/ripple/app/misc/impl/AccountTxPaging.cpp b/src/ripple/app/misc/impl/AccountTxPaging.cpp index f06c439f4..1babbc7ce 100644 --- a/src/ripple/app/misc/impl/AccountTxPaging.cpp +++ b/src/ripple/app/misc/impl/AccountTxPaging.cpp @@ -44,8 +44,7 @@ convertBlobsToTxResult( auto tr = std::make_shared(txn, reason, app); - auto metaset = - std::make_shared(tr->getID(), tr->getLedger(), rawMeta); + auto metaset = std::make_shared(tr->getID(), ledger_index, rawMeta); // if properly formed meta is available we can use it to generate ctid if (metaset->getAsObject().isFieldPresent(sfTransactionIndex)) From 5400f43359b74eb1d3126e99ec4a92a6e5c87741 Mon Sep 17 00:00:00 2001 From: tequ Date: Thu, 24 Apr 2025 16:46:09 +0900 Subject: [PATCH 06/12] Supress logs for Catalogue_test, Import_test (#495) --- src/test/app/Import_test.cpp | 18 +++++++++++++++--- src/test/rpc/Catalogue_test.cpp | 33 +++++++++++---------------------- 2 files changed, 26 insertions(+), 25 deletions(-) diff --git a/src/test/app/Import_test.cpp b/src/test/app/Import_test.cpp index 09ffd4a02..2bb56fbe9 100644 --- a/src/test/app/Import_test.cpp +++ b/src/test/app/Import_test.cpp @@ -5469,7 +5469,11 @@ class Import_test : public beast::unit_test::suite // burn 100'000 coins { test::jtx::Env env{ - *this, network::makeNetworkVLConfig(21337, keys)}; + *this, + network::makeNetworkVLConfig(21337, keys), + nullptr, + beast::severities::kDisabled, + }; auto const envCoins = env.current()->info().drops; BEAST_EXPECT(envCoins == 100'000'000'000'000'000); @@ -5509,7 +5513,11 @@ class Import_test : public beast::unit_test::suite // burn all coins { test::jtx::Env env{ - *this, network::makeNetworkVLConfig(21337, keys)}; + *this, + network::makeNetworkVLConfig(21337, keys), + nullptr, + beast::severities::kDisabled, + }; auto const envCoins = env.current()->info().drops; BEAST_EXPECT(envCoins == 100'000'000'000'000'000); @@ -5549,7 +5557,11 @@ class Import_test : public beast::unit_test::suite // burn no coins { test::jtx::Env env{ - *this, network::makeNetworkVLConfig(21337, keys)}; + *this, + network::makeNetworkVLConfig(21337, keys), + nullptr, + beast::severities::kDisabled, + }; auto const envCoins = env.current()->info().drops; BEAST_EXPECT(envCoins == 100'000'000'000'000'000); diff --git a/src/test/rpc/Catalogue_test.cpp b/src/test/rpc/Catalogue_test.cpp index 5be24d9a0..981e05940 100644 --- a/src/test/rpc/Catalogue_test.cpp +++ b/src/test/rpc/Catalogue_test.cpp @@ -101,8 +101,7 @@ class Catalogue_test : public beast::unit_test::suite { testcase("catalogue_create: Invalid parameters"); using namespace test::jtx; - Env env{ - *this, envconfig(), features, nullptr, beast::severities::kInfo}; + Env env{*this, envconfig(), features}; // No parameters { @@ -177,8 +176,7 @@ class Catalogue_test : public beast::unit_test::suite using namespace test::jtx; // Create environment and some test ledgers - Env env{ - *this, envconfig(), features, nullptr, beast::severities::kInfo}; + Env env{*this, envconfig(), features}; prepareLedgerData(env, 5); boost::filesystem::path tempDir = @@ -217,8 +215,7 @@ class Catalogue_test : public beast::unit_test::suite { testcase("catalogue_load: Invalid parameters"); using namespace test::jtx; - Env env{ - *this, envconfig(), features, nullptr, beast::severities::kInfo}; + Env env{*this, envconfig(), features}; // No parameters { @@ -265,8 +262,7 @@ class Catalogue_test : public beast::unit_test::suite using namespace test::jtx; // Create environment and test data - Env env{ - *this, envconfig(), features, nullptr, beast::severities::kInfo}; + Env env{*this, envconfig(), features}; prepareLedgerData(env, 5); // Store some key state information before catalogue creation @@ -322,8 +318,7 @@ class Catalogue_test : public beast::unit_test::suite *this, test::jtx::envconfig(test::jtx::port_increment, 3), features, - nullptr, - beast::severities::kInfo}; + }; // Now load the catalogue Json::Value params{Json::objectValue}; @@ -518,8 +513,7 @@ class Catalogue_test : public beast::unit_test::suite return cfg; }), features, - nullptr, - beast::severities::kInfo}; + }; prepareLedgerData(env1, 5); // Create catalogue with network ID 123 @@ -544,8 +538,7 @@ class Catalogue_test : public beast::unit_test::suite return cfg; }), features, - nullptr, - beast::severities::kInfo}; + }; { Json::Value params{Json::objectValue}; @@ -568,8 +561,7 @@ class Catalogue_test : public beast::unit_test::suite using namespace test::jtx; // Create environment and test data - Env env{ - *this, envconfig(), features, nullptr, beast::severities::kInfo}; + Env env{*this, envconfig(), features}; prepareLedgerData(env, 3); boost::filesystem::path tempDir = @@ -660,8 +652,7 @@ class Catalogue_test : public beast::unit_test::suite using namespace test::jtx; // Create environment and test data - Env env{ - *this, envconfig(), features, nullptr, beast::severities::kInfo}; + Env env{*this, envconfig(), features}; prepareLedgerData(env, 3); boost::filesystem::path tempDir = @@ -736,8 +727,7 @@ class Catalogue_test : public beast::unit_test::suite using namespace test::jtx; // Create environment and test data - Env env{ - *this, envconfig(), features, nullptr, beast::severities::kInfo}; + Env env{*this, envconfig(), features}; prepareLedgerData(env, 5); boost::filesystem::path tempDir = @@ -826,8 +816,7 @@ class Catalogue_test : public beast::unit_test::suite using namespace test::jtx; // Create environment - Env env{ - *this, envconfig(), features, nullptr, beast::severities::kInfo}; + Env env{*this, envconfig(), features}; boost::filesystem::path tempDir = boost::filesystem::temp_directory_path() / From 7b581443d1bdc8943e0baebe04c3f1fb48f489a5 Mon Sep 17 00:00:00 2001 From: tequ Date: Tue, 29 Apr 2025 07:25:55 +0900 Subject: [PATCH 07/12] Suppress build warning introduced in Catalogue (#499) --- src/ripple/app/ledger/Ledger.cpp | 2 +- src/ripple/rpc/handlers/Catalogue.cpp | 6 ++--- src/ripple/shamap/impl/SHAMap.cpp | 12 ++++------ src/test/rpc/Catalogue_test.cpp | 32 ++++++++++++++------------- 4 files changed, 25 insertions(+), 27 deletions(-) diff --git a/src/ripple/app/ledger/Ledger.cpp b/src/ripple/app/ledger/Ledger.cpp index 345c3bb28..b013b35a0 100644 --- a/src/ripple/app/ledger/Ledger.cpp +++ b/src/ripple/app/ledger/Ledger.cpp @@ -311,10 +311,10 @@ Ledger::Ledger( Family& family, SHAMap const& baseState) : mImmutable(false) - , info_(info) , txMap_(SHAMapType::TRANSACTION, family) , stateMap_(baseState, true) , rules_{config.features} + , info_(info) , j_(beast::Journal(beast::Journal::getNullSink())) { } diff --git a/src/ripple/rpc/handlers/Catalogue.cpp b/src/ripple/rpc/handlers/Catalogue.cpp index de025de1a..8ca4425d6 100644 --- a/src/ripple/rpc/handlers/Catalogue.cpp +++ b/src/ripple/rpc/handlers/Catalogue.cpp @@ -69,7 +69,7 @@ static constexpr uint16_t CATALOGUE_VERSION_MASK = 0x00FF; // Lower 8 bits for version static constexpr uint16_t CATALOGUE_COMPRESS_LEVEL_MASK = 0x0F00; // Bits 8-11: compression level -static constexpr uint16_t CATALOGUE_RESERVED_MASK = +[[maybe_unused]] static constexpr uint16_t CATALOGUE_RESERVED_MASK = 0xF000; // Bits 12-15: reserved std::string @@ -229,7 +229,7 @@ class CatalogueSizePredictor private: uint32_t minLedger_; uint32_t maxLedger_; - uint64_t headerSize_; + [[maybe_unused]] uint64_t headerSize_; // Keep track of actual bytes uint64_t totalBytesWritten_; @@ -246,9 +246,9 @@ public: : minLedger_(minLedger) , maxLedger_(maxLedger) , headerSize_(headerSize) - , processedLedgers_(0) , totalBytesWritten_(headerSize) , firstLedgerSize_(0) + , processedLedgers_(0) { } diff --git a/src/ripple/shamap/impl/SHAMap.cpp b/src/ripple/shamap/impl/SHAMap.cpp index 972cc64fb..46c61bbc4 100644 --- a/src/ripple/shamap/impl/SHAMap.cpp +++ b/src/ripple/shamap/impl/SHAMap.cpp @@ -1301,10 +1301,8 @@ SHAMap::serializeToStream( std::size_t nodeCount = 0; - auto serializeLeaf = [&stream, - &localBytesWritten, - flushThreshold, - &tryFlush](SHAMapLeafNode const& node) -> bool { + auto serializeLeaf = [&stream, &localBytesWritten, &tryFlush]( + SHAMapLeafNode const& node) -> bool { // write the node type auto t = node.getType(); stream.write(reinterpret_cast(&t), 1); @@ -1335,10 +1333,8 @@ SHAMap::serializeToStream( return !stream.fail(); }; - auto serializeRemovedLeaf = [&stream, - &localBytesWritten, - flushThreshold, - &tryFlush](uint256 const& key) -> bool { + auto serializeRemovedLeaf = + [&stream, &localBytesWritten, &tryFlush](uint256 const& key) -> bool { // to indicate a node is removed it is written with a removal type auto t = SHAMapNodeType::tnREMOVE; stream.write(reinterpret_cast(&t), 1); diff --git a/src/test/rpc/Catalogue_test.cpp b/src/test/rpc/Catalogue_test.cpp index 981e05940..9cafebd07 100644 --- a/src/test/rpc/Catalogue_test.cpp +++ b/src/test/rpc/Catalogue_test.cpp @@ -401,18 +401,8 @@ class Catalogue_test : public beast::unit_test::suite sourceLedger->info().accepted == loadedLedger->info().accepted); // Check SLE counts - std::size_t sourceCount = 0; - std::size_t loadedCount = 0; - - for (auto const& sle : sourceLedger->sles) - { - sourceCount++; - } - - for (auto const& sle : loadedLedger->sles) - { - loadedCount++; - } + std::size_t sourceCount = std::ranges::distance(sourceLedger->sles); + std::size_t loadedCount = std::ranges::distance(loadedLedger->sles); BEAST_EXPECT(sourceCount == loadedCount); @@ -561,7 +551,13 @@ class Catalogue_test : public beast::unit_test::suite using namespace test::jtx; // Create environment and test data - Env env{*this, envconfig(), features}; + Env env{ + *this, + envconfig(), + features, + nullptr, + beast::severities::kDisabled, + }; prepareLedgerData(env, 3); boost::filesystem::path tempDir = @@ -652,7 +648,13 @@ class Catalogue_test : public beast::unit_test::suite using namespace test::jtx; // Create environment and test data - Env env{*this, envconfig(), features}; + Env env{ + *this, + envconfig(), + features, + nullptr, + beast::severities::kDisabled, + }; prepareLedgerData(env, 3); boost::filesystem::path tempDir = @@ -829,7 +831,7 @@ class Catalogue_test : public beast::unit_test::suite { auto result = env.client().invoke( "catalogue_status", Json::objectValue)[jss::result]; - std::cout << to_string(result) << "\n"; + // std::cout << to_string(result) << "\n"; BEAST_EXPECT(result[jss::job_status] == "no_job_running"); } From e4478fa88368ffe3a30cec079ca8f25ac10c4332 Mon Sep 17 00:00:00 2001 From: Denis Angell Date: Wed, 30 Apr 2025 10:33:43 +0200 Subject: [PATCH 08/12] revert build in docker changes --- .github/workflows/build-in-docker.yml | 72 +++++++++++++++++---------- 1 file changed, 46 insertions(+), 26 deletions(-) diff --git a/.github/workflows/build-in-docker.yml b/.github/workflows/build-in-docker.yml index 621ea43cc..9d959bbc9 100644 --- a/.github/workflows/build-in-docker.yml +++ b/.github/workflows/build-in-docker.yml @@ -1,4 +1,4 @@ -name: Release - SH Runner +name: Build using Docker on: push: @@ -11,38 +11,53 @@ concurrency: cancel-in-progress: true env: - DEBUG_CONTAINERS: 1 - REMOVE_CONTAINERS: 0 + DEBUG_BUILD_CONTAINERS_AFTER_CLEANUP: 1 jobs: - build-and-test: + checkout: runs-on: [self-hosted, vanity] + outputs: + checkout_path: ${{ steps.vars.outputs.checkout_path }} steps: - - name: Prepare workspace + - name: Prepare checkout path + id: vars run: | SAFE_BRANCH=$(echo "${{ github.ref_name }}" | sed -e 's/[^a-zA-Z0-9._-]/-/g') CHECKOUT_PATH="${SAFE_BRANCH}-${{ github.sha }}" - echo "CHECKOUT_PATH=${CHECKOUT_PATH}" >> $GITHUB_ENV - mkdir -p "$CHECKOUT_PATH" + echo "checkout_path=${CHECKOUT_PATH}" >> "$GITHUB_OUTPUT" - - name: Checkout code - uses: actions/checkout@v4 + - uses: actions/checkout@v4 with: - path: ${{ env.CHECKOUT_PATH }} + path: ${{ steps.vars.outputs.checkout_path }} clean: true - fetch-depth: 2 + fetch-depth: 2 # Only get the last 2 commits, to avoid fetching all history + checkpatterns: + runs-on: [self-hosted, vanity] + needs: checkout + defaults: + run: + working-directory: ${{ needs.checkout.outputs.checkout_path }} + steps: + - name: Check for suspicious patterns + run: /bin/bash suspicious_patterns.sh + + build: + runs-on: [self-hosted, vanity] + needs: [checkpatterns, checkout] + defaults: + run: + working-directory: ${{ needs.checkout.outputs.checkout_path }} + steps: - name: Set Cleanup Script Path run: | echo "JOB_CLEANUP_SCRIPT=$(mktemp)" >> $GITHUB_ENV - name: Build using Docker - working-directory: ${{ env.CHECKOUT_PATH }} run: /bin/bash release-builder.sh - - name: Stop Container (Cleanup after build) + - name: Stop Container (Cleanup) if: always() - working-directory: ${{ env.CHECKOUT_PATH }} run: | echo "Running cleanup script: $JOB_CLEANUP_SCRIPT" /bin/bash -e -x "$JOB_CLEANUP_SCRIPT" @@ -56,30 +71,35 @@ jobs: echo "⚠️ Cleanup script failed! Keeping for debugging: $JOB_CLEANUP_SCRIPT" fi - if [[ "${DEBUG_CONTAINERS}" == "1" ]]; then + if [[ "${DEBUG_BUILD_CONTAINERS_AFTER_CLEANUP}" == "1" ]]; then echo "🔍 Checking for leftover containers..." BUILD_CONTAINERS=$(docker ps --format '{{.Names}}' | grep '^xahaud_cached_builder' || echo "") - CONTAINER_NAME="xahaud_cached_builder_${{ github.workflow }}-${{ github.ref_name }}" - if [[ -n "$BUILD_CONTAINERS" && "${REMOVE_CONTAINERS}" == "1" ]]; then + + if [[ -n "$BUILD_CONTAINERS" ]]; then echo "⚠️ WARNING: Some build containers are still running" echo "$BUILD_CONTAINERS" - echo "Attempting to stop build containers.." - echo "Stopping container: $CONTAINER_NAME" - docker stop "$CONTAINER_NAME" || echo "Failed to stop container: $CONTAINER_NAME" - echo "Removing container: $CONTAINER_NAME" - docker rm -f "$CONTAINER_NAME" || echo "Failed to remove container: $CONTAINER_NAME" - echo "✅ Build container stopped and removed" else echo "✅ No build containers found" fi fi - - name: Run unit tests - working-directory: ${{ env.CHECKOUT_PATH }} + tests: + runs-on: [self-hosted, vanity] + needs: [build, checkout] + defaults: + run: + working-directory: ${{ needs.checkout.outputs.checkout_path }} + steps: + - name: Unit tests run: /bin/bash docker-unit-tests.sh + cleanup: + runs-on: [self-hosted, vanity] + needs: [tests, checkout] + if: always() + steps: - name: Cleanup workspace - if: always() run: | + CHECKOUT_PATH="${{ needs.checkout.outputs.checkout_path }}" echo "Cleaning workspace for ${CHECKOUT_PATH}" rm -rf "${{ github.workspace }}/${CHECKOUT_PATH}" From dd5d580b971a29e502678676449cb51a9226137a Mon Sep 17 00:00:00 2001 From: Denis Angell Date: Wed, 30 Apr 2025 10:39:12 +0200 Subject: [PATCH 09/12] [fold] bad merge --- src/test/rpc/Catalogue_test.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/test/rpc/Catalogue_test.cpp b/src/test/rpc/Catalogue_test.cpp index f5d2153fa..9cafebd07 100644 --- a/src/test/rpc/Catalogue_test.cpp +++ b/src/test/rpc/Catalogue_test.cpp @@ -504,7 +504,6 @@ class Catalogue_test : public beast::unit_test::suite }), features, }; - prepareLedgerData(env1, 5); // Create catalogue with network ID 123 @@ -552,7 +551,6 @@ class Catalogue_test : public beast::unit_test::suite using namespace test::jtx; // Create environment and test data - Env env{ *this, envconfig(), @@ -560,7 +558,6 @@ class Catalogue_test : public beast::unit_test::suite nullptr, beast::severities::kDisabled, }; - prepareLedgerData(env, 3); boost::filesystem::path tempDir = @@ -658,7 +655,6 @@ class Catalogue_test : public beast::unit_test::suite nullptr, beast::severities::kDisabled, }; - prepareLedgerData(env, 3); boost::filesystem::path tempDir = From 80a71975905941307f6521bb24d2eb1be454d321 Mon Sep 17 00:00:00 2001 From: Denis Angell Date: Wed, 30 Apr 2025 11:51:58 +0200 Subject: [PATCH 10/12] fix warnings (#505) --- src/ripple/app/misc/NetworkOPs.cpp | 6 +++--- src/ripple/app/misc/SHAMapStoreImp.cpp | 2 -- src/ripple/app/rdb/backend/FlatmapDatabase.h | 4 +--- src/ripple/app/rdb/backend/RWDBDatabase.h | 1 - src/ripple/app/tx/impl/Escrow.cpp | 8 ++++---- src/ripple/json/impl/json_reader.cpp | 5 ++--- src/ripple/net/HTTPClient.h | 4 ++++ src/ripple/overlay/impl/Handshake.cpp | 2 ++ src/ripple/protocol/impl/PublicKey.cpp | 1 + src/ripple/rpc/impl/ServerHandlerImp.cpp | 5 ----- src/test/app/SetHook_test.cpp | 1 - src/test/jtx/TrustedPublisherServer.h | 2 +- src/test/rpc/AccountNamespace_test.cpp | 16 ---------------- 13 files changed, 18 insertions(+), 39 deletions(-) diff --git a/src/ripple/app/misc/NetworkOPs.cpp b/src/ripple/app/misc/NetworkOPs.cpp index 6db57fda1..0e5b8ef5f 100644 --- a/src/ripple/app/misc/NetworkOPs.cpp +++ b/src/ripple/app/misc/NetworkOPs.cpp @@ -199,7 +199,7 @@ public: strOperatingMode(bool const admin = false) const override; StateAccounting::CounterData - getStateAccountingData(); + getStateAccountingData() override; // // Transaction operations. @@ -711,10 +711,10 @@ private: std::mutex validationsMutex_; RCLConsensus& - getConsensus(); + getConsensus() override; LedgerMaster& - getLedgerMaster(); + getLedgerMaster() override; private: struct Stats diff --git a/src/ripple/app/misc/SHAMapStoreImp.cpp b/src/ripple/app/misc/SHAMapStoreImp.cpp index 1fd8ed1e6..acfa41122 100644 --- a/src/ripple/app/misc/SHAMapStoreImp.cpp +++ b/src/ripple/app/misc/SHAMapStoreImp.cpp @@ -296,8 +296,6 @@ SHAMapStoreImp::run() fullBelowCache_ = &(*app_.getNodeFamily().getFullBelowCache(0)); treeNodeCache_ = &(*app_.getNodeFamily().getTreeNodeCache(0)); - bool const isMem = app_.config().mem_backend(); - if (advisoryDelete_) canDelete_ = state_db_.getCanDelete(); diff --git a/src/ripple/app/rdb/backend/FlatmapDatabase.h b/src/ripple/app/rdb/backend/FlatmapDatabase.h index 34814655f..00927b1d4 100644 --- a/src/ripple/app/rdb/backend/FlatmapDatabase.h +++ b/src/ripple/app/rdb/backend/FlatmapDatabase.h @@ -52,8 +52,6 @@ private: }; Application& app_; - Config const& config_; - JobQueue& jobQueue_; boost::unordered::concurrent_flat_map ledgers_; boost::unordered:: @@ -67,7 +65,7 @@ private: public: FlatmapDatabase(Application& app, Config const& config, JobQueue& jobQueue) - : app_(app), config_(config), jobQueue_(jobQueue) + : app_(app) { } diff --git a/src/ripple/app/rdb/backend/RWDBDatabase.h b/src/ripple/app/rdb/backend/RWDBDatabase.h index 9c6d70e7e..3981691bc 100644 --- a/src/ripple/app/rdb/backend/RWDBDatabase.h +++ b/src/ripple/app/rdb/backend/RWDBDatabase.h @@ -794,7 +794,6 @@ public: // reached before the result set has been exhausted (we always query for // one more than the limit), then we return an opaque marker that can be // supplied in a subsequent query. - std::uint32_t queryLimit = numberOfResults + 1; std::uint32_t findLedger = 0, findSeq = 0; if (lookingForMarker) diff --git a/src/ripple/app/tx/impl/Escrow.cpp b/src/ripple/app/tx/impl/Escrow.cpp index 74ee09a1a..552a0ebaa 100644 --- a/src/ripple/app/tx/impl/Escrow.cpp +++ b/src/ripple/app/tx/impl/Escrow.cpp @@ -446,8 +446,8 @@ EscrowFinish::preflight(PreflightContext const& ctx) { if ((!ctx.tx.isFieldPresent(sfEscrowID) && !ctx.tx.isFieldPresent(sfOfferSequence)) || - ctx.tx.isFieldPresent(sfEscrowID) && - ctx.tx.isFieldPresent(sfOfferSequence)) + (ctx.tx.isFieldPresent(sfEscrowID) && + ctx.tx.isFieldPresent(sfOfferSequence))) return temMALFORMED; } @@ -729,8 +729,8 @@ EscrowCancel::preflight(PreflightContext const& ctx) { if ((!ctx.tx.isFieldPresent(sfEscrowID) && !ctx.tx.isFieldPresent(sfOfferSequence)) || - ctx.tx.isFieldPresent(sfEscrowID) && - ctx.tx.isFieldPresent(sfOfferSequence)) + (ctx.tx.isFieldPresent(sfEscrowID) && + ctx.tx.isFieldPresent(sfOfferSequence))) return temMALFORMED; } diff --git a/src/ripple/json/impl/json_reader.cpp b/src/ripple/json/impl/json_reader.cpp index c92bea6d7..dd2f9a694 100644 --- a/src/ripple/json/impl/json_reader.cpp +++ b/src/ripple/json/impl/json_reader.cpp @@ -924,9 +924,8 @@ Reader::getLocationLineAndColumn(Location location) const { int line, column; getLocationLineAndColumn(location, line, column); - char buffer[18 + 16 + 16 + 1]; - sprintf(buffer, "Line %d, Column %d", line, column); - return buffer; + return "Line " + std::to_string(line) + ", Column " + + std::to_string(column); } std::string diff --git a/src/ripple/net/HTTPClient.h b/src/ripple/net/HTTPClient.h index 8cadc84bc..f133e3ee0 100644 --- a/src/ripple/net/HTTPClient.h +++ b/src/ripple/net/HTTPClient.h @@ -24,7 +24,11 @@ #include #include #include + #include +#include +#include +#include namespace ripple { diff --git a/src/ripple/overlay/impl/Handshake.cpp b/src/ripple/overlay/impl/Handshake.cpp index 937570888..11b75e28d 100644 --- a/src/ripple/overlay/impl/Handshake.cpp +++ b/src/ripple/overlay/impl/Handshake.cpp @@ -25,7 +25,9 @@ #include #include #include + #include + #include #include diff --git a/src/ripple/protocol/impl/PublicKey.cpp b/src/ripple/protocol/impl/PublicKey.cpp index ac86634f1..42ff1c68d 100644 --- a/src/ripple/protocol/impl/PublicKey.cpp +++ b/src/ripple/protocol/impl/PublicKey.cpp @@ -24,6 +24,7 @@ #include #include #include +#include namespace ripple { diff --git a/src/ripple/rpc/impl/ServerHandlerImp.cpp b/src/ripple/rpc/impl/ServerHandlerImp.cpp index c4e41fa27..5c9f43538 100644 --- a/src/ripple/rpc/impl/ServerHandlerImp.cpp +++ b/src/ripple/rpc/impl/ServerHandlerImp.cpp @@ -499,11 +499,6 @@ ServerHandlerImp::processUDP( return jr; } - auto required = RPC::roleRequired( - apiVersion, - app_.config().BETA_RPC_API, - jv.isMember(jss::command) ? jv[jss::command].asString() - : jv[jss::method].asString()); if (Role::FORBID == role) { jr[jss::result] = rpcError(rpcFORBIDDEN); diff --git a/src/test/app/SetHook_test.cpp b/src/test/app/SetHook_test.cpp index e122616e6..7f94b62fd 100644 --- a/src/test/app/SetHook_test.cpp +++ b/src/test/app/SetHook_test.cpp @@ -1110,7 +1110,6 @@ public: env.fund(XRP(10000000), bob); auto const preHookCount = (*env.le(alice))[sfHookStateCount]; - auto const preOwnerCount = (*env.le(alice))[sfOwnerCount]; std::string hook = "0061736D01000000012A0660057F7F7F7F7F017E60027F7F017E60027F7F017F60" diff --git a/src/test/jtx/TrustedPublisherServer.h b/src/test/jtx/TrustedPublisherServer.h index 266f8d922..64be52185 100644 --- a/src/test/jtx/TrustedPublisherServer.h +++ b/src/test/jtx/TrustedPublisherServer.h @@ -643,7 +643,7 @@ private: auto const sleep_sec = boost::lexical_cast(path.substr(7)); std::this_thread::sleep_for( - std::chrono::seconds{sleep_sec}); + std::chrono::seconds(sleep_sec)); } else if (boost::starts_with(path, "/redirect")) { diff --git a/src/test/rpc/AccountNamespace_test.cpp b/src/test/rpc/AccountNamespace_test.cpp index b63b52e68..aeeb04049 100644 --- a/src/test/rpc/AccountNamespace_test.cpp +++ b/src/test/rpc/AccountNamespace_test.cpp @@ -110,14 +110,6 @@ public: } // test errors on marker { - auto const key = uint256::fromVoid( - (std::array{ - 0x00U, 0x00U, 0x00U, 0x00U, 0x00U, 0x00U, 0x00U, 0x00U, - 0x00U, 0x00U, 0x00U, 0x00U, 0x00U, 0x00U, 0x00U, 0x00U, - 0x00U, 0x00U, 0x00U, 0x00U, 0x00U, 0x00U, 0x00U, 0x00U, - 0x00U, 0x00U, 0x00U, 0x00U, 'k', 'e', 'y', 0x00U}) - .data()); - auto const ns = uint256::fromVoid( (std::array{ 0xCAU, 0xFEU, 0xCAU, 0xFEU, 0xCAU, 0xFEU, 0xCAU, 0xFEU, @@ -126,14 +118,6 @@ public: 0xCAU, 0xFEU, 0xCAU, 0xFEU, 0xCAU, 0xFEU, 0xCAU, 0xFEU}) .data()); - auto const nons = uint256::fromVoid( - (std::array{ - 0xCAU, 0xFEU, 0xCAU, 0xFEU, 0xCAU, 0xFEU, 0xCAU, 0xFEU, - 0xCAU, 0xFEU, 0xCAU, 0xFEU, 0xCAU, 0xFEU, 0xCAU, 0xFEU, - 0xCAU, 0xFEU, 0xCAU, 0xFEU, 0xCAU, 0xFEU, 0xCAU, 0xFEU, - 0xCAU, 0xFEU, 0xCAU, 0xFEU, 0xCAU, 0xFEU, 0xCAU, 0xFFU}) - .data()); - // Lambda to create a hook. auto setHook = [](test::jtx::Account const& account) { std::string const createCodeHex = From 5e005cd6eebf338f8090f9a07bbc11d0b804ee88 Mon Sep 17 00:00:00 2001 From: RichardAH Date: Thu, 1 May 2025 09:54:41 +1000 Subject: [PATCH 11/12] remove false positives from sus pat finder (#506) --- suspicious_patterns.sh | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/suspicious_patterns.sh b/suspicious_patterns.sh index 6010c2494..076c1a929 100755 --- a/suspicious_patterns.sh +++ b/suspicious_patterns.sh @@ -13,8 +13,8 @@ for file in $files_changed; do # Check if the file exists (it might have been deleted) if [ -f "$absolute_path" ]; then - # Search the file for the given patterns - grep_output=$(grep -n -E '(([^rpshnaf39wBUDNEGHJKLM4PQRST7VWXYZ2bcdeCg65jkm8oFqi1tuvAxyz]|^)(s|p)[rpshnaf39wBUDNEGHJKLM4PQRST7VWXYZ2bcdeCg65jkm8oFqi1tuvAxyz]{25,60}([^(]|$)))|([^A-Fa-f0-9](02|03|ED)[A-Fa-f0-9]{64})' "$absolute_path") + # Search the file for the given patterns, but exclude lines containing 'public_key' + grep_output=$(grep -n -E '(([^rpshnaf39wBUDNEGHJKLM4PQRST7VWXYZ2bcdeCg65jkm8oFqi1tuvAxyz]|^)(s|p)[rpshnaf39wBUDNEGHJKLM4PQRST7VWXYZ2bcdeCg65jkm8oFqi1tuvAxyz]{25,60}([^(]|$)))|([^A-Fa-f0-9](02|03|ED)[A-Fa-f0-9]{64})' "$absolute_path" | grep -v "public_key") # Check if grep found any matches if [ ! -z "$grep_output" ]; then @@ -25,7 +25,3 @@ for file in $files_changed; do fi fi done - -# If the loop completes without finding any suspicious patterns -echo "Success: No suspicious patterns found in the diff." -exit 0 \ No newline at end of file From 615f56570af4a32d647bf8684e9f817514edcb79 Mon Sep 17 00:00:00 2001 From: RichardAH Date: Thu, 1 May 2025 17:23:56 +1000 Subject: [PATCH 12/12] Sus pat (#507) --- suspicious_patterns.sh | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/suspicious_patterns.sh b/suspicious_patterns.sh index 076c1a929..39287a6a1 100755 --- a/suspicious_patterns.sh +++ b/suspicious_patterns.sh @@ -8,6 +8,11 @@ files_changed=$(git diff --name-only --relative HEAD~1 HEAD) # Loop through each file and search for the patterns for file in $files_changed; do + # Skip if the file is Import_test.cpp (exact filename match regardless of path) + if [[ "$(basename "$file")" == "Import_test.cpp" ]]; then + continue + fi + # Construct the absolute path absolute_path="$repo_root/$file"