feat(swap): rpc to find best swap with liquidity routing for ask #2362

dimxy · 2025-02-16T18:30:57Z

Initial code for liquidity routing (LR) support for KDF.

The added RPC finds best (most price efficient) swap path from a list of provided ask orders, to fill them with a User token converting into orders' tokens with an interim LR-swap.

This PR covers only finding the best quote for one case (the ask order when 'rel' is token). Next steps are adding/fixing RPCs to get quotes for order types (bids) and RPCs to run swaps with LR.

TODO:

validation code for requests in new RPCs
add support trade_preimage_rpc for TPU to calculate total fees, return total fees including fees for LR with this PR's best quote RPC
getting quotes for bid orders when rel=token_x, with routing token_x into my_token after the dex-swap
getting quotes for ask orders when base=token_x, with routing token_x into my_token after the dex-swap
getting quotes for bid orders when base=token_x, with routing User's my_token into token_x before the dex-swap
add RPC to run swaps with LR (using LR swap data from the best quote RPCs), extending TPU swaps workflow

TODO to research:

stream the best quote price to the GUI so the user could wait for better price or refuse from staring the swap if price becomes unsatisfying
study the provider RPC how to find most liquid tokens for LR (use the KDF orderbook as the source for tokens) and return them to the user, to use them in the find best swap with LR RPC

…oken, test added

* dev: fix(derive_key_from_path): check length of current_key_material (#2356) chore(release): bump mm2 version to 2.4.0-beta (#2346) fix(tests): add additional testnet sepolia nodes to test code (#2358) fix(swaps): maintain legacy compatibility for negotiation messages (#2353) refactor(SwapOps): impl defaults for protocol specific swapops fns (#2354) feat(tpu-v2): provide swap protocol versioning (#2324) feat(wallet): add change mnemonic password rpc (#2317) fix(tpu-v2): fix tpu-v2 wait for payment spend and extract secret (#2261) feat(tendermint): unstaking/undelegation (#2330) fix(utxo-withdraw): get hw ctx only when `PrivKeyPolicy` is trezor (#2333) feat(event-streaming): API-driven subscription management (#2172) fix(hash-types): remove panic, enforce fixed-size arrays (#2279) fix(ARRR): store unconfirmed change output (#2276) feat(tendermint): staking/delegation (#2322) chore(deps): `timed-map` migration (#2247) fix(mem-leak): `running_swap` never shrinks (#2301) chore(dep-bump): libp2p (#2326) refactor(build script): rewrite the main build script (#2319)

mariocynicys

Thanks! esp for the excessive comments :)

this was a light review to grasp new thigns. nothing is blocking from my side.

will read the SoW for more context and review again next review week if this is still unmerged.

mariocynicys · 2025-02-22T05:11:37Z

mm2src/mm2_main/src/rpc/lp_commands/lr_swap.rs

@@ -0,0 +1,263 @@
+//? RPC implementations for swaps with liquidity routing (LR) of EVM tokens


module docs start like this: //!
also other instances

mariocynicys · 2025-02-22T05:16:19Z

mm2src/mm2_main/src/rpc/lp_commands/lr_swap.rs

+/// TODO: currently supported only ask orders with rel=token_x, with routing User's my_token into token_x before the dex-swap.
+/// The RPC should also support:
+/// bid orders with rel=token_x, with routing token_x into my_token after the dex-swap
+/// ask orders with base=token_x, with routing token_x into my_token after the dex-swap
+/// bid orders with base=token_x, with routing User's my_token into token_x before the dex-swap


isn't ask with rel=token_x === bid with base=token_x, can't we always consider everything as a bid for simplicity?

Yes we can indeed.
I am going to add support for all cases in the next PR

mariocynicys · 2025-02-22T05:42:56Z

mm2src/mm2_main/src/rpc/lp_commands/lr_swap.rs

+            base: base_ticker,
+            amount: "0.123".into(),
+            asks,
+            my_token: weth_ticker,


maybe we can use rel instead of my_token? makes sense since we have base

Yeah, I'll need to refactor ticker params in the next PR where whole set of bid/asks options are to be added

mariocynicys · 2025-02-22T06:12:36Z

mm2src/mm2_main/src/rpc/lp_commands/lr_swap/lr_impl.rs

+            let src_contract = eth_addr_to_hex(
+                lr_data
+                    .src_contract
+                    .as_ref()
+                    .ok_or(ApiIntegrationRpcError::InternalError("no contract".to_owned()))?,
+            );
+            let dst_contract = eth_addr_to_hex(
+                lr_data
+                    .dst_contract
+                    .as_ref()
+                    .ok_or(ApiIntegrationRpcError::InternalError("no contract".to_owned()))?,
+            );
+            let chain_id = lr_data
+                .chain_id
+                .ok_or(ApiIntegrationRpcError::InternalError("no chain id".to_owned()))?;


nit: we have these fields optionalized but then error whenever one of them is None while doing this calculation. what about having them non-optional and initialized right inside new()

mariocynicys · 2025-02-22T06:24:40Z

mm2src/mm2_main/src/rpc/lp_commands/lr_swap/lr_impl.rs

+            .map(|((src, dst), series)| {
+                let dst_price = cross_prices_average(series);


Q: this series is a list of all prices on 1inch ha? should we average them or pick the best x prices that will fill our volume?

Yes this is prices from 1inch.
Maybe we should use best prices, I think, tests will show.
This price is obtained to estimate how much source token amount we need to fill the swap when 1inch as a liquidity routing provider (and the actual LR conversion is done for the best quote price).

shamardy

Some comments from the wip review. Only this comment #2362 (comment) needs changes in this PR, other comments are for future PRs and discussion.

shamardy · 2025-02-21T21:53:10Z

mm2src/coins/eth.rs

+/// Find a EVM token name in the coins file by a given contract address.
+/// If contract_addr is empty the function returns the platform coin name
+pub async fn find_token_by_address(ctx: &MmArc, chain_id: u64, contract_addr: Option<EthAddress>) -> Option<String> {
+    let coin_ctx = CoinsContext::from_ctx(ctx).unwrap();
+    let coins = coin_ctx.coins.lock().await;
+    coins
+        .iter()
+        .find(|(_ticker, coin_struct)| {
+            if let MmCoinEnum::EthCoin(eth_coin) = &coin_struct.inner {
+                match eth_coin.coin_type {
+                    EthCoinType::Erc20 { token_addr, .. } => {
+                        if let Some(contract_addr) = contract_addr {
+                            if token_addr == contract_addr && eth_coin.chain_id() == chain_id {
+                                return true;
+                            }
+                        }
+                    },
+                    EthCoinType::Eth => {
+                        if contract_addr.is_none() && eth_coin.chain_id() == chain_id {
+                            return true;
+                        }
+                    },
+                    EthCoinType::Nft { .. } => {},
+                }
+            }
+            false
+        })
+        .map(|(ticker, _)| ticker.clone())
+}


Isn't this function a bit similar to this

komodo-defi-framework/mm2src/coins/eth/erc20.rs

Lines 93 to 107 in 22719c0

/// Finds an enabled ERC20 token by its contract address and returns it as `MmCoinEnum`.

pub async fn get_enabled_erc20_by_contract(

ctx: &MmArc,

contract_address: Address,

) -> MmResult<Option<MmCoinEnum>, String> {

let cctx = CoinsContext::from_ctx(ctx)?;

let coins = cctx.coins.lock().await;

Ok(coins.values().find_map(|coin| match &coin.inner {

MmCoinEnum::EthCoin(eth_coin) if eth_coin.erc20_token_address() == Some(contract_address) => {

Some(coin.inner.clone())

},

_ => None,

}))

}

I see that I didn't add chain_id to get_enabled_erc20_by_contract which is a mistake. We should have one function, please merge those in the next PR.

Yeah, my function searches in the enabled coins, but for LR tokens it is not necessary to have them enabled, I guess.
So for LR tokens info it's better to look into the coins file directly.
Ah, sorry, you mentioned the get_enabled_erc20_by_contract fn (which searches in enabled tokens) but I actually began to look into another fn get_erc20_ticker_by_contract_address, which searches in the coins file and it is almost what I needed

Here is my proposed change c850505 to reuse the get_erc20_ticker_by_contract_address fn (instead of find_token_by_address fn).

Also I'll add chain_id to get_enabled_erc20_by_contract in a dedicated PR.

shamardy · 2025-02-21T21:54:11Z

mm2src/coins_activation/src/erc20_token_activation.rs

@@ -72,6 +72,7 @@ impl TryFromCoinProtocol for Erc20Protocol {
                contract_address,
            } => {
                let token_addr = valid_addr_from_str(&contract_address).map_err(|_| CoinProtocol::ERC20 {
+                    // TODO: maybe add error description to this err (we're losing 'Invalid address checksum' here)


True. Should have a separate issue for this.

shamardy · 2025-02-22T10:43:59Z

mm2src/mm2_main/src/rpc/dispatcher/dispatcher.rs

+        "lr_best_quote" => handle_mmrpc(ctx, request, lr_best_quote_rpc).await,
+        "lr_quotes_for_tokens" => handle_mmrpc(ctx, request, lr_quotes_for_tokens_rpc).await,
+        "lr_fill_order" => handle_mmrpc(ctx, request, lr_fill_order_rpc).await,


How about adding those to a new namespace, preview/experimental or test. I like preview most.

btw, we might be asked to combine multiple functionalities in an aggregator RPC, so asks are chosen automatically from ordebook and fed to best quote. This #1609 can help a lot in liquidity routing too.

btw, we might be asked to combine multiple functionalities in an aggregator RPC

I thought about an extra RPC to find most suitable (liquid) tokens for LR (with the orderbook as a source for tokens) - added a TODO to the PR description. Ofc maybe this could be part of an aggregator RPC.

Added preview:: prefix to the lr_.. rpcs c850505

shamardy · 2025-02-22T11:02:33Z

mm2src/mm2_main/src/rpc/lp_commands/lr_swap.rs

+/// bid orders with rel=token_x, with routing token_x into my_token after the dex-swap
+/// ask orders with base=token_x, with routing token_x into my_token after the dex-swap
+/// bid orders with base=token_x, with routing User's my_token into token_x before the dex-swap
+pub async fn lr_best_quote_rpc(


Is there a way to get streamed events for the best quote in the future? I see that only fusion+ has a websocket API so it's probably not possible. But we can stream it periodically I guess instead of GUI polling it. Please add this to the issue checklist.

A somewhat related comment. Price changes are very fast, so we would have to stream it every second at most, we can make it configurable, but this is where slippage comes in when actually filling the order as an error can be returned if execution price is different from the one shown to the user (the price minus slippage of course).

The OHLC API returns token prices with granularity of 5 minutes minimum.
Getting a quote each second ver 1inch API I guess would cost much their infra fees (unless we setup our own service which can interact with their contract directly, with the needed interval).

Added a TODO about streaming to this PR description.

shamardy · 2025-02-22T11:03:44Z

mm2src/trading_api/src/one_inch_api/portfolio_types.rs

+/// API params builder to get OHLC price history for token pair
+/// See 1inch docs: https://portal.1inch.dev/documentation/apis/portfolio/swagger?method=get&path=%2Fintegrations%2Fprices%2Fv1%2Ftime_range%2Fcross_prices
+#[derive(Default)]
+pub struct CrossPriceParams {
+    chain_id: u64,
+    /// Base token address
+    token0_address: String,
+    /// Quote token address
+    token1_address: String,
+    /// Returned time series intervals
+    granularity: Option<DataGranularity>,
+    /// max number of time series
+    limit: Option<u32>,
+}


Why would we need OHLC data, why not use spot price API?

The spot price returns a token price in fiat currency.
The OHLC API returns prices directly for token pairs we needed, I thought this is more accurate.

shamardy · 2025-02-22T11:11:37Z

@dimxy please merge with dev for the CI fixes

…ct_address fn, add new get_platform_ticker fn

* dev: feat(tendermint): claim delegation rewards (#2351) fix(eth-tpu): remove state from funding validation (#2334) improvement(rpc-server): rpc server dynamic port allocation (#2342) fix(tests): fix or ignore unstable tests (#2365) fix(fs): make `filter_files_by_extension` return only files (#2364)

dimxy added 10 commits January 21, 2025 20:35

add LR swap rpc scaffolding

84663a4

refactor LR rpcs

05c15fc

fix doc comments

43d2bd6

fix draft lr rpc params

e5c73b5

WiP: find_best_lr_buy_for_multiple_orders initial compiled code

6b4124d

add missed lr_impl.rs

e2f8a53

fix comment

0ea3dd9

wip: working code for finding best swap with LR to for ask with rel=t…

92bf147

…oken, test added

remove unused test code

08ff400

dimxy added status: in progress priority: medium Moderately important tasks that should be completed but are not urgent. feature: swap feature: RPC labels Feb 16, 2025

dimxy changed the title ~~feat(swap): rpc to find best swap with liquidity routing to fill ask when rel is token~~ feat(swap): rpc to find best swap with liquidity routing for ask Feb 16, 2025

dimxy added 5 commits February 17, 2025 12:17

remove debug println

6e55e7e

remove unused derive and import

54d13c3

fix fmt

5553bdc

refactor: eliminate SomeError in LR code

1e3a5d8

wrap LR test with feature

7ab51fc

shamardy added the 2.5.0-beta label Feb 17, 2025

shamardy self-requested a review February 17, 2025 10:10

dimxy added 2 commits February 17, 2025 18:49

fix LR error desc

5e96440

add another coin to LR quote test

bbb115e

shamardy added priority: high Important tasks that need attention soon. status: pending review and removed priority: medium Moderately important tasks that should be completed but are not urgent. status: in progress labels Feb 21, 2025

mariocynicys self-requested a review February 21, 2025 13:27

mariocynicys reviewed Feb 22, 2025

View reviewed changes

shamardy reviewed Feb 22, 2025

View reviewed changes

dimxy added 4 commits February 26, 2025 20:29

review (shamardy): eliminate dup fn, reuse get_erc20_ticker_by_contra…

c850505

…ct_address fn, add new get_platform_ticker fn

fix doc comment

f4fba47

fix wasm build

18e4668

shamardy requested review from shamardy and mariocynicys March 3, 2025 11:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(swap): rpc to find best swap with liquidity routing for ask #2362

feat(swap): rpc to find best swap with liquidity routing for ask #2362

dimxy commented Feb 16, 2025 •

edited

Loading

mariocynicys left a comment

mariocynicys Feb 22, 2025

mariocynicys Feb 22, 2025

dimxy Feb 26, 2025

mariocynicys Feb 22, 2025

dimxy Feb 26, 2025

mariocynicys Feb 22, 2025

mariocynicys Feb 22, 2025

dimxy Feb 26, 2025

shamardy left a comment •

edited

Loading

shamardy Feb 21, 2025

dimxy Feb 26, 2025 •

edited

Loading

dimxy Feb 26, 2025

shamardy Feb 21, 2025

shamardy Feb 22, 2025

shamardy Feb 22, 2025

dimxy Feb 26, 2025

dimxy Feb 26, 2025

shamardy Feb 22, 2025

shamardy Feb 22, 2025

dimxy Feb 26, 2025 •

edited

Loading

shamardy Feb 22, 2025

dimxy Feb 26, 2025

shamardy commented Feb 22, 2025

		@@ -0,0 +1,263 @@
		//? RPC implementations for swaps with liquidity routing (LR) of EVM tokens

		.map(\|((src, dst), series)\| {
		let dst_price = cross_prices_average(series);

	/// Finds an enabled ERC20 token by its contract address and returns it as `MmCoinEnum`.
	pub async fn get_enabled_erc20_by_contract(
	ctx: &MmArc,
	contract_address: Address,
	) -> MmResult<Option<MmCoinEnum>, String> {
	let cctx = CoinsContext::from_ctx(ctx)?;
	let coins = cctx.coins.lock().await;

	Ok(coins.values().find_map(\|coin\| match &coin.inner {
	MmCoinEnum::EthCoin(eth_coin) if eth_coin.erc20_token_address() == Some(contract_address) => {
	Some(coin.inner.clone())
	},
	_ => None,
	}))
	}

feat(swap): rpc to find best swap with liquidity routing for ask #2362

Are you sure you want to change the base?

feat(swap): rpc to find best swap with liquidity routing for ask #2362

Conversation

dimxy commented Feb 16, 2025 • edited Loading

mariocynicys left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shamardy left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dimxy Feb 26, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dimxy Feb 26, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shamardy commented Feb 22, 2025

dimxy commented Feb 16, 2025 •

edited

Loading

shamardy left a comment •

edited

Loading

dimxy Feb 26, 2025 •

edited

Loading

dimxy Feb 26, 2025 •

edited

Loading