diff --git a/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/configuration/BesuNodeFactory.java b/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/configuration/BesuNodeFactory.java index b35cb48c6db..3708bcc1a3b 100644 --- a/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/configuration/BesuNodeFactory.java +++ b/acceptance-tests/dsl/src/main/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/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/transaction/eth/EthGetCodeTransaction.java b/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/transaction/eth/EthGetCodeTransaction.java new file mode 100644 index 00000000000..52750916df8 --- /dev/null +++ b/acceptance-tests/dsl/src/main/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 { + + 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/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/transaction/eth/EthTransactions.java b/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/transaction/eth/EthTransactions.java index b8b72052d0b..194919b13c7 100644 --- a/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/transaction/eth/EthTransactions.java +++ b/acceptance-tests/dsl/src/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/acceptance-tests/tests/src/test/java/org/hyperledger/besu/tests/acceptance/ethereum/CodeDelegationTransactionAcceptanceTest.java b/acceptance-tests/tests/src/test/java/org/hyperledger/besu/tests/acceptance/ethereum/CodeDelegationTransactionAcceptanceTest.java index cb7b8d4158e..eb517f66d64 100644 --- a/acceptance-tests/tests/src/test/java/org/hyperledger/besu/tests/acceptance/ethereum/CodeDelegationTransactionAcceptanceTest.java +++ b/acceptance-tests/tests/src/test/java/org/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 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 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 index b5d08734291..a69d9350b6b 100644 --- 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 index 12a8d4d33c9..0fb8818fa58 100644 --- 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/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/AbstractBlockProcessor.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/AbstractBlockProcessor.java index 91e179ce34e..42efc85e5c9 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/AbstractBlockProcessor.java +++ b/ethereum/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/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/CodeDelegationProcessor.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/CodeDelegationProcessor.java index 8d6fcb206a1..f8bb18b031c 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/CodeDelegationProcessor.java +++ b/ethereum/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 index 20d75a67577..01395d37c1b 100644 --- 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/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/trie/diffbased/common/worldview/accumulator/DiffBasedWorldStateUpdateAccumulator.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/trie/diffbased/common/worldview/accumulator/DiffBasedWorldStateUpdateAccumulator.java index b97ffe747b3..3273292b4bb 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/trie/diffbased/common/worldview/accumulator/DiffBasedWorldStateUpdateAccumulator.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/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 getAllAccountStorage(final Address address, final Has return results; } + /** + * Marks the boundary of a transaction by clearing tracking collections. + * + *

These tracking collections store changes made during the transaction. After committing the + * transaction, they become unnecessary and can be safely cleared. + * + *

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/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/mainnet/CodeDelegationProcessorTest.java b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/mainnet/CodeDelegationProcessorTest.java index e69d57bbe3c..1f54f1076ca 100644 --- a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/mainnet/CodeDelegationProcessorTest.java +++ b/ethereum/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 diff --git a/evm/src/main/java/org/hyperledger/besu/evm/worldstate/CodeDelegationService.java b/evm/src/main/java/org/hyperledger/besu/evm/worldstate/CodeDelegationService.java index 3b838895eda..fbec94deed4 100644 --- a/evm/src/main/java/org/hyperledger/besu/evm/worldstate/CodeDelegationService.java +++ b/evm/src/main/java/org/hyperledger/besu/evm/worldstate/CodeDelegationService.java @@ -59,7 +59,7 @@ public void processCodeDelegation( } /** - * Returns if the provided account has either no code set or has already delegated code. + * Returns true if the provided account has either no code set or has already delegated code. * * @param account the account to check. * @return {@code true} if the account can set delegated code, {@code false} otherwise.