From 5514ed5593f722b460a710b20d08a7cbcb9491e2 Mon Sep 17 00:00:00 2001 From: Simon Dudley Date: Mon, 11 Oct 2021 08:18:03 +0100 Subject: [PATCH] Qbft RPCs should be disabled after starting with or switching to use validator contract (#2817) Signed-off-by: Simon Dudley --- .../AbstractVoteProposerMethodTest.java | 17 +++++++ .../methods/QbftDiscardValidatorVote.java | 2 +- .../methods/QbftProposeValidatorVote.java | 2 +- .../NoOpTransactionVoteProvider.java | 47 ------------------- .../TransactionValidatorProvider.java | 2 +- .../methods/QbftDiscardValidatorVoteTest.java | 16 ++++++- .../methods/QbftProposeValidatorVoteTest.java | 18 ++++++- .../TransactionValidatorProviderTest.java | 8 ++++ 8 files changed, 59 insertions(+), 53 deletions(-) delete mode 100644 consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/validator/NoOpTransactionVoteProvider.java 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 23406ae3065..3e9dd745ef4 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 @@ -24,12 +24,15 @@ import org.hyperledger.besu.datatypes.Address; import org.hyperledger.besu.ethereum.api.jsonrpc.internal.JsonRpcRequest; import org.hyperledger.besu.ethereum.api.jsonrpc.internal.JsonRpcRequestContext; +import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcError; +import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcErrorResponse; import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcResponse; import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcSuccessResponse; import java.util.Optional; import com.google.common.collect.ImmutableMap; +import org.assertj.core.api.Assertions; import org.junit.Test; public abstract class AbstractVoteProposerMethodTest { @@ -74,4 +77,18 @@ public void testConversionFromVoteTypeToBoolean() { assertThat(response).isEqualToComparingFieldByField(expectedResponse); } + + @Test + public void methodNotEnabledWhenNoVoteProvider() { + final JsonRpcRequestContext request = + new JsonRpcRequestContext( + 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()); + + final JsonRpcResponse response = getMethod().response(request); + + Assertions.assertThat(response).usingRecursiveComparison().isEqualTo(expectedResponse); + } } 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 f712793d78d..21c8d019a8f 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 @@ -50,7 +50,7 @@ public JsonRpcResponse response(final JsonRpcRequestContext requestContext) { return new JsonRpcSuccessResponse(requestContext.getRequest().getId(), true); } else { return new JsonRpcErrorResponse( - requestContext.getRequest().getId(), JsonRpcError.METHOD_UNIMPLEMENTED); + requestContext.getRequest().getId(), JsonRpcError.METHOD_NOT_ENABLED); } } } 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 9303019f660..74dbbaaf770 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 @@ -60,7 +60,7 @@ public JsonRpcResponse response(final JsonRpcRequestContext requestContext) { return new JsonRpcSuccessResponse(requestContext.getRequest().getId(), true); } else { return new JsonRpcErrorResponse( - requestContext.getRequest().getId(), JsonRpcError.METHOD_UNIMPLEMENTED); + requestContext.getRequest().getId(), JsonRpcError.METHOD_NOT_ENABLED); } } } diff --git a/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/validator/NoOpTransactionVoteProvider.java b/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/validator/NoOpTransactionVoteProvider.java deleted file mode 100644 index 8eb5435a464..00000000000 --- a/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/validator/NoOpTransactionVoteProvider.java +++ /dev/null @@ -1,47 +0,0 @@ -package org.hyperledger.besu.consensus.qbft.validator; -/* - * Copyright ConsenSys AG. - * - * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on - * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the - * specific language governing permissions and limitations under the License. - * - * SPDX-License-Identifier: Apache-2.0 - */ -import org.hyperledger.besu.consensus.common.validator.ValidatorVote; -import org.hyperledger.besu.consensus.common.validator.VoteProvider; -import org.hyperledger.besu.consensus.common.validator.VoteType; -import org.hyperledger.besu.datatypes.Address; -import org.hyperledger.besu.ethereum.core.BlockHeader; - -import java.util.Collections; -import java.util.Map; -import java.util.Optional; - -public class NoOpTransactionVoteProvider implements VoteProvider { - - @Override - public Optional getVoteAfterBlock( - final BlockHeader header, final Address localAddress) { - return Optional.empty(); - } - - @Override - public void authVote(final Address address) {} - - @Override - public void dropVote(final Address address) {} - - @Override - public void discardVote(final Address address) {} - - @Override - public Map getProposals() { - return Collections.emptyMap(); - } -} 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 934953fe187..305d01d7e51 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 @@ -69,6 +69,6 @@ public Collection
getValidatorsForBlock(final BlockHeader header) { @Override public Optional getVoteProvider() { - return Optional.of(new NoOpTransactionVoteProvider()); + 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 d15c2a7453e..a29fb3eeb54 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 @@ -25,6 +25,8 @@ import org.hyperledger.besu.ethereum.api.jsonrpc.internal.JsonRpcRequest; import org.hyperledger.besu.ethereum.api.jsonrpc.internal.JsonRpcRequestContext; import org.hyperledger.besu.ethereum.api.jsonrpc.internal.exception.InvalidJsonRpcParameters; +import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcError; +import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcErrorResponse; import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcResponse; import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcSuccessResponse; @@ -75,6 +77,18 @@ public void exceptionWhenInvalidAddressParameterSupplied() { method.response(request); } + @Test + 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()); + + final JsonRpcResponse response = method.response(request); + + assertThat(response).usingRecursiveComparison().isEqualTo(expectedResponse); + } + @Test public void discardValidator() { final Address parameterAddress = Address.fromHexString("1"); @@ -85,7 +99,7 @@ public void discardValidator() { final JsonRpcResponse response = method.response(request); - assertThat(response).isEqualToComparingFieldByField(expectedResponse); + assertThat(response).usingRecursiveComparison().isEqualTo(expectedResponse); verify(voteProvider).discardVote(parameterAddress); } 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 018207c874a..550baff4c32 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 @@ -25,6 +25,8 @@ import org.hyperledger.besu.ethereum.api.jsonrpc.internal.JsonRpcRequest; import org.hyperledger.besu.ethereum.api.jsonrpc.internal.JsonRpcRequestContext; import org.hyperledger.besu.ethereum.api.jsonrpc.internal.exception.InvalidJsonRpcParameters; +import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcError; +import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcErrorResponse; import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcResponse; import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcSuccessResponse; @@ -95,6 +97,18 @@ public void exceptionWhenInvalidBoolParameterSupplied() { method.response(request); } + @Test + 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()); + + final JsonRpcResponse response = method.response(request); + + assertThat(response).usingRecursiveComparison().isEqualTo(expectedResponse); + } + @Test public void addValidator() { final Address parameterAddress = Address.fromHexString("1"); @@ -104,7 +118,7 @@ public void addValidator() { final JsonRpcResponse response = method.response(request); - assertThat(response).isEqualToComparingFieldByField(expectedResponse); + assertThat(response).usingRecursiveComparison().isEqualTo(expectedResponse); verify(voteProvider).authVote(parameterAddress); } @@ -118,7 +132,7 @@ public void removeValidator() { final JsonRpcResponse response = method.response(request); - assertThat(response).isEqualToComparingFieldByField(expectedResponse); + assertThat(response).usingRecursiveComparison().isEqualTo(expectedResponse); verify(voteProvider).dropVote(parameterAddress); } 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 cddaeeafab5..b9870185db6 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 @@ -172,4 +172,12 @@ public void validatorsMustBeSorted() { validators.stream().sorted().collect(Collectors.toList()); assertThat(result).containsExactlyElementsOf(expectedValidators); } + + @Test + public void voteProviderIsEmpty() { + TransactionValidatorProvider transactionValidatorProvider = + new TransactionValidatorProvider(blockChain, validatorContractController); + + assertThat(transactionValidatorProvider.getVoteProvider()).isEmpty(); + } }