Fix code delegation issue found in pectra devnet (#8203)

* add acceptance tests to verify that code delegations are persisted even if the tx itself fails * set node to use bonsai, set correct nonce for code delegation, make sure revert in contract has the 2 necessary items on the stack * Hive test fix: only add account if authority nonce is valid (0) --------- Signed-off-by: Karim Taam <[email protected]> Signed-off-by: Daniel Lehrner <[email protected]> Signed-off-by: Simon Dudley <[email protected]> Co-authored-by: Daniel Lehrner <[email protected]> Co-authored-by: Simon Dudley <[email protected]>
hyperledger · Jan 31, 2025 · 68665db · 68665db
1 parent 97a69b0
commit 68665db
Show file tree

Hide file tree

Showing 12 changed files with 287 additions and 5 deletions.
diff --git a/...in/java/org/hyperledger/besu/tests/acceptance/dsl/node/configuration/BesuNodeFactory.java b/...in/java/org/hyperledger/besu/tests/acceptance/dsl/node/configuration/BesuNodeFactory.java
@@ -532,6 +532,7 @@ public BesuNode createExecutionEngineGenesisNode(final String name, final String
             .jsonRpcTxPool()
             .engineRpcEnabled(true)
             .jsonRpcDebug()
+            .dataStorageConfiguration(DataStorageConfiguration.DEFAULT_BONSAI_CONFIG)
             .build());
   }
 

diff --git a/...java/org/hyperledger/besu/tests/acceptance/dsl/transaction/eth/EthGetCodeTransaction.java b/...java/org/hyperledger/besu/tests/acceptance/dsl/transaction/eth/EthGetCodeTransaction.java
@@ -0,0 +1,50 @@
+/*
+ * Copyright contributors to Hyperledger Besu.
+ *
+ * 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.tests.acceptance.dsl.transaction.eth;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.web3j.protocol.core.DefaultBlockParameterName.LATEST;
+
+import org.hyperledger.besu.tests.acceptance.dsl.account.Account;
+import org.hyperledger.besu.tests.acceptance.dsl.transaction.NodeRequests;
+import org.hyperledger.besu.tests.acceptance.dsl.transaction.Transaction;
+
+import java.io.IOException;
+
+import org.apache.tuweni.bytes.Bytes;
+import org.web3j.protocol.core.methods.response.EthGetCode;
+
+public class EthGetCodeTransaction implements Transaction<Bytes> {
+
+  private final Account account;
+
+  public EthGetCodeTransaction(final Account account) {
+    this.account = account;
+  }
+
+  @Override
+  public Bytes execute(final NodeRequests node) {
+    try {
+      final EthGetCode result = node.eth().ethGetCode(account.getAddress(), LATEST).send();
+      assertThat(result).isNotNull();
+      assertThat(result.hasError()).isFalse();
+
+      return Bytes.fromHexString(result.getCode());
+
+    } catch (final IOException e) {
+      throw new RuntimeException(e);
+    }
+  }
+}
diff --git a/.../main/java/org/hyperledger/besu/tests/acceptance/dsl/transaction/eth/EthTransactions.java b/.../main/java/org/hyperledger/besu/tests/acceptance/dsl/transaction/eth/EthTransactions.java
@@ -43,6 +43,10 @@ public EthGetBalanceTransaction getBalance(final Account account) {
     return new EthGetBalanceTransaction(account);
   }
 
+  public EthGetCodeTransaction getCode(final Account account) {
+    return new EthGetCodeTransaction(account);
+  }
+
   public EthGetBalanceAtBlockTransaction getBalanceAtBlock(
       final Account account, final BigInteger block) {
     return new EthGetBalanceAtBlockTransaction(account, block);

diff --git a/...g/hyperledger/besu/tests/acceptance/ethereum/CodeDelegationTransactionAcceptanceTest.java b/...g/hyperledger/besu/tests/acceptance/ethereum/CodeDelegationTransactionAcceptanceTest.java
@@ -46,6 +46,9 @@ public class CodeDelegationTransactionAcceptanceTest extends AcceptanceTestBase
   public static final Address SEND_ALL_ETH_CONTRACT_ADDRESS =
       Address.fromHexStringStrict("0000000000000000000000000000000000009999");
 
+  public static final Address ALWAYS_REVERT_CONTRACT_ADDRESS =
+      Address.fromHexStringStrict("0000000000000000000000000000000000000666");
+
   private final Account authorizer =
       accounts.createAccount(
           Address.fromHexStringStrict("8da48afC965480220a3dB9244771bd3afcB5d895"));
@@ -232,4 +235,124 @@ public void shouldCheckNonceAfterNonceIncreaseOfSender() throws IOException {
     assertThat(otherAccountBalanceAfterFirstTx.add(BigInteger.ONE))
         .isEqualTo(otherAccountBalanceAfterSecondTx);
   }
+
+  /**
+   * EIP-7702 code delegation should be persisted even if the transaction that contains the
+   * authorization is reverted.
+   */
+  @Test
+  public void shouldPersistCodeDelegationAfterRevert() throws IOException {
+    final long GAS_LIMIT = 1_000_000L;
+
+    // check the authorizer has no code before the transaction
+    final Bytes authorizerCodeBeforeCodeDelegation =
+        besuNode.execute(ethTransactions.getCode(authorizer));
+    assertThat(authorizerCodeBeforeCodeDelegation).isEqualTo(Bytes.EMPTY);
+
+    // valid 7702 code delegation to SEND_ALL_ETH_CONTRACT_ADDRESS
+    final CodeDelegation codeDelegation =
+        org.hyperledger.besu.ethereum.core.CodeDelegation.builder()
+            .chainId(BigInteger.valueOf(20211))
+            .nonce(0L)
+            .address(SEND_ALL_ETH_CONTRACT_ADDRESS)
+            .signAndBuild(
+                secp256k1.createKeyPair(
+                    secp256k1.createPrivateKey(AUTHORIZER_PRIVATE_KEY.toUnsignedBigInteger())));
+
+    // the transaction will revert, because the to address is a contract that always reverts
+    final Transaction tx =
+        Transaction.builder()
+            .type(TransactionType.DELEGATE_CODE)
+            .chainId(BigInteger.valueOf(20211))
+            .nonce(0)
+            .maxPriorityFeePerGas(Wei.of(1_000_000_000))
+            .maxFeePerGas(Wei.fromHexString("0x02540BE400"))
+            .gasLimit(GAS_LIMIT)
+            .to(ALWAYS_REVERT_CONTRACT_ADDRESS)
+            .value(Wei.ZERO)
+            .payload(Bytes.EMPTY)
+            .codeDelegations(List.of(codeDelegation))
+            .signAndBuild(
+                secp256k1.createKeyPair(
+                    secp256k1.createPrivateKey(
+                        TRANSACTION_SPONSOR_PRIVATE_KEY.toUnsignedBigInteger())));
+
+    // include the tx in the next block
+    final String txHash =
+        besuNode.execute(ethTransactions.sendRawTransaction(tx.encoded().toHexString()));
+    testHelper.buildNewBlock();
+
+    // check that the transaction was included and has indeed reverted
+    Optional<TransactionReceipt> maybeTransactionReceipt =
+        besuNode.execute(ethTransactions.getTransactionReceipt(txHash));
+    assertThat(maybeTransactionReceipt).isPresent();
+    assertThat(maybeTransactionReceipt.get().getStatus()).isEqualTo("0x0");
+
+    // check the authorizer has the code delegation after the transaction even though it has
+    // reverted
+    final Bytes expectedCode =
+        Bytes.concatenate(Bytes.fromHexString("ef0100"), SEND_ALL_ETH_CONTRACT_ADDRESS);
+    final Bytes authorizerCode = besuNode.execute(ethTransactions.getCode(authorizer));
+    assertThat(authorizerCode).isEqualTo(expectedCode);
+  }
+
+  /**
+   * EIP-7702 code delegation should be persisted even if the transaction that contains the
+   * authorization is reverted and the transaction sender is the same as the code delegation
+   * authorizer.
+   */
+  @Test
+  public void shouldPersistCodeDelegationAfterRevertWhenSelfSponsored() throws IOException {
+    final long GAS_LIMIT = 1_000_000L;
+
+    // check the authorizer has no code before the transaction
+    final Bytes authorizerCodeBeforeCodeDelegation =
+        besuNode.execute(ethTransactions.getCode(authorizer));
+    assertThat(authorizerCodeBeforeCodeDelegation).isEqualTo(Bytes.EMPTY);
+
+    // valid 7702 code delegation to SEND_ALL_ETH_CONTRACT_ADDRESS
+    final CodeDelegation codeDelegation =
+        org.hyperledger.besu.ethereum.core.CodeDelegation.builder()
+            .chainId(BigInteger.valueOf(20211))
+            .nonce(1L)
+            .address(SEND_ALL_ETH_CONTRACT_ADDRESS)
+            .signAndBuild(
+                secp256k1.createKeyPair(
+                    secp256k1.createPrivateKey(AUTHORIZER_PRIVATE_KEY.toUnsignedBigInteger())));
+
+    // the transaction will revert, because the to address is a contract that always reverts
+    final Transaction tx =
+        Transaction.builder()
+            .type(TransactionType.DELEGATE_CODE)
+            .chainId(BigInteger.valueOf(20211))
+            .nonce(0)
+            .maxPriorityFeePerGas(Wei.of(1_000_000_000))
+            .maxFeePerGas(Wei.fromHexString("0x02540BE400"))
+            .gasLimit(GAS_LIMIT)
+            .to(ALWAYS_REVERT_CONTRACT_ADDRESS)
+            .value(Wei.ZERO)
+            .payload(Bytes.EMPTY)
+            .codeDelegations(List.of(codeDelegation))
+            .signAndBuild(
+                secp256k1.createKeyPair(
+                    secp256k1.createPrivateKey(AUTHORIZER_PRIVATE_KEY.toUnsignedBigInteger())));
+
+    // include the tx in the next block
+    final String txHash =
+        besuNode.execute(ethTransactions.sendRawTransaction(tx.encoded().toHexString()));
+    testHelper.buildNewBlock();
+
+    // check that the transaction was included and has indeed reverted
+    Optional<TransactionReceipt> maybeTransactionReceipt =
+        besuNode.execute(ethTransactions.getTransactionReceipt(txHash));
+    assertThat(maybeTransactionReceipt).isPresent();
+    assertThat(maybeTransactionReceipt.get().getStatus()).isEqualTo("0x0");
+
+    // check the authorizer has the code delegation after the transaction even though it has
+    // reverted
+    final Bytes expectedCode =
+        Bytes.concatenate(Bytes.fromHexString("ef0100"), SEND_ALL_ETH_CONTRACT_ADDRESS);
+    final Bytes authorizerCode = besuNode.execute(ethTransactions.getCode(authorizer));
+    assertThat(authorizerCode).isEqualTo(expectedCode);
+  }
 }
