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.

Sign up for GitHub

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

Jump to bottom

Token type is !Send + !Sync #6478

Open
greged93 opened this issue Mar 22, 2024 · 6 comments
Open

Token type is !Send + !Sync #6478

greged93 opened this issue Mar 22, 2024 · 6 comments
Assignees
@greged93

Comments

@greged93
Copy link

The Token type in token/client/src/token.rs seems to have been designed in order to be Send + Sync, given the fact that Arc was used for the different fields. However, marker traits Send + Sync are not added to the traits objects and as such fails Token fails to capture this feature.

I'm willing to implement this if the issue is accepted.
@buffalojoec
Copy link
Contributor

buffalojoec commented Mar 22, 2024 •
edited

Hey there, are you talking about right here?

solana-program-library/token/client/src/token.rs

Lines 383 to 386 in ce0389b

impl<T> Token<T>
where
T: SendTransaction + SimulateTransaction,
{

And could you maybe elaborate on what the motivation would be to add this change? It's not immediately clear from the description.

Thanks!

@greged93
Copy link
Author

hey @buffalojoec, I was talking about the type below. From what I can see from the fields, it seems like the type was made in order to be Send + Sync? Otherwise why use Arc for client, payer,... ? I think we could make Token thread-safe by adding Send + Sync as a bounds for client, payer and nonce_authority.

solana-program-library/token/client/src/token.rs

Lines 331 to 342 in ce0389b

pub struct Token<T> {
client: Arc<dyn ProgramClient<T>>,
pubkey: Pubkey, /* token mint */
decimals: Option<u8>,
payer: Arc<dyn Signer>,
program_id: Pubkey,
nonce_account: Option<Pubkey>,
nonce_authority: Option<Arc<dyn Signer>>,
nonce_blockhash: Option<Hash>,
memo: Arc<RwLock<Option<TokenMemo>>>,
transfer_hook_accounts: Option<Vec<AccountMeta>>,
}

@buffalojoec
Copy link
Contributor

@greged93 Sorry I'm just getting back here!

Yeah I think making Token thread-safe makes sense. If you think it's important go for it!

@greged93
Copy link
Author

Great! Can you assign the issue?

@1500256797
Copy link

Why can't we directly send Token between threads? I think this is necessary to fully utilize multithreading

@1500256797
Copy link

If ProgramClient cannot be sent or synchronized directly between threads, how should it be handled in concurrent scenarios? I believe it needs to be implemented with the Send and Sync traits to address multithreading concurrency issues.

@1500256797 1500256797 mentioned this issue Mar 28, 2024
Closed
@joncinque joncinque mentioned this issue Mar 29, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@greged93 greged93

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

token-client: Make Token properly Send + Sync joncinque/solana-program-library
3 participants
@1500256797 @buffalojoec @greged93