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.

Sign up for GitHub

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

Jump to bottom

feat: add token verification source count and link to block explorer #27759

Merged
merged 35 commits into from
Nov 2, 2024
Merged

feat: add token verification source count and link to block explorer #27759

merged 35 commits into from
Nov 2, 2024
+136 −74

Conversation

bfullam
Copy link
Contributor

@bfullam bfullam commented Oct 10, 2024 •
edited
Loading

Description

In a previous redesign, the information about the number of sources a token has been verified on and the link to that token on the relevant block explorer was removed from the swap page. This returns that information.

Open in GitHub Codespaces

Related issues

Fixes:

Manual testing steps

  1. Go to the swap page
  2. Select swap assets and amount
  3. See token verification info and block explorer link

Screenshots/Recordings

Before

Screenshot 2024-10-10 at 14 30 01

After

Screenshot 2024-10-10 at 14 30 21

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.

@bfullam
feat: add token verification source count and link to block explorer
6081700 
In a previous redesign, the information about the number of sources a token has been verified on and the link to that token on the relevant block explorer was removed from the swap page. This returns that information.
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Oct 10, 2024 •
edited
Loading

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.

@bfullam
Copy link
Contributor Author

bfullam commented Oct 10, 2024

I have read the CLA Document and I hereby sign the CLA

@metamaskbot
Copy link
Collaborator

Builds ready [5b410dc]
Page Load Metrics (2061 ± 261 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint29526661801560269
domContentLoaded158140282035536258
load159440532061543261
domInteractive26112512311
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 159 Bytes (0.00%)
  • common: 1 Bytes (0.00%)

@bfullam bfullam marked this pull request as ready for review October 10, 2024 13:23
@bfullam bfullam requested review from a team as code owners October 10, 2024 13:23
@vstern1
Copy link

vstern1 commented Oct 10, 2024

Looks good. Could we update the copy?

Verified on x sources. You can check on [block explorer].

Can we specify which block explorer? I think we did on Extension before it disappeared.
@coreyjanssen does this look ok?

@metamaskbot
Copy link
Collaborator

Builds ready [8523b5b]
Page Load Metrics (1844 ± 109 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint154323341839229110
domContentLoaded153523231817224108
load154423331844226109
domInteractive2196502311
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 159 Bytes (0.00%)
  • common: 1 Bytes (0.00%)

@jclancy93
Copy link
Contributor

jclancy93 commented Oct 10, 2024 •
edited
Loading

I noticed something similar to @vstern1

image

In the designs we are using human readable names (Etherscan, PolygonScan, LineaScan, etc.) but in the current implementation we are using human readable URLs instead (etherscan.io, lineascan.build, etc.). Hopefully we have a way to get human readable block explorer names instead

Update: looks like there's not directly in the BlockExplorer component as we have defined. But we could consider including a new enum that maps the chain id to block explorer readable name

ejwessel
ejwessel previously approved these changes Oct 10, 2024
View reviewed changes
@martahj
Copy link
Contributor

martahj commented Oct 10, 2024

We probably shouldn't show the message for the native token, as this makes a highly trustable token (ETH) look unreliable.
Screenshot 2024-10-10 at 10 50 54 AM

@coreyjanssen
Copy link
Contributor

Looks good to me. Approved by content design.

@bfullam
fix: update message copy
c7d20bb 
Create new message to handle updated copy for swap token verified sources.
@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
66.7% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

@metamaskbot
Copy link
Collaborator

Builds ready [5b010b0]
Page Load Metrics (2038 ± 147 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint27328371935482231
domContentLoaded165327741972268129
load166928332038307147
domInteractive32249724521
backgroundConnect8319598642
firstReactRender462871135225
getState5389478441
initialActions00000
loadScripts120822561486242116
setupStore11188504622
uiStartup186333772352429206
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 254 Bytes (0.00%)
  • common: 736 Bytes (0.01%)

infiniteflower
infiniteflower reviewed Oct 21, 2024
View reviewed changes
Copy link
Contributor

@infiniteflower infiniteflower left a comment

Choose a reason for hiding this comment

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

We probably shouldn't show anything if no To token has been selected

Screenshot 2024-10-21 at 1 54 43 PM

@bfullam
Copy link
Contributor Author

bfullam commented Oct 22, 2024

Absolutely right, good catch @infiniteflower! Fix made 👌

@metamaskbot
Copy link
Collaborator

Builds ready [8f8d708]
Page Load Metrics (1865 ± 109 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint156224721867229110
domContentLoaded15522315181318187
load156125151865227109
domInteractive25266595124
backgroundConnect9196505527
firstReactRender521991103818
getState4128313718
initialActions01000
loadScripts11321737134414469
setupStore108322199
uiStartup171030772139368177
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 451 Bytes (0.01%)
  • common: 730 Bytes (0.01%)

infiniteflower
infiniteflower previously approved these changes Oct 22, 2024
View reviewed changes
@bfullam bfullam requested a review from a team as a code owner October 23, 2024 14:44
@metamaskbot
Copy link
Collaborator

Builds ready [8eded5d]
Page Load Metrics (1833 ± 94 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint46223071771355170
domContentLoaded15402199180917885
load15502324183319594
domInteractive16277505526
backgroundConnect8128252713
firstReactRender482641024924
getState46612178
initialActions01000
loadScripts11081794134815976
setupStore1073352512
uiStartup169827042067229110
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 697 Bytes (0.01%)
  • common: 639 Bytes (0.01%)

@metamaskbot
Copy link
Collaborator

Builds ready [7284081]
Page Load Metrics (2249 ± 120 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint30726931939709340
domContentLoaded178826032213237114
load181527002249249120
domInteractive30190693416
backgroundConnect10106382713
firstReactRender541361032010
getState46422199
initialActions00000
loadScripts128620321669213102
setupStore1184442612
uiStartup202030202490273131
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 971 Bytes (0.01%)
  • common: 639 Bytes (0.01%)

@bfullam bfullam added this pull request to the merge queue Oct 31, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 31, 2024
@bfullam bfullam added this pull request to the merge queue Nov 2, 2024
Merged via the queue into develop with commit d773dd6 Nov 2, 2024
76 checks passed
@bfullam bfullam deleted the MMS-1564-swaps-token-verification-info branch November 2, 2024 08:39
@github-actions github-actions bot locked and limited conversation to collaborators Nov 2, 2024
@metamaskbot metamaskbot added the release-12.8.0 Issue or pull request that will be included in release 12.8.0 label Nov 2, 2024
@jclancy93 jclancy93 restored the MMS-1564-swaps-token-verification-info branch November 25, 2024 12:25
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@martahj martahj martahj left review comments

@jclancy93 jclancy93 jclancy93 approved these changes

@shane-t shane-t shane-t approved these changes

@zone-live zone-live zone-live approved these changes

@infiniteflower infiniteflower infiniteflower left review comments

@ejwessel ejwessel ejwessel left review comments

Assignees
No one assigned
Labels
release-12.8.0 Issue or pull request that will be included in release 12.8.0 team-swaps
Projects
PR review queue
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@bfullam @metamaskbot @vstern1 @jclancy93 @martahj @coreyjanssen @zone-live @shane-t @ejwessel @infiniteflower