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

Gas limit parameter as integer in TxParams #2285

Closed
coccoinomane opened this issue Jan 2, 2022 · 4 comments
Closed

Gas limit parameter as integer in TxParams #2285

coccoinomane opened this issue Jan 2, 2022 · 4 comments
Labels
Breaking Change v6 Breaking changes that were considered for v6 (old)

Comments

@coccoinomane
Copy link
Contributor

coccoinomane commented Jan 2, 2022

  • Version: 5.25.0
  • Python: 3.9
  • OS: osx

What was wrong?

Hi there!
Shouldn't the gas parameter type in TxParams be an integer instead of Wei?
Here's the line.

How can it be fixed?

I am happy to do a pull request if you can confirm it is the case :-)

@stolpa4
Copy link

stolpa4 commented Jan 12, 2022

@coccoinomane Wei is just an alias for int, so it's not a problem, I think. Just use

from typing import cast

from web3.types import Wei

gas_limit: int = 1000

print(cast(Wei, gas_limit))

@coccoinomane
Copy link
Contributor Author

Hi @stolpa4 , thank you for your reply 🙂
I see how casting to Wei would do it... but would it not be simpler to just set gas_limit's type to int in TxParams?
It would be more natural, given that gas limit is a pure number. It is the gas price that carries the dimensionality (Wei).
Cheers,
Guido

@fselmo
Copy link
Collaborator

fselmo commented Jan 14, 2022

@coccoinomane This is definitely a good change. This would technically be a breaking change so we will likely wait to include it in the next major version release. Thanks for submitting!

@fselmo fselmo added Breaking Change v6 Breaking changes that were considered for v6 (old) labels Jan 14, 2022
@kclowes
Copy link
Collaborator

kclowes commented Feb 28, 2022

The fix has been merged! Thanks for the report @coccoinomane!

@kclowes kclowes closed this as completed Feb 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Breaking Change v6 Breaking changes that were considered for v6 (old)
Projects
None yet
Development

No branches or pull requests

4 participants