From 71e6b0ccf180c7cbbe2c0c318ee16ac5e5548a32 Mon Sep 17 00:00:00 2001 From: Simon Dudley Date: Thu, 14 Oct 2021 00:25:16 +0100 Subject: [PATCH] Fix for Select correct ValidatorProvider when transitioning from validator contract to block header mode (#2869) During the transition from contract mode to block header mode, when creating the extra data for a new block, we need to look ahead to the next block's voteProvider to obtain the nonEmpty one associated with the BlockValidatorProvider. This only applies to ForkingValidatorProvider which is currently only used for QBFT. However, we don't want QBFT to know it's using a ForkingValidatorProvider. Therefore we need to implement getVoteProviderAfterBlock on the ValidatorProvider interface even though it's implementation is equivalent to getVoteProvider. Signed-off-by: Simon Dudley --- .../blockcreation/CliqueBlockCreator.java | 4 +- .../clique/jsonrpc/methods/Discard.java | 5 +- .../clique/jsonrpc/methods/Propose.java | 7 +- .../blockcreation/CliqueBlockCreatorTest.java | 4 +- .../clique/jsonrpc/methods/DiscardTest.java | 18 +- .../clique/jsonrpc/methods/ProposeTest.java | 24 +-- .../blockcreation/BftBlockCreatorFactory.java | 7 +- .../jsonrpc/AbstractVoteProposerMethod.java | 4 +- .../common/validator/ValidatorProvider.java | 11 +- .../blockbased/BlockValidatorProvider.java | 2 +- .../AbstractVoteProposerMethodTest.java | 4 +- .../methods/IbftDiscardValidatorVote.java | 5 +- .../methods/IbftProposeValidatorVote.java | 7 +- .../methods/IbftDiscardValidatorVoteTest.java | 2 +- .../methods/IbftProposeValidatorVoteTest.java | 2 +- .../qbft/test/ValidatorContractTest.java | 166 ++++++++++++++++-- ...nesis_migrating_validator_blockheader.json | 35 ++++ .../genesis_migrating_validator_contract.json | 19 +- .../methods/QbftDiscardValidatorVote.java | 4 +- .../methods/QbftProposeValidatorVote.java | 6 +- .../statemachine/QbftBlockHeightManager.java | 2 +- .../validator/ForkingValidatorProvider.java | 10 +- .../TransactionValidatorProvider.java | 2 +- .../methods/QbftDiscardValidatorVoteTest.java | 4 +- .../methods/QbftProposeValidatorVoteTest.java | 4 +- .../ForkingValidatorProviderTest.java | 51 +++++- .../TransactionValidatorProviderTest.java | 2 +- 27 files changed, 318 insertions(+), 93 deletions(-) create mode 100644 consensus/qbft/src/integration-test/resources/genesis_migrating_validator_blockheader.json diff --git a/consensus/clique/src/main/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueBlockCreator.java b/consensus/clique/src/main/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueBlockCreator.java index 39e82bb9d96..00953418b5a 100644 --- a/consensus/clique/src/main/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueBlockCreator.java +++ b/consensus/clique/src/main/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueBlockCreator.java @@ -108,11 +108,11 @@ private Optional determineCliqueVote( } else { final CliqueContext cliqueContext = protocolContext.getConsensusState(CliqueContext.class); checkState( - cliqueContext.getValidatorProvider().getVoteProvider().isPresent(), + cliqueContext.getValidatorProvider().getVoteProviderAtHead().isPresent(), "Clique requires a vote provider"); return cliqueContext .getValidatorProvider() - .getVoteProvider() + .getVoteProviderAtHead() .get() .getVoteAfterBlock(parentHeader, Util.publicKeyToAddress(nodeKey.getPublicKey())); } diff --git a/consensus/clique/src/main/java/org/hyperledger/besu/consensus/clique/jsonrpc/methods/Discard.java b/consensus/clique/src/main/java/org/hyperledger/besu/consensus/clique/jsonrpc/methods/Discard.java index c556bb8cc19..dd2d14fbef4 100644 --- a/consensus/clique/src/main/java/org/hyperledger/besu/consensus/clique/jsonrpc/methods/Discard.java +++ b/consensus/clique/src/main/java/org/hyperledger/besu/consensus/clique/jsonrpc/methods/Discard.java @@ -38,9 +38,10 @@ public String getName() { @Override public JsonRpcResponse response(final JsonRpcRequestContext requestContext) { - checkState(validatorProvider.getVoteProvider().isPresent(), "Clique requires a vote provider"); + checkState( + validatorProvider.getVoteProviderAtHead().isPresent(), "Clique requires a vote provider"); final Address address = requestContext.getRequiredParameter(0, Address.class); - validatorProvider.getVoteProvider().get().discardVote(address); + validatorProvider.getVoteProviderAtHead().get().discardVote(address); return new JsonRpcSuccessResponse(requestContext.getRequest().getId(), true); } } diff --git a/consensus/clique/src/main/java/org/hyperledger/besu/consensus/clique/jsonrpc/methods/Propose.java b/consensus/clique/src/main/java/org/hyperledger/besu/consensus/clique/jsonrpc/methods/Propose.java index 94510fb8737..a7496ed4d30 100644 --- a/consensus/clique/src/main/java/org/hyperledger/besu/consensus/clique/jsonrpc/methods/Propose.java +++ b/consensus/clique/src/main/java/org/hyperledger/besu/consensus/clique/jsonrpc/methods/Propose.java @@ -41,7 +41,8 @@ public String getName() { @Override public JsonRpcResponse response(final JsonRpcRequestContext requestContext) { - checkState(validatorProvider.getVoteProvider().isPresent(), "Clique requires a vote provider"); + checkState( + validatorProvider.getVoteProviderAtHead().isPresent(), "Clique requires a vote provider"); final Address address = requestContext.getRequiredParameter(0, Address.class); final Boolean auth = requestContext.getRequiredParameter(1, Boolean.class); if (address.equals(CliqueBlockInterface.NO_VOTE_SUBJECT)) { @@ -50,9 +51,9 @@ public JsonRpcResponse response(final JsonRpcRequestContext requestContext) { } if (auth) { - validatorProvider.getVoteProvider().get().authVote(address); + validatorProvider.getVoteProviderAtHead().get().authVote(address); } else { - validatorProvider.getVoteProvider().get().dropVote(address); + validatorProvider.getVoteProviderAtHead().get().dropVote(address); } // Return true regardless, the vote is always recorded return new JsonRpcSuccessResponse(requestContext.getRequest().getId(), true); diff --git a/consensus/clique/src/test/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueBlockCreatorTest.java b/consensus/clique/src/test/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueBlockCreatorTest.java index 0a8c6244c17..d0c85e7b629 100644 --- a/consensus/clique/src/test/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueBlockCreatorTest.java +++ b/consensus/clique/src/test/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueBlockCreatorTest.java @@ -97,7 +97,7 @@ public void setup() { validatorProvider = mock(ValidatorProvider.class); voteProvider = mock(VoteProvider.class); - when(validatorProvider.getVoteProvider()).thenReturn(Optional.of(voteProvider)); + when(validatorProvider.getVoteProviderAtHead()).thenReturn(Optional.of(voteProvider)); when(validatorProvider.getValidatorsAfterBlock(any())).thenReturn(validatorList); final CliqueContext cliqueContext = new CliqueContext(validatorProvider, null, blockInterface); @@ -196,7 +196,7 @@ public void insertsNoVoteWhenAtEpoch() { CliqueExtraData.createWithoutProposerSeal(Bytes.wrap(new byte[32]), validatorList); final Address a1 = Address.fromHexString("5"); final Address coinbase = AddressHelpers.ofValue(1); - when(validatorProvider.getVoteProvider().get().getVoteAfterBlock(any(), any())) + when(validatorProvider.getVoteProviderAtHead().get().getVoteAfterBlock(any(), any())) .thenReturn(Optional.of(new ValidatorVote(VoteType.ADD, coinbase, a1))); final CliqueBlockCreator blockCreator = diff --git a/consensus/clique/src/test/java/org/hyperledger/besu/consensus/clique/jsonrpc/methods/DiscardTest.java b/consensus/clique/src/test/java/org/hyperledger/besu/consensus/clique/jsonrpc/methods/DiscardTest.java index 0f11c6e843d..3ae452899d8 100644 --- a/consensus/clique/src/test/java/org/hyperledger/besu/consensus/clique/jsonrpc/methods/DiscardTest.java +++ b/consensus/clique/src/test/java/org/hyperledger/besu/consensus/clique/jsonrpc/methods/DiscardTest.java @@ -64,7 +64,7 @@ public void discardEmpty() { assertThat(response.getType()).isEqualTo(JsonRpcResponseType.SUCCESS); final JsonRpcSuccessResponse successResponse = (JsonRpcSuccessResponse) response; assertThat(successResponse.getResult()).isEqualTo(true); - assertThat(validatorProvider.getVoteProvider().get().getProposals().get(a0)).isNull(); + assertThat(validatorProvider.getVoteProviderAtHead().get().getProposals().get(a0)).isNull(); } @Test @@ -72,11 +72,11 @@ public void discardAuth() { final Discard discard = new Discard(validatorProvider); final Address a0 = Address.fromHexString("0"); - validatorProvider.getVoteProvider().get().authVote(a0); + validatorProvider.getVoteProviderAtHead().get().authVote(a0); final JsonRpcResponse response = discard.response(requestWithParams(a0)); - assertThat(validatorProvider.getVoteProvider().get().getProposals().get(a0)).isNull(); + assertThat(validatorProvider.getVoteProviderAtHead().get().getProposals().get(a0)).isNull(); assertThat(response.getType()).isEqualTo(JsonRpcResponseType.SUCCESS); final JsonRpcSuccessResponse successResponse = (JsonRpcSuccessResponse) response; assertThat(successResponse.getResult()).isEqualTo(true); @@ -87,11 +87,11 @@ public void discardDrop() { final Discard discard = new Discard(validatorProvider); final Address a0 = Address.fromHexString("0"); - validatorProvider.getVoteProvider().get().dropVote(a0); + validatorProvider.getVoteProviderAtHead().get().dropVote(a0); final JsonRpcResponse response = discard.response(requestWithParams(a0)); - assertThat(validatorProvider.getVoteProvider().get().getProposals().get(a0)).isNull(); + assertThat(validatorProvider.getVoteProviderAtHead().get().getProposals().get(a0)).isNull(); assertThat(response.getType()).isEqualTo(JsonRpcResponseType.SUCCESS); final JsonRpcSuccessResponse successResponse = (JsonRpcSuccessResponse) response; assertThat(successResponse.getResult()).isEqualTo(true); @@ -103,13 +103,13 @@ public void discardIsolation() { final Address a0 = Address.fromHexString("0"); final Address a1 = Address.fromHexString("1"); - validatorProvider.getVoteProvider().get().authVote(a0); - validatorProvider.getVoteProvider().get().authVote(a1); + validatorProvider.getVoteProviderAtHead().get().authVote(a0); + validatorProvider.getVoteProviderAtHead().get().authVote(a1); final JsonRpcResponse response = discard.response(requestWithParams(a0)); - assertThat(validatorProvider.getVoteProvider().get().getProposals().get(a0)).isNull(); - assertThat(validatorProvider.getVoteProvider().get().getProposals().get(a1)) + assertThat(validatorProvider.getVoteProviderAtHead().get().getProposals().get(a0)).isNull(); + assertThat(validatorProvider.getVoteProviderAtHead().get().getProposals().get(a1)) .isEqualTo(VoteType.ADD); assertThat(response.getType()).isEqualTo(JsonRpcResponseType.SUCCESS); final JsonRpcSuccessResponse successResponse = (JsonRpcSuccessResponse) response; diff --git a/consensus/clique/src/test/java/org/hyperledger/besu/consensus/clique/jsonrpc/methods/ProposeTest.java b/consensus/clique/src/test/java/org/hyperledger/besu/consensus/clique/jsonrpc/methods/ProposeTest.java index c9a98cc7fee..141a2b7b790 100644 --- a/consensus/clique/src/test/java/org/hyperledger/besu/consensus/clique/jsonrpc/methods/ProposeTest.java +++ b/consensus/clique/src/test/java/org/hyperledger/besu/consensus/clique/jsonrpc/methods/ProposeTest.java @@ -57,7 +57,7 @@ public void testAuth() { final JsonRpcResponse response = propose.response(requestWithParams(a1, true)); - assertThat(validatorProvider.getVoteProvider().get().getProposals().get(a1)) + assertThat(validatorProvider.getVoteProviderAtHead().get().getProposals().get(a1)) .isEqualTo(VoteType.ADD); assertThat(response.getType()).isEqualTo(JsonRpcResponseType.SUCCESS); final JsonRpcSuccessResponse successResponse = (JsonRpcSuccessResponse) response; @@ -71,7 +71,7 @@ public void testAuthWithAddressZeroResultsInError() { final JsonRpcResponse response = propose.response(requestWithParams(a0, true)); - assertThat(validatorProvider.getVoteProvider().get().getProposals().get(a0)).isNull(); + assertThat(validatorProvider.getVoteProviderAtHead().get().getProposals().get(a0)).isNull(); assertThat(response.getType()).isEqualTo(JsonRpcResponseType.ERROR); final JsonRpcErrorResponse errorResponse = (JsonRpcErrorResponse) response; assertThat(errorResponse.getError()).isEqualTo(JsonRpcError.INVALID_REQUEST); @@ -84,7 +84,7 @@ public void testDrop() { final JsonRpcResponse response = propose.response(requestWithParams(a1, false)); - assertThat(validatorProvider.getVoteProvider().get().getProposals().get(a1)) + assertThat(validatorProvider.getVoteProviderAtHead().get().getProposals().get(a1)) .isEqualTo(VoteType.DROP); assertThat(response.getType()).isEqualTo(JsonRpcResponseType.SUCCESS); final JsonRpcSuccessResponse successResponse = (JsonRpcSuccessResponse) response; @@ -98,7 +98,7 @@ public void testDropWithAddressZeroResultsInError() { final JsonRpcResponse response = propose.response(requestWithParams(a0, false)); - assertThat(validatorProvider.getVoteProvider().get().getProposals().get(a0)).isNull(); + assertThat(validatorProvider.getVoteProviderAtHead().get().getProposals().get(a0)).isNull(); assertThat(response.getType()).isEqualTo(JsonRpcResponseType.ERROR); final JsonRpcErrorResponse errorResponse = (JsonRpcErrorResponse) response; assertThat(errorResponse.getError()).isEqualTo(JsonRpcError.INVALID_REQUEST); @@ -109,10 +109,10 @@ public void testRepeatAuth() { final Propose propose = new Propose(validatorProvider); final Address a1 = Address.fromHexString("1"); - validatorProvider.getVoteProvider().get().authVote(a1); + validatorProvider.getVoteProviderAtHead().get().authVote(a1); final JsonRpcResponse response = propose.response(requestWithParams(a1, true)); - assertThat(validatorProvider.getVoteProvider().get().getProposals().get(a1)) + assertThat(validatorProvider.getVoteProviderAtHead().get().getProposals().get(a1)) .isEqualTo(VoteType.ADD); assertThat(response.getType()).isEqualTo(JsonRpcResponseType.SUCCESS); final JsonRpcSuccessResponse successResponse = (JsonRpcSuccessResponse) response; @@ -124,10 +124,10 @@ public void testRepeatDrop() { final Propose propose = new Propose(validatorProvider); final Address a1 = Address.fromHexString("1"); - validatorProvider.getVoteProvider().get().dropVote(a1); + validatorProvider.getVoteProviderAtHead().get().dropVote(a1); final JsonRpcResponse response = propose.response(requestWithParams(a1, false)); - assertThat(validatorProvider.getVoteProvider().get().getProposals().get(a1)) + assertThat(validatorProvider.getVoteProviderAtHead().get().getProposals().get(a1)) .isEqualTo(VoteType.DROP); assertThat(response.getType()).isEqualTo(JsonRpcResponseType.SUCCESS); final JsonRpcSuccessResponse successResponse = (JsonRpcSuccessResponse) response; @@ -139,10 +139,10 @@ public void testChangeToAuth() { final Propose propose = new Propose(validatorProvider); final Address a1 = Address.fromHexString("1"); - validatorProvider.getVoteProvider().get().dropVote(a1); + validatorProvider.getVoteProviderAtHead().get().dropVote(a1); final JsonRpcResponse response = propose.response(requestWithParams(a1, true)); - assertThat(validatorProvider.getVoteProvider().get().getProposals().get(a1)) + assertThat(validatorProvider.getVoteProviderAtHead().get().getProposals().get(a1)) .isEqualTo(VoteType.ADD); assertThat(response.getType()).isEqualTo(JsonRpcResponseType.SUCCESS); final JsonRpcSuccessResponse successResponse = (JsonRpcSuccessResponse) response; @@ -154,10 +154,10 @@ public void testChangeToDrop() { final Propose propose = new Propose(validatorProvider); final Address a0 = Address.fromHexString("1"); - validatorProvider.getVoteProvider().get().authVote(a0); + validatorProvider.getVoteProviderAtHead().get().authVote(a0); final JsonRpcResponse response = propose.response(requestWithParams(a0, false)); - assertThat(validatorProvider.getVoteProvider().get().getProposals().get(a0)) + assertThat(validatorProvider.getVoteProviderAtHead().get().getProposals().get(a0)) .isEqualTo(VoteType.DROP); assertThat(response.getType()).isEqualTo(JsonRpcResponseType.SUCCESS); final JsonRpcSuccessResponse successResponse = (JsonRpcSuccessResponse) response; diff --git a/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/blockcreation/BftBlockCreatorFactory.java b/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/blockcreation/BftBlockCreatorFactory.java index a7fc85b7480..1a4fe27b5f9 100644 --- a/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/blockcreation/BftBlockCreatorFactory.java +++ b/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/blockcreation/BftBlockCreatorFactory.java @@ -23,6 +23,7 @@ import org.hyperledger.besu.consensus.common.bft.Vote; import org.hyperledger.besu.consensus.common.validator.ValidatorProvider; import org.hyperledger.besu.consensus.common.validator.ValidatorVote; +import org.hyperledger.besu.consensus.common.validator.VoteProvider; import org.hyperledger.besu.datatypes.Address; import org.hyperledger.besu.datatypes.Wei; import org.hyperledger.besu.ethereum.ProtocolContext; @@ -105,9 +106,11 @@ public Wei getMinTransactionGasPrice() { public Bytes createExtraData(final int round, final BlockHeader parentHeader) { final BftContext bftContext = protocolContext.getConsensusState(BftContext.class); final ValidatorProvider validatorProvider = bftContext.getValidatorProvider(); - checkState(validatorProvider.getVoteProvider().isPresent(), "Bft requires a vote provider"); + Optional voteProviderAfterBlock = + validatorProvider.getVoteProviderAfterBlock(parentHeader); + checkState(voteProviderAfterBlock.isPresent(), "Bft requires a vote provider"); final Optional proposal = - validatorProvider.getVoteProvider().get().getVoteAfterBlock(parentHeader, localAddress); + voteProviderAfterBlock.get().getVoteAfterBlock(parentHeader, localAddress); final List
validators = new ArrayList<>(validatorProvider.getValidatorsAfterBlock(parentHeader)); diff --git a/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/jsonrpc/AbstractVoteProposerMethod.java b/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/jsonrpc/AbstractVoteProposerMethod.java index 12235b2f3b4..69f7cec1b85 100644 --- a/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/jsonrpc/AbstractVoteProposerMethod.java +++ b/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/jsonrpc/AbstractVoteProposerMethod.java @@ -34,9 +34,9 @@ public AbstractVoteProposerMethod(final ValidatorProvider validatorProvider) { } public JsonRpcResponse response(final JsonRpcRequestContext requestContext) { - if (validatorProvider.getVoteProvider().isPresent()) { + if (validatorProvider.getVoteProviderAtHead().isPresent()) { final Map proposals = - validatorProvider.getVoteProvider().get().getProposals().entrySet().stream() + validatorProvider.getVoteProviderAtHead().get().getProposals().entrySet().stream() .collect( Collectors.toMap( proposal -> proposal.getKey().toString(), diff --git a/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/validator/ValidatorProvider.java b/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/validator/ValidatorProvider.java index ea8ba2253dd..fa40f1b2823 100644 --- a/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/validator/ValidatorProvider.java +++ b/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/validator/ValidatorProvider.java @@ -28,5 +28,14 @@ public interface ValidatorProvider { Collection
getValidatorsForBlock(BlockHeader header); - Optional getVoteProvider(); + Optional getVoteProviderAtHead(); + + /* + * ForkingValidatorProvider has a specific implementation but we don't want the client code to + * know it's using a ForkingValidatorProvider. ForkingValidatorProvider's voteProvider can be + * different per block. Other ValidatorProviders yield the same voteProvider at every block. + */ + default Optional getVoteProviderAfterBlock(final BlockHeader header) { + return getVoteProviderAtHead(); + } } diff --git a/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/validator/blockbased/BlockValidatorProvider.java b/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/validator/blockbased/BlockValidatorProvider.java index 6e3e6e39fd6..5586dea301f 100644 --- a/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/validator/blockbased/BlockValidatorProvider.java +++ b/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/validator/blockbased/BlockValidatorProvider.java @@ -85,7 +85,7 @@ public Collection
getValidatorsForBlock(final BlockHeader header) { } @Override - public Optional getVoteProvider() { + public Optional getVoteProviderAtHead() { return Optional.of(voteProvider); } } diff --git a/consensus/common/src/test/java/org/hyperledger/besu/consensus/common/jsonrpc/AbstractVoteProposerMethodTest.java b/consensus/common/src/test/java/org/hyperledger/besu/consensus/common/jsonrpc/AbstractVoteProposerMethodTest.java index 3e9dd745ef4..0ed99dc4d75 100644 --- a/consensus/common/src/test/java/org/hyperledger/besu/consensus/common/jsonrpc/AbstractVoteProposerMethodTest.java +++ b/consensus/common/src/test/java/org/hyperledger/besu/consensus/common/jsonrpc/AbstractVoteProposerMethodTest.java @@ -55,7 +55,7 @@ public void testConversionFromVoteTypeToBoolean() { new JsonRpcRequestContext( new JsonRpcRequest(JSON_RPC_VERSION, getMethodName(), new Object[] {})); - when(validatorProvider.getVoteProvider()).thenReturn(Optional.of(voteProvider)); + when(validatorProvider.getVoteProviderAtHead()).thenReturn(Optional.of(voteProvider)); when(voteProvider.getProposals()) .thenReturn( ImmutableMap.of( @@ -85,7 +85,7 @@ public void methodNotEnabledWhenNoVoteProvider() { new JsonRpcRequest(JSON_RPC_VERSION, getMethodName(), new Object[] {})); final JsonRpcResponse expectedResponse = new JsonRpcErrorResponse(request.getRequest().getId(), JsonRpcError.METHOD_NOT_ENABLED); - when(validatorProvider.getVoteProvider()).thenReturn(Optional.empty()); + when(validatorProvider.getVoteProviderAtHead()).thenReturn(Optional.empty()); final JsonRpcResponse response = getMethod().response(request); diff --git a/consensus/ibft/src/main/java/org/hyperledger/besu/consensus/ibft/jsonrpc/methods/IbftDiscardValidatorVote.java b/consensus/ibft/src/main/java/org/hyperledger/besu/consensus/ibft/jsonrpc/methods/IbftDiscardValidatorVote.java index eadf074a7f6..45696346a52 100644 --- a/consensus/ibft/src/main/java/org/hyperledger/besu/consensus/ibft/jsonrpc/methods/IbftDiscardValidatorVote.java +++ b/consensus/ibft/src/main/java/org/hyperledger/besu/consensus/ibft/jsonrpc/methods/IbftDiscardValidatorVote.java @@ -42,10 +42,11 @@ public String getName() { @Override public JsonRpcResponse response(final JsonRpcRequestContext requestContext) { - checkState(validatorProvider.getVoteProvider().isPresent(), "Ibft requires a vote provider"); + checkState( + validatorProvider.getVoteProviderAtHead().isPresent(), "Ibft requires a vote provider"); final Address validatorAddress = requestContext.getRequiredParameter(0, Address.class); LOG.trace("Received RPC rpcName={} address={}", getName(), validatorAddress); - validatorProvider.getVoteProvider().get().discardVote(validatorAddress); + validatorProvider.getVoteProviderAtHead().get().discardVote(validatorAddress); return new JsonRpcSuccessResponse(requestContext.getRequest().getId(), true); } diff --git a/consensus/ibft/src/main/java/org/hyperledger/besu/consensus/ibft/jsonrpc/methods/IbftProposeValidatorVote.java b/consensus/ibft/src/main/java/org/hyperledger/besu/consensus/ibft/jsonrpc/methods/IbftProposeValidatorVote.java index 69c53fb9a0f..0bbe1de4af2 100644 --- a/consensus/ibft/src/main/java/org/hyperledger/besu/consensus/ibft/jsonrpc/methods/IbftProposeValidatorVote.java +++ b/consensus/ibft/src/main/java/org/hyperledger/besu/consensus/ibft/jsonrpc/methods/IbftProposeValidatorVote.java @@ -43,7 +43,8 @@ public String getName() { @Override public JsonRpcResponse response(final JsonRpcRequestContext requestContext) { - checkState(validatorProvider.getVoteProvider().isPresent(), "Ibft requires a vote provider"); + checkState( + validatorProvider.getVoteProviderAtHead().isPresent(), "Ibft requires a vote provider"); final Address validatorAddress = requestContext.getRequiredParameter(0, Address.class); final Boolean add = requestContext.getRequiredParameter(1, Boolean.class); LOG.trace( @@ -53,9 +54,9 @@ public JsonRpcResponse response(final JsonRpcRequestContext requestContext) { validatorAddress); if (add) { - validatorProvider.getVoteProvider().get().authVote(validatorAddress); + validatorProvider.getVoteProviderAtHead().get().authVote(validatorAddress); } else { - validatorProvider.getVoteProvider().get().dropVote(validatorAddress); + validatorProvider.getVoteProviderAtHead().get().dropVote(validatorAddress); } return new JsonRpcSuccessResponse(requestContext.getRequest().getId(), true); diff --git a/consensus/ibft/src/test/java/org/hyperledger/besu/consensus/ibft/jsonrpc/methods/IbftDiscardValidatorVoteTest.java b/consensus/ibft/src/test/java/org/hyperledger/besu/consensus/ibft/jsonrpc/methods/IbftDiscardValidatorVoteTest.java index dcfe0c04c9a..e6b619a243f 100644 --- a/consensus/ibft/src/test/java/org/hyperledger/besu/consensus/ibft/jsonrpc/methods/IbftDiscardValidatorVoteTest.java +++ b/consensus/ibft/src/test/java/org/hyperledger/besu/consensus/ibft/jsonrpc/methods/IbftDiscardValidatorVoteTest.java @@ -47,7 +47,7 @@ public class IbftDiscardValidatorVoteTest { @Before public void setup() { method = new IbftDiscardValidatorVote(validatorProvider); - when(validatorProvider.getVoteProvider()).thenReturn(Optional.of(voteProvider)); + when(validatorProvider.getVoteProviderAtHead()).thenReturn(Optional.of(voteProvider)); } @Test diff --git a/consensus/ibft/src/test/java/org/hyperledger/besu/consensus/ibft/jsonrpc/methods/IbftProposeValidatorVoteTest.java b/consensus/ibft/src/test/java/org/hyperledger/besu/consensus/ibft/jsonrpc/methods/IbftProposeValidatorVoteTest.java index fec296e07f5..747f68bf0cd 100644 --- a/consensus/ibft/src/test/java/org/hyperledger/besu/consensus/ibft/jsonrpc/methods/IbftProposeValidatorVoteTest.java +++ b/consensus/ibft/src/test/java/org/hyperledger/besu/consensus/ibft/jsonrpc/methods/IbftProposeValidatorVoteTest.java @@ -47,7 +47,7 @@ public class IbftProposeValidatorVoteTest { @Before public void setup() { method = new IbftProposeValidatorVote(validatorProvider); - when(validatorProvider.getVoteProvider()).thenReturn(Optional.of(voteProvider)); + when(validatorProvider.getVoteProviderAtHead()).thenReturn(Optional.of(voteProvider)); } @Test diff --git a/consensus/qbft/src/integration-test/java/org/hyperledger/besu/consensus/qbft/test/ValidatorContractTest.java b/consensus/qbft/src/integration-test/java/org/hyperledger/besu/consensus/qbft/test/ValidatorContractTest.java index f7698c25963..7207022e93a 100644 --- a/consensus/qbft/src/integration-test/java/org/hyperledger/besu/consensus/qbft/test/ValidatorContractTest.java +++ b/consensus/qbft/src/integration-test/java/org/hyperledger/besu/consensus/qbft/test/ValidatorContractTest.java @@ -15,6 +15,8 @@ package org.hyperledger.besu.consensus.qbft.test; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; import org.hyperledger.besu.config.BftFork; import org.hyperledger.besu.config.JsonUtil; @@ -22,13 +24,17 @@ import org.hyperledger.besu.config.QbftFork.VALIDATOR_SELECTION_MODE; import org.hyperledger.besu.consensus.common.bft.ConsensusRoundIdentifier; import org.hyperledger.besu.consensus.common.bft.events.BlockTimerExpiry; +import org.hyperledger.besu.consensus.common.bft.events.NewChainHead; import org.hyperledger.besu.consensus.common.bft.inttest.NodeParams; import org.hyperledger.besu.consensus.common.validator.ValidatorProvider; import org.hyperledger.besu.consensus.qbft.QbftExtraDataCodec; +import org.hyperledger.besu.consensus.qbft.support.RoundSpecificPeers; import org.hyperledger.besu.consensus.qbft.support.TestContext; import org.hyperledger.besu.consensus.qbft.support.TestContextBuilder; +import org.hyperledger.besu.consensus.qbft.support.ValidatorPeer; import org.hyperledger.besu.crypto.NodeKeyUtils; import org.hyperledger.besu.datatypes.Address; +import org.hyperledger.besu.ethereum.core.Block; import org.hyperledger.besu.ethereum.core.BlockHeader; import java.time.Clock; @@ -50,11 +56,17 @@ public class ValidatorContractTest { public static final Bytes32 NODE_PRIVATE_KEY = Bytes32.fromHexString("0xa3bdf521b0f286a80918c4b67000dfd2a2bdef97e94d268016ef9ec86648eac3"); + private static final Address NODE_2_ADDRESS = + Address.fromHexString("0xe98d92560fac3069ccff53ef348ded26a51d4b68"); + private static final Bytes32 NODE_2_PRIVATE_KEY = + Bytes32.fromHexString("0xa3bdf521b0f286a80918c4b67000dfd2a2bdef97e94d268016ef9ec86648eac4"); + private final long blockTimeStamp = 100; private final Clock fixedClock = Clock.fixed(Instant.ofEpochSecond(blockTimeStamp), ZoneId.systemDefault()); + private final Clock tickingClock = setupTickingClock(); - private final ConsensusRoundIdentifier roundId = new ConsensusRoundIdentifier(1, 0); + private final QbftExtraDataCodec extraDataCodec = new QbftExtraDataCodec(); @Test public void retrievesValidatorsFromValidatorContract() { @@ -68,9 +80,7 @@ public void retrievesValidatorsFromValidatorContract() { .useValidatorContract(true) .buildAndStart(); - // create new block - context.getController().handleBlockTimerExpiry(new BlockTimerExpiry(roundId)); - assertThat(context.getBlockchain().getChainHeadBlockNumber()).isEqualTo(1); + createNewBlockAsProposer(context, 1); final ValidatorProvider validatorProvider = context.getValidatorProvider(); final BlockHeader genesisBlock = context.getBlockchain().getBlockHeader(0).get(); @@ -80,8 +90,7 @@ public void retrievesValidatorsFromValidatorContract() { } @Test - public void migratesFromBlockToValidatorContract() { - final QbftExtraDataCodec extraDataCodec = new QbftExtraDataCodec(); + public void transitionsFromBlockHeaderModeToValidatorContractMode() { final List qbftForks = List.of(createContractFork(1, TestContextBuilder.VALIDATOR_CONTRACT_ADDRESS)); final TestContext context = @@ -100,13 +109,9 @@ public void migratesFromBlockToValidatorContract() { // block 1 onwards uses validator contract with 2 validators final List
block0Addresses = List.of(NODE_ADDRESS); final List
block1Addresses = - Stream.of(NODE_ADDRESS, Address.fromHexString("ca1c5ff73ed8370397114006dd1258e16433f41b")) - .sorted() - .collect(Collectors.toList()); + Stream.of(NODE_ADDRESS, NODE_2_ADDRESS).sorted().collect(Collectors.toList()); - // create new block - context.getController().handleBlockTimerExpiry(new BlockTimerExpiry(roundId)); - assertThat(context.getBlockchain().getChainHeadBlockNumber()).isEqualTo(1); + createNewBlockAsProposer(context, 1); final ValidatorProvider validatorProvider = context.getValidatorProvider(); final BlockHeader genesisBlock = context.getBlockchain().getBlockHeader(0).get(); @@ -121,6 +126,122 @@ public void migratesFromBlockToValidatorContract() { assertThat(extraDataCodec.decode(block1).getVote()).isEmpty(); } + @Test + public void transitionsFromValidatorContractModeToBlockHeaderMode() { + final List qbftForks = List.of(createBlockHeaderFork(1)); + final TestContext context = + new TestContextBuilder() + .indexOfFirstLocallyProposedBlock(0) + .nodeParams( + List.of(new NodeParams(NODE_ADDRESS, NodeKeyUtils.createFrom(NODE_PRIVATE_KEY)))) + .clock(fixedClock) + .genesisFile( + Resources.getResource("genesis_migrating_validator_blockheader.json").getFile()) + .useValidatorContract(true) + .qbftForks(qbftForks) + .buildAndStart(); + + // block 0 uses validator contract with 1 validator + // block 1 onwards uses block header voting (which reuses previous block's validators upon + // switching) + final List
block0Addresses = Stream.of(NODE_ADDRESS).collect(Collectors.toList()); + + createNewBlockAsProposer(context, 1); + + final ValidatorProvider validatorProvider = context.getValidatorProvider(); + final BlockHeader genesisBlock = context.getBlockchain().getBlockHeader(0).get(); + final BlockHeader block1 = context.getBlockchain().getBlockHeader(1).get(); + + // contract block extra data cannot contain validators or vote + assertThat(validatorProvider.getValidatorsForBlock(genesisBlock)).isEqualTo(block0Addresses); + assertThat(extraDataCodec.decode(genesisBlock).getValidators()).isEmpty(); + assertThat(extraDataCodec.decode(genesisBlock).getVote()).isEmpty(); + + // uses previous block's validators + assertThat(validatorProvider.getValidatorsForBlock(block1)).isEqualTo(block0Addresses); + assertThat(extraDataCodec.decode(block1).getValidators()).containsExactly(NODE_ADDRESS); + } + + @Test + public void transitionsFromBlockHeaderModeToValidatorContractModeThenBack() { + final List qbftForks = + List.of( + createContractFork(1, TestContextBuilder.VALIDATOR_CONTRACT_ADDRESS), + createBlockHeaderFork(2)); + + final TestContext context = + new TestContextBuilder() + .indexOfFirstLocallyProposedBlock(1) + .nodeParams( + List.of( + new NodeParams(NODE_ADDRESS, NodeKeyUtils.createFrom(NODE_PRIVATE_KEY)), + new NodeParams(NODE_2_ADDRESS, NodeKeyUtils.createFrom(NODE_2_PRIVATE_KEY)))) + .clock(tickingClock) + .genesisFile( + Resources.getResource("genesis_migrating_validator_contract.json").getFile()) + .useValidatorContract(false) + .qbftForks(qbftForks) + .buildAndStart(); + + // block 0 uses block header voting with 1 validator + // block 1 uses validator contract with 2 validators + // block 2 uses block header voting with block 1's validators + final List
block0Addresses = List.of(NODE_ADDRESS); + final List
block1Addresses = + Stream.of(NODE_ADDRESS, NODE_2_ADDRESS).sorted().collect(Collectors.toList()); + + createNewBlockAsProposer(context, 1L); + remotePeerProposesNewBlock( + context, + 2L); // tickingClock moves forward to avoid the block failing TimestampMoreRecentThanParent + // validation rule + + final ValidatorProvider validatorProvider = context.getValidatorProvider(); + final BlockHeader genesisBlock = context.getBlockchain().getBlockHeader(0).get(); + final BlockHeader block1 = context.getBlockchain().getBlockHeader(1).get(); + final BlockHeader block2 = context.getBlockchain().getBlockHeader(2).get(); + + assertThat(validatorProvider.getValidatorsForBlock(genesisBlock)).isEqualTo(block0Addresses); + assertThat(extraDataCodec.decode(genesisBlock).getValidators()).containsExactly(NODE_ADDRESS); + + // contract block extra data cannot contain validators or vote + assertThat(validatorProvider.getValidatorsForBlock(block1)).isEqualTo(block1Addresses); + assertThat(extraDataCodec.decode(block1).getValidators()).isEmpty(); + assertThat(extraDataCodec.decode(block1).getVote()).isEmpty(); + + // uses previous block's validators + assertThat(validatorProvider.getValidatorsForBlock(block2)).isEqualTo(block1Addresses); + assertThat(extraDataCodec.decode(block2).getValidators()) + .containsExactly(NODE_2_ADDRESS, NODE_ADDRESS); + } + + private void createNewBlockAsProposer(final TestContext context, final long blockNumber) { + ConsensusRoundIdentifier roundId = new ConsensusRoundIdentifier(blockNumber, 0); + + context.getController().handleBlockTimerExpiry(new BlockTimerExpiry(roundId)); + assertThat(context.getBlockchain().getChainHeadBlockNumber()).isEqualTo(blockNumber); + context + .getController() + .handleNewBlockEvent(new NewChainHead(context.getBlockchain().getChainHeadHeader())); + } + + private void remotePeerProposesNewBlock(final TestContext context, final long blockNumber) { + ConsensusRoundIdentifier roundId = new ConsensusRoundIdentifier(blockNumber, 0); + + RoundSpecificPeers peers = context.roundSpecificPeers(roundId); + ValidatorPeer remoteProposer = peers.getProposer(); + final Block blockToPropose = + context.createBlockForProposalFromChainHead( + tickingClock.millis(), remoteProposer.getNodeAddress()); + remoteProposer.injectProposal(roundId, blockToPropose); + remoteProposer.injectCommit(roundId, blockToPropose); + assertThat(context.getBlockchain().getChainHeadBlockNumber()).isEqualTo(blockNumber); + + context + .getController() + .handleNewBlockEvent(new NewChainHead(context.getBlockchain().getChainHeadHeader())); + } + private QbftFork createContractFork(final long block, final Address contractAddress) { return new QbftFork( JsonUtil.objectNodeFromMap( @@ -132,4 +253,25 @@ private QbftFork createContractFork(final long block, final Address contractAddr QbftFork.VALIDATOR_CONTRACT_ADDRESS_KEY, contractAddress.toHexString()))); } + + private QbftFork createBlockHeaderFork(final long block) { + return new QbftFork( + JsonUtil.objectNodeFromMap( + Map.of( + BftFork.FORK_BLOCK_KEY, + block, + QbftFork.VALIDATOR_SELECTION_MODE_KEY, + VALIDATOR_SELECTION_MODE.BLOCKHEADER))); + } + + private Clock setupTickingClock() { + Clock tickingClock = mock(Clock.class); + Instant blockInstant = Instant.ofEpochSecond(blockTimeStamp); + // supports creation of up to three blocks + when(tickingClock.millis()) + .thenReturn(blockInstant.toEpochMilli()) + .thenReturn(blockInstant.plusSeconds(1).toEpochMilli()) + .thenReturn(blockInstant.plusSeconds(2).toEpochMilli()); + return tickingClock; + } } diff --git a/consensus/qbft/src/integration-test/resources/genesis_migrating_validator_blockheader.json b/consensus/qbft/src/integration-test/resources/genesis_migrating_validator_blockheader.json new file mode 100644 index 00000000000..44820ec94c2 --- /dev/null +++ b/consensus/qbft/src/integration-test/resources/genesis_migrating_validator_blockheader.json @@ -0,0 +1,35 @@ +{ + "nonce": "0x0", + "timestamp": "0x0", + "extraData": "0xe5a00000000000000000000000000000000000000000000000000000000000000000c0c080c0", + "gasLimit": "0x29b92700", + "difficulty": "0x1", + "mixHash": "0x63746963616c2062797a616e74696e65206661756c7420746f6c6572616e6365", + "coinbase": "0x0000000000000000000000000000000000000000", + "alloc": { + "64d9be4177f418bcf4e56adad85f33e3a64efe22": { + "balance": "0x446c3b15f9926687d2c40534fdb564000000000000" + }, + "9f66f8a0f0a6537e4a36aa1799673ea7ae97a166": { + "balance": "0x446c3b15f9926687d2c40534fdb564000000000000" + }, + "a7f25969fb6f3d5ac09a88862c90b5ff664557a7": { + "balance": "0x446c3b15f9926687d2c40534fdb564000000000000" + }, + "f4bbfd32c11c9d63e9b4c77bb225810f840342df": { + "balance": "0x446c3b15f9926687d2c40534fdb564000000000000" + }, + "0x0000000000000000000000000000000000008888": { + "comment": "validator smart contract. This is compiled from validator_contract.sol using solc --evm-version byzantium --bin-runtime validator_contract.sol", + "balance": "0", + "code": "608060405234801561001057600080fd5b5060043610610048576000357c010000000000000000000000000000000000000000000000000000000090048063b7ab4db51461004d575b600080fd5b61005561006b565b604051610062919061017e565b60405180910390f35b606060008054806020026020016040519081016040528092919081815260200182805480156100ef57602002820191906000526020600020905b8160009054906101000a900473ffffffffffffffffffffffffffffffffffffffff1673ffffffffffffffffffffffffffffffffffffffff16815260200190600101908083116100a5575b5050505050905090565b60006101058383610111565b60208301905092915050565b61011a816101d9565b82525050565b600061012b826101b0565b61013581856101c8565b9350610140836101a0565b8060005b8381101561017157815161015888826100f9565b9750610163836101bb565b925050600181019050610144565b5085935050505092915050565b600060208201905081810360008301526101988184610120565b905092915050565b6000819050602082019050919050565b600081519050919050565b6000602082019050919050565b600082825260208201905092915050565b60006101e4826101eb565b9050919050565b600073ffffffffffffffffffffffffffffffffffffffff8216905091905056fea26469706673582212206d880cf012c1677c691bf6f2f0a0e4eadf57866ffe5cd2d9833d3cfdf27b15f664736f6c63430008060033", + "storage": { + "0000000000000000000000000000000000000000000000000000000000000000": "0000000000000000000000000000000000000000000000000000000000000001", + "290decd9548b62a8d60345a988386fc84ba6bc95484008f6362f93160ef3e563": "000000000000000000000000eac51e3fe1afc9894f0dfeab8ceb471899b932df" + } + } + }, + "number": "0x0", + "gasUsed": "0x0", + "parentHash": "0x0000000000000000000000000000000000000000000000000000000000000000" +} diff --git a/consensus/qbft/src/integration-test/resources/genesis_migrating_validator_contract.json b/consensus/qbft/src/integration-test/resources/genesis_migrating_validator_contract.json index 046049c8054..a9e47a5baf7 100644 --- a/consensus/qbft/src/integration-test/resources/genesis_migrating_validator_contract.json +++ b/consensus/qbft/src/integration-test/resources/genesis_migrating_validator_contract.json @@ -1,21 +1,4 @@ { - "config": { - "chainid": 2017, - "homesteadBlock": 0, - "eip150Block": 0, - "eip155Block": 0, - "eip158Block": 0, - "byzantiumBlock": 0 - }, - "transitions": { - "qbft": [ - { - "block": 1, - "validatorSelectionMode": "contract", - "validatorContractAddress": "0x0000000000000000000000000000000000008888" - } - ] - }, "nonce": "0x0", "timestamp": "0x0", "extraData": "0xf83aa00000000000000000000000000000000000000000000000000000000000000000d594eac51e3fe1afc9894f0dfeab8ceb471899b932dfc080c0", @@ -42,7 +25,7 @@ "code": "608060405234801561001057600080fd5b5060043610610048576000357c010000000000000000000000000000000000000000000000000000000090048063b7ab4db51461004d575b600080fd5b61005561006b565b604051610062919061017e565b60405180910390f35b606060008054806020026020016040519081016040528092919081815260200182805480156100ef57602002820191906000526020600020905b8160009054906101000a900473ffffffffffffffffffffffffffffffffffffffff1673ffffffffffffffffffffffffffffffffffffffff16815260200190600101908083116100a5575b5050505050905090565b60006101058383610111565b60208301905092915050565b61011a816101d9565b82525050565b600061012b826101b0565b61013581856101c8565b9350610140836101a0565b8060005b8381101561017157815161015888826100f9565b9750610163836101bb565b925050600181019050610144565b5085935050505092915050565b600060208201905081810360008301526101988184610120565b905092915050565b6000819050602082019050919050565b600081519050919050565b6000602082019050919050565b600082825260208201905092915050565b60006101e4826101eb565b9050919050565b600073ffffffffffffffffffffffffffffffffffffffff8216905091905056fea26469706673582212206d880cf012c1677c691bf6f2f0a0e4eadf57866ffe5cd2d9833d3cfdf27b15f664736f6c63430008060033", "storage": { "0000000000000000000000000000000000000000000000000000000000000000": "0000000000000000000000000000000000000000000000000000000000000002", - "290decd9548b62a8d60345a988386fc84ba6bc95484008f6362f93160ef3e563": "000000000000000000000000ca1c5ff73ed8370397114006dd1258e16433f41b", + "290decd9548b62a8d60345a988386fc84ba6bc95484008f6362f93160ef3e563": "000000000000000000000000e98d92560fac3069ccff53ef348ded26a51d4b68", "290decd9548b62a8d60345a988386fc84ba6bc95484008f6362f93160ef3e564": "000000000000000000000000eac51e3fe1afc9894f0dfeab8ceb471899b932df" } } diff --git a/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/jsonrpc/methods/QbftDiscardValidatorVote.java b/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/jsonrpc/methods/QbftDiscardValidatorVote.java index 21c8d019a8f..ac1474188af 100644 --- a/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/jsonrpc/methods/QbftDiscardValidatorVote.java +++ b/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/jsonrpc/methods/QbftDiscardValidatorVote.java @@ -42,10 +42,10 @@ public String getName() { @Override public JsonRpcResponse response(final JsonRpcRequestContext requestContext) { - if (validatorProvider.getVoteProvider().isPresent()) { + if (validatorProvider.getVoteProviderAtHead().isPresent()) { final Address validatorAddress = requestContext.getRequiredParameter(0, Address.class); LOG.trace("Received RPC rpcName={} address={}", getName(), validatorAddress); - validatorProvider.getVoteProvider().get().discardVote(validatorAddress); + validatorProvider.getVoteProviderAtHead().get().discardVote(validatorAddress); return new JsonRpcSuccessResponse(requestContext.getRequest().getId(), true); } else { diff --git a/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/jsonrpc/methods/QbftProposeValidatorVote.java b/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/jsonrpc/methods/QbftProposeValidatorVote.java index 74dbbaaf770..8a63578e8ab 100644 --- a/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/jsonrpc/methods/QbftProposeValidatorVote.java +++ b/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/jsonrpc/methods/QbftProposeValidatorVote.java @@ -43,7 +43,7 @@ public String getName() { @Override public JsonRpcResponse response(final JsonRpcRequestContext requestContext) { - if (validatorProvider.getVoteProvider().isPresent()) { + if (validatorProvider.getVoteProviderAtHead().isPresent()) { final Address validatorAddress = requestContext.getRequiredParameter(0, Address.class); final Boolean add = requestContext.getRequiredParameter(1, Boolean.class); LOG.trace( @@ -53,9 +53,9 @@ public JsonRpcResponse response(final JsonRpcRequestContext requestContext) { validatorAddress); if (add) { - validatorProvider.getVoteProvider().get().authVote(validatorAddress); + validatorProvider.getVoteProviderAtHead().get().authVote(validatorAddress); } else { - validatorProvider.getVoteProvider().get().dropVote(validatorAddress); + validatorProvider.getVoteProviderAtHead().get().dropVote(validatorAddress); } return new JsonRpcSuccessResponse(requestContext.getRequest().getId(), true); } else { diff --git a/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/statemachine/QbftBlockHeightManager.java b/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/statemachine/QbftBlockHeightManager.java index dc97f97b7d3..a4f4f63dc6a 100644 --- a/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/statemachine/QbftBlockHeightManager.java +++ b/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/statemachine/QbftBlockHeightManager.java @@ -108,7 +108,7 @@ public void handleBlockTimerExpiry(final ConsensusRoundIdentifier roundIdentifie LOG.warn( "Block timer expired for round ({}) after round has already started on round ({})", roundIdentifier, - currentRound.get()); + currentRound.get().getRoundIdentifier()); return; } diff --git a/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/validator/ForkingValidatorProvider.java b/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/validator/ForkingValidatorProvider.java index f13c33abb44..354927d47c8 100644 --- a/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/validator/ForkingValidatorProvider.java +++ b/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/validator/ForkingValidatorProvider.java @@ -64,8 +64,14 @@ public Collection
getValidatorsForBlock(final BlockHeader header) { } @Override - public Optional getVoteProvider() { - return resolveValidatorProvider(blockchain.getChainHeadHeader().getNumber()).getVoteProvider(); + public Optional getVoteProviderAtHead() { + return resolveValidatorProvider(blockchain.getChainHeadHeader().getNumber()) + .getVoteProviderAtHead(); + } + + @Override + public Optional getVoteProviderAfterBlock(final BlockHeader header) { + return resolveValidatorProvider(header.getNumber() + 1).getVoteProviderAtHead(); } private Collection
getValidators( diff --git a/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/validator/TransactionValidatorProvider.java b/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/validator/TransactionValidatorProvider.java index 305d01d7e51..327cf2706c7 100644 --- a/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/validator/TransactionValidatorProvider.java +++ b/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/validator/TransactionValidatorProvider.java @@ -68,7 +68,7 @@ public Collection
getValidatorsForBlock(final BlockHeader header) { } @Override - public Optional getVoteProvider() { + public Optional getVoteProviderAtHead() { return Optional.empty(); } } diff --git a/consensus/qbft/src/test/java/org/hyperledger/besu/consensus/qbft/jsonrpc/methods/QbftDiscardValidatorVoteTest.java b/consensus/qbft/src/test/java/org/hyperledger/besu/consensus/qbft/jsonrpc/methods/QbftDiscardValidatorVoteTest.java index a29fb3eeb54..db15f36d2ee 100644 --- a/consensus/qbft/src/test/java/org/hyperledger/besu/consensus/qbft/jsonrpc/methods/QbftDiscardValidatorVoteTest.java +++ b/consensus/qbft/src/test/java/org/hyperledger/besu/consensus/qbft/jsonrpc/methods/QbftDiscardValidatorVoteTest.java @@ -49,7 +49,7 @@ public class QbftDiscardValidatorVoteTest { @Before public void setup() { method = new QbftDiscardValidatorVote(validatorProvider); - when(validatorProvider.getVoteProvider()).thenReturn(Optional.of(voteProvider)); + when(validatorProvider.getVoteProviderAtHead()).thenReturn(Optional.of(voteProvider)); } @Test @@ -82,7 +82,7 @@ public void methodNotEnabledWhenNoVoteProvider() { final JsonRpcRequestContext request = requestWithParams(Address.fromHexString("1")); final JsonRpcResponse expectedResponse = new JsonRpcErrorResponse(request.getRequest().getId(), JsonRpcError.METHOD_NOT_ENABLED); - when(validatorProvider.getVoteProvider()).thenReturn(Optional.empty()); + when(validatorProvider.getVoteProviderAtHead()).thenReturn(Optional.empty()); final JsonRpcResponse response = method.response(request); diff --git a/consensus/qbft/src/test/java/org/hyperledger/besu/consensus/qbft/jsonrpc/methods/QbftProposeValidatorVoteTest.java b/consensus/qbft/src/test/java/org/hyperledger/besu/consensus/qbft/jsonrpc/methods/QbftProposeValidatorVoteTest.java index 550baff4c32..397ea3098f3 100644 --- a/consensus/qbft/src/test/java/org/hyperledger/besu/consensus/qbft/jsonrpc/methods/QbftProposeValidatorVoteTest.java +++ b/consensus/qbft/src/test/java/org/hyperledger/besu/consensus/qbft/jsonrpc/methods/QbftProposeValidatorVoteTest.java @@ -49,7 +49,7 @@ public class QbftProposeValidatorVoteTest { @Before public void setup() { method = new QbftProposeValidatorVote(validatorProvider); - when(validatorProvider.getVoteProvider()).thenReturn(Optional.of(voteProvider)); + when(validatorProvider.getVoteProviderAtHead()).thenReturn(Optional.of(voteProvider)); } @Test @@ -102,7 +102,7 @@ public void methodNotEnabledWhenNoVoteProvider() { final JsonRpcRequestContext request = requestWithParams(Address.fromHexString("1")); final JsonRpcResponse expectedResponse = new JsonRpcErrorResponse(request.getRequest().getId(), JsonRpcError.METHOD_NOT_ENABLED); - when(validatorProvider.getVoteProvider()).thenReturn(Optional.empty()); + when(validatorProvider.getVoteProviderAtHead()).thenReturn(Optional.empty()); final JsonRpcResponse response = method.response(request); diff --git a/consensus/qbft/src/test/java/org/hyperledger/besu/consensus/qbft/validator/ForkingValidatorProviderTest.java b/consensus/qbft/src/test/java/org/hyperledger/besu/consensus/qbft/validator/ForkingValidatorProviderTest.java index f76fff53449..f3959dde940 100644 --- a/consensus/qbft/src/test/java/org/hyperledger/besu/consensus/qbft/validator/ForkingValidatorProviderTest.java +++ b/consensus/qbft/src/test/java/org/hyperledger/besu/consensus/qbft/validator/ForkingValidatorProviderTest.java @@ -19,6 +19,8 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.hyperledger.besu.ethereum.core.InMemoryKeyValueStorageProvider.createInMemoryBlockchain; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; import org.hyperledger.besu.config.JsonQbftConfigOptions; @@ -41,6 +43,7 @@ import java.util.Optional; import org.apache.tuweni.bytes.Bytes; +import org.assertj.core.api.SoftAssertions; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -160,7 +163,7 @@ public void migratesFromContractToContractValidatorProvider() { } @Test - public void voteProviderIsDelegatesToHeadFork() { + public void voteProviderIsDelegatesToHeadFork_whenHeadIsContractFork() { final BftForksSchedule forksSchedule = new BftForksSchedule<>( createBlockFork(0), @@ -170,10 +173,50 @@ public void voteProviderIsDelegatesToHeadFork() { new ForkingValidatorProvider( blockChain, forksSchedule, blockValidatorProvider, contractValidatorProvider); - final VoteProvider voteProvider = Mockito.mock(VoteProvider.class); - when(contractValidatorProvider.getVoteProvider()).thenReturn(Optional.of(voteProvider)); + validatorProvider.getVoteProviderAtHead(); - assertThat(validatorProvider.getVoteProvider()).contains(voteProvider); + verify(contractValidatorProvider).getVoteProviderAtHead(); + verifyNoInteractions(blockValidatorProvider); + } + + @Test + public void voteProviderIsDelegatesToHeadFork_whenHeadIsBlockFork() { + final BftForksSchedule forksSchedule = + new BftForksSchedule<>(createBlockFork(0), emptyList()); + + final ForkingValidatorProvider validatorProvider = + new ForkingValidatorProvider( + blockChain, forksSchedule, blockValidatorProvider, contractValidatorProvider); + + validatorProvider.getVoteProviderAtHead(); + + verify(blockValidatorProvider).getVoteProviderAtHead(); + verifyNoInteractions(contractValidatorProvider); + } + + @Test + public void getVoteProviderAfterBlock_correctVoteProviderIsResolved() { + final BftForksSchedule forksSchedule = + new BftForksSchedule<>( + createBlockFork(0), + List.of(createBlockFork(1), createContractFork(2, CONTRACT_ADDRESS_1))); + final ForkingValidatorProvider validatorProvider = + new ForkingValidatorProvider( + blockChain, forksSchedule, blockValidatorProvider, contractValidatorProvider); + + final VoteProvider voteProviderForBlockValidator = Mockito.mock(VoteProvider.class); + when(blockValidatorProvider.getVoteProviderAtHead()) + .thenReturn(Optional.of(voteProviderForBlockValidator)); + when(contractValidatorProvider.getVoteProviderAtHead()).thenReturn(Optional.empty()); + + SoftAssertions.assertSoftly( + (softly) -> { + softly + .assertThat(validatorProvider.getVoteProviderAfterBlock(genesisHeader)) + .contains(voteProviderForBlockValidator); + softly.assertThat(validatorProvider.getVoteProviderAfterBlock(header1)).isEmpty(); + softly.assertThat(validatorProvider.getVoteProviderAfterBlock(header2)).isEmpty(); + }); } private BftForkSpec createContractFork( diff --git a/consensus/qbft/src/test/java/org/hyperledger/besu/consensus/qbft/validator/TransactionValidatorProviderTest.java b/consensus/qbft/src/test/java/org/hyperledger/besu/consensus/qbft/validator/TransactionValidatorProviderTest.java index b9870185db6..80be8c2bb6f 100644 --- a/consensus/qbft/src/test/java/org/hyperledger/besu/consensus/qbft/validator/TransactionValidatorProviderTest.java +++ b/consensus/qbft/src/test/java/org/hyperledger/besu/consensus/qbft/validator/TransactionValidatorProviderTest.java @@ -178,6 +178,6 @@ public void voteProviderIsEmpty() { TransactionValidatorProvider transactionValidatorProvider = new TransactionValidatorProvider(blockChain, validatorContractController); - assertThat(transactionValidatorProvider.getVoteProvider()).isEmpty(); + assertThat(transactionValidatorProvider.getVoteProviderAtHead()).isEmpty(); } }