From 5bdf698b725121a182c4842871598c8b8caf347a Mon Sep 17 00:00:00 2001 From: tmohay <37158202+rain-on@users.noreply.github.com> Date: Fri, 14 Dec 2018 16:21:45 +1100 Subject: [PATCH] Repair IbftBlockCreator and add tests (#421) IbftBlockCreator is required to correctly set the MixHash, and also ensure the IbftBlockHashing function used to hash the block includes the round number, but not the commit seals. --- .../ibft/blockcreation/IbftBlockCreator.java | 16 +-- .../blockcreation/IbftBlockCreatorTest.java | 117 ++++++++++++++++++ 2 files changed, 126 insertions(+), 7 deletions(-) create mode 100644 consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/blockcreation/IbftBlockCreatorTest.java diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/blockcreation/IbftBlockCreator.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/blockcreation/IbftBlockCreator.java index de0fe34829..67396cc692 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/blockcreation/IbftBlockCreator.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/blockcreation/IbftBlockCreator.java @@ -12,25 +12,26 @@ */ package tech.pegasys.pantheon.consensus.ibft.blockcreation; +import tech.pegasys.pantheon.consensus.ibft.IbftBlockHashing; import tech.pegasys.pantheon.consensus.ibft.IbftContext; +import tech.pegasys.pantheon.consensus.ibft.IbftExtraData; +import tech.pegasys.pantheon.consensus.ibft.IbftHelpers; import tech.pegasys.pantheon.ethereum.ProtocolContext; import tech.pegasys.pantheon.ethereum.blockcreation.AbstractBlockCreator; import tech.pegasys.pantheon.ethereum.core.Address; -import tech.pegasys.pantheon.ethereum.core.BlockHashFunction; import tech.pegasys.pantheon.ethereum.core.BlockHeader; import tech.pegasys.pantheon.ethereum.core.BlockHeaderBuilder; -import tech.pegasys.pantheon.ethereum.core.Hash; import tech.pegasys.pantheon.ethereum.core.PendingTransactions; import tech.pegasys.pantheon.ethereum.core.SealableBlockHeader; import tech.pegasys.pantheon.ethereum.core.Wei; import tech.pegasys.pantheon.ethereum.mainnet.ProtocolSchedule; -import tech.pegasys.pantheon.ethereum.mainnet.ScheduleBasedBlockHashFunction; import java.util.function.Function; // This class is responsible for creating a block without committer seals (basically it was just // too hard to coordinate with the state machine). public class IbftBlockCreator extends AbstractBlockCreator { + public IbftBlockCreator( final Address localAddress, final ExtraDataCalculator extraDataCalculator, @@ -55,15 +56,16 @@ public class IbftBlockCreator extends AbstractBlockCreator { @Override protected BlockHeader createFinalBlockHeader(final SealableBlockHeader sealableBlockHeader) { - final BlockHashFunction blockHashFunction = - ScheduleBasedBlockHashFunction.create(protocolSchedule); + final IbftExtraData extraData = IbftExtraData.decode(sealableBlockHeader.getExtraData()); final BlockHeaderBuilder builder = BlockHeaderBuilder.create() .populateFrom(sealableBlockHeader) - .mixHash(Hash.ZERO) + .mixHash(IbftHelpers.EXPECTED_MIX_HASH) .nonce(0L) - .blockHashFunction(blockHashFunction); + .blockHashFunction( + blockHeader -> + IbftBlockHashing.calculateDataHashForCommittedSeal(blockHeader, extraData)); return builder.buildBlockHeader(); } diff --git a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/blockcreation/IbftBlockCreatorTest.java b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/blockcreation/IbftBlockCreatorTest.java new file mode 100644 index 0000000000..71f45a372d --- /dev/null +++ b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/blockcreation/IbftBlockCreatorTest.java @@ -0,0 +1,117 @@ +/* + * Copyright 2018 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. + */ +package tech.pegasys.pantheon.consensus.ibft.blockcreation; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import static tech.pegasys.pantheon.ethereum.core.InMemoryStorageProvider.createInMemoryWorldStateArchive; + +import tech.pegasys.pantheon.config.GenesisConfigFile; +import tech.pegasys.pantheon.consensus.common.VoteProposer; +import tech.pegasys.pantheon.consensus.common.VoteTally; +import tech.pegasys.pantheon.consensus.ibft.IbftBlockHashing; +import tech.pegasys.pantheon.consensus.ibft.IbftBlockHeaderValidationRulesetFactory; +import tech.pegasys.pantheon.consensus.ibft.IbftContext; +import tech.pegasys.pantheon.consensus.ibft.IbftExtraData; +import tech.pegasys.pantheon.consensus.ibft.IbftProtocolSchedule; +import tech.pegasys.pantheon.ethereum.ProtocolContext; +import tech.pegasys.pantheon.ethereum.chain.MutableBlockchain; +import tech.pegasys.pantheon.ethereum.core.Address; +import tech.pegasys.pantheon.ethereum.core.AddressHelpers; +import tech.pegasys.pantheon.ethereum.core.Block; +import tech.pegasys.pantheon.ethereum.core.BlockHeader; +import tech.pegasys.pantheon.ethereum.core.BlockHeaderTestFixture; +import tech.pegasys.pantheon.ethereum.core.PendingTransactions; +import tech.pegasys.pantheon.ethereum.core.Wei; +import tech.pegasys.pantheon.ethereum.mainnet.BlockHeaderValidator; +import tech.pegasys.pantheon.ethereum.mainnet.HeaderValidationMode; +import tech.pegasys.pantheon.ethereum.mainnet.ProtocolSchedule; +import tech.pegasys.pantheon.util.bytes.BytesValue; + +import java.time.Instant; +import java.util.Collections; +import java.util.List; +import java.util.Optional; + +import com.google.common.collect.Lists; +import org.junit.Test; + +public class IbftBlockCreatorTest { + + @Test + public void createdBlockPassesValidationRulesAndHasAppropriateHashAndMixHash() { + // Construct a parent block. + final BlockHeaderTestFixture blockHeaderBuilder = new BlockHeaderTestFixture(); + blockHeaderBuilder.gasLimit(5000); // required to pass validation rule checks. + final BlockHeader parentHeader = blockHeaderBuilder.buildHeader(); + final Optional optionalHeader = Optional.of(parentHeader); + + // Construct a block chain and world state + final MutableBlockchain blockchain = mock(MutableBlockchain.class); + when(blockchain.getChainHeadHash()).thenReturn(parentHeader.getHash()); + when(blockchain.getBlockHeader(any())).thenReturn(optionalHeader); + + final List
initialValidatorList = Lists.newArrayList(); + for (int i = 0; i < 4; i++) { + initialValidatorList.add(AddressHelpers.ofValue(i)); + } + + final VoteTally voteTally = new VoteTally(initialValidatorList); + + final ProtocolSchedule protocolSchedule = + IbftProtocolSchedule.create( + GenesisConfigFile.fromConfig("{\"config\": {\"spuriousDragonBlock\":0}}") + .getConfigOptions()); + final ProtocolContext protContext = + new ProtocolContext<>( + blockchain, + createInMemoryWorldStateArchive(), + new IbftContext(voteTally, new VoteProposer())); + + final IbftBlockCreator blockCreator = + new IbftBlockCreator( + initialValidatorList.get(0), + parent -> + new IbftExtraData( + BytesValue.wrap(new byte[32]), + Collections.emptyList(), + Optional.empty(), + 0, + initialValidatorList) + .encode(), + new PendingTransactions(1), + protContext, + protocolSchedule, + parentGasLimit -> parentGasLimit, + Wei.ZERO, + parentHeader); + + final Block block = blockCreator.createBlock(Instant.now().getEpochSecond()); + + final BlockHeaderValidator rules = + IbftBlockHeaderValidationRulesetFactory.ibftProposedBlockValidator(0); + + final boolean validationResult = + rules.validateHeader( + block.getHeader(), parentHeader, protContext, HeaderValidationMode.FULL); + + assertThat(validationResult).isTrue(); + + final BlockHeader header = block.getHeader(); + final IbftExtraData extraData = IbftExtraData.decode(header.getExtraData()); + assertThat(block.getHash()) + .isEqualTo(IbftBlockHashing.calculateDataHashForCommittedSeal(header, extraData)); + } +}