diff --git a/src/ripple/app/main/Amendments.cpp b/src/ripple/app/main/Amendments.cpp index c286e927bc..c64997ee90 100644 --- a/src/ripple/app/main/Amendments.cpp +++ b/src/ripple/app/main/Amendments.cpp @@ -56,7 +56,8 @@ supportedAmendments () { "42EEA5E28A97824821D4EF97081FE36A54E9593C6E4F20CBAE098C69D2E072DC fix1373" }, { "DC9CA96AEA1DCF83E527D1AFC916EFAF5D27388ECA4060A88817C1238CAEE0BF EnforceInvariants" }, { "3012E8230864E95A58C60FD61430D7E1B4D3353195F2981DC12B0C7C0950FFAC FlowCross" }, - { "CC5ABAE4F3EC92E94A59B1908C2BE82D2228B6485C00AFF8F22DF930D89C194E SortedDirectories" } + { "CC5ABAE4F3EC92E94A59B1908C2BE82D2228B6485C00AFF8F22DF930D89C194E SortedDirectories" }, + { "B4D44CC3111ADD964E846FC57760C8B50FFCD5A82C86A72756F6B058DDDF96AD fix1201" } }; } diff --git a/src/ripple/app/tx/impl/SetAccount.cpp b/src/ripple/app/tx/impl/SetAccount.cpp index 5b87239108..83408704c9 100644 --- a/src/ripple/app/tx/impl/SetAccount.cpp +++ b/src/ripple/app/tx/impl/SetAccount.cpp @@ -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; } } @@ -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); diff --git a/src/ripple/protocol/Feature.h b/src/ripple/protocol/Feature.h index 219ea3d8ab..4085bd9e84 100644 --- a/src/ripple/protocol/Feature.h +++ b/src/ripple/protocol/Feature.h @@ -67,7 +67,8 @@ class FeatureCollections "CryptoConditionsSuite", "fix1373", "EnforceInvariants", - "SortedDirectories"}; + "SortedDirectories", + "fix1201"}; std::vector features; boost::container::flat_map featureToIndex; @@ -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 diff --git a/src/ripple/protocol/impl/Feature.cpp b/src/ripple/protocol/impl/Feature.cpp index 129e082eaa..37b795f0c8 100644 --- a/src/ripple/protocol/impl/Feature.cpp +++ b/src/ripple/protocol/impl/Feature.cpp @@ -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 diff --git a/src/ripple/protocol/impl/TER.cpp b/src/ripple/protocol/impl/TER.cpp index 0ee0bc27e4..b3a5269c08 100644 --- a/src/ripple/protocol/impl/TER.cpp +++ b/src/ripple/protocol/impl/TER.cpp @@ -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." } }, diff --git a/src/test/jtx/Env.h b/src/test/jtx/Env.h index c0a7dc9047..f3f0c3fd1b 100644 --- a/src/test/jtx/Env.h +++ b/src/test/jtx/Env.h @@ -624,6 +624,9 @@ public: std::shared_ptr tx() const; + void + enableFeature(uint256 const feature); + private: void fund (bool setDefaultRipple, diff --git a/src/test/jtx/impl/Env.cpp b/src/test/jtx/impl/Env.cpp index 0d6bb2dc08..753d7046bc 100644 --- a/src/test/jtx/impl/Env.cpp +++ b/src/test/jtx/impl/Env.cpp @@ -492,6 +492,14 @@ Env::do_rpc(std::vector 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 diff --git a/src/test/rpc/AccountSet_test.cpp b/src/test/rpc/AccountSet_test.cpp index b5c33ec023..7ff5426dc2 100644 --- a/src/test/rpc/AccountSet_test.cpp +++ b/src/test/rpc/AccountSet_test.cpp @@ -17,10 +17,13 @@ */ //============================================================================== -#include -#include -#include #include +#include +#include +#include +#include +#include +#include namespace ripple { @@ -203,24 +206,124 @@ public: 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 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)); - // set a bad value (< QUALITY_ONE) - jt[sfTransferRate.fieldName] = 10u; - env(jt, ter(temBAD_TRANSFER_RATE)); + // 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 }}); + } + + { + 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 (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 (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) @@ -303,6 +406,7 @@ public: testSetAndResetAccountTxnID(); testSetNoFreeze(); testDomain(); + testGateway(); testMessageKey(); testWalletID(); testEmailHash();