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

Fix error message for LackOfFundForMaxFee #858

Merged
merged 6 commits into from
Nov 12, 2023
Merged

Conversation

bertmiller
Copy link
Contributor

Instead of returning the fee for a transaction, the LackOfFundForMaxFee error currently returns gas_limit. This PR fixes that.

@bertmiller bertmiller changed the title Fix error handling for LackOfFundForMaxFee Fix error message for LackOfFundForMaxFee Nov 10, 2023
@@ -180,7 +180,7 @@ pub enum InvalidTransaction {
RejectCallerWithCode,
/// Transaction account does not have enough amount of ether to cover transferred value and gas_limit*gas_price.
LackOfFundForMaxFee {
fee: u64,
fee: U256,
Copy link
Member

Choose a reason for hiding this comment

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

Could you make them as Box<U256> it will reduce the size of the type, and balance too?

Copy link
Member

@rakita rakita left a comment

Choose a reason for hiding this comment

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

lgtm, one nit if you are up to change it

@rakita
Copy link
Member

rakita commented Nov 12, 2023

After changing the field to the Box i sow that optimism downcasted fee to the u64 so this PR even fixes that

@rakita
Copy link
Member

rakita commented Nov 12, 2023

Will ignore fmt error, in local or CL logs it is not showing that anything is wrong

@rakita rakita merged commit 8e45250 into bluealloy:main Nov 12, 2023
7 of 8 checks passed
@github-actions github-actions bot mentioned this pull request Jan 12, 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.

2 participants