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: move calculate_intrinsic_gas_after_merge to tx pool #8914

Merged
merged 10 commits into from
Jun 19, 2024

Conversation

kamuik16
Copy link
Contributor

Closes #8899

@kamuik16
Copy link
Contributor Author

revm is not used in reth-primitives now, so giving a warning of unused crates but some features use it as a dep and removing revm from features seems to break some things. used allow for now to pass ci.

@DaniPopes
Copy link
Member

What's breaking ? Ideally we want to remove revm from here, is it maybe usage in tests?

@kamuik16
Copy link
Contributor Author

kamuik16 commented Jun 18, 2024

What's breaking ? Ideally we want to remove revm from here, is it maybe usage in tests?

I tried to do it in 82b569d. Checkout its CI failures.

By mistake closed the PR. sorry for that.

@kamuik16 kamuik16 closed this Jun 18, 2024
@kamuik16 kamuik16 reopened this Jun 18, 2024
@DaniPopes
Copy link
Member

DaniPopes commented Jun 18, 2024

Oh, I think you just need to propagate the optimism feature to revm-primitives instead of revm

optimism = [
"reth-chainspec/optimism",
"reth-codecs/optimism",
"reth-ethereum-forks/optimism",
"revm/optimism",
]

@kamuik16
Copy link
Contributor Author

Oh, I think you just need to propagate the optimism feature to revm-primitives instead of revm

optimism = [
"reth-chainspec/optimism",
"reth-codecs/optimism",
"reth-ethereum-forks/optimism",
"revm/optimism",
]

Got a different error now.

@onbjerg
Copy link
Member

onbjerg commented Jun 18, 2024

revm is not used in reth-primitives now, so giving a warning of unused crates but some features use it as a dep and removing revm from features seems to break some things. used allow for now to pass ci.

mark it as optional, and then use dep:revm in the features that use it

@onbjerg onbjerg added the C-enhancement New feature or request label Jun 18, 2024
@kamuik16
Copy link
Contributor Author

revm is not used in reth-primitives now, so giving a warning of unused crates but some features use it as a dep and removing revm from features seems to break some things. used allow for now to pass ci.

mark it as optional, and then use dep:revm in the features that use it

still giving me a warning that revm is an unused crate, i tried this earlier too.

@DaniPopes DaniPopes enabled auto-merge June 18, 2024 11:27
@shekhirin shekhirin disabled auto-merge June 19, 2024 11:29
@shekhirin shekhirin added C-debt Refactor of code section that is hard to understand or maintain and removed C-enhancement New feature or request labels Jun 19, 2024
@mattsse mattsse enabled auto-merge June 19, 2024 12:09
@mattsse mattsse added this pull request to the merge queue Jun 19, 2024
Merged via the queue into paradigmxyz:main with commit a3fd112 Jun 19, 2024
31 checks passed
@kamuik16 kamuik16 deleted the kamuik16/8899 branch June 19, 2024 12:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-debt Refactor of code section that is hard to understand or maintain
Projects
None yet
Development

Successfully merging this pull request may close these issues.

move calculate_intrinsic_gas_after_merge to txpool eth validator
5 participants