Change signature of `ClientOption.signTransaction` to be able to pass `KeyPair`

[ifropc]

basicNodeSigner is a great helper, but currently it is hard to find and is not referenced anywhere. I propose to should change the signature of signTransaction to current function | KeyPair and if user passes keypair, we simply use basicNodeSigner as the signer function. Currently, you need to dig into the source code to understand how it works. I like that we make it very extensible, but IMO in a lot of cases (especially testing), people just want to sign with a key pair.

[ShantelPeters]

Please can i be assigned to issue @Ifropc this would be my first time contributing to this ecosystem

[BernalHQ]

I am applying to this issue via OnlyDust platform.

My background and how it can be leveraged

Hi, my name is Bernal, and I’m a software developer with four years of experience. I’m passionate about contributing to projects and learn, and I would love the opportunity to contribute to this project

How I plan on tackling this issue

The first step is to understand the project and how it works. Once I have a good understanding of it, I’ll propose a solution to the issue. After that, I’ll implement the solution and run some tests to ensure everything works correctly.

[JoE11-y]

I am applying to this issue via OnlyDust platform.

My background and how it can be leveraged

With over four years in blockchain and backend development, I’ve worked across different ecosystems, handling everything from smart contract design to on-chain interactions and protocol integration. I focus on building secure, scalable, and reliable blockchain applications, managing both on-chain and off-chain infrastructure.

How I plan on tackling this issue

Start by studying the current design of the codebase, then proceed to implementing the task coupled with research.

[gregemax]

I am applying to this issue via OnlyDust platform.

My background and how it can be leveraged

I have a strong background in JavaScript and TypeScript, with experience in enhancing APIs for better usability. My familiarity with blockchain applications and transaction signing will enable me to effectively implement the proposed change to ClientOption.signTransaction for improved extensibility and ease of use.

How I plan on tackling this issue

I would modify the signature of ClientOption.signTransaction to accept a KeyPair as an argument. Then, I’d implement the logic to use basicNodeSigner when a KeyPair is provided. I’d also update the documentation to reflect this change, ensuring clarity for users on how to use the new functionality.

[od-hunter]

I am applying to this issue via OnlyDust platform.

My background and how it can be leveraged

Hi, please can I be assigned this issue? I am a blockchain developer and I have experience in html, css, react, JavaScript,TypeScript and solidity, python and Cairo. I'd love to contribute to this repo please.

How I plan on tackling this issue

To solve this issue, I’d take the following steps: 1.⁠ ⁠I’ll modify the signTransaction function's signature to accept a KeyPair directly as a parameter(to allow users to pass their key pair more easily). 2.⁠ ⁠⁠I’ll update the implementation of signTransaction to check if a KeyPair is provided. If it is, I’ll use the basicNodeSigner as the signer function for signing the transaction. 3.⁠ ⁠⁠I’ll then enhance the documentation to clearly explain how the new function signature works and provide examples of how to use signTransaction with a KeyPair(to better help users understand the functionality without needing to explore the source code). 4.⁠ ⁠⁠I’ll create or update unit tests to ensure that the modified signTransaction function behaves correctly when a KeyPair is provided, and that it still functions as expected in other scenarios. I believe this should work.

Please assign me.

[martinvibes]

I am applying to this issue via OnlyDust platform.

My background and how it can be leveraged

hello i am a frontend dev and blockchain developer please can i work on this issue :) and would love to be a contributor

How I plan on tackling this issue

Proposed Changes: Modify the function signature to accept either a current function or a KeyPair. If a KeyPair is provided, use basicNodeSigner as the signer function. Rationale: The current implementation makes it difficult to find and use basicNodeSigner, as it's not referenced well in the documentation. This change aims to improve usability, particularly for testing scenarios, by allowing developers to sign transactions directly with a KeyPair without needing to dig into the source code. This will enhance extensibility while simplifying the signing process for users.

[MullerTheScientist]

I am applying to this issue via OnlyDust platform.

My background and how it can be leveraged

I am a full-stack developer with experience in QA testing and languages like Python, Cairo, Solidity, React, and JavaScript.

How I plan on tackling this issue

i will Update ClientOption Interface Modify the ClientOption interface to update the signTransaction method signature: Change the parameter type to KeyPair Update the method documentation (if necessary) Update signTransaction Implementation Update the signTransaction method implementation to work with KeyPair Use the KeyPair object to sign the transaction Ensure compatibility with existing signing logic (if applicable)

[ShantelPeters]

I am applying to this issue via OnlyDust platform.

My background and how it can be leveraged

I am a blockchain developer , this would be my first time contributing to this ecosystem

How I plan on tackling this issue

i will solve this issue by modifying the signtransaction function to accept a KeyPair as a parameter. If a user provides a KeyPair,i will use basicNodeSigner as the signing function, Then simplifying the signing process for users and reducing the need to reference the source code for understanding. This change will enhance usability, especially in testing scenarios.

[od-hunter]

Thank you, will push a pr soon please

[chadoh]

@Ifropc I like this idea! But will the suggested implementation cause problems?

signTransaction is explicitly designed to match the typing of the method from Freighter and other SEP-43 wallets. This allows you to:

import { signTransaction } from '@stellar/freighter-api'
new Client({ signTransaction, ... })

If we change the typing of the function accepted by Client and AssembledTransaction, then I think we will end up with a bunch of typing errors.

I understand that this makes Node-based workflows more challenging, but it is my hypothesis that these workflows are more frequent for power users, who are more comfortable looking at other examples & source code to figure out the basicNodeSigner workaround.

That said, maybe there's some other way to simplify this. Maybe instead of overriding signTransaction, we can allow publicKey to be a Keypair?

[ifropc]

Hmm I'm not super familiar with TS, but if it's a enum type (i.e. can be either KeyPair or signTransaction's type), shouldn't it be backward compatible?

[chadoh]

Actually, maybe you're right. I guess the only remaining question I have is which do you think would be more clear: That publicKey can be a Keypair, or that signTransaction can?

[ifropc]

I'd argue publicKey should NOT be a KeyPair, as I'd expect KP to hold a secret key inside it (so it's very confusing that private key can be passed to the publicKey field)

[silence48]

Hmm I'm not super familiar with TS, but if it's a enum type (i.e. can be either KeyPair or signTransaction's type), shouldn't it be backward compatible?

This isn't type safe and no. if anything we need to add keypair as an optional input to the options as reflected in my review of #1127

[silence48]

Hmm I'm not super familiar with TS, but if it's a enum type (i.e. can be either KeyPair or signTransaction's type), shouldn't it be backward compatible?

This isn't type safe and no. if anything we need to add keypair as an optional input to the options as reflected in my review of #1127

In fact i don't even think we should change this. the best way is to just pass a signer function directly, not try to confuse people by accepting a KeyPair object or a function signature (regardless of if we overload signTransaction or if there is both objects in the options... i think we should keep signTransaction a function signature and scrap adding a keypair. However, I did add a review to the pr that reflects what i think would be the best solution if we have to do that... thanks ::)