Version v11.3.0 #21173
Merged
Version v11.3.0 #21173
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* refactor: remove keyringMemStore from DetectTokensController * fix: use getState to get initial lock state * test: add case for lock event
Add the Name component to display a saved petname or a proposed name, for raw values such as Ethereum account addresses. Add the nested NameDetails component when the Name component is clicked, which includes a modal containing a text field and dropdown of all available proposed names. Add the FormComboField component to provide a free entry text field with a dropdown of suggested options including both primary and secondary text.
* chore: add mmi e2e test trigger workflow * Implement action with curl * update secret use --------- Co-authored-by: Albert Olivé <[email protected]>
* refactor: remove eth-keyring-controller from DecryptMessageController * refactor: apply @Gudahtt suggestions * test: add cases for invalid encryption data
* wip * toggle settings e2e * fix test * removed unused code * fix failing snapshot * lint fix * fix git conflict * test: ppom ci (#20896) * ppom ci * Move ppom test inside snaps folder * fix failing test * fix failing snapshot test * fix snapshot test * improve variable naming * improve testid * update snapshot --------- Co-authored-by: seaona <[email protected]>
Fixes 2 issues when converting an ERC-20 token to an NFT: - The contract address was not pre-populated in the modal - The NFT was not removed from the tokens tab
* feat(templates): new templates for issues and PRs We've discussed with stakeholders from Extension and Mobile teams how we could align templates for issues and PRs on the different repos, starting with metamask-extension and metamask-mobile repos. This is the proposal we landed on after our first workshops. * fix(templates): links were not working * fix(templates): tiny adjustments * fix(templates): add expected behavior section to bug report template This change was suggested by Nicolas Massart. * fix(templates): add a section to indicate what issue is fixed by the PR This change was suggested by Nicolas Massart. * fix(guidelines): add a coding guideline regarding the use of useEffect * fix(guidelines): rename guidelines folder * fix(contributing.md): update link to good first issues * fix(contributing.md): recommend contributors to read coding guidelines * fix(guidelines): coding guidelines for component file structure are different in extension * fix(guidelines): TypeScript shall be written in camelcase * fix(guidelines): simplify component naming guideline Comment from George Marshall: Because we use kebab-case for file names but retain PascalCase for component and enum names it may make sense to reword this sentence * fix(guidelines): tiny changes to adapt mobile guidelines to extension context * feat(guidelines): add links to contributor docs * fix(issue template): make instruction clearer for external contributors
In the flask build type only, use the Name component to display addresses in typed V3 and V4 signature requests.
* adds stories file and unit test * adds the logic to open custodian url or not * lint * fixes styles, and clean up * test update * remove unused locale * small update * lint * ran verify locales fix * fix typo * updates code with configuration api v2 * clean up * lint and prettier * test fix * prettier * packages update * updates for config v2 * updates for config v2 * clean up * Update LavaMoat policies --------- Co-authored-by: Albert Olivé <[email protected]> Co-authored-by: MetaMask Bot <[email protected]> Co-authored-by: Danica Shen <[email protected]>
… private key" (#20920) Catch the rejection if you submit an incorrect password to "Show private key" and also fixes warnings that happen if you enter the wrong password. Co-authored-by: montelaidev <[email protected]>
* Fix copy and add 3rd party API links * Extract uppercase logic into a variable * Fix snapshot tests * fix e2e test * fix lint error
* Bump Browserify * Update LavaMoat policies * Update LavaMoat policies --------- Co-authored-by: MetaMask Bot <[email protected]> Co-authored-by: jiexi <[email protected]> Co-authored-by: legobeat <[email protected]>
Moves the prompt for importing detected tokens from the bottom to the top of the assets view. So that users can see it without scrolling to the bottom.
* Initial work * updated tests * Update test * Finished adding tests * prettier * Fixed tests * Changed custodian name to envName and fixed tests * Updated tests * Updated tests
* updates custody-keyring package * dedupe
* updated placeholder for erc-1155 image * removed code duplication * updated collections * updated snapshot * updated stories * updated condition from includes to startsWith * Use startsWith instead of includes --------- Co-authored-by: Danica Shen <[email protected]> Co-authored-by: David Walsh <[email protected]> Co-authored-by: chloeYue <[email protected]>
* integrate snaps release 8.0 * update yarn.lock * rebuilt policy * fix url * ran policy generation again * Revert "rebuilt policy" This reverts commit a7e53dd. * Add missing events --------- Co-authored-by: Matthew Walsh <[email protected]>
|
@SocketSecurity ignore-all All of these packages are controlled by us |
… resolution in unlock-wallet.spec.js (#21422) Fixes the flakiness of the `should send first three Page metric events upon fullscreen page load` test when run as part of the ` test-e2e-chrome-mmi ` job. While this seems to fix the issue, I don't have a 100% definite explanation. My hypothesis in making the change in this PR is that occasionaly on mmi builds the e2e tests would take longer to get past the unlock stage than expected, and so it is possible that not all metrics requests that the `unlock-wallet.spec.js` test is waiiting for have be made by the time the test starts looking for them. Anecdotal evidence seems to suggest that this has worked, and that this test is passing.
## **Description** Since many teams are now building features in and shipping to Flask the snaps team does not need to be a specific CODEOWNER of the `**/flask/**` paths. By removing this line in the CODEOWNERS file, these files will be owned by `extension-devs` once again.
## **Description** - bump `@babel/*` packages to latest to get latest security fixes - bump `storybook` in order to dedupe `@babel/core` - port patch for `@babel/core` and `@babel/runtime` ## **Related issues** - Based on/includes #21393 ## **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 clearly explained: - [ ] What problem this PR is solving. - [ ] How this problem was solved. - [ ] How reviewers can test my changes. - [ ] I’ve indicated what issue this PR is linked to: Fixes #??? - [x] I’ve included tests if applicable. - [x] I’ve documented any added code. - [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)). - [ ] I’ve properly set the pull request status: - [ ] In case it's not yet "ready for review", I've set it to "draft". - [ ] In case it's "ready for review", I've changed it from "draft" to "non-draft". ## **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: Danica Shen <[email protected]> Co-authored-by: Dan J Miller <[email protected]>
## **Description** Error is prompted in the console when attempting to cancel a transaction and transaction is not cancelled. The error comes from a call to selector where we are passing in `state.metamask` object instead of state object. To fix, we pass in an object where `metamask` is a property and value is `state.metamask` ## **Expected behavior** No errors are shown and transaction is cancelled. ## **Manual testing steps** 1. Open MetaMask and enter your password 2. Switch to a test network: e.g. Goerli 3. Proceed to the send flow 4. Send 0.1 ETH to an address: e.g. 0x3D0b9b4056D86f8d9889B2C8e26FFCedC62D3036 5. Edit the gas settings in Market --> Advanced: Max base fee to 0.0000001 and Priority Fee to 0.0000001 6. Confirm the transaction 7. Cancel transaction ## **Screenshots/Recordings** ### **Before** https://github.com/MetaMask/metamask-extension/assets/120041701/d371c902-b0bb-4e52-b152-549f1a487375 ### **After** https://github.com/MetaMask/metamask-extension/assets/44811/40dd2071-a9ca-4d31-9241-dfc76aaacd9f ## **Related issues** _Fixes #21261 ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [x] I've clearly explained: - [x] What problem this PR is solving. - [x] How this problem was solved. - [x] How reviewers can test my changes. - [x] I’ve indicated what issue this PR is linked to: Fixes #??? - [ ] I’ve included tests if applicable. - [x] I’ve documented any added code. - [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)). - [x] I’ve properly set the pull request status: - [x] In case it's not yet "ready for review", I've set it to "draft". - [x] In case it's "ready for review", I've changed it from "draft" to "non-draft". ## **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. Signed-off-by: Akintayo A. Olusegun <[email protected]>
## **Description** This PR addresses adding custom network validation via the JSON RPC request flow. It accomplishes that by using the existing `useSafeChainsListValidation` boolean in the appropriate place in the `add-ethereum-chain.js` file. One e2e test is added to cover this scenario. ## **Manual testing steps** See #21208 ## **Related issues** Fixes: #21208 ## **Pre-merge author checklist** - [ ] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've clearly explained: - [ ] What problem this PR is solving. - [ ] How this problem was solved. - [ ] How reviewers can test my changes. - [ ] I’ve indicated what issue this PR is linked to: Fixes #??? - [ ] I’ve included tests if applicable. - [ ] I’ve documented any added code. - [ ] 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)). - [ ] I’ve properly set the pull request status: - [ ] In case it's not yet "ready for review", I've set it to "draft". - [ ] In case it's "ready for review", I've changed it from "draft" to "non-draft". ## **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.
#21418) … so the app doesn't crash on certain ipfs urls Fixes #21417 ## **Description** See the code comment added in this PR for full details: ``` // With v11.1.0, we started seeing errors thrown that included this // line in the stack trace. The cause is that the `getIpfsCIDv1AndPath` // method within assets-controllers/src/assetsUtil.ts can throw // if part of the ipfsUrl, i.e. the `image` variable within this function, // contains characters not in the Base58 alphabet. Details on that are // here https://digitalbazaar.github.io/base58-spec/#alphabet. This happens // with some NFTs, when we attempt to parse part of their IPFS image address // with the `CID.parse` function (CID is part of the multiform package) // // Before v11.1.0 `getFormattedIpfsUrl` was not used in the extension codebase. // Its use within assets-controllers always ensures that errors are caught // and ignored. So while we were handling NFTs that can cause this error before, // we were always catching and ignoring the error. As of PR #20172, we started // passing all NFTs image URLs to `getAssetImageURL` from nft-items.js, which is // why we started seeing these errors cause crashes for users in v11.1.0 // // For the sake of a quick fix, we are wrapping this call in a try-catch, which // the assets-controllers already do in some form in all cases where this function // is called. This probably does not affect user experience, as we would not have // correctly rendered these NFTs before v11.1.0 either (due to the same error // disuccessed in this code comment). // // In the future, we can look into solving the root cause, which might require // no longer using multiform's CID.parse() method within the assets-controller ``` ## **Manual testing steps** Not sure of an easy way to test. One way would be: 1. Add an NFT to your wallet which has an ipfs image url with the character I in it 2. Go to the NFT tab. That NFT should not be visible, but the app should not crash ## **Related issues** Related to #18564, but does not fix it. ## **Pre-merge author checklist** - [ ] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've clearly explained: - [ ] What problem this PR is solving. - [ ] How this problem was solved. - [ ] How reviewers can test my changes. - [ ] I’ve indicated what issue this PR is linked to: Fixes #??? - [ ] I’ve included tests if applicable. - [ ] I’ve documented any added code. - [ ] 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)). - [ ] I’ve properly set the pull request status: - [ ] In case it's not yet "ready for review", I've set it to "draft". - [ ] In case it's "ready for review", I've changed it from "draft" to "non-draft". ## **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.
## **Description** This PR fixes an error spotted on Sentry when creating approvals for unapproved transactions. Sentry error: https://metamask.sentry.io/issues/4552465127/?project=273505&query=is%3Aunresolved+release%3A11.2.0&referrer=issue-stream&sort=freq&statsPeriod=14d&stream_index=5
|
Regarding the two most recent commits added: I have QA'd a51fd86. It fixes an issue in prod that is causing crashes. I was able to repro the issue on this branch before cherry-picking the commit and the problem was fixed after cherry-picking. I also validated that no other NFT behaviour was affected. The change only catches and then ignores an error, and only for NFTs that would not have been rendered otherwise, so this should not have any risk of regression 6b5704a has zero risk of regression, but may prevent some errors we are seeing in sentry |
Co-authored-by: Matthew Walsh <[email protected]> Co-authored-by: Frederik Bolding <[email protected]>
…changes (#21440) #21255 was a bump to a Snaps package. However, it included this in the description: > As part of bumping the packages we had to update TypeScript as well. We had a > long-standing patch to TypeScript that could be easily replaced with a small patch to > LavaMoat to enable overrideTaming. This feature is also set to be enabled in the next > version of LavaMoat. This specific change is needed to get the v11.3.0 build passing, as commit 913d0fb causes `yarn build dist` on Version-v11.3.0 to fail without it. ## **Manual testing steps** All CI jobs should pass
Builds ready [99615eb]
Page Load Metrics (984 ± 386 ms)
|
|
@chloeYue and @benjisclowder said in slack that the release looks good from their perspective |
…ting localStore (#21447) ## **Description** The purpose of e471f0a was to fix the background.js script so that the following code is run when then extension is first installed: ``` addAppInstalledEvent(); platform.openExtensionInBrowser(); ``` Prior to e471f0a, these function were being call in the handler of the `browser.runtime.onInstalled` listener. However, #21015 caused that listener to stop working. Because the listener needs to be created in the first javascript event loop, but that PR caused a delay to the background.js script being run, so the listener would be created after the first javascript event loop. The solution in e471f0a was to write a property to state during the first time background.js is run, and then use the absence of that property to decide we were in a scenario where the extension was being installed. When doing final QA on v11.3.0, I realized that this solution doesn't account for users who already have MetaMask installed. These users won't have the property in state, and so after MetaMask updates the logic of e471f0a would treat them as if they were first installing MetaMask, even though they are not. To prevent that we could write a data migration, but this PR opts for an even simpler approach. First, this PR reverts e471f0a. Next, it modifies the background.js script so that the `browser.runtime.onInstalled` listener is run every time background.js is run, but it only calls the `addAppInstalledEvent` and `platform.openExtensionInBrowser()` functions if there is no data existing in storage (which will only be the case if the user is just installing and running background.js for the first time). This PR is most easily reviewed by looking at each commit separately. The first is a pure revert, and so doesn't really require review, and then second commit is where the new code is. ## **Manual testing steps** 1. Download the build from the comment in this PR 2. Install the build. MetaMask should automatically open 3. Open the background console to the network tab 4. Go through onboarding, selecting that you want to participate in MetaMetrics 5. There should be a segment network request with an `'App Installed'` event Then: 1. Install v11.2.0 and subsequently install v11.3.0, causing a version update. 2. MetaMask should not automatically open. 3. There should be no 'App Installed' metrics event ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [x] I've clearly explained: - [ ] What problem this PR is solving. - [ ] How this problem was solved. - [ ] How reviewers can test my changes. - [ ] I’ve indicated what issue this PR is linked to: Fixes #??? - [ ] I’ve included tests if applicable. - [x] I’ve documented any added code. - [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)). - [x] I’ve properly set the pull request status: - [ ] In case it's not yet "ready for review", I've set it to "draft". - [ ] In case it's "ready for review", I've changed it from "draft" to "non-draft". ## **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.
Builds ready [f127ef0]
Page Load Metrics (1087 ± 390 ms)
|
|
I QA'd the latest commit that was added f127ef0 I tested that the app installed event, and automatic browser open, occur when installing v11.3.0 I also tested that if v11.2.0 is already installed and the updated to v11.3.0, then the browser open event does not occur |
DDDDDanica
approved these changes
Oct 19, 2023
chloeYue
approved these changes
Oct 19, 2023
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.