Skip to content
This repository has been archived by the owner on Nov 10, 2023. It is now read-only.

(Fix) Duplicated txs #508

Merged
merged 7 commits into from
Feb 10, 2020
Merged

(Fix) Duplicated txs #508

merged 7 commits into from
Feb 10, 2020

Conversation

fernandomg
Copy link
Contributor

This PR closes #489, by:

  • Verifying the last tx execution status, before attempting to mark as executable a tx based on the safe's threshold.

@fernandomg fernandomg self-assigned this Feb 3, 2020
@ghost
Copy link

ghost commented Feb 3, 2020

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

@fernandomg
Merge branch 'development' into fix/#489-duplicated-txs
7451c3d 
# Conflicts:
#	src/routes/safe/store/actions/createTransaction.js
@ghost
Copy link

ghost commented Feb 5, 2020

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

@ghost
Copy link

ghost commented Feb 5, 2020

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

francovenica
francovenica reviewed Feb 6, 2020
View reviewed changes
Copy link
Contributor

@francovenica francovenica left a comment

Choose a reason for hiding this comment

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

I had some issues:
Rejection is not working properly. I've check that is creating rejections for nounces that don't exist yet (create a rejection for a nounce 88 when the last tx created was the 83)

Other issues I had was creating 2 Tx quickly and seen them both with the same nounce or sometimes one of them not showing up at all.

In resume: it seems the nounce is not being properly generated for new Tx's

@fernandomg
Copy link
Contributor Author

@francovenica, there was a mess in the logic to calculate the nonce after merging development. My bad. Now it's working.

@ghost
Copy link

ghost commented Feb 6, 2020

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

francovenica
francovenica approved these changes Feb 6, 2020
View reviewed changes
Copy link
Contributor

@francovenica francovenica left a comment •
edited by fernandomg
Loading

Choose a reason for hiding this comment

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

Now it works fine, no more duplicated tx, no more issues with the nonces.

@fernandomg
Copy link
Contributor Author

@mikheevm can you give it a look?

mmv08
mmv08 approved these changes Feb 7, 2020
View reviewed changes
src/routes/safe/store/actions/createTransaction.js Outdated Show resolved Hide resolved
@ghost
Copy link

ghost commented Feb 7, 2020

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

@mmv08 mmv08 merged commit 30f3483 into development Feb 10, 2020
@mmv08 mmv08 deleted the fix/#489-duplicated-txs branch February 10, 2020 13:29
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@mmv08 mmv08 mmv08 approved these changes

@francovenica francovenica francovenica approved these changes

@lukasschor lukasschor Awaiting requested review from lukasschor

@Agupane Agupane Awaiting requested review from Agupane

Assignees

@fernandomg fernandomg

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@fernandomg @francovenica @mmv08