Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(ethereum): < -adds new eth tx type #15

Merged
merged 3 commits into from
Jan 30, 2024
Merged

feat(ethereum): < -adds new eth tx type #15

merged 3 commits into from
Jan 30, 2024

Conversation

gskapka
Copy link
Contributor

@gskapka gskapka commented Jan 30, 2024

...per title.

The up and coming dencun fork implements EIP4844 which introduces a new receipt type for blob-carrying transactions.

This PR allows parsing of such transactions, and includes a new test with a goerli block containing the new type of txs.

@gskapka gskapka self-assigned this Jan 30, 2024
@gskapka gskapka requested a review from Oghma January 30, 2024 12:09
@Oghma
Copy link
Contributor

Oghma commented Jan 30, 2024

LGTM!

Some notes:

  • This PR only updates the transaction type, block validation is still missing. Sentinel cannot validate a block an EIP4844 block.
  • Reviewing the PR, I noticed that type 2 transactions are mislabelled. They should be EIP1559 and not EIP2718. The latter, introduces the field TransactionType. EIP1559 introduces type 2 transactions with new fees https://eips.ethereum.org/EIPS/eip-1559

@gskapka
Copy link
Contributor Author

gskapka commented Jan 30, 2024

Noted re point one. That will come in a following PR.

Re point two - fixing now, & will tag for re-review once ready.

@gskapka
Copy link
Contributor Author

gskapka commented Jan 30, 2024

Updated. Note that I had to update one of the submission materials - I'm not sure why but the original one for an eth mainnet block but where the type field in a receipt was a string: EIP2718. Re-pulling that submission material now shows the correct field: type: "0x2", which parses correctly with the updated ReceiptTypes enum, and so I've replaced that submission material test sample.

Updated to include backwards compatible bug fix for the wrong EthReceiptType.

@gskapka gskapka force-pushed the new-eth-tx-type branch 2 times, most recently from 005456c to f0a2950 Compare January 30, 2024 16:07
Copy link
Contributor

@Oghma Oghma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

common/ethereum/src/eth_receipt_type.rs Outdated Show resolved Hide resolved
common/ethereum/src/eth_receipt_type.rs Outdated Show resolved Hide resolved
@gskapka gskapka merged commit cb0a15e into master Jan 30, 2024
5 checks passed
@gskapka gskapka deleted the new-eth-tx-type branch January 30, 2024 17:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants