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

Allow to set gas price directly for each transaction #1526

Open
hugjobk opened this issue Dec 15, 2023 · 2 comments
Open

Allow to set gas price directly for each transaction #1526

hugjobk opened this issue Dec 15, 2023 · 2 comments

Comments

@hugjobk
Copy link

hugjobk commented Dec 15, 2023

Suggest changes:

Give users more control over gas prices! Instead of locking them in the SigningStargateClientOptions, let them adjust it on the fly with signAndBroadcast and signAndBroadcastSync. No more client recreations just to tweak that gas!

Author: Doan Phi Hung <hugjobk@gmail.com>
Date:   Fri Dec 15 15:16:14 2023 +0700

    Allow to set gas price directly for each transaction

diff --git a/packages/stargate/src/index.ts b/packages/stargate/src/index.ts
index e424bed7c2..52ae6b0341 100644
--- a/packages/stargate/src/index.ts
+++ b/packages/stargate/src/index.ts
@@ -117,6 +117,7 @@ export { isSearchTxQueryArray, SearchPair, SearchTxQuery } from "./search";
 export {
   createDefaultAminoConverters,
   defaultRegistryTypes,
+  Fee,
   SignerData,
   SigningStargateClient,
   SigningStargateClientOptions,
diff --git a/packages/stargate/src/signingstargateclient.ts b/packages/stargate/src/signingstargateclient.ts
index a01817bc83..60e4cee8e8 100644
--- a/packages/stargate/src/signingstargateclient.ts
+++ b/packages/stargate/src/signingstargateclient.ts
@@ -63,6 +63,15 @@ export const defaultRegistryTypes: ReadonlyArray<[string, GeneratedType]> = [
   ...vestingTypes,
 ];
 
+export interface Fee {
+  readonly gas: "auto" | number;
+  readonly gasPrice: GasPrice;
+}
+
+function instanceofFee(object: any): boolean {
+  return typeof object === "object" && "gas" in object && "gasPrice" in object;
+}
+
 /**
  * Signing information for a single signer that is not included in the transaction.
  *
@@ -196,7 +205,7 @@ export class SigningStargateClient extends StargateClient {
     senderAddress: string,
     recipientAddress: string,
     amount: readonly Coin[],
-    fee: StdFee | "auto" | number,
+    fee: StdFee | "auto" | number | Fee,
     memo = "",
   ): Promise<DeliverTxResponse> {
     const sendMsg: MsgSendEncodeObject = {
@@ -214,7 +223,7 @@ export class SigningStargateClient extends StargateClient {
     delegatorAddress: string,
     validatorAddress: string,
     amount: Coin,
-    fee: StdFee | "auto" | number,
+    fee: StdFee | "auto" | number | Fee,
     memo = "",
   ): Promise<DeliverTxResponse> {
     const delegateMsg: MsgDelegateEncodeObject = {
@@ -232,7 +241,7 @@ export class SigningStargateClient extends StargateClient {
     delegatorAddress: string,
     validatorAddress: string,
     amount: Coin,
-    fee: StdFee | "auto" | number,
+    fee: StdFee | "auto" | number | Fee,
     memo = "",
   ): Promise<DeliverTxResponse> {
     const undelegateMsg: MsgUndelegateEncodeObject = {
@@ -249,7 +258,7 @@ export class SigningStargateClient extends StargateClient {
   public async withdrawRewards(
     delegatorAddress: string,
     validatorAddress: string,
-    fee: StdFee | "auto" | number,
+    fee: StdFee | "auto" | number | Fee,
     memo = "",
   ): Promise<DeliverTxResponse> {
     const withdrawMsg: MsgWithdrawDelegatorRewardEncodeObject = {
@@ -277,7 +286,7 @@ export class SigningStargateClient extends StargateClient {
     timeoutHeight: Height | undefined,
     /** timeout in seconds */
     timeoutTimestamp: number | undefined,
-    fee: StdFee | "auto" | number,
+    fee: StdFee | "auto" | number | Fee,
     memo = "",
   ): Promise<DeliverTxResponse> {
     const timeoutTimestampNanoseconds = timeoutTimestamp
@@ -301,20 +310,24 @@ export class SigningStargateClient extends StargateClient {
   public async signAndBroadcast(
     signerAddress: string,
     messages: readonly EncodeObject[],
-    fee: StdFee | "auto" | number,
+    fee: StdFee | "auto" | number | Fee,
     memo = "",
     timeoutHeight?: bigint,
   ): Promise<DeliverTxResponse> {
     let usedFee: StdFee;
-    if (fee == "auto" || typeof fee === "number") {
-      assertDefined(this.gasPrice, "Gas price must be set in the client options when auto gas is used.");
+    if (fee == "auto" || typeof fee === "number" || instanceofFee(fee)) {
+      // @ts-ignore
+      const gas = instanceofFee(fee) ? fee.gas : fee;
+      // @ts-ignore
+      const gasPrice = instanceofFee(fee) ? fee.gasPrice : this.gasPrice;
+      assertDefined(gasPrice, "Gas price must be set in the client options when auto gas is used.");
       const gasEstimation = await this.simulate(signerAddress, messages, memo);
       // Starting with Cosmos SDK 0.47, we see many cases in which 1.3 is not enough anymore
       // E.g. https://github.com/cosmos/cosmos-sdk/issues/16020
-      const multiplier = typeof fee === "number" ? fee : 1.4;
-      usedFee = calculateFee(Math.round(gasEstimation * multiplier), this.gasPrice);
+      const multiplier = typeof gas === "number" ? gas : 1.4;
+      usedFee = calculateFee(Math.round(gasEstimation * multiplier), gasPrice);
     } else {
-      usedFee = fee;
+      usedFee = fee as StdFee;
     }
     const txRaw = await this.sign(signerAddress, messages, usedFee, memo, undefined, timeoutHeight);
     const txBytes = TxRaw.encode(txRaw).finish();
@@ -330,18 +343,22 @@ export class SigningStargateClient extends StargateClient {
   public async signAndBroadcastSync(
     signerAddress: string,
     messages: readonly EncodeObject[],
-    fee: StdFee | "auto" | number,
+    fee: StdFee | "auto" | number | Fee,
     memo = "",
     timeoutHeight?: bigint,
   ): Promise<string> {
     let usedFee: StdFee;
-    if (fee == "auto" || typeof fee === "number") {
-      assertDefined(this.gasPrice, "Gas price must be set in the client options when auto gas is used.");
+    if (fee == "auto" || typeof fee === "number" || instanceofFee(fee)) {
+      // @ts-ignore
+      const gas = instanceofFee(fee) ? fee.gas : fee;
+      // @ts-ignore
+      const gasPrice = instanceofFee(fee) ? fee.gasPrice : this.gasPrice;
+      assertDefined(gasPrice, "Gas price must be set in the client options when auto gas is used.");
       const gasEstimation = await this.simulate(signerAddress, messages, memo);
-      const multiplier = typeof fee === "number" ? fee : 1.3;
-      usedFee = calculateFee(Math.round(gasEstimation * multiplier), this.gasPrice);
+      const multiplier = typeof gas === "number" ? gas : 1.3;
+      usedFee = calculateFee(Math.round(gasEstimation * multiplier), gasPrice);
     } else {
-      usedFee = fee;
+      usedFee = fee as StdFee;
     }
     const txRaw = await this.sign(signerAddress, messages, usedFee, memo, undefined, timeoutHeight);
     const txBytes = TxRaw.encode(txRaw).finish();
@webmaster128
Copy link
Member

hmm, this makes the API pretty complex. The whole idea of setting the gas price is that you don't need to set it per tx. Otherwise you can also just calculate the fee on demand.

Could you elaborate on the use case for this? Would a setGasPrice setter on the client do the job as well?

@hugjobk
Copy link
Author

hugjobk commented Dec 20, 2023

Currently, gas price is defined within the client options and cannot be dynamically adjusted based on market fluctuations. This limitation can be addressed by:

  • Introducing an optional gasPrice argument to the function: This allows specifying the gas price directly during function call, offering real-time adaptability.
  • Preserving fallback behavior: When the gasPrice argument is omitted, the existing gasPrice from the client options will be used seamlessly, ensuring backwards compatibility.
  • Prevents race conditions: Unlike the setGasPrice setter, setting the gas price within the function call avoids potential concurrency issues.

This feature is intended for scenarios where you want to set gas=auto and have the gasPrice automatically adjust in real-time based on current market conditions.

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

No branches or pull requests

2 participants