search

cowprotocol / cow-sdk

CoW protocol SDK
https://docs.cow.fi/cow-protocol/reference/sdks/cow-sdk
Other
31 stars 9 forks source link

feat: utility to calculate quote amounts and costs #210

Closed shoom3301 closed 3 months ago

shoom3301 commented 3 months ago

This is a refactored and improved version of:

  1. https://github.com/cowprotocol/cowswap/blob/0132c63db04a605826df6086480613faacacff97/apps/cowswap-frontend/src/legacy/state/swap/TradeGp.ts
  2. https://github.com/cowprotocol/cowswap/blob/f0bf8cbc57f4983de31929b848bd544d0a47294a/apps/cowswap-frontend/src/legacy/state/swap/extension.ts

Goals of the refactoring:

  1. Reduce count of inputs and abstractions. The only source of data should be OrderParameters which is a DTO from /quote API.
  2. Replace *withFee / *withoutFee with *afterFees / *beforeFees. Depending on sell/buy order we substract/add network costs to the amounts and it's not super clear what with/without means.
  3. Differentiate network costs and partner fees. Currently, network costs are named as feeAmount in OrderParameters which is not super clear.
  4. Get rid of Uniswap abstractions in favor of native bigint.
  5. Share and reuse the utility in CoW Swap and Explorer (at least).
coveralls commented 3 months ago

Coverage Status

coverage: 79.56% (+0.8%) from 78.809% when pulling 60dcedffd0a8c289a07be7183f2d126631cfb1fe on feat/quote-and-amounts-costs into 16155d179fae966953a937ca705ebabd05da3ef5 on main.

socket-security[bot] commented 3 months ago

No dependency changes detected. Learn more about Socket for GitHub ↗︎

👍 No dependency changes detected in pull request

alfetopito commented 3 months ago

Soft approve. I agree with leandro that the price being a number is error prone. As a financial app is better to not have rounding issues

Approved, but keep in mind that even after removing the price from the response, the calculation still uses native number for deriving the returned values.