Skip to content

Commit

Permalink
Fix for Select correct ValidatorProvider when transitioning from vali…
Browse files Browse the repository at this point in the history
…dator 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 <simon.dudley@consensys.net>
  • Loading branch information
siladu authored Oct 13, 2021
1 parent 90fca1c commit 71e6b0c
Show file tree
Hide file tree
Showing 27 changed files with 318 additions and 93 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -108,11 +108,11 @@ private Optional<ValidatorVote> 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()));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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 =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,19 +64,19 @@ 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
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);
Expand All @@ -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);
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand All @@ -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;
Expand All @@ -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);
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<VoteProvider> voteProviderAfterBlock =
validatorProvider.getVoteProviderAfterBlock(parentHeader);
checkState(voteProviderAfterBlock.isPresent(), "Bft requires a vote provider");
final Optional<ValidatorVote> proposal =
validatorProvider.getVoteProvider().get().getVoteAfterBlock(parentHeader, localAddress);
voteProviderAfterBlock.get().getVoteAfterBlock(parentHeader, localAddress);

final List<Address> validators =
new ArrayList<>(validatorProvider.getValidatorsAfterBlock(parentHeader));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<String, Boolean> proposals =
validatorProvider.getVoteProvider().get().getProposals().entrySet().stream()
validatorProvider.getVoteProviderAtHead().get().getProposals().entrySet().stream()
.collect(
Collectors.toMap(
proposal -> proposal.getKey().toString(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,5 +28,14 @@ public interface ValidatorProvider {

Collection<Address> getValidatorsForBlock(BlockHeader header);

Optional<VoteProvider> getVoteProvider();
Optional<VoteProvider> 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<VoteProvider> getVoteProviderAfterBlock(final BlockHeader header) {
return getVoteProviderAtHead();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ public Collection<Address> getValidatorsForBlock(final BlockHeader header) {
}

@Override
public Optional<VoteProvider> getVoteProvider() {
public Optional<VoteProvider> getVoteProviderAtHead() {
return Optional.of(voteProvider);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading

0 comments on commit 71e6b0c

Please sign in to comment.