Skip to content
This repository has been archived by the owner on Jan 22, 2025. It is now read-only.

Cudos 842: Extend gas fees handling #101

Merged
merged 10 commits into from
Jul 19, 2022
Merged

Cudos 842: Extend gas fees handling #101

merged 10 commits into from
Jul 19, 2022

Conversation

aemil145
Copy link
Contributor

  • Introduced gas limit and gas multiplier
  • updated documentation to reflect changes

Other fixes:

  • gitignore child packages' pachage-jock.json as they are not used
  • added a more readable BlastError in case the user calls Blast commands from wrong cwd

@aemil145 aemil145 requested review from tgntr, Holymir and maptuhec June 22, 2022 11:02
Copy link

@Holymir Holymir left a comment

Choose a reason for hiding this comment

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

I think that we will have to have the option for gasLimit to be set in every transaction as a optional param.

}
return gasFee
if (gasLimit === GAS_AUTO) {
return gasMultiplier === GAS_AUTO ? GAS_AUTO : gasMultiplier
Copy link
Contributor

Choose a reason for hiding this comment

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

can you skip this if statement and put it in row 19? I thinkg that this whole logic could be made simpler

const signer = await SigningCosmWasmClient.connectWithSigner(nodeUrl, wallet)
// gasPrice in signing client is considered only when auto gas is used
const signer = await SigningCosmWasmClient.connectWithSigner(
nodeUrl, wallet, { gasPrice: GasPrice.fromString(getGasPrice()) })
Copy link
Contributor

Choose a reason for hiding this comment

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

didn't we discuss to remove the gas price from the signer?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The cosmwasm lib requires the signing client to have gas price when auto gas is used (and actually reads it from there to make its calculations)

@aemil145 aemil145 merged commit 57a9ad8 into cudos-dev Jul 19, 2022
@aemil145 aemil145 deleted the CUDOS-842 branch July 19, 2022 14:14
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants