Add support for Mainnet Sepolia and Arbitrum Sepolia testnets #3633
Conversation
Although it was previously possible to add Sepolia testnet as a custom network, we want to add it to the list of networks automatically added to the networks list upon switching on the 'Show testnet networks' setting. We want that because the only testnet currently added to that list is Goerli, which will become deprecated at the end of 2023 (see https://ethereum-magicians.org/t/proposal-predictable-ethereum-testnet-lifecycle/11575). So far we want to still keep Goerli on the list, as it is still widely used. We'll need to remove it once it'll reach end of its life. As at the moment `0x` project which we use for swaps does not support Sepolia, the sweep functionality will be disabled on this testnet. We need to enable it once the support is added.
| @@ -162,6 +175,7 @@ export const CHAIN_ID_TO_0X_API_BASE: { | |||
| [POLYGON.chainID]: "polygon.api.0x.org", | |||
| [OPTIMISM.chainID]: "optimism.api.0x.org", | |||
| [GOERLI.chainID]: "goerli.api.0x.org", | |||
| // TODO: Add Swap API for Sepolia once 0x supports it. | |||
There was a problem hiding this comment.
I haven't found any official confirmation they plan to do that. But I saw a couple of people requesting this support on their Discord. I've asked them there if they plan to support it since Goerli becomes deprecated, but I didn't get a response yet.
There was a problem hiding this comment.
Got a response from a member of their community:
Yes, they're looking to deploy on Sepolia in the near future.
[EDIT] Another update:
there is an active feature request for Sepolia support here, where you can track progress, upvote, etc. https://0x.canny.io/request-features
This feature request is currently under review.
background/constants/networks.ts
Outdated
| // TODO: Add `SEPOLIA` once `ethersproject` adds support for it to v5 (see | ||
| // https://github.com/ethers-io/ethers.js/issues/4374). |
There was a problem hiding this comment.
According to the linked issue ethersproject plans to release a patch to v5 with a couple of small changes, including support for Sepolia, but it's not high on their priority list, if I understood correctly.
If we don't want to wait, we can upgrade @ethersproject/providers to the 6.0.0-beta.8 version, which does support Sepolia (see https://www.npmjs.com/package/@ethersproject/providers/v/5.7.2?activeTab=code vs https://www.npmjs.com/package/@ethersproject/providers/v/6.0.0-beta.8?activeTab=code).
But I'm not sure how big of a deal is it to upgrate to v6 - would that be problematic / have potential to break things?
There was a problem hiding this comment.
Yeah, 6.x is a breaking change.
Ethers is set up to resolve static functions like getUrl recursively through the inheritance hierarchy. Let's create a TahoAlchemyProvider in our codebase that does something like this:
export class AlchemyProvider extends UrlJsonRpcProvider {
static getUrl(network: Network, apiKey: string): ConnectionInfo {
static getUrl(network: Network, apiKey: string): ConnectionInfo {
let host = null;
switch (network.name) {
case "sepolia":
host = "eth-sepolia.alchemyapi.io/v2/";
break;
default:
return AlchemyProvider.getUrl(network, apiKey)
}
return {
allowGzip: true,
url: ("https:/" + "/" + host + apiKey),
throttleCallback: (attempt: number, url: string) => {
if (apiKey === defaultApiKey) {
showThrottleMessage();
}
return Promise.resolve(true);
}
};
}
}And see if that fixes Alchemy.
Note that I believe all transactions are forcibly sent via Alchemy, so probably transaction submission won't work without this... But not 100% sure about that.
There was a problem hiding this comment.
@Shadowfiend, where in the codebase should I add this? In some separate file, or maybe in networks.ts? (Sorry, my knowledge of the codebase and TS is fragmentary 😞).
There was a problem hiding this comment.
A good place would probably be in a separate file under the same folder as the serial fallback provider (I think)
There was a problem hiding this comment.
Yeah that would be fine enough 👆
There was a problem hiding this comment.
I've tested the extension with and without this file and didn't notice a difference in behavior.
Ah, sorry, I forgot to add SEPOLIA to list of ALCHEMY_SUPPORTED_CHAIN_IDS when testing with the taho-alchemy-provider.ts file. Now I tested it locally again and it looks something is still off with the Alchemy config. I'm getting
Uncaught (in promise) Error: unsupported network (argument="network", value={"name":"sepolia","chainId":11155111,"ensAddress":null}, code=INVALID_ARGUMENT, version=providers/5.7.2)
at _ethersproject_logger_lib_esm_Logger.makeError (index.js:224:23)
at _ethersproject_logger_lib_esm_Logger.throwError (index.js:233:20)
at _ethersproject_logger_lib_esm_Logger.throwArgumentError (index.js:236:21)
at getUrl (alchemy-provider.js:67:24)
at new UrlJsonRpcProvider (url-json-rpc-provider.js:56:28)
at new AlchemyProvider (alchemy-provider.js:26:8)
at SerialFallbackProvider.creator [as alchemyProviderCreator] (serial-fallback-provider.ts:1100:15)
at new SerialFallbackProvider (serial-fallback-provider.ts:314:35)
at makeSerialFallbackProvider (serial-fallback-provider.ts:1128:10)
at index.ts:373:11
It looks like that the taho-alchemy-provider.ts file isn't taken under consideration. Am I missing something?
Also, maybe we can live without SEPOLIA added to the list of ALCHEMY_SUPPORTED_CHAIN_IDS? The transactions are successful without it.
There was a problem hiding this comment.
And sometimes there was a POST https://ethereum-sepolia-rpc.allthatnode.com/ 429 error in logs and transaction was performed, but got recognized as of Contract Interaction type.
That I managed to fix by changing RPC in c039fb1.
There was a problem hiding this comment.
It looks like that the
taho-alchemy-provider.tsfile isn't taken under consideration. Am I missing something?
Yes but updating the serial fallback provider to reference the new custom provider should do the trick.
Using the example shared, I ended up with something like this (Also noticed the URL was incorrect and fixed that.):
a245e8a
Unless it has already been mentioned, would like to point out that it would be easier to patch the ethers library and add the missing URL handler there.
There was a problem hiding this comment.
Thanks for the help @hyphenized! I confirmed your setup works and fixes the problem with reading historical data from the Ethereum Sepolia chain. Committed in eaf5023.
We still have the problem with reading historical data from Arbitrum Sepolia, but Alchemy does not have yet an RPC for this testnet. I've asked them on Discord if they plan to add it. Once it's added, I'm going to add a case for arbitrum-sepolia to TahoAlchemyProvider (but I think we don't need to do it in this PR).
There was a problem hiding this comment.
They posted an answer:
Yes, Alchemy is aware of the upcoming deprecation of the Goerli testnet for Arbitrum. We are actively working on adding support for the Arbitrum Sepolia RPC to ensure a smooth transition for our users. Keep an eye on our 📢│announcements updates for the official announcement!
| // "internal" is supported only on Ethereum Mainnet, Goerli and Sepolia atm | ||
| // https://docs.alchemy.com/alchemy/enhanced-apis/transfers-api#alchemy_getassettransfers-testnets-and-layer-2s |
There was a problem hiding this comment.
Actually, according to the link it is also supported on Polygon Mainnet, but I didn't want to do anything about that observation, as I don't know the system well enough...
We're adding support for Arbitrum Sepolia testnet network. Thanks to this change, Arbitrum Sepolia will be available on the list of networks after user chooses 'Show testnet networks' setting. We're also renaming our previously supported testnets to avoid confusion.
| [ARBITRUM_SEPOLIA.chainID]: { | ||
| title: "Arbiscan", | ||
| url: "https://sepolia.arbiscan.io/", | ||
| }, |
There was a problem hiding this comment.
There is no block explorer for Sepolia Arbitrum yet. The address I provided is the expected one, once ArbiScan adds a support. I've sent them a question whether and when they plan to add the support - no response yet.
With this entry, the links to scan website on Arbitrum Sepolia will currently lead to non-existing page. An alternative would be to not have the scan website entry for this testnet, but then there is an Unexpected Error when user opens details of transaction.
There is probably also an option to not display the link to the scan website for this testnet. Should I go with this approach?
There was a problem hiding this comment.
Since we're not needing to ship this like, tomorrow, let's wait and see if they answer. If it doesn't exist, we should drop the scanner until it does.
There was a problem hiding this comment.
Got an answer from ArbiScan:
Yes, we will be supporting the Sepolia testnet (on both Arbitrum and Optimism) and it should be ready soon. We will make sure to let our users know when they're up.
As a support for two new networks was added, we need to update tests that assert the number of supported networks.
Previously used RPC was sometimes causing errors in the logs and wrong recognition of type of transaction (`Contract interaction` instead of `Send`).
We want to add Sepolia to the list of Alchemy-supported chains. This will allow the extension to fill the list of historical transactions on that chain. We can't just add `SEPOLIA` to `ALCHEMY_SUPPORTED_CHAIN_IDS` because the `@ethersproject/providers@5.7.2` package that we use as a dependency does not support Sepolia yet (no `sepolia` case in the `getUrl` function of `./src.ts/alchemy-provider.ts`). The support is planned to be added in the upcoming patch, but as it's release date is unknown, we're adding the Sepolia support by creating `TahoAlchemyProvider` class that handles this case. In the future we may want to add there another case, for `arbitrum-epolia`, but we can't do that yet, as Alchemy dos not offers an RPC for Arbitrum Sepolia yet.
|
Ready for review.
|
|
Did some testing of the available build
I will note that I did not enjoy having to scroll on the network selector. Not sure if I would change it, but thought I would mention it in case someone has an idea. Once more external factors are addressed to support the features mentioned I will proceed with verifying those as well. |
| baseAsset: ARBITRUM_SEPOLIA_ETH, | ||
| chainID: "421614", | ||
| family: "EVM", | ||
| coingeckoPlatformID: "ethereum", |
There was a problem hiding this comment.
Should we leave these Coingecko references here and/or in the asset definitions? Wondering if these will give false prices for assets.
There was a problem hiding this comment.
I must admit, the coingeckoPlatformID value slipped my attention here and I copied the value from GOERLI/SEPOLIA networks. If I understand correctly, the coingeckoPlatformID value is used to fetch a price of the asset, so that we could show how much is the asset worth in USD. The list of supported values is here: https://api.coingecko.com/api/v3/asset_platforms. As there are no testnets listed there, I think in the extension we can either show a mainnet price (which is not a real value of the testnet asset) or not show the price at all. I prefer showing the mainnet value in that case - this allows us to test if the USD price is being shown in correct place, etc.
So what I think would make sense here is changing the coingeckoPlatformID's value to arbitrum-one for ARBITRUM_SEPOLIA and leaving the rest unchanged.
Unless there's something I'm missing?
| } from "@ethersproject/providers" | ||
| import { ConnectionInfo } from "@ethersproject/web" | ||
|
|
||
| export default class TahoAlchemyProvider extends AlchemyProvider { |
There was a problem hiding this comment.
Let's drop a comment noting why this exists.
| @@ -545,6 +545,7 @@ | |||
| "beta": "Mainnet (beta)", | |||
| "testnet": "Test Network", | |||
| "l2": "L2 scaling solution", | |||
| "l2Testnet": "L2 Test Network", | |||
There was a problem hiding this comment.
I would just use testnet, fwiw.
|
Tested by moving stuff from old test wallet to new one, let's |
michalinacienciala
changed the title
|
@Shadowfiend , I addressed your comments in #3636. |
In this PR we address the leftover comments from the review of #3633 (_Add support for Mainnet Sepolia and Arbitrum Sepolia testnets_). Latest build: [extension-builds-3636](https://github.com/tahowallet/extension/suites/16906558298/artifacts/965879218) (as of Thu, 05 Oct 2023 11:15:11 GMT).
## What's Changed * v0.49.0 by @Shadowfiend in #3625 * Run pledge sync daily by @michalinacienciala in #3626 * Add support for Mainnet Sepolia and Arbitrum Sepolia testnets by @michalinacienciala in #3633 * Sepolia support - small fixes by @michalinacienciala in #3636 * Update e2e tests after change of testing accounts by @michalinacienciala in #3639 **Full Changelog**: v0.49.0...v0.50.0 Latest build: [extension-builds-3643](https://github.com/tahowallet/extension/suites/17356530333/artifacts/991818546) (as of Wed, 18 Oct 2023 10:26:11 GMT).
Although it was previously possible to add Sepolia testnet as a custom network, we want to add it to the list of networks automatically added to the networks list upon switching on the 'Show testnet networks' setting. We want that because the only testnet currently added to that list is Goerli, which will become deprecated at the end of 2023 (see
https://ethereum-magicians.org/t/proposal-predictable-ethereum-testnet-lifecycle/11575). So far we want to still keep Goerli on the list, as it is still widely used. We'll need to remove it once it'll reach end of its life.
As at the moment
0xproject which we use for swaps does not support Sepolia, the sweep functionality will be disabled on this testnet. We need to enable it once the support is added.Latest build: extension-builds-3633 (as of Fri, 29 Sep 2023 16:39:01 GMT).