Skip to content

Commit

Permalink
Remove conditionals for fix1201 enabled 14Nov2017
Browse files Browse the repository at this point in the history
  • Loading branch information
@scottschurr
scottschurr committed Apr 9, 2020
1 parent 9f8d648 commit 0d83223
Show file tree
Hide file tree
Showing 4 changed files with 132 additions and 88 deletions.
2 changes: 1 addition & 1 deletion src/ripple/app/tx/impl/SetAccount.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ SetAccount::preflight (PreflightContext const& ctx)
return temBAD_TRANSFER_RATE;
}

if (ctx.rules.enabled(fix1201) && (uRate > 2 * QUALITY_ONE))
if (uRate > 2 * QUALITY_ONE)
{
JLOG(j.trace()) << "Malformed transaction: Transfer rate too large.";
return temBAD_TRANSFER_RATE;
Expand Down
27 changes: 15 additions & 12 deletions src/ripple/protocol/Feature.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -365,23 +365,11 @@ foreachFeature(FeatureBitset bs, F&& f)

extern uint256 const featureTickets;
extern uint256 const featureOwnerPaysFee;
extern uint256 const retiredPayChan;
extern uint256 const featureFlow;
extern uint256 const featureCompareTakerFlowCross;
extern uint256 const featureFlowCross;
extern uint256 const retiredCryptoConditions;
extern uint256 const retiredTickSize;
extern uint256 const retiredFix1368;
extern uint256 const retiredEscrow;
extern uint256 const featureCryptoConditionsSuite;
extern uint256 const retiredFix1373;
extern uint256 const retiredEnforceInvariants;
extern uint256 const retiredSortedDirectories;
extern uint256 const fix1201;
extern uint256 const retiredFix1512;
extern uint256 const fix1513;
extern uint256 const retiredFix1523;
extern uint256 const retiredFix1528;
extern uint256 const featureDepositAuth;
extern uint256 const featureChecks;
extern uint256 const fix1571;
Expand All @@ -400,6 +388,21 @@ extern uint256 const fixQualityUpperBound;
extern uint256 const featureRequireFullyCanonicalSig;
extern uint256 const fix1781;

// The following amendments have been active for at least two years.
// Their pre-amendment code has been removed.
extern uint256 const retiredPayChan;
extern uint256 const retiredCryptoConditions;
extern uint256 const retiredTickSize;
extern uint256 const retiredFix1368;
extern uint256 const retiredEscrow;
extern uint256 const retiredFix1373;
extern uint256 const retiredEnforceInvariants;
extern uint256 const retiredSortedDirectories;
extern uint256 const retiredFix1201;
extern uint256 const retiredFix1512;
extern uint256 const retiredFix1523;
extern uint256 const retiredFix1528;

} // ripple

#endif
27 changes: 15 additions & 12 deletions src/ripple/protocol/impl/Feature.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -156,23 +156,11 @@ uint256 bitsetIndexToFeature(size_t i)

uint256 const featureTickets = *getRegisteredFeature("Tickets");
uint256 const featureOwnerPaysFee = *getRegisteredFeature("OwnerPaysFee");
uint256 const retiredPayChan = *getRegisteredFeature("PayChan");
uint256 const featureFlow = *getRegisteredFeature("Flow");
uint256 const featureCompareTakerFlowCross = *getRegisteredFeature("CompareTakerFlowCross");
uint256 const featureFlowCross = *getRegisteredFeature("FlowCross");
uint256 const retiredCryptoConditions = *getRegisteredFeature("CryptoConditions");
uint256 const retiredTickSize = *getRegisteredFeature("TickSize");
uint256 const retiredFix1368 = *getRegisteredFeature("fix1368");
uint256 const retiredEscrow = *getRegisteredFeature("Escrow");
uint256 const featureCryptoConditionsSuite = *getRegisteredFeature("CryptoConditionsSuite");
uint256 const retiredFix1373 = *getRegisteredFeature("fix1373");
uint256 const retiredEnforceInvariants = *getRegisteredFeature("EnforceInvariants");
uint256 const retiredSortedDirectories = *getRegisteredFeature("SortedDirectories");
uint256 const fix1201 = *getRegisteredFeature("fix1201");
uint256 const retiredFix1512 = *getRegisteredFeature("fix1512");
uint256 const fix1513 = *getRegisteredFeature("fix1513");
uint256 const retiredFix1523 = *getRegisteredFeature("fix1523");
uint256 const retiredFix1528 = *getRegisteredFeature("fix1528");
uint256 const featureDepositAuth = *getRegisteredFeature("DepositAuth");
uint256 const featureChecks = *getRegisteredFeature("Checks");
uint256 const fix1571 = *getRegisteredFeature("fix1571");
Expand All @@ -191,4 +179,19 @@ uint256 const fixQualityUpperBound = *getRegisteredFeature("fixQualityUpperBound
uint256 const featureRequireFullyCanonicalSig = *getRegisteredFeature("RequireFullyCanonicalSig");
uint256 const fix1781 = *getRegisteredFeature("fix1781");

// The following amendments have been active for at least two years.
// Their pre-amendment code has been removed.
uint256 const retiredPayChan = *getRegisteredFeature("PayChan");
uint256 const retiredCryptoConditions = *getRegisteredFeature("CryptoConditions");
uint256 const retiredTickSize = *getRegisteredFeature("TickSize");
uint256 const retiredFix1368 = *getRegisteredFeature("fix1368");
uint256 const retiredEscrow = *getRegisteredFeature("Escrow");
uint256 const retiredFix1373 = *getRegisteredFeature("fix1373");
uint256 const retiredEnforceInvariants = *getRegisteredFeature("EnforceInvariants");
uint256 const retiredSortedDirectories = *getRegisteredFeature("SortedDirectories");
uint256 const retiredFix1201 = *getRegisteredFeature("fix1201");
uint256 const retiredFix1512 = *getRegisteredFeature("fix1512");
uint256 const retiredFix1523 = *getRegisteredFeature("fix1523");
uint256 const retiredFix1528 = *getRegisteredFeature("fix1528");

} // ripple
164 changes: 101 additions & 63 deletions src/test/rpc/AccountSet_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ class AccountSet_test : public beast::unit_test::suite

void testNullAccountSet()
{
testcase ("No AccountSet");

using namespace test::jtx;
Env env(*this);
Account const alice ("alice");
Expand All @@ -44,6 +46,8 @@ class AccountSet_test : public beast::unit_test::suite

void testMostFlags()
{
testcase ("Most Flags");

using namespace test::jtx;
Account const alice ("alice");

Expand Down Expand Up @@ -112,6 +116,8 @@ class AccountSet_test : public beast::unit_test::suite

void testSetAndResetAccountTxnID()
{
testcase ("Set and reset AccountTxnID");

using namespace test::jtx;
Env env(*this);
Account const alice ("alice");
Expand All @@ -132,6 +138,8 @@ class AccountSet_test : public beast::unit_test::suite

void testSetNoFreeze()
{
testcase ("Set NoFreeze");

using namespace test::jtx;
Env env(*this);
Account const alice ("alice");
Expand All @@ -150,6 +158,8 @@ class AccountSet_test : public beast::unit_test::suite

void testDomain()
{
testcase ("Domain");

using namespace test::jtx;
Env env(*this);
Account const alice ("alice");
Expand Down Expand Up @@ -197,6 +207,8 @@ class AccountSet_test : public beast::unit_test::suite

void testMessageKey()
{
testcase ("MessageKey");

using namespace test::jtx;
Env env(*this);
Account const alice ("alice");
Expand All @@ -219,6 +231,8 @@ class AccountSet_test : public beast::unit_test::suite

void testWalletID()
{
testcase ("WalletID");

using namespace test::jtx;
Env env(*this);
Account const alice ("alice");
Expand All @@ -237,6 +251,8 @@ class AccountSet_test : public beast::unit_test::suite

void testEmailHash()
{
testcase ("EmailHash");

using namespace test::jtx;
Env env(*this);
Account const alice ("alice");
Expand All @@ -262,6 +278,8 @@ class AccountSet_test : public beast::unit_test::suite
double get;
};

testcase ("TransferRate");

using namespace test::jtx;
auto doTests = [this] (FeatureBitset const& features,
std::initializer_list<test_results> testData)
Expand All @@ -274,6 +292,7 @@ class AccountSet_test : public beast::unit_test::suite
for (auto const& r : testData)
{
env(rate(alice, r.set), ter(r.code));
env.close();

// If the field is not present expect the default value
if (!(*env.le(alice))[~sfTransferRate])
Expand All @@ -284,100 +303,117 @@ class AccountSet_test : public beast::unit_test::suite
}
};

{
testcase ("Setting transfer rate (without fix1201)");
doTests (supported_amendments().reset(fix1201),
{
{ 1.0, tesSUCCESS, 1.0 },
{ 1.1, tesSUCCESS, 1.1 },
{ 2.0, tesSUCCESS, 2.0 },
{ 2.1, tesSUCCESS, 2.1 },
{ 0.0, tesSUCCESS, 1.0 },
{ 2.0, tesSUCCESS, 2.0 },
{ 0.9, temBAD_TRANSFER_RATE, 2.0 }});
}

{
testcase ("Setting transfer rate (with fix1201)");
doTests (supported_amendments(),
{
{ 1.0, tesSUCCESS, 1.0 },
{ 1.1, tesSUCCESS, 1.1 },
{ 2.0, tesSUCCESS, 2.0 },
{ 2.1, temBAD_TRANSFER_RATE, 2.0 },
{ 0.0, tesSUCCESS, 1.0 },
{ 2.0, tesSUCCESS, 2.0 },
{ 0.9, temBAD_TRANSFER_RATE, 2.0 }});
}
doTests (supported_amendments(),
{
{ 1.0, tesSUCCESS, 1.0 },
{ 1.1, tesSUCCESS, 1.1 },
{ 2.0, tesSUCCESS, 2.0 },
{ 2.1, temBAD_TRANSFER_RATE, 2.0 },
{ 0.0, tesSUCCESS, 1.0 },
{ 2.0, tesSUCCESS, 2.0 },
{ 0.9, temBAD_TRANSFER_RATE, 2.0 }
});
}

void testGateway()
{
testcase ("Gateway");

using namespace test::jtx;
auto runTest = [](Env&& env, double tr)
{
Account const alice ("alice");
Account const bob ("bob");
Account const gw ("gateway");
auto const USD = gw["USD"];

Account const alice ("alice");
Account const bob ("bob");
Account const gw ("gateway");
auto const USD = gw["USD"];

// Test gateway with a variety of allowed transfer rates
for (double transferRate = 1.0;
transferRate <= 2.0; transferRate += 0.03125)
{
Env env (*this);
env.fund(XRP(10000), gw, alice, bob);
env.trust(USD(3), alice, bob);
env(rate(gw, tr));
env.close();
env.trust(USD(10), alice, bob);
env.close();
env(rate(gw, transferRate));
env.close();

auto const amount = USD(1);
Rate const rate (tr * QUALITY_ONE);
Rate const rate (transferRate * QUALITY_ONE);
auto const amountWithRate =
toAmount<STAmount> (multiply(amount.value(), rate));

env(pay(gw, alice, USD(3)));
env(pay(alice, bob, USD(1)), sendmax(USD(3)));
env(pay(gw, alice, USD(10)));
env.close();
env(pay(alice, bob, USD(1)), sendmax(USD(10)));
env.close();

env.require(balance(alice, USD(3) - amountWithRate));
env.require(balance(alice, USD(10) - amountWithRate));
env.require(balance(bob, USD(1)));
};

// Test gateway with allowed transfer rates
auto const noFix1201 = supported_amendments().reset(fix1201);
runTest (Env{*this, noFix1201}, 1.02);
runTest (Env{*this, noFix1201}, 1);
runTest (Env{*this, noFix1201}, 2);
runTest (Env{*this, noFix1201}, 2.1);
runTest (Env{*this, supported_amendments()}, 1.02);
runTest (Env{*this, supported_amendments()}, 2);
}

// Test gateway when amendment is set after transfer rate
// Since fix1201 was enabled on Nov 14 2017 a rate in excess of
// 2.0 has been blocked by the transactor. But there are a few
// accounts on the MainNet that have larger-than-currently-allowed
// TransferRates. We'll bypass the transactor so we can check
// operation of these legacy TransferRates.
//
// Two out-of-bound values are currently in the ledger (March 2020)
// They are 4.0 and 4.294967295. So those are the values we test.
for (double transferRate : {4.0, 4.294967295})
{
Env env (*this, noFix1201);
Account const alice ("alice");
Account const bob ("bob");
Account const gw ("gateway");
auto const USD = gw["USD"];
double const tr = 2.75;

Env env (*this);
env.fund(XRP(10000), gw, alice, bob);
env.trust(USD(3), alice, bob);
env(rate(gw, tr));
env.close();
env.enableFeature(fix1201);
env.trust(USD(10), alice, bob);
env.close();

// We'd like to use transferRate here, but the transactor
// blocks transfer rates that large. So we use an acceptable
// transfer rate here and later hack the ledger to replace
// the acceptable value with an out-of-bounds value.
env(rate(gw, 2.0));
env.close();

// Note that we're bypassing almost all of the ledger's safety
// checks with this modify() call. If you call close() between
// here and the end of the test all the effort will be lost.
env.app().openLedger().modify(
[&gw, transferRate] (OpenView& view, beast::Journal j)
{
// Get the account root we want to hijack.
auto const sle = view.read (keylet::account(gw.id()));
if (! sle)
return false; // This would be really surprising!

// We'll insert a replacement for the account root
// with the higher (currently invalid) transfer rate.
auto replacement =
std::make_shared<SLE>(*sle, sle->key());
(*replacement)[sfTransferRate] =
static_cast<std::uint32_t>(transferRate * QUALITY_ONE);
view.rawReplace (replacement);
return true;
});

auto const amount = USD(1);
Rate const rate (tr * QUALITY_ONE);
auto const amountWithRate =
toAmount<STAmount> (multiply(amount.value(), rate));
toAmount<STAmount> (
multiply(amount.value(),
Rate (transferRate * QUALITY_ONE)));

env(pay(gw, alice, USD(3)));
env(pay(alice, bob, amount), sendmax(USD(3)));
env(pay(gw, alice, USD(10)));
env(pay(alice, bob, amount), sendmax(USD(10)));

env.require(balance(alice, USD(3) - amountWithRate));
env.require(balance(alice, USD(10) - amountWithRate));
env.require(balance(bob, amount));
}
}

void testBadInputs()
{
testcase ("Bad inputs");

using namespace test::jtx;
Env env (*this);
Account const alice ("alice");
Expand Down Expand Up @@ -418,6 +454,8 @@ class AccountSet_test : public beast::unit_test::suite

void testRequireAuthWithDir()
{
testcase ("Require auth");

using namespace test::jtx;
Env env(*this);
Account const alice ("alice");
Expand Down

0 comments on commit 0d83223

Please sign in to comment.