From 8ac8e95225de5a8b5b1d230cbe579a49b1134347 Mon Sep 17 00:00:00 2001 From: Jason Carver Date: Wed, 24 Feb 2021 15:16:04 -0800 Subject: [PATCH] EIP-2718: Add dynamic transaction type for Berlin - Refactor to split the responsibility for encode/decode and core transaction API - Test that legacy transactions still work in Berlin - Test invalid vs unrecognized transaction types Invalid types are disallowed by EIP-2718, unrecognized ones are just not specified yet. (Although one will be soon, in EIP-2930) --- eth/abc.py | 99 ++++++++++++++----- eth/chains/base.py | 2 +- eth/db/chain.py | 13 +-- eth/exceptions.py | 12 +++ eth/rlp/blocks.py | 12 +-- eth/rlp/transactions.py | 9 +- eth/vm/base.py | 9 +- eth/vm/forks/berlin/blocks.py | 12 ++- eth/vm/forks/berlin/transactions.py | 93 ++++++++++++++--- eth/vm/forks/byzantium/blocks.py | 4 +- eth/vm/forks/constantinople/blocks.py | 4 +- eth/vm/forks/frontier/blocks.py | 11 ++- eth/vm/forks/frontier/transactions.py | 17 ++++ eth/vm/forks/homestead/blocks.py | 4 +- eth/vm/forks/istanbul/blocks.py | 4 +- eth/vm/forks/muir_glacier/blocks.py | 4 +- eth/vm/forks/petersburg/blocks.py | 4 +- eth/vm/forks/spurious_dragon/blocks.py | 4 +- newsfragments/1973.feature.rst | 2 + .../test_transaction_encoding.py | 64 ++++++++++++ tests/database/test_eth1_chaindb.py | 4 +- tests/json-fixtures/test_transactions.py | 12 +-- 22 files changed, 310 insertions(+), 89 deletions(-) create mode 100644 newsfragments/1973.feature.rst create mode 100644 tests/core/transaction-utils/test_transaction_encoding.py diff --git a/eth/abc.py b/eth/abc.py index 24269f2d51..ff08ec0792 100644 --- a/eth/abc.py +++ b/eth/abc.py @@ -267,6 +267,76 @@ def as_signed_transaction(self, private_key: PrivateKey) -> 'SignedTransactionAP ... +class TransactionBuilderAPI(ABC): + """ + Responsible for creating and encoding transactions. + + Most simply, the builder is responsible for some pieces of the encoding for + RLP. In legacy transactions, this happens using rlp.Serializeable. It is + also responsible for initializing the transactions. The two transaction + initializers assume legacy transactions, for now. + + Some VMs support multiple distinct transaction types. In that case, the + builder is responsible for dispatching on the different types. + """ + @classmethod + @abstractmethod + def deserialize(cls, encoded: bytes) -> 'SignedTransactionAPI': + """ + Extract a transaction from an encoded RLP object. + + This method is used by rlp.decode(..., sedes=TransactionBuilderAPI). + """ + ... + + @classmethod + @abstractmethod + def serialize(cls, obj: 'SignedTransactionAPI') -> bytes: + """ + Encode a transaction to a series of bytes used by RLP. + + In the case of legacy transactions, it will actually be a list of + bytes. That doesn't show up here, because pyrlp doesn't export type + annotations. + + This method is used by rlp.encode(obj). + """ + ... + + @classmethod + @abstractmethod + def create_unsigned_transaction(cls, + *, + nonce: int, + gas_price: int, + gas: int, + to: Address, + value: int, + data: bytes) -> UnsignedTransactionAPI: + """ + Create an unsigned transaction. + """ + ... + + @classmethod + @abstractmethod + def new_transaction( + cls, + nonce: int, + gas_price: int, + gas: int, + to: Address, + value: int, + data: bytes, + v: int, + r: int, + s: int) -> 'SignedTransactionAPI': + """ + Create a signed transaction. + """ + ... + + class SignedTransactionAPI(BaseTransactionAPI, TransactionFieldsAPI): def __init__(self, *args: Any, **kwargs: Any) -> None: @@ -345,21 +415,6 @@ def get_message_for_signing(self) -> bytes: """ ... - @classmethod - @abstractmethod - def create_unsigned_transaction(cls, - *, - nonce: int, - gas_price: int, - gas: int, - to: Address, - value: int, - data: bytes) -> UnsignedTransactionAPI: - """ - Create an unsigned transaction. - """ - ... - # We can remove this API and inherit from rlp.Serializable when it becomes typesafe def as_dict(self) -> Dict[Hashable, Any]: """ @@ -374,7 +429,7 @@ class BlockAPI(ABC): """ header: BlockHeaderAPI transactions: Tuple[SignedTransactionAPI, ...] - transaction_class: Type[SignedTransactionAPI] = None + transaction_builder: Type[TransactionBuilderAPI] = None uncles: Tuple[BlockHeaderAPI, ...] @abstractmethod @@ -386,9 +441,9 @@ def __init__(self, @classmethod @abstractmethod - def get_transaction_class(cls) -> Type[SignedTransactionAPI]: + def get_transaction_builder(cls) -> Type[TransactionBuilderAPI]: """ - Return the transaction class that is valid for the block. + Return the transaction builder for the block. """ ... @@ -812,7 +867,7 @@ def add_transaction(self, def get_block_transactions( self, block_header: BlockHeaderAPI, - transaction_class: Type[SignedTransactionAPI]) -> Tuple[SignedTransactionAPI, ...]: + transaction_builder: Type[TransactionBuilderAPI]) -> Tuple[SignedTransactionAPI, ...]: """ Return an iterable of transactions for the block speficied by the given block header. @@ -851,7 +906,7 @@ def get_transaction_by_index( self, block_number: BlockNumber, transaction_index: int, - transaction_class: Type[SignedTransactionAPI]) -> SignedTransactionAPI: + transaction_builder: Type[TransactionBuilderAPI]) -> SignedTransactionAPI: """ Return the transaction at the specified `transaction_index` from the block specified by `block_number` from the canonical chain. @@ -2987,9 +3042,9 @@ def create_unsigned_transaction(cls, @classmethod @abstractmethod - def get_transaction_class(cls) -> Type[SignedTransactionAPI]: + def get_transaction_builder(cls) -> Type[TransactionBuilderAPI]: """ - Return the class that this VM uses for transactions. + Return the class that this VM uses to build and encode transactions. """ ... diff --git a/eth/chains/base.py b/eth/chains/base.py index abc6060b87..6b93fbbaf2 100644 --- a/eth/chains/base.py +++ b/eth/chains/base.py @@ -391,7 +391,7 @@ def get_canonical_transaction_by_index(self, return self.chaindb.get_transaction_by_index( block_number, index, - VM_class.get_transaction_class(), + VM_class.get_transaction_builder(), ) def create_transaction(self, *args: Any, **kwargs: Any) -> SignedTransactionAPI: diff --git a/eth/db/chain.py b/eth/db/chain.py index f02e684a34..26e95ba351 100644 --- a/eth/db/chain.py +++ b/eth/db/chain.py @@ -30,6 +30,7 @@ AtomicDatabaseAPI, ReceiptAPI, SignedTransactionAPI, + TransactionBuilderAPI, ) from eth.constants import ( EMPTY_UNCLE_HASH, @@ -308,8 +309,8 @@ def add_transaction(self, def get_block_transactions( self, header: BlockHeaderAPI, - transaction_class: Type[SignedTransactionAPI]) -> Tuple[SignedTransactionAPI, ...]: - return self._get_block_transactions(header.transaction_root, transaction_class) + transaction_builder: Type[TransactionBuilderAPI]) -> Tuple[SignedTransactionAPI, ...]: + return self._get_block_transactions(header.transaction_root, transaction_builder) def get_block_transaction_hashes(self, block_header: BlockHeaderAPI) -> Tuple[Hash32, ...]: """ @@ -348,7 +349,7 @@ def get_transaction_by_index( self, block_number: BlockNumber, transaction_index: int, - transaction_class: Type[SignedTransactionAPI]) -> SignedTransactionAPI: + transaction_builder: Type[TransactionBuilderAPI]) -> SignedTransactionAPI: try: block_header = self.get_canonical_block_header_by_number(block_number) except HeaderNotFound: @@ -357,7 +358,7 @@ def get_transaction_by_index( encoded_index = rlp.encode(transaction_index) encoded_transaction = transaction_db[encoded_index] if encoded_transaction != b'': - return rlp.decode(encoded_transaction, sedes=transaction_class) + return rlp.decode(encoded_transaction, sedes=transaction_builder) else: raise TransactionNotFound( f"No transaction is at index {transaction_index} of block {block_number}" @@ -412,12 +413,12 @@ def _get_block_transaction_data(db: DatabaseAPI, transaction_root: Hash32) -> It def _get_block_transactions( self, transaction_root: Hash32, - transaction_class: Type[SignedTransactionAPI]) -> Iterable[SignedTransactionAPI]: + transaction_builder: Type[TransactionBuilderAPI]) -> Iterable[SignedTransactionAPI]: """ Memoizable version of `get_block_transactions` """ for encoded_transaction in self._get_block_transaction_data(self.db, transaction_root): - yield rlp.decode(encoded_transaction, sedes=transaction_class) + yield rlp.decode(encoded_transaction, sedes=transaction_builder) @staticmethod def _remove_transaction_from_canonical_chain(db: DatabaseAPI, transaction_hash: Hash32) -> None: diff --git a/eth/exceptions.py b/eth/exceptions.py index f920f61a15..28de8bd6d4 100644 --- a/eth/exceptions.py +++ b/eth/exceptions.py @@ -46,6 +46,18 @@ class TransactionNotFound(PyEVMError): pass +class UnrecognizedTransactionType(PyEVMError): + """ + Raised when an encoded transaction is using a first byte that is valid, but + unrecognized. According to EIP 2718, the byte may be in the range [0, 0x7f]. + As of the Berlin hard fork, all of those versions are undefined, except for + 0x01 in EIP 2930. + """ + @property + def type_int(self) -> int: + return self.args[0] + + class ReceiptNotFound(PyEVMError): """ Raised when the Receipt with the given receipt index does not exist. diff --git a/eth/rlp/blocks.py b/eth/rlp/blocks.py index 6e15a54448..96f034af66 100644 --- a/eth/rlp/blocks.py +++ b/eth/rlp/blocks.py @@ -12,18 +12,18 @@ ) from eth.abc import ( BlockAPI, - SignedTransactionAPI, + TransactionBuilderAPI, ) class BaseBlock(Configurable, rlp.Serializable, BlockAPI): - transaction_class: Type[SignedTransactionAPI] = None + transaction_builder: Type[TransactionBuilderAPI] = None @classmethod - def get_transaction_class(cls) -> Type[SignedTransactionAPI]: - if cls.transaction_class is None: - raise AttributeError("Block subclasses must declare a transaction_class") - return cls.transaction_class + def get_transaction_builder(cls) -> Type[TransactionBuilderAPI]: + if cls.transaction_builder is None: + raise AttributeError("Block subclasses must declare a transaction_builder") + return cls.transaction_builder @property def is_genesis(self) -> bool: diff --git a/eth/rlp/transactions.py b/eth/rlp/transactions.py index d91690fc56..ad5c3b56eb 100644 --- a/eth/rlp/transactions.py +++ b/eth/rlp/transactions.py @@ -19,6 +19,7 @@ BaseTransactionAPI, ComputationAPI, SignedTransactionAPI, + TransactionBuilderAPI, TransactionFieldsAPI, UnsignedTransactionAPI, ) @@ -60,7 +61,13 @@ def hash(self) -> Hash32: return keccak(rlp.encode(self)) -class BaseTransaction(BaseTransactionFields, BaseTransactionMethods, SignedTransactionAPI): # noqa: E501 +class BaseTransaction(BaseTransactionFields, BaseTransactionMethods, SignedTransactionAPI, TransactionBuilderAPI): # noqa: E501 + # "Legacy" transactions implemented by BaseTransaction are a combination of + # the transaction codec (TransactionBuilderAPI) *and* the transaction + # object (SignedTransactionAPI). In a multi-transaction-type world, that + # becomes less desirable, and that responsibility splits up. See Berlin + # transactions, for example. + # this is duplicated to make the rlp library happy, otherwise it complains # about no fields being defined but inheriting from multiple `Serializable` # bases. diff --git a/eth/vm/base.py b/eth/vm/base.py index 629ec74c7c..82d9b4849c 100644 --- a/eth/vm/base.py +++ b/eth/vm/base.py @@ -39,6 +39,7 @@ ReceiptAPI, SignedTransactionAPI, StateAPI, + TransactionBuilderAPI, UnsignedTransactionAPI, VirtualMachineAPI, ) @@ -477,7 +478,7 @@ def previous_hashes(self) -> Optional[Iterable[Hash32]]: # Transactions # def create_transaction(self, *args: Any, **kwargs: Any) -> SignedTransactionAPI: - return self.get_transaction_class()(*args, **kwargs) + return self.get_transaction_builder().new_transaction(*args, **kwargs) @classmethod def create_unsigned_transaction(cls, @@ -488,7 +489,7 @@ def create_unsigned_transaction(cls, to: Address, value: int, data: bytes) -> UnsignedTransactionAPI: - return cls.get_transaction_class().create_unsigned_transaction( + return cls.get_transaction_builder().create_unsigned_transaction( nonce=nonce, gas_price=gas_price, gas=gas, @@ -498,8 +499,8 @@ def create_unsigned_transaction(cls, ) @classmethod - def get_transaction_class(cls) -> Type[SignedTransactionAPI]: - return cls.get_block_class().get_transaction_class() + def get_transaction_builder(cls) -> Type[TransactionBuilderAPI]: + return cls.get_block_class().get_transaction_builder() # # Validate diff --git a/eth/vm/forks/berlin/blocks.py b/eth/vm/forks/berlin/blocks.py index 61df903a39..021d023cbe 100644 --- a/eth/vm/forks/berlin/blocks.py +++ b/eth/vm/forks/berlin/blocks.py @@ -1,6 +1,12 @@ +from typing import Type + from rlp.sedes import ( CountableList, ) + +from eth.abc import ( + TransactionBuilderAPI, +) from eth.rlp.headers import ( BlockHeader, ) @@ -9,14 +15,14 @@ ) from .transactions import ( - BerlinTransaction, + BerlinTransactionBuilder, ) class BerlinBlock(MuirGlacierBlock): - transaction_class = BerlinTransaction + transaction_builder: Type[TransactionBuilderAPI] = BerlinTransactionBuilder # type: ignore fields = [ ('header', BlockHeader), - ('transactions', CountableList(transaction_class)), + ('transactions', CountableList(transaction_builder)), ('uncles', CountableList(BlockHeader)) ] diff --git a/eth/vm/forks/berlin/transactions.py b/eth/vm/forks/berlin/transactions.py index 74ee8fba8a..239e578f44 100644 --- a/eth/vm/forks/berlin/transactions.py +++ b/eth/vm/forks/berlin/transactions.py @@ -1,6 +1,18 @@ from eth_keys.datatypes import PrivateKey from eth_typing import Address +from eth_utils import ( + to_int, +) +from rlp.exceptions import ( + DeserializationError, +) +from eth.abc import ( + SignedTransactionAPI, + TransactionBuilderAPI, + UnsignedTransactionAPI, +) +from eth.exceptions import UnrecognizedTransactionType from eth.vm.forks.muir_glacier.transactions import ( MuirGlacierTransaction, MuirGlacierUnsignedTransaction, @@ -11,25 +23,16 @@ ) -class BerlinTransaction(MuirGlacierTransaction): - @classmethod - def create_unsigned_transaction(cls, - *, - nonce: int, - gas_price: int, - gas: int, - to: Address, - value: int, - data: bytes) -> 'BerlinUnsignedTransaction': - return BerlinUnsignedTransaction(nonce, gas_price, gas, to, value, data) +class BerlinLegacyTransaction(MuirGlacierTransaction): + pass -class BerlinUnsignedTransaction(MuirGlacierUnsignedTransaction): +class BerlinUnsignedLegacyTransaction(MuirGlacierUnsignedTransaction): def as_signed_transaction(self, private_key: PrivateKey, - chain_id: int = None) -> BerlinTransaction: + chain_id: int = None) -> BerlinLegacyTransaction: v, r, s = create_transaction_signature(self, private_key, chain_id=chain_id) - return BerlinTransaction( + return BerlinLegacyTransaction( nonce=self.nonce, gas_price=self.gas_price, gas=self.gas, @@ -40,3 +43,65 @@ def as_signed_transaction(self, r=r, s=s, ) + + +class BerlinTransactionBuilder(TransactionBuilderAPI): + """ + Responsible for serializing transactions of ambiguous type. + + It dispatches to either the legacy transaction type or the new typed + transaction, depending on the nature of the encoded/decoded transaction. + """ + legacy_signed = BerlinLegacyTransaction + legacy_unsigned = BerlinUnsignedLegacyTransaction + + @classmethod + def deserialize(cls, encoded: bytes) -> SignedTransactionAPI: + if len(encoded) == 0: + raise DeserializationError( + "Encoded transaction was empty, which makes it invalid", + encoded, + ) + + if isinstance(encoded, bytes): + transaction_type = to_int(encoded[0]) + if transaction_type == 1: + raise UnrecognizedTransactionType(transaction_type, "TODO: Implement EIP-2930") + elif transaction_type in range(0, 0x80): + raise UnrecognizedTransactionType(transaction_type, "Unknown transaction type") + else: + raise DeserializationError( + f"Typed Transaction must start with 0-0x7f, but got {hex(transaction_type)}", + encoded, + ) + else: + return cls.legacy_signed.deserialize(encoded) + + @classmethod + def serialize(cls, obj: SignedTransactionAPI) -> bytes: + return cls.legacy_signed.serialize(obj) + + @classmethod + def create_unsigned_transaction(cls, + *, + nonce: int, + gas_price: int, + gas: int, + to: Address, + value: int, + data: bytes) -> UnsignedTransactionAPI: + return cls.legacy_unsigned(nonce, gas_price, gas, to, value, data) + + @classmethod + def new_transaction( + cls, + nonce: int, + gas_price: int, + gas: int, + to: Address, + value: int, + data: bytes, + v: int, + r: int, + s: int) -> SignedTransactionAPI: + return cls.legacy_signed(nonce, gas_price, gas, to, value, data, v, r, s) diff --git a/eth/vm/forks/byzantium/blocks.py b/eth/vm/forks/byzantium/blocks.py index 7edd62b4e5..3ce0d707f3 100644 --- a/eth/vm/forks/byzantium/blocks.py +++ b/eth/vm/forks/byzantium/blocks.py @@ -14,9 +14,9 @@ class ByzantiumBlock(SpuriousDragonBlock): - transaction_class = ByzantiumTransaction + transaction_builder = ByzantiumTransaction fields = [ ('header', BlockHeader), - ('transactions', CountableList(transaction_class)), + ('transactions', CountableList(transaction_builder)), ('uncles', CountableList(BlockHeader)) ] diff --git a/eth/vm/forks/constantinople/blocks.py b/eth/vm/forks/constantinople/blocks.py index 819c299081..8bd7480596 100644 --- a/eth/vm/forks/constantinople/blocks.py +++ b/eth/vm/forks/constantinople/blocks.py @@ -14,9 +14,9 @@ class ConstantinopleBlock(ByzantiumBlock): - transaction_class = ConstantinopleTransaction + transaction_builder = ConstantinopleTransaction fields = [ ('header', BlockHeader), - ('transactions', CountableList(transaction_class)), + ('transactions', CountableList(transaction_builder)), ('uncles', CountableList(BlockHeader)) ] diff --git a/eth/vm/forks/frontier/blocks.py b/eth/vm/forks/frontier/blocks.py index bf55b70ea1..1f62359924 100644 --- a/eth/vm/forks/frontier/blocks.py +++ b/eth/vm/forks/frontier/blocks.py @@ -26,6 +26,7 @@ ChainDatabaseAPI, ReceiptAPI, SignedTransactionAPI, + TransactionBuilderAPI, ) from eth.constants import ( EMPTY_UNCLE_HASH, @@ -50,10 +51,10 @@ class FrontierBlock(BaseBlock): - transaction_class = FrontierTransaction + transaction_builder = FrontierTransaction fields = [ ('header', BlockHeader), - ('transactions', CountableList(transaction_class)), + ('transactions', CountableList(transaction_builder)), ('uncles', CountableList(BlockHeader)) ] @@ -92,8 +93,8 @@ def hash(self) -> Hash32: # Transaction class for this block class # @classmethod - def get_transaction_class(cls) -> Type[SignedTransactionAPI]: - return cls.transaction_class + def get_transaction_builder(cls) -> Type[TransactionBuilderAPI]: + return cls.transaction_builder # # Receipts API @@ -120,7 +121,7 @@ def from_header(cls, header: BlockHeaderAPI, chaindb: ChainDatabaseAPI) -> "Fron raise BlockNotFound(f"Uncles not found in database for {header}: {exc}") from exc try: - transactions = chaindb.get_block_transactions(header, cls.get_transaction_class()) + transactions = chaindb.get_block_transactions(header, cls.get_transaction_builder()) except MissingTrieNode as exc: raise BlockNotFound(f"Transactions not found in database for {header}: {exc}") from exc diff --git a/eth/vm/forks/frontier/transactions.py b/eth/vm/forks/frontier/transactions.py index 4bfbd70148..81f25f79e5 100644 --- a/eth/vm/forks/frontier/transactions.py +++ b/eth/vm/forks/frontier/transactions.py @@ -8,6 +8,9 @@ Address, ) +from eth.abc import ( + SignedTransactionAPI, +) from eth.constants import ( CREATE_CONTRACT_ADDRESS, GAS_TX, @@ -112,6 +115,20 @@ def create_unsigned_transaction(cls, data: bytes) -> 'FrontierUnsignedTransaction': return FrontierUnsignedTransaction(nonce, gas_price, gas, to, value, data) + @classmethod + def new_transaction( + cls, + nonce: int, + gas_price: int, + gas: int, + to: Address, + value: int, + data: bytes, + v: int, + r: int, + s: int) -> SignedTransactionAPI: + return cls(nonce, gas_price, gas, to, value, data, v, r, s) + class FrontierUnsignedTransaction(BaseUnsignedTransaction): diff --git a/eth/vm/forks/homestead/blocks.py b/eth/vm/forks/homestead/blocks.py index e2d5bf3827..5555f26e3f 100644 --- a/eth/vm/forks/homestead/blocks.py +++ b/eth/vm/forks/homestead/blocks.py @@ -13,9 +13,9 @@ class HomesteadBlock(FrontierBlock): - transaction_class = HomesteadTransaction + transaction_builder = HomesteadTransaction fields = [ ('header', BlockHeader), - ('transactions', CountableList(transaction_class)), + ('transactions', CountableList(transaction_builder)), ('uncles', CountableList(BlockHeader)) ] diff --git a/eth/vm/forks/istanbul/blocks.py b/eth/vm/forks/istanbul/blocks.py index 83eb4b4ca2..414ee1ac1d 100644 --- a/eth/vm/forks/istanbul/blocks.py +++ b/eth/vm/forks/istanbul/blocks.py @@ -14,9 +14,9 @@ class IstanbulBlock(PetersburgBlock): - transaction_class = IstanbulTransaction + transaction_builder = IstanbulTransaction fields = [ ('header', BlockHeader), - ('transactions', CountableList(transaction_class)), + ('transactions', CountableList(transaction_builder)), ('uncles', CountableList(BlockHeader)) ] diff --git a/eth/vm/forks/muir_glacier/blocks.py b/eth/vm/forks/muir_glacier/blocks.py index c33221eaf9..c53915065a 100644 --- a/eth/vm/forks/muir_glacier/blocks.py +++ b/eth/vm/forks/muir_glacier/blocks.py @@ -14,9 +14,9 @@ class MuirGlacierBlock(IstanbulBlock): - transaction_class = MuirGlacierTransaction + transaction_builder = MuirGlacierTransaction fields = [ ('header', BlockHeader), - ('transactions', CountableList(transaction_class)), + ('transactions', CountableList(transaction_builder)), ('uncles', CountableList(BlockHeader)) ] diff --git a/eth/vm/forks/petersburg/blocks.py b/eth/vm/forks/petersburg/blocks.py index cbd20eb92c..4c244af159 100644 --- a/eth/vm/forks/petersburg/blocks.py +++ b/eth/vm/forks/petersburg/blocks.py @@ -14,9 +14,9 @@ class PetersburgBlock(ByzantiumBlock): - transaction_class = PetersburgTransaction + transaction_builder = PetersburgTransaction fields = [ ('header', BlockHeader), - ('transactions', CountableList(transaction_class)), + ('transactions', CountableList(transaction_builder)), ('uncles', CountableList(BlockHeader)) ] diff --git a/eth/vm/forks/spurious_dragon/blocks.py b/eth/vm/forks/spurious_dragon/blocks.py index 69fa847bb8..fe9371d1e5 100644 --- a/eth/vm/forks/spurious_dragon/blocks.py +++ b/eth/vm/forks/spurious_dragon/blocks.py @@ -13,9 +13,9 @@ class SpuriousDragonBlock(HomesteadBlock): - transaction_class = SpuriousDragonTransaction + transaction_builder = SpuriousDragonTransaction fields = [ ('header', BlockHeader), - ('transactions', CountableList(transaction_class)), + ('transactions', CountableList(transaction_builder)), ('uncles', CountableList(BlockHeader)) ] diff --git a/newsfragments/1973.feature.rst b/newsfragments/1973.feature.rst new file mode 100644 index 0000000000..1a9be2bbee --- /dev/null +++ b/newsfragments/1973.feature.rst @@ -0,0 +1,2 @@ +Implement EIP-2718: Typed Transactions -- not much action here, mostly refactoring in preparation +for EIP-2930. (Though it does churn the code a fair bit, to support multiple transaction formats) diff --git a/tests/core/transaction-utils/test_transaction_encoding.py b/tests/core/transaction-utils/test_transaction_encoding.py new file mode 100644 index 0000000000..55b4c91f5a --- /dev/null +++ b/tests/core/transaction-utils/test_transaction_encoding.py @@ -0,0 +1,64 @@ +from eth_utils import ( + decode_hex, + to_bytes, +) +import pytest +import rlp + +from eth.exceptions import UnrecognizedTransactionType +from eth.vm.forks import ( + BerlinVM, +) + +UNRECOGNIZED_TRANSACTION_TYPES = tuple( + (to_bytes(val), UnrecognizedTransactionType) + for val in range(0, 0x80) +) + +# These are valid RLP byte-strings, but invalid for EIP-2718 +INVALID_TRANSACTION_TYPES = tuple( + (rlp.encode(to_bytes(val)), rlp.exceptions.DeserializationError) + for val in range(0x80, 0x100) +) + + +@pytest.mark.parametrize('vm_class', [BerlinVM]) +@pytest.mark.parametrize( + 'encoded, expected', + ( + ( + decode_hex('0xdd80010294ffffffffffffffffffffffffffffffffffffffff0380040506'), + dict( + nonce=0, + gas_price=1, + gas=2, + to=b'\xff' * 20, + value=3, + data=b'', + v=4, + r=5, + s=6, + ), + ), + ( + decode_hex('0xc0'), + rlp.exceptions.DeserializationError, + ), + ) + + UNRECOGNIZED_TRANSACTION_TYPES + + INVALID_TRANSACTION_TYPES +) +def test_transaction_decode(vm_class, encoded, expected): + sedes = vm_class.get_transaction_builder() + if type(expected) is type and issubclass(expected, Exception): + with pytest.raises(expected): + rlp.decode(encoded, sedes=sedes) + else: + # Check that the given transaction encodes to the start encoding + expected_txn = sedes.new_transaction(**expected) + expected_encoding = rlp.encode(expected_txn) + assert encoded == expected_encoding + + # Check that the encoded bytes decode to the given data + decoded = rlp.decode(encoded, sedes=sedes) + assert decoded == expected_txn diff --git a/tests/database/test_eth1_chaindb.py b/tests/database/test_eth1_chaindb.py index 222129f85e..20f30a196e 100644 --- a/tests/database/test_eth1_chaindb.py +++ b/tests/database/test_eth1_chaindb.py @@ -332,7 +332,7 @@ def test_chaindb_get_receipt_and_tx_by_index(chain, funded_address, funded_addre if block.header.block_number == REQUIRED_BLOCK_NUMBER: actual_receipt = receipts[REQUIRED_RECEIPT_INDEX] actual_tx = block.transactions[REQUIRED_RECEIPT_INDEX] - tx_class = block.transaction_class + tx_class = block.transaction_builder # Check that the receipt retrieved is indeed the actual one chaindb_retrieved_receipt = chain.chaindb.get_receipt_by_index( @@ -402,7 +402,7 @@ def test_chaindb_persist_unexecuted_block(chain, if block.header.block_number == REQUIRED_BLOCK_NUMBER: actual_receipt = receipts[REQUIRED_RECEIPT_INDEX] actual_tx = block.transactions[REQUIRED_RECEIPT_INDEX] - tx_class = block.transaction_class + tx_class = block.transaction_builder if use_persist_unexecuted_block: second_chain.chaindb.persist_unexecuted_block(block, receipts) diff --git a/tests/json-fixtures/test_transactions.py b/tests/json-fixtures/test_transactions.py index ab3e17db09..805e0c6f1f 100644 --- a/tests/json-fixtures/test_transactions.py +++ b/tests/json-fixtures/test_transactions.py @@ -37,12 +37,6 @@ from eth.vm.forks.istanbul.transactions import ( IstanbulTransaction ) -from eth.vm.forks.muir_glacier.transactions import ( - MuirGlacierTransaction -) -from eth.vm.forks.berlin.transactions import ( - BerlinTransaction -) from eth_typing.enums import ( ForkName @@ -111,11 +105,7 @@ def fixture_transaction_class(fixture_data): elif fork_name == ForkName.ConstantinopleFix: return PetersburgTransaction elif fork_name == ForkName.Istanbul: - return IstanbulTransaction - elif fork_name == ForkName.MuirGlacier: - return MuirGlacierTransaction - elif fork_name == ForkName.Berlin: - return BerlinTransaction + return IstanbulTransaction # There seem to be no new transaction tests since Istanbul elif fork_name == ForkName.Metropolis: pytest.skip("Metropolis Transaction class has not been implemented") else: