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

fix: fixing gas estimate on IBC transfers #1607

Merged
merged 2 commits into from
Feb 5, 2025

Conversation

pedrorezende
Copy link
Collaborator

This PR fixes the unit displayed and used to calculate if user has funds to make an IBC transfer.

@@ -132,14 +141,16 @@ export const TransferModule = ({
return undefined;
}

if (!gasConfig || gasConfig.gasToken !== selectedAssetAddress) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

The gasConfig.gasToken !== selectedAssetAddress is important because we can pay the gas with a different token and, if this is the case, we shouldn't subtract the gas from the availableAmount

Screenshot 2025-02-05 at 12 15 56

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good catch!

@@ -118,7 +115,7 @@ export const ShieldAllPanel = ({
<Stack as="footer" gap={4}>
<footer className="flex justify-between items-center">
<img src={ibcTransferImageBlack} className="w-20" />
{gasConfig && <TransactionFee gasConfig={gasConfig} />}
{/*gasConfig && <TransactionFee gasConfig={gasConfig} />*/}
Copy link
Collaborator

Choose a reason for hiding this comment

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

can we delete this and add a comment like // TODO add transaction fee?

@@ -54,10 +54,12 @@ const useBuildGasOption = ({
...override,
};

const displayAmount = getDisplayGasFee(option);
const displayGasFee = getDisplayGasFee(option, chainAssetsMap);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Because of the uosmo address from two channels, we have an intermittent case of showing the osmo fee as zero

But probably something to handle in a different PR

Screenshot 2025-02-05 at 11 58 52

@pedrorezende pedrorezende merged commit be1e7d7 into main Feb 5, 2025
7 checks passed
@pedrorezende pedrorezende deleted the feat/fixing-ibc-gas-estimate branch February 5, 2025 16:33
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.

2 participants