✨ Add Spot Price In Swap Simulation Query Responses #77
Conversation
…eries" This reverts commit fc8c18e.
NotJeremyLiu
changed the title
| @@ -220,6 +221,22 @@ pub fn query(deps: Deps, _env: Env, msg: QueryMsg) -> ContractResult<Binary> { | |||
| asset_out, | |||
| swap_operations, | |||
| )?), | |||
| QueryMsg::SimulateSwapExactAssetInWithSpotPrice { | |||
There was a problem hiding this comment.
It looks like we are added new queries because the old query returns an Asset, and now we need to return two values.
I think there is a learning here, we should always return a "Response" object as a return value as oppose to a raw value like "Asset".
Let's change this name to "SimulateSwapExactAssetInWithMetadata", and added a
#[deprecated(since="{version}", note="please use `SimulateSwapExactAssetInWithMetadata` instead")]
to the enum for SimulateSwapExactAssetIn (similar for Out)
or just put a deprecation comment along those lines, since I think that can only be used on functions.
With the intent of removing the old queries once we update our simulation, and it feels safe to do so.
There was a problem hiding this comment.
Re: our conversion, I believe we are on the same page that we will continue to use the non-metadata queries for the entrypoint contract, so they will not be deprecated. I assume you no longer want me to add these deprecation notices due to them staying in use.
| @@ -322,6 +339,127 @@ fn query_simulate_swap_exact_asset_out( | |||
| Ok(asset_in_needed) | |||
| } | |||
|
|
|||
| // Queries the astroport router contract to simulate a swap exact amount in with spot price | |||
| fn query_simulate_swap_exact_asset_in_with_spot_price( | |||
There was a problem hiding this comment.
Made a alteration here for how I think this code should be organzied
https://gist.github.com/zrbecker/dccb56a2a81f4fc0be6ab4fb9d931d09
If these methods are used internally, we should probably the logic into another function simulate_swap_exact_asset_in, which query_simulate_swap_exact_asset_in and this method can call.
There was a problem hiding this comment.
completed my take of this new design here: 1c28a99
In particular, instead of always giving back all metadata fields, the metadata queries accept "include_X" boolean params that are checked and used to build up the response as needed.
| @@ -322,6 +339,127 @@ fn query_simulate_swap_exact_asset_out( | |||
| Ok(asset_in_needed) | |||
| } | |||
|
|
|||
| // Queries the astroport router contract to simulate a swap exact amount in with spot price | |||
| fn query_simulate_swap_exact_asset_in_with_spot_price( | |||
There was a problem hiding this comment.
I recommend with_metadata instead of spot price, so that we can extend this.
There was a problem hiding this comment.
changed here: dfb4042
| @@ -274,3 +291,47 @@ fn query_simulate_swap_exact_asset_out( | |||
| } | |||
| .into()) | |||
| } | |||
|
|
|||
| // Queries the osmosis poolmanager module to simulate a swap exact amount in with spot price | |||
| fn query_simulate_swap_exact_asset_in_with_spot_price( | |||
There was a problem hiding this comment.
This is doing what I had in mine for astroport, but I guess the contract extends to it better.
There was a problem hiding this comment.
Yeah for Astroport we get the spot price from the simulation responses themselves, so had initially coupled the logic together so that it had one iteration. Now with the new design of getting the responses during simulation and then iterating in the future for spot price, this gets a tad cleaner, but not as clean as the decoupled osmosis version since they actually have spot price queries.
|
Ready for re-review |
NotJeremyLiu
changed the title
Background
This PR