Skip to content

Commit 3f87aeb

Browse files
committed
Update several unit tests to work with the new number rules
- For AMM tests, just disable the SAV amendment because there are a boatload of results that depend on the old math. Those will be updated later.
1 parent db52b34 commit 3f87aeb

File tree

7 files changed

+136
-70
lines changed

7 files changed

+136
-70
lines changed

src/test/app/AMMExtended_test.cpp

Lines changed: 32 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -45,10 +45,6 @@ struct AMMExtended_test : public jtx::AMMTest
4545
// funded and not used for the payment.
4646

4747
using namespace jtx;
48-
// For now, just disable SAV entirely, which locks in the small Number
49-
// mantissas
50-
features =
51-
features - featureSingleAssetVault /* - featureLendingProtocol */;
5248

5349
Env env{*this, features};
5450

@@ -1426,7 +1422,12 @@ struct AMMExtended_test : public jtx::AMMTest
14261422
testOffers()
14271423
{
14281424
using namespace jtx;
1429-
FeatureBitset const all{testable_amendments()};
1425+
// For now, just disable SAV entirely, which locks in the small Number
1426+
// mantissas
1427+
FeatureBitset const all{
1428+
testable_amendments() -
1429+
featureSingleAssetVault /* - featureLendingProtocol */};
1430+
14301431
testRmFundedOffer(all);
14311432
testRmFundedOffer(all - fixAMMv1_1 - fixAMMv1_3);
14321433
testEnforceNoRipple(all);
@@ -3754,7 +3755,11 @@ struct AMMExtended_test : public jtx::AMMTest
37543755
testFlow()
37553756
{
37563757
using namespace jtx;
3757-
FeatureBitset const all{testable_amendments()};
3758+
// For now, just disable SAV entirely, which locks in the small Number
3759+
// mantissas in the transaction engine
3760+
FeatureBitset const all{
3761+
testable_amendments() -
3762+
featureSingleAssetVault /* - featureLendingProtocol */};
37583763

37593764
testFalseDry(all);
37603765
testBookStep(all);
@@ -3768,7 +3773,11 @@ struct AMMExtended_test : public jtx::AMMTest
37683773
testCrossingLimits()
37693774
{
37703775
using namespace jtx;
3771-
FeatureBitset const all{testable_amendments()};
3776+
// For now, just disable SAV entirely, which locks in the small Number
3777+
// mantissas in the transaction engine
3778+
FeatureBitset const all{
3779+
testable_amendments() -
3780+
featureSingleAssetVault /* - featureLendingProtocol */};
37723781
testStepLimit(all);
37733782
testStepLimit(all - fixAMMv1_1 - fixAMMv1_3);
37743783
}
@@ -3777,23 +3786,36 @@ struct AMMExtended_test : public jtx::AMMTest
37773786
testDeliverMin()
37783787
{
37793788
using namespace jtx;
3780-
FeatureBitset const all{testable_amendments()};
3789+
// For now, just disable SAV entirely, which locks in the small Number
3790+
// mantissas in the transaction engine
3791+
FeatureBitset const all{
3792+
testable_amendments() -
3793+
featureSingleAssetVault /* - featureLendingProtocol */};
37813794
test_convert_all_of_an_asset(all);
37823795
test_convert_all_of_an_asset(all - fixAMMv1_1 - fixAMMv1_3);
37833796
}
37843797

37853798
void
37863799
testDepositAuth()
37873800
{
3788-
testPayment(jtx::testable_amendments());
3801+
// For now, just disable SAV entirely, which locks in the small Number
3802+
// mantissas in the transaction engine
3803+
FeatureBitset const all{
3804+
jtx::testable_amendments() -
3805+
featureSingleAssetVault /* - featureLendingProtocol */};
3806+
testPayment(all);
37893807
testPayIOU();
37903808
}
37913809

37923810
void
37933811
testFreeze()
37943812
{
37953813
using namespace test::jtx;
3796-
auto const sa = testable_amendments();
3814+
// For now, just disable SAV entirely, which locks in the small Number
3815+
// mantissas in the transaction engine
3816+
FeatureBitset const sa{
3817+
testable_amendments() -
3818+
featureSingleAssetVault /* - featureLendingProtocol */};
37973819
testRippleState(sa);
37983820
testGlobalFreeze(sa);
37993821
testOffersWhenFrozen(sa);

src/test/app/EscrowToken_test.cpp

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -559,12 +559,16 @@ struct EscrowToken_test : public beast::unit_test::suite
559559
env(pay(gw, bob, USD(1)));
560560
env.close();
561561

562+
bool const largeMantissa = features[featureSingleAssetVault] /* ||
563+
features[featureLendingProtocol]*/
564+
;
565+
562566
// alice cannot create escrow for 1/10 iou - precision loss
563567
env(escrow::create(alice, bob, USD(1)),
564568
escrow::condition(escrow::cb1),
565569
escrow::finish_time(env.now() + 1s),
566570
fee(baseFee * 150),
567-
ter(tecPRECISION_LOSS));
571+
ter(largeMantissa ? (TER)tesSUCCESS : (TER)tecPRECISION_LOSS));
568572
env.close();
569573
}
570574
}
@@ -2076,12 +2080,16 @@ struct EscrowToken_test : public beast::unit_test::suite
20762080
env(pay(gw, bob, USD(1)));
20772081
env.close();
20782082

2083+
bool const largeMantissa = features[featureSingleAssetVault] /* ||
2084+
features[featureLendingProtocol]*/
2085+
;
2086+
20792087
// alice cannot create escrow for 1/10 iou - precision loss
20802088
env(escrow::create(alice, bob, USD(1)),
20812089
escrow::condition(escrow::cb1),
20822090
escrow::finish_time(env.now() + 1s),
20832091
fee(baseFee * 150),
2084-
ter(tecPRECISION_LOSS));
2092+
ter(largeMantissa ? (TER)tesSUCCESS : (TER)tecPRECISION_LOSS));
20852093
env.close();
20862094

20872095
auto const seq1 = env.seq(alice);
@@ -3924,9 +3932,13 @@ struct EscrowToken_test : public beast::unit_test::suite
39243932
{
39253933
using namespace test::jtx;
39263934
FeatureBitset const all{testable_amendments()};
3927-
testIOUWithFeats(all);
3928-
testMPTWithFeats(all);
3929-
testMPTWithFeats(all - fixTokenEscrowV1);
3935+
for (FeatureBitset const& feats :
3936+
{all - featureSingleAssetVault /*- featureLendingProtocol*/, all})
3937+
{
3938+
testIOUWithFeats(feats);
3939+
testMPTWithFeats(feats);
3940+
testMPTWithFeats(feats - fixTokenEscrowV1);
3941+
}
39303942
}
39313943
};
39323944

src/test/basics/IOUAmount_test.cpp

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -141,15 +141,35 @@ class IOUAmount_test : public beast::unit_test::suite
141141
{
142142
testcase("IOU strings");
143143

144-
BEAST_EXPECT(to_string(IOUAmount(-2, 0)) == "-2");
145-
BEAST_EXPECT(to_string(IOUAmount(0, 0)) == "0");
146-
BEAST_EXPECT(to_string(IOUAmount(2, 0)) == "2");
147-
BEAST_EXPECT(to_string(IOUAmount(25, -3)) == "0.025");
148-
BEAST_EXPECT(to_string(IOUAmount(-25, -3)) == "-0.025");
149-
BEAST_EXPECT(to_string(IOUAmount(25, 1)) == "250");
150-
BEAST_EXPECT(to_string(IOUAmount(-25, 1)) == "-250");
151-
BEAST_EXPECT(to_string(IOUAmount(2, 20)) == "2000000000000000e5");
152-
BEAST_EXPECT(to_string(IOUAmount(-2, -20)) == "-2000000000000000e-35");
144+
auto test = [this](IOUAmount const& n, std::string const& expected) {
145+
auto const result = to_string(n);
146+
std::stringstream ss;
147+
ss << "to_string(" << result << "). Expected: " << expected;
148+
BEAST_EXPECTS(result == expected, ss.str());
149+
};
150+
151+
for (auto const mantissaSize :
152+
{MantissaRange::small, MantissaRange::large})
153+
{
154+
NumberMantissaScaleGuard mg(mantissaSize);
155+
156+
test(IOUAmount(-2, 0), "-2");
157+
test(IOUAmount(0, 0), "0");
158+
test(IOUAmount(2, 0), "2");
159+
test(IOUAmount(25, -3), "0.025");
160+
test(IOUAmount(-25, -3), "-0.025");
161+
test(IOUAmount(25, 1), "250");
162+
test(IOUAmount(-25, 1), "-250");
163+
test(
164+
IOUAmount(2, 20),
165+
mantissaSize == MantissaRange::small ? "2000000000000000e5"
166+
: "2000000000000000000e2");
167+
test(
168+
IOUAmount(-2, -20),
169+
mantissaSize == MantissaRange::small
170+
? "-2000000000000000e-35"
171+
: "-2000000000000000000e-38");
172+
}
153173
}
154174

155175
void

src/test/rpc/GetAggregatePrice_test.cpp

Lines changed: 51 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -191,18 +191,41 @@ class GetAggregatePrice_test : public beast::unit_test::suite
191191
// Aggregate data set includes all price oracle instances, no trimming
192192
// or time threshold
193193
{
194-
Env env(*this);
195-
OraclesData oracles;
196-
prep(env, oracles);
197-
// entire and trimmed stats
198-
auto ret = Oracle::aggregatePrice(env, "XRP", "USD", oracles);
199-
BEAST_EXPECT(ret[jss::entire_set][jss::mean] == "74.45");
200-
BEAST_EXPECT(ret[jss::entire_set][jss::size].asUInt() == 10);
201-
BEAST_EXPECT(
202-
ret[jss::entire_set][jss::standard_deviation] ==
203-
"0.3027650354097492");
204-
BEAST_EXPECT(ret[jss::median] == "74.45");
205-
BEAST_EXPECT(ret[jss::time] == 946694900);
194+
auto const all = testable_amendments();
195+
for (auto const& feats :
196+
{all - featureSingleAssetVault /* -
197+
featureLendingProtocol */
198+
,
199+
all})
200+
{
201+
for (auto const mantissaSize :
202+
{MantissaRange::small, MantissaRange::large})
203+
{
204+
// Regardless of the features enabled, RPC is controlled by
205+
// the global mantissa size. And since it's a thread-local,
206+
// overriding it locally won't make a difference either.
207+
// This will mean all RPC will use the default of "large".
208+
NumberMantissaScaleGuard mg(mantissaSize);
209+
210+
Env env(*this, feats);
211+
OraclesData oracles;
212+
prep(env, oracles);
213+
// entire and trimmed stats
214+
auto ret =
215+
Oracle::aggregatePrice(env, "XRP", "USD", oracles);
216+
BEAST_EXPECT(ret[jss::entire_set][jss::mean] == "74.45");
217+
BEAST_EXPECT(
218+
ret[jss::entire_set][jss::size].asUInt() == 10);
219+
// Short: 0.3027650354097492
220+
BEAST_EXPECTS(
221+
ret[jss::entire_set][jss::standard_deviation] ==
222+
"0.3027650354097491666",
223+
ret[jss::entire_set][jss::standard_deviation]
224+
.asString());
225+
BEAST_EXPECT(ret[jss::median] == "74.45");
226+
BEAST_EXPECT(ret[jss::time] == 946694900);
227+
}
228+
}
206229
}
207230

208231
// Aggregate data set includes all price oracle instances
@@ -215,15 +238,19 @@ class GetAggregatePrice_test : public beast::unit_test::suite
215238
Oracle::aggregatePrice(env, "XRP", "USD", oracles, 20, 100);
216239
BEAST_EXPECT(ret[jss::entire_set][jss::mean] == "74.45");
217240
BEAST_EXPECT(ret[jss::entire_set][jss::size].asUInt() == 10);
218-
BEAST_EXPECT(
241+
// Short: "0.3027650354097492",
242+
BEAST_EXPECTS(
219243
ret[jss::entire_set][jss::standard_deviation] ==
220-
"0.3027650354097492");
244+
"0.3027650354097491666",
245+
ret[jss::entire_set][jss::standard_deviation].asString());
221246
BEAST_EXPECT(ret[jss::median] == "74.45");
222247
BEAST_EXPECT(ret[jss::trimmed_set][jss::mean] == "74.45");
223248
BEAST_EXPECT(ret[jss::trimmed_set][jss::size].asUInt() == 6);
224-
BEAST_EXPECT(
249+
// Short: "0.187082869338697",
250+
BEAST_EXPECTS(
225251
ret[jss::trimmed_set][jss::standard_deviation] ==
226-
"0.187082869338697");
252+
"0.1870828693386970693",
253+
ret[jss::trimmed_set][jss::standard_deviation].asString());
227254
BEAST_EXPECT(ret[jss::time] == 946694900);
228255
}
229256

@@ -274,15 +301,19 @@ class GetAggregatePrice_test : public beast::unit_test::suite
274301
Oracle::aggregatePrice(env, "XRP", "USD", oracles, 20, "200");
275302
BEAST_EXPECT(ret[jss::entire_set][jss::mean] == "74.6");
276303
BEAST_EXPECT(ret[jss::entire_set][jss::size].asUInt() == 7);
277-
BEAST_EXPECT(
304+
// Short: 0.2160246899469287
305+
BEAST_EXPECTS(
278306
ret[jss::entire_set][jss::standard_deviation] ==
279-
"0.2160246899469287");
307+
"0.2160246899469286744",
308+
ret[jss::entire_set][jss::standard_deviation].asString());
280309
BEAST_EXPECT(ret[jss::median] == "74.6");
281310
BEAST_EXPECT(ret[jss::trimmed_set][jss::mean] == "74.6");
282311
BEAST_EXPECT(ret[jss::trimmed_set][jss::size].asUInt() == 5);
283-
BEAST_EXPECT(
312+
// Short: 0.158113883008419
313+
BEAST_EXPECTS(
284314
ret[jss::trimmed_set][jss::standard_deviation] ==
285-
"0.158113883008419");
315+
"0.1581138830084189666",
316+
ret[jss::trimmed_set][jss::standard_deviation].asString());
286317
BEAST_EXPECT(ret[jss::time] == 946694900);
287318
}
288319

src/xrpld/app/tx/detail/Transactor.cpp

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -592,24 +592,6 @@ Transactor::ticketDelete(
592592
return tesSUCCESS;
593593
}
594594

595-
bool
596-
Transactor::useOldNumberRules(TxType txType)
597-
{
598-
constexpr auto skipTransactions = std::to_array<TxType>(
599-
{ttAMM_BID,
600-
ttAMM_CLAWBACK,
601-
ttAMM_CREATE,
602-
ttAMM_DELETE,
603-
ttAMM_DEPOSIT,
604-
ttAMM_VOTE,
605-
ttAMM_WITHDRAW});
606-
607-
return std::find(
608-
std::begin(skipTransactions),
609-
std::end(skipTransactions),
610-
txType) != std::end(skipTransactions);
611-
}
612-
613595
// check stuff before you bother to lock the ledger
614596
void
615597
Transactor::preCompute()
@@ -1132,8 +1114,6 @@ Transactor::operator()()
11321114
// fixUniversalNumber predate the rulesGuard and should be replaced.
11331115
NumberSO stNumberSO{view().rules().enabled(fixUniversalNumber)};
11341116
CurrentTransactionRulesGuard currentTransctionRulesGuard(view().rules());
1135-
if (Transactor::useOldNumberRules(ctx_.tx.getTxnType()))
1136-
Number::setMantissaScale(MantissaRange::small);
11371117

11381118
#ifdef DEBUG
11391119
{

src/xrpld/app/tx/detail/Transactor.h

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -230,9 +230,6 @@ class Transactor
230230
uint256 const& ticketIndex,
231231
beast::Journal j);
232232

233-
static bool
234-
useOldNumberRules(TxType txType);
235-
236233
protected:
237234
TER
238235
apply();

src/xrpld/app/tx/detail/applySteps.cpp

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,15 +51,19 @@ with_txn_type(Rules const& rules, TxType txnType, F&& f)
5151
//
5252
std::optional<NumberSO> stNumberSO;
5353
std::optional<CurrentTransactionRulesGuard> rulesGuard;
54+
std::optional<NumberMantissaScaleGuard> mantissaScaleGuard;
5455
if (rules.enabled(featureSingleAssetVault) /*|| rules.enabled(featureLendingProtocol)*/)
5556
{
5657
// raii classes for the current ledger rules.
57-
// fixUniversalNumber predate the rulesGuard and should be replaced.
58+
// fixUniversalNumber predates the rulesGuard and should be replaced.
5859
stNumberSO.emplace(rules.enabled(fixUniversalNumber));
5960
rulesGuard.emplace(rules);
6061
}
61-
if (Transactor::useOldNumberRules(txnType))
62-
Number::setMantissaScale(MantissaRange::small);
62+
else
63+
{
64+
// Without those features enabled, always use the old number rules.
65+
mantissaScaleGuard.emplace(MantissaRange::small);
66+
}
6367

6468
switch (txnType)
6569
{

0 commit comments

Comments
 (0)