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

Support multiple routes in astroport adapter #99

Merged
merged 10 commits into from
Apr 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
93 changes: 58 additions & 35 deletions contracts/adapters/swap/astroport/src/contract.rs
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,
Expand All @@ -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,
},
};

Expand Down Expand Up @@ -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),
}
}

Expand All @@ -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,
Expand All @@ -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!()
}
Expand All @@ -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)?;
Expand All @@ -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))?;
Copy link
Member Author

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

Copy link
Member

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?

Copy link
Member

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

Copy link
Member Author

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

Copy link
Member Author

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 in execute_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.


// 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(),
Copy link
Member

Choose a reason for hiding this comment

The 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),
};
Expand All @@ -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 {
Expand Down
4 changes: 3 additions & 1 deletion contracts/adapters/swap/astroport/src/state.rs
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");
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,15 @@ use cosmwasm_std::{
ReplyOn::Never,
SubMsg, SystemResult, Uint128, WasmMsg, WasmQuery,
};
use cw20::{BalanceResponse, Cw20ExecuteMsg};
use skip::swap::{ExecuteMsg, SwapOperation};
use skip_api_swap_adapter_astroport::error::{ContractError, ContractResult};
use cw20::{BalanceResponse, Cw20Coin, Cw20ExecuteMsg};
use skip::{
asset::Asset,
swap::{ExecuteMsg, SwapOperation},
};
use skip_api_swap_adapter_astroport::{
error::{ContractError, ContractResult},
state::PRE_SWAP_OUT_ASSET_AMOUNT,
};
use test_case::test_case;

/*
Expand All @@ -31,6 +37,8 @@ Expect Error
struct Params {
caller: String,
contract_balance: Vec<Coin>,
pre_swap_out_asset_amount: Uint128,
offer_asset: Option<Asset>,
swap_operation: SwapOperation,
expected_message: Option<SubMsg>,
expected_error: Option<ContractError>,
Expand All @@ -41,6 +49,8 @@ struct Params {
Params {
caller: "swap_contract_address".to_string(),
contract_balance: vec![Coin::new(100, "os")],
pre_swap_out_asset_amount: Uint128::new(0),
offer_asset: Some(Asset::Native(Coin::new(100, "os"))),
swap_operation: SwapOperation {
pool: "pool_1".to_string(),
denom_in: "os".to_string(),
Expand Down Expand Up @@ -76,6 +86,11 @@ struct Params {
Params {
caller: "swap_contract_address".to_string(),
contract_balance: vec![],
pre_swap_out_asset_amount: Uint128::new(0),
offer_asset: Some(Asset::Cw20(Cw20Coin {
address: "neutron123".to_string(),
amount: Uint128::from(100u128),
})),
swap_operation: SwapOperation {
pool: "pool_1".to_string(),
denom_in: "neutron123".to_string(),
Expand Down Expand Up @@ -104,10 +119,50 @@ struct Params {
expected_error: None,
};
"Cw20 Swap Operation")]
#[test_case(
Params {
caller: "swap_contract_address".to_string(),
contract_balance: vec![Coin::new(100, "os")],
pre_swap_out_asset_amount: Uint128::new(50),
offer_asset: None,
swap_operation: SwapOperation {
pool: "pool_1".to_string(),
denom_in: "os".to_string(),
denom_out: "ua".to_string(),
interface: None,
},
expected_message: Some(SubMsg {
id: 0,
msg: WasmMsg::Execute {
contract_addr: "pool_1".to_string(),
msg: to_json_binary(&AstroportPairExecuteMsg::Swap {
offer_asset: AstroportAsset {
info: AssetInfo::NativeToken {
denom: "os".to_string(),
},
amount: Uint128::new(50),
},
ask_asset_info: None,
belief_price: None,
max_spread: Some(Decimal::percent(50)),
to: None,
})?,
funds: vec![Coin::new(50, "os")],
}
.into(),
gas_limit: None,
reply_on: Never
}),
expected_error: None,
};
"Deducts pre swap out asset amount from contract balance before swap"
)]
#[test_case(
Params {
caller: "swap_contract_address".to_string(),
contract_balance: vec![],
pre_swap_out_asset_amount: Uint128::new(0),
offer_asset: None,
swap_operation: SwapOperation {
pool: "pool_1".to_string(),
denom_in: "os".to_string(),
Expand All @@ -122,6 +177,8 @@ struct Params {
Params {
caller: "swap_contract_address".to_string(),
contract_balance: vec![],
pre_swap_out_asset_amount: Uint128::new(0),
offer_asset: None,
swap_operation: SwapOperation {
pool: "pool_1".to_string(),
denom_in: "randomcw20".to_string(),
Expand All @@ -138,6 +195,8 @@ struct Params {
contract_balance: vec![
Coin::new(100, "un"),
],
pre_swap_out_asset_amount: Uint128::new(0),
offer_asset: Some(Asset::Native(Coin::new(100, "un"))),
swap_operation: SwapOperation{
pool: "".to_string(),
denom_in: "".to_string(),
Expand Down Expand Up @@ -188,6 +247,8 @@ fn test_execute_astroport_pool_swap(params: Params) -> ContractResult<()> {
let mut env = mock_env();
env.contract.address = Addr::unchecked("swap_contract_address");

PRE_SWAP_OUT_ASSET_AMOUNT.save(&mut deps.storage, &params.pre_swap_out_asset_amount)?;

// Create mock info
let info = mock_info(&params.caller, &[]);

Expand All @@ -197,6 +258,7 @@ fn test_execute_astroport_pool_swap(params: Params) -> ContractResult<()> {
env,
info,
ExecuteMsg::AstroportPoolSwap {
offer_asset: params.offer_asset,
operation: params.swap_operation,
},
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,10 @@ struct Params {
msg: WasmMsg::Execute {
contract_addr: "swap_contract_address".to_string(),
msg: to_json_binary(&ExecuteMsg::AstroportPoolSwap {
offer_asset: Some(Asset::Cw20(Cw20Coin {
address: "neutron123".to_string(),
amount: Uint128::from(100u128),
})),
operation: SwapOperation {
pool: "pool_1".to_string(),
denom_in: "neutron123".to_string(),
Expand Down Expand Up @@ -115,6 +119,10 @@ struct Params {
msg: WasmMsg::Execute {
contract_addr: "swap_contract_address".to_string(),
msg: to_json_binary(&ExecuteMsg::AstroportPoolSwap {
offer_asset: Some(Asset::Cw20(Cw20Coin {
address: "neutron123".to_string(),
amount: Uint128::from(100u128),
})),
operation: SwapOperation {
pool: "pool_1".to_string(),
denom_in: "neutron123".to_string(),
Expand Down
Loading
Loading