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

Test wagmi compatibility of chains #99

Open
bbenligiray opened this issue Oct 4, 2023 · 5 comments
Open

Test wagmi compatibility of chains #99

bbenligiray opened this issue Oct 4, 2023 · 5 comments

Comments

@bbenligiray
Copy link
Member

#84 has the package export chains formatted as objects that wagmi recognizes. However, we don't want to do this for chains on which wagmi doesn't work (for whatever reason). See #85 (comment) for more information.

Ideally, we would have end to end tests somewhere that confirms that wagmi works with the exported object. I'm aware that this would bloat up this repo a lot, but we need it somewhere to avoid replicating in all the other frontend implementations that depend on this.

@andreogle
Copy link
Member

andreogle commented Oct 14, 2023

I can't think of a reason why Viem (Wagmi) wouldn't work with a given chain, other than if we messed up the configuration here somehow. In my understanding, Viem is just a simple interface between your code and the RPC provider (aka an ethers.js replacement). Wagmi is just the layer on top of Viem that provides some nice React hooks.

Currently, the ping providers script does the following for each chain:

  1. Creates a simple Viem HTTP public client
  2. Calls .getChainId() to check that the chain ID in the JSON file is the same as what the RPC provider is reporting
  3. Calls .client.getBlock({ blockTag: 'latest' }) to check the the latest block is still somewhat recently produced

In this comment you mentioned testing that the "wallet connection actually works". We could quite easily add an automated .simulateContract() call (same as ethers.js .callStatic) but I'm not sure if this requires the wallet to be funded or not. Maybe each chain is different here too (?). Each chain would need a configured contract address to call. I'm not really sure what additional benefit this adds either.

I guess it boils down to what are we really trying to test here. With the current setup we are testing that:

  1. The RPC provider responds to requests
  2. It reports the correct chain ID
  3. The network is still producing blocks

Imo, these are already a pretty good guarantee that the config in this repo is good.

@bbenligiray
Copy link
Member Author

Mainly two things can go wrong

  1. Something like provider URL or blockchain explorer API URL is wrong, which will cause wagmi problems downstream. I guess it's reasonable to say that these should be caught early on. (That being said hardhat-etherscan is janky enough that something like Ping block explorer APIs as a CI step #91 is needed.)
  2. The chain (or rather it's provider) does some weird thing that causes viem to crash and burn. A good example is https://github.com/api3dao/tasks/issues/85 Essentially, a new feature is recently introduced to the Polygon testnet which causes provide.getBlock() to error due to invalid chain ID (ethers types chain ID as a Number and not a BigNumberish, which is arguably overly strict). I couldn't find any reference to this at the time it was first diagnosed so it's likely that we're the first ones that noticed this and there is no reason for this to not happen again. (This is also shows that this check should be done continuously.)

So in short, we need a good guarantee about that when we integrate a new chain to the Market, it will work. Currently I do the manual QA so it's not scalable, and it also limits the reusability of @api3/chains on other frontends (of ours). This may not be the correct solution but the problem stands, "it should work on this chain" and "it works on this chain" are very different things, the former necessitates QA downstream.

@Ashar2shahid
Copy link
Contributor

not sure if this should be a new issue but it would be nice to have a viem export similar to HardhatNetworksConfig

@bbenligiray
Copy link
Member Author

@Ashar2shahid
Copy link
Contributor

ah right, didn't see that

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

No branches or pull requests

3 participants