From aa7a6dafc5647e7fcfc6eba97a7ac64f3feaeda3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Diego=20L=C3=B3pez=20Le=C3=B3n?= Date: Thu, 22 Dec 2022 13:28:00 -0300 Subject: [PATCH] [Shandong] Fix EOFv1 validation (#4711) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Add failing test for deploying invalid contract Signed-off-by: Diego López León Signed-off-by: Diego López León * Validate contractCode from frame Signed-off-by: Diego López León Signed-off-by: Diego López León * Only receive frame output data in ContractValidationRule#validate Signed-off-by: Diego López León Signed-off-by: Diego López León Signed-off-by: Diego López León Signed-off-by: Diego López León --- .../mainnet/MainnetProtocolSpecs.java | 5 ++- .../MainnetContractCreationProcessorTest.java | 31 +++++++++++++++++++ .../CachedInvalidCodeRule.java | 23 +++++++++++--- .../ContractValidationRule.java | 5 +-- .../contractvalidation/MaxCodeSizeRule.java | 6 ++-- .../contractvalidation/PrefixCodeRule.java | 6 ++-- .../processor/ContractCreationProcessor.java | 2 +- 7 files changed, 63 insertions(+), 15 deletions(-) diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetProtocolSpecs.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetProtocolSpecs.java index 5626eb06f3..f73d4b3333 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetProtocolSpecs.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/MainnetProtocolSpecs.java @@ -36,6 +36,7 @@ import org.hyperledger.besu.ethereum.privacy.PrivateTransactionProcessor; import org.hyperledger.besu.ethereum.privacy.PrivateTransactionValidator; import org.hyperledger.besu.ethereum.privacy.storage.PrivateMetadataUpdater; import org.hyperledger.besu.ethereum.processing.TransactionProcessingResult; +import org.hyperledger.besu.evm.EvmSpecVersion; import org.hyperledger.besu.evm.MainnetEVMs; import org.hyperledger.besu.evm.account.MutableAccount; import org.hyperledger.besu.evm.contractvalidation.CachedInvalidCodeRule; @@ -727,7 +728,9 @@ public abstract class MainnetProtocolSpecs { gasCalculator, evm, true, - List.of(MaxCodeSizeRule.of(contractSizeLimit), CachedInvalidCodeRule.of()), + List.of( + MaxCodeSizeRule.of(contractSizeLimit), + CachedInvalidCodeRule.of(EvmSpecVersion.SHANDONG)), 1, SPURIOUS_DRAGON_FORCE_DELETE_WHEN_EMPTY_ADDRESSES)) .name("Shandong"); diff --git a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/mainnet/MainnetContractCreationProcessorTest.java b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/mainnet/MainnetContractCreationProcessorTest.java index fe06b92a00..9357dadb20 100644 --- a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/mainnet/MainnetContractCreationProcessorTest.java +++ b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/mainnet/MainnetContractCreationProcessorTest.java @@ -19,8 +19,12 @@ import static org.hyperledger.besu.evm.frame.MessageFrame.State.COMPLETED_SUCCES import static org.hyperledger.besu.evm.frame.MessageFrame.State.EXCEPTIONAL_HALT; import static org.mockito.Mockito.when; +import org.hyperledger.besu.datatypes.Hash; import org.hyperledger.besu.ethereum.core.MessageFrameTestFixture; import org.hyperledger.besu.evm.EVM; +import org.hyperledger.besu.evm.EvmSpecVersion; +import org.hyperledger.besu.evm.code.CodeFactory; +import org.hyperledger.besu.evm.contractvalidation.CachedInvalidCodeRule; import org.hyperledger.besu.evm.contractvalidation.MaxCodeSizeRule; import org.hyperledger.besu.evm.contractvalidation.PrefixCodeRule; import org.hyperledger.besu.evm.frame.ExceptionalHaltReason; @@ -124,6 +128,33 @@ public class MainnetContractCreationProcessorTest { .contains(ExceptionalHaltReason.CODE_TOO_LARGE); } + @Test + public void shouldThrowAnExceptionWhenDeployingInvalidContract() { + EvmSpecVersion evmSpecVersion = EvmSpecVersion.SHANDONG; + processor = + new ContractCreationProcessor( + gasCalculator, + evm, + true, + Collections.singletonList(CachedInvalidCodeRule.of(evmSpecVersion)), + 1, + Collections.emptyList()); + final Bytes contractCreateCode = Bytes.fromHexString("0x67ef0001010001006060005260086018f3"); + final MessageFrame messageFrame = + new MessageFrameTestFixture() + .code( + CodeFactory.createCode( + contractCreateCode, + Hash.hash(contractCreateCode), + evmSpecVersion.getMaxEofVersion(), + true)) + .build(); + messageFrame.setOutputData(Bytes.fromHexString("0xef00010100010060")); + + processor.codeSuccess(messageFrame, OperationTracer.NO_TRACING); + assertThat(messageFrame.getState()).isEqualTo(EXCEPTIONAL_HALT); + } + @Test public void shouldNotThrowAnExceptionWhenCodeContractTooLarge() { processor = diff --git a/evm/src/main/java/org/hyperledger/besu/evm/contractvalidation/CachedInvalidCodeRule.java b/evm/src/main/java/org/hyperledger/besu/evm/contractvalidation/CachedInvalidCodeRule.java index d8b889a810..5cdf5f411d 100644 --- a/evm/src/main/java/org/hyperledger/besu/evm/contractvalidation/CachedInvalidCodeRule.java +++ b/evm/src/main/java/org/hyperledger/besu/evm/contractvalidation/CachedInvalidCodeRule.java @@ -16,23 +16,36 @@ package org.hyperledger.besu.evm.contractvalidation; +import org.hyperledger.besu.datatypes.Hash; +import org.hyperledger.besu.evm.Code; +import org.hyperledger.besu.evm.EvmSpecVersion; +import org.hyperledger.besu.evm.code.CodeFactory; import org.hyperledger.besu.evm.frame.ExceptionalHaltReason; -import org.hyperledger.besu.evm.frame.MessageFrame; import java.util.Optional; +import org.apache.tuweni.bytes.Bytes; + public class CachedInvalidCodeRule implements ContractValidationRule { + private final int maxEofVersion; + + public CachedInvalidCodeRule(final int maxEofVersion) { + this.maxEofVersion = maxEofVersion; + } + @Override - public Optional validate(final MessageFrame frame) { - if (!frame.getCode().isValid()) { + public Optional validate(final Bytes contractCode) { + final Code code = + CodeFactory.createCode(contractCode, Hash.hash(contractCode), maxEofVersion, false); + if (!code.isValid()) { return Optional.of(ExceptionalHaltReason.INVALID_CODE); } else { return Optional.empty(); } } - public static ContractValidationRule of() { - return new CachedInvalidCodeRule(); + public static ContractValidationRule of(final EvmSpecVersion specVersion) { + return new CachedInvalidCodeRule(specVersion.getMaxEofVersion()); } } diff --git a/evm/src/main/java/org/hyperledger/besu/evm/contractvalidation/ContractValidationRule.java b/evm/src/main/java/org/hyperledger/besu/evm/contractvalidation/ContractValidationRule.java index 61bd371d27..554a4d325d 100644 --- a/evm/src/main/java/org/hyperledger/besu/evm/contractvalidation/ContractValidationRule.java +++ b/evm/src/main/java/org/hyperledger/besu/evm/contractvalidation/ContractValidationRule.java @@ -15,12 +15,13 @@ package org.hyperledger.besu.evm.contractvalidation; import org.hyperledger.besu.evm.frame.ExceptionalHaltReason; -import org.hyperledger.besu.evm.frame.MessageFrame; import java.util.Optional; +import org.apache.tuweni.bytes.Bytes; + @FunctionalInterface public interface ContractValidationRule { - Optional validate(MessageFrame frame); + Optional validate(Bytes contractCode); } diff --git a/evm/src/main/java/org/hyperledger/besu/evm/contractvalidation/MaxCodeSizeRule.java b/evm/src/main/java/org/hyperledger/besu/evm/contractvalidation/MaxCodeSizeRule.java index d84bc56bde..caba87c3d0 100644 --- a/evm/src/main/java/org/hyperledger/besu/evm/contractvalidation/MaxCodeSizeRule.java +++ b/evm/src/main/java/org/hyperledger/besu/evm/contractvalidation/MaxCodeSizeRule.java @@ -15,10 +15,10 @@ package org.hyperledger.besu.evm.contractvalidation; import org.hyperledger.besu.evm.frame.ExceptionalHaltReason; -import org.hyperledger.besu.evm.frame.MessageFrame; import java.util.Optional; +import org.apache.tuweni.bytes.Bytes; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -33,8 +33,8 @@ public class MaxCodeSizeRule implements ContractValidationRule { } @Override - public Optional validate(final MessageFrame frame) { - final int contractCodeSize = frame.getOutputData().size(); + public Optional validate(final Bytes contractCode) { + final int contractCodeSize = contractCode.size(); if (contractCodeSize <= maxCodeSize) { return Optional.empty(); } else { diff --git a/evm/src/main/java/org/hyperledger/besu/evm/contractvalidation/PrefixCodeRule.java b/evm/src/main/java/org/hyperledger/besu/evm/contractvalidation/PrefixCodeRule.java index 3fb3c7cfbe..59df921e82 100644 --- a/evm/src/main/java/org/hyperledger/besu/evm/contractvalidation/PrefixCodeRule.java +++ b/evm/src/main/java/org/hyperledger/besu/evm/contractvalidation/PrefixCodeRule.java @@ -15,10 +15,10 @@ package org.hyperledger.besu.evm.contractvalidation; import org.hyperledger.besu.evm.frame.ExceptionalHaltReason; -import org.hyperledger.besu.evm.frame.MessageFrame; import java.util.Optional; +import org.apache.tuweni.bytes.Bytes; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -30,8 +30,8 @@ public class PrefixCodeRule implements ContractValidationRule { @Override // As per https://eips.ethereum.org/EIPS/eip-3541 - public Optional validate(final MessageFrame frame) { - if (!frame.getOutputData().isEmpty() && frame.getOutputData().get(0) == FORMAT_RESERVED) { + public Optional validate(final Bytes contractCode) { + if (!contractCode.isEmpty() && contractCode.get(0) == FORMAT_RESERVED) { LOG.trace("Contract creation error: code cannot start with {}", FORMAT_RESERVED); return Optional.of(ExceptionalHaltReason.INVALID_CODE); } else { diff --git a/evm/src/main/java/org/hyperledger/besu/evm/processor/ContractCreationProcessor.java b/evm/src/main/java/org/hyperledger/besu/evm/processor/ContractCreationProcessor.java index a21e26010e..2d9bbe6087 100644 --- a/evm/src/main/java/org/hyperledger/besu/evm/processor/ContractCreationProcessor.java +++ b/evm/src/main/java/org/hyperledger/besu/evm/processor/ContractCreationProcessor.java @@ -140,7 +140,7 @@ public class ContractCreationProcessor extends AbstractMessageProcessor { } else { final var invalidReason = contractValidationRules.stream() - .map(rule -> rule.validate(frame)) + .map(rule -> rule.validate(contractCode)) .filter(Optional::isPresent) .findFirst(); if (invalidReason.isEmpty()) {