stellar / js-stellar-sdk

Main Stellar client library for the JavaScript language.
https://stellar.github.io/js-stellar-sdk/
Apache License 2.0
628 stars 310 forks source link

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

Open Ifropc opened 1 week ago

Ifropc commented 1 week ago

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 commented 1 week ago

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

BernalHQ commented 1 week ago

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 commented 1 week ago

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 commented 1 week ago

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 commented 1 week ago

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 commented 1 week ago

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 commented 1 week ago

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 commented 1 week ago

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 commented 1 day ago

Thank you, will push a pr soon please