From 21d5c54d58ae9f65d8215afdf28423c4d4dda9cf Mon Sep 17 00:00:00 2001 From: davidkngo <32287546+davidkngo@users.noreply.github.com> Date: Fri, 21 Jul 2023 16:29:20 +0700 Subject: [PATCH] Revert "Add hooks to AbstractCreateOperation for library users (#5656)" This reverts commit 2c9cdd9fcbc4a558a897f68373814e3ad0b2b286. --- CHANGELOG.md | 2 - .../operation/AbstractCreateOperation.java | 44 +--- .../AbstractCreateOperationTest.java | 235 ------------------ 3 files changed, 1 insertion(+), 280 deletions(-) delete mode 100644 evm/src/test/java/org/hyperledger/besu/evm/operation/AbstractCreateOperationTest.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 87b5be8d08d..54a4ae5c926 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,3 @@ - # Changelog ## 23.4.5 @@ -8,7 +7,6 @@ ### Additions and Improvements - EvmTool now executes the `execution-spec-tests` via the `t8n` and `b11r`. See the [README](ethereum/evmtool/README.md) in EvmTool for more instructions. - Improve lifecycle management of the transaction pool [#5634](https://github.com/hyperledger/besu/pull/5634) -- Add extension points in AbstractCreateOperation for EVM libraries to react to contract creations [#5656](https://github.com/hyperledger/besu/pull/5656) ### Bug Fixes - Use the node's configuration to determine if DNS enode URLs are allowed in calls to `admin_addPeer` and `admin_removePeer` [#5584](https://github.com/hyperledger/besu/pull/5584) diff --git a/evm/src/main/java/org/hyperledger/besu/evm/operation/AbstractCreateOperation.java b/evm/src/main/java/org/hyperledger/besu/evm/operation/AbstractCreateOperation.java index f3d04f772e7..5ea9737299f 100644 --- a/evm/src/main/java/org/hyperledger/besu/evm/operation/AbstractCreateOperation.java +++ b/evm/src/main/java/org/hyperledger/besu/evm/operation/AbstractCreateOperation.java @@ -22,14 +22,11 @@ import org.hyperledger.besu.evm.EVM; import org.hyperledger.besu.evm.account.MutableAccount; import org.hyperledger.besu.evm.code.CodeFactory; -import org.hyperledger.besu.evm.code.CodeInvalid; import org.hyperledger.besu.evm.frame.ExceptionalHaltReason; import org.hyperledger.besu.evm.frame.MessageFrame; import org.hyperledger.besu.evm.gascalculator.GasCalculator; import org.hyperledger.besu.evm.internal.Words; -import java.util.Optional; - import org.apache.tuweni.bytes.Bytes; /** The Abstract create operation. */ @@ -188,57 +185,18 @@ private void complete(final MessageFrame frame, final MessageFrame childFrame, f if (childFrame.getState() == MessageFrame.State.COMPLETED_SUCCESS) { frame.mergeWarmedUpFields(childFrame); - Address createdAddress = childFrame.getContractAddress(); - frame.pushStackItem(Words.fromAddress(createdAddress)); - onSuccess(frame, createdAddress); + frame.pushStackItem(Words.fromAddress(childFrame.getContractAddress())); } else { frame.setReturnData(childFrame.getOutputData()); frame.pushStackItem(FAILURE_STACK_ITEM); - onFailure(frame, childFrame.getExceptionalHaltReason()); } } else { frame.getWorldUpdater().deleteAccount(childFrame.getRecipientAddress()); frame.setReturnData(childFrame.getOutputData()); frame.pushStackItem(FAILURE_STACK_ITEM); - onInvalid(frame, (CodeInvalid) outputCode); } final int currentPC = frame.getPC(); frame.setPC(currentPC + 1); } - - /** - * Called when the child {@code CONTRACT_CREATION} message has completed successfully, used to - * give library users a chance to do implementation specific logic. - * - * @param frame the frame running the successful operation - * @param createdAddress the address of the newly created contract - */ - protected void onSuccess(final MessageFrame frame, final Address createdAddress) { - // no-op by default - } - - /** - * Called when the child {@code CONTRACT_CREATION} message has failed to execute, used to give - * library users a chance to do implementation specific logic. - * - * @param frame the frame running the successful operation - * @param haltReason the exceptional halt reason of the child frame - */ - protected void onFailure( - final MessageFrame frame, final Optional haltReason) { - // no-op by default - } - - /** - * Called when the child {@code CONTRACT_CREATION} message has completed successfully but the - * returned contract is invalid per chain rules, used to give library users a chance to do - * implementation specific logic. - * - * @param frame the frame running the successful operation - * @param invalidCode the code object containing the invalid code - */ - protected void onInvalid(final MessageFrame frame, final CodeInvalid invalidCode) { - // no-op by default - } } diff --git a/evm/src/test/java/org/hyperledger/besu/evm/operation/AbstractCreateOperationTest.java b/evm/src/test/java/org/hyperledger/besu/evm/operation/AbstractCreateOperationTest.java deleted file mode 100644 index cccd5194878..00000000000 --- a/evm/src/test/java/org/hyperledger/besu/evm/operation/AbstractCreateOperationTest.java +++ /dev/null @@ -1,235 +0,0 @@ -/* - * 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.evm.operation; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; - -import org.hyperledger.besu.datatypes.Address; -import org.hyperledger.besu.datatypes.Hash; -import org.hyperledger.besu.datatypes.Wei; -import org.hyperledger.besu.evm.EVM; -import org.hyperledger.besu.evm.MainnetEVMs; -import org.hyperledger.besu.evm.account.Account; -import org.hyperledger.besu.evm.account.MutableAccount; -import org.hyperledger.besu.evm.code.CodeFactory; -import org.hyperledger.besu.evm.code.CodeInvalid; -import org.hyperledger.besu.evm.frame.BlockValues; -import org.hyperledger.besu.evm.frame.ExceptionalHaltReason; -import org.hyperledger.besu.evm.frame.MessageFrame; -import org.hyperledger.besu.evm.gascalculator.ConstantinopleGasCalculator; -import org.hyperledger.besu.evm.gascalculator.GasCalculator; -import org.hyperledger.besu.evm.internal.EvmConfiguration; -import org.hyperledger.besu.evm.internal.Words; -import org.hyperledger.besu.evm.processor.ContractCreationProcessor; -import org.hyperledger.besu.evm.tracing.OperationTracer; -import org.hyperledger.besu.evm.worldstate.WorldUpdater; -import org.hyperledger.besu.evm.worldstate.WrappedEvmAccount; - -import java.util.ArrayDeque; -import java.util.List; -import java.util.Optional; - -import org.apache.tuweni.bytes.Bytes; -import org.apache.tuweni.units.bigints.UInt256; -import org.junit.jupiter.api.Test; - -class AbstractCreateOperationTest { - - private final WorldUpdater worldUpdater = mock(WorldUpdater.class); - private final WrappedEvmAccount account = mock(WrappedEvmAccount.class); - private final WrappedEvmAccount newAccount = mock(WrappedEvmAccount.class); - private final MutableAccount mutableAccount = mock(MutableAccount.class); - private final MutableAccount newMutableAccount = mock(MutableAccount.class); - private final FakeCreateOperation operation = - new FakeCreateOperation(new ConstantinopleGasCalculator(), Integer.MAX_VALUE); - - private static final Bytes SIMPLE_CREATE = - Bytes.fromHexString( - "0x" - + "6000" // PUSH1 0x00 - + "6000" // PUSH1 0x00 - + "F3" // RETURN - ); - private static final Bytes POP_UNDERFLOW_CREATE = - Bytes.fromHexString( - "0x" - + "50" // POP (but empty stack) - + "6000" // PUSH1 0x00 - + "6000" // PUSH1 0x00 - + "F3" // RETURN - ); - public static final Bytes INVALID_EOF = - Bytes.fromHexString( - "0x" - + "73EF99010100040200010001030000000000000000" // PUSH20 contract - + "6000" // PUSH1 0x00 - + "52" // MSTORE - + "6014" // PUSH1 20 - + "600c" // PUSH1 12 - + "F3" // RETURN - ); - public static final String SENDER = "0xdeadc0de00000000000000000000000000000000"; - - /** The Create operation. */ - public static class FakeCreateOperation extends AbstractCreateOperation { - - private MessageFrame successFrame; - private Address successCreatedAddress; - private MessageFrame failureFrame; - private Optional failureHaltReason; - private MessageFrame invalidFrame; - private CodeInvalid invalidInvalidCode; - - /** - * Instantiates a new Create operation. - * - * @param gasCalculator the gas calculator - * @param maxInitcodeSize Maximum init code size - */ - public FakeCreateOperation(final GasCalculator gasCalculator, final int maxInitcodeSize) { - super(0xEF, "FAKECREATE", 3, 1, gasCalculator, maxInitcodeSize); - } - - @Override - public long cost(final MessageFrame frame) { - return gasCalculator().createOperationGasCost(frame); - } - - @Override - protected Address targetContractAddress(final MessageFrame frame) { - final Account sender = frame.getWorldUpdater().get(frame.getRecipientAddress()); - // Decrement nonce by 1 to normalize the effect of transaction execution - final Address address = - Address.contractAddress(frame.getRecipientAddress(), sender.getNonce() - 1L); - frame.warmUpAddress(address); - return address; - } - - @Override - protected void onSuccess(final MessageFrame frame, final Address createdAddress) { - successFrame = frame; - successCreatedAddress = createdAddress; - } - - @Override - protected void onFailure( - final MessageFrame frame, final Optional haltReason) { - failureFrame = frame; - failureHaltReason = haltReason; - } - - @Override - protected void onInvalid(final MessageFrame frame, final CodeInvalid invalidCode) { - invalidFrame = frame; - invalidInvalidCode = invalidCode; - } - } - - private void executeOperation(final Bytes contract, final EVM evm) { - final UInt256 memoryOffset = UInt256.fromHexString("0xFF"); - final ArrayDeque messageFrameStack = new ArrayDeque<>(); - final MessageFrame messageFrame = - MessageFrame.builder() - .type(MessageFrame.Type.CONTRACT_CREATION) - .contract(Address.ZERO) - .inputData(Bytes.EMPTY) - .sender(Address.fromHexString(SENDER)) - .value(Wei.ZERO) - .apparentValue(Wei.ZERO) - .code(CodeFactory.createCode(SIMPLE_CREATE, 0, true)) - .depth(1) - .completer(__ -> {}) - .address(Address.fromHexString(SENDER)) - .blockHashLookup(n -> Hash.hash(Words.longBytes(n))) - .blockValues(mock(BlockValues.class)) - .gasPrice(Wei.ZERO) - .messageFrameStack(messageFrameStack) - .miningBeneficiary(Address.ZERO) - .originator(Address.ZERO) - .initialGas(100000L) - .worldUpdater(worldUpdater) - .build(); - messageFrame.pushStackItem(Bytes.ofUnsignedLong(contract.size())); - messageFrame.pushStackItem(memoryOffset); - messageFrame.pushStackItem(Bytes.EMPTY); - messageFrame.expandMemory(0, 500); - messageFrame.writeMemory(memoryOffset.trimLeadingZeros().toInt(), contract.size(), contract); - - when(account.getMutable()).thenReturn(mutableAccount); - when(account.getNonce()).thenReturn(55L); - when(mutableAccount.getBalance()).thenReturn(Wei.ZERO); - when(worldUpdater.getAccount(any())).thenReturn(account); - when(worldUpdater.get(any())).thenReturn(account); - when(worldUpdater.getSenderAccount(any())).thenReturn(account); - when(worldUpdater.getOrCreate(any())).thenReturn(newAccount); - when(newAccount.getMutable()).thenReturn(newMutableAccount); - when(newMutableAccount.getCode()).thenReturn(Bytes.EMPTY); - when(worldUpdater.updater()).thenReturn(worldUpdater); - - operation.execute(messageFrame, evm); - final MessageFrame createFrame = messageFrameStack.peek(); - final ContractCreationProcessor ccp = - new ContractCreationProcessor(evm.getGasCalculator(), evm, false, List.of(), 0, List.of()); - ccp.process(createFrame, OperationTracer.NO_TRACING); - } - - @Test - void onSuccess() { - final EVM evm = MainnetEVMs.london(EvmConfiguration.DEFAULT); - - executeOperation(SIMPLE_CREATE, evm); - - assertThat(operation.successFrame).isNotNull(); - assertThat(operation.successCreatedAddress) - .isEqualTo(Address.fromHexString("0xecccb0113190dfd26a044a7f26f45152a4270a64")); - assertThat(operation.failureFrame).isNull(); - assertThat(operation.failureHaltReason).isNull(); - assertThat(operation.invalidFrame).isNull(); - assertThat(operation.invalidInvalidCode).isNull(); - } - - @Test - void onFailure() { - final EVM evm = MainnetEVMs.london(EvmConfiguration.DEFAULT); - - executeOperation(POP_UNDERFLOW_CREATE, evm); - - assertThat(operation.successFrame).isNull(); - assertThat(operation.successCreatedAddress).isNull(); - assertThat(operation.failureFrame).isNotNull(); - assertThat(operation.failureHaltReason) - .contains(ExceptionalHaltReason.INSUFFICIENT_STACK_ITEMS); - assertThat(operation.invalidFrame).isNull(); - assertThat(operation.invalidInvalidCode).isNull(); - } - - @Test - void onInvalid() { - final EVM evm = MainnetEVMs.futureEips(EvmConfiguration.DEFAULT); - - executeOperation(INVALID_EOF, evm); - - assertThat(operation.successFrame).isNull(); - assertThat(operation.successCreatedAddress).isNull(); - assertThat(operation.failureFrame).isNull(); - assertThat(operation.failureHaltReason).isNull(); - assertThat(operation.invalidFrame).isNotNull(); - assertThat(operation.invalidInvalidCode).isNotNull(); - } -}