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

(Fix) Pending transaction amount set to zero#1316

Merged
mmv08 merged 16 commits intodevelopmentfrom
fix/#1305-pending-txs-amount
Sep 14, 2020
Merged

(Fix) Pending transaction amount set to zero#1316
mmv08 merged 16 commits intodevelopmentfrom
fix/#1305-pending-txs-amount

Conversation

@fernandomg
Copy link
Contributor

This PR closes #1305, by essentially adding dataDecoded to the mocked transaction.

There were side effects that showed up while developing:

Also, there were some runtime errors that complicates the development:

@fernandomg fernandomg self-assigned this Sep 2, 2020
@github-actions
Copy link

github-actions bot commented Sep 2, 2020

CLA Assistant Lite All Contributors have signed the CLA.

@github-actions
Copy link

github-actions bot commented Sep 2, 2020

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 4 0
Ignored 0 N/A
  • Result: ✅ success

  • Annotations: 4 total


[warning] @typescript-eslint/explicit-module-boundary-types

Require explicit return and argument types on exported functions' and classes' public class methods


Report generated by eslint-plus-action

@ghost
Copy link

ghost commented Sep 2, 2020

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

@ghost
Copy link

ghost commented Sep 3, 2020

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

@ghost
Copy link

ghost commented Sep 7, 2020

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

@ghost
Copy link

ghost commented Sep 7, 2020

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

@fernandomg
Copy link
Contributor Author

@mikheevm, @dasanra, @nicosampler, @Agupane can you guys have a look at this PR?

Copy link
Contributor

@mmv08 mmv08 left a comment

Choose a reason for hiding this comment

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

Good job! Only a few questions about some changes

@ghost
Copy link

ghost commented Sep 9, 2020

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

@ghost
Copy link

ghost commented Sep 10, 2020

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

@fernandomg
Copy link
Contributor Author

All "runtime fixes" were reverted. And I'll create issues for the different refactors/fixes required.

Thus, this PR remains unchanged

@fernandomg
Copy link
Contributor Author

@mikheevm can you approve this one to unlock it? I'll wait for @dasanra approval before moving it to Final QA

Thanks!

@francovenica
Copy link
Contributor

francovenica commented Sep 11, 2020

Tried sending daiC since that's a token that would show the 0 token being sent bug. That was fixed
Signed with 2 owners without executing right away. The pending status now show up
Same as before, but rejecting the tx
Same as before, but now executing right away. The pending status is back for this scenario as well
Tested 2 tx pending of executing, making sure that the one that is being executed won't jump to the top of the list for no reason. Looks good
image.png

Wasn't necessary but I tested by signing with ledger and trezor just in case.

Looks good to me

@francovenica francovenica self-requested a review September 11, 2020 19:09
@ghost
Copy link

ghost commented Sep 11, 2020

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

@ghost
Copy link

ghost commented Sep 14, 2020

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

@mmv08 mmv08 merged commit ac92f49 into development Sep 14, 2020
@mmv08 mmv08 deleted the fix/#1305-pending-txs-amount branch September 14, 2020 11:46
@github-actions github-actions bot locked and limited conversation to collaborators Sep 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

4 participants