From 23d9e97edfadd4ec322172a70978b202618f1688 Mon Sep 17 00:00:00 2001 From: Jason Carver Date: Tue, 5 Nov 2019 10:21:09 -0800 Subject: [PATCH 1/6] Configure Istanbul block number for Goerli --- eth/chains/goerli/__init__.py | 3 +++ eth/chains/goerli/constants.py | 2 ++ 2 files changed, 5 insertions(+) diff --git a/eth/chains/goerli/__init__.py b/eth/chains/goerli/__init__.py index 64c6507ee7..76c2b95cd4 100644 --- a/eth/chains/goerli/__init__.py +++ b/eth/chains/goerli/__init__.py @@ -1,6 +1,7 @@ from eth_utils import decode_hex from .constants import ( + ISTANBUL_GOERLI_BLOCK, PETERSBURG_GOERLI_BLOCK, ) @@ -8,11 +9,13 @@ from eth.rlp.headers import BlockHeader from eth.vm.forks import ( + IstanbulVM, PetersburgVM, ) GOERLI_VM_CONFIGURATION = ( (PETERSBURG_GOERLI_BLOCK, PetersburgVM), + (ISTANBUL_GOERLI_BLOCK, IstanbulVM), ) diff --git a/eth/chains/goerli/constants.py b/eth/chains/goerli/constants.py index 036b645e2c..de60c898dd 100644 --- a/eth/chains/goerli/constants.py +++ b/eth/chains/goerli/constants.py @@ -9,3 +9,5 @@ # Petersburg # PETERSBURG_GOERLI_BLOCK = BlockNumber(0) + +ISTANBUL_GOERLI_BLOCK = BlockNumber(1561651) From 7011a539244d8d37c81bd8267ba6b56fe6285d66 Mon Sep 17 00:00:00 2001 From: Jason Carver Date: Tue, 5 Nov 2019 10:20:46 -0800 Subject: [PATCH 2/6] Configure Istanbul block number for mainnet --- eth/chains/mainnet/__init__.py | 4 ++++ eth/chains/mainnet/constants.py | 2 ++ 2 files changed, 6 insertions(+) diff --git a/eth/chains/mainnet/__init__.py b/eth/chains/mainnet/__init__.py index 60aa5d499c..629479eac3 100644 --- a/eth/chains/mainnet/__init__.py +++ b/eth/chains/mainnet/__init__.py @@ -13,6 +13,7 @@ from .constants import ( MAINNET_CHAIN_ID, BYZANTIUM_MAINNET_BLOCK, + ISTANBUL_MAINNET_BLOCK, PETERSBURG_MAINNET_BLOCK, TANGERINE_WHISTLE_MAINNET_BLOCK, HOMESTEAD_MAINNET_BLOCK, @@ -34,6 +35,7 @@ ByzantiumVM, FrontierVM, HomesteadVM, + IstanbulVM, PetersburgVM, SpuriousDragonVM, TangerineWhistleVM, @@ -80,6 +82,7 @@ class MainnetHomesteadVM(MainnetDAOValidatorVM): SPURIOUS_DRAGON_MAINNET_BLOCK, BYZANTIUM_MAINNET_BLOCK, PETERSBURG_MAINNET_BLOCK, + ISTANBUL_MAINNET_BLOCK, ) MAINNET_VMS = ( FrontierVM, @@ -88,6 +91,7 @@ class MainnetHomesteadVM(MainnetDAOValidatorVM): SpuriousDragonVM, ByzantiumVM, PetersburgVM, + IstanbulVM, ) MAINNET_VM_CONFIGURATION = tuple(zip(MAINNET_FORK_BLOCKS, MAINNET_VMS)) diff --git a/eth/chains/mainnet/constants.py b/eth/chains/mainnet/constants.py index 669d895ca9..ae953a6752 100644 --- a/eth/chains/mainnet/constants.py +++ b/eth/chains/mainnet/constants.py @@ -42,3 +42,5 @@ # Petersburg Block # PETERSBURG_MAINNET_BLOCK = BlockNumber(7280000) + +ISTANBUL_MAINNET_BLOCK = BlockNumber(9069000) From ea7ebe09cde2a4e7e45fcad903bb8390ae378c58 Mon Sep 17 00:00:00 2001 From: Jason Carver Date: Tue, 5 Nov 2019 10:34:51 -0800 Subject: [PATCH 3/6] Default the tester chain to Istanbul Also add an explicit test with Istanbul --- .../tester/test_generate_vm_configuration.py | 24 +++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/tests/core/tester/test_generate_vm_configuration.py b/tests/core/tester/test_generate_vm_configuration.py index 57ef011a5f..1678fac83c 100644 --- a/tests/core/tester/test_generate_vm_configuration.py +++ b/tests/core/tester/test_generate_vm_configuration.py @@ -17,6 +17,7 @@ class Forks(enum.Enum): Byzantium = 'Byzantium' Constantinople = 'Constantinople' Petersburg = 'Petersburg' + Istanbul = 'Istanbul' class CustomFrontierVM(FrontierVM): @@ -29,7 +30,7 @@ class CustomFrontierVM(FrontierVM): ( tuple(), {}, - ((0, Forks.Petersburg),), + ((0, Forks.Istanbul),), ), ( ((0, 'tangerine-whistle'), (1, 'spurious-dragon')), @@ -116,7 +117,7 @@ class CustomFrontierVM(FrontierVM): (1, 'homestead'), (2, 'tangerine-whistle'), (3, 'byzantium'), - (5, 'petersburg') + (5, 'petersburg'), ), {}, ( @@ -127,6 +128,25 @@ class CustomFrontierVM(FrontierVM): (5, Forks.Petersburg), ), ), + ( + ( + (0, 'frontier'), + (1, 'homestead'), + (2, 'tangerine-whistle'), + (3, 'byzantium'), + (5, 'petersburg'), + (7, 'istanbul'), + ), + {}, + ( + (0, Forks.Frontier), + (1, Forks.Homestead), + (2, Forks.TangerineWhistle), + (3, Forks.Byzantium), + (5, Forks.Petersburg), + (7, Forks.Istanbul), + ), + ), ), ) def test_generate_vm_configuration(args, kwargs, expected): From 0741923b3cb10c3bb1450fcc5b50a754b56965c7 Mon Sep 17 00:00:00 2001 From: Jason Carver Date: Tue, 5 Nov 2019 10:47:03 -0800 Subject: [PATCH 4/6] Prefer computation.raise_if_error() over assert Raising the exception immediately tells you what's wrong if a test fails, rather than giving you a no-context assertion failure. --- scripts/benchmark/checks/deploy_dos.py | 6 +++--- scripts/benchmark/checks/erc20_interact.py | 12 ++++++++---- .../chain-object/test_build_block_incrementally.py | 4 ++-- tests/core/chain-object/test_chain.py | 4 ++-- tests/database/test_eth1_chaindb.py | 2 +- 5 files changed, 16 insertions(+), 12 deletions(-) diff --git a/scripts/benchmark/checks/deploy_dos.py b/scripts/benchmark/checks/deploy_dos.py index f04b1acc01..1f29e95ba1 100644 --- a/scripts/benchmark/checks/deploy_dos.py +++ b/scripts/benchmark/checks/deploy_dos.py @@ -153,7 +153,7 @@ def deploy_dos_contract(self, chain: MiningChain) -> None: block, receipt, computation = chain.apply_transaction(tx) self.deployed_contract_address = computation.msg.storage_address - assert computation.is_success + computation.raise_if_error() # Interact with the deployed contract by calling the totalSupply() API ????? self.dos_contract = self.w3.eth.contract( @@ -176,7 +176,7 @@ def sstore_uint64(self, chain: MiningChain) -> None: block, receipt, computation = chain.apply_transaction(tx2) - assert computation.is_success + computation.raise_if_error() def create_empty_contract(self, chain: MiningChain) -> None: w3_tx3 = self.dos_contract.functions.createEmptyContract().buildTransaction(W3_TX_DEFAULTS) @@ -193,7 +193,7 @@ def create_empty_contract(self, chain: MiningChain) -> None: block, receipt, computation = chain.apply_transaction(tx3) - assert computation.is_success + computation.raise_if_error() def sstore_uint64_revert(self, chain: MiningChain) -> None: w3_tx4 = self.dos_contract.functions.storageEntropyRevert().buildTransaction(W3_TX_DEFAULTS) diff --git a/scripts/benchmark/checks/erc20_interact.py b/scripts/benchmark/checks/erc20_interact.py index b56bb6db6e..97e7d212b4 100644 --- a/scripts/benchmark/checks/erc20_interact.py +++ b/scripts/benchmark/checks/erc20_interact.py @@ -149,7 +149,8 @@ def _deploy_simple_token(self, chain: MiningChain) -> None: # Keep track of deployed contract address self.deployed_contract_address = computation.msg.storage_address - assert computation.is_success + computation.raise_if_error() + # Keep track of simple_token object self.simple_token = self.w3.eth.contract( address=Web3.toChecksumAddress(encode_hex(self.deployed_contract_address)), @@ -174,7 +175,8 @@ def _erc_transfer(self, addr: str, chain: MiningChain) -> None: block, receipt, computation = chain.apply_transaction(tx) - assert computation.is_success + computation.raise_if_error() + assert to_int(computation.output) == 1 def _erc_approve(self, addr2: str, chain: MiningChain) -> None: @@ -195,7 +197,8 @@ def _erc_approve(self, addr2: str, chain: MiningChain) -> None: block, receipt, computation = chain.apply_transaction(tx) - assert computation.is_success + computation.raise_if_error() + assert to_int(computation.output) == 1 def _erc_transfer_from(self, addr1: str, addr2: str, chain: MiningChain) -> None: @@ -218,7 +221,8 @@ def _erc_transfer_from(self, addr1: str, addr2: str, chain: MiningChain) -> None block, receipt, computation = chain.apply_transaction(tx) - assert computation.is_success + computation.raise_if_error() + assert to_int(computation.output) == 1 diff --git a/tests/core/chain-object/test_build_block_incrementally.py b/tests/core/chain-object/test_build_block_incrementally.py index 523f4143f0..d1a96d086a 100644 --- a/tests/core/chain-object/test_build_block_incrementally.py +++ b/tests/core/chain-object/test_build_block_incrementally.py @@ -30,7 +30,7 @@ def test_building_block_incrementally_with_single_transaction( private_key=funded_address_private_key, ) _, _, computation = chain.apply_transaction(tx) - assert computation.is_success + computation.raise_if_error() # test that the *latest* block hasn't changed assert chain.get_canonical_head().hash == head_hash @@ -57,7 +57,7 @@ def test_building_block_incrementally_with_multiple_transactions( ) txns.append(tx) _, _, computation = chain.apply_transaction(tx) - assert computation.is_success + computation.raise_if_error() # test that the pending block has the expected number of transactions vm = chain.get_vm() diff --git a/tests/core/chain-object/test_chain.py b/tests/core/chain-object/test_chain.py index 32d85c7a17..52bfc87cf3 100644 --- a/tests/core/chain-object/test_chain.py +++ b/tests/core/chain-object/test_chain.py @@ -59,11 +59,11 @@ def test_import_block(chain, tx): if hasattr(chain, 'apply_transaction'): # working on a Mining chain which can directly apply transactions new_block, _, computation = chain.apply_transaction(tx) - assert computation.is_success + computation.raise_if_error() else: # working on a non-mining chain, so we have to build the block to apply manually new_block, receipts, computations = chain.build_block_with_transactions([tx]) - assert computations[0].is_success + computations[0].raise_if_error() block, _, _ = chain.import_block(new_block) diff --git a/tests/database/test_eth1_chaindb.py b/tests/database/test_eth1_chaindb.py index 9d66d28590..33c9a5d0fe 100644 --- a/tests/database/test_eth1_chaindb.py +++ b/tests/database/test_eth1_chaindb.py @@ -167,7 +167,7 @@ def test_chaindb_get_receipt_by_index( private_key=funded_address_private_key, ) new_block, tx_receipt, computation = chain.apply_transaction(tx) - assert computation.is_success + computation.raise_if_error() if (block_number + 1) == REQUIRED_BLOCK_NUMBER and tx_index == REQUIRED_RECEIPT_INDEX: actual_receipt = tx_receipt From 7be93c85187225cac2829d5111fea91c2ea6462d Mon Sep 17 00:00:00 2001 From: Jason Carver Date: Tue, 5 Nov 2019 10:47:51 -0800 Subject: [PATCH 5/6] Increase gas limit in SSTORE benchmarks Istanbul increases several gas prices. The benchmark doesn't need to verify gas usage, so no need to be super-specific on the transaction's gas limit. --- scripts/benchmark/checks/deploy_dos.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/benchmark/checks/deploy_dos.py b/scripts/benchmark/checks/deploy_dos.py index 1f29e95ba1..dd0f436705 100644 --- a/scripts/benchmark/checks/deploy_dos.py +++ b/scripts/benchmark/checks/deploy_dos.py @@ -44,8 +44,8 @@ ) FIRST_TX_GAS_LIMIT = 367724 -SECOND_TX_GAS_LIMIT = 62050 -THIRD_TX_GAS_LIMIT = 104789 +SECOND_TX_GAS_LIMIT = 120000 +THIRD_TX_GAS_LIMIT = 200000 FORTH_TX_GAS_LIMIT = 21272 FIFTH_TX_GAS_LIMIT = 21272 From fa0888445e107e9d95d004a3da2c2d64e49b8943 Mon Sep 17 00:00:00 2001 From: Jason Carver Date: Tue, 5 Nov 2019 10:51:02 -0800 Subject: [PATCH 6/6] Add #1870 release notes --- newsfragments/1839.feature.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 newsfragments/1839.feature.rst diff --git a/newsfragments/1839.feature.rst b/newsfragments/1839.feature.rst new file mode 100644 index 0000000000..66a714e8dd --- /dev/null +++ b/newsfragments/1839.feature.rst @@ -0,0 +1 @@ +Add Istanbul block number to chain configuration for Mainnet and Goerli