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

WT-1734 Add fees to swap and bridge routes #944

Merged
merged 5 commits into from
Oct 6, 2023
Merged

WT-1734 Add fees to swap and bridge routes #944

merged 5 commits into from
Oct 6, 2023

Conversation

imx-mikhala
Copy link
Contributor

@imx-mikhala imx-mikhala commented Oct 5, 2023

Summary

Adds the fees to the swap and bridge funding steps
Removed the total fees at top level as fees can be in various tokens (even if all currently native) so different token fees cant really be aggregated into one value and doesn't make sense for the on-ramp route. Think it makes more sense to just have the fees associated with the funding step.

@imx-mikhala imx-mikhala requested a review from a team as a code owner October 5, 2023 05:29
@@ -450,34 +450,22 @@ export type NoRouteOptions = {
* Represents a funding route
* @property {number} priority - The priority of the route
* @property {FundingStep[]} steps - The steps associated with this funding route
* @property {TotalFees | undefined} totalFees - The total fees for this route
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Think it makes more sense to just leave fees at the funding step level. Fees can in theory be various tokens and so they would need to be aggregated by token. Think it makes more sense to have only at the funding step level, especially because onramp cant even have one of these fee fields

@imx-mikhala imx-mikhala merged commit 36855bf into main Oct 6, 2023
6 checks passed
@imx-mikhala imx-mikhala deleted the WT-1734 branch October 6, 2023 00:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants