Add typed transactions - EIP 2718 - Berlin #1978
Conversation
carver
force-pushed
the
eip-2718-typed-txns
branch
from
7935801
to
54a828c
Compare
|
Ok, I think this has the basic shape that we want. I won't ask for a final merge review yet, but it's far enough along if anyone wants to give architectural feedback, or just follow what's happening. cc @kclowes @marcgarreau |
|
Ok, now ready for a proper review. Are you folks up for a call tomorrow morning to look over it together? I know you're new to the repo, so hopefully we'll catch anything funky as I try to explain out loud what's going on. :) |
|
@carver will give it a look now, but would love to talk through it in the morning - thanks :) |
|
@carver yep, tomorrow morning would be great! |
| rlp.decode(encoded, sedes=sedes) | ||
| else: | ||
| # Check that the given transaction encodes to the start encoding | ||
| expected_txn = BerlinLegacyTransaction(**expected) |
There was a problem hiding this comment.
Use the new_transaction() API.
| expected_txn = BerlinLegacyTransaction(**expected) | |
| expected_txn = sedes.new_transaction(**expected) |
| ) | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( |
There was a problem hiding this comment.
| @pytest.mark.parametrize( | |
| @pytest.mark.parametrize('vm_class', [BerlinVM]) | |
| @pytest.mark.parametrize( |
eth/vm/forks/berlin/transactions.py
Outdated
|
|
||
| # TODO: subclass rlp.sedes.dynamic.DynamicSerializable, when it's released | ||
| # See the example in pyrlp:tests/core/test_dynamic_entries.py | ||
| class BerlinTransaction(TransactionBuilderAPI): |
There was a problem hiding this comment.
| class BerlinTransaction(TransactionBuilderAPI): | |
| class BerlinTransactionBuilder(TransactionBuilderAPI): |
| + INVALID_TRANSACTION_TYPES | ||
| ) | ||
| def test_transaction_decode(encoded, expected): | ||
| sedes = BerlinVM.get_transaction_class() |
There was a problem hiding this comment.
Refactor:
| sedes = BerlinVM.get_transaction_class() | |
| sedes = BerlinVM.get_transaction_builder() |
|
Handled all the notes taken during review. |
- 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)
carver
force-pushed
the
eip-2718-typed-txns
branch
from
9f7cb72
to
8ac8e95
Compare
What was wrong?
Fixes #1973
How was it fixed?
Uses the basic structure from ethereum/pyrlp#131 to enable dynamic encodings. It kicks the can down the road a bit to #1975 for actually supported any specific typed transactions, since they aren't defined until then.
Had to split the transaction responsibility up a bit: isolated a transaction builder/serializer from the actual transaction API. It causes some code churn, but not quite as much as I feared.
To-Do
Cute Animal Picture