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

feat(sdk-coin-sol): add sol token recovery support #4034

Merged
merged 1 commit into from
Nov 27, 2023

Conversation

alia-bitgo
Copy link
Contributor

@alia-bitgo alia-bitgo commented Nov 1, 2023

Adding in token recovery support for solana. This has been tested e2e for both cold and hot wallets.

TICKET: WP-930

@alia-bitgo alia-bitgo force-pushed the WP-930-add-sol-token-recovery-support branch from 52ded52 to ddc96b5 Compare November 1, 2023 16:10
@alia-bitgo alia-bitgo force-pushed the WP-930-add-sol-token-recovery-support branch 6 times, most recently from fbc1d2e to a03b8e8 Compare November 17, 2023 19:17
@alia-bitgo alia-bitgo marked this pull request as ready for review November 17, 2023 19:51
@alia-bitgo alia-bitgo requested review from a team as code owners November 17, 2023 19:51
rlutzbitgo
rlutzbitgo previously approved these changes Nov 20, 2023
Copy link
Contributor

@zahin-mohammad zahin-mohammad left a comment

Choose a reason for hiding this comment

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

Replied to the comment, but I am not convinced that hard coding this structre outside of statics is the best way to list out these tokens, maybe theres a constraint I'm not aware of that makes this harder to do. Lets discuss in slack/focus hour?

@alia-bitgo alia-bitgo force-pushed the WP-930-add-sol-token-recovery-support branch 3 times, most recently from fcddefc to 5157d36 Compare November 22, 2023 20:13
@alia-bitgo alia-bitgo force-pushed the WP-930-add-sol-token-recovery-support branch from 5157d36 to 8a46e48 Compare November 22, 2023 20:14
Copy link
Contributor

@zahin-mohammad zahin-mohammad left a comment

Choose a reason for hiding this comment

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

Minor comment, buy WP owned files lgtm

@@ -572,6 +600,38 @@ export class Sol extends BaseCoin {
};
}

protected async getTokenAccountsByOwner(pubKey = ''): Promise<[] | TokenAccount[]> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should an empty pubkey be valid for this?

@alia-bitgo alia-bitgo merged commit 94d6afb into master Nov 27, 2023
7 checks passed
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.

3 participants