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

ProgramClient : add Send and Sync maker trait #6510

Closed
wants to merge 1 commit into from

Conversation

1500256797
Copy link

I plan to use ProgramClient to build a service, which is a multi-threaded service. However, the current design prevents ProgramClient from being sent or shared directly between multiple threads. Therefore, I have added these two markers to the trait.

@mergify mergify bot added the community Community contribution label Mar 28, 2024
@joncinque
Copy link
Contributor

Sorry, I'm a bit confused about how this is supposed to help.

A marker on the trait here shouldn't make a difference, since you should be able to add Send + Sync where you need it in the code, ie:

pub fn do_something_with_client<T: ProgramClient<ST> + Send + Sync, ST>(client: T) {}

Can you give a code example that's failing to build because of how the trait is currently implemented? That might help figure out the right solution

@1500256797
Copy link
Author

ok here is my code

    #[tokio::test]
    async fn test_sol_ladyf_swap() {
        tokio::spawn(async move {
            let payer = Keypair::read_from_file("sol_wallet.json")
                .expect("Failed to read keypair from file");

            let raydium_client = RaydiumCliemt::new(
                payer.into(),
                "https://solana-mainnet.g.alchemy.com/v2/erQnfrGnzxJOZe7uWmoZdFM-IAYrifG-",
            );
            let pool_cache = Some("raydium_pools.json".to_string());
            let price_cache = None;
            let allow_unofficial = Some(true);
            let hash = raydium_client
                .swap_exact_token_for_token(
                    "So11111111111111111111111111111111111111112"
                        .parse::<Pubkey>()
                        .unwrap(),
                    "3X8GcLiH2HttjyqePg7MazpMbwbgq5URUMTyDz5tkmdE"
                        .parse::<Pubkey>()
                        .unwrap(),
                    LAMPORTS_PER_SOLANA / 100000 * 10,
                    0,
                    pool_cache,
                    price_cache,
                    allow_unofficial,
                )
                .await
                .unwrap();
            println!(
                "Succesfuly executed swap from solana to ladyf. Signature {:?}",
                hash
            );
        });
    }

and here is error

image

The system prompt indicates that ProgramClient has not implemented the Sync trait, which prevents it from being sent between multiple threads.

@joncinque
Copy link
Contributor

This seems to be using RaydiumClient instead of ProgramClient -- do you have an example using the program client directly?

@1500256797
Copy link
Author

Precisely, my raydiumClient encapsulates a simple function. In this function, I instantiate ProgramClient to interact with Token. In the context of Tokio multithreading, it's not possible to assign ProgramClient to Token.

here is my function

        let token_in = Token::new(
            Arc::new(ProgramRpcClient::new(
                Arc::new(RpcClient::new_with_commitment(
                    self.rpc_url.clone(),
                    CommitmentConfig {
                        commitment: solana_sdk::commitment_config::CommitmentLevel::Confirmed,
                    },
                )),
                ProgramRpcClientSendTransaction,
            )),
            &spl_token::ID,
            &token_in,
            None,
            Arc::new(keypair_clone(&self.payer)),
        );

here is cargo check error

error: future cannot be sent between threads safely
    --> src/solana/raydium_client.rs:1304:9
     |
1304 | /         tokio::spawn(async move {
1305 | |             let payer = Keypair::read_from_file("sol_wallet.json")
1306 | |                 .expect("Failed to read keypair from file");
1307 | |
...    |
1334 | |             );
1335 | |         });
     | |__________^ future created by async block is not `Send`
     |
     = help: the trait `Sync` is not implemented for `(dyn ProgramClient<spl_token_client::client::ProgramRpcClientSendTransaction> + 'static)`, which is required by `{async block@src/solana/raydium_client.rs:1304:22: 1335:10}: std::marker::Send`
note: future is not `Send` as this value is used across an await
    --> src/solana/raydium_client.rs:580:10
     |
494  |         let token_in = Token::new(
     |             -------- has type `Token<spl_token_client::client::ProgramRpcClientSendTransaction>` which is not `Send`
...
580  |         .await
     |          ^^^^^ await occurs here, with `token_in` maybe used later
note: required by a bound in `tokio::spawn`
    --> /Users/ouhuang/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.36.0/src/task/spawn.rs:166:21
     |
164  |     pub fn spawn<F>(future: F) -> JoinHandle<F::Output>
     |            ----- required by a bound in this function
165  |     where
166  |         F: Future + Send + 'static,
     |                     ^^^^ required by this bound in `spawn`


@1500256797
Copy link
Author

I noticed that someone else also raised the same issue, that is, Token cannot be sent or synchronized, thus Token cannot be used in a multithreaded environment. Therefore, I submitted this pull request.

@1500256797
Copy link
Author

the same error : #6478

@joncinque
Copy link
Contributor

Thanks for giving an example! I tried it out, but unfortunately this takes a lot more to fix because we also need to be sure that the Signer and Signers present on all functions are also Send + Sync, and ends up breaking a lot of things.

I got a branch at https://github.com/joncinque/solana-program-library/tree/tkclientsend which gets token-client to compile and all the token-2022 tests to pass, but there's more work needed to fixup all uses of it. Would you be up to fixing it for the whole repo?

@1500256797
Copy link
Author

First of all, I am more than happy to do this and contribute to the open source community. I am honored to have the opportunity. However, I have only been learning Rust development for a year, so facing such a massive project, I feel a bit intimidated. But I will do my best to give it a try.

@joncinque
Copy link
Contributor

No worries at all. If you're up to it, I'd say give it a shot, and if you get stuck, let me know! I can always take over. I've created draft pull request #6523, you should be able to develop on that branch, or you can create your own PR from it

@1500256797
Copy link
Author

I will give it a try.

@github-actions github-actions bot added the stale [bot only] Added to stale content; will be closed soon label Apr 15, 2024
@github-actions github-actions bot closed this Apr 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community Community contribution stale [bot only] Added to stale content; will be closed soon
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants