Merge pull request #3414 from SirTyson/bucket-test-fix

Fixes protocol upgrade bug in BucketManager test Reviewed-by: marta-lokhova
stellar · May 19, 2022 · 921917b · 921917b
2 parents 5d2a863 + aa51954
commit 921917b
Show file tree

Hide file tree

Showing 6 changed files with 37 additions and 22 deletions.
diff --git a/src/bucket/test/BucketManagerTests.cpp b/src/bucket/test/BucketManagerTests.cpp
@@ -156,7 +156,8 @@ clearFutures(Application::pointer app, BucketList& bl)
 }
 
 static Hash
-closeLedger(Application& app, std::optional<SecretKey> skToSignValue)
+closeLedger(Application& app, std::optional<SecretKey> skToSignValue,
+            xdr::xvector<UpgradeType, 6> upgrades = emptyUpgradeSteps)
 {
     auto& lm = app.getLedgerManager();
     auto lcl = lm.getLastClosedLedgerHeader();
@@ -166,8 +167,8 @@ closeLedger(Application& app, std::optional<SecretKey> skToSignValue)
               hexAbbrev(app.getBucketManager().getBucketList().getHash()));
     auto txSet = std::make_shared<TxSetFrame const>(lcl.hash);
     app.getHerder().externalizeValue(txSet, ledgerNum,
-                                     lcl.header.scpValue.closeTime,
-                                     emptyUpgradeSteps, skToSignValue);
+                                     lcl.header.scpValue.closeTime, upgrades,
+                                     skToSignValue);
     return lm.getLastClosedLedgerHeader().hash;
 }
 
@@ -1245,7 +1246,24 @@ class StopAndRestartBucketMergesTest
             resolveAllMerges(app->getBucketManager().getBucketList());
             auto countersBeforeClose =
                 app->getBucketManager().readMergeCounters();
-            closeLedger(*app);
+
+            if (firstProtocol != secondProtocol && i == protocolSwitchLedger)
+            {
+                CLOG_INFO(Bucket,
+                          "Switching protocol at ledger {} from protocol {} "
+                          "to protocol {}",
+                          i, firstProtocol, secondProtocol);
+
+                auto ledgerUpgrade = LedgerUpgrade{LEDGER_UPGRADE_VERSION};
+                ledgerUpgrade.newLedgerVersion() = secondProtocol;
+                closeLedger(*app, std::nullopt,
+                            {LedgerTestUtils::toUpgradeType(ledgerUpgrade)});
+                currProtocol = secondProtocol;
+            }
+            else
+            {
+                closeLedger(*app);
+            }
 
             assert(i == app->getLedgerManager()
                             .getLastClosedLedgerHeader()
@@ -1275,16 +1293,11 @@ class StopAndRestartBucketMergesTest
                           "Stopping application after closing ledger {}", i);
                 app.reset();
 
+                // Prepare for protocol upgrade
                 if (firstProtocol != secondProtocol &&
                     i == protocolSwitchLedger)
                 {
-                    CLOG_INFO(
-                        Bucket,
-                        "Switching protocol at ledger {} from protocol {} "
-                        "to protocol {}",
-                        i, firstProtocol, secondProtocol);
-                    cfg.TESTING_UPGRADE_LEDGER_PROTOCOL_VERSION =
-                        secondProtocol;
+                    cfg.LEDGER_PROTOCOL_VERSION = secondProtocol;
                 }
 
                 // Restart the application.

diff --git a/src/herder/test/UpgradesTests.cpp b/src/herder/test/UpgradesTests.cpp
@@ -31,6 +31,7 @@
 
 using namespace stellar;
 using namespace stellar::txtest;
+using stellar::LedgerTestUtils::toUpgradeType;
 
 struct LedgerUpgradeableData
 {

diff --git a/src/ledger/test/LedgerTestUtils.cpp b/src/ledger/test/LedgerTestUtils.cpp
@@ -535,5 +535,13 @@ generateLedgerHeadersForCheckpoint(
     makeValid(res, firstLedger, state);
     return res;
 }
+
+UpgradeType
+toUpgradeType(LedgerUpgrade const& upgrade)
+{
+    auto v = xdr::xdr_to_opaque(upgrade);
+    auto result = UpgradeType{v.begin(), v.end()};
+    return result;
+}
 }
 }
diff --git a/src/ledger/test/LedgerTestUtils.h b/src/ledger/test/LedgerTestUtils.h
@@ -59,5 +59,7 @@ std::vector<LedgerHeaderHistoryEntry> generateLedgerHeadersForCheckpoint(
     LedgerHeaderHistoryEntry firstLedger, uint32_t size,
     HistoryManager::LedgerVerificationStatus state =
         HistoryManager::VERIFY_STATUS_OK);
+
+UpgradeType toUpgradeType(LedgerUpgrade const& upgrade);
 }
 }
diff --git a/src/test/TxTests.cpp b/src/test/TxTests.cpp
@@ -12,6 +12,7 @@
 #include "ledger/LedgerTxn.h"
 #include "ledger/LedgerTxnEntry.h"
 #include "ledger/LedgerTxnHeader.h"
+#include "ledger/test/LedgerTestUtils.h"
 #include "main/Application.h"
 #include "test/TestAccount.h"
 #include "test/TestExceptions.h"
@@ -1510,14 +1511,6 @@ makeBaseReserveUpgrade(int baseReserve)
     return result;
 }
 
-UpgradeType
-toUpgradeType(LedgerUpgrade const& upgrade)
-{
-    auto v = xdr::xdr_to_opaque(upgrade);
-    auto result = UpgradeType{v.begin(), v.end()};
-    return result;
-}
-
 LedgerHeader
 executeUpgrades(Application& app, xdr::xvector<UpgradeType, 6> const& upgrades)
 {
@@ -1534,7 +1527,7 @@ executeUpgrades(Application& app, xdr::xvector<UpgradeType, 6> const& upgrades)
 LedgerHeader
 executeUpgrade(Application& app, LedgerUpgrade const& lupgrade)
 {
-    return executeUpgrades(app, {toUpgradeType(lupgrade)});
+    return executeUpgrades(app, {LedgerTestUtils::toUpgradeType(lupgrade)});
 };
 
 // trades is a vector of pairs, where the bool indicates if assetA or assetB is

diff --git a/src/test/TxTests.h b/src/test/TxTests.h
@@ -271,8 +271,6 @@ transactionFrameFromOps(Hash const& networkID, TestAccount& source,
 
 LedgerUpgrade makeBaseReserveUpgrade(int baseReserve);
 
-UpgradeType toUpgradeType(LedgerUpgrade const& upgrade);
-
 LedgerHeader executeUpgrades(Application& app,
                              xdr::xvector<UpgradeType, 6> const& upgrades);