improvement(p2p): remove hardcoded seeds by onur-ozkan · Pull Request #2439 · GLEECBTC/komodo-defi-framework

onur-ozkan · 2025-05-06T07:16:47Z

Removes hardcoded seeds from KDF. It's better to provide some seed node addresses in the KDF docs for people running KDF manually without our GUI apps.
Adds 2 new boolean config fields is_bootstrap_node and disable_p2p.
Adds a p2p precheck flow to validate fields like i_am_seed, disable_p2p, seednodes, is_bootstrap_node, p2p_in_memory, etc.

Docs issues: GLEECBTC/komodo-docs-mdx#483 and GLEECBTC/komodo-docs-mdx#488

Breakage: KDF will no longer connect to 8762 mainnet by default if seednodes field is not configured properly.

Fixes #2409

Signed-off-by: onur-ozkan <work@onurozkan.dev>

… into remove-hardcoded-seeds

Signed-off-by: onur-ozkan <work@onurozkan.dev>

smk762 · 2025-05-06T23:12:12Z

Currently KDF will launch without seednodes param, though console logs will show 06 23:07:06, libp2p_gossipsub::behaviour:3424] INFO HEARTBEAT: relays low. Contains: 0 needs: 2

Should it rather fail to launch in this case? Without seednodes, not all functionality is lost (user can still activate coins and transact, sign messages etc) so there is a use case without them.

shamardy · 2025-05-07T02:23:45Z

Should it rather fail to launch in this case? Without seednodes, not all functionality is lost (user can still activate coins and transact, sign messages etc) so there is a use case without them.

We need a clear log message other than Currently KDF will launch without seed nodes param, though console logs will show 06 23:07:06, libp2p_gossipsub::behaviour:3424] INFO HEARTBEAT: relays low. Contains: 0 needs: 2. How about introducing a startup parameter like "wallet_only" (maybe choose a better name as wallet_only is also used in coins configs) and if set to true we can launch without seed nodes. Default behaviour will be to fail to launch if this is not set explicitly.

onur-ozkan · 2025-05-07T06:12:12Z

We can add an enable_p2p option to enable/disable the entire P2P network layer but it shouldn't be controlled by the seednodes values (for example on a bootstrap node, it shouldn't be set to false).

Should it rather fail to launch in this case? Without seednodes, not all functionality is lost (user can still activate coins and transact, sign messages etc) so there is a use case without them.

This is also problem for bootstrap seed nodes.

Signed-off-by: onur-ozkan <work@onurozkan.dev>

cipig · 2025-05-07T10:39:06Z

pleased keep in mind that Web and Mobile GUI code does not have the possibility to pass seednodes, while Desktop has: https://github.com/KomodoPlatform/komodo-wallet-desktop/blob/dev/src/core/atomicdex/config/kdf.cfg.hpp#L38-L39
so atm one can only build a working Desktop GUI

onur-ozkan · 2025-05-07T11:12:38Z

pleased keep in mind that Web and Mobile GUI code does not have the possibility to pass seednodes, while Desktop has: https://github.com/KomodoPlatform/komodo-wallet-desktop/blob/dev/src/core/atomicdex/config/kdf.cfg.hpp#L38-L39
so atm one can only build a working Desktop GUI

I don't know if they are doing or not doing it, but it's very possible to do it. We even have an example/dummy web app that runs KDF(WASM) with a custom configuration.

smk762 · 2025-05-07T11:41:26Z

pleased keep in mind that Web and Mobile GUI code does not have the possibility to pass seednodes, while Desktop has: https://github.com/KomodoPlatform/komodo-wallet-desktop/blob/dev/src/core/atomicdex/config/kdf.cfg.hpp#L38-L39 so atm one can only build a working Desktop GUI

GUIs should be able to add the param in backend either hardcoded or via an api endpoint (with fallback) to keep seeds fresh. cc: @CharlVS

shamardy · 2025-05-07T23:36:36Z

We can add an enable_p2p option to enable/disable the entire P2P network layer but it shouldn't be controlled by the seednodes values (for example on a bootstrap node, it shouldn't be set to false).

Default value for enable_p2p should be true. So, it makes more sense to name it disable_p2p and if not found, p2p is enabled but will fail to launch if no seed nodes are specified in config.

This is also problem for bootstrap seed nodes.

For bootstrap, we also have to specify it in config as a different mode. What do you think @onur-ozkan ?

onur-ozkan · 2025-05-20T05:01:28Z

@onur-ozkan Could you please clarify why you added "is_bootstrap_node" if we don't use "bootstrap_node" anywhere in the code (or I just can't find it)?
We have "i_am_seed", which indicates that the node is a seed (relay) node. But where is the bootstrap logic in repo (except for the is_bootstrap_node check itself)?

Please re-check lp_native_dex module. It's used there.

mm2src/mm2_main/src/lp_native_dex.rs

shamardy

Thanks for the PR! Only nits and 1 question!

README.md

mm2src/mm2_core/src/mm_ctx.rs

mm2src/mm2_p2p/src/behaviours/atomicdex.rs

shamardy · 2025-05-22T01:46:01Z

@onur-ozkan please merge with latest dev for wasm tests CI job.

… into remove-hardcoded-seeds

Signed-off-by: onur-ozkan <work@onurozkan.dev>

onur-ozkan · 2025-05-22T03:00:00Z

Can we have QA coverage on this PR? Just the swap functionality part.

cc @KomodoPlatform/qa

Signed-off-by: onur-ozkan <work@onurozkan.dev>

shamardy

LGTM!

shamardy · 2025-05-23T01:47:11Z

test_trade_base_rel_mycoin_mycoin1_coins_burnkey_as_alice is failing here but isn't failing in dev. Maybe it needs logs timeout updating similar to other tests.

shamardy · 2025-05-23T02:08:26Z

Also test_qrc20_tx_history fails in wasm for the same reason (waiting for logs timeouts), these are shared runners related failures I assume.

Signed-off-by: onur-ozkan <work@onurozkan.dev>

onur-ozkan · 2025-05-23T05:42:54Z

They both fixed now.

shamardy · 2025-05-27T06:15:27Z

Can we have QA coverage on this PR? Just the swap functionality part.

cc @KomodoPlatform/qa

@smk762

laruh

Thanks for the improvement!

smk762

Confirm fail to launch if no seednode param in config P2P initializing error: 'Precheck failed: 'Bootstrap node must also be a seed node.'' ✅
Confirm successful launch when param included.
Confirm swap initiates as expected
Entering the taker_swap_loop DOC/MARTY with uuid: d3bc6160-ce60-4fe0-a600-04dd1d9d587e
...
[swap uuid=d3bc6160-ce60-4fe0-a600-04dd1d9d587e] Finished

Can we have QA coverage on this PR? Just the swap functionality part.

Further testing to be done alongside docs drafting for the is_bootstrap_node and disable_p2p, but happy to approve the launch/swap portion and will offer any feedback on these configs if required.

Confirm disable_p2p allows launch without seednode param WARN P2P is disabled. Features that require a P2P network (like swaps, peer health checks, etc.) will not work.
Confirm fails to launch if disable_p2p & i_am_seed are both true P2P initializing error: 'Precheck failed: 'Seed nodes cannot disable P2P.''
Confirm fails to launch if i_am_seed is false & is_bootstrap_node is true P2P initializing error: 'Precheck failed: 'Bootstrap node must also be a seed node.''
Confirm fails to launch if i_am_seed is true, is_bootstrap_node is , and seednodes is not set 'Precheck failed: 'Non-bootstrap node must have seed nodes configured to connect.''

* 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) ...

- Removes all hardcoded seed nodes from KDF. Users must now supply their own seed node addresses in the `seednodes` config field; KDF will no longer auto-connect to 8762 mainnet by default. - Adds two explicit boolean config fields: `is_bootstrap_node` and `disable_p2p`. - Implements a P2P precheck flow that validates the combination of `i_am_seed`, `disable_p2p`, `is_bootstrap_node`, `seednodes`, `p2p_in_memory`, etc, and prevents misconfiguration. **BREAKING:** KDF will not connect to any seed nodes by default. If `seednodes` is not properly set, mainnet connections will fail. Fixes #2409

remove hardcoded seeds

cdab208

Signed-off-by: onur-ozkan <work@onurozkan.dev>

onur-ozkan added breaking-change: P2P status: in progress improvement: P2P labels May 6, 2025

switch to memory mode if seednodes not provided

b9df0d9

Signed-off-by: onur-ozkan <work@onurozkan.dev>

onur-ozkan force-pushed the remove-hardcoded-seeds branch 4 times, most recently from 8de43c6 to 6439234 Compare May 6, 2025 11:08

fix some tests

e9a3160

Signed-off-by: onur-ozkan <work@onurozkan.dev>

onur-ozkan force-pushed the remove-hardcoded-seeds branch from 6439234 to e9a3160 Compare May 6, 2025 11:42

Merge branch 'dev' of github.com:KomodoPlatform/komodo-defi-framework…

cfc6625

… into remove-hardcoded-seeds

onur-ozkan force-pushed the remove-hardcoded-seeds branch 3 times, most recently from 8f43fc0 to 3ff6b65 Compare May 6, 2025 18:13

fix lp_native_dex::seednodes

8a37ed0

Signed-off-by: onur-ozkan <work@onurozkan.dev>

onur-ozkan force-pushed the remove-hardcoded-seeds branch from 3ff6b65 to 8a37ed0 Compare May 6, 2025 18:17

onur-ozkan added status: pending review and removed status: in progress labels May 6, 2025

onur-ozkan mentioned this pull request May 6, 2025

documenting seednodes GLEECBTC/komodo-docs-mdx#483

Closed

smk762 mentioned this pull request May 6, 2025

update kdf: seednodes - optional -> required GLEECBTC/komodo-docs-mdx#484

Merged

shamardy added the priority: urgent Critical tasks requiring immediate action. label May 6, 2025

add WARN log

3393bc1

Signed-off-by: onur-ozkan <work@onurozkan.dev>

laruh reviewed May 20, 2025

View reviewed changes

mm2src/mm2_main/src/lp_native_dex.rs Show resolved Hide resolved

shamardy reviewed May 22, 2025

View reviewed changes

README.md Outdated Show resolved Hide resolved

mm2src/mm2_core/src/mm_ctx.rs Show resolved Hide resolved

mm2src/mm2_p2p/src/behaviours/atomicdex.rs Show resolved Hide resolved

onur-ozkan added 2 commits May 22, 2025 05:45

Merge branch 'dev' of github.com:KomodoPlatform/komodo-defi-framework…

ec1a62f

… into remove-hardcoded-seeds

nits

b4e2924

Signed-off-by: onur-ozkan <work@onurozkan.dev>

treat seed nodes from user config as explicit ones

6f066e3

Signed-off-by: onur-ozkan <work@onurozkan.dev>

shamardy previously approved these changes May 23, 2025

View reviewed changes

onur-ozkan dismissed shamardy’s stale review via d09c623 May 23, 2025 03:30

onur-ozkan force-pushed the remove-hardcoded-seeds branch 2 times, most recently from 392dd11 to fc98e10 Compare May 23, 2025 04:52

remove explicit peers if they are unsynced by clock

ff95dcb

Signed-off-by: onur-ozkan <work@onurozkan.dev>

onur-ozkan force-pushed the remove-hardcoded-seeds branch from fc98e10 to ff95dcb Compare May 23, 2025 05:42

shamardy requested a review from smk762 May 27, 2025 06:15

laruh approved these changes May 27, 2025

View reviewed changes

smk762 approved these changes May 28, 2025

View reviewed changes

shamardy added the 2.5.0-beta label May 28, 2025

shamardy approved these changes May 28, 2025

View reviewed changes

shamardy merged commit 0e6d246 into dev May 28, 2025
21 of 25 checks passed

shamardy deleted the remove-hardcoded-seeds branch May 28, 2025 07:03

shamardy mentioned this pull request May 28, 2025

Add test coverage for hard-coded seeds availabilities #2409

Closed

shamardy mentioned this pull request Jun 16, 2025

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

Merged

Conversation

onur-ozkan commented May 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

smk762 commented May 6, 2025

Uh oh!

shamardy commented May 7, 2025

Uh oh!

onur-ozkan commented May 7, 2025

Uh oh!

cipig commented May 7, 2025

Uh oh!

onur-ozkan commented May 7, 2025

Uh oh!

smk762 commented May 7, 2025

Uh oh!

shamardy commented May 7, 2025

Uh oh!

onur-ozkan commented May 20, 2025

Uh oh!

Uh oh!

shamardy left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

shamardy commented May 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

onur-ozkan commented May 22, 2025

Uh oh!

shamardy left a comment

Choose a reason for hiding this comment

Uh oh!

shamardy commented May 23, 2025

Uh oh!

shamardy commented May 23, 2025

Uh oh!

onur-ozkan commented May 23, 2025

Uh oh!

shamardy commented May 27, 2025

Uh oh!

laruh left a comment

Choose a reason for hiding this comment

Uh oh!

smk762 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!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

onur-ozkan commented May 6, 2025 •

edited

Loading

shamardy commented May 22, 2025 •

edited

Loading

smk762 left a comment •

edited

Loading