Skip to content

Commit

Permalink
Use correct list of validators when transitioning to different a vali…
Browse files Browse the repository at this point in the history
…datorselectionmode (#2935)

When producing the next block, the previousBlockHeader.number + 1 needs to be used to query the transition forkSchedule.
This is because the list of validators associated with each forkSchedule may be a different set to the current block.

This commit changes the implementation for getValidatorsAfterBlock to support this and points getValidatorsAtHead at this implementation.
Note getValidatorsForBlock's forkSchedule query is unchanged since we have already mined the block at the point this is called (e.g. for RPCs qbft_getValidatorsBy...).

For TransactionValidatorProvider, we need to use the next block's contract address but still use the previous block's validators in the contract call itself.
Querying validators using afterBlock vs forBlock may yield different (blockNumber -> validatorAddresses) combinations when transitioning into CONTRACT mode.
There we also split TransactionValidatorProvider validatorCache into two caches.

Add special case to ForkingValidatorProvider (BlockValidatorProvider.hasValidatorOverridesForBlockNumber) to support transitioning to blockheader mode with some validator overrides. This might be needed if the current validator set loses quorum and we
are no longer able to vote new ones in.

New integration test cases for validatorselectionmode transitions:
 - contract -> contract
 - contract1 -> blockheader -> contract2
 - contract1 -> blockheader with overridden validators -> contract2

Fixes #2901

Signed-off-by: Simon Dudley <[email protected]>
  • Loading branch information
siladu authored Nov 8, 2021
1 parent 7659c76 commit 0ffe977
Show file tree
Hide file tree
Showing 15 changed files with 513 additions and 169 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@ protected BftContext createConsensusContext(
new TransactionSimulator(blockchain, worldStateArchive, protocolSchedule);
transactionValidatorProvider =
new TransactionValidatorProvider(
blockchain, new ValidatorContractController(transactionSimulator, qbftForksSchedule));
blockchain, new ValidatorContractController(transactionSimulator), qbftForksSchedule);

final ValidatorProvider validatorProvider =
new ForkingValidatorProvider(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ public class BlockValidatorProvider implements ValidatorProvider {
private final VoteTallyCache voteTallyCache;
private final VoteProvider voteProvider;
private final BlockInterface blockInterface;
private final Optional<BftValidatorOverrides> bftValidatorOverrides;

public static BlockValidatorProvider forkingValidatorProvider(
final Blockchain blockchain,
Expand Down Expand Up @@ -67,6 +68,7 @@ private BlockValidatorProvider(
: new VoteTallyCache(blockchain, voteTallyUpdater, epochManager, blockInterface);
this.voteProvider = new BlockVoteProvider(voteTallyCache, voteProposer);
this.blockInterface = blockInterface;
this.bftValidatorOverrides = bftValidatorOverrides;
}

@Override
Expand All @@ -88,4 +90,8 @@ public Collection<Address> getValidatorsForBlock(final BlockHeader header) {
public Optional<VoteProvider> getVoteProviderAtHead() {
return Optional.of(voteProvider);
}

public boolean hasValidatorOverridesForBlockNumber(final long blockNumber) {
return bftValidatorOverrides.map(bvo -> bvo.getForBlock(blockNumber).isPresent()).orElse(false);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
/*
* Copyright Hyperledger Besu contributors.
*
* 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
*/
package org.hyperledger.besu.consensus.common.validator.blockbased;

import static java.util.Collections.emptyMap;
import static org.assertj.core.api.Assertions.assertThat;

import org.hyperledger.besu.consensus.common.BftValidatorOverrides;
import org.hyperledger.besu.consensus.common.BlockInterface;
import org.hyperledger.besu.consensus.common.EpochManager;
import org.hyperledger.besu.datatypes.Address;
import org.hyperledger.besu.ethereum.chain.Blockchain;

import java.util.List;
import java.util.Map;

import org.assertj.core.api.SoftAssertions;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.Mock;
import org.mockito.junit.MockitoJUnitRunner;

@RunWith(MockitoJUnitRunner.class)
public class BlockValidatorProviderTest {

@Mock private BlockInterface blockInterface;
@Mock private Blockchain blockchain;
@Mock private EpochManager epochManager;

@Test
public void nonForkingValidatorProviderHasNoOverrides() {
final BlockValidatorProvider blockValidatorProvider =
BlockValidatorProvider.nonForkingValidatorProvider(
blockchain, epochManager, blockInterface);

assertThat(blockValidatorProvider.hasValidatorOverridesForBlockNumber(0)).isFalse();
}

@Test
public void forkingValidatorProviderHasNoOverrides() {
final BlockValidatorProvider blockValidatorProvider =
BlockValidatorProvider.forkingValidatorProvider(
blockchain, epochManager, blockInterface, new BftValidatorOverrides(emptyMap()));

assertThat(blockValidatorProvider.hasValidatorOverridesForBlockNumber(0)).isFalse();
}

@Test
public void forkingValidatorProviderHasOverridesForBlock1() {
final Map<Long, List<Address>> overriddenValidators =
Map.of(1L, List.of(Address.fromHexString("0")));
final BftValidatorOverrides bftValidatorOverrides =
new BftValidatorOverrides(overriddenValidators);
final BlockValidatorProvider blockValidatorProvider =
BlockValidatorProvider.forkingValidatorProvider(
blockchain, epochManager, blockInterface, bftValidatorOverrides);

SoftAssertions.assertSoftly(
(softly) -> {
softly
.assertThat(blockValidatorProvider.hasValidatorOverridesForBlockNumber(0))
.as("Block 0 should have no overridden validators")
.isFalse();
softly
.assertThat(blockValidatorProvider.hasValidatorOverridesForBlockNumber(1))
.as("Block 1 should have some overridden validators")
.isTrue();
softly
.assertThat(blockValidatorProvider.hasValidatorOverridesForBlockNumber(2))
.as("Block 2 should have no overridden validators")
.isFalse();
});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import static org.hyperledger.besu.ethereum.core.InMemoryKeyValueStorageProvider.createInMemoryWorldStateArchive;
import static org.mockito.Mockito.mock;

import org.hyperledger.besu.config.BftFork;
import org.hyperledger.besu.config.JsonQbftConfigOptions;
import org.hyperledger.besu.config.JsonUtil;
import org.hyperledger.besu.config.QbftConfigOptions;
Expand Down Expand Up @@ -108,6 +109,7 @@
import java.time.ZoneId;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -413,8 +415,7 @@ private static ControllerAndState createControllerAndFinalState(
QbftProtocolSchedule.create(
genesisConfigOptions, forksSchedule, BFT_EXTRA_DATA_ENCODER, EvmConfiguration.DEFAULT);

final BftValidatorOverrides validatorOverrides =
new BftValidatorOverrides(Collections.emptyMap());
final BftValidatorOverrides validatorOverrides = convertBftForks(qbftForks);
final TransactionSimulator transactionSimulator =
new TransactionSimulator(blockChain, worldStateArchive, protocolSchedule);

Expand All @@ -423,7 +424,7 @@ private static ControllerAndState createControllerAndFinalState(
blockChain, epochManager, blockInterface, validatorOverrides);
final TransactionValidatorProvider transactionValidatorProvider =
new TransactionValidatorProvider(
blockChain, new ValidatorContractController(transactionSimulator, forksSchedule));
blockChain, new ValidatorContractController(transactionSimulator), forksSchedule);
final ValidatorProvider validatorProvider =
new ForkingValidatorProvider(
blockChain, forksSchedule, blockValidatorProvider, transactionValidatorProvider);
Expand Down Expand Up @@ -532,4 +533,21 @@ private static QbftConfigOptions createGenesisConfig(final boolean useValidatorC
}
return qbftConfigOptions;
}

private static BftValidatorOverrides convertBftForks(final List<QbftFork> bftForks) {
final Map<Long, List<Address>> result = new HashMap<>();

for (final BftFork fork : bftForks) {
fork.getValidators()
.ifPresent(
validators ->
result.put(
fork.getForkBlock(),
validators.stream()
.map(Address::fromHexString)
.collect(Collectors.toList())));
}

return new BftValidatorOverrides(result);
}
}
Loading

0 comments on commit 0ffe977

Please sign in to comment.