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

getMethodData() default to abi decoded method if registry lookup errors #6367

Closed
wants to merge 1 commit into from

Conversation

danjm
Copy link
Contributor

@danjm danjm commented Mar 28, 2019

This fixes a bug that was causing the confirm screen to navigate to the simple send confirm screen when it should have been navigating to the token transfer confirm screen. This could happen in when the http request made by "registry.lookup" failed.

Our router depends on the contract method name when the registry lookup failed, the name passed to the router was undefined.

This PR adds a fallback in our getMethodData() function - the use of abiDecoder.decodeMethod -which prevents the above described issue. W

@whymarrh whymarrh force-pushed the default-method-name-to-abi-lookup branch from f2ecbcc to afe116e Compare March 29, 2019 13:59
@danjm danjm closed this in #6374 Mar 29, 2019
danjm added a commit that referenced this pull request Mar 29, 2019
* getMethodData() default to abi decoded method if registry lookup errors

* Update e2e tests to work with getMethodData() fallback changes

* Remove indeterminency in gas input key entering in send token e2e test.
@whymarrh whymarrh deleted the default-method-name-to-abi-lookup branch April 2, 2019 15:18
vittominacori added a commit to vittominacori/metamask-extension that referenced this pull request Apr 3, 2019
* 'master' of https://github.com/MetaMask/metamask-extension: (84 commits)
  Version 6.3.1
  Check token name constants against the lower case methodData name in conf tx switch container.
  Ensure correct confirm screen in case of smartcontract check fail
  About tab copy changes; Update route for Info & Help in account menu (MetaMask#6356)
  Revert to providing inline defaults in getHexGasTotal and increaseLastGasPrice (MetaMask#6361)
  Further e2e fixes in wake of MetaMask#6364 and MetaMask#6367 (MetaMask#6374)
  Split e2e run-all commands onto multiple lines
  Run from-import e2e tests with additional account, to avoid conflict with accounts in earlier test runs
  Use localhost instead of mainnent/infura for custom rpc tests in e2e tests.
  ci: Add build:test Gulp task for e2e build
  e2e: Update Contract Interaction wording
  e2e: Skip approving token test
  send-footer.utils.js - addressIsNew - improve readability
  test - unit - ui - actions - setCompletedOnboarding - fix stub to call callback
  lint fix
  actions - setCompletedOnboarding - make async with pify
  notice-controller - make markAllNoticesRead sync
  lib - nodeify - correctly wrap synchronous functions
  notices - markAllNoticesRead - use async/await
  buy-eth-url - lint fix
  ...
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.

1 participant