From ef50390398b37cc7b1bf1b02962392bace19653e Mon Sep 17 00:00:00 2001 From: "S. Matthew English" Date: Thu, 18 Oct 2018 22:46:44 -0400 Subject: [PATCH] [NC-1711] Ommer blocks should be considered valid even when they are from the future (#92) * eliminate black for relevant tests * partition TimestampValidationRule * adapt existing tests * further adaptions of status quo * create creator * adding ommer validator * resolving builder sequence * remove blank lines, fix comment * rename classes * remove unnecessary blank lines --- .../BlockHeaderValidationRulesetFactory.java | 6 +- .../consensus/clique/CliqueProtocolSpecs.java | 1 + ...ftBlockHeaderValidationRulesetFactory.java | 6 +- .../consensus/ibft/IbftProtocolSpecs.java | 1 + .../mainnet/MainnetBlockBodyValidator.java | 2 +- .../mainnet/MainnetBlockHeaderValidator.java | 19 +++++- .../mainnet/MainnetProtocolSpecs.java | 1 + .../ethereum/mainnet/ProtocolSpec.java | 14 +++++ .../ethereum/mainnet/ProtocolSpecBuilder.java | 13 +++++ .../TimestampBoundedByFutureParameter.java | 58 +++++++++++++++++++ ...ava => TimestampMoreRecentThanParent.java} | 35 ++--------- .../TimestampValidationRuleTest.java | 36 ++++++++---- .../vm/BlockchainReferenceTestTools.java | 4 -- .../methods/EthGetTransactionReceiptTest.java | 2 + 14 files changed, 146 insertions(+), 52 deletions(-) create mode 100644 ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/headervalidationrules/TimestampBoundedByFutureParameter.java rename ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/headervalidationrules/{TimestampValidationRule.java => TimestampMoreRecentThanParent.java} (58%) diff --git a/consensus/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/BlockHeaderValidationRulesetFactory.java b/consensus/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/BlockHeaderValidationRulesetFactory.java index 7362d21906..7c22e15fad 100644 --- a/consensus/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/BlockHeaderValidationRulesetFactory.java +++ b/consensus/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/BlockHeaderValidationRulesetFactory.java @@ -25,7 +25,8 @@ import tech.pegasys.pantheon.ethereum.mainnet.headervalidationrules.AncestryVali import tech.pegasys.pantheon.ethereum.mainnet.headervalidationrules.ConstantFieldValidationRule; import tech.pegasys.pantheon.ethereum.mainnet.headervalidationrules.GasLimitRangeAndDeltaValidationRule; import tech.pegasys.pantheon.ethereum.mainnet.headervalidationrules.GasUsageValidationRule; -import tech.pegasys.pantheon.ethereum.mainnet.headervalidationrules.TimestampValidationRule; +import tech.pegasys.pantheon.ethereum.mainnet.headervalidationrules.TimestampBoundedByFutureParameter; +import tech.pegasys.pantheon.ethereum.mainnet.headervalidationrules.TimestampMoreRecentThanParent; public class BlockHeaderValidationRulesetFactory { @@ -46,7 +47,8 @@ public class BlockHeaderValidationRulesetFactory { .addRule(new AncestryValidationRule()) .addRule(new GasUsageValidationRule()) .addRule(new GasLimitRangeAndDeltaValidationRule(5000, 0x7fffffffffffffffL)) - .addRule(new TimestampValidationRule(10, secondsBetweenBlocks)) + .addRule(new TimestampBoundedByFutureParameter(10)) + .addRule(new TimestampMoreRecentThanParent(secondsBetweenBlocks)) .addRule(new ConstantFieldValidationRule<>("MixHash", BlockHeader::getMixHash, Hash.ZERO)) .addRule( new ConstantFieldValidationRule<>( diff --git a/consensus/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/CliqueProtocolSpecs.java b/consensus/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/CliqueProtocolSpecs.java index 060f72337c..662764722c 100644 --- a/consensus/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/CliqueProtocolSpecs.java +++ b/consensus/clique/src/main/java/tech/pegasys/pantheon/consensus/clique/CliqueProtocolSpecs.java @@ -71,6 +71,7 @@ public class CliqueProtocolSpecs { final EpochManager epochManager = new EpochManager(epochLength); return specBuilder .changeConsensusContextType( + difficultyCalculator -> cliqueBlockHeaderValidator(secondsBetweenBlocks, epochManager), difficultyCalculator -> cliqueBlockHeaderValidator(secondsBetweenBlocks, epochManager), MainnetBlockBodyValidator::new, MainnetBlockImporter::new, diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftBlockHeaderValidationRulesetFactory.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftBlockHeaderValidationRulesetFactory.java index be1ff69435..69bdb79b78 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftBlockHeaderValidationRulesetFactory.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftBlockHeaderValidationRulesetFactory.java @@ -21,7 +21,8 @@ import tech.pegasys.pantheon.ethereum.mainnet.headervalidationrules.AncestryVali import tech.pegasys.pantheon.ethereum.mainnet.headervalidationrules.ConstantFieldValidationRule; import tech.pegasys.pantheon.ethereum.mainnet.headervalidationrules.GasLimitRangeAndDeltaValidationRule; import tech.pegasys.pantheon.ethereum.mainnet.headervalidationrules.GasUsageValidationRule; -import tech.pegasys.pantheon.ethereum.mainnet.headervalidationrules.TimestampValidationRule; +import tech.pegasys.pantheon.ethereum.mainnet.headervalidationrules.TimestampBoundedByFutureParameter; +import tech.pegasys.pantheon.ethereum.mainnet.headervalidationrules.TimestampMoreRecentThanParent; import tech.pegasys.pantheon.util.uint.UInt256; public class IbftBlockHeaderValidationRulesetFactory { @@ -56,7 +57,8 @@ public class IbftBlockHeaderValidationRulesetFactory { .addRule(new AncestryValidationRule()) .addRule(new GasUsageValidationRule()) .addRule(new GasLimitRangeAndDeltaValidationRule(5000, 0x7fffffffffffffffL)) - .addRule(new TimestampValidationRule(1, secondsBetweenBlocks)) + .addRule(new TimestampBoundedByFutureParameter(1)) + .addRule(new TimestampMoreRecentThanParent(secondsBetweenBlocks)) .addRule( new ConstantFieldValidationRule<>( "MixHash", BlockHeader::getMixHash, IbftHelpers.EXPECTED_MIX_HASH)) diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftProtocolSpecs.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftProtocolSpecs.java index 79c4ed0efc..1b3efcb77c 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftProtocolSpecs.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftProtocolSpecs.java @@ -44,6 +44,7 @@ public class IbftProtocolSpecs { final EpochManager epochManager = new EpochManager(epochLength); return MainnetProtocolSpecs.spuriousDragonDefinition(chainId) .changeConsensusContextType( + difficultyCalculator -> ibftBlockHeaderValidator(secondsBetweenBlocks), difficultyCalculator -> ibftBlockHeaderValidator(secondsBetweenBlocks), MainnetBlockBodyValidator::new, (blockHeaderValidator, blockBodyValidator, blockProcessor) -> diff --git a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/MainnetBlockBodyValidator.java b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/MainnetBlockBodyValidator.java index d7ce45f8df..3f675ca720 100644 --- a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/MainnetBlockBodyValidator.java +++ b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/MainnetBlockBodyValidator.java @@ -219,7 +219,7 @@ public class MainnetBlockBodyValidator implements BlockBodyValidator { final HeaderValidationMode ommerValidationMode) { final ProtocolSpec protocolSpec = protocolSchedule.getByBlockNumber(ommer.getNumber()); if (!protocolSpec - .getBlockHeaderValidator() + .getOmmerHeaderValidator() .validateHeader(ommer, context, ommerValidationMode)) { return false; } diff --git a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/MainnetBlockHeaderValidator.java b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/MainnetBlockHeaderValidator.java index a92b453917..dee87d5b29 100644 --- a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/MainnetBlockHeaderValidator.java +++ b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/MainnetBlockHeaderValidator.java @@ -20,7 +20,8 @@ import tech.pegasys.pantheon.ethereum.mainnet.headervalidationrules.ExtraDataMax import tech.pegasys.pantheon.ethereum.mainnet.headervalidationrules.GasLimitRangeAndDeltaValidationRule; import tech.pegasys.pantheon.ethereum.mainnet.headervalidationrules.GasUsageValidationRule; import tech.pegasys.pantheon.ethereum.mainnet.headervalidationrules.ProofOfWorkValidationRule; -import tech.pegasys.pantheon.ethereum.mainnet.headervalidationrules.TimestampValidationRule; +import tech.pegasys.pantheon.ethereum.mainnet.headervalidationrules.TimestampBoundedByFutureParameter; +import tech.pegasys.pantheon.ethereum.mainnet.headervalidationrules.TimestampMoreRecentThanParent; import tech.pegasys.pantheon.util.bytes.BytesValue; public final class MainnetBlockHeaderValidator { @@ -46,6 +47,19 @@ public final class MainnetBlockHeaderValidator { .build(); } + static BlockHeaderValidator createOmmerValidator( + final DifficultyCalculator difficultyCalculator) { + return new BlockHeaderValidator.Builder() + .addRule(new CalculatedDifficultyValidationRule<>(difficultyCalculator)) + .addRule(new AncestryValidationRule()) + .addRule(new GasLimitRangeAndDeltaValidationRule(MIN_GAS_LIMIT, MAX_GAS_LIMIT)) + .addRule(new GasUsageValidationRule()) + .addRule(new TimestampMoreRecentThanParent(MINIMUM_SECONDS_SINCE_PARENT)) + .addRule(new ExtraDataMaxLengthValidationRule(BlockHeader.MAX_EXTRA_DATA_BYTES)) + .addRule(new ProofOfWorkValidationRule()) + .build(); + } + private static BlockHeaderValidator.Builder createValidator( final DifficultyCalculator difficultyCalculator) { return new BlockHeaderValidator.Builder() @@ -53,7 +67,8 @@ public final class MainnetBlockHeaderValidator { .addRule(new AncestryValidationRule()) .addRule(new GasLimitRangeAndDeltaValidationRule(MIN_GAS_LIMIT, MAX_GAS_LIMIT)) .addRule(new GasUsageValidationRule()) - .addRule(new TimestampValidationRule(TIMESTAMP_TOLERANCE_S, MINIMUM_SECONDS_SINCE_PARENT)) + .addRule(new TimestampMoreRecentThanParent(MINIMUM_SECONDS_SINCE_PARENT)) + .addRule(new TimestampBoundedByFutureParameter(TIMESTAMP_TOLERANCE_S)) .addRule(new ExtraDataMaxLengthValidationRule(BlockHeader.MAX_EXTRA_DATA_BYTES)) .addRule(new ProofOfWorkValidationRule()); } diff --git a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/MainnetProtocolSpecs.java b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/MainnetProtocolSpecs.java index afc0c24548..23feee2bab 100644 --- a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/MainnetProtocolSpecs.java +++ b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/MainnetProtocolSpecs.java @@ -82,6 +82,7 @@ public abstract class MainnetProtocolSpecs { false)) .difficultyCalculator(MainnetDifficultyCalculators.FRONTIER) .blockHeaderValidatorBuilder(MainnetBlockHeaderValidator::create) + .ommerHeaderValidatorBuilder(MainnetBlockHeaderValidator::createOmmerValidator) .blockBodyValidatorBuilder(MainnetBlockBodyValidator::new) .transactionReceiptFactory(MainnetProtocolSpecs::frontierTransactionReceiptFactory) .blockReward(FRONTIER_BLOCK_REWARD) diff --git a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/ProtocolSpec.java b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/ProtocolSpec.java index f9ac1d9735..49089f911e 100644 --- a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/ProtocolSpec.java +++ b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/ProtocolSpec.java @@ -30,6 +30,8 @@ public class ProtocolSpec { private final BlockHeaderValidator blockHeaderValidator; + private final BlockHeaderValidator ommerHeaderValidator; + private final BlockBodyValidator blockBodyValidator; private final BlockImporter blockImporter; @@ -54,6 +56,7 @@ public class ProtocolSpec { * @param transactionValidator the transaction validator to use * @param transactionProcessor the transaction processor to use * @param blockHeaderValidator the block header validator to use + * @param ommerHeaderValidator the rules used to validate an ommer * @param blockBodyValidator the block body validator to use * @param blockProcessor the block processor to use * @param blockImporter the block importer to use @@ -70,6 +73,7 @@ public class ProtocolSpec { final TransactionValidator transactionValidator, final TransactionProcessor transactionProcessor, final BlockHeaderValidator blockHeaderValidator, + final BlockHeaderValidator ommerHeaderValidator, final BlockBodyValidator blockBodyValidator, final BlockProcessor blockProcessor, final BlockImporter blockImporter, @@ -84,6 +88,7 @@ public class ProtocolSpec { this.transactionValidator = transactionValidator; this.transactionProcessor = transactionProcessor; this.blockHeaderValidator = blockHeaderValidator; + this.ommerHeaderValidator = ommerHeaderValidator; this.blockBodyValidator = blockBodyValidator; this.blockProcessor = blockProcessor; this.blockImporter = blockImporter; @@ -148,6 +153,15 @@ public class ProtocolSpec { return blockHeaderValidator; } + /** + * Returns the block ommer header validator used in this specification. + * + * @return the block ommer header validator + */ + public BlockHeaderValidator getOmmerHeaderValidator() { + return ommerHeaderValidator; + } + /** * Returns the block body validator used in this specification. * diff --git a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/ProtocolSpecBuilder.java b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/ProtocolSpecBuilder.java index ac7223aa63..33616b56aa 100644 --- a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/ProtocolSpecBuilder.java +++ b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/ProtocolSpecBuilder.java @@ -34,6 +34,7 @@ public class ProtocolSpecBuilder { private Function evmBuilder; private Function transactionValidatorBuilder; private Function, BlockHeaderValidator> blockHeaderValidatorBuilder; + private Function, BlockHeaderValidator> ommerHeaderValidatorBuilder; private Function, BlockBodyValidator> blockBodyValidatorBuilder; private BiFunction contractCreationProcessorBuilder; private Function precompileContractRegistryBuilder; @@ -91,6 +92,13 @@ public class ProtocolSpecBuilder { return this; } + public ProtocolSpecBuilder ommerHeaderValidatorBuilder( + final Function, BlockHeaderValidator> + ommerHeaderValidatorBuilder) { + this.ommerHeaderValidatorBuilder = ommerHeaderValidatorBuilder; + return this; + } + public ProtocolSpecBuilder blockBodyValidatorBuilder( final Function, BlockBodyValidator> blockBodyValidatorBuilder) { this.blockBodyValidatorBuilder = blockBodyValidatorBuilder; @@ -154,6 +162,7 @@ public class ProtocolSpecBuilder { public ProtocolSpecBuilder changeConsensusContextType( final Function, BlockHeaderValidator> blockHeaderValidatorBuilder, + final Function, BlockHeaderValidator> ommerHeaderValidatorBuilder, final Function, BlockBodyValidator> blockBodyValidatorBuilder, final BlockImporterBuilder blockImporterBuilder, final DifficultyCalculator difficultyCalculator) { @@ -166,6 +175,7 @@ public class ProtocolSpecBuilder { .messageCallProcessorBuilder(messageCallProcessorBuilder) .transactionProcessorBuilder(transactionProcessorBuilder) .blockHeaderValidatorBuilder(blockHeaderValidatorBuilder) + .ommerHeaderValidatorBuilder(ommerHeaderValidatorBuilder) .blockBodyValidatorBuilder(blockBodyValidatorBuilder) .blockProcessorBuilder(blockProcessorBuilder) .blockImporterBuilder(blockImporterBuilder) @@ -214,6 +224,8 @@ public class ProtocolSpecBuilder { gasCalculator, transactionValidator, contractCreationProcessor, messageCallProcessor); final BlockHeaderValidator blockHeaderValidator = blockHeaderValidatorBuilder.apply(difficultyCalculator); + final BlockHeaderValidator ommerHeaderValidator = + ommerHeaderValidatorBuilder.apply(difficultyCalculator); final BlockBodyValidator blockBodyValidator = blockBodyValidatorBuilder.apply(protocolSchedule); final BlockProcessor blockProcessor = @@ -230,6 +242,7 @@ public class ProtocolSpecBuilder { transactionValidator, transactionProcessor, blockHeaderValidator, + ommerHeaderValidator, blockBodyValidator, blockProcessor, blockImporter, diff --git a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/headervalidationrules/TimestampBoundedByFutureParameter.java b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/headervalidationrules/TimestampBoundedByFutureParameter.java new file mode 100644 index 0000000000..2d7a77e133 --- /dev/null +++ b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/headervalidationrules/TimestampBoundedByFutureParameter.java @@ -0,0 +1,58 @@ +/* + * 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.ethereum.mainnet.headervalidationrules; + +import tech.pegasys.pantheon.ethereum.core.BlockHeader; +import tech.pegasys.pantheon.ethereum.mainnet.DetachedBlockHeaderValidationRule; + +import java.util.concurrent.TimeUnit; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + +/** + * Responsible for ensuring the timestamp of a block is not more than "acceptableClockDriftSeconds' + * into the future. + */ +public class TimestampBoundedByFutureParameter implements DetachedBlockHeaderValidationRule { + + private final Logger LOG = LogManager.getLogger(); + private final long acceptableClockDriftSeconds; + + public TimestampBoundedByFutureParameter(final long acceptableClockDriftSeconds) { + this.acceptableClockDriftSeconds = acceptableClockDriftSeconds; + } + + @Override + public boolean validate(final BlockHeader header, final BlockHeader parent) { + return validateTimestamp(header.getTimestamp()); + } + + private boolean validateTimestamp(final long timestamp) { + return validateHeaderNotAheadOfCurrentSystemTime(timestamp); + } + + private boolean validateHeaderNotAheadOfCurrentSystemTime(final long timestamp) { + final long timestampMargin = + TimeUnit.SECONDS.convert(System.currentTimeMillis(), TimeUnit.MILLISECONDS) + + acceptableClockDriftSeconds; + if (Long.compareUnsigned(timestamp, timestampMargin) > 0) { + LOG.trace( + "Invalid block header: timestamp {} is greater than the timestamp margin {}", + timestamp, + timestampMargin); + return false; + } + return true; + } +} diff --git a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/headervalidationrules/TimestampValidationRule.java b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/headervalidationrules/TimestampMoreRecentThanParent.java similarity index 58% rename from ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/headervalidationrules/TimestampValidationRule.java rename to ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/headervalidationrules/TimestampMoreRecentThanParent.java index 953774bd3f..18cda7fc97 100644 --- a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/headervalidationrules/TimestampValidationRule.java +++ b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/headervalidationrules/TimestampMoreRecentThanParent.java @@ -17,25 +17,17 @@ import static com.google.common.base.Preconditions.checkArgument; import tech.pegasys.pantheon.ethereum.core.BlockHeader; import tech.pegasys.pantheon.ethereum.mainnet.DetachedBlockHeaderValidationRule; -import java.util.concurrent.TimeUnit; - import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -/** - * Responsible for ensuring the timestamp of a block is newer than its parent, but also that it has - * a timestamp not more than "acceptableClockDriftSeconds' into the future. - */ -public class TimestampValidationRule implements DetachedBlockHeaderValidationRule { +/** Responsible for ensuring the timestamp of a block is newer than its parent. */ +public class TimestampMoreRecentThanParent implements DetachedBlockHeaderValidationRule { - private final Logger LOG = LogManager.getLogger(TimestampValidationRule.class); - private final long acceptableClockDriftSeconds; + private final Logger LOG = LogManager.getLogger(); private final long minimumSecondsSinceParent; - public TimestampValidationRule( - final long acceptableClockDriftSeconds, final long minimumSecondsSinceParent) { + public TimestampMoreRecentThanParent(final long minimumSecondsSinceParent) { checkArgument(minimumSecondsSinceParent >= 0, "minimumSecondsSinceParent must be positive"); - this.acceptableClockDriftSeconds = acceptableClockDriftSeconds; this.minimumSecondsSinceParent = minimumSecondsSinceParent; } @@ -45,10 +37,7 @@ public class TimestampValidationRule implements DetachedBlockHeaderValidationRul } private boolean validateTimestamp(final long timestamp, final long parentTimestamp) { - boolean result = validateHeaderSufficientlyAheadOfParent(timestamp, parentTimestamp); - result &= validateHeaderNotAheadOfCurrentSystemTime(timestamp); - - return result; + return validateHeaderSufficientlyAheadOfParent(timestamp, parentTimestamp); } private boolean validateHeaderSufficientlyAheadOfParent( @@ -63,18 +52,4 @@ public class TimestampValidationRule implements DetachedBlockHeaderValidationRul return true; } - - private boolean validateHeaderNotAheadOfCurrentSystemTime(final long timestamp) { - final long timestampMargin = - TimeUnit.SECONDS.convert(System.currentTimeMillis(), TimeUnit.MILLISECONDS) - + acceptableClockDriftSeconds; - if (Long.compareUnsigned(timestamp, timestampMargin) > 0) { - LOG.trace( - "Invalid block header: timestamp {} is greater than the timestamp margin {}", - timestamp, - timestampMargin); - return false; - } - return true; - } } diff --git a/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/mainnet/headervalidationrules/TimestampValidationRuleTest.java b/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/mainnet/headervalidationrules/TimestampValidationRuleTest.java index 75cbfdaad8..b6aac2e3ab 100644 --- a/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/mainnet/headervalidationrules/TimestampValidationRuleTest.java +++ b/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/mainnet/headervalidationrules/TimestampValidationRuleTest.java @@ -26,7 +26,8 @@ public class TimestampValidationRuleTest { @Test public void headerTimestampSufficientlyFarIntoFutureVadidatesSuccessfully() { - final TimestampValidationRule uut = new TimestampValidationRule(0, 10); + final TimestampBoundedByFutureParameter uut00 = new TimestampBoundedByFutureParameter(0); + final TimestampMoreRecentThanParent uut01 = new TimestampMoreRecentThanParent(10); final BlockHeaderTestFixture headerBuilder = new BlockHeaderTestFixture(); @@ -37,18 +38,20 @@ public class TimestampValidationRuleTest { headerBuilder.timestamp(parent.getTimestamp() + 11); final BlockHeader header = headerBuilder.buildHeader(); - assertThat(uut.validate(header, parent)).isTrue(); + assertThat(uut00.validate(header, parent)).isTrue(); + assertThat(uut01.validate(header, parent)).isTrue(); } @Test public void headerTimestampDifferenceMustBePositive() { - Assertions.assertThatThrownBy(() -> new TimestampValidationRule(0, -1)) + Assertions.assertThatThrownBy(() -> new TimestampMoreRecentThanParent(-1)) .hasMessage("minimumSecondsSinceParent must be positive"); } @Test public void headerTimestampTooCloseToParentFailsValidation() { - final TimestampValidationRule uut = new TimestampValidationRule(0, 10); + final TimestampBoundedByFutureParameter uut00 = new TimestampBoundedByFutureParameter(0); + final TimestampMoreRecentThanParent uut01 = new TimestampMoreRecentThanParent(10); final BlockHeaderTestFixture headerBuilder = new BlockHeaderTestFixture(); @@ -59,12 +62,14 @@ public class TimestampValidationRuleTest { headerBuilder.timestamp(parent.getTimestamp() + 1); final BlockHeader header = headerBuilder.buildHeader(); - assertThat(uut.validate(header, parent)).isFalse(); + assertThat(uut00.validate(header, parent)).isTrue(); + assertThat(uut01.validate(header, parent)).isFalse(); } @Test public void headerTimestampIsBehindParentFailsValidation() { - final TimestampValidationRule uut = new TimestampValidationRule(0, 10); + final TimestampBoundedByFutureParameter uut00 = new TimestampBoundedByFutureParameter(0); + final TimestampMoreRecentThanParent uut01 = new TimestampMoreRecentThanParent(10); final BlockHeaderTestFixture headerBuilder = new BlockHeaderTestFixture(); @@ -75,13 +80,17 @@ public class TimestampValidationRuleTest { headerBuilder.timestamp(parent.getTimestamp() - 11); final BlockHeader header = headerBuilder.buildHeader(); - assertThat(uut.validate(header, parent)).isFalse(); + assertThat(uut00.validate(header, parent)).isTrue(); + assertThat(uut01.validate(header, parent)).isFalse(); } @Test public void headerNewerThanCurrentSystemFailsValidation() { final long acceptableClockDrift = 5; - final TimestampValidationRule uut = new TimestampValidationRule(acceptableClockDrift, 10); + + final TimestampBoundedByFutureParameter uut00 = + new TimestampBoundedByFutureParameter(acceptableClockDrift); + final TimestampMoreRecentThanParent uut01 = new TimestampMoreRecentThanParent(10); final BlockHeaderTestFixture headerBuilder = new BlockHeaderTestFixture(); @@ -94,13 +103,17 @@ public class TimestampValidationRuleTest { headerBuilder.timestamp(parent.getTimestamp() + acceptableClockDrift + 1); final BlockHeader header = headerBuilder.buildHeader(); - assertThat(uut.validate(header, parent)).isFalse(); + assertThat(uut00.validate(header, parent)).isFalse(); + assertThat(uut01.validate(header, parent)).isFalse(); } @Test public void futureHeadersAreValidIfTimestampWithinTolerance() { final long acceptableClockDrift = 5; - final TimestampValidationRule uut = new TimestampValidationRule(acceptableClockDrift, 10); + + final TimestampBoundedByFutureParameter uut00 = + new TimestampBoundedByFutureParameter(acceptableClockDrift); + final TimestampMoreRecentThanParent uut01 = new TimestampMoreRecentThanParent(10); final BlockHeaderTestFixture headerBuilder = new BlockHeaderTestFixture(); @@ -114,6 +127,7 @@ public class TimestampValidationRuleTest { headerBuilder.timestamp(parent.getTimestamp() + acceptableClockDrift - 1); final BlockHeader header = headerBuilder.buildHeader(); - assertThat(uut.validate(header, parent)).isFalse(); + assertThat(uut00.validate(header, parent)).isTrue(); + assertThat(uut01.validate(header, parent)).isFalse(); } } diff --git a/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/vm/BlockchainReferenceTestTools.java b/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/vm/BlockchainReferenceTestTools.java index 22fbc3ce8b..ad62e4d571 100644 --- a/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/vm/BlockchainReferenceTestTools.java +++ b/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/vm/BlockchainReferenceTestTools.java @@ -66,10 +66,6 @@ public class BlockchainReferenceTestTools { // Consumes a huge amount of memory params.blacklist("static_Call1MB1024Calldepth_d1g0v0_(Byzantium|Constantinople)"); - // Pantheon is incorrectly rejecting Uncle block timestamps in the future - params.blacklist("futureUncleTimestampDifficultyDrop2"); - params.blacklist("futureUncleTimestampDifficultyDrop"); - // Needs investigation params.blacklist("RevertInCreateInInit_d0g0v0_Byzantium"); params.blacklist("RevertInCreateInInit_d0g0v0_Constantinople"); diff --git a/ethereum/jsonrpc/src/test/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/EthGetTransactionReceiptTest.java b/ethereum/jsonrpc/src/test/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/EthGetTransactionReceiptTest.java index 4d400ff642..ae349b3ae9 100644 --- a/ethereum/jsonrpc/src/test/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/EthGetTransactionReceiptTest.java +++ b/ethereum/jsonrpc/src/test/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/EthGetTransactionReceiptTest.java @@ -88,6 +88,7 @@ public class EthGetTransactionReceiptTest { null, null, null, + null, TransactionReceiptType.ROOT, BlockHeader::getCoinbase); private final ProtocolSpec statusTransactionTypeSpec = @@ -104,6 +105,7 @@ public class EthGetTransactionReceiptTest { null, null, null, + null, TransactionReceiptType.STATUS, BlockHeader::getCoinbase);