Skip to content
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

Misc/initialize ido pool #1

Open
wants to merge 20 commits into
base: master
Choose a base branch
from
Open

Misc/initialize ido pool #1

wants to merge 20 commits into from

Conversation

sjillen
Copy link

@sjillen sjillen commented Nov 5, 2021

Need to check that the accounts arrays are correct.
At the moment we are creating the required accounts live (they are just logged into the console).
Maybe we prefer to initialize them manually beforehand and input them via the form?

@sjillen sjillen requested a review from acamill November 5, 2021 19:04
@vercel
Copy link

vercel bot commented Nov 5, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/soteriateam/uxd-multisig-ui/A1iVC3Ukq5jLwyaZ12XVVKEAeRUJ
✅ Preview: https://uxd-multisig-ui-git-misc-initialize-ido-pool-soteriateam.vercel.app

@acamill
Copy link
Member

acamill commented Nov 5, 2021

I think we want to create them

@acamill
Copy link
Member

acamill commented Nov 5, 2021

Maybe can do the redeem

@sjillen
Copy link
Author

sjillen commented Nov 6, 2021

So far it seems that our variables are now correctly instantiated but we are still an issue right when we are sending the transaction.
Capture d’écran 2021-11-07 à 04 47 58

It looks like somewhere inside createTransaction, is not instantiated as a PublicKey (probably just a string).
I modified how we encode the data for the transaction with some hint from the guys from anchor.

// pool_signer -- this is the multisig PDA : 35F3GaWyShU5N5ygYAFWDw6bGVNHnAHSe8RKzqRD2RkT
//? While testing we used to use a derivation from the uxp mint for creating this account
{
pubkey: multisigPDA,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldnt it be changed back to what you were doing before with derivation?

@@ -96,7 +98,7 @@ function NewMultisigButton() {
}

export function MultisigInstance({ multisig }: { multisig: PublicKey }) {
const { multisigClient } = useWallet();
const { multisigClient, uxdClient } = useWallet();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

idoClient

const multisigPDA = new PublicKey("35F3GaWyShU5N5ygYAFWDw6bGVNHnAHSe8RKzqRD2RkT"); //? Can we actually get that from the multisigClient?
const uxpMint = new PublicKey('MNDEFzGvMt87ueuHvVU9VcTqsAP5b3fTGPsHuuPA5ey');//new PublicKey("UXPhBoR3qG4UCiGNJfV7MqhHyFqKN68g45GoYvAeL2M");
const usdcMint = new PublicKey("2wmVCSfPxGPjrnMMn7rchp4uaeoTqN39mXFC2zhPdri9"); //* That"s the mainnet one
const uxpMultisigTokenAccount = new PublicKey("GJgkVjjsYZeY2RLKcd7346A2dreykurTxkeNw6ysVQkc"); //! We should be able to get it from chain
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one needs to be the MNDE token account for the test, as the TMP change line 847

Comment and use 4BWt4xZ5okZRfd3KtXujjHJaHCLfjekJPLMdUzhFmSQW instead

const usdcMint = new PublicKey("2wmVCSfPxGPjrnMMn7rchp4uaeoTqN39mXFC2zhPdri9"); //* That"s the mainnet one
const uxpMultisigTokenAccount = new PublicKey("GJgkVjjsYZeY2RLKcd7346A2dreykurTxkeNw6ysVQkc"); //! We should be able to get it from chain

function WithdrawUSDCPoolListItemDetails({
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should just be withdrawIdoPool, code could stay generic and not use any mention of specific mint. For a later refactor I guess, let's just make this work

// We use the uxp mint address as the seed, could use something else though.
const [_poolSigner] = await anchor.web3.PublicKey.findProgramAddress(
[uxpMint.toBuffer()],
UXDIDOProgramAdress
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same comments about genericity (UXD Ido and uxp mint) but w/e

},
];
const transaction = new Keypair();
const txSize = 1000; // todo
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TX size might end up being bigger than 1000, if you still have your issue please double check this, shouldn't be though, but keep in mind

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks ok at least until execution

multisigClient.programId,
accounts,
data,
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

^ So you call the multisig program createTransaction, sending the multisigProgramID, the accounts this specific transaction expect, and the data serialized as expected by this transaction.

Below you should be doing the same but for the "nested" instruction.

as said on slack earlier. You pass the multisig programID but that should be the IDOprogramId. The account should be the account the Initialize account expect. And you should probably by passing some data too.

Remember to zoom out, and this "what is this instruction actually doing"

I'm not sure about where the data should be cause I don't have the IDE open, but please read the function signature of multisig. createTransaction and determine wether it should be passed where it is now or in the nesting

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also should have an result checking for the success to be dsplayed. non essential for now

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That one was quite the trick. See code update for how it is supposed to work

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah will do later, I'm planning maybe a small refactor tonight for more clarity and separating components into their own file, easier to navigate

{
pubkey: poolSigner,
isWritable: false,
isSigner: true,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a signer. Please check IDL

},
// uxp_mint -- This is UXP UXPhBoR3qG4UCiGNJfV7MqhHyFqKN68g45GoYvAeL2M token () https://solscan.io/token/UXPhBoR3qG4UCiGNJfV7MqhHyFqKN68g45GoYvAeL2M ()
{
pubkey: new PublicKey("UXPhBoR3qG4UCiGNJfV7MqhHyFqKN68g45GoYvAeL2M"),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please no hardcoded variable. How do we keep up with this. I wanna rewrite it already, feel like so easy to get mixed up

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also same as above, please prepare variable with explicit names before and assign the value in one place, there here use said variable for explicitness so we can reason

uxpMint,
poolUxp,
poolUsdc,
creatorUxp: uxpMultisigTokenAccount,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

multisigUxpTokenAccount

@acamill
Copy link
Member

acamill commented Nov 13, 2021

@ThomasProust Not sure what's the state of this PR. Can we maybe close it is it's stale/unused, and add the close to another one/remove/hide depending on what's available. I don't think we have the time not to make a proper tool

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants