Skip to content

prefer chainId when building block explorer urls#10587

Merged
brad-decker merged 1 commit intodevelopfrom
use-chainid-for-etherscan-link
Mar 9, 2021
Merged

prefer chainId when building block explorer urls#10587
brad-decker merged 1 commit intodevelopfrom
use-chainid-for-etherscan-link

Conversation

@brad-decker
Copy link
Copy Markdown
Contributor

@brad-decker brad-decker commented Mar 4, 2021

Progresses #8668, #7864, #10041

For the last two, #7864 and #10041 -- more work is needed to completely resolve this but @BboyAkers has inflight work that will help. This PR doesn't change how we are generating custom block explorer URLs.

Relies on MetaMask/etherscan-link#32

Details:

  • adds new utility method for building a blockExplorerUrl that prefers the chainId parameter of a transaction, but falls back to metamaskNetworkId
  • Remove a now unused older version that only used metamaskNetworkId
  • Use the new utility method in all places where building a link for a transaction
  • In the swaps page networkId/rpcPrefs were being pulled from redux using useSelector but then were only being passed into AwaitingSwap. Moved those selectors directly into AwaitingSwap
  • After confirming that the aforementioned state items were only used as part of building a blockExplorerUrl, removed networkId in favor of chainId.
  • Noticed that when building activity entries for a transaction's detail view, the activity entry copies the hash of the current transaction into each activity object. Later, in the click handler for clicking on an activity entry, the hash of the activity object is used along with the metamaskNetworkId of the primaryTransaction. I realize it is unlikely, perhaps impossible, for the metamaskNetworkId to differ from one transaction in a group to another but it caused me some confusion so I opted to clean this up and add metamaskNetworkId and chainId to the activity object.
  • grabs rpcPrefs for current network and sends them to showTransactionNotification so that we can link custom network notifications properly.
  • adds rpcPrefs in activity list so that proper URL generation can occur

@brad-decker brad-decker changed the title Version v9.1.0 use chainId when building block explorer urls Mar 4, 2021
@brad-decker brad-decker changed the title use chainId when building block explorer urls prefer chainId when building block explorer urls Mar 4, 2021
@brad-decker brad-decker force-pushed the use-chainid-for-etherscan-link branch 2 times, most recently from 705c2ab to 7384473 Compare March 4, 2021 13:17
@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [7384473]
Page Load Metrics (560 ± 38 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint43795884
domContentLoaded3726465587938
load3746475607938
domInteractive3726465587938

Base automatically changed from use-etherscan-link to develop March 4, 2021 18:16
@brad-decker brad-decker force-pushed the use-chainid-for-etherscan-link branch 2 times, most recently from b4c9e44 to cb605e5 Compare March 4, 2021 18:51
@brad-decker brad-decker marked this pull request as ready for review March 4, 2021 19:15
@brad-decker brad-decker requested a review from a team as a code owner March 4, 2021 19:15
@brad-decker
Copy link
Copy Markdown
Contributor Author

brad-decker commented Mar 4, 2021

Unit test failure is expected until MetaMask/etherscan-link#32 lands
it landed, and tests pass 🎉

@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [18f231e]
Page Load Metrics (550 ± 42 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint43785694
domContentLoaded3246415498943
load3256435508842
domInteractive3246415488943

@brad-decker brad-decker force-pushed the use-chainid-for-etherscan-link branch from 18f231e to cd23b1e Compare March 9, 2021 21:06
@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [cd23b1e]
Page Load Metrics (701 ± 47 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint46776494
domContentLoaded4428716999847
load4438727019847
domInteractive4428716999847

Copy link
Copy Markdown
Contributor

@NiranjanaBinoy NiranjanaBinoy left a comment

Choose a reason for hiding this comment

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

LGTM

@brad-decker brad-decker merged commit 38fe75b into develop Mar 9, 2021
@brad-decker brad-decker deleted the use-chainid-for-etherscan-link branch March 9, 2021 21:37
@github-actions github-actions bot locked and limited conversation to collaborators Mar 9, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants