From 0a599d757cccd550611fd446c9c71955bbc39582 Mon Sep 17 00:00:00 2001 From: Garand Tyson Date: Mon, 3 Feb 2025 18:37:55 -0800 Subject: [PATCH] Fixed intra ledger resrtore bug in p23 --- .../InvokeHostFunctionOpFrame.cpp | 10 +++--- .../test/InvokeHostFunctionTests.cpp | 31 +++++++++++++++++++ .../InvokeHostFunctionTests.json | 6 ++-- 3 files changed, 40 insertions(+), 7 deletions(-) diff --git a/src/transactions/InvokeHostFunctionOpFrame.cpp b/src/transactions/InvokeHostFunctionOpFrame.cpp index 7de739ea74..9c5a35d543 100644 --- a/src/transactions/InvokeHostFunctionOpFrame.cpp +++ b/src/transactions/InvokeHostFunctionOpFrame.cpp @@ -408,11 +408,11 @@ InvokeHostFunctionOpFrame::doApply( // If ttlLtxe doesn't exist, this is a new Soroban entry // Starting in protocol 23, we must check the Hot Archive for // new keys. If a new key is actually archived, fail the op. - if (isPersistentEntry(lk) && - protocolVersionStartsFrom( - ltx.getHeader().ledgerVersion, - HotArchiveBucket:: - FIRST_PROTOCOL_SUPPORTING_PERSISTENT_EVICTION)) + else if (isPersistentEntry(lk) && + protocolVersionStartsFrom( + ltx.getHeader().ledgerVersion, + HotArchiveBucket:: + FIRST_PROTOCOL_SUPPORTING_PERSISTENT_EVICTION)) { auto archiveEntry = hotArchive->load(lk); if (archiveEntry) diff --git a/src/transactions/test/InvokeHostFunctionTests.cpp b/src/transactions/test/InvokeHostFunctionTests.cpp index 58ed5998b8..992ba204cb 100644 --- a/src/transactions/test/InvokeHostFunctionTests.cpp +++ b/src/transactions/test/InvokeHostFunctionTests.cpp @@ -3124,6 +3124,37 @@ TEST_CASE("persistent entry archival", "[tx][soroban][archival]") // Restored entries are deleted from Hot Archive REQUIRE(!hotArchive->load(lk)); } + + SECTION("key accessible after restore in same ledger") + { + auto writeSrcAccount = test.getRoot().create("src", 500000000); + + SorobanResources restoreResources; + restoreResources.footprint.readWrite = {lk}; + restoreResources.instructions = 0; + restoreResources.readBytes = 10'000; + restoreResources.writeBytes = 10'000; + + auto resourceFee = 300'000 + 40'000; + auto restoreTx = + test.createRestoreTx(restoreResources, 1'000, resourceFee); + + auto writeInvocation = client.getContract().prepareInvocation( + "put_persistent", {makeSymbolSCVal("key"), makeU64SCVal(200)}, + client.writeKeySpec("key", ContractDataDurability::PERSISTENT)); + + auto writeTx = + writeInvocation.withExactNonRefundableResourceFee().createTx( + &writeSrcAccount); + + closeLedger(test.getApp(), {restoreTx, writeTx}, + /*strictOrder=*/true); + + // Restore should succeed and entry value should be updated since + // writeTx comes after restore + REQUIRE(test.isEntryLive(lk, test.getLCLSeq())); + client.get("key", ContractDataDurability::PERSISTENT, 200); + } }; SECTION("eviction") diff --git a/test-tx-meta-baseline-next/InvokeHostFunctionTests.json b/test-tx-meta-baseline-next/InvokeHostFunctionTests.json index 2548c2d5c3..b4f5c05d9b 100644 --- a/test-tx-meta-baseline-next/InvokeHostFunctionTests.json +++ b/test-tx-meta-baseline-next/InvokeHostFunctionTests.json @@ -1344,8 +1344,10 @@ "bKDF6V5IzTo=", "bKDF6V5IzTo=" ], - "persistent entry archival|eviction" : [ "bKDF6V5IzTo=", "bKDF6V5IzTo=" ], - "persistent entry archival|expiration without eviction" : [ "bKDF6V5IzTo=", "bKDF6V5IzTo=" ], + "persistent entry archival|eviction" : [ "bKDF6V5IzTo=", "bKDF6V5IzTo=", "bKDF6V5IzTo=" ], + "persistent entry archival|eviction|key accessible after restore in same ledger" : [ "iLiZOFVcRzA=" ], + "persistent entry archival|expiration without eviction" : [ "bKDF6V5IzTo=", "bKDF6V5IzTo=", "bKDF6V5IzTo=" ], + "persistent entry archival|expiration without eviction|key accessible after restore in same ledger" : [ "iLiZOFVcRzA=" ], "refund account merged" : [ "bKDF6V5IzTo=", "398Rd+u+jdE=", "b4KxXJCxvM0=", "7/h1q7KjqKA=" ], "refund is sent to fee-bump source|protocol version 20" : [ "bKDF6V5IzTo=", "JRiUwIP1h2Q=", "LyDINL0VaLk=" ], "refund is sent to fee-bump source|protocol version 21" : [ "bKDF6V5IzTo=", "JRiUwIP1h2Q=", "LyDINL0VaLk=" ],