-
Notifications
You must be signed in to change notification settings - Fork 16
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
Token Transfers MVP #2443
Token Transfers MVP #2443
Conversation
PR deployed in Google Cloud |
PR deployed in Google Cloud |
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.
Code looks good to me! 🔥
@@ -31,7 +31,7 @@ export function useActiveDomains(poolId: string, suspense?: boolean) { | |||
const manager = await cent.liquidityPools.getManagerFromRouter([r.router], { | |||
rpcProvider, | |||
}) | |||
const pool = await cent.liquidityPools.getPool([r.chainId, manager, poolId], { rpcProvider }) | |||
const pool = await cent.liquidityPools.getPool([r.chainId, manager, poolId!], { rpcProvider }) |
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.
why not just throw an error if the poolId doesn't exist? Seems odd to mark it as optional and then force it here
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.
the query is only enabled when the poolId is set, a few lines down. So this is only called when there's a poolId. Only typescript doesn't get that. Adding checks and throwing instead of the assertion would just add noise IMO
Description
#2408
Approvals
Screenshots
Impact