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 6d5a3c8a62..e69277f140 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 @@ -23,7 +23,6 @@ import org.hyperledger.besu.ethereum.ProtocolContext; import org.hyperledger.besu.ethereum.chain.BadBlockManager; import org.hyperledger.besu.ethereum.chain.MutableBlockchain; import org.hyperledger.besu.ethereum.core.Block; -import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.eth.manager.EthContext; import org.hyperledger.besu.ethereum.eth.sync.state.SyncState; import org.hyperledger.besu.ethereum.mainnet.HeaderValidationMode; @@ -253,7 +252,6 @@ public class BackwardSyncContext { protected Void saveBlock(final Block block) { traceLambda(LOG, "Going to validate block {}", block::toLogString); - checkFinalizedSuccessionRuleBeforeSave(block); var optResult = this.getBlockValidatorForBlock(block) .validateAndProcessBlock( @@ -285,63 +283,6 @@ public class BackwardSyncContext { return null; } - @VisibleForTesting - protected synchronized void checkFinalizedSuccessionRuleBeforeSave(final Block block) { - final Optional finalized = findMaybeFinalized(); - if (finalized.isPresent()) { - final Optional maybeFinalizedHeader = - protocolContext - .getBlockchain() - .getBlockByHash(finalized.get()) - .map(Block::getHeader) - .or(() -> backwardChain.getHeader(finalized.get())); - if (maybeFinalizedHeader.isEmpty()) { - throw new BackwardSyncException( - "We know a block " - + finalized.get().toHexString() - + " was finalized, but we don't have it downloaded yet, cannot save new block", - true); - } - final BlockHeader finalizedHeader = maybeFinalizedHeader.get(); - if (finalizedHeader.getHash().equals(block.getHash())) { - debugLambda(LOG, "Saving new finalized block {}", block::toLogString); - return; - } - - if (finalizedHeader.getNumber() == block.getHeader().getNumber()) { - throw new BackwardSyncException( - "This block is not the target finalized block. Is " - + block.toLogString() - + " but was expecting " - + finalizedHeader.toLogString()); - } - if (!getProtocolContext().getBlockchain().contains(finalizedHeader.getHash())) { - debugLambda( - LOG, - "Saving block {} before finalized {} reached", - block::toLogString, - finalizedHeader::toLogString); // todo: some check here?? - return; - } - final Hash canonicalHash = - getProtocolContext() - .getBlockchain() - .getBlockByNumber(finalizedHeader.getNumber()) - .orElseThrow() - .getHash(); - if (finalizedHeader.getNumber() < block.getHeader().getNumber() - && !canonicalHash.equals(finalizedHeader.getHash())) { - throw new BackwardSyncException( - "Finalized block " - + finalizedHeader.toLogString() - + " is not on canonical chain. Canonical is" - + canonicalHash.toHexString() - + ". We need to reorg before saving this block."); - } - } - LOG.debug("Finalized block not known yet..."); - } - @VisibleForTesting protected void possiblyMoveHead(final Block lastSavedBlock) { final MutableBlockchain blockchain = getProtocolContext().getBlockchain(); 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 dd5e76bf94..fb001b92a9 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 @@ -258,63 +258,6 @@ public class BackwardSyncContextTest { assertThat(localBlockchain.getChainHeadBlock().getHeader().getNumber()).isEqualTo(4); } - @Test - public void testSuccessionRuleAfterUpdatingFinalized() { - - backwardChain.appendTrustedBlock( - remoteBlockchain.getBlockByNumber(LOCAL_HEIGHT + 1).orElseThrow()); - // null check - context.updateHeads(null, null); - context.checkFinalizedSuccessionRuleBeforeSave(null); - // zero check - context.updateHeads(null, Hash.ZERO); - context.checkFinalizedSuccessionRuleBeforeSave(null); - - // cannot save if we don't know what is finalized - context.updateHeads( - null, remoteBlockchain.getBlockHashByNumber(LOCAL_HEIGHT + 10).orElseThrow()); - assertThatThrownBy(() -> context.checkFinalizedSuccessionRuleBeforeSave(null)) - .isInstanceOf(BackwardSyncException.class) - .hasMessageContaining( - "was finalized, but we don't have it downloaded yet, cannot save new block"); - - // updating with new finalized - context.updateHeads( - null, remoteBlockchain.getBlockHashByNumber(LOCAL_HEIGHT + 1).orElseThrow()); - context.checkFinalizedSuccessionRuleBeforeSave( - remoteBlockchain.getBlockByNumber(LOCAL_HEIGHT + 1).orElseThrow()); - - // updating when we know finalized is in futre - context.updateHeads( - null, remoteBlockchain.getBlockHashByNumber(LOCAL_HEIGHT + 4).orElseThrow()); - context.checkFinalizedSuccessionRuleBeforeSave( - remoteBlockchain.getBlockByNumber(LOCAL_HEIGHT + 1).orElseThrow()); - - // updating with block that is not finalized when we expected finalized on this height - context.updateHeads( - null, remoteBlockchain.getBlockHashByNumber(LOCAL_HEIGHT + 1).orElseThrow()); - assertThatThrownBy( - () -> - context.checkFinalizedSuccessionRuleBeforeSave( - createUncle(LOCAL_HEIGHT + 1, localBlockchain.getChainHeadHash()))) - .isInstanceOf(BackwardSyncException.class) - .hasMessageContaining("This block is not the target finalized block"); - - // updating with a block when finalized is not on canonical chain - context.updateHeads(null, uncle.getHash()); - assertThatThrownBy( - () -> - context.checkFinalizedSuccessionRuleBeforeSave( - remoteBlockchain.getBlockByNumber(LOCAL_HEIGHT + 1).orElseThrow())) - .isInstanceOf(BackwardSyncException.class) - .hasMessageContaining("is not on canonical chain. Canonical is"); - - // updating when finalized is on canonical chain - context.updateHeads(null, localBlockchain.getBlockHashByNumber(UNCLE_HEIGHT).orElseThrow()); - context.checkFinalizedSuccessionRuleBeforeSave( - remoteBlockchain.getBlockByNumber(LOCAL_HEIGHT + 1).orElseThrow()); - } - @Test public void shouldProcessExceptionsCorrectly() { assertThatThrownBy(