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: add EIP-4844 transaction when auto-mining #398

Merged
merged 43 commits into from
May 9, 2024
Merged

feat: add EIP-4844 transaction when auto-mining #398

merged 43 commits into from
May 9, 2024

Conversation

Wodann
Copy link
Collaborator

@Wodann Wodann commented Apr 22, 2024

Closes #341

tenchleb and others added 22 commits April 16, 2024 11:12
…on & library related typing to hardhat-core"

This reverts commit 18740ec.
* removed "export" from throwers,
* removed "throwOnInvalidLibrariesAddress" thrower,
* merged address resolution into primary for loop,
…nking

issues/4652:  added support for library linking in `hardhat-viem`
@Wodann Wodann self-assigned this Apr 22, 2024
@Wodann Wodann had a problem deploying to github-action-benchmark April 22, 2024 12:50 — with GitHub Actions Error
@Wodann Wodann had a problem deploying to github-action-benchmark April 22, 2024 12:57 — with GitHub Actions Error
@Wodann Wodann changed the title feat: add EIP-4844 feat: add EIP-4844 transaction when auto-mining Apr 22, 2024
@Wodann Wodann had a problem deploying to github-action-benchmark April 24, 2024 22:51 — with GitHub Actions Error
@Wodann Wodann requested a review from fvictorio April 24, 2024 22:52
@Wodann Wodann marked this pull request as ready for review April 25, 2024 15:04
@Wodann Wodann had a problem deploying to github-action-benchmark April 25, 2024 19:01 — with GitHub Actions Error
Copy link
Member

@fvictorio fvictorio left a comment

Choose a reason for hiding this comment

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

Very incomplete review, but so far it looks good to me. I also compared it with geth's behavior (e.g., how the excess blob gas and blob base fee are computed) and seems perfect.

I'll try to give it another look, but feel free to merge after Agost's approval. Also: if there's something else you would like me to double-check against geth, let me know.

.changeset/tough-students-compare.md Outdated Show resolved Hide resolved
.changeset/wicked-walls-itch.md Outdated Show resolved Hide resolved
crates/edr_provider/tests/eip4844.rs Outdated Show resolved Hide resolved
crates/edr_provider/tests/eip4844.rs Show resolved Hide resolved
crates/edr_provider/tests/eip4844.rs Outdated Show resolved Hide resolved
crates/edr_provider/src/error.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@agostbiro agostbiro left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM!

@Wodann Wodann requested a review from fvictorio May 7, 2024 14:14
Copy link
Member

@fvictorio fvictorio left a comment

Choose a reason for hiding this comment

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

Tbh something I don't quite like is that the same error is shown for eth_call and eth_estimateGas, which might be confusing for users. In this particular case I don't mind a lot, since it's a somewhat niche use case, but AFAICT this is also the case for other validations. We might want to improve this in the future, but it's fine for now.

Also: I fixed a merge conflict. I think it's pretty straightforward, but take a look just in case.

@fvictorio fvictorio temporarily deployed to github-action-benchmark May 8, 2024 08:53 — with GitHub Actions Inactive
@Wodann Wodann temporarily deployed to github-action-benchmark May 9, 2024 05:09 — with GitHub Actions Inactive
@Wodann Wodann merged commit a4d5b62 into main May 9, 2024
32 checks passed
@Wodann Wodann deleted the feat/eip4844 branch May 9, 2024 06:02
@Wodann Wodann added this to the EDR v0.3.8 milestone May 21, 2024
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.

Add support for raw blob txs in automine mode
6 participants