-
Notifications
You must be signed in to change notification settings - Fork 12
Add option to use ethgasstation.info for gas price estimation #1
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
Add option to use ethgasstation.info for gas price estimation #1
Conversation
|
Hello @alexandre-abrioux ! Thank you very much for your PR. Testing with mocha is very welcome too. The basic algorithm of using the Consider the following idea:
Let me know what you think about this design, and if you need more clarification or help with the code. Thanks again |
|
Hello @tuler, thanks for your review, this makes sense! I'll find some time during the week to try and implement those changes and I'll get back to you :) |
|
@tuler Just finished implementing the requested changes, it feels much cleaner now! Tell me what you think about it. |
tuler
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks great! Thanks for the changes.
Can you include the copyright header to all added files?
|
Added the copyright headers! |
src/block.ts
Outdated
| const nonce = pos.signer.getTransactionCount("latest"); | ||
| const gasPriceProvider = createGasPriceProvider( | ||
| pos.provider, | ||
| chainId |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there is a confusion here. The chainId in this context is the sidechain chainId, which starts with 0 and goes on as we can create additional chains in the future (for several reasons). It is not the Ethereum chain (mainnet, goerli, Rinkeby, etc).
It's our fault we are using the name chainId and causing this confusion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the mix-up, I just fixed that in a new commit. I kept the GAS_STATION_API_CHAIN_ID config that I introduced, I think it is still relevant, but it is now checked against the provider chainId in the createGasPriceProvider function.
|
Hello @alexandre-abrioux , as described in our contributing guidelines I'd like to ask you to sign the contributing agreement that is there and send over to [email protected]. Thank you again! |
|
@tuler I've filled up the Google Form "Enter E-mail Below to Request DocuSign". Is that OK? I haven't received anything yet. |
That's it. We'll send you the doc soon. Thank you! |
|
OK thanks :) |
|
Hi again! I don't know if you got back the signed document from DocuSign, please tell me if I can help. |
|
Hey @alexandre-abrioux , we got it, thanks! |
|
Your contribution has been integrated to the codebase through a rebase along with other changes. |
|
Nice! Thanks. |
Hi!
First of all, thanks to the team for creating and developing this amazing project.
I've been running a
noethernode for almost a month now and I've suffered several failed transaction due to high volatility/fluctuation of Ethereum gas price.During those hiccups the gas price set by Infura would be too low for transactions to be processed quickly enough by Ethereum network to fit in the block producing eligibility window.
Some transactions (not linked for privacy but can do through private channel if needed) would take up to an hour to get validated by Ethereum network and, once approved, would be refused by the contract for producing new blocks (as intended as they were delayed too much).
The solution I am bringing to the code base is to fetch the gas price from ethgasstation.info before sending the transaction. I tested it for several weeks now and found the best way to ensure no transaction fails is to use the
fastgas price provided by the service, as I got failed transactions with both thecheapandaveragegas price profiles.The API URL is configurable (
GAS_STATION_API_URL) so that a similar services or privately hosted stations could be used.A timeout (
GAS_STATION_API_REQUEST_TIMEOUT_MS), set to 10 seconds by default, ensures that the node produces new blocks in case the API becomes unreachable.The whole "fetch gas from gas station" feature can also be disabled in the config (
GAS_STATION_API_ENABLED).The
GAS_MULTIPLIERconfig has been split into two different constants:GAS_LIMIT_MULTIPLIERGAS_PRICE_MULTIPLIER; so that the gas price multiplier could be configured independently from the gas limit multiplier.
The gas price multiplier is used when the gas station feature is disabled or when the API is unreachable.
The gas station profile (
fastby default) is also configurable (GAS_STATION_API_PROFILE).The gas price is fetched only for the
GAS_STATION_API_CHAIN_ID(1by default). So for other chains it uses Infura gas price along with theGAS_PRICE_MULTIPLIER.The developped code is tested with
mochaso I had to add some newdevdependencies:mocha,chai,sinonfor testing ;moxiosto easily mockaxios;waffleto mock the provider.I had to add
gitin theDockerfilebecause some dependencies of the new packages can only be fetched throughgit.I also added a
tsconfig.prod.jsonto prevent building test files in the production build.If this PR gets accepted we could in the future add those tests to the CI pipeline.
I hope my explanations were clear enough, please tell me if you have any question or you would like to see more improvement to the work I've done. Thanks!