Set gas fees to zero when simulating a transaction and not enforcing balance checks (#2918)

Before we set the account balance to maximum allowed, but this had side effects in smart contracts. Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
3 years ago · cff352b0b8
parent e7d1365084
commit cff352b0b8
5 changed files with 103 additions and 56 deletions
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@ -5,6 +5,7 @@
 ### Additions and Improvements

 ### Bug Fixes
+- Do not change the sender balance, but set gas fee to zero, when simulating a transaction without enforcing balance checks. [#2454](https://github.com/hyperledger/besu/pull/2454)

 ### Early Access Features

--- a/ethereum/api/src/test/resources/org/hyperledger/besu/ethereum/api/jsonrpc/eth/eth_call_valueTooHigh_block_8.json
+++ b/ethereum/api/src/test/resources/org/hyperledger/besu/ethereum/api/jsonrpc/eth/eth_call_valueTooHigh_block_8.json
@ -1,18 +1,5 @@
 {
  "request": [
-    {
-      "id": 3,
-      "jsonrpc": "2.0",
-      "method": "eth_call",
-      "params": [
-        {
-          "to": "0x6295ee1b4f6dd65047762f924ecd367c17eabf8f",
-          "from": "a94f5374fce5edbc8e2a8697c15331677e6ebf0b",
-          "value": "0x340ab63a021fc9aa"
-        },
-        "0x8"
-      ]
-    },
    {
      "id": 4,
      "jsonrpc": "2.0",
@ -29,11 +16,6 @@
    }
  ],
  "response": [
-    {
-      "jsonrpc": "2.0",
-      "id": 3,
-      "result": "0x"
-    },
    {
      "jsonrpc": "2.0",
      "id": 4,
--- a/ethereum/core/build.gradle
+++ b/ethereum/core/build.gradle
@ -70,6 +70,7 @@ dependencies {
  runtimeOnly 'org.apache.logging.log4j:log4j-core'

  testImplementation project(path: ':config', configuration: 'testSupportArtifacts')
+  testImplementation project(path: ':ethereum:api')
  testImplementation project(path: ':ethereum:referencetests')
  testImplementation project(path: ':ethereum:referencetests', configuration: 'testOutput')
  testImplementation project(':testutil')
--- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/transaction/TransactionSimulator.java
+++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/transaction/TransactionSimulator.java
@ -48,7 +48,6 @@ import java.util.Optional;
 import com.google.common.base.Supplier;
 import com.google.common.base.Suppliers;
 import org.apache.tuweni.bytes.Bytes;
-import org.apache.tuweni.units.bigints.UInt256;

 /*
 * Used to process transactions for eth_call and eth_estimateGas.
@ -159,8 +158,10 @@ public class TransactionSimulator {

    BlockHeader blockHeaderToProcess = header;

+    final Wei gasPrice;
+    final Wei maxFeePerGas;
+    final Wei maxPriorityFeePerGas;
    if (transactionValidationParams.isAllowExceedingBalance()) {
-      updater.getOrCreate(senderAddress).getMutable().setBalance(Wei.of(UInt256.MAX_VALUE));
      if (header.getBaseFee().isPresent()) {
        blockHeaderToProcess =
            BlockHeaderBuilder.fromHeader(header)
@ -168,11 +169,17 @@ public class TransactionSimulator {
                .blockHeaderFunctions(protocolSpec.getBlockHeaderFunctions())
                .buildBlockHeader();
      }
+      gasPrice = Wei.ZERO;
+      maxFeePerGas = Wei.ZERO;
+      maxPriorityFeePerGas = Wei.ZERO;
+    } else {
+      gasPrice = callParams.getGasPrice() != null ? callParams.getGasPrice() : Wei.ZERO;
+      maxFeePerGas = callParams.getMaxFeePerGas().orElse(gasPrice);
+      maxPriorityFeePerGas = callParams.getMaxPriorityFeePerGas().orElse(gasPrice);
    }

    final Account sender = publicWorldState.get(senderAddress);
    final long nonce = sender != null ? sender.getNonce() : 0L;
-    final Wei gasPrice = callParams.getGasPrice() != null ? callParams.getGasPrice() : Wei.ZERO;
    final long gasLimit =
        callParams.getGasLimit() >= 0
            ? callParams.getGasLimit()
@ -198,9 +205,7 @@ public class TransactionSimulator {
    if (header.getBaseFee().isEmpty()) {
      transactionBuilder.gasPrice(gasPrice);
    } else if (protocolSchedule.getChainId().isPresent()) {
-      transactionBuilder
-          .maxFeePerGas(callParams.getMaxFeePerGas().orElse(gasPrice))
-          .maxPriorityFeePerGas(callParams.getMaxPriorityFeePerGas().orElse(gasPrice));
+      transactionBuilder.maxFeePerGas(maxFeePerGas).maxPriorityFeePerGas(maxPriorityFeePerGas);
    } else {
      return Optional.empty();
    }
--- a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/transaction/TransactionSimulatorTest.java
+++ b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/transaction/TransactionSimulatorTest.java
@ -20,7 +20,6 @@ import static org.mockito.ArgumentMatchers.anyBoolean;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.verifyNoInteractions;
 import static org.mockito.Mockito.when;

 import org.hyperledger.besu.crypto.SECPSignature;
@ -31,6 +30,8 @@ import org.hyperledger.besu.datatypes.Hash;
 import org.hyperledger.besu.datatypes.Wei;
 import org.hyperledger.besu.ethereum.chain.Blockchain;
 import org.hyperledger.besu.ethereum.core.BlockHeader;
+import org.hyperledger.besu.ethereum.core.BlockHeaderFunctions;
+import org.hyperledger.besu.ethereum.core.Difficulty;
 import org.hyperledger.besu.ethereum.core.MutableWorldState;
 import org.hyperledger.besu.ethereum.core.Transaction;
 import org.hyperledger.besu.ethereum.mainnet.ImmutableTransactionValidationParams;
@ -41,10 +42,7 @@ import org.hyperledger.besu.ethereum.processing.TransactionProcessingResult;
 import org.hyperledger.besu.ethereum.processing.TransactionProcessingResult.Status;
 import org.hyperledger.besu.ethereum.worldstate.WorldStateArchive;
 import org.hyperledger.besu.evm.account.Account;
-import org.hyperledger.besu.evm.account.EvmAccount;
-import org.hyperledger.besu.evm.account.MutableAccount;
 import org.hyperledger.besu.evm.tracing.OperationTracer;
-import org.hyperledger.besu.evm.worldstate.WorldUpdater;
 import org.hyperledger.besu.plugin.data.TransactionType;

 import java.math.BigInteger;
@ -53,10 +51,10 @@ import java.util.Optional;
 import com.google.common.base.Supplier;
 import com.google.common.base.Suppliers;
 import org.apache.tuweni.bytes.Bytes;
-import org.apache.tuweni.units.bigints.UInt256;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
+import org.mockito.Answers;
 import org.mockito.Mock;
 import org.mockito.junit.MockitoJUnitRunner;

@ -81,7 +79,6 @@ public class TransactionSimulatorTest {
  @Mock private Blockchain blockchain;
  @Mock private WorldStateArchive worldStateArchive;
  @Mock private MutableWorldState worldState;
-  @Mock private WorldUpdater worldUpdater;
  @Mock private ProtocolSchedule protocolSchedule;
  @Mock private ProtocolSpec protocolSpec;
  @Mock private MainnetTransactionProcessor transactionProcessor;
@ -131,8 +128,8 @@ public class TransactionSimulatorTest {
  }

  @Test
-  public void shouldIncreaseBalanceAccountWhenExceedingBalanceAllowed() {
-    final CallParameter callParameter = legacyTransactionCallParameter();
+  public void shouldSetGasPriceToZeroWhenExceedingBalanceAllowed() {
+    final CallParameter callParameter = legacyTransactionCallParameter(Wei.ONE);

    mockBlockchainForBlockHeader(Hash.ZERO, 1L);
    mockWorldStateForAccount(Hash.ZERO, callParameter.getFrom(), 1L);
@ -141,7 +138,7 @@ public class TransactionSimulatorTest {
        Transaction.builder()
            .type(TransactionType.FRONTIER)
            .nonce(1L)
-            .gasPrice(callParameter.getGasPrice())
+            .gasPrice(Wei.ZERO)
            .gasLimit(callParameter.getGasLimit())
            .to(callParameter.getTo())
            .sender(callParameter.getFrom())
@ -152,8 +149,38 @@ public class TransactionSimulatorTest {

    mockProcessorStatusForTransaction(1L, expectedTransaction, Status.SUCCESSFUL);

-    final MutableAccount mutableAccount =
-        mockWorldUpdaterForAccount(Hash.ZERO, callParameter.getFrom());
+    transactionSimulator.process(
+        callParameter,
+        ImmutableTransactionValidationParams.builder().isAllowExceedingBalance(true).build(),
+        OperationTracer.NO_TRACING,
+        1L);
+
+    verifyTransactionWasProcessed(expectedTransaction);
+  }
+
+  @Test
+  public void shouldSetFeePerGasToZeroWhenExceedingBalanceAllowed() {
+    final CallParameter callParameter = eip1559TransactionCallParameter(Wei.ONE, Wei.ONE);
+
+    mockBlockchainForBlockHeader(Hash.ZERO, 1L, 1L);
+    mockWorldStateForAccount(Hash.ZERO, callParameter.getFrom(), 1L);
+
+    final Transaction expectedTransaction =
+        Transaction.builder()
+            .type(TransactionType.EIP1559)
+            .chainId(BigInteger.ONE)
+            .nonce(1L)
+            .gasLimit(callParameter.getGasLimit())
+            .maxFeePerGas(Wei.ZERO)
+            .maxPriorityFeePerGas(Wei.ZERO)
+            .to(callParameter.getTo())
+            .sender(callParameter.getFrom())
+            .value(callParameter.getValue())
+            .payload(callParameter.getPayload())
+            .signature(FAKE_SIGNATURE)
+            .build();
+
+    mockProcessorStatusForTransaction(1L, expectedTransaction, Status.SUCCESSFUL);

    transactionSimulator.process(
        callParameter,
@ -161,12 +188,12 @@ public class TransactionSimulatorTest {
        OperationTracer.NO_TRACING,
        1L);

-    verify(mutableAccount).setBalance(Wei.of(UInt256.MAX_VALUE));
+    verifyTransactionWasProcessed(expectedTransaction);
  }

  @Test
-  public void shouldNotIncreaseBalanceAccountWhenExceedingBalanceIsNotAllowed() {
-    final CallParameter callParameter = legacyTransactionCallParameter();
+  public void shouldNotSetGasPriceToZeroWhenExceedingBalanceIsNotAllowed() {
+    final CallParameter callParameter = legacyTransactionCallParameter(Wei.ONE);

    mockBlockchainForBlockHeader(Hash.ZERO, 1L);
    mockWorldStateForAccount(Hash.ZERO, callParameter.getFrom(), 1L);
@ -186,8 +213,38 @@ public class TransactionSimulatorTest {

    mockProcessorStatusForTransaction(1L, expectedTransaction, Status.SUCCESSFUL);

-    final MutableAccount mutableAccount =
-        mockWorldUpdaterForAccount(Hash.ZERO, callParameter.getFrom());
+    transactionSimulator.process(
+        callParameter,
+        ImmutableTransactionValidationParams.builder().isAllowExceedingBalance(false).build(),
+        OperationTracer.NO_TRACING,
+        1L);
+
+    verifyTransactionWasProcessed(expectedTransaction);
+  }
+
+  @Test
+  public void shouldNotSetFeePerGasToZeroWhenExceedingBalanceIsNotAllowed() {
+    final CallParameter callParameter = eip1559TransactionCallParameter(Wei.ONE, Wei.ONE);
+
+    mockBlockchainForBlockHeader(Hash.ZERO, 1L, 1L);
+    mockWorldStateForAccount(Hash.ZERO, callParameter.getFrom(), 1L);
+
+    final Transaction expectedTransaction =
+        Transaction.builder()
+            .type(TransactionType.EIP1559)
+            .chainId(BigInteger.ONE)
+            .nonce(1L)
+            .gasLimit(callParameter.getGasLimit())
+            .maxFeePerGas(callParameter.getMaxFeePerGas().orElseThrow())
+            .maxPriorityFeePerGas(callParameter.getMaxPriorityFeePerGas().orElseThrow())
+            .to(callParameter.getTo())
+            .sender(callParameter.getFrom())
+            .value(callParameter.getValue())
+            .payload(callParameter.getPayload())
+            .signature(FAKE_SIGNATURE)
+            .build();
+
+    mockProcessorStatusForTransaction(1L, expectedTransaction, Status.SUCCESSFUL);

    transactionSimulator.process(
        callParameter,
@ -195,7 +252,7 @@ public class TransactionSimulatorTest {
        OperationTracer.NO_TRACING,
        1L);

-    verifyNoInteractions(mutableAccount);
+    verifyTransactionWasProcessed(expectedTransaction);
  }

  @Test
@ -441,17 +498,6 @@ public class TransactionSimulatorTest {
    when(worldState.get(any())).thenReturn(null);
  }

-  private MutableAccount mockWorldUpdaterForAccount(final Hash stateRoot, final Address address) {
-    final EvmAccount account = mock(EvmAccount.class);
-    final MutableAccount mutableAccount = mock(MutableAccount.class);
-    when(worldStateArchive.getMutable(eq(stateRoot), any(), anyBoolean()))
-        .thenReturn(Optional.of(worldState));
-    when(worldState.updater()).thenReturn(worldUpdater);
-    when(worldUpdater.getOrCreate(eq(address))).thenReturn(account);
-    when(account.getMutable()).thenReturn(mutableAccount);
-    return mutableAccount;
-  }
-
  private void mockBlockchainForBlockHeader(final Hash stateRoot, final long blockNumber) {
    mockBlockchainForBlockHeader(stateRoot, blockNumber, Hash.ZERO);
  }
@ -467,19 +513,22 @@ public class TransactionSimulatorTest {

  private void mockBlockchainForBlockHeader(
      final Hash stateRoot, final long blockNumber, final long baseFee) {
-    final BlockHeader blockHeader = mock(BlockHeader.class);
+    final BlockHeader blockHeader = mock(BlockHeader.class, Answers.RETURNS_MOCKS);
    when(blockHeader.getStateRoot()).thenReturn(stateRoot);
    when(blockHeader.getNumber()).thenReturn(blockNumber);
    when(blockHeader.getBaseFee()).thenReturn(Optional.of(baseFee));
+    when(blockHeader.getDifficulty()).thenReturn(Difficulty.ONE);
    when(blockchain.getBlockHeader(blockNumber)).thenReturn(Optional.of(blockHeader));
  }

  private void mockProcessorStatusForTransaction(
      final long blockNumber, final Transaction transaction, final Status status) {
+    final BlockHeaderFunctions blockHeaderFunctions = mock(BlockHeaderFunctions.class);
    when(protocolSchedule.getChainId()).thenReturn(Optional.of(BigInteger.ONE));
    when(protocolSchedule.getByBlockNumber(eq(blockNumber))).thenReturn(protocolSpec);
    when(protocolSpec.getTransactionProcessor()).thenReturn(transactionProcessor);
    when(protocolSpec.getMiningBeneficiaryCalculator()).thenReturn(BlockHeader::getCoinbase);
+    when(protocolSpec.getBlockHeaderFunctions()).thenReturn(blockHeaderFunctions);

    final TransactionProcessingResult result = mock(TransactionProcessingResult.class);
    switch (status) {
@ -504,23 +553,32 @@ public class TransactionSimulatorTest {
  }

  private CallParameter legacyTransactionCallParameter() {
+    return legacyTransactionCallParameter(Wei.ZERO);
+  }
+
+  private CallParameter legacyTransactionCallParameter(final Wei gasPrice) {
    return new CallParameter(
        Address.fromHexString("0x0"),
        Address.fromHexString("0x0"),
        0,
-        Wei.of(0),
+        gasPrice,
        Wei.of(0),
        Bytes.EMPTY);
  }

  private CallParameter eip1559TransactionCallParameter() {
+    return eip1559TransactionCallParameter(Wei.ZERO, Wei.ZERO);
+  }
+
+  private CallParameter eip1559TransactionCallParameter(
+      final Wei maxFeePerGas, final Wei maxPriorityFeePerGas) {
    return new CallParameter(
        Address.fromHexString("0x0"),
        Address.fromHexString("0x0"),
        0,
        Wei.of(0),
-        Optional.of(Wei.of(0)),
-        Optional.of(Wei.of(0)),
+        Optional.of(maxFeePerGas),
+        Optional.of(maxPriorityFeePerGas),
        Wei.of(0),
        Bytes.EMPTY);
  }