Skip to content

Commit

Permalink
Enforce TransferRate Maximum (RIPD-1201):
Browse files Browse the repository at this point in the history 
Sets a maximum TransferRate value of 100%. Squashes any
requested TransferRate over the limit to the max value.

This change requires an amendment ("fix1201") in rippled.

Adds test functionality for enabling an amendment mid-test.
Additionally, adds tests utilizing varying transfer rates both
with and without the amendment enabled.
  • Loading branch information
@xasos @nbougalis
xasos authored and nbougalis committed Aug 10, 2017
1 parent cca574c commit 35d81e6
Show file tree
Hide file tree
Showing 8 changed files with 146 additions and 21 deletions.
3 changes: 2 additions & 1 deletion src/ripple/app/main/Amendments.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@ supportedAmendments ()
{ "42EEA5E28A97824821D4EF97081FE36A54E9593C6E4F20CBAE098C69D2E072DC fix1373" },
{ "DC9CA96AEA1DCF83E527D1AFC916EFAF5D27388ECA4060A88817C1238CAEE0BF EnforceInvariants" },
{ "3012E8230864E95A58C60FD61430D7E1B4D3353195F2981DC12B0C7C0950FFAC FlowCross" },
{ "CC5ABAE4F3EC92E94A59B1908C2BE82D2228B6485C00AFF8F22DF930D89C194E SortedDirectories" }
{ "CC5ABAE4F3EC92E94A59B1908C2BE82D2228B6485C00AFF8F22DF930D89C194E SortedDirectories" },
{ "B4D44CC3111ADD964E846FC57760C8B50FFCD5A82C86A72756F6B058DDDF96AD fix1201" }
};
}

Expand Down
10 changes: 8 additions & 2 deletions src/ripple/app/tx/impl/SetAccount.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,13 @@ SetAccount::preflight (PreflightContext const& ctx)

if (uRate && (uRate < QUALITY_ONE))
{
JLOG(j.trace()) << "Malformed transaction: Bad transfer rate.";
JLOG(j.trace()) << "Malformed transaction: Transfer rate too small.";
return temBAD_TRANSFER_RATE;
}

if (ctx.rules.enabled(fix1201) && (uRate > 2 * QUALITY_ONE))
{
JLOG(j.trace()) << "Malformed transaction: Transfer rate too large.";
return temBAD_TRANSFER_RATE;
}
}
Expand Down Expand Up @@ -453,7 +459,7 @@ SetAccount::doApply ()
JLOG(j_.trace()) << "unset transfer rate";
sle->makeFieldAbsent (sfTransferRate);
}
else if (uRate > QUALITY_ONE)
else
{
JLOG(j_.trace()) << "set transfer rate";
sle->setFieldU32 (sfTransferRate, uRate);
Expand Down
4 changes: 3 additions & 1 deletion src/ripple/protocol/Feature.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,8 @@ class FeatureCollections
"CryptoConditionsSuite",
"fix1373",
"EnforceInvariants",
"SortedDirectories"};
"SortedDirectories",
"fix1201"};

std::vector<uint256> features;
boost::container::flat_map<uint256, std::size_t> featureToIndex;
Expand Down Expand Up @@ -160,6 +161,7 @@ extern uint256 const featureCryptoConditionsSuite;
extern uint256 const fix1373;
extern uint256 const featureEnforceInvariants;
extern uint256 const featureSortedDirectories;
extern uint256 const fix1201;

} // ripple

Expand Down
1 change: 1 addition & 0 deletions src/ripple/protocol/impl/Feature.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,5 +114,6 @@ uint256 const featureCryptoConditionsSuite = *getRegisteredFeature("CryptoCondit
uint256 const fix1373 = *getRegisteredFeature("fix1373");
uint256 const featureEnforceInvariants = *getRegisteredFeature("EnforceInvariants");
uint256 const featureSortedDirectories = *getRegisteredFeature("SortedDirectories");
uint256 const fix1201 = *getRegisteredFeature("fix1201");

} // ripple
2 changes: 1 addition & 1 deletion src/ripple/protocol/impl/TER.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ transResults()
{ temBAD_SIGNATURE, { "temBAD_SIGNATURE", "Malformed: Bad signature." } },
{ temBAD_SIGNER, { "temBAD_SIGNER", "Malformed: No signer may duplicate account or other signers." } },
{ temBAD_SRC_ACCOUNT, { "temBAD_SRC_ACCOUNT", "Malformed: Bad source account." } },
{ temBAD_TRANSFER_RATE, { "temBAD_TRANSFER_RATE", "Malformed: Transfer rate must be >= 1.0" } },
{ temBAD_TRANSFER_RATE, { "temBAD_TRANSFER_RATE", "Malformed: Transfer rate must be >= 1.0 and <= 2.0" } },
{ temBAD_WEIGHT, { "temBAD_WEIGHT", "Malformed: Weight must be a positive value." } },
{ temDST_IS_SRC, { "temDST_IS_SRC", "Destination may not be source." } },
{ temDST_NEEDED, { "temDST_NEEDED", "Destination not specified." } },
Expand Down
3 changes: 3 additions & 0 deletions src/test/jtx/Env.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -624,6 +624,9 @@ class Env
std::shared_ptr<STTx const>
tx() const;

void
enableFeature(uint256 const feature);

private:
void
fund (bool setDefaultRipple,
Expand Down
8 changes: 8 additions & 0 deletions src/test/jtx/impl/Env.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -492,6 +492,14 @@ Env::do_rpc(std::vector<std::string> const& args)
return response;
}

void
Env::enableFeature(uint256 const feature)
{
// Env::close() must be called for feature
// enable to take place.
app().config().features.insert(feature);
}

} // jtx

} // test
Expand Down
136 changes: 120 additions & 16 deletions src/test/rpc/AccountSet_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,13 @@
*/
//==============================================================================

#include <ripple/protocol/JsonFields.h>
#include <ripple/basics/StringUtilities.h>
#include <ripple/protocol/AmountConversions.h>
#include <ripple/protocol/Feature.h>
#include <ripple/protocol/JsonFields.h>
#include <ripple/protocol/Quality.h>
#include <ripple/protocol/Rate.h>
#include <test/jtx.h>
#include <ripple/basics/StringUtilities.h>

namespace ripple {

Expand Down Expand Up @@ -203,24 +206,124 @@ class AccountSet_test : public beast::unit_test::suite

void testTransferRate()
{
struct test_results
{
double set;
TER code;
double get;
};

using namespace test::jtx;
Env env(*this);
Account const alice ("alice");
env.fund(XRP(10000), alice);
auto jt = noop(alice);
auto doTests = [this] (FeatureBitset const& features,
std::initializer_list<test_results> testData)
{
Env env (*this, features);

uint32 xfer_rate = 2000000000;
jt[sfTransferRate.fieldName] = xfer_rate;
env(jt);
BEAST_EXPECT((*env.le(alice))[ sfTransferRate ] == xfer_rate);
Account const alice ("alice");
env.fund(XRP(10000), alice);

jt[sfTransferRate.fieldName] = 0u;
env(jt);
BEAST_EXPECT(! env.le(alice)->isFieldPresent(sfTransferRate));
for (auto const& r : testData)
{
env(rate(alice, r.set), ter(r.code));

// If the field is not present expect the default value
if (!(*env.le(alice))[~sfTransferRate])
BEAST_EXPECT(r.get == 1.0);
else
BEAST_EXPECT(*(*env.le(alice))[~sfTransferRate] ==
r.get * QUALITY_ONE);
}
};

{
testcase ("Setting transfer rate (without fix1201)");
doTests (all_features_except(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 }});
}

// set a bad value (< QUALITY_ONE)
jt[sfTransferRate.fieldName] = 10u;
env(jt, ter(temBAD_TRANSFER_RATE));
{
testcase ("Setting transfer rate (with fix1201)");
doTests (all_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()
{
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"];

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

auto const amount = USD(1);
Rate const rate (tr * 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.require(balance(alice, USD(3) - amountWithRate));
env.require(balance(bob, USD(1)));
};

// Test gateway with allowed transfer rates
runTest (Env{*this, all_features_except(fix1201)}, 1.02);
runTest (Env{*this, all_features_except(fix1201)}, 1);
runTest (Env{*this, all_features_except(fix1201)}, 2);
runTest (Env{*this, all_features_except(fix1201)}, 2.1);
runTest (Env{*this, all_amendments()}, 1.02);
runTest (Env{*this, all_amendments()}, 2);

// Test gateway when amendment is set after transfer rate
{
Env env (*this, all_features_except(fix1201));
Account const alice ("alice");
Account const bob ("bob");
Account const gw ("gateway");
auto const USD = gw["USD"];
double const tr = 2.75;

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

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

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

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

void testBadInputs(bool withFeatures)
Expand Down Expand Up @@ -303,6 +406,7 @@ class AccountSet_test : public beast::unit_test::suite
testSetAndResetAccountTxnID();
testSetNoFreeze();
testDomain();
testGateway();
testMessageKey();
testWalletID();
testEmailHash();
Expand Down

0 comments on commit 35d81e6

Please sign in to comment.