Skip to content

[Bug]: Confirmations - Spam filter is activated despite Rejecting it #30333

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

Closed
seaona opened this issue Feb 14, 2025 · 0 comments · Fixed by #30388
Closed

[Bug]: Confirmations - Spam filter is activated despite Rejecting it #30333

seaona opened this issue Feb 14, 2025 · 0 comments · Fixed by #30388
Assignees
Labels
regression-RC-12.13.0 Regression bug that was found in release candidate (RC) for release 12.13.0 release-12.14.0 Issue or pull request that will be included in release 12.14.0 release-blocker This bug is blocking the next release Sev2-normal Normal severity; minor loss of service or inconvenience. team-confirmations Push issues to confirmations team type-bug Something isn't working

Comments

@seaona
Copy link
Contributor

seaona commented Feb 14, 2025

Describe the bug

Spam filter is activated despite Rejecting the modal when it appears

Expected behavior

No response

Screenshots/Recordings

spam-filter-active-without-consent.mp4

Steps to reproduce

  1. Go to the test dapp
  2. Trigger x20 batch tx
  3. Reject a couple
  4. See spam filter modal
  5. Reject it
  6. See how your next tx's are blocked

Error messages or log output

Detection stage

During release testing

Version

12.13.0

Build type

None

Browser

Chrome

Operating system

Linux

Hardware wallet

No response

Additional context

No response

Severity

No response

@seaona seaona added Sev2-normal Normal severity; minor loss of service or inconvenience. team-confirmations Push issues to confirmations team type-bug Something isn't working labels Feb 14, 2025
@github-project-automation github-project-automation bot moved this to To be fixed in Bugs by severity Feb 14, 2025
@metamaskbot metamaskbot added the regression-RC-12.13.0 Regression bug that was found in release candidate (RC) for release 12.13.0 label Feb 14, 2025
@github-project-automation github-project-automation bot moved this to To be fixed in Bugs by team Feb 14, 2025
@bschorchit bschorchit added the release-blocker This bug is blocking the next release label Feb 17, 2025
@OGPoyraz OGPoyraz self-assigned this Feb 17, 2025
OGPoyraz added a commit to MetaMask/core that referenced this issue Feb 19, 2025
## Explanation

<!--
Thanks for your contribution! Take a moment to answer these questions so
that reviewers have the information they need to properly understand
your changes:

* What is the current state of things and why does it need to change?
* What is the solution your changes offer and how does it work?
* Are there any changes whose purpose might not obvious to those
unfamiliar with the domain?
* If your primary goal was to update one package but you found you had
to update another one along the way, why did you do so?
* If you had to upgrade a dependency, why did you do so?
-->

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

## References

<!--
Are there any issues that this pull request is tied to?
Are there other links that reviewers should consult to understand these
changes better?
Are there client or consumer pull requests to adopt any breaking
changes?

For example:

* Fixes #12345
* Related to #67890
-->

* Related to MetaMask/metamask-extension#30333

## Changelog

<!--
If you're making any consumer-facing changes, list those changes here as
if you were updating a changelog, using the template below as a guide.

(CATEGORY is one of BREAKING, ADDED, CHANGED, DEPRECATED, REMOVED, or
FIXED. For security-related issues, follow the Security Advisory
process.)

Please take care to name the exact pieces of the API you've added or
changed (e.g. types, interfaces, functions, or methods).

If there are any breaking changes, make sure to offer a solution for
consumers to follow once they upgrade to the changes.

Finally, if you're only making changes to development scripts or tests,
you may replace the template below with "None".
-->

### `@metamask/transaction-controller`

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

## Checklist

- [X] I've updated the test suite for new or updated code as appropriate
- [X] I've updated documentation (JSDoc, Markdown, etc.) for new or
updated code as appropriate
- [X] I've highlighted breaking changes using the "BREAKING" category
above as appropriate
- [X] I've prepared draft pull requests for clients and consumer
packages to resolve any breaking changes
github-merge-queue bot pushed a commit that referenced this issue 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]>
@github-project-automation github-project-automation bot moved this from To be fixed to Fixed in Bugs by team Feb 19, 2025
@github-project-automation github-project-automation bot moved this from To be fixed to Fixed in Bugs by severity Feb 19, 2025
@metamaskbot metamaskbot added the release-12.14.0 Issue or pull request that will be included in release 12.14.0 label Feb 19, 2025
runway-github bot added a commit that referenced this issue 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 that referenced this issue 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 that referenced this issue 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 that referenced this issue 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 that referenced this issue 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
regression-RC-12.13.0 Regression bug that was found in release candidate (RC) for release 12.13.0 release-12.14.0 Issue or pull request that will be included in release 12.14.0 release-blocker This bug is blocking the next release Sev2-normal Normal severity; minor loss of service or inconvenience. team-confirmations Push issues to confirmations team type-bug Something isn't working
Projects
Archived in project
4 participants