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

"Speed up" button should ignore cancellation attempts #5732

Closed
bdresser opened this issue Nov 12, 2018 · 15 comments
Closed

"Speed up" button should ignore cancellation attempts #5732

bdresser opened this issue Nov 12, 2018 · 15 comments
Assignees

Comments

@bdresser
Copy link
Contributor

Right now the "speed up" shows on the most recent tx with lowest nonce.

If that most recent tx is a cancellation attempt, the "speed up" button shows on the cancellation attempt.

So, if I submit a tx, it takes a long time, I try to cancel, then I decide I should just speed up the original tx, I'm unable to do so since the "Speed up" button is now on the cancellation attempt and not on the original tx.

If a user tries to cancel, we should probably show the "Speed up" button on both the original transaction AND the cancellation attemp

inspired by this comment

@alextsg

@bdresser
Copy link
Contributor Author

@cjeria what do you think? a community member might take this on once we agree on an acceptable behavior.

@bdresser
Copy link
Contributor Author

Or, put more simply, we could keep the "speed up" button on all pending tx with the lowest nonce.

@mehmeta
Copy link

mehmeta commented Nov 12, 2018

Thanks @bdresser, as discussed, happy to work on this once the design of the feature is agreed upon. I think it makes sense to display the button on all lowest nonce transactions.

@danfinlay
Copy link
Contributor

This is definitely addressed in a design we already have by Christian, where we bundle all cancel/speed-up attempts under a common tx attempt history entry. This should be treated as an extra reason to increase priority on completing that design.

Previously I'd lowered the priority of completing that full design because we already had another feature on the roadmap which would make it easier to implement: A data-layer abstraction around groups of related changes to a single "intended" transaction. #4260.

Maybe I need to get out of the way with this roadmap-optimization, though. That feature would be great for devs, but could probably be built on top of however the txs are grouped otherwise (Ideally by adding a new metamask-unique "intention ID" to new transactions, and copying it to retries and cancellations). @alextsg

@bdresser
Copy link
Contributor Author

bdresser commented Nov 12, 2018

This is definitely addressed in a design we already have by Christian

#5544, adding a couple open questions :)

@alextsg
Copy link
Contributor

alextsg commented Nov 12, 2018

Yeah I can start working on implementing the new grouping. I'll add questions to #5544 to verify some things with the new design. We can probably add this Speed Up button change first.

  • We don't want to keep the "speed up" button on all pending tx with the lowest nonce because we want to allow the user to speed up only the most recently sped up transaction right? Ie. If I create a tx (A), speed it up (B), then we want to show the Speed Up button only on B and not A right?
  • We currently show the Cancel button on all pending transactions, but in the same sense as the previous point, should we only show the Cancel button on the most recent Cancel Attempt? (in case the user wants to increase the gas price on a cancel attempt?)

@cjeria
Copy link
Contributor

cjeria commented Nov 12, 2018

This is a reasonable feature request, however I must ask, besides one user wanting to the ability to retry the original transaction (and essentially cancel the cancel tx attempt), is this a commonly requested feature?

Regardless of the answer to my above question, here are my initial thoughts on how this could work:

When we group tx's by nonce (which we currently don't do), the most recent pending tx with lowest nonce should always have a speed up button no matter its latest state (by "tx state", I mean the latest user action + highest gas price attempt).

Here's how I'm thinking this UX could work.
image

The new step to the speed up flow (which would only be initiated if a cancel attempt has been made and the user wants to speed up the tx) would be a modal that asks the user what do they want to speed up, the two options are:

  • Speed up original transaction
  • Speed up cancel attempt

And remove the cancel button if cancelling the tx has already been attempted.

@bdresser @alextsg @danfinlay @omnat thoughts?

@danfinlay
Copy link
Contributor

If I create a tx (A), speed it up (B), then we want to show the Speed Up button only on B and not A right?

Right.

should we only show the Cancel button on the most recent Cancel Attempt?

Yeah, for MVP that makes sense.

Eventually it should only show as one tx, of course, so it becomes moot.

Also for mvp, worth keeping in mind:

Since speed up is never on a cancel attempt tx, if you try to speed up a tx with multiple cancel attempt txs, it needs to increment its gas over their gas price values.

@danfinlay
Copy link
Contributor

Oh hey I just now saw Christian's awesome sketchwork. I like it. Minimal sketches ftw.

@omnat
Copy link

omnat commented Nov 13, 2018

@cjeria Love the low-fi sketches!

Are the following actions basically the same when a transaction is pending,
(a) speeding up the cancellation OR speeding up original transaction
(b) undo the cancellation (i.e., to speed up the transaction so it goes through faster than it gets cancelled)

If yes, I wonder which of the (a) or (b) would be more intuitive to users? My guess is (b), but then its a guess. Thoughts?

@omnat
Copy link

omnat commented Nov 13, 2018

img_20181112_193954

What do you think about the option on the right side? Giving the option to speed Go / No-go transactions instead of a global 'speed up' option.
Potential benefit: 'speed up' attached to the intended action, this is 1 step earlier
Potential risk: Cluttered screen per transaction (to avoid this, could Tx#1 history be collapsable?)

@alextsg
Copy link
Contributor

alextsg commented Nov 28, 2018

@cjeria @omnat @bdresser I'm working on grouping transactions by nonce and I just have the Speed up/Cancel flows left to add. Have we reached a decision on which UX to go with?

@cjeria
Copy link
Contributor

cjeria commented Nov 28, 2018

Since I don't know which option will test the best, would it be too much to include both options?

  • Speed up button on the top area of the of the details area > modal
  • and inline "speed up" per tx attempt within the activity log?

I do think the inline option is a clever solution, but I'm just not sure if by itself is enough for users to see it and therefore know to use it.

CC @bdresser @omnat @danfinlay

image

@bdresser
Copy link
Contributor Author

I like the inline option! It's explicit, simple, and lets us deal with edge cases well.

With the inline options present, I think the initial buttons up top become confusing/redundant

If the user has sped up multiple times

  • we should probably only show one inline speed up link (on the most recent attempt) - multiple links would all do the same thing basically.
  • show no speed up up top
  • still show cancel up top

If the user has attempted cancel, we should only show speed up

  • on the most recent (highest gas) cancellation attempt
  • on the most recent (highest gas) tx submission attempt
  • and remove the cancel button up top.

Maybe the text link can be more explicit? Speed up this transaction / Speed up this cancellation

Also happy to take this to slack if we wanna discuss there @cjeria @alextsg

@alextsg
Copy link
Contributor

alextsg commented Nov 28, 2018

Just a comment about the inline solution, but we also include the time and date for each activity in the Activity Log, so the line item is a bit longer than what's shown in the mock. Will that make things look too cluttered?

image (Don't mind the outdated colors/styling - will fix that up)

And sure, we can discuss this further in slack.

@ghost ghost assigned alextsg Dec 6, 2018
@ghost ghost added the in progress label Dec 6, 2018
@ghost ghost removed the in progress label Dec 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants