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

fix: refactor blockaid metrics sync functions to async #10518

Merged
merged 37 commits into from
Sep 6, 2024

Conversation

vinistevam
Copy link
Contributor

@vinistevam vinistevam commented Aug 1, 2024

Description

After this PR to use the Security alert API to get the supported chains via isChainSupported which now it's an async function and we saw that it is used across the Blockaid metrics in the mobile client. So this PR aims to refactor every place that uses it to support it asynchronously.

Related issues

Fixes: https://github.com/MetaMask/mobile-planning/issues/1893
Related: https://github.com/MetaMask/mobile-planning/issues/1870

Manual testing steps

  1. Go to this test dapp and perform a regression test using "PPOM - Malicious Transactions and Signatures"
  2. Check if metrics are going to Segment (the env SEGMENT_WRITE_KEY will be required to perform this test)

Screenshots/Recordings

test-ppom.webm

Before

After

Pre-merge author checklist

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.

@vinistevam vinistevam added the team-confirmations Push issues to confirmations team label Aug 1, 2024
Copy link
Contributor

github-actions bot commented Aug 1, 2024

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.

Base automatically changed from feat/add-utility-function-supported-chains to main August 1, 2024 10:24
@vinistevam vinistevam changed the title Fix: Refacor blockaid metrics methods to async Fix: Refactor blockaid metrics sync functions to async Aug 1, 2024
@vinistevam vinistevam added the Run Smoke E2E Triggers smoke e2e on Bitrise label Aug 2, 2024
Copy link
Contributor

github-actions bot commented Aug 2, 2024

https://bitrise.io/ Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: c29298c
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/7cff7b00-2194-41e4-b36d-33f7f6327c4e

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

@vinistevam vinistevam added the Run Smoke E2E Triggers smoke e2e on Bitrise label Aug 23, 2024
@MetaMask MetaMask deleted a comment from github-actions bot Aug 26, 2024
@vinistevam vinistevam removed the Run Smoke E2E Triggers smoke e2e on Bitrise label Aug 26, 2024
@vinistevam vinistevam added the Run Smoke E2E Triggers smoke e2e on Bitrise label Aug 27, 2024
Copy link
Contributor

github-actions bot commented Aug 27, 2024

https://bitrise.io/ Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: 7f8d052
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/1ee3b676-4758-4d72-a1ec-294a6f43029b

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

matthewwalsh0
matthewwalsh0 previously approved these changes Sep 2, 2024
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.

Changes look great, I left some small feedbacks to be addressed.

@vinistevam vinistevam added Run Smoke E2E Triggers smoke e2e on Bitrise and removed Run Smoke E2E Triggers smoke e2e on Bitrise labels Sep 6, 2024
Copy link
Contributor

github-actions bot commented Sep 6, 2024

https://bitrise.io/ Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: cb5c855
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/792556c7-b84c-4084-b428-1aa32bd10adc

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

@vinistevam vinistevam added Run Smoke E2E Triggers smoke e2e on Bitrise and removed Run Smoke E2E Triggers smoke e2e on Bitrise labels Sep 6, 2024
Copy link
Contributor

github-actions bot commented Sep 6, 2024

https://bitrise.io/ Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: bf51625
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/8b6dfeb9-bb62-4002-93d9-bf002edcf321

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

Copy link

sonarqubecloud bot commented Sep 6, 2024

@vinistevam vinistevam merged commit c953018 into main Sep 6, 2024
35 checks passed
@vinistevam vinistevam deleted the fix/refacor-blockaid-metrics-to-async branch September 6, 2024 13:38
@github-actions github-actions bot locked and limited conversation to collaborators Sep 6, 2024
@metamaskbot metamaskbot added the release-7.31.0 Issue or pull request that will be included in release 7.31.0 label Sep 6, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
release-7.31.0 Issue or pull request that will be included in release 7.31.0 Run Smoke E2E Triggers smoke e2e on Bitrise team-confirmations Push issues to confirmations team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants