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

State corruption in swapper when multiple trades are in flight #7161

Closed
woodenfurniture opened this issue Jun 18, 2024 · 3 comments · Fixed by #7551
Closed

State corruption in swapper when multiple trades are in flight #7161

woodenfurniture opened this issue Jun 18, 2024 · 3 comments · Fixed by #7551
Assignees
Labels
bug Something isn't working

Comments

@woodenfurniture
Copy link
Member

woodenfurniture commented Jun 18, 2024

Overview

The swapper is getting state updates from other trades when there is more than 1 trade currently in flight, resulting in corrupted state and incorrect states in the UI, as well as incorrect tx links.

References and additional details

Performing RUNE -> BTC trade immediately followed by BTC -> RUNE trade results in the execution polling both txs and shoving state from both into the same UI - note state flip-flopping between inbound and outbound and incorrect tx links:

Screen.Recording.2024-06-18.at.10.31.43.AM.mov

On completion the status corrects itself and the bogus tx link is removed:
image

the correct tx link:
https://viewblock.io/thorchain/tx/6c99c929485f1fc19494319fabccdc89eee841f0c8c682a0bf6f04a5395352fb
the bogus one:
https://viewblock.io/thorchain/tx/73D82DFCBB714CF911C321D51582D7517368E26EC80D5117184D2A158346B73C

See src/components/MultiHopTrade/components/MultiHopTradeConfirm/hooks/useTradeExecution.tsx - polling is supposed to be canceled on unmont, so that is either broken or the component isn't actually unmounted when going back to tradeinput page:
image

Acceptance Criteria

A given trade should only receive status updates for itself. Other trade polling should be unmounted or cancelled.

Need By Date

No response

Screenshots/Mockups

No response

Estimated effort

No response

@woodenfurniture woodenfurniture added the bug Something isn't working label Jun 18, 2024
@NeOMakinG NeOMakinG assigned NeOMakinG and unassigned NeOMakinG Jun 26, 2024
@NeOMakinG
Copy link
Collaborator

NeOMakinG commented Jun 27, 2024

I did try to reproduce but it's super hard and cost really much, I'm not even sure that this is a cancelPolling issue as both are called and defined, it might be linked to something else

Also I couldn't reproduce using an EVM tx as well as a thorchain Native to Native swap at the same time, might be only scoped to thorchain swaps

We might go further by using something like AVAX => BCH as it would cost less

@gomesalexandre
Copy link
Contributor

Note for grooming: we may want to use this as an opportunity to discuss handling pending Txs in the app, e.g initiating a THOR Tx but leaving the swapper, or quitting the app/leaving the swapper mid multi-hop Txs - somehow related to #7255

@0xean 0xean moved this from Backlog to Up next / groomed in ShapeShift Dashboard Jul 8, 2024
@woodenfurniture
Copy link
Member Author

Note for assignee taking this task:
TRade execution state is stored flat in redux, i.e is not stored by trade ID meaning state from previous trades will overwrite the current one.

The solution will be to:

  1. Store trade execution state in redux by trade ID to eliminate the possibility of other trades interfering with the current state
    a. This also opens up the possibility to "resume" a multi-hop trade to sign the second tx after closing the page
  2. Be sure to unmount any components/polling likely to persist state updates after leaving that page

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants