Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement EIP 1234 #1303

Merged
merged 2 commits into from
Sep 22, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 12 additions & 6 deletions eth/vm/forks/byzantium/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@
Type,
)

from cytoolz import (
curry,
)

from eth_utils import (
encode_hex,
ValidationError,
Expand Down Expand Up @@ -44,6 +48,13 @@ def make_byzantium_receipt(base_header, transaction, computation, state):
return frontier_receipt.copy(state_root=status_code)


@curry
def get_uncle_reward(block_reward, block_number, uncle):
block_number_delta = block_number - uncle.block_number
validate_lte(block_number_delta, MAX_UNCLE_DEPTH)
return (8 - block_number_delta) * block_reward // 8


EIP658_STATUS_CODES = {
EIP658_TRANSACTION_STATUS_CODE_SUCCESS,
EIP658_TRANSACTION_STATUS_CODE_FAILURE,
Expand All @@ -63,6 +74,7 @@ class ByzantiumVM(SpuriousDragonVM):
compute_difficulty = staticmethod(compute_byzantium_difficulty)
configure_header = configure_byzantium_header
make_receipt = staticmethod(make_byzantium_receipt)
get_uncle_reward = staticmethod(get_uncle_reward(EIP649_BLOCK_REWARD))

@classmethod
def validate_receipt(cls, receipt: Receipt) -> None:
Expand All @@ -80,9 +92,3 @@ def validate_receipt(cls, receipt: Receipt) -> None:
@staticmethod
def get_block_reward():
return EIP649_BLOCK_REWARD

@staticmethod
def get_uncle_reward(block_number, uncle):
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively, we could make this a @classmethod and then access the block reward on cls.block_reward which would allow us drop any uncle reward refinement in ConstantinopleVM. Thoughts?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Conceptually I'm fine with that but I think it goes against the direction we're trying to go with composition > inheritance. I like the mechanism you've got in place now with the curried re-usable utilities so if you're happy with that I say keep it (and maybe backport it to the ByzantiumVM)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, I like it as-is, too. I kind of like the idea of pushing the get_uncle_reward(...) definition as far back as possible, so that even the import carries semantic meaning, like:

from eth.vm.forks.frontier import get_uncle_reward

Would mean that the uncle mechanism has stayed the same since frontier, with only the block reward changing in: get_uncle_reward = staticmethod(get_uncle_reward(EIP649_BLOCK_REWARD)).

(I don't remember when/if the uncle mechanism changed, but the general concept stands.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SGTM 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, I like it as-is, too. I kind of like the idea of pushing the get_uncle_reward(...) definition as far back as possible

It was last changed in byzantium so I think it is already pushed as far back as possible.

block_number_delta = block_number - uncle.block_number
validate_lte(block_number_delta, MAX_UNCLE_DEPTH)
return (8 - block_number_delta) * EIP649_BLOCK_REWARD // 8
41 changes: 35 additions & 6 deletions eth/vm/forks/byzantium/headers.py
Original file line number Diff line number Diff line change
@@ -1,17 +1,30 @@
from typing import (
Any,
Callable,
)
from cytoolz import (
curry,
)
from eth.constants import (
EMPTY_UNCLE_HASH,
DIFFICULTY_ADJUSTMENT_DENOMINATOR,
DIFFICULTY_MINIMUM,
BOMB_EXPONENTIAL_PERIOD,
BOMB_EXPONENTIAL_FREE_PERIODS,
)
from eth.rlp.headers import (
BlockHeader,
)
from eth.utils.db import (
get_parent_header,
)
from eth.validation import (
validate_gt,
validate_header_params_for_configuration,
)
from eth.vm.base import (
BaseVM
)
from eth.vm.forks.frontier.headers import (
create_frontier_header_from_parent,
)
Expand All @@ -21,7 +34,11 @@
)


def compute_byzantium_difficulty(parent_header, timestamp):
@curry
def compute_difficulty(
bomb_delay: int,
parent_header: BlockHeader,
timestamp: int) -> int:
"""
https://github.com/ethereum/EIPs/issues/100
"""
Expand All @@ -46,7 +63,7 @@ def compute_byzantium_difficulty(parent_header, timestamp):
num_bomb_periods = (
max(
0,
parent_header.block_number + 1 - 3000000,
parent_header.block_number + 1 - bomb_delay,
) // BOMB_EXPONENTIAL_PERIOD
) - BOMB_EXPONENTIAL_FREE_PERIODS

Expand All @@ -56,27 +73,39 @@ def compute_byzantium_difficulty(parent_header, timestamp):
return difficulty


def create_byzantium_header_from_parent(parent_header, **header_params):
@curry
def create_header_from_parent(difficulty_fn: Callable[[BlockHeader, int], int],
parent_header: BlockHeader,
**header_params: Any) -> BlockHeader:

if 'difficulty' not in header_params:
header_params.setdefault('timestamp', parent_header.timestamp + 1)

header_params['difficulty'] = compute_byzantium_difficulty(
header_params['difficulty'] = difficulty_fn(
parent_header=parent_header,
timestamp=header_params['timestamp'],
)
return create_frontier_header_from_parent(parent_header, **header_params)


def configure_byzantium_header(vm, **header_params):
@curry
def configure_header(difficulty_fn: Callable[[BlockHeader, int], int],
vm: BaseVM,
**header_params: Any) -> BlockHeader:
validate_header_params_for_configuration(header_params)

with vm.block.header.build_changeset(**header_params) as changeset:
if 'timestamp' in header_params and changeset.block_number > 0:
parent_header = get_parent_header(changeset.build_rlp(), vm.chaindb)
changeset.difficulty = compute_byzantium_difficulty(
changeset.difficulty = difficulty_fn(
parent_header,
header_params['timestamp'],
)

header = changeset.commit()
return header


compute_byzantium_difficulty = compute_difficulty(3000000)
create_byzantium_header_from_parent = create_header_from_parent(compute_byzantium_difficulty)
configure_byzantium_header = configure_header(compute_byzantium_difficulty)
21 changes: 20 additions & 1 deletion eth/vm/forks/constantinople/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,19 @@
)

from eth.rlp.blocks import BaseBlock # noqa: F401
from eth.vm.forks.byzantium import ByzantiumVM
from eth.vm.forks.byzantium import (
ByzantiumVM,
get_uncle_reward,
)
from eth.vm.state import BaseState # noqa: F401

from .blocks import ConstantinopleBlock
from .constants import EIP1234_BLOCK_REWARD
from .headers import (
compute_constantinople_difficulty,
configure_constantinople_header,
create_constantinople_header_from_parent,
)
from .state import ConstantinopleState


Expand All @@ -17,3 +26,13 @@ class ConstantinopleVM(ByzantiumVM):
# classes
block_class = ConstantinopleBlock # type: Type[BaseBlock]
_state_class = ConstantinopleState # type: Type[BaseState]

# Methods
create_header_from_parent = staticmethod(create_constantinople_header_from_parent)
compute_difficulty = staticmethod(compute_constantinople_difficulty)
configure_header = configure_constantinople_header
get_uncle_reward = staticmethod(get_uncle_reward(EIP1234_BLOCK_REWARD))

@staticmethod
def get_block_reward():
return EIP1234_BLOCK_REWARD
5 changes: 5 additions & 0 deletions eth/vm/forks/constantinople/constants.py
Original file line number Diff line number Diff line change
@@ -1 +1,6 @@
from eth_utils import denoms


GAS_EXTCODEHASH_EIP1052 = 400

EIP1234_BLOCK_REWARD = 2 * denoms.ether
13 changes: 13 additions & 0 deletions eth/vm/forks/constantinople/headers.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
from eth.vm.forks.byzantium.headers import (
configure_header,
create_header_from_parent,
compute_difficulty,
)


compute_constantinople_difficulty = compute_difficulty(5000000)

create_constantinople_header_from_parent = create_header_from_parent(
compute_constantinople_difficulty
)
configure_constantinople_header = configure_header(compute_constantinople_difficulty)
84 changes: 84 additions & 0 deletions tests/core/vm/test_rewards.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
import pytest

from eth_utils import (
to_wei
)

from eth.chains.base import (
MiningChain
)
from eth.tools.builder.chain import (
at_block_number,
build,
disable_pow_check,
mine_block,
byzantium_at,
frontier_at,
homestead_at,
spurious_dragon_at,
tangerine_whistle_at,
constantinople_at,
genesis,
)


@pytest.mark.parametrize(
'vm_fn, miner_1_balance, miner_2_balance',
(
(frontier_at, 15.15625, 4.375),
(homestead_at, 15.15625, 4.375),
(tangerine_whistle_at, 15.15625, 4.375),
(spurious_dragon_at, 15.15625, 4.375),
(byzantium_at, 9.09375, 2.625),
(constantinople_at, 6.0625, 1.75),
)
)
def test_rewards(vm_fn, miner_1_balance, miner_2_balance):

OTHER_MINER_ADDRESS = 20 * b'\x01'
TOTAL_BLOCKS_CANONICAL_CHAIN = 3

chain = build(
MiningChain,
vm_fn(0),
disable_pow_check(),
genesis(),
mine_block(), # 1
mine_block(), # 2
)

fork_chain = build(
chain,
at_block_number(1),
mine_block(extra_data=b'fork-it!', coinbase=OTHER_MINER_ADDRESS), # fork 2
)

# we don't use canonical head here because the fork chain is non-canonical.
uncle = fork_chain.get_block_header_by_hash(fork_chain.header.parent_hash)
assert uncle.block_number == 2
assert uncle != chain.get_canonical_head()

chain = build(
chain,
mine_block(uncles=[uncle]), # 3
)

header = chain.get_canonical_head()
block = chain.get_block_by_hash(header.hash)
assert len(block.uncles) == 1
assert block.uncles[0] == uncle

vm = chain.get_vm()
coinbase_balance = vm.state.account_db.get_balance(block.header.coinbase)
other_miner_balance = vm.state.account_db.get_balance(uncle.coinbase)

# We first test if the balance matches what we would determine
# if we made all the API calls involved ourselves.
assert coinbase_balance == (vm.get_block_reward() *
TOTAL_BLOCKS_CANONICAL_CHAIN +
vm.get_nephew_reward())
assert other_miner_balance == vm.get_uncle_reward(block.number, uncle)

# But we also ensure the balance matches the numbers that we calculated on paper
assert coinbase_balance == to_wei(miner_1_balance, 'ether')
assert other_miner_balance == to_wei(miner_2_balance, 'ether')