Skip to content
This repository has been archived by the owner on Jun 24, 2022. It is now read-only.

Fix race condition when replacing/cancelling on-chain txs #2474

Merged
merged 1 commit into from
Mar 2, 2022

Conversation

ramirotw
Copy link
Contributor

Summary

Close #1832

This fixes the case where an on-chain tx (wrap/unwrap/approve) is sent and then the user decides to either Cancel or Speed-up that tx. Two things can happen:

  1. The Cancel/Speed-up tx wins and gets mined before the original tx. This scenario was already working fine
  2. The original tx gets mined before the Cancel/Speed-up one. This scenario was causing issues

The solution avoids removing the old tx from the transactions state, so the perceived UI change is that now both transactions are going to be visible in the Recent Activities modal, and after one gets mined, the other is removed.

@ramirotw ramirotw added the Protofire Handled by Protofire development team label Feb 18, 2022
@github-actions
Copy link
Contributor

CLA Assistant Lite All Contributors have signed the CLA.

@github-actions
Copy link
Contributor

  • 🔭 GP Swap: CoW Protocol v2 Swap UI

ramirotw pushed a commit that referenced this pull request Feb 18, 2022
#2474)

* fix(SafariSquareDots): fixed by using border-top-with instead of border-width

* fix(SafariSqaureDots): fixed by using a min width of 5px and then following the width of the screen if the screen is too wide

* fix(SafariSquareDots): fixed by using Dot SVG instead of border-style CSS

* fix(SafariSquareDots):  Fixed code follows suggestions from the Pull Request

* regenerate snapshots

Co-authored-by: NITIPON CHINGTHONGCHAI <[email protected]>
Co-authored-by: Justin Domingue <[email protected]>
Copy link
Contributor

@alfetopito alfetopito left a comment

Choose a reason for hiding this comment

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

Code-wise looks ok.

@elena-zh can you check if you can still reproduce the issue here?

@elena-zh
Copy link

@alfetopito , I'm still testing.
For now I have found that the #1954 issue is still reproducible in Gnosis Chain, found another case #2492 that might be good to fix a bit later.

Copy link

@elena-zh elena-zh left a comment

Choose a reason for hiding this comment

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

The issue LGTM besides the mentioned above cases:

Besides, I have verified #2486 issue in this PR, and it is also fixed.

@alfetopito alfetopito merged commit 9059360 into develop Mar 2, 2022
@alfetopito alfetopito deleted the ramirotw/issue-1832-race-condition branch March 2, 2022 18:59
@github-actions github-actions bot locked and limited conversation to collaborators Mar 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Protofire Handled by Protofire development team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix race condition when cancelling/speeding up onchain transactions
3 participants