diff --git a/src/ripple/app/tx/impl/SetAccount.cpp b/src/ripple/app/tx/impl/SetAccount.cpp index 93e41f680c..aac7d109f1 100644 --- a/src/ripple/app/tx/impl/SetAccount.cpp +++ b/src/ripple/app/tx/impl/SetAccount.cpp @@ -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; diff --git a/src/ripple/protocol/Feature.h b/src/ripple/protocol/Feature.h index 497bcd1fe6..180f02bdd8 100644 --- a/src/ripple/protocol/Feature.h +++ b/src/ripple/protocol/Feature.h @@ -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; @@ -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 diff --git a/src/ripple/protocol/impl/Feature.cpp b/src/ripple/protocol/impl/Feature.cpp index 65541fa301..6a7cfc18fd 100644 --- a/src/ripple/protocol/impl/Feature.cpp +++ b/src/ripple/protocol/impl/Feature.cpp @@ -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"); @@ -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 diff --git a/src/test/rpc/AccountSet_test.cpp b/src/test/rpc/AccountSet_test.cpp index ca76326343..8c893fcee0 100644 --- a/src/test/rpc/AccountSet_test.cpp +++ b/src/test/rpc/AccountSet_test.cpp @@ -33,6 +33,8 @@ public: void testNullAccountSet() { + testcase ("No AccountSet"); + using namespace test::jtx; Env env(*this); Account const alice ("alice"); @@ -44,6 +46,8 @@ public: void testMostFlags() { + testcase ("Most Flags"); + using namespace test::jtx; Account const alice ("alice"); @@ -112,6 +116,8 @@ public: void testSetAndResetAccountTxnID() { + testcase ("Set and reset AccountTxnID"); + using namespace test::jtx; Env env(*this); Account const alice ("alice"); @@ -132,6 +138,8 @@ public: void testSetNoFreeze() { + testcase ("Set NoFreeze"); + using namespace test::jtx; Env env(*this); Account const alice ("alice"); @@ -150,6 +158,8 @@ public: void testDomain() { + testcase ("Domain"); + using namespace test::jtx; Env env(*this); Account const alice ("alice"); @@ -197,6 +207,8 @@ public: void testMessageKey() { + testcase ("MessageKey"); + using namespace test::jtx; Env env(*this); Account const alice ("alice"); @@ -219,6 +231,8 @@ public: void testWalletID() { + testcase ("WalletID"); + using namespace test::jtx; Env env(*this); Account const alice ("alice"); @@ -237,6 +251,8 @@ public: void testEmailHash() { + testcase ("EmailHash"); + using namespace test::jtx; Env env(*this); Account const alice ("alice"); @@ -262,6 +278,8 @@ public: double get; }; + testcase ("TransferRate"); + using namespace test::jtx; auto doTests = [this] (FeatureBitset const& features, std::initializer_list testData) @@ -274,6 +292,7 @@ public: 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]) @@ -284,100 +303,117 @@ public: } }; - { - 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 (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->key()); + (*replacement)[sfTransferRate] = + static_cast(transferRate * QUALITY_ONE); + view.rawReplace (replacement); + return true; + }); + auto const amount = USD(1); - Rate const rate (tr * QUALITY_ONE); auto const amountWithRate = - toAmount (multiply(amount.value(), rate)); + toAmount ( + 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"); @@ -418,6 +454,8 @@ public: void testRequireAuthWithDir() { + testcase ("Require auth"); + using namespace test::jtx; Env env(*this); Account const alice ("alice");