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

Move the weighted gas price strategy under web3.tools #1641

Open
pipermerriam opened this issue May 7, 2020 · 3 comments
Open

Move the weighted gas price strategy under web3.tools #1641

pipermerriam opened this issue May 7, 2020 · 3 comments

Comments

@pipermerriam
Copy link
Member

related to #1463

What was wrong?

I believe that the weighted gas price strategy should be deprecated or moved to live under web3.tools. This is based on the high subjectivity of gas prices which I believe means that we will be unable to supply gas price strategies that work well across the many disparate use cases that Web3.py is intended for.

How can it be fixed?

The most extreme approach would be to deprecate and remove it. I'm not convinced this is the right choice.

I believe that the following course of action would be appropriate.

  1. Move the weighted gas price strategy to live somewhere under web3.tools. This serves to designate this as not being part of the core library API, and thus to not be subject to as stringent of requirements for things like breaking changes.
  2. Review our documentation around gas price strategies. Make sure that it is sufficiently informative so that someone could write their own gas pricing strategy that would serve their own needs.
@evowilliamson
Copy link

@pipermerriam Could you please assign this issue to me please ?

@pipermerriam
Copy link
Member Author

@evowilliamson we don't really use assignees as part of our workflow. If you'd like to work on this you are welcome to simply open a pull request and mention this issue in the PR to link them together. Doing that is completely adequate for claiming an issue.

@evowilliamson
Copy link

evowilliamson commented May 21, 2020

@pipermerriam,

In his comment #1614 (review), @kclowes was suggesting refactoring the time_based_gas_price_strategy strategy, which is currently used both for the time and weighted strategy logic, into its own entity. Do you envision this issue (#1641) to be only effecting the weighted part of that strategy (kwarg weighted + code), or do you want to move the whole function to the web3.tools package and refactor it perhaps into two different strategies?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants