Skip to content

Commit

Permalink
Drop check_seal support from validate_header
Browse files Browse the repository at this point in the history
  • Loading branch information
cburgdorf committed Jan 10, 2020
1 parent 8b87c90 commit befb75f
Show file tree
Hide file tree
Showing 6 changed files with 34 additions and 44 deletions.
5 changes: 2 additions & 3 deletions eth/abc.py
Expand Up @@ -2696,12 +2696,11 @@ def validate_block(self, block: BlockAPI) -> None:
"""
...

@classmethod
@abstractmethod
def validate_header(self,
header: BlockHeaderAPI,
parent_header: BlockHeaderAPI,
check_seal: bool = True
) -> None:
parent_header: BlockHeaderAPI) -> None:
"""
:raise eth.exceptions.ValidationError: if the header is not valid
"""
Expand Down
8 changes: 6 additions & 2 deletions eth/chains/base.py
Expand Up @@ -164,12 +164,15 @@ def validate_chain(
should_check_seal = index in indices_to_check_seal
vm = self.get_vm(child)
try:
vm.validate_header(child, parent, check_seal=should_check_seal)
vm.validate_header(child, parent)
except ValidationError as exc:
raise ValidationError(
f"{child} is not a valid child of {parent}: {exc}"
) from exc

if should_check_seal:
vm.validate_seal(child)

def validate_chain_extension(self, headers: Tuple[BlockHeaderAPI, ...]) -> None:
for index, header in enumerate(headers):
vm = self.get_vm(header)
Expand Down Expand Up @@ -506,7 +509,8 @@ def validate_block(self, block: BlockAPI) -> None:
raise ValidationError("Cannot validate genesis block this way")
vm = self.get_vm(block.header)
parent_header = self.get_block_header_by_hash(block.header.parent_hash)
vm.validate_header(block.header, parent_header, check_seal=True)
vm.validate_header(block.header, parent_header)
vm.validate_seal(block.header)
vm.validate_seal_extension(block.header, ())
self.validate_uncles(block)
self.validate_gaslimit(block.header)
Expand Down
12 changes: 6 additions & 6 deletions eth/chains/mainnet/__init__.py
Expand Up @@ -68,15 +68,15 @@ def validate_header_is_on_intended_dao_fork(support_dao_fork: bool,
class MainnetDAOValidatorVM(HomesteadVM):
"""Only on mainnet, TheDAO fork is accompanied by special extra data. Validate those headers"""

def validate_header(self,
@classmethod
def validate_header(cls,
header: BlockHeaderAPI,
previous_header: BlockHeaderAPI,
check_seal: bool=True) -> None:
previous_header: BlockHeaderAPI) -> None:

super().validate_header(header, previous_header, check_seal)
super().validate_header(header, previous_header)
validate_header_is_on_intended_dao_fork(
self.support_dao_fork,
self.get_dao_fork_block_number(),
cls.support_dao_fork,
cls.get_dao_fork_block_number(),
header
)

Expand Down
30 changes: 14 additions & 16 deletions eth/vm/base.py
Expand Up @@ -548,16 +548,17 @@ def validate_block(self, block: BlockAPI) -> None:
f" - header uncle_hash: {block.header.uncles_hash}"
)

def validate_header(self,
@classmethod
def validate_header(cls,
header: BlockHeaderAPI,
parent_header: BlockHeaderAPI,
check_seal: bool = True) -> None:
parent_header: BlockHeaderAPI) -> None:

if parent_header is None:
# to validate genesis header, check if it equals canonical header at block number 0
raise ValidationError("Must have access to parent header to validate current header")
else:
validate_length_lte(
header.extra_data, self.extra_data_max_bytes, title="BlockHeader.extra_data")
header.extra_data, cls.extra_data_max_bytes, title="BlockHeader.extra_data")

validate_gas_limit(header.gas_limit, parent_header.gas_limit)

Expand All @@ -577,19 +578,16 @@ def validate_header(self,
f"- parent : {parent_header.timestamp}. "
)

if check_seal:
try:
self.validate_seal(header)
except ValidationError as exc:
self.cls_logger.warning(
"Failed to validate seal on header: %r. Error: %s",
header.as_dict(),
exc,
)
raise

def validate_seal(self, header: BlockHeaderAPI) -> None:
self._consensus.validate_seal(header)
try:
self._consensus.validate_seal(header)
except ValidationError as exc:
self.cls_logger.warning(
"Failed to validate seal on header: %r. Error: %s",
header.as_dict(),
exc,
)
raise

def validate_seal_extension(self,
header: BlockHeaderAPI,
Expand Down
3 changes: 3 additions & 0 deletions newsfragments/1909.removal.rst
@@ -0,0 +1,3 @@
Drop optional ``check_seal`` param from ``VM.validate_header`` and turn it into a ``classmethod``.
Seal checks now need to be made explicitly via ``VM.check_seal`` which is also aligned
with ``VM.check_seal_extension``.
20 changes: 3 additions & 17 deletions tests/core/vm/test_mainnet_dao_fork.py
Expand Up @@ -10,9 +10,7 @@
from eth.chains.mainnet import (
MainnetHomesteadVM,
)
from eth.consensus import ConsensusContext
from eth.rlp.headers import BlockHeader
from eth.vm.chain_context import ChainContext


class ETC_VM(MainnetHomesteadVM):
Expand Down Expand Up @@ -269,11 +267,6 @@ def header_pairs(VM, headers, valid):
yield VM, pair[1], pair[0], valid


class FakeChainDB:
def __init__(self, db):
self.db = db


@pytest.mark.parametrize(
'VM, header, previous_header, valid',
header_pairs(MainnetHomesteadVM, ETH_HEADERS_NEAR_FORK, valid=True) + (
Expand All @@ -287,19 +280,12 @@ def __init__(self, db):
),
)
def test_mainnet_dao_fork_header_validation(VM, header, previous_header, valid):
chain_db = FakeChainDB({})
consensus_context = ConsensusContext(chain_db.db)
vm = VM(
header=previous_header,
chaindb=chain_db,
chain_context=ChainContext(1),
consensus_context=consensus_context
)

if valid:
vm.validate_header(header, previous_header, check_seal=True)
VM.validate_header(header, previous_header)
else:
try:
vm.validate_header(header, previous_header, check_seal=True)
VM.validate_header(header, previous_header)
except ValidationError:
pass
else:
Expand Down

0 comments on commit befb75f

Please sign in to comment.