From 86b9c38015590923c3c24d0599a17385c55c7332 Mon Sep 17 00:00:00 2001 From: Chaminda Divitotawela Date: Wed, 19 Jun 2024 10:28:05 +1000 Subject: [PATCH 1/3] container verify GitHub workflow (#7239) Container verification step in release process automated with the container verify GitHub workflow. New workflow is triggered at the end of the release workflow which will check the release container images starts successfully. Verification test only checks container starts and reach the Ethereum main loop Signed-off-by: Chaminda Divitotawela --- .github/workflows/BesuContainerVerify.sh | 70 ++++++++++++++++++++++++ .github/workflows/container-verify.yml | 57 +++++++++++++++++++ .github/workflows/release.yml | 14 +++++ 3 files changed, 141 insertions(+) create mode 100644 .github/workflows/BesuContainerVerify.sh create mode 100644 .github/workflows/container-verify.yml diff --git a/.github/workflows/BesuContainerVerify.sh b/.github/workflows/BesuContainerVerify.sh new file mode 100644 index 00000000000..81537f32648 --- /dev/null +++ b/.github/workflows/BesuContainerVerify.sh @@ -0,0 +1,70 @@ +#!/bin/bash +## +## 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 +## + +CONTAINER_NAME=${CONTAINER_NAME:-besu} +VERSION=${VERSION} +TAG=${TAG} +CHECK_LATEST=${CHECK_LATEST} +RETRY=${RETRY:-10} +SLEEP=${SLEEP:-5} + +# Helper function to throw error +log_error() { + echo "::error $1" + exit 1 +} + +# Check container is in running state +_RUN_STATE=$(docker inspect --type=container -f={{.State.Status}} ${CONTAINER_NAME}) +if [[ "${_RUN_STATE}" != "running" ]] +then + log_error "container is not running" +fi + +# Check for specific log message in container logs to verify besu started +_SUCCESS=false +while [[ ${_SUCCESS} != "true" && $RETRY -gt 0 ]] +do + docker logs ${CONTAINER_NAME} | grep -q "Ethereum main loop is up" && { + _SUCCESS=true + continue + } + echo "Waiting for the besu to start. Remaining retries $RETRY ..." + RETRY=$(expr $RETRY - 1) + sleep $SLEEP +done + +# Log entry does not present after all retries, fail the script with a message +if [[ ${_SUCCESS} != "true" ]] +then + docker logs --tail=100 ${CONTAINER_NAME} + log_error "could not find the log message 'Ethereum main loop is up'" +else + echo "Besu container started and entered main loop" +fi + +# For the latest tag check the version match +if [[ ${TAG} == "latest" && ${CHECK_LATEST} == "true" ]] +then + _VERSION_IN_LOG=$(docker logs ${CONTAINER_NAME} | grep "#" | grep "Besu version" | cut -d " " -f 4 | sed 's/\s//g') + echo "Extracted version from logs [$_VERSION_IN_LOG]" + if [[ "$_VERSION_IN_LOG" != "${VERSION}" ]] + then + log_error "version [$_VERSION_IN_LOG] extracted from container logs does not match the expected version [${VERSION}]" + else + echo "Latest Besu container version matches" + fi +fi diff --git a/.github/workflows/container-verify.yml b/.github/workflows/container-verify.yml new file mode 100644 index 00000000000..c8f5726af75 --- /dev/null +++ b/.github/workflows/container-verify.yml @@ -0,0 +1,57 @@ +name: container verify + +on: + workflow_dispatch: + inputs: + version: + description: 'Besu version' + required: true + verify-latest-version: + description: 'Check latest container version' + required: false + type: choice + default: "true" + options: + - "true" + - "false" + +jobs: + verify: + timeout-minutes: 4 + strategy: + matrix: + combination: + - tag: ${{ inputs.version }} + platform: '' + runner: ubuntu-latest + - tag: ${{ inputs.version }}-amd64 + platform: 'linux/amd64' + runner: ubuntu-latest + - tag: latest + platform: '' + runner: ubuntu-latest + - tag: ${{ inputs.version }}-arm64 + platform: '' + runner: besu-arm64 + runs-on: ${{ matrix.combination.runner }} + env: + CONTAINER_NAME: besu-check + steps: + - name: Checkout + uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 + + - name: Start container + run: | + PLATFORM_OPT="" + [[ x${{ matrix.combination.platform }} != 'x' ]] && PLATFORM_OPT="--platform ${{ matrix.combination.platform }}" + docker run -d $PLATFORM_OPT --name ${{ env.CONTAINER_NAME }} hyperledger/besu:${{ matrix.combination.tag }} + + - name: Verify besu container + run: bash .github/workflows/BesuContainerVerify.sh + env: + TAG: ${{ matrix.combination.tag }} + VERSION: ${{ inputs.version }} + CHECK_LATEST: ${{ inputs.verify-latest-version }} + + - name: Stop container + run: docker stop ${{ env.CONTAINER_NAME }} diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index b1f8fb2fc5e..2aff0bb48e5 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -265,3 +265,17 @@ jobs: run: ./gradlew "-Prelease.releaseVersion=${{ github.event.release.name }}" "-PdockerOrgName=${{ env.registry }}/${{ secrets.DOCKER_ORG }}" dockerUploadRelease - name: Docker manifest run: ./gradlew "-Prelease.releaseVersion=${{ github.event.release.name }}" "-PdockerOrgName=${{ env.registry }}/${{ secrets.DOCKER_ORG }}" manifestDockerRelease + + verifyContainer: + needs: dockerPromoteX64 + runs-on: ubuntu-22.04 + permissions: + contents: read + actions: write + steps: + - name: Checkout + uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 + - name: Trigger container verify + run: echo '{"version":"${{ github.event.release.name }}","verify-latest-version":"true"}' | gh workflow run container-verify.yml --json + env: + GH_TOKEN: ${{ github.token }} From f5e5ad53e770715f5438d3e9c74a77cfe8cfc97b Mon Sep 17 00:00:00 2001 From: Stefan Pingel <16143240+pinges@users.noreply.github.com> Date: Thu, 20 Jun 2024 14:54:20 +1000 Subject: [PATCH 2/3] Investigate chain halts when syncing (#7162) Fix some reasons for chain download halts when syncing Signed-off-by: stefan.pingel@consensys.net Signed-off-by: Stefan Pingel <16143240+pinges@users.noreply.github.com> Co-authored-by: Sally MacFarlane --- CHANGELOG.md | 2 + .../MergeBesuControllerBuilder.java | 2 +- .../besu/ethereum/eth/manager/EthPeers.java | 51 ++++++++++++------- .../ethereum/eth/manager/EthScheduler.java | 5 -- .../RetryingGetAccountRangeFromPeerTask.java | 6 ++- .../task/AbstractRetryingPeerTask.java | 11 ++-- .../sync/fastsync/PivotBlockRetriever.java | 2 +- .../eth/sync/fastsync/SyncTargetManager.java | 21 +++++--- .../fullsync/BetterSyncTargetEvaluator.java | 2 +- .../eth/sync/range/RangeHeadersFetcher.java | 38 +++++++++++--- .../eth/sync/range/SyncTargetRangeSource.java | 23 ++++++--- .../eth/sync/tasks/CompleteBlocksTask.java | 2 +- .../tasks/DownloadHeaderSequenceTask.java | 2 +- .../sync/tasks/GetReceiptsForHeadersTask.java | 2 +- .../BetterSyncTargetEvaluatorTest.java | 2 +- .../connections/AbstractPeerConnection.java | 23 +++++---- .../rlpx/wire/messages/DisconnectMessage.java | 1 + 17 files changed, 130 insertions(+), 65 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3724ed84a43..bac2d7c555d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,8 @@ ### Bug fixes - Validation errors ignored in accounts-allowlist and empty list [#7138](https://github.com/hyperledger/besu/issues/7138) - Fix "Invalid block detected" for BFT chains using Bonsai DB [#7204](https://github.com/hyperledger/besu/pull/7204) +- Fix "Could not confirm best peer had pivot block" [#7109](https://github.com/hyperledger/besu/issues/7109) +- Fix "Chain Download Halt" [#6884](https://github.com/hyperledger/besu/issues/6884) ## 24.6.0 diff --git a/besu/src/main/java/org/hyperledger/besu/controller/MergeBesuControllerBuilder.java b/besu/src/main/java/org/hyperledger/besu/controller/MergeBesuControllerBuilder.java index e391f920ceb..1e8da674a50 100644 --- a/besu/src/main/java/org/hyperledger/besu/controller/MergeBesuControllerBuilder.java +++ b/besu/src/main/java/org/hyperledger/besu/controller/MergeBesuControllerBuilder.java @@ -104,7 +104,7 @@ protected EthProtocolManager createEthProtocolManager( var mergeBestPeerComparator = new TransitionBestPeerComparator( genesisConfigOptions.getTerminalTotalDifficulty().map(Difficulty::of).orElseThrow()); - ethPeers.setBestChainComparator(mergeBestPeerComparator); + ethPeers.setBestPeerComparator(mergeBestPeerComparator); mergeContext.observeNewIsPostMergeState(mergeBestPeerComparator); Optional filterToUse = Optional.of(new MergePeerFilter()); diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java index 2a4469220e0..bef7a1a038f 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java @@ -198,9 +198,12 @@ private boolean registerDisconnect(final EthPeer peer, final PeerConnection conn peer.handleDisconnect(); abortPendingRequestsAssignedToDisconnectedPeers(); if (peer.getReputation().getScore() > USEFULL_PEER_SCORE_THRESHOLD) { - LOG.debug("Disconnected USEFULL peer {}", peer); + LOG.atDebug().setMessage("Disconnected USEFULL peer {}").addArgument(peer).log(); } else { - LOG.debug("Disconnected EthPeer {}", peer.getLoggableId()); + LOG.atDebug() + .setMessage("Disconnected EthPeer {}") + .addArgument(peer.getLoggableId()) + .log(); } } } @@ -318,11 +321,11 @@ public Stream streamAvailablePeers() { public Stream streamBestPeers() { return streamAvailablePeers() .filter(EthPeer::isFullyValidated) - .sorted(getBestChainComparator().reversed()); + .sorted(getBestPeerComparator().reversed()); } public Optional bestPeer() { - return streamAvailablePeers().max(getBestChainComparator()); + return streamAvailablePeers().max(getBestPeerComparator()); } public Optional bestPeerWithHeightEstimate() { @@ -331,15 +334,15 @@ public Optional bestPeerWithHeightEstimate() { } public Optional bestPeerMatchingCriteria(final Predicate matchesCriteria) { - return streamAvailablePeers().filter(matchesCriteria).max(getBestChainComparator()); + return streamAvailablePeers().filter(matchesCriteria).max(getBestPeerComparator()); } - public void setBestChainComparator(final Comparator comparator) { + public void setBestPeerComparator(final Comparator comparator) { LOG.info("Updating the default best peer comparator"); bestPeerComparator = comparator; } - public Comparator getBestChainComparator() { + public Comparator getBestPeerComparator() { return bestPeerComparator; } @@ -394,8 +397,7 @@ public boolean shouldConnect(final Peer peer, final boolean inbound) { public void disconnectWorstUselessPeer() { streamAvailablePeers() - .sorted(getBestChainComparator()) - .findFirst() + .min(getBestPeerComparator()) .ifPresent( peer -> { LOG.atDebug() @@ -551,10 +553,11 @@ private boolean addPeerToEthPeers(final EthPeer peer) { if (!randomPeerPriority) { // Disconnect if too many peers if (!canExceedPeerLimits(id) && peerCount() >= peerUpperBound) { - LOG.trace( - "Too many peers. Disconnect connection: {}, max connections {}", - connection, - peerUpperBound); + LOG.atTrace() + .setMessage("Too many peers. Disconnect connection: {}, max connections {}") + .addArgument(connection) + .addArgument(peerUpperBound) + .log(); connection.disconnect(DisconnectMessage.DisconnectReason.TOO_MANY_PEERS); return false; } @@ -562,18 +565,28 @@ private boolean addPeerToEthPeers(final EthPeer peer) { if (connection.inboundInitiated() && !canExceedPeerLimits(id) && remoteConnectionLimitReached()) { - LOG.trace( - "Too many remotely-initiated connections. Disconnect incoming connection: {}, maxRemote={}", - connection, - maxRemotelyInitiatedConnections); + LOG.atTrace() + .setMessage( + "Too many remotely-initiated connections. Disconnect incoming connection: {}, maxRemote={}") + .addArgument(connection) + .addArgument(maxRemotelyInitiatedConnections) + .log(); connection.disconnect(DisconnectMessage.DisconnectReason.TOO_MANY_PEERS); return false; } final boolean added = (completeConnections.putIfAbsent(id, peer) == null); if (added) { - LOG.trace("Added peer {} with connection {} to completeConnections", id, connection); + LOG.atTrace() + .setMessage("Added peer {} with connection {} to completeConnections") + .addArgument(id) + .addArgument(connection) + .log(); } else { - LOG.trace("Did not add peer {} with connection {} to completeConnections", id, connection); + LOG.atTrace() + .setMessage("Did not add peer {} with connection {} to completeConnections") + .addArgument(id) + .addArgument(connection) + .log(); } return added; } else { diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthScheduler.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthScheduler.java index 717a0bb24ef..dcb66696662 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthScheduler.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthScheduler.java @@ -47,7 +47,6 @@ public class EthScheduler { private static final Logger LOG = LoggerFactory.getLogger(EthScheduler.class); - private final Duration defaultTimeout = Duration.ofSeconds(5); private final AtomicBoolean stopped = new AtomicBoolean(false); private final CountDownLatch shutdown = new CountDownLatch(1); private static final int TX_WORKER_CAPACITY = 1_000; @@ -219,10 +218,6 @@ public CompletableFuture scheduleBlockCreationTask(final Runnable task) { return CompletableFuture.runAsync(task, blockCreationExecutor); } - public CompletableFuture timeout(final EthTask task) { - return timeout(task, defaultTimeout); - } - public CompletableFuture timeout(final EthTask task, final Duration timeout) { final CompletableFuture future = task.run(); final CompletableFuture result = timeout(future, timeout); diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetAccountRangeFromPeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetAccountRangeFromPeerTask.java index 103172b0870..36d6b75e6ee 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetAccountRangeFromPeerTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/snap/RetryingGetAccountRangeFromPeerTask.java @@ -30,6 +30,7 @@ public class RetryingGetAccountRangeFromPeerTask extends AbstractRetryingPeerTask { + public static final int MAX_RETRIES = 4; private final EthContext ethContext; private final Bytes32 startKeyHash; private final Bytes32 endKeyHash; @@ -43,7 +44,10 @@ private RetryingGetAccountRangeFromPeerTask( final BlockHeader blockHeader, final MetricsSystem metricsSystem) { super( - ethContext, 4, data -> data.accounts().isEmpty() && data.proofs().isEmpty(), metricsSystem); + ethContext, + MAX_RETRIES, + data -> data.accounts().isEmpty() && data.proofs().isEmpty(), + metricsSystem); this.ethContext = ethContext; this.startKeyHash = startKeyHash; this.endKeyHash = endKeyHash; diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingPeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingPeerTask.java index cf48d69847d..46c3cf1b226 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingPeerTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/AbstractRetryingPeerTask.java @@ -122,15 +122,18 @@ protected void handleTaskError(final Throwable error) { () -> ethContext .getScheduler() + // wait for a new peer for up to 5 seconds .timeout(waitTask, Duration.ofSeconds(5)) + // execute the task again .whenComplete((r, t) -> executeTaskTimed())); return; } - LOG.debug( - "Retrying after recoverable failure from peer task {}: {}", - this.getClass().getSimpleName(), - cause.getMessage()); + LOG.atDebug() + .setMessage("Retrying after recoverable failure from peer task {}: {}") + .addArgument(this.getClass().getSimpleName()) + .addArgument(cause.getMessage()) + .log(); // Wait before retrying on failure executeSubTask( () -> diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/PivotBlockRetriever.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/PivotBlockRetriever.java index 6ddd7db9012..d8ff3627101 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/PivotBlockRetriever.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/PivotBlockRetriever.java @@ -39,7 +39,7 @@ public class PivotBlockRetriever { private static final Logger LOG = LoggerFactory.getLogger(PivotBlockRetriever.class); - public static final int MAX_QUERY_RETRIES_PER_PEER = 4; + public static final int MAX_QUERY_RETRIES_PER_PEER = 5; private static final int DEFAULT_MAX_PIVOT_BLOCK_RESETS = 250; private static final int SUSPICIOUS_NUMBER_OF_RETRIES = 5; diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/SyncTargetManager.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/SyncTargetManager.java index c19e06f993b..b587b18f264 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/SyncTargetManager.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/SyncTargetManager.java @@ -31,6 +31,7 @@ import org.hyperledger.besu.ethereum.worldstate.WorldStateStorageCoordinator; import org.hyperledger.besu.plugin.services.MetricsSystem; +import java.time.Duration; import java.util.List; import java.util.Optional; import java.util.concurrent.CompletableFuture; @@ -41,6 +42,7 @@ public class SyncTargetManager extends AbstractSyncTargetManager { private static final Logger LOG = LoggerFactory.getLogger(SyncTargetManager.class); + private static final int SECONDS_PER_REQUEST = 6; // 5s per request + 1s wait between retries private final WorldStateStorageCoordinator worldStateStorageCoordinator; private final ProtocolSchedule protocolSchedule; @@ -93,7 +95,9 @@ protected CompletableFuture> selectBestAvailableSyncTarget() { return completedFuture(Optional.empty()); } else { final EthPeer bestPeer = maybeBestPeer.get(); - if (bestPeer.chainState().getEstimatedHeight() < pivotBlockHeader.getNumber()) { + // Do not check the best peers estimated height if we are doing PoS + if (!protocolSchedule.getByBlockHeader(pivotBlockHeader).isPoS() + && bestPeer.chainState().getEstimatedHeight() < pivotBlockHeader.getNumber()) { LOG.info( "Best peer {} has chain height {} below pivotBlock height {}. Waiting for better peers. Current {} of max {}", maybeBestPeer.map(EthPeer::getLoggableId).orElse("none"), @@ -121,7 +125,8 @@ private CompletableFuture> confirmPivotBlockHeader(final EthPe task.assignPeer(bestPeer); return ethContext .getScheduler() - .timeout(task) + // Task is a retrying task. Make sure that the timeout is long enough to allow for retries. + .timeout(task, Duration.ofSeconds(MAX_QUERY_RETRIES_PER_PEER * SECONDS_PER_REQUEST + 2)) .thenCompose( result -> { if (peerHasDifferentPivotBlock(result)) { @@ -147,11 +152,13 @@ private CompletableFuture> confirmPivotBlockHeader(final EthPe }) .exceptionally( error -> { - LOG.debug( - "Could not confirm best peer {} had pivot block {}", - bestPeer.getLoggableId(), - pivotBlockHeader.getNumber(), - error); + LOG.atDebug() + .setMessage("Could not confirm best peer {} had pivot block {}, {}") + .addArgument(bestPeer.getLoggableId()) + .addArgument(pivotBlockHeader.getNumber()) + .addArgument(error) + .log(); + bestPeer.disconnect(DisconnectReason.USELESS_PEER_CANNOT_CONFIRM_PIVOT_BLOCK); return Optional.empty(); }); } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/BetterSyncTargetEvaluator.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/BetterSyncTargetEvaluator.java index 68aa5ec95f8..1df59c654c1 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/BetterSyncTargetEvaluator.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/BetterSyncTargetEvaluator.java @@ -40,7 +40,7 @@ public boolean shouldSwitchSyncTarget(final EthPeer currentSyncTarget) { return maybeBestPeer .map( bestPeer -> { - if (ethPeers.getBestChainComparator().compare(bestPeer, currentSyncTarget) <= 0) { + if (ethPeers.getBestPeerComparator().compare(bestPeer, currentSyncTarget) <= 0) { // Our current target is better or equal to the best peer return false; } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/range/RangeHeadersFetcher.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/range/RangeHeadersFetcher.java index 8ee1dc28a43..e73c5aa9f5f 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/range/RangeHeadersFetcher.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/range/RangeHeadersFetcher.java @@ -68,6 +68,10 @@ public RangeHeadersFetcher( public CompletableFuture> getNextRangeHeaders( final EthPeer peer, final BlockHeader previousRangeHeader) { + LOG.atTrace() + .setMessage("Requesting next range headers from peer {}") + .addArgument(peer.getLoggableId()) + .log(); final int skip = syncConfig.getDownloaderChainSegmentSize() - 1; final int maximumHeaderRequestSize = syncConfig.getDownloaderHeaderRequestSize(); final long previousRangeNumber = previousRangeHeader.getNumber(); @@ -78,11 +82,20 @@ public CompletableFuture> getNextRangeHeaders( final BlockHeader targetHeader = finalRangeHeader.get(); final long blocksUntilTarget = targetHeader.getNumber() - previousRangeNumber; if (blocksUntilTarget <= 0) { + LOG.atTrace() + .setMessage("Requesting next range headers: no blocks until target: {}") + .addArgument(blocksUntilTarget) + .log(); return completedFuture(emptyList()); } final long maxHeadersToRequest = blocksUntilTarget / (skip + 1); additionalHeaderCount = (int) Math.min(maxHeadersToRequest, maximumHeaderRequestSize); if (additionalHeaderCount == 0) { + LOG.atTrace() + .setMessage( + "Requesting next range headers: additional header count is 0, blocks until target: {}") + .addArgument(blocksUntilTarget) + .log(); return completedFuture(singletonList(targetHeader)); } } else { @@ -97,11 +110,12 @@ private CompletableFuture> requestHeaders( final BlockHeader referenceHeader, final int headerCount, final int skip) { - LOG.trace( - "Requesting {} range headers, starting from {}, {} blocks apart", - headerCount, - referenceHeader.getNumber(), - skip); + LOG.atTrace() + .setMessage("Requesting {} range headers, starting from {}, {} blocks apart") + .addArgument(headerCount) + .addArgument(referenceHeader.getNumber()) + .addArgument(skip) + .log(); return GetHeadersFromPeerByHashTask.startingAtHash( protocolSchedule, ethContext, @@ -114,7 +128,19 @@ private CompletableFuture> requestHeaders( .assignPeer(peer) .run() .thenApply(PeerTaskResult::getResult) - .thenApply(headers -> stripExistingRangeHeaders(referenceHeader, headers)); + .thenApply( + headers -> { + if (headers.size() < headerCount) { + LOG.atTrace() + .setMessage( + "Peer {} returned fewer headers than requested. Expected: {}, Actual: {}") + .addArgument(peer) + .addArgument(headerCount) + .addArgument(headers.size()) + .log(); + } + return stripExistingRangeHeaders(referenceHeader, headers); + }); } private List stripExistingRangeHeaders( diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/range/SyncTargetRangeSource.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/range/SyncTargetRangeSource.java index 8c6186448ed..05d7899226b 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/range/SyncTargetRangeSource.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/range/SyncTargetRangeSource.java @@ -40,6 +40,7 @@ public class SyncTargetRangeSource implements Iterator { private static final Logger LOG = LoggerFactory.getLogger(SyncTargetRangeSource.class); private static final Duration RETRY_DELAY_DURATION = Duration.ofSeconds(2); + public static final int DEFAULT_TIME_TO_WAIT_IN_SECONDS = 6; private final RangeHeadersFetcher fetcher; private final SyncTargetChecker syncTargetChecker; @@ -70,7 +71,7 @@ public SyncTargetRangeSource( peer, commonAncestor, retriesPermitted, - Duration.ofSeconds(5), + Duration.ofSeconds(DEFAULT_TIME_TO_WAIT_IN_SECONDS), terminationCondition); } @@ -153,7 +154,7 @@ private SyncTargetRange getRangeFromPendingRequest() { if (retryCount >= retriesPermitted) { LOG.atDebug() .setMessage( - "Disconnecting target peer for providing useless or empty range header: {}.") + "Disconnecting target peer {} for providing useless or empty range headers.") .addArgument(peer) .log(); peer.disconnect(DisconnectMessage.DisconnectReason.USELESS_PEER_USELESS_RESPONSES); @@ -169,12 +170,20 @@ private SyncTargetRange getRangeFromPendingRequest() { } catch (final InterruptedException e) { LOG.trace("Interrupted while waiting for new range headers", e); return null; - } catch (final ExecutionException e) { - LOG.debug("Failed to retrieve new range headers", e); - this.pendingRequests = Optional.empty(); + } catch (final ExecutionException | TimeoutException e) { + if (e instanceof ExecutionException) { + this.pendingRequests = Optional.empty(); + } retryCount++; - return null; - } catch (final TimeoutException e) { + if (retryCount >= retriesPermitted) { + LOG.atDebug() + .setMessage( + "Disconnecting target peer {} for not providing useful range headers: Exception: {}.") + .addArgument(peer) + .addArgument(e) + .log(); + peer.disconnect(DisconnectMessage.DisconnectReason.USELESS_PEER_USELESS_RESPONSES); + } return null; } } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/tasks/CompleteBlocksTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/tasks/CompleteBlocksTask.java index bdd59260e72..524851af5f6 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/tasks/CompleteBlocksTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/tasks/CompleteBlocksTask.java @@ -51,7 +51,7 @@ public class CompleteBlocksTask extends AbstractRetryingPeerTask> { private static final Logger LOG = LoggerFactory.getLogger(CompleteBlocksTask.class); private static final int MIN_SIZE_INCOMPLETE_LIST = 1; - private static final int DEFAULT_RETRIES = 4; + private static final int DEFAULT_RETRIES = 5; private final EthContext ethContext; private final ProtocolSchedule protocolSchedule; diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/tasks/DownloadHeaderSequenceTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/tasks/DownloadHeaderSequenceTask.java index 3049d3f1ac2..8ca376902e4 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/tasks/DownloadHeaderSequenceTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/tasks/DownloadHeaderSequenceTask.java @@ -56,7 +56,7 @@ */ public class DownloadHeaderSequenceTask extends AbstractRetryingPeerTask> { private static final Logger LOG = LoggerFactory.getLogger(DownloadHeaderSequenceTask.class); - private static final int DEFAULT_RETRIES = 4; + private static final int DEFAULT_RETRIES = 5; private final EthContext ethContext; private final ProtocolContext protocolContext; diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/tasks/GetReceiptsForHeadersTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/tasks/GetReceiptsForHeadersTask.java index 699bf9c4b17..58c4d3a7afa 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/tasks/GetReceiptsForHeadersTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/tasks/GetReceiptsForHeadersTask.java @@ -42,7 +42,7 @@ public class GetReceiptsForHeadersTask extends AbstractRetryingPeerTask>> { private static final Logger LOG = LoggerFactory.getLogger(GetReceiptsForHeadersTask.class); - private static final int DEFAULT_RETRIES = 4; + private static final int DEFAULT_RETRIES = 5; private final EthContext ethContext; diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/BetterSyncTargetEvaluatorTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/BetterSyncTargetEvaluatorTest.java index 9acf45965d6..0bc98aaa9ad 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/BetterSyncTargetEvaluatorTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/BetterSyncTargetEvaluatorTest.java @@ -49,7 +49,7 @@ public class BetterSyncTargetEvaluatorTest { @BeforeEach public void setupMocks() { - when(ethPeers.getBestChainComparator()).thenReturn(EthPeers.HEAVIEST_CHAIN); + when(ethPeers.getBestPeerComparator()).thenReturn(EthPeers.HEAVIEST_CHAIN); } @Test diff --git a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/AbstractPeerConnection.java b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/AbstractPeerConnection.java index 7fcc02687db..59cf3490566 100644 --- a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/AbstractPeerConnection.java +++ b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/connections/AbstractPeerConnection.java @@ -185,15 +185,20 @@ public void terminateConnection(final DisconnectReason reason, final boolean pee @Override public void disconnect(final DisconnectReason reason) { if (disconnected.compareAndSet(false, true)) { - connectionEventDispatcher.dispatchDisconnect(this, reason, false); - doSend(null, DisconnectMessage.create(reason)); - LOG.atDebug() - .setMessage("Disconnecting connection {}, peer {} reason {}") - .addArgument(this.hashCode()) - .addArgument(peer.getLoggableId()) - .addArgument(reason) - .log(); - closeConnection(); + try { + // send the disconnect message first, in case the dispatchDisconnect throws an exception + doSend(null, DisconnectMessage.create(reason)); + LOG.atDebug() + .setMessage("Disconnecting connection {}, peer {} reason {}") + .addArgument(this.hashCode()) + .addArgument(peer.getLoggableId()) + .addArgument(reason) + .log(); + connectionEventDispatcher.dispatchDisconnect(this, reason, false); + } finally { + // always close the connection + closeConnection(); + } } } diff --git a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/wire/messages/DisconnectMessage.java b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/wire/messages/DisconnectMessage.java index 66337903358..87df88c40ea 100644 --- a/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/wire/messages/DisconnectMessage.java +++ b/ethereum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/rlpx/wire/messages/DisconnectMessage.java @@ -131,6 +131,7 @@ public enum DisconnectReason { USELESS_PEER_MISMATCHED_PIVOT_BLOCK((byte) 0x03, "Mismatched pivot block"), USELESS_PEER_FAILED_TO_RETRIEVE_CHAIN_STATE( (byte) 0x03, "Failed to retrieve header for chain state"), + USELESS_PEER_CANNOT_CONFIRM_PIVOT_BLOCK((byte) 0x03, "Peer failed to confirm pivot block"), USELESS_PEER_BY_REPUTATION((byte) 0x03, "Lowest reputation score"), USELESS_PEER_BY_CHAIN_COMPARATOR((byte) 0x03, "Lowest by chain height comparator"), TOO_MANY_PEERS((byte) 0x04), From 3026268f1542da4d329cf016e81b6455d82ddc61 Mon Sep 17 00:00:00 2001 From: Danno Ferrin Date: Thu, 20 Jun 2024 10:20:40 -0600 Subject: [PATCH 3/3] Check for EOFCreate subcontainer rules (#7232) Check and test for the unused container rule, and only returncontract targets can have truncated data rule. Also test the other subcontainer rules in unit tests. Signed-off-by: Danno Ferrin --- .../ethereum/eof/EOFReferenceTestTools.java | 18 + .../besu/evm/code/CodeV1Validation.java | 12 + .../hyperledger/besu/evm/code/EOFLayout.java | 10 + .../besu/evm/code/CodeFactoryTest.java | 438 +++++++++++++++++- .../besu/evm/code/EOFLayoutTest.java | 16 +- 5 files changed, 480 insertions(+), 14 deletions(-) diff --git a/ethereum/referencetests/src/reference-test/java/org/hyperledger/besu/ethereum/eof/EOFReferenceTestTools.java b/ethereum/referencetests/src/reference-test/java/org/hyperledger/besu/ethereum/eof/EOFReferenceTestTools.java index cf7ce66b076..22bf3839010 100644 --- a/ethereum/referencetests/src/reference-test/java/org/hyperledger/besu/ethereum/eof/EOFReferenceTestTools.java +++ b/ethereum/referencetests/src/reference-test/java/org/hyperledger/besu/ethereum/eof/EOFReferenceTestTools.java @@ -72,6 +72,24 @@ public class EOFReferenceTestTools { // TXCREATE still in tests, but has been removed params.ignore("EOF1_undefined_opcodes_186"); + + // embedded containers rules changed + params.ignore("EOF1_embedded_container"); + + // truncated data is only allowed in embedded containers + params.ignore("ori/validInvalid-Prague\\[validInvalid_48\\]"); + params.ignore("efExample/validInvalid-Prague\\[validInvalid_1\\]"); + params.ignore("efValidation/EOF1_truncated_section-Prague\\[EOF1_truncated_section_3\\]"); + params.ignore("efValidation/EOF1_truncated_section-Prague\\[EOF1_truncated_section_4\\]"); + params.ignore("EIP3540/validInvalid-Prague\\[validInvalid_2\\]"); + params.ignore("EIP3540/validInvalid-Prague\\[validInvalid_3\\]"); + + // Orphan containers are no longer allowed + params.ignore("efValidation/EOF1_returncontract_valid-Prague\\[EOF1_returncontract_valid_1\\]"); + params.ignore("efValidation/EOF1_returncontract_valid-Prague\\[EOF1_returncontract_valid_2\\]"); + params.ignore("efValidation/EOF1_eofcreate_valid-Prague\\[EOF1_eofcreate_valid_1\\]"); + params.ignore("efValidation/EOF1_eofcreate_valid-Prague\\[EOF1_eofcreate_valid_2\\]"); + params.ignore("efValidation/EOF1_section_order-Prague\\[EOF1_section_order_6\\]"); } private EOFReferenceTestTools() { diff --git a/evm/src/main/java/org/hyperledger/besu/evm/code/CodeV1Validation.java b/evm/src/main/java/org/hyperledger/besu/evm/code/CodeV1Validation.java index b52f1dfebbd..c940a7d930b 100644 --- a/evm/src/main/java/org/hyperledger/besu/evm/code/CodeV1Validation.java +++ b/evm/src/main/java/org/hyperledger/besu/evm/code/CodeV1Validation.java @@ -67,6 +67,8 @@ private CodeV1Validation() { * @param layout The parsed EOFLayout of the code * @return either null, indicating no error, or a String describing the validation error. */ + @SuppressWarnings( + "ReferenceEquality") // comparison `container != layout` is deliberate and correct public static String validate(final EOFLayout layout) { Queue workList = new ArrayDeque<>(layout.getSubcontainerCount()); workList.add(layout); @@ -74,6 +76,16 @@ public static String validate(final EOFLayout layout) { while (!workList.isEmpty()) { EOFLayout container = workList.poll(); workList.addAll(List.of(container.subContainers())); + if (container != layout && container.containerMode().get() == null) { + return "Unreferenced container #" + layout.indexOfSubcontainer(container); + } + if (container.containerMode().get() != RUNTIME + && container.data().size() != container.dataLength()) { + return "Incomplete data section " + + (container == layout + ? " at root" + : " in container #" + layout.indexOfSubcontainer(container)); + } final String codeValidationError = CodeV1Validation.validateCode(container); if (codeValidationError != null) { diff --git a/evm/src/main/java/org/hyperledger/besu/evm/code/EOFLayout.java b/evm/src/main/java/org/hyperledger/besu/evm/code/EOFLayout.java index 95ad7f95dd2..1cce49b4531 100644 --- a/evm/src/main/java/org/hyperledger/besu/evm/code/EOFLayout.java +++ b/evm/src/main/java/org/hyperledger/besu/evm/code/EOFLayout.java @@ -405,6 +405,16 @@ public EOFLayout getSubcontainer(final int i) { return subContainers[i]; } + /** + * Finds the first instance of the subcontainer in the list of container, or -1 if not present + * + * @param container the container to search for + * @return the index of the container, or -1 if not found. + */ + public int indexOfSubcontainer(final EOFLayout container) { + return Arrays.asList(subContainers).indexOf(container); + } + /** * Is valid. * diff --git a/evm/src/test/java/org/hyperledger/besu/evm/code/CodeFactoryTest.java b/evm/src/test/java/org/hyperledger/besu/evm/code/CodeFactoryTest.java index d3335d077cf..75ec9eb01bf 100644 --- a/evm/src/test/java/org/hyperledger/besu/evm/code/CodeFactoryTest.java +++ b/evm/src/test/java/org/hyperledger/besu/evm/code/CodeFactoryTest.java @@ -15,10 +15,10 @@ package org.hyperledger.besu.evm.code; import static org.assertj.core.api.Assertions.assertThat; +import static org.hyperledger.besu.evm.EOFTestConstants.bytesFromPrettyPrint; import org.hyperledger.besu.evm.Code; -import org.apache.tuweni.bytes.Bytes; import org.junit.jupiter.api.Test; class CodeFactoryTest { @@ -178,13 +178,445 @@ void invalidCodeUnknownSectionId3() { invalidCode("0xEF0001010002030004006000AABBCCDD"); } + @Test + void invalidDataTruncated() { + invalidCode("EF0001 010004 0200010001 040003 00 00800000 FE BEEF", "Incomplete data section"); + } + + @Test + void validComboEOFCreateReturnContract() { + validCode( + """ + 0x # EOF + ef0001 # Magic and Version ( 1 ) + 010004 # Types length ( 4 ) + 020001 # Total code sections ( 1 ) + 0011 # Code section 0 , 17 bytes + 030001 # Total subcontainers ( 1 ) + 0032 # Sub container 0, 50 byte + 040000 # Data section length( 0 ) + 00 # Terminator (end of header) + # Code section 0 types + 00 # 0 inputs\s + 80 # 0 outputs (Non-returning function) + 0004 # max stack: 4 + # Code section 0 - in=0 out=non-returning height=4 + 6000 # [0] PUSH1(0) + 6000 # [2] PUSH1(0) + 6000 # [4] PUSH1(0) + 6000 # [6] PUSH1(0) + ec00 # [8] EOFCREATE(0) + 612015 # [10] PUSH2(0x2015) + 6001 # [13] PUSH1(1) + 55 # [15] SSTORE + 00 # [16] STOP + # Subcontainer 0 starts here + ef0001 # Magic and Version ( 1 ) + 010004 # Types length ( 4 ) + 020001 # Total code sections ( 1 ) + 0006 # Code section 0 , 6 bytes + 030001 # Total subcontainers ( 1 ) + 0014 # Sub container 0, 20 byte + 040000 # Data section length( 0 ) \s + 00 # Terminator (end of header) + # Code section 0 types + 00 # 0 inputs\s + 80 # 0 outputs (Non-returning function) + 0002 # max stack: 2 + # Code section 0 - in=0 out=non-returning height=2 + 6000 # [0] PUSH1(0) + 6000 # [2] PUSH1(0) + ee00 # [4] RETURNCONTRACT(0) + # Subcontainer 0.0 starts here + ef0001 # Magic and Version ( 1 ) + 010004 # Types length ( 4 ) + 020001 # Total code sections ( 1 ) + 0001 # Code section 0 , 1 bytes + 040000 # Data section length( 0 ) \s + 00 # Terminator (end of header) + # Code section 0 types + 00 # 0 inputs\s + 80 # 0 outputs (Non-returning function) + 0000 # max stack: 0 + # Code section 0 - in=0 out=non-returning height=0 + 00 # [0] STOP + # Data section (empty) + # Subcontainer 0.0 ends + # Data section (empty) + # Subcontainer 0 ends + # Data section (empty) + """); + } + + @Test + void validComboReturnContactStop() { + validCode( + """ + 0x # EOF + ef0001 # Magic and Version ( 1 ) + 010004 # Types length ( 4 ) + 020001 # Total code sections ( 1 ) + 000c # Code section 0 , 12 bytes + 030001 # Total subcontainers ( 1 ) + 0014 # Sub container 0, 20 byte + 040000 # Data section length( 0 ) + 00 # Terminator (end of header) + # Code section 0 types + 00 # 0 inputs\s + 80 # 0 outputs (Non-returning function) + 0002 # max stack: 2 + # Code section 0 - in=0 out=non-returning height=2 + 612015 # [0] PUSH2(0x2015) + 6001 # [3] PUSH1(1) + 55 # [5] SSTORE + 6000 # [6] PUSH1(0) + 6000 # [8] PUSH1(0) + ee00 # [10] RETURNCONTRACT(0) + # Subcontainer 0 starts here + ef0001 # Magic and Version ( 1 ) + 010004 # Types length ( 4 ) + 020001 # Total code sections ( 1 ) + 0001 # Code section 0 , 1 bytes + 040000 # Data section length( 0 ) \s + 00 # Terminator (end of header) + # Code section 0 types + 00 # 0 inputs\s + 80 # 0 outputs (Non-returning function) + 0000 # max stack: 0 + # Code section 0 - in=0 out=non-returning height=0 + 00 # [0] STOP + # Data section (empty) + # Subcontainer 0 ends + # Data section (empty) + """); + } + + @Test + void validComboReturnContactReturn() { + validCode( + """ + 0x # EOF + ef0001 # Magic and Version ( 1 ) + 010004 # Types length ( 4 ) + 020001 # Total code sections ( 1 ) + 000c # Code section 0 , 12 bytes + 030001 # Total subcontainers ( 1 ) + 0018 # Sub container 0, 24 byte + 040000 # Data section length( 0 ) + 00 # Terminator (end of header) + # Code section 0 types + 00 # 0 inputs\s + 80 # 0 outputs (Non-returning function) + 0002 # max stack: 2 + # Code section 0 - in=0 out=non-returning height=2 + 612015 # [0] PUSH2(0x2015) + 6001 # [3] PUSH1(1) + 55 # [5] SSTORE + 6000 # [6] PUSH1(0) + 6000 # [8] PUSH1(0) + ee00 # [10] RETURNCONTRACT(0) + # Subcontainer 0 starts here + ef0001 # Magic and Version ( 1 ) + 010004 # Types length ( 4 ) + 020001 # Total code sections ( 1 ) + 0005 # Code section 0 , 5 bytes + 040000 # Data section length( 0 ) \s + 00 # Terminator (end of header) + # Code section 0 types + 00 # 0 inputs\s + 80 # 0 outputs (Non-returning function) + 0002 # max stack: 2 + # Code section 0 - in=0 out=non-returning height=2 + 6000 # [0] PUSH1(0) + 6000 # [2] PUSH1(0) + f3 # [4] RETURN + # Data section (empty) + # Subcontainer 0 ends + # Data section (empty) + """); + } + + @Test + void validComboEOFCreateRevert() { + validCode( + """ + 0x # EOF + ef0001 # Magic and Version ( 1 ) + 010004 # Types length ( 4 ) + 020001 # Total code sections ( 1 ) + 0011 # Code section 0 , 17 bytes + 030001 # Total subcontainers ( 1 ) + 0018 # Sub container 0, 24 byte + 040000 # Data section length( 0 ) + 00 # Terminator (end of header) + # Code section 0 types + 00 # 0 inputs\s + 80 # 0 outputs (Non-returning function) + 0004 # max stack: 4 + # Code section 0 - in=0 out=non-returning height=4 + 6000 # [0] PUSH1(0) + 6000 # [2] PUSH1(0) + 6000 # [4] PUSH1(0) + 6000 # [6] PUSH1(0) + ec00 # [8] EOFCREATE(0) + 612015 # [10] PUSH2(0x2015) + 6001 # [13] PUSH1(1) + 55 # [15] SSTORE + 00 # [16] STOP + # Subcontainer 0 starts here + ef0001 # Magic and Version ( 1 ) + 010004 # Types length ( 4 ) + 020001 # Total code sections ( 1 ) + 0005 # Code section 0 , 5 bytes + 040000 # Data section length( 0 ) \s + 00 # Terminator (end of header) + # Code section 0 types + 00 # 0 inputs\s + 80 # 0 outputs (Non-returning function) + 0002 # max stack: 2 + # Code section 0 - in=0 out=non-returning height=2 + 6000 # [0] PUSH1(0) + 6000 # [2] PUSH1(0) + fd # [4] REVERT + # Data section (empty) + # Subcontainer 0 ends + # Data section (empty) + """); + } + + @Test + void validComboReturncontractRevert() { + validCode( + """ + 0x # EOF + ef0001 # Magic and Version ( 1 ) + 010004 # Types length ( 4 ) + 020001 # Total code sections ( 1 ) + 000c # Code section 0 , 12 bytes + 030001 # Total subcontainers ( 1 ) + 0018 # Sub container 0, 24 byte + 040000 # Data section length( 0 ) + 00 # Terminator (end of header) + # Code section 0 types + 00 # 0 inputs\s + 80 # 0 outputs (Non-returning function) + 0002 # max stack: 2 + # Code section 0 - in=0 out=non-returning height=2 + 612015 # [0] PUSH2(0x2015) + 6001 # [3] PUSH1(1) + 55 # [5] SSTORE + 6000 # [6] PUSH1(0) + 6000 # [8] PUSH1(0) + ee00 # [10] RETURNCONTRACT(0) + # Subcontainer 0 starts here + ef0001 # Magic and Version ( 1 ) + 010004 # Types length ( 4 ) + 020001 # Total code sections ( 1 ) + 0005 # Code section 0 , 5 bytes + 040000 # Data section length( 0 ) \s + 00 # Terminator (end of header) + # Code section 0 types + 00 # 0 inputs\s + 80 # 0 outputs (Non-returning function) + 0002 # max stack: 2 + # Code section 0 - in=0 out=non-returning height=2 + 6000 # [0] PUSH1(0) + 6000 # [2] PUSH1(0) + fd # [4] REVERT + # Data section (empty) + # Subcontainer 0 ends + # Data section (empty) + """); + } + + @Test + void invalidComboEOFCreateStop() { + invalidCode( + """ + 0x # EOF + ef0001 # Magic and Version ( 1 ) + 010004 # Types length ( 4 ) + 020001 # Total code sections ( 1 ) + 0011 # Code section 0 , 17 bytes + 030001 # Total subcontainers ( 1 ) + 0014 # Sub container 0, 20 byte + 040000 # Data section length( 0 ) + 00 # Terminator (end of header) + # Code section 0 types + 00 # 0 inputs\s + 80 # 0 outputs (Non-returning function) + 0004 # max stack: 4 + # Code section 0 - in=0 out=non-returning height=4 + 6000 # [0] PUSH1(0) + 6000 # [2] PUSH1(0) + 6000 # [4] PUSH1(0) + 6000 # [6] PUSH1(0) + ec00 # [8] EOFCREATE(0) + 612015 # [10] PUSH2(0x2015) + 6001 # [13] PUSH1(1) + 55 # [15] SSTORE + 00 # [16] STOP + # Subcontainer 0 starts here + ef0001 # Magic and Version ( 1 ) + 010004 # Types length ( 4 ) + 020001 # Total code sections ( 1 ) + 0001 # Code section 0 , 1 bytes + 040000 # Data section length( 0 ) \s + 00 # Terminator (end of header) + # Code section 0 types + 00 # 0 inputs\s + 80 # 0 outputs (Non-returning function) + 0000 # max stack: 0 + # Code section 0 - in=0 out=non-returning height=0 + 00 # [0] STOP + # Data section (empty) + # Subcontainer 0 ends + # Data section (empty) + """, + "STOP is only a valid opcode in containers used for runtime operations."); + } + + @Test + void invalidComboEOFCretateReturn() { + invalidCode( + """ + 0x # EOF + ef0001 # Magic and Version ( 1 ) + 010004 # Types length ( 4 ) + 020001 # Total code sections ( 1 ) + 0011 # Code section 0 , 17 bytes + 030001 # Total subcontainers ( 1 ) + 0018 # Sub container 0, 24 byte + 040000 # Data section length( 0 ) + 00 # Terminator (end of header) + # Code section 0 types + 00 # 0 inputs\s + 80 # 0 outputs (Non-returning function) + 0004 # max stack: 4 + # Code section 0 - in=0 out=non-returning height=4 + 6000 # [0] PUSH1(0) + 6000 # [2] PUSH1(0) + 6000 # [4] PUSH1(0) + 6000 # [6] PUSH1(0) + ec00 # [8] EOFCREATE(0) + 612015 # [10] PUSH2(0x2015) + 6001 # [13] PUSH1(1) + 55 # [15] SSTORE + 00 # [16] STOP + # Subcontainer 0 starts here + ef0001 # Magic and Version ( 1 ) + 010004 # Types length ( 4 ) + 020001 # Total code sections ( 1 ) + 0005 # Code section 0 , 5 bytes + 040000 # Data section length( 0 ) \s + 00 # Terminator (end of header) + # Code section 0 types + 00 # 0 inputs\s + 80 # 0 outputs (Non-returning function) + 0002 # max stack: 2 + # Code section 0 - in=0 out=non-returning height=2 + 6000 # [0] PUSH1(0) + 6000 # [2] PUSH1(0) + f3 # [4] RETURN + # Data section (empty) + # Subcontainer 0 ends + # Data section (empty) + """, + "RETURN is only a valid opcode in containers used for runtime operations."); + } + + @Test + void invalidReturncontractReturncontract() { + invalidCode( + """ + 0x # EOF + ef0001 # Magic and Version ( 1 ) + 010004 # Types length ( 4 ) + 020001 # Total code sections ( 1 ) + 000c # Code section 0 , 12 bytes + 030001 # Total subcontainers ( 1 ) + 0032 # Sub container 0, 50 byte + 040000 # Data section length( 0 ) + 00 # Terminator (end of header) + # Code section 0 types + 00 # 0 inputs\s + 80 # 0 outputs (Non-returning function) + 0002 # max stack: 2 + # Code section 0 - in=0 out=non-returning height=2 + 612015 # [0] PUSH2(0x2015) + 6001 # [3] PUSH1(1) + 55 # [5] SSTORE + 6000 # [6] PUSH1(0) + 6000 # [8] PUSH1(0) + ee00 # [10] RETURNCONTRACT(0) + # Subcontainer 0 starts here + ef0001 # Magic and Version ( 1 ) + 010004 # Types length ( 4 ) + 020001 # Total code sections ( 1 ) + 0006 # Code section 0 , 6 bytes + 030001 # Total subcontainers ( 1 ) + 0014 # Sub container 0, 20 byte + 040000 # Data section length( 0 ) \s + 00 # Terminator (end of header) + # Code section 0 types + 00 # 0 inputs\s + 80 # 0 outputs (Non-returning function) + 0002 # max stack: 2 + # Code section 0 - in=0 out=non-returning height=2 + 6000 # [0] PUSH1(0) + 6000 # [2] PUSH1(0) + ee00 # [4] RETURNCONTRACT(0) + # Subcontainer 0.0 starts here + ef0001 # Magic and Version ( 1 ) + 010004 # Types length ( 4 ) + 020001 # Total code sections ( 1 ) + 0001 # Code section 0 , 1 bytes + 040000 # Data section length( 0 ) \s + 00 # Terminator (end of header) + # Code section 0 types + 00 # 0 inputs\s + 80 # 0 outputs (Non-returning function) + 0000 # max stack: 0 + # Code section 0 - in=0 out=non-returning height=0 + 00 # [0] STOP + # Data section (empty) + # Subcontainer 0.0 ends + # Data section (empty) + # Subcontainer 0 ends + # Data section (empty) + """, + "RETURNCONTRACT is only a valid opcode in containers used for initcode"); + } + + // // valid subcontainer references + // // invalid subcontainer references + // + // { + // "EF0001 010004 0200010001 040003 00 00800000 FE BEEF", + // "Incomplete data section", + // "Incomplete data section", + // 1 + // }, + // + + private static void validCode(final String str) { + Code code = CodeFactory.createCode(bytesFromPrettyPrint(str), 1); + assertThat(code.isValid()).isTrue(); + } + + private static void invalidCode(final String str, final String error) { + Code code = CodeFactory.createCode(bytesFromPrettyPrint(str), 1); + assertThat(code.isValid()).isFalse(); + assertThat(((CodeInvalid) code).getInvalidReason()).contains(error); + } + private static void invalidCode(final String str) { - Code code = CodeFactory.createCode(Bytes.fromHexString(str), 1); + Code code = CodeFactory.createCode(bytesFromPrettyPrint(str), 1); assertThat(code.isValid()).isFalse(); } private static void invalidCode(final String str, final boolean legacy) { - Code code = CodeFactory.createCode(Bytes.fromHexString(str), 1, legacy, false); + Code code = CodeFactory.createCode(bytesFromPrettyPrint(str), 1, legacy, false); assertThat(code.isValid()).isFalse(); } } diff --git a/evm/src/test/java/org/hyperledger/besu/evm/code/EOFLayoutTest.java b/evm/src/test/java/org/hyperledger/besu/evm/code/EOFLayoutTest.java index 94f1d9598e0..5324d7adf05 100644 --- a/evm/src/test/java/org/hyperledger/besu/evm/code/EOFLayoutTest.java +++ b/evm/src/test/java/org/hyperledger/besu/evm/code/EOFLayoutTest.java @@ -143,16 +143,10 @@ public static Collection containersWithFormatErrors() { }, { "EF0001 010004 0200010001 040003 00 00800000 FE DEADBEEF", - "Incomplete data section", + "Excess data section", "Dangling data after end of all sections", 1 }, - { - "EF0001 010004 0200010001 040003 00 00800000 FE BEEF", - "Incomplete data section", - "Incomplete data section", - 1 - }, { "EF0001 0200010001 040001 00 FE DA", "type section missing", @@ -343,9 +337,9 @@ public static Collection subContainers() { @ParameterizedTest(name = "{1}") @MethodSource({ - // "correctContainers", - // "containersWithFormatErrors", - // "typeSectionTests", + "correctContainers", + "containersWithFormatErrors", + "typeSectionTests", "subContainers" }) void test( @@ -354,7 +348,7 @@ void test( final String failureReason, final int expectedVersion) { final Bytes container = Bytes.fromHexString(containerString.replaceAll("[^a-fxA-F0-9]", "")); - final EOFLayout layout = EOFLayout.parseEOF(container); + final EOFLayout layout = EOFLayout.parseEOF(container, true); assertThat(layout.version()).isEqualTo(expectedVersion); assertThat(layout.invalidReason()).isEqualTo(failureReason);