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

[Bug]: Gas - MV3 - Editting a gas from a dapp tx breaks with Cannot read properties of undefined (reading 'low') #24732

Closed
seaona opened this issue May 23, 2024 · 1 comment · Fixed by #24810
Assignees
Labels
release-11.16.6 Issue or pull request that will be included in release 11.16.6 release-12.0.0 Issue or pull request that will be included in release 12.0.0 Sev2-normal Normal severity; minor loss of service or inconvenience. team-extension-platform type-bug

Comments

@seaona
Copy link
Contributor

seaona commented May 23, 2024

Describe the bug

Whenever I am trying to edit the gas from a transaction that comes from a dapp, MetaMask breaks with the error Cannot read properties of undefined (reading 'low').

Note: it feels like this should be captured by a test, so we should probably add one here alongside with the fix

Expected behavior

No response

Screenshots/Recordings

Screenshot from 2024-05-23 08-45-16

edit-gas-udnefined-low.mp4

Steps to reproduce

  1. Go to the test dapp
  2. Trigger any tx
  3. Click the Edit gas button
  4. See MM breaks with the error Cannot read properties of undefined (reading 'low')

Error messages or log output

No response

Version

develop MV3 build

Build type

None

Browser

Chrome

Operating system

Linux

Hardware wallet

No response

Additional context

No response

Severity

No response

@github-project-automation github-project-automation bot moved this to To be fixed in Bugs by severity May 23, 2024
@github-project-automation github-project-automation bot moved this to To be fixed in Bugs by team May 23, 2024
@seaona seaona added the Sev2-normal Normal severity; minor loss of service or inconvenience. label May 23, 2024
@DDDDDanica DDDDDanica moved this from To be fixed to Fix in Progress in Bugs by severity May 27, 2024
DDDDDanica added a commit that referenced this issue May 29, 2024
…24810)

<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**
There is a racing condition captured in MV3 build. When sending
transaction from dapp directly, if user chooses to edit the gas fee, and
when `suggestedGasFees ` request is processing slow, `gasFeeEstimates`
could be undefined within hook `useCustomTimeEstimate.js`. The fix is to
add a condition for accessing this object before forwarding to UI.


<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/24810?quickstart=1)

## **Related issues**

Fixes: #24732

## **Manual testing steps**

1. Build in MV3
2. Trigger a txn from dapp
3. Open network panel, and trigger edit gas button before
`suggestedGasFees` is finished fetching
4. You should see the network edition panel

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**
 by @seaona 


https://github.com/MetaMask/metamask-extension/assets/12678455/f3e660a8-2d7b-4d1d-be31-69d5cd66d374


<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**


https://github.com/MetaMask/metamask-extension/assets/12678455/42b2df9d-a861-41ec-914e-89229493e276


- [ ] 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
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] 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.
@github-project-automation github-project-automation bot moved this from Fix in Progress to Fixed in Bugs by severity May 29, 2024
@github-project-automation github-project-automation bot moved this from To be fixed to Fixed in Bugs by team May 29, 2024
danjm pushed a commit that referenced this issue May 31, 2024
…24810)

<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**
There is a racing condition captured in MV3 build. When sending
transaction from dapp directly, if user chooses to edit the gas fee, and
when `suggestedGasFees ` request is processing slow, `gasFeeEstimates`
could be undefined within hook `useCustomTimeEstimate.js`. The fix is to
add a condition for accessing this object before forwarding to UI.


<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/24810?quickstart=1)

## **Related issues**

Fixes: #24732

## **Manual testing steps**

1. Build in MV3
2. Trigger a txn from dapp
3. Open network panel, and trigger edit gas button before
`suggestedGasFees` is finished fetching
4. You should see the network edition panel

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**
 by @seaona 


https://github.com/MetaMask/metamask-extension/assets/12678455/f3e660a8-2d7b-4d1d-be31-69d5cd66d374


<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**


https://github.com/MetaMask/metamask-extension/assets/12678455/42b2df9d-a861-41ec-914e-89229493e276


- [ ] 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
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] 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.
danjm pushed a commit that referenced this issue May 31, 2024
…24810)

<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**
There is a racing condition captured in MV3 build. When sending
transaction from dapp directly, if user chooses to edit the gas fee, and
when `suggestedGasFees ` request is processing slow, `gasFeeEstimates`
could be undefined within hook `useCustomTimeEstimate.js`. The fix is to
add a condition for accessing this object before forwarding to UI.


<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/24810?quickstart=1)

## **Related issues**

Fixes: #24732

## **Manual testing steps**

1. Build in MV3
2. Trigger a txn from dapp
3. Open network panel, and trigger edit gas button before
`suggestedGasFees` is finished fetching
4. You should see the network edition panel

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**
 by @seaona 


https://github.com/MetaMask/metamask-extension/assets/12678455/f3e660a8-2d7b-4d1d-be31-69d5cd66d374


<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**


https://github.com/MetaMask/metamask-extension/assets/12678455/42b2df9d-a861-41ec-914e-89229493e276


- [ ] 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
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] 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.
danjm pushed a commit that referenced this issue May 31, 2024
…24810)

<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**
There is a racing condition captured in MV3 build. When sending
transaction from dapp directly, if user chooses to edit the gas fee, and
when `suggestedGasFees ` request is processing slow, `gasFeeEstimates`
could be undefined within hook `useCustomTimeEstimate.js`. The fix is to
add a condition for accessing this object before forwarding to UI.


<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/24810?quickstart=1)

## **Related issues**

Fixes: #24732

## **Manual testing steps**

1. Build in MV3
2. Trigger a txn from dapp
3. Open network panel, and trigger edit gas button before
`suggestedGasFees` is finished fetching
4. You should see the network edition panel

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**
 by @seaona 


https://github.com/MetaMask/metamask-extension/assets/12678455/f3e660a8-2d7b-4d1d-be31-69d5cd66d374


<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**


https://github.com/MetaMask/metamask-extension/assets/12678455/42b2df9d-a861-41ec-914e-89229493e276


- [ ] 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
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] 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.
danjm pushed a commit that referenced this issue May 31, 2024
…24810)

<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**
There is a racing condition captured in MV3 build. When sending
transaction from dapp directly, if user chooses to edit the gas fee, and
when `suggestedGasFees ` request is processing slow, `gasFeeEstimates`
could be undefined within hook `useCustomTimeEstimate.js`. The fix is to
add a condition for accessing this object before forwarding to UI.


<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/24810?quickstart=1)

## **Related issues**

Fixes: #24732

## **Manual testing steps**

1. Build in MV3
2. Trigger a txn from dapp
3. Open network panel, and trigger edit gas button before
`suggestedGasFees` is finished fetching
4. You should see the network edition panel

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**
 by @seaona 


https://github.com/MetaMask/metamask-extension/assets/12678455/f3e660a8-2d7b-4d1d-be31-69d5cd66d374


<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**


https://github.com/MetaMask/metamask-extension/assets/12678455/42b2df9d-a861-41ec-914e-89229493e276


- [ ] 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
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] 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.
danjm pushed a commit that referenced this issue May 31, 2024
…24810)

<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**
There is a racing condition captured in MV3 build. When sending
transaction from dapp directly, if user chooses to edit the gas fee, and
when `suggestedGasFees ` request is processing slow, `gasFeeEstimates`
could be undefined within hook `useCustomTimeEstimate.js`. The fix is to
add a condition for accessing this object before forwarding to UI.


<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/24810?quickstart=1)

## **Related issues**

Fixes: #24732

## **Manual testing steps**

1. Build in MV3
2. Trigger a txn from dapp
3. Open network panel, and trigger edit gas button before
`suggestedGasFees` is finished fetching
4. You should see the network edition panel

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**
 by @seaona 


https://github.com/MetaMask/metamask-extension/assets/12678455/f3e660a8-2d7b-4d1d-be31-69d5cd66d374


<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**


https://github.com/MetaMask/metamask-extension/assets/12678455/42b2df9d-a861-41ec-914e-89229493e276


- [ ] 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
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] 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.
danjm pushed a commit that referenced this issue Jun 4, 2024
…24810)

<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**
There is a racing condition captured in MV3 build. When sending
transaction from dapp directly, if user chooses to edit the gas fee, and
when `suggestedGasFees ` request is processing slow, `gasFeeEstimates`
could be undefined within hook `useCustomTimeEstimate.js`. The fix is to
add a condition for accessing this object before forwarding to UI.


<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/24810?quickstart=1)

## **Related issues**

Fixes: #24732

## **Manual testing steps**

1. Build in MV3
2. Trigger a txn from dapp
3. Open network panel, and trigger edit gas button before
`suggestedGasFees` is finished fetching
4. You should see the network edition panel

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**
 by @seaona 


https://github.com/MetaMask/metamask-extension/assets/12678455/f3e660a8-2d7b-4d1d-be31-69d5cd66d374


<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**


https://github.com/MetaMask/metamask-extension/assets/12678455/42b2df9d-a861-41ec-914e-89229493e276


- [ ] 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
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] 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.
@metamaskbot metamaskbot added the release-11.16.6 Issue or pull request that will be included in release 11.16.6 label Jun 4, 2024
@metamaskbot
Copy link
Collaborator

Missing release label release-11.16.6 on issue. Adding release label release-11.16.6 on issue, as issue is linked to PR #24810 which has this release label.

@gauthierpetetin gauthierpetetin added the release-12.0.0 Issue or pull request that will be included in release 12.0.0 label Jun 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-11.16.6 Issue or pull request that will be included in release 11.16.6 release-12.0.0 Issue or pull request that will be included in release 12.0.0 Sev2-normal Normal severity; minor loss of service or inconvenience. team-extension-platform type-bug
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants