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

Wrapping current gas estimators with fetchable gas pricing #25

Merged
merged 6 commits into from Mar 1, 2022

Conversation

mtps
Copy link
Collaborator

@mtps mtps commented Mar 1, 2022

  • Break out logic for gas estimations into lambda parameters.
  • Associate legacy naming for parameters to newer lambdas.
  • Add floating gas price fetching
  • Remove fromSimulation(), logic moved into cosmosSimulation estimator.

Example Usage:

val priceGetter = UrlGasPrices("https://oracle.my.domain/gas-price").cached()
val estimator =
  if (provenance.network.version < 1.8) GasEstimationMethod.COSMOS_SIMULATION
  else GasEstimationMethod.MSG_FEE_CALCULATION
val pbClient = PbClient(
    chainId = "pio-testnet-1",
    channelUri = URI("grpcs://grpc.test.provenance.io"),
    gasEstimationMethod = floatingGasPrices(estimator, priceGetter)
)

pbClient.estimateTx(...)

@mtps mtps requested a review from vwagner as a code owner March 1, 2022 20:18
@mtps mtps requested review from afremuth-figure, arnabmitra, piercetrey-figure and jdfigure and removed request for vwagner March 1, 2022 20:18
Copy link
Collaborator

@vwagner vwagner left a comment

Choose a reason for hiding this comment

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

thanks for handling this

*/
open class UrlGasPrices(private val uri: String) : GasPrices {
private val client = HttpClientBuilder.create().build()
private val gson = Gson().newBuilder().create()
Copy link
Contributor

Choose a reason for hiding this comment

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

no jackson 😹

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

jackson wasn't in the current deps. gson was. 🤷

@@ -77,6 +80,12 @@ open class PbClient(
val upgradeClient = cosmos.upgrade.v1beta1.QueryGrpc.newBlockingStub(channel)
val wasmClient = cosmwasm.wasm.v1.QueryGrpc.newBlockingStub(channel)

// @TestnetFeaturePreview
Copy link
Collaborator

Choose a reason for hiding this comment

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

@mtps did you mean to leave this commented out?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I removed it in the spirit of quick iteration. I like the concept of guarding in this manner though.


override fun invoke(): CoinOuterClass.Coin {
val result = client.execute(HttpGet(uri))
require(result.statusLine.statusCode in 200..299) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

do we want to have a (optional, use if supplied) fallback default price to use in the event that the uri is unreachable?

Copy link
Collaborator Author

@mtps mtps Mar 1, 2022

Choose a reason for hiding this comment

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

cached values would still exist for the transaction (if caching is enabled). what would we fall back to? (semi-rhetorical question, i honestly don't know what a good "fallback" value would be).

@mtps mtps merged commit 7217d98 into main Mar 1, 2022
@mtps mtps deleted the mtps/wrapping-gas-estimator branch March 1, 2022 23:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants