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 311 forks source link

contract client: add `sign` method, in addition to `signAndSend` #979

Closed chadoh closed 3 months ago

chadoh commented 4 months ago

Is your feature request related to a problem? Please describe.

Right now, using a contract client, there is no way to sign a transaction without also sending. There's some nice functionality around signing that's trapped inside the private send method of the `SentTransaction class.

Describe the solution you'd like

Let's move that logic to AssembledTransaction.sign, which will set this.signed on AssembledTransaction rather than SentTransaction. Then AssembledTransaction#signAndSend will call this, before calling SentTransaction.init, and SentTransaction.init will check that this.assembled.signed exists.

In addition to AssembledTransaction#sign, we will need an AssembledTransaction.send. It will need to check that this.signed is present. And, probably, signAndSend can check if this.signed is present, and avoid re-signing if so, making it equivalent to send in that case.

Describe alternatives you've considered

When first creating contract client, I really wanted an interface like

await assembledTx.sign().send()

But you can't do that, it ends up like

await (await assembledTx.sign()).send()

...or something equivalently ugly. It gets even worse if you wanted to require people to also include simulate in the chain:

await (await (await assembledTx.simulate()).sign()).send() 

OK, we COULD do the "bundled-await" approach, and I almost did, but @willemneal talked me out of it because it would lead to a large number of foot-guns and quite complex code.

After hemming and hawing and being annoyed at JS for a long time, we opted to bundle signAndSend(), since we guessed that people would want that 95+% of the time.

@kalepail would now like to be able to sign without sending, so it makes sense to add sign and send, in addition to the combined signAndSend.

chadoh commented 3 months ago

@Shaptic I can't assign people to issues in this repo. Could you give me permission? @BlaineHeffron is going to work on this one.