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 committed Mar 13, 2020
1 parent 27d5021 commit 47df91b
Show file tree
Hide file tree
Showing 4 changed files with 93 additions and 52 deletions.
2 changes: 1 addition & 1 deletion src/ripple/app/tx/impl/SetAccount.cpp
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
2 changes: 1 addition & 1 deletion src/ripple/protocol/Feature.h
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,7 @@ extern uint256 const featureCryptoConditionsSuite;
extern uint256 const retiredFix1373;
extern uint256 const retiredEnforceInvariants;
extern uint256 const retiredSortedDirectories;
extern uint256 const fix1201;
extern uint256 const retiredFix1201;
extern uint256 const retiredFix1512;
extern uint256 const fix1513;
extern uint256 const retiredFix1523;
Expand Down
2 changes: 1 addition & 1 deletion src/ripple/protocol/impl/Feature.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ uint256 const featureCryptoConditionsSuite = *getRegisteredFeature("CryptoCondit
uint256 const retiredFix1373 = *getRegisteredFeature("fix1373");
uint256 const retiredEnforceInvariants = *getRegisteredFeature("EnforceInvariants");
uint256 const retiredSortedDirectories = *getRegisteredFeature("SortedDirectories");
uint256 const fix1201 = *getRegisteredFeature("fix1201");
uint256 const retiredFix1201 = *getRegisteredFeature("fix1201");
uint256 const retiredFix1512 = *getRegisteredFeature("fix1512");
uint256 const fix1513 = *getRegisteredFeature("fix1513");
uint256 const retiredFix1523 = *getRegisteredFeature("fix1523");
Expand Down
139 changes: 90 additions & 49 deletions src/test/rpc/AccountSet_test.cpp
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,45 +303,36 @@ 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)

auto runTest = [this](double tr)
{
Account const alice ("alice");
Account const bob ("bob");
Account const gw ("gateway");
auto const USD = gw["USD"];

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

Expand All @@ -331,53 +341,82 @@ class AccountSet_test : public beast::unit_test::suite
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 with a variety of allowed transfer rates
for (double tr = 1.0; tr <= 2.0; tr += 0.03125)
{
runTest (tr);
}

// 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 tr : {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();
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, tr] (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>(tr * 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 (tr * 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 +457,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 47df91b

Please sign in to comment.