Merge pull request #138 from Phoenix-Protocol-Group/137-multihop-fix-…

…authorization-bug

Multihop: fix authorization bug

CHANGELOG.md

@@ -10,9 +10,11 @@ and this project adheres to
 
 - Stake: Implement APR query ([#101])
 - Multihop: Provide swap implementation and a testing framework ([#133])
+- Multihop: Refactor swap algorithm and fix authorization issue on subsequent swaps ([#138])
 
 [#101]: https://github.com/Phoenix-Protocol-Group/phoenix-contracts/pull/101
 [#133]: https://github.com/Phoenix-Protocol-Group/phoenix-contracts/pull/133
+[#138]: https://github.com/Phoenix-Protocol-Group/phoenix-contracts/pull/138
 
 ## [0.6.0] - 2023-09-20
 

contracts/multihop/Makefile

@@ -7,7 +7,6 @@ test:
 
 build:
 	$(MAKE) -C ../factory build || break;
-	$(MAKE) -C ../token build || break;
 	cargo build --target wasm32-unknown-unknown --release
 
 lint: fmt clippy

contracts/multihop/src/contract.rs

@@ -1,8 +1,8 @@
-use soroban_sdk::{contract, contractimpl, contractmeta, Address, Env, IntoVal, Symbol, Val, Vec};
+use soroban_sdk::{contract, contractimpl, contractmeta, Address, Env, Vec};
 
 use crate::error::ContractError;
 use crate::storage::{get_factory, save_admin, save_factory, Swap};
-use crate::{factory_contract, lp_contract, token_contract};
+use crate::{factory_contract, lp_contract};
 
 // Metadata that is added on to the WASM custom section
 contractmeta!(
@@ -49,12 +49,10 @@ impl MultihopTrait for Multihop {
 
         recipient.require_auth();
 
-        let mut offer_amount: i128 = amount;
+        // first offer amount is an input from the user,
+        // subsequent are the results of the previous swap
+        let mut next_offer_amount: i128 = amount;
         let mut offer_token_addr: Address = operations.get(0).unwrap().offer_asset.clone();
-        let mut offer_token_client = token_contract::Client::new(&env, &offer_token_addr);
-
-        // first transfer token to multihop contract
-        offer_token_client.transfer(&recipient, &env.current_contract_address(), &offer_amount);
 
         let factory_client =
             factory_contract::Client::new(&env, &get_factory(&env).expect("factory not found"));
@@ -64,28 +62,17 @@ impl MultihopTrait for Multihop {
                 .query_for_pool_by_pair_tuple(&(op.offer_asset, op.ask_asset.clone()));
 
             let lp_client = lp_contract::Client::new(&env, &liquidity_pool_addr);
-            lp_client.swap(
-                &env.current_contract_address(),
+            next_offer_amount = lp_client.swap(
+                &recipient,
                 &true,
-                &offer_amount,
+                &next_offer_amount,
                 &None::<i64>,
                 &Some(5000i64),
             );
 
-            offer_token_client = token_contract::Client::new(&env, &op.ask_asset);
-            offer_amount = offer_token_client.balance(&env.current_contract_address());
             offer_token_addr = op.ask_asset.clone();
         });
 
-        // in each loop iteration, last asked token becomes an offer; after loop we can rename it
-        let asked_amount = offer_amount;
-        let asked_token_addr = offer_token_addr;
-
-        let token_func_name = &Symbol::new(&env, "transfer");
-        let token_call_args: Vec<Val> =
-            (env.current_contract_address(), recipient, asked_amount).into_val(&env);
-        env.invoke_contract::<Val>(&asked_token_addr, token_func_name, token_call_args);
-
         Ok(())
     }
 }

contracts/multihop/src/lib.rs

@@ -5,15 +5,6 @@ mod error;
 
 mod storage;
 
-pub mod token_contract {
-    // The import will code generate:
-    // - A ContractClient type that can be used to invoke functions on the contract.
-    // - Any types in the contract that were annotated with #[contracttype].
-    soroban_sdk::contractimport!(
-        file = "../../target/wasm32-unknown-unknown/release/soroban_token_contract.wasm"
-    );
-}
-
 #[allow(clippy::too_many_arguments)]
 pub mod lp_contract {
     // The import will code generate:

contracts/multihop/src/tests/swap.rs

@@ -150,8 +150,6 @@ fn swap_three_equal_pools_no_fees() {
 
     let operations = vec![&env, swap1, swap2, swap3];
 
-    // ignore the compiler err highlight
-    env.mock_all_auths_allowing_non_root_auth();
     multihop.swap(&recipient, &operations, &50i128);
 
     // 5. check if it goes according to plan

contracts/pair/src/contract.rs

@@ -59,14 +59,15 @@ pub trait LiquidityPoolTrait {
     // If "buy_a" is true, the swap will buy token_a and sell token_b. This is flipped if "buy_a" is false.
     // "out" is the amount being bought, with in_max being a safety to make sure you receive at least that amount.
     // swap will transfer the selling token "to" to this contract, and then the contract will transfer the buying token to "to".
+    // Returns the amount of the token being bought.
     fn swap(
         env: Env,
         sender: Address,
         sell_a: bool,
         offer_amount: i128,
         belief_price: Option<i64>,
         max_spread_bps: Option<i64>,
-    ) -> Result<(), ContractError>;
+    ) -> Result<i128, ContractError>;
 
     // transfers share_amount of pool share tokens to this contract, burns all pools share tokens in this contracts, and sends the
     // corresponding amount of token_a and token_b to "to".
@@ -339,7 +340,7 @@ impl LiquidityPoolTrait for LiquidityPool {
         offer_amount: i128,
         belief_price: Option<i64>,
         max_spread_bps: Option<i64>,
-    ) -> Result<(), ContractError> {
+    ) -> Result<i128, ContractError> {
         validate_int_parameters!(offer_amount);
 
         sender.require_auth();
@@ -597,7 +598,7 @@ fn do_swap(
     offer_amount: i128,
     belief_price: Option<i64>,
     max_spread: Option<i64>,
-) -> Result<(), ContractError> {
+) -> Result<i128, ContractError> {
     let config = get_config(&env)?;
 
     let belief_price = belief_price.map(Decimal::percent);
@@ -680,7 +681,7 @@ fn do_swap(
     env.events()
         .publish(("swap", "spread_amount"), spread_amount);
 
-    Ok(())
+    Ok(return_amount)
 }
 
 /// This function divides the deposit in such a way that when swapping it for the other token,

contracts/pair/src/tests/swap.rs

@@ -98,7 +98,7 @@ fn simple_swap() {
 
     // false means selling B token
     // this time 100 units
-    pool.swap(&user1, &false, &1_000, &None, &Some(spread));
+    let output_amount = pool.swap(&user1, &false, &1_000, &None, &Some(spread));
     let result = pool.query_pool_info();
     assert_eq!(
         result,
@@ -117,6 +117,7 @@ fn simple_swap() {
             },
         }
     );
+    assert_eq!(output_amount, 1000);
     assert_eq!(token1.balance(&user1), 1999); // 999 + 1_000 as a result of swap
     assert_eq!(token2.balance(&user1), 1001 - 1000); // user1 sold 1k of token B on second swap
 }