Skip to content

Commit 26ed78f

Browse files
godexsoftgodexsoft
committed
Cherry-pick XRPFees bugfix
1 parent 8575f78 commit 26ed78f

18 files changed

+376
-48
lines changed

‎CMakeLists.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,7 @@ if (tests)
267267
unittests/rpc/handlers/AMMInfoTests.cpp
268268
# Backend
269269
unittests/data/BackendFactoryTests.cpp
270+
unittests/data/BackendInterfaceTests.cpp
270271
unittests/data/BackendCountersTests.cpp
271272
unittests/data/cassandra/BaseTests.cpp
272273
unittests/data/cassandra/BackendTests.cpp

‎conanfile.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ class Clio(ConanFile):
2626
'protobuf/3.21.12',
2727
'grpc/1.50.1',
2828
'openssl/1.1.1u',
29-
'xrpl/2.0.0',
29+
'xrpl/2.1.0',
3030
'libbacktrace/cci.20210118'
3131
]
3232

‎src/data/BackendInterface.cpp‎

Lines changed: 34 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -344,14 +344,42 @@ BackendInterface::fetchFees(std::uint32_t const seq, boost::asio::yield_context
344344
ripple::SerialIter it(bytes->data(), bytes->size());
345345
ripple::SLE const sle{it, key};
346346

347-
if (sle.getFieldIndex(ripple::sfBaseFee) != -1)
348-
fees.base = sle.getFieldU64(ripple::sfBaseFee);
347+
// XRPFees amendment introduced new fields for fees calculations.
348+
// New fields are set and the old fields are removed via `set_fees` tx.
349+
// Fallback to old fields if `set_fees` was not yet used to update the fields on this tx.
350+
auto hasNewFields = false;
351+
{
352+
auto const baseFeeXRP = sle.at(~ripple::sfBaseFeeDrops);
353+
auto const reserveBaseXRP = sle.at(~ripple::sfReserveBaseDrops);
354+
auto const reserveIncrementXRP = sle.at(~ripple::sfReserveIncrementDrops);
349355

350-
if (sle.getFieldIndex(ripple::sfReserveBase) != -1)
351-
fees.reserve = sle.getFieldU32(ripple::sfReserveBase);
356+
if (baseFeeXRP)
357+
fees.base = baseFeeXRP->xrp();
352358

353-
if (sle.getFieldIndex(ripple::sfReserveIncrement) != -1)
354-
fees.increment = sle.getFieldU32(ripple::sfReserveIncrement);
359+
if (reserveBaseXRP)
360+
fees.reserve = reserveBaseXRP->xrp();
361+
362+
if (reserveIncrementXRP)
363+
fees.increment = reserveIncrementXRP->xrp();
364+
365+
hasNewFields = baseFeeXRP || reserveBaseXRP || reserveIncrementXRP;
366+
}
367+
368+
if (not hasNewFields) {
369+
// Fallback to old fields
370+
auto const baseFee = sle.at(~ripple::sfBaseFee);
371+
auto const reserveBase = sle.at(~ripple::sfReserveBase);
372+
auto const reserveIncrement = sle.at(~ripple::sfReserveIncrement);
373+
374+
if (baseFee)
375+
fees.base = baseFee.value();
376+
377+
if (reserveBase)
378+
fees.reserve = reserveBase.value();
379+
380+
if (reserveIncrement)
381+
fees.increment = reserveIncrement.value();
382+
}
355383

356384
return fees;
357385
}

‎unittests/CMakeLists.txt‎

Lines changed: 177 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,177 @@
1+
add_executable(clio_tests)
2+
3+
target_sources(
4+
clio_tests
5+
PRIVATE # Common
6+
ConfigTests.cpp
7+
data/BackendCountersTests.cpp
8+
data/BackendFactoryTests.cpp
9+
data/BackendInterfaceTests.cpp
10+
data/cassandra/AsyncExecutorTests.cpp
11+
# Webserver
12+
data/cassandra/BackendTests.cpp
13+
data/cassandra/BaseTests.cpp
14+
data/cassandra/ExecutionStrategyTests.cpp
15+
data/cassandra/RetryPolicyTests.cpp
16+
data/cassandra/SettingsProviderTests.cpp
17+
DOSGuardTests.cpp
18+
etl/AmendmentBlockHandlerTests.cpp
19+
etl/CacheLoaderSettingsTests.cpp
20+
etl/CacheLoaderTests.cpp
21+
etl/CursorFromAccountProviderTests.cpp
22+
etl/CursorFromDiffProviderTests.cpp
23+
etl/CursorFromFixDiffNumProviderTests.cpp
24+
etl/ETLStateTests.cpp
25+
etl/ExtractionDataPipeTests.cpp
26+
etl/ExtractorTests.cpp
27+
etl/ForwardingCacheTests.cpp
28+
etl/ForwardingSourceTests.cpp
29+
etl/GrpcSourceTests.cpp
30+
etl/LedgerPublisherTests.cpp
31+
etl/SourceTests.cpp
32+
etl/SubscriptionSourceDependenciesTests.cpp
33+
etl/SubscriptionSourceTests.cpp
34+
etl/TransformerTests.cpp
35+
# RPC
36+
feed/BookChangesFeedTests.cpp
37+
feed/ForwardFeedTests.cpp
38+
feed/LedgerFeedTests.cpp
39+
feed/ProposedTransactionFeedTests.cpp
40+
feed/SingleFeedBaseTests.cpp
41+
feed/SubscriptionManagerTests.cpp
42+
feed/TrackableSignalTests.cpp
43+
feed/TransactionFeedTests.cpp
44+
JsonUtilTests.cpp
45+
LoggerTests.cpp
46+
Main.cpp
47+
Playground.cpp
48+
ProfilerTests.cpp
49+
rpc/AmendmentsTests.cpp
50+
rpc/APIVersionTests.cpp
51+
rpc/BaseTests.cpp
52+
rpc/CountersTests.cpp
53+
rpc/ErrorTests.cpp
54+
rpc/ForwardingProxyTests.cpp
55+
rpc/handlers/AccountChannelsTests.cpp
56+
rpc/handlers/AccountCurrenciesTests.cpp
57+
rpc/handlers/AccountInfoTests.cpp
58+
rpc/handlers/AccountLinesTests.cpp
59+
rpc/handlers/AccountNFTsTests.cpp
60+
rpc/handlers/AccountObjectsTests.cpp
61+
rpc/handlers/AccountOffersTests.cpp
62+
rpc/handlers/AccountTxTests.cpp
63+
rpc/handlers/AMMInfoTests.cpp
64+
# Backend
65+
rpc/handlers/BookChangesTests.cpp
66+
rpc/handlers/BookOffersTests.cpp
67+
rpc/handlers/DefaultProcessorTests.cpp
68+
rpc/handlers/DepositAuthorizedTests.cpp
69+
rpc/handlers/GatewayBalancesTests.cpp
70+
rpc/handlers/LedgerDataTests.cpp
71+
rpc/handlers/LedgerEntryTests.cpp
72+
rpc/handlers/LedgerRangeTests.cpp
73+
rpc/handlers/LedgerTests.cpp
74+
rpc/handlers/NFTBuyOffersTests.cpp
75+
rpc/handlers/NFTHistoryTests.cpp
76+
rpc/handlers/NFTInfoTests.cpp
77+
rpc/handlers/NFTsByIssuerTest.cpp
78+
rpc/handlers/NFTSellOffersTests.cpp
79+
rpc/handlers/NoRippleCheckTests.cpp
80+
rpc/handlers/PingTests.cpp
81+
rpc/handlers/RandomTests.cpp
82+
rpc/handlers/ServerInfoTests.cpp
83+
rpc/handlers/SubscribeTests.cpp
84+
rpc/handlers/TestHandlerTests.cpp
85+
rpc/handlers/TransactionEntryTests.cpp
86+
rpc/handlers/TxTests.cpp
87+
rpc/handlers/UnsubscribeTests.cpp
88+
rpc/handlers/VersionHandlerTests.cpp
89+
rpc/JsonBoolTests.cpp
90+
# RPC handlers
91+
rpc/RPCHelpersTests.cpp
92+
rpc/WorkQueueTests.cpp
93+
util/AssertTests.cpp
94+
util/async/AnyExecutionContextTests.cpp
95+
util/async/AnyOperationTests.cpp
96+
util/async/AnyStopTokenTests.cpp
97+
util/async/AnyStrandTests.cpp
98+
util/async/AsyncExecutionContextTests.cpp
99+
# Requests framework
100+
util/BatchingTests.cpp
101+
util/LedgerUtilsTests.cpp
102+
# Prometheus support
103+
util/prometheus/BoolTests.cpp
104+
util/prometheus/CounterTests.cpp
105+
util/prometheus/GaugeTests.cpp
106+
util/prometheus/HistogramTests.cpp
107+
util/prometheus/HttpTests.cpp
108+
util/prometheus/LabelTests.cpp
109+
util/prometheus/MetricBuilderTests.cpp
110+
util/prometheus/MetricsFamilyTests.cpp
111+
util/prometheus/OStreamTests.cpp
112+
util/requests/RequestBuilderTests.cpp
113+
util/requests/SslContextTests.cpp
114+
util/requests/WsConnectionTests.cpp
115+
# ETL
116+
util/RetryTests.cpp
117+
# Async framework
118+
util/StringUtils.cpp
119+
util/TestGlobals.cpp
120+
util/TestHttpServer.cpp
121+
util/TestObject.cpp
122+
util/TestWsServer.cpp
123+
util/TxUtilTests.cpp
124+
web/AdminVerificationTests.cpp
125+
web/RPCServerHandlerTests.cpp
126+
web/ServerTests.cpp
127+
web/SweepHandlerTests.cpp
128+
# Feed
129+
web/WhitelistHandlerTests.cpp
130+
)
131+
132+
include(deps/gtest)
133+
134+
# See https://github.com/google/googletest/issues/3475
135+
gtest_discover_tests(clio_tests DISCOVERY_TIMEOUT 90)
136+
137+
# Fix for dwarf5 bug on ci
138+
target_compile_options(clio_options INTERFACE -gdwarf-4)
139+
140+
target_compile_definitions(clio_tests PUBLIC UNITTEST_BUILD)
141+
target_include_directories(clio_tests PRIVATE .)
142+
target_link_libraries(clio_tests PUBLIC clio gtest::gtest)
143+
set_target_properties(clio_tests PROPERTIES RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR})
144+
145+
# Generate `coverage_report` target if coverage is enabled
146+
if (coverage)
147+
if (DEFINED CODE_COVERAGE_REPORT_FORMAT)
148+
set(CODE_COVERAGE_FORMAT ${CODE_COVERAGE_REPORT_FORMAT})
149+
else ()
150+
set(CODE_COVERAGE_FORMAT html-details)
151+
endif ()
152+
153+
if (DEFINED CODE_COVERAGE_TESTS_ARGS)
154+
set(TESTS_ADDITIONAL_ARGS ${CODE_COVERAGE_TESTS_ARGS})
155+
separate_arguments(TESTS_ADDITIONAL_ARGS)
156+
else ()
157+
set(TESTS_ADDITIONAL_ARGS "")
158+
endif ()
159+
160+
set(GCOVR_ADDITIONAL_ARGS --exclude-throw-branches -s)
161+
162+
setup_target_for_coverage_gcovr(
163+
NAME
164+
coverage_report
165+
FORMAT
166+
${CODE_COVERAGE_FORMAT}
167+
EXECUTABLE
168+
clio_tests
169+
EXECUTABLE_ARGS
170+
--gtest_brief=1
171+
${TESTS_ADDITIONAL_ARGS}
172+
EXCLUDE
173+
"unittests"
174+
DEPENDENCIES
175+
clio_tests
176+
)
177+
endif ()

‎unittests/data/BackendInterfaceTests.cpp‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
//------------------------------------------------------------------------------
2+
/*
3+
This file is part of clio: https://github.com/XRPLF/clio
4+
Copyright (c) 2024, the clio developers.
5+
6+
Permission to use, copy, modify, and distribute this software for any
7+
purpose with or without fee is hereby granted, provided that the above
8+
copyright notice and this permission notice appear in all copies.
9+
10+
THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
11+
WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
12+
MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
13+
ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
14+
WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
15+
ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
16+
OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
17+
*/
18+
//==============================================================================
19+
20+
#include "data/BackendInterface.hpp"
21+
#include "util/Fixtures.hpp"
22+
#include "util/MockPrometheus.hpp"
23+
#include "util/TestObject.hpp"
24+
25+
#include <gmock/gmock.h>
26+
#include <gtest/gtest.h>
27+
28+
using namespace data;
29+
using namespace util::prometheus;
30+
using namespace testing;
31+
32+
constexpr static auto MAXSEQ = 30;
33+
constexpr static auto MINSEQ = 10;
34+
35+
struct BackendInterfaceTest : MockBackendTestNaggy, SyncAsioContextTest, WithPrometheus {};
36+
37+
TEST_F(BackendInterfaceTest, FetchFeesSuccessPath)
38+
{
39+
using namespace ripple;
40+
backend->setRange(MINSEQ, MAXSEQ);
41+
42+
// New fee setting (after XRPFees amendment)
43+
EXPECT_CALL(*backend, doFetchLedgerObject(keylet::fees().key, MAXSEQ, _))
44+
.WillRepeatedly(Return(CreateFeeSettingBlob(XRPAmount(1), XRPAmount(2), XRPAmount(3), 0)));
45+
46+
runSpawn([this](auto yield) {
47+
auto fees = backend->fetchFees(MAXSEQ, yield);
48+
49+
EXPECT_TRUE(fees.has_value());
50+
EXPECT_EQ(fees->base, XRPAmount(1));
51+
EXPECT_EQ(fees->increment, XRPAmount(2));
52+
EXPECT_EQ(fees->reserve, XRPAmount(3));
53+
});
54+
}
55+
56+
TEST_F(BackendInterfaceTest, FetchFeesLegacySuccessPath)
57+
{
58+
using namespace ripple;
59+
backend->setRange(MINSEQ, MAXSEQ);
60+
61+
// Legacy fee setting (before XRPFees amendment)
62+
EXPECT_CALL(*backend, doFetchLedgerObject(keylet::fees().key, MAXSEQ, _))
63+
.WillRepeatedly(Return(CreateLegacyFeeSettingBlob(1, 2, 3, 4, 0)));
64+
65+
runSpawn([this](auto yield) {
66+
auto fees = backend->fetchFees(MAXSEQ, yield);
67+
68+
EXPECT_TRUE(fees.has_value());
69+
EXPECT_EQ(fees->base, XRPAmount(1));
70+
EXPECT_EQ(fees->increment, XRPAmount(2));
71+
EXPECT_EQ(fees->reserve, XRPAmount(3));
72+
});
73+
}

‎unittests/data/cassandra/BackendTests.cpp‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
#include "rpc/RPCHelpers.hpp"
2828
#include "util/Fixtures.hpp"
2929
#include "util/LedgerUtils.hpp"
30+
#include "util/MockPrometheus.hpp"
3031
#include "util/Random.hpp"
3132
#include "util/StringUtils.hpp"
3233
#include "util/TestGlobals.hpp"
@@ -65,11 +66,12 @@
6566
using namespace util;
6667
using namespace std;
6768
using namespace rpc;
69+
using namespace prometheus;
6870
namespace json = boost::json;
6971

7072
using namespace data::cassandra;
7173

72-
class BackendCassandraTest : public SyncAsioContextTest {
74+
class BackendCassandraTest : public SyncAsioContextTest, public WithPrometheus {
7375
protected:
7476
Config cfg{json::parse(fmt::format(
7577
R"JSON({{

‎unittests/etl/LedgerPublisherTests.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ TEST_F(ETLLedgerPublisherTest, PublishLedgerInfoInRange)
128128
// mock fetch fee
129129
EXPECT_CALL(*backend, doFetchLedgerObject).Times(1);
130130
ON_CALL(*backend, doFetchLedgerObject(ripple::keylet::fees().key, SEQ, _))
131-
.WillByDefault(Return(CreateFeeSettingBlob(1, 2, 3, 4, 0)));
131+
.WillByDefault(Return(CreateLegacyFeeSettingBlob(1, 2, 3, 4, 0)));
132132

133133
// mock fetch transactions
134134
EXPECT_CALL(*backend, fetchAllTransactionsInLedger).Times(1);
@@ -176,7 +176,7 @@ TEST_F(ETLLedgerPublisherTest, PublishLedgerInfoCloseTimeGreaterThanNow)
176176
// mock fetch fee
177177
EXPECT_CALL(*backend, doFetchLedgerObject).Times(1);
178178
ON_CALL(*backend, doFetchLedgerObject(ripple::keylet::fees().key, SEQ, _))
179-
.WillByDefault(Return(CreateFeeSettingBlob(1, 2, 3, 4, 0)));
179+
.WillByDefault(Return(CreateLegacyFeeSettingBlob(1, 2, 3, 4, 0)));
180180

181181
// mock fetch transactions
182182
EXPECT_CALL(*backend, fetchAllTransactionsInLedger).Times(1);
@@ -265,7 +265,7 @@ TEST_F(ETLLedgerPublisherTest, PublishMultipleTxInOrder)
265265
// mock fetch fee
266266
EXPECT_CALL(*backend, doFetchLedgerObject).Times(1);
267267
ON_CALL(*backend, doFetchLedgerObject(ripple::keylet::fees().key, SEQ, _))
268-
.WillByDefault(Return(CreateFeeSettingBlob(1, 2, 3, 4, 0)));
268+
.WillByDefault(Return(CreateLegacyFeeSettingBlob(1, 2, 3, 4, 0)));
269269

270270
// mock fetch transactions
271271
EXPECT_CALL(*backend, fetchAllTransactionsInLedger).Times(1);

‎unittests/feed/LedgerFeedTests.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ TEST_F(FeedLedgerTest, SubPub)
4242
auto const ledgerInfo = CreateLedgerInfo(LEDGERHASH, 30);
4343
EXPECT_CALL(*backend, fetchLedgerBySequence).WillOnce(testing::Return(ledgerInfo));
4444

45-
auto const feeBlob = CreateFeeSettingBlob(1, 2, 3, 4, 0);
45+
auto const feeBlob = CreateLegacyFeeSettingBlob(1, 2, 3, 4, 0);
4646
EXPECT_CALL(*backend, doFetchLedgerObject).WillOnce(testing::Return(feeBlob));
4747
// check the function response
4848
// Information about the ledgers on hand and current fee schedule. This
@@ -103,7 +103,7 @@ TEST_F(FeedLedgerTest, AutoDisconnect)
103103
auto const ledgerinfo = CreateLedgerInfo(LEDGERHASH, 30);
104104
EXPECT_CALL(*backend, fetchLedgerBySequence).WillOnce(testing::Return(ledgerinfo));
105105

106-
auto const feeBlob = CreateFeeSettingBlob(1, 2, 3, 4, 0);
106+
auto const feeBlob = CreateLegacyFeeSettingBlob(1, 2, 3, 4, 0);
107107
EXPECT_CALL(*backend, doFetchLedgerObject).WillOnce(testing::Return(feeBlob));
108108
constexpr static auto LedgerResponse =
109109
R"({

‎unittests/feed/SubscriptionManagerTests.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -280,7 +280,7 @@ TEST_F(SubscriptionManagerTest, LedgerTest)
280280
auto const ledgerinfo = CreateLedgerInfo(LEDGERHASH, 30);
281281
EXPECT_CALL(*backend, fetchLedgerBySequence).WillOnce(testing::Return(ledgerinfo));
282282

283-
auto const feeBlob = CreateFeeSettingBlob(1, 2, 3, 4, 0);
283+
auto const feeBlob = CreateLegacyFeeSettingBlob(1, 2, 3, 4, 0);
284284
EXPECT_CALL(*backend, doFetchLedgerObject).WillOnce(testing::Return(feeBlob));
285285
// check the function response
286286
// Information about the ledgers on hand and current fee schedule. This

0 commit comments

Comments
 (0)