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 }} diff --git a/CHANGELOG.md b/CHANGELOG.md index 9204ec3238a..ab172e17f3d 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), 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);