-
Notifications
You must be signed in to change notification settings - Fork 252
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
Improve function APIs with better generics #42
Conversation
ecb71af
to
7252ce6
Compare
…nd also the errors that get thrown
8e02299
to
c1db23a
Compare
…pe abstraction that allows for them
"@eth-optimism/contracts-ts": "^0.15.0", | ||
"@roninjin10/rollup-chains": "^0.0.3", | ||
"@wagmi/chains": "^1.7.0", | ||
"viem": "^1.4.2", | ||
"wagmi": "^1.3.9" | ||
"viem": "^1.9.5" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
based dep set
TChain, | ||
TChainOverride | ||
>, | ||
// TODO consider moving GetL2ChainId to make this even more generic |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i.e. we could move it out of this type and append it just in time in some places, or possibly we could simplify and do like targetChain
instead of L2ChainId
. But the name has to work for both reads and writes. I.e. to
doesn't work cause in some cases we are specifying where we want to read data.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, in the case of L2 -> L1 actions we never pass the L1 chain ID. It should be known. So maybe this is fine. But we could update these types to have L1 and L2 base write actions
No description provided.