Skip to content

Commit

Permalink
Fix failing unit-test. Remove code/tests for FlowCross disabled
Browse files Browse the repository at this point in the history
  • Loading branch information
@gregtatcam
gregtatcam committed Sep 26, 2024
1 parent 62fa17d commit b442ede
Show file tree
Hide file tree
Showing 25 changed files with 428 additions and 454 deletions.
23 changes: 23 additions & 0 deletions include/xrpl/protocol/Asset.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,9 @@ class Asset

friend constexpr std::weak_ordering
operator<=>(Asset const& lhs, Asset const& rhs);

friend constexpr bool
equalCurrencyOrMPTID(Asset const& lhs, Asset const& rhs);
};

template <ValidIssueType TIss>
Expand Down Expand Up @@ -168,6 +171,26 @@ operator<=>(Asset const& lhs, Asset const& rhs)
rhs.value());
}

constexpr bool
equalCurrencyOrMPTID(Asset const& lhs, Asset const& rhs)
{
return std::visit(
[&]<typename TLhs, typename TRhs>(
TLhs const& issLhs, TRhs const& issRhs) {
if constexpr (
std::is_same_v<TLhs, Issue> && std::is_same_v<TRhs, Issue>)
return issLhs.currency == issRhs.currency;
else if constexpr (
std::is_same_v<TLhs, MPTIssue> &&
std::is_same_v<TRhs, MPTIssue>)
return issLhs.getMptID() == issRhs.getMptID();
else
return false;
},
lhs.value(),
rhs.value());
}

inline bool
isXRP(Asset const& asset)
{
Expand Down
39 changes: 8 additions & 31 deletions src/test/app/CrossingLimits_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,8 @@ class CrossingLimits_test : public beast::unit_test::suite
auto const gw = Account("gateway");
auto const USD = gw["USD"];

// The number of allowed offers to cross is different between
// Taker and FlowCross. Taker allows 850 and FlowCross allows 1000.
// Accommodate that difference in the test.
int const maxConsumed = features[featureFlowCross] ? 1000 : 850;
// FlowCross allows 1000.
int const maxConsumed = 1000;

env.fund(XRP(100000000), gw, "alice", "bob", "carol");
int const bobsOfferCount = maxConsumed + 150;
Expand Down Expand Up @@ -118,11 +116,8 @@ class CrossingLimits_test : public beast::unit_test::suite

env.fund(XRP(100000000), gw, "alice", "bob", "carol", "dan", "evita");

// The number of offers allowed to cross is different between
// Taker and FlowCross. Taker allows 850 and FlowCross allows 1000.
// Accommodate that difference in the test.
bool const isFlowCross{features[featureFlowCross]};
int const maxConsumed = isFlowCross ? 1000 : 850;
// FlowCross allows 1000.
int const maxConsumed = 1000;

int const evitasOfferCount{maxConsumed + 49};
env.trust(USD(1000), "alice");
Expand All @@ -132,14 +127,11 @@ class CrossingLimits_test : public beast::unit_test::suite
env.trust(USD(evitasOfferCount + 1), "evita");
env(pay(gw, "evita", USD(evitasOfferCount + 1)));

// Taker and FlowCross have another difference we must accommodate.
// Taker allows a total of 1000 unfunded offers to be consumed
// beyond the 850 offers it can take. FlowCross draws no such
// distinction; its limit is 1000 funded or unfunded.
// FlowCross limit is 1000 funded or unfunded.
//
// Give carol an extra 150 (unfunded) offers when we're using Taker
// to accommodate that difference.
int const carolsOfferCount{isFlowCross ? 700 : 850};
int const carolsOfferCount{700};
n_offers(env, 400, "alice", XRP(1), USD(1));
n_offers(env, carolsOfferCount, "carol", XRP(1), USD(1));
n_offers(env, evitasOfferCount, "evita", XRP(1), USD(1));
Expand Down Expand Up @@ -454,21 +446,10 @@ class CrossingLimits_test : public beast::unit_test::suite
void
testAutoBridgedLimits(FeatureBitset features)
{
// Taker and FlowCross are too different in the way they handle
// autobridging to make one test suit both approaches.
//
// o Taker alternates between books, completing one full increment
// before returning to make another pass.
//
// o FlowCross extracts as much as possible in one book at one Quality
// before proceeding to the other book. This reduces the number of
// times we change books.
//
// So the tests for the two forms of autobridging are separate.
if (features[featureFlowCross])
testAutoBridgedLimitsFlowCross(features);
else
testAutoBridgedLimitsTaker(features);
testAutoBridgedLimitsFlowCross(features);
}

void
Expand Down Expand Up @@ -521,11 +502,10 @@ class CrossingLimits_test : public beast::unit_test::suite
n_offers(env, 998, alice, XRP(0.96), USD(1));
n_offers(env, 998, alice, XRP(0.95), USD(1));

bool const withFlowCross = features[featureFlowCross];
bool const withSortStrands = features[featureFlowSortStrands];

auto const expectedTER = [&]() -> TER {
if (withFlowCross && !withSortStrands)
if (!withSortStrands)
return TER{tecOVERSIZE};
return tesSUCCESS;
}();
Expand All @@ -534,8 +514,6 @@ class CrossingLimits_test : public beast::unit_test::suite
env.close();

auto const expectedUSD = [&] {
if (!withFlowCross)
return USD(850);
if (!withSortStrands)
return USD(0);
return USD(1996);
Expand All @@ -558,7 +536,6 @@ class CrossingLimits_test : public beast::unit_test::suite
auto const sa = supported_amendments();
testAll(sa);
testAll(sa - featureFlowSortStrands);
testAll(sa - featureFlowCross - featureFlowSortStrands);
}
};

Expand Down
1 change: 0 additions & 1 deletion src/test/app/DeliverMin_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,6 @@ class DeliverMin_test : public beast::unit_test::suite
{
using namespace jtx;
auto const sa = supported_amendments();
test_convert_all_of_an_asset(sa - featureFlowCross);
test_convert_all_of_an_asset(sa);
}
};
Expand Down
1 change: 0 additions & 1 deletion src/test/app/Discrepancy_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,6 @@ class Discrepancy_test : public beast::unit_test::suite
{
using namespace test::jtx;
auto const sa = supported_amendments();
testXRPDiscrepancy(sa - featureFlowCross);
testXRPDiscrepancy(sa);
}
};
Expand Down
4 changes: 0 additions & 4 deletions src/test/app/Flow_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1435,7 +1435,6 @@ struct Flow_test : public beast::unit_test::suite

using namespace jtx;
auto const sa = supported_amendments();
testWithFeats(sa - featureFlowCross);
testWithFeats(sa);
testEmptyStrand(sa);
}
Expand All @@ -1448,11 +1447,8 @@ struct Flow_manual_test : public Flow_test
{
using namespace jtx;
auto const all = supported_amendments();
FeatureBitset const flowCross{featureFlowCross};
FeatureBitset const f1513{fix1513};

testWithFeats(all - flowCross - f1513);
testWithFeats(all - flowCross);
testWithFeats(all - f1513);
testWithFeats(all);

Expand Down
1 change: 0 additions & 1 deletion src/test/app/Freeze_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -518,7 +518,6 @@ class Freeze_test : public beast::unit_test::suite
};
using namespace test::jtx;
auto const sa = supported_amendments();
testAll(sa - featureFlowCross);
testAll(sa);
}
};
Expand Down
83 changes: 15 additions & 68 deletions src/test/app/MPToken_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1059,18 +1059,27 @@ class MPToken_test : public beast::unit_test::suite
using namespace test::jtx;

std::set<std::string> txWithAmounts;
std::set<std::string> supportedTx = {
jss::Clawback.c_str(),
jss::SetFee.c_str(),
jss::Payment.c_str(),
jss::OfferCreate.c_str(),
jss::Payment.c_str(),
jss::AMMCreate.c_str(),
jss::CheckCreate.c_str(),
jss::CheckCash.c_str()};
for (auto const& format : TxFormats::getInstance())
{
for (auto const& e : format.getSOTemplate())
{
// Transaction has amount fields.
// Exclude Clawback, which only supports sfAmount and is checked
// in the transactor for amendment enable/disable. Exclude
// pseudo-transaction SetFee. Don't consider the Fee field since
// Exclude transactions supporting MPT.
// Exclude pseudo-transaction SetFee.
// Don't consider the Fee field since
// it's included in every transaction.
if (e.supportMPT() != soeMPTNone &&
e.sField().getName() != jss::Fee &&
format.getName() != jss::Clawback &&
!supportedTx.contains(format.getName()) &&
format.getName() != jss::SetFee)
{
txWithAmounts.insert(format.getName());
Expand Down Expand Up @@ -1111,22 +1120,6 @@ class MPToken_test : public beast::unit_test::suite
// All transactions with sfAmount, which don't support MPT
// and transactions with amount fields, which can't be MPT

// AMMCreate
auto ammCreate = [&](SField const& field) {
Json::Value jv;
jv[jss::TransactionType] = jss::AMMCreate;
jv[jss::Account] = alice.human();
jv[jss::Amount] = (field.fieldName == sfAmount.fieldName)
? mpt.getJson(JsonOptions::none)
: "100000000";
jv[jss::Amount2] = (field.fieldName == sfAmount2.fieldName)
? mpt.getJson(JsonOptions::none)
: "100000000";
jv[jss::TradingFee] = 0;
test(jv);
};
ammCreate(sfAmount);
ammCreate(sfAmount2);
// AMMDeposit
auto ammDeposit = [&](SField const& field) {
Json::Value jv;
Expand All @@ -1138,11 +1131,8 @@ class MPToken_test : public beast::unit_test::suite
jv[jss::Flags] = tfSingleAsset;
test(jv);
};
ammDeposit(sfAmount);
for (SField const& field :
{std::ref(sfAmount2),
std::ref(sfEPrice),
std::ref(sfLPTokenOut)})
{std::ref(sfEPrice), std::ref(sfLPTokenOut)})
ammDeposit(field);
// AMMWithdraw
auto ammWithdraw = [&](SField const& field) {
Expand All @@ -1155,11 +1145,8 @@ class MPToken_test : public beast::unit_test::suite
jv[field.fieldName] = mpt.getJson(JsonOptions::none);
test(jv);
};
ammWithdraw(sfAmount);
for (SField const& field :
{std::ref(sfAmount2),
std::ref(sfEPrice),
std::ref(sfLPTokenIn)})
{std::ref(sfEPrice), std::ref(sfLPTokenIn)})
ammWithdraw(field);
// AMMBid
auto ammBid = [&](SField const& field) {
Expand All @@ -1173,26 +1160,6 @@ class MPToken_test : public beast::unit_test::suite
};
ammBid(sfBidMin);
ammBid(sfBidMax);
// CheckCash
auto checkCash = [&](SField const& field) {
Json::Value jv;
jv[jss::TransactionType] = jss::CheckCash;
jv[jss::Account] = alice.human();
jv[sfCheckID.fieldName] = to_string(uint256{1});
jv[field.fieldName] = mpt.getJson(JsonOptions::none);
test(jv);
};
checkCash(sfAmount);
checkCash(sfDeliverMin);
// CheckCreate
{
Json::Value jv;
jv[jss::TransactionType] = jss::CheckCreate;
jv[jss::Account] = alice.human();
jv[jss::Destination] = carol.human();
jv[jss::SendMax] = mpt.getJson(JsonOptions::none);
test(jv);
}
// EscrowCreate
{
Json::Value jv;
Expand All @@ -1202,11 +1169,6 @@ class MPToken_test : public beast::unit_test::suite
jv[jss::Amount] = mpt.getJson(JsonOptions::none);
test(jv);
}
// OfferCreate
{
Json::Value const jv = offer(alice, USD(100), mpt);
test(jv);
}
// PaymentChannelCreate
{
Json::Value jv;
Expand Down Expand Up @@ -1236,21 +1198,6 @@ class MPToken_test : public beast::unit_test::suite
jv[jss::Amount] = mpt.getJson(JsonOptions::none);
test(jv);
}
// Payment
auto payment = [&](SField const& field) {
Json::Value jv;
jv[jss::TransactionType] = jss::Payment;
jv[jss::Account] = alice.human();
jv[jss::Destination] = carol.human();
jv[jss::Amount] = mpt.getJson(JsonOptions::none);
if (field == sfSendMax)
jv[jss::SendMax] = mpt.getJson(JsonOptions::none);
else
jv[jss::DeliverMin] = mpt.getJson(JsonOptions::none);
test(jv);
};
payment(sfSendMax);
payment(sfDeliverMin);
// NFTokenCreateOffer
{
Json::Value jv;
Expand Down
Loading

0 comments on commit b442ede

Please sign in to comment.