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: set tx types gas_limit to u128 #9571

Closed
wants to merge 3 commits into from

Conversation

leruaa
Copy link
Contributor

@leruaa leruaa commented Jul 17, 2024

This PR is a prerequisite for #9484, as gas_limit is u128 on alloy tx types

@leruaa leruaa requested a review from fgimenez as a code owner July 17, 2024 10:46
Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

confirming @joshieDo that we can't do this due to how compact behaves for u128 vs u64 and we need to cast when we do the compact bridge which should be fine because we don't expect > u64 values anyway

@@ -24,7 +24,7 @@ pub struct TxEip1559 {
/// this transaction. This is paid up-front, before any
/// computation is done and may not be increased
/// later; formally Tg.
pub gas_limit: u64,
pub gas_limit: u128,
Copy link
Collaborator

Choose a reason for hiding this comment

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

we can't do that here because the codec is slightly different for u128

instead we need to cast these fields when we implement the codec roundtrip

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah exactly

@mattsse mattsse closed this Jul 17, 2024
@mattsse
Copy link
Collaborator

mattsse commented Jul 17, 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.

3 participants