From 4743a00af7f27679af8ed7fb3e7521fb08293ff8 Mon Sep 17 00:00:00 2001 From: shemnon Date: Fri, 18 Jan 2019 13:05:36 -0700 Subject: [PATCH 1/6] NC-2147 Implement Petersburg hardfork * create a new fork definition * make it constantinople without EIP-1283 * update tests where appropriate --- .../pegasys/pantheon/config/GenesisConfigOptions.java | 2 ++ .../pantheon/config/JsonGenesisConfigOptions.java | 5 +++++ config/src/main/resources/dev.json | 1 + config/src/main/resources/goerli.json | 1 + config/src/main/resources/mainnet.json | 2 ++ .../pantheon/config/StubGenesisConfigOptions.java | 11 +++++++++++ .../pantheon/config/GenesisConfigOptionsTest.java | 7 +++++++ .../vm/operations/BlockHashOperationBenchmark.java | 1 + .../ethereum/mainnet/MainnetProtocolSpecs.java | 6 ++++++ .../ethereum/mainnet/ProtocolScheduleBuilder.java | 4 ++++ .../ethereum/core/ExecutionContextTestFixture.java | 2 +- .../pantheon/ethereum/core/TransactionTest.java | 5 +++++ .../ethereum/core/TransactionTestCaseSpec.java | 2 ++ .../ethereum/mainnet/MainnetProtocolScheduleTest.java | 6 ++++-- 14 files changed, 52 insertions(+), 3 deletions(-) diff --git a/config/src/main/java/tech/pegasys/pantheon/config/GenesisConfigOptions.java b/config/src/main/java/tech/pegasys/pantheon/config/GenesisConfigOptions.java index 8141836faa..a2a7806605 100644 --- a/config/src/main/java/tech/pegasys/pantheon/config/GenesisConfigOptions.java +++ b/config/src/main/java/tech/pegasys/pantheon/config/GenesisConfigOptions.java @@ -43,5 +43,7 @@ public interface GenesisConfigOptions { OptionalLong getConstantinopleBlockNumber(); + OptionalLong getPetersburgBlockNumber(); + OptionalInt getChainId(); } diff --git a/config/src/main/java/tech/pegasys/pantheon/config/JsonGenesisConfigOptions.java b/config/src/main/java/tech/pegasys/pantheon/config/JsonGenesisConfigOptions.java index 8bc1cb4866..5c279ae0a9 100644 --- a/config/src/main/java/tech/pegasys/pantheon/config/JsonGenesisConfigOptions.java +++ b/config/src/main/java/tech/pegasys/pantheon/config/JsonGenesisConfigOptions.java @@ -100,6 +100,11 @@ public OptionalLong getConstantinopleBlockNumber() { return getOptionalLong("constantinopleblock"); } + @Override + public OptionalLong getPetersburgBlockNumber() { + return getOptionalLong("petersburgblock"); + } + @Override public OptionalInt getChainId() { return configRoot.containsKey("chainid") diff --git a/config/src/main/resources/dev.json b/config/src/main/resources/dev.json index 0239cff435..815d282f89 100644 --- a/config/src/main/resources/dev.json +++ b/config/src/main/resources/dev.json @@ -8,6 +8,7 @@ "eip158Block": 0, "byzantiumBlock": 0, "constantinopleBlock": 0, + "petersburgBlock": 0, "ethash": { } }, diff --git a/config/src/main/resources/goerli.json b/config/src/main/resources/goerli.json index 711883da4f..1bdb624173 100644 --- a/config/src/main/resources/goerli.json +++ b/config/src/main/resources/goerli.json @@ -9,6 +9,7 @@ "eip160Block":0, "byzantiumBlock":0, "constantinopleBlock":0, + "petersburgBlock": 0, "clique":{ "period":15, "epoch":30000 diff --git a/config/src/main/resources/mainnet.json b/config/src/main/resources/mainnet.json index b853e7bcf6..19bffe30d1 100644 --- a/config/src/main/resources/mainnet.json +++ b/config/src/main/resources/mainnet.json @@ -9,6 +9,8 @@ "eip155Block": 2675000, "eip158Block": 2675000, "byzantiumBlock": 4370000, + "constantinopleBlock": 7280000, + "petersburgBlock": 7280000, "ethash": { } diff --git a/config/src/test-support/java/tech/pegasys/pantheon/config/StubGenesisConfigOptions.java b/config/src/test-support/java/tech/pegasys/pantheon/config/StubGenesisConfigOptions.java index 3e7788efce..495164fe3d 100644 --- a/config/src/test-support/java/tech/pegasys/pantheon/config/StubGenesisConfigOptions.java +++ b/config/src/test-support/java/tech/pegasys/pantheon/config/StubGenesisConfigOptions.java @@ -23,6 +23,7 @@ public class StubGenesisConfigOptions implements GenesisConfigOptions { private OptionalLong spuriousDragonBlockNumber = OptionalLong.empty(); private OptionalLong byzantiumBlockNumber = OptionalLong.empty(); private OptionalLong constantinopleBlockNumber = OptionalLong.empty(); + private OptionalLong petersburgBlockNumber = OptionalLong.empty(); private OptionalInt chainId = OptionalInt.empty(); @Override @@ -90,6 +91,11 @@ public OptionalLong getConstantinopleBlockNumber() { return constantinopleBlockNumber; } + @Override + public OptionalLong getPetersburgBlockNumber() { + return petersburgBlockNumber; + } + @Override public OptionalInt getChainId() { return chainId; @@ -125,6 +131,11 @@ public StubGenesisConfigOptions constantinopleBlock(final long blockNumber) { return this; } + public StubGenesisConfigOptions petersburgBlock(final long blockNumber) { + petersburgBlockNumber = OptionalLong.of(blockNumber); + return this; + } + public StubGenesisConfigOptions chainId(final int chainId) { this.chainId = OptionalInt.of(chainId); return this; diff --git a/config/src/test/java/tech/pegasys/pantheon/config/GenesisConfigOptionsTest.java b/config/src/test/java/tech/pegasys/pantheon/config/GenesisConfigOptionsTest.java index c4a385b934..c39a64d8de 100644 --- a/config/src/test/java/tech/pegasys/pantheon/config/GenesisConfigOptionsTest.java +++ b/config/src/test/java/tech/pegasys/pantheon/config/GenesisConfigOptionsTest.java @@ -101,6 +101,12 @@ public void shouldGetConstantinopleBlockNumber() { assertThat(config.getConstantinopleBlockNumber()).hasValue(1000); } + @Test + public void shouldGetPetersburgBlockNumber() { + final GenesisConfigOptions config = fromConfigOptions(singletonMap("petersburgBlock", 1000)); + assertThat(config.getPetersburgBlockNumber()).hasValue(1000); + } + @Test public void shouldNotReturnEmptyOptionalWhenBlockNumberNotSpecified() { final GenesisConfigOptions config = fromConfigOptions(emptyMap()); @@ -110,6 +116,7 @@ public void shouldNotReturnEmptyOptionalWhenBlockNumberNotSpecified() { assertThat(config.getSpuriousDragonBlockNumber()).isEmpty(); assertThat(config.getByzantiumBlockNumber()).isEmpty(); assertThat(config.getConstantinopleBlockNumber()).isEmpty(); + assertThat(config.getPetersburgBlockNumber()).isEmpty(); } @Test diff --git a/ethereum/core/src/jmh/java/tech/pegasys/pantheon/ethereum/vm/operations/BlockHashOperationBenchmark.java b/ethereum/core/src/jmh/java/tech/pegasys/pantheon/ethereum/vm/operations/BlockHashOperationBenchmark.java index 7250837953..89592a8259 100644 --- a/ethereum/core/src/jmh/java/tech/pegasys/pantheon/ethereum/vm/operations/BlockHashOperationBenchmark.java +++ b/ethereum/core/src/jmh/java/tech/pegasys/pantheon/ethereum/vm/operations/BlockHashOperationBenchmark.java @@ -42,6 +42,7 @@ public class BlockHashOperationBenchmark { @Setup public void prepare() throws Exception { operationBenchmarkHelper = OperationBenchmarkHelper.create(); + // ?? do we create another benchmark ?? operation = new BlockHashOperation(new ConstantinopleGasCalculator()); frame = operationBenchmarkHelper.createMessageFrame(); } 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 6941e02ce6..1f652fec99 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 @@ -193,6 +193,12 @@ public static ProtocolSpecBuilder constantinopleDefinition(final int chain .name("Constantinople"); } + public static ProtocolSpecBuilder petersburgDefinition(final int chainId) { + return constantinopleDefinition(chainId) + .gasCalculator(TangerineWhistleGasCalculator::new) + .name("Petersburg"); + } + private static TransactionReceipt frontierTransactionReceiptFactory( final TransactionProcessor.Result result, final WorldState worldState, final long gasUsed) { return new TransactionReceipt(worldState.rootHash(), gasUsed, result.getLogs()); diff --git a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/ProtocolScheduleBuilder.java b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/ProtocolScheduleBuilder.java index 33d5f6e9b5..52ebf16574 100644 --- a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/ProtocolScheduleBuilder.java +++ b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/ProtocolScheduleBuilder.java @@ -80,6 +80,10 @@ public ProtocolSchedule createProtocolSchedule() { protocolSchedule, config.getConstantinopleBlockNumber(), MainnetProtocolSpecs.constantinopleDefinition(chainId)); + addProtocolSpec( + protocolSchedule, + config.getPetersburgBlockNumber(), + MainnetProtocolSpecs.petersburgDefinition(chainId)); return protocolSchedule; } diff --git a/ethereum/core/src/test-support/java/tech/pegasys/pantheon/ethereum/core/ExecutionContextTestFixture.java b/ethereum/core/src/test-support/java/tech/pegasys/pantheon/ethereum/core/ExecutionContextTestFixture.java index 3d9bfa96df..fce0224a03 100644 --- a/ethereum/core/src/test-support/java/tech/pegasys/pantheon/ethereum/core/ExecutionContextTestFixture.java +++ b/ethereum/core/src/test-support/java/tech/pegasys/pantheon/ethereum/core/ExecutionContextTestFixture.java @@ -113,7 +113,7 @@ public ExecutionContextTestFixture build() { if (protocolSchedule == null) { protocolSchedule = new ProtocolScheduleBuilder<>( - new StubGenesisConfigOptions().constantinopleBlock(0), 42, Function.identity()) + new StubGenesisConfigOptions().petersburgBlock(0), 42, Function.identity()) .createProtocolSchedule(); } if (keyValueStorage == null) { diff --git a/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/core/TransactionTest.java b/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/core/TransactionTest.java index 7349e2f65e..2408161f92 100644 --- a/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/core/TransactionTest.java +++ b/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/core/TransactionTest.java @@ -93,6 +93,11 @@ public void constantinople() { milestone("Constantinople"); } + @Test + public void petersburg() { + milestone("Petersburg"); + } + public void milestone(final String milestone) { final TransactionTestCaseSpec.Expectation expected = spec.expectation(milestone); diff --git a/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/core/TransactionTestCaseSpec.java b/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/core/TransactionTestCaseSpec.java index 362bd4f4ef..b3a8c49daf 100644 --- a/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/core/TransactionTestCaseSpec.java +++ b/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/core/TransactionTestCaseSpec.java @@ -69,6 +69,7 @@ public TransactionTestCaseSpec( @JsonProperty("EIP158") final Expectation EIP158Expectation, @JsonProperty("Byzantium") final Expectation byzantiumExpectation, @JsonProperty("Constantinople") final Expectation constantinopleExpectation, + @JsonProperty("Petersburg") final Expectation petersburgExpectation, @JsonProperty("rlp") final String rlp) { expectations = new HashMap<>(); expectations.put("Frontier", frontierExpectation); @@ -77,6 +78,7 @@ public TransactionTestCaseSpec( expectations.put("EIP158", EIP158Expectation); expectations.put("Byzantium", byzantiumExpectation); expectations.put("Constantinople", constantinopleExpectation); + expectations.put("Petersburg", petersburgExpectation); BytesValue parsedRlp = null; try { diff --git a/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/mainnet/MainnetProtocolScheduleTest.java b/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/mainnet/MainnetProtocolScheduleTest.java index da866a8405..fb3c67567a 100644 --- a/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/mainnet/MainnetProtocolScheduleTest.java +++ b/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/mainnet/MainnetProtocolScheduleTest.java @@ -39,7 +39,8 @@ public void shouldReturnDefaultProtocolSpecsWhenCustomNumbersAreNotUsed() { Assertions.assertThat(sched.getByBlockNumber(4_730_000L).getName()).isEqualTo("Byzantium"); // Constantinople was originally scheduled for 7_080_000, but postponed Assertions.assertThat(sched.getByBlockNumber(7_080_000L).getName()).isEqualTo("Byzantium"); - Assertions.assertThat(sched.getByBlockNumber(Long.MAX_VALUE).getName()).isEqualTo("Byzantium"); + Assertions.assertThat(sched.getByBlockNumber(7_280_000L).getName()).isEqualTo("Petersburg"); + Assertions.assertThat(sched.getByBlockNumber(Long.MAX_VALUE).getName()).isEqualTo("Petersburg"); } @Test @@ -55,7 +56,7 @@ public void shouldOnlyUseFrontierWhenEmptyJsonConfigIsUsed() { public void createFromConfigWithSettings() { final JsonObject json = new JsonObject( - "{\"config\": {\"homesteadBlock\": 2, \"daoForkBlock\": 3, \"eip150Block\": 14, \"eip158Block\": 15, \"byzantiumBlock\": 16, \"constantinopleBlock\": 18, \"chainId\":1234}}"); + "{\"config\": {\"homesteadBlock\": 2, \"daoForkBlock\": 3, \"eip150Block\": 14, \"eip158Block\": 15, \"byzantiumBlock\": 16, \"constantinopleBlock\": 18, \"petersburgBlock\": 19, \"chainId\":1234}}"); final ProtocolSchedule sched = MainnetProtocolSchedule.fromConfig(GenesisConfigFile.fromConfig(json).getConfigOptions()); Assertions.assertThat(sched.getByBlockNumber(1).getName()).isEqualTo("Frontier"); @@ -67,6 +68,7 @@ public void createFromConfigWithSettings() { Assertions.assertThat(sched.getByBlockNumber(15).getName()).isEqualTo("SpuriousDragon"); Assertions.assertThat(sched.getByBlockNumber(16).getName()).isEqualTo("Byzantium"); Assertions.assertThat(sched.getByBlockNumber(18).getName()).isEqualTo("Constantinople"); + Assertions.assertThat(sched.getByBlockNumber(19).getName()).isEqualTo("Petersburg"); } @Test From ffe0925c4b95b9ae38c58b2845d3a6532b2413d4 Mon Sep 17 00:00:00 2001 From: shemnon Date: Fri, 18 Jan 2019 16:11:56 -0700 Subject: [PATCH 2/6] Petersburg Hard Fork * Add PetersbergGasCalculator --- .../BlockHashOperationBenchmark.java | 5 +- .../mainnet/MainnetProtocolSpecs.java | 2 +- .../mainnet/PetersburgGasCalculator.java | 56 +++++++++ .../ethereum/core/TransactionTest.java | 5 - .../mainnet/PetersburgSstoreGasTest.java | 110 ++++++++++++++++++ 5 files changed, 169 insertions(+), 9 deletions(-) create mode 100644 ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/PetersburgGasCalculator.java create mode 100644 ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/mainnet/PetersburgSstoreGasTest.java diff --git a/ethereum/core/src/jmh/java/tech/pegasys/pantheon/ethereum/vm/operations/BlockHashOperationBenchmark.java b/ethereum/core/src/jmh/java/tech/pegasys/pantheon/ethereum/vm/operations/BlockHashOperationBenchmark.java index 89592a8259..b169c61c2f 100644 --- a/ethereum/core/src/jmh/java/tech/pegasys/pantheon/ethereum/vm/operations/BlockHashOperationBenchmark.java +++ b/ethereum/core/src/jmh/java/tech/pegasys/pantheon/ethereum/vm/operations/BlockHashOperationBenchmark.java @@ -12,7 +12,7 @@ */ package tech.pegasys.pantheon.ethereum.vm.operations; -import tech.pegasys.pantheon.ethereum.mainnet.ConstantinopleGasCalculator; +import tech.pegasys.pantheon.ethereum.mainnet.PetersburgGasCalculator; import tech.pegasys.pantheon.ethereum.vm.BlockHashLookup; import tech.pegasys.pantheon.ethereum.vm.MessageFrame; import tech.pegasys.pantheon.util.bytes.Bytes32; @@ -42,8 +42,7 @@ public class BlockHashOperationBenchmark { @Setup public void prepare() throws Exception { operationBenchmarkHelper = OperationBenchmarkHelper.create(); - // ?? do we create another benchmark ?? - operation = new BlockHashOperation(new ConstantinopleGasCalculator()); + operation = new BlockHashOperation(new PetersburgGasCalculator()); frame = operationBenchmarkHelper.createMessageFrame(); } 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 1f652fec99..ed2b0f4b40 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 @@ -195,7 +195,7 @@ public static ProtocolSpecBuilder constantinopleDefinition(final int chain public static ProtocolSpecBuilder petersburgDefinition(final int chainId) { return constantinopleDefinition(chainId) - .gasCalculator(TangerineWhistleGasCalculator::new) + .gasCalculator(PetersburgGasCalculator::new) .name("Petersburg"); } diff --git a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/PetersburgGasCalculator.java b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/PetersburgGasCalculator.java new file mode 100644 index 0000000000..f6f03fc62d --- /dev/null +++ b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/PetersburgGasCalculator.java @@ -0,0 +1,56 @@ +/* + * 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; + +import tech.pegasys.pantheon.ethereum.core.Account; +import tech.pegasys.pantheon.ethereum.core.Gas; +import tech.pegasys.pantheon.util.uint.UInt256; + +/** + * Gas Calculator for Petersberg Hard Fork. Rollback EIP-1283. + * + *

Neither {@link TangerineWhistleGasCalculator} nor {@link SpuriousDragonGasCalculator} overrode + * these two methods so {@link FrontierGasCalculator} is the source. + */ +public class PetersburgGasCalculator extends ConstantinopleGasCalculator { + + /** Same as {#link {@link FrontierGasCalculator#STORAGE_SET_GAS_COST} */ + private static final Gas STORAGE_SET_GAS_COST = Gas.of(20_000L); + /** Same as {#link {@link FrontierGasCalculator#STORAGE_RESET_GAS_COST} */ + private static final Gas STORAGE_RESET_GAS_COST = Gas.of(5_000L); + /** Same as {#link {@link FrontierGasCalculator#STORAGE_RESET_REFUND_AMOUNT} */ + private static final Gas STORAGE_RESET_REFUND_AMOUNT = Gas.of(15_000L); + + /** + * Same as {#link {@link FrontierGasCalculator#calculateStorageCost(Account, UInt256, UInt256)} + */ + @Override + public Gas calculateStorageCost( + final Account account, final UInt256 key, final UInt256 newValue) { + return !newValue.isZero() && account.getStorageValue(key).isZero() + ? STORAGE_SET_GAS_COST + : STORAGE_RESET_GAS_COST; + } + + /** + * Same as {#link {@link FrontierGasCalculator#calculateStorageRefundAmount(Account, UInt256, + * UInt256)} + */ + @Override + public Gas calculateStorageRefundAmount( + final Account account, final UInt256 key, final UInt256 newValue) { + return newValue.isZero() && !account.getStorageValue(key).isZero() + ? STORAGE_RESET_REFUND_AMOUNT + : Gas.ZERO; + } +} diff --git a/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/core/TransactionTest.java b/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/core/TransactionTest.java index 2408161f92..7349e2f65e 100644 --- a/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/core/TransactionTest.java +++ b/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/core/TransactionTest.java @@ -93,11 +93,6 @@ public void constantinople() { milestone("Constantinople"); } - @Test - public void petersburg() { - milestone("Petersburg"); - } - public void milestone(final String milestone) { final TransactionTestCaseSpec.Expectation expected = spec.expectation(milestone); diff --git a/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/mainnet/PetersburgSstoreGasTest.java b/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/mainnet/PetersburgSstoreGasTest.java new file mode 100644 index 0000000000..54228571cf --- /dev/null +++ b/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/mainnet/PetersburgSstoreGasTest.java @@ -0,0 +1,110 @@ +/* + * 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; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import static tech.pegasys.pantheon.util.uint.UInt256.ONE; +import static tech.pegasys.pantheon.util.uint.UInt256.ZERO; + +import tech.pegasys.pantheon.ethereum.core.Account; +import tech.pegasys.pantheon.ethereum.core.Gas; +import tech.pegasys.pantheon.util.uint.UInt256; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameter; +import org.junit.runners.Parameterized.Parameters; + +@RunWith(Parameterized.class) +public class PetersburgSstoreGasTest { + + private static final UInt256 TWO = UInt256.of(2); + + private final PetersburgGasCalculator gasCalculator = new PetersburgGasCalculator(); + + @Parameters(name = "original: {0}, current: {1}, new: {2}") + public static Object[][] scenarios() { + return new Object[][] { + // Zero no-op + {ZERO, ZERO, ZERO, Gas.of(5_000), Gas.ZERO}, + + // Zero fresh change + {ZERO, ZERO, ONE, Gas.of(20_000), Gas.ZERO}, + + // Dirty, reset to zero + {ZERO, ONE, ZERO, Gas.of(5_000), Gas.of(15_000)}, + + // Dirty, changed but not reset + {ZERO, ONE, TWO, Gas.of(5_000), Gas.ZERO}, + + // Dirty no-op + {ZERO, ONE, ONE, Gas.of(5_000), Gas.ZERO}, + + // Dirty, zero no-op + {ONE, ZERO, ZERO, Gas.of(5_000), Gas.ZERO}, + + // Dirty, reset to non-zero + {ONE, ZERO, ONE, Gas.of(20_000), Gas.ZERO}, + + // Fresh change to zero + {ONE, ONE, ZERO, Gas.of(5_000), Gas.of(15_000)}, + + // Fresh change with all non-zero + {ONE, ONE, TWO, Gas.of(5_000), Gas.ZERO}, + + // Dirty, clear originally set value + {ONE, TWO, ZERO, Gas.of(5_000), Gas.of(15_000)}, + + // Non-zero no-op + {ONE, ONE, ONE, Gas.of(5_000), Gas.ZERO}, + }; + } + + @Parameter public UInt256 originalValue; + + @Parameter(value = 1) + public UInt256 currentValue; + + @Parameter(value = 2) + public UInt256 newValue; + + @Parameter(value = 3) + public Gas expectedGasCost; + + @Parameter(value = 4) + public Gas expectedGasRefund; + + private final Account account = mock(Account.class); + + @Before + public void setUp() { + when(account.getOriginalStorageValue(UInt256.ZERO)).thenReturn(originalValue); + when(account.getStorageValue(UInt256.ZERO)).thenReturn(currentValue); + } + + @Test + public void shouldChargeCorrectGas() { + assertThat(gasCalculator.calculateStorageCost(account, UInt256.ZERO, newValue)) + .isEqualTo(expectedGasCost); + } + + @Test + public void shouldRefundCorrectGas() { + assertThat(gasCalculator.calculateStorageRefundAmount(account, UInt256.ZERO, newValue)) + .isEqualTo(expectedGasRefund); + } +} From 59699b5a327353e2b46fa8fa393394c5ed36e6f8 Mon Sep 17 00:00:00 2001 From: shemnon Date: Tue, 22 Jan 2019 16:41:26 -0700 Subject: [PATCH 3/6] * Add fork ordering test * Add rinkeby Constantinople block * Petersburg -> ConstantinopleFix --- .../pantheon/config/GenesisConfigOptions.java | 2 +- .../config/JsonGenesisConfigOptions.java | 4 +-- config/src/main/resources/dev.json | 2 +- config/src/main/resources/goerli.json | 2 +- config/src/main/resources/mainnet.json | 2 +- config/src/main/resources/rinkeby.json | 1 + config/src/main/resources/ropsten.json | 1 + .../config/StubGenesisConfigOptions.java | 10 +++--- .../config/GenesisConfigOptionsTest.java | 9 ++--- .../BlockHashOperationBenchmark.java | 4 +-- ...va => ConstantinopleFixGasCalculator.java} | 2 +- .../mainnet/MainnetProtocolSpecs.java | 6 ++-- .../mainnet/ProtocolScheduleBuilder.java | 36 +++++++++++++++++-- .../core/ExecutionContextTestFixture.java | 4 ++- .../core/TransactionTestCaseSpec.java | 4 +-- ...va => ConstantinopleFixSstoreGasTest.java} | 4 +-- .../mainnet/MainnetProtocolScheduleTest.java | 24 ++++++++++--- 17 files changed, 85 insertions(+), 32 deletions(-) rename ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/{PetersburgGasCalculator.java => ConstantinopleFixGasCalculator.java} (96%) rename ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/mainnet/{PetersburgSstoreGasTest.java => ConstantinopleFixSstoreGasTest.java} (95%) diff --git a/config/src/main/java/tech/pegasys/pantheon/config/GenesisConfigOptions.java b/config/src/main/java/tech/pegasys/pantheon/config/GenesisConfigOptions.java index a2a7806605..eb2e3b04fc 100644 --- a/config/src/main/java/tech/pegasys/pantheon/config/GenesisConfigOptions.java +++ b/config/src/main/java/tech/pegasys/pantheon/config/GenesisConfigOptions.java @@ -43,7 +43,7 @@ public interface GenesisConfigOptions { OptionalLong getConstantinopleBlockNumber(); - OptionalLong getPetersburgBlockNumber(); + OptionalLong getConstantinopleFixBlockNumber(); OptionalInt getChainId(); } diff --git a/config/src/main/java/tech/pegasys/pantheon/config/JsonGenesisConfigOptions.java b/config/src/main/java/tech/pegasys/pantheon/config/JsonGenesisConfigOptions.java index 5c279ae0a9..c0ce1b88fe 100644 --- a/config/src/main/java/tech/pegasys/pantheon/config/JsonGenesisConfigOptions.java +++ b/config/src/main/java/tech/pegasys/pantheon/config/JsonGenesisConfigOptions.java @@ -101,8 +101,8 @@ public OptionalLong getConstantinopleBlockNumber() { } @Override - public OptionalLong getPetersburgBlockNumber() { - return getOptionalLong("petersburgblock"); + public OptionalLong getConstantinopleFixBlockNumber() { + return getOptionalLong("constantinopleFixblock"); } @Override diff --git a/config/src/main/resources/dev.json b/config/src/main/resources/dev.json index 815d282f89..f515cd9cad 100644 --- a/config/src/main/resources/dev.json +++ b/config/src/main/resources/dev.json @@ -8,7 +8,7 @@ "eip158Block": 0, "byzantiumBlock": 0, "constantinopleBlock": 0, - "petersburgBlock": 0, + "constantinopleFixBlock": 0, "ethash": { } }, diff --git a/config/src/main/resources/goerli.json b/config/src/main/resources/goerli.json index 1bdb624173..f8fa6ed508 100644 --- a/config/src/main/resources/goerli.json +++ b/config/src/main/resources/goerli.json @@ -9,7 +9,7 @@ "eip160Block":0, "byzantiumBlock":0, "constantinopleBlock":0, - "petersburgBlock": 0, + "constantinopleFixBlock": 0, "clique":{ "period":15, "epoch":30000 diff --git a/config/src/main/resources/mainnet.json b/config/src/main/resources/mainnet.json index 19bffe30d1..925b6e575c 100644 --- a/config/src/main/resources/mainnet.json +++ b/config/src/main/resources/mainnet.json @@ -10,7 +10,7 @@ "eip158Block": 2675000, "byzantiumBlock": 4370000, "constantinopleBlock": 7280000, - "petersburgBlock": 7280000, + "constantinopleFixBlock": 7280000, "ethash": { } diff --git a/config/src/main/resources/rinkeby.json b/config/src/main/resources/rinkeby.json index 1508923164..8aace7b247 100644 --- a/config/src/main/resources/rinkeby.json +++ b/config/src/main/resources/rinkeby.json @@ -7,6 +7,7 @@ "eip155Block": 3, "eip158Block": 3, "byzantiumBlock": 1035301, + "constantinopleBlock": 3660663, "clique": { "period": 15, "epoch": 30000 diff --git a/config/src/main/resources/ropsten.json b/config/src/main/resources/ropsten.json index fbf5e3f5b3..710f15acc9 100644 --- a/config/src/main/resources/ropsten.json +++ b/config/src/main/resources/ropsten.json @@ -7,6 +7,7 @@ "eip158Block": 10, "byzantiumBlock": 1700000, "constantinopleBlock": 4230000, + "constantinopleFixBlock": 4939394, "ethash": { } }, diff --git a/config/src/test-support/java/tech/pegasys/pantheon/config/StubGenesisConfigOptions.java b/config/src/test-support/java/tech/pegasys/pantheon/config/StubGenesisConfigOptions.java index 495164fe3d..9748b52950 100644 --- a/config/src/test-support/java/tech/pegasys/pantheon/config/StubGenesisConfigOptions.java +++ b/config/src/test-support/java/tech/pegasys/pantheon/config/StubGenesisConfigOptions.java @@ -23,7 +23,7 @@ public class StubGenesisConfigOptions implements GenesisConfigOptions { private OptionalLong spuriousDragonBlockNumber = OptionalLong.empty(); private OptionalLong byzantiumBlockNumber = OptionalLong.empty(); private OptionalLong constantinopleBlockNumber = OptionalLong.empty(); - private OptionalLong petersburgBlockNumber = OptionalLong.empty(); + private OptionalLong constantinopleFixBlockNumber = OptionalLong.empty(); private OptionalInt chainId = OptionalInt.empty(); @Override @@ -92,8 +92,8 @@ public OptionalLong getConstantinopleBlockNumber() { } @Override - public OptionalLong getPetersburgBlockNumber() { - return petersburgBlockNumber; + public OptionalLong getConstantinopleFixBlockNumber() { + return constantinopleFixBlockNumber; } @Override @@ -131,8 +131,8 @@ public StubGenesisConfigOptions constantinopleBlock(final long blockNumber) { return this; } - public StubGenesisConfigOptions petersburgBlock(final long blockNumber) { - petersburgBlockNumber = OptionalLong.of(blockNumber); + public StubGenesisConfigOptions constantinopleFixBlock(final long blockNumber) { + constantinopleFixBlockNumber = OptionalLong.of(blockNumber); return this; } diff --git a/config/src/test/java/tech/pegasys/pantheon/config/GenesisConfigOptionsTest.java b/config/src/test/java/tech/pegasys/pantheon/config/GenesisConfigOptionsTest.java index c39a64d8de..a7fac49800 100644 --- a/config/src/test/java/tech/pegasys/pantheon/config/GenesisConfigOptionsTest.java +++ b/config/src/test/java/tech/pegasys/pantheon/config/GenesisConfigOptionsTest.java @@ -102,9 +102,10 @@ public void shouldGetConstantinopleBlockNumber() { } @Test - public void shouldGetPetersburgBlockNumber() { - final GenesisConfigOptions config = fromConfigOptions(singletonMap("petersburgBlock", 1000)); - assertThat(config.getPetersburgBlockNumber()).hasValue(1000); + public void shouldGetConstantinopleFixBlockNumber() { + final GenesisConfigOptions config = + fromConfigOptions(singletonMap("constantinopleFixBlock", 1000)); + assertThat(config.getConstantinopleFixBlockNumber()).hasValue(1000); } @Test @@ -116,7 +117,7 @@ public void shouldNotReturnEmptyOptionalWhenBlockNumberNotSpecified() { assertThat(config.getSpuriousDragonBlockNumber()).isEmpty(); assertThat(config.getByzantiumBlockNumber()).isEmpty(); assertThat(config.getConstantinopleBlockNumber()).isEmpty(); - assertThat(config.getPetersburgBlockNumber()).isEmpty(); + assertThat(config.getConstantinopleFixBlockNumber()).isEmpty(); } @Test diff --git a/ethereum/core/src/jmh/java/tech/pegasys/pantheon/ethereum/vm/operations/BlockHashOperationBenchmark.java b/ethereum/core/src/jmh/java/tech/pegasys/pantheon/ethereum/vm/operations/BlockHashOperationBenchmark.java index b169c61c2f..8fb6658efd 100644 --- a/ethereum/core/src/jmh/java/tech/pegasys/pantheon/ethereum/vm/operations/BlockHashOperationBenchmark.java +++ b/ethereum/core/src/jmh/java/tech/pegasys/pantheon/ethereum/vm/operations/BlockHashOperationBenchmark.java @@ -12,7 +12,7 @@ */ package tech.pegasys.pantheon.ethereum.vm.operations; -import tech.pegasys.pantheon.ethereum.mainnet.PetersburgGasCalculator; +import tech.pegasys.pantheon.ethereum.mainnet.ConstantinopleFixGasCalculator; import tech.pegasys.pantheon.ethereum.vm.BlockHashLookup; import tech.pegasys.pantheon.ethereum.vm.MessageFrame; import tech.pegasys.pantheon.util.bytes.Bytes32; @@ -42,7 +42,7 @@ public class BlockHashOperationBenchmark { @Setup public void prepare() throws Exception { operationBenchmarkHelper = OperationBenchmarkHelper.create(); - operation = new BlockHashOperation(new PetersburgGasCalculator()); + operation = new BlockHashOperation(new ConstantinopleFixGasCalculator()); frame = operationBenchmarkHelper.createMessageFrame(); } diff --git a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/PetersburgGasCalculator.java b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/ConstantinopleFixGasCalculator.java similarity index 96% rename from ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/PetersburgGasCalculator.java rename to ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/ConstantinopleFixGasCalculator.java index f6f03fc62d..034d84bce7 100644 --- a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/PetersburgGasCalculator.java +++ b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/ConstantinopleFixGasCalculator.java @@ -22,7 +22,7 @@ *

Neither {@link TangerineWhistleGasCalculator} nor {@link SpuriousDragonGasCalculator} overrode * these two methods so {@link FrontierGasCalculator} is the source. */ -public class PetersburgGasCalculator extends ConstantinopleGasCalculator { +public class ConstantinopleFixGasCalculator extends ConstantinopleGasCalculator { /** Same as {#link {@link FrontierGasCalculator#STORAGE_SET_GAS_COST} */ private static final Gas STORAGE_SET_GAS_COST = Gas.of(20_000L); 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 ed2b0f4b40..36da2eee28 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 @@ -193,10 +193,10 @@ public static ProtocolSpecBuilder constantinopleDefinition(final int chain .name("Constantinople"); } - public static ProtocolSpecBuilder petersburgDefinition(final int chainId) { + public static ProtocolSpecBuilder constantinopleFixDefinition(final int chainId) { return constantinopleDefinition(chainId) - .gasCalculator(PetersburgGasCalculator::new) - .name("Petersburg"); + .gasCalculator(ConstantinopleFixGasCalculator::new) + .name("ConstantinopleFix"); } private static TransactionReceipt frontierTransactionReceiptFactory( diff --git a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/ProtocolScheduleBuilder.java b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/ProtocolScheduleBuilder.java index 52ebf16574..dce4480b0a 100644 --- a/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/ProtocolScheduleBuilder.java +++ b/ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/mainnet/ProtocolScheduleBuilder.java @@ -36,6 +36,8 @@ public ProtocolSchedule createProtocolSchedule() { final int chainId = config.getChainId().orElse(defaultChainId); final MutableProtocolSchedule protocolSchedule = new MutableProtocolSchedule<>(chainId); + validateForkOrdering(); + addProtocolSpec( protocolSchedule, OptionalLong.of(0), MainnetProtocolSpecs.frontierDefinition()); addProtocolSpec( @@ -82,8 +84,8 @@ public ProtocolSchedule createProtocolSchedule() { MainnetProtocolSpecs.constantinopleDefinition(chainId)); addProtocolSpec( protocolSchedule, - config.getPetersburgBlockNumber(), - MainnetProtocolSpecs.petersburgDefinition(chainId)); + config.getConstantinopleFixBlockNumber(), + MainnetProtocolSpecs.constantinopleFixDefinition(chainId)); return protocolSchedule; } @@ -97,4 +99,34 @@ private void addProtocolSpec( protocolSchedule.putMilestone( number, protocolSpecAdapter.apply(definition).build(protocolSchedule))); } + + private long validateForkOrder( + final String forkName, final OptionalLong thisForkBlock, final long lastForkBlock) { + final long referenceForkBlock = thisForkBlock.orElse(lastForkBlock); + if (lastForkBlock > referenceForkBlock) { + throw new RuntimeException( + String.format( + "Genesis Config Error: '%s' is scheduled for block %d but it must be on or after block %d.", + forkName, thisForkBlock.getAsLong(), lastForkBlock)); + } + return referenceForkBlock; + } + + private void validateForkOrdering() { + long lastForkBlock = 0; + lastForkBlock = validateForkOrder("Homestead", config.getHomesteadBlockNumber(), lastForkBlock); + lastForkBlock = validateForkOrder("DaoFork", config.getDaoForkBlock(), lastForkBlock); + lastForkBlock = + validateForkOrder( + "TangerineWhistle", config.getTangerineWhistleBlockNumber(), lastForkBlock); + lastForkBlock = + validateForkOrder("SpuriousDragon", config.getSpuriousDragonBlockNumber(), lastForkBlock); + lastForkBlock = validateForkOrder("Byzantium", config.getByzantiumBlockNumber(), lastForkBlock); + lastForkBlock = + validateForkOrder("Constantinople", config.getConstantinopleBlockNumber(), lastForkBlock); + lastForkBlock = + validateForkOrder( + "ConstantinopleFix", config.getConstantinopleFixBlockNumber(), lastForkBlock); + assert (lastForkBlock >= 0); + } } diff --git a/ethereum/core/src/test-support/java/tech/pegasys/pantheon/ethereum/core/ExecutionContextTestFixture.java b/ethereum/core/src/test-support/java/tech/pegasys/pantheon/ethereum/core/ExecutionContextTestFixture.java index fce0224a03..94cf237eb9 100644 --- a/ethereum/core/src/test-support/java/tech/pegasys/pantheon/ethereum/core/ExecutionContextTestFixture.java +++ b/ethereum/core/src/test-support/java/tech/pegasys/pantheon/ethereum/core/ExecutionContextTestFixture.java @@ -113,7 +113,9 @@ public ExecutionContextTestFixture build() { if (protocolSchedule == null) { protocolSchedule = new ProtocolScheduleBuilder<>( - new StubGenesisConfigOptions().petersburgBlock(0), 42, Function.identity()) + new StubGenesisConfigOptions().constantinopleFixBlock(0), + 42, + Function.identity()) .createProtocolSchedule(); } if (keyValueStorage == null) { diff --git a/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/core/TransactionTestCaseSpec.java b/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/core/TransactionTestCaseSpec.java index b3a8c49daf..f051100b65 100644 --- a/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/core/TransactionTestCaseSpec.java +++ b/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/core/TransactionTestCaseSpec.java @@ -69,7 +69,7 @@ public TransactionTestCaseSpec( @JsonProperty("EIP158") final Expectation EIP158Expectation, @JsonProperty("Byzantium") final Expectation byzantiumExpectation, @JsonProperty("Constantinople") final Expectation constantinopleExpectation, - @JsonProperty("Petersburg") final Expectation petersburgExpectation, + @JsonProperty("ConstantinopleFix") final Expectation constantinopleFixExpectation, @JsonProperty("rlp") final String rlp) { expectations = new HashMap<>(); expectations.put("Frontier", frontierExpectation); @@ -78,7 +78,7 @@ public TransactionTestCaseSpec( expectations.put("EIP158", EIP158Expectation); expectations.put("Byzantium", byzantiumExpectation); expectations.put("Constantinople", constantinopleExpectation); - expectations.put("Petersburg", petersburgExpectation); + expectations.put("ConstantinopleFix", constantinopleFixExpectation); BytesValue parsedRlp = null; try { diff --git a/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/mainnet/PetersburgSstoreGasTest.java b/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/mainnet/ConstantinopleFixSstoreGasTest.java similarity index 95% rename from ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/mainnet/PetersburgSstoreGasTest.java rename to ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/mainnet/ConstantinopleFixSstoreGasTest.java index 54228571cf..d75d49cbd4 100644 --- a/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/mainnet/PetersburgSstoreGasTest.java +++ b/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/mainnet/ConstantinopleFixSstoreGasTest.java @@ -30,11 +30,11 @@ import org.junit.runners.Parameterized.Parameters; @RunWith(Parameterized.class) -public class PetersburgSstoreGasTest { +public class ConstantinopleFixSstoreGasTest { private static final UInt256 TWO = UInt256.of(2); - private final PetersburgGasCalculator gasCalculator = new PetersburgGasCalculator(); + private final ConstantinopleFixGasCalculator gasCalculator = new ConstantinopleFixGasCalculator(); @Parameters(name = "original: {0}, current: {1}, new: {2}") public static Object[][] scenarios() { diff --git a/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/mainnet/MainnetProtocolScheduleTest.java b/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/mainnet/MainnetProtocolScheduleTest.java index fb3c67567a..00d599acaa 100644 --- a/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/mainnet/MainnetProtocolScheduleTest.java +++ b/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/mainnet/MainnetProtocolScheduleTest.java @@ -39,8 +39,10 @@ public void shouldReturnDefaultProtocolSpecsWhenCustomNumbersAreNotUsed() { Assertions.assertThat(sched.getByBlockNumber(4_730_000L).getName()).isEqualTo("Byzantium"); // Constantinople was originally scheduled for 7_080_000, but postponed Assertions.assertThat(sched.getByBlockNumber(7_080_000L).getName()).isEqualTo("Byzantium"); - Assertions.assertThat(sched.getByBlockNumber(7_280_000L).getName()).isEqualTo("Petersburg"); - Assertions.assertThat(sched.getByBlockNumber(Long.MAX_VALUE).getName()).isEqualTo("Petersburg"); + Assertions.assertThat(sched.getByBlockNumber(7_280_000L).getName()) + .isEqualTo("ConstantinopleFix"); + Assertions.assertThat(sched.getByBlockNumber(Long.MAX_VALUE).getName()) + .isEqualTo("ConstantinopleFix"); } @Test @@ -56,7 +58,7 @@ public void shouldOnlyUseFrontierWhenEmptyJsonConfigIsUsed() { public void createFromConfigWithSettings() { final JsonObject json = new JsonObject( - "{\"config\": {\"homesteadBlock\": 2, \"daoForkBlock\": 3, \"eip150Block\": 14, \"eip158Block\": 15, \"byzantiumBlock\": 16, \"constantinopleBlock\": 18, \"petersburgBlock\": 19, \"chainId\":1234}}"); + "{\"config\": {\"homesteadBlock\": 2, \"daoForkBlock\": 3, \"eip150Block\": 14, \"eip158Block\": 15, \"byzantiumBlock\": 16, \"constantinopleBlock\": 18, \"constantinopleFixBlock\": 19, \"chainId\":1234}}"); final ProtocolSchedule sched = MainnetProtocolSchedule.fromConfig(GenesisConfigFile.fromConfig(json).getConfigOptions()); Assertions.assertThat(sched.getByBlockNumber(1).getName()).isEqualTo("Frontier"); @@ -68,7 +70,21 @@ public void createFromConfigWithSettings() { Assertions.assertThat(sched.getByBlockNumber(15).getName()).isEqualTo("SpuriousDragon"); Assertions.assertThat(sched.getByBlockNumber(16).getName()).isEqualTo("Byzantium"); Assertions.assertThat(sched.getByBlockNumber(18).getName()).isEqualTo("Constantinople"); - Assertions.assertThat(sched.getByBlockNumber(19).getName()).isEqualTo("Petersburg"); + Assertions.assertThat(sched.getByBlockNumber(19).getName()).isEqualTo("ConstantinopleFix"); + } + + @Test + public void outOfOrderForksFails() { + final JsonObject json = + new JsonObject( + "{\"config\": {\"homesteadBlock\": 2, \"daoForkBlock\": 3, \"eip150Block\": 14, \"eip158Block\": 15, \"byzantiumBlock\": 16, \"constantinopleBlock\": 18, \"constantinopleFixBlock\": 17, \"chainId\":1234}}"); + Assertions.assertThatExceptionOfType(RuntimeException.class) + .describedAs( + "Genesis Config Error: 'ConstantinopleFix' is scheduled for block 17 but it must be on or after block 18.") + .isThrownBy( + () -> + MainnetProtocolSchedule.fromConfig( + GenesisConfigFile.fromConfig(json).getConfigOptions())); } @Test From 8e903b779c51fdad77b21e5b1d4edabb1a557bff Mon Sep 17 00:00:00 2001 From: shemnon Date: Tue, 22 Jan 2019 22:18:20 -0700 Subject: [PATCH 4/6] * fix json config bug * add Ropsten constantinopleFix block * run reference tests * remove ConstantinopleFix from TransactionTestCases (there are no tests) --- .../pegasys/pantheon/config/JsonGenesisConfigOptions.java | 2 +- .../pantheon/ethereum/core/TransactionTestCaseSpec.java | 4 +--- .../ethereum/mainnet/MainnetProtocolScheduleTest.java | 3 ++- .../pantheon/ethereum/vm/BlockchainReferenceTestTools.java | 5 +++-- .../pantheon/ethereum/vm/GeneralStateReferenceTestTools.java | 4 ++-- .../pantheon/ethereum/vm/ReferenceTestProtocolSchedules.java | 3 +++ 6 files changed, 12 insertions(+), 9 deletions(-) diff --git a/config/src/main/java/tech/pegasys/pantheon/config/JsonGenesisConfigOptions.java b/config/src/main/java/tech/pegasys/pantheon/config/JsonGenesisConfigOptions.java index c0ce1b88fe..7ec0f1c8a7 100644 --- a/config/src/main/java/tech/pegasys/pantheon/config/JsonGenesisConfigOptions.java +++ b/config/src/main/java/tech/pegasys/pantheon/config/JsonGenesisConfigOptions.java @@ -102,7 +102,7 @@ public OptionalLong getConstantinopleBlockNumber() { @Override public OptionalLong getConstantinopleFixBlockNumber() { - return getOptionalLong("constantinopleFixblock"); + return getOptionalLong("constantinoplefixblock"); } @Override diff --git a/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/core/TransactionTestCaseSpec.java b/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/core/TransactionTestCaseSpec.java index f051100b65..43e7647535 100644 --- a/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/core/TransactionTestCaseSpec.java +++ b/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/core/TransactionTestCaseSpec.java @@ -69,7 +69,6 @@ public TransactionTestCaseSpec( @JsonProperty("EIP158") final Expectation EIP158Expectation, @JsonProperty("Byzantium") final Expectation byzantiumExpectation, @JsonProperty("Constantinople") final Expectation constantinopleExpectation, - @JsonProperty("ConstantinopleFix") final Expectation constantinopleFixExpectation, @JsonProperty("rlp") final String rlp) { expectations = new HashMap<>(); expectations.put("Frontier", frontierExpectation); @@ -78,7 +77,6 @@ public TransactionTestCaseSpec( expectations.put("EIP158", EIP158Expectation); expectations.put("Byzantium", byzantiumExpectation); expectations.put("Constantinople", constantinopleExpectation); - expectations.put("ConstantinopleFix", constantinopleFixExpectation); BytesValue parsedRlp = null; try { @@ -98,7 +96,7 @@ public Expectation expectation(final String milestone) { final Expectation expectation = expectations.get(milestone); if (expectation == null) { - throw new IllegalStateException("Expectation for milestone %s not found" + milestone); + throw new IllegalStateException("Expectation for milestone " + milestone + " not found"); } return expectation; diff --git a/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/mainnet/MainnetProtocolScheduleTest.java b/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/mainnet/MainnetProtocolScheduleTest.java index 00d599acaa..c49c62f381 100644 --- a/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/mainnet/MainnetProtocolScheduleTest.java +++ b/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/mainnet/MainnetProtocolScheduleTest.java @@ -100,7 +100,8 @@ public void shouldCreateRopstenConfig() throws Exception { Assertions.assertThat(sched.getByBlockNumber(10).getName()).isEqualTo("SpuriousDragon"); Assertions.assertThat(sched.getByBlockNumber(1700000).getName()).isEqualTo("Byzantium"); Assertions.assertThat(sched.getByBlockNumber(4230000).getName()).isEqualTo("Constantinople"); + Assertions.assertThat(sched.getByBlockNumber(4939394).getName()).isEqualTo("ConstantinopleFix"); Assertions.assertThat(sched.getByBlockNumber(Long.MAX_VALUE).getName()) - .isEqualTo("Constantinople"); + .isEqualTo("ConstantinopleFix"); } } 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 12b4d6ffbf..1e2ccb281a 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 @@ -43,7 +43,7 @@ public class BlockchainReferenceTestTools { System.getProperty( "test.ethereum.blockchain.eips", "FrontierToHomesteadAt5,HomesteadToEIP150At5,HomesteadToDaoAt5,EIP158ToByzantiumAt5," - + "Frontier,Homestead,EIP150,EIP158,Byzantium,Constantinople"); + + "Frontier,Homestead,EIP150,EIP158,Byzantium,Constantinople,ConstantinopleFix"); NETWORKS_TO_RUN = Arrays.asList(networks.split(",")); } @@ -64,7 +64,8 @@ public class BlockchainReferenceTestTools { params.blacklist("RevertPrecompiledTouch_d0g0v0_(EIP158|Byzantium)"); // Consumes a huge amount of memory - params.blacklist("static_Call1MB1024Calldepth_d1g0v0_(Byzantium|Constantinople)"); + params.blacklist( + "static_Call1MB1024Calldepth_d1g0v0_(Byzantium|Constantinople|ConstantinopleFix)"); } public static Collection generateTestParametersForConfig(final String[] filePath) { diff --git a/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/vm/GeneralStateReferenceTestTools.java b/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/vm/GeneralStateReferenceTestTools.java index b54d8c03ba..b9d4041e44 100644 --- a/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/vm/GeneralStateReferenceTestTools.java +++ b/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/vm/GeneralStateReferenceTestTools.java @@ -51,7 +51,7 @@ private static TransactionProcessor transactionProcessor(final String name) { final String eips = System.getProperty( "test.ethereum.state.eips", - "Frontier,Homestead,EIP150,EIP158,Byzantium,Constantinople"); + "Frontier,Homestead,EIP150,EIP158,Byzantium,Constantinople,ConstantinopleFix"); EIPS_TO_RUN = Arrays.asList(eips.split(",")); } @@ -84,7 +84,7 @@ private static TransactionProcessor transactionProcessor(final String name) { // Gas integer value is too large to construct a valid transaction. params.blacklist("OverflowGasRequire"); // Consumes a huge amount of memory - params.blacklist("static_Call1MB1024Calldepth-(Byzantium|Constantinople)"); + params.blacklist("static_Call1MB1024Calldepth-(Byzantium|Constantinople|ConstantinopleFix)"); } public static Collection generateTestParametersForConfig(final String[] filePath) { diff --git a/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/vm/ReferenceTestProtocolSchedules.java b/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/vm/ReferenceTestProtocolSchedules.java index afd1f769c8..0628b7aa32 100644 --- a/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/vm/ReferenceTestProtocolSchedules.java +++ b/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/vm/ReferenceTestProtocolSchedules.java @@ -46,6 +46,9 @@ public static ReferenceTestProtocolSchedules create() { builder.put("Byzantium", createSchedule(new StubGenesisConfigOptions().byzantiumBlock(0))); builder.put( "Constantinople", createSchedule(new StubGenesisConfigOptions().constantinopleBlock(0))); + builder.put( + "ConstantinopleFix", + createSchedule(new StubGenesisConfigOptions().constantinopleFixBlock(0))); return new ReferenceTestProtocolSchedules(builder.build()); } From 88a068e5777b7514103194007b6c0d67d5c0c802 Mon Sep 17 00:00:00 2001 From: shemnon Date: Wed, 23 Jan 2019 08:28:57 -0700 Subject: [PATCH 5/6] clique test genesis file violates block ordering rules --- acceptance-tests/src/test/resources/clique/clique.json | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/acceptance-tests/src/test/resources/clique/clique.json b/acceptance-tests/src/test/resources/clique/clique.json index 66146e0483..971ccb4041 100644 --- a/acceptance-tests/src/test/resources/clique/clique.json +++ b/acceptance-tests/src/test/resources/clique/clique.json @@ -4,9 +4,11 @@ "homesteadBlock": 1, "eip150Block": 2, "eip150Hash": "0x0000000000000000000000000000000000000000000000000000000000000000", - "eip155Block": 0, - "eip158Block": 0, - "byzantiumBlock": 0, + "eip155Block": 3, + "eip158Block": 4, + "byzantiumBlock": 5, + "constantinopleBlock": 6, + "constantinopleFixBlock": 7, "clique": { "blockperiodseconds": 10, "epochlength": 30000 From 06e67f4f2062593fea2ef30ead8e5438cdf62db1 Mon Sep 17 00:00:00 2001 From: shemnon Date: Wed, 23 Jan 2019 22:33:59 -0700 Subject: [PATCH 6/6] missed a merge item --- .../pantheon/ethereum/mainnet/MainnetProtocolScheduleTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/mainnet/MainnetProtocolScheduleTest.java b/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/mainnet/MainnetProtocolScheduleTest.java index 94dc16b0fe..788109c1fc 100644 --- a/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/mainnet/MainnetProtocolScheduleTest.java +++ b/ethereum/core/src/test/java/tech/pegasys/pantheon/ethereum/mainnet/MainnetProtocolScheduleTest.java @@ -87,7 +87,8 @@ public void outOfOrderForksFails() { .isThrownBy( () -> MainnetProtocolSchedule.fromConfig( - GenesisConfigFile.fromConfig(json).getConfigOptions())); + GenesisConfigFile.fromConfig(json).getConfigOptions(), + PrivacyParameters.noPrivacy())); } @Test