Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(UTXO): balance event streaming for Electrum clients #2013

Merged
merged 23 commits into from
Dec 21, 2023

Conversation

onur-ozkan
Copy link
Member

@onur-ozkan onur-ozkan commented Nov 23, 2023

This PR aims to:

  • Implement balance events for UTXOs.
  • Fix electrum notification receiving bug.
  • Support broadcasting multiple balance events.
  • Implement subscribing to electrum scripthash.

Can be tested in a same way of #1978 PR.

Signed-off-by: onur-ozkan <work@onurozkan.dev>
Signed-off-by: onur-ozkan <work@onurozkan.dev>
@onur-ozkan onur-ozkan added enhancement New feature or request in progress Changes will be made from the author labels Nov 23, 2023
Signed-off-by: onur-ozkan <work@onurozkan.dev>
@onur-ozkan onur-ozkan changed the title feat(UTXO): balance event streaming for Electrum and Native clients feat(UTXO): balance event streaming for Electrum clients Nov 24, 2023
Signed-off-by: onur-ozkan <work@onurozkan.dev>
Signed-off-by: onur-ozkan <work@onurozkan.dev>
Signed-off-by: onur-ozkan <work@onurozkan.dev>
Signed-off-by: onur-ozkan <work@onurozkan.dev>
Signed-off-by: onur-ozkan <work@onurozkan.dev>
Signed-off-by: onur-ozkan <work@onurozkan.dev>
Signed-off-by: onur-ozkan <work@onurozkan.dev>
Signed-off-by: onur-ozkan <work@onurozkan.dev>
Signed-off-by: onur-ozkan <work@onurozkan.dev>
Signed-off-by: onur-ozkan <work@onurozkan.dev>
Signed-off-by: onur-ozkan <work@onurozkan.dev>
@onur-ozkan onur-ozkan added under review and removed in progress Changes will be made from the author labels Nov 27, 2023
@onur-ozkan onur-ozkan marked this pull request as ready for review November 27, 2023 11:54
@onur-ozkan onur-ozkan self-assigned this Nov 27, 2023
Copy link
Collaborator

@shamardy shamardy left a comment

Choose a reason for hiding this comment

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

Thank you for the PR! Only one note at this stage of the review.

mm2src/coins/utxo/utxo_balance_events.rs Outdated Show resolved Hide resolved
Copy link
Member

@artemii235 artemii235 left a comment

Choose a reason for hiding this comment

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

I also have few notes 🙂

mm2src/coins/utxo/utxo_balance_events.rs Outdated Show resolved Hide resolved
mm2src/coins/utxo/utxo_balance_events.rs Outdated Show resolved Hide resolved
mm2src/coins/utxo/utxo_balance_events.rs Outdated Show resolved Hide resolved
mm2src/mm2_core/src/mm_ctx.rs Outdated Show resolved Hide resolved
Signed-off-by: onur-ozkan <work@onurozkan.dev>
Copy link
Member

@artemii235 artemii235 left a comment

Choose a reason for hiding this comment

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

One more note.

mm2src/coins/utxo/utxo_balance_events.rs Outdated Show resolved Hide resolved
Copy link
Member

@artemii235 artemii235 left a comment

Choose a reason for hiding this comment

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

Next iteration.

mm2src/coins/utxo/utxo_balance_events.rs Outdated Show resolved Hide resolved
Comment on lines 144 to 145
log::error!("{e}");
continue;
Copy link
Member

Choose a reason for hiding this comment

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

It's worth triggering an event here to notify GUI/user that balance is likely changed, but its value is unknown at this point due to error. They will need to request it using RPC.

Copy link
Member Author

Choose a reason for hiding this comment

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

True, however, I think GUI needs to tell what kind of error(like the format of error structure) they want to handle from event streaming. Right now GUI not using this feature so it's not clear what kind of errors we should send. I will leave a TODO here.

