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

Allow speeding up of all submitted txs from the activity log. #9928

Closed
wants to merge 3 commits into from

Conversation

tmashuang
Copy link
Contributor

@tmashuang tmashuang commented Nov 20, 2020

Closes #9927

Although the root cause is a larger issue (#9053) with our grouping txs with nonces/speedups/cancels/failures(both from the node and on-chain).
This PR allows all submitted transactions to be sped up through the use of the inline speedup in the activity log.

I do not know if this is the appropriate way we should handle this, and could lead to more user errors than solutions with their state of the tx queue.

Manual testing steps:

  1. Ensure the Customize transaction nonce setting is on in Settings > Advanced
  2. On a testnet (Rinkeby) send a tx with a gas price of 0.1.
  3. Proceed to send another tx with same nonce as the first tx, the gas price can be the same but not >1. Note: it might be beneficial to change up some of the params to keep track of the txs.
  4. Since the tx are grouped be nonce, the 2nd tx will be primaryTransaction and the tx we show. But as pointed out in Replacing transaction with custom nonce and then Speedup reuses the original #9927 clicking speed up will speed up the 1st tx.

Although the root cause is a larger issue (#9053) with our grouping txs with nonces/speedups/cancels/failures(both from the node and on-chain).
This PR allows all submitted transactions to be sped up through the use of the inline speedup in the activity log.

I do not know if this is the appropriate way we should handle this, and could lead to more user errors than solutions with their state of the tx queue.
@tmashuang tmashuang requested a review from a team as a code owner November 20, 2020 23:42
@github-actions
Copy link
Contributor

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@metamaskbot
Copy link
Collaborator

Builds ready [65f3ef6]
Page Load Metrics (373 ± 44 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint308441126
domContentLoaded2816133729144
load2826153739144
domInteractive2806133729144

@Gudahtt
Copy link
Member

Gudahtt commented Jan 22, 2021

When I tried to go through the repro steps for this, I was able to see the speedup button on both transactions:

double-speedup

However, both speedup buttons did the same thing. They both sped up the original transaction, not the newer one. I kept track by setting a different amount on the 2nd transaction. But regardless which speedup button I used, the original amount was used.

@tmashuang
Copy link
Contributor Author

Closing this stale PR, there needs to be a larger PR to tackle the all the issues that encompasses speeding/canceling txs.

@tmashuang tmashuang closed this Feb 11, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Feb 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replacing transaction with custom nonce and then Speedup reuses the original
4 participants