diff --git a/CHANGELOG.md b/CHANGELOG.md index 4a2b86f087b..dc6f1fbc461 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,7 +18,6 @@ and in case a rollback is needed, before installing a previous version, the migr - Add healing flat db mechanism with experimental CLI options `--Xsnapsync-synchronizer-flat-db-healing-enabled=true` [#5319](https://github.com/hyperledger/besu/pull/5319) ### Bug Fixes -- Fix backwards sync bug where chain is rolled back too far, especially when restarting Nimbus [#5497](https://github.com/hyperledger/besu/pull/5497) - Check to ensure storage and transactions are not closed prior to reading/writing [#5527](https://github.com/hyperledger/besu/pull/5527) - Fix the unavailability of account code and storage on GraphQl/Bonsai [#5548](https://github.com/hyperledger/besu/pull/5548) diff --git a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinator.java b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinator.java index e069448d0ba..19ed8e2afb4 100644 --- a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinator.java +++ b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinator.java @@ -467,7 +467,7 @@ public Optional getOrSyncHeadByHash(final Hash headHash, final Hash .setMessage("Appending new head block hash {} to backward sync") .addArgument(headHash::toHexString) .log(); - backwardSyncContext.maybeUpdateTargetHeight(headHash); + backwardSyncContext.updateHead(headHash); backwardSyncContext .syncBackwardsUntil(headHash) .thenRun(() -> updateFinalized(finalizedHash)); diff --git a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthGetLogs.java b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthGetLogs.java index 506aa7549e7..7561191adfc 100644 --- a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthGetLogs.java +++ b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthGetLogs.java @@ -53,7 +53,6 @@ public String getName() { @Override public JsonRpcResponse response(final JsonRpcRequestContext requestContext) { final FilterParameter filter = requestContext.getRequiredParameter(0, FilterParameter.class); - LOG.atTrace().setMessage("eth_getLogs FilterParameter: {}").addArgument(filter).log(); if (!filter.isValid()) { return new JsonRpcErrorResponse( diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/GetBodiesFromPeerTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/GetBodiesFromPeerTask.java index 9e1df148c12..c5a2d1b727a 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/GetBodiesFromPeerTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/GetBodiesFromPeerTask.java @@ -84,11 +84,6 @@ public static GetBodiesFromPeerTask forHeaders( protected PendingPeerRequest sendRequest() { final List blockHashes = headers.stream().map(BlockHeader::getHash).collect(Collectors.toList()); - LOG.atTrace() - .setMessage("Requesting {} bodies with hashes {}.") - .addArgument(blockHashes.size()) - .addArgument(blockHashes) - .log(); final long minimumRequiredBlockNumber = headers.get(headers.size() - 1).getNumber(); return sendRequestToPeer( @@ -133,13 +128,6 @@ protected Optional> processResponse( // Clear processed headers headers.clear(); } - LOG.atTrace() - .setMessage("Associated {} bodies with {} headers to get {} blocks with these hashes: {}") - .addArgument(bodies.size()) - .addArgument(headers.size()) - .addArgument(blocks.size()) - .addArgument(() -> blocks.stream().map(Block::toLogString).toList()) - .log(); return Optional.of(blocks); } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/RetryingGetHeadersEndingAtFromPeerByHashTask.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/RetryingGetHeadersEndingAtFromPeerByHashTask.java index 5b5b0cef359..e467bfba119 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/RetryingGetHeadersEndingAtFromPeerByHashTask.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/task/RetryingGetHeadersEndingAtFromPeerByHashTask.java @@ -92,8 +92,8 @@ protected CompletableFuture> executeTaskOnCurrentPeer( return executeSubTask(task::run) .thenApply( peerResult -> { - LOG.trace( - "Got {} block headers by hash {} from peer {} has result {}", + LOG.debug( + "Get {} block headers by hash {} from peer {} has result {}", count, referenceHash, currentPeer, diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/BackwardSyncContext.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/BackwardSyncContext.java index fc02409ce78..bc618e7c133 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/BackwardSyncContext.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/BackwardSyncContext.java @@ -55,6 +55,7 @@ public class BackwardSyncContext { private final AtomicReference currentBackwardSyncStatus = new AtomicReference<>(); private final BackwardChain backwardChain; private int batchSize = BATCH_SIZE; + private Optional maybeHead = Optional.empty(); private final int maxRetries; private final long millisBetweenRetries = DEFAULT_MILLIS_BETWEEN_RETRIES; private final Subscribers badChainListeners = Subscribers.create(); @@ -100,21 +101,17 @@ public synchronized boolean isSyncing() { .orElse(Boolean.FALSE); } - public synchronized void maybeUpdateTargetHeight(final Hash headHash) { - if (!Hash.ZERO.equals(headHash)) { + public synchronized void updateHead(final Hash headHash) { + if (Hash.ZERO.equals(headHash)) { + maybeHead = Optional.empty(); + } else { + maybeHead = Optional.of(headHash); Optional maybeCurrentStatus = Optional.ofNullable(currentBackwardSyncStatus.get()); maybeCurrentStatus.ifPresent( status -> backwardChain .getBlock(headHash) - .ifPresent( - block -> { - LOG.atTrace() - .setMessage("updateTargetHeight to {}") - .addArgument(block::toLogString) - .log(); - status.updateTargetHeight(block.getHeader().getNumber()); - })); + .ifPresent(block -> status.updateTargetHeight(block.getHeader().getNumber()))); } } @@ -334,12 +331,20 @@ protected Void saveBlock(final Block block) { @VisibleForTesting protected void possiblyMoveHead(final Block lastSavedBlock) { final MutableBlockchain blockchain = getProtocolContext().getBlockchain(); + if (maybeHead.isEmpty()) { + LOG.debug("Nothing to do with the head"); + return; + } - if (blockchain.getChainHead().getHash().equals(lastSavedBlock.getHash())) { - LOG.atDebug() - .setMessage("Head is already properly set to {}") - .addArgument(lastSavedBlock::toLogString) - .log(); + final Hash head = maybeHead.get(); + if (blockchain.getChainHead().getHash().equals(head)) { + LOG.debug("Head is already properly set"); + return; + } + + if (blockchain.contains(head)) { + LOG.debug("Changing head to {}", head); + blockchain.rewindToBlock(head); return; } @@ -437,6 +442,10 @@ public boolean progressLogDue() { return false; } + public CompletableFuture getCurrentFuture() { + return currentFuture; + } + public long getTargetChainHeight() { return targetChainHeight; } diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/BackwardSyncContextTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/BackwardSyncContextTest.java index 7c744c6b520..80bfbba1035 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/BackwardSyncContextTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/backwardsync/BackwardSyncContextTest.java @@ -59,7 +59,6 @@ import java.util.concurrent.ExecutionException; import javax.annotation.Nonnull; -import org.apache.tuweni.bytes.Bytes; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -257,45 +256,13 @@ private Block getBlockByNumber(final int number) { } @Test - public void shouldMoveHead() { - final Block lastSavedBlock = localBlockchain.getBlockByNumber(4).orElseThrow(); - context.possiblyMoveHead(lastSavedBlock); + public void testUpdatingHead() { + context.updateHead(localBlockchain.getBlockByNumber(4).orElseThrow().getHash()); + context.possiblyMoveHead(null); assertThat(localBlockchain.getChainHeadBlock().getHeader().getNumber()).isEqualTo(4); } - @Test - public void shouldNotMoveHeadWhenAlreadyHead() { - final Block lastSavedBlock = localBlockchain.getBlockByNumber(25).orElseThrow(); - context.possiblyMoveHead(lastSavedBlock); - - assertThat(localBlockchain.getChainHeadBlock().getHeader().getNumber()).isEqualTo(25); - } - - @Test - public void shouldUpdateTargetHeightWhenStatusPresent() { - // Given - BlockHeader blockHeader = Mockito.mock(BlockHeader.class); - when(blockHeader.getParentHash()).thenReturn(Hash.fromHexStringLenient("0x41")); - when(blockHeader.getHash()).thenReturn(Hash.fromHexStringLenient("0x42")); - when(blockHeader.getNumber()).thenReturn(42L); - Block unknownBlock = Mockito.mock(Block.class); - when(unknownBlock.getHeader()).thenReturn(blockHeader); - when(unknownBlock.getHash()).thenReturn(Hash.fromHexStringLenient("0x42")); - when(unknownBlock.toRlp()).thenReturn(Bytes.EMPTY); - context.syncBackwardsUntil(unknownBlock); // set the status - assertThat(context.getStatus().getTargetChainHeight()).isEqualTo(42); - final Hash backwardChainHash = - remoteBlockchain.getBlockByNumber(LOCAL_HEIGHT + 4).get().getHash(); - final Block backwardChainBlock = backwardChain.getTrustedBlock(backwardChainHash); - - // When - context.maybeUpdateTargetHeight(backwardChainBlock.getHash()); - - // Then - assertThat(context.getStatus().getTargetChainHeight()).isEqualTo(29); - } - @Test public void shouldProcessExceptionsCorrectly() { assertThatThrownBy(