Skip to content

fix: incorrect non-evm url in useActivityBlockExplorer#33396

Open
stanleyyconsensys wants to merge 4 commits into
mainfrom
fix/useActivityBlockExplorer
Open

fix: incorrect non-evm url in useActivityBlockExplorer#33396
stanleyyconsensys wants to merge 4 commits into
mainfrom
fix/useActivityBlockExplorer

Conversation

@stanleyyconsensys

@stanleyyconsensys stanleyyconsensys commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Description

hook useActivityBlockExplorer is return a incorrect explorer url for non-evm, it assume every non-evm is using ${baseURl}/tx/${hash} pattern

the fix using the predefine function getTransactionUrl from multichain urls, which already done in a better sharp

@vinnyhoward
This useActivityBlockExplorer somehow is redundant from useMultichainBlockExplorerTxUrl, but the PR is not going to make change to replace useActivityBlockExplorer, but we should consider to consolidate it

Changelog

CHANGELOG entry: Fixed useActivityBlockExplorer return incorrect URL for non-evm

Related issues

Fixes:

Manual testing steps

Feature: my feature name

  Scenario: user [verb for user action]
    Given [describe expected initial app state]

    When user [verb for user action]
    Then [describe expected outcome]

Screenshots/Recordings

Before

After

Pre-merge author checklist

Performance checks (if applicable)

  • I've tested on Android
    • Ideally on a mid-range device; emulator is acceptable
  • I've tested with a power user scenario
    • Use these power-user SRPs to import wallets with many accounts and tokens
  • I've instrumented key operations with Sentry traces for production performance metrics

For performance guidelines and tooling, see the Performance Guide.

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.

Note

Low Risk
Scoped change to activity explorer link resolution with existing multichain URL helpers; EVM path unchanged and tests cover the new behavior.

Overview
Activity details non-EVM “view on explorer” links were built with a one-size-fits-all ${baseUrl}/tx/${hash} (or simple template) rule, which produced wrong URLs for chains like Solana and Tron.

Non-EVM resolution now delegates to multichain getTransactionUrl, which uses the existing per-chain explorer URL formats. EVM behavior is unchanged: it still resolves the base URL from network configuration and getBlockExplorerTxUrl. Unsupported non-EVM chain IDs return no link when getTransactionUrl yields an empty URL.

Tests were aligned to assert Solana and Tron mainnet URLs and to expect undefined for unknown non-EVM chains instead of the old generic templating cases.

Reviewed by Cursor Bugbot for commit 51676dc. Bugbot is set up for automated code reviews on this repo. Configure here.

@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.

@metamask-ci

metamask-ci Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

PR template — items to address before "Ready for review"

Warnings — informational, address before merging:

  • Related issues section is empty. Add Fixes: #123 / Closes: <URL> / Refs: <Jira key>, or write a short rationale after the colon.
  • Manual testing steps still contain template content (the Gherkin example title or a [...] placeholder). Replace with real steps, or write N/A — <reason>.
  • Screenshots/Recordings section is empty. Add an image/video for user-facing changes, logs/console output for non-user-facing changes, or write N/A if no evidence is applicable.
  • Pre-merge author checklist has unchecked items (e.g. "I've tested on Android"). Every box must be consciously checked — see docs/readme/ready-for-review.md.

See docs/readme/ready-for-review.md for the full Definition of Ready for Review.

@github-actions github-actions Bot added size-S risk:low AI analysis: low risk labels Jul 16, 2026
}

if (isEvmChainId(chainId)) {
const base = findBlockExplorerUrlForChain(chainId, networkConfigurations);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this only use when it is evm, so move it to evm block

const url = buildNonEvmTxUrl(base, hash);
return { url, title: hostnameOf(url) };
const url = getNonEvmTransactionUrl(hash, chainId);
return url ? { url, title: hostnameOf(url) } : undefined;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

url can be empty, so align the undefined response

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Fixes non‑EVM activity “View on explorer” links by switching the non‑EVM URL construction in useActivityBlockExplorer to use the shared multichain explorer URL formatter (core/Multichain/utils#getTransactionUrl), avoiding malformed/nonstandard /tx/ URL patterns for chains like Solana and Tron.

Changes:

  • Update useActivityBlockExplorer to use getTransactionUrl for non‑EVM chains while keeping the existing EVM explorer resolution path.
  • Update hook tests to validate non‑EVM URLs for Solana and Tron mainnet scopes and to return undefined for unsupported non‑EVM chains.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
app/components/Views/ActivityDetails/hooks/useActivityBlockExplorer.ts Uses multichain explorer URL utility for non‑EVM tx links; keeps EVM behavior via existing EVM explorer helpers.
app/components/Views/ActivityDetails/hooks/useActivityBlockExplorer.test.ts Updates tests to cover Solana/Tron non‑EVM URLs and unsupported-chain behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@github-actions github-actions Bot added risk:medium AI analysis: medium risk and removed risk:low AI analysis: low risk labels Jul 16, 2026
@github-actions github-actions Bot added risk:low AI analysis: low risk and removed risk:medium AI analysis: medium risk labels Jul 16, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🔍 Smart E2E Test Selection

  • Selected E2E tags: None (no tests recommended)
  • Selected Performance tags: None (no tests recommended)
  • Risk Level: low
  • AI Confidence: 90%
click to see 🤖 AI reasoning details

E2E Test Selection:
The PR changes are limited to two files in the ActivityDetails hooks directory:

  1. useActivityBlockExplorer.ts: Refactors non-EVM block explorer URL generation by replacing a local buildNonEvmTxUrl function with the shared getTransactionUrl utility from app/core/Multichain/utils. Also fixes a bug where findBlockExplorerUrlForChain was called for non-EVM chains before checking if the chain is EVM.

  2. useActivityBlockExplorer.test.ts: Updates unit tests to reflect the new behavior, now testing with SolScope.Mainnet and TrxScope.Mainnet from @metamask/keyring-api.

This is a contained refactor/bug fix with:

  • No changes to navigation, wallet creation, transaction sending, or any critical user flows
  • No changes to shared components (TabBar, modals, confirmations)
  • Proper unit test coverage for the changed logic
  • No E2E smoke tests directly test block explorer link generation in activity details

The change only affects the URL displayed when a user taps "View on block explorer" in the ActivityDetails screen for non-EVM transactions (Solana, Tron). This is a minor UI detail not covered by any smoke E2E test suite. No E2E tags are warranted.

Performance Test Selection:
The changes are a refactor of a URL-building utility function in the ActivityDetails hook. There is no performance-sensitive code path affected - no rendering loops, no data fetching, no state management changes. The change simply replaces a local URL-building function with a shared utility. No performance tests are warranted.

View GitHub Actions results

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.40%. Comparing base (cef3872) to head (51676dc).
⚠️ Report is 7 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #33396   +/-   ##
=======================================
  Coverage   84.40%   84.40%           
=======================================
  Files        6132     6135    +3     
  Lines      163836   163813   -23     
  Branches    39995    39990    -5     
=======================================
- Hits       138288   138269   -19     
- Misses      16053    16054    +1     
+ Partials     9495     9490    -5     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@sonarqubecloud

Copy link
Copy Markdown

@jeremytsng jeremytsng self-requested a review July 16, 2026 08:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk:low AI analysis: low risk size-S team-networks Networks team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants