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 BSC bridges #5855

Merged
merged 4 commits into from
Jun 17, 2024
Merged

Fix BSC bridges #5855

merged 4 commits into from
Jun 17, 2024

Conversation

walmat
Copy link
Contributor

@walmat walmat commented Jun 17, 2024

Fixes APP-1598

What changed (plus any additional context for devs)

Screen recordings / screenshots

What to test

Copy link

linear bot commented Jun 17, 2024

Comment on lines +339 to +340
buyAmountDisplay: isNativeWrapOrUnwrap ? quoteData.buyAmount : quoteData.buyAmountDisplay,
sellAmountDisplay: isNativeWrapOrUnwrap ? quoteData.sellAmount : quoteData.sellAmountDisplay,
Copy link
Member

Choose a reason for hiding this comment

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

what's the logic for overriding these two? I don't think it's necessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was done on the BX so figured it was necessary. If it's not I can remove it

...quoteData,
buyAmountDisplay: isNativeWrapOrUnwrap ? quoteData.buyAmount : quoteData.buyAmountDisplay,
sellAmountDisplay: isNativeWrapOrUnwrap ? quoteData.sellAmount : quoteData.sellAmountDisplay,
feeInEth: isNativeWrapOrUnwrap ? '0' : quoteData.feeInEth,
Copy link
Member

Choose a reason for hiding this comment

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

Also this. The SDK should handle it internally

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also brought this over from the BX

Copy link
Member

@brunobar79 brunobar79 left a comment

Choose a reason for hiding this comment

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

LGTM. Just tested this, working fine.

@walmat walmat merged commit 0e8bf36 into develop Jun 17, 2024
6 checks passed
@walmat walmat deleted the @matthew/APP-1598 branch June 17, 2024 18:59
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