From 4ace9e42c0e87425612005d00cf4b5d574cd432b Mon Sep 17 00:00:00 2001 From: daniellehrner Date: Mon, 29 Jul 2024 17:45:15 +0200 Subject: [PATCH] 7702 bugfixes for devnet-1 (#7394) * process the authority list before increasing the sender nonce, make sure that the updated balance of the sender is calculated correctly if they sign an authorization as well Signed-off-by: Daniel Lehrner --- .../SetCodeTransactionAcceptanceTest.java | 72 ++++++++++++++++++- .../ethereum/mainnet/AuthorityProcessor.java | 6 +- .../mainnet/MainnetTransactionProcessor.java | 24 ++++--- 3 files changed, 86 insertions(+), 16 deletions(-) diff --git a/acceptance-tests/tests/src/test/java/org/hyperledger/besu/tests/acceptance/ethereum/SetCodeTransactionAcceptanceTest.java b/acceptance-tests/tests/src/test/java/org/hyperledger/besu/tests/acceptance/ethereum/SetCodeTransactionAcceptanceTest.java index f1b2558a97..b134b1f5c0 100644 --- a/acceptance-tests/tests/src/test/java/org/hyperledger/besu/tests/acceptance/ethereum/SetCodeTransactionAcceptanceTest.java +++ b/acceptance-tests/tests/src/test/java/org/hyperledger/besu/tests/acceptance/ethereum/SetCodeTransactionAcceptanceTest.java @@ -34,6 +34,7 @@ import java.util.Optional; import org.apache.tuweni.bytes.Bytes; import org.apache.tuweni.bytes.Bytes32; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.web3j.protocol.core.methods.response.TransactionReceipt; @@ -57,6 +58,8 @@ public class SetCodeTransactionAcceptanceTest extends AcceptanceTestBase { public static final Bytes TRANSACTION_SPONSOR_PRIVATE_KEY = Bytes.fromHexString("3a4ff6d22d7502ef2452368165422861c01a0f72f851793b372b87888dc3c453"); + private final Account otherAccount = accounts.createAccount("otherAccount"); + private BesuNode besuNode; private PragueAcceptanceTestHelper testHelper; @@ -68,11 +71,17 @@ public class SetCodeTransactionAcceptanceTest extends AcceptanceTestBase { testHelper = new PragueAcceptanceTestHelper(besuNode, ethTransactions); } + @AfterEach + void tearDown() { + besuNode.close(); + cluster.close(); + } + /** * At the beginning of the test both the authorizer and the transaction sponsor have a balance of * 90000 ETH. The authorizer creates an authorization for a contract that send all its ETH to any - * given address. The transaction sponsor created a 7702 transaction with it and sends all the ETH - * from the authorizer to itself. The authorizer balance should be 0 and the transaction sponsor + * given address. The transaction sponsor sponsors the 7702 transaction and sends all the ETH from + * the authorizer to itself. The authorizer balance should be 0 and the transaction sponsor's * balance should be 180000 ETH minus the transaction costs. */ @Test @@ -122,4 +131,63 @@ public class SetCodeTransactionAcceptanceTest extends AcceptanceTestBase { BigInteger expectedSponsorBalance = new BigInteger("180000000000000000000000").subtract(txCost); cluster.verify(transactionSponsor.balanceEquals(Amount.wei(expectedSponsorBalance))); } + + /** + * The authorizer creates an authorization for a contract that sends all its ETH to any given + * address. But the nonce is 1 and the authorization list is processed before the nonce increase + * of the sender. Therefore, the authorization should be invalid and will be ignored. No balance + * change, except for a decrease for paying the transaction cost should occur. + */ + @Test + public void shouldCheckNonceBeforeNonceIncreaseOfSender() throws IOException { + + cluster.verify(authorizer.balanceEquals(Amount.ether(90000))); + + final org.hyperledger.besu.datatypes.SetCodeAuthorization authorization = + SetCodeAuthorization.builder() + .chainId(BigInteger.valueOf(20211)) + .nonces( + Optional.of( + 1L)) // nonce is 1, but because it is validated before the nonce increase, it + // should be 0 + .address(SEND_ALL_ETH_CONTRACT_ADDRESS) + .signAndBuild( + secp256k1.createKeyPair( + secp256k1.createPrivateKey(AUTHORIZER_PRIVATE_KEY.toUnsignedBigInteger()))); + + final Transaction tx = + Transaction.builder() + .type(TransactionType.SET_CODE) + .chainId(BigInteger.valueOf(20211)) + .nonce(0) + .maxPriorityFeePerGas(Wei.of(1000000000)) + .maxFeePerGas(Wei.fromHexString("0x02540BE400")) + .gasLimit(1000000) + .to(Address.fromHexStringStrict(authorizer.getAddress())) + .value(Wei.ZERO) + .payload(Bytes32.leftPad(Bytes.fromHexString(otherAccount.getAddress()))) + .accessList(List.of()) + .setCodeTransactionPayloads(List.of(authorization)) + .signAndBuild( + secp256k1.createKeyPair( + secp256k1.createPrivateKey(AUTHORIZER_PRIVATE_KEY.toUnsignedBigInteger()))); + + final String txHash = + besuNode.execute(ethTransactions.sendRawTransaction(tx.encoded().toHexString())); + testHelper.buildNewBlock(); + + Optional maybeTransactionReceipt = + besuNode.execute(ethTransactions.getTransactionReceipt(txHash)); + assertThat(maybeTransactionReceipt).isPresent(); + + // verify that the balance of the other account has not changed + cluster.verify(otherAccount.balanceEquals(0)); + + final String gasPriceWithout0x = + maybeTransactionReceipt.get().getEffectiveGasPrice().substring(2); + final BigInteger txCost = + maybeTransactionReceipt.get().getGasUsed().multiply(new BigInteger(gasPriceWithout0x, 16)); + BigInteger expectedSenderBalance = new BigInteger("90000000000000000000000").subtract(txCost); + cluster.verify(authorizer.balanceEquals(Amount.wei(expectedSenderBalance))); + } } diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/AuthorityProcessor.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/AuthorityProcessor.java index f79e2c98e3..c40eca0f74 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/AuthorityProcessor.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/AuthorityProcessor.java @@ -38,7 +38,7 @@ public class AuthorityProcessor { } public void addContractToAuthority( - final WorldUpdater worldUpdater, + final WorldUpdater worldState, final AuthorizedCodeService authorizedCodeService, final Transaction transaction) { @@ -60,7 +60,7 @@ public class AuthorityProcessor { } final Optional maybeAccount = - Optional.ofNullable(worldUpdater.getAccount(authorityAddress)); + Optional.ofNullable(worldState.getAccount(authorityAddress)); final long accountNonce = maybeAccount.map(AccountState::getNonce).orElse(0L); @@ -74,7 +74,7 @@ public class AuthorityProcessor { } Optional codeAccount = - Optional.ofNullable(worldUpdater.get(payload.address())); + Optional.ofNullable(worldState.get(payload.address())); final Bytes code; if (codeAccount.isPresent()) { code = codeAccount.get().getCode(); diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetTransactionProcessor.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetTransactionProcessor.java index 5fa2b119f1..cb9cd24e23 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetTransactionProcessor.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetTransactionProcessor.java @@ -306,7 +306,6 @@ public class MainnetTransactionProcessor { } final Address senderAddress = transaction.getSender(); - final MutableAccount sender = worldState.getOrCreateSenderAccount(senderAddress); validationResult = @@ -318,6 +317,19 @@ public class MainnetTransactionProcessor { operationTracer.tracePrepareTransaction(worldState, transaction); + final Set
addressList = new BytesTrieSet<>(Address.SIZE); + + if (transaction.getAuthorizationList().isPresent()) { + if (maybeAuthorityProcessor.isEmpty()) { + throw new RuntimeException("Authority processor is required for 7702 transactions"); + } + + maybeAuthorityProcessor + .get() + .addContractToAuthority(worldState, authorizedCodeService, transaction); + addressList.addAll(authorizedCodeService.getAuthorities()); + } + final long previousNonce = sender.incrementNonce(); LOG.trace( "Incremented sender {} nonce ({} -> {})", @@ -343,7 +355,6 @@ public class MainnetTransactionProcessor { final List accessListEntries = transaction.getAccessList().orElse(List.of()); // we need to keep a separate hash set of addresses in case they specify no storage. // No-storage is a common pattern, especially for Externally Owned Accounts - final Set
addressList = new BytesTrieSet<>(Address.SIZE); final Multimap storageList = HashMultimap.create(); int accessListStorageCount = 0; for (final var entry : accessListEntries) { @@ -408,15 +419,6 @@ public class MainnetTransactionProcessor { if (transaction.getVersionedHashes().isPresent()) { commonMessageFrameBuilder.versionedHashes( Optional.of(transaction.getVersionedHashes().get().stream().toList())); - } else if (transaction.getAuthorizationList().isPresent()) { - if (maybeAuthorityProcessor.isEmpty()) { - throw new RuntimeException("Authority processor is required for 7702 transactions"); - } - - maybeAuthorityProcessor - .get() - .addContractToAuthority(worldUpdater, authorizedCodeService, transaction); - addressList.addAll(authorizedCodeService.getAuthorities()); } else { commonMessageFrameBuilder.versionedHashes(Optional.empty()); }