From 44fe0e1fc47e3710d4f6040f34cfd8f35c09ef68 Mon Sep 17 00:00:00 2001 From: Scott Schurr Date: Fri, 11 Dec 2020 14:36:44 -0800 Subject: [PATCH] Add support for the 'TicketBatch' amendment: Support for 'out-of-sequence' transaction execution was introduced in commit 7724cca384487bf4cdc17fa34ff8d5372a54b110. The changes in that commit were gated under a feature but there was no corresponding amendment introduced that would allow the network to vote on this amendment. This commit introduces 'TicketBatch' amendment as the amendment that is associated with the tickets feature. If the amendment is enabled, it will activate support for tickets. This commit also removes several workarounds that are no longer needed in unit tests. --- src/ripple/protocol/impl/Feature.cpp | 2 +- src/test/app/AccountDelete_test.cpp | 4 ++-- src/test/app/Check_test.cpp | 2 +- src/test/app/DepositAuth_test.cpp | 4 ++-- src/test/app/Escrow_test.cpp | 4 ++-- src/test/app/Flow_test.cpp | 7 +++---- src/test/app/MultiSign_test.cpp | 5 +---- src/test/app/Offer_test.cpp | 10 ++-------- src/test/app/PayChan_test.cpp | 8 ++++---- src/test/app/SetRegularKey_test.cpp | 2 +- src/test/app/SetTrust_test.cpp | 2 +- src/test/app/Ticket_test.cpp | 14 +++++++------- src/test/app/TxQ_test.cpp | 27 +++++++-------------------- src/test/jtx/Env_test.cpp | 2 +- src/test/rpc/AccountObjects_test.cpp | 2 +- src/test/rpc/AccountSet_test.cpp | 2 +- src/test/rpc/AccountTx_test.cpp | 2 +- src/test/rpc/LedgerData_test.cpp | 6 +----- src/test/rpc/LedgerRPC_test.cpp | 3 +-- 19 files changed, 40 insertions(+), 68 deletions(-) diff --git a/src/ripple/protocol/impl/Feature.cpp b/src/ripple/protocol/impl/Feature.cpp index 7025117aa..c850ed07f 100644 --- a/src/ripple/protocol/impl/Feature.cpp +++ b/src/ripple/protocol/impl/Feature.cpp @@ -132,7 +132,7 @@ detail::supportedAmendments() "HardenedValidations", "fixAmendmentMajorityCalc", //"NegativeUNL", // Commented out to prevent automatic enablement - //"TicketBatch", // Commented out to prevent automatic enablement + "TicketBatch", }; return supported; } diff --git a/src/test/app/AccountDelete_test.cpp b/src/test/app/AccountDelete_test.cpp index 09ebd7148..8532929f2 100644 --- a/src/test/app/AccountDelete_test.cpp +++ b/src/test/app/AccountDelete_test.cpp @@ -110,7 +110,7 @@ public: testcase("Basics"); - Env env{*this, supported_amendments() | featureTicketBatch}; + Env env{*this}; Account const alice("alice"); Account const becky("becky"); Account const carol("carol"); @@ -791,7 +791,7 @@ public: Account const alice{"alice"}; Account const bob{"bob"}; - Env env{*this, supported_amendments() | featureTicketBatch}; + Env env{*this}; env.fund(XRP(100000), alice, bob); env.close(); diff --git a/src/test/app/Check_test.cpp b/src/test/app/Check_test.cpp index fb5f6301a..3552d24bb 100644 --- a/src/test/app/Check_test.cpp +++ b/src/test/app/Check_test.cpp @@ -1758,7 +1758,7 @@ class Check_test : public beast::unit_test::suite Account const bob{"bob"}; IOU const USD{gw["USD"]}; - Env env{*this, supported_amendments() | featureTicketBatch}; + Env env{*this}; env.fund(XRP(1000), gw, alice, bob); env.close(); diff --git a/src/test/app/DepositAuth_test.cpp b/src/test/app/DepositAuth_test.cpp index ea3413ff2..f10633c5e 100644 --- a/src/test/app/DepositAuth_test.cpp +++ b/src/test/app/DepositAuth_test.cpp @@ -434,7 +434,7 @@ struct DepositPreauth_test : public beast::unit_test::suite { // Verify that an account can be preauthorized and unauthorized // using tickets. - Env env(*this, supported_amendments() | featureTicketBatch); + Env env(*this); env.fund(XRP(10000), alice, becky); env.close(); @@ -729,7 +729,7 @@ struct DepositPreauth_test : public beast::unit_test::suite { testEnable(); testInvalid(); - auto const supported{jtx::supported_amendments() | featureTicketBatch}; + auto const supported{jtx::supported_amendments()}; testPayment(supported - featureDepositPreauth); testPayment(supported); } diff --git a/src/test/app/Escrow_test.cpp b/src/test/app/Escrow_test.cpp index 74d488d9c..e3220234f 100644 --- a/src/test/app/Escrow_test.cpp +++ b/src/test/app/Escrow_test.cpp @@ -1508,7 +1508,7 @@ struct Escrow_test : public beast::unit_test::suite { // Create escrow and finish using tickets. - Env env(*this, supported_amendments() | featureTicketBatch); + Env env(*this); env.fund(XRP(5000), alice, bob); env.close(); @@ -1569,7 +1569,7 @@ struct Escrow_test : public beast::unit_test::suite { // Create escrow and cancel using tickets. - Env env(*this, supported_amendments() | featureTicketBatch); + Env env(*this); env.fund(XRP(5000), alice, bob); env.close(); diff --git a/src/test/app/Flow_test.cpp b/src/test/app/Flow_test.cpp index e9d2e89bc..273f76690 100644 --- a/src/test/app/Flow_test.cpp +++ b/src/test/app/Flow_test.cpp @@ -754,7 +754,7 @@ struct Flow_test : public beast::unit_test::suite Account const carol("carol"); { - Env env(*this, supported_amendments()); + Env env(*this); env.fund(XRP(10000), alice, bob, carol, gw); @@ -1360,7 +1360,6 @@ struct Flow_test : public beast::unit_test::suite auto const bob = Account("bob"); Env env(*this, features); - BEAST_EXPECT(features[featureTicketBatch]); env.fund(XRP(10000), alice); @@ -1407,7 +1406,7 @@ struct Flow_test : public beast::unit_test::suite testRIPD1449(); using namespace jtx; - auto const sa = supported_amendments() | featureTicketBatch; + auto const sa = supported_amendments(); testWithFeats(sa - featureFlowCross); testWithFeats(sa); testEmptyStrand(sa); @@ -1420,7 +1419,7 @@ struct Flow_manual_test : public Flow_test run() override { using namespace jtx; - auto const all = supported_amendments() | featureTicketBatch; + auto const all = supported_amendments(); FeatureBitset const flowCross{featureFlowCross}; FeatureBitset const f1513{fix1513}; diff --git a/src/test/app/MultiSign_test.cpp b/src/test/app/MultiSign_test.cpp index 433d0685a..d354a27d6 100644 --- a/src/test/app/MultiSign_test.cpp +++ b/src/test/app/MultiSign_test.cpp @@ -1491,9 +1491,6 @@ public: env.fund(XRP(2000), alice); env.close(); - // If featureTicketBatch is not enabled expect massive failures. - BEAST_EXPECT(features[featureTicketBatch]); - // Create a few tickets that alice can use up. std::uint32_t aliceTicketSeq{env.seq(alice) + 1}; env(ticket::create(alice, 20)); @@ -1550,7 +1547,7 @@ public: run() override { using namespace jtx; - auto const all = supported_amendments() | featureTicketBatch; + auto const all = supported_amendments(); // The reserve required on a signer list changes based on. // featureMultiSignReserve. Test both with and without. diff --git a/src/test/app/Offer_test.cpp b/src/test/app/Offer_test.cpp index 5ef89d690..f30fc19e1 100644 --- a/src/test/app/Offer_test.cpp +++ b/src/test/app/Offer_test.cpp @@ -5157,9 +5157,6 @@ public: using namespace jtx; - // Should be called with TicketBatch enabled. - BEAST_EXPECT(features[featureTicketBatch]); - // Two goals for this test. // // o Verify that offers can be created using tickets. @@ -5280,9 +5277,6 @@ public: using namespace jtx; - // Should be called with TicketBatch enabled. - BEAST_EXPECT(features[featureTicketBatch]); - // Verify that offers created with or without tickets can be canceled // by transactions with or without tickets. Env env{*this, features}; @@ -5471,7 +5465,7 @@ public: run() override { using namespace jtx; - FeatureBitset const all{supported_amendments() | featureTicketBatch}; + FeatureBitset const all{supported_amendments()}; FeatureBitset const flowCross{featureFlowCross}; FeatureBitset const takerDryOffer{fixTakerDryOfferRemoval}; @@ -5489,7 +5483,7 @@ class Offer_manual_test : public Offer_test run() override { using namespace jtx; - FeatureBitset const all{supported_amendments() | featureTicketBatch}; + FeatureBitset const all{supported_amendments()}; FeatureBitset const flowCross{featureFlowCross}; FeatureBitset const f1513{fix1513}; FeatureBitset const takerDryOffer{fixTakerDryOfferRemoval}; diff --git a/src/test/app/PayChan_test.cpp b/src/test/app/PayChan_test.cpp index 602063017..af501ad6b 100644 --- a/src/test/app/PayChan_test.cpp +++ b/src/test/app/PayChan_test.cpp @@ -652,7 +652,7 @@ struct PayChan_test : public beast::unit_test::suite { // Create a channel where dst disallows XRP. Ignore that flag, // since it's just advisory. - Env env(*this, supported_amendments()); + Env env(*this); env.fund(XRP(10000), alice, bob); env(fset(bob, asfDisallowXRP)); auto const chan = channel(alice, bob, env.seq(alice)); @@ -677,7 +677,7 @@ struct PayChan_test : public beast::unit_test::suite // Claim to a channel where dst disallows XRP (channel is // created before disallow xrp is set). Ignore that flag // since it is just advisory. - Env env(*this, supported_amendments()); + Env env(*this); env.fund(XRP(10000), alice, bob); auto const chan = channel(alice, bob, env.seq(alice)); env(create(alice, bob, XRP(1000), 3600s, alice.pk())); @@ -1588,7 +1588,7 @@ struct PayChan_test : public beast::unit_test::suite { // Test with adding the paychan to the recipient's owner directory - Env env(*this, supported_amendments()); + Env env(*this); env.fund(XRP(10000), alice, bob); env(create(alice, bob, XRP(1000), settleDelay, pk)); env.close(); @@ -1884,7 +1884,7 @@ struct PayChan_test : public beast::unit_test::suite testcase("using tickets"); using namespace jtx; using namespace std::literals::chrono_literals; - Env env(*this, supported_amendments() | featureTicketBatch); + Env env(*this); auto const alice = Account("alice"); auto const bob = Account("bob"); auto USDA = alice["USD"]; diff --git a/src/test/app/SetRegularKey_test.cpp b/src/test/app/SetRegularKey_test.cpp index effbaace0..799b7797b 100644 --- a/src/test/app/SetRegularKey_test.cpp +++ b/src/test/app/SetRegularKey_test.cpp @@ -204,7 +204,7 @@ public: using namespace test::jtx; testcase("Ticket regular key"); - Env env{*this, supported_amendments() | featureTicketBatch}; + Env env{*this}; Account const alice{"alice", KeyType::ed25519}; env.fund(XRP(1000), alice); env.close(); diff --git a/src/test/app/SetTrust_test.cpp b/src/test/app/SetTrust_test.cpp index 7c6d0fccd..45a9e5c76 100644 --- a/src/test/app/SetTrust_test.cpp +++ b/src/test/app/SetTrust_test.cpp @@ -114,7 +114,7 @@ public: using namespace jtx; // Verify that TrustSet transactions can use tickets. - Env env{*this, supported_amendments() | featureTicketBatch}; + Env env{*this}; auto const gw = Account{"gateway"}; auto const alice = Account{"alice"}; auto const USD = gw["USD"]; diff --git a/src/test/app/Ticket_test.cpp b/src/test/app/Ticket_test.cpp index f9b3e5f4e..331ed68be 100644 --- a/src/test/app/Ticket_test.cpp +++ b/src/test/app/Ticket_test.cpp @@ -425,7 +425,7 @@ class Ticket_test : public beast::unit_test::suite testcase("Create Tickets that fail Preflight"); using namespace test::jtx; - Env env{*this, supported_amendments() | featureTicketBatch}; + Env env{*this}; Account const master{env.master}; @@ -474,7 +474,7 @@ class Ticket_test : public beast::unit_test::suite using namespace test::jtx; { // Create tickets on a non-existent account. - Env env{*this, supported_amendments() | featureTicketBatch}; + Env env{*this}; Account alice{"alice"}; env.memoize(alice); @@ -485,7 +485,7 @@ class Ticket_test : public beast::unit_test::suite { // Exceed the threshold where tickets can no longer be // added to an account. - Env env{*this, supported_amendments() | featureTicketBatch}; + Env env{*this}; Account alice{"alice"}; env.fund(XRP(100000), alice); @@ -524,7 +524,7 @@ class Ticket_test : public beast::unit_test::suite } { // Explore exceeding the ticket threshold from another angle. - Env env{*this, supported_amendments() | featureTicketBatch}; + Env env{*this}; Account alice{"alice"}; env.fund(XRP(100000), alice); @@ -564,7 +564,7 @@ class Ticket_test : public beast::unit_test::suite testcase("Create Ticket Insufficient Reserve"); using namespace test::jtx; - Env env{*this, supported_amendments() | featureTicketBatch}; + Env env{*this}; Account alice{"alice"}; // Fund alice not quite enough to make the reserve for a Ticket. @@ -624,7 +624,7 @@ class Ticket_test : public beast::unit_test::suite testcase("Using Tickets"); using namespace test::jtx; - Env env{*this, supported_amendments() | featureTicketBatch}; + Env env{*this}; Account alice{"alice"}; env.fund(XRP(10000), alice); @@ -720,7 +720,7 @@ class Ticket_test : public beast::unit_test::suite testcase("Transaction Database With Tickets"); using namespace test::jtx; - Env env{*this, supported_amendments() | featureTicketBatch}; + Env env{*this}; Account alice{"alice"}; env.fund(XRP(10000), alice); diff --git a/src/test/app/TxQ_test.cpp b/src/test/app/TxQ_test.cpp index 5ffbf3a91..db5cb5f78 100644 --- a/src/test/app/TxQ_test.cpp +++ b/src/test/app/TxQ_test.cpp @@ -24,7 +24,6 @@ #include #include #include -#include #include #include #include @@ -383,10 +382,7 @@ public: using namespace jtx; testcase("queue ticket"); - Env env( - *this, - makeConfig({{"minimum_txn_in_ledger_standalone", "3"}}), - supported_amendments() | featureTicketBatch); + Env env(*this, makeConfig({{"minimum_txn_in_ledger_standalone", "3"}})); auto alice = Account("alice"); @@ -1797,10 +1793,7 @@ public: auto queued = ter(terQUEUED); - Env env( - *this, - makeConfig({{"minimum_txn_in_ledger_standalone", "3"}}), - supported_amendments() | featureTicketBatch); + Env env(*this, makeConfig({{"minimum_txn_in_ledger_standalone", "3"}})); BEAST_EXPECT(env.current()->fees().base == 10); @@ -2310,7 +2303,7 @@ public: using namespace std::chrono; testcase("consequences"); - Env env(*this, supported_amendments() | featureTicketBatch); + Env env(*this); auto const alice = Account("alice"); env.memoize(alice); env.memoize("bob"); @@ -2648,8 +2641,7 @@ public: makeConfig( {{"minimum_txn_in_ledger_standalone", "1"}, {"ledgers_in_queue", "10"}, - {"maximum_txn_per_account", "11"}}), - supported_amendments() | featureTicketBatch); + {"maximum_txn_per_account", "11"}})); // Alice will have the gaps. Bob will keep the queue busy with // high fee transactions so alice's transactions can expire to leave @@ -4037,10 +4029,7 @@ public: testcase("Ticket in queue and open ledger"); using namespace jtx; - Env env( - *this, - makeConfig({{"minimum_txn_in_ledger_standalone", "3"}}), - supported_amendments() | featureTicketBatch); + Env env(*this, makeConfig({{"minimum_txn_in_ledger_standalone", "3"}})); auto alice = Account("alice"); @@ -4340,8 +4329,7 @@ public: {"maximum_txn_per_account", "30"}, {"minimum_queue_size", "50"}}); - Env env( - *this, std::move(cfg), supported_amendments() | featureTicketBatch); + Env env(*this, std::move(cfg)); // The noripple is to reduce the number of transactions required to // fund the accounts. There is no rippling in this test. @@ -4546,8 +4534,7 @@ public: {"maximum_txn_per_account", "30"}, {"minimum_queue_size", "50"}}); - Env env( - *this, std::move(cfg), supported_amendments() | featureTicketBatch); + Env env(*this, std::move(cfg)); // The noripple is to reduce the number of transactions required to // fund the accounts. There is no rippling in this test. diff --git a/src/test/jtx/Env_test.cpp b/src/test/jtx/Env_test.cpp index 370cfbcac..b36c1c821 100644 --- a/src/test/jtx/Env_test.cpp +++ b/src/test/jtx/Env_test.cpp @@ -483,7 +483,7 @@ public: ticket::create("alice", 1); { - Env env(*this, supported_amendments() | featureTicketBatch); + Env env(*this); env.fund(XRP(10000), "alice"); env(noop("alice"), require(owners("alice", 0), tickets("alice", 0))); diff --git a/src/test/rpc/AccountObjects_test.cpp b/src/test/rpc/AccountObjects_test.cpp index 70c70e697..64e77b930 100644 --- a/src/test/rpc/AccountObjects_test.cpp +++ b/src/test/rpc/AccountObjects_test.cpp @@ -346,7 +346,7 @@ public: Account const gw{"gateway"}; auto const USD = gw["USD"]; - Env env(*this, supported_amendments() | featureTicketBatch); + Env env(*this); // Make a lambda we can use to get "account_objects" easily. auto acct_objs = [&env](Account const& acct, char const* type) { diff --git a/src/test/rpc/AccountSet_test.cpp b/src/test/rpc/AccountSet_test.cpp index a3fe45e77..23716b665 100644 --- a/src/test/rpc/AccountSet_test.cpp +++ b/src/test/rpc/AccountSet_test.cpp @@ -507,7 +507,7 @@ public: testTicket() { using namespace test::jtx; - Env env(*this, supported_amendments() | featureTicketBatch); + Env env(*this); Account const alice("alice"); env.fund(XRP(10000), alice); diff --git a/src/test/rpc/AccountTx_test.cpp b/src/test/rpc/AccountTx_test.cpp index dbcd791ff..8734dc6fe 100644 --- a/src/test/rpc/AccountTx_test.cpp +++ b/src/test/rpc/AccountTx_test.cpp @@ -252,7 +252,7 @@ class AccountTx_test : public beast::unit_test::suite using namespace test::jtx; using namespace std::chrono_literals; - Env env(*this, supported_amendments() | featureTicketBatch); + Env env(*this); Account const alice{"alice"}; Account const alie{"alie"}; Account const gw{"gw"}; diff --git a/src/test/rpc/LedgerData_test.cpp b/src/test/rpc/LedgerData_test.cpp index 05e0876c6..2a43dc010 100644 --- a/src/test/rpc/LedgerData_test.cpp +++ b/src/test/rpc/LedgerData_test.cpp @@ -18,7 +18,6 @@ //============================================================================== #include -#include #include #include @@ -309,10 +308,7 @@ public: // Put a bunch of different LedgerEntryTypes into a ledger using namespace test::jtx; using namespace std::chrono; - Env env{ - *this, - envconfig(validator, ""), - supported_amendments() | featureTicketBatch}; + Env env{*this, envconfig(validator, "")}; Account const gw{"gateway"}; auto const USD = gw["USD"]; diff --git a/src/test/rpc/LedgerRPC_test.cpp b/src/test/rpc/LedgerRPC_test.cpp index f026db121..13a2f9824 100644 --- a/src/test/rpc/LedgerRPC_test.cpp +++ b/src/test/rpc/LedgerRPC_test.cpp @@ -22,7 +22,6 @@ #include #include #include -#include #include #include @@ -1107,7 +1106,7 @@ class LedgerRPC_test : public beast::unit_test::suite { testcase("ledger_entry Request Ticket"); using namespace test::jtx; - Env env{*this, supported_amendments() | featureTicketBatch}; + Env env{*this}; env.close(); // Create two tickets.