Skip to content

fix: isInternalAccountInPermittedAccountIds case where only wallet:eip155 permission exists for a given address#5980

Merged
adonesky1 merged 10 commits intomainfrom
ad/fix-isAddressWithParsedScopesInPermittedAccountIds
Jun 13, 2025
Merged

fix: isInternalAccountInPermittedAccountIds case where only wallet:eip155 permission exists for a given address#5980
adonesky1 merged 10 commits intomainfrom
ad/fix-isAddressWithParsedScopesInPermittedAccountIds

Conversation

@adonesky1
Copy link
Copy Markdown
Contributor

@adonesky1 adonesky1 commented Jun 13, 2025

Explanation

We aren't handling the case where a permitted account with the wallet:eip155 scope prefix is passed into isInternalAccountInPermittedAccountIds - where it is the only representation of its address in the permission set. In this case we wouldn't find a permitted account matching a passed in internal account and would ultimately throw an error resulting in the app crashing.

References

Changelog

@metamask/chain-agnostic-permission

  • Fix isInternalAccountInPermittedAccountIds and isCaipAccountIdInPermittedAccountIds to correctly handle comparison against permittedAccounts values of the wallet:<namespace>:<address> format

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • I've communicated my changes to consumers by updating changelogs for packages I've changed, highlighting breaking changes as necessary
  • I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes

@adonesky1 adonesky1 requested a review from a team as a code owner June 13, 2025 17:52
@adonesky1 adonesky1 changed the title Ad/fix is address with parsed scopes in permitted account ids fix: isInternalAccountInPermittedAccountIds case where only wallet:eip155 permission exists for a given address Jun 13, 2025
@jiexi jiexi requested a review from a team as a code owner June 13, 2025 18:27
@adonesky1 adonesky1 enabled auto-merge (squash) June 13, 2025 20:00
jiexi
jiexi previously approved these changes Jun 13, 2025
jiexi
jiexi previously approved these changes Jun 13, 2025
@adonesky1 adonesky1 merged commit 839c3e0 into main Jun 13, 2025
214 checks passed
@adonesky1 adonesky1 deleted the ad/fix-isAddressWithParsedScopesInPermittedAccountIds branch June 13, 2025 20:21
@adonesky1 adonesky1 mentioned this pull request Jun 13, 2025
adonesky1 added a commit that referenced this pull request Jun 13, 2025
## @metamask/chain-agnostic-permission

## [0.7.1]

### Changed

- Bump `@metamask/keyring-internal-api` to `^6.2.0`
([#5871](#5871))
- Bump `@metamask/controller-utils` to `^11.10.0`
([#5935](#5935))
- Bump `@metamask/network-controller` to `^23.6.0`
([#5935](https://github.com/MetaMask/core/pull/5935),[#5882](https://github.com/MetaMask/core/pull/5882))
- Change `caip25CaveatBuilder` to list unsupported scopes in the
unsupported scopes error
([#5806](#5806))
### Fixed
- Fix `isInternalAccountInPermittedAccountIds` and
`isCaipAccountIdInPermittedAccountIds` to correctly handle comparison
against `permittedAccounts` values of the `wallet:<namespace>:<address>`
format ([#5980](#5980))
github-merge-queue bot pushed a commit to MetaMask/metamask-mobile that referenced this pull request Jun 17, 2025
…o helper function that was causing error thrown in certain cases when browser tabs were opened (#16383)

[Core PR with fix to `isInternalAccountInPermittedAccountIds` &
`isCaipAccountIdInPermittedAccountIds`
](MetaMask/core#5980) which was causing this
bug.

Bumps `@metamask/chain-agnostic-permission` to latest: `0.7.1`

## **Related issues**

[See slack thread where bug was initially
reported](https://consensys.slack.com/archives/C08UFPWB3GB/p1749821334454809)

## **Screenshots/Recordings**

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

### **Before**


https://github.com/user-attachments/assets/921b48a5-87d0-4900-ac60-f69219881d15

### **After**


https://github.com/user-attachments/assets/b3f06069-66aa-485d-b90d-997ed4544466

## **Pre-merge author checklist**

- [ ] I’ve followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile
Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.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-mobile/blob/main/.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-merge-queue bot pushed a commit to MetaMask/metamask-mobile that referenced this pull request Jun 17, 2025
…with fix to helper function that was causing error thrown in certain cases when browser tabs were opened (#16383)

[Core PR with fix to `isInternalAccountInPermittedAccountIds` &
`isCaipAccountIdInPermittedAccountIds`
](MetaMask/core#5980) which was causing this
bug.

Bumps `@metamask/chain-agnostic-permission` to latest: `0.7.1`

## **Related issues**

[See slack thread where bug was initially
reported](https://consensys.slack.com/archives/C08UFPWB3GB/p1749821334454809)

## **Screenshots/Recordings**

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

### **Before**


https://github.com/user-attachments/assets/921b48a5-87d0-4900-ac60-f69219881d15

### **After**


https://github.com/user-attachments/assets/b3f06069-66aa-485d-b90d-997ed4544466

## **Pre-merge author checklist**

- [ ] I’ve followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile
Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.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-mobile/blob/main/.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.
runway-github bot added a commit to MetaMask/metamask-mobile that referenced this pull request Jun 17, 2025
…ion` with fix to helper function that was causing error thrown in certain cases when browser tabs were opened (#16383)

[Core PR with fix to `isInternalAccountInPermittedAccountIds` &
`isCaipAccountIdInPermittedAccountIds`
](MetaMask/core#5980) which was causing this
bug.

Bumps `@metamask/chain-agnostic-permission` to latest: `0.7.1`

## **Related issues**

[See slack thread where bug was initially
reported](https://consensys.slack.com/archives/C08UFPWB3GB/p1749821334454809)

## **Screenshots/Recordings**

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

### **Before**


https://github.com/user-attachments/assets/921b48a5-87d0-4900-ac60-f69219881d15

### **After**


https://github.com/user-attachments/assets/b3f06069-66aa-485d-b90d-997ed4544466

## **Pre-merge author checklist**

- [ ] I’ve followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile
Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.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-mobile/blob/main/.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.
runway-github bot added a commit to MetaMask/metamask-mobile that referenced this pull request Jun 17, 2025
…ion` with fix to helper function that was causing error thrown in certain cases when browser tabs were opened (#16383)

[Core PR with fix to `isInternalAccountInPermittedAccountIds` &
`isCaipAccountIdInPermittedAccountIds`
](MetaMask/core#5980) which was causing this
bug.

Bumps `@metamask/chain-agnostic-permission` to latest: `0.7.1`

## **Related issues**

[See slack thread where bug was initially
reported](https://consensys.slack.com/archives/C08UFPWB3GB/p1749821334454809)

## **Screenshots/Recordings**

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

### **Before**


https://github.com/user-attachments/assets/921b48a5-87d0-4900-ac60-f69219881d15

### **After**


https://github.com/user-attachments/assets/b3f06069-66aa-485d-b90d-997ed4544466

## **Pre-merge author checklist**

- [ ] I’ve followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile
Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.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-mobile/blob/main/.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.
runway-github bot added a commit to MetaMask/metamask-mobile that referenced this pull request Jun 17, 2025
…ion` with fix to helper function that was causing error thrown in certain cases when browser tabs were opened (#16383)

[Core PR with fix to `isInternalAccountInPermittedAccountIds` &
`isCaipAccountIdInPermittedAccountIds`
](MetaMask/core#5980) which was causing this
bug.

Bumps `@metamask/chain-agnostic-permission` to latest: `0.7.1`

## **Related issues**

[See slack thread where bug was initially
reported](https://consensys.slack.com/archives/C08UFPWB3GB/p1749821334454809)

## **Screenshots/Recordings**

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

### **Before**


https://github.com/user-attachments/assets/921b48a5-87d0-4900-ac60-f69219881d15

### **After**


https://github.com/user-attachments/assets/b3f06069-66aa-485d-b90d-997ed4544466

## **Pre-merge author checklist**

- [ ] I’ve followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile
Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.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-mobile/blob/main/.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.
runway-github bot added a commit to MetaMask/metamask-mobile that referenced this pull request Jun 17, 2025
…ion` with fix to helper function that was causing error thrown in certain cases when browser tabs were opened (#16383)

[Core PR with fix to `isInternalAccountInPermittedAccountIds` &
`isCaipAccountIdInPermittedAccountIds`
](MetaMask/core#5980) which was causing this
bug.

Bumps `@metamask/chain-agnostic-permission` to latest: `0.7.1`

## **Related issues**

[See slack thread where bug was initially
reported](https://consensys.slack.com/archives/C08UFPWB3GB/p1749821334454809)

## **Screenshots/Recordings**

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

### **Before**


https://github.com/user-attachments/assets/921b48a5-87d0-4900-ac60-f69219881d15

### **After**


https://github.com/user-attachments/assets/b3f06069-66aa-485d-b90d-997ed4544466

## **Pre-merge author checklist**

- [ ] I’ve followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile
Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.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-mobile/blob/main/.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.
adonesky1 added a commit to MetaMask/metamask-mobile that referenced this pull request Jun 17, 2025
…th fix to helper function that was causing error thrown in certain cases when browser tabs were opened (#16383)

[Core PR with fix to `isInternalAccountInPermittedAccountIds` &
`isCaipAccountIdInPermittedAccountIds`
](MetaMask/core#5980) which was causing this
bug.

Bumps `@metamask/chain-agnostic-permission` to latest: `0.7.1`
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.

2 participants