Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent invalid inline speedup #7085

Merged

Conversation

Gudahtt
Copy link
Member

@Gudahtt Gudahtt commented Aug 30, 2019

Speeding up anything but the pending transaction with the lowest nonce is ineffectual, as the transaction with the lowest nonce blocks the others from completing first. The inline speed-up button in the transaction activity log has been removed for these invalid cases.

The button will show up in the activity log for the pending transaction with the lowest nonce, but not for the others.

Screenshots:

speed-up-this-transaction
This shows two pending transactions, both cancelled. Only the oldest transaction (the one on top) shows the inline "Speed up this cancellation" button.

speed-up-inline
This screenshot shows the inline "Speed up this transaction" button. This is just to show that it still works - it's essentially unchanged, as this inline button only shows up after the top-level "Speed Up" button is used, and that initial speed-up isn't possible on any but the oldest pending transaction.

Closes #6844

@Gudahtt Gudahtt requested review from danjm and whymarrh as code owners August 30, 2019 22:32
@Gudahtt Gudahtt force-pushed the i6844-prevent-invalid-inline-speedup branch 3 times, most recently from 4aefd33 to 65165ca Compare August 31, 2019 15:49
Copy link
Contributor

@whymarrh whymarrh left a comment

Choose a reason for hiding this comment

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

Can we leave showRetry (instead of showSpeedUp) or replace the word retry with speed up everywhere? (I'm thinking of onRetry and handleRetry but there may be other instances.)

@Gudahtt
Copy link
Member Author

Gudahtt commented Sep 9, 2019

Yeah that rename doesn't really make sense anymore - I'll remove that commit. I had done that originally because that prop was used for cancellation as well in an earlier version of this branch. That isn't the case anymore though.

Speeding up anything but the pending transaction with the lowest nonce
is ineffectual, as the transaction with the lowest nonce blocks the
others from completing first. The inline speed-up button in the
transaction activity log has been removed for these invalid cases.

The button will show up in the activity log for the pending transaction
with the lowest nonce, but not for the others.

Closes MetaMask#6844
@Gudahtt Gudahtt force-pushed the i6844-prevent-invalid-inline-speedup branch from 65165ca to 54fef0c Compare September 9, 2019 19:15
Copy link
Contributor

@danjm danjm left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@whymarrh whymarrh left a comment

Choose a reason for hiding this comment

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

LGTM

@Gudahtt Gudahtt merged commit 0ffee10 into MetaMask:develop Sep 9, 2019
@Gudahtt Gudahtt added this to the UI Sprint 19 [Aug 19] milestone Sep 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cancel tx cancels wrong transaction if there is more than 1 pending
3 participants