Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Chore: Ensure Backward Sync Correctly Handles Finalized Block Hash #8217

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
+27 −35
Open

Chore: Ensure Backward Sync Correctly Handles Finalized Block Hash #8217

Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3950e1d
chore:solved backwards synch handle finalized code block has zero cor…
pr9t Feb 2, 2025
5bc24ef
issue attempted
pr9t Feb 3, 2025
60f7286
Chore: Ensure Backward Sync Correctly Handles Finalized Block Hash
pr9t Feb 3, 2025
507e576
Chore: handle Hash.ZERO as a valid finalized block hash
pr9t Feb 4, 2025
ade7872
Chore: updated the logging statements to use LOG.atDebug()
pr9t Feb 5, 2025
ddedaf1
Merge branch 'main' into chore/BackwardsSynchouldHandleFinalizedCode
jflo Feb 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 20 additions & 35 deletions ...ge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,8 @@
*/
package org.hyperledger.besu.consensus.merge.blockcreation;

import static java.util.stream.Collectors.joining;
import static org.hyperledger.besu.consensus.merge.blockcreation.MergeMiningCoordinator.ForkchoiceResult.Status.INVALID;
import static org.hyperledger.besu.ethereum.trie.diffbased.common.provider.WorldStateQueryParams.withBlockHeaderAndUpdateNodeHead;

import com.google.common.annotations.VisibleForTesting;
import org.apache.tuweni.bytes.Bytes32;
import org.hyperledger.besu.consensus.merge.MergeContext;
import org.hyperledger.besu.consensus.merge.PayloadWrapper;
import org.hyperledger.besu.datatypes.Address;
Expand All @@ -30,14 +28,7 @@
import org.hyperledger.besu.ethereum.chain.BadBlockManager;
import org.hyperledger.besu.ethereum.chain.Blockchain;
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.core.BlockWithReceipts;
import org.hyperledger.besu.ethereum.core.Difficulty;
import org.hyperledger.besu.ethereum.core.MiningConfiguration;
import org.hyperledger.besu.ethereum.core.MutableWorldState;
import org.hyperledger.besu.ethereum.core.Transaction;
import org.hyperledger.besu.ethereum.core.Withdrawal;
import org.hyperledger.besu.ethereum.core.*;
import org.hyperledger.besu.ethereum.eth.manager.EthScheduler;
import org.hyperledger.besu.ethereum.eth.sync.backwardsync.BackwardSyncContext;
import org.hyperledger.besu.ethereum.eth.sync.backwardsync.BadChainListener;
Expand All @@ -47,6 +38,8 @@
import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule;
import org.hyperledger.besu.ethereum.trie.MerkleTrieException;
import org.hyperledger.besu.plugin.services.exception.StorageException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.io.PrintWriter;
import java.io.StringWriter;
Expand All @@ -61,10 +54,9 @@
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.Supplier;

import com.google.common.annotations.VisibleForTesting;
import org.apache.tuweni.bytes.Bytes32;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import static java.util.stream.Collectors.joining;
import static org.hyperledger.besu.consensus.merge.blockcreation.MergeMiningCoordinator.ForkchoiceResult.Status.INVALID;
import static org.hyperledger.besu.ethereum.trie.diffbased.common.provider.WorldStateQueryParams.withBlockHeaderAndUpdateNodeHead;

/** The Merge coordinator. */
public class MergeCoordinator implements MergeMiningCoordinator, BadChainListener {
Expand Down Expand Up @@ -524,35 +516,28 @@ public Optional<BlockHeader> getOrSyncHeadByHash(final Hash headHash, final Hash
}

private void updateFinalized(final Hash finalizedHash) {
// If finalizedHash is already set, do nothing
if (mergeContext
.getFinalized()
.map(BlockHeader::getHash)
.map(finalizedHash::equals)
.orElse(Boolean.FALSE)) {
LOG.atDebug()
.setMessage("Finalized block already set to {}, nothing to do")
.addArgument(finalizedHash)
.log();
LOG.debug("Finalized block already set to {}, nothing to do", finalizedHash);
return;
}

protocolContext
.getBlockchain()
.getBlockHeader(finalizedHash)
.ifPresentOrElse(
finalizedHeader -> {
LOG.atDebug()
.setMessage("Setting finalized block header to {}")
.addArgument(finalizedHeader::toLogString)
.log();
mergeContext.setFinalized(finalizedHeader);
},
() ->
LOG.warn(
"Internal error, backward sync completed but failed to import finalized block {}",
finalizedHash));
// Check if the finalized block actually exists in the blockchain
Optional<BlockHeader> maybeFinalizedHeader = protocolContext.getBlockchain().getBlockHeader(finalizedHash);

if (maybeFinalizedHeader.isPresent()) {
LOG.debug("Setting finalized block header to {}", maybeFinalizedHeader.get().toLogString());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

atDebug() is better since it delays the evaluation of the log parameters, which most of the time aren't required unless debug logging is enabled.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for pointing this out! I will update the logging statements to use LOG.atDebug().

mergeContext.setFinalized(maybeFinalizedHeader.get());
} else {
LOG.warn("Backward sync completed but failed to import finalized block {}", finalizedHash);
}
}


@Override
public BlockProcessingResult validateBlock(final Block block) {
final var validationResult =
Expand Down