Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
94 changes: 83 additions & 11 deletions packages/cosmwasm/src/signingcosmwasmclient.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { encodeSecp256k1Pubkey, makeSignDoc as makeSignDocAmino } from "@cosmjs/amino";
import { sha256 } from "@cosmjs/crypto";
import { fromBase64, toHex, toUtf8 } from "@cosmjs/encoding";
import { Int53, Uint53 } from "@cosmjs/math";
import { Decimal, Int53, Uint53 } from "@cosmjs/math";
import {
EncodeObject,
encodePubkey,
Expand All @@ -20,6 +20,7 @@
createDefaultAminoConverters,
defaultRegistryTypes as defaultStargateTypes,
DeliverTxResponse,
DynamicGasPriceConfig,
Event,
GasPrice,
isDeliverTxFailure,
Expand All @@ -28,6 +29,8 @@
MsgSendEncodeObject,
MsgUndelegateEncodeObject,
MsgWithdrawDelegatorRewardEncodeObject,
multiplyDecimalByNumber,
queryDynamicGasPrice,
SignerData,
StdFee,
} from "@cosmjs/stargate";
Expand Down Expand Up @@ -190,7 +193,8 @@
readonly aminoTypes?: AminoTypes;
readonly broadcastTimeoutMs?: number;
readonly broadcastPollIntervalMs?: number;
readonly gasPrice?: GasPrice;
/** Gas price configuration. Can be a static GasPrice or a DynamicGasPriceConfig for dynamic pricing. */
readonly gasPrice?: GasPrice | DynamicGasPriceConfig;
}

export class SigningCosmWasmClient extends CosmWasmClient {
Expand All @@ -200,10 +204,12 @@

private readonly signer: OfflineSigner;
private readonly aminoTypes: AminoTypes;
private readonly gasPrice: GasPrice | undefined;
private readonly gasPrice: GasPrice | DynamicGasPriceConfig | undefined;
// 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
private readonly defaultGasMultiplier = 1.4;
// Default multiplier for dynamic gas price (applied on top of queried price)
private readonly defaultDynamicGasMultiplier = 1.3;
Copy link
Member

Choose a reason for hiding this comment

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

This is a gas price multiplier, not a gas multiplier, right? I think it is important to highlight this e.g. by renaming

Suggested change
private readonly defaultDynamicGasMultiplier = 1.3;
private readonly defaultDynamicGasPriceMultiplier = 1.3;


/**
* Creates an instance by connecting to the given CometBFT RPC endpoint.
Expand Down Expand Up @@ -615,10 +621,7 @@
): 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.");
const gasEstimation = await this.simulate(signerAddress, messages, memo);
const multiplier = typeof fee === "number" ? fee : this.defaultGasMultiplier;
usedFee = calculateFee(Math.round(gasEstimation * multiplier), this.gasPrice);
usedFee = await this.calculateFeeForTransaction(signerAddress, messages, memo, fee);
} else {
usedFee = fee;
}
Expand Down Expand Up @@ -651,10 +654,7 @@
): 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.");
const gasEstimation = await this.simulate(signerAddress, messages, memo);
const multiplier = typeof fee === "number" ? fee : this.defaultGasMultiplier;
usedFee = calculateFee(Math.round(gasEstimation * multiplier), this.gasPrice);
usedFee = await this.calculateFeeForTransaction(signerAddress, messages, memo, fee);
} else {
usedFee = fee;
}
Expand All @@ -663,6 +663,78 @@
return this.broadcastTxSync(txBytes);
}

private async calculateFeeForTransaction(
signerAddress: string,
messages: readonly EncodeObject[],
memo: string,
fee: "auto" | number,
): Promise<StdFee> {
const gasEstimation = await this.simulate(signerAddress, messages, memo);
const multiplier = typeof fee === "number" ? fee : this.defaultGasMultiplier;
Copy link
Member

Choose a reason for hiding this comment

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

At this point we know the gas limit, right? Everything else below is just about the price.

Adding

const gasLimit = Math.ceil(gasEstimation * multiplier)

here would remove the 4x Math.round(gasEstimation * multiplier) from below and increase readability.


const gasPriceConfig = this.gasPrice;

// Check if gasPrice is dynamic config or static GasPrice
if (gasPriceConfig && "minGasPrice" in gasPriceConfig) {
// Dynamic gas price config
const dynamicGasConfig = gasPriceConfig;
const multiplierValue = dynamicGasConfig.multiplier ?? this.defaultDynamicGasMultiplier;
const minGasPrice = dynamicGasConfig.minGasPrice;
const maxGasPrice = dynamicGasConfig.maxGasPrice;

try {
const chainId = await this.getChainId();
const queryClient = this.forceGetQueryClient();
const dynamicGasPriceDecimal = await queryDynamicGasPrice(
queryClient,
dynamicGasConfig.denom,
chainId,
);

// Multiply by multiplier 18 fractional digits for Dec type
const fractionalDigits = minGasPrice.amount.fractionalDigits;
const adjustedGasPrice = multiplyDecimalByNumber(
dynamicGasPriceDecimal,
multiplierValue,
fractionalDigits,
);

// Apply min and max constraints using comparison methods
let finalGasPrice = adjustedGasPrice.isGreaterThan(minGasPrice.amount)
? adjustedGasPrice
: minGasPrice.amount;
if (maxGasPrice) {
// Normalize maxGasPrice to same fractional digits if needed (user might create it differently)
const normalizedMaxGasPrice =
maxGasPrice.amount.fractionalDigits === fractionalDigits
? maxGasPrice.amount
: Decimal.fromUserInput(maxGasPrice.amount.toString(), fractionalDigits);
finalGasPrice = finalGasPrice.isLessThan(normalizedMaxGasPrice)
? finalGasPrice
: normalizedMaxGasPrice;
}
const dynamicGasPriceObj = new GasPrice(finalGasPrice, dynamicGasConfig.denom);
return calculateFee(Math.round(gasEstimation * multiplier), dynamicGasPriceObj);
} catch (error) {

Check warning on line 718 in packages/cosmwasm/src/signingcosmwasmclient.ts

View workflow job for this annotation

GitHub Actions / lint

'error' is defined but never used. Allowed unused caught errors must match /^_/u
// Fallback to minGasPrice if query fails
return calculateFee(Math.round(gasEstimation * multiplier), minGasPrice);
}
} else {
// Static gas price
if (!gasPriceConfig) {
throw new Error("Gas price must be set in the client options when auto gas is used.");
}
if (
!(gasPriceConfig instanceof GasPrice) &&
!("amount" in gasPriceConfig && "denom" in gasPriceConfig)
) {
throw new Error("Gas price must be a GasPrice instance when using static pricing.");
}
const staticGasPrice = gasPriceConfig as GasPrice;

Check failure on line 733 in packages/cosmwasm/src/signingcosmwasmclient.ts

View workflow job for this annotation

GitHub Actions / lint

This assertion is unnecessary since it does not change the type of the expression
return calculateFee(Math.round(gasEstimation * multiplier), staticGasPrice);
}
}

public async sign(
signerAddress: string,
messages: readonly EncodeObject[],
Expand Down
Loading
Loading