Merge pull request #3787 from marta-lokhova/loadgen_source_accounts

tests: turn on LIMIT_TX_QUEUE_SOURCE_ACCOUNT by default and rewrite test cases to support it

Reviewed-by: sisuresh

src/herder/TxSetFrame.cpp

@@ -641,9 +641,6 @@ TxSetFrame::checkValid(Application& app, uint64_t lowerBoundCloseTimeOffset,
     {
         // First, ensure the tx set does not contain multiple txs per source
         // account
-        // FIXME: Our test suite relies on tx chains per source account, so
-        // introducing this invariant causes a fallout. When the test suite is
-        // updated to accommodate 1 tx/source account, remove this flag.
         if (app.getConfig().LIMIT_TX_QUEUE_SOURCE_ACCOUNT)
         {
             std::unordered_set<AccountID> seenAccounts;

src/herder/test/PendingEnvelopesTests.cpp

@@ -35,7 +35,13 @@ TEST_CASE("PendingEnvelopes recvSCPEnvelope", "[herder]")
     auto& herder = static_cast<HerderImpl&>(app->getHerder());
 
     auto root = TestAccount::createRoot(*app);
-    auto a1 = TestAccount{*app, getAccount("A")};
+    size_t numAccounts = 50;
+    std::vector<TestAccount> accs;
+    for (size_t i = 0; i < numAccounts; i++)
+    {
+        accs.push_back(TestAccount{*app, getAccount("A" + std::to_string(i))});
+    }
+
     using TxPair = std::pair<Value, TxSetFrameConstPtr>;
     auto makeTxPair = [&](TxSetFrameConstPtr txSet, uint64_t closeTime,
                           StellarValueType svt) {
@@ -61,10 +67,12 @@ TEST_CASE("PendingEnvelopes recvSCPEnvelope", "[herder]")
         herder.signEnvelope(s, envelope);
         return envelope;
     };
+    size_t index = 0;
     auto makeTransactions = [&](Hash hash, int n) {
+        REQUIRE(n <= accs.size());
         std::vector<TransactionFrameBasePtr> txs(n);
         std::generate(std::begin(txs), std::end(txs),
-                      [&]() { return root.tx({createAccount(a1, 10000000)}); });
+                      [&]() { return accs[index++].tx({payment(root, 1)}); });
         return TxSetFrame::makeFromTransactions(txs, *app, 0, 0);
     };
 
@@ -102,7 +110,7 @@ TEST_CASE("PendingEnvelopes recvSCPEnvelope", "[herder]")
     }
     auto bigQSetHash = sha256(xdr::xdr_to_opaque(bigQSet));
 
-    auto txSet = makeTransactions(lcl.hash, 50);
+    auto txSet = makeTransactions(lcl.hash, numAccounts);
     auto p = makeTxPair(txSet, 10, STELLAR_VALUE_SIGNED);
     auto saneEnvelope = makeEnvelope(p, saneQSetHash, lcl.header.ledgerSeq + 1);
     auto bigEnvelope = makeEnvelope(p, bigQSetHash, lcl.header.ledgerSeq + 1);

src/herder/test/TxSetTests.cpp

@@ -445,12 +445,16 @@ TEST_CASE("generalized tx set XDR conversion", "[txset]")
         static_cast<uint32_t>(GENERALIZED_TX_SET_PROTOCOL_VERSION);
     Application::pointer app = createTestApplication(clock, cfg);
     auto root = TestAccount::createRoot(*app);
+    int accountId = 0;
     auto createTxs = [&](int cnt, int fee) {
         std::vector<TransactionFrameBasePtr> txs;
         for (int i = 0; i < cnt; ++i)
         {
+            auto source =
+                root.create("unique " + std::to_string(accountId++),
+                            app->getLedgerManager().getLastMinBalance(2));
             txs.emplace_back(transactionFromOperations(
-                *app, root.getSecretKey(), root.nextSequenceNumber(),
+                *app, source.getSecretKey(), source.nextSequenceNumber(),
                 {createAccount(getAccount(std::to_string(i)).getPublicKey(),
                                1)},
                 fee));
@@ -547,12 +551,8 @@ TEST_CASE("generalized tx set XDR conversion", "[txset]")
     {
         auto const& lclHeader =
             app->getLedgerManager().getLastClosedLedgerHeader();
-        std::vector<TransactionFrameBasePtr> txs;
-        for (int i = 0; i < 5; ++i)
-        {
-            txs.push_back(root.tx({createAccount(
-                getAccount(std::to_string(i)).getPublicKey(), 1)}));
-        }
+        std::vector<TransactionFrameBasePtr> txs =
+            createTxs(5, lclHeader.header.baseFee);
         auto txSet = TxSetFrame::makeFromTransactions(txs, *app, 0, 0);
 
         GeneralizedTransactionSet txSetXdr;
@@ -655,8 +655,11 @@ TEST_CASE("generalized tx set fees", "[txset]")
             ops.emplace_back(createAccount(
                 getAccount(std::to_string(accountId++)).getPublicKey(), 1));
         }
-        return transactionFromOperations(*app, root.getSecretKey(),
-                                         root.nextSequenceNumber(), ops, fee);
+        // Create a new unique accounts to ensure there are no collisions
+        auto source = root.create("unique " + std::to_string(accountId),
+                                  app->getLedgerManager().getLastMinBalance(2));
+        return transactionFromOperations(*app, source.getSecretKey(),
+                                         source.nextSequenceNumber(), ops, fee);
     };
 
     SECTION("valid txset")

src/history/test/HistoryTestsUtils.cpp

@@ -423,35 +423,51 @@ CatchupSimulation::generateRandomLedger(uint32_t version)
     auto carol = TestAccount{mApp, getAccount("carol")};
 
     std::vector<TransactionFrameBasePtr> txs;
-    // Root sends to alice every tx, bob every other tx, carol every 4rd tx.
     if (ledgerSeq < 5)
     {
-        txs.push_back(root.tx({createAccount(alice, big)}));
-        txs.push_back(root.tx({createAccount(bob, big)}));
-        txs.push_back(root.tx({createAccount(carol, big)}));
+        txs.push_back(
+            root.tx({createAccount(alice, big), createAccount(bob, big),
+                     createAccount(carol, big)}));
     }
     // Allow an occasional empty ledger
     else if (rand_flip() || rand_flip())
     {
-        txs.push_back(root.tx({payment(alice, big)}));
-        txs.push_back(root.tx({payment(bob, big)}));
-        txs.push_back(root.tx({payment(carol, big)}));
-
         // They all randomly send a little to one another every ledger after #4
         if (rand_flip())
-            txs.push_back(alice.tx({payment(bob, small)}));
+        {
+            txs.push_back(root.tx({payment(alice, big)}));
+        }
+        else
+        {
+            txs.push_back(root.tx({payment(bob, big)}));
+        }
+
         if (rand_flip())
+        {
+            txs.push_back(alice.tx({payment(bob, small)}));
+        }
+        else
+        {
             txs.push_back(alice.tx({payment(carol, small)}));
+        }
 
         if (rand_flip())
+        {
             txs.push_back(bob.tx({payment(alice, small)}));
-        if (rand_flip())
+        }
+        else
+        {
             txs.push_back(bob.tx({payment(carol, small)}));
+        }
 
         if (rand_flip())
+        {
             txs.push_back(carol.tx({payment(alice, small)}));
-        if (rand_flip())
+        }
+        else
+        {
             txs.push_back(carol.tx({payment(bob, small)}));
+        }
     }
     TxSetFrameConstPtr txSet =
         TxSetFrame::makeFromTransactions(txs, mApp, 0, 0);

src/main/Config.cpp

@@ -167,12 +167,7 @@ Config::Config() : NODE_SEED(SecretKey::random())
     USE_CONFIG_FOR_GENESIS = false;
     FAILURE_SAFETY = -1;
     UNSAFE_QUORUM = false;
-    LIMIT_TX_QUEUE_SOURCE_ACCOUNT =
-#ifdef ENABLE_NEXT_PROTOCOL_VERSION_UNSAFE_FOR_PRODUCTION
-        true;
-#else
-        false;
-#endif
+    LIMIT_TX_QUEUE_SOURCE_ACCOUNT = true;
     DISABLE_BUCKET_GC = false;
     DISABLE_XDR_FSYNC = false;
     MAX_SLOTS_TO_REMEMBER = 12;

src/simulation/test/LoadGeneratorTests.cpp

@@ -21,7 +21,6 @@ TEST_CASE("generate load with unique accounts", "[loadgen]")
     Simulation::pointer simulation =
         Topologies::pair(Simulation::OVER_LOOPBACK, networkID, [](int i) {
             auto cfg = getTestConfig(i);
-            cfg.LIMIT_TX_QUEUE_SOURCE_ACCOUNT = true;
             cfg.TESTING_UPGRADE_MAX_TX_SET_SIZE = 5000;
             return cfg;
         });
@@ -97,7 +96,6 @@ TEST_CASE("Multi-op pretend transactions are valid", "[loadgen]")
     Simulation::pointer simulation =
         Topologies::pair(Simulation::OVER_LOOPBACK, networkID, [](int i) {
             auto cfg = getTestConfig(i);
-            cfg.LIMIT_TX_QUEUE_SOURCE_ACCOUNT = true;
             // 50% of transactions contain 2 ops,
             // and 50% of transactions contain 3 ops.
             cfg.LOADGEN_OP_COUNT_FOR_TESTING = {2, 3};
@@ -170,7 +168,6 @@ TEST_CASE("Multi-op mixed transactions are valid", "[loadgen]")
     Simulation::pointer simulation =
         Topologies::pair(Simulation::OVER_LOOPBACK, networkID, [](int i) {
             auto cfg = getTestConfig(i);
-            cfg.LIMIT_TX_QUEUE_SOURCE_ACCOUNT = true;
             cfg.LOADGEN_OP_COUNT_FOR_TESTING = {3};
             cfg.LOADGEN_OP_COUNT_DISTRIBUTION_FOR_TESTING = {1};
             cfg.TESTING_UPGRADE_MAX_TX_SET_SIZE = 1000;

src/test/test.cpp

@@ -231,11 +231,6 @@ getTestConfig(int instanceNumber, Config::TestDbMode mode)
 
         thisConfig.INVARIANT_CHECKS = {".*"};
 
-        // Enabling this config causes a major fallout in the test suite, as
-        // many test use account chains We'll deal with this later - tests that
-        // mix Soroban and classic txs enable this flag manually anyway
-        thisConfig.LIMIT_TX_QUEUE_SOURCE_ACCOUNT = false;
-
         thisConfig.ALLOW_LOCALHOST_FOR_TESTING = true;
 
         // this forces to pick up any other potential upgrades

src/transactions/test/MergeTests.cpp

@@ -652,17 +652,10 @@ TEST_CASE_VERSIONS("merge", "[tx][merge]")
             }
             SECTION("merge source account")
             {
-                auto tx1 = transactionFrameFromOps(app->getNetworkID(), a1,
-                                                   {accountMerge(b1)}, {});
-
-                // Add some intermediate transactions between the merge and the
-                // gap tx so MAX_SEQ_NUM_TO_APPLY receives additional updates
-                auto tx2 = transactionFrameFromOps(app->getNetworkID(), a1,
-                                                   {payment(root, 1)}, {});
-                auto tx3 = transactionFrameFromOps(app->getNetworkID(), a1,
-                                                   {payment(root, 1)}, {});
-
-                auto r = closeLedger(*app, {tx1, tx2, tx3, txMinSeqNumSrc});
+                auto tx1 =
+                    transactionFrameFromOps(app->getNetworkID(), gateway,
+                                            {a1.op(accountMerge(b1))}, {a1});
+                auto r = closeLedger(*app, {tx1, txMinSeqNumSrc}, true);
 
                 REQUIRE(r[0].first.result.result.results()[0]
                             .tr()
@@ -676,8 +669,9 @@ TEST_CASE_VERSIONS("merge", "[tx][merge]")
                 auto tx1 = transactionFrameFromOps(
                     app->getNetworkID(), root,
                     {a1.op(bumpSequence(curStartSeqNum))}, {a1});
-                auto tx2 = transactionFrameFromOps(
-                    app->getNetworkID(), root, {a1.op(accountMerge(b1))}, {a1});
+                auto tx2 =
+                    transactionFrameFromOps(app->getNetworkID(), gateway,
+                                            {a1.op(accountMerge(b1))}, {a1});
 
                 auto r = closeLedger(*app, {tx1, tx2}, true);
 

src/transactions/test/PaymentTests.cpp

@@ -256,10 +256,9 @@ TEST_CASE_VERSIONS("payment", "[tx][payment]")
         int64 startingBalance = paymentAmount + 5 +
                                 app->getLedgerManager().getLastMinBalance(0) +
                                 txfee * 2;
-        auto b1 = root.create("B", startingBalance);
-        auto rootBalance = root.getBalance();
-
         for_versions_to(8, *app, [&] {
+            auto b1 = root.create("B", startingBalance);
+            auto rootBalance = root.getBalance();
             auto tx1 = b1.tx({payment(root, paymentAmount)});
             auto tx2 = b1.tx({payment(root, 6)});
 
@@ -275,10 +274,16 @@ TEST_CASE_VERSIONS("payment", "[tx][payment]")
         });
 
         for_versions_from(9, *app, [&] {
+            // Starting balance adjusted to have enough fees for 1 tx only
+            // (since tx1 and tx2 have different fee)
+            auto b1 = root.create("B", startingBalance - txfee);
+            auto b2 = root.create("B2", startingBalance - txfee);
+            auto rootBalance = root.getBalance();
             auto tx1 = b1.tx({payment(root, paymentAmount)});
-            auto tx2 = b1.tx({payment(root, 6)});
+            auto tx2 = b2.tx({b1.op(payment(root, 6))});
+            tx2->addSignature(b1);
 
-            auto r = closeLedger(*app, {tx1, tx2});
+            auto r = closeLedger(*app, {tx1, tx2}, /* strictOrder */ true);
             checkTx(0, r, txSUCCESS);
             checkTx(1, r, txFAILED);
             REQUIRE(r[1].first.result.result.results()[0]

src/transactions/test/TxEnvelopeTests.cpp

@@ -1304,9 +1304,9 @@ TEST_CASE_VERSIONS("txenvelope", "[tx][envelope]")
                             tx1 = b.tx({setOptions(
                                 setMasterWeight(1) | setLowThreshold(1) |
                                 setMedThreshold(2) | setHighThreshold(3))});
-                            tx2 = b.tx(
-                                {payment(root, 100), root.op(payment(b, 100))},
-                                b.getLastSequenceNumber() + 1);
+                            tx2 = root.tx(
+                                {b.op(payment(root, 100)), payment(b, 100)},
+                                root.getLastSequenceNumber() + 2);
 
                             SignerKey sk = alternative.createSigner(*tx2);
                             Signer sk1(sk, 100); // high rights account
@@ -1316,12 +1316,12 @@ TEST_CASE_VERSIONS("txenvelope", "[tx][envelope]")
                         };
                         for_versions(3, 9, *app, [&] {
                             setup();
-                            closeLedgerOn(*app, 1, 1, 2010, {tx1, tx2});
+                            closeLedgerOn(*app, 1, 1, 2010, {tx1, tx2}, true);
                             REQUIRE(getAccountSigners(root, *app).size() == 1);
                         });
                         for_versions_from(10, *app, [&] {
                             setup();
-                            closeLedgerOn(*app, 1, 1, 2010, {tx1, tx2});
+                            closeLedgerOn(*app, 1, 1, 2010, {tx1, tx2}, true);
                             REQUIRE(getAccountSigners(root, *app).size() ==
                                     (alternative.autoRemove ? 0 : 1));
                         });
@@ -1335,9 +1335,9 @@ TEST_CASE_VERSIONS("txenvelope", "[tx][envelope]")
                             tx1 = b.tx({setOptions(
                                 setMasterWeight(1) | setLowThreshold(1) |
                                 setMedThreshold(2) | setHighThreshold(3))});
-                            tx2 = b.tx(
-                                {root.op(payment(b, 100)), payment(root, 100)},
-                                b.getLastSequenceNumber() + 1);
+                            tx2 = root.tx(
+                                {payment(b, 100), b.op(payment(root, 100))},
+                                root.getLastSequenceNumber() + 2);
 
                             SignerKey sk = alternative.createSigner(*tx2);
                             Signer sk1(sk, 100); // high rights account
@@ -1347,12 +1347,12 @@ TEST_CASE_VERSIONS("txenvelope", "[tx][envelope]")
                         };
                         for_versions(3, 9, *app, [&] {
                             setup();
-                            closeLedgerOn(*app, 1, 1, 2010, {tx1, tx2});
+                            closeLedgerOn(*app, 1, 1, 2010, {tx1, tx2}, true);
                             REQUIRE(getAccountSigners(root, *app).size() == 1);
                         });
                         for_versions_from(10, *app, [&] {
                             setup();
-                            closeLedgerOn(*app, 1, 1, 2010, {tx1, tx2});
+                            closeLedgerOn(*app, 1, 1, 2010, {tx1, tx2}, true);
                             REQUIRE(getAccountSigners(root, *app).size() ==
                                     (alternative.autoRemove ? 0 : 1));
                         });
@@ -2382,13 +2382,16 @@ TEST_CASE_VERSIONS("txenvelope", "[tx][envelope]")
             SECTION("multiple tx")
             {
                 for_versions_from(10, *app, [&] {
-                    auto tx1 = a.tx({setOptions(setSigner(makeSigner(b, 1)))});
+                    auto tx1 = root.tx(
+                        {a.op(setOptions(setSigner(makeSigner(b, 1))))});
+                    tx1->addSignature(a);
                     tx1->addSignature(b);
+
                     auto tx2 = a.tx({payment(root, 1000),
                                      setOptions(setSigner(makeSigner(b, 2)))});
                     tx2->addSignature(b);
 
-                    auto r = closeLedgerOn(*app, 1, 2, 2016, {tx1, tx2});
+                    auto r = closeLedgerOn(*app, 1, 2, 2016, {tx1, tx2}, true);
 
                     checkTx(0, r, txSUCCESS);
                     checkTx(1, r, txFAILED);