add snapshot transaction cloning, change snapshot based worldstate to…

… extend persisted worldstate rather than in-memory worldstate

Signed-off-by: garyschulte <garyschulte@gmail.com>

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/BonsaiSnapshotWorldState.java

@@ -26,7 +26,7 @@
  * creation and/or point-in-time queries since the snapshot worldstate is fully isolated from the
  * main BonsaiPersistedWorldState.
  */
-public class BonsaiSnapshotWorldState extends BonsaiInMemoryWorldState
+public class BonsaiSnapshotWorldState extends BonsaiPersistedWorldState
     implements SnapshotMutableWorldState {
   //  private static final Logger LOG = LoggerFactory.getLogger(BonsaiSnapshotWorldState.class);
 
@@ -60,18 +60,20 @@ public static BonsaiSnapshotWorldState create(
 
   @Override
   public MutableWorldState copy() {
-    // The copy method interface doesn't make much sense in this context since
-    // this class is a snapshot based copy of the worldstate and *is* an
-    // independent/isolated version of the worldstate, so just return ourselves.
-    return this;
+    // return a clone-based copy of worldstate storage
+    return new BonsaiSnapshotWorldState(
+        archive,
+        new BonsaiSnapshotWorldStateKeyValueStorage(
+            accountSnap.cloneFromSnapshot(),
+            codeSnap.cloneFromSnapshot(),
+            storageSnap.cloneFromSnapshot(),
+            trieBranchSnap.cloneFromSnapshot(),
+            worldStateStorage.trieLogStorage));
   }
 
   @Override
   public void close() throws Exception {
-    // TODO: close and free snapshot transactions
-    this.accountSnap.getSnapshotTransaction().rollback();
-    this.codeSnap.getSnapshotTransaction().rollback();
-    this.storageSnap.getSnapshotTransaction().rollback();
-    this.trieBranchSnap.getSnapshotTransaction().rollback();
+    // TODO: consider releasing snapshot or marking for release
+    // no-op.
   }
 }

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/BonsaiSnapshotWorldStateKeyValueStorage.java

@@ -26,7 +26,6 @@
 import org.apache.tuweni.bytes.Bytes32;
 
 public class BonsaiSnapshotWorldStateKeyValueStorage extends BonsaiWorldStateKeyValueStorage {
-  private final SnapshotUpdater updater;
 
   public BonsaiSnapshotWorldStateKeyValueStorage(
       final SnappedKeyValueStorage accountStorage,
@@ -35,14 +34,16 @@ public BonsaiSnapshotWorldStateKeyValueStorage(
       final SnappedKeyValueStorage trieBranchStorage,
       final KeyValueStorage trieLogStorage) {
     super(accountStorage, codeStorage, storageStorage, trieBranchStorage, trieLogStorage);
-    this.updater =
-        new SnapshotUpdater(
-            accountStorage, codeStorage, storageStorage, trieBranchStorage, trieLogStorage);
   }
 
   @Override
   public BonsaiUpdater updater() {
-    return updater;
+    return new SnapshotUpdater(
+        (SnappedKeyValueStorage) accountStorage,
+        (SnappedKeyValueStorage) codeStorage,
+        (SnappedKeyValueStorage) storageStorage,
+        (SnappedKeyValueStorage) trieBranchStorage,
+        trieLogStorage);
   }
 
   public static class SnapshotUpdater implements BonsaiWorldStateKeyValueStorage.BonsaiUpdater {

ethereum/core/src/test/java/org/hyperledger/besu/ethereum/bonsai/BonsaiSnapshotIsolationTests.java

@@ -179,6 +179,76 @@ public void testIsolatedSnapshotMutation() {
     assertThat(ws.get().rootHash()).isEqualTo(firstBlock.getHeader().getStateRoot());
   }
 
+  @Test
+  public void testSnapshotCloneIsolation() {
+    Address testAddress = Address.fromHexString("0xdeadbeef");
+    Address altTestAddress = Address.fromHexString("0xd1ffbeef");
+
+    // create a snapshot worldstate, and then clone it:
+    var isolated = archive.getMutableSnapshot(genesisState.getBlock().getHash()).get();
+    var isolatedClone =
+        archive.getMutableSnapshot(genesisState.getBlock().getHash()).get(); // isolated.copy();
+
+    // execute a block with a single transaction on the first snapshot:
+    var firstBlock = forTransactions(List.of(burnTransaction(sender1, 0L, testAddress)));
+    var res = executeBlock(isolated, firstBlock);
+    isolated.persist(firstBlock.getHeader());
+
+    assertThat(res.isSuccessful()).isTrue();
+    Runnable checkIsolatedState =
+        () -> {
+          assertThat(isolated.rootHash()).isEqualTo(firstBlock.getHeader().getStateRoot());
+          assertThat(isolated.get(testAddress)).isNotNull();
+          assertThat(isolated.get(altTestAddress)).isNull();
+          assertThat(isolated.get(testAddress).getBalance())
+              .isEqualTo(Wei.of(1_000_000_000_000_000_000L));
+        };
+    checkIsolatedState.run();
+
+    // assert clone is isolated and unmodified:
+    assertThat(isolatedClone.get(testAddress)).isNull();
+    assertThat(isolatedClone.rootHash())
+        .isEqualTo(genesisState.getBlock().getHeader().getStateRoot());
+
+    // assert clone isolated block execution
+    var cloneForkBlock = forTransactions(List.of(burnTransaction(sender1, 0L, altTestAddress)));
+    var altRes = executeBlock(isolatedClone, cloneForkBlock);
+    isolatedClone.persist(cloneForkBlock.getHeader());
+
+    assertThat(altRes.isSuccessful()).isTrue();
+    assertThat(isolatedClone.rootHash()).isEqualTo(cloneForkBlock.getHeader().getStateRoot());
+    assertThat(isolatedClone.get(altTestAddress)).isNotNull();
+    assertThat(isolatedClone.get(testAddress)).isNull();
+    assertThat(isolatedClone.get(altTestAddress).getBalance())
+        .isEqualTo(Wei.of(1_000_000_000_000_000_000L));
+    assertThat(isolatedClone.rootHash()).isEqualTo(cloneForkBlock.getHeader().getStateRoot());
+
+    // re-check isolated state remains unchanged:
+    checkIsolatedState.run();
+
+    // assert that the actual persisted worldstate remains unchanged:
+    var persistedWorldState = archive.getMutable();
+    assertThat(persistedWorldState.rootHash())
+        .isEqualTo(genesisState.getBlock().getHeader().getStateRoot());
+    assertThat(persistedWorldState.get(testAddress)).isNull();
+    assertThat(persistedWorldState.get(altTestAddress)).isNull();
+
+    // assert that trieloglayers exist for both of the isolated states:
+    var firstBlockTrieLog = archive.getTrieLogManager().getTrieLogLayer(firstBlock.getHash());
+    assertThat(firstBlockTrieLog).isNotEmpty();
+    assertThat(firstBlockTrieLog.get().getAccount(testAddress)).isNotEmpty();
+    assertThat(firstBlockTrieLog.get().getAccount(altTestAddress)).isEmpty();
+    var cloneForkTrieLog = archive.getTrieLogManager().getTrieLogLayer(cloneForkBlock.getHash());
+    assertThat(cloneForkTrieLog.get().getAccount(testAddress)).isEmpty();
+    assertThat(cloneForkTrieLog.get().getAccount(altTestAddress)).isNotEmpty();
+  }
+
+  @Test
+  public void assertSnapshotDoesNotClose() {
+    // TODO: add unit test to assert snapshot does not close on clone if parent tx is closed
+
+  }
+
   @Test
   public void testSnapshotRollToTrieLogBlockHash() {
     // assert we can roll a snapshot to a specific worldstate without mutating head
@@ -232,12 +302,13 @@ public void assertCloseDisposesOfStateWithoutCommitting() {
     try (var shouldCloseSnapshot =
         archive.getMutableSnapshot(genesisState.getBlock().getHash()).get()) {
 
-      var tx1 = burnTransaction(sender1, 0L, Address.ZERO);
+      var tx1 = burnTransaction(sender1, 0L, testAddress);
       Block oneTx = forTransactions(List.of(tx1));
 
       var res = executeBlock(shouldCloseSnapshot, oneTx);
-
       assertThat(res.isSuccessful()).isTrue();
+      shouldCloseSnapshot.persist(oneTx.getHeader());
+
       assertThat(shouldCloseSnapshot.get(testAddress)).isNotNull();
       assertThat(shouldCloseSnapshot.get(testAddress).getBalance())
           .isEqualTo(Wei.of(1_000_000_000_000_000_000L));

plugin-api/build.gradle

@@ -65,7 +65,7 @@ Calculated : ${currentHash}
 tasks.register('checkAPIChanges', FileStateChecker) {
   description = "Checks that the API for the Plugin-API project does not change without deliberate thought"
   files = sourceSets.main.allJava.files
-  knownHash = 'XC8yUiSOLc+dKdIFk8l9aEsc6w3EqedptcH53kvfag4='
+  knownHash = 't1ECxSKuhCHrMq9uKYC9datxfFqqTpCPc6GFmUfC8Pg='
 }
 check.dependsOn('checkAPIChanges')
 

plugin-api/src/main/java/org/hyperledger/besu/plugin/services/storage/SnappedKeyValueStorage.java

@@ -18,4 +18,6 @@
 public interface SnappedKeyValueStorage extends KeyValueStorage {
 
   KeyValueStorageTransaction getSnapshotTransaction();
+
+  SnappedKeyValueStorage cloneFromSnapshot();
 }

plugins/rocksdb/src/main/java/org/hyperledger/besu/plugin/services/storage/rocksdb/segmented/RocksDBColumnarKeyValueSnapshot.java

@@ -43,6 +43,12 @@ public class RocksDBColumnarKeyValueSnapshot implements SnappedKeyValueStorage {
     this.snapTx = new RocksDBSnapshotTransaction(db, segment.get(), metrics);
   }
 
+  private RocksDBColumnarKeyValueSnapshot(
+      final OptimisticTransactionDB db, final RocksDBSnapshotTransaction snapTx) {
+    this.db = db;
+    this.snapTx = snapTx;
+  }
+
   @Override
   public Optional<byte[]> get(final byte[] key) throws StorageException {
     return snapTx.get(key);
@@ -91,4 +97,9 @@ public void close() throws IOException {
   public KeyValueStorageTransaction getSnapshotTransaction() {
     return snapTx;
   }
+
+  @Override
+  public SnappedKeyValueStorage cloneFromSnapshot() {
+    return new RocksDBColumnarKeyValueSnapshot(db, snapTx.copy());
+  }
 }

plugins/rocksdb/src/main/java/org/hyperledger/besu/plugin/services/storage/rocksdb/segmented/RocksDBSnapshotTransaction.java

@@ -60,6 +60,20 @@ public class RocksDBSnapshotTransaction implements KeyValueStorageTransaction, A
     this.readOptions = new ReadOptions().setSnapshot(snapshot);
   }
 
+  private RocksDBSnapshotTransaction(
+      final OptimisticTransactionDB db,
+      final ColumnFamilyHandle columnFamilyHandle,
+      final RocksDBMetrics metrics,
+      final Snapshot snapshot) {
+    this.metrics = metrics;
+    this.db = db;
+    this.columnFamilyHandle = columnFamilyHandle;
+    this.snapshot = snapshot;
+    this.writeOptions = new WriteOptions();
+    this.snapTx = db.beginTransaction(writeOptions);
+    this.readOptions = new ReadOptions().setSnapshot(snapshot);
+  }
+
   public Optional<byte[]> get(final byte[] key) {
     try (final OperationTimer.TimingContext ignored = metrics.getReadLatency().startTimer()) {
       return Optional.ofNullable(snapTx.get(columnFamilyHandle, readOptions, key));
@@ -122,8 +136,16 @@ public void rollback() {
     }
   }
 
+  public RocksDBSnapshotTransaction copy() {
+    // TODO: if we use snapshot as the basis of a cloned state, we need to ensure close() of this
+    // Transaction
+    // does not release and close the snapshot in use by the cloned state.
+    return new RocksDBSnapshotTransaction(db, columnFamilyHandle, metrics, snapshot);
+  }
+
   @Override
   public void close() {
+    db.releaseSnapshot(snapshot);
     snapshot.close();
     snapTx.close();
     writeOptions.close();