-
Notifications
You must be signed in to change notification settings - Fork 191
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
feat: decouple network fees from quote #8432
Conversation
src/components/MultiHopTrade/components/LimitOrder/hooks/useAllowanceApproval.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First code pass, pending functional review
src/components/MultiHopTrade/components/LimitOrder/hooks/useAllowanceApproval.tsx
Outdated
Show resolved
Hide resolved
...nts/MultiHopTrade/components/MultiHopTradeConfirm/hooks/useTradeNetworkFeeCryptoBaseUnit.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Smoke test
https://jam.dev/c/3d533484-3638-4c92-b0be-9df32be54ada
Looks good so far, couldn't notice anything super weird
Stamping it as I think most of the comments are not blockers but questions
Description
Note: This is exclusively tackling network fees estimate for current hop, and is used within the new trade flow components.
This PR refetches network fees at interval for current hop, using similar logic as
getUnsignedXyzTransaction
to get calldata and estimate network fees from that.TODO:
useTradeNetworkFeesCryptoBaseUnit
instead of colocated with tradeExecution<TradeConfirmFooter />
remove from final quote (maybe?)probably not (at least not now)Issue (if applicable)
closes #8270
Risk
Low - new trade flow only, not in prod yet. Though, if we consider flagged paths as production paths, this should be considered medium risk, as displayed fees could be off, and Txs could fail as a result.
Testing
Engineering
Operations
Screenshots (if applicable)
https://jam.dev/c/9cf1706f-53d3-46d0-9a4c-cafd5f494462