diff --git a/src/ripple/app/main/Amendments.cpp b/src/ripple/app/main/Amendments.cpp index c8d4ed944..ab826d54d 100644 --- a/src/ripple/app/main/Amendments.cpp +++ b/src/ripple/app/main/Amendments.cpp @@ -59,6 +59,7 @@ supportedAmendments () { "CC5ABAE4F3EC92E94A59B1908C2BE82D2228B6485C00AFF8F22DF930D89C194E SortedDirectories" }, { "B4D44CC3111ADD964E846FC57760C8B50FFCD5A82C86A72756F6B058DDDF96AD fix1201" }, { "6C92211186613F9647A89DFFBAB8F94C99D4C7E956D495270789128569177DA1 fix1512" }, + { "67A34F2CF55BFC0F93AACD5B281413176FEE195269FA6D95219A2DF738671172 fix1513" }, { "B9E739B8296B4A1BB29BE990B17D66E21B62A300A909F25AC55C22D6C72E1F9D fix1523" }, { "1D3463A5891F9E589C5AE839FFAC4A917CE96197098A1EF22304E1BC5B98A454 fix1528" } }; diff --git a/src/ripple/app/misc/impl/TxQ.cpp b/src/ripple/app/misc/impl/TxQ.cpp index b7fcd79ea..444c73a27 100644 --- a/src/ripple/app/misc/impl/TxQ.cpp +++ b/src/ripple/app/misc/impl/TxQ.cpp @@ -265,6 +265,9 @@ TxQ::MaybeTx::MaybeTx( std::pair TxQ::MaybeTx::apply(Application& app, OpenView& view) { + boost::optional saved; + if (view.rules().enabled(fix1513)) + saved.emplace(view.info().parentCloseTime); // If the rules or flags change, preflight again assert(pfresult); if (pfresult->rules != view.rules() || @@ -517,8 +520,13 @@ TxQ::tryClearAccountQueue(Application& app, OpenView& view, } // Apply the current tx. Because the state of the view has been changed // by the queued txs, we also need to preclaim again. - auto const pcresult = preclaim(pfresult, app, view); - auto txResult = doApply(pcresult, app, view); + auto txResult = [&]{ + boost::optional saved; + if (view.rules().enabled(fix1513)) + saved.emplace(view.info().parentCloseTime); + auto const pcresult = preclaim(pfresult, app, view); + return doApply(pcresult, app, view); + }(); if (txResult.second) { @@ -614,11 +622,15 @@ TxQ::apply(Application& app, OpenView& view, auto const transactionID = tx->getTransactionID(); auto const tSeq = tx->getSequence(); + boost::optional saved; + if (view.rules().enabled(fix1513)) + saved.emplace(view.info().parentCloseTime); + // See if the transaction is valid, properly formed, // etc. before doing potentially expensive queue // replace and multi-transaction operations. auto const pfresult = preflight(app, view.rules(), - *tx, flags, j); + *tx, flags, j); if (pfresult.ter != tesSUCCESS) return{ pfresult.ter, false }; @@ -929,7 +941,7 @@ TxQ::apply(Application& app, OpenView& view, // See if the transaction is likely to claim a fee. assert(!multiTxn || multiTxn->openView); auto const pcresult = preclaim(pfresult, app, - multiTxn ? *multiTxn->openView : view); + multiTxn ? *multiTxn->openView : view); if (!pcresult.likelyToClaimFee) return{ pcresult.ter, false }; diff --git a/src/ripple/protocol/Feature.h b/src/ripple/protocol/Feature.h index aadde6e3b..1cbfea265 100644 --- a/src/ripple/protocol/Feature.h +++ b/src/ripple/protocol/Feature.h @@ -71,6 +71,7 @@ class FeatureCollections "SortedDirectories", "fix1201", "fix1512", + "fix1513", "fix1523", "fix1528" }; @@ -168,6 +169,7 @@ extern uint256 const featureEnforceInvariants; extern uint256 const featureSortedDirectories; extern uint256 const fix1201; extern uint256 const fix1512; +extern uint256 const fix1513; extern uint256 const fix1523; extern uint256 const fix1528; diff --git a/src/ripple/protocol/impl/Feature.cpp b/src/ripple/protocol/impl/Feature.cpp index 97a280f12..973548bcc 100644 --- a/src/ripple/protocol/impl/Feature.cpp +++ b/src/ripple/protocol/impl/Feature.cpp @@ -116,6 +116,7 @@ uint256 const featureEnforceInvariants = *getRegisteredFeature("EnforceInvariant uint256 const featureSortedDirectories = *getRegisteredFeature("SortedDirectories"); uint256 const fix1201 = *getRegisteredFeature("fix1201"); uint256 const fix1512 = *getRegisteredFeature("fix1512"); +uint256 const fix1513 = *getRegisteredFeature("fix1513"); uint256 const fix1523 = *getRegisteredFeature("fix1523"); uint256 const fix1528 = *getRegisteredFeature("fix1528"); diff --git a/src/test/app/Flow_test.cpp b/src/test/app/Flow_test.cpp index 89393dfcc..43e719cee 100644 --- a/src/test/app/Flow_test.cpp +++ b/src/test/app/Flow_test.cpp @@ -1254,6 +1254,34 @@ struct Flow_test : public beast::unit_test::suite ter(temBAD_PATH)); } + template + void testWithFeats(bool haveFlow, T&&... fs) + { + testLineQuality({ fs... }); + testFalseDry({ fs... }); + if (!haveFlow) + return; + testDirectStep({ fs... }); + testBookStep({ fs... }); + testDirectStep({ featureOwnerPaysFee, fs... }); + testBookStep({ featureOwnerPaysFee, fs... }); + testTransferRate({ featureOwnerPaysFee, fs... }); + testSelfPayment1({ fs... }); + testSelfPayment2({ fs... }); + testSelfFundedXRPEndpoint(false, { fs... }); + testSelfFundedXRPEndpoint(true, { fs... }); + testUnfundedOffer(true, { fs... }); + testUnfundedOffer(false, { fs... }); + testReexecuteDirectStep({ fix1368, fs... }); + testSelfPayLowQualityOffer({ fs... }); + } + + template + void testWithFeats(T&&... fs) + { + testWithFeats(!!sizeof...(fs), fs...); + } + void run() override { testLimitQuality(); @@ -1262,35 +1290,50 @@ struct Flow_test : public beast::unit_test::suite testRIPD1449(true); testRIPD1449(false); - auto testWithFeats = [this](auto&&... fs) - { - testLineQuality({fs...}); - testFalseDry({fs...}); - if (!sizeof...(fs)) - return; - testDirectStep({fs...}); - testBookStep({fs...}); - testDirectStep({featureOwnerPaysFee, fs...}); - testBookStep({featureOwnerPaysFee, fs...}); - testTransferRate({featureOwnerPaysFee, fs...}); - testSelfPayment1({fs...}); - testSelfPayment2({fs...}); - testSelfFundedXRPEndpoint(false, {fs...}); - testSelfFundedXRPEndpoint(true, {fs...}); - testUnfundedOffer(true, {fs...}); - testUnfundedOffer(false, {fs...}); - testReexecuteDirectStep({fix1368, fs...}); - testSelfPayLowQualityOffer({fs...}); - }; + testWithFeats(false, featureFeeEscalation, fix1513); + testWithFeats(featureFlow, featureFeeEscalation, fix1513); + testWithFeats(featureFlow, fix1373, featureFeeEscalation, fix1513); + testWithFeats(featureFlow, fix1373, featureFlowCross, + featureFeeEscalation, fix1513); + testEmptyStrand({featureFlow, fix1373, featureFlowCross, + featureFeeEscalation, fix1513 }); + } +}; + +struct Flow_manual_test : public Flow_test +{ + void run() override + { testWithFeats(); + testWithFeats(false, + featureFeeEscalation); + testWithFeats(false, + featureFeeEscalation, fix1513); testWithFeats(featureFlow); + testWithFeats(featureFlow, + featureFeeEscalation); + testWithFeats(featureFlow, + featureFeeEscalation, fix1513); testWithFeats(featureFlow, fix1373); + testWithFeats(featureFlow, fix1373, + featureFeeEscalation); + testWithFeats(featureFlow, fix1373, + featureFeeEscalation, fix1513); testWithFeats(featureFlow, fix1373, featureFlowCross); - testEmptyStrand({featureFlow, fix1373, featureFlowCross}); + testWithFeats(featureFlow, fix1373, featureFlowCross, + featureFeeEscalation); + testWithFeats(featureFlow, fix1373, featureFlowCross, + featureFeeEscalation, fix1513); + testEmptyStrand({ featureFlow, fix1373, featureFlowCross }); + testEmptyStrand({ featureFlow, fix1373, featureFlowCross, + featureFeeEscalation }); + testEmptyStrand({ featureFlow, fix1373, featureFlowCross, + featureFeeEscalation, fix1513 }); } }; BEAST_DEFINE_TESTSUITE(Flow,app,ripple); +BEAST_DEFINE_TESTSUITE_MANUAL(Flow_manual,app,ripple); } // test } // ripple diff --git a/src/test/app/Offer_test.cpp b/src/test/app/Offer_test.cpp index 003b4c06e..1cab2f87f 100644 --- a/src/test/app/Offer_test.cpp +++ b/src/test/app/Offer_test.cpp @@ -253,12 +253,20 @@ public: for (int i=0;i<101;++i) env (offer (carol, USD (1), EUR (2))); + auto hasFeature = [](Env& env, uint256 const& f) + { + return (env.app().config().features.find(f) != + env.app().config().features.end()); + }; + for (auto d : {-1, 1}) { auto const closeTime = STAmountSO::soTime + d * env.closed()->info().closeTimeResolution; env.close (closeTime); - *stAmountCalcSwitchover = closeTime > STAmountSO::soTime; + *stAmountCalcSwitchover = closeTime > STAmountSO::soTime || + (hasFeature(env, featureFeeEscalation) && + !hasFeature(env, fix1513)); // Will fail without the underflow fix auto expectedResult = *stAmountCalcSwitchover ? tesSUCCESS : tecPATH_PARTIAL; @@ -4519,74 +4527,111 @@ public: BEAST_EXPECT (++it == offers.end()); } - void run () + void testAll(std::initializer_list fs) + { + testCanceledOffer(fs); + testRmFundedOffer(fs); + testTinyPayment(fs); + testXRPTinyPayment(fs); + testEnforceNoRipple(fs); + testInsufficientReserve(fs); + testFillModes(fs); + testMalformed(fs); + testExpiration(fs); + testUnfundedCross(fs); + testSelfCross(false, fs); + testSelfCross(true, fs); + testNegativeBalance(fs); + testOfferCrossWithXRP(true, fs); + testOfferCrossWithXRP(false, fs); + testOfferCrossWithLimitOverride(fs); + testOfferAcceptThenCancel(fs); + testOfferCancelPastAndFuture(fs); + testCurrencyConversionEntire(fs); + testCurrencyConversionIntoDebt(fs); + testCurrencyConversionInParts(fs); + testCrossCurrencyStartXRP(fs); + testCrossCurrencyEndXRP(fs); + testCrossCurrencyBridged(fs); + testOfferFeesConsumeFunds(fs); + testOfferCreateThenCross(fs); + testSellFlagBasic(fs); + testSellFlagExceedLimit(fs); + testGatewayCrossCurrency(fs); + testPartialCross(fs); + testXRPDirectCross(fs); + testDirectCross(fs); + testBridgedCross(fs); + testSellOffer(fs); + testSellWithFillOrKill(fs); + testTransferRateOffer(fs); + testSelfCrossOffer(fs); + testSelfIssueOffer(fs); + testBadPathAssert(fs); + testDirectToDirectPath(fs); + testSelfCrossLowQualityOffer(fs); + testOfferInScaling(fs); + testOfferInScalingWithXferRate(fs); + testOfferThresholdWithReducedFunds(fs); + testTinyOffer(fs); + testSelfPayXferFeeOffer(fs); + testSelfPayUnlimitedFunds(fs); + testRequireAuth(fs); + testMissingAuth(fs); + testRCSmoketest(fs); + testSelfAuth(fs); + testTickSize(fs); + } + void run () override { - auto testAll = [this](std::initializer_list fs) { - testCanceledOffer(fs); - testRmFundedOffer(fs); - testTinyPayment(fs); - testXRPTinyPayment(fs); - testEnforceNoRipple(fs); - testInsufficientReserve(fs); - testFillModes(fs); - testMalformed(fs); - testExpiration(fs); - testUnfundedCross(fs); - testSelfCross(false, fs); - testSelfCross(true, fs); - testNegativeBalance(fs); - testOfferCrossWithXRP(true, fs); - testOfferCrossWithXRP(false, fs); - testOfferCrossWithLimitOverride(fs); - testOfferAcceptThenCancel(fs); - testOfferCancelPastAndFuture(fs); - testCurrencyConversionEntire(fs); - testCurrencyConversionIntoDebt(fs); - testCurrencyConversionInParts(fs); - testCrossCurrencyStartXRP(fs); - testCrossCurrencyEndXRP(fs); - testCrossCurrencyBridged(fs); - testOfferFeesConsumeFunds(fs); - testOfferCreateThenCross(fs); - testSellFlagBasic(fs); - testSellFlagExceedLimit(fs); - testGatewayCrossCurrency(fs); - testPartialCross (fs); - testXRPDirectCross (fs); - testDirectCross (fs); - testBridgedCross (fs); - testSellOffer (fs); - testSellWithFillOrKill (fs); - testTransferRateOffer(fs); - testSelfCrossOffer (fs); - testSelfIssueOffer (fs); - testBadPathAssert (fs); - testDirectToDirectPath (fs); - testSelfCrossLowQualityOffer (fs); - testOfferInScaling (fs); - testOfferInScalingWithXferRate (fs); - testOfferThresholdWithReducedFunds (fs); - testTinyOffer (fs); - testSelfPayXferFeeOffer (fs); - testSelfPayUnlimitedFunds (fs); - testRequireAuth (fs); - testMissingAuth (fs); - testRCSmoketest (fs); - testSelfAuth (fs); - testTickSize (fs); - }; // The first three test variants below passed at one time in the past (and // should still pass) but are commented out to conserve test time. // testAll(jtx::no_features ); // testAll({ featureFlowCross }); // testAll({featureFlow }); - testAll({featureFlow, featureFlowCross }); - testAll({featureFlow, fix1373 }); - testAll({featureFlow, fix1373, featureFlowCross }); + testAll({featureFlow, featureFlowCross, + featureFeeEscalation, fix1513 }); + testAll({ featureFlow, fix1373, + featureFeeEscalation, fix1513 }); + testAll({featureFlow, fix1373, featureFlowCross, + featureFeeEscalation, fix1513 }); + } +}; + +class Offer_manual_test : public Offer_test +{ + void run() override + { + testAll({}); + testAll({ featureFeeEscalation }); + testAll({ featureFeeEscalation, fix1513 }); + testAll({ featureFlowCross }); + testAll({ featureFlowCross, + featureFeeEscalation }); + testAll({ featureFlowCross, + featureFeeEscalation, fix1513 }); + testAll({featureFlow }); + testAll({ featureFlow, featureFeeEscalation }); + testAll({ featureFlow, featureFeeEscalation, fix1513 }); + testAll({featureFlow, featureFlowCross}); + testAll({featureFlow, featureFlowCross, + featureFeeEscalation }); + testAll({featureFlow, featureFlowCross, + featureFeeEscalation, fix1513 }); + testAll({featureFlow, fix1373 }); + testAll({ featureFlow, fix1373, featureFeeEscalation }); + testAll({ featureFlow, fix1373, featureFeeEscalation, fix1513 }); + testAll({ featureFlow, fix1373, featureFlowCross }); + testAll({featureFlow, fix1373, featureFlowCross, + featureFeeEscalation }); + testAll({featureFlow, fix1373, featureFlowCross, + featureFeeEscalation, fix1513 }); + } }; BEAST_DEFINE_TESTSUITE (Offer, tx, ripple); +BEAST_DEFINE_TESTSUITE_MANUAL (Offer_manual, tx, ripple); } // test } // ripple diff --git a/src/test/app/Regression_test.cpp b/src/test/app/Regression_test.cpp index 7cd62d01f..d98654c28 100644 --- a/src/test/app/Regression_test.cpp +++ b/src/test/app/Regression_test.cpp @@ -171,7 +171,7 @@ struct Regression_test : public beast::unit_test::suite .set("minimum_txn_in_ledger_standalone", "3"); return cfg; }), - with_features(featureFeeEscalation)); + with_features(featureFeeEscalation, fix1513)); Env_ss envs(env); auto const alice = Account("alice"); diff --git a/src/test/app/TxQ_test.cpp b/src/test/app/TxQ_test.cpp index 9782d3d08..a11b678ae 100644 --- a/src/test/app/TxQ_test.cpp +++ b/src/test/app/TxQ_test.cpp @@ -169,7 +169,7 @@ public: using namespace std::chrono; Env env(*this, makeConfig({ {"minimum_txn_in_ledger_standalone", "3"} }), - with_features(featureFeeEscalation)); + with_features(featureFeeEscalation, fix1513)); auto& txq = env.app().getTxQ(); auto alice = Account("alice"); @@ -356,7 +356,7 @@ public: using namespace std::chrono; Env env(*this, makeConfig({ { "minimum_txn_in_ledger_standalone", "2" } }), - with_features(featureFeeEscalation)); + with_features(featureFeeEscalation, fix1513)); auto alice = Account("alice"); auto bob = Account("bob"); @@ -412,7 +412,7 @@ public: using namespace std::chrono; Env env(*this, makeConfig({ { "minimum_txn_in_ledger_standalone", "2" } }), - with_features(featureFeeEscalation)); + with_features(featureFeeEscalation, fix1513)); auto alice = Account("alice"); auto bob = Account("bob"); @@ -520,7 +520,7 @@ public: using namespace std::chrono; Env env(*this, makeConfig({ { "minimum_txn_in_ledger_standalone", "2" } }), - with_features(featureFeeEscalation)); + with_features(featureFeeEscalation, fix1513)); auto alice = Account("alice"); auto bob = Account("bob"); @@ -626,7 +626,7 @@ public: { using namespace jtx; - Env env(*this, makeConfig(), with_features(featureFeeEscalation)); + Env env(*this, makeConfig(), with_features(featureFeeEscalation, fix1513)); auto alice = Account("alice"); auto bob = Account("bob"); @@ -651,7 +651,7 @@ public: using namespace jtx; Env env(*this, makeConfig({ { "minimum_txn_in_ledger_standalone", "2" } }), - with_features(featureFeeEscalation)); + with_features(featureFeeEscalation, fix1513)); auto alice = Account("alice"); auto bob = Account("bob"); @@ -708,7 +708,7 @@ public: makeConfig( {{"minimum_txn_in_ledger_standalone", "3"}}, {{"account_reserve", "200"}, {"owner_reserve", "50"}}), - with_features(featureFeeEscalation)); + with_features(featureFeeEscalation, fix1513)); auto alice = Account("alice"); auto bob = Account("bob"); @@ -936,7 +936,7 @@ public: using namespace std::chrono; Env env(*this, makeConfig({ { "minimum_txn_in_ledger_standalone", "4" } }), - with_features(featureFeeEscalation)); + with_features(featureFeeEscalation, fix1513)); auto alice = Account("alice"); auto bob = Account("bob"); @@ -1095,7 +1095,7 @@ public: using namespace jtx; Env env(*this, makeConfig({ { "minimum_txn_in_ledger_standalone", "1" } }), - with_features(featureFeeEscalation)); + with_features(featureFeeEscalation, fix1513)); auto alice = Account("alice"); @@ -1138,7 +1138,7 @@ public: { {"minimum_txn_in_ledger_standalone", "2"}, {"target_txn_in_ledger", "4"}, {"maximum_txn_in_ledger", "5"} }), - with_features(featureFeeEscalation)); + with_features(featureFeeEscalation, fix1513)); auto alice = Account("alice"); auto queued = ter(terQUEUED); @@ -1168,7 +1168,7 @@ public: makeConfig( {{"minimum_txn_in_ledger_standalone", "3"}}, {{"account_reserve", "200"}, {"owner_reserve", "50"}}), - with_features(featureFeeEscalation)); + with_features(featureFeeEscalation, fix1513)); auto alice = Account("alice"); auto bob = Account("bob"); @@ -1258,7 +1258,7 @@ public: Env env(*this, makeConfig({ { "minimum_txn_in_ledger_standalone", "3" } }), - with_features(featureFeeEscalation, featureMultiSign)); + with_features(featureFeeEscalation, featureMultiSign, fix1513)); auto alice = Account("alice"); auto bob = Account("bob"); @@ -1323,7 +1323,7 @@ public: Env env(*this, makeConfig({ { "minimum_txn_in_ledger_standalone", "3" } }), - with_features(featureFeeEscalation, featureTickets)); + with_features(featureFeeEscalation, featureTickets, fix1513)); auto alice = Account("alice"); auto charlie = Account("charlie"); @@ -1636,7 +1636,7 @@ public: { using namespace jtx; { - Env env(*this, with_features(featureFeeEscalation)); + Env env(*this, with_features(featureFeeEscalation, fix1513)); auto fee = env.rpc("fee"); @@ -1725,7 +1725,7 @@ public: Env env(*this, makeConfig({ { "minimum_txn_in_ledger_standalone", "1" }, {"ledgers_in_queue", "10"}, {"maximum_txn_per_account", "20"} }), - with_features(featureFeeEscalation)); + with_features(featureFeeEscalation, fix1513)); // Alice will recreate the scenario. Bob will block. auto const alice = Account("alice"); @@ -1800,7 +1800,7 @@ public: testcase("Autofilled sequence should account for TxQ"); using namespace jtx; Env env(*this, makeConfig({ {"minimum_txn_in_ledger_standalone", "6"} }), - with_features(featureFeeEscalation)); + with_features(featureFeeEscalation, fix1513)); Env_ss envs(env); auto const& txQ = env.app().getTxQ(); @@ -1930,7 +1930,7 @@ public: { using namespace jtx; Env env(*this, makeConfig({ { "minimum_txn_in_ledger_standalone", "3" } }), - with_features(featureFeeEscalation)); + with_features(featureFeeEscalation, fix1513)); Env_ss envs(env); Account const alice{ "alice" }; @@ -2200,7 +2200,7 @@ public: { using namespace jtx; Env env(*this, makeConfig({ { "minimum_txn_in_ledger_standalone", "3" } }), - with_features(featureFeeEscalation)); + with_features(featureFeeEscalation, fix1513)); Env_ss envs(env); Account const alice{ "alice" }; @@ -2423,7 +2423,7 @@ public: using namespace jtx; Env env(*this, makeConfig({ { "minimum_txn_in_ledger_standalone", "3" } }), - with_features(featureFeeEscalation)); + with_features(featureFeeEscalation, fix1513)); Json::Value stream; stream[jss::streams] = Json::arrayValue; @@ -2592,7 +2592,7 @@ public: Env env(*this, makeConfig({ { "minimum_txn_in_ledger_standalone", "3" } }), - with_features(featureFeeEscalation)); + with_features(featureFeeEscalation, fix1513)); auto alice = Account("alice"); auto bob = Account("bob"); diff --git a/src/test/rpc/JSONRPC_test.cpp b/src/test/rpc/JSONRPC_test.cpp index 4f31b953b..8056ef9f3 100644 --- a/src/test/rpc/JSONRPC_test.cpp +++ b/src/test/rpc/JSONRPC_test.cpp @@ -1942,7 +1942,7 @@ public: .set("minimum_txn_in_ledger_standalone", "3"); return cfg; }), - with_features(featureFeeEscalation)}; + with_features(featureFeeEscalation, fix1513)}; LoadFeeTrack const& feeTrack = env.app().getFeeTrack(); { diff --git a/src/test/rpc/LedgerRPC_test.cpp b/src/test/rpc/LedgerRPC_test.cpp index 3828dc8e7..a1a5908dc 100644 --- a/src/test/rpc/LedgerRPC_test.cpp +++ b/src/test/rpc/LedgerRPC_test.cpp @@ -477,7 +477,7 @@ class LedgerRPC_test : public beast::unit_test::suite .set("minimum_txn_in_ledger_standalone", "3"); return cfg; }), - with_features(featureFeeEscalation)}; + with_features(featureFeeEscalation, fix1513)}; Json::Value jv; jv[jss::ledger_index] = "current";