mm2src/coins/utxo/utxo_balance_events.rs Outdated Show resolved Hide resolved
Signed-off-by: onur-ozkan <work@onurozkan.dev>
artemii235
artemii235 previously approved these changes Dec 4, 2023
Copy link
Member

@artemii235 artemii235 left a comment

Choose a reason for hiding this comment

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

Looks great, thanks for fixes! 🙂

Copy link
Collaborator

@shamardy shamardy left a comment

Choose a reason for hiding this comment

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

Thank you for the fixes! Next review iteration!

mm2src/coins/utxo/utxo_common.rs Outdated Show resolved Hide resolved
mm2src/coins/utxo/utxo_common.rs Outdated Show resolved Hide resolved
mm2src/coins/rpc_command/init_scan_for_new_addresses.rs Outdated Show resolved Hide resolved
mm2src/coins/utxo.rs Show resolved Hide resolved
mm2src/coins/utxo/utxo_balance_events.rs Outdated Show resolved Hide resolved
mm2src/coins/utxo/rpc_clients.rs Show resolved Hide resolved
Signed-off-by: onur-ozkan <work@onurozkan.dev>
…alanceOps` trait

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

@shamardy shamardy left a comment

Choose a reason for hiding this comment

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

🔥

@smk762
Copy link

smk762 commented Dec 9, 2023

Working here in CLI testing, though it seems every event is logged to the page twice
image

@onur-ozkan
Copy link
Member Author

Working here in CLI testing, though it seems every event is logged to the page twice image

Just checked, it's the electrum servers sending notification twice. It shouldn't cause any problem anyway I think.

Copy link

@ca333 ca333 left a comment

Choose a reason for hiding this comment

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

secure code reviewed

@ca333 ca333 requested a review from DeckerSU December 12, 2023 06:49
@onur-ozkan
Copy link
Member Author

I need this to be merged for fixing UTXO TODOs related to error events. @shamardy

@shamardy shamardy merged commit 21ca5cc into dev Dec 21, 2023
29 of 36 checks passed
@shamardy shamardy deleted the utxo-balance-stream-event branch December 21, 2023 11:47
dimxy added a commit to dimxy/komodo-defi-framework that referenced this pull request Dec 21, 2023
* dev: (22 commits)
  chore(config): remove vscode launchjson (KomodoPlatform#2040)
  feat(trading-proto-upgrade): wasm DB, kickstart, refund states, v2 RPCs (KomodoPlatform#2015)
  feat(UTXO): balance event streaming for Electrum clients (KomodoPlatform#2013)
  feat(tx): add new sign_raw_transaction rpc for UTXO and EVM coins (KomodoPlatform#1930)
  fix(p2p): handle encode_and_sign errors (KomodoPlatform#2038)
  chore(release): add changelog entries for v2.0.0-beta (KomodoPlatform#2037)
  chore(network): write network information to stdout (KomodoPlatform#2034)
  fix(price_endpoints): add cached url (KomodoPlatform#2032)
  deps(network): sync with upstream yamux (KomodoPlatform#2030)
  fix(config): accept a string as rpcport value (KomodoPlatform#2026)
  feat(nft): move db lock, add tx fee and confirmations (KomodoPlatform#1989)
  chore(network): update seednodes for netid 8762 (KomodoPlatform#2024)
  chore(network): add todo on peer storage behaviour (KomodoPlatform#2025)
  chore(network): exclude `168.119.236.249` from the seednode list (KomodoPlatform#2021)
  feat(network): deprecate 7777 network (KomodoPlatform#2020)
  chore(release): bump mm2 version to 2.0.0-beta (KomodoPlatform#2018)
  feat(UTXO swaps): kmd burn plan impl (KomodoPlatform#2006)
  chore(docs): fix the link to simple market maker in README.md (KomodoPlatform#2011)
  refactor(cli): cli dependency updates and warn on bad config perm (KomodoPlatform#1956)
  chore(containers and docs): update docs and container images (KomodoPlatform#2003)
  ...

# Conflicts:
#	mm2src/mm2_main/tests/mm2_tests/mm2_tests_inner.rs
#	mm2src/mm2_test_helpers/src/for_tests.rs
dimxy added a commit to dimxy/komodo-defi-framework that referenced this pull request Jan 23, 2024
* dev: (24 commits)
  chore(release): bump mm2 version to 2.1.0-beta (KomodoPlatform#2044)
  feat(trezor): add segwit support for withdraw with trezor (KomodoPlatform#1984)
  chore(config): remove vscode launchjson (KomodoPlatform#2040)
  feat(trading-proto-upgrade): wasm DB, kickstart, refund states, v2 RPCs (KomodoPlatform#2015)
  feat(UTXO): balance event streaming for Electrum clients (KomodoPlatform#2013)
  feat(tx): add new sign_raw_transaction rpc for UTXO and EVM coins (KomodoPlatform#1930)
  fix(p2p): handle encode_and_sign errors (KomodoPlatform#2038)
  chore(release): add changelog entries for v2.0.0-beta (KomodoPlatform#2037)
  chore(network): write network information to stdout (KomodoPlatform#2034)
  fix(price_endpoints): add cached url (KomodoPlatform#2032)
  deps(network): sync with upstream yamux (KomodoPlatform#2030)
  fix(config): accept a string as rpcport value (KomodoPlatform#2026)
  feat(nft): move db lock, add tx fee and confirmations (KomodoPlatform#1989)
  chore(network): update seednodes for netid 8762 (KomodoPlatform#2024)
  chore(network): add todo on peer storage behaviour (KomodoPlatform#2025)
  chore(network): exclude `168.119.236.249` from the seednode list (KomodoPlatform#2021)
  feat(network): deprecate 7777 network (KomodoPlatform#2020)
  chore(release): bump mm2 version to 2.0.0-beta (KomodoPlatform#2018)
  feat(UTXO swaps): kmd burn plan impl (KomodoPlatform#2006)
  chore(docs): fix the link to simple market maker in README.md (KomodoPlatform#2011)
  ...
dimxy added a commit to dimxy/komodo-defi-framework that referenced this pull request Feb 18, 2024
* evm-hd-wallet: (27 commits)
  Fix todo comments
  Fix HDAddressOps::Address trait bounds
  fix(indexeddb): fix IDB cursor.continue_() call after drop (KomodoPlatform#2028)
  security bump for `h2` (KomodoPlatform#2062)
  fix(makerbot): allow more than one prices url in makerbot (KomodoPlatform#2027)
  fix(wasm worker env): refactor direct usage of `window` (KomodoPlatform#1953)
  feat(nft): nft abi in withdraw_nft RPC, clear_nft_db RPC (KomodoPlatform#2039)
  refactor(utxo): refactor utxo output script creation (KomodoPlatform#1960)
  feat(ETH): balance event streaming for ETH (KomodoPlatform#2041)
  chore(release): bump mm2 version to 2.1.0-beta (KomodoPlatform#2044)
  feat(trezor): add segwit support for withdraw with trezor (KomodoPlatform#1984)
  chore(config): remove vscode launchjson (KomodoPlatform#2040)
  feat(trading-proto-upgrade): wasm DB, kickstart, refund states, v2 RPCs (KomodoPlatform#2015)
  feat(UTXO): balance event streaming for Electrum clients (KomodoPlatform#2013)
  feat(tx): add new sign_raw_transaction rpc for UTXO and EVM coins (KomodoPlatform#1930)
  fix(p2p): handle encode_and_sign errors (KomodoPlatform#2038)
  chore(release): add changelog entries for v2.0.0-beta (KomodoPlatform#2037)
  chore(network): write network information to stdout (KomodoPlatform#2034)
  fix(price_endpoints): add cached url (KomodoPlatform#2032)
  deps(network): sync with upstream yamux (KomodoPlatform#2030)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants