Skip to content

fix: Persist user rejection optional data in rejected error #5355

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

Merged
merged 3 commits into from
Feb 19, 2025

Conversation

OGPoyraz
Copy link
Member

Explanation

This PR aims to persist optional data in rejected error from TransactionController

References

Changelog

@metamask/transaction-controller

  • Added: Adds persist of user rejected optional data on approval rejection

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 highlighted breaking changes using the "BREAKING" category above as appropriate
  • I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes

@OGPoyraz OGPoyraz marked this pull request as ready for review February 18, 2025 10:58
@OGPoyraz OGPoyraz requested a review from a team as a code owner February 18, 2025 10:58
@OGPoyraz
Copy link
Member Author

@metamaskbot publish-preview

Copy link
Contributor

Preview builds have been published. See these instructions for more information about preview builds.

Expand for full list of packages and versions.
{
  "@metamask-previews/accounts-controller": "24.0.0-preview-7a21cbc",
  "@metamask-previews/address-book-controller": "6.0.3-preview-7a21cbc",
  "@metamask-previews/announcement-controller": "7.0.3-preview-7a21cbc",
  "@metamask-previews/approval-controller": "7.1.3-preview-7a21cbc",
  "@metamask-previews/assets-controllers": "50.0.0-preview-7a21cbc",
  "@metamask-previews/base-controller": "8.0.0-preview-7a21cbc",
  "@metamask-previews/bridge-controller": "0.0.0-preview-7a21cbc",
  "@metamask-previews/build-utils": "3.0.3-preview-7a21cbc",
  "@metamask-previews/composable-controller": "11.0.0-preview-7a21cbc",
  "@metamask-previews/controller-utils": "11.5.0-preview-7a21cbc",
  "@metamask-previews/earn-controller": "0.4.0-preview-7a21cbc",
  "@metamask-previews/ens-controller": "15.0.2-preview-7a21cbc",
  "@metamask-previews/eth-json-rpc-provider": "4.1.8-preview-7a21cbc",
  "@metamask-previews/gas-fee-controller": "22.0.3-preview-7a21cbc",
  "@metamask-previews/json-rpc-engine": "10.0.3-preview-7a21cbc",
  "@metamask-previews/json-rpc-middleware-stream": "8.0.7-preview-7a21cbc",
  "@metamask-previews/keyring-controller": "19.1.0-preview-7a21cbc",
  "@metamask-previews/logging-controller": "6.0.4-preview-7a21cbc",
  "@metamask-previews/message-manager": "12.0.1-preview-7a21cbc",
  "@metamask-previews/multichain": "2.1.1-preview-7a21cbc",
  "@metamask-previews/multichain-network-controller": "0.1.1-preview-7a21cbc",
  "@metamask-previews/multichain-transactions-controller": "0.4.0-preview-7a21cbc",
  "@metamask-previews/name-controller": "8.0.3-preview-7a21cbc",
  "@metamask-previews/network-controller": "22.2.1-preview-7a21cbc",
  "@metamask-previews/notification-services-controller": "0.21.0-preview-7a21cbc",
  "@metamask-previews/permission-controller": "11.0.6-preview-7a21cbc",
  "@metamask-previews/permission-log-controller": "3.0.3-preview-7a21cbc",
  "@metamask-previews/phishing-controller": "12.3.2-preview-7a21cbc",
  "@metamask-previews/polling-controller": "12.0.3-preview-7a21cbc",
  "@metamask-previews/preferences-controller": "15.0.2-preview-7a21cbc",
  "@metamask-previews/profile-sync-controller": "8.0.0-preview-7a21cbc",
  "@metamask-previews/queued-request-controller": "9.0.1-preview-7a21cbc",
  "@metamask-previews/rate-limit-controller": "6.0.3-preview-7a21cbc",
  "@metamask-previews/remote-feature-flag-controller": "1.5.0-preview-7a21cbc",
  "@metamask-previews/selected-network-controller": "21.0.1-preview-7a21cbc",
  "@metamask-previews/signature-controller": "23.2.1-preview-7a21cbc",
  "@metamask-previews/token-search-discovery-controller": "2.1.0-preview-7a21cbc",
  "@metamask-previews/transaction-controller": "46.0.0-preview-7a21cbc",
  "@metamask-previews/user-operation-controller": "25.0.0-preview-7a21cbc"
}

@OGPoyraz OGPoyraz merged commit 342332e into main Feb 19, 2025
133 checks passed
@OGPoyraz OGPoyraz deleted the fix/persist-user-rejection-data branch February 19, 2025 09:37
github-merge-queue bot pushed a commit to MetaMask/metamask-extension that referenced this pull request Feb 19, 2025
…ectAllApprovals" (#30388)

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

This PR aims to fix `release-blocker` in `v12.13.0` which disables
origin throttling middleware if `cause` is `rejectAllApprovals`.

Patch note: 
Currently bumping `transaction-controller` is blocked because it's
breaking e2e tests
https://consensys.slack.com/archives/CTQAGKY5V/p1739968847746929?thread_ts=1739869734.799669&cid=CTQAGKY5V
To avoid applying version newer version of `transaction-controller`
needed change patched in this PR. Required change is already in the core
repository MetaMask/core#5355. And will most
likely be released in version `46.0.1`. Then we will most likely bump
`transaction-controller` in `main`.

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

## **Related issues**

Fixes: #30333

## **Manual testing steps**

See the task details

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

- [X] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [X] I've completed the PR template to the best of my ability
- [X] 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/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.

---------

Co-authored-by: MetaMask Bot <[email protected]>
runway-github bot added a commit to MetaMask/metamask-extension that referenced this pull request Feb 19, 2025
… "rejectAllApprovals" (#30388)

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

This PR aims to fix `release-blocker` in `v12.13.0` which disables
origin throttling middleware if `cause` is `rejectAllApprovals`.

Patch note: 
Currently bumping `transaction-controller` is blocked because it's
breaking e2e tests
https://consensys.slack.com/archives/CTQAGKY5V/p1739968847746929?thread_ts=1739869734.799669&cid=CTQAGKY5V
To avoid applying version newer version of `transaction-controller`
needed change patched in this PR. Required change is already in the core
repository MetaMask/core#5355. And will most
likely be released in version `46.0.1`. Then we will most likely bump
`transaction-controller` in `main`.

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

## **Related issues**

Fixes: #30333

## **Manual testing steps**

See the task details

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

- [X] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [X] I've completed the PR template to the best of my ability
- [X] 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/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.

---------

Co-authored-by: MetaMask Bot <[email protected]>
runway-github bot added a commit to MetaMask/metamask-extension that referenced this pull request Feb 19, 2025
… "rejectAllApprovals" (#30388)

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

This PR aims to fix `release-blocker` in `v12.13.0` which disables
origin throttling middleware if `cause` is `rejectAllApprovals`.

Patch note:
Currently bumping `transaction-controller` is blocked because it's
breaking e2e tests
https://consensys.slack.com/archives/CTQAGKY5V/p1739968847746929?thread_ts=1739869734.799669&cid=CTQAGKY5V
To avoid applying version newer version of `transaction-controller`
needed change patched in this PR. Required change is already in the core
repository MetaMask/core#5355. And will most
likely be released in version `46.0.1`. Then we will most likely bump
`transaction-controller` in `main`.

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

## **Related issues**

Fixes: #30333

## **Manual testing steps**

See the task details

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

- [X] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [X] I've completed the PR template to the best of my ability
- [X] 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/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.

---------

Co-authored-by: MetaMask Bot <[email protected]>
runway-github bot pushed a commit to MetaMask/metamask-extension that referenced this pull request Feb 19, 2025
…middleware if cause is "rejectAllApprovals" (#30388)

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

This PR aims to fix `release-blocker` in `v12.13.0` which disables
origin throttling middleware if `cause` is `rejectAllApprovals`.

Patch note: 
Currently bumping `transaction-controller` is blocked because it's
breaking e2e tests
https://consensys.slack.com/archives/CTQAGKY5V/p1739968847746929?thread_ts=1739869734.799669&cid=CTQAGKY5V
To avoid applying version newer version of `transaction-controller`
needed change patched in this PR. Required change is already in the core
repository MetaMask/core#5355. And will most
likely be released in version `46.0.1`. Then we will most likely bump
`transaction-controller` in `main`.

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

## **Related issues**

Fixes: #30333

## **Manual testing steps**

See the task details

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

- [X] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [X] I've completed the PR template to the best of my ability
- [X] 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/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.

---------

Co-authored-by: MetaMask Bot <[email protected]>
javiergarciavera pushed a commit to MetaMask/metamask-extension that referenced this pull request Feb 20, 2025
…ectAllApprovals" (#30388)

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

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

This PR aims to fix `release-blocker` in `v12.13.0` which disables
origin throttling middleware if `cause` is `rejectAllApprovals`.

Patch note:
Currently bumping `transaction-controller` is blocked because it's
breaking e2e tests
https://consensys.slack.com/archives/CTQAGKY5V/p1739968847746929?thread_ts=1739869734.799669&cid=CTQAGKY5V
To avoid applying version newer version of `transaction-controller`
needed change patched in this PR. Required change is already in the core
repository MetaMask/core#5355. And will most
likely be released in version `46.0.1`. Then we will most likely bump
`transaction-controller` in `main`.

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

Fixes: #30333

See the task details

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

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

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

- [X] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [X] I've completed the PR template to the best of my ability
- [X] 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/main/.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: MetaMask Bot <[email protected]>
danjm added a commit to MetaMask/metamask-extension that referenced this pull request Feb 26, 2025
…middleware if cause is "rejectAllApprovals" (#30448)

- fix: cp-12.13.0 Disable origin throttling middleware if cause is
"rejectAllApprovals" (#30388)

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

This PR aims to fix `release-blocker` in `v12.13.0` which disables
origin throttling middleware if `cause` is `rejectAllApprovals`.

Patch note: 
Currently bumping `transaction-controller` is blocked because it's
breaking e2e tests

https://consensys.slack.com/archives/CTQAGKY5V/p1739968847746929?thread_ts=1739869734.799669&cid=CTQAGKY5V
To avoid applying version newer version of `transaction-controller`
needed change patched in this PR. Required change is already in the core
repository MetaMask/core#5355. And will most
likely be released in version `46.0.1`. Then we will most likely bump
`transaction-controller` in `main`.

[![Open in GitHub

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

## **Related issues**

Fixes: #30333

## **Manual testing steps**

See the task details

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

- [X] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding

Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [X] I've completed the PR template to the best of my ability
- [X] 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/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.

---------

Co-authored-by: MetaMask Bot <[email protected]>
[42bd11d](42bd11d)

Co-authored-by: OGPoyraz <[email protected]>
Co-authored-by: MetaMask Bot <[email protected]>
Co-authored-by: Dan J Miller <[email protected]>
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