Fix some problems with nonces and accounts with GoQuorum privacy (#1922)

* fix some problems with nonces and accounts

Signed-off-by: Stefan Pingel <stefan.pingel@consensys.net>

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/UpdateTrackingAccount.java

@@ -36,7 +36,7 @@ import org.apache.tuweni.units.bigints.UInt256;
  * the underlying trie node will have to be updated, and so knowing if the nonce and balance where
  * updated or not doesn't matter, we just need their new value).
  */
-public class UpdateTrackingAccount<A extends Account> implements MutableAccount {
+public class UpdateTrackingAccount<A extends Account> implements MutableAccount, EvmAccount {
   private final Address address;
   private final Hash addressHash;
 
@@ -273,4 +273,9 @@ public class UpdateTrackingAccount<A extends Account> implements MutableAccount
         "%s -> {nonce: %s, balance:%s, code:%s, storage:%s }",
         address, nonce, balance, updatedCode == null ? "[not updated]" : updatedCode, storage);
   }
+
+  @Override
+  public MutableAccount getMutable() throws ModificationNotAllowedException {
+    return this;
+  }
 }

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/WorldUpdater.java

@@ -70,6 +70,17 @@ public interface WorldUpdater extends MutableWorldView {
     return account == null ? createAccount(address) : account;
   }
 
+  /**
+   * Retrieves the provided account for a sender of a transaction if it exists, or creates it if it
+   * doesn't.
+   *
+   * @param address the address of the account.
+   * @return the account of the sender for {@code address}
+   */
+  default EvmAccount getOrCreateSenderAccount(final Address address) {
+    return getOrCreate(address);
+  }
+
   /**
    * Retrieves the provided account, returning a modifiable object (whose updates are accumulated by
    * this updater).

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/goquorum/GoQuorumBlockProcessor.java

@@ -40,7 +40,7 @@ import org.hyperledger.besu.ethereum.processing.TransactionProcessingResult;
 import org.hyperledger.besu.ethereum.transaction.TransactionInvalidReason;
 import org.hyperledger.besu.ethereum.vm.BlockHashLookup;
 import org.hyperledger.besu.ethereum.vm.OperationTracer;
-import org.hyperledger.besu.ethereum.worldstate.DefaultMutablePrivateWorldStateUpdater;
+import org.hyperledger.besu.ethereum.worldstate.GoQuorumMutablePrivateWorldStateUpdater;
 
 import java.util.ArrayList;
 import java.util.Collections;
@@ -115,7 +115,7 @@ public class GoQuorumBlockProcessor extends MainnetBlockProcessor {
           effectiveTransaction = retrievePrivateTransactionFromEnclave(transaction);
 
           effectiveWorldUpdater =
-              new DefaultMutablePrivateWorldStateUpdater(
+              new GoQuorumMutablePrivateWorldStateUpdater(
                   publicWorldStateUpdater, privateWorldState.updater());
 
         } catch (final EnclaveClientException e) { // private transaction but not party to it
@@ -170,6 +170,7 @@ public class GoQuorumBlockProcessor extends MainnetBlockProcessor {
               .getOrCreate(effectiveTransaction.getSender())
               .getMutable()
               .incrementNonce();
+          effectiveWorldUpdater.commit();
         } else { // public transaction
           final long gasUsed = transaction.getGasLimit() - result.getGasRemaining();
           currentGasUsed += gasUsed;
@@ -178,6 +179,7 @@ public class GoQuorumBlockProcessor extends MainnetBlockProcessor {
               transactionReceiptFactory.create(
                   transaction.getType(), result, publicWorldState, currentGasUsed));
           privateTxReceipts.add(null);
+          effectiveWorldUpdater.commit();
         }
       } else { // private transaction we are not party to
         publicTxReceipts.add(
@@ -187,13 +189,9 @@ public class GoQuorumBlockProcessor extends MainnetBlockProcessor {
                 publicWorldState,
                 currentGasUsed));
         privateTxReceipts.add(null);
-        publicWorldStateUpdater
+        publicWorldStateUpdater.getOrCreate(transaction.getSender()).getMutable().incrementNonce();
-            .getOrCreate(effectiveTransaction.getSender())
-            .getMutable()
-            .incrementNonce();
       }
 
-      effectiveWorldUpdater.commit();
       publicWorldStateUpdater.commit();
     }
 

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetContractCreationProcessor.java

@@ -111,7 +111,7 @@ public class MainnetContractCreationProcessor extends AbstractMessageProcessor {
     }
     try {
       final MutableAccount sender =
-          frame.getWorldState().getAccount(frame.getSenderAddress()).getMutable();
+          frame.getWorldState().getOrCreateSenderAccount(frame.getSenderAddress()).getMutable();
       sender.decrementBalance(frame.getValue());
 
       final MutableAccount contract =

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetMessageCallProcessor.java

@@ -86,7 +86,7 @@ public class MainnetMessageCallProcessor extends AbstractMessageProcessor {
    */
   private void transferValue(final MessageFrame frame) {
     final MutableAccount senderAccount =
-        frame.getWorldState().getAccount(frame.getSenderAddress()).getMutable();
+        frame.getWorldState().getOrCreateSenderAccount(frame.getSenderAddress()).getMutable();
     // The yellow paper explicitly states that if the recipient account doesn't exist at this
     // point, it is created.
     final MutableAccount recipientAccount =

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetTransactionProcessor.java

@@ -37,6 +37,7 @@ import org.hyperledger.besu.ethereum.vm.GasCalculator;
 import org.hyperledger.besu.ethereum.vm.MessageFrame;
 import org.hyperledger.besu.ethereum.vm.OperationTracer;
 import org.hyperledger.besu.ethereum.vm.operations.ReturnStack;
+import org.hyperledger.besu.ethereum.worldstate.GoQuorumMutablePrivateWorldStateUpdater;
 import org.hyperledger.besu.plugin.data.TransactionType;
 
 import java.util.ArrayDeque;
@@ -266,7 +267,8 @@ public class MainnetTransactionProcessor {
 
       final Address senderAddress = transaction.getSender();
 
-      final EvmAccount sender = worldState.getOrCreate(senderAddress);
+      final EvmAccount sender = worldState.getOrCreateSenderAccount(senderAddress);
+
       validationResult =
           transactionValidator.validateForSender(transaction, sender, transactionValidationParams);
       if (!validationResult.isValid()) {
@@ -332,7 +334,7 @@ public class MainnetTransactionProcessor {
       final MessageFrame initialFrame;
       if (transaction.isContractCreation()) {
         final Address contractAddress =
-            Address.contractAddress(senderAddress, sender.getNonce() - 1L);
+            Address.contractAddress(senderAddress, senderMutableAccount.getNonce() - 1L);
 
         initialFrame =
             commonMessageFrameBuilder
@@ -387,6 +389,8 @@ public class MainnetTransactionProcessor {
       final Gas gasUsedByTransaction =
           Gas.of(transaction.getGasLimit()).minus(initialFrame.getRemainingGas());
 
+      if (!worldState.getClass().equals(GoQuorumMutablePrivateWorldStateUpdater.class)) {
+        // if this is not a private GoQuorum transaction we have to update the coinbase
         final MutableAccount coinbase = worldState.getOrCreate(miningBeneficiary).getMutable();
         final Gas coinbaseFee = Gas.of(transaction.getGasLimit()).minus(refunded);
         if (blockHeader.getBaseFee().isPresent()) {
@@ -409,6 +413,7 @@ public class MainnetTransactionProcessor {
             coinbaseCreditService.price(coinbaseFee, transactionGasPrice, blockHeader.getBaseFee());
 
         coinbase.incrementBalance(coinbaseWeiDelta);
+      }
 
       initialFrame.getSelfDestructs().forEach(worldState::deleteAccount);
 

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/worldstate/DefaultMutablePrivateWorldStateUpdater.java

@@ -29,7 +29,7 @@ import java.util.Optional;
 // the public world state, but cannot write to it.
 public class DefaultMutablePrivateWorldStateUpdater implements WorldUpdater {
 
-  private final WorldUpdater publicWorldUpdater;
+  protected final WorldUpdater publicWorldUpdater;
   private final WorldUpdater privateWorldUpdater;
 
   public DefaultMutablePrivateWorldStateUpdater(

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/worldstate/GoQuorumMutablePrivateWorldStateUpdater.java

@@ -0,0 +1,37 @@
+/*
+ * Copyright ConsenSys AG.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
+ * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations under the License.
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+package org.hyperledger.besu.ethereum.worldstate;
+
+import org.hyperledger.besu.ethereum.core.Address;
+import org.hyperledger.besu.ethereum.core.EvmAccount;
+import org.hyperledger.besu.ethereum.core.UpdateTrackingAccount;
+import org.hyperledger.besu.ethereum.core.WorldUpdater;
+
+// This class uses a public WorldUpdater and a private WorldUpdater to provide a
+// MutableWorldStateUpdater that can read and write from the private world state and can read from
+// the public world state, but cannot write to it.
+public class GoQuorumMutablePrivateWorldStateUpdater
+    extends DefaultMutablePrivateWorldStateUpdater {
+
+  public GoQuorumMutablePrivateWorldStateUpdater(
+      final WorldUpdater publicWorldUpdater, final WorldUpdater privateWorldUpdater) {
+    super(publicWorldUpdater, privateWorldUpdater);
+  }
+
+  @Override
+  public EvmAccount getOrCreateSenderAccount(final Address address) {
+    return new UpdateTrackingAccount<EvmAccount>(publicWorldUpdater.getOrCreate(address));
+  }
+}

ethereum/core/src/test/java/org/hyperledger/besu/ethereum/goquorum/GoQuorumBlockProcessorTest.java

@@ -0,0 +1,149 @@
+/*
+ * Copyright ConsenSys AG.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
+ * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations under the License.
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+package org.hyperledger.besu.ethereum.goquorum;
+
+import static java.util.Collections.emptyList;
+import static java.util.Collections.emptyMap;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import org.hyperledger.besu.enclave.EnclaveServerException;
+import org.hyperledger.besu.enclave.GoQuorumEnclave;
+import org.hyperledger.besu.ethereum.chain.Blockchain;
+import org.hyperledger.besu.ethereum.core.Block;
+import org.hyperledger.besu.ethereum.core.BlockBody;
+import org.hyperledger.besu.ethereum.core.BlockHeader;
+import org.hyperledger.besu.ethereum.core.BlockHeaderTestFixture;
+import org.hyperledger.besu.ethereum.core.GoQuorumPrivacyParameters;
+import org.hyperledger.besu.ethereum.core.Hash;
+import org.hyperledger.besu.ethereum.core.MutableWorldState;
+import org.hyperledger.besu.ethereum.core.Transaction;
+import org.hyperledger.besu.ethereum.core.Wei;
+import org.hyperledger.besu.ethereum.core.fees.TransactionGasBudgetCalculator;
+import org.hyperledger.besu.ethereum.mainnet.AbstractBlockProcessor;
+import org.hyperledger.besu.ethereum.mainnet.MainnetTransactionProcessor;
+import org.hyperledger.besu.ethereum.referencetests.ReferenceTestBlockchain;
+import org.hyperledger.besu.ethereum.referencetests.ReferenceTestWorldState;
+
+import java.util.Collections;
+import java.util.Optional;
+
+import org.apache.tuweni.bytes.Bytes;
+import org.junit.Test;
+
+public class GoQuorumBlockProcessorTest {
+
+  private final MainnetTransactionProcessor transactionProcessor =
+      mock(MainnetTransactionProcessor.class);
+  private final AbstractBlockProcessor.TransactionReceiptFactory transactionReceiptFactory =
+      mock(AbstractBlockProcessor.TransactionReceiptFactory.class);
+
+  private final GoQuorumEnclave goQuorumEnclave = mock(GoQuorumEnclave.class);
+  private final GoQuorumPrivateStorage goQuorumPrivateStorage = mock(GoQuorumPrivateStorage.class);
+  private final GoQuorumPrivacyParameters goQuorumPrivacyParameters =
+      new GoQuorumPrivacyParameters(goQuorumEnclave, "123", goQuorumPrivateStorage, null);
+
+  @Test
+  public void noAccountCreatedWhenBlockRewardIsZeroAndSkipped() {
+    final Blockchain blockchain = new ReferenceTestBlockchain();
+    final GoQuorumBlockProcessor blockProcessor =
+        new GoQuorumBlockProcessor(
+            transactionProcessor,
+            transactionReceiptFactory,
+            Wei.ZERO,
+            BlockHeader::getCoinbase,
+            true,
+            TransactionGasBudgetCalculator.frontier(),
+            Optional.of(goQuorumPrivacyParameters));
+
+    final MutableWorldState worldState = ReferenceTestWorldState.create(emptyMap());
+    final Hash initialHash = worldState.rootHash();
+
+    final BlockHeader emptyBlockHeader =
+        new BlockHeaderTestFixture()
+            .transactionsRoot(Hash.EMPTY_LIST_HASH)
+            .ommersHash(Hash.EMPTY_LIST_HASH)
+            .buildHeader();
+    blockProcessor.processBlock(blockchain, worldState, emptyBlockHeader, emptyList(), emptyList());
+
+    // An empty block with 0 reward should not change the world state
+    assertThat(worldState.rootHash()).isEqualTo(initialHash);
+  }
+
+  @Test
+  public void accountCreatedWhenBlockRewardIsZeroAndNotSkipped() {
+    final Blockchain blockchain = new ReferenceTestBlockchain();
+    final GoQuorumBlockProcessor blockProcessor =
+        new GoQuorumBlockProcessor(
+            transactionProcessor,
+            transactionReceiptFactory,
+            Wei.ZERO,
+            BlockHeader::getCoinbase,
+            false,
+            TransactionGasBudgetCalculator.frontier(),
+            Optional.of(goQuorumPrivacyParameters));
+
+    final MutableWorldState worldState = ReferenceTestWorldState.create(emptyMap());
+    final Hash initialHash = worldState.rootHash();
+
+    final BlockHeader emptyBlockHeader =
+        new BlockHeaderTestFixture()
+            .transactionsRoot(Hash.EMPTY_LIST_HASH)
+            .ommersHash(Hash.EMPTY_LIST_HASH)
+            .buildHeader();
+    blockProcessor.processBlock(blockchain, worldState, emptyBlockHeader, emptyList(), emptyList());
+
+    // An empty block with 0 reward should change the world state prior to EIP158
+    assertThat(worldState.rootHash()).isNotEqualTo(initialHash);
+  }
+
+  @Test
+  public void enclaveNotAvailable() {
+    final Blockchain blockchain = new ReferenceTestBlockchain();
+    final GoQuorumBlockProcessor blockProcessor =
+        new GoQuorumBlockProcessor(
+            transactionProcessor,
+            transactionReceiptFactory,
+            Wei.ZERO,
+            BlockHeader::getCoinbase,
+            false,
+            TransactionGasBudgetCalculator.frontier(),
+            Optional.of(goQuorumPrivacyParameters));
+
+    final MutableWorldState worldState = ReferenceTestWorldState.create(emptyMap());
+
+    final Block block = mock(Block.class);
+    final BlockHeader blockHeader = mock(BlockHeader.class);
+    final BlockBody blockBody = mock(BlockBody.class);
+    final Transaction transaction = mock(Transaction.class);
+    when(transaction.getGasLimit()).thenReturn(1000L);
+    when(transaction.isGoQuorumPrivateTransaction()).thenReturn(true);
+    when(transaction.getPayload()).thenReturn(Bytes.wrap(new byte[] {(byte) 1}));
+    when(block.getBody()).thenReturn(blockBody);
+    when(blockBody.getTransactions()).thenReturn(Collections.singletonList(transaction));
+    when(blockBody.getOmmers()).thenReturn(Collections.emptyList());
+    when(blockHeader.getNumber()).thenReturn(20000L);
+    when(blockHeader.getGasLimit()).thenReturn(20000L);
+    when(block.getHeader()).thenReturn(blockHeader);
+    when(goQuorumEnclave.receive(any())).thenThrow(new EnclaveServerException(1, "a"));
+
+    assertThatThrownBy(() -> blockProcessor.processBlock(blockchain, worldState, worldState, block))
+        .isExactlyInstanceOf(EnclaveServerException.class)
+        .hasMessageContaining("a");
+  }
+}