Bug #1384: Deadlock with pending older transactions

[mmv08]

This PR:

• Fixes Deadlock with pending older transactions #1384
• Removes magic in the safe reducer where it tries to search the transaction from the state by the nonce, which may not work correctly if there are multiple transactions with the same nonce
• Enhance cancellation tx logic by checking if tx's nonce is less than current safe's nonce
• Remove an assumption of the pending status of the transaction if there are no confirmations. This is coming from an older version of the app but not needed now

[github-actions]

CLA Assistant Lite All Contributors have signed the CLA.

[github-actions]

ESLint Summary View Full Report

Annotations are provided inline on the Files Changed tab. You can also see all annotations that were generated on the annotations page.

Type	Occurrences	Fixable
Errors	0	0
Warnings	0	0
Ignored	1	N/A

• Result: ✅ success
• Annotations: 0 total

---

^{Report generated by eslint-plus-action}

[ghost]

Travis automatic deployment:
https://pr1455--safereact.review.gnosisdev.com/app

[ghost]

Travis automatic deployment:
https://pr1455--safereact.review.gnosisdev.com/app

[francovenica]

Since this issue seems to be caused by creating tx outside of the safe app I asked Dani to help me out.

He created a ton of tx quickly and showed me the comparison between this PR and the dev env.
In the dev env it would show only 1 tx per nonce, even if there were more than 1 tx with the same nonce.
In this PR every tx was shown, even if they had their nonce repeated, ordered by date of arrival.
Dani was signing those tx in order, and the ones with the repeated nonce were labeled as "Cancelled" automatically as the other were successfully executed.
It looks good to me.

The only thing that looks odd is having some tx being executed when others still were not executed yet prior to that one, this is because you have the tx ordered by date and not by nonce:
tx nonce 2 (pending)
tx nonce 1 (If this one is executed)
tx nonce 2 (pending
tx nonce 1 (Then this one would look cancelled, and the tx with nonce 2 above this one would look pending even when there is one above that one that is already executed)

We should let the user know with the "Other tx needs to be executed first" message that that "first" means the one with lower nonce and not the one with an older date of arrival

[mmv08]

I can confirm the ordering issue, however the current ordering logic is some magic I couldn’t understand, thus can’t fix it. This is definitely something we need to address

[dasanra]

I detected another bug related to this. If you have two transactions with the same nonce and one of them gets a reject signature, both will show that have a reject signature. This is caused because cancel transactions are fetched by nonce.

A further refactor is needed in order to relate signature by txHash. Will create a new ticket as the bug this ticket solves is bigger than the new detected.

[ghost]

Travis automatic deployment:
https://pr1455--safereact.review.gnosisdev.com/app

[mmv08]

A further refactor is needed in order to relate signature by txHash

how?

[mmv08]

Also, I'm questioning whether it's a bug at all

[mmv08]

Because even if you relate it to a safeTxHash somehow, the nonce is still the same, so canceling one transaction would still cancel the other one

[dasanra]

@mikheevm at least is a bug from the UX point of view. If I'm rejecting one transaction I don't expect to be rejecting two or more other transactions. Maybe I want to signal, hey this is the wrong transaction but both, the correct one and the one I want to reject get the reject signature.

I also found what you added later, that rejecting one transaction actually cancels all transactions with the same nonce. From an user point of view this can seem confusing and worth a check.

[dasanra]

Will squash and merge this now, as some reported issues are related to this (in xDai branch)

DO NOT DELETE THE BRANCH UNTIL FIX IS IN PRODUCTION

Synthetix is using this branch whenever they are locked by this bug. Will get in contact with them once this is released and branch deleted