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

miscellaneous gauntlet deployment fixes #97

Merged
merged 6 commits into from
Jan 12, 2022
Merged

Conversation

aalu1418
Copy link
Collaborator

No description provided.

@@ -111,6 +111,7 @@ export default class Initialize extends SolanaCommand {
STATE ACCOUNTS:
- State: ${state.publicKey}
- Transmissions: ${transmissions}
- StoreAuthority: ${storeAuthority.toString()}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Used in store:set_writer, printing for ease of use

Comment on lines 73 to 79
const to = await Token.getAssociatedTokenAddress(
ASSOCIATED_TOKEN_PROGRAM_ID,
TOKEN_PROGRAM_ID,
token.publicKey,
new PublicKey(payee),
)
const info = await token.getAccountInfo(to)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This could be a misinterpretation on my end, but it seems like the payee parameter should be a normal solana address, and then the associated token address should be calculated before checking if exists

Copy link
Collaborator

Choose a reason for hiding this comment

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

If to is the associated token address, what is the info.address then?

Ok, they are the same by looking at this client code.

Conclusion, the const info = await token.getAccountInfo(to) LOC is unnecessary.

Copy link
Collaborator

@krebernisak krebernisak Jan 12, 2022

Choose a reason for hiding this comment

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

Additionally, what if node op did not prepare his associated token address?

The deployment will fail and be blocked, right?. So what should we do in that case?
Should we create the associated accounts (fund) on their behalf like described in this Example: Transferring tokens to another user, with sender-funding?

Why did this work before this change where we didn't check for associated acc?

  1. did we/they create all associated token accounts in advance?
  2. or does the contract creates/funds them on the first transfer?
  3. or we didn't even test withdrawals?

cc @archseer @RodrigoAD

Copy link
Member

Choose a reason for hiding this comment

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

Payee address needs to be a token associated address. NodeOps created it in advance on past deployments.
You can allow sender-funding with this option
If the NodeOp didn't prepare the payee address, we prepare it for him, and run the command again.

Copy link
Collaborator Author

@aalu1418 aalu1418 Jan 12, 2022

Choose a reason for hiding this comment

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

ok, seems like I was interpreting things incorrectly. the adminAddress from RDD should be the associatedTokenAccount. which makes these changes incorrect, i will remove them 😄

update: removed!

@@ -69,8 +69,13 @@ export default class CreateFeed extends SolanaCommand {
console.log(`
- Decimals: ${decimals}
- Description: ${description}
- Live Length: ${liveLength.toNumber()}
- Granularity (historical): ${granularity.toNumber()}
- Total Length: ${length.toNumber()}
Copy link
Collaborator

Choose a reason for hiding this comment

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

You could also compute the historical length (length - liveLength)

Copy link
Member

@RodrigoAD RodrigoAD left a comment

Choose a reason for hiding this comment

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

LGTM

@aalu1418 aalu1418 merged commit 33b3281 into develop Jan 12, 2022
@aalu1418 aalu1418 deleted the deployment-gauntlet-fixes branch January 12, 2022 14:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants