Fix Unable to determine contract standard error by amerkadicE · Pull Request #18300 · MetaMask/metamask-extension

amerkadicE · 2023-03-23T12:54:22Z

Explanation

Inside the useTransactionDisplayData hook we were sending the wrong token address to the getAndSetAssetDetails function and that would cause error : Unable to determine Contract Standard

This PR will ensure to send correct token address.

Fixes #18135
Fixes #17287

Manual Testing Steps

Go to test dapp https://metamask.github.io/test-dapp/
Deploy Collectibles
Mint a collectible
Add collectible to Wallet
Click NFT details
Send NFT
Confirm -- check MetaMask UI console
Notice there is no warning from the bug ticket

Pre-merge author checklist

I've clearly explained:
- What problem this PR is solving
- How this problem was solved
- How reviewers can test my changes
Sufficient automated test coverage has been added

Pre-merge reviewer checklist

Manual testing (e.g. pull and build branch, run in browser, test code being changed)
PR is linked to the appropriate GitHub issue
IF this PR fixes a bug in the release milestone, add this PR to the release milestone

If further QA is required (e.g. new feature, complex testing steps, large refactor), add the Extension QA Board label.

In this case, a QA Engineer approval will be be required.

github-actions · 2023-03-23T12:54:35Z

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 · 2023-03-23T13:39:12Z

Builds ready [18a7bd7]

builds: chrome, firefox
builds (beta): chrome, firefox
builds (flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4
- common: 0, 1, 2, 3, 4, 5
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- inpage: inpage
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1685 ± 101 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	99	174	128	20	10
		domContentLoaded	1409	2139	1671	215	103
		load	1424	2152	1685	210	101
		domInteractive	1409	2139	1671	215	103

Bundle size diffs

background: 0 bytes
ui: 2 bytes
common: 0 bytes

codecov · 2023-03-23T13:39:19Z

Codecov Report

Merging #18300 (18a7bd7) into develop (f92e463) will decrease coverage by 0.64%.
The diff coverage is n/a.

❗ Current head 18a7bd7 differs from pull request most recent head e32e400. Consider uploading reports for the commit e32e400 to get more accurate results

@@             Coverage Diff             @@
##           develop   #18300      +/-   ##
===========================================
- Coverage    65.15%   64.51%   -0.64%     
===========================================
  Files          936      913      -23     
  Lines        35965    35326     -639     
  Branches      9231     9071     -160     
===========================================
- Hits         23432    22788     -644     
- Misses       12533    12538       +5

Impacted Files	Coverage Δ
ui/hooks/useTransactionDisplayData.js	`71.33% <ø> (ø)`

... and 183 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

dzfjz · 2023-03-23T13:42:11Z

Verified by QA

…-error

metamaskbot · 2023-04-10T12:18:50Z

Builds ready [e32e400]

builds: chrome, firefox
builds (beta): chrome, firefox
builds (flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4
- common: 0, 1, 2, 3, 4, 5
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- inpage: inpage
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1728 ± 108 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	103	153	121	17	8
		domContentLoaded	1491	2311	1706	209	100
		load	1503	2324	1728	224	108
		domInteractive	1491	2311	1706	209	100

Bundle size diffs

background: 0 bytes
ui: 2 bytes
common: 0 bytes

We collect metric data for each phase of a transaction's lifecycle. During one phase, we try to detect the type of asset being transacted. This step was broken due to a missing `await`, so the asset type was never identified. Additionally, the missing `await` meant that errors were not being handled correctly, resulting in many junk Sentry error events. Fixes #18300

## **Description** We collect metric data for each phase of a transaction's lifecycle. During one phase, we try to detect the type of asset being transacted. This step was broken due to a missing `await`, so the asset type was never identified. Additionally, the missing `await` meant that errors were not being handled correctly, resulting in many junk Sentry error events. This seems to have been broken as part of a TypeScript refactor, in #23445, perhaps due to a pre-existing invalid type for this function that was added in #21330 [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/24770?quickstart=1) ## **Related issues** Fixes #18300 ## **Manual testing steps** TODO I haven't reproduced any actual bug/missing metrics data from this yet, but presumably we should see that and be able to test this. I discovered this by looking into the Sentry error volume side-effect. ## **Screenshots/Recordings** N/A ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. Co-authored-by: Derek Brans <dbrans@gmail.com>

We collect metric data for each phase of a transaction's lifecycle. During one phase, we try to detect the type of asset being transacted. This step was broken due to a missing `await`, so the asset type was never identified. Additionally, the missing `await` meant that errors were not being handled correctly, resulting in many junk Sentry error events. This seems to have been broken as part of a TypeScript refactor, in that was added in #21330 [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/24770?quickstart=1) Fixes #18300 TODO I haven't reproduced any actual bug/missing metrics data from this yet, but presumably we should see that and be able to test this. I discovered this by looking into the Sentry error volume side-effect. N/A - [x] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. Co-authored-by: Derek Brans <dbrans@gmail.com>

Fix Unable to determine contract standard error

18a7bd7

amerkadicE force-pushed the fix/unable-to-determine-contract-standard-error branch from 431edd0 to 18a7bd7 Compare March 23, 2023 12:55

amerkadicE marked this pull request as ready for review March 23, 2023 13:57

amerkadicE requested a review from a team as a code owner March 23, 2023 13:57

amerkadicE requested a review from jpuri March 23, 2023 13:57

danjm approved these changes Mar 23, 2023

View reviewed changes

Merge branch 'develop' into fix/unable-to-determine-contract-standard…

e32e400

…-error

legobeat approved these changes Apr 10, 2023

View reviewed changes

bschorchit merged commit 463fe40 into develop Apr 11, 2023

bschorchit deleted the fix/unable-to-determine-contract-standard-error branch April 11, 2023 00:56

github-actions bot locked and limited conversation to collaborators Apr 11, 2023

Gudahtt restored the fix/unable-to-determine-contract-standard-error branch April 12, 2023 01:38

HowardBraham deleted the fix/unable-to-determine-contract-standard-error branch January 19, 2026 19:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix Unable to determine contract standard error#18300

Fix Unable to determine contract standard error#18300
bschorchit merged 2 commits intodevelopfrom
fix/unable-to-determine-contract-standard-error

amerkadicE commented Mar 23, 2023 •

edited

Loading

Uh oh!

github-actions bot commented Mar 23, 2023

Uh oh!

metamaskbot commented Mar 23, 2023

Uh oh!

codecov bot commented Mar 23, 2023 •

edited

Loading

Uh oh!

dzfjz commented Mar 23, 2023

Uh oh!

metamaskbot commented Apr 10, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Conversation

amerkadicE commented Mar 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Explanation

Manual Testing Steps

Pre-merge author checklist

Pre-merge reviewer checklist

Uh oh!

github-actions bot commented Mar 23, 2023

Uh oh!

metamaskbot commented Mar 23, 2023

Uh oh!

codecov bot commented Mar 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

dzfjz commented Mar 23, 2023

Uh oh!

metamaskbot commented Apr 10, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

amerkadicE commented Mar 23, 2023 •

edited

Loading

codecov bot commented Mar 23, 2023 •

edited

Loading