Skip to content
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

flow to report false positive for blockaid #20856

Merged
merged 18 commits into from
Oct 4, 2023

Conversation

blackdevelopa
Copy link
Contributor

@blackdevelopa blackdevelopa commented Sep 13, 2023

Explanation

We should allow users and developers to report false positives for the security provider warnings.

Tasks

  • Add option on UI to report false design. See deign. The support url might change in the future.
  • Add new property to transactions and signature events

Screenshots/Screencaps

Before

Screenshot 2023-09-15 at 16 53 46

After

Screenshot 2023-09-21 at 11 33 07 Screenshot 2023-09-21 at 11 33 24

Manual Testing Steps

Pre-merge author checklist

  • I've clearly explained:
    • What problem this PR is solving
    • How this problem was solved
    • How reviewers can test my changes
  • Sufficient automated test coverage has been added

Pre-merge reviewer checklist

  • Manual testing (e.g. pull and build branch, run in browser, test code being changed)
  • PR is linked to the appropriate GitHub issue
  • IF this PR fixes a bug in the release milestone, add this PR to the release milestone

If further QA is required (e.g. new feature, complex testing steps, large refactor), add the Extension QA Board label.

In this case, a QA Engineer approval will be be required.

@github-actions
Copy link
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.

@blackdevelopa blackdevelopa self-assigned this Sep 14, 2023
@blackdevelopa blackdevelopa added the team-confirmations-secure-ux-PR PRs from the confirmations team label Sep 14, 2023
@blackdevelopa blackdevelopa force-pushed the 1061/report-false-positive-blockaid branch from 11f239c to e76a939 Compare September 15, 2023 15:49
@blackdevelopa blackdevelopa marked this pull request as ready for review September 15, 2023 15:54
@blackdevelopa blackdevelopa requested a review from a team as a code owner September 15, 2023 15:54
@codecov
Copy link

codecov bot commented Sep 15, 2023

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (2e1ab26) 68.64% compared to head (260bb19) 68.64%.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #20856      +/-   ##
===========================================
- Coverage    68.64%   68.64%   -0.00%     
===========================================
  Files         1016     1016              
  Lines        40758    40763       +5     
  Branches     10885    10885              
===========================================
+ Hits         27977    27980       +3     
- Misses       12781    12783       +2     
Files Coverage Δ
...der-banner-alert/security-provider-banner-alert.js 100.00% <ø> (ø)
ui/helpers/constants/zendesk-url.js 100.00% <ø> (ø)
...saction-base/confirm-transaction-base.component.js 56.60% <ø> (ø)
...ponents/app/signature-request/signature-request.js 82.19% <50.00%> (-0.91%) ⬇️
...nents/app/transaction-alerts/transaction-alerts.js 97.30% <66.67%> (-2.70%) ⬇️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@metamaskbot
Copy link
Collaborator

Builds ready [f5b061b]
Page Load Metrics (1530 ± 70 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint108169133178
domContentLoaded13981888152914770
load13991888153014770
domInteractive13981888152914770
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 50 Bytes (0.00%)
  • common: 70 Bytes (0.00%)

@blackdevelopa blackdevelopa added the needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) label Sep 18, 2023
jpuri
jpuri previously approved these changes Sep 18, 2023
Copy link
Contributor

@jpuri jpuri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good 👍

segun
segun previously approved these changes Sep 18, 2023
@segun
Copy link
Contributor

segun commented Sep 18, 2023

Looks good, fix the CI failures.

@blackdevelopa blackdevelopa dismissed stale reviews from segun and jpuri via b31f46d September 19, 2023 14:58
Copy link
Contributor

@digiwand digiwand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding some more comments and suggestions. Is there are related issue ticket for this work? If so, can we add it to the description?

ui/components/app/transaction-alerts/transaction-alerts.js Outdated Show resolved Hide resolved
app/_locales/en/messages.json Outdated Show resolved Hide resolved
ui/components/app/transaction-alerts/transaction-alerts.js Outdated Show resolved Hide resolved
ui/components/app/transaction-alerts/transaction-alerts.js Outdated Show resolved Hide resolved
digiwand
digiwand previously approved these changes Sep 20, 2023
Copy link
Contributor

@digiwand digiwand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for addressing the suggestions and updates! LGTM!

@metamaskbot
Copy link
Collaborator

Builds ready [37b21fc]
Page Load Metrics (1629 ± 95 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint107172137178
domContentLoaded13852026162819895
load13852026162919895
domInteractive13852026162819895
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 197 Bytes (0.00%)
  • common: 173 Bytes (0.00%)

@metamaskbot
Copy link
Collaborator

Builds ready [ca092be]
Page Load Metrics (1635 ± 47 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint119201145199
domContentLoaded1499194216349747
load1500194216359747
domInteractive1499194216349747
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 197 Bytes (0.00%)
  • common: 173 Bytes (0.00%)

@blackdevelopa blackdevelopa dismissed stale reviews from jpuri, digiwand, and segun via 05294a5 October 4, 2023 11:48
@blackdevelopa blackdevelopa force-pushed the 1061/report-false-positive-blockaid branch from ca092be to 05294a5 Compare October 4, 2023 11:48
@metamaskbot
Copy link
Collaborator

Builds ready [260bb19]
Page Load Metrics (825 ± 355 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint811209194
domContentLoaded6811886126
load791651825738355
domInteractive6811886126
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 197 Bytes (0.00%)
  • common: 173 Bytes (0.00%)

@blackdevelopa blackdevelopa merged commit c3e72e9 into develop Oct 4, 2023
9 checks passed
@blackdevelopa blackdevelopa deleted the 1061/report-false-positive-blockaid branch October 4, 2023 14:59
@github-actions github-actions bot locked and limited conversation to collaborators Oct 4, 2023
@metamaskbot metamaskbot added the release-11.4.0 Issue or pull request that will be included in release 11.4.0 label Oct 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
QA Passed release-11.4.0 Issue or pull request that will be included in release 11.4.0 team-confirmations-secure-ux-PR PRs from the confirmations team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants