Skip to content
This repository has been archived by the owner on Oct 19, 2024. It is now read-only.

fix: wrong unit for gas_price (ether -> gwei) #1316

Merged
merged 1 commit into from
Jun 2, 2022
Merged

fix: wrong unit for gas_price (ether -> gwei) #1316

merged 1 commit into from
Jun 2, 2022

Conversation

naiba
Copy link
Contributor

@naiba naiba commented May 27, 2022

No description provided.

Copy link
Owner

@gakonst gakonst left a comment

Choose a reason for hiding this comment

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

@recmo wdyt

@gakonst
Copy link
Owner

gakonst commented Jun 1, 2022

@naiba can we please add a unit test for this, explaining why it was wrong before? realize this is a bit pedantic- appreciate the patience

@naiba
Copy link
Contributor Author

naiba commented Jun 1, 2022

@naiba can we please add a unit test for this, explaining why it was wrong before? realize this is a bit pedantic- appreciate the patience

Gas fees are dynamic, and if writing unit tests we can only assume gas fees on the polygon chain will never go below and above a certain value, which is kind of useless.

The following two links are descriptions of gas fee units. I believe you have already understood the unit conversion from gwei to wei.

https://www.google.com/search?q=ethereum+gas+price+unit

https://docs.polygon.technology/docs/develop/tools/polygon-gas-station/

Note this text: {'safelow', 'standard', 'fast', 'estimatedBaseFee'} are gas prices in GWei

Copy link
Owner

@gakonst gakonst left a comment

Choose a reason for hiding this comment

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

ok this seems fine - just realized that polygon returns gwei and not wei - cc @recmo

@gakonst gakonst merged commit 214d24d into gakonst:master Jun 2, 2022
@naiba naiba deleted the patch-1 branch June 2, 2022 15:59
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants