-
-
Notifications
You must be signed in to change notification settings - Fork 0
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
Transition panopticPool contract address to be variable #32
Conversation
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.
Nice!
@@ -49,6 +53,7 @@ export interface CalculateGammaResponse { | |||
|
|||
export interface GreekQueryRequest extends PanopticRequest { | |||
address: string; | |||
panopticPool?: string, |
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.
It looks like you could just have GreekQueryRequest extend PanopticPoolRequest like the others. But maybe this was intentional because we have the possibility of making it so the params here are a token pair instead of a PanopticPool?
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 motivation for leaving this optional was to allow the possibility of using either the "internal" greek methods (no need for PanopticPool address), or the contract methods (need PanopticPool address). Easy enough to choose just one of the cases and change appropriately.
The panopticPool contract address was previously hardcoded - it is made variable in this PR.
Closes #14