From 565aadd3b21b36bc457b343d29827da45c24490e Mon Sep 17 00:00:00 2001 From: Sergey Kuznetsov Date: Fri, 16 May 2025 17:20:25 +0100 Subject: [PATCH] Revert "fix: CTID issue (#2001)" This reverts commit 8859250d5326ccf734cb2581d478cffa5e79e04a. --- src/etl/ETLState.cpp | 2 +- src/etl/ETLState.hpp | 7 +------ src/etl/LoadBalancer.cpp | 7 ++++--- src/etlng/LoadBalancer.cpp | 7 ++++--- src/rpc/handlers/Tx.hpp | 28 ++++++++++++++++------------ tests/unit/etl/ETLStateTests.cpp | 5 +++-- tests/unit/rpc/ErrorTests.cpp | 1 - tests/unit/rpc/handlers/TxTests.cpp | 8 +------- 8 files changed, 30 insertions(+), 35 deletions(-) diff --git a/src/etl/ETLState.cpp b/src/etl/ETLState.cpp index f0ca5cd9f..cd0bbe0f8 100644 --- a/src/etl/ETLState.cpp +++ b/src/etl/ETLState.cpp @@ -40,7 +40,7 @@ tag_invoke(boost::json::value_to_tag, boost::json::value const& jv) if (jsonObject.contains(JS(result)) && jsonObject.at(JS(result)).as_object().contains(JS(info))) { auto const rippledInfo = jsonObject.at(JS(result)).as_object().at(JS(info)).as_object(); if (rippledInfo.contains(JS(network_id))) - state.networkID = boost::json::value_to(rippledInfo.at(JS(network_id))); + state.networkID.emplace(boost::json::value_to(rippledInfo.at(JS(network_id)))); } return state; diff --git a/src/etl/ETLState.hpp b/src/etl/ETLState.hpp index 3b447d985..0302aab5f 100644 --- a/src/etl/ETLState.hpp +++ b/src/etl/ETLState.hpp @@ -38,12 +38,7 @@ namespace etl { * @brief This class is responsible for fetching and storing the state of the ETL information, such as the network id */ struct ETLState { - /* - * NOTE: Rippled NetworkID: Mainnet = 0; Testnet = 1; Devnet = 2 - * However, if rippled is running on neither of these (ie. standalone mode) rippled will default to 0, but - * is not included in the stateOpt response. Must manually add it here. - */ - uint32_t networkID{0}; + std::optional networkID; /** * @brief Fetch the ETL state from the rippled server diff --git a/src/etl/LoadBalancer.cpp b/src/etl/LoadBalancer.cpp index cc8367cff..567c744b5 100644 --- a/src/etl/LoadBalancer.cpp +++ b/src/etl/LoadBalancer.cpp @@ -174,11 +174,12 @@ LoadBalancer::LoadBalancer( if (!stateOpt) { LOG(log_.warn()) << "Failed to fetch ETL state from source = " << source->toString() << " Please check the configuration and network"; - } else if (etlState_ && etlState_->networkID != stateOpt->networkID) { + } 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 + *(stateOpt->networkID), + *(etlState_->networkID) )); } else { etlState_ = stateOpt; diff --git a/src/etlng/LoadBalancer.cpp b/src/etlng/LoadBalancer.cpp index d6b96c7f9..fa34af78e 100644 --- a/src/etlng/LoadBalancer.cpp +++ b/src/etlng/LoadBalancer.cpp @@ -142,11 +142,12 @@ LoadBalancer::LoadBalancer( if (!stateOpt) { LOG(log_.warn()) << "Failed to fetch ETL state from source = " << source->toString() << " Please check the configuration and network"; - } else if (etlState_ && etlState_->networkID != stateOpt->networkID) { + } 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 + *(stateOpt->networkID), + *(etlState_->networkID) )); } else { etlState_ = stateOpt; diff --git a/src/rpc/handlers/Tx.hpp b/src/rpc/handlers/Tx.hpp index fe15929c9..acc5de4a3 100644 --- a/src/rpc/handlers/Tx.hpp +++ b/src/rpc/handlers/Tx.hpp @@ -21,6 +21,7 @@ #include "data/BackendInterface.hpp" #include "data/Types.hpp" +#include "etl/ETLService.hpp" #include "etlng/ETLServiceInterface.hpp" #include "rpc/Errors.hpp" #include "rpc/JS.hpp" @@ -38,7 +39,6 @@ #include #include #include -#include #include #include #include @@ -214,15 +214,17 @@ public: // input.transaction might be not available, get hash via tx object if (txn.contains(JS(hash))) output.hash = txn.at(JS(hash)).as_string(); - } - // append ctid here to mimic rippled behavior - auto const txnIdx = boost::json::value_to(meta.at("TransactionIndex")); - if (txnIdx <= 0xFFFFU && dbResponse->ledgerSequence < 0x0FFF'FFFFUL && currentNetId && - *currentNetId <= 0xFFFFU) { - output.ctid = rpc::encodeCTID( - dbResponse->ledgerSequence, static_cast(txnIdx), static_cast(*currentNetId) - ); + // append ctid here to mimic rippled 1.12 behavior: return ctid even binary=true + // rippled will change it in the future, ctid should be part of tx json which not available in binary + // mode + auto const txnIdx = boost::json::value_to(meta.at("TransactionIndex")); + if (txnIdx <= 0xFFFFU && dbResponse->ledgerSequence < 0x0FFF'FFFFUL && currentNetId && + *currentNetId <= 0xFFFFU) { + output.ctid = rpc::encodeCTID( + dbResponse->ledgerSequence, static_cast(txnIdx), static_cast(*currentNetId) + ); + } } output.date = dbResponse->date; @@ -279,10 +281,12 @@ private: if (output.tx) { obj[JS(tx_json)] = *output.tx; obj[JS(tx_json)].as_object()[JS(date)] = output.date; - if (output.ctid) - obj[JS(tx_json)].as_object()[JS(ctid)] = *output.ctid; - obj[JS(tx_json)].as_object()[JS(ledger_index)] = output.ledgerIndex; + // move ctid from tx_json to root + if (obj[JS(tx_json)].as_object().contains(JS(ctid))) { + obj[JS(ctid)] = obj[JS(tx_json)].as_object()[JS(ctid)]; + obj[JS(tx_json)].as_object().erase(JS(ctid)); + } // move hash from tx_json to root if (obj[JS(tx_json)].as_object().contains(JS(hash))) { obj[JS(hash)] = obj[JS(tx_json)].as_object()[JS(hash)]; diff --git a/tests/unit/etl/ETLStateTests.cpp b/tests/unit/etl/ETLStateTests.cpp index 7fc046f35..2d48b6635 100644 --- a/tests/unit/etl/ETLStateTests.cpp +++ b/tests/unit/etl/ETLStateTests.cpp @@ -58,7 +58,8 @@ TEST_F(ETLStateTest, NetworkIdValid) EXPECT_CALL(source, forwardToRippled).WillOnce(Return(json.as_object())); auto const state = etl::ETLState::fetchETLStateFromSource(source); ASSERT_TRUE(state.has_value()); - EXPECT_EQ(state->networkID, 12); + ASSERT_TRUE(state->networkID.has_value()); + EXPECT_EQ(state->networkID.value(), 12); } TEST_F(ETLStateTest, NetworkIdInvalid) @@ -75,7 +76,7 @@ TEST_F(ETLStateTest, NetworkIdInvalid) EXPECT_CALL(source, forwardToRippled).WillOnce(Return(json.as_object())); auto const state = etl::ETLState::fetchETLStateFromSource(source); ASSERT_TRUE(state.has_value()); - EXPECT_NE(state->networkID, 12); + EXPECT_FALSE(state->networkID.has_value()); } TEST_F(ETLStateTest, ResponseHasError) diff --git a/tests/unit/rpc/ErrorTests.cpp b/tests/unit/rpc/ErrorTests.cpp index 81ecbb76b..05f039697 100644 --- a/tests/unit/rpc/ErrorTests.cpp +++ b/tests/unit/rpc/ErrorTests.cpp @@ -67,7 +67,6 @@ TEST(RPCErrorsTest, StatusAsBool) RippledError::rpcUNKNOWN_COMMAND, RippledError::rpcTOO_BUSY, RippledError::rpcNO_NETWORK, - RippledError::rpcWRONG_NETWORK, RippledError::rpcACT_MALFORMED, RippledError::rpcBAD_MARKET, ClioError::RpcMalformedCurrency, diff --git a/tests/unit/rpc/handlers/TxTests.cpp b/tests/unit/rpc/handlers/TxTests.cpp index a96695586..fe41646e5 100644 --- a/tests/unit/rpc/handlers/TxTests.cpp +++ b/tests/unit/rpc/handlers/TxTests.cpp @@ -71,7 +71,6 @@ constexpr auto kDEFAULT_OUT1 = R"({ "TakerPays": "300", "TransactionType": "OfferCreate", "hash": "2E2FBAAFF767227FE4381C4BE9855986A6B9F96C62F6E443731AB36F7BBB8A08", - "ctid": "C000006400640000", "meta": { "AffectedNodes": [ { @@ -120,10 +119,8 @@ constexpr auto kDEFAULT_OUT2 = R"({ "TransactionIndex": 100, "TransactionResult": "tesSUCCESS" }, - "ctid": "C000006400640000", "tx_json": { "Account": "rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn", - "ctid": "C000006400640000", "date": 123456, "Fee": "2", "ledger_index": 100, @@ -495,8 +492,7 @@ TEST_F(RPCTxTest, ReturnBinary) "date": 123456, "ledger_index": 100, "inLedger": 100, - "validated": true, - "ctid": "C000006400640000" + "validated": true })"; TransactionAndMetadata tx; @@ -582,7 +578,6 @@ TEST_F(RPCTxTest, MintNFT) "SigningPubKey": "74657374", "TransactionType": "NFTokenMint", "hash": "C74463F49CFDCBEF3E9902672719918CDE5042DC7E7660BEBD1D1105C4B6DFF4", - "ctid": "C000006400000000", "meta": {{ "AffectedNodes": [ {{ @@ -834,7 +829,6 @@ TEST_F(RPCTxTest, CTIDNotMatch) ASSERT_FALSE(output); auto const err = rpc::makeError(output.result.error()); - // TODO: https://github.com/XRPLF/clio/issues/2002 EXPECT_EQ(err.at("error_code").as_uint64(), 4); EXPECT_EQ( err.at("error_message").as_string(),