Ensure block height manager is restarted when BFT coordinator is (#8308)

* Ensure block height manager is restarted when BFT coordinator is

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>

* Refactor and add runtime exception if an attempt is made to restart without calling reset

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>

* Update BFT adaptor

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>

* Uupdate changelog

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>

* Add QBFT implementation and tests

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>

* Update IBFT tests

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>

* Rename reset() -> stop()

Signed-off-by: Matthew Whitehead <matthew.whitehead@kaleido.io>

* Replace the height manager with a no-op height manager while it's stopped

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>

* Javadoc

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>

---------

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>
Signed-off-by: Matthew Whitehead <matthew.whitehead@kaleido.io>
Signed-off-by: Matt Whitehead <matthew.whitehead@kaleido.io>
This commit is contained in:
Matt Whitehead
2025-03-12 10:16:12 +00:00
committed by GitHub
parent 72ac7bdbe4
commit 083b1d3986
13 changed files with 117 additions and 2 deletions

View File

@@ -48,6 +48,7 @@
### Bug fixes
- Add missing RPC method `debug_accountRange` to `RpcMethod.java` so this method can be used with `--rpc-http-api-method-no-auth` [#8153](https://github.com/hyperledger/besu/issues/8153)
- Add a fallback pivot strategy when the safe block does not change for a long time, to make possible to complete the initial sync in case the chain is not finalizing [#8395](https://github.com/hyperledger/besu/pull/8395)
- Fix issue with new QBFT/IBFT blocks being produced under certain circumstances. [#8308](https://github.com/hyperledger/besu/issues/8308)
## 25.2.2 hotfix
- Pectra - Sepolia: Fix for deposit contract log decoding [#8383](https://github.com/hyperledger/besu/pull/8383)

View File

@@ -115,6 +115,7 @@ public class BftMiningCoordinator implements MiningCoordinator, BlockAddedObserv
LOG.debug("Interrupted while waiting for BftProcessor to stop.", e);
Thread.currentThread().interrupt();
}
eventHandler.stop();
bftExecutors.stop();
}
}

View File

@@ -77,6 +77,20 @@ public abstract class BaseBftController implements BftEventHandler {
public void start() {
if (started.compareAndSet(false, true)) {
startNewHeightManager(blockchain.getChainHeadHeader());
} else {
// In normal circumstances the height manager should only be started once. If the caller
// has stopped the height manager (e.g. while sync completes) they must call stop() before
// starting the height manager again.
throw new IllegalStateException(
"Attempt to start new height manager without stopping previous manager");
}
}
@Override
public void stop() {
if (started.compareAndSet(true, false)) {
stopCurrentHeightManager(blockchain.getChainHeadHeader());
LOG.debug("Height manager stopped");
}
}
@@ -206,6 +220,13 @@ public abstract class BaseBftController implements BftEventHandler {
*/
protected abstract BaseBlockHeightManager getCurrentHeightManager();
/**
* Stop the current height manager by creating a no-op block height manager.
*
* @param parentHeader the parent header
*/
protected abstract void stopCurrentHeightManager(final BlockHeader parentHeader);
private void startNewHeightManager(final BlockHeader parentHeader) {
createNewHeightManager(parentHeader);
final long newChainHeight = getCurrentHeightManager().getChainHeight();

View File

@@ -25,6 +25,9 @@ public interface BftEventHandler {
/** Start. */
void start();
/** Stop. */
void stop();
/**
* Handle message event.
*

View File

@@ -68,7 +68,14 @@ public class IbftBlockHeightManagerFactory {
}
}
private BaseIbftBlockHeightManager createNoOpBlockHeightManager(final BlockHeader parentHeader) {
/**
* Create a no-op block height manager.
*
* @param parentHeader the parent header
* @return the no-op height manager
*/
protected BaseIbftBlockHeightManager createNoOpBlockHeightManager(
final BlockHeader parentHeader) {
return new NoOpBlockHeightManager(parentHeader);
}

View File

@@ -117,4 +117,9 @@ public class IbftController extends BaseBftController {
protected BaseBlockHeightManager getCurrentHeightManager() {
return currentHeightManager;
}
@Override
protected void stopCurrentHeightManager(final BlockHeader parentHeader) {
currentHeightManager = ibftBlockHeightManagerFactory.createNoOpBlockHeightManager(parentHeader);
}
}

View File

@@ -14,6 +14,8 @@
*/
package org.hyperledger.besu.consensus.ibft.statemachine;
import static org.assertj.core.api.Assertions.assertThatNoException;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.assertj.core.util.Lists.newArrayList;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyLong;
@@ -478,4 +480,21 @@ public class IbftControllerTest {
when(roundChangeMessageData.decode()).thenReturn(roundChange);
roundChangeMessage = new DefaultMessage(null, roundChangeMessageData);
}
@Test
public void heightManagerCanOnlyBeStartedOnceIfNotStopped() {
constructIbftController();
ibftController.start();
assertThatThrownBy(() -> ibftController.start())
.isInstanceOf(IllegalStateException.class)
.hasMessage("Attempt to start new height manager without stopping previous manager");
}
@Test
public void heightManagerCanBeRestartedIfStopped() {
constructIbftController();
ibftController.start();
ibftController.stop();
assertThatNoException().isThrownBy(() -> ibftController.start());
}
}

View File

@@ -85,7 +85,13 @@ public class QbftBlockHeightManagerFactory {
this.isEarlyRoundChangeEnabled = isEarlyRoundChangeEnabled;
}
private BaseQbftBlockHeightManager createNoOpBlockHeightManager(
/**
* Creates a no-op height manager
*
* @param parentHeader the parent header
* @return the no-op height manager
*/
protected BaseQbftBlockHeightManager createNoOpBlockHeightManager(
final QbftBlockHeader parentHeader) {
return new NoOpBlockHeightManager(parentHeader);
}

View File

@@ -139,10 +139,29 @@ public class QbftController implements QbftEventHandler {
return currentHeightManager;
}
/* Replace the current height manager with a no-op height manager. */
private void stopCurrentHeightManager(final QbftBlockHeader parentHeader) {
currentHeightManager = qbftBlockHeightManagerFactory.createNoOpBlockHeightManager(parentHeader);
}
@Override
public void start() {
if (started.compareAndSet(false, true)) {
startNewHeightManager(blockchain.getChainHeadHeader());
} else {
// In normal circumstances the height manager should only be started once. If the caller
// has stopped the height manager (e.g. while sync completes) they must call stop() before
// starting the height manager again.
throw new IllegalStateException(
"Attempt to start new height manager without stopping previous manager");
}
}
@Override
public void stop() {
if (started.compareAndSet(true, false)) {
stopCurrentHeightManager(blockchain.getChainHeadHeader());
LOG.debug("QBFT height manager stop");
}
}

View File

@@ -24,6 +24,9 @@ public interface QbftEventHandler {
/** Start. */
void start();
/** Stop. */
void stop();
/**
* Handle errorMessage event.
*

View File

@@ -14,6 +14,8 @@
*/
package org.hyperledger.besu.consensus.qbft.core.statemachine;
import static org.assertj.core.api.Assertions.assertThatNoException;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.assertj.core.util.Lists.newArrayList;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyLong;
@@ -484,4 +486,21 @@ public class QbftControllerTest {
when(roundChangeMessageData.decode(blockEncoder)).thenReturn(roundChange);
roundChangeMessage = new DefaultMessage(null, roundChangeMessageData);
}
@Test
public void heightManagerCanOnlyBeStartedOnceIfNotStopped() {
constructQbftController();
qbftController.start();
assertThatThrownBy(() -> qbftController.start())
.isInstanceOf(IllegalStateException.class)
.hasMessage("Attempt to start new height manager without stopping previous manager");
}
@Test
public void heightManagerCanBeRestartedIfStopped() {
constructQbftController();
qbftController.start();
qbftController.stop();
assertThatNoException().isThrownBy(() -> qbftController.start());
}
}

View File

@@ -41,6 +41,11 @@ public class BftEventHandlerAdaptor implements BftEventHandler {
qbftEventHandler.start();
}
@Override
public void stop() {
qbftEventHandler.stop();
}
@Override
public void handleMessageEvent(final BftReceivedMessageEvent msg) {
qbftEventHandler.handleMessageEvent(msg);

View File

@@ -52,6 +52,12 @@ class BftEventHandlerAdaptorTest {
verify(qbftEventHandler).start();
}
@Test
void stopDelegatesToQbftEventHandler() {
handler.stop();
verify(qbftEventHandler).stop();
}
@Test
void handleMessageEventDelegatesToQbftEventHandler() {
handler.handleMessageEvent(bftReceivedMessageEvent);