-
Notifications
You must be signed in to change notification settings - Fork 52
Pool add getDepositQuote, getWithdrawQuote api's #47
Conversation
@odcheung - Thanks for the review! Pushed new commits which makes these changes:
|
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.
Hey @0xscuba,
Thank you for being patient with me so far! Public APIs are often difficult to adjust/recall after it goes out, so I want to make sure we get it right to make the right balance between feature-support & our burden of maintenance responsibility. I promise you that I only have this many opinions on 1 way door type decisions ><
We have a slight miscommunication on the %-usecase in the previous review. I don't think we should explicitly support that inside the SDK because it requires orca-pool to be aware of the user-wallet & it adds input-type complexity that is difficult for the user to use & especially difficult for us to maintain down the road.
Hey @odcheung, haha no worries! I really appreciate all the feedback, and sdk is better for it 🙂 Agree that we should be thoughtful around one-way-door decisions. I think in this case we definitely wanted to get it right because getDepositQuote, getWithdrawQuote will be an integral part of the SDK. Tangentially though, I think it's ok to push an api, then deprecate and add new versions of it without having api breaking changes (e.g. pool.someCoolMethod, later introduce pool.someCoolMethodV2). Updated the PR with the following changes:
|
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.
thank you for addressing the comments! Looks real good to me. Approved
one nit on the description of getPoolTokenMint. (no need to wait for further approve)
@odcheung - thanks for the review! updated the commented, and merging now. I'll also bump the version and publish it |
Context
Changes
getDepositQuote(maxTokenAIn, maxTokenBIn, slippage?)
getWithdrawQuote(poolTokenIn, slippage?)
Usage Example