From e5d707a00aa5fe7db8bed99e67e09152ae4ec377 Mon Sep 17 00:00:00 2001 From: Siddharth Suresh Date: Wed, 11 Feb 2026 11:30:34 -0800 Subject: [PATCH 1/4] Fix ArchivedStateConsistency invariant for live restore checks --- src/invariant/ArchivedStateConsistency.cpp | 26 +++++++++++++++------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/src/invariant/ArchivedStateConsistency.cpp b/src/invariant/ArchivedStateConsistency.cpp index 102988a1a7..f7b995059b 100644 --- a/src/invariant/ArchivedStateConsistency.cpp +++ b/src/invariant/ArchivedStateConsistency.cpp @@ -428,7 +428,7 @@ ArchivedStateConsistency::checkRestoreInvariants( "in live state: {}"), xdrToCerealString(key, "key")); } - else if (liveEntry->second != entry) + else if (key.type() != TTL && liveEntry->second != entry) { return fmt::format( FMT_STRING("ArchivedStateConsistency invariant failed: " @@ -438,14 +438,24 @@ ArchivedStateConsistency::checkRestoreInvariants( xdrToCerealString(entry, "entry_to_restore")); } - if (key.type() == TTL && isLive(entry, ledgerSeq)) + if (key.type() == TTL) { - return fmt::format( - FMT_STRING("ArchivedStateConsistency invariant failed: " - "Restored entry from live BucketList is not " - "expired: Entry: {}, TTL Entry: {}"), - xdrToCerealString(entry, "entry"), - xdrToCerealString(entry, "ttl_entry")); + if (!isLive(entry, ledgerSeq)) + { + return fmt::format( + FMT_STRING("ArchivedStateConsistency invariant failed: " + "Restored entries updated state is still " + "expired: TTL Entry: {}"), + xdrToCerealString(entry, "ttl_entry")); + } + if (isLive(liveEntry->second, ledgerSeq)) + { + return fmt::format( + FMT_STRING("ArchivedStateConsistency invariant failed: " + "Restored entry from live BucketList is not " + "expired: TTL Entry: {}"), + xdrToCerealString(liveEntry->second, "ttl_entry")); + } } } From 8305a7afa390803677ca5c327c8c614c4c677e79 Mon Sep 17 00:00:00 2001 From: Siddharth Suresh Date: Wed, 11 Feb 2026 13:12:43 -0800 Subject: [PATCH 2/4] Add and use markRestoredFromLiveBucketList --- src/ledger/LedgerTxn.cpp | 35 +++++++++++++++++++++++++ src/ledger/LedgerTxn.h | 12 +++++++++ src/ledger/LedgerTxnImpl.h | 9 +++++++ src/ledger/test/InMemoryLedgerTxn.cpp | 8 ++++++ src/ledger/test/InMemoryLedgerTxn.h | 2 ++ src/transactions/ParallelApplyUtils.cpp | 16 ++++++++++- 6 files changed, 81 insertions(+), 1 deletion(-) diff --git a/src/ledger/LedgerTxn.cpp b/src/ledger/LedgerTxn.cpp index 5523cd2c5d..9bb0baa221 100644 --- a/src/ledger/LedgerTxn.cpp +++ b/src/ledger/LedgerTxn.cpp @@ -915,6 +915,41 @@ LedgerTxn::Impl::markRestoredFromHotArchive(LedgerEntry const& ledgerEntry, addKey(ttlEntry); } +void +LedgerTxn::markRestoredFromLiveBucketList(LedgerEntry const& ledgerEntry, + LedgerEntry const& ttlEntry) +{ + getImpl()->markRestoredFromLiveBucketList(ledgerEntry, ttlEntry); +} + +void +LedgerTxn::Impl::markRestoredFromLiveBucketList(LedgerEntry const& ledgerEntry, + LedgerEntry const& ttlEntry) +{ + abortIfWrongThread("markRestoredFromLiveBucketList"); + throwIfSealed(); + throwIfChild(); + + if (!isPersistentEntry(ledgerEntry.data)) + { + throw std::runtime_error( + "Key type not supported for live BucketList restore"); + } + + // Mark the keys as restored + auto addKey = [this](LedgerEntry const& entry) { + auto [_, inserted] = mRestoredEntries.liveBucketList.emplace( + LedgerEntryKey(entry), entry); + if (!inserted) + { + throw std::runtime_error( + "Key already restored from Live BucketList"); + } + }; + addKey(ledgerEntry); + addKey(ttlEntry); +} + LedgerTxnEntry LedgerTxn::restoreFromLiveBucketList(LedgerEntry const& entry, uint32_t ttl) { diff --git a/src/ledger/LedgerTxn.h b/src/ledger/LedgerTxn.h index e044edc4ef..da58cca925 100644 --- a/src/ledger/LedgerTxn.h +++ b/src/ledger/LedgerTxn.h @@ -617,6 +617,13 @@ class AbstractLedgerTxn : public AbstractLedgerTxnParent // restored. This just adds the information to the map tracking entries // restored from the hot archive. The actual restoration of the entry is // handled separately. + // - markRestoredFromLiveBucketList: + // Indicates that an entry in the live BucketList is being restored. + // Used by the parallel apply path to signal to LedgerTxn that the + // entry and TTL should be treated as if they have been restored. This + // just adds the information to the map tracking entries restored from + // the live BucketList. The actual restoration of the entry is handled + // separately. // All of these functions throw if the AbstractLedgerTxn is sealed or if // the AbstractLedgerTxn has a child. virtual LedgerTxnHeader loadHeader() = 0; @@ -626,6 +633,9 @@ class AbstractLedgerTxn : public AbstractLedgerTxnParent uint32_t ttl) = 0; virtual void markRestoredFromHotArchive(LedgerEntry const& ledgerEntry, LedgerEntry const& ttlEntry) = 0; + virtual void + markRestoredFromLiveBucketList(LedgerEntry const& ledgerEntry, + LedgerEntry const& ttlEntry) = 0; virtual LedgerTxnEntry load(InternalLedgerKey const& key) = 0; virtual ConstLedgerTxnEntry loadWithoutRecord(InternalLedgerKey const& key) = 0; @@ -774,6 +784,8 @@ class LedgerTxn : public AbstractLedgerTxn uint32_t ttl) override; void markRestoredFromHotArchive(LedgerEntry const& ledgerEntry, LedgerEntry const& ttlEntry) override; + void markRestoredFromLiveBucketList(LedgerEntry const& ledgerEntry, + LedgerEntry const& ttlEntry) override; UnorderedMap getAllOffers() override; diff --git a/src/ledger/LedgerTxnImpl.h b/src/ledger/LedgerTxnImpl.h index 7e9c9b1e3d..30057b545f 100644 --- a/src/ledger/LedgerTxnImpl.h +++ b/src/ledger/LedgerTxnImpl.h @@ -360,9 +360,18 @@ class LedgerTxn::Impl // markRestoredFromHotArchive has the basic exception safety guarantee. If // it throws an exception, then + // - the restored entries map may contain only a partial record (e.g. the + // data entry without its corresponding TTL entry). void markRestoredFromHotArchive(LedgerEntry const& ledgerEntry, LedgerEntry const& ttlEntry); + // markRestoredFromLiveBucketList has the basic exception safety guarantee. + // If it throws an exception, then + // - the restored entries map may contain only a partial record (e.g. the + // data entry without its corresponding TTL entry). + void markRestoredFromLiveBucketList(LedgerEntry const& ledgerEntry, + LedgerEntry const& ttlEntry); + // restoreFromLiveBucketList has the basic exception safety guarantee. If it // throws an exception, then LedgerTxnEntry restoreFromLiveBucketList(LedgerTxn& self, diff --git a/src/ledger/test/InMemoryLedgerTxn.cpp b/src/ledger/test/InMemoryLedgerTxn.cpp index 8bd3314889..d95e7733f4 100644 --- a/src/ledger/test/InMemoryLedgerTxn.cpp +++ b/src/ledger/test/InMemoryLedgerTxn.cpp @@ -282,6 +282,14 @@ InMemoryLedgerTxn::restoreFromLiveBucketList(LedgerEntry const& entry, "called restoreFromLiveBucketList on InMemoryLedgerTxn"); } +void +InMemoryLedgerTxn::markRestoredFromLiveBucketList( + LedgerEntry const& ledgerEntry, LedgerEntry const& ttlEntry) +{ + throw std::runtime_error( + "called markRestoredFromLiveBucketList on InMemoryLedgerTxn"); +} + LedgerTxnEntry InMemoryLedgerTxn::load(InternalLedgerKey const& key) { diff --git a/src/ledger/test/InMemoryLedgerTxn.h b/src/ledger/test/InMemoryLedgerTxn.h index 8437111d81..ab0c501f89 100644 --- a/src/ledger/test/InMemoryLedgerTxn.h +++ b/src/ledger/test/InMemoryLedgerTxn.h @@ -114,6 +114,8 @@ class InMemoryLedgerTxn : public LedgerTxn void erase(InternalLedgerKey const& key) override; LedgerTxnEntry restoreFromLiveBucketList(LedgerEntry const& entry, uint32_t ttl) override; + void markRestoredFromLiveBucketList(LedgerEntry const& ledgerEntry, + LedgerEntry const& ttlEntry) override; LedgerTxnEntry load(InternalLedgerKey const& key) override; ConstLedgerTxnEntry loadWithoutRecord(InternalLedgerKey const& key) override; diff --git a/src/transactions/ParallelApplyUtils.cpp b/src/transactions/ParallelApplyUtils.cpp index 9c26466bd5..0e46a44eca 100644 --- a/src/transactions/ParallelApplyUtils.cpp +++ b/src/transactions/ParallelApplyUtils.cpp @@ -426,7 +426,10 @@ GlobalParallelApplyLedgerState::commitChangesToLedgerTxn( // While the final state of a restored key that will be written to the // Live BucketList is already handled in mGlobalEntryMap, we need to - // let the ltx know what keys need to be removed from the Hot Archive. + // let the ltx know what keys were restored so that: + // 1. Hot Archive restores can be removed from the Hot Archive BucketList + // 2. The ArchivedStateConsistency invariant can validate both hot archive + // and live BucketList restores for (auto const& kvp : mGlobalRestoredEntries.hotArchive) { // We will search for the ttl key in the hot archive when the entry @@ -439,6 +442,17 @@ GlobalParallelApplyLedgerState::commitChangesToLedgerTxn( ltxInner.markRestoredFromHotArchive(kvp.second, it->second); } } + for (auto const& kvp : mGlobalRestoredEntries.liveBucketList) + { + if (kvp.first.type() != TTL) + { + auto it = mGlobalRestoredEntries.liveBucketList.find( + getTTLKey(kvp.first)); + releaseAssertOrThrow(it != + mGlobalRestoredEntries.liveBucketList.end()); + ltxInner.markRestoredFromLiveBucketList(kvp.second, it->second); + } + } ltxInner.commit(); } From 6142e25f26c2ca20161dcada5f96b25ccc92609c Mon Sep 17 00:00:00 2001 From: Siddharth Suresh Date: Wed, 11 Feb 2026 13:33:45 -0800 Subject: [PATCH 3/4] Remove unnecessary copies and have ltxs track their own restores. --- src/ledger/LedgerTxn.cpp | 35 +++++++++++------------------------ src/ledger/LedgerTxn.h | 3 +-- 2 files changed, 12 insertions(+), 26 deletions(-) diff --git a/src/ledger/LedgerTxn.cpp b/src/ledger/LedgerTxn.cpp index 9bb0baa221..f28eafe5fb 100644 --- a/src/ledger/LedgerTxn.cpp +++ b/src/ledger/LedgerTxn.cpp @@ -251,11 +251,10 @@ RestoredEntries::addLiveBucketlistRestore(LedgerKey const& key, } void -RestoredEntries::addRestoresFrom(RestoredEntries const& other, - bool allowDuplicates) +RestoredEntries::addRestoresFrom(RestoredEntries const& other) { ZoneScoped; - // This method is called from three different call sites. In 2 of them it is + // This method is called from three different call sites. In all three it is // correct to assert that each restore is new/disjoint from any existing // restore: // @@ -272,19 +271,19 @@ RestoredEntries::addRestoresFrom(RestoredEntries const& other, // entry -- it'd be a concurrency bug if not! -- so there should not be // any other restores of the same entry from other threads. // - // In the third place we're committing from an ltx to its parent, and the - // ltx was actually starting with a copy of the restored-maps from the - // parent, so there are going to be duplicates. We allow duplicates in that - // case. - for (auto kvp : other.hotArchive) + // - In the third call site we're committing from a child ltx to its + // parent. Since child LedgerTxns only track their own restores (they + // do not start with a copy of the parent's restored entries), there + // should be no duplicates. + for (auto const& kvp : other.hotArchive) { auto [_, inserted] = hotArchive.emplace(kvp.first, kvp.second); - releaseAssert(inserted || allowDuplicates); + releaseAssert(inserted); } - for (auto kvp : other.liveBucketList) + for (auto const& kvp : other.liveBucketList) { auto [_, inserted] = liveBucketList.emplace(kvp.first, kvp.second); - releaseAssert(inserted || allowDuplicates); + releaseAssert(inserted); } } @@ -435,15 +434,6 @@ LedgerTxn::Impl::Impl(LedgerTxn& self, AbstractLedgerTxnParent& parent, , mConsistency(LedgerTxnConsistency::EXACT) , mActiveThreadId(std::this_thread::get_id()) { - for (auto const& [key, entry] : mParent.getRestoredHotArchiveKeys()) - { - mRestoredEntries.hotArchive.emplace(key, entry); - } - for (auto const& [key, entry] : mParent.getRestoredLiveBucketListKeys()) - { - mRestoredEntries.liveBucketList.emplace(key, entry); - } - mParent.addChild(self, mode); } @@ -703,10 +693,7 @@ LedgerTxn::Impl::commitChild(EntryIterator iter, printErrorAndAbort("unknown fatal error during commit to LedgerTxn"); } - // The child will have started with a copy of the parents mRestoredEntries, - // so we can see duplicates here, but duplicate restores would've been - // caught during restoration in the restoreFrom* functions. - mRestoredEntries.addRestoresFrom(restoredEntries, /*allowDuplicates=*/true); + mRestoredEntries.addRestoresFrom(restoredEntries); // std::unique_ptr<...>::swap does not throw mHeader.swap(childHeader); diff --git a/src/ledger/LedgerTxn.h b/src/ledger/LedgerTxn.h index da58cca925..b9decf389b 100644 --- a/src/ledger/LedgerTxn.h +++ b/src/ledger/LedgerTxn.h @@ -359,8 +359,7 @@ struct RestoredEntries LedgerEntry const& entry, LedgerKey const& ttlKey, LedgerEntry const& ttlEntry); - void addRestoresFrom(RestoredEntries const& other, - bool allowDuplicates = false); + void addRestoresFrom(RestoredEntries const& other); }; class AbstractLedgerTxn; From 53d138c5a66eec1dca44cc59f7ab9af3a2f9d8c1 Mon Sep 17 00:00:00 2001 From: Siddharth Suresh Date: Wed, 11 Feb 2026 13:48:49 -0800 Subject: [PATCH 4/4] Clean up pre-v23 restore meta logic --- src/transactions/TransactionMeta.cpp | 31 +++++----------------------- 1 file changed, 5 insertions(+), 26 deletions(-) diff --git a/src/transactions/TransactionMeta.cpp b/src/transactions/TransactionMeta.cpp index 782a242dbd..344b59cf26 100644 --- a/src/transactions/TransactionMeta.cpp +++ b/src/transactions/TransactionMeta.cpp @@ -362,32 +362,11 @@ OperationMetaBuilder::setLedgerChanges(AbstractLedgerTxn& opLtx, opLtx.getHeader().ledgerVersion, ProtocolVersion::V_23)); } - // getRestoredHotArchiveKeys and getRestoredLiveBucketListKeys return all - // entries that have been restored this ledger, not just by this op. - // However, processOpLedgerEntryChanges expects just the map of restores for - // this op. This function only gets called for opRestoredLiveBucketListKeys{}; - auto allRestoredLiveBucketListKeys = opLtx.getRestoredLiveBucketListKeys(); - auto opModifiedKeys = opLtx.getAllKeysWithoutSealing(); - if (mOp.getOperation().body.type() == OperationType::RESTORE_FOOTPRINT) - { - for (auto const& [key, entry] : allRestoredLiveBucketListKeys) - { - if (isSorobanEntry(key)) - { - auto ttlKey = getTTLKey(key); - if (opModifiedKeys.find(ttlKey) != opModifiedKeys.end()) - { - opRestoredLiveBucketListKeys[key] = entry; - opRestoredLiveBucketListKeys[ttlKey] = - allRestoredLiveBucketListKeys.at(ttlKey); - } - } - } - } + // We should only have restored live BucketList keys for the restore + // operation pre-v23 + auto opRestoredLiveBucketListKeys = opLtx.getRestoredLiveBucketListKeys(); + releaseAssertOrThrow(opRestoredLiveBucketListKeys.empty() || + opType == OperationType::RESTORE_FOOTPRINT); // Note: Hot Archive restore map is always empty since this is never called // in p23.