-
Notifications
You must be signed in to change notification settings - Fork 360
fix: Disable queued Tx interaction while replacement Tx is pending #3405
Conversation
|
CLA Assistant Lite All Contributors have signed the CLA. |
ESLint Summary View Full Report
Report generated by eslint-plus-action |
|
E2E Tests Failed Failed tests:
|
Pull Request Test Coverage Report for Build 1789441569
💛 - Coveralls |
2dc9611 to
a6082c3
Compare
iamacook
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice selector! Just a couple things
|
|
||
| useEffect(() => { | ||
| if (activeHover && activeHover !== transaction.id) { | ||
| if ((activeHover && activeHover !== transaction.id) || (!isPending && nonce === pendingTxNonce)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If they are both undefined (nonce and pendingTxNonce) this condition will pass. You should check that they both exist as well.
4f3f253 to
b7da931
Compare
b7da931 to
ab4be46
Compare
|
|
||
| setTx(transaction) | ||
| }, [activeHover, transaction]) | ||
| }, [activeHover, transaction, isPending, nonce, pendingTxNonce]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You need to add isReplacementTxPending as a dependancy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch! Fixed it
iamacook
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀
What it solves
Resolves #3359
How this PR fixes it
We now take the pending state of a transaction into account to decide whether their replacement transaction should be disabled or not
How to test it
Screenshots