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.

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

WIP QBFT Empty Block Period #4634

Closed
wants to merge 5 commits into from
Closed
Changes from 1 commit
Commits
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
Prev Previous commit
Next Next commit
First draft of working solution
Delay start of round by cancelling roundChangeTimer and adding short delay to a new BlockTimer to create a loop
In that loop (In QbftBlockHeightManager), check if emptyBlockPeriod has been surpassed.
Hardcoded values and buggy delay timing to fix.
Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
  • Loading branch information
siladu committed Nov 6, 2022
commit 3c388d3cc023268013eb6a98688b5e5d297c855b
Original file line number Diff line number Diff line change
@@ -20,13 +20,20 @@
import org.hyperledger.besu.ethereum.core.BlockHeader;

import java.time.Clock;
import java.time.Instant;
import java.time.ZoneId;
import java.util.Optional;
import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.TimeUnit;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/** Class for starting and keeping organised block timers */
public class BlockTimer {

private static final Logger LOG = LoggerFactory.getLogger(BlockTimer.class);

private final ForksSchedule<? extends BftConfigOptions> forksSchedule;
private final BftExecutors bftExecutors;
private Optional<ScheduledFuture<?>> currentTimerTask;
@@ -86,6 +93,10 @@ public synchronized void startTimer(
final long minimumTimeBetweenBlocksMillis = blockPeriodSeconds * 1000L;
final long expiryTime = chainHeadHeader.getTimestamp() * 1_000 + minimumTimeBetweenBlocksMillis;

System.err.println(
"*** TODO SLD | BlockTimer.startTimer() | cancelling existing timer and submitting new timerTask with expiryTime = "
+ expiryTime);

if (expiryTime > now) {
final long delay = expiryTime - now;

@@ -98,4 +109,41 @@ public synchronized void startTimer(
queue.add(new BlockTimerExpiry(round));
}
}

public synchronized void startTimer(
final ConsensusRoundIdentifier round,
final BlockHeader chainHeadHeader,
final long delayDuringEmptyBlockPeriod) {
cancelTimer();

final long now = clock.millis();

// absolute time when the timer is supposed to expire
final int blockPeriodSeconds =
forksSchedule.getFork(round.getSequenceNumber()).getValue().getBlockPeriodSeconds();
final long minimumTimeBetweenBlocksMillis =
blockPeriodSeconds * 1000L + delayDuringEmptyBlockPeriod;
final long expiryTime = chainHeadHeader.getTimestamp() * 1_000 + minimumTimeBetweenBlocksMillis;

LOG.info(
"*** TODO SLD | BlockTimer.startTimer() | cancelling existing timer and submitting new timerTask with expiryTime = {} (extra delay of {})",
Instant.ofEpochMilli(expiryTime).atZone(ZoneId.systemDefault()),
delayDuringEmptyBlockPeriod);

if (expiryTime > now) {
final long delay = expiryTime - now;

final Runnable newTimerRunnable =
() -> {
LOG.info("TODO SLD BlockTimer expired -> new BlockTimerExpiry({})", round);
queue.add(new BlockTimerExpiry(round));
};

final ScheduledFuture<?> newTimerTask =
bftExecutors.scheduleTask(newTimerRunnable, delay, TimeUnit.MILLISECONDS);
currentTimerTask = Optional.of(newTimerTask);
} else {
queue.add(new BlockTimerExpiry(round));
}
}
}
Original file line number Diff line number Diff line change
@@ -63,19 +63,21 @@ public synchronized boolean isRunning() {
* @param round The round identifier which this timer is tracking
*/
public synchronized void startTimer(final ConsensusRoundIdentifier round) {
startTimer(round, 0L);
}

public synchronized void startTimer(final ConsensusRoundIdentifier round, final long delayInMillis) {
cancelTimer();

final long expiryTime = (baseExpiryMillis * (long) Math.pow(2, round.getRoundNumber())) + delayInMillis; // TODO SLD can add delay here?
System.err.println("*** TODO SLD | RoundTimer.startTimer() | cancelling existing timer and submitting new timerTask with expiryTime = "+expiryTime);
final long expiryTime = (baseExpiryMillis * (long) Math.pow(2, round.getRoundNumber()));
System.err.println(
"*** TODO SLD | RoundTimer.startTimer() | cancelling existing timer and submitting new timerTask with expiryTime = "
+ expiryTime);

final Runnable newTimerRunnable = () -> {
System.err.println("*** TODO SLD | RoundTimer.startTimer() | time expired -> queue.add(new RoundExpiry("+round+"))");
queue.add(new RoundExpiry(round));
};
final Runnable newTimerRunnable =
() -> {
System.err.println(
"*** TODO SLD | RoundTimer.startTimer() | time expired -> queue.add(new RoundExpiry("
+ round
+ "))");
queue.add(new RoundExpiry(round));
};

final ScheduledFuture<?> newTimerTask =
bftExecutors.scheduleTask(newTimerRunnable, expiryTime, TimeUnit.MILLISECONDS);
Original file line number Diff line number Diff line change
@@ -27,17 +27,21 @@
import org.hyperledger.besu.consensus.qbft.payload.MessageFactory;
import org.hyperledger.besu.consensus.qbft.validation.FutureRoundProposalMessageValidator;
import org.hyperledger.besu.consensus.qbft.validation.MessageValidatorFactory;
import org.hyperledger.besu.ethereum.core.Block;
import org.hyperledger.besu.ethereum.core.BlockHeader;
import org.hyperledger.besu.plugin.services.securitymodule.SecurityModuleException;

import java.time.Clock;
import java.time.Instant;
import java.time.ZoneId;
import java.util.Collection;
import java.util.Map;
import java.util.Optional;
import java.util.concurrent.TimeUnit;
import java.util.function.BiConsumer;
import java.util.function.Consumer;
import java.util.function.Function;
import java.util.function.Supplier;

import com.google.common.collect.Maps;
import org.slf4j.Logger;
@@ -104,38 +108,66 @@ public QbftBlockHeightManager(

@Override
public void handleBlockTimerExpiry(final ConsensusRoundIdentifier roundIdentifier) {
LOG.info("TODO SLD in handleBlockTimerExpiry");
if (currentRound.isPresent()) {
// It is possible for the block timer to take longer than it should due to the precision of
// the timer in Java and the OS. This means occasionally the proposal can arrive before the
// block timer expiry and hence the round has already been set. There is no negative impact
// on the protocol in this case.
LOG.info("TODO SLD currentRound isPresent, returning.");
return;
}

// createBlock

LOG.info("TODO SLD start new round");
startNewRound(0);

final QbftRound qbftRound = currentRound.get();
// mining will be checked against round 0 as the current round is initialised to 0 above
final boolean isProposer =
finalState.isLocalNodeProposerForRound(qbftRound.getRoundIdentifier());

logIf(() -> !isProposer, "TODO SLD I is NOT proposer");
logIf(
() -> !roundIdentifier.equals(qbftRound.getRoundIdentifier()),
"TODO SLD roundIdentifier does not equal qbftRound.getRoundIdentifier");

if (isProposer) {
LOG.info("TODO SLD I is proposer");
if (roundIdentifier.equals(qbftRound.getRoundIdentifier())) {
LOG.info("TODO SLD round is current");
final long headerTimeStampSeconds = Math.round(clock.millis() / 1000D);
qbftRound.blockWithTransactions(headerTimeStampSeconds)
.ifPresentOrElse(qbftRound::sendProposalMessage,
() -> {
// startNewRound(0);
// finalState.getRoundTimer().cancelTimer(); // TODO SLD startTimer calls cancelTimer anyway
long emptyBlockPeriodInMillis = 30_000L;
long requestTimeoutInMillis = 10_000L;
long delayInMillis = emptyBlockPeriodInMillis - requestTimeoutInMillis; // TODO SLD should be -blockPeriod?
LOG.debug("TODO SLD skipping proposal due to empty block. Restarting round timer with {} delayInMillis", delayInMillis);
finalState.getRoundTimer().startTimer(roundIdentifier, delayInMillis);
// finalState.getBlockTimer().startTimer(roundIdentifier, parentHeader);
// currentRound = Optional.empty();
});
// qbftRound.createAndSendProposalMessage(headerTimeStampSeconds);
final Block block = qbftRound.createBlock(headerTimeStampSeconds);
LOG.info("TODO SLD created block {}", block);
final boolean blockHasTransactions = !block.getBody().getTransactions().isEmpty();
LOG.info(
"TODO SLD block.getTransactions.size() = {}", block.getBody().getTransactions().size());
if (blockHasTransactions) {
LOG.info("TODO SLD blockHasTransactions so send proposal");
qbftRound.sendProposalMessage(block);
} else {
LOG.info("TODO SLD EMPTY BLOCK DETECTED");
long emptyBlockPeriodInSeconds = 60L;
final long nowInMillis = finalState.getClock().millis();
final long nowInSeconds = nowInMillis / 1000;
final long maxEmptyBlockPeriodTimestampInSeconds =
parentHeader.getTimestamp() + emptyBlockPeriodInSeconds;
if (nowInSeconds > maxEmptyBlockPeriodTimestampInSeconds) {
qbftRound.sendProposalMessage(block);
} else {
finalState.getRoundTimer().cancelTimer();
long blockPeriodInMillis = 1_000L;
final long newExpiry = nowInMillis + blockPeriodInMillis;
final long delay = newExpiry - (parentHeader.getTimestamp() * 1000);
LOG.info(
"TODO SLD emptyBlock so delay proposal for {} millis until {}",
delay,
Instant.ofEpochMilli(newExpiry).atZone(ZoneId.systemDefault()));
finalState.getBlockTimer().startTimer(roundIdentifier, parentHeader, delay);
currentRound = Optional.empty();
}
}
} else {
LOG.trace(
"Block timer expired for a round ({}) other than current ({})",
@@ -145,6 +177,12 @@ public void handleBlockTimerExpiry(final ConsensusRoundIdentifier roundIdentifie
}
}

private void logIf(final Supplier<Boolean> condition, final String message) {
if (condition.get()) {
LOG.info(message);
}
}

@Override
public void roundExpired(final RoundExpiry expire) {
if (currentRound.isEmpty()) {
Original file line number Diff line number Diff line change
@@ -95,14 +95,9 @@ public ConsensusRoundIdentifier getRoundIdentifier() {
return roundState.getRoundIdentifier();
}

public Optional<Block> blockWithTransactions(final long headerTimeStampSeconds) {
public Block createBlock(final long headerTimeStampSeconds) {
LOG.debug("Creating proposed block. round={}", roundState.getRoundIdentifier());
final Block block = blockCreator.createBlock(headerTimeStampSeconds).getBlock();
if (block.getBody().getTransactions().isEmpty()) {
return Optional.empty();
} else {
return Optional.of(block);
}
return blockCreator.createBlock(headerTimeStampSeconds).getBlock();
}

public void sendProposalMessage(final Block block) {
@@ -114,13 +109,8 @@ public void createAndSendProposalMessage(final long headerTimeStampSeconds) {
LOG.debug("Creating proposed block. round={}", roundState.getRoundIdentifier());
final Block block = blockCreator.createBlock(headerTimeStampSeconds).getBlock();

// // TODO SLD use block or check txpool directly?
// if (block.getBody().getTransactions().isEmpty()) {
// LOG.info("TODO SLD Skipping Empty Block"); // TODO SLD also handle roundChange
// } else {
LOG.trace("Creating proposed block blockHeader={}", block.getHeader());
updateStateWithProposalAndTransmit(block, emptyList(), emptyList());
// }
LOG.trace("Creating proposed block blockHeader={}", block.getHeader());
updateStateWithProposalAndTransmit(block, emptyList(), emptyList());
}

public void startRoundWith(
@@ -138,15 +128,10 @@ public void startRoundWith(
blockToPublish = bestPreparedCertificate.get().getBlock();
}

// if (blockToPublish.getBody().getTransactions().isEmpty()) {
// LOG.info("TODO SLD startRoundWith: Skipping Empty Block");
// } else {
// // TODO SLD log here?
updateStateWithProposalAndTransmit(
blockToPublish,
roundChangeArtifacts.getRoundChanges(),
bestPreparedCertificate.map(PreparedCertificate::getPrepares).orElse(emptyList()));
// }
updateStateWithProposalAndTransmit(
blockToPublish,
roundChangeArtifacts.getRoundChanges(),
bestPreparedCertificate.map(PreparedCertificate::getPrepares).orElse(emptyList()));
}

protected void updateStateWithProposalAndTransmit(