Switch signers to generic in `RequestBuilder`

esolskjaer commented 4 weeks ago

We have

pub struct RequestBuilder<'a, C> {
    cluster: String,
    program_id: Pubkey,
    accounts: Vec<AccountMeta>,
    options: CommitmentConfig,
    instructions: Vec<Instruction>,
    payer: C,
    // Serialized instruction data for the target RPC.
    instruction_data: Option<Vec<u8>>,
    signers: Vec<&'a dyn Signer>,
    #[cfg(not(feature = "async"))]
    handle: &'a Handle,
}

The fact that payer in RequestBuilder is kept generic is very useful, as that makes it easy to extend functionality (e.g. when we need a threadsafe signer when using tokio like in my issue here https://github.com/coral-xyz/anchor/issues/3004). On the flip side, the fact that signer is explicitly constrained to Signer messes this all up, which is the problem I ran into in my issue.

This PR changes signers to be a vec of C. I'm opening it as a draft PR for now, as I don't believe this is the perfect solution, but I do think it is a step in the right direction:

On one hand I understand why signers is not a vec of C, as this would require all Signers to be of the exact same type.
On the other hand, not allowing this to be more fixated makes some use cases like tokio extremely difficult if not impossible right now. Referring to my issue - in my current branch, it's as simple as defining a custom that extends Signer and passing it in:


pub trait SyncSigner: Sync + Signer {}

/// A wrapper around a `Keypair` that implements the `SyncSigner` trait
pub struct SyncKeypair(pub Keypair);

impl Signer for SyncKeypair {
    #[inline]
    fn pubkey(&self) -> Pubkey {
        self.0.pubkey()
    }

    fn try_pubkey(&self) -> Result<Pubkey, SignerError> {
        self.0.try_pubkey()
    }

    fn sign_message(&self, message: &[u8]) -> Signature {
        self.0.sign_message(message)
    }

    fn try_sign_message(&self, message: &[u8]) -> Result<Signature, SignerError> {
        self.0.try_sign_message(message)
    }

    fn is_interactive(&self) -> bool {
        self.0.is_interactive()
    }
}

Maybe having a separate RequestBuilder that allows for this might be a solution, although idk how confusing that would be. Anyway, would love to get your take on this @acheroncrypto

vercel[bot] commented 4 weeks ago

@esolskjaer is attempting to deploy a commit to the coral-xyz Team on Vercel.

A member of the Team first needs to authorize it.

esolskjaer commented 4 weeks ago

Implemented your suggestion, but this still keeps it incompatible with normal keypairs in some cases unfortunately, look in client/example/blocking.rs for example. Reason being that from within functions receiving a Client<C> there is no fixed definition for what C is in that scope, meaning if we want to add signers these also have to be the generic type C.

One way I'd suggest to fix this would be to have two kinds of RequestBuilder - one with dyn Signer like we have it right now, and one with a second generic that can be set by the user for the signers. Wdyt @acheroncrypto ?

coral-xyz / anchor

Switch signers to generic in `RequestBuilder` #3006