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

[Feature Request]: configurable gas price #30

Closed
tuxcanfly opened this issue Jan 9, 2024 · 0 comments · Fixed by #31
Closed

[Feature Request]: configurable gas price #30

tuxcanfly opened this issue Jan 9, 2024 · 0 comments · Fixed by #31
Labels
enhancement New feature or request

Comments

@tuxcanfly
Copy link
Contributor

tuxcanfly commented Jan 9, 2024

Implementation ideas

The latest release v0.0.1 allows users to pass in SubmitOptions which include GasLimit and Fee. Individually they are not very useful because both depend on blob size of the transaction they're submitting. If the client configures a fixed GasLimit and Fee, they risk either not getting their transaction processed for larger blobs or overpaying for smaller blobs.

Instead it would be better to allow users to configure a GasPrice per transaction so that GasLimit and Fee can be derived from it based on the blob size.

So, there should be a new minor version bump release which replaces *SubmitOptions with GasPrice float64. If the GasPrice is less than zero, the DA server can default to DefaultSubmitOptions.

@tuxcanfly tuxcanfly added the enhancement New feature or request label Jan 9, 2024
distractedm1nd added a commit to celestiaorg/celestia-node that referenced this issue Jan 23, 2024
This PR modifies the blob interface to replace SubmitOptions with
GasLimit. The reasoning here is that setting a gas price is a lot more
intuitive/easier than calculating the fee and gas limit, which take
cosmossdk and celestia-app imports to calculate without much effort.
Related: rollkit/go-da#30

Based on a suggestion from @vgonkivs we alias float64 to be able to
provide a default value, but now that its there I think we can just have
float64 and the same constructor, as it doesn't change that much.

Breaks the API, cc @jcstein, @tuxcanfly, and @Ferret-san .

Closes #3053.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

1 participant