Skip to content

Commit

Permalink
Add upper limit validation to withdrawal fields + some cleanup
Browse files Browse the repository at this point in the history
- Validate withdrawal fields as per EIP-4895
- Add some gut check tests for validation of withdrawal fields
- Simplify empty account cleanup logic when withdrawal amount is 0
  • Loading branch information
fselmo committed Mar 1, 2023
1 parent a4eb26e commit 1db1353
Show file tree
Hide file tree
Showing 6 changed files with 120 additions and 12 deletions.
7 changes: 7 additions & 0 deletions eth/abc.py
Original file line number Diff line number Diff line change
Expand Up @@ -765,6 +765,13 @@ def hash(self) -> Hash32:
"""
...

@abstractmethod
def validate(self) -> None:
"""
Validate withdrawal fields.
"""
...

@abstractmethod
def encode(self) -> bytes:
"""
Expand Down
4 changes: 2 additions & 2 deletions eth/validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ def validate_uint64(value: int, title: str = "Value") -> None:
)
if value > UINT_64_MAX:
raise ValidationError(
f"{title} exeeds maximum UINT256 size. Got: {value}"
f"{title} exceeds maximum uint64 size. Got: {value}"
)


Expand All @@ -168,7 +168,7 @@ def validate_uint256(value: int, title: str = "Value") -> None:
)
if value > UINT_256_MAX:
raise ValidationError(
f"{title} exceeds maximum UINT256 size. Got: {value}"
f"{title} exceeds maximum uint256 size. Got: {value}"
)


Expand Down
3 changes: 3 additions & 0 deletions eth/vm/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,9 @@ def apply_all_withdrawals(
self._validate_header_before_apply(base_header, vm_header)

for withdrawal in withdrawals:
# validate withdrawal fields
withdrawal.validate()

self.apply_withdrawal(withdrawal)

#
Expand Down
6 changes: 1 addition & 5 deletions eth/vm/forks/shanghai/state.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,5 @@ def apply_withdrawal(self, withdrawal: WithdrawalAPI) -> None:
amount_in_wei = withdrawal.amount * 10 ** 9
self.delta_balance(withdrawal.address, amount_in_wei)

# delete account if it is empty
if (
self.get_balance(withdrawal.address) == 0
and self.get_code(withdrawal.address) == b''
):
if self.account_is_empty(withdrawal.address):
self.delete_account(withdrawal.address)
24 changes: 19 additions & 5 deletions eth/vm/forks/shanghai/withdrawals.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,25 +2,33 @@
from typing import cast

from eth.abc import WithdrawalAPI
from eth_typing import Address, Hash32
from eth.validation import (
validate_canonical_address,
validate_uint64,
)
from eth_typing import (
Address,
Hash32,
)

import rlp
from eth.rlp.sedes import address
from eth_utils import keccak
from rlp.sedes import big_endian_int


class Withdrawal(rlp.Serializable):
fields = [
('index', rlp.sedes.big_endian_int),
('validator_index', rlp.sedes.big_endian_int),
('index', big_endian_int),
('validator_index', big_endian_int),
('address', address),
('amount', rlp.sedes.big_endian_int),
('amount', big_endian_int),
]

def __init__(
self,
index: int = 0,
validator_index: int = None,
validator_index: int = 0,
address: Address = None,
amount: int = 0,
) -> None:
Expand All @@ -31,6 +39,12 @@ def __init__(
amount=amount,
)

def validate(self) -> None:
validate_uint64(self.index, "Withdrawal.index")
validate_uint64(self.validator_index, "Withdrawal.validator_index")
validate_canonical_address(self.address, "Withdrawal.address")
validate_uint64(self.amount, "Withdrawal.amount")

@classmethod
def decode(cls, encoded: bytes) -> WithdrawalAPI:
return rlp.decode(encoded, sedes=cls)
Expand Down
88 changes: 88 additions & 0 deletions tests/core/validation/test_withdrawal_validation.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
import pytest
from eth_typing import Address

from eth.vm.forks.shanghai.withdrawals import Withdrawal
from eth.validation import UINT_64_MAX
from eth_utils import (
ValidationError,
to_hex,
)


@pytest.mark.parametrize(
"withdrawal",
(
Withdrawal(0, 0, Address(b"\x00" * 20), 0),
Withdrawal(1, 1, Address(b"\xff" * 20), 1),
Withdrawal(
1337,
1337,
Address(
b"\x85\x82\xa2\x89V\xb9%\x93M\x03\xdd\xb4Xu\xe1\x8e\x85\x93\x12\xc1"
),
1337,
),
Withdrawal(UINT_64_MAX, UINT_64_MAX, Address(b"\x00" * 20), UINT_64_MAX),
)
)
def test_valid_withdrawal_fields(withdrawal):
withdrawal.validate()


@pytest.mark.parametrize(
"withdrawal,message",
(
# validate `index`, `validator_index`, and `amount` fields
(Withdrawal(-1, 0, Address(b"\x00" * 20), 0), "cannot be negative"),
(Withdrawal(0, -1, Address(b"\x00" * 20), 1), "cannot be negative"),
(Withdrawal(0, 0, Address(b"\x00" * 20), -1), "cannot be negative"),
(
Withdrawal(
UINT_64_MAX + 1,
UINT_64_MAX,
Address(b"\x00" * 20),
UINT_64_MAX,
),
"exceeds maximum uint64 size",
),
(
Withdrawal(
UINT_64_MAX,
UINT_64_MAX + 1,
Address(b"\x00" * 20),
UINT_64_MAX,
),
"exceeds maximum uint64 size",
),
(
Withdrawal(
UINT_64_MAX,
UINT_64_MAX,
Address(b"\x00" * 20),
UINT_64_MAX + 1,
),
"exceeds maximum uint64 size",
),
# validate `address` field
(Withdrawal(0, 0, Address(b"\x00" * 19), 0), "not a valid canonical address"),
(Withdrawal(0, 0, Address(b"\x00" * 21), 0), "not a valid canonical address"),
(Withdrawal(0, 0, Address(b"\x00"), 0), "not a valid canonical address"),
(Withdrawal(0, 0, to_hex(b"\x00" * 20), 0), "not a valid canonical address"),
),
ids=[
"negative index",
"negative validator_index",
"negative amount",
"index is UINT_64_MAX plus one",
"validator_index is UINT_64_MAX plus one",
"amount is UINT_64_MAX plus one",
"address size is 19 bytes",
"address size is 21 bytes",
"address size is 1 byte",
"address is valid but provided as hex string",
]
)
def test_invalid_withdrawal_fields(withdrawal, message):
with pytest.raises(ValidationError, match=message):
withdrawal.validate()

0 comments on commit 1db1353

Please sign in to comment.