Chore: add estimatedpriceimpact to quote report [TKR-440]

[idokleinman]

Description

add optional estimatedPriceImpact to quote report for data team improved slippage model

Testing Instructions

Checklist

• [ ] Update documentation as needed. Website Documentation PR:

• [ ] Prefix PR title with [WIP] if necessary.

• [ ] Add tests to cover changes as needed.

• [ ] Test changes on the staging environment with Matcha API Staging.

  • [ ] SRA/Limit orders
  • [ ] Swap endpoints
  • [ ] Meta transaction endpoints
  • [ ] Depth charts

  For more information see 0x API Matcha smoke test runbook in Quip.

[robpal1990]

Will this return the price impact of each source in the quote? It seems like the API calculates it in _calculateEstimatedPriceImpact ( swap_service.ts ) which does not go source by source, only aggregated number.

Overall, we'd like to get a number per each quote report entry, not just per header. This probably requires more digging.

[idokleinman]

Indeed this is just the aggregated number. The whole QuoteReport entry seems to be referencing to the entire quote vs specific sources. The only point where there's a per-source reference is in quoteReportSources but including priceImpact there is currently not how the code/data is structured. I see fillData for each source is being passed on - maybe that calculation can be done downstream on your side?

[robpal1990]

Indeed this is just the aggregated number. The whole QuoteReport entry seems to be referencing to the entire quote vs specific sources. The only point where there's a per-source reference is in quoteReportSources but including priceImpact there is currently not how the code/data is structured. I see fillData for each source is being passed on - maybe that calculation can be done downstream on your side?

My assumption is you'd need to pull it from the internals of asset-swapper (which would source it from the sampler). fillData is not enough I think, it's only the token paths/amounts, not liquidity -- correct me if I'm wrong. Also -- does it currently include the pool fee? (that fee would be a yet another useful info in the quote).

Tbh, I think the datapoint we need is currently not available and requires a refactor by the codebase owner. However if we ever want to use it in production in the future as an input to a model, we will need to be able to serve this datapoint during the quoting process, so it will need a restructuring anyway.

[zhehao-0x]

Indeed this is just the aggregated number. The whole QuoteReport entry seems to be referencing to the entire quote vs specific sources. The only point where there's a per-source reference is in quoteReportSources but including priceImpact there is currently not how the code/data is structured. I see fillData for each source is being passed on - maybe that calculation can be done downstream on your side?

Ido, do you know who the best person is to understand how long /hard it takes to get this information in? I guess it is Jacob and I am planning to create a more formal request, given slippage estimate is a high priority project to make more money

[idokleinman]

@zhehao-0x , well officially my team (Liquidity integration) is the owner of the /swap endpoint which includes the mentioned code paths. However chatting with @dekz to further understand the complexities of this refactor is a good idea, he also has much more context on SlippageProtect in general and might provide additional ideas on how to solve this issue.

Emanuelle (Liquidity PM) will be able to help with prioritizing this task once we conclude what it is against our other ongoing activities.

Lastly @robpal1990 - if the PR in its current form does not provide value to your team I guess I can proceed to close it without merging? (also on Asset-Swapper) - Let me know

[zhehao-0x]

@zhehao-0x , well officially my team (Liquidity integration) is the owner of the /swap endpoint which includes the mentioned code paths. However chatting with @dekz to further understand the complexities of this refactor is a good idea, he also has much more context on SlippageProtect in general and might provide additional ideas on how to solve this issue.

Emanuelle (Liquidity PM) will be able to help with prioritizing this task once we conclude what it is against our other ongoing activities.

Lastly @robpal1990 - if the PR in its current form does not provide value to your team I guess I can proceed to close it without merging? (also on Asset-Swapper) - Let me know

Thank you Ido! Let me reach out to Jacob to get his take, and I will create a formal request for you and Manu

[joshrrose]

I'm going to close this PR. The changes have been included into https://github.com/0xProject/0x-api/pull/1141 per request from product