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

Improve contract method data fetching #6623

Merged
merged 8 commits into from
May 28, 2019

Conversation

danjm
Copy link
Contributor

@danjm danjm commented May 17, 2019

Fixes #6529.
Fixes #6530.
Fixes #6591.

This PR significantly refactors how we decide which type of confirm screen to refactor and how we fetch contract method data when rendering a contract method call confirm screen (e.g. "Contract Interaction.") This also changes how contract method data (specifically, the name) is fetched for items in the transaction list.

Goals include:

  • minimize the number of network requests made when loading the send screen and transaction list
  • simplify and improve maintainability of the logic for correctly routing the send screen and correctly retrieving data needed to render
  • stop blocking the loading of the send screen on contract method fetch requests

This PR also includes a fix to the loading of gas data on the send screen: it no longer blocks rendering and users can now edit gas price even while it is loading.

This PR is best reviewed commit by commit
Remove async call from getTransactionActionKey() 2efb441
Stop blocking confirm screen rendering on method data loading, and ba… 4eff4fb
Remove use of withMethodData, fix use of knownMethodData, in relation… e2a7681
Load data contract method data progressively, making it non-blocking;… b2bb2e6
Allow editing of gas price while loading on the confirm screen. 91bdef3

@danjm danjm requested a review from whymarrh May 17, 2019 04:34
@danjm danjm force-pushed the 4byte-fallback branch from ea67ca9 to a3475ee Compare May 22, 2019 15:29
@danjm danjm force-pushed the improve-contract-method-data-fetching branch from 91bdef3 to 82d8a0a Compare May 22, 2019 15:34
@metamaskbot
Copy link
Collaborator

Builds ready [18a5866]: chrome, firefox, edge, opera

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

@danjm danjm merged commit cfa916a into 4byte-fallback May 28, 2019
@whymarrh whymarrh deleted the improve-contract-method-data-fetching branch May 28, 2019 15:53
danjm added a commit that referenced this pull request Jun 17, 2019
* Remove async call from getTransactionActionKey()

* Stop blocking confirm screen rendering on method data loading, and base screen route on transactionCategory

* Remove use of withMethodData, fix use of knownMethodData, in relation to transaction-list-item.component

* Load data contract method data progressively, making it non-blocking; requires simplifying conf-tx-base lifecycle logic.

* Allow editing of gas price while loading on the confirm screen.

* Fix transactionAction component and its unit tests.

* Fix confirm transaction components for cases of route transitions within metamask.

* Only call toString on id if truthy in getNavigateTxData()
danjm pushed a commit that referenced this pull request Jun 18, 2019
* Adds 4byte registry fallback to getMethodData() (#6435)

* Adds fetchWithCache to guard against unnecessary API calls

* Add custom fetch wrapper with abort on timeout

* Use opts and cacheRefreshTime in fetch-with-cache util

* Use custom fetch wrapper with timeout for fetch-with-cache

* Improve contract method data fetching (#6623)

* Remove async call from getTransactionActionKey()

* Stop blocking confirm screen rendering on method data loading, and base screen route on transactionCategory

* Remove use of withMethodData, fix use of knownMethodData, in relation to transaction-list-item.component

* Load data contract method data progressively, making it non-blocking; requires simplifying conf-tx-base lifecycle logic.

* Allow editing of gas price while loading on the confirm screen.

* Fix transactionAction component and its unit tests.

* Fix confirm transaction components for cases of route transitions within metamask.

* Only call toString on id if truthy in getNavigateTxData()

* Fix knownMethodData retrieval and data fetching from fourbyte
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.

3 participants