From 38699af6176933bf697e0e75f07e7ef5eeef1134 Mon Sep 17 00:00:00 2001 From: lubkoll <11710767+lubkoll@users.noreply.github.com> Date: Sat, 27 Jul 2024 17:28:28 +0000 Subject: [PATCH 1/4] Enable clippy in CI for dex-router-osmosis test-tube tests (#719) --- .github/workflows/rust_basic.yml | 2 +- .github/workflows/test_tube_osmosis.yml | 3 +++ .../tests/initialize/mod.rs | 20 +++++++++---------- .../dex-router-osmosis/tests/integration.rs | 4 ++-- 4 files changed, 16 insertions(+), 13 deletions(-) diff --git a/.github/workflows/rust_basic.yml b/.github/workflows/rust_basic.yml index 742285708..1746f3915 100644 --- a/.github/workflows/rust_basic.yml +++ b/.github/workflows/rust_basic.yml @@ -32,7 +32,7 @@ jobs: key: ${{ runner.os }}-cargo-${{ inputs.workspace }}-$GITHUB_SHA restore-keys: ${{ runner.os }}-cargo-${{ inputs.workspace }} - name: Rust lint - run: RUSTFLAGS="-Dwarnings" cargo clippy --workspace -- -D warnings --A deprecated + run: cargo clippy --workspace -- -D warnings --A deprecated working-directory: smart-contracts/${{ inputs.workspace }}/${{ inputs.target }} - name: Rust format check run: cargo fmt --all -- --check diff --git a/.github/workflows/test_tube_osmosis.yml b/.github/workflows/test_tube_osmosis.yml index 2a99400ce..7258d87a7 100644 --- a/.github/workflows/test_tube_osmosis.yml +++ b/.github/workflows/test_tube_osmosis.yml @@ -48,6 +48,9 @@ jobs: smart-contracts/osmosis/**/target key: ${{ runner.os }}-cargo-osmosis-$GITHUB_SHA restore-keys: ${{ runner.os }}-cargo-osmosis + - name: Rust lint + run: cargo clippy --features test-tube --all-targets -- -D warnings --A deprecated + working-directory: smart-contracts/osmosis/contracts/dex-router-osmosis - name: Build merkle-incentives run: cargo test-tube-build working-directory: smart-contracts/osmosis/contracts/merkle-incentives diff --git a/smart-contracts/osmosis/contracts/dex-router-osmosis/tests/initialize/mod.rs b/smart-contracts/osmosis/contracts/dex-router-osmosis/tests/initialize/mod.rs index 1d827d1f3..e19f2f311 100644 --- a/smart-contracts/osmosis/contracts/dex-router-osmosis/tests/initialize/mod.rs +++ b/smart-contracts/osmosis/contracts/dex-router-osmosis/tests/initialize/mod.rs @@ -55,7 +55,7 @@ pub fn single_cl_pool_fixture( let pm = PoolManager::new(app); let cl = ConcentratedLiquidity::new(app); - let gov = GovWithAppAccess::new(&app); + let gov = GovWithAppAccess::new(app); gov.propose_and_execute( CreateConcentratedLiquidityPoolsProposal::TYPE_URL.to_string(), @@ -65,12 +65,12 @@ pub fn single_cl_pool_fixture( pool_records: vec![PoolRecord { denom0: cl_pool.denom0.clone(), denom1: cl_pool.denom1.clone(), - tick_spacing: cl_pool.tick_spacing.clone(), + tick_spacing: cl_pool.tick_spacing, spread_factor: cl_pool.spread_factor.clone(), }], }, admin.address(), - &admin, + admin, ) .unwrap(); @@ -110,7 +110,7 @@ pub fn single_cl_pool_fixture( token_min_amount0: "1".to_string(), token_min_amount1: "1".to_string(), }, - &admin, + admin, ) .unwrap(); @@ -199,18 +199,18 @@ pub fn single_gamm_pool_fixture( .unwrap(), }, ], - &admin, + admin, ) .unwrap(); let ty = "pool_created"; - let keys = vec!["pool_id"]; + let key = "pool_id"; let pool_id: u64 = response .events .iter() .filter(|event| event.ty == ty) .flat_map(|event| event.attributes.clone()) - .filter(|attribute| keys.contains(&attribute.key.as_str())) + .filter(|attribute| key == attribute.key.as_str()) .collect::>() .first() .unwrap() @@ -220,7 +220,7 @@ pub fn single_gamm_pool_fixture( let _ = MsgJoinPool { sender: admin.address().to_string(), - pool_id: pool_id.clone(), + pool_id, share_out_amount: "100".to_string(), token_in_maxs: vec![ Coin { @@ -303,7 +303,7 @@ pub fn setup_paths( admin: &SigningAccount, ) { wasm.execute( - &contract_address.to_string(), + contract_address.as_ref(), &ExecuteMsg::SetPath { path, bidirectional: true, @@ -339,7 +339,7 @@ pub fn perform_swap( admin: &SigningAccount, ) -> Result, osmosis_test_tube::RunnerError> { wasm.execute( - &contract_address.to_string(), + contract_address.as_ref(), &ExecuteMsg::Swap { out_denom: ask_denom, path: Some(path.first().unwrap().clone()), diff --git a/smart-contracts/osmosis/contracts/dex-router-osmosis/tests/integration.rs b/smart-contracts/osmosis/contracts/dex-router-osmosis/tests/integration.rs index 5af1f04bb..2dbff1237 100644 --- a/smart-contracts/osmosis/contracts/dex-router-osmosis/tests/integration.rs +++ b/smart-contracts/osmosis/contracts/dex-router-osmosis/tests/integration.rs @@ -44,7 +44,7 @@ fn default_init_works() { let resp: BestPathForPairResponse = wasm .query( - &contract_address.to_string(), + contract_address.as_ref(), &QueryMsg::BestPathForPair { offer: Coin::new( Uint128::from(100000000u128).into(), @@ -152,7 +152,7 @@ fn test_set_and_remove_path() { ); let _ = wasm .execute( - &contract_address.to_string(), + contract_address.as_ref(), &ExecuteMsg::RemovePath { path: vec![pools.first().unwrap().pool], bidirectional: true, From 95074bf228838714cbf651f46c1f0da683ec0584 Mon Sep 17 00:00:00 2001 From: lubkoll <11710767+lubkoll@users.noreply.github.com> Date: Mon, 29 Jul 2024 07:10:12 +0000 Subject: [PATCH 2/4] Enable clippy in the CI for test-tube for merkle-incentives (#720) --- .github/workflows/test_tube_osmosis.yml | 2 +- scripts/git/pre-commit | 6 +-- .../contracts/cl-vault/src/contract.rs | 4 +- .../cl-vault/src/helpers/coinlist.rs | 16 ++++---- .../contracts/cl-vault/src/test_helpers.rs | 6 +-- .../contracts/cl-vault/src/vault/admin.rs | 3 +- .../src/vault/concentrated_liquidity.rs | 2 +- .../contracts/cl-vault/src/vault/withdraw.rs | 3 +- .../osmosis/contracts/cl-vault/tests/admin.rs | 2 +- .../contracts/cl-vault/tests/any_deposit.rs | 6 +-- .../osmosis/contracts/cl-vault/tests/authz.rs | 22 ++++------- .../cl-vault/tests/deposit_withdraw.rs | 8 ++-- .../contracts/cl-vault/tests/setup/mod.rs | 14 +++---- .../merkle-incentives/src/admin/execute.rs | 4 +- .../src/incentives/helpers.rs | 8 ++-- .../merkle-incentives/src/incentives/mod.rs | 8 ++-- .../merkle-incentives/src/incentives/query.rs | 2 +- .../merkle-incentives/tests/merkle.rs | 38 +++++++------------ 18 files changed, 66 insertions(+), 88 deletions(-) diff --git a/.github/workflows/test_tube_osmosis.yml b/.github/workflows/test_tube_osmosis.yml index 7258d87a7..73cf14160 100644 --- a/.github/workflows/test_tube_osmosis.yml +++ b/.github/workflows/test_tube_osmosis.yml @@ -50,7 +50,7 @@ jobs: restore-keys: ${{ runner.os }}-cargo-osmosis - name: Rust lint run: cargo clippy --features test-tube --all-targets -- -D warnings --A deprecated - working-directory: smart-contracts/osmosis/contracts/dex-router-osmosis + working-directory: smart-contracts/osmosis - name: Build merkle-incentives run: cargo test-tube-build working-directory: smart-contracts/osmosis/contracts/merkle-incentives diff --git a/scripts/git/pre-commit b/scripts/git/pre-commit index 4e9a9b52f..824d4d02e 100755 --- a/scripts/git/pre-commit +++ b/scripts/git/pre-commit @@ -18,11 +18,11 @@ update_schemas_and_formatting() { if [[ $CHANGES == *contracts/${contract}* ]] then echo "Changes in ${contract} contract" - cd ${REPO_ROOT}/smart-contracts/contracts/${contract} + cd ${REPO_ROOT}/smart-contracts/osmosis/contracts/${contract} # generate schemas cargo schema - NEW_CHANGES=$(git diff --name-only origin/main) + NEW_CHANGES=$(git diff --name-only origin/main..HEAD) if [[ $NEW_CHANGES == *schema/* ]] then git add schema @@ -33,7 +33,7 @@ update_schemas_and_formatting() { # fix formatting cargo fmt --all cd ${REPO_ROOT} - NEW_CHANGES=$(git diff --name-only origin/main --diff-filter=A) + NEW_CHANGES=$(git diff --name-only origin/main..HEAD --diff-filter=AMR) git add $NEW_CHANGES fi } diff --git a/smart-contracts/osmosis/contracts/cl-vault/src/contract.rs b/smart-contracts/osmosis/contracts/cl-vault/src/contract.rs index 62264466a..0718d4329 100644 --- a/smart-contracts/osmosis/contracts/cl-vault/src/contract.rs +++ b/smart-contracts/osmosis/contracts/cl-vault/src/contract.rs @@ -559,7 +559,7 @@ mod tests { if let Some(SubMsg { msg: CosmosMsg::Bank(BankMsg::Send { to_address, amount }), .. - }) = migrate_resp.messages.get(0) + }) = migrate_resp.messages.first() { assert_eq!(to_address, "treasury"); assert_eq!(amount, &rewards_coins); @@ -622,7 +622,7 @@ mod tests { } } - fn assert_multi_send(msg: &CosmosMsg, expected_user: &String, user_rewards_coins: &Vec) { + fn assert_multi_send(msg: &CosmosMsg, expected_user: &String, user_rewards_coins: &[Coin]) { if let CosmosMsg::Stargate { type_url, value } = msg { // Decode and validate the MsgMultiSend message // This has been decoded manually rather than encoding the expected message because its simpler to assert the values diff --git a/smart-contracts/osmosis/contracts/cl-vault/src/helpers/coinlist.rs b/smart-contracts/osmosis/contracts/cl-vault/src/helpers/coinlist.rs index 164a486dd..8c89b2696 100644 --- a/smart-contracts/osmosis/contracts/cl-vault/src/helpers/coinlist.rs +++ b/smart-contracts/osmosis/contracts/cl-vault/src/helpers/coinlist.rs @@ -173,7 +173,7 @@ mod tests { fn coins_works() { let mut rewards = CoinList::new(); rewards - .update_rewards(&vec![ + .update_rewards(&[ OsmoCoin { denom: "uosmo".into(), amount: "1000".into(), @@ -200,7 +200,7 @@ mod tests { fn coins_only_positive_works() { let mut rewards = CoinList::new(); rewards - .update_rewards(&vec![ + .update_rewards(&[ OsmoCoin { denom: "uosmo".into(), amount: "1000".into(), @@ -227,7 +227,7 @@ mod tests { fn sub_works() { let mut rewards = CoinList::new(); rewards - .update_rewards(&vec![ + .update_rewards(&[ OsmoCoin { denom: "uosmo".into(), amount: "1000".into(), @@ -290,7 +290,7 @@ mod tests { fn percentage_works() { let mut rewards = CoinList::new(); rewards - .update_rewards(&vec![ + .update_rewards(&[ OsmoCoin { denom: "uosmo".into(), amount: "1000".into(), @@ -321,7 +321,7 @@ mod tests { fn sub_percentage_works() { let mut rewards = CoinList::new(); rewards - .update_rewards(&vec![ + .update_rewards(&[ OsmoCoin { denom: "uosmo".into(), amount: "1000".into(), @@ -365,7 +365,7 @@ mod tests { fn add_works() { let mut rewards = CoinList::new(); rewards - .update_rewards(&vec![ + .update_rewards(&[ OsmoCoin { denom: "uosmo".into(), amount: "1000".into(), @@ -403,7 +403,7 @@ mod tests { fn update_rewards_works() { let mut rewards = CoinList::new(); rewards - .update_rewards(&vec![ + .update_rewards(&[ OsmoCoin { denom: "uosmo".into(), amount: "1000".into(), @@ -420,7 +420,7 @@ mod tests { .unwrap(); rewards - .update_rewards(&vec![ + .update_rewards(&[ OsmoCoin { denom: "uosmo".into(), amount: "1000".into(), diff --git a/smart-contracts/osmosis/contracts/cl-vault/src/test_helpers.rs b/smart-contracts/osmosis/contracts/cl-vault/src/test_helpers.rs index 9402048c6..1d66c85cd 100644 --- a/smart-contracts/osmosis/contracts/cl-vault/src/test_helpers.rs +++ b/smart-contracts/osmosis/contracts/cl-vault/src/test_helpers.rs @@ -54,7 +54,7 @@ impl QuasarQuerier { impl Querier for QuasarQuerier { fn raw_query(&self, bin_request: &[u8]) -> cosmwasm_std::QuerierResult { - let request: QueryRequest = from_json(&Binary::from(bin_request)).unwrap(); + let request: QueryRequest = from_json(Binary::from(bin_request)).unwrap(); match request { QueryRequest::Stargate { path, data } => match path.as_str() { "/osmosis.concentratedliquidity.v1beta1.Query/PositionById" => { @@ -89,11 +89,11 @@ impl Querier for QuasarQuerier { )) } "/cosmos.bank.v1beta.Query/Balance" => { - let query: BankQuery = from_json(&Binary::from(bin_request)).unwrap(); + let query: BankQuery = from_json(Binary::from(bin_request)).unwrap(); self.bank.query(&query) } "/cosmos.bank.v1beta.Query/AllBalances" => { - let query: BankQuery = from_json(&Binary::from(bin_request)).unwrap(); + let query: BankQuery = from_json(Binary::from(bin_request)).unwrap(); self.bank.query(&query) } "/osmosis.poolmanager.v1beta1.Query/Pool" => { diff --git a/smart-contracts/osmosis/contracts/cl-vault/src/vault/admin.rs b/smart-contracts/osmosis/contracts/cl-vault/src/vault/admin.rs index 61e239095..ebeb6a5bf 100644 --- a/smart-contracts/osmosis/contracts/cl-vault/src/vault/admin.rs +++ b/smart-contracts/osmosis/contracts/cl-vault/src/vault/admin.rs @@ -181,8 +181,7 @@ mod tests { let mut deps = mock_dependencies(); build_tick_exp_cache(&mut deps.storage).unwrap(); - let verify_resp = verify_tick_exp_cache(&mut deps.storage).unwrap(); - assert_eq!((), verify_resp); + assert!(verify_tick_exp_cache(&deps.storage).is_ok()); } #[test] diff --git a/smart-contracts/osmosis/contracts/cl-vault/src/vault/concentrated_liquidity.rs b/smart-contracts/osmosis/contracts/cl-vault/src/vault/concentrated_liquidity.rs index d65ab2382..aaefc9a5c 100644 --- a/smart-contracts/osmosis/contracts/cl-vault/src/vault/concentrated_liquidity.rs +++ b/smart-contracts/osmosis/contracts/cl-vault/src/vault/concentrated_liquidity.rs @@ -211,7 +211,7 @@ mod tests { ) .unwrap(); - let result = withdraw_from_position(&mut deps.storage, &env, liquidity_amount).unwrap(); + let result = withdraw_from_position(&deps.storage, &env, liquidity_amount).unwrap(); assert_eq!( result, diff --git a/smart-contracts/osmosis/contracts/cl-vault/src/vault/withdraw.rs b/smart-contracts/osmosis/contracts/cl-vault/src/vault/withdraw.rs index 0d439b2a3..390f24c6f 100644 --- a/smart-contracts/osmosis/contracts/cl-vault/src/vault/withdraw.rs +++ b/smart-contracts/osmosis/contracts/cl-vault/src/vault/withdraw.rs @@ -348,8 +348,7 @@ mod tests { amount0: "1000".to_string(), amount1: "1000".to_string(), } - .try_into() - .unwrap(); + .into(); let response = handle_withdraw_user_reply( deps.as_mut(), diff --git a/smart-contracts/osmosis/contracts/cl-vault/tests/admin.rs b/smart-contracts/osmosis/contracts/cl-vault/tests/admin.rs index cb6882616..f166a19e0 100644 --- a/smart-contracts/osmosis/contracts/cl-vault/tests/admin.rs +++ b/smart-contracts/osmosis/contracts/cl-vault/tests/admin.rs @@ -46,5 +46,5 @@ fn admin_build_tick_cache_works() { )), ) .unwrap(); - assert_eq!((), verify_resp.result.unwrap()); + assert!(verify_resp.result.is_ok()); } diff --git a/smart-contracts/osmosis/contracts/cl-vault/tests/any_deposit.rs b/smart-contracts/osmosis/contracts/cl-vault/tests/any_deposit.rs index 5298ff46f..b1b695dea 100644 --- a/smart-contracts/osmosis/contracts/cl-vault/tests/any_deposit.rs +++ b/smart-contracts/osmosis/contracts/cl-vault/tests/any_deposit.rs @@ -95,8 +95,7 @@ fn do_and_verify_any_deposit( .find(|coin| coin.denom == DENOM_QUOTE) .map(|coin| { let quote_amount = Uint128::from(coin.amount.parse::().unwrap()); - let quote_as_base = quote_amount * spot_price_quote_to_base; - quote_as_base + quote_amount * spot_price_quote_to_base }) .unwrap_or_else(Uint128::zero); @@ -115,8 +114,7 @@ fn do_and_verify_any_deposit( .find(|coin| coin.denom == DENOM_QUOTE) .map(|coin| { let quote_amount = Uint128::from(coin.amount.parse::().unwrap()); - let quote_as_base = quote_amount * spot_price_quote_to_base; - quote_as_base + quote_amount * spot_price_quote_to_base }) .unwrap_or_else(Uint128::zero); diff --git a/smart-contracts/osmosis/contracts/cl-vault/tests/authz.rs b/smart-contracts/osmosis/contracts/cl-vault/tests/authz.rs index f15777c6c..2f3946a27 100644 --- a/smart-contracts/osmosis/contracts/cl-vault/tests/authz.rs +++ b/smart-contracts/osmosis/contracts/cl-vault/tests/authz.rs @@ -67,14 +67,11 @@ fn exact_deposit_withdraw_equal() { assert_eq!(deposit_response.data, authz_deposit_response.data); assert_eq!( - deposit_response - .events - .iter() - .find(|e| e.ty == "wasm".to_string()), + deposit_response.events.iter().find(|e| e.ty == *"wasm"), authz_deposit_response .events .iter() - .find(|e| e.ty == "wasm".to_string()) + .find(|e| e.ty == *"wasm") ); let shares: UserSharesBalanceResponse = wasm @@ -117,14 +114,11 @@ fn exact_deposit_withdraw_equal() { assert_eq!(withdraw_response.data, authz_withdraw_response.data); assert_eq!( - withdraw_response - .events - .iter() - .find(|e| e.ty == "wasm".to_string()), + withdraw_response.events.iter().find(|e| e.ty == *"wasm"), authz_withdraw_response .events .iter() - .find(|e| e.ty == "wasm".to_string()) + .find(|e| e.ty == *"wasm") ); } @@ -175,12 +169,12 @@ fn any_deposit_withdraw_equal() { let deposit_event = deposit_response .events .iter() - .find(|e| e.ty == "wasm".to_string()) + .find(|e| e.ty == *"wasm") .unwrap(); let authz_deposit_event = authz_deposit_response .events .iter() - .find(|e| e.ty == "wasm".to_string()) + .find(|e| e.ty == *"wasm") .unwrap(); // Assert events are equal assert_eq!(deposit_event.ty, authz_deposit_event.ty); @@ -250,12 +244,12 @@ fn any_deposit_withdraw_equal() { let withdraw_event = withdraw_response .events .iter() - .find(|e| e.ty == "wasm".to_string()) + .find(|e| e.ty == *"wasm") .unwrap(); let authz_withdraw_event = authz_withdraw_response .events .iter() - .find(|e| e.ty == "wasm".to_string()) + .find(|e| e.ty == *"wasm") .unwrap(); // Assert events are equal assert_eq!(withdraw_event.ty, authz_withdraw_event.ty); diff --git a/smart-contracts/osmosis/contracts/cl-vault/tests/deposit_withdraw.rs b/smart-contracts/osmosis/contracts/cl-vault/tests/deposit_withdraw.rs index b866c66f3..48d393cb8 100644 --- a/smart-contracts/osmosis/contracts/cl-vault/tests/deposit_withdraw.rs +++ b/smart-contracts/osmosis/contracts/cl-vault/tests/deposit_withdraw.rs @@ -72,20 +72,20 @@ fn single_deposit_withdraw_works() { // assert that the refund + used funds are equal to what we deposited let refund0: u128 = get_event_attributes_by_ty_and_key(&response, "wasm", vec!["refund0"]) - .get(0) + .first() .map(|attr| attr.value.parse().unwrap()) .unwrap_or(0); let refund1: u128 = get_event_attributes_by_ty_and_key(&response, "wasm", vec!["refund1"]) - .get(0) + .first() .map(|attr| attr.value.parse().unwrap()) .unwrap_or(0); let deposited0: u128 = get_event_attributes_by_ty_and_key(&response, "wasm", vec!["amount0"]) - .get(0) + .first() .map(|attr| attr.value.parse().unwrap()) .unwrap_or(0); let deposited1: u128 = get_event_attributes_by_ty_and_key(&response, "wasm", vec!["amount1"]) - .get(0) + .first() .map(|attr| attr.value.parse().unwrap()) .unwrap_or(0); diff --git a/smart-contracts/osmosis/contracts/cl-vault/tests/setup/mod.rs b/smart-contracts/osmosis/contracts/cl-vault/tests/setup/mod.rs index 8b8df58aa..d88c0c924 100644 --- a/smart-contracts/osmosis/contracts/cl-vault/tests/setup/mod.rs +++ b/smart-contracts/osmosis/contracts/cl-vault/tests/setup/mod.rs @@ -126,9 +126,8 @@ pub fn fixture_dex_router( ) } -// INIT PRIVATE METHODS - // TODO: This is pub because of the proptest still not having a default_init implementation +#[allow(clippy::too_many_arguments)] pub fn init_test_contract( filename: &str, admin_balance: &[Coin], @@ -260,6 +259,7 @@ pub fn init_test_contract( ) } +#[allow(clippy::too_many_arguments)] fn init_test_contract_with_dex_router_and_swap_pools( filename_cl: &str, filename_dex: &str, @@ -366,7 +366,7 @@ fn init_test_contract_with_dex_router_and_swap_pools( // TODO: We could be using a mixed set of CL and gAMM pools here let mut lp_pools = vec![]; for pool_coins in &pools_coins { - let lp_pool = gm.create_basic_pool(&pool_coins, &admin).unwrap(); + let lp_pool = gm.create_basic_pool(pool_coins, &admin).unwrap(); lp_pools.push(lp_pool.data.pool_id); } @@ -480,8 +480,8 @@ fn init_test_contract_with_dex_router_and_swap_pools( fn set_dex_router_paths( app: &OsmosisTestApp, dex_router: String, - pools: &Vec, - pools_coins: &Vec>, + pools: &[u64], + pools_coins: &[Vec], admin: &SigningAccount, ) { let wasm = Wasm::new(app); @@ -498,7 +498,7 @@ fn set_dex_router_paths( bidirectional: true, }, vec![].as_ref(), - &admin, + admin, ) .unwrap(); } @@ -530,7 +530,7 @@ pub fn get_balance_amount(app: &OsmosisTestApp, address: String, denom: String) .unwrap() } -pub fn get_amount_from_denom(value: &String) -> u128 { +pub fn get_amount_from_denom(value: &str) -> u128 { // Find the position where the non-numeric part starts let pos = value.find(|c: char| !c.is_numeric()).unwrap_or(value.len()); // Extract the numeric part from the string diff --git a/smart-contracts/osmosis/contracts/merkle-incentives/src/admin/execute.rs b/smart-contracts/osmosis/contracts/merkle-incentives/src/admin/execute.rs index dd309d7c8..df9281dfd 100644 --- a/smart-contracts/osmosis/contracts/merkle-incentives/src/admin/execute.rs +++ b/smart-contracts/osmosis/contracts/merkle-incentives/src/admin/execute.rs @@ -89,7 +89,7 @@ mod tests { fn test_update_merkle_root_valid() { let mut deps = mock_dependencies(); let env = mock_env(); - let info = mock_info("admin", &vec![]); + let info = mock_info("admin", &[]); // Set incentives admin INCENTIVES_ADMIN @@ -112,7 +112,7 @@ mod tests { fn test_update_merkle_root_invalid() { let mut deps = mock_dependencies(); let env = mock_env(); - let info = mock_info("admin", &vec![]); + let info = mock_info("admin", &[]); // Set incentives admin INCENTIVES_ADMIN diff --git a/smart-contracts/osmosis/contracts/merkle-incentives/src/incentives/helpers.rs b/smart-contracts/osmosis/contracts/merkle-incentives/src/incentives/helpers.rs index dbd73c555..fa20e9de0 100644 --- a/smart-contracts/osmosis/contracts/merkle-incentives/src/incentives/helpers.rs +++ b/smart-contracts/osmosis/contracts/merkle-incentives/src/incentives/helpers.rs @@ -116,7 +116,7 @@ mod tests { for proof in &proof { if proof.len() == 32 { let mut arr = [0u8; 32]; - arr.copy_from_slice(&proof); + arr.copy_from_slice(proof); proof_hashes.push(arr); } else { eprintln!("Error: Hash is not 32 bytes."); @@ -277,7 +277,7 @@ mod tests { #[test] fn test_verify_success() { verify_proof( - &MERKLE_ROOT_STRING.to_string(), + MERKLE_ROOT_STRING, get_proof_hashes(), &[0usize], 10usize, @@ -289,7 +289,7 @@ mod tests { #[test] fn test_verify_bad_root() { let result = verify_proof( - &MERKLE_ROOT_INVALID_STRING.to_string(), + MERKLE_ROOT_INVALID_STRING, get_proof_hashes(), &[0usize], 10usize, @@ -302,7 +302,7 @@ mod tests { #[test] fn test_verify_bad_claim() { let result = verify_proof( - &MERKLE_ROOT_STRING.to_string(), + MERKLE_ROOT_STRING, get_proof_hashes(), &[0usize], 10usize, diff --git a/smart-contracts/osmosis/contracts/merkle-incentives/src/incentives/mod.rs b/smart-contracts/osmosis/contracts/merkle-incentives/src/incentives/mod.rs index 52d743d2d..704115547 100644 --- a/smart-contracts/osmosis/contracts/merkle-incentives/src/incentives/mod.rs +++ b/smart-contracts/osmosis/contracts/merkle-incentives/src/incentives/mod.rs @@ -173,8 +173,8 @@ mod tests { amount: Uint128::from(100u128), }]); - assert_eq!(false, coin_vec2.le(&coin_vec)); - assert_eq!(false, coin_vec2.ge(&coin_vec)); + assert!(!coin_vec2.le(&coin_vec)); + assert!(!coin_vec2.ge(&coin_vec)); let coin_vec = CoinVec(vec![Coin { denom: "uusd".to_string(), @@ -211,8 +211,8 @@ mod tests { }]); // coin vec should not be gt or lt coin vec 2 as this case should not pass through - assert_eq!(false, coin_vec.lt(&coin_vec2)); - assert_eq!(false, coin_vec.gt(&coin_vec2)); + assert!(!coin_vec.lt(&coin_vec2)); + assert!(!coin_vec.gt(&coin_vec2)); } #[test] diff --git a/smart-contracts/osmosis/contracts/merkle-incentives/src/incentives/query.rs b/smart-contracts/osmosis/contracts/merkle-incentives/src/incentives/query.rs index 9eb48e091..e07cbbe14 100644 --- a/smart-contracts/osmosis/contracts/merkle-incentives/src/incentives/query.rs +++ b/smart-contracts/osmosis/contracts/merkle-incentives/src/incentives/query.rs @@ -107,7 +107,7 @@ mod tests { fn test_query_merkle_root() { let mut deps = mock_dependencies(); let env = mock_env(); - let info = mock_info("admin", &vec![]); + let info = mock_info("admin", &[]); // Set incentives admin INCENTIVES_ADMIN diff --git a/smart-contracts/osmosis/contracts/merkle-incentives/tests/merkle.rs b/smart-contracts/osmosis/contracts/merkle-incentives/tests/merkle.rs index 2df4b5da5..4504f868e 100644 --- a/smart-contracts/osmosis/contracts/merkle-incentives/tests/merkle.rs +++ b/smart-contracts/osmosis/contracts/merkle-incentives/tests/merkle.rs @@ -77,28 +77,16 @@ fn merkle_complete_cycle_works() { .unwrap(); let leaves_str = vec![ - format!("{}900000000ugauge", accounts[0].address().to_string()).to_string(), - format!("{}9000000000ugauge", accounts[0].address().to_string()).to_string(), - format!("{}90000000000ugauge", accounts[0].address().to_string()).to_string(), - format!("{}900000000000ugauge", accounts[0].address().to_string()).to_string(), - format!("{}9000000000000ugauge", accounts[0].address().to_string()).to_string(), - format!("{}90000000000000ugauge", accounts[0].address().to_string()).to_string(), - format!("{}900000000000000ugauge", accounts[0].address().to_string()).to_string(), - format!( - "{}9000000000900000ugauge", - accounts[0].address().to_string() - ) - .to_string(), - format!( - "{}90000000009000000ugauge", - accounts[0].address().to_string() - ) - .to_string(), - format!( - "{}900000000090000000ugauge", - accounts[0].address().to_string() - ) - .to_string(), + format!("{}900000000ugauge", accounts[0].address()).to_string(), + format!("{}9000000000ugauge", accounts[0].address()).to_string(), + format!("{}90000000000ugauge", accounts[0].address()).to_string(), + format!("{}900000000000ugauge", accounts[0].address()).to_string(), + format!("{}9000000000000ugauge", accounts[0].address()).to_string(), + format!("{}90000000000000ugauge", accounts[0].address()).to_string(), + format!("{}900000000000000ugauge", accounts[0].address()).to_string(), + format!("{}9000000000900000ugauge", accounts[0].address()).to_string(), + format!("{}90000000009000000ugauge", accounts[0].address()).to_string(), + format!("{}900000000090000000ugauge", accounts[0].address()).to_string(), ]; let leaves = leaves_str @@ -187,7 +175,7 @@ fn merkle_complete_cycle_works() { for proof in &claim_account.proof { if proof.len() == 32 { let mut arr = [0u8; 32]; - arr.copy_from_slice(&proof); + arr.copy_from_slice(proof); proof_hashes.push(arr); } else { eprintln!("Error: Hash is not 32 bytes."); @@ -205,7 +193,7 @@ fn merkle_complete_cycle_works() { total_leaves_count: 10usize, }), &[], - &accounts.get(index).unwrap(), + accounts.get(index).unwrap(), ) .unwrap(); @@ -220,7 +208,7 @@ fn merkle_complete_cycle_works() { claim_account .coins .coins() - .get(0) + .first() .unwrap() .amount .to_string() From f77df3c2f4546fb999c2e69bcb78454214edae9a Mon Sep 17 00:00:00 2001 From: lubkoll <11710767+lubkoll@users.noreply.github.com> Date: Mon, 29 Jul 2024 08:36:12 +0000 Subject: [PATCH 3/4] Unify and re-use calculate_swap_amount (#716) --- .../osmosis/contracts/cl-vault/src/state.rs | 2 +- .../cl-vault/src/vault/any_deposit.rs | 286 ++++++------------ .../contracts/cl-vault/src/vault/range.rs | 241 ++++----------- .../contracts/cl-vault/src/vault/swap.rs | 126 ++++---- 4 files changed, 218 insertions(+), 437 deletions(-) diff --git a/smart-contracts/osmosis/contracts/cl-vault/src/state.rs b/smart-contracts/osmosis/contracts/cl-vault/src/state.rs index 7eb8b761c..bf10efa4a 100644 --- a/smart-contracts/osmosis/contracts/cl-vault/src/state.rs +++ b/smart-contracts/osmosis/contracts/cl-vault/src/state.rs @@ -1,6 +1,6 @@ use crate::{ helpers::coinlist::CoinList, - vault::{merge::CurrentMergeWithdraw, range::SwapDirection}, + vault::{merge::CurrentMergeWithdraw, swap::SwapDirection}, }; use cosmwasm_schema::cw_serde; use cosmwasm_std::{Addr, Decimal, Decimal256, Uint128}; diff --git a/smart-contracts/osmosis/contracts/cl-vault/src/vault/any_deposit.rs b/smart-contracts/osmosis/contracts/cl-vault/src/vault/any_deposit.rs index 5ec61379d..64a743c7f 100644 --- a/smart-contracts/osmosis/contracts/cl-vault/src/vault/any_deposit.rs +++ b/smart-contracts/osmosis/contracts/cl-vault/src/vault/any_deposit.rs @@ -1,31 +1,29 @@ use cosmwasm_std::{ - attr, coin, Addr, Coin, Decimal, DepsMut, Env, Fraction, MessageInfo, Response, SubMsg, - SubMsgResult, Uint128, Uint256, + attr, coin, Addr, Coin, Decimal, DepsMut, Env, MessageInfo, Response, SubMsg, SubMsgResult, + Uint128, Uint256, }; -use osmosis_std::types::osmosis::poolmanager::v1beta1::MsgSwapExactAmountInResponse; -use osmosis_std::types::osmosis::tokenfactory::v1beta1::MsgMint; - -use crate::helpers::assert::must_pay_one_or_two; -use crate::helpers::getters::{ - get_asset0_value, get_depositable_tokens, get_single_sided_deposit_0_to_1_swap_amount, - get_single_sided_deposit_1_to_0_swap_amount, get_twap_price, +use osmosis_std::types::osmosis::{ + poolmanager::v1beta1::MsgSwapExactAmountInResponse, tokenfactory::v1beta1::MsgMint, }; -use crate::helpers::msgs::swap_msg; -use crate::query::query_total_vault_token_supply; -use crate::reply::Replies; -use crate::state::{PoolConfig, CURRENT_SWAP_ANY_DEPOSIT}; -use crate::vault::concentrated_liquidity::get_cl_pool_info; -use crate::vault::range::SwapDirection; -use crate::vault::swap::SwapParams; + use crate::{ - query::query_total_assets, - state::{POOL_CONFIG, SHARES, VAULT_DENOM}, - vault::concentrated_liquidity::get_position, + helpers::{ + assert::must_pay_one_or_two, + getters::{ + get_asset0_value, get_depositable_tokens, get_single_sided_deposit_0_to_1_swap_amount, + get_single_sided_deposit_1_to_0_swap_amount, + }, + }, + query::{query_total_assets, query_total_vault_token_supply}, + reply::Replies, + state::{CURRENT_SWAP_ANY_DEPOSIT, POOL_CONFIG, SHARES, VAULT_DENOM}, + vault::{ + concentrated_liquidity::{get_cl_pool_info, get_position}, + swap::{calculate_swap_amount, SwapDirection}, + }, ContractError, }; -use super::swap::SwapCalculationResult; - pub fn execute_any_deposit( mut deps: DepsMut, env: Env, @@ -33,7 +31,6 @@ pub fn execute_any_deposit( recipient: Option, max_slippage: Decimal, ) -> Result { - // Unwrap recipient or use caller's address let recipient = recipient.map_or(Ok(info.sender.clone()), |x| deps.api.addr_validate(&x))?; let pool_config = POOL_CONFIG.load(deps.storage)?; @@ -51,111 +48,89 @@ pub fn execute_any_deposit( let (deposit_amount_in_ratio, swappable_amount): ((Uint128, Uint128), (Uint128, Uint128)) = get_depositable_tokens(deps.branch(), token0.clone(), token1.clone())?; - // Swap logic - // TODO_FUTURE: Optimize this if conditions - if !swappable_amount.0.is_zero() { - let (swap_amount, swap_direction) = ( - // range is above current tick - if pool_details.current_tick > position.upper_tick { - swappable_amount.0 - } else { - get_single_sided_deposit_0_to_1_swap_amount( - swappable_amount.0, - position.lower_tick, - pool_details.current_tick, - position.upper_tick, - )? - }, - SwapDirection::ZeroToOne, - ); - let swap_calc_result = calculate_swap_amount( - deps, - &env, - pool_config, - swap_direction, - swap_amount, - swappable_amount, - deposit_amount_in_ratio, - max_slippage, - &recipient, - )?; - - // rest minting logic remains same - Ok(Response::new() - .add_submessage(SubMsg::reply_on_success( - swap_calc_result.swap_msg, - Replies::AnyDepositSwap.into(), - )) - .add_attributes(vec![ - attr("method", "execute"), - attr("action", "any_deposit"), - attr( - "token_in", - format!("{}{}", swap_amount, swap_calc_result.token_in_denom), - ), - attr( - "token_out_min", - format!("{}", swap_calc_result.token_out_min_amount), - ), - ])) - } else if !swappable_amount.1.is_zero() { - let (swap_amount, swap_direction) = ( - // current tick is above range - if pool_details.current_tick < position.lower_tick { - swappable_amount.1 - } else { - get_single_sided_deposit_1_to_0_swap_amount( - swappable_amount.1, - position.lower_tick, - pool_details.current_tick, - position.upper_tick, - )? - }, - SwapDirection::OneToZero, - ); - let swap_calc_result = calculate_swap_amount( - deps, - &env, - pool_config, - swap_direction, - swap_amount, - swappable_amount, - deposit_amount_in_ratio, - max_slippage, - &recipient, - )?; - - // rest minting logic remains same - Ok(Response::new() - .add_submessage(SubMsg::reply_on_success( - swap_calc_result.swap_msg, - Replies::AnyDepositSwap.into(), - )) - .add_attributes(vec![ - attr("method", "execute"), - attr("action", "any_deposit"), - attr( - "token_in", - format!("{}{}", swap_amount, swap_calc_result.token_in_denom), - ), - attr( - "token_out_min", - format!("{}", swap_calc_result.token_out_min_amount), - ), - ])) - } else { + if swappable_amount.0.is_zero() && swappable_amount.1.is_zero() { let (mint_msg, user_shares) = mint_msg_user_shares(deps, &env, &deposit_amount_in_ratio, &recipient)?; - Ok(Response::new() + return Ok(Response::new() .add_attribute("method", "execute") .add_attribute("action", "any_deposit") .add_attribute("amount0", deposit_amount_in_ratio.0) .add_attribute("amount1", deposit_amount_in_ratio.1) .add_message(mint_msg) .add_attribute("mint_shares_amount", user_shares) - .add_attribute("receiver", recipient.as_str())) + .add_attribute("receiver", recipient.as_str())); } + + // Swap logic + // TODO_FUTURE: Optimize this if conditions + let (swap_amount, swap_direction, left_over_amount) = if !swappable_amount.0.is_zero() { + // range is above current tick + let swap_amount = if pool_details.current_tick > position.upper_tick { + swappable_amount.0 + } else { + get_single_sided_deposit_0_to_1_swap_amount( + swappable_amount.0, + position.lower_tick, + pool_details.current_tick, + position.upper_tick, + )? + }; + let left_over_amount = swappable_amount.0.checked_sub(swap_amount)?; + (swap_amount, SwapDirection::ZeroToOne, left_over_amount) + } else { + // current tick is above range + let swap_amount = if pool_details.current_tick < position.lower_tick { + swappable_amount.1 + } else { + get_single_sided_deposit_1_to_0_swap_amount( + swappable_amount.1, + position.lower_tick, + pool_details.current_tick, + position.upper_tick, + )? + }; + let left_over_amount = swappable_amount.1.checked_sub(swap_amount)?; + (swap_amount, SwapDirection::OneToZero, left_over_amount) + }; + CURRENT_SWAP_ANY_DEPOSIT.save( + deps.storage, + &( + swap_direction.clone(), + left_over_amount, + recipient.clone(), + deposit_amount_in_ratio, + ), + )?; + let swap_calc_result = calculate_swap_amount( + deps, + &env, + pool_config, + swap_direction, + swap_amount, + max_slippage, + None, // TODO: check this None + 24u64, + )?; + + // rest minting logic remains same + Ok(Response::new() + .add_submessage(SubMsg::reply_on_success( + swap_calc_result.swap_msg, + Replies::AnyDepositSwap.into(), + )) + .add_attributes(vec![ + attr("method", "execute"), + attr("action", "any_deposit"), + attr( + "token_in", + format!("{}{}", swap_amount, swap_calc_result.token_in_denom), + ), + attr( + "token_out_min", + format!("{}", swap_calc_result.token_out_min_amount), + ), + ])) } pub fn handle_any_deposit_swap_reply( @@ -275,80 +250,3 @@ fn mint_msg_user_shares( Ok((mint_msg, user_shares)) } - -#[allow(clippy::too_many_arguments)] -fn calculate_swap_amount( - deps: DepsMut, - env: &Env, - pool_config: PoolConfig, - swap_direction: SwapDirection, - token_in_amount: Uint128, - swappable_amount: (Uint128, Uint128), - deposit_amount_in_ratio: (Uint128, Uint128), - max_slippage: Decimal, - recipient: &Addr, -) -> Result { - // TODO check that this math is right with spot price (numerators, denominators) if taken by legacy gamm module instead of poolmanager - // TODO check on the twap_window_seconds (taking hardcoded value for now) - let twap_price = get_twap_price(deps.storage, &deps.querier, env, 24u64)?; - let (token_in_denom, token_out_denom, token_out_ideal_amount, left_over_amount) = - match swap_direction { - SwapDirection::ZeroToOne => ( - &pool_config.token0, - &pool_config.token1, - token_in_amount - .checked_multiply_ratio(twap_price.numerator(), twap_price.denominator()), - swappable_amount.0.checked_sub(token_in_amount)?, - ), - SwapDirection::OneToZero => ( - &pool_config.token1, - &pool_config.token0, - token_in_amount - .checked_multiply_ratio(twap_price.denominator(), twap_price.numerator()), - swappable_amount.1.checked_sub(token_in_amount)?, - ), - }; - - CURRENT_SWAP_ANY_DEPOSIT.save( - deps.storage, - &( - swap_direction, - left_over_amount, - recipient.clone(), - deposit_amount_in_ratio, - ), - )?; - - let token_out_min_amount = token_out_ideal_amount? - .checked_multiply_ratio(max_slippage.numerator(), max_slippage.denominator())?; - - if !pool_config.pool_contains_token(token_in_denom) { - return Err(ContractError::BadTokenForSwap { - base_token: pool_config.token0, - quote_token: pool_config.token1, - }); - } - - // generate a swap message with recommended path as the current - // pool on which the vault is running - let swap_msg = swap_msg( - &deps, - env, - SwapParams { - pool_id: pool_config.pool_id, - token_in_amount, - token_out_min_amount, - token_in_denom: token_in_denom.clone(), - token_out_denom: token_out_denom.clone(), - forced_swap_route: None, // TODO: check this None - }, - )?; - - Ok(SwapCalculationResult { - swap_msg, - token_in_denom: token_in_denom.to_string(), - token_out_min_amount, - token_in_amount, - position_id: None, - }) -} diff --git a/smart-contracts/osmosis/contracts/cl-vault/src/vault/range.rs b/smart-contracts/osmosis/contracts/cl-vault/src/vault/range.rs index cdfc29700..0a31b3c04 100644 --- a/smart-contracts/osmosis/contracts/cl-vault/src/vault/range.rs +++ b/smart-contracts/osmosis/contracts/cl-vault/src/vault/range.rs @@ -3,25 +3,24 @@ use crate::{ assert::assert_range_admin, getters::{ get_single_sided_deposit_0_to_1_swap_amount, - get_single_sided_deposit_1_to_0_swap_amount, get_tokens_provided, get_twap_price, + get_single_sided_deposit_1_to_0_swap_amount, get_tokens_provided, get_unused_pair_balances, }, - msgs::swap_msg, }, math::tick::price_to_tick, msg::{ExecuteMsg, MergePositionMsg}, reply::Replies, state::{ - ModifyRangeState, PoolConfig, Position, SwapDepositMergeState, MODIFY_RANGE_STATE, - POOL_CONFIG, POSITION, SWAP_DEPOSIT_MERGE_STATE, + ModifyRangeState, Position, SwapDepositMergeState, MODIFY_RANGE_STATE, POOL_CONFIG, + POSITION, SWAP_DEPOSIT_MERGE_STATE, }, vault::{ - concentrated_liquidity::{create_position, get_position}, + concentrated_liquidity::{create_position, get_cl_pool_info, get_position}, merge::MergeResponse, + swap::{calculate_swap_amount, SwapDirection}, }, ContractError, }; -use cosmwasm_schema::cw_serde; use cosmwasm_std::{ attr, to_json_binary, Decimal, Decimal256, DepsMut, Env, Fraction, MessageInfo, Response, SubMsg, SubMsgResult, Uint128, @@ -32,11 +31,6 @@ use osmosis_std::types::osmosis::{ }; use std::str::FromStr; -use super::{ - concentrated_liquidity::get_cl_pool_info, - swap::{SwapCalculationResult, SwapParams}, -}; - /// This function is the entrypoint into the dsm routine that will go through the following steps /// * how much liq do we have in current range /// * so how much of each asset given liq would we have at current price @@ -284,161 +278,92 @@ pub fn do_swap_deposit_merge( let pool_config = POOL_CONFIG.load(deps.storage)?; let pool_details = get_cl_pool_info(&deps.querier, pool_config.pool_id)?; - let swap_calc_result = calculate_swap_amount( - deps, - env, - &pool_config, - pool_details.current_tick, - position_id, - balance0, - balance1, - target_lower_tick, - target_upper_tick, - twap_window_seconds, - )?; - // Start constructing the response let response = Response::new() .add_attribute("method", "reply") .add_attribute("action", "do_swap_deposit_merge"); + if balance0.is_zero() && balance1.is_zero() { + let position = POSITION.load(deps.storage)?; - // Check if there is a swap message and append accordingly - if let Some(calculated) = swap_calc_result { - Ok(response - .add_submessage(SubMsg::reply_on_success( - calculated.swap_msg, - Replies::Swap.into(), - )) - .add_attributes(vec![ - attr( - "token_in", - format!( - "{}{}", - calculated.token_in_amount, calculated.token_in_denom - ), - ), - attr("token_out_min", calculated.token_out_min_amount.to_string()), - ])) - } else { - // If no swap message, add a new position attribute - Ok(response.add_attribute("new_position", position_id.unwrap().to_string())) + // if we have not tokens to swap, that means all tokens we correctly used in the create position + // this means we can save the position id of the first create_position + POSITION.save( + deps.storage, + &Position { + position_id: position_id.expect("position id should be set if no swap is needed"), + join_time: position.join_time, + claim_after: position.claim_after, + }, + )?; + + SWAP_DEPOSIT_MERGE_STATE.remove(deps.storage); + return Ok(response.add_attribute("new_position", position_id.unwrap().to_string())); } -} -#[allow(clippy::too_many_arguments)] -fn calculate_swap_amount( - deps: DepsMut, - env: Env, - pool_config: &PoolConfig, - current_tick: i64, - position_id: Option, - balance0: Uint128, - balance1: Uint128, - target_lower_tick: i64, - target_upper_tick: i64, - twap_window_seconds: u64, -) -> Result, ContractError> { + let mrs = MODIFY_RANGE_STATE.load(deps.storage)?.unwrap(); + //TODO: further optimizations can be made by increasing the swap amount by half of our expected slippage, // to reduce the total number of non-deposited tokens that we will then need to refund let (token_in_amount, swap_direction) = if !balance0.is_zero() { ( // range is above current tick - if current_tick > target_upper_tick { + if pool_details.current_tick > target_upper_tick { balance0 } else { get_single_sided_deposit_0_to_1_swap_amount( balance0, target_lower_tick, - current_tick, + pool_details.current_tick, target_upper_tick, )? }, SwapDirection::ZeroToOne, ) - } else if !balance1.is_zero() { + } else { ( // current tick is above range - if current_tick < target_lower_tick { + if pool_details.current_tick < target_lower_tick { balance1 } else { get_single_sided_deposit_1_to_0_swap_amount( balance1, target_lower_tick, - current_tick, + pool_details.current_tick, target_upper_tick, )? }, SwapDirection::OneToZero, ) - } else { - // Load the current Position to extract join_time and claim_after which is unchangeable in this context - let position = POSITION.load(deps.storage)?; - - // if we have not tokens to swap, that means all tokens we correctly used in the create position - // this means we can save the position id of the first create_position - POSITION.save( - deps.storage, - &Position { - // if position not found, then we should panic here anyway ? - position_id: position_id.expect("position id should be set if no swap is needed"), - join_time: position.join_time, - claim_after: position.claim_after, - }, - )?; - - SWAP_DEPOSIT_MERGE_STATE.remove(deps.storage); - - return Ok(None); }; - - // TODO: check that this math is right with spot price (numerators, denominators) if taken by legacy gamm module instead of poolmanager - let twap_price = get_twap_price(deps.storage, &deps.querier, &env, twap_window_seconds)?; - let (token_in_denom, token_out_denom, token_out_ideal_amount) = match swap_direction { - SwapDirection::ZeroToOne => ( - &pool_config.token0, - &pool_config.token1, - token_in_amount - .checked_multiply_ratio(twap_price.numerator(), twap_price.denominator()), - ), - SwapDirection::OneToZero => ( - &pool_config.token1, - &pool_config.token0, - token_in_amount - .checked_multiply_ratio(twap_price.denominator(), twap_price.numerator()), - ), - }; - - let mrs = MODIFY_RANGE_STATE.load(deps.storage)?.unwrap(); - let token_out_min_amount = token_out_ideal_amount? - .checked_multiply_ratio(mrs.max_slippage.numerator(), mrs.max_slippage.denominator())?; - - if !pool_config.pool_contains_token(token_in_denom) { - return Err(ContractError::BadTokenForSwap { - base_token: pool_config.token0.clone(), - quote_token: pool_config.token1.clone(), - }); - } - - let swap_msg = swap_msg( - &deps, + let swap_calc_result = calculate_swap_amount( + deps, &env, - SwapParams { - pool_id: pool_config.pool_id, - token_in_amount, - token_out_min_amount, - token_in_denom: token_in_denom.clone(), - token_out_denom: token_out_denom.to_string(), - forced_swap_route: mrs.forced_swap_route, - }, + pool_config, + swap_direction, + token_in_amount, + mrs.max_slippage, + mrs.forced_swap_route, + twap_window_seconds, )?; - Ok(Some(SwapCalculationResult { - swap_msg, - token_in_denom: token_in_denom.to_string(), - token_in_amount, - token_out_min_amount, - position_id: None, - })) + Ok(response + .add_submessage(SubMsg::reply_on_success( + swap_calc_result.swap_msg, + Replies::Swap.into(), + )) + .add_attributes(vec![ + attr( + "token_in", + format!( + "{}{}", + swap_calc_result.token_in_amount, swap_calc_result.token_in_denom + ), + ), + attr( + "token_out_min", + swap_calc_result.token_out_min_amount.to_string(), + ), + ])) } // do deposit @@ -570,12 +495,6 @@ pub fn handle_merge_reply( .add_attribute("status", "success")) } -#[cw_serde] -pub enum SwapDirection { - ZeroToOne, - OneToZero, -} - #[cfg(test)] mod tests { use std::str::FromStr; @@ -658,59 +577,6 @@ mod tests { let info = mock_info("addr0000", &[]); let env = mock_env(); - // let mut deps = mock_deps_with_querier_with_balance( - // &info, - // &[(MOCK_CONTRACT_ADDR, &[coin(11234, "token1")])], - // ); - - // // moving into a range - // MODIFY_RANGE_STATE - // .save( - // deps.as_mut().storage, - // &Some(crate::state::ModifyRangeState { - // lower_tick: 100, - // upper_tick: 1000, // since both times we are moving into range and in the quasarquerier we configured the current_tick as 500, this would mean we are trying to move into range - // new_range_position_ids: vec![], - // max_slippage: Decimal::zero(), - // ratio_of_swappable_funds_to_use: Decimal::one(), - // twap_window_seconds: 45, - // recommended_swap_route: None, - // force_swap_route: false, - // }), - // ) - // .unwrap(); - - // // Reply - // //first test fully one-sided withdraw - // let data = SubMsgResult::Ok(SubMsgResponse { - // events: vec![], - // data: Some( - // MsgWithdrawPositionResponse { - // amount0: "0".to_string(), - // amount1: "10000".to_string(), - // } - // .try_into() - // .unwrap(), - // ), - // }); - - // let res = super::handle_withdraw_position_reply(deps.as_mut(), env.clone(), data).unwrap(); - - // // verify that we went straight to swap_deposit_merge - // assert_eq!(res.messages.len(), 1); - // assert_eq!(res.attributes[1].value, "do_swap_deposit_merge"); - // // check that our token1 attribute is incremented with the local balance - // assert_eq!( - // res.attributes - // .iter() - // .find(|a| { a.key == "token_in" }) - // .unwrap() - // .value, - // "5962token1" - // ); - - // SECOND CASE STARTS HERE - let mut deps = mock_deps_with_querier_with_balance( &info, &[( @@ -719,7 +585,6 @@ mod tests { )], ); - // moving into a range MODIFY_RANGE_STATE .save( deps.as_mut().storage, diff --git a/smart-contracts/osmosis/contracts/cl-vault/src/vault/swap.rs b/smart-contracts/osmosis/contracts/cl-vault/src/vault/swap.rs index d2d5e15e3..05f49eb5d 100644 --- a/smart-contracts/osmosis/contracts/cl-vault/src/vault/swap.rs +++ b/smart-contracts/osmosis/contracts/cl-vault/src/vault/swap.rs @@ -1,11 +1,18 @@ -use cosmwasm_std::{CosmosMsg, DepsMut, Env, Fraction, MessageInfo, Response, Uint128}; +use cosmwasm_std::{CosmosMsg, Decimal, DepsMut, Env, Fraction, MessageInfo, Response, Uint128}; use osmosis_std::types::osmosis::poolmanager::v1beta1::SwapAmountInRoute; -use crate::helpers::assert::assert_range_admin; -use crate::helpers::msgs::swap_msg; -use crate::msg::SwapOperation; -use crate::state::POOL_CONFIG; -use crate::{state::VAULT_CONFIG, ContractError}; +use crate::{ + helpers::{assert::assert_range_admin, getters::get_twap_price, msgs::swap_msg}, + msg::SwapOperation, + state::{PoolConfig, POOL_CONFIG, VAULT_CONFIG}, + ContractError, +}; + +#[cosmwasm_schema::cw_serde] +pub enum SwapDirection { + ZeroToOne, + OneToZero, +} /// SwapCalculationResult holds the result of a swap calculation pub struct SwapCalculationResult { @@ -13,7 +20,6 @@ pub struct SwapCalculationResult { pub token_in_denom: String, pub token_in_amount: Uint128, pub token_out_min_amount: Uint128, - pub position_id: Option, } /// SwapParams holds the parameters for a swap @@ -117,53 +123,65 @@ pub fn execute_swap_non_vault_funds( .add_attribute("action", "swap_non_vault_funds")) } -/// estimate_swap can be used to pass correct token_out_min_amount values into swap() -/// for now this function can only be used for our pool -/// this will likely be expanded once we allow arbitrary pool swaps -// pub fn _estimate_swap( -// querier: &QuerierWrapper, -// storage: &mut dyn Storage, -// _env: &Env, -// token_in_amount: Uint128, -// token_in_denom: &String, -// _token_out_min_amount: Uint128, -// ) -> Result { -// let pool_config = POOL_CONFIG.load(storage)?; - -// if !pool_config.pool_contains_token(token_in_denom) { -// return Err(ContractError::BadTokenForSwap { -// base_token: pool_config.token0, -// quote_token: pool_config.token1, -// }); -// } - -// // get token_out_denom -// let token_out_denom = if *token_in_denom == pool_config.token0 { -// pool_config.token1 -// } else { -// pool_config.token0 -// }; - -// // we will only ever have a route length of one, this will likely change once we start selecting different routes -// let pool_route = SwapAmountInRoute { -// pool_id: pool_config.pool_id, -// token_out_denom: token_out_denom.to_string(), -// }; - -// let pm_querier = -// osmosis_std::types::osmosis::poolmanager::v1beta1::PoolmanagerQuerier::new(querier); - -// let result = pm_querier.estimate_swap_exact_amount_in( -// pool_config.pool_id, -// token_in_amount.to_string() + token_in_denom, -// vec![pool_route], -// )?; - -// Ok(Coin { -// denom: token_out_denom, -// amount: Uint128::from_str(&result.token_out_amount)?, -// }) -// } +#[allow(clippy::too_many_arguments)] +pub fn calculate_swap_amount( + deps: DepsMut, + env: &Env, + pool_config: PoolConfig, + swap_direction: SwapDirection, + token_in_amount: Uint128, + max_slippage: Decimal, + forced_swap_route: Option>, + twap_window_seconds: u64, +) -> Result { + let twap_price = get_twap_price(deps.storage, &deps.querier, env, twap_window_seconds)?; + let (token_in_denom, token_out_denom, token_out_ideal_amount) = match swap_direction { + SwapDirection::ZeroToOne => ( + &pool_config.token0, + &pool_config.token1, + token_in_amount + .checked_multiply_ratio(twap_price.numerator(), twap_price.denominator()), + ), + SwapDirection::OneToZero => ( + &pool_config.token1, + &pool_config.token0, + token_in_amount + .checked_multiply_ratio(twap_price.denominator(), twap_price.numerator()), + ), + }; + + let token_out_min_amount = token_out_ideal_amount? + .checked_multiply_ratio(max_slippage.numerator(), max_slippage.denominator())?; + + if !pool_config.pool_contains_token(token_in_denom) { + return Err(ContractError::BadTokenForSwap { + base_token: pool_config.token0, + quote_token: pool_config.token1, + }); + } + + // generate a swap message with recommended path as the current + // pool on which the vault is running + let swap_msg = swap_msg( + &deps, + env, + SwapParams { + pool_id: pool_config.pool_id, + token_in_amount, + token_out_min_amount, + token_in_denom: token_in_denom.clone(), + token_out_denom: token_out_denom.clone(), + forced_swap_route, + }, + )?; + + Ok(SwapCalculationResult { + swap_msg, + token_in_denom: token_in_denom.to_string(), + token_out_min_amount, + token_in_amount, + }) +} #[cfg(test)] mod tests { From 0641702744c6d69776e9c98701ed071a3248c2e8 Mon Sep 17 00:00:00 2001 From: lubkoll <11710767+lubkoll@users.noreply.github.com> Date: Mon, 29 Jul 2024 09:51:45 +0000 Subject: [PATCH 4/4] Remove code duplication in any_deposit and exact_deposit (#717) --- .../contracts/cl-vault/src/helpers/assert.rs | 57 ++-- .../contracts/cl-vault/src/helpers/getters.rs | 241 +++++++++++++++- .../contracts/cl-vault/src/instantiate.rs | 2 +- .../cl-vault/src/vault/any_deposit.rs | 144 ++------- .../cl-vault/src/vault/exact_deposit.rs | 273 +++--------------- 5 files changed, 323 insertions(+), 394 deletions(-) diff --git a/smart-contracts/osmosis/contracts/cl-vault/src/helpers/assert.rs b/smart-contracts/osmosis/contracts/cl-vault/src/helpers/assert.rs index f5c92932a..6152fc149 100644 --- a/smart-contracts/osmosis/contracts/cl-vault/src/helpers/assert.rs +++ b/smart-contracts/osmosis/contracts/cl-vault/src/helpers/assert.rs @@ -1,4 +1,4 @@ -use cosmwasm_std::{coin, Addr, Coin, Deps, MessageInfo, Storage}; +use cosmwasm_std::{coin, Addr, Coin, Deps, Storage}; use crate::{ state::{ADMIN_ADDRESS, RANGE_ADMIN}, @@ -26,25 +26,23 @@ pub fn assert_range_admin(storage: &mut dyn Storage, sender: &Addr) -> Result<() /// Returns the Coin of the needed denoms in the order given in denoms pub(crate) fn must_pay_one_or_two( - info: &MessageInfo, + funds: &[Coin], denoms: (String, String), ) -> Result<(Coin, Coin), ContractError> { - if info.funds.len() != 2 && info.funds.len() != 1 { + if funds.len() != 2 && funds.len() != 1 { return Err(ContractError::IncorrectAmountFunds); } - let token0 = info - .funds - .clone() - .into_iter() + let token0 = funds + .iter() .find(|coin| coin.denom == denoms.0) + .cloned() .unwrap_or(coin(0, denoms.0)); - let token1 = info - .funds - .clone() - .into_iter() + let token1 = funds + .iter() .find(|coin| coin.denom == denoms.1) + .cloned() .unwrap_or(coin(0, denoms.1)); Ok((token0, token1)) @@ -76,7 +74,7 @@ pub(crate) fn must_pay_one_or_two_from_balance( #[cfg(test)] mod tests { - use cosmwasm_std::{coin, Addr}; + use cosmwasm_std::coin; use super::*; @@ -84,12 +82,9 @@ mod tests { fn must_pay_one_or_two_works_ordered() { let expected0 = coin(100, "uatom"); let expected1 = coin(200, "uosmo"); - let info = MessageInfo { - sender: Addr::unchecked("sender"), - funds: vec![expected0.clone(), expected1.clone()], - }; + let funds = vec![expected0.clone(), expected1.clone()]; let (token0, token1) = - must_pay_one_or_two(&info, ("uatom".to_string(), "uosmo".to_string())).unwrap(); + must_pay_one_or_two(&funds, ("uatom".to_string(), "uosmo".to_string())).unwrap(); assert_eq!(expected0, token0); assert_eq!(expected1, token1); } @@ -98,12 +93,9 @@ mod tests { fn must_pay_one_or_two_works_unordered() { let expected0 = coin(100, "uatom"); let expected1 = coin(200, "uosmo"); - let info = MessageInfo { - sender: Addr::unchecked("sender"), - funds: vec![expected1.clone(), expected0.clone()], - }; + let funds = vec![expected0.clone(), expected1.clone()]; let (token0, token1) = - must_pay_one_or_two(&info, ("uatom".to_string(), "uosmo".to_string())).unwrap(); + must_pay_one_or_two(&funds, ("uatom".to_string(), "uosmo".to_string())).unwrap(); assert_eq!(expected0, token0); assert_eq!(expected1, token1); } @@ -112,31 +104,22 @@ mod tests { fn must_pay_one_or_two_rejects_three() { let expected0 = coin(100, "uatom"); let expected1 = coin(200, "uosmo"); - let info = MessageInfo { - sender: Addr::unchecked("sender"), - funds: vec![expected1, expected0, coin(200, "uqsr")], - }; + let funds = vec![expected0, expected1, coin(200, "uqsr")]; let _err = - must_pay_one_or_two(&info, ("uatom".to_string(), "uosmo".to_string())).unwrap_err(); + must_pay_one_or_two(&funds, ("uatom".to_string(), "uosmo".to_string())).unwrap_err(); } #[test] fn must_pay_one_or_two_accepts_second_token() { - let info = MessageInfo { - sender: Addr::unchecked("sender"), - funds: vec![coin(200, "uosmo")], - }; - let res = must_pay_one_or_two(&info, ("uatom".to_string(), "uosmo".to_string())).unwrap(); + let funds = vec![coin(200, "uosmo")]; + let res = must_pay_one_or_two(&funds, ("uatom".to_string(), "uosmo".to_string())).unwrap(); assert_eq!((coin(0, "uatom"), coin(200, "uosmo")), res) } #[test] fn must_pay_one_or_two_accepts_first_token() { - let info = MessageInfo { - sender: Addr::unchecked("sender"), - funds: vec![coin(200, "uatom")], - }; - let res = must_pay_one_or_two(&info, ("uatom".to_string(), "uosmo".to_string())).unwrap(); + let funds = vec![coin(200, "uatom")]; + let res = must_pay_one_or_two(&funds, ("uatom".to_string(), "uosmo".to_string())).unwrap(); assert_eq!((coin(200, "uatom"), coin(0, "uosmo")), res) } } diff --git a/smart-contracts/osmosis/contracts/cl-vault/src/helpers/getters.rs b/smart-contracts/osmosis/contracts/cl-vault/src/helpers/getters.rs index e6ba63f4a..adc3d785c 100644 --- a/smart-contracts/osmosis/contracts/cl-vault/src/helpers/getters.rs +++ b/smart-contracts/osmosis/contracts/cl-vault/src/helpers/getters.rs @@ -1,5 +1,3 @@ -use crate::math::tick::tick_to_price; -use crate::state::{PoolConfig, RANGE_ADMIN}; use std::str::FromStr; use osmosis_std::shim::Timestamp as OsmoTimestamp; @@ -7,7 +5,12 @@ use osmosis_std::types::osmosis::poolmanager::v1beta1::PoolmanagerQuerier; use osmosis_std::types::osmosis::twap::v1beta1::TwapQuerier; use crate::vault::concentrated_liquidity::get_position; -use crate::{state::POOL_CONFIG, ContractError}; +use crate::{ + helpers::assert::must_pay_one_or_two, + math::tick::tick_to_price, + state::{PoolConfig, POOL_CONFIG, RANGE_ADMIN}, + ContractError, +}; use cosmwasm_std::{ Addr, Coin, Decimal, Decimal256, Deps, DepsMut, Env, Fraction, QuerierWrapper, Storage, Uint128, Uint256, @@ -64,10 +67,24 @@ pub fn get_twap_price( Ok(Decimal::from_str(&twap_price.arithmetic_twap)?) } -/// Calculate the amount of tokens that can be deposited while maintaining the current position ratio in the vault. #[allow(clippy::type_complexity)] pub fn get_depositable_tokens( - deps: DepsMut, + deps: &DepsMut, + funds: &[Coin], + pool_config: &PoolConfig, +) -> Result<((Uint128, Uint128), (Uint128, Uint128)), ContractError> { + let (token0, token1) = must_pay_one_or_two( + funds, + (pool_config.token0.clone(), pool_config.token1.clone()), + )?; + + get_depositable_tokens_impl(deps, token0, token1) +} + +/// Calculate the amount of tokens that can be deposited while maintaining the current position ratio in the vault. +#[allow(clippy::type_complexity)] +fn get_depositable_tokens_impl( + deps: &DepsMut, token0: Coin, token1: Coin, ) -> Result<((Uint128, Uint128), (Uint128, Uint128)), ContractError> { @@ -247,14 +264,84 @@ pub fn get_tokens_provided( #[cfg(test)] mod tests { - use std::collections::HashMap; - use cosmwasm_std::testing::mock_dependencies; + use std::collections::HashMap; + use std::{marker::PhantomData, str::FromStr}; + + use cosmwasm_std::{ + coin, + testing::{MockApi, MockStorage, MOCK_CONTRACT_ADDR}, + Coin, Decimal256, Empty, OwnedDeps, + }; + + use osmosis_std::types::{ + cosmos::base::v1beta1::Coin as OsmoCoin, + osmosis::concentratedliquidity::v1beta1::{ + FullPositionBreakdown, Position as OsmoPosition, + }, + }; + + use crate::{ + state::{Position, POSITION}, + test_helpers::QuasarQuerier, + }; use crate::math::tick::{build_tick_exp_cache, price_to_tick}; use super::*; + fn mock_deps_with_position( + token0: Option, + token1: Option, + ) -> OwnedDeps { + let position_id = 2; + + let mut deps = OwnedDeps { + storage: MockStorage::default(), + api: MockApi::default(), + querier: QuasarQuerier::new( + FullPositionBreakdown { + position: Some(OsmoPosition { + position_id, + address: MOCK_CONTRACT_ADDR.to_string(), + pool_id: 1, + lower_tick: 100, + upper_tick: 1000, + join_time: None, + liquidity: "1000000.2".to_string(), + }), + asset0: token0.map(|c| c.into()), + asset1: token1.map(|c| c.into()), + claimable_spread_rewards: vec![ + OsmoCoin { + denom: "token0".to_string(), + amount: "100".to_string(), + }, + OsmoCoin { + denom: "token1".to_string(), + amount: "100".to_string(), + }, + ], + claimable_incentives: vec![], + forfeited_incentives: vec![], + }, + 500, + ), + custom_query_type: PhantomData, + }; + POSITION + .save( + deps.as_mut().storage, + &Position { + position_id, + join_time: 0, + claim_after: None, + }, + ) + .unwrap(); + deps + } + #[test] fn test_0_to_1_swap() { let mut deps = mock_dependencies(); @@ -371,4 +458,144 @@ mod tests { assert_eq!(swap_amount, result); } } + + #[test] + fn test_position_in_both_asset() { + let token0 = Coin { + denom: "token0".to_string(), + amount: Uint128::new(1_000_000_000u128), + }; + let token1 = Coin { + denom: "token1".to_string(), + amount: Uint128::new(100_000_000_000_000_000_000_000_000_000u128), + }; + + let mut deps = mock_deps_with_position(Some(token0.clone()), Some(token1.clone())); + let mutdeps = deps.as_mut(); + + let result = get_depositable_tokens_impl(&mutdeps, token0, token1).unwrap(); + assert_eq!( + result, + ( + ( + Uint128::zero(), + Uint128::new(100_000_000_000_000_000_000_000_000_000u128) + ), + (Uint128::new(1_000_000_000u128), Uint128::zero()) + ) + ); + } + + #[test] + fn test_position_in_asset1_only() { + let token0 = Coin { + denom: "token0".to_string(), + amount: Uint128::new(50), + }; + let token1 = Coin { + denom: "token1".to_string(), + amount: Uint128::new(100), + }; + + // Osmosis is not using None for missing assets, but Some with amount 0, so we need to mimic that here + let mut deps = mock_deps_with_position( + Some(Coin { + denom: "token0".to_string(), + amount: Uint128::zero(), + }), + Some(token1.clone()), + ); + + let result = get_depositable_tokens_impl(&deps.as_mut(), token0, token1).unwrap(); + assert_eq!( + result, + ( + (Uint128::zero(), Uint128::new(100)), + (Uint128::new(50), Uint128::zero()) + ) + ); + } + + #[test] + fn test_position_in_asset0_only() { + let token0 = Coin { + denom: "token0".to_string(), + amount: Uint128::new(50), + }; + let token1 = Coin { + denom: "token1".to_string(), + amount: Uint128::new(100), + }; + + // Osmosis is not using None for missing assets, but Some with amount 0, so we need to mimic that here + let mut deps = mock_deps_with_position( + Some(token0.clone()), + Some(Coin { + denom: "token1".to_string(), + amount: Uint128::zero(), + }), + ); + + let result = get_depositable_tokens_impl(&deps.as_mut(), token0, token1).unwrap(); + assert_eq!( + result, + ( + (Uint128::new(50), Uint128::zero()), + (Uint128::zero(), Uint128::new(100)) + ) + ); + } + + #[test] + fn test_both_assets_present_token0_limiting() { + let token0 = Coin { + denom: "token0".to_string(), + amount: Uint128::new(50), + }; + let token1 = Coin { + denom: "token1".to_string(), + amount: Uint128::new(100), + }; + + // we use a ratio of 1/2 + let mut deps = mock_deps_with_position(Some(token0.clone()), Some(token1.clone())); + + let result = + get_depositable_tokens_impl(&deps.as_mut(), coin(2000, "token0"), coin(5000, "token1")) + .unwrap(); + assert_eq!( + result, + ( + (Uint128::new(2000), Uint128::new(4000)), + (Uint128::zero(), Uint128::new(1000)) + ) + ); + } + + #[test] + fn test_both_assets_present_token1_limiting() { + let token0 = Coin { + denom: "token0".to_string(), + amount: Uint128::new(50), + }; + let token1 = Coin { + denom: "token1".to_string(), + amount: Uint128::new(100), + }; + + // we use a ratio of 1/2 + let mut deps = mock_deps_with_position(Some(token0.clone()), Some(token1.clone())); + let mutdeps = deps.as_mut(); + + let result = + get_depositable_tokens_impl(&mutdeps, coin(2000, "token0"), coin(3000, "token1")) + .unwrap(); + assert_eq!( + result, + ( + (Uint128::new(1500), Uint128::new(3000)), + (Uint128::new(500), Uint128::zero()) + ) + ); + } } diff --git a/smart-contracts/osmosis/contracts/cl-vault/src/instantiate.rs b/smart-contracts/osmosis/contracts/cl-vault/src/instantiate.rs index 32d5e5b86..86830cda7 100644 --- a/smart-contracts/osmosis/contracts/cl-vault/src/instantiate.rs +++ b/smart-contracts/osmosis/contracts/cl-vault/src/instantiate.rs @@ -81,7 +81,7 @@ pub fn handle_instantiate( .into(); // in order to create the initial position, we need some funds to throw in there, these funds should be seen as burned - let (initial0, initial1) = must_pay_one_or_two(&info, (pool.token0, pool.token1))?; + let (initial0, initial1) = must_pay_one_or_two(&info.funds, (pool.token0, pool.token1))?; let create_position_msg = create_position( deps, diff --git a/smart-contracts/osmosis/contracts/cl-vault/src/vault/any_deposit.rs b/smart-contracts/osmosis/contracts/cl-vault/src/vault/any_deposit.rs index 64a743c7f..92ea665e2 100644 --- a/smart-contracts/osmosis/contracts/cl-vault/src/vault/any_deposit.rs +++ b/smart-contracts/osmosis/contracts/cl-vault/src/vault/any_deposit.rs @@ -1,30 +1,24 @@ use cosmwasm_std::{ - attr, coin, Addr, Coin, Decimal, DepsMut, Env, MessageInfo, Response, SubMsg, SubMsgResult, - Uint128, Uint256, -}; -use osmosis_std::types::osmosis::{ - poolmanager::v1beta1::MsgSwapExactAmountInResponse, tokenfactory::v1beta1::MsgMint, + attr, coin, Coin, Decimal, DepsMut, Env, MessageInfo, Response, SubMsg, SubMsgResult, Uint128, }; +use osmosis_std::types::osmosis::poolmanager::v1beta1::MsgSwapExactAmountInResponse; use crate::{ - helpers::{ - assert::must_pay_one_or_two, - getters::{ - get_asset0_value, get_depositable_tokens, get_single_sided_deposit_0_to_1_swap_amount, - get_single_sided_deposit_1_to_0_swap_amount, - }, + helpers::getters::{ + get_depositable_tokens, get_single_sided_deposit_0_to_1_swap_amount, + get_single_sided_deposit_1_to_0_swap_amount, }, - query::{query_total_assets, query_total_vault_token_supply}, reply::Replies, - state::{CURRENT_SWAP_ANY_DEPOSIT, POOL_CONFIG, SHARES, VAULT_DENOM}, + state::{CURRENT_SWAP_ANY_DEPOSIT, POOL_CONFIG}, vault::{ concentrated_liquidity::{get_cl_pool_info, get_position}, + exact_deposit::execute_deposit, swap::{calculate_swap_amount, SwapDirection}, }, ContractError, }; -pub fn execute_any_deposit( +pub(crate) fn execute_any_deposit( mut deps: DepsMut, env: Env, info: MessageInfo, @@ -39,27 +33,21 @@ pub fn execute_any_deposit( .position .ok_or(ContractError::MissingPosition {})?; - let (token0, token1) = must_pay_one_or_two( - &info, - (pool_config.token0.clone(), pool_config.token1.clone()), - )?; - // get the amount of funds we can deposit from this ratio let (deposit_amount_in_ratio, swappable_amount): ((Uint128, Uint128), (Uint128, Uint128)) = - get_depositable_tokens(deps.branch(), token0.clone(), token1.clone())?; + get_depositable_tokens(&deps.branch(), &info.funds, &pool_config)?; if swappable_amount.0.is_zero() && swappable_amount.1.is_zero() { - let (mint_msg, user_shares) = - mint_msg_user_shares(deps, &env, &deposit_amount_in_ratio, &recipient)?; - - return Ok(Response::new() - .add_attribute("method", "execute") - .add_attribute("action", "any_deposit") - .add_attribute("amount0", deposit_amount_in_ratio.0) - .add_attribute("amount1", deposit_amount_in_ratio.1) - .add_message(mint_msg) - .add_attribute("mint_shares_amount", user_shares) - .add_attribute("receiver", recipient.as_str())); + return execute_deposit( + &mut deps, + env, + recipient, + deposit_amount_in_ratio, + ( + coin(0u128, pool_config.token0), + coin(0u128, pool_config.token1), + ), + ); } // Swap logic @@ -143,8 +131,7 @@ pub fn handle_any_deposit_swap_reply( let (swap_direction, left_over_amount, recipient, deposit_amount_in_ratio) = CURRENT_SWAP_ANY_DEPOSIT.load(deps.storage)?; - - let pool_config = POOL_CONFIG.load(deps.storage)?; + CURRENT_SWAP_ANY_DEPOSIT.remove(deps.storage); // get post swap balances to create positions with let (balance0, balance1): (Uint128, Uint128) = match swap_direction { @@ -158,7 +145,7 @@ pub fn handle_any_deposit_swap_reply( ), }; - // Create the tuple for minting coins + let pool_config = POOL_CONFIG.load(deps.storage)?; let coins_to_mint_for = ( Coin { denom: pool_config.token0.clone(), @@ -170,83 +157,14 @@ pub fn handle_any_deposit_swap_reply( }, ); - let (mint_msg, user_shares) = mint_msg_user_shares( - deps.branch(), - &env, - &(coins_to_mint_for.0.amount, coins_to_mint_for.1.amount), - &recipient, - )?; - - CURRENT_SWAP_ANY_DEPOSIT.remove(deps.storage); - - Ok(Response::new() - .add_attribute("method", "reply") - .add_attribute("action", "handle_any_deposit_swap") - .add_attribute("amount0", balance0) - .add_attribute("amount1", balance1) - .add_message(mint_msg) - .add_attribute("mint_shares_amount", user_shares) - .add_attribute("receiver", recipient.as_str())) -} - -fn mint_msg_user_shares( - deps: DepsMut, - env: &Env, - deposit_amount_in_ratio: &(Uint128, Uint128), - recipient: &Addr, -) -> Result<(MsgMint, Uint128), ContractError> { - // calculate the amount of shares we can mint for this - let total_assets = query_total_assets(deps.as_ref(), env.clone())?; - let total_assets_value = get_asset0_value( - deps.storage, - &deps.querier, - total_assets.token0.amount, - total_assets.token1.amount, - )?; - - let vault_denom = VAULT_DENOM.load(deps.storage)?; - let total_vault_shares: Uint256 = query_total_vault_token_supply(deps.as_ref())?.total.into(); - - let user_value = get_asset0_value( - deps.storage, - &deps.querier, - deposit_amount_in_ratio.0, - deposit_amount_in_ratio.1, - )?; - - // total_vault_shares.is_zero() should never be zero. This should ideally always enter the else and we are just sanity checking. - let user_shares: Uint128 = if total_vault_shares.is_zero() { - user_value - } else { - total_vault_shares - .checked_mul(user_value.into())? - .checked_div(total_assets_value.into())? - .try_into()? - }; - - // save the shares in the user map - SHARES.update( - deps.storage, - recipient.clone(), - |old| -> Result { - if let Some(existing_user_shares) = old { - Ok(user_shares + existing_user_shares) - } else { - Ok(user_shares) - } - }, - )?; - - // TODO the locking of minted shares is a band-aid for giving out rewards to users, - // once tokenfactory has send hooks, we can remove the lockup and have the users - // own the shares in their balance - // we mint shares to the contract address here, so we can lock those shares for the user later in the same call - // this is blocked by Osmosis v17 update - let mint_msg = MsgMint { - sender: env.clone().contract.address.to_string(), - amount: Some(coin(user_shares.into(), vault_denom).into()), - mint_to_address: env.clone().contract.address.to_string(), - }; - - Ok((mint_msg, user_shares)) + execute_deposit( + &mut deps, + env, + recipient, + (coins_to_mint_for.0.amount, coins_to_mint_for.1.amount), + ( + coin(0u128, pool_config.token0), + coin(0u128, pool_config.token1), + ), + ) } diff --git a/smart-contracts/osmosis/contracts/cl-vault/src/vault/exact_deposit.rs b/smart-contracts/osmosis/contracts/cl-vault/src/vault/exact_deposit.rs index 8378b3e28..1dee1568d 100644 --- a/smart-contracts/osmosis/contracts/cl-vault/src/vault/exact_deposit.rs +++ b/smart-contracts/osmosis/contracts/cl-vault/src/vault/exact_deposit.rs @@ -1,8 +1,7 @@ -use cosmwasm_std::{coin, DepsMut, Env, MessageInfo, Response, Uint128, Uint256}; +use cosmwasm_std::{coin, Addr, Coin, DepsMut, Env, MessageInfo, Response, Uint128, Uint256}; use osmosis_std::types::osmosis::tokenfactory::v1beta1::MsgMint; -use crate::helpers::assert::must_pay_one_or_two; use crate::helpers::getters::{get_asset0_value, get_depositable_tokens}; use crate::helpers::msgs::refund_bank_msg; use crate::query::query_total_vault_token_supply; @@ -12,29 +11,49 @@ use crate::{ ContractError, }; -/// Try to deposit as much user funds as we can in the current ratio of the vault and -/// refund the rest to the caller. pub(crate) fn execute_exact_deposit( mut deps: DepsMut, env: Env, info: MessageInfo, recipient: Option, ) -> Result { - // Unwrap recipient or use caller's address let recipient = recipient.map_or(Ok(info.sender.clone()), |x| deps.api.addr_validate(&x))?; - - let pool = POOL_CONFIG.load(deps.storage)?; - let (token0, token1) = must_pay_one_or_two(&info, (pool.token0.clone(), pool.token1.clone()))?; - + let pool_config = POOL_CONFIG.load(deps.storage)?; // get the amount of funds we can deposit from this ratio let (deposit, refund): ((Uint128, Uint128), (Uint128, Uint128)) = - get_depositable_tokens(deps.branch(), token0.clone(), token1.clone())?; + get_depositable_tokens(&deps, &info.funds, &pool_config)?; + execute_deposit( + &mut deps, + env, + recipient, + deposit, + ( + coin(refund.0.into(), pool_config.token0), + coin(refund.1.into(), pool_config.token1), + ), + ) +} + +/// Try to deposit as much user funds as we can in the current ratio of the vault and +/// refund the rest to the caller. +pub(crate) fn execute_deposit( + deps: &mut DepsMut, + env: Env, + recipient: Addr, + deposit: (Uint128, Uint128), + refund: (Coin, Coin), +) -> Result { let vault_denom = VAULT_DENOM.load(deps.storage)?; let total_vault_shares: Uint256 = query_total_vault_token_supply(deps.as_ref())?.total.into(); let user_value = get_asset0_value(deps.storage, &deps.querier, deposit.0, deposit.1)?; - let refund_value = get_asset0_value(deps.storage, &deps.querier, refund.0, refund.1)?; + let refund_value = get_asset0_value( + deps.storage, + &deps.querier, + refund.0.amount, + refund.1.amount, + )?; // calculate the amount of shares we can mint for this let total_assets = query_total_assets(deps.as_ref(), env.clone())?; @@ -86,18 +105,15 @@ pub(crate) fn execute_exact_deposit( let mut resp = Response::new() .add_attribute("method", "execute") - .add_attribute("action", "exact_deposit") + .add_attribute("action", "deposit") .add_attribute("amount0", deposit.0) .add_attribute("amount1", deposit.1) .add_message(mint_msg) .add_attribute("mint_shares_amount", user_shares) .add_attribute("receiver", recipient.as_str()); - if let Some((bank_msg, bank_attr)) = refund_bank_msg( - recipient, - Some(coin(refund.0.u128(), pool.token0)), - Some(coin(refund.1.u128(), pool.token1)), - )? { + if let Some((bank_msg, bank_attr)) = refund_bank_msg(recipient, Some(refund.0), Some(refund.1))? + { resp = resp.add_message(bank_msg).add_attributes(bank_attr); } @@ -106,167 +122,18 @@ pub(crate) fn execute_exact_deposit( #[cfg(test)] mod tests { - use std::{marker::PhantomData, str::FromStr}; + use std::str::FromStr; - use cosmwasm_std::{ - testing::{mock_env, MockApi, MockStorage, MOCK_CONTRACT_ADDR}, - Addr, BankMsg, Coin, Decimal256, Empty, Fraction, OwnedDeps, Uint256, - }; - - use osmosis_std::types::{ - cosmos::base::v1beta1::Coin as OsmoCoin, - osmosis::concentratedliquidity::v1beta1::{ - FullPositionBreakdown, Position as OsmoPosition, - }, - }; + use cosmwasm_std::{testing::mock_env, Addr, BankMsg, Decimal256, Fraction, Uint256}; use crate::{ - helpers::{getters::get_depositable_tokens, msgs::refund_bank_msg}, + helpers::msgs::refund_bank_msg, state::{Position, POSITION}, - test_helpers::{mock_deps_with_querier, QuasarQuerier}, + test_helpers::mock_deps_with_querier, }; use super::*; - #[test] - fn test_position_in_both_asset() { - let token0 = Coin { - denom: "token0".to_string(), - amount: Uint128::new(1_000_000_000u128), - }; - let token1 = Coin { - denom: "token1".to_string(), - amount: Uint128::new(100_000_000_000_000_000_000_000_000_000u128), - }; - - let mut deps = mock_deps_with_position(Some(token0.clone()), Some(token1.clone())); - let mutdeps = deps.as_mut(); - - let result = get_depositable_tokens(mutdeps, token0, token1).unwrap(); - assert_eq!( - result, - ( - ( - Uint128::zero(), - Uint128::new(100_000_000_000_000_000_000_000_000_000u128) - ), - (Uint128::new(1_000_000_000u128), Uint128::zero()) - ) - ); - } - - #[test] - fn test_position_in_asset1_only() { - let token0 = Coin { - denom: "token0".to_string(), - amount: Uint128::new(50), - }; - let token1 = Coin { - denom: "token1".to_string(), - amount: Uint128::new(100), - }; - - // Osmosis is not using None for missing assets, but Some with amount 0, so we need to mimic that here - let mut deps = mock_deps_with_position( - Some(Coin { - denom: "token0".to_string(), - amount: Uint128::zero(), - }), - Some(token1.clone()), - ); - - let result = get_depositable_tokens(deps.as_mut(), token0, token1).unwrap(); - assert_eq!( - result, - ( - (Uint128::zero(), Uint128::new(100)), - (Uint128::new(50), Uint128::zero()) - ) - ); - } - - #[test] - fn test_position_in_asset0_only() { - let token0 = Coin { - denom: "token0".to_string(), - amount: Uint128::new(50), - }; - let token1 = Coin { - denom: "token1".to_string(), - amount: Uint128::new(100), - }; - - // Osmosis is not using None for missing assets, but Some with amount 0, so we need to mimic that here - let mut deps = mock_deps_with_position( - Some(token0.clone()), - Some(Coin { - denom: "token1".to_string(), - amount: Uint128::zero(), - }), - ); - - let result = get_depositable_tokens(deps.as_mut(), token0, token1).unwrap(); - assert_eq!( - result, - ( - (Uint128::new(50), Uint128::zero()), - (Uint128::zero(), Uint128::new(100)) - ) - ); - } - - #[test] - fn test_both_assets_present_token0_limiting() { - let token0 = Coin { - denom: "token0".to_string(), - amount: Uint128::new(50), - }; - let token1 = Coin { - denom: "token1".to_string(), - amount: Uint128::new(100), - }; - - // we use a ratio of 1/2 - let mut deps = mock_deps_with_position(Some(token0.clone()), Some(token1.clone())); - - let result = - get_depositable_tokens(deps.as_mut(), coin(2000, "token0"), coin(5000, "token1")) - .unwrap(); - assert_eq!( - result, - ( - (Uint128::new(2000), Uint128::new(4000)), - (Uint128::zero(), Uint128::new(1000)) - ) - ); - } - - #[test] - fn test_both_assets_present_token1_limiting() { - let token0 = Coin { - denom: "token0".to_string(), - amount: Uint128::new(50), - }; - let token1 = Coin { - denom: "token1".to_string(), - amount: Uint128::new(100), - }; - - // we use a ratio of 1/2 - let mut deps = mock_deps_with_position(Some(token0.clone()), Some(token1.clone())); - let mutdeps = deps.as_mut(); - - let result = - get_depositable_tokens(mutdeps, coin(2000, "token0"), coin(3000, "token1")).unwrap(); - assert_eq!( - result, - ( - (Uint128::new(1500), Uint128::new(3000)), - (Uint128::new(500), Uint128::zero()) - ) - ); - } - #[test] fn execute_exact_deposit_works() { let mut deps = mock_deps_with_querier(&MessageInfo { @@ -289,20 +156,6 @@ mod tests { ) .unwrap(); - // STRATEGIST_REWARDS - // .save(deps.as_mut().storage, &CoinList::new()) - // .unwrap(); - // POOL_CONFIG - // .save( - // deps.as_mut().storage, - // &PoolConfig { - // pool_id: 1, - // token0: "token0".to_string(), - // token1: "token1".to_string(), - // }, - // ) - // .unwrap(); - execute_exact_deposit( deps.as_mut(), env, @@ -396,56 +249,4 @@ mod tests { } ) } - - fn mock_deps_with_position( - token0: Option, - token1: Option, - ) -> OwnedDeps { - let position_id = 2; - - let mut deps = OwnedDeps { - storage: MockStorage::default(), - api: MockApi::default(), - querier: QuasarQuerier::new( - FullPositionBreakdown { - position: Some(OsmoPosition { - position_id, - address: MOCK_CONTRACT_ADDR.to_string(), - pool_id: 1, - lower_tick: 100, - upper_tick: 1000, - join_time: None, - liquidity: "1000000.2".to_string(), - }), - asset0: token0.map(|c| c.into()), - asset1: token1.map(|c| c.into()), - claimable_spread_rewards: vec![ - OsmoCoin { - denom: "token0".to_string(), - amount: "100".to_string(), - }, - OsmoCoin { - denom: "token1".to_string(), - amount: "100".to_string(), - }, - ], - claimable_incentives: vec![], - forfeited_incentives: vec![], - }, - 500, - ), - custom_query_type: PhantomData, - }; - POSITION - .save( - deps.as_mut().storage, - &Position { - position_id, - join_time: 0, - claim_after: None, - }, - ) - .unwrap(); - deps - } }