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

🐛 Astroport Slippage Fix #76

Merged
merged 4 commits into from
Nov 14, 2023
Merged

🐛 Astroport Slippage Fix #76

merged 4 commits into from
Nov 14, 2023

Conversation

NotJeremyLiu
Copy link
Member

@NotJeremyLiu NotJeremyLiu commented Nov 10, 2023

Background

  • It was observed there was a bug in which our Astroport simulation queries can return results that are not able to be executed at the swap stage due to having a max_spread limit at swap which isn't taken into account on Astroport's queries.
  • Relatedly, on the swap side we do not set a max_spread when we call the Astroport router contract, which defaults to a slippage limit of .005. This was decided to be changed to the max (.50) allowed on Astroport to reduce the amount of errors received from the Astroport contracts since our entry point handles the slippage we care about.

This PR

Swap Side

  1. Adds a max_spread amount of .50 (50%) to the Astroport router swaps, which is the max limit that can be set (Previously without setting this param, the max_spread defaulted to Astroport's default of .005)

Query Side:

  1. Asserts the max spread limit is not crossed in both swap exact in and swap exact out simulation query for Astroport
  2. Changes/refactors swap exact in simulation to call simulation query on each pool instead of the router (since the router doesn't return back enough info to assert the max spread limit calculation enforcement) for Astroport
  • And deploys the contracts to terra

Testing

  1. Tested the new swap in query gives back the same result as the astroport router:
  2. Tested that the max_spread_assertion query fails when expected
  3. Tested a successful swap tx that was previously failing with an error of max slippage limit crossed: https://www.mintscan.io/osmosis/tx/4867E0A4914F90CB6F97406406B8F67B3D8A44AF845BC090ACDFCFE2A3B4F246?height=12288371

@NotJeremyLiu NotJeremyLiu changed the title Astroport Slippage Fix 🐛 Astroport Slippage Fix Nov 10, 2023
// Convert the swap operations to astroport swap operations
let astroport_swap_operations = swap_operations.into_iter().map(From::from).collect();
// Assert the operation does not exceed the max spread limit
assert_max_spread(res.return_amount, res.spread_amount)?;
Copy link
Member

Choose a reason for hiding this comment

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

Does the ? force this to return early if the assertion is false?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yup, exactly, it can be used after any expression that returns a result, and it'll unwrap the result into the value if it's a value, and pass up the error if it's an error.

@NotJeremyLiu NotJeremyLiu merged commit b5b5ec1 into main Nov 14, 2023
5 checks passed
@NotJeremyLiu NotJeremyLiu deleted the jl/astroport-slippage-fix branch November 14, 2023 06:29
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