From a4ffec24bc5574e48277eb0554511ece3c56f35b Mon Sep 17 00:00:00 2001
From: Danno Ferrin <danno.ferrin@gmail.com>
Date: Fri, 26 Feb 2021 08:25:02 -0700
Subject: [PATCH] Bonsai Tries: Respect account deletion across transacitons
 (#1952)

When in bonsai mode, if inside one block an account is deleted via self
destruct and then the account is re-created the latter sstore operation
reports the last SSTORE value prior to self destruct. Thus if the new
write is the same as the value before delete a non-updating gas cost
will mistakenly be applied.  Thus we first check accountWasDeleted when
reading original slot values.

Found in mainnet block 11905274 for account
0x7f150bd6f54c40a34d7c3d5e9f56.

Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
---
 .../hyperledger/besu/ethereum/bonsai/BonsaiValue.java |  5 +++++
 .../besu/ethereum/bonsai/BonsaiWorldStateArchive.java |  3 +++
 .../besu/ethereum/bonsai/BonsaiWorldStateUpdater.java | 11 ++++++-----
 3 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/BonsaiValue.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/BonsaiValue.java
index 91dd4ac072..cc1e885051 100644
--- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/BonsaiValue.java
+++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/BonsaiValue.java
@@ -68,4 +68,9 @@ public class BonsaiValue<T> {
   boolean isUnchanged() {
     return Objects.equals(updated, original);
   }
+
+  @Override
+  public String toString() {
+    return "BonsaiValue{" + "original=" + original + ", updated=" + updated + '}';
+  }
 }
diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/BonsaiWorldStateArchive.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/BonsaiWorldStateArchive.java
index d45b2375ef..8429e2c4a2 100644
--- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/BonsaiWorldStateArchive.java
+++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/BonsaiWorldStateArchive.java
@@ -129,13 +129,16 @@ public class BonsaiWorldStateArchive implements WorldStateArchive {
             (BonsaiWorldStateUpdater) persistedState.updater();
         try {
           for (final TrieLogLayer rollBack : rollBacks) {
+            LOG.debug("Attempting Rollback of {}", rollBack.getBlockHash());
             bonsaiUpdater.rollBack(rollBack);
           }
           for (int i = rollForwards.size() - 1; i >= 0; i--) {
+            LOG.debug("Attempting Rollforward of {}", rollForwards.get(i).getBlockHash());
             bonsaiUpdater.rollForward(rollForwards.get(i));
           }
           bonsaiUpdater.commit();
           persistedState.persist(blockchain.getBlockHeader(blockHash).get());
+          LOG.debug("Archive rolling finished, now at {}", blockHash);
           return Optional.of(persistedState);
         } catch (final Exception e) {
           // if we fail we must clean up the updater
diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/BonsaiWorldStateUpdater.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/BonsaiWorldStateUpdater.java
index 31f5229ee4..03d7764ab4 100644
--- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/BonsaiWorldStateUpdater.java
+++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/bonsai/BonsaiWorldStateUpdater.java
@@ -154,9 +154,7 @@ public class BonsaiWorldStateUpdater extends AbstractWorldUpdater<BonsaiWorldVie
           accountsToUpdate.computeIfAbsent(
               deletedAddress,
               __ -> loadAccountFromParent(deletedAddress, new BonsaiValue<>(null, null)));
-      if (accountValue.getOriginal() != null) {
-        storageToClear.add(deletedAddress);
-      }
+      storageToClear.add(deletedAddress);
       final BonsaiValue<Bytes> codeValue = codeToUpdate.get(deletedAddress);
       if (codeValue != null) {
         codeValue.setUpdated(null);
@@ -271,7 +269,7 @@ public class BonsaiWorldStateUpdater extends AbstractWorldUpdater<BonsaiWorldVie
     if (localCode == null) {
       return wrappedWorldView().getCode(address);
     } else {
-      return Optional.of(localCode.getUpdated());
+      return Optional.ofNullable(localCode.getUpdated());
     }
   }
 
@@ -294,7 +292,7 @@ public class BonsaiWorldStateUpdater extends AbstractWorldUpdater<BonsaiWorldVie
         storageToUpdate.computeIfAbsent(address, key -> new HashMap<>());
     final BonsaiValue<UInt256> value = localAccountStorage.get(slotHash);
     if (value != null) {
-      return Optional.of(value.getUpdated());
+      return Optional.ofNullable(value.getUpdated());
     } else {
       final Optional<UInt256> valueUInt =
           wrappedWorldView().getStorageValueBySlotHash(address, slotHash);
@@ -306,6 +304,9 @@ public class BonsaiWorldStateUpdater extends AbstractWorldUpdater<BonsaiWorldVie
   @Override
   public UInt256 getOriginalStorageValue(final Address address, final UInt256 storageKey) {
     // TODO maybe log the read into the trie layer?
+    if (storageToClear.contains(address)) {
+      return UInt256.ZERO;
+    }
     final Map<Hash, BonsaiValue<UInt256>> localAccountStorage =
         storageToUpdate.computeIfAbsent(address, key -> new HashMap<>());
     final Hash slotHashBytes = Hash.hash(storageKey.toBytes());