From 187ef150ad4ee3dedb75bd0bcb178b30d32d74a5 Mon Sep 17 00:00:00 2001 From: akildemir Date: Thu, 21 Sep 2023 17:17:15 +0300 Subject: [PATCH] bug fixes & optimizations --- substrate/client/src/serai/dex.rs | 4 +- substrate/client/tests/dex.rs | 171 ++++++++++++++---- substrate/in-instructions/pallet/src/lib.rs | 97 ++++++---- .../in-instructions/primitives/src/lib.rs | 38 +++- substrate/primitives/src/networks.rs | 4 + substrate/runtime/src/lib.rs | 4 +- substrate/tokens/pallet/src/lib.rs | 9 + 7 files changed, 245 insertions(+), 82 deletions(-) diff --git a/substrate/client/src/serai/dex.rs b/substrate/client/src/serai/dex.rs index 1f4f2f7ec..2f5f49dc7 100644 --- a/substrate/client/src/serai/dex.rs +++ b/substrate/client/src/serai/dex.rs @@ -67,9 +67,9 @@ impl Serai { amount_out_min: Amount, address: SeraiAddress, ) -> Payload> { - let path = if to_coin == Coin::Serai { + let path = if to_coin.is_native() { BoundedVec::truncate_from(vec![from_coin, Coin::Serai]) - } else if from_coin == Coin::Serai { + } else if from_coin.is_native() { BoundedVec::truncate_from(vec![Coin::Serai, to_coin]) } else { BoundedVec::truncate_from(vec![from_coin, Coin::Serai, to_coin]) diff --git a/substrate/client/tests/dex.rs b/substrate/client/tests/dex.rs index 8ef51d209..5ef0ddef8 100644 --- a/substrate/client/tests/dex.rs +++ b/substrate/client/tests/dex.rs @@ -6,9 +6,10 @@ use serai_runtime::in_instructions::primitives::DexCall; use serai_client::{ primitives::{ Amount, NetworkId, Coin, Balance, BlockHash, insecure_pair_from_name, ExternalAddress, + SeraiAddress, }, in_instructions::primitives::{ - InInstruction, InInstructionWithBalance, Batch, IN_INSTRUCTION_EXECUTOR, + InInstruction, InInstructionWithBalance, Batch, IN_INSTRUCTION_EXECUTOR, OutAddress, }, dex::DexEvent, Serai, @@ -90,7 +91,8 @@ serai_test!( let coin2 = Coin::Ether; let pair = insecure_pair_from_name("Ferdie"); let serai = serai().await; - let mut batch_id = 0; + let mut coin1_batch_id = 0; + let mut coin2_batch_id = 0; // create pools common_create_pool(coin1, 0, pair.clone()).await; @@ -100,18 +102,19 @@ serai_test!( mint_coin( Balance { coin: coin1, amount: Amount(100_000000000000) }, NetworkId::Monero, - batch_id, + coin1_batch_id, pair.clone().public().into(), ) .await; - batch_id += 1; + coin1_batch_id += 1; mint_coin( Balance { coin: coin2, amount: Amount(100_000000000000) }, NetworkId::Ethereum, - 0, + coin2_batch_id, pair.clone().public().into(), ) .await; + coin2_batch_id += 1; // add liquidity to pools common_add_liquidity(coin1, Amount(50_000000000000), Amount(50_000000000000), 2, pair.clone()) @@ -119,42 +122,134 @@ serai_test!( common_add_liquidity(coin2, Amount(50_000000000000), Amount(50_000000000000), 3, pair.clone()) .await; - // make an address to send the eth to + // rand address bytes let mut rand_bytes = vec![0; 32]; OsRng.fill_bytes(&mut rand_bytes); - let external_address = ExternalAddress::new(rand_bytes).unwrap(); - - // now that we have our pools, we can try to swap - let mut block_hash = BlockHash([0; 32]); - OsRng.fill_bytes(&mut block_hash.0); - let batch = Batch { - network: NetworkId::Monero, - id: batch_id, - block: block_hash, - instructions: vec![InInstructionWithBalance { - instruction: InInstruction::Dex(DexCall::Swap(coin2, external_address, Amount(1))), - balance: Balance { coin: coin1, amount: Amount(20_000000000000) }, - }], - }; - - let block = provide_batch(batch).await; - let mut events = serai.dex_events(block).await.unwrap(); - events.retain(|e| matches!(e, DexEvent::SwapExecuted { .. })); - // we should have only 1 swap event. - assert_eq!(events.len(), 1); - - let path = BoundedVec::truncate_from(vec![coin1, Coin::Serai, coin2]); - assert_eq!( - events, - vec![DexEvent::SwapExecuted { - who: IN_INSTRUCTION_EXECUTOR.into(), - send_to: IN_INSTRUCTION_EXECUTOR.into(), - path, - amount_in: 20_000000000000, - amount_out: 11066655622377 - }] - ); + // coin -> coin(XMR -> ETH) + { + // make an out address + let out_address = OutAddress::External(ExternalAddress::new(rand_bytes.clone()).unwrap()); + + // amount is the min out amount + let out_balance = Balance { coin: coin2, amount: Amount(1) }; + + // now that we have our pools, we can try to swap + let mut block_hash = BlockHash([0; 32]); + OsRng.fill_bytes(&mut block_hash.0); + let batch = Batch { + network: NetworkId::Monero, + id: coin1_batch_id, + block: block_hash, + instructions: vec![InInstructionWithBalance { + instruction: InInstruction::Dex(DexCall::Swap(out_balance, out_address)), + balance: Balance { coin: coin1, amount: Amount(20_000000000000) }, + }], + }; + + let block = provide_batch(batch).await; + coin1_batch_id += 1; + let mut events = serai.dex_events(block).await.unwrap(); + events.retain(|e| matches!(e, DexEvent::SwapExecuted { .. })); + + // we should have only 1 swap event. + assert_eq!(events.len(), 1); + + let path = BoundedVec::truncate_from(vec![coin1, Coin::Serai, coin2]); + assert_eq!( + events, + vec![DexEvent::SwapExecuted { + who: IN_INSTRUCTION_EXECUTOR.into(), + send_to: IN_INSTRUCTION_EXECUTOR.into(), + path, + amount_in: 20_000000000000, + amount_out: 11066655622377 + }] + ); + } + + // coin -> coin(with internal address, ETH -> XMR) + { + // make an out address + let out_address = + OutAddress::Serai(SeraiAddress::new(rand_bytes.clone().try_into().unwrap())); + + // amount is the min out amount + let out_balance = Balance { coin: coin1, amount: Amount(1) }; + + // now that we have our pools, we can try to swap + let mut block_hash = BlockHash([0; 32]); + OsRng.fill_bytes(&mut block_hash.0); + let batch = Batch { + network: NetworkId::Ethereum, + id: coin2_batch_id, + block: block_hash, + instructions: vec![InInstructionWithBalance { + instruction: InInstruction::Dex(DexCall::Swap(out_balance, out_address.clone())), + balance: Balance { coin: coin2, amount: Amount(20_000000000000) }, + }], + }; + + let block = provide_batch(batch).await; + let mut events = serai.dex_events(block).await.unwrap(); + events.retain(|e| matches!(e, DexEvent::SwapExecuted { .. })); + + // we should have only 1 swap event. + assert_eq!(events.len(), 1); + + let path = BoundedVec::truncate_from(vec![coin2, Coin::Serai, coin1]); + assert_eq!( + events, + vec![DexEvent::SwapExecuted { + who: IN_INSTRUCTION_EXECUTOR.into(), + send_to: out_address.as_native().unwrap().into(), + path, + amount_in: 20_000000000000, + amount_out: 26440798801319 + }] + ); + } + + // coin -> SRI(XMR -> SRI) + { + // make an out address + let out_address = OutAddress::Serai(SeraiAddress::new(rand_bytes.try_into().unwrap())); + + // amount is the min out amount + let out_balance = Balance { coin: Coin::Serai, amount: Amount(1) }; + + // now that we have our pools, we can try to swap + let mut block_hash = BlockHash([0; 32]); + OsRng.fill_bytes(&mut block_hash.0); + let batch = Batch { + network: NetworkId::Monero, + id: coin1_batch_id, + block: block_hash, + instructions: vec![InInstructionWithBalance { + instruction: InInstruction::Dex(DexCall::Swap(out_balance, out_address.clone())), + balance: Balance { coin: coin1, amount: Amount(10_000000000000) }, + }], + }; + + let block = provide_batch(batch).await; + let mut events = serai.dex_events(block).await.unwrap(); + events.retain(|e| matches!(e, DexEvent::SwapExecuted { .. })); + + // we should have only 1 swap event. + assert_eq!(events.len(), 1); + + let path = BoundedVec::truncate_from(vec![coin1, Coin::Serai]); + assert_eq!( + events, + vec![DexEvent::SwapExecuted { + who: IN_INSTRUCTION_EXECUTOR.into(), + send_to: out_address.as_native().unwrap().into(), + path, + amount_in: 10_000000000000, + amount_out: 10711005507065 + }] + ); + } // TODO: check balances? } diff --git a/substrate/in-instructions/pallet/src/lib.rs b/substrate/in-instructions/pallet/src/lib.rs index 3bed32d81..0979046e9 100644 --- a/substrate/in-instructions/pallet/src/lib.rs +++ b/substrate/in-instructions/pallet/src/lib.rs @@ -61,6 +61,12 @@ pub mod pallet { InstructionFailure { network: NetworkId, id: u32, index: u32 }, } + #[pallet::error] + pub enum Error { + /// Coin and OutAddress types don't match. + InvalidAddressForCoin, + } + #[pallet::pallet] pub struct Pallet(PhantomData); @@ -116,9 +122,9 @@ pub mod pallet { origin.clone().into(), path, half, - 1, // minimum out, so that we accept whatever we get. + 1, // minimum out, so we accept whatever we get. IN_INSTRUCTION_EXECUTOR.into(), - false, + true, )?; // get how much we got for our swap @@ -140,57 +146,78 @@ pub mod pallet { // TODO: minimums are set to 1 above to guarantee successful adding liq call. // Ideally we either get this info from user or send the leftovers back to user. - // If leftovers stay on our IIExecutor account, they can accumulate and would - // give us wrong SRI amounts next time another users call here(since we know - // how much to add liq by checking the balance on it). Which then would - // make addling liq fail. So Let's send the leftovers back to user. + // Let's send the leftovers back to user for now. let coin_balance = Tokens::::balance(coin, IN_INSTRUCTION_EXECUTOR); let sri_balance = BalancesPallet::::free_balance(PublicKey::from(IN_INSTRUCTION_EXECUTOR)); + if coin_balance != 0 { - Tokens::::transfer(origin.clone().into(), coin, address, coin_balance)?; + Tokens::::transfer_keep_alive( + origin.clone().into(), + coin, + address, + coin_balance, + )?; } if sri_balance != 0 { - // unwrap here. First, it doesn't panic, second we have no choice but to empty - // IIE account. - BalancesPallet::::transfer_allow_death(origin.into(), address, sri_balance) + // unwrap here. First, it doesn't panic for full amount, + // second we should empty IIE account anyway. + BalancesPallet::::transfer_keep_alive(origin.into(), address, sri_balance) .unwrap(); } - - // TODO: ideally we would get the coin and sri balances again and - // make sure they are 0. But we already made 3 calls and that - // would make 5. should we do it? } - DexCall::Swap(coin2, address, amount_out_min) => { - let origin = RawOrigin::Signed(IN_INSTRUCTION_EXECUTOR.into()); + DexCall::Swap(out_balance, out_address) => { + let send_to_external = !out_address.is_native(); + let native_coin = out_balance.coin.is_native(); + + // we can't send native coin to external chain + if native_coin && send_to_external { + Err(Error::::InvalidAddressForCoin)?; + } // mint the given coin on our account Tokens::::mint(IN_INSTRUCTION_EXECUTOR, instruction.balance); - // do the swap on our account - let path = - BoundedVec::truncate_from(vec![instruction.balance.coin, Coin::Serai, coin2]); + // get the path + let mut path = vec![instruction.balance.coin, Coin::Serai]; + if !native_coin { + path.push(out_balance.coin); + } + + // get the swap address + // if the address is internal, we can directly swap to it. + // if not, we swap to ourselves and burn the coins to send them back + // on the external chain. + let send_to = if send_to_external { + IN_INSTRUCTION_EXECUTOR + } else { + out_address.clone().as_native().unwrap() + }; + + // do the swap + let origin = RawOrigin::Signed(IN_INSTRUCTION_EXECUTOR.into()); Dex::::swap_exact_tokens_for_tokens( - origin.clone().into(), - path, + origin.into(), + BoundedVec::truncate_from(path), instruction.balance.amount.0, - amount_out_min.0, - IN_INSTRUCTION_EXECUTOR.into(), - false, + out_balance.amount.0, + send_to.into(), + true, )?; - // see how much we got - let coin2_balance = Tokens::::balance(coin2, IN_INSTRUCTION_EXECUTOR); - // burn the received coins so that they sent back to the user - let balance = Balance { coin: coin2, amount: Amount(coin2_balance) }; - - // TODO: data shouldn't come here from processor just to go back to it. - Tokens::::burn_internal( - IN_INSTRUCTION_EXECUTOR, - balance, - OutInstruction { address, data: None }, - )?; + // if it is requested to an external address. + if send_to_external { + // see how much we got + let coin2_balance = Tokens::::balance(out_balance.coin, IN_INSTRUCTION_EXECUTOR); + let balance = Balance { coin: out_balance.coin, amount: Amount(coin2_balance) }; + // TODO: data shouldn't come here from processor just to go back to it. + Tokens::::burn_internal( + IN_INSTRUCTION_EXECUTOR, + balance, + OutInstruction { address: out_address.as_external().unwrap(), data: None }, + )?; + } } } } diff --git a/substrate/in-instructions/primitives/src/lib.rs b/substrate/in-instructions/primitives/src/lib.rs index 60c17b2c5..b3a5dd0a0 100644 --- a/substrate/in-instructions/primitives/src/lib.rs +++ b/substrate/in-instructions/primitives/src/lib.rs @@ -16,9 +16,8 @@ use sp_application_crypto::sr25519::Signature; use sp_std::vec::Vec; use sp_runtime::RuntimeDebug; -use serai_primitives::{ - BlockHash, Balance, NetworkId, SeraiAddress, ExternalAddress, Coin, Amount, pallet_address, -}; +#[rustfmt::skip] +use serai_primitives::{BlockHash, Balance, NetworkId, SeraiAddress, ExternalAddress, pallet_address}; mod shorthand; pub use shorthand::*; @@ -39,6 +38,35 @@ pub enum InInstruction { Dex(DexCall), } +#[derive( + Clone, PartialEq, Eq, Debug, Serialize, Deserialize, Encode, Decode, MaxEncodedLen, TypeInfo, +)] +#[cfg_attr(feature = "std", derive(Zeroize))] +pub enum OutAddress { + Serai(SeraiAddress), + External(ExternalAddress), +} + +impl OutAddress { + pub fn is_native(&self) -> bool { + matches!(self, Self::Serai(_)) + } + + pub fn as_native(self) -> Option { + match self { + Self::Serai(addr) => Some(addr), + _ => None, + } + } + + pub fn as_external(self) -> Option { + match self { + Self::External(addr) => Some(addr), + Self::Serai(_) => None, + } + } +} + #[derive( Clone, PartialEq, Eq, Debug, Serialize, Deserialize, Encode, Decode, MaxEncodedLen, TypeInfo, )] @@ -46,8 +74,8 @@ pub enum InInstruction { pub enum DexCall { // address to sent the lp tokens to AddLiquidity(SeraiAddress), - // to_coin, to_address, min_out_amount. TODO: sync with docs. - Swap(Coin, ExternalAddress, Amount), + // out balance and out address + Swap(Balance, OutAddress), } #[derive( diff --git a/substrate/primitives/src/networks.rs b/substrate/primitives/src/networks.rs index 3c68cad2d..4ec1777b9 100644 --- a/substrate/primitives/src/networks.rs +++ b/substrate/primitives/src/networks.rs @@ -70,6 +70,10 @@ impl Coin { Coin::Monero => NetworkId::Monero, } } + + pub fn is_native(&self) -> bool { + matches!(self, Coin::Serai) + } } // Max of 8 coins per network diff --git a/substrate/runtime/src/lib.rs b/substrate/runtime/src/lib.rs index d28f0e2d5..ab7ab3a0d 100644 --- a/substrate/runtime/src/lib.rs +++ b/substrate/runtime/src/lib.rs @@ -369,12 +369,12 @@ impl MultiAssetIdConverter for CoinConverter { /// Returns true if the given MultiAssetId is the native currency. fn is_native(coin: &Coin) -> bool { - *coin == Coin::Serai + coin.is_native() } /// If it's not native, returns the AssetId for the given MultiAssetId. fn try_convert(coin: &Coin) -> Result { - if *coin == Coin::Serai { + if coin.is_native() { return Err(()); } Ok(*coin) diff --git a/substrate/tokens/pallet/src/lib.rs b/substrate/tokens/pallet/src/lib.rs index 22e92dcf9..41e5f2d4c 100644 --- a/substrate/tokens/pallet/src/lib.rs +++ b/substrate/tokens/pallet/src/lib.rs @@ -76,6 +76,15 @@ pub mod pallet { ) -> DispatchResult { AssetsPallet::::transfer(origin, coin, target, amount) } + + pub fn transfer_keep_alive( + origin: OriginFor, + coin: Coin, + target: SeraiAddress, + amount: SubstrateAmount, + ) -> DispatchResult { + AssetsPallet::::transfer_keep_alive(origin, coin, target, amount) + } } #[pallet::call]