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

Fixed Vivek's bug #5860

Merged
merged 4 commits into from Nov 30, 2018

Conversation

@danfinlay
Copy link
Contributor

commented Nov 29, 2018

Fixes #5850

What was happening:

It seems that his MetaMask had crashed while some new transactions had
been loading defaults. He probably had a network connectivity issue to
Infura (which we are working with Infura to address).

As a result of this network cutout, his three unapproved transactions
were not marked failed, and were not marked as loadingDefaults = false, as their gas prices had not yet been estimated.

Normally this behavior is supposed to clean itself up when the
transaction controller starts up, via the
TransactionController._onBootCleanUp() function, but in this case,
during unlock, that function was unable to do its job because when it
requested the transaction list, the current network was in the loading
state, making it proceed as if there were no pending transactions.

Since we had unapproved transactions stuck in the loadingDefaults state, this triggered the loading spinner permanently on the confirmation screen.

To fix this, I am doing two things:

  • Setting transactions to loadingDefaults = false in more catch blocks.
  • Calling onBootCleanUp() when the network store's status changes, so
    that it will re-trigger when network loading completes.
Fixes #5850

What was happening:

It seems that his MetaMask had crashed while some new transactions had
been loading defaults. He probably had a network connectivity issue to
Infura (which we are working with Infura to address).

As a result of this network cutout, his three unapproved transactions
were not marked failed, and were not marked as `loadingDefaults =
false`, as their gas prices had not yet been estimated.

Normally this behavior is supposed to clean itself up when the
transaction controller starts up, via the
`TransactionController._onBootCleanUp()` function, but in this case,
during unlock, that function was unable to do its job because when it
requested the transaction list, the current network was in the `loading`
state, making it proceed as if there were no pending transactions.

To fix this, I am doing two things:
- Setting transactions to loadingDefaults = false in more catch blocks.
- Calling `onBootCleanUp()` when the network store's status changes, so
that it will re-trigger when loading completes.
@danfinlay danfinlay requested a review from frankiebee as a code owner Nov 29, 2018
@danfinlay

This comment has been minimized.

Copy link
Contributor Author

commented Nov 29, 2018

Closing until I fix this bug

@danfinlay danfinlay closed this Nov 29, 2018
Was refreshing the tx list on every tx state change instead of just
network changes, creating an infinite loop.
@danfinlay

This comment has been minimized.

Copy link
Contributor Author

commented Nov 29, 2018

That should do it.

@danfinlay danfinlay reopened this Nov 29, 2018
@metamaskbot

This comment has been minimized.

Copy link
Collaborator

commented Nov 29, 2018

Builds ready [5bbd329]: mascara, chrome, firefox, edge, opera
@frankiebee frankiebee dismissed their stale review Nov 29, 2018

unnescisary

Copy link
Member

left a comment

LGTM

Copy link
Member

left a comment

Thank you!

@metamaskbot

This comment has been minimized.

Copy link
Collaborator

commented Nov 29, 2018

Builds ready [59f89b9]: mascara, chrome, firefox, edge, opera
@metamaskbot

This comment has been minimized.

Copy link
Collaborator

commented Nov 29, 2018

Builds ready [59f89b9]: mascara, chrome, firefox, edge, opera
@danfinlay danfinlay merged commit c7233e2 into develop Nov 30, 2018
17 checks passed
17 checks passed
ci/circleci: all-tests-pass Your tests passed on CircleCI!
Details
ci/circleci: job-publish-prerelease Your tests passed on CircleCI!
Details
ci/circleci: job-screens Your tests passed on CircleCI!
Details
ci/circleci: prep-build Your tests passed on CircleCI!
Details
ci/circleci: prep-deps-npm Your tests passed on CircleCI!
Details
ci/circleci: prep-docs Your tests passed on CircleCI!
Details
ci/circleci: prep-scss Your tests passed on CircleCI!
Details
ci/circleci: test-deps Your tests passed on CircleCI!
Details
ci/circleci: test-e2e-beta-chrome Your tests passed on CircleCI!
Details
ci/circleci: test-e2e-beta-drizzle Your tests passed on CircleCI!
Details
ci/circleci: test-e2e-beta-firefox Your tests passed on CircleCI!
Details
ci/circleci: test-integration-flat-chrome Your tests passed on CircleCI!
Details
ci/circleci: test-integration-flat-firefox Your tests passed on CircleCI!
Details
ci/circleci: test-lint Your tests passed on CircleCI!
Details
ci/circleci: test-mozilla-lint Your tests passed on CircleCI!
Details
ci/circleci: test-unit Your tests passed on CircleCI!
Details
coverage/coveralls Coverage increased (+0.03%) to 65.752%
Details
@danfinlay danfinlay deleted the VivekBug branch Nov 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.