diff --git a/src/bucket/Bucket.cpp b/src/bucket/Bucket.cpp index 99235fb027..1a9e404d01 100644 --- a/src/bucket/Bucket.cpp +++ b/src/bucket/Bucket.cpp @@ -919,44 +919,19 @@ mergeCasesWithEqualKeys(MergeCounters& mc, BucketInputIterator& oi, if (newEntry.type() == INITENTRY) { - // For all entries except TEMPORARY entries, the only legal new-is-INIT - // case is merging a delete+create to an update. For TEMPORARY entries, - // an INIT entry may merge with another INIT entry as long as the older - // INIT entry is expired. Because merging occurs on a background thread - // and different validators may start a merge at different times, it is - // not possible to accurately know the current ledgerSeq or to know if a - // given TEMPORARY entry has expired. Due to this, we don't check this - // invariant for TEMPORARY entries - - // TODO: Add invariant check for TEMPORARY entries based on ledgerSeq - // when the given bucket started to merge + // The only legal new-is-INIT case is merging a delete+create to an + // update. if (oldEntry.type() != DEADENTRY) { -#ifdef ENABLE_NEXT_PROTOCOL_VERSION_UNSAFE_FOR_PRODUCTION - if (auto type = oldEntry.liveEntry().data.type(); - type == CONTRACT_DATA || type == CONTRACT_CODE) - { - // Treat merge as if old entry did not exist - ++mc.mNewEntriesDefaultAccepted; - Bucket::checkProtocolLegality(newEntry, protocolVersion); - countNewEntryType(mc, newEntry); - maybePut(out, newEntry, shadowIterators, - keepShadowedLifecycleEntries, mc); - } - else -#endif - throw std::runtime_error( - "Malformed bucket: old non-DEAD + new INIT."); - } - else - { - BucketEntry newLive; - newLive.type(LIVEENTRY); - newLive.liveEntry() = newEntry.liveEntry(); - ++mc.mNewInitEntriesMergedWithOldDead; - maybePut(out, newLive, shadowIterators, - keepShadowedLifecycleEntries, mc); + throw std::runtime_error( + "Malformed bucket: old non-DEAD + new INIT."); } + BucketEntry newLive; + newLive.type(LIVEENTRY); + newLive.liveEntry() = newEntry.liveEntry(); + ++mc.mNewInitEntriesMergedWithOldDead; + maybePut(out, newLive, shadowIterators, keepShadowedLifecycleEntries, + mc); } else if (oldEntry.type() == INITENTRY) { diff --git a/src/bucket/BucketApplicator.cpp b/src/bucket/BucketApplicator.cpp index 2faa4325d9..e4abf4c995 100644 --- a/src/bucket/BucketApplicator.cpp +++ b/src/bucket/BucketApplicator.cpp @@ -125,10 +125,7 @@ BucketApplicator::advance(BucketApplicator::Counters& counters) // Prior to protocol 11, INITENTRY didn't exist, so we need // to check ltx to see if this is an update or a create auto key = InternalLedgerEntry(e.liveEntry()).toKey(); - - // Bucket Apply should process every entry in bucket - // including expired ones to get the DB in the correct state - if (ltx->getNewestVersion(key, /*loadExpiredEntry=*/true)) + if (ltx->getNewestVersion(key)) { ltx->updateWithoutLoading(e.liveEntry()); } @@ -158,8 +155,7 @@ BucketApplicator::advance(BucketApplicator::Counters& counters) { // Prior to protocol 11, DEAD entries could exist // without LIVE entries in between - if (ltx->getNewestVersion(e.deadEntry(), - /*loadExpiredEntry=*/true)) + if (ltx->getNewestVersion(e.deadEntry())) { ltx->eraseWithoutLoading(e.deadEntry()); } diff --git a/src/bucket/test/BucketListTests.cpp b/src/bucket/test/BucketListTests.cpp index b620a85e42..c3ab1f951c 100644 --- a/src/bucket/test/BucketListTests.cpp +++ b/src/bucket/test/BucketListTests.cpp @@ -674,7 +674,7 @@ TEST_CASE_VERSIONS("network config snapshots BucketList size", "[bucketlist]") LedgerKey key(CONFIG_SETTING); key.configSetting().configSettingID = ConfigSettingID::CONFIG_SETTING_BUCKETLIST_SIZE_WINDOW; - auto txle = ltx.loadWithoutRecord(key, /*loadExpiredEntry=*/false); + auto txle = ltx.loadWithoutRecord(key); releaseAssert(txle); auto const& leVector = txle.current().data.configSetting().bucketListSizeWindow(); @@ -721,59 +721,6 @@ TEST_CASE_VERSIONS("network config snapshots BucketList size", "[bucketlist]") } }); } - -TEST_CASE_VERSIONS("new temp entry merges with expired entry with same key", - "[bucket][bucketlist]") -{ - VirtualClock clock; - Config cfg(getTestConfig()); - Application::pointer app = createTestApplication(clock, cfg); - BucketList& bl = app->getBucketManager().getBucketList(); - uint32_t ledgerSeq = 1; - - for_versions_from(20, *app, [&] { - auto startingLedger = ledgerSeq; - auto tempEntry = - LedgerTestUtils::generateValidLedgerEntryOfType(CONTRACT_DATA); - setExpirationLedger(tempEntry, startingLedger + 4); - tempEntry.data.contractData().durability = TEMPORARY; - - bl.addBatch(*app, ledgerSeq, getAppLedgerVersion(app), {tempEntry}, {}, - {}); - ++ledgerSeq; - - // Run BucketList until entry has expired - for (; ledgerSeq <= startingLedger + 4; ++ledgerSeq) - { - bl.addBatch(*app, ledgerSeq, getAppLedgerVersion(app), {}, {}, {}); - } - - setExpirationLedger(tempEntry, startingLedger + 500); - bl.addBatch(*app, ledgerSeq, getAppLedgerVersion(app), {tempEntry}, {}, - {}); - ++ledgerSeq; - - // Run BucketList until entry has expired - for (; ledgerSeq <= startingLedger + 1024; ++ledgerSeq) - { - bl.addBatch(*app, ledgerSeq, getAppLedgerVersion(app), {}, {}, {}); - } - - bool foundValidEntry = false; - auto b = bl.getLevel(5).getCurr(); - for (BucketInputIterator iter(b); iter; ++iter) - { - auto entry = *iter; - if (entry.type() == INITENTRY && entry.liveEntry() == tempEntry) - { - foundValidEntry = true; - break; - } - } - - REQUIRE(foundValidEntry); - }); -} #endif static std::string diff --git a/src/herder/Upgrades.cpp b/src/herder/Upgrades.cpp index 8175fe1f07..4c575f7ffa 100644 --- a/src/herder/Upgrades.cpp +++ b/src/herder/Upgrades.cpp @@ -125,7 +125,7 @@ readMaxSorobanTxSetSize(AbstractLedgerTxn& ltx) LedgerKey key(LedgerEntryType::CONFIG_SETTING); key.configSetting().configSettingID = ConfigSettingID::CONFIG_SETTING_CONTRACT_EXECUTION_LANES; - return ltx.loadWithoutRecord(key, /*loadExpiredEntry=*/false) + return ltx.loadWithoutRecord(key) .current() .data.configSetting() .contractExecutionLanes() @@ -852,7 +852,7 @@ getAvailableLimitExcludingLiabilities(AccountID const& accountID, LedgerKey key(TRUSTLINE); key.trustLine().accountID = accountID; key.trustLine().asset = assetToTrustLineAsset(asset); - auto trust = ltx.loadWithoutRecord(key, /*loadExpiredEntry=*/false); + auto trust = ltx.loadWithoutRecord(key); if (trust && isAuthorizedToMaintainLiabilities(trust)) { auto const& tl = trust.current().data.trustLine(); @@ -1300,16 +1300,15 @@ ConfigUpgradeSetFrameConstPtr ConfigUpgradeSetFrame::makeFromKey(AbstractLedgerTxn& ltx, ConfigUpgradeSetKey const& key) { - auto ltxe = ltx.loadWithoutRecord(ConfigUpgradeSetFrame::getLedgerKey(key), - /*loadExpiredEntry=*/false); - if (!ltxe) + auto ltxe = ltx.loadWithoutRecord(ConfigUpgradeSetFrame::getLedgerKey(key)); + if (!ltxe || !isLive(ltxe.current(), ltx.getHeader().ledgerSeq)) { return nullptr; } auto const& contractData = ltxe.current().data.contractData(); if (contractData.body.bodyType() != DATA_ENTRY || contractData.body.data().val.type() != SCV_BYTES || - contractData.durability != PERSISTENT) + contractData.durability != TEMPORARY) { return nullptr; } @@ -1407,7 +1406,7 @@ ConfigUpgradeSetFrame::getLedgerKey(ConfigUpgradeSetKey const& upgradeKey) lk.contractData().contract.type(SC_ADDRESS_TYPE_CONTRACT); lk.contractData().contract.contractId() = upgradeKey.contractID; lk.contractData().key = v; - lk.contractData().durability = PERSISTENT; + lk.contractData().durability = TEMPORARY; return lk; } @@ -1424,9 +1423,9 @@ ConfigUpgradeSetFrame::upgradeNeeded(AbstractLedgerTxn& ltx, { LedgerKey key(LedgerEntryType::CONFIG_SETTING); key.configSetting().configSettingID = updatedEntry.configSettingID(); - bool isSame = ltx.loadWithoutRecord(key, /*loadExpiredEntry=*/false) - .current() - .data.configSetting() == updatedEntry; + bool isSame = + ltx.loadWithoutRecord(key).current().data.configSetting() == + updatedEntry; if (!isSame) { return true; diff --git a/src/herder/test/UpgradesTests.cpp b/src/herder/test/UpgradesTests.cpp index 48b782f987..a1fa9032d4 100644 --- a/src/herder/test/UpgradesTests.cpp +++ b/src/herder/test/UpgradesTests.cpp @@ -876,7 +876,7 @@ TEST_CASE("config upgrades applied to ledger", "[soroban][upgrades]") LedgerKey key(CONFIG_SETTING); key.configSetting().configSettingID = ConfigSettingID::CONFIG_SETTING_CONTRACT_LEDGER_COST_V0; - auto le = ltx2.loadWithoutRecord(key, false).current(); + auto le = ltx2.loadWithoutRecord(key).current(); auto configSetting = le.data.configSetting(); configSetting.contractLedgerCost().txMaxWriteBytes = upgradeVal; diff --git a/src/invariant/BucketListIsConsistentWithDatabase.cpp b/src/invariant/BucketListIsConsistentWithDatabase.cpp index 46ba50732d..f6709e2f2f 100644 --- a/src/invariant/BucketListIsConsistentWithDatabase.cpp +++ b/src/invariant/BucketListIsConsistentWithDatabase.cpp @@ -28,10 +28,7 @@ namespace stellar static std::string checkAgainstDatabase(AbstractLedgerTxn& ltx, LedgerEntry const& entry) { - // Database should contain same expried entries as BucketList even if they - // are not accesible - auto fromDb = - ltx.loadWithoutRecord(LedgerEntryKey(entry), /*loadExpiredEntry=*/true); + auto fromDb = ltx.loadWithoutRecord(LedgerEntryKey(entry)); if (!fromDb) { std::string s{ @@ -56,9 +53,7 @@ checkAgainstDatabase(AbstractLedgerTxn& ltx, LedgerEntry const& entry) static std::string checkAgainstDatabase(AbstractLedgerTxn& ltx, LedgerKey const& key) { - // Database should contain same expried entries as BucketList even if they - // are not accesible - auto fromDb = ltx.loadWithoutRecord(key, /*loadExpiredEntry=*/true); + auto fromDb = ltx.loadWithoutRecord(key); if (!fromDb) { return {}; diff --git a/src/invariant/test/BucketListIsConsistentWithDatabaseTests.cpp b/src/invariant/test/BucketListIsConsistentWithDatabaseTests.cpp index 3f337ed80b..1d8aa2e6d3 100644 --- a/src/invariant/test/BucketListIsConsistentWithDatabaseTests.cpp +++ b/src/invariant/test/BucketListIsConsistentWithDatabaseTests.cpp @@ -295,10 +295,8 @@ struct SelectBucketListGenerator : public BucketListGenerator auto iter = filteredKeys.begin(); std::advance(iter, dist(gRandomEngine)); - // For BucketList generation, expirationLedger shouldn't matter mSelected = std::make_shared( - ltx.loadWithoutRecord(*iter, /*loadExpiredEntry=*/true) - .current()); + ltx.loadWithoutRecord(*iter).current()); } } return BucketListGenerator::generateLiveEntries(ltx); diff --git a/src/ledger/InMemoryLedgerTxn.cpp b/src/ledger/InMemoryLedgerTxn.cpp index fb311fd9bd..bcdaca07a2 100644 --- a/src/ledger/InMemoryLedgerTxn.cpp +++ b/src/ledger/InMemoryLedgerTxn.cpp @@ -233,14 +233,6 @@ InMemoryLedgerTxn::create(InternalLedgerEntry const& entry) throw std::runtime_error("called create on InMemoryLedgerTxn"); } -#ifdef ENABLE_NEXT_PROTOCOL_VERSION_UNSAFE_FOR_PRODUCTION -LedgerTxnEntry -InMemoryLedgerTxn::restore(InternalLedgerEntry const& entry) -{ - throw std::runtime_error("called restore on InMemoryLedgerTxn"); -} -#endif - void InMemoryLedgerTxn::erase(InternalLedgerKey const& key) { @@ -254,8 +246,7 @@ InMemoryLedgerTxn::load(InternalLedgerKey const& key) } ConstLedgerTxnEntry -InMemoryLedgerTxn::loadWithoutRecord(InternalLedgerKey const& key, - bool loadExpiredEntry) +InMemoryLedgerTxn::loadWithoutRecord(InternalLedgerKey const& key) { throw std::runtime_error("called loadWithoutRecord on InMemoryLedgerTxn"); } @@ -280,7 +271,7 @@ InMemoryLedgerTxn::getOffersByAccountAndAsset(AccountID const& account, continue; } - auto newest = getNewestVersion(key, /*loadExpiredEntry=*/false); + auto newest = getNewestVersion(key); if (!newest) { throw std::runtime_error("Invalid ledger state"); @@ -317,10 +308,8 @@ InMemoryLedgerTxn::getPoolShareTrustLinesByAccountAndAsset( continue; } - auto pool = getNewestVersion( - liquidityPoolKey( - key.ledgerKey().trustLine().asset.liquidityPoolID()), - /*loadExpiredEntry=*/false); + auto pool = getNewestVersion(liquidityPoolKey( + key.ledgerKey().trustLine().asset.liquidityPoolID())); if (!pool) { throw std::runtime_error("Invalid ledger state"); @@ -330,7 +319,7 @@ InMemoryLedgerTxn::getPoolShareTrustLinesByAccountAndAsset( auto const& cp = lp.body.constantProduct(); if (cp.params.assetA == asset || cp.params.assetB == asset) { - auto newest = getNewestVersion(key, /*loadExpiredEntry=*/false); + auto newest = getNewestVersion(key); if (!newest) { throw std::runtime_error("Invalid ledger state"); diff --git a/src/ledger/InMemoryLedgerTxn.h b/src/ledger/InMemoryLedgerTxn.h index bfe36325e6..76cf56fcae 100644 --- a/src/ledger/InMemoryLedgerTxn.h +++ b/src/ledger/InMemoryLedgerTxn.h @@ -88,15 +88,10 @@ class InMemoryLedgerTxn : public LedgerTxn void eraseWithoutLoading(InternalLedgerKey const& key) override; LedgerTxnEntry create(InternalLedgerEntry const& entry) override; - -#ifdef ENABLE_NEXT_PROTOCOL_VERSION_UNSAFE_FOR_PRODUCTION - LedgerTxnEntry restore(InternalLedgerEntry const& entry) override; -#endif - void erase(InternalLedgerKey const& key) override; LedgerTxnEntry load(InternalLedgerKey const& key) override; - ConstLedgerTxnEntry loadWithoutRecord(InternalLedgerKey const& key, - bool loadExpiredEntry) override; + ConstLedgerTxnEntry + loadWithoutRecord(InternalLedgerKey const& key) override; UnorderedMap getOffersByAccountAndAsset(AccountID const& account, diff --git a/src/ledger/InMemoryLedgerTxnRoot.cpp b/src/ledger/InMemoryLedgerTxnRoot.cpp index 6cf6e2662a..7dc817e57b 100644 --- a/src/ledger/InMemoryLedgerTxnRoot.cpp +++ b/src/ledger/InMemoryLedgerTxnRoot.cpp @@ -90,8 +90,7 @@ InMemoryLedgerTxnRoot::getInflationWinners(size_t maxWinners, } std::shared_ptr -InMemoryLedgerTxnRoot::getNewestVersion(InternalLedgerKey const& key, - bool loadExpiredEntry) const +InMemoryLedgerTxnRoot::getNewestVersion(InternalLedgerKey const& key) const { return nullptr; } diff --git a/src/ledger/InMemoryLedgerTxnRoot.h b/src/ledger/InMemoryLedgerTxnRoot.h index 183db9040c..820a418a89 100644 --- a/src/ledger/InMemoryLedgerTxnRoot.h +++ b/src/ledger/InMemoryLedgerTxnRoot.h @@ -62,8 +62,7 @@ class InMemoryLedgerTxnRoot : public AbstractLedgerTxnParent getInflationWinners(size_t maxWinners, int64_t minBalance) override; std::shared_ptr - getNewestVersion(InternalLedgerKey const& key, - bool loadExpiredEntry) const override; + getNewestVersion(InternalLedgerKey const& key) const override; uint64_t countObjects(LedgerEntryType let) const override; uint64_t countObjects(LedgerEntryType let, diff --git a/src/ledger/LedgerTxn.cpp b/src/ledger/LedgerTxn.cpp index c4926504fb..98bdac4afc 100644 --- a/src/ledger/LedgerTxn.cpp +++ b/src/ledger/LedgerTxn.cpp @@ -605,18 +605,18 @@ LedgerTxn::create(InternalLedgerEntry const& entry) return getImpl()->create(*this, entry); } -#ifdef ENABLE_NEXT_PROTOCOL_VERSION_UNSAFE_FOR_PRODUCTION LedgerTxnEntry -LedgerTxn::restore(InternalLedgerEntry const& entry) +LedgerTxn::Impl::create(LedgerTxn& self, InternalLedgerEntry const& entry) { - return getImpl()->restore(*this, entry); -} -#endif + throwIfSealed(); + throwIfChild(); + + auto key = entry.toKey(); + if (getNewestVersion(key)) + { + throw std::runtime_error("Key already exists"); + } -LedgerTxnEntry -LedgerTxn::Impl::createRestoreCommon(LedgerTxn& self, - InternalLedgerEntry const& entry) -{ auto current = std::make_shared(entry); auto impl = LedgerTxnEntry::makeSharedImpl(self, *current); @@ -624,7 +624,7 @@ LedgerTxn::Impl::createRestoreCommon(LedgerTxn& self, // can throw and the LedgerTxnEntry destructor requires that mActive // contains key. LedgerTxnEntry constructor does not throw so this is // still exception safe. - mActive.emplace(entry.toKey(), toEntryImplBase(impl)); + mActive.emplace(key, toEntryImplBase(impl)); LedgerTxnEntry ltxe(impl); auto it = mEntry.end(); // hint that key is not in mEntry @@ -633,81 +633,11 @@ LedgerTxn::Impl::createRestoreCommon(LedgerTxn& self, // after this INIT entry is merged with the DELETED will be a LIVE. This is // because the entry would have been a LIVE before the delete. If it were an // INIT instead, the key would've been annihilated. - updateEntry(entry.toKey(), &it, LedgerEntryPtr::Init(current), + updateEntry(key, &it, LedgerEntryPtr::Init(current), /* effectiveActive */ true); return ltxe; } -LedgerTxnEntry -LedgerTxn::Impl::create(LedgerTxn& self, InternalLedgerEntry const& entry) -{ - throwIfSealed(); - throwIfChild(); - - auto key = entry.toKey(); - - // For entries that can be restored, we need to check the key for both - // expired entries and live entries - auto checkExpired = key.type() == InternalLedgerEntryType::LEDGER_ENTRY && - isRestorableEntry(key.ledgerKey()); - if (getNewestVersion(key, checkExpired)) - { - throw std::runtime_error("Key already exists"); - } - - return createRestoreCommon(self, entry); -} - -#ifdef ENABLE_NEXT_PROTOCOL_VERSION_UNSAFE_FOR_PRODUCTION -LedgerTxnEntry -LedgerTxn::Impl::restore(LedgerTxn& self, InternalLedgerEntry const& entry) -{ - throwIfSealed(); - throwIfChild(); - - auto key = entry.toKey(); - - if (!isRestorableEntry(key.ledgerKey())) - { - throw std::runtime_error( - "Restored entry that is not a restorable type"); - } - - auto expiredVersion = getNewestVersion(key, /*loadExpiredEntry=*/true); - if (!expiredVersion) - { - throw std::runtime_error("Restored entry that does not exist"); - } - - if (getNewestVersion(key, /*loadExpiredEntry=*/false)) - { - throw std::runtime_error("Restored live entry"); - } - - // Check that data fields of expired version and new version are identical. - bool integrityCheck; - auto const& expiredLE = expiredVersion->ledgerEntry(); - if (key.ledgerKey().type() == CONTRACT_DATA) - { - integrityCheck = expiredLE.data.contractData().body == - entry.ledgerEntry().data.contractData().body; - } - else - { - integrityCheck = expiredLE.data.contractCode().body == - entry.ledgerEntry().data.contractCode().body; - } - - if (!integrityCheck) - { - throw std::runtime_error( - "Restored version has different data than expired version"); - } - - return createRestoreCommon(self, entry); -} -#endif - void LedgerTxn::createWithoutLoading(InternalLedgerEntry const& entry) { @@ -1218,8 +1148,7 @@ LedgerTxn::Impl::getChanges() } else { - auto previous = - mParent.getNewestVersion(key, /*loadExpiredEntry=*/false); + auto previous = mParent.getNewestVersion(key); // entry is not init, so previous must exist. If not, then // we're modifying an entry that doesn't exist. releaseAssert(previous); @@ -1264,8 +1193,7 @@ LedgerTxn::Impl::getDelta() continue; } - auto previous = - mParent.getNewestVersion(key, /*loadExpiredEntry=*/false); + auto previous = mParent.getNewestVersion(key); // Deep copy is not required here because getDelta causes // LedgerTxn to enter the sealed state, meaning subsequent @@ -1327,8 +1255,7 @@ LedgerTxn::Impl::getDeltaVotes() const } } - auto previous = - mParent.getNewestVersion(key, /*loadExpiredEntry=*/false); + auto previous = mParent.getNewestVersion(key); if (previous) { auto const& acc = previous->ledgerEntry().data.account(); @@ -1506,22 +1433,20 @@ LedgerTxn::Impl::getAllEntries(std::vector& initEntries, } std::shared_ptr -LedgerTxn::getNewestVersion(InternalLedgerKey const& key, - bool loadExpiredEntry) const +LedgerTxn::getNewestVersion(InternalLedgerKey const& key) const { - return getImpl()->getNewestVersion(key, loadExpiredEntry); + return getImpl()->getNewestVersion(key); } std::shared_ptr -LedgerTxn::Impl::getNewestVersion(InternalLedgerKey const& key, - bool loadExpiredEntry) const +LedgerTxn::Impl::getNewestVersion(InternalLedgerKey const& key) const { auto iter = mEntry.find(key); if (iter != mEntry.end()) { return iter->second.get(); } - return mParent.getNewestVersion(key, loadExpiredEntry); + return mParent.getNewestVersion(key); } std::pair, @@ -1533,8 +1458,7 @@ LedgerTxn::Impl::getNewestVersionEntryMap(InternalLedgerKey const& key) { return std::make_pair(iter->second.get(), iter); } - return std::make_pair( - mParent.getNewestVersion(key, /*loadExpiredEntry=*/false), iter); + return std::make_pair(mParent.getNewestVersion(key), iter); } UnorderedMap @@ -1622,10 +1546,8 @@ LedgerTxn::Impl::getPoolShareTrustLinesByAccountAndAsset( // The trust line wasn't in our result set, and was updated // in self. We need to check the corresponding LiquidityPool // to find its constituent assets. - auto newest = getNewestVersion( - liquidityPoolKey( - key.trustLine().asset.liquidityPoolID()), - /*loadExpiredEntry=*/false); + auto newest = getNewestVersion(liquidityPoolKey( + key.trustLine().asset.liquidityPoolID())); if (!newest) { throw std::runtime_error("Invalid ledger state"); @@ -1917,16 +1839,14 @@ LedgerTxn::Impl::loadPoolShareTrustLinesByAccountAndAsset( } ConstLedgerTxnEntry -LedgerTxn::loadWithoutRecord(InternalLedgerKey const& key, - bool loadExpiredEntry) +LedgerTxn::loadWithoutRecord(InternalLedgerKey const& key) { - return getImpl()->loadWithoutRecord(*this, key, loadExpiredEntry); + return getImpl()->loadWithoutRecord(*this, key); } ConstLedgerTxnEntry LedgerTxn::Impl::loadWithoutRecord(LedgerTxn& self, - InternalLedgerKey const& key, - bool loadExpiredEntry) + InternalLedgerKey const& key) { throwIfSealed(); throwIfChild(); @@ -1935,7 +1855,7 @@ LedgerTxn::Impl::loadWithoutRecord(LedgerTxn& self, throw std::runtime_error("Key is active"); } - auto newest = getNewestVersion(key, loadExpiredEntry); + auto newest = getNewestVersion(key); if (!newest) { return {}; @@ -2540,20 +2460,6 @@ LedgerTxnRoot::Impl::~Impl() } } -LedgerTxnRoot::Impl::CacheEntry -LedgerTxnRoot::Impl::EntryCache::get(LedgerKey const& k, - std::optional expirationCutoff) -{ - auto ce = RandomEvictionCache::get(k); - if (expirationCutoff && ce.entry && !isLive(*ce.entry, *expirationCutoff)) - { - // If the entry is expired, return null - ce.entry = nullptr; - } - - return ce; -} - #ifdef BUILD_TESTS void LedgerTxnRoot::Impl::resetForFuzzer() @@ -3662,15 +3568,13 @@ LedgerTxnRoot::Impl::getInflationWinners(size_t maxWinners, int64_t minVotes) } std::shared_ptr -LedgerTxnRoot::getNewestVersion(InternalLedgerKey const& key, - bool loadExpiredEntry) const +LedgerTxnRoot::getNewestVersion(InternalLedgerKey const& key) const { - return mImpl->getNewestVersion(key, loadExpiredEntry); + return mImpl->getNewestVersion(key); } std::shared_ptr -LedgerTxnRoot::Impl::getNewestVersion(InternalLedgerKey const& gkey, - bool loadExpiredEntry) const +LedgerTxnRoot::Impl::getNewestVersion(InternalLedgerKey const& gkey) const { ZoneScoped; // Right now, only LEDGER_ENTRY are recorded in the SQL database @@ -3679,11 +3583,12 @@ LedgerTxnRoot::Impl::getNewestVersion(InternalLedgerKey const& gkey, return nullptr; } auto const& key = gkey.ledgerKey(); + if (mEntryCache.exists(key)) { std::string zoneTxt("hit"); ZoneText(zoneTxt.c_str(), zoneTxt.size()); - return getFromEntryCache(key, loadExpiredEntry); + return getFromEntryCache(key); } else { @@ -3756,14 +3661,12 @@ LedgerTxnRoot::Impl::getNewestVersion(InternalLedgerKey const& gkey, putInEntryCache(key, entry, LoadType::IMMEDIATE); if (entry) { - // Enforce expirationLedger - if (loadExpiredEntry || isLive(*entry, mHeader->ledgerSeq)) - { - return std::make_shared(*entry); - } + return std::make_shared(*entry); + } + else + { + return nullptr; } - - return nullptr; } void @@ -3801,15 +3704,11 @@ LedgerTxnRoot::Impl::rollbackChild() noexcept } std::shared_ptr -LedgerTxnRoot::Impl::getFromEntryCache(LedgerKey const& key, - bool loadExpiredEntry) const +LedgerTxnRoot::Impl::getFromEntryCache(LedgerKey const& key) const { try { - std::optional expirationCutoff = - loadExpiredEntry ? std::nullopt - : std::make_optional(mHeader->ledgerSeq); - auto cached = mEntryCache.get(key, expirationCutoff); + auto cached = mEntryCache.get(key); if (cached.type == LoadType::PREFETCH) { ++mPrefetchHits; diff --git a/src/ledger/LedgerTxn.h b/src/ledger/LedgerTxn.h index eb0f5113a8..798ded4796 100644 --- a/src/ledger/LedgerTxn.h +++ b/src/ledger/LedgerTxn.h @@ -119,9 +119,9 @@ // access this entry in this LedgerTxn." See below for the // concurrency-control issues this is designed to trap. // -// - Entries are made-active by calling load(), create(), or restore(), each -// of which returns a LedgerTxnEntry which is a handle that can be used to -// get at the underlying LedgerEntry. References to the underlying +// - Entries are made-active by calling load() or create(), each of which +// returns a LedgerTxnEntry which is a handle that can be used to get at +// the underlying LedgerEntry. References to the underlying // LedgerEntries should generally not be retained anywhere, because the // LedgerTxnEntry handles may be "deactivated", and access to a // deactivated entry is a _logic error_ in the client that this @@ -434,8 +434,7 @@ class AbstractLedgerTxnParent // invoking getNewestVersion on its parent. Returns nullptr if the key does // not exist or if the corresponding LedgerEntry has been erased. virtual std::shared_ptr - getNewestVersion(InternalLedgerKey const& key, - bool loadExpiredEntry) const = 0; + getNewestVersion(InternalLedgerKey const& key) const = 0; // Return the count of the number of ledger objects of type `let`. Will // throw when called on anything other than a (real or stub) root LedgerTxn. @@ -541,10 +540,10 @@ class AbstractLedgerTxn : public AbstractLedgerTxnParent virtual void commit() noexcept = 0; virtual void rollback() noexcept = 0; - // loadHeader, create, restore, erase, load, and loadWithoutRecord provide - // the main interface to interact with data stored in the AbstractLedgerTxn. - // These functions only allow one instance of a particular data to be active - // at a time. + // loadHeader, create, erase, load, and loadWithoutRecord provide the main + // interface to interact with data stored in the AbstractLedgerTxn. These + // functions only allow one instance of a particular data to be active at a + // time. // - loadHeader // Loads the current LedgerHeader. Throws if there is already an active // LedgerTxnHeader. @@ -552,11 +551,6 @@ class AbstractLedgerTxn : public AbstractLedgerTxnParent // Creates a new LedgerTxnEntry from entry. Throws if the key // associated with this entry is already associated with an entry in // this AbstractLedgerTxn or any parent. - // - restore - // Creates a new LedgerTxnEntry from expired entry. Throws if the key - // associated with this entry is already associated with an entry in - // this AbstractLedgerTxn or any parent or if the expired entry being - // restored does not exist // - erase // Erases the existing entry associated with key. Throws if the key is // not already associated with an entry in this AbstractLedgerTxn or @@ -577,15 +571,10 @@ class AbstractLedgerTxn : public AbstractLedgerTxnParent // the AbstractLedgerTxn has a child. virtual LedgerTxnHeader loadHeader() = 0; virtual LedgerTxnEntry create(InternalLedgerEntry const& entry) = 0; - -#ifdef ENABLE_NEXT_PROTOCOL_VERSION_UNSAFE_FOR_PRODUCTION - virtual LedgerTxnEntry restore(InternalLedgerEntry const& entry) = 0; -#endif - virtual void erase(InternalLedgerKey const& key) = 0; virtual LedgerTxnEntry load(InternalLedgerKey const& key) = 0; - virtual ConstLedgerTxnEntry loadWithoutRecord(InternalLedgerKey const& key, - bool loadExpiredEntry) = 0; + virtual ConstLedgerTxnEntry + loadWithoutRecord(InternalLedgerKey const& key) = 0; // Somewhat unsafe, non-recommended access methods: for use only during // bulk-loading as in catchup from buckets. These methods set an entry @@ -716,10 +705,6 @@ class LedgerTxn : public AbstractLedgerTxn LedgerTxnEntry create(InternalLedgerEntry const& entry) override; -#ifdef ENABLE_NEXT_PROTOCOL_VERSION_UNSAFE_FOR_PRODUCTION - LedgerTxnEntry restore(InternalLedgerEntry const& entry) override; -#endif - void erase(InternalLedgerKey const& key) override; UnorderedMap getAllOffers() override; @@ -757,8 +742,7 @@ class LedgerTxn : public AbstractLedgerTxn std::vector& deadEntries) override; std::shared_ptr - getNewestVersion(InternalLedgerKey const& key, - bool loadExpiredEntry) const override; + getNewestVersion(InternalLedgerKey const& key) const override; LedgerTxnEntry load(InternalLedgerKey const& key) override; @@ -781,8 +765,8 @@ class LedgerTxn : public AbstractLedgerTxn loadPoolShareTrustLinesByAccountAndAsset(AccountID const& account, Asset const& asset) override; - ConstLedgerTxnEntry loadWithoutRecord(InternalLedgerKey const& key, - bool loadExpiredEntry) override; + ConstLedgerTxnEntry + loadWithoutRecord(InternalLedgerKey const& key) override; void rollback() noexcept override; @@ -895,8 +879,7 @@ class LedgerTxnRoot : public AbstractLedgerTxnParent getInflationWinners(size_t maxWinners, int64_t minBalance) override; std::shared_ptr - getNewestVersion(InternalLedgerKey const& key, - bool loadExpiredEntry) const override; + getNewestVersion(InternalLedgerKey const& key) const override; void rollbackChild() noexcept override; diff --git a/src/ledger/LedgerTxnImpl.h b/src/ledger/LedgerTxnImpl.h index cb4934ef32..c7b0e61f0e 100644 --- a/src/ledger/LedgerTxnImpl.h +++ b/src/ledger/LedgerTxnImpl.h @@ -13,7 +13,6 @@ #include #include #include -#include #include #endif @@ -440,11 +439,6 @@ class LedgerTxn::Impl std::pair, EntryMap::iterator> getNewestVersionEntryMap(InternalLedgerKey const& key); - // Common logic for create and restore code paths. Input correctness - // checking should be done before calling this function - LedgerTxnEntry createRestoreCommon(LedgerTxn& self, - InternalLedgerEntry const& entry); - public: // Constructor has the strong exception safety guarantee Impl(LedgerTxn& self, AbstractLedgerTxnParent& parent, @@ -464,15 +458,6 @@ class LedgerTxn::Impl // - the entry cache may be, but is not guaranteed to be, cleared. LedgerTxnEntry create(LedgerTxn& self, InternalLedgerEntry const& entry); - // restore has the basic exception safety guarantee. If it throws an - // exception, then - // - the prepared statement cache may be, but is not guaranteed to be, - // modified - // - the entry cache may be, but is not guaranteed to be, cleared. -#ifdef ENABLE_NEXT_PROTOCOL_VERSION_UNSAFE_FOR_PRODUCTION - LedgerTxnEntry restore(LedgerTxn& self, InternalLedgerEntry const& entry); -#endif - // deactivate has the strong exception safety guarantee void deactivate(InternalLedgerKey const& key); @@ -566,7 +551,7 @@ class LedgerTxn::Impl // modified // - the entry cache may be, but is not guaranteed to be, cleared. std::shared_ptr - getNewestVersion(InternalLedgerKey const& key, bool loadExpiredEntry) const; + getNewestVersion(InternalLedgerKey const& key) const; // load has the basic exception safety guarantee. If it throws an exception, // then @@ -632,8 +617,7 @@ class LedgerTxn::Impl // modified // - the entry cache may be, but is not guaranteed to be, cleared. ConstLedgerTxnEntry loadWithoutRecord(LedgerTxn& self, - InternalLedgerKey const& key, - bool loadExpiredEntry); + InternalLedgerKey const& key); void rollback() noexcept; void rollbackChild() noexcept; @@ -720,20 +704,7 @@ class LedgerTxnRoot::Impl LoadType type; }; - // RandomEvictionCache, but override get to account for expiration behavior - class EntryCache : public RandomEvictionCache - { - public: - // Load entry from cache. If expirationCutoff is not empty, only returns - // an entry if its expirationLedger > expirationCutoff - CacheEntry get(LedgerKey const& k, - std::optional expirationCutoff); - - using RandomEvictionCache::RandomEvictionCache; - - private: - using RandomEvictionCache::get; - }; + typedef RandomEvictionCache EntryCache; typedef AssetPair BestOffersKey; @@ -850,7 +821,7 @@ class LedgerTxnRoot::Impl // database for the keyset that it has entries for. It's a precise // image of a subset of the database. std::shared_ptr - getFromEntryCache(LedgerKey const& key, bool loadExpiredEntry) const; + getFromEntryCache(LedgerKey const& key) const; void putInEntryCache(LedgerKey const& key, std::shared_ptr const& entry, LoadType type) const; @@ -979,7 +950,7 @@ class LedgerTxnRoot::Impl // modified // - the entry cache may be, but is not guaranteed to be, cleared. std::shared_ptr - getNewestVersion(InternalLedgerKey const& key, bool loadExpiredEntry) const; + getNewestVersion(InternalLedgerKey const& key) const; void rollbackChild() noexcept; diff --git a/src/ledger/LedgerTypeUtils.h b/src/ledger/LedgerTypeUtils.h index 4fea783516..23cb82ed19 100644 --- a/src/ledger/LedgerTypeUtils.h +++ b/src/ledger/LedgerTypeUtils.h @@ -29,7 +29,6 @@ bool autoBumpEnabled(LedgerEntry const& e); template bool isSorobanExtEntry(T const& e); template bool isSorobanDataEntry(T const& e); -template bool isRestorableEntry(T const& e); template bool diff --git a/src/ledger/NetworkConfig.cpp b/src/ledger/NetworkConfig.cpp index e86ca6c190..ec34ee7c39 100644 --- a/src/ledger/NetworkConfig.cpp +++ b/src/ledger/NetworkConfig.cpp @@ -562,7 +562,7 @@ SorobanNetworkConfig::loadMaxContractSize(AbstractLedgerTxn& ltx) LedgerKey key(CONFIG_SETTING); key.configSetting().configSettingID = ConfigSettingID::CONFIG_SETTING_CONTRACT_MAX_SIZE_BYTES; - auto le = ltx.loadWithoutRecord(key, /*loadExpiredEntry=*/false).current(); + auto le = ltx.loadWithoutRecord(key).current(); mMaxContractSizeBytes = le.data.configSetting().contractMaxSizeBytes(); #endif } @@ -574,7 +574,7 @@ SorobanNetworkConfig::loadMaxContractDataKeySize(AbstractLedgerTxn& ltx) LedgerKey key(CONFIG_SETTING); key.configSetting().configSettingID = ConfigSettingID::CONFIG_SETTING_CONTRACT_DATA_KEY_SIZE_BYTES; - auto le = ltx.loadWithoutRecord(key, /*loadExpiredEntry=*/false).current(); + auto le = ltx.loadWithoutRecord(key).current(); mMaxContractDataKeySizeBytes = le.data.configSetting().contractDataKeySizeBytes(); #endif @@ -587,7 +587,7 @@ SorobanNetworkConfig::loadMaxContractDataEntrySize(AbstractLedgerTxn& ltx) LedgerKey key(CONFIG_SETTING); key.configSetting().configSettingID = ConfigSettingID::CONFIG_SETTING_CONTRACT_DATA_ENTRY_SIZE_BYTES; - auto le = ltx.loadWithoutRecord(key, /*loadExpiredEntry=*/false).current(); + auto le = ltx.loadWithoutRecord(key).current(); mMaxContractDataEntrySizeBytes = le.data.configSetting().contractDataEntrySizeBytes(); #endif @@ -600,7 +600,7 @@ SorobanNetworkConfig::loadComputeSettings(AbstractLedgerTxn& ltx) LedgerKey key(CONFIG_SETTING); key.configSetting().configSettingID = ConfigSettingID::CONFIG_SETTING_CONTRACT_COMPUTE_V0; - auto le = ltx.loadWithoutRecord(key, /*loadExpiredEntry=*/false).current(); + auto le = ltx.loadWithoutRecord(key).current(); auto const& configSetting = le.data.configSetting().contractCompute(); mLedgerMaxInstructions = configSetting.ledgerMaxInstructions; mTxMaxInstructions = configSetting.txMaxInstructions; @@ -617,7 +617,7 @@ SorobanNetworkConfig::loadLedgerAccessSettings(AbstractLedgerTxn& ltx) LedgerKey key(CONFIG_SETTING); key.configSetting().configSettingID = ConfigSettingID::CONFIG_SETTING_CONTRACT_LEDGER_COST_V0; - auto le = ltx.loadWithoutRecord(key, /*loadExpiredEntry=*/false).current(); + auto le = ltx.loadWithoutRecord(key).current(); auto const& configSetting = le.data.configSetting().contractLedgerCost(); mLedgerMaxReadLedgerEntries = configSetting.ledgerMaxReadLedgerEntries; mLedgerMaxReadBytes = configSetting.ledgerMaxReadBytes; @@ -645,7 +645,7 @@ SorobanNetworkConfig::loadHistoricalSettings(AbstractLedgerTxn& ltx) LedgerKey key(CONFIG_SETTING); key.configSetting().configSettingID = ConfigSettingID::CONFIG_SETTING_CONTRACT_HISTORICAL_DATA_V0; - auto le = ltx.loadWithoutRecord(key, /*loadExpiredEntry=*/false).current(); + auto le = ltx.loadWithoutRecord(key).current(); auto const& configSetting = le.data.configSetting().contractHistoricalData(); mFeeHistorical1KB = configSetting.feeHistorical1KB; @@ -659,7 +659,7 @@ SorobanNetworkConfig::loadMetaDataSettings(AbstractLedgerTxn& ltx) LedgerKey key(CONFIG_SETTING); key.configSetting().configSettingID = ConfigSettingID::CONFIG_SETTING_CONTRACT_META_DATA_V0; - auto le = ltx.loadWithoutRecord(key, /*loadExpiredEntry=*/false).current(); + auto le = ltx.loadWithoutRecord(key).current(); auto const& configSetting = le.data.configSetting().contractMetaData(); mFeeExtendedMetaData1KB = configSetting.feeExtendedMetaData1KB; mTxMaxExtendedMetaDataSizeBytes = @@ -674,7 +674,7 @@ SorobanNetworkConfig::loadBandwidthSettings(AbstractLedgerTxn& ltx) LedgerKey key(CONFIG_SETTING); key.configSetting().configSettingID = ConfigSettingID::CONFIG_SETTING_CONTRACT_BANDWIDTH_V0; - auto le = ltx.loadWithoutRecord(key, /*loadExpiredEntry=*/false).current(); + auto le = ltx.loadWithoutRecord(key).current(); auto const& configSetting = le.data.configSetting().contractBandwidth(); mLedgerMaxPropagateSizeBytes = configSetting.ledgerMaxPropagateSizeBytes; mTxMaxSizeBytes = configSetting.txMaxSizeBytes; @@ -689,7 +689,7 @@ SorobanNetworkConfig::loadCpuCostParams(AbstractLedgerTxn& ltx) LedgerKey key(CONFIG_SETTING); key.configSetting().configSettingID = ConfigSettingID::CONFIG_SETTING_CONTRACT_COST_PARAMS_CPU_INSTRUCTIONS; - auto le = ltx.loadWithoutRecord(key, /*loadExpiredEntry=*/false).current(); + auto le = ltx.loadWithoutRecord(key).current(); mCpuCostParams = le.data.configSetting().contractCostParamsCpuInsns(); #endif } @@ -701,7 +701,7 @@ SorobanNetworkConfig::loadMemCostParams(AbstractLedgerTxn& ltx) LedgerKey key(CONFIG_SETTING); key.configSetting().configSettingID = ConfigSettingID::CONFIG_SETTING_CONTRACT_COST_PARAMS_MEMORY_BYTES; - auto le = ltx.loadWithoutRecord(key, /*loadExpiredEntry=*/false).current(); + auto le = ltx.loadWithoutRecord(key).current(); mMemCostParams = le.data.configSetting().contractCostParamsMemBytes(); #endif } @@ -713,7 +713,7 @@ SorobanNetworkConfig::loadExecutionLanesSettings(AbstractLedgerTxn& ltx) LedgerKey key(CONFIG_SETTING); key.configSetting().configSettingID = ConfigSettingID::CONFIG_SETTING_CONTRACT_EXECUTION_LANES; - auto le = ltx.loadWithoutRecord(key, /*loadExpiredEntry=*/false).current(); + auto le = ltx.loadWithoutRecord(key).current(); auto const& configSetting = le.data.configSetting().contractExecutionLanes(); mLedgerMaxTxCount = configSetting.ledgerMaxTxCount; @@ -727,7 +727,7 @@ SorobanNetworkConfig::loadBucketListSizeWindow(AbstractLedgerTxn& ltx) LedgerKey key(CONFIG_SETTING); key.configSetting().configSettingID = ConfigSettingID::CONFIG_SETTING_BUCKETLIST_SIZE_WINDOW; - auto txle = ltx.loadWithoutRecord(key, /*loadExpiredEntry=*/false); + auto txle = ltx.loadWithoutRecord(key); releaseAssert(txle); auto const& leVector = txle.current().data.configSetting().bucketListSizeWindow(); @@ -807,7 +807,7 @@ SorobanNetworkConfig::loadStateExpirationSettings(AbstractLedgerTxn& ltx) LedgerKey key(CONFIG_SETTING); key.configSetting().configSettingID = ConfigSettingID::CONFIG_SETTING_STATE_EXPIRATION; - auto le = ltx.loadWithoutRecord(key, /*loadExpiredEntry=*/false).current(); + auto le = ltx.loadWithoutRecord(key).current(); mStateExpirationSettings = le.data.configSetting().stateExpirationSettings(); #endif diff --git a/src/ledger/TrustLineWrapper.cpp b/src/ledger/TrustLineWrapper.cpp index 7743791a6e..356a3183f1 100644 --- a/src/ledger/TrustLineWrapper.cpp +++ b/src/ledger/TrustLineWrapper.cpp @@ -420,7 +420,7 @@ ConstTrustLineWrapper::ConstTrustLineWrapper(AbstractLedgerTxn& ltx, LedgerKey key(TRUSTLINE); key.trustLine().accountID = accountID; key.trustLine().asset = assetToTrustLineAsset(asset); - auto entry = ltx.loadWithoutRecord(key, /*loadExpiredEntry=*/false); + auto entry = ltx.loadWithoutRecord(key); if (entry) { mImpl = std::make_unique(std::move(entry)); diff --git a/src/ledger/test/LedgerTxnTests.cpp b/src/ledger/test/LedgerTxnTests.cpp index 0c2b07fc2d..ea2fd08041 100644 --- a/src/ledger/test/LedgerTxnTests.cpp +++ b/src/ledger/test/LedgerTxnTests.cpp @@ -532,7 +532,7 @@ TEST_CASE("LedgerTxn rollback and commit deactivate", "[ledgertxn]") { LedgerTxn ltx(root, false); ltx.create(le); - auto entry = ltx.loadWithoutRecord(key, /*loadExpiredEntry=*/false); + auto entry = ltx.loadWithoutRecord(key); REQUIRE(entry); f(ltx); REQUIRE_THROWS_AS(!entry, std::runtime_error); @@ -871,9 +871,7 @@ TEST_CASE("LedgerTxn eraseWithoutLoading", "[ledgertxn]") LedgerTxn ltx1(app->getLedgerTxnRoot()); REQUIRE_NOTHROW(ltx1.eraseWithoutLoading(key)); REQUIRE_THROWS_AS(ltx1.getDelta(), std::runtime_error); - REQUIRE( - ltx1.getNewestVersion(key, /*loadExpiredEntry=*/false).get() == - nullptr); + REQUIRE(ltx1.getNewestVersion(key).get() == nullptr); } SECTION("when key exists in parent") @@ -884,9 +882,7 @@ TEST_CASE("LedgerTxn eraseWithoutLoading", "[ledgertxn]") LedgerTxn ltx2(ltx1); REQUIRE_NOTHROW(ltx2.eraseWithoutLoading(key)); REQUIRE_THROWS_AS(ltx2.getDelta(), std::runtime_error); - REQUIRE( - ltx2.getNewestVersion(key, /*loadExpiredEntry=*/false).get() == - nullptr); + REQUIRE(ltx2.getNewestVersion(key).get() == nullptr); } SECTION("when key exists in grandparent, erased in parent") @@ -907,9 +903,7 @@ TEST_CASE("LedgerTxn eraseWithoutLoading", "[ledgertxn]") LedgerTxn ltx3(ltx2); REQUIRE_NOTHROW(ltx3.eraseWithoutLoading(key)); REQUIRE_THROWS_AS(ltx3.getDelta(), std::runtime_error); - REQUIRE( - ltx3.getNewestVersion(key, /*loadExpiredEntry=*/false).get() == - nullptr); + REQUIRE(ltx3.getNewestVersion(key).get() == nullptr); } }; @@ -1469,41 +1463,6 @@ TEST_CASE_VERSIONS("LedgerTxn load", "[ledgertxn]") ltx1.commit(); } }); - -#ifdef ENABLE_NEXT_PROTOCOL_VERSION_UNSAFE_FOR_PRODUCTION - SECTION("Do not load expired entry") - { - auto codeEntry = - LedgerTestUtils::generateValidLedgerEntryWithTypes( - {LedgerEntryType::CONTRACT_CODE}); - auto dataEntry = - LedgerTestUtils::generateValidLedgerEntryWithTypes( - {LedgerEntryType::CONTRACT_DATA}); - setExpirationLedger(codeEntry, 0); - setExpirationLedger(dataEntry, 0); - LedgerTxn ltx1(app->getLedgerTxnRoot()); - REQUIRE(ltx1.create(codeEntry)); - REQUIRE(ltx1.create(dataEntry)); - ltx1.commit(); - - LedgerTxn ltx2(app->getLedgerTxnRoot()); - auto codeKey = LedgerEntryKey(codeEntry); - auto dataKey = LedgerEntryKey(dataEntry); - REQUIRE(!ltx2.load(codeKey)); - REQUIRE(!ltx2.load(dataKey)); - - // Check that you can still load expired entry with flag set - auto loadedCode = - ltx2.loadWithoutRecord(codeKey, /*loadExpiredEntry=*/true); - REQUIRE(loadedCode); - REQUIRE(loadedCode.current().data == codeEntry.data); - - auto loadedData = - ltx2.loadWithoutRecord(dataKey, /*loadExpiredEntry=*/true); - REQUIRE(loadedData); - REQUIRE(loadedData.current().data == dataEntry.data); - } -#endif } SECTION("load tests for all versions") @@ -1599,24 +1558,20 @@ TEST_CASE("LedgerTxn loadWithoutRecord", "[ledgertxn]") { LedgerTxn ltx1(app->getLedgerTxnRoot()); LedgerTxn ltx2(ltx1); - REQUIRE_THROWS_AS( - ltx1.loadWithoutRecord(key, /*loadExpiredEntry=*/false), - std::runtime_error); + REQUIRE_THROWS_AS(ltx1.loadWithoutRecord(key), std::runtime_error); } SECTION("fails if sealed") { LedgerTxn ltx1(app->getLedgerTxnRoot()); ltx1.getDelta(); - REQUIRE_THROWS_AS( - ltx1.loadWithoutRecord(key, /*loadExpiredEntry=*/false), - std::runtime_error); + REQUIRE_THROWS_AS(ltx1.loadWithoutRecord(key), std::runtime_error); } SECTION("when key does not exist") { LedgerTxn ltx1(app->getLedgerTxnRoot()); - REQUIRE(!ltx1.loadWithoutRecord(key, /*loadExpiredEntry=*/false)); + REQUIRE(!ltx1.loadWithoutRecord(key)); validate(ltx1, {}); } @@ -1626,7 +1581,7 @@ TEST_CASE("LedgerTxn loadWithoutRecord", "[ledgertxn]") REQUIRE(ltx1.create(le)); LedgerTxn ltx2(ltx1); - REQUIRE(ltx2.loadWithoutRecord(key, /*loadExpiredEntry=*/false)); + REQUIRE(ltx2.loadWithoutRecord(key)); validate(ltx2, {}); } @@ -1639,7 +1594,7 @@ TEST_CASE("LedgerTxn loadWithoutRecord", "[ledgertxn]") REQUIRE_NOTHROW(ltx2.erase(key)); LedgerTxn ltx3(ltx2); - REQUIRE(!ltx3.loadWithoutRecord(key, /*loadExpiredEntry=*/false)); + REQUIRE(!ltx3.loadWithoutRecord(key)); validate(ltx3, {}); } } @@ -2628,25 +2583,20 @@ TEST_CASE("LedgerTxnEntry and LedgerTxnHeader move assignment", "[ledgertxn]") entry1 = std::move(entryRef); REQUIRE(entry1.current() == le1); REQUIRE_THROWS_AS(ltx.load(key1), std::runtime_error); - REQUIRE_THROWS_AS( - ltx.loadWithoutRecord(key1, /*loadExpiredEntry=*/false), - std::runtime_error); + REQUIRE_THROWS_AS(ltx.loadWithoutRecord(key1), std::runtime_error); } SECTION("const entry") { LedgerTxn ltx(root, false); ltx.create(le1); - auto entry1 = - ltx.loadWithoutRecord(key1, /*loadExpiredEntry=*/false); + auto entry1 = ltx.loadWithoutRecord(key1); // Avoid warning for explicit move-to-self ConstLedgerTxnEntry& entryRef = entry1; entry1 = std::move(entryRef); REQUIRE(entry1.current() == le1); REQUIRE_THROWS_AS(ltx.load(key1), std::runtime_error); - REQUIRE_THROWS_AS( - ltx.loadWithoutRecord(key1, /*loadExpiredEntry=*/false), - std::runtime_error); + REQUIRE_THROWS_AS(ltx.loadWithoutRecord(key1), std::runtime_error); } SECTION("header") @@ -2672,8 +2622,7 @@ TEST_CASE("LedgerTxnEntry and LedgerTxnHeader move assignment", "[ledgertxn]") REQUIRE(entry1.current() == le2); REQUIRE_THROWS_AS(ltx.load(key2), std::runtime_error); REQUIRE(ltx.load(key1).current() == le1); - REQUIRE(ltx.loadWithoutRecord(key1, /*loadExpiredEntry=*/false) - .current() == le1); + REQUIRE(ltx.loadWithoutRecord(key1).current() == le1); } SECTION("const entry") @@ -2681,16 +2630,13 @@ TEST_CASE("LedgerTxnEntry and LedgerTxnHeader move assignment", "[ledgertxn]") LedgerTxn ltx(root, false); ltx.create(le1); ltx.create(le2); - auto entry1 = - ltx.loadWithoutRecord(key1, /*loadExpiredEntry=*/false); - auto entry2 = - ltx.loadWithoutRecord(key2, /*loadExpiredEntry=*/false); + auto entry1 = ltx.loadWithoutRecord(key1); + auto entry2 = ltx.loadWithoutRecord(key2); entry1 = std::move(entry2); REQUIRE(entry1.current() == le2); REQUIRE_THROWS_AS(ltx.load(key2), std::runtime_error); REQUIRE(ltx.load(key1).current() == le1); - REQUIRE(ltx.loadWithoutRecord(key1, /*loadExpiredEntry=*/false) - .current() == le1); + REQUIRE(ltx.loadWithoutRecord(key1).current() == le1); } SECTION("header") @@ -2766,57 +2712,6 @@ TEST_CASE("LedgerTxnRoot prefetch", "[ledgertxn]") REQUIRE(root.prefetch(keysToPrefetch) == keysToPrefetch.size()); ltx2.commit(); } - -#ifdef ENABLE_NEXT_PROTOCOL_VERSION_UNSAFE_FOR_PRODUCTION - SECTION("do not prefetch expired entries") - { - auto sorobanEntries = - LedgerTestUtils::generateValidUniqueLedgerEntriesWithTypes( - {CONTRACT_DATA, CONTRACT_CODE}, 100); - UnorderedSet sorobanKeysToPrefetch; - UnorderedSet expiredKeys; - - LedgerTxn ltx3(root); - for (auto e : sorobanEntries) - { - auto k = LedgerEntryKey(e); - sorobanKeysToPrefetch.emplace(k); - if (rand_flip()) - { - expiredKeys.emplace(k); - setExpirationLedger(e, 0); - } - else - { - setExpirationLedger(e, 2); - } - - ltx3.create(e); - } - ltx3.commit(); - - REQUIRE(root.prefetch(sorobanKeysToPrefetch) == - sorobanKeysToPrefetch.size()); - - LedgerTxn ltx4(root); - for (auto const& key : sorobanKeysToPrefetch) - { - auto loadedEntry = ltx4.load(key); - if (expiredKeys.find(key) == expiredKeys.end()) - { - REQUIRE(loadedEntry); - } - else - { - REQUIRE(!loadedEntry); - } - } - - // 100% hit rate but make it floating point - REQUIRE(fabs(ltx4.getPrefetchHitRate() - 1.0f) < 0.0001f); - ltx4.commit(); - } -#endif }; SECTION("default") @@ -3655,8 +3550,7 @@ TEST_CASE("LedgerTxn in memory order book", "[ledgertxn]") LedgerTxn ltx(app->getLedgerTxnRoot()); { - auto lte = ltx.loadWithoutRecord(LedgerEntryKey(le1a), - /*loadExpiredEntry=*/false); + auto lte = ltx.loadWithoutRecord(LedgerEntryKey(le1a)); checkOrderBook(ltx, {}); } checkOrderBook(ltx, {}); @@ -3668,8 +3562,7 @@ TEST_CASE("LedgerTxn in memory order book", "[ledgertxn]") checkOrderBook(ltx, {{assets, {le1a}}}); { - auto lte = ltx.loadWithoutRecord(LedgerEntryKey(le1a), - /*loadExpiredEntry=*/false); + auto lte = ltx.loadWithoutRecord(LedgerEntryKey(le1a)); checkOrderBook(ltx, {}); } checkOrderBook(ltx, {{assets, {le1a}}}); @@ -3847,12 +3740,10 @@ TEST_CASE("Access deactivated entry", "[ledgertxn]") SECTION("loadWithoutRecord") { - auto entry = - ltx1.loadWithoutRecord(lk1, /*loadExpiredEntry=*/false); + auto entry = ltx1.loadWithoutRecord(lk1); REQUIRE(entry); - auto missingEntry = ltx1.loadWithoutRecord( - missingEntryKey, /*loadExpiredEntry=*/false); + auto missingEntry = ltx1.loadWithoutRecord(missingEntryKey); REQUIRE(!missingEntry); // this will deactivate entry @@ -3892,7 +3783,7 @@ TEST_CASE("Access deactivated entry", "[ledgertxn]") SECTION("loadWithoutRecord and move assign") { ConstLedgerTxnEntry entry; - entry = ltx1.loadWithoutRecord(lk1, /*loadExpiredEntry=*/false); + entry = ltx1.loadWithoutRecord(lk1); REQUIRE(entry); ConstLedgerTxnEntry ltxe2; @@ -3901,8 +3792,7 @@ TEST_CASE("Access deactivated entry", "[ledgertxn]") } SECTION("loadWithoutRecord and move construct") { - auto entry = - ltx1.loadWithoutRecord(lk1, /*loadExpiredEntry=*/false); + auto entry = ltx1.loadWithoutRecord(lk1); REQUIRE(entry); ConstLedgerTxnEntry ltxe2(std::move(entry)); diff --git a/src/main/CommandHandler.cpp b/src/main/CommandHandler.cpp index 9e4f3a96d4..096c7e721e 100644 --- a/src/main/CommandHandler.cpp +++ b/src/main/CommandHandler.cpp @@ -720,7 +720,7 @@ CommandHandler::getLedgerEntry(std::string const& params, std::string& retStr) LedgerKey k; fromOpaqueBase64(k, key); - auto le = ltx.loadWithoutRecord(k, /*loadExpiredEntry=*/false); + auto le = ltx.loadWithoutRecord(k); if (le) { root["state"] = "live"; diff --git a/src/test/TxTests.cpp b/src/test/TxTests.cpp index 1397523299..cc5da84b1f 100644 --- a/src/test/TxTests.cpp +++ b/src/test/TxTests.cpp @@ -1726,7 +1726,7 @@ makeConfigUpgradeSet(AbstractLedgerTxn& ltx, ConfigUpgradeSet configUpgradeSet) le.data.contractData().body.bodyType(DATA_ENTRY); le.data.contractData().contract.type(SC_ADDRESS_TYPE_CONTRACT); le.data.contractData().contract.contractId() = contractID; - le.data.contractData().durability = PERSISTENT; + le.data.contractData().durability = TEMPORARY; le.data.contractData().expirationLedgerSeq = UINT32_MAX; le.data.contractData().key = key; le.data.contractData().body.data().val = val; diff --git a/src/transactions/BumpFootprintExpirationOpFrame.cpp b/src/transactions/BumpFootprintExpirationOpFrame.cpp index e015d1c07c..a49839809a 100644 --- a/src/transactions/BumpFootprintExpirationOpFrame.cpp +++ b/src/transactions/BumpFootprintExpirationOpFrame.cpp @@ -68,10 +68,10 @@ BumpFootprintExpirationOpFrame::doApply(Application& app, ledgerSeq + mBumpFootprintExpirationOp.ledgersToExpire; for (auto const& lk : footprint.readOnly) { - // When we move to use EXPIRATION_EXTENSIONS, this should become a + // TODO: when we move to use EXPIRATION_EXTENSIONS, this should become a // loadWithoutRecord, and the metrics should be updated. auto ltxe = ltx.load(lk); - if (!ltxe) + if (!ltxe || !isLive(ltxe.current(), ledgerSeq)) { // Skip the missing entries. Since this happens at apply // time and we refund the unspent fees, it is more beneficial diff --git a/src/transactions/CreateAccountOpFrame.cpp b/src/transactions/CreateAccountOpFrame.cpp index 7b2a7e6edf..a0671cd74e 100644 --- a/src/transactions/CreateAccountOpFrame.cpp +++ b/src/transactions/CreateAccountOpFrame.cpp @@ -47,8 +47,7 @@ CreateAccountOpFrame::doApplyBeforeV14(AbstractLedgerTxn& ltx) bool doesAccountExist = protocolVersionStartsFrom(header.current().ledgerVersion, ProtocolVersion::V_8) || - (bool)ltx.loadWithoutRecord(accountKey(getSourceID()), - /*loadExpiredEntry=*/false); + (bool)ltx.loadWithoutRecord(accountKey(getSourceID())); auto sourceAccount = loadSourceAccount(ltx, header); if (getAvailableBalance(header, sourceAccount) < diff --git a/src/transactions/InvokeHostFunctionOpFrame.cpp b/src/transactions/InvokeHostFunctionOpFrame.cpp index cdfdbe7d08..2f64223531 100644 --- a/src/transactions/InvokeHostFunctionOpFrame.cpp +++ b/src/transactions/InvokeHostFunctionOpFrame.cpp @@ -418,39 +418,54 @@ InvokeHostFunctionOpFrame::doApply(Application& app, AbstractLedgerTxn& ltx, // accidentally override RW entry with RO flag. entryRentChange.readOnly = readOnly; // Load without record for readOnly to avoid writing them later - auto ltxe = ltx.loadWithoutRecord(lk, /*loadExpiredEntry=*/false); + auto ltxe = ltx.loadWithoutRecord(lk); if (ltxe) { auto const& le = ltxe.current(); - auto buf = toCxxBuf(le); - entrySize = static_cast(buf.data->size()); - ledgerEntryCxxBufs.emplace_back(std::move(buf)); - if (isSorobanEntry(le.data)) + bool shouldAddEntry = true; + if (!isLive(le, ltx.getHeader().ledgerSeq)) { - uint32_t const totalReadSize = keySize + entrySize; - entryRentChange.oldSize = totalReadSize; - entryRentChange.newSize = totalReadSize; - - entryRentChange.oldExpirationLedger = - getExpirationLedger(le); - entryRentChange.newExpirationLedger = - entryRentChange.oldExpirationLedger; - if (autobumpLedgerCount > 0 && autoBumpEnabled(le)) + if (isTemporaryEntry(lk)) { - // Add the autobump ledgers on top of the old - // expiration. Since expiration is inclusive, the rent - // must be already payed for `oldExpirationLedger`. - entryRentChange.newExpirationLedger += - autobumpLedgerCount; + // For temporary entries, treat the expired entry as if + // the key did not exist + shouldAddEntry = false; + } + else + { + // Cannot access an expired entry + this->innerResult().code( + INVOKE_HOST_FUNCTION_ENTRY_EXPIRED); + return false; + } + } + + if (shouldAddEntry) + { + auto buf = toCxxBuf(le); + entrySize = static_cast(buf.data->size()); + ledgerEntryCxxBufs.emplace_back(std::move(buf)); + if (isSorobanEntry(le.data)) + { + uint32_t const totalReadSize = keySize + entrySize; + entryRentChange.oldSize = totalReadSize; + entryRentChange.newSize = totalReadSize; + + entryRentChange.oldExpirationLedger = + getExpirationLedger(le); + entryRentChange.newExpirationLedger = + entryRentChange.oldExpirationLedger; + if (autobumpLedgerCount > 0 && autoBumpEnabled(le)) + { + // Add the autobump ledgers on top of the old + // expiration. Since expiration is inclusive, the + // rent must be already payed for + // `oldExpirationLedger`. + entryRentChange.newExpirationLedger += + autobumpLedgerCount; + } } } - } - else if (!isTemporaryEntry(lk) && - ltx.loadWithoutRecord(lk, /*loadExpiredEntry=*/true)) - { - // Cannot access an expired entry - this->innerResult().code(INVOKE_HOST_FUNCTION_ENTRY_EXPIRED); - return false; } metrics.noteReadEntry(isCodeKey(lk), keySize, entrySize); diff --git a/src/transactions/MergeOpFrame.cpp b/src/transactions/MergeOpFrame.cpp index 324d70f9b8..4e4c5a0ff6 100644 --- a/src/transactions/MergeOpFrame.cpp +++ b/src/transactions/MergeOpFrame.cpp @@ -98,8 +98,7 @@ MergeOpFrame::doApplyBeforeV16(AbstractLedgerTxn& ltx) // in versions < 8, merge account could be called with a stale account LedgerKey key(ACCOUNT); key.account().accountID = getSourceID(); - auto thisAccount = - ltx.loadWithoutRecord(key, /*loadExpiredEntry=*/false); + auto thisAccount = ltx.loadWithoutRecord(key); if (!thisAccount) { innerResult().code(ACCOUNT_MERGE_NO_ACCOUNT); diff --git a/src/transactions/RestoreFootprintOpFrame.cpp b/src/transactions/RestoreFootprintOpFrame.cpp index a754c6cf6e..7128901db8 100644 --- a/src/transactions/RestoreFootprintOpFrame.cpp +++ b/src/transactions/RestoreFootprintOpFrame.cpp @@ -74,27 +74,35 @@ RestoreFootprintOpFrame::doApply(Application& app, AbstractLedgerTxn& ltx, for (auto const& lk : footprint.readWrite) { auto keySize = static_cast(xdr::xdr_size(lk)); - auto ltxe = ltx.loadWithoutRecord(lk, /*loadExpiredEntry=*/true); - if (!ltxe) + uint32_t entrySize = UINT32_MAX; { - // Skip entries that don't exist. - continue; + auto const_ltxe = ltx.loadWithoutRecord(lk); + if (!const_ltxe) + { + continue; + } + + entrySize = + static_cast(xdr::xdr_size(const_ltxe.current())); + metrics.mLedgerReadByte += keySize + entrySize; + if (resources.readBytes < metrics.mLedgerReadByte) + { + innerResult().code(RESTORE_FOOTPRINT_RESOURCE_LIMIT_EXCEEDED); + return false; + } + + if (isLive(const_ltxe.current(), ledgerSeq)) + { + // Skip entries that are already live. + continue; + } } - auto entrySize = static_cast(xdr::xdr_size(ltxe.current())); - metrics.mLedgerReadByte += keySize + entrySize; - if (resources.readBytes < metrics.mLedgerReadByte) - { - innerResult().code(RESTORE_FOOTPRINT_RESOURCE_LIMIT_EXCEEDED); - return false; - } + // Entry exists if we get this this point due to the loadWithoutRecord + // logic above. + auto ltxe = ltx.load(lk); - if (isLive(ltxe.current(), ledgerSeq)) - { - // Skip entries that are already live. - continue; - } - LedgerEntry restoredEntry = ltxe.current(); + auto& restoredEntry = ltxe.current(); metrics.mLedgerWriteByte += keySize + entrySize; if (resources.extendedMetaDataSizeBytes < @@ -116,7 +124,6 @@ RestoreFootprintOpFrame::doApply(Application& app, AbstractLedgerTxn& ltx, rustChange.new_size_bytes = keySize + entrySize; rustChange.new_expiration_ledger = restoredExpirationLedger; setExpirationLedger(restoredEntry, restoredExpirationLedger); - ltx.restore(restoredEntry); } int64_t rentFee = rust_bridge::compute_rent_fee( app.getConfig().CURRENT_LEDGER_PROTOCOL_VERSION, diff --git a/src/transactions/TransactionFrame.cpp b/src/transactions/TransactionFrame.cpp index 21c1e0b4e4..e56a788203 100644 --- a/src/transactions/TransactionFrame.cpp +++ b/src/transactions/TransactionFrame.cpp @@ -361,8 +361,7 @@ TransactionFrame::loadSourceAccount(AbstractLedgerTxn& ltx, // this is buggy caching that existed in old versions of the protocol if (res) { - auto newest = ltx.getNewestVersion(LedgerEntryKey(res.current()), - /*loadExpiredEntry=*/false); + auto newest = ltx.getNewestVersion(LedgerEntryKey(res.current())); mCachedAccount = newest; } else @@ -395,8 +394,7 @@ TransactionFrame::loadAccount(AbstractLedgerTxn& ltx, res = ltx.create(*mCachedAccount); } - auto newest = ltx.getNewestVersion(LedgerEntryKey(res.current()), - /*loadExpiredEntry=*/false); + auto newest = ltx.getNewestVersion(LedgerEntryKey(res.current())); mCachedAccount = newest; return res; } diff --git a/src/transactions/TransactionUtils.cpp b/src/transactions/TransactionUtils.cpp index 97a7279cf5..81dbb45974 100644 --- a/src/transactions/TransactionUtils.cpp +++ b/src/transactions/TransactionUtils.cpp @@ -333,8 +333,7 @@ ConstLedgerTxnEntry loadAccountWithoutRecord(AbstractLedgerTxn& ltx, AccountID const& accountID) { ZoneScoped; - return ltx.loadWithoutRecord(accountKey(accountID), - /*loadExpiredEntry=*/false); + return ltx.loadWithoutRecord(accountKey(accountID)); } LedgerTxnEntry @@ -436,21 +435,18 @@ loadLiquidityPool(AbstractLedgerTxn& ltx, PoolID const& poolID) #ifdef ENABLE_NEXT_PROTOCOL_VERSION_UNSAFE_FOR_PRODUCTION ConstLedgerTxnEntry loadContractData(AbstractLedgerTxn& ltx, SCAddress const& contract, - SCVal const& dataKey, ContractDataDurability type, - bool loadExpiredEntry) + SCVal const& dataKey, ContractDataDurability type) { ZoneScoped; return ltx.loadWithoutRecord( - contractDataKey(contract, dataKey, type, DATA_ENTRY), loadExpiredEntry); + contractDataKey(contract, dataKey, type, DATA_ENTRY)); } ConstLedgerTxnEntry -loadContractCode(AbstractLedgerTxn& ltx, Hash const& hash, - bool loadExpiredEntry) +loadContractCode(AbstractLedgerTxn& ltx, Hash const& hash) { ZoneScoped; - return ltx.loadWithoutRecord(contractCodeKey(hash, DATA_ENTRY), - loadExpiredEntry); + return ltx.loadWithoutRecord(contractCodeKey(hash, DATA_ENTRY)); } #endif @@ -1408,10 +1404,8 @@ prefetchForRevokeFromPoolShareTrustLines( { // prefetching shouldn't affect the protocol, so use loadWithoutRecord // to not touch lastModified - auto pool = ltx.loadWithoutRecord( - liquidityPoolKey( - trustLine.current().data.trustLine().asset.liquidityPoolID()), - /*loadExpiredEntry=*/false); + auto pool = ltx.loadWithoutRecord(liquidityPoolKey( + trustLine.current().data.trustLine().asset.liquidityPoolID())); auto const& params = pool.current().data.liquidityPool().body.constantProduct().params; diff --git a/src/transactions/TransactionUtils.h b/src/transactions/TransactionUtils.h index 2164924b79..adf7db5cea 100644 --- a/src/transactions/TransactionUtils.h +++ b/src/transactions/TransactionUtils.h @@ -148,10 +148,8 @@ LedgerTxnEntry loadLiquidityPool(AbstractLedgerTxn& ltx, PoolID const& poolID); ConstLedgerTxnEntry loadContractData(AbstractLedgerTxn& ltx, SCAddress const& contract, SCVal const& dataKey, - ContractDataDurability type, - bool loadExpiredEntry = false); -ConstLedgerTxnEntry loadContractCode(AbstractLedgerTxn& ltx, Hash const& hash, - bool loadExpiredEntry = false); + ContractDataDurability type); +ConstLedgerTxnEntry loadContractCode(AbstractLedgerTxn& ltx, Hash const& hash); #endif void acquireLiabilities(AbstractLedgerTxn& ltx, LedgerTxnHeader const& header, diff --git a/src/transactions/test/InvokeHostFunctionTests.cpp b/src/transactions/test/InvokeHostFunctionTests.cpp index e5932a495b..d3e64ecb72 100644 --- a/src/transactions/test/InvokeHostFunctionTests.cpp +++ b/src/transactions/test/InvokeHostFunctionTests.cpp @@ -111,6 +111,24 @@ makeU32(uint32_t u32) return val; } +static uint32_t +getLedgerSeq(Application& app) +{ + LedgerTxn ltx(app.getLedgerTxnRoot()); + return ltx.loadHeader().current().ledgerSeq; +} + +static LedgerEntry +loadStorageEntry(Application& app, SCAddress const& contractID, + std::string const& key, ContractDataDurability type) +{ + auto keySymbol = makeSymbolSCVal(key); + LedgerTxn ltx(app.getLedgerTxnRoot()); + auto ltxe = loadContractData(ltx, contractID, keySymbol, type); + REQUIRE(ltxe); + return ltxe.current(); +} + static void submitTxToUploadWasm(Application& app, Operation const& op, SorobanResources const& resources, @@ -467,20 +485,7 @@ TEST_CASE("contract storage", "[tx][soroban]") VirtualClock clock; auto app = createTestApplication(clock, getTestConfig()); auto root = TestAccount::createRoot(*app); - - { - // Autobump is disabled by default, so so enable it here for testing. - LedgerTxn ltx(app->getLedgerTxnRoot()); - auto networkConfig = - app->getLedgerManager().getSorobanNetworkConfig(ltx); - auto stateExpirationSettings = networkConfig.stateExpirationSettings(); - stateExpirationSettings.autoBumpLedgers = 10; - networkConfig.stateExpirationSettings() = stateExpirationSettings; - app->getLedgerManager().setSorobanNetworkConfig(networkConfig); - } - auto const contractDataWasm = rust_bridge::get_test_wasm_contract_data(); - auto contractKeys = deployContractWithSourceAccount(*app, contractDataWasm); auto const& contractID = contractKeys[0].contractData().contract; auto checkContractData = [&](SCVal const& key, ContractDataDurability type, @@ -501,27 +506,6 @@ TEST_CASE("contract storage", "[tx][soroban]") } }; - auto getContractDataExpiration = [&](std::string const& key, - ContractDataDurability type, - uint32_t flags = 0) { - auto keySymbol = makeSymbolSCVal(key); - LedgerTxn ltx(app->getLedgerTxnRoot()); - auto ltxe = loadContractData(ltx, contractID, keySymbol, type); - REQUIRE(ltxe); - REQUIRE(ltxe.current().data.contractData().body.data().flags == flags); - return getExpirationLedger(ltxe.current()); - }; - - auto isEntryLive = [&](std::string const& key, ContractDataDurability type, - uint32_t ledgerSeq) { - auto keySymbol = makeSymbolSCVal(key); - LedgerTxn ltx(app->getLedgerTxnRoot()); - auto ltxe = loadContractData(ltx, contractID, keySymbol, type, - /*loadExpiredEntry*/ true); - REQUIRE(ltxe); - return isLive(ltxe.current(), ledgerSeq); - }; - auto createTx = [&](xdr::xvector const& readOnly, xdr::xvector const& readWrite, uint32_t writeBytes, SCAddress const& contractAddress, @@ -639,6 +623,47 @@ TEST_CASE("contract storage", "[tx][soroban]") return hasWithFootprint(key, readOnly, 0, true, type); }; + auto getWithFootprint = + [&](std::string const& key, xdr::xvector const& readOnly, + xdr::xvector const& readWrite, uint32_t writeBytes, + bool expectSuccess, ContractDataDurability type) { + auto keySymbol = makeSymbolSCVal(key); + + std::string funcStr; + switch (type) + { + case ContractDataDurability::TEMPORARY: + funcStr = "get_temporary"; + break; + case ContractDataDurability::PERSISTENT: + funcStr = "get_persistent"; + break; + } + + auto [tx, ltx, txm] = + createTx(readOnly, readWrite, writeBytes, contractID, + makeSymbol(funcStr), {keySymbol}); + + if (expectSuccess) + { + REQUIRE(tx->apply(*app, *ltx, *txm)); + ltx->commit(); + } + else + { + REQUIRE(!tx->apply(*app, *ltx, *txm)); + ltx->commit(); + } + }; + + auto get = [&](std::string const& key, ContractDataDurability type, + bool expectSuccess) { + getWithFootprint(key, contractKeys, + {contractDataKey(contractID, makeSymbolSCVal(key), + type, DATA_ENTRY)}, + 1000, expectSuccess, type); + }; + auto bumpWithFootprint = [&](std::string const& key, uint32_t bumpAmount, xdr::xvector const& readOnly, xdr::xvector const& readWrite, @@ -728,6 +753,7 @@ TEST_CASE("contract storage", "[tx][soroban]") REQUIRE(root.getBalance() - balanceAfterFeeCharged == refundableFee - expectedRefundableFeeCharged); }; + auto bumpOp = [&](uint32_t bumpAmount, xdr::xvector const& readOnly, int64_t expectedRefundableFeeCharged) { @@ -738,15 +764,15 @@ TEST_CASE("contract storage", "[tx][soroban]") SorobanResources bumpResources; bumpResources.footprint.readOnly = readOnly; bumpResources.instructions = 0; - bumpResources.readBytes = 1000; + bumpResources.readBytes = 5'000; bumpResources.writeBytes = 0; - bumpResources.extendedMetaDataSizeBytes = 1000; + bumpResources.extendedMetaDataSizeBytes = 10'000; auto tx = sorobanTransactionFrameFromOps(app->getNetworkID(), root, {bumpOp}, - {}, bumpResources, 100'000, 1200); + {}, bumpResources, 100'000, 2'000); - runExpirationOp(root, tx, 1200, expectedRefundableFeeCharged); + runExpirationOp(root, tx, 2'000, expectedRefundableFeeCharged); }; auto restoreOp = [&](xdr::xvector const& readWrite, @@ -757,15 +783,15 @@ TEST_CASE("contract storage", "[tx][soroban]") SorobanResources bumpResources; bumpResources.footprint.readWrite = readWrite; bumpResources.instructions = 0; - bumpResources.readBytes = 1000; - bumpResources.writeBytes = 1000; - bumpResources.extendedMetaDataSizeBytes = 1000; + bumpResources.readBytes = 5'000; + bumpResources.writeBytes = 5'000; + bumpResources.extendedMetaDataSizeBytes = 10'000; // submit operation auto tx = sorobanTransactionFrameFromOps(app->getNetworkID(), root, {restoreOp}, {}, bumpResources, - 100'000, 1'200); - runExpirationOp(root, tx, 1200, expectedRefundableFeeCharged); + 100'000, 2'000); + runExpirationOp(root, tx, 2'000, expectedRefundableFeeCharged); }; auto delWithFootprint = [&](std::string const& key, @@ -810,6 +836,34 @@ TEST_CASE("contract storage", "[tx][soroban]") true, type); }; + auto checkContractDataExpirationLedger = + [&](std::string const& key, ContractDataDurability type, + uint32_t expectedExpirationLedger, uint32_t flags = 0) { + auto le = loadStorageEntry(*app, contractID, key, type); + REQUIRE(le.data.contractData().body.data().flags == flags); + REQUIRE(getExpirationLedger(le) == expectedExpirationLedger); + }; + + auto checkContractDataExpirationState = + [&](std::string const& key, ContractDataDurability type, + uint32_t ledgerSeq, bool expectedIsLive) { + auto le = loadStorageEntry(*app, contractID, key, type); + REQUIRE(isLive(le, ledgerSeq) == expectedIsLive); + + // Make sure entry is accessible/inaccessible + get(key, type, expectedIsLive); + }; + + auto checkKeyExpirationLedger = [&](LedgerKey const& key, + uint32_t ledgerSeq, + uint32_t expectedExpirationLedger) { + LedgerTxn ltx(app->getLedgerTxnRoot()); + auto ltxe = ltx.load(key); + REQUIRE(ltxe); + REQUIRE(expectedExpirationLedger == + getExpirationLedger(ltxe.current())); + }; + SECTION("default limits") { put("key1", 0, ContractDataDurability::PERSISTENT); @@ -846,12 +900,14 @@ TEST_CASE("contract storage", "[tx][soroban]") } SorobanNetworkConfig refConfig; - uint32_t ledgerSeq; { LedgerTxn ltx(app->getLedgerTxnRoot()); refConfig = app->getLedgerManager().getSorobanNetworkConfig(ltx); - ledgerSeq = ltx.loadHeader().current().ledgerSeq; } + + auto const& stateExpirationSettings = refConfig.stateExpirationSettings(); + auto ledgerSeq = getLedgerSeq(*app); + SECTION("failure: entry exceeds max size") { refConfig.maxContractDataKeySizeBytes() = 300; @@ -869,14 +925,15 @@ TEST_CASE("contract storage", "[tx][soroban]") { // Check that each type is in their own keyspace uint64_t uniqueVal = 0; - uint64_t recreatableVal = 1; uint64_t temporaryVal = 2; + put("key", uniqueVal, ContractDataDurability::PERSISTENT); put("key", temporaryVal, TEMPORARY); + auto uniqueScVal = makeU64(uniqueVal); - auto recreatableScVal = makeU64(recreatableVal); auto temporaryScVal = makeU64(temporaryVal); auto keySymbol = makeSymbolSCVal("key"); + checkContractData(keySymbol, ContractDataDurability::PERSISTENT, &uniqueScVal); checkContractData(keySymbol, TEMPORARY, &temporaryScVal); @@ -884,34 +941,203 @@ TEST_CASE("contract storage", "[tx][soroban]") put("key2", 3, ContractDataDurability::PERSISTENT); auto key2Symbol = makeSymbolSCVal("key2"); auto uniqueScVal2 = makeU64(3); + checkContractData(key2Symbol, ContractDataDurability::PERSISTENT, &uniqueScVal2); checkContractData(key2Symbol, TEMPORARY, nullptr); } - auto const& stateExpirationSettings = refConfig.stateExpirationSettings(); - auto autoBump = stateExpirationSettings.autoBumpLedgers; + SECTION("contract instance and wasm expiration") + { + auto originalExpectedExpiration = + stateExpirationSettings.minPersistentEntryExpiration + ledgerSeq - + 1; + + for (uint32_t i = app->getLedgerManager().getLastClosedLedgerNum(); + i <= originalExpectedExpiration + 1; ++i) + { + closeLedgerOn(*app, i, 2, 1, 2016); + } + + // Contract instance and code are expired, an TX should fail + putWithFootprint("temp", 0, contractKeys, + {contractDataKey(contractID, makeSymbolSCVal("temp"), + TEMPORARY, DATA_ENTRY)}, + 1000, /*expectSuccess*/ false, + ContractDataDurability::TEMPORARY); + + ledgerSeq = getLedgerSeq(*app); + auto newExpectedExpiration = + stateExpirationSettings.minPersistentEntryExpiration + ledgerSeq - + 1; - SECTION("Enforce rent minimums") + SECTION("restore contract instance and wasm") + { + // Restore Instance and WASM + restoreOp(contractKeys, 1715); + + // Instance should now be useable + putWithFootprint( + "temp", 0, contractKeys, + {contractDataKey(contractID, makeSymbolSCVal("temp"), TEMPORARY, + DATA_ENTRY)}, + 1000, /*expectSuccess*/ true, + ContractDataDurability::TEMPORARY); + + checkKeyExpirationLedger(contractKeys[0], ledgerSeq, + newExpectedExpiration); + checkKeyExpirationLedger(contractKeys[1], ledgerSeq, + newExpectedExpiration); + } + + SECTION("restore contract instance, not wasm") + { + // Only restore contract instance + restoreOp({contractKeys[0]}, 68); + + // invocation should fail + putWithFootprint( + "temp", 0, contractKeys, + {contractDataKey(contractID, makeSymbolSCVal("temp"), TEMPORARY, + DATA_ENTRY)}, + 1000, /*expectSuccess*/ false, + ContractDataDurability::TEMPORARY); + + checkKeyExpirationLedger(contractKeys[0], ledgerSeq, + newExpectedExpiration); + checkKeyExpirationLedger(contractKeys[1], ledgerSeq, + originalExpectedExpiration); + } + + SECTION("restore contract wasm, not instance") + { + // Only restore WASM + restoreOp({contractKeys[1]}, 1648); + + // invocation should fail + putWithFootprint( + "temp", 0, contractKeys, + {contractDataKey(contractID, makeSymbolSCVal("temp"), TEMPORARY, + DATA_ENTRY)}, + 1000, /*expectSuccess*/ false, + ContractDataDurability::TEMPORARY); + + checkKeyExpirationLedger(contractKeys[0], ledgerSeq, + originalExpectedExpiration); + checkKeyExpirationLedger(contractKeys[1], ledgerSeq, + newExpectedExpiration); + } + + SECTION("lifetime extensions") + { + // Restore Instance and WASM + restoreOp(contractKeys, 1715); + + auto instanceBumpAmount = 10'000; + auto wasmBumpAmount = 15'000; + + // bump instance + bumpOp(instanceBumpAmount, {contractKeys[0]}, 69); + + // bump WASM + bumpOp(wasmBumpAmount, {contractKeys[1]}, 1754); + + checkKeyExpirationLedger(contractKeys[0], ledgerSeq, + ledgerSeq + instanceBumpAmount); + checkKeyExpirationLedger(contractKeys[1], ledgerSeq, + ledgerSeq + wasmBumpAmount); + } + } + + SECTION("contract storage expiration") { put("unique", 0, ContractDataDurability::PERSISTENT); put("temp", 0, TEMPORARY); auto expectedTempExpiration = stateExpirationSettings.minTempEntryExpiration + ledgerSeq - 1; - auto expectedRestorableExpiration = + auto expectedPersistentExpiration = stateExpirationSettings.minPersistentEntryExpiration + ledgerSeq - 1; - REQUIRE(getContractDataExpiration("unique", - ContractDataDurability::PERSISTENT) == - expectedRestorableExpiration); - REQUIRE(getContractDataExpiration("temp", TEMPORARY) == - expectedTempExpiration); + // Check for expected minimum lifetime values + checkContractDataExpirationLedger("unique", + ContractDataDurability::PERSISTENT, + expectedPersistentExpiration); + checkContractDataExpirationLedger("temp", TEMPORARY, + expectedTempExpiration); + + // Close ledgers until temp entry expires + uint32 currLedger = app->getLedgerManager().getLastClosedLedgerNum(); + for (; currLedger <= expectedTempExpiration + 1; ++currLedger) + { + closeLedgerOn(*app, currLedger, 2, 1, 2016); + } + REQUIRE(app->getLedgerManager().getLastClosedLedgerNum() == + expectedTempExpiration + 1); + + // Check that temp entry has expired + checkContractDataExpirationState( + "temp", TEMPORARY, app->getLedgerManager().getLastClosedLedgerNum(), + false); + + checkContractDataExpirationState( + "unique", ContractDataDurability::PERSISTENT, + app->getLedgerManager().getLastClosedLedgerNum(), true); + + // Check that we can recreate an expired TEMPORARY entry + putWithFootprint("temp", 0, contractKeys, + {contractDataKey(contractID, makeSymbolSCVal("temp"), + TEMPORARY, DATA_ENTRY)}, + 1000, /*expectSuccess*/ true, + ContractDataDurability::TEMPORARY); + + // Recreated entry should be live + ledgerSeq = getLedgerSeq(*app); + checkContractDataExpirationState( + "temp", TEMPORARY, app->getLedgerManager().getLastClosedLedgerNum(), + true); + checkContractDataExpirationLedger( + "temp", TEMPORARY, + stateExpirationSettings.minTempEntryExpiration + ledgerSeq - 1); + + // Close ledgers until PERSISTENT entry expires + for (; currLedger <= expectedPersistentExpiration + 1; ++currLedger) + { + closeLedgerOn(*app, currLedger, 2, 1, 2016); + } + + REQUIRE(app->getLedgerManager().getLastClosedLedgerNum() == + expectedPersistentExpiration + 1); + checkContractDataExpirationState( + "unique", ContractDataDurability::PERSISTENT, + app->getLedgerManager().getLastClosedLedgerNum(), false); + + // Check that we can't recreate expired PERSISTENT + putWithFootprint( + "unique", 0, contractKeys, + {contractDataKey(contractID, makeSymbolSCVal("unique"), + ContractDataDurability::PERSISTENT, DATA_ENTRY)}, + 1000, /*expectSuccess*/ false, ContractDataDurability::PERSISTENT); } + // Autobump is disabled by default, enable it for some tests + auto enableAutobump = [&]() { + auto autobumpAmount = 10; + LedgerTxn ltx(app->getLedgerTxnRoot()); + auto networkConfig = + app->getLedgerManager().getSorobanNetworkConfig(ltx); + auto newStateExpirationSettings = stateExpirationSettings; + newStateExpirationSettings.autoBumpLedgers = autobumpAmount; + networkConfig.stateExpirationSettings() = newStateExpirationSettings; + app->getLedgerManager().setSorobanNetworkConfig(networkConfig); + return autobumpAmount; + }; + SECTION("autobump") { + auto autobump = enableAutobump(); + put("rw", 0, ContractDataDurability::PERSISTENT); put("ro", 0, ContractDataDurability::PERSISTENT); @@ -932,53 +1158,47 @@ TEST_CASE("contract storage", "[tx][soroban]") stateExpirationSettings.minPersistentEntryExpiration + ledgerSeq - 1; - REQUIRE(getContractDataExpiration("rw", - ContractDataDurability::PERSISTENT) == - expectedInitialExpiration + autoBump); - REQUIRE(getContractDataExpiration("ro", - ContractDataDurability::PERSISTENT) == - expectedInitialExpiration + autoBump); + checkContractDataExpirationLedger("rw", + ContractDataDurability::PERSISTENT, + expectedInitialExpiration + autobump); + checkContractDataExpirationLedger("ro", + ContractDataDurability::PERSISTENT, + expectedInitialExpiration + autobump); // Contract instance and Wasm should have minimum life and 3 invocations // worth of autobumps - LedgerTxn ltx(app->getLedgerTxnRoot()); for (auto const& key : contractKeys) { - uint32_t mult = key.type() == CONTRACT_CODE ? 4 : 3; - auto ltxe = ltx.loadWithoutRecord(key, /*loadExpiredEntry=*/false); - REQUIRE(ltxe); - REQUIRE(getExpirationLedger(ltxe.current()) == - expectedInitialExpiration + (autoBump * mult)); + checkKeyExpirationLedger( + key, ledgerSeq, expectedInitialExpiration + (autobump * 3)); } } SECTION("manual bump") { + // Large bump, followed by smaller bump put("key", 0, ContractDataDurability::PERSISTENT); bump("key", ContractDataDurability::PERSISTENT, 10'000); - REQUIRE(getContractDataExpiration("key", - ContractDataDurability::PERSISTENT) == - ledgerSeq + 10'000 - 1); + checkContractDataExpirationLedger( + "key", ContractDataDurability::PERSISTENT, ledgerSeq + 10'000 - 1); - // Expiration already above 5'000, should be a nop (other than autobump) + // Expiration already above 5'000, should be a no-op bump("key", ContractDataDurability::PERSISTENT, 5'000); - REQUIRE(getContractDataExpiration("key", - ContractDataDurability::PERSISTENT) == - ledgerSeq + 10'000 - 1 + autoBump); + checkContractDataExpirationLedger( + "key", ContractDataDurability::PERSISTENT, ledgerSeq + 10'000 - 1); + // Small bump followed by larger bump put("key2", 0, ContractDataDurability::PERSISTENT); bump("key2", ContractDataDurability::PERSISTENT, 5'000); - REQUIRE(getContractDataExpiration("key2", - ContractDataDurability::PERSISTENT) == - ledgerSeq + 5'000 - 1); + checkContractDataExpirationLedger( + "key2", ContractDataDurability::PERSISTENT, ledgerSeq + 5'000 - 1); put("key3", 0, ContractDataDurability::PERSISTENT); bump("key3", ContractDataDurability::PERSISTENT, 50'000); - REQUIRE(getContractDataExpiration("key3", - ContractDataDurability::PERSISTENT) == - ledgerSeq + 50'000 - 1); + checkContractDataExpirationLedger( + "key3", ContractDataDurability::PERSISTENT, ledgerSeq + 50'000 - 1); - // Bump to live 10100 ledger from now + // Bump multiple keys to live 10100 ledger from now bumpOp( 10100, {contractDataKey(contractID, makeSymbolSCVal("key"), @@ -989,71 +1209,93 @@ TEST_CASE("contract storage", "[tx][soroban]") ContractDataDurability::PERSISTENT, DATA_ENTRY)}, 178); - REQUIRE(getContractDataExpiration("key", - ContractDataDurability::PERSISTENT) == - ledgerSeq + 10'100); - REQUIRE(getContractDataExpiration("key2", - ContractDataDurability::PERSISTENT) == - ledgerSeq + 10'100); + checkContractDataExpirationLedger( + "key", ContractDataDurability::PERSISTENT, ledgerSeq + 10'100); + checkContractDataExpirationLedger( + "key2", ContractDataDurability::PERSISTENT, ledgerSeq + 10'100); // No change for key3 since expiration is already past 10100 ledgers // from now - REQUIRE(getContractDataExpiration("key3", - ContractDataDurability::PERSISTENT) == - ledgerSeq + 50'000 - 1); + checkContractDataExpirationLedger( + "key3", ContractDataDurability::PERSISTENT, ledgerSeq + 50'000 - 1); } + SECTION("restore expired entry") { - auto minBump = - InitialSorobanNetworkConfig::MINIMUM_PERSISTENT_ENTRY_LIFETIME; + uint32_t initExpirationLedger = + stateExpirationSettings.minPersistentEntryExpiration + ledgerSeq - + 1; + + // Bump instance and WASM so that they don't expire during the test + bumpOp(10'000, contractKeys, 1744); + put("key", 0, ContractDataDurability::PERSISTENT); - uint32_t initExpirationLedger = ledgerSeq + minBump - 1; - REQUIRE(getContractDataExpiration("key", - ContractDataDurability::PERSISTENT) == - initExpirationLedger); + checkContractDataExpirationLedger( + "key", ContractDataDurability::PERSISTENT, initExpirationLedger); + + // Crank until entry expires for (uint32 i = app->getLedgerManager().getLastClosedLedgerNum(); i <= initExpirationLedger + 1; ++i) { closeLedgerOn(*app, i, 2, 1, 2016); } + REQUIRE(app->getLedgerManager().getLastClosedLedgerNum() == initExpirationLedger + 1); - REQUIRE(!isEntryLive("key", ContractDataDurability::PERSISTENT, - app->getLedgerManager().getLastClosedLedgerNum())); - // Trying to use this expired entry should fail. - putWithFootprint( - "key", 0, contractKeys, - {contractDataKey(contractID, makeSymbolSCVal("key"), - ContractDataDurability::PERSISTENT, DATA_ENTRY)}, - 1000, /*expectSuccess*/ false, ContractDataDurability::PERSISTENT); + checkContractDataExpirationState( + "key", ContractDataDurability::PERSISTENT, + app->getLedgerManager().getLastClosedLedgerNum(), false); + + auto lk = + contractDataKey(contractID, makeSymbolSCVal("key"), + ContractDataDurability::PERSISTENT, DATA_ENTRY); + auto roKeys = contractKeys; + roKeys.emplace_back(lk); + + // Trying to read this expired entry should fail. + getWithFootprint("key", roKeys, {}, 1000, /*expectSuccess*/ false, + ContractDataDurability::PERSISTENT); + + // Recreation of persistent entries should fail. + putWithFootprint("key", 0, contractKeys, {lk}, 1000, + /*expectSuccess*/ false, + ContractDataDurability::PERSISTENT); + + // Bump operation should skip expired entries + bumpOp(1'000, {lk}, 0); + + // Make sure expirationLedger is unchanged by bumpOp + checkContractDataExpirationLedger( + "key", ContractDataDurability::PERSISTENT, initExpirationLedger); + + // Restore the entry + restoreOp({lk}, 61); + + ledgerSeq = getLedgerSeq(*app); + checkContractDataExpirationState( + "key", ContractDataDurability::PERSISTENT, ledgerSeq, true); - // Restore the entry and then write to it. - restoreOp( - {contractDataKey(contractID, makeSymbolSCVal("key"), - ContractDataDurability::PERSISTENT, DATA_ENTRY)}, - 61); - REQUIRE( - isEntryLive("key", ContractDataDurability::PERSISTENT, - app->getLedgerManager().getLastClosedLedgerNum() + 1)); // Entry is considered to be restored on lclNum (as we didn't close an // additional ledger). - REQUIRE(getContractDataExpiration("key", - ContractDataDurability::PERSISTENT) == - app->getLedgerManager().getLastClosedLedgerNum() + minBump - 1); + checkContractDataExpirationLedger( + "key", ContractDataDurability::PERSISTENT, + ledgerSeq + stateExpirationSettings.minPersistentEntryExpiration - + 1); + // Write to entry to check that is is live put("key", 1, ContractDataDurability::PERSISTENT); } + SECTION("re-create expired temporary entry") { auto minBump = InitialSorobanNetworkConfig::MINIMUM_TEMP_ENTRY_LIFETIME; put("key", 0, ContractDataDurability::TEMPORARY); REQUIRE(has("key", ContractDataDurability::TEMPORARY)); - uint32_t initExpirationLedger = ledgerSeq + minBump + autoBump - 1; - REQUIRE(getContractDataExpiration("key", - ContractDataDurability::TEMPORARY) == - initExpirationLedger); + uint32_t initExpirationLedger = ledgerSeq + minBump - 1; + checkContractDataExpirationLedger( + "key", ContractDataDurability::TEMPORARY, initExpirationLedger); for (size_t i = app->getLedgerManager().getLastClosedLedgerNum(); i <= initExpirationLedger + 1; ++i) @@ -1062,8 +1304,9 @@ TEST_CASE("contract storage", "[tx][soroban]") } REQUIRE(app->getLedgerManager().getLastClosedLedgerNum() == initExpirationLedger + 1); - REQUIRE(!isEntryLive("key", ContractDataDurability::TEMPORARY, - app->getLedgerManager().getLastClosedLedgerNum())); + checkContractDataExpirationState( + "key", ContractDataDurability::TEMPORARY, + app->getLedgerManager().getLastClosedLedgerNum(), false); // Entry has expired REQUIRE(!has("key", ContractDataDurability::TEMPORARY)); @@ -1073,38 +1316,37 @@ TEST_CASE("contract storage", "[tx][soroban]") REQUIRE(has("key", ContractDataDurability::TEMPORARY)); uint32_t newExpirationLedger = - app->getLedgerManager().getLastClosedLedgerNum() + minBump + - autoBump - 1; - REQUIRE(getContractDataExpiration("key", - ContractDataDurability::TEMPORARY) == - newExpirationLedger); + app->getLedgerManager().getLastClosedLedgerNum() + minBump - 1; + checkContractDataExpirationLedger( + "key", ContractDataDurability::TEMPORARY, newExpirationLedger); } + SECTION("max expiration") { + auto autobump = enableAutobump(); + REQUIRE(autobump > 1); + // Check that manual bump doesn't go over max put("key", 0, ContractDataDurability::PERSISTENT); bump("key", ContractDataDurability::PERSISTENT, UINT32_MAX); auto maxExpiration = ledgerSeq + stateExpirationSettings.maxEntryExpiration - 1; - REQUIRE(getContractDataExpiration("key", - ContractDataDurability::PERSISTENT) == - maxExpiration); + checkContractDataExpirationLedger( + "key", ContractDataDurability::PERSISTENT, maxExpiration); // Manual bump to almost max, then autobump to check that autobump // doesn't go over max put("key2", 0, ContractDataDurability::PERSISTENT); bump("key2", ContractDataDurability::PERSISTENT, stateExpirationSettings.maxEntryExpiration - 1); - REQUIRE(getContractDataExpiration("key2", - ContractDataDurability::PERSISTENT) == - maxExpiration - 1); + checkContractDataExpirationLedger( + "key2", ContractDataDurability::PERSISTENT, maxExpiration - 1); // Autobump should only add a single ledger to bring expiration to max put("key2", 1, ContractDataDurability::PERSISTENT); - REQUIRE(getContractDataExpiration("key2", - ContractDataDurability::PERSISTENT) == - maxExpiration); + checkContractDataExpirationLedger( + "key2", ContractDataDurability::PERSISTENT, maxExpiration); } }