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

Add Mezo testnet as a builtin network #3780

Merged
merged 10 commits into from
Feb 28, 2025
Merged

Add Mezo testnet as a builtin network #3780

merged 10 commits into from
Feb 28, 2025

Conversation

hyphenized
Copy link
Collaborator

@hyphenized hyphenized commented Feb 14, 2025

Adds Mezo testnet as a built-in network, there's no support yet for reverse name resolution. The price for the network base asset is resolved as the price for tBTC.

Testing env

SUPPORT_MEZO_NETWORK=true

Latest build: extension-builds-3780 (as of Fri, 28 Feb 2025 13:20:05 GMT).

Adds mezo network to the list of built in networks. The price for tBTC is
retrieved as the price for the base network asset if coingecko fails.
Does not support reverse lookups yet
Pushes Ethereum and Mezo to the top of the list
@hyphenized hyphenized marked this pull request as ready for review February 25, 2025 23:08
@Shadowfiend
Copy link
Contributor

Shadowfiend commented Feb 25, 2025

(1) This doesn't seem to work if I've already added matsnet and then remove it.
(2) I believe the team has added reverse name resolution, will ping them.

@Shadowfiend
Copy link
Contributor

Just to check—can you drop a screenshot of what I should see in the network dropdown? I'll look at this some more tonight/tomorrow afternoon.

@hyphenized
Copy link
Collaborator Author

hyphenized commented Feb 26, 2025

Just to check—can you drop a screenshot of what I should see in the network dropdown? I'll look at this some more tonight/tomorrow afternoon.

When a custom network becomes a built-in network we want to work with the
defined built-in network attributes over the custom ones. This also pushes
stale data out of state
// TBD: naming the normal reducer and async thunks
// Initialize redux from the db
// !!! Important: this action belongs to a regular reducer.
// NOT to be confused with the setNewCurrentAddress asyncThunk
this.store.dispatch(setSelectedAccount(dbAddressNetwork))
const { address, network } = dbAddressNetwork
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we kill the comment block above here? It's not doing anything for us.

Comment on lines +1354 to +1361
let supportedNetwork = this.chainService.supportedNetworks.find(
(net) => sameNetwork(network, net),
)

if (!supportedNetwork) {
// eslint-disable-next-line prefer-destructuring
supportedNetwork = this.chainService.supportedNetworks[0]
}
Copy link
Contributor

Choose a reason for hiding this comment

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

When would this take effect?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hmmm think I was just being paranoid safe here 😬

) {
await this.networks.put(defaultNetwork)
}
await this.networks.put(defaultNetwork)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm torn about this change. Part of the reason we do this is we want people to be able to replace the default RPC with one that they prefer—this makes that completely impossible, I believe. We're also being hobbled a bit here by not allowing folks to (a) turn off built-in networks and (b) have the same network with a different RPC hooked up.

My thought overall is that if the user has already added the network, maybe we leave it alone—but if they remove it, our default entry should bubble up. Where I am in my local right now is that I had added matsnet, then I enabled the flag and opened the extension, then I removed matsnet, and currently I don't see it anywhere.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hmmm yeah I agree, I think we should give users the option to choose between a builtin network, and a custom one, with different RPCs even if they have the same chain ID.

Right now what this does is replaces the existing mezo "network" object, changing only it's name. Then here:

private async initializeRPCs(): Promise<void> {
await Promise.all(
Object.entries(ChainDatabase.defaultSettings.CHAIN_ID_TO_RPC_URLS).map(
async ([chainId, rpcUrls]) => {
if (rpcUrls) {
await this.addRpcUrls(chainId, rpcUrls)
}
},
),
)
}

Default RPCs are merged with the already added custom RPC urls.

I'm having a hard time reproducing that last scenario, let's follow up on discord

@Shadowfiend
Copy link
Contributor

Shadowfiend commented Feb 28, 2025

Ok got this working. My local env was totally messed from previous stuff, unrelated to this PR.

Seems like we probably need a token list that at least lists mUSD to start, yeah? Not a blocker though.

Copy link
Contributor

@Shadowfiend Shadowfiend left a comment

Choose a reason for hiding this comment

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

Given that we're behind a feature flag and in the interest of keeping things moving, going to go ahead and land this. Missing pieces:

@Shadowfiend Shadowfiend merged commit 81c917f into main Feb 28, 2025
4 of 6 checks passed
@Shadowfiend Shadowfiend deleted the mezo-network branch February 28, 2025 12:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants