-
Notifications
You must be signed in to change notification settings - Fork 32
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
Support multiple routes in astroport adapter #99
Merged
Merged
Changes from 7 commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
7c03aaf
support multiple routes in astroport adapter
thal0x a7fd3e7
cargo fmt
thal0x a3a0804
make clippy happy
thal0x efa08ee
idiomatic
NotJeremyLiu 087b2f2
clippy
NotJeremyLiu 581dadd
use internal accounting
thal0x 450c28c
reset pre swap out asset amount state
thal0x 69fd31f
fmt
thal0x f7e7c9e
move comment
thal0x d94c9b1
Merge branch 'main' into jw/astroport-multi-route
thal0x File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
use crate::{ | ||
error::{ContractError, ContractResult}, | ||
state::ENTRY_POINT_CONTRACT_ADDRESS, | ||
state::{ENTRY_POINT_CONTRACT_ADDRESS, PRE_SWAP_OUT_ASSET_AMOUNT}, | ||
}; | ||
use astroport::pair::{ | ||
Cw20HookMsg as PairCw20HookMsg, ExecuteMsg as PairExecuteMsg, QueryMsg as PairQueryMsg, | ||
|
@@ -15,10 +15,9 @@ use cw20::{Cw20Coin, Cw20ReceiveMsg}; | |
use cw_utils::one_coin; | ||
use skip::{ | ||
asset::{get_current_asset_available, Asset}, | ||
error::SkipError, | ||
swap::{ | ||
execute_transfer_funds_back, Cw20HookMsg, ExecuteMsg, InstantiateMsg, MigrateMsg, QueryMsg, | ||
SimulateSwapExactAssetInResponse, SimulateSwapExactAssetOutResponse, SwapOperation, | ||
Route, SimulateSwapExactAssetInResponse, SimulateSwapExactAssetOutResponse, SwapOperation, | ||
}, | ||
}; | ||
|
||
|
@@ -102,15 +101,7 @@ pub fn receive_cw20( | |
info.sender = deps.api.addr_validate(&cw20_msg.sender)?; | ||
|
||
match from_json(&cw20_msg.msg)? { | ||
Cw20HookMsg::Swap { routes } => { | ||
if routes.len() != 1 { | ||
return Err(ContractError::Skip(SkipError::MustBeSingleRoute)); | ||
} | ||
|
||
let operations = routes.first().unwrap().operations.clone(); | ||
|
||
execute_swap(deps, env, info, operations) | ||
} | ||
Cw20HookMsg::Swap { routes } => execute_swap(deps, env, info, routes), | ||
} | ||
} | ||
|
||
|
@@ -130,13 +121,7 @@ pub fn execute( | |
ExecuteMsg::Swap { routes } => { | ||
one_coin(&info)?; | ||
|
||
if routes.len() != 1 { | ||
return Err(ContractError::Skip(SkipError::MustBeSingleRoute)); | ||
} | ||
|
||
let operations = routes.first().unwrap().operations.clone(); | ||
|
||
execute_swap(deps, env, info, operations) | ||
execute_swap(deps, env, info, routes) | ||
} | ||
ExecuteMsg::TransferFundsBack { | ||
swapper, | ||
|
@@ -148,9 +133,10 @@ pub fn execute( | |
swapper, | ||
return_denom, | ||
)?), | ||
ExecuteMsg::AstroportPoolSwap { operation } => { | ||
execute_astroport_pool_swap(deps, env, info, operation) | ||
} | ||
ExecuteMsg::AstroportPoolSwap { | ||
operation, | ||
offer_asset, | ||
} => execute_astroport_pool_swap(deps, env, info, operation, offer_asset), | ||
_ => { | ||
unimplemented!() | ||
} | ||
|
@@ -161,7 +147,7 @@ fn execute_swap( | |
deps: DepsMut, | ||
env: Env, | ||
info: MessageInfo, | ||
operations: Vec<SwapOperation>, | ||
routes: Vec<Route>, | ||
) -> ContractResult<Response> { | ||
// Get entry point contract address from storage | ||
let entry_point_contract_address = ENTRY_POINT_CONTRACT_ADDRESS.load(deps.storage)?; | ||
|
@@ -171,22 +157,40 @@ fn execute_swap( | |
return Err(ContractError::Unauthorized); | ||
} | ||
|
||
// reset the pre swap out asset amount | ||
PRE_SWAP_OUT_ASSET_AMOUNT.save(deps.storage, &Uint128::new(0))?; | ||
|
||
// Create a response object to return | ||
let mut response: Response = Response::new().add_attribute("action", "execute_swap"); | ||
|
||
// Add an astroport pool swap message to the response for each swap operation | ||
for operation in &operations { | ||
let swap_msg = WasmMsg::Execute { | ||
contract_addr: env.contract.address.to_string(), | ||
msg: to_json_binary(&ExecuteMsg::AstroportPoolSwap { | ||
operation: operation.clone(), | ||
})?, | ||
funds: vec![], | ||
}; | ||
response = response.add_message(swap_msg); | ||
// Add an astroport pool swap message to the response for each swap operation in each route | ||
for route in &routes { | ||
for (idx, operation) in route.operations.iter().enumerate() { | ||
// if first operation in a route, set offer asset to the route's offer asset | ||
// otherwise, the offer asset will be determined by the contracts balance | ||
let offer_asset = if idx == 0 { | ||
Some(route.offer_asset.clone()) | ||
} else { | ||
None | ||
}; | ||
|
||
let swap_msg = WasmMsg::Execute { | ||
contract_addr: env.contract.address.to_string(), | ||
msg: to_json_binary(&ExecuteMsg::AstroportPoolSwap { | ||
operation: operation.clone(), | ||
offer_asset, | ||
})?, | ||
funds: vec![], | ||
}; | ||
|
||
response = response.add_message(swap_msg); | ||
} | ||
} | ||
|
||
let return_denom = match operations.last() { | ||
let return_denom = match routes | ||
.last() | ||
.and_then(|last_route| last_route.operations.last()) | ||
{ | ||
Some(last_op) => last_op.denom_out.clone(), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah let's make sure we do validation on entrypoint that the first op denom / offer asset of all routes must be the same, and the last op denom of all routes must be the same in the entrypoint. Then this line is safe |
||
None => return Err(ContractError::SwapOperationsEmpty), | ||
}; | ||
|
@@ -211,20 +215,39 @@ fn execute_astroport_pool_swap( | |
env: Env, | ||
info: MessageInfo, | ||
operation: SwapOperation, | ||
offer_asset: Option<Asset>, | ||
) -> ContractResult<Response> { | ||
// Ensure the caller is the contract itself | ||
if info.sender != env.contract.address { | ||
return Err(ContractError::Unauthorized); | ||
} | ||
|
||
// Get the current asset available on contract to swap in | ||
let offer_asset = get_current_asset_available(&deps, &env, &operation.denom_in)?; | ||
let offer_asset = match offer_asset { | ||
Some(offer_asset) => offer_asset, | ||
None => { | ||
let pre_swap_out_asset_amount = PRE_SWAP_OUT_ASSET_AMOUNT | ||
.load(deps.storage) | ||
.unwrap_or(Uint128::zero()); | ||
|
||
let mut current_balance = | ||
get_current_asset_available(&deps, &env, &operation.denom_in)?; | ||
|
||
current_balance.sub(pre_swap_out_asset_amount)?; | ||
|
||
current_balance | ||
} | ||
}; | ||
|
||
// Error if the offer asset amount is zero | ||
if offer_asset.amount().is_zero() { | ||
return Err(ContractError::NoOfferAssetAmount); | ||
} | ||
|
||
let pre_swap_out_asset_amount = | ||
get_current_asset_available(&deps, &env, operation.denom_out.as_str())?.amount(); | ||
PRE_SWAP_OUT_ASSET_AMOUNT.save(deps.storage, &pre_swap_out_asset_amount)?; | ||
|
||
// Create the astroport pool swap msg depending on the offer asset type | ||
let msg = match offer_asset { | ||
Asset::Native(_) => to_json_binary(&PairExecuteMsg::Swap { | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,6 @@ | ||
use cosmwasm_std::Addr; | ||
use cosmwasm_std::{Addr, Uint128}; | ||
use cw_storage_plus::Item; | ||
|
||
pub const ENTRY_POINT_CONTRACT_ADDRESS: Item<Addr> = Item::new("entry_point_contract_address"); | ||
|
||
pub const PRE_SWAP_OUT_ASSET_AMOUNT: Item<Uint128> = Item::new("pre_swap_out_asset_amount"); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
I'm pretty sure in theory this isn't needed, but might be good to be safe
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.
Why in theory is this not needed? Just because we don't expect coinhall to ever give us routes that touch the same denom out as the offer asset?
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.
And for this to be correct shouldn't this query the contract's balance for the first hop's denom out instead of 0? Which ideally is 0 most of the time but doesn't seem like we should always assume it's 0
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.
I believe it doesn't query the balance because it does it later, but let me recheck the logic
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.
Ok now that I'm looking at the code again, it's not querying the balance in
execute_swap
because it queries the balance inexecute_astroport_pool_swap
.And this is why I said that it's not needed in theory, because whatever value gets saved here, gets overwritten anyways.