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.

Sign up for GitHub

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

Jump to bottom

[FEATURE] Manage transaction state #582

Merged
merged 42 commits into from Sep 9, 2021
Merged

[FEATURE] Manage transaction state #582

merged 42 commits into from Sep 9, 2021

Conversation

gantunesr
Copy link
Member

@gantunesr gantunesr commented Sep 7, 2021 •
edited

Description

This development only affects the mobile app since the extension has its own Transaction Controller

Improvement on how we process transaction state by updating the Transaction Controller. The extension currently has a Transaction State Controller to handle user scenarios, something we lack.

Use Cases

  • On transaction history we're currently using the gas values that the user sent the transaction with, but in many cases the actual gas spent was less so we are showing misleading information.
  • Wrong transaction state: Showing failed transaction, when the tx was successful.
  • Wrong transaction state: “Dropped and replaced” shows up in etherscan and our end shows pending
  • The states of transactions 2 transactions with same nonce are shown wrongly.
  • Transactions get stuck in the "Submitted" status.

gantunesr added 29 commits September 1, 2021 12:14
feat(TxState): Add logs to debug
23de831
feat(TxController): Add StateReconcileMethod enum
b26d881
feat(TxController): Add method to get outdated txs
de7f56a
feat(TxController): Add method to get array of outdated txs
23f6096
feat(TxController): Add method to get array of new txs
3e5818c
feat(TxController): Add method to reconcile txs from etherscan
376a972
feat(TxController): Add method to reconcile txs
2e3199c
feat(TxController): Add reconcile method to etherscan fetch
0af90be
test(TransactionController): Unify mocks in a single file
b9e7992
Update brach to main
ef99cff
feat(getEtherscanApiUrl): Modify method to construct any url
3bc9203
test(getEtherscanApiUrl): Update
2f53370
feat(TransactionController): Add EtherscanTransactionStatus
93e0561
feat(TransactionController): Add new status for TransactionStatus
fff6ddc
feat(TransactionController): Add new failed condition for blockchain …
94508a2 
…queries
feat(TransactionController): Get transaction from state
42c74c9
feat(TransactionController): Update transactionStateReconciler method
389a027
feat(TransactionController): Update etherscanTransactionStateReconcil…
d70848e 
…er method
feat(TransactionController): Update getOutdatedTransactions method
cc5848e
feat(TransactionController): Add isTransactionOutdated method
7323630
feat(TransactionController): Update isStatusOutdated method
3c054c9
feat(TransactionController): Add isGasDataOutdated method
053e9ec
feat(TransactionController): Update trimTransactionsForState method
146f26a
feat(TransactionController): Relocate isFinalState method
f00d46f
feat(handletransationStatusFetch): Add method to check tx status in e…
eeea0f2 
…therscan
feat(handleTransactionFetch): Update method
9b8e3ef
style(util): Indentation
5b9a7aa
test(TransactionController): Update
31b19ce
Merge branch 'main' of https://github.com/MetaMask/controllers into f…
35c46e7 
…eat/tx-state
@gantunesr gantunesr requested review from a team and brad-decker and removed request for a team September 8, 2021 20:24
sethkfman
sethkfman previously approved these changes Sep 8, 2021
View reviewed changes
Copy link
Contributor

@sethkfman sethkfman left a comment

Choose a reason for hiding this comment

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

LGTM

adonesky1
adonesky1 reviewed Sep 8, 2021
View reviewed changes
Copy link
Contributor

@adonesky1 adonesky1 left a comment

Choose a reason for hiding this comment

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

A few questions! Nothing major though.

src/transaction/TransactionController.ts Outdated Show resolved Hide resolved
src/transaction/TransactionController.ts Show resolved Hide resolved
src/transaction/TransactionController.ts Outdated Show resolved Hide resolved
src/transaction/TransactionController.ts Outdated Show resolved Hide resolved
src/util.ts Outdated Show resolved Hide resolved
adonesky1
adonesky1 previously approved these changes Sep 9, 2021
View reviewed changes
Copy link
Contributor

@adonesky1 adonesky1 left a comment

Choose a reason for hiding this comment

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

Tiny comment non-blocking nits. LGTM!

@gantunesr gantunesr merged commit 3f5ca30 into main Sep 9, 2021
@gantunesr gantunesr deleted the feat/tx-state branch September 9, 2021 15:03
MajorLift pushed a commit that referenced this pull request Oct 11, 2023
@MajorLift
[FEATURE] Manage transaction state (#582)
e770157
MajorLift pushed a commit that referenced this pull request Oct 11, 2023
@MajorLift
[FEATURE] Manage transaction state (#582)
2c139cb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adonesky1 adonesky1 adonesky1 approved these changes

@brad-decker brad-decker Awaiting requested review from brad-decker

@Gudahtt Gudahtt Awaiting requested review from Gudahtt

@sethkfman sethkfman Awaiting requested review from sethkfman

Assignees

@gantunesr gantunesr

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@gantunesr @Gudahtt @sethkfman @adonesky1