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

active/passive modes for parent coins #1763

Merged
merged 15 commits into from
May 9, 2023
Merged

active/passive modes for parent coins #1763

merged 15 commits into from
May 9, 2023

Conversation

onur-ozkan
Copy link
Member

@onur-ozkan onur-ozkan commented Apr 24, 2023

This PR brings active/passive modes for parent coins which aims to keep children token states in memory when disable_coin is executed for parent coin.

Progress:

  • adding is_available atomic flag, is_available and update_is_available to coins context.
  • using is_available and update_is_available on certain conditions around coin activations and disable_coin.
  • when parent coin is passived(disabled while having active tokens), only update is_available to true on re-activation.
  • implement force_disable_all RPC to drop everything on target coin. add force_disable flag to disable_coin RPC
  • update related tests like mm2_test_helpers::for_tests::disable_coin_err
  • add unit and integration tests for testing this implementation
  • return passivized flag on disable_coin RPC response

Breaking Changes:

dependent_tokens is removed from the RPC response. Because we no longer care about if there are active tokens or not.

--

Docs for GUI:

cc @yurii-khi @KomodoPlatform/qa

  • new param force_disable: bool(false as default) for disable_coin RPC request payload

    • When this is not true, if platform has tokens enabled, disable_coin will passive the platform coin with keeping all the tokens active and usable. When force_disable is true, no matter if platform passive, or active, it will be disabled with all it's tokens.
  • new field passivized on disable_coin RPC response

    • This is only true if requested coin was passivized. This can be useful for showing the passivized coin with different color on GUI side(maybe even let user to force_disable passivized coins to disable all tokens and drop swaps which is faster in terms of UX)

When passive coin is re-activated, it only changes it's passive status(and activates the tokens if enabled with tokens) without updating the other fields like rpc urls, tx history, etc. If these fields needs to be changed, the coin must be disabled with force_disable flag which cleans up the memory from that platform.

--

Resolves #1641

Signed-off-by: ozkanonur <work@onurozkan.dev>
@onur-ozkan onur-ozkan added in progress Changes will be made from the author enhancement New feature or request labels Apr 24, 2023
@onur-ozkan onur-ozkan self-assigned this Apr 24, 2023
@onur-ozkan onur-ozkan force-pushed the passive-coin-state branch 3 times, most recently from 6b6c4ed to ffdec19 Compare April 26, 2023 21:00
@onur-ozkan onur-ozkan changed the title passive coin state active/passive modes for parent coins Apr 26, 2023
Signed-off-by: ozkanonur <work@onurozkan.dev>
Signed-off-by: ozkanonur <work@onurozkan.dev>
Signed-off-by: ozkanonur <work@onurozkan.dev>
Signed-off-by: ozkanonur <work@onurozkan.dev>
Signed-off-by: ozkanonur <work@onurozkan.dev>
Signed-off-by: ozkanonur <work@onurozkan.dev>
Signed-off-by: ozkanonur <work@onurozkan.dev>
@onur-ozkan onur-ozkan force-pushed the passive-coin-state branch 2 times, most recently from aefca72 to b8ed77c Compare May 3, 2023 11:38
@onur-ozkan onur-ozkan requested a review from shamardy May 3, 2023 12:32
@onur-ozkan onur-ozkan added under review and removed in progress Changes will be made from the author labels May 3, 2023
mm2src/coins/lightning.rs Outdated Show resolved Hide resolved
Signed-off-by: ozkanonur <work@onurozkan.dev>
@yurii-khi
Copy link

I ran a few tests using the web GUI, and everything is working smoothly so far without any changes on our side. Thanks, @ozkanonur !

Signed-off-by: ozkanonur <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.

Thanks a lot for the fix!
This is a first review iteration since I will resume reviewing the rest of the changes after getting your opinion @ozkanonur on my third comment.

mm2src/coins/lp_coins.rs Outdated Show resolved Hide resolved
mm2src/coins/qrc20.rs Outdated Show resolved Hide resolved
mm2src/coins/lp_coins.rs Outdated Show resolved Hide resolved
Signed-off-by: ozkanonur <work@onurozkan.dev>
shamardy
shamardy previously approved these changes May 4, 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.

LGTM! only one non-blocker :)

mm2src/coins/lp_coins.rs Outdated Show resolved Hide resolved
Signed-off-by: ozkanonur <work@onurozkan.dev>
Copy link

@caglaryucekaya caglaryucekaya left a comment

Choose a reason for hiding this comment

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

Great work!

mm2src/coins/lp_coins.rs Outdated Show resolved Hide resolved
mm2src/coins/lp_coins.rs Outdated Show resolved Hide resolved
Signed-off-by: ozkanonur <work@onurozkan.dev>
Signed-off-by: ozkanonur <work@onurozkan.dev>
@shamardy
Copy link
Collaborator

shamardy commented May 9, 2023

@caglaryucekaya did you approve this PR, or are you doing another review?

Copy link

@caglaryucekaya caglaryucekaya left a comment

Choose a reason for hiding this comment

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

LGTM 🔥

@shamardy shamardy merged commit 6168109 into dev May 9, 2023
@shamardy shamardy deleted the passive-coin-state branch May 9, 2023 18:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request under review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants