Skip to content

fix(event-streaming): initial addresses registration in utxo balance streaming#2431

Merged
shamardy merged 6 commits intodevfrom
fix-utxo-balance-events
May 1, 2025
Merged

fix(event-streaming): initial addresses registration in utxo balance streaming#2431
shamardy merged 6 commits intodevfrom
fix-utxo-balance-events

Conversation

@mariocynicys
Copy link
Copy Markdown
Collaborator

@mariocynicys mariocynicys commented Apr 29, 2025

We need to initially register the addresses we wanna get balance notification for. We used to do that in the past at coin enablement time (since event streaming config was static and doesn't change at runtime), that was removed since it doesn't make sense with runtime enabled/disabled streaming. But was never replaced with the appropriate logic to register these addresses once the streamer was enabled.

This PR register our addresses with the event streamer on initialization so it could track them.

@mariocynicys
do not error in address balance subscription or notification if the s…
c6c66d7 
…treamer isn't enabled

check utxo_prepare_addresses_for_balance_stream_if_enabled for context. we should not error when the streamer isn't enabled. we now silently proceed.
@mariocynicys
register all the wallet addresses initially in utxo balance event str…
8496a22 
…eamer
mariocynicys
mariocynicys commented Apr 29, 2025
View reviewed changes
borngraced
borngraced suggested changes Apr 30, 2025
View reviewed changes
mm2src/coins/utxo/utxo_balance_events.rs Outdated
.add_subscriptions(&scripthash_to_address_map)
.await;
// Convert the hashmap back to btreemap.
Ok(scripthash_to_address_map.into_iter().map(|(k, v)| (k, v)).collect())
Copy link
Copy Markdown

@borngraced borngraced Apr 30, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Ok(scripthash_to_address_map.into_iter().map(|(k, v)| (k, v)).collect())
Ok(scripthash_to_address_map.into_iter().collect())

Copy link
Copy Markdown
Collaborator Author

@mariocynicys mariocynicys Apr 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

c3b2047

@mariocynicys
Copy link
Copy Markdown
Collaborator Author

mariocynicys commented Apr 30, 2025 •
edited
Loading

@smk762 this should fix the utxo balance streaming issue. would be nice if u could also test the latency of the enable response (meant enable of the balance streamer - but not needed anymore anyways) for a large HD wallet (i.e. that has many addresses).

@mariocynicys mariocynicys requested a review from smk762 April 30, 2025 13:18
smk762
smk762 previously approved these changes May 1, 2025
View reviewed changes
Copy link
Copy Markdown

@smk762 smk762 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed working ✔️
Activation of a wallet with "min_addresses_number": 50 in request returned a status of Ok in seconds (completed on second status request after init).
Balance changes across different addresses were communicated by the stream as expected. LGTM, thanks!

shamardy
shamardy reviewed May 1, 2025
View reviewed changes
shamardy
shamardy reviewed May 1, 2025
View reviewed changes
shamardy
shamardy reviewed May 1, 2025
View reviewed changes
@shamardy
Copy link
Copy Markdown
Collaborator

shamardy commented May 1, 2025

@borngraced do you have any more comments or upcoming review for this PR?

borngraced
borngraced approved these changes May 1, 2025
View reviewed changes
Copy link
Copy Markdown

@borngraced borngraced left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well done

@shamardy shamardy changed the title fix(event-streaming): perform an initial registration for our addresses in utxo balance streaming fix(event-streaming): initial addresses registration in utxo balance streaming May 1, 2025
@shamardy shamardy merged commit 88deafb into dev May 1, 2025
24 of 25 checks passed
@shamardy shamardy deleted the fix-utxo-balance-events branch May 1, 2025 15:39
@shamardy shamardy mentioned this pull request May 2, 2025
Merged
dimxy pushed a commit to dimxy/komodo-defi-framework that referenced this pull request May 13, 2025
Merge branch 'dev' into fix-utxo-tx-fee-no-doge
864b5b0 
* dev: (26 commits)
  chore(deps): remove base58 and replace it completely with bs58 (GLEECBTC#2427)
  feat(tron): initial groundwork for full TRON integration (GLEECBTC#2425)
  fix(UTXO): improve tx fee calculation and min relay fee handling (GLEECBTC#2316)
  deps(timed-map): bump to 1.3.1 (GLEECBTC#2413)
  improvement(tendermint): safer IBC channel handler (GLEECBTC#2298)
  chore(release): complete v2.4.0-beta changelogs  (GLEECBTC#2436)
  fix(event-streaming): initial addresses registration in utxo balance streaming (GLEECBTC#2431)
  improvement(watchers): re-write use-watchers handling (GLEECBTC#2430)
  fix(evm): make withdraw_nft work in HD mode (GLEECBTC#2424)
  feat(taproot): support parsing taproot output address types
  chore(RPC): use consistent param name for QTUM delegation (GLEECBTC#2419)
  fix(makerbot): add LiveCoinWatch price provider (GLEECBTC#2416)
  chore(release): add changelog entries for v2.4.0-beta (GLEECBTC#2415)
  fix(wallets): prevent path traversal in `wallet_file_path` and update file extension (GLEECBTC#2400)
  fix(nft): make `update_nft` work with hd wallets using the enabled address (GLEECBTC#2386)
  fix(wasm): unify error handling for mm2_main (GLEECBTC#2389)
  fix(tx-history): token information and query (GLEECBTC#2404)
  test(electrums): fix failing test_one_unavailable_electrum_proto_version (GLEECBTC#2412)
  improvement(network): remove static IPs from seed lists (GLEECBTC#2407)
  improvement(best-orders): return an rpc error when we can't find best orders (GLEECBTC#2318)
  ...
dimxy pushed a commit that referenced this pull request May 28, 2025
Merge branch 'dev' into lr-swap-wip
d05697c 
* 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
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@shamardy shamardy shamardy approved these changes

+2 more reviewers

@borngraced borngraced borngraced approved these changes

@smk762 smk762 smk762 left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

status: pending review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@mariocynicys @shamardy @smk762 @borngraced