From e062121917b799146ddbd3f27fb83c3eeda2dc11 Mon Sep 17 00:00:00 2001 From: cyan317 <120398799+cindyyan317@users.noreply.github.com> Date: Fri, 20 Oct 2023 16:22:25 +0100 Subject: [PATCH] Add config to run without valid etl (#946) Fix #943 --- example-config.json | 1 + src/etl/ETLService.h | 3 +- src/etl/ETLState.cpp | 15 ------ src/etl/ETLState.h | 21 ++++++-- src/etl/LoadBalancer.cpp | 54 +++++++++++++-------- src/etl/LoadBalancer.h | 5 +- src/rpc/handlers/Tx.h | 4 +- unittests/etl/ETLStateTests.cpp | 10 ++-- unittests/rpc/handlers/TxTests.cpp | 77 +++++++++++++++++++++++++++++- unittests/util/MockETLService.h | 2 +- 10 files changed, 142 insertions(+), 50 deletions(-) diff --git a/example-config.json b/example-config.json index 151f62bb..65f9018f 100644 --- a/example-config.json +++ b/example-config.json @@ -26,6 +26,7 @@ // --- } }, + "allow_no_etl": false, // Allow Clio to run without valid ETL source, otherwise Clio will stop if ETL check fails "etl_sources": [ { "ip": "127.0.0.1", diff --git a/src/etl/ETLService.h b/src/etl/ETLService.h index c0d5036f..08b87365 100644 --- a/src/etl/ETLService.h +++ b/src/etl/ETLService.h @@ -208,8 +208,9 @@ public: /** * @brief Get the etl nodes' state + * @return the etl nodes' state, nullopt if etl nodes are not connected */ - etl::ETLState + std::optional getETLState() const noexcept { return loadBalancer_->getETLState(); diff --git a/src/etl/ETLState.cpp b/src/etl/ETLState.cpp index 3d1dd6a5..fb5497af 100644 --- a/src/etl/ETLState.cpp +++ b/src/etl/ETLState.cpp @@ -17,26 +17,11 @@ */ //============================================================================== -#include #include -#include #include namespace etl { -ETLState -ETLState::fetchETLStateFromSource(Source const& source) noexcept -{ - auto const serverInfoRippled = data::synchronous([&source](auto yield) { - return source.forwardToRippled({{"command", "server_info"}}, std::nullopt, yield); - }); - - if (serverInfoRippled) - return boost::json::value_to(boost::json::value(*serverInfoRippled)); - - return ETLState{}; -} - ETLState tag_invoke(boost::json::value_to_tag, boost::json::value const& jv) { diff --git a/src/etl/ETLState.h b/src/etl/ETLState.h index 857136e7..08f273c4 100644 --- a/src/etl/ETLState.h +++ b/src/etl/ETLState.h @@ -19,6 +19,8 @@ #pragma once +#include + #include #include @@ -26,8 +28,6 @@ namespace etl { -class Source; - /** * @brief This class is responsible for fetching and storing the state of the ETL information, such as the network id */ @@ -36,9 +36,22 @@ struct ETLState { /** * @brief Fetch the ETL state from the rippled server + * @param source The source to fetch the state from + * @return The ETL state, nullopt if source not available */ - static ETLState - fetchETLStateFromSource(Source const& source) noexcept; + template + static std::optional + fetchETLStateFromSource(Forward const& source) noexcept + { + auto const serverInfoRippled = data::synchronous([&source](auto yield) { + return source.forwardToRippled({{"command", "server_info"}}, std::nullopt, yield); + }); + + if (serverInfoRippled) + return boost::json::value_to(boost::json::value(*serverInfoRippled)); + + return std::nullopt; + } }; ETLState diff --git a/src/etl/LoadBalancer.cpp b/src/etl/LoadBalancer.cpp index 8649bed3..17ffdaac 100644 --- a/src/etl/LoadBalancer.cpp +++ b/src/etl/LoadBalancer.cpp @@ -83,33 +83,44 @@ LoadBalancer::LoadBalancer( downloadRanges_ = 4; } + auto const allowNoEtl = config.valueOr("allow_no_etl", false); + + auto const checkOnETLFailure = [this, allowNoEtl](std::string const& log) { + LOG(log_.error()) << log; + + if (!allowNoEtl) { + LOG(log_.error()) << "Set allow_no_etl as true in config to allow clio run without valid ETL sources."; + throw std::logic_error("ETL configuration error."); + } + }; + for (auto const& entry : config.array("etl_sources")) { std::unique_ptr source = make_Source(entry, ioc, backend, subscriptions, validatedLedgers, *this); // checking etl node validity - auto const state = ETLState::fetchETLStateFromSource(*source); + auto const stateOpt = ETLState::fetchETLStateFromSource(*source); - if (!state.networkID) { - LOG(log_.error()) << "Failed to fetch ETL state from source = " << source->toString() - << " Please check the configuration and network"; - throw std::logic_error("ETL node not available"); + if (!stateOpt) { + checkOnETLFailure(fmt::format( + "Failed to fetch ETL state from source = {} Please check the configuration and network", + source->toString() + )); + } else if (etlState_ && etlState_->networkID && stateOpt->networkID && etlState_->networkID != stateOpt->networkID) { + checkOnETLFailure(fmt::format( + "ETL sources must be on the same network. Source network id = {} does not match others network id = {}", + *(stateOpt->networkID), + *(etlState_->networkID) + )); + } else { + etlState_ = stateOpt; } - if (etlState_ && etlState_->networkID != state.networkID) { - LOG(log_.error()) << "ETL sources must be on the same network. " - << "Source network id = " << *(state.networkID) - << " does not match others network id = " << *(etlState_->networkID); - throw std::logic_error("ETL nodes are not in the same network"); - } - etlState_ = state; sources_.push_back(std::move(source)); LOG(log_.info()) << "Added etl source - " << sources_.back()->toString(); } - if (sources_.empty()) { - LOG(log_.error()) << "No ETL sources configured. Please check the configuration"; - throw std::logic_error("No ETL sources configured"); - } + if (sources_.empty()) + checkOnETLFailure("No ETL sources configured. Please check the configuration"); } LoadBalancer::~LoadBalancer() @@ -256,11 +267,14 @@ LoadBalancer::execute(Func f, uint32_t ledgerSequence) return true; } -ETLState -LoadBalancer::getETLState() const noexcept +std::optional +LoadBalancer::getETLState() noexcept { - assert(etlState_); // etlState_ is set in the constructor - return *etlState_; + if (!etlState_) { + // retry ETLState fetch + etlState_ = ETLState::fetchETLStateFromSource(*this); + } + return etlState_; } } // namespace etl diff --git a/src/etl/LoadBalancer.h b/src/etl/LoadBalancer.h index d1264a40..8c30cc0b 100644 --- a/src/etl/LoadBalancer.h +++ b/src/etl/LoadBalancer.h @@ -181,9 +181,10 @@ public: /** * @brief Return state of ETL nodes. + * @return ETL state, nullopt if etl nodes not available */ - ETLState - getETLState() const noexcept; + std::optional + getETLState() noexcept; private: /** diff --git a/src/rpc/handlers/Tx.h b/src/rpc/handlers/Tx.h index 8e3279f5..ac0294b3 100644 --- a/src/rpc/handlers/Tx.h +++ b/src/rpc/handlers/Tx.h @@ -98,7 +98,9 @@ public: return Error{Status{RippledError::rpcEXCESSIVE_LGR_RANGE}}; } - auto const currentNetId = etl_->getETLState().networkID; + std::optional currentNetId = std::nullopt; + if (auto const& etlState = etl_->getETLState(); etlState.has_value()) + currentNetId = etlState->networkID; std::optional dbResponse; diff --git a/unittests/etl/ETLStateTests.cpp b/unittests/etl/ETLStateTests.cpp index 4b540ccd..8fda84da 100644 --- a/unittests/etl/ETLStateTests.cpp +++ b/unittests/etl/ETLStateTests.cpp @@ -36,7 +36,7 @@ TEST_F(ETLStateTest, Error) { EXPECT_CALL(source, forwardToRippled).WillOnce(Return(std::nullopt)); auto const state = etl::ETLState::fetchETLStateFromSource(source); - EXPECT_FALSE(state.networkID.has_value()); + EXPECT_FALSE(state); } TEST_F(ETLStateTest, NetworkIdValid) @@ -52,8 +52,9 @@ TEST_F(ETLStateTest, NetworkIdValid) ); EXPECT_CALL(source, forwardToRippled).WillOnce(Return(json.as_object())); auto const state = etl::ETLState::fetchETLStateFromSource(source); - ASSERT_TRUE(state.networkID.has_value()); - EXPECT_EQ(state.networkID.value(), 12); + ASSERT_TRUE(state.has_value()); + ASSERT_TRUE(state->networkID.has_value()); + EXPECT_EQ(state->networkID.value(), 12); } TEST_F(ETLStateTest, NetworkIdInvalid) @@ -69,5 +70,6 @@ TEST_F(ETLStateTest, NetworkIdInvalid) ); EXPECT_CALL(source, forwardToRippled).WillOnce(Return(json.as_object())); auto const state = etl::ETLState::fetchETLStateFromSource(source); - EXPECT_FALSE(state.networkID.has_value()); + ASSERT_TRUE(state.has_value()); + EXPECT_FALSE(state->networkID.has_value()); } diff --git a/unittests/rpc/handlers/TxTests.cpp b/unittests/rpc/handlers/TxTests.cpp index 810ebdbe..ccde75ce 100644 --- a/unittests/rpc/handlers/TxTests.cpp +++ b/unittests/rpc/handlers/TxTests.cpp @@ -410,7 +410,7 @@ TEST_F(RPCTxTest, ReturnBinaryWithCTID) TEST_F(RPCTxTest, MintNFT) { // Note: `inLedger` is API v1 only. See DefaultOutput_* - auto const static OUT = fmt::format( + auto static const OUT = fmt::format( R"({{ "Account": "rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn", "Fee": "50", @@ -756,9 +756,82 @@ TEST_F(RPCTxTest, ReturnCTIDForTxInput) }); } +TEST_F(RPCTxTest, NotReturnCTIDIfETLNotAvaiable) +{ + auto constexpr static OUT = R"({ + "Account":"rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn", + "Fee":"2", + "Sequence":100, + "SigningPubKey":"74657374", + "TakerGets": + { + "currency":"0158415500000000C1F76FF6ECB0BAC600000000", + "issuer":"rLEsXccBGNR3UPuPu2hUXPjziKC3qKSBun", + "value":"200" + }, + "TakerPays":"300", + "TransactionType":"OfferCreate", + "hash":"2E2FBAAFF767227FE4381C4BE9855986A6B9F96C62F6E443731AB36F7BBB8A08", + "meta": + { + "AffectedNodes": + [ + { + "CreatedNode": + { + "LedgerEntryType":"Offer", + "NewFields": + { + "TakerGets":"200", + "TakerPays": + { + "currency":"0158415500000000C1F76FF6ECB0BAC600000000", + "issuer":"rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn", + "value":"300" + } + } + } + } + ], + "TransactionIndex":100, + "TransactionResult":"tesSUCCESS" + }, + "date":123456, + "ledger_index":100, + "inLedger":100, + "validated": true + })"; + auto const rawBackendPtr = dynamic_cast(mockBackendPtr.get()); + TransactionAndMetadata tx; + tx.metadata = CreateMetaDataForCreateOffer(CURRENCY, ACCOUNT, 100, 200, 300).getSerializer().peekData(); + tx.transaction = + CreateCreateOfferTransactionObject(ACCOUNT, 2, 100, CURRENCY, ACCOUNT2, 200, 300).getSerializer().peekData(); + tx.date = 123456; + tx.ledgerSequence = 100; + EXPECT_CALL(*rawBackendPtr, fetchTransaction(ripple::uint256{TXNID}, _)).WillOnce(Return(tx)); + + auto const rawETLPtr = dynamic_cast(mockETLServicePtr.get()); + ASSERT_NE(rawETLPtr, nullptr); + EXPECT_CALL(*rawETLPtr, getETLState).WillOnce(Return(std::nullopt)); + + runSpawn([this](auto yield) { + auto const handler = AnyHandler{TestTxHandler{mockBackendPtr, mockETLServicePtr}}; + auto const req = json::parse(fmt::format( + R"({{ + "command": "tx", + "transaction": "{}" + }})", + TXNID + )); + auto const output = handler.process(req, Context{yield}); + ASSERT_TRUE(output); + EXPECT_EQ(*output, json::parse(OUT)); + }); +} + TEST_F(RPCTxTest, ViaCTID) { - auto const static OUT = fmt::format( + auto static const OUT = fmt::format( R"({{ "Account":"rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn", "Fee":"2", diff --git a/unittests/util/MockETLService.h b/unittests/util/MockETLService.h index 9bbb9b52..7d6884a0 100644 --- a/unittests/util/MockETLService.h +++ b/unittests/util/MockETLService.h @@ -32,5 +32,5 @@ struct MockETLService { MOCK_METHOD(std::uint32_t, lastPublishAgeSeconds, (), (const)); MOCK_METHOD(std::uint32_t, lastCloseAgeSeconds, (), (const)); MOCK_METHOD(bool, isAmendmentBlocked, (), (const)); - MOCK_METHOD(etl::ETLState, getETLState, (), (const)); + MOCK_METHOD(std::optional, getETLState, (), (const)); };