added support for eip-2718 && 2930

[malonaz]

What was wrong?

The library does not support EIP-2718 (Typed Transactions) and EIP-2930 (Optional Access Lists).

Issue #114

How was it fixed?

EIP-2718 throws away the well-understood transaction format, leaving individual new transaction types free to define their encoding format. I initially attempted to plumb this through the existing transactions type, but eventually decided to introduce a typed_transactions library. The philosophy is simple: A lot of complexity in eth transactions (v vs chain etc) exist because of the traditional transaction format. The introduction of the transaction types in EIP-2718 means we no longer have to do hacky +V_OFFSET etc. I found it hard to code around these in the existing transaction library without introducing more complexity to an already complex piece of code. In addition, the traditional encoding format (RLP) leaks in and out of functions within transactions, making it tricky to introduce a new encoding format.

Cute Animal Picture

To-Do

• Add tests

• Add entry to the release notes

[wolovim]

First, a big thank you for the work that's gone into this. I think all of us would-be reviewers are knee-deep in other issues at the moment, but someone will circle back as soon as we're able. These are nontrivial updates, so pinging @carver and @pipermerriam directly in case they want to take a peek.

[carver]

I took just a very quick look first, focusing on the API level, and it looks roughly like what I'd expect. Thanks for putting this together!

I'll do a deeper dive in a bit. The lint looks like an easy fix, just deleting the "type: ignore" comment.

[malonaz]

I tried deleting the three instances of 'type: ignore' comments, but it did not fix the issue.

[palkeo]

This PR would be the ideal foundation for adding the EIP-1559 transaction type.
I think it will be important to have before the London hard fork.

[lightclient]

@carver seems like the lint is failing on master. I fixed it in #116.

[kclowes]

I wasn't able to get to this today, but planning on doing a deep-dive review on Wednesday. Thanks for the PR @malonaz!

[kclowes]

I started going through this today - like others have said it looks good from a high level! I'll continue going though tomorrow. One nitpicky thing I noticed is that you use assert a lot for validation. We prefer to raise instead, since it's easy for users to turn off AssertionErrors. Thanks!

[malonaz]

I've switched to raise. thanks @kclowes

[kclowes]

Thanks for your patience @malonaz! I finished going through and doing some testing and this looks solid to me. I only have two comments:

• Do you mind adding either a note or example to the docstrings for sign_transaction and recover_transaction in eth_account/account.py to show usage for the 2930 access list type?
• I added one note in the PR about making an abstract class that will enforce that certain methods are present on any child Transaction classes. Let me know what you think or if that needs clarification!

[kclowes]

Thanks @malonaz, this is looking good to me! @carver - do you want to take another look?

[malonaz]

hey @kclowes / @carver any updates on this? Thanks

[kclowes]

@malonaz thank for the reminder! I have a few other things at the top of my priority queue but should get to it by Wednesday at the latest!

[kclowes]

Thanks @malonaz! I'll rebase and merge shortly!

[malonaz]

Awesome. Thanks

…

On Wed, 14 Jul 2021 at 21:24, kclowes ***@***.***> wrote: ***@***.**** approved this pull request. Thanks @malonaz <https://github.com/malonaz>! I'll rebase and merge shortly! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#115 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AEBJQ7R7YIJQOAJDIQFR5STTXXP6RANCNFSM46HKPETQ> .