feat(db-arch): more dbdir to address_dir replacements #2398

mariocynicys · 2025-03-20T13:20:40Z

address_dir() will now default to dbdir() if the new-db-arch feature is disabled. That won't affect dev environment while working on this feature.

In this PR, a couple of dbdir() & ctx.sqlite_connection usages were replaces with address_dir() & address_db(). Suggesting to review each commit on its own.

~~TODO: Implement the global DB querying logic for new-db-arch targets.~~
Doing this in the next iteration/PR as I don't want to grow this one excessively and increase the review time.

NOTE: Please open a discussion regarding the FIXMEs in this PR and reply to the questions in them if possible.

…ation features

address available and easy to swap funcs

…on compilation features

also fixes non-set own script pubkey for recently spent output tracking on trezor

the gist of this commit is moving my_z_addr calcuations to ZCoinBuilder::new instead of ZCoinBuilder::build. this makes it so we can have a `my_z_addr_encoded` field in the builder, and we can use it to find the db path for wallet and cache dbs

remove an unused arg

and not pass it via func args

… dbs

mariocynicys · 2025-03-25T14:02:56Z

these are ready for review. please review it commit by commit, otherwise u lose coherency and merge unrelated contexts.

i wanted to stop the PR here and create a new one so to keep them small, but since it should be reviewed in a commit-by-commit fashion anyway, i will just add new commits onto here. i won't rebase already committed commits.

mariocynicys · 2025-03-25T14:13:36Z

base branch deleted by mistake.

this is simpiler - than having to require a specific address directory - and has no down side

we call address_dir() and then join the retruned pathbuf with other directory(s), i.e. nesting more directories (see e.g. eth_trace_path() or ln_data_dir()). this means we really need to also create not just the address directory but the directories we nest on top. this commit removes the directory creation in address_dir() and makes this method infallible, this simplifies it's usage. BUT, now the caller needs to create the address directory (or make sure it's created) and any nested directories needed on top before using that directory or accessing any files in it

to follow: 1- correctly set the address_dir for SavedSwap on initialization, i.e., set it to address_from_pubkey(maker_htlc_pubkey) instead of String::new() that is currenlty set. 2- allow load_my_swap_from_db to accept an optional address_dir to shortcut the roundtrip to the global DB if possible (sometiems in the code we already have the address_dir and we don't need to hit the global DB for that). 3- implemented the global DB swap address querying logic

…m DB this serves as a shortcut so we don't have to query the global DB to find the address directory of that UUID in question

mm2src/coins/z_coin.rs

mm2src/mm2_io/src/fs.rs

just like global and address dir

Co-authored-by: Samuel Onoja <[email protected]>

dimxy · 2025-05-16T18:14:56Z

mm2src/mm2_main/src/lp_swap/maker_swap.rs

-    let swap = match SavedSwap::load_my_swap_from_db(ctx, swap.uuid).await {
+    let maker_coin_pub = swap.my_maker_coin_htlc_pub();
+    let maker_coin_address = try_s!(swap.maker_coin.address_from_pubkey(&maker_coin_pub));
+    let swap = match SavedSwap::load_my_swap_from_db(ctx, Some(&maker_coin_address), swap.uuid).await {


I see here we pass some address to load_my_swap_from_db in the save_my_maker_swap_event fn and looks like we pass some only when we storing swaps.
I could not find where we pass Some(address) in functions where we actually read swaps, like load_from_db_by_uuid active_swaps_rpc. We always pass None there.
Is this correct?

so when an address is passed, this just skips the global DB query to find the address. the global DB hosts a uuid to address mapping. but if we already know the address, we can skip that.

dimxy · 2025-05-16T18:20:39Z

mm2src/coins/z_coin.rs

-            .await
+        #[cfg(not(target_arch = "wasm32"))]
+        let blocks_db = {
+            let cache_db_path = self.ctx.global_dir().join(format!("{}_cache.db", self.ticker));


We have directory functions and subpath consts like "{}_cache.db" "TX_CACHE" "SWAPS" etc in different places in several sources.
I am thinking of a single DirectoryManager object which would encapsulate all such consts inside and provide methods to access all required dirs

okay it's a TODO for now

thanks, will put the todo in the coming PR.

mm2src/coins/z_coin.rs

…global dir This partially reverts commit 6149dbc. and puts tx cache in global direcotry.

shamardy

🔥 ! Only non-blockers below.

mm2src/coins/utxo/utxo_builder/utxo_coin_builder.rs

mm2src/coins/z_coin.rs

onur-ozkan

LGTM other than simple questions.

onur-ozkan · 2025-05-20T04:41:28Z

mm2src/mm2_main/src/lp_swap/recreate_swap_data.rs

+        taker_swap.maker_address = negotiated_event
+            .maker_coin_htlc_pubkey
+            .and_then(|pubkey| maker_coin.address_from_pubkey(&pubkey).ok())
+            .unwrap_or("Couldn't get the maker coin address. Please set it manually.".to_string());


Shouldn't this be expect instead?

so this goes as follows: if we aren't able to get the address from pubkey for any reason. we will print this message in the resulting json as a placeholder, instead of failing.

who ever is importing the swap should manually edit the [m/t]aker_address to their own correct address to get a valid [M/T]akerSavedSwap.

this is only temporary, i plan in the future to include only the htlc pubkey (which is nonfallible), and let the importer of the recreated-swap convert such pubkey to an address. this avoids the cases where we can't generate the address from a simple pubkey (like zcoin) and the cases where we don't currently have this coin enabled to convert its pubkey to an address.

Thanks, let's keep this "unresolved" to keep it visible as a future ref.

mm2src/mm2_main/src/lp_swap/recreate_swap_data.rs

shamardy · 2025-05-20T07:55:23Z

@dimxy please check if all your comments were resolved and you have no more comments so that I can merge this PR. @mariocynicys can you please fix the conflicts?

* dev: fix(evm-api): find enabled erc20 token using platform ticker (#2445) chore(docs): add DeepWiki badge to README (#2463) chore(core): organize deps using workspace.dependencies (#2449) feat(db-arch): more dbdir to address_dir replacements (#2398)

* dev: (29 commits) fix(p2pk): validate expected pubkey correctly for p2pk inputs (#2408) chore(docs): update old urls referencing atomicdex or old docs pages (#2428) improvement(p2p): remove hardcoded seeds (#2439) fix(evm-api): find enabled erc20 token using platform ticker (#2445) chore(docs): add DeepWiki badge to README (#2463) chore(core): organize deps using workspace.dependencies (#2449) feat(db-arch): more dbdir to address_dir replacements (#2398) chore(build-artifacts): remove duplicated mm2 build artifacts (#2448) feat(pubkey-banning): expirable bans (#2455) fix(eth-balance-events): serialize eth address using AddrToString (#2440) chore(deps): remove base58 and replace it completely with bs58 (#2427) feat(tron): initial groundwork for full TRON integration (#2425) fix(UTXO): improve tx fee calculation and min relay fee handling (#2316) deps(timed-map): bump to 1.3.1 (#2413) improvement(tendermint): safer IBC channel handler (#2298) chore(release): complete v2.4.0-beta changelogs (#2436) fix(event-streaming): initial addresses registration in utxo balance streaming (#2431) improvement(watchers): re-write use-watchers handling (#2430) fix(evm): make withdraw_nft work in HD mode (#2424) feat(taproot): support parsing taproot output address types ...

* lr-swap-wip: (37 commits) fix custom token error name fix getting chain_id from protocol_data refactor (review): use dedicated large error cfg, add new fn to FromApiValueError, fix TODO, use experimental namespace for lr rpc, more Ticker alias feat(walletconnect): add WalletConnect v2 support for EVM and Cosmos (GLEECBTC#2223) feat(ibc-routing-part-1): supporting entire Cosmos network for swaps (GLEECBTC#2459) fix(test): fix HD Wallet message signing tests (GLEECBTC#2474) improvement(builds): enable static CRT linking for MSVC builds (GLEECBTC#2464) feat(wallet): implement HD multi-address support for message signing (GLEECBTC#2432) fix(p2pk): validate expected pubkey correctly for p2pk inputs (GLEECBTC#2408) chore(docs): update old urls referencing atomicdex or old docs pages (GLEECBTC#2428) improvement(p2p): remove hardcoded seeds (GLEECBTC#2439) fix(evm-api): find enabled erc20 token using platform ticker (GLEECBTC#2445) chore(docs): add DeepWiki badge to README (GLEECBTC#2463) chore(core): organize deps using workspace.dependencies (GLEECBTC#2449) feat(db-arch): more dbdir to address_dir replacements (GLEECBTC#2398) chore(build-artifacts): remove duplicated mm2 build artifacts (GLEECBTC#2448) feat(pubkey-banning): expirable bans (GLEECBTC#2455) fix(eth-balance-events): serialize eth address using AddrToString (GLEECBTC#2440) chore(deps): remove base58 and replace it completely with bs58 (GLEECBTC#2427) feat(tron): initial groundwork for full TRON integration (GLEECBTC#2425) ...

mariocynicys added the status: in progress label Mar 24, 2025

Base automatically changed from split-db to dev March 25, 2025 13:02

mariocynicys force-pushed the split-db-temp branch from 58f8c64 to daec676 Compare March 25, 2025 13:27

mariocynicys added 12 commits March 25, 2025 15:30

store stats files in global db directory

ac20d84

let MmCtx::address_dir() default back to Ctx::dbdir() based on compil…

b219751

…ation features

replace dbdir with address_dir in eth/erc20 traces path

8d48284

address available and easy to swap funcs

base the LN data directory and backup directory on address_dir

fa5134b

let MmCtx::address_db() default back to Ctx::sqlite_connection based …

74b9ecb

…on compilation features

use the address_db for LN sqlite DB

1e024d4

put tx cache in proper address directory

6149dbc

also fixes non-set own script pubkey for recently spent output tracking on trezor

fix clippy for tests

190e6e1

remove an unused arg

use the z_spending_key field inside the builder

ce446c3

and not pass it via func args

add a disucssion point regarding address dir for zcoin wallet & cache…

a1c7347

… dbs

fix wrong stats taker swap directory

97d9849

mariocynicys force-pushed the split-db-temp branch from daec676 to 97d9849 Compare March 25, 2025 13:47

mariocynicys added status: pending review improvement: database and removed status: in progress labels Mar 25, 2025

borngraced closed this Mar 25, 2025

borngraced deleted the split-db-temp branch March 25, 2025 14:07

mariocynicys restored the split-db-temp branch March 25, 2025 14:12

mariocynicys reopened this Mar 25, 2025

mariocynicys added 5 commits March 27, 2025 13:27

use global directory for swap locks

2cd8ecd

this is simpiler - than having to require a specific address directory - and has no down side

add cargo lock

302e23f

give the option to specify an address directory when loading swap fro…

abbe716

…m DB this serves as a shortcut so we don't have to query the global DB to find the address directory of that UUID in question

add a todo to getive different names to address_from_pubkey

2e74dff

dimxy reviewed May 15, 2025

View reviewed changes

mm2src/coins/z_coin.rs Outdated Show resolved Hide resolved

dimxy reviewed May 15, 2025

View reviewed changes

mm2src/mm2_io/src/fs.rs Outdated Show resolved Hide resolved

mariocynicys and others added 6 commits May 16, 2025 16:24

use checksummed address for eth address dir

83e4089

create similar abstraction for wallet dir

830100c

just like global and address dir

Update mm2src/coins/z_coin.rs

6665136

Co-authored-by: Samuel Onoja <[email protected]>

fix formatting

4e11c57

review(dimxy): refactor z_coin.rs to not lose trace location

1741f34

review(dimxy): don't use dyn in write/create_parents

e1f478f

dimxy reviewed May 16, 2025

View reviewed changes

mm2src/coins/z_coin.rs Outdated Show resolved Hide resolved

Revert "put tx cache in proper address directory" + move tx cache in …

28dc4ea

…global dir This partially reverts commit 6149dbc. and puts tx cache in global direcotry.

shamardy previously approved these changes May 20, 2025

View reviewed changes

mm2src/coins/utxo/utxo_builder/utxo_coin_builder.rs Show resolved Hide resolved

mm2src/coins/z_coin.rs Outdated Show resolved Hide resolved

onur-ozkan previously approved these changes May 20, 2025

View reviewed changes

shamardy mentioned this pull request May 20, 2025

fix(zcoin): correctly track unconfirmed z-coin notes #2331

Merged

put back the todo regarding recently spent outputs

6d8328a

mariocynicys dismissed stale reviews from onur-ozkan and shamardy via 6d8328a May 20, 2025 06:18

use mm_err instead of .map_err(|e| e.map(|e| ...

f76866a

merge with origin/dev

f40b39f

shamardy approved these changes May 21, 2025

View reviewed changes

shamardy merged commit b8b98cf into dev May 21, 2025
20 of 24 checks passed

shamardy deleted the split-db-temp branch May 21, 2025 02:40

shamardy mentioned this pull request Jun 16, 2025

chore(release): v2.5.0-beta #2491

Merged

takenagain mentioned this pull request Jun 24, 2025

Regression: UTXO activation via Trezor fails on staging (d62c20b4c), works on main and alt-main #2502

Closed

feat(db-arch): more dbdir to address_dir replacements #2398

feat(db-arch): more dbdir to address_dir replacements #2398

Uh oh!

Conversation

mariocynicys commented Mar 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mariocynicys commented Mar 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mariocynicys commented Mar 25, 2025

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

shamardy left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

onur-ozkan left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

shamardy commented May 20, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

mariocynicys commented Mar 20, 2025 •

edited

Loading

mariocynicys commented Mar 25, 2025 •

edited

Loading

onur-ozkan left a comment •

edited

Loading