Fix replay over empty-tx-set ledgers (#5355)

marta-lokhova · web-flow · commit 8612ca32f7a7 · 2026-07-17T21:52:57.000Z
`ApplyCheckpointWork` contained a check that the transaction set hash of
the transaction set to apply exactly matches the one specified in a
ledger header. With CAP-83 however, there is one case where this is not
true. An empty-tx-set ledger will have an all zeros hash in the ledger
header, but the actual constructed empty transaction set will have a
different hash. This PR fixes the check to properly handle empty-tx-set
ledgers. It also adds a few additional checks that an empty-tx-set
ledger is well-formed.

I audited the code for other places where I may have missed these checks
in the original CAP-83 PR, and this is the only one I found.
diff --git a/src/catchup/ApplyCheckpointWork.cpp b/src/catchup/ApplyCheckpointWork.cpp
@@ -6,6 +6,7 @@
 #include "bucket/BucketManager.h"
 #include "bucket/LiveBucketList.h"
 #include "catchup/ApplyLedgerWork.h"
+#include "herder/Herder.h"
 #include "history/FileTransferInfo.h"
 #include "history/HistoryManager.h"
 #include "history/HistoryUtils.h"
@@ -14,6 +15,7 @@
 #include "ledger/LedgerManager.h"
 #include "main/Application.h"
 #include "util/GlobalChecks.h"
+#include "util/ProtocolVersion.h"
 #include "util/XDRCereal.h"
 #include <Tracy.hpp>
 #include <fmt/format.h>
@@ -273,11 +275,46 @@ ApplyCheckpointWork::getNextLedgerCloseData()
     }
 #endif
 
+#ifdef CAP_0083
+    // Empty-tx-set values should have the empty-tx-set hash (and vice versa)
+    if ((header.scpValue.txSetHash == Herder::EMPTY_TX_SET_HASH) !=
+        (header.scpValue.ext.v() == STELLAR_VALUE_EMPTY_TX_SET))
+    {
+        throw std::runtime_error(fmt::format(
+            FMT_STRING("ledger header for {:d} has mismatched empty-tx-set "
+                       "hash and StellarValue type {:d}"),
+            header.ledgerSeq, static_cast<int32_t>(header.scpValue.ext.v())));
+    }
+#endif // CAP_0083
+
     // We've verified the ledgerHeader (in the "trusted part of history"
     // sense) in CATCHUP_VERIFY phase; we now need to check that the
     // txhash we're about to apply is the one denoted by that ledger
-    // header.
-    if (header.scpValue.txSetHash != txset->getContentsHash())
+    // header
+    if (header.scpValue.txSetHash == Herder::EMPTY_TX_SET_HASH)
+    {
+        // Should not see empty-tx-set values prior to the protocol version that
+        // enables them
+        if (!protocolVersionStartsFrom(lclHeader.header.ledgerVersion,
+                                       EMPTY_TX_SET_PROTOCOL_VERSION))
+        {
+            throw std::runtime_error(fmt::format(
+                FMT_STRING("ledger header for {:d} carries the empty-tx-set "
+                           "hash prior to protocol-level support"),
+                header.ledgerSeq));
+        }
+
+        // Empty-tx-set values must have empty tx sets
+        if (txset->sizeTxTotal() != 0)
+        {
+            throw std::runtime_error(fmt::format(
+                FMT_STRING("replay txset for {:d} contains {:d} transactions, "
+                           "but its ledger header carries the empty-tx-set "
+                           "hash"),
+                header.ledgerSeq, txset->sizeTxTotal()));
+        }
+    }
+    else if (header.scpValue.txSetHash != txset->getContentsHash())
     {
         throw std::runtime_error(
             fmt::format(FMT_STRING("replay txset hash differs from txset hash "
diff --git a/src/history/test/HistoryTests.cpp b/src/history/test/HistoryTests.cpp
@@ -4,9 +4,11 @@
 
 #include "bucket/BucketManager.h"
 #include "bucket/test/BucketTestUtils.h"
+#include "catchup/ApplyCheckpointWork.h"
 #include "catchup/DownloadApplyTxsWork.h"
 #include "catchup/LedgerApplyManagerImpl.h"
 #include "catchup/test/CatchupWorkTests.h"
+#include "crypto/SHA.h"
 #include "herder/HerderPersistence.h"
 #include "history/CheckpointBuilder.h"
 #include "history/FileTransferInfo.h"
@@ -26,6 +28,7 @@
 #include "test/test.h"
 #include "util/Fs.h"
 #include "util/Logging.h"
+#include "util/ProtocolVersion.h"
 #include "work/WorkScheduler.h"
 
 #include "historywork/BatchDownloadWork.h"
@@ -1127,6 +1130,203 @@ TEST_CASE("History catchup with extra validation", "[history][publish]")
     REQUIRE(catchupSimulation.catchupOffline(app, checkpointLedger, true));
 }
 
+#ifdef CAP_0083
+TEST_CASE("History catchup over empty-tx-set ledgers", "[history][catchup]")
+{
+    CatchupSimulation catchupSimulation{};
+
+    // Generate a few random ledgers
+    while (
+        catchupSimulation.getApp().getLedgerManager().getLastClosedLedgerNum() <
+        8)
+    {
+        catchupSimulation.generateRandomLedger();
+    }
+
+    // One isolated empty-tx-set ledger, a normal ledger, then two consecutive
+    // empty-tx-set ledgers, all within the first published checkpoint.
+    catchupSimulation.generateEmptyTxSetLedger();
+    catchupSimulation.generateRandomLedger();
+    catchupSimulation.generateEmptyTxSetLedger();
+    catchupSimulation.generateEmptyTxSetLedger();
+
+    auto checkpointLedger = catchupSimulation.getLastCheckpointLedger(1);
+    catchupSimulation.ensureOnlineCatchupPossible(checkpointLedger, 5);
+
+    SECTION("offline")
+    {
+        auto app = catchupSimulation.createCatchupApplication(
+            std::numeric_limits<uint32_t>::max(),
+            Config::TESTDB_BUCKET_DB_PERSISTENT, "skip-offline");
+        REQUIRE(catchupSimulation.catchupOffline(app, checkpointLedger, true));
+    }
+
+    SECTION("online")
+    {
+        auto app = catchupSimulation.createCatchupApplication(
+            std::numeric_limits<uint32_t>::max(),
+            Config::TESTDB_BUCKET_DB_PERSISTENT, "skip-online");
+        REQUIRE(catchupSimulation.catchupOnline(app, checkpointLedger, 5));
+    }
+}
+
+TEST_CASE("History catchup rejects empty-tx-set ledgers with transactions",
+          "[history][catchup]")
+{
+    CatchupSimulation catchupSimulation{};
+    auto& simApp = catchupSimulation.getApp();
+
+    while (simApp.getLedgerManager().getLastClosedLedgerNum() < 8)
+    {
+        catchupSimulation.generateRandomLedger();
+    }
+    catchupSimulation.generateEmptyTxSetLedger();
+    uint32_t const emptyTxSetSeq =
+        simApp.getLedgerManager().getLastClosedLedgerNum();
+
+    auto checkpointLedger = catchupSimulation.getLastCheckpointLedger(1);
+    catchupSimulation.ensureOfflineCatchupPossible(checkpointLedger);
+
+    // Forge a non-empty TransactionHistoryEntry for the empty-tx-set ledger
+    // into the published transactions file, keeping the file sorted by
+    // ledgerSeq.
+    std::string archiveDir =
+        catchupSimulation.getHistoryConfigurator().getArchiveDirName();
+    FileTransferInfo txFileInfo(FileType::HISTORY_FILE_TYPE_TRANSACTIONS,
+                                checkpointLedger, simApp.getConfig());
+    std::string gzPath = archiveDir + "/" + txFileInfo.remoteName();
+    fs::checkGzipSuffix(gzPath);
+    auto nonGzPath = gzPath.substr(0, gzPath.size() - 3);
+
+    auto& wm = simApp.getWorkScheduler();
+    REQUIRE(wm.executeWork<GunzipFileWork>(gzPath)->getState() ==
+            BasicWork::State::WORK_SUCCESS);
+    {
+        XDRInputFileStream in;
+        in.open(nonGzPath);
+        std::vector<TransactionHistoryEntry> txs;
+        TransactionHistoryEntry tx;
+        while (in && in.readOne(tx))
+        {
+            txs.push_back(tx);
+        }
+        in.close();
+        REQUIRE(!txs.empty());
+        REQUIRE(std::filesystem::remove(nonGzPath));
+
+        // Copy an existing non-empty entry and retarget it at the empty-tx-set
+        // ledger
+        TransactionHistoryEntry forged = txs.front();
+        forged.ledgerSeq = emptyTxSetSeq;
+        auto insertAt =
+            std::find_if(txs.begin(), txs.end(), [&](auto const& e) {
+                return e.ledgerSeq > emptyTxSetSeq;
+            });
+        txs.insert(insertAt, forged);
+
+        XDROutputFileStream out(simApp.getClock().getIOContext(), true);
+        out.open(nonGzPath);
+        for (auto const& e : txs)
+        {
+            out.writeOne(e);
+        }
+        out.close();
+    }
+    REQUIRE(wm.executeWork<GzipFileWork>(nonGzPath)->getState() ==
+            BasicWork::State::WORK_SUCCESS);
+
+    // Catch up a fresh node through the tampered checkpoint, driving the
+    // works directly
+    auto app = catchupSimulation.createCatchupApplication(
+        std::numeric_limits<uint32_t>::max(),
+        Config::TESTDB_BUCKET_DB_PERSISTENT, "skip-tamper");
+
+    auto& appWm = app->getWorkScheduler();
+    auto tmpDir = app->getTmpDirManager().tmpDir("skip-tamper-download");
+    LedgerRange range = LedgerRange::inclusive(
+        LedgerManager::GENESIS_LEDGER_SEQ + 1, checkpointLedger);
+    CheckpointRange checkpointRange{range, app->getHistoryManager()};
+
+    auto downloadHeaders = appWm.executeWork<BatchDownloadWork>(
+        checkpointRange, FileType::HISTORY_FILE_TYPE_LEDGER, tmpDir);
+    REQUIRE(downloadHeaders->getState() == BasicWork::State::WORK_SUCCESS);
+
+    auto lastApplied = app->getLedgerManager().getLastClosedLedgerHeader();
+    auto work = appWm.executeWork<DownloadApplyTxsWork>(
+        tmpDir, range, lastApplied, /*waitForPublish=*/true, nullptr);
+    REQUIRE(work->getState() == BasicWork::State::WORK_FAILURE);
+    // Replay stops exactly at the skip ledger whose forged entry was rejected
+    REQUIRE(app->getLedgerManager().getLastClosedLedgerNum() ==
+            emptyTxSetSeq - 1);
+}
+
+TEST_CASE("ApplyCheckpointWork rejects malformed empty-tx-set ledger headers",
+          "[history][catchup]")
+{
+    auto runWithFabricatedHeader =
+        [](uint32_t genesisVersion, std::function<void(StellarValue&)> mutate) {
+            Config cfg(getTestConfig(0));
+            cfg.TESTING_UPGRADE_LEDGER_PROTOCOL_VERSION = genesisVersion;
+            VirtualClock clock;
+            auto app = createTestApplication(clock, cfg);
+            auto const& lcl =
+                app->getLedgerManager().getLastClosedLedgerHeader();
+
+            LedgerHeaderHistoryEntry entry;
+            entry.header.ledgerSeq = lcl.header.ledgerSeq + 1;
+            entry.header.previousLedgerHash = lcl.hash;
+            entry.header.ledgerVersion = lcl.header.ledgerVersion;
+            mutate(entry.header.scpValue);
+            entry.hash = sha256(xdr::xdr_to_opaque(entry.header));
+
+            auto checkpoint = HistoryManager::checkpointContainingLedger(
+                entry.header.ledgerSeq, app->getConfig());
+            auto tmpDir =
+                app->getTmpDirManager().tmpDir("malformed-empty-tx-set-header");
+            FileTransferInfo hi(tmpDir, FileType::HISTORY_FILE_TYPE_LEDGER,
+                                checkpoint);
+            {
+                XDROutputFileStream out(app->getClock().getIOContext(), true);
+                out.open(hi.localPath_nogz());
+                out.writeOne(entry);
+            }
+            // The transactions file must exist even when empty
+            FileTransferInfo ti(
+                tmpDir, FileType::HISTORY_FILE_TYPE_TRANSACTIONS, checkpoint);
+            {
+                XDROutputFileStream out(app->getClock().getIOContext(), true);
+                out.open(ti.localPath_nogz());
+            }
+
+            auto range = LedgerRange::inclusive(lcl.header.ledgerSeq,
+                                                entry.header.ledgerSeq);
+            auto w = app->getWorkScheduler().executeWork<ApplyCheckpointWork>(
+                tmpDir, range, OnFailureCallback{});
+            return w->getState();
+        };
+
+    SECTION("empty-tx-set hash without empty-tx-set value")
+    {
+        REQUIRE(runWithFabricatedHeader(Config::CURRENT_LEDGER_PROTOCOL_VERSION,
+                                        [](StellarValue& sv) {
+                                            sv.txSetHash =
+                                                Herder::EMPTY_TX_SET_HASH;
+                                            // ext stays STELLAR_VALUE_BASIC
+                                        }) == BasicWork::State::WORK_FAILURE);
+    }
+
+    SECTION("empty-tx-set value before protocol support")
+    {
+        REQUIRE(runWithFabricatedHeader(
+                    static_cast<uint32_t>(EMPTY_TX_SET_PROTOCOL_VERSION) - 1,
+                    [](StellarValue& sv) {
+                        sv.txSetHash = Herder::EMPTY_TX_SET_HASH;
+                        sv.ext.v(STELLAR_VALUE_EMPTY_TX_SET);
+                    }) == BasicWork::State::WORK_FAILURE);
+    }
+}
+#endif // CAP_0083
+
 TEST_CASE("Publish works correctly post shadow removal", "[history]")
 {
     // Given a HAS, verify that appropriate levels have "next" cleared, while
diff --git a/src/history/test/HistoryTestsUtils.cpp b/src/history/test/HistoryTestsUtils.cpp
@@ -757,6 +757,85 @@ CatchupSimulation::generateRandomLedger(uint32_t version)
     stroopySeqs.push_back(stroopy.loadSequenceNumber());
 }
 
+#ifdef CAP_0083
+void
+CatchupSimulation::generateEmptyTxSetLedger()
+{
+    auto& lm = getApp().getLedgerManager();
+    auto const lcl = lm.getLastClosedLedgerHeader();
+    uint32_t ledgerSeq = lcl.header.ledgerSeq + 1;
+    uint64_t closeTime = 60 * 5 * ledgerSeq;
+
+    // An empty-tx-set value replaces a proposed value whose tx set the network
+    // gave up waiting for. Craft such a proposal (the referenced tx set
+    // deliberately does not exist anywhere) and convert it exactly as balloting
+    // does when voting to drop the tx set.
+    Hash proposedTxSetHash;
+    proposedTxSetHash.fill(0xAB);
+    StellarValue proposal = getApp().getHerder().makeStellarValue(
+        proposedTxSetHash, closeTime, emptyUpgradeSteps,
+        getApp().getConfig().NODE_SEED);
+
+    auto& herder = static_cast<HerderImpl&>(getApp().getHerder());
+    Value emptyValue = herder.getHerderSCPDriver().makeEmptyTxSetValueFromValue(
+        xdr::xdr_to_opaque(proposal));
+    StellarValue sv;
+    xdr::xdr_from_opaque(emptyValue, sv);
+
+    // The applied tx set is the empty set pinned to the current LCL
+    TxSetXDRFrameConstPtr txSet = TxSetXDRFrame::makeEmpty(lcl);
+
+    CLOG_INFO(History, "Closing synthetic empty-tx-set ledger {}", ledgerSeq);
+
+    mLedgerCloseDatas.emplace_back(ledgerSeq, txSet, sv);
+
+    lm.applyLedger(mLedgerCloseDatas.back());
+
+    auto const& lclh = lm.getLastClosedLedgerHeader();
+    // The header must record the empty-tx-set hash, not the applied
+    // (empty) tx set's contents hash.
+    REQUIRE(lclh.header.scpValue.txSetHash == Herder::EMPTY_TX_SET_HASH);
+    REQUIRE(lclh.header.scpValue.ext.v() == STELLAR_VALUE_EMPTY_TX_SET);
+
+    auto root = getApp().getRoot();
+    auto alice = TestAccount{getApp(), getAccount("alice")};
+    auto bob = TestAccount{getApp(), getAccount("bob")};
+    auto carol = TestAccount{getApp(), getAccount("carol")};
+    auto eve = TestAccount{getApp(), getAccount("eve")};
+    auto stroopy = TestAccount{getApp(), getAccount("stroopy")};
+
+    mLedgerSeqs.push_back(lclh.header.ledgerSeq);
+    mLedgerHashes.push_back(lclh.hash);
+    mBucketListHashes.push_back(lclh.header.bucketListHash);
+    mBucket0Hashes.push_back(getApp()
+                                 .getBucketManager()
+                                 .getLiveBucketList()
+                                 .getLevel(0)
+                                 .getCurr()
+                                 ->getHash());
+    mBucket1Hashes.push_back(getApp()
+                                 .getBucketManager()
+                                 .getLiveBucketList()
+                                 .getLevel(2)
+                                 .getCurr()
+                                 ->getHash());
+
+    rootBalances.push_back(root->getBalance());
+    aliceBalances.push_back(alice.getBalance());
+    bobBalances.push_back(bob.getBalance());
+    carolBalances.push_back(carol.getBalance());
+    eveBalances.push_back(eve.getBalance());
+    stroopyBalances.push_back(stroopy.getBalance());
+
+    rootSeqs.push_back(root->loadSequenceNumber());
+    aliceSeqs.push_back(alice.loadSequenceNumber());
+    bobSeqs.push_back(bob.loadSequenceNumber());
+    carolSeqs.push_back(carol.loadSequenceNumber());
+    eveSeqs.push_back(eve.loadSequenceNumber());
+    stroopySeqs.push_back(stroopy.loadSequenceNumber());
+}
+#endif // CAP_0083
+
 void
 CatchupSimulation::setUpgradeLedger(uint32_t ledger,
                                     ProtocolVersion upgradeProtocolVersion)
diff --git a/src/history/test/HistoryTestsUtils.h b/src/history/test/HistoryTestsUtils.h
@@ -261,6 +261,11 @@ class CatchupSimulation
 
     void generateRandomLedger(uint32_t version = 0);
 
+#ifdef CAP_0083
+    // Closes a CAP-0083 empty-tx-set ledger
+    void generateEmptyTxSetLedger();
+#endif
+
     void ensurePublishesComplete();
     void
     ensureLedgerAvailable(uint32_t targetLedger,
