Always closeLedger in applyCheck

src/herder/test/HerderTests.cpp

@@ -274,8 +274,7 @@ testTxSet(uint32 protocolVersion)
     int64_t amountPop =
         nbTransactions * app->getLedgerManager().getLastTxFee() + minBalance0;
 
-    TxSetFramePtr txSet = std::make_shared<TxSetFrame>(
-        app->getLedgerManager().getLastClosedLedgerHeader().hash);
+    TxSetFramePtr txSet = std::make_shared<TxSetFrame>(Hash{});
 
     auto genTx = [&](int nbTxs) {
         std::string accountName = fmt::format("A{}", accounts.size());
@@ -293,13 +292,18 @@ testTxSet(uint32 protocolVersion)
         genTx(nbTransactions);
     }
 
+    txSet->previousLedgerHash() =
+        app->getLedgerManager().getLastClosedLedgerHeader().hash;
+
     SECTION("too many txs")
     {
         while (txSet->mTransactions.size() <=
                cfg.TESTING_UPGRADE_MAX_TX_SET_SIZE)
         {
             genTx(1);
         }
+        txSet->previousLedgerHash() =
+            app->getLedgerManager().getLastClosedLedgerHeader().hash;
         txSet->sortForHash();
         REQUIRE(!txSet->checkValid(*app, 0, 0));
     }
@@ -446,14 +450,14 @@ testTxSetWithFeeBumps(uint32 protocolVersion)
     auto const minBalance0 = app->getLedgerManager().getLastMinBalance(0);
     auto const minBalance2 = app->getLedgerManager().getLastMinBalance(2);
 
-    auto txSet = std::make_shared<TxSetFrame>(
-        app->getLedgerManager().getLastClosedLedgerHeader().hash);
-
     auto root = TestAccount::createRoot(*app);
     auto account1 = root.create("a1", minBalance2);
     auto account2 = root.create("a2", minBalance2);
     auto account3 = root.create("a3", minBalance2);
 
+    auto txSet = std::make_shared<TxSetFrame>(
+        app->getLedgerManager().getLastClosedLedgerHeader().hash);
+
     auto checkTrimCheck = [&](std::vector<TransactionFrameBasePtr> const& txs) {
         txSet->sortForHash();
         REQUIRE(!txSet->checkValid(*app, 0, 0));
@@ -688,8 +692,7 @@ TEST_CASE("txset base fee", "[herder][txset]")
 
         auto accounts = std::vector<TestAccount>{};
 
-        TxSetFramePtr txSet = std::make_shared<TxSetFrame>(
-            app->getLedgerManager().getLastClosedLedgerHeader().hash);
+        TxSetFramePtr txSet = std::make_shared<TxSetFrame>(Hash{});
 
         for (uint32 i = 0; i < nbTransactions; i++)
         {
@@ -713,6 +716,9 @@ TEST_CASE("txset base fee", "[herder][txset]")
 
         REQUIRE(txSet->size(lhCopy) == lim);
         REQUIRE(extraAccounts >= 2);
+
+        txSet->previousLedgerHash() =
+            app->getLedgerManager().getLastClosedLedgerHeader().hash;
         txSet->sortForHash();
         REQUIRE(txSet->checkValid(*app, 0, 0));
 
@@ -727,7 +733,7 @@ TEST_CASE("txset base fee", "[herder][txset]")
         auto balancesBefore = getBalances();
 
         // apply this
-        closeLedgerOn(*app, 2, 1, 1, 2020, txSet->mTransactions);
+        closeLedger(*app, txSet->mTransactions);
 
         auto balancesAfter = getBalances();
         int64_t lowFee = INT64_MAX, highFee = 0;
@@ -2490,8 +2496,11 @@ externalize(SecretKey const& sk, LedgerManager& lm, HerderImpl& herder,
     herder.getPendingEnvelopes().putTxSet(txSet->getContentsHash(), ledgerSeq,
                                           txSet);
 
-    StellarValue sv = herder.makeStellarValue(
-        txSet->getContentsHash(), 2, xdr::xvector<UpgradeType, 6>{}, sk);
+    auto lastCloseTime = lcl.header.scpValue.closeTime;
+
+    StellarValue sv =
+        herder.makeStellarValue(txSet->getContentsHash(), lastCloseTime,
+                                xdr::xvector<UpgradeType, 6>{}, sk);
     herder.getHerderSCPDriver().valueExternalized(ledgerSeq,
                                                   xdr::xdr_to_opaque(sv));
 }

src/herder/test/TransactionQueueTests.cpp

@@ -893,12 +893,14 @@ TEST_CASE("transaction queue starting sequence boundary",
     auto root = TestAccount::createRoot(*app);
     auto acc1 = root.create("a1", minBalance2);
 
-    closeLedgerOn(*app, 2, 1, 1, 2020);
-    closeLedgerOn(*app, 3, 1, 1, 2020);
+    closeLedger(*app);
+    closeLedger(*app);
+
+    auto nextLedgerSeq = app->getLedgerManager().getLastClosedLedgerNum();
 
     SECTION("check a single transaction")
     {
-        int64_t startingSeq = static_cast<int64_t>(4) << 32;
+        int64_t startingSeq = static_cast<int64_t>(nextLedgerSeq) << 32;
         REQUIRE(acc1.loadSequenceNumber() < startingSeq);
         acc1.bumpSequence(startingSeq - 1);
         REQUIRE(acc1.loadSequenceNumber() == startingSeq - 1);
@@ -921,7 +923,7 @@ TEST_CASE("transaction queue starting sequence boundary",
 
     SECTION("check a chain of transactions")
     {
-        int64_t startingSeq = static_cast<int64_t>(4) << 32;
+        int64_t startingSeq = static_cast<int64_t>(nextLedgerSeq) << 32;
         REQUIRE(acc1.loadSequenceNumber() < startingSeq);
         acc1.bumpSequence(startingSeq - 3);
         REQUIRE(acc1.loadSequenceNumber() == startingSeq - 3);
@@ -1435,9 +1437,10 @@ TEST_CASE("remove applied", "[herder][transactionqueue]")
         herder.getPendingEnvelopes().putTxSet(txSet->getContentsHash(),
                                               ledgerSeq, txSet);
 
-        StellarValue sv = herder.makeStellarValue(txSet->getContentsHash(), 2,
-                                                  emptyUpgradeSteps,
-                                                  app->getConfig().NODE_SEED);
+        auto lastCloseTime = lcl.header.scpValue.closeTime;
+        StellarValue sv = herder.makeStellarValue(
+            txSet->getContentsHash(), lastCloseTime, emptyUpgradeSteps,
+            app->getConfig().NODE_SEED);
         herder.getHerderSCPDriver().valueExternalized(ledgerSeq,
                                                       xdr::xdr_to_opaque(sv));
     }

src/test/TxTests.cpp

@@ -114,10 +114,11 @@ expectedResult(int64_t fee, size_t opsCount, TransactionResultCode code,
 bool
 applyCheck(TransactionFramePtr tx, Application& app, bool checkSeqNum)
 {
+    // Close the ledger here to advance ledgerSeq
+    closeLedger(app);
+
     LedgerTxn ltx(app.getLedgerTxnRoot());
-    // Increment ledgerSeq to simulate the behavior of closeLedger, which begins
-    // by advancing the ledgerSeq.
-    ++ltx.loadHeader().current().ledgerSeq;
+
     auto ledgerVersion = ltx.loadHeader().current().ledgerVersion;
 
     bool check = false;
@@ -341,11 +342,8 @@ applyCheck(TransactionFramePtr tx, Application& app, bool checkSeqNum)
         recordOrCheckGlobalTestTxMetadata(tm);
     }
 
-    // Undo the increment from the beginning of this function. Note that if this
-    // function exits without reaching this point, then ltx will not be
-    // committed and the increment will be rolled back anyway.
-    --ltx.loadHeader().current().ledgerSeq;
     ltx.commit();
+
     return res;
 }
 
@@ -427,6 +425,15 @@ closeLedgerOn(Application& app, uint32 ledgerSeq, int day, int month, int year,
                          strictOrder);
 }
 
+TxSetResultMeta
+closeLedgerOn(Application& app, int day, int month, int year,
+              std::vector<TransactionFrameBasePtr> const& txs, bool strictOrder)
+{
+    auto nextLedgerSeq = app.getLedgerManager().getLastClosedLedgerNum() + 1;
+    return closeLedgerOn(app, nextLedgerSeq, getTestDate(day, month, year), txs,
+                         strictOrder);
+}
+
 class TxSetFrameStrictOrderForTesting : public TxSetFrame
 {
   public:
@@ -442,10 +449,31 @@ class TxSetFrameStrictOrderForTesting : public TxSetFrame
     void sortForHash() override{};
 };
 
+TxSetResultMeta
+closeLedger(Application& app, std::vector<TransactionFrameBasePtr> const& txs,
+            bool strictOrder)
+{
+    auto lastCloseTime = app.getLedgerManager()
+                             .getLastClosedLedgerHeader()
+                             .header.scpValue.closeTime;
+
+    auto nextLedgerSeq = app.getLedgerManager().getLastClosedLedgerNum() + 1;
+
+    return closeLedgerOn(app, nextLedgerSeq, lastCloseTime, txs, strictOrder);
+}
+
 TxSetResultMeta
 closeLedgerOn(Application& app, uint32 ledgerSeq, time_t closeTime,
               std::vector<TransactionFrameBasePtr> const& txs, bool strictOrder)
 {
+    auto lastCloseTime = app.getLedgerManager()
+                             .getLastClosedLedgerHeader()
+                             .header.scpValue.closeTime;
+    if (closeTime < lastCloseTime)
+    {
+        closeTime = lastCloseTime;
+    }
+
     std::shared_ptr<TxSetFrame> txSet;
     auto lclHash = app.getLedgerManager().getLastClosedLedgerHeader().hash;
     if (strictOrder)
@@ -1459,8 +1487,9 @@ executeUpgrades(Application& app, xdr::xvector<UpgradeType, 6> const& upgrades)
     auto const& lcl = lm.getLastClosedLedgerHeader();
     auto txSet = std::make_shared<TxSetFrame>(lcl.hash);
 
-    app.getHerder().externalizeValue(txSet, lcl.header.ledgerSeq + 1, 2,
-                                     upgrades);
+    auto lastCloseTime = lcl.header.scpValue.closeTime;
+    app.getHerder().externalizeValue(txSet, lcl.header.ledgerSeq + 1,
+                                     lastCloseTime, upgrades);
     return lm.getLastClosedLedgerHeader().header;
 };
 

src/test/TxTests.h

@@ -85,6 +85,16 @@ void checkLiquidityPool(Application& app, PoolID const& poolID,
                         int64_t totalPoolShares,
                         int64_t poolSharesTrustLineCount);
 
+TxSetResultMeta
+closeLedger(Application& app,
+            std::vector<TransactionFrameBasePtr> const& txs = {},
+            bool strictOrder = false);
+
+TxSetResultMeta
+closeLedgerOn(Application& app, int day, int month, int year,
+              std::vector<TransactionFrameBasePtr> const& txs = {},
+              bool strictOrder = false);
+
 TxSetResultMeta
 closeLedgerOn(Application& app, uint32 ledgerSeq, time_t closeTime,
               std::vector<TransactionFrameBasePtr> const& txs = {},

src/transactions/test/BumpSequenceTests.cpp

@@ -86,12 +86,10 @@ TEST_CASE_VERSIONS("bump sequence", "[tx][bumpsequence]")
     SECTION("seqnum equals starting sequence")
     {
         for_versions_from(10, *app, [&]() {
-            closeLedgerOn(*app, 2, 1, 1, 2020);
-
             int64_t newSeq = 0;
             {
                 LedgerTxn ltx(app->getLedgerTxnRoot());
-                auto ledgerSeq = ltx.loadHeader().current().ledgerSeq + 1;
+                auto ledgerSeq = ltx.loadHeader().current().ledgerSeq + 2;
                 newSeq = getStartingSequenceNumber(ledgerSeq) - 1;
             }
 

src/transactions/test/ChangeTrustTests.cpp

@@ -79,7 +79,6 @@ TEST_CASE_VERSIONS("change trust", "[tx][changetrust]")
         }
         SECTION("edit existing")
         {
-            closeLedgerOn(*app, 2, 1, 1, 2016);
             for_all_versions(*app, [&] {
                 root.changeTrust(idr, 100);
                 // Merge gateway back into root (the trustline still exists)

src/transactions/test/ClaimableBalanceTests.cpp

@@ -1101,15 +1101,13 @@ TEST_CASE_VERSIONS("claimableBalance", "[tx][claimablebalance]")
             auto accA = root.create("accA", minBalance3);
             auto accB = root.create("accB", lm.getLastMinBalance(4));
 
-            // Allow accA to submit an op from accB. This will bump accB's
-            // seqnum up by 1
+            // Allow accA to submit an op from accB.
             auto sk1 = makeSigner(accA, 100);
             accB.setOptions(setSigner(sk1));
 
-            // Move accA seqnum up by one so accA and accB have the same seqnum
+            // Move accA seqnum to accB's
             accA.bumpSequence(accB.getLastSequenceNumber());
-            REQUIRE(accA.getLastSequenceNumber() ==
-                    accB.getLastSequenceNumber());
+            REQUIRE(accA.loadSequenceNumber() == accB.loadSequenceNumber());
 
             // accB and accA have the same seq num. Create a claimable balance
             // with accB twice. Once using accB as the Tx account, and once with

src/transactions/test/FeeBumpTransactionTests.cpp

@@ -268,7 +268,6 @@ TEST_CASE_VERSIONS("fee bump transactions", "[tx][feebump]")
         SECTION("fee source does not exist")
         {
             auto acc = root.create("A", 2 * reserve + 3 * fee);
-            closeLedgerOn(*app, 2, 1, 2, 2016);
             for_versions_from(13, *app, [&] {
                 auto fb = feeBump(app->getNetworkID(), acc, root, root, 2 * fee,
                                   fee, 1);

src/transactions/test/InflationTests.cpp

@@ -466,34 +466,34 @@ TEST_CASE_VERSIONS("inflation", "[tx][inflation]")
     SECTION("not time")
     {
         for_versions_to(11, *app, [&] {
-            closeLedgerOn(*app, 2, 30, 6, 2014);
+            closeLedgerOn(*app, 30, 6, 2014);
             REQUIRE_THROWS_AS(root.inflation(), ex_INFLATION_NOT_TIME);
 
             REQUIRE(getInflationSeq() == 0);
 
-            closeLedgerOn(*app, 3, 1, 7, 2014);
+            closeLedgerOn(*app, 1, 7, 2014);
 
             auto txFrame = root.tx({inflation()});
 
-            closeLedgerOn(*app, 4, 7, 7, 2014, {txFrame});
+            closeLedgerOn(*app, 7, 7, 2014, {txFrame});
             REQUIRE(getInflationSeq() == 1);
 
             REQUIRE_THROWS_AS(root.inflation(), ex_INFLATION_NOT_TIME);
             REQUIRE(getInflationSeq() == 1);
 
-            closeLedgerOn(*app, 5, 8, 7, 2014);
+            closeLedgerOn(*app, 8, 7, 2014);
             root.inflation();
             REQUIRE(getInflationSeq() == 2);
 
-            closeLedgerOn(*app, 6, 14, 7, 2014);
+            closeLedgerOn(*app, 14, 7, 2014);
             REQUIRE_THROWS_AS(root.inflation(), ex_INFLATION_NOT_TIME);
             REQUIRE(getInflationSeq() == 2);
 
-            closeLedgerOn(*app, 7, 15, 7, 2014);
+            closeLedgerOn(*app, 15, 7, 2014);
             root.inflation();
             REQUIRE(getInflationSeq() == 3);
 
-            closeLedgerOn(*app, 8, 21, 7, 2014);
+            closeLedgerOn(*app, 21, 7, 2014);
             REQUIRE_THROWS_AS(root.inflation(), ex_INFLATION_NOT_TIME);
             REQUIRE(getInflationSeq() == 3);
         });
@@ -517,7 +517,8 @@ TEST_CASE_VERSIONS("inflation", "[tx][inflation]")
                 if (nbAccounts != 0)
                 {
                     createTestAccounts(*app, nbAccounts, balanceFunc, voteFunc);
-                    closeLedgerOn(*app, 2, 21, 7, 2014);
+
+                    closeLedgerOn(*app, 21, 7, 2014);
 
                     doInflation(*app, getLedgerVersion(), nbAccounts,
                                 balanceFunc, voteFunc, expectedWinners);
@@ -634,7 +635,7 @@ TEST_CASE_VERSIONS("inflation", "[tx][inflation]")
             });
             root.pay(a0, txfee);
 
-            closeLedgerOn(*app, 2, 21, 7, 2014);
+            closeLedgerOn(*app, 21, 7, 2014);
 
             size_t expectedWinners = (expectedPayout > 0);
             doInflation(*app, getLedgerVersion(), 2, balanceFunc, voteFunc,

src/transactions/test/ManageBuyOfferTests.cpp

@@ -157,7 +157,6 @@ TEST_CASE_VERSIONS("manage buy offer failure modes", "[tx][offers]")
                 auto a1 = root.create("a1", minBalance3PlusFees);
                 a1.changeTrust(cur1, INT64_MAX);
                 issuer1.pay(a1, cur1, 1);
-                closeLedgerOn(*app, 2, 1, 1, 2016);
 
                 // remove issuer
                 issuer1.merge(root);