Skip to content

Conversation

ScreamingHawk
Copy link
Contributor

  • Networks leverages format used by devops, except:
    • chain_id is bigint
    • rpc added
    • NetworkType.LOCAL added
    • Deprecated testnet field removed (using type instead)
  • Consistent usage of bigint for ChainId in dapp-client
  • Removes 0xsequence/network

Note: Keeps deprecated chains and ids as these were available in devops. Should we remove these?

@ScreamingHawk ScreamingHawk requested review from a team as code owners August 13, 2025 23:42
@corbanbrook
Copy link
Contributor

Going through the review i made note of a couple things that might require further discussion.

  1. chainId bigint or number: webevm:0x/viem/wagmi use number for their chainIds, ethers uses bigint in v6. Im in do whatever webevm does camp (number) as its what we use in sequence v3 and wallet v3 but would love to hear other opinions on this.

  2. viem library includes chains and they use this type: https://github.com/wevm/viem/blob/fc15f342728c1c5bed716b1a7e1707ebc778ae24/src/types/chain.ts#L18

There may be properties within their Chain type which we havent considered yet.

In some cases we will need to work with viem chain types, for instance when dealing with a wagmi connector and convert them to our own network type. Do we want to make our Network type compatible? or semi compatible? I think it might be good practice to at least follow some of their naming conventions even if we arent compatible.

  1. Treeshaking: Do we want all chain configs in an array/object? or exported separately?

@ScreamingHawk
Copy link
Contributor Author

I'm on board with updating chain id to number. Bigint has other issues like it requires non standard de/serialization and can't be used as a map key. Can go in another PR

@ScreamingHawk ScreamingHawk merged commit 86fe835 into master Aug 14, 2025
3 checks passed
@ScreamingHawk ScreamingHawk deleted the networks branch August 14, 2025 19:30
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