-
Notifications
You must be signed in to change notification settings - Fork 171
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
add optional position mint params #121
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.
return value of getEphemeralSigners
Could you check the return value of getEphemeralSigners method ?
As far as I read the documentation, the return value is Pubkey.
If it is a PDA, the private key should not exist theoretically.
Checking if a method returns a different value each time
It is explained that this function is for use with createAccount, so we should be able to create an account.
However, we don't want the same value returned over and over again. Could you please ask with the Squads team to see if it changes each time or if it can be changed.
If we explain to them what we want to do, they will probably have some good advice.
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.
LGTM with nit.
sdk/src/whirlpool-client.ts
Outdated
@@ -1,6 +1,6 @@ | |||
import { Address } from "@coral-xyz/anchor"; | |||
import { Percentage, TransactionBuilder } from "@orca-so/common-sdk"; | |||
import { PublicKey } from "@solana/web3.js"; | |||
import {Keypair, PublicKey } from "@solana/web3.js"; |
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.
nit: Keypair
is no longer used.
sdk/src/impl/whirlpool-impl.ts
Outdated
@@ -276,14 +281,15 @@ export class WhirlpoolImpl implements Whirlpool { | |||
`upper tick ${tickUpper} is not an initializable tick for tick-spacing ${whirlpool.tickSpacing}` | |||
); | |||
|
|||
const positionMintKeypair = Keypair.generate(); | |||
const positionMintKeypair = Keypair.generate(); |
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.
super nit: This change is not needed (" " added)
Note
see also |
Add an optional parameter to specify the positionMint instead of generating it internally. This allows the ability to assign an ephemeral keypair that can be saved for later execution in the case of a multi-sig wallet.