feat(tpu-stats): send and store tpu swap status #2645

mariocynicys · 2025-10-09T12:22:44Z

This PR adds support for broadcasting and persisting finished (either successfully or refundinly) swaps' status for v2 swaps.

Here there are no JSON files or so. We only store the status in the stats DB.
The stats DB has been exteded to account for new optional fields (market_margin & is_maker_bot) and also swap_version that is set to 0 for old persisted swaps.

SwapStatus p2p message was modefied in a backward compatible manner allow the new tpu-status to be sent as it couldn't fit to the existing SavedSwap structure (esp. regarding MakerSavedEvent/TakerSavedEvents).

The FIXMEs should be resolved via discussion here.

also add a couple of columns for new data in stats table. we will be using the same stats table as before with these extra optional columns. also the events list is sent over but not stored as json like with legacy swaps. will decide later what do to with them (if they are supposed to be stored)

this also wraps the structs found in SwapStatusData inside a box because they are very different in size (i.e. for perf purposes)

we don't do so in aborted states though. we might wanna change that

this is prepared for tests rather than real usage, but could ofc be amended later. for now it only exposes a couple of fields just to make sure the swap persisted in the db correctly

onur-ozkan · 2025-10-16T06:02:29Z

mm2src/mm2_main/src/database/stats_swaps.rs

+    // NULL = unknown, 0 = not a bot, 1 = bot
+    "ALTER TABLE stats_swaps ADD COLUMN is_maker_bot INTEGER;",


I wonder if making it maker_type with backing it Rust enum would be any good. 🤔 Just for the extensibility.

So it would look something like this:

enum MakerType { Unknown, WebUser, DesktopUser, Bot, . . . }

and can be extended to anything without changing things on the DB side.

I wonder if making it maker_type with backing it Rust enum would be any good.

like the idea. we can have a new issue about that to harvest such data (cc/ @shamardy)

we can ofc do a migration to change the table to the appropriate format later.

mm2src/mm2_main/src/lp_swap.rs

onur-ozkan · 2025-10-16T06:09:28Z

mm2src/mm2_main/src/lp_swap.rs

+    common::async_blocking(move || {
+        if let Some(conn) = ctx.sqlite_conn_opt() {
+            crate::database::stats_swaps::add_v2swap_to_index(&conn, &swap)


Why not making add_v2swap_to_index and spawning the task inside of it? If it's a heavy blocking task, then it should do this in a self-contained way as the caller may not always remember to wrap it with a spawning task.

crate::database::stats_swaps::add_v2swap_to_index is only used by one method which is lp_swap::add_v2swap_to_stats_db_index (the latter is a simple wrapper around the former)

same for crate::database::stats_swaps::add_swap_to_index and lp_swap::add_swap_to_db_index

so having the async_blocking at either level is the doable. i went for adding the async_blocking to the top level as the db-level functions take &Connection which isn't gonna play nice with the requirements of async_blocking (we can ofc pass the whole MmArc to the db-level functions but i think them taking a &Connection is more intuitive).

also other db methods follow the same sync interface, so that aligns with them (except our &AsyncConnection-based code)

I think other db functions should do the same but I don't want to block this PR with other tech debts. It's fine if you don't want to do it that way.

onur-ozkan · 2025-10-16T06:09:37Z

mm2src/mm2_main/src/lp_swap.rs

+    common::async_blocking(move || {
+        if let Some(conn) = ctx.sqlite_conn_opt() {
+            crate::database::stats_swaps::add_swap_to_index(&conn, &swap)


mm2src/mm2_main/src/lp_swap.rs

mm2src/mm2_main/src/lp_swap/maker_swap_v2.rs

onur-ozkan · 2025-10-16T06:23:02Z

mm2src/mm2_main/src/lp_swap/maker_swap_v2.rs

        self: Box<Self>,
        state_machine: &mut Self::StateMachine,
    ) -> <Self::StateMachine as StateMachineTrait>::Result {
+        // FIXME: Do we want to broadcast the swap status here?


this covers other cases like (e.g.) a maker who didn't see the funding transaction for some reason and aborted the swap, or a taker that failed to send the funding transaction and aborted the swap. idk if we want statistics about failures like these.

though a swap finished via a refund is accounted for and sent for stats.

I think this is something we would like to monitor.

Opinions ? @shamardy @smk762 @dimxy

Swap stat is collected on seed nodes right?
As far as I can see we only store if the swap status is success or not, in the stat db.
So even if one party aborts the swap its status still will be updated by other party when it gets to the last state (and broadcasts the final state)

Reasons for failure could be good intel for future works to mitigate.
Correct, seednodes do collect, in stats_swaps table column is_success.
https://dev.komodo-docs.pages.dev/en/docs/komodo-defi-framework/tutorials/query-the-mm2-database/#stats-swaps

mm2src/mm2_main/src/lp_swap/swap_v2_common.rs

…ions

onur-ozkan

LGTM other than #2645 (comment), which is awaiting others responses.

dimxy · 2025-10-21T11:38:24Z

mm2src/mm2_main/src/lp_swap.rs

+    Ok(())
+}

+async fn broadcast_my_v2swap_status(ctx: &MmArc, status: TPUSwapStatusForStats) -> Result<(), String> {


should this function not be called save_and_broadcast_my_v2swap_status?

hmm save is done only for native, maybe no need to rename then

dimxy · 2025-10-21T16:34:02Z

mm2src/mm2_main/src/lp_swap.rs

-    if let Some(conn) = ctx.sqlite_conn_opt() {
-        crate::database::stats_swaps::add_swap_to_index(&conn, swap)
-    }
+pub async fn add_v2swap_to_stats_db_index(ctx: &MmArc, swap: TPUSwapStatusForStats) -> Result<(), String> {


why do we have _index in add_v2swap_to_stats_db_index and add_v2swap_to_index fn names?
I think this mattered for the legacy swaps as there is indeed an index over json files with stat data.

For TPU we have only a db, no jsons, so could we name them, like: save_swap_v2_stat and add_swap_v2_stat_to_db?

i was more or less following the names we have for legacy swaps. that being said, i think of index as tables/sqls.

db used to mean a sqldb to me, but it's used in a different way in our codebase. we treat json files as db (file/directory DB basically), see for example load_from_maker_stats_db.

that's why i find the _index distinction helpful in this case as to denote that such db storage is a table format rather than file dump. but i can remove it too if u find it way off.

github-actions · 2025-11-30T12:01:24Z