diff --git a/acceptance-tests/tests/src/test/resources/dev/dev_prague.json b/acceptance-tests/tests/src/test/resources/dev/dev_prague.json
@@ -57,6 +57,13 @@
       "privateKey": "11f2e7b6a734ab03fa682450e0d4681d18a944f8b83c99bf7b9b4de6c0f35ea1",
       "balance": "90000000000000000000000"
     },
+    "0x0000000000000000000000000000000000000666": {
+      "comment": "Contract reverts immediately when called",
+      "balance": "0",
+      "code": "5F5FFD",
+      "codeDecompiled": "PUSH0 PUSH0 REVERT",
+      "storage": {}
+    },
     "0x0000000000000000000000000000000000009999": {
       "comment": "Contract sends all its Ether to the address provided as a call data.",
       "balance": "0",

diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/CodeDelegation.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/CodeDelegation.java
@@ -272,4 +272,20 @@ public org.hyperledger.besu.datatypes.CodeDelegation build() {
       return new CodeDelegation(chainId, address, nonce, signature);
     }
   }
+
+  @Override
+  public String toString() {
+    return "CodeDelegation{"
+        + "chainId="
+        + chainId
+        + ", address="
+        + address
+        + ", nonce="
+        + nonce
+        + ", signature="
+        + signature
+        + ", authorizerSupplier="
+        + authorizerSupplier
+        + '}';
+  }
 }
diff --git a/...reum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/AbstractBlockProcessor.java b/...reum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/AbstractBlockProcessor.java
@@ -192,6 +192,7 @@ protected BlockProcessingResult processBlock(
       }
 
       blockUpdater.commit();
+      blockUpdater.markTransactionBoundary();
 
       currentGasUsed += transaction.getGasLimit() - transactionProcessingResult.getGasRemaining();
       if (transaction.getVersionedHashes().isPresent()) {

diff --git a/...eum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/CodeDelegationProcessor.java b/...eum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/CodeDelegationProcessor.java
@@ -126,6 +126,10 @@ private void processCodeDelegation(
     MutableAccount authority;
     boolean authorityDoesAlreadyExist = false;
     if (maybeAuthorityAccount.isEmpty()) {
+      // only create an account if nonce is valid
+      if (codeDelegation.nonce() != 0) {
+        return;
+      }
       authority = evmWorldUpdater.createAccount(authorizer.get());
     } else {
       authority = maybeAuthorityAccount.get();
@@ -146,7 +150,7 @@ private void processCodeDelegation(
     }
 
     if (authorityDoesAlreadyExist) {
-      result.incremenentAlreadyExistingDelegators();
+      result.incrementAlreadyExistingDelegators();
     }
 
     evmWorldUpdater

diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/CodeDelegationResult.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/CodeDelegationResult.java
@@ -27,7 +27,7 @@ public void addAccessedDelegatorAddress(final Address address) {
     accessedDelegatorAddresses.add(address);
   }
 
-  public void incremenentAlreadyExistingDelegators() {
+  public void incrementAlreadyExistingDelegators() {
     alreadyExistingDelegators += 1;
   }
 

diff --git a/...eum/trie/diffbased/common/worldview/accumulator/DiffBasedWorldStateUpdateAccumulator.java b/...eum/trie/diffbased/common/worldview/accumulator/DiffBasedWorldStateUpdateAccumulator.java
@@ -499,8 +499,6 @@ public void commit() {
                 tracked.setStorageWasCleared(false); // storage already cleared for this transaction
               }
             });
-    getUpdatedAccounts().clear();
-    getDeletedAccounts().clear();
   }
 
   @Override
@@ -598,6 +596,21 @@ public Map<Bytes32, Bytes> getAllAccountStorage(final Address address, final Has
     return results;
   }
 
+  /**
+   * Marks the boundary of a transaction by clearing tracking collections.
+   *
+   * <p>These tracking collections store changes made during the transaction. After committing the
+   * transaction, they become unnecessary and can be safely cleared.
+   *
+   * <p>Note: If the transaction is not committed before this method is called, any uncommitted
+   * changes will be lost.
+   */
+  @Override
+  public void markTransactionBoundary() {
+    getUpdatedAccounts().clear();
+    getDeletedAccounts().clear();
+  }
+
   @Override
   public boolean isModifyingHeadWorldState() {
     return true;

diff --git a/...core/src/test/java/org/hyperledger/besu/ethereum/mainnet/CodeDelegationProcessorTest.java b/...core/src/test/java/org/hyperledger/besu/ethereum/mainnet/CodeDelegationProcessorTest.java
@@ -18,6 +18,7 @@
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
@@ -112,6 +113,23 @@ void shouldProcessValidDelegationForNewAccount() {
     verify(delegationCodeService).processCodeDelegation(authority, DELEGATE_ADDRESS);
   }
 
+  @Test
+  void shouldNotCreateAccountIfNonceIsInvalid() {
+    // Arrange
+    CodeDelegation codeDelegation = createCodeDelegation(CHAIN_ID, 1L);
+    when(transaction.getCodeDelegationList()).thenReturn(Optional.of(List.of(codeDelegation)));
+    when(worldUpdater.getAccount(any())).thenReturn(null);
+
+    // Act
+    CodeDelegationResult result = processor.process(worldUpdater, transaction);
+
+    // Assert
+    assertThat(result.alreadyExistingDelegators()).isZero();
+    verify(worldUpdater, never()).createAccount(any());
+    verify(authority, never()).incrementNonce();
+    verify(delegationCodeService, never()).processCodeDelegation(authority, DELEGATE_ADDRESS);
+  }
+
   @Test
   void shouldProcessValidDelegationForExistingAccount() {
     // Arrange
@@ -150,6 +168,51 @@ void shouldRejectDelegationWithInvalidNonce() {
     verify(delegationCodeService, never()).processCodeDelegation(any(), any());
   }
 
+  @Test
+  void shouldSkipOverInvalidMultipleInvalidNonceDelegationsForSameAuthorityForNewAccount() {
+    // Arrange
+    when(worldUpdater.codeDelegationService()).thenReturn(delegationCodeService);
+    var signature1 = new SECPSignature(BigInteger.ONE, BigInteger.ONE, (byte) 0);
+    long cd1_invalidNonce = 2L;
+    var cd1_invalid =
+        new org.hyperledger.besu.ethereum.core.CodeDelegation(
+            CHAIN_ID,
+            Address.fromHexString("0x0000000000000000000000000000000000001000"),
+            cd1_invalidNonce,
+            signature1);
+    var signature2 = new SECPSignature(BigInteger.TWO, BigInteger.TWO, (byte) 0);
+    final long cd2_validNonce = 0L;
+    var cd2_valid =
+        new org.hyperledger.besu.ethereum.core.CodeDelegation(
+            CHAIN_ID,
+            Address.fromHexString("0x0000000000000000000000000000000000001100"),
+            cd2_validNonce,
+            signature2);
+    var signature3 = new SECPSignature(BigInteger.TWO, BigInteger.TWO, (byte) 0);
+    final long cd3_invalidNonce = 0L;
+    var cd3_invalid =
+        new org.hyperledger.besu.ethereum.core.CodeDelegation(
+            CHAIN_ID,
+            Address.fromHexString("0x0000000000000000000000000000000000001200"),
+            cd3_invalidNonce,
+            signature3);
+    when(transaction.getCodeDelegationList())
+        .thenReturn(Optional.of(List.of(cd1_invalid, cd2_valid, cd3_invalid)));
+
+    when(worldUpdater.getAccount(any())).thenReturn(null).thenReturn(null).thenReturn(authority);
+    when(worldUpdater.createAccount(any())).thenReturn(authority);
+    when(authority.getNonce()).thenReturn(0L).thenReturn(1L);
+    when(delegationCodeService.canSetCodeDelegation(any())).thenReturn(true);
+
+    // Act
+    CodeDelegationResult result = processor.process(worldUpdater, transaction);
+
+    // Assert
+    assertThat(result.alreadyExistingDelegators()).isZero();
+    verify(authority, times(1)).incrementNonce();
+    verify(delegationCodeService, times(1)).processCodeDelegation(any(), any());
+  }
+
   @Test
   void shouldRejectDelegationWithSGreaterThanHalfCurveOrder() {
     // Arrange