Skip to content

fix: 2 Duplicated Signature requests events are triggered for any signature#21791

Merged
segun merged 1 commit intoolu/fix-release-blocker-21739from
olu/fix-double-signature-requests
Nov 21, 2023
Merged

fix: 2 Duplicated Signature requests events are triggered for any signature#21791
segun merged 1 commit intoolu/fix-release-blocker-21739from
olu/fix-double-signature-requests

Conversation

@segun
Copy link
Copy Markdown
Contributor

@segun segun commented Nov 10, 2023

Description

When you click on a signature request that is flagged by PPOM, 2 events are sent to metrics instead of 1.

Expected behavior

Just 1 Signature request should be triggered for a Signature event flagged by PPOM

Related issues

Fixes: #21770

Manual testing steps

  1. Launch MM
  2. Open background console and open the network app, make sure it's recording network requests
  3. Open testdapp
  4. Click on any of the signature requests for PPOM e.g Malicious Trade Order
  5. Go to background console network tab and see 2 signature requests called as shown in the video
  6. Checkout this branch, build and run MM
  7. Repeat steps 2-4
  8. On the background console, you should now see only 1 signature request sent.

Screenshots/Recordings

Before

2-signature-requests.mp4

After

Screen.Recording.2023-11-10.at.10.39.43.mov

Pre-merge author checklist

  • I’ve followed MetaMask Coding Standards.
  • I've clearly explained what problem this PR is solving and how it is solved.
  • I've linked related issues
  • I've included manual testing steps
  • I've included screenshots/recordings if applicable
  • I’ve included tests if applicable
  • I’ve documented my code using JSDoc format if applicable
  • I’ve applied the right labels on the PR (see labeling guidelines). Not required for external contributors.
  • 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.

@segun segun added team-confirmations-secure-ux-PR PRs from the confirmations team needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) labels Nov 10, 2023
@segun segun self-assigned this Nov 10, 2023
@segun segun requested a review from a team as a November 10, 2023 09:42
@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.

@segun segun force-pushed the olu/fix-release-blocker-21739 branch from abd0763 to 367ee79 Compare November 10, 2023 09:53
@segun segun force-pushed the olu/fix-double-signature-requests branch from dd2f4b1 to aad167d Compare November 10, 2023 09:54
@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [aad167d]
Page Load Metrics (421 ± 247 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint84141100147
domContentLoaded7213488178
load851425421514247
domInteractive7213488178
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 175 Bytes (0.00%)
  • ui: -3.79 KiB (-0.06%)
  • common: 1.18 KiB (0.03%)

@segun segun force-pushed the olu/fix-release-blocker-21739 branch from 367ee79 to 3ce1d27 Compare November 12, 2023 19:48
@segun segun force-pushed the olu/fix-double-signature-requests branch from aad167d to ed2900d Compare November 12, 2023 19:58
@segun segun force-pushed the olu/fix-release-blocker-21739 branch 2 times, most recently from 1f3669a to 1f80255 Compare November 17, 2023 09:03
@segun segun force-pushed the olu/fix-double-signature-requests branch from ed2900d to 057275e Compare November 17, 2023 09:03
@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [057275e]
Page Load Metrics (680 ± 261 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint8412497115
domContentLoaded6812089126
load781466680544261
domInteractive6812089126
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 175 Bytes (0.00%)
  • ui: -3.79 KiB (-0.06%)
  • common: 1.18 KiB (0.03%)

@segun segun merged commit 8b14f94 into olu/fix-release-blocker-21739 Nov 21, 2023
@segun segun deleted the olu/fix-double-signature-requests branch November 21, 2023 17:58
@github-actions github-actions bot locked and limited conversation to collaborators Nov 21, 2023
@github-actions github-actions bot removed the needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) label Nov 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

team-confirmations-secure-ux-PR PRs from the confirmations team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants