Skip to content

Version v12.5.1 RC#28091

Merged
danjm merged 5 commits intomasterfrom
Version-v12.5.1
Oct 31, 2024
Merged

Version v12.5.1 RC#28091
danjm merged 5 commits intomasterfrom
Version-v12.5.1

Conversation

@metamaskbot
Copy link
Copy Markdown
Collaborator

📦 🚀

@github-actions github-actions bot added the team-bots Bot team (for MetaMask Bot, Runway Bot, etc.) label Oct 24, 2024
@metamaskbot
Copy link
Copy Markdown
Collaborator Author

Builds ready [40febb6]
Page Load Metrics (1830 ± 93 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint16452540183918689
domContentLoaded16332380178415373
load16442573183019493
domInteractive14158443014

Cherry-picks #28136
(1fd9510). Original PR description:

## **Description**

Fixes an issue where token balance showed as 0 during send flow. This
occurred when clicking the token in the token list, then clicking the
send button from the token details page.

When going send first and then picking a token, picking `decimals` was a
number:



![image](https://github.com/user-attachments/assets/95d32a68-1076-4050-b129-187672694794)

But when going token first and then clicking send , `decimals` was a
string and skipped calculating the balance.



![image](https://github.com/user-attachments/assets/99a8fcd4-bd29-4969-b98d-6639d50553ff)


`calcTokenAmount` seems to work with either string or number, so
changing logic from
#27083 which
introduced the number check


[![Open in GitHub

Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/28136?quickstart=1)

## **Related issues**

Fixes: #28112

## **Manual testing steps**

1. Click on an erc20 token on the token list
2. Click the send button on the token details page
3. Choose a destination account
4. The balance under the asset picker should be accurate

## **Screenshots/Recordings**

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

### **Before**

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

### **After**

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

## **Pre-merge author checklist**

- [ ] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension 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.


<!--
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**

<!--
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/28151?quickstart=1)

## **Related issues**

Fixes:

## **Manual testing steps**

1. Go to this page...
2.
3.

## **Screenshots/Recordings**

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

### **Before**

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

### **After**

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

## **Pre-merge author checklist**

- [ ] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension 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.

Co-authored-by: Brian Bergeron <brian.e.bergeron@gmail.com>
@github-actions
Copy link
Copy Markdown
Contributor

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
Copy link
Copy Markdown
Collaborator Author

Builds ready [d7108ed]
Page Load Metrics (2011 ± 135 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint52726711912399192
domContentLoaded164526601989277133
load165326772011280135
domInteractive22114472110

danjm and others added 2 commits October 31, 2024 00:16
Cherry-picks #28145
to v12.5.1, and brings
#28177 into v12.5.1
as well. The latter originally targeted v12.6.0, but we need it for a
hotfix ahead of that. The PR description for that was:

## **Description**

Forcing resolutions to fix `yarn audit` warnings and more specifically
this issue:
- GHSA-584q-6j8j-r5pm

I decided to be very explicit about the resolution itself based on the
output of:
```console
$ yarn why secp256k1
├─ ethereum-cryptography@npm:0.1.3
│  └─ secp256k1@npm:4.0.4 (via npm:^4.0.1)
│
├─ ganache@npm:7.9.2
│  └─ secp256k1@npm:4.0.3 (via npm:4.0.3)
│
├─ ganache@patch:ganache@npm%3A7.9.2#~/.yarn/patches/ganache-npm-7.9.2-a70dc8da34.patch::version=7.9.2&hash=7d7c66
│  └─ secp256k1@npm:4.0.3 (via npm:4.0.3)
│
├─ gridplus-sdk@npm:2.5.1
│  └─ secp256k1@npm:4.0.2 (via npm:4.0.2)
│
└─ hdkey@npm:2.1.0
   └─ secp256k1@npm:4.0.4 (via npm:^4.0.0)
```

We could also have a more straightforward resolution like:
```json
  ...
  "resolutions": {
    ...
    "secp256k1": "4.0.4"
  }
  ...
```

But that could also catch version with different major.

Let me know what would be the preferred solution here.

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

## **Related issues**

Fixes: GHSA-584q-6j8j-r5pm

## **Manual testing steps**

N/A

## **Screenshots/Recordings**

### **Before**

### **After**

## **Pre-merge author checklist**

- [ ] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension 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.

---------

Co-authored-by: Charly Chevalier <charly.chevalier@consensys.net>
Co-authored-by: sahar-fehri <sahar.fehri@consensys.net>
This PR cherry-picks
2c86162#diff-63ab7391d870a62d8bcd3cc5d5371432068538deb98e5effcb899434ed8649bb

---------

Co-authored-by: MetaMask Bot <metamaskbot@users.noreply.github.com>
Co-authored-by: Matthew Walsh <matthew.walsh@consensys.net>
@socket-security
Copy link
Copy Markdown

socket-security bot commented Oct 31, 2024

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher
npm/@metamask/base-controller@7.0.2 None +1 1.06 MB metamaskbot

🚮 Removed packages: npm/@metamask/base-controller@7.0.1, npm/@metamask/controller-utils@11.3.0, npm/secp256k1@4.0.3

View full report↗︎

@metamaskbot
Copy link
Copy Markdown
Collaborator Author

Builds ready [5dbbda3]
Page Load Metrics (1862 ± 106 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint38226041788390187
domContentLoaded163625361836211101
load164726081862221106
domInteractive1897482512

@danjm
Copy link
Copy Markdown
Contributor

danjm commented Oct 31, 2024

I manually tested d7108ed again ✔️

@OGPoyraz
Copy link
Copy Markdown
Member

@danjm danjm marked this pull request as ready for review October 31, 2024 11:17
@danjm danjm requested review from a team as code owners October 31, 2024 11:17
@metamaskbot
Copy link
Copy Markdown
Collaborator Author

Builds ready [82a548c]
Page Load Metrics (1866 ± 85 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint24224101801406195
domContentLoaded15262122182614670
load15352302186617785
domInteractive16235584421

@danjm danjm merged commit 83bfd33 into master Oct 31, 2024
@danjm danjm deleted the Version-v12.5.1 branch October 31, 2024 12:26
@github-actions github-actions bot locked and limited conversation to collaborators Oct 31, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

team-bots Bot team (for MetaMask Bot, Runway Bot, etc.)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants