cosmjs icon indicating copy to clipboard operation
cosmjs copied to clipboard

Support multi signers in `@cosm/stargate`

Open youngjoon-lee opened this issue 4 years ago • 7 comments

The current sign() function of SigningStargateClient accepts only one signerAddress, but Cosmos basically support multi-signers. https://github.com/cosmos/cosmjs/blob/06fbc34f72f12c30a396c3ca296f80eca9fa60b0/packages/stargate/src/signingstargateclient.ts#L301-L307

I'd like to suggest to create a new signWithMultiSigners() function that looks like below:

public async signWithMultiSigners(
    signerAddresses: string[],
    messages: readonly EncodeObject[],
    fee: StdFee,
    memo: string,
    explicitSignerDatas?: SignerData[],
  ): Promise<TxRaw>

I've already implemented this function by following the Cosmos' guide, so that I can use it for my Cosmos-based blockchain. I think this is widely useful. If you think it's reasonable, I'm ready to open a PR :)

youngjoon-lee avatar Jun 21 '21 15:06 youngjoon-lee

Hi @youngjoon-lee,

which of those two use cases do you have?

  1. Multiple signatures because you use multiple messages in one transaction (e.g. Alice sends to Bob and Bob sends to Alice in one tx)
  2. Use a multisig address that has its own account and is controlled by a treshold of pre-defined public keys?

For 1. you just use sign two times. We might need a stargate version of SigningCosmosClient.appendSignature (@cosmjs/launchpad) though. It could be done by hand but requires a lot of knowledge of the transaction structure and protobuf.

For 2. this is implemented and demonstrated in this test case.

The interface you proposed does not seem right to me because a single client would need to hold private keys for multiple signers. This does not seem practical for real applications.

webmaster128 avatar Jun 21 '21 15:06 webmaster128

Hi @webmaster128 ,

My use case is none of 1 or 2. Actually, my use case is similar as 1. But, my transaction contains only one message and it needs to be signed by two accounts (not multisig address).

As you said, it can be done by SigningCosmosClient.appendSignature in Launchpad, but unfortunately, that approach doesn't work for Stagate as mentioned in the cosmos-sdk doc. It's because we must gather all SignerInfos first before singing with multiple accounts: https://docs.cosmos.network/v0.42/run-node/txs.html#signing-a-transaction-2

I just created a draft PR to share my idea, but I'm also thinking about:

The interface you proposed does not seem right to me because a single client would need to hold private keys for multiple signers.

Maybe, we need multiple wallets.

youngjoon-lee avatar Jun 21 '21 15:06 youngjoon-lee

My use case is none of 1 or 2. Actually, my use case is similar as 1.

I see. Yeah, I consider it a variant of 1. Multiple messsages is the easiest example but in general the case is: the set of all signers required by all messages.

but unfortunately, that approach doesn't work for Stagate as mentioned in the cosmos-sdk doc. It's because we must gather all SignerInfos first before singing with multiple accounts: https://docs.cosmos.network/v0.42/run-node/txs.html#signing-a-transaction-2

Yes, right. This is much more pain in Stargate than it was before.

Maybe, we need multiple wallets.

I have no issues writing that code. My issue is if you and me have to sign a transaction, I don't want to hand over my private key such that both signatures can be created on one machine in one call.

webmaster128 avatar Jun 21 '21 17:06 webmaster128

I have no issues writing that code. My issue is if you and me have to sign a transaction, I don't want to hand over my private key such that both signatures can be created on one machine in one call.

Yes. Thank you for your reply. I understand. In Launchpad, a signed tx can be sent to another machine, so that it can be signed by another key.

In my company, two addresses are managed by one person because of some reasons in the medical industry. But, it's probably not a general case.

(Actually, we need only public keys of all for gathering SingerInfos, but anyway, it's tricky.)

If you think it's not reasonable to deal with multiple wallets on one machine, I am willing to fork your repo for my project. But, I would like to ask you consider supporting this feature again. There might be some use cases that need multiple keys on a machine. I think that's why the guide is introduced in the cosmos-sdk doc.

youngjoon-lee avatar Jun 21 '21 21:06 youngjoon-lee

Re-opening as the request is very valid. We can implement a solution that includes the use case described here but supports a multi-machine setup ad well. Many of the changes from #838 make sense, specially looking at the signture of makeAuthInfoBytes.

webmaster128 avatar Jul 12 '21 09:07 webmaster128

#854 implements the ability to use different sequences for different signers. While this is not enough to close this ticket, it is an important step that allows you to maintain a much smaller diff in your fork.

webmaster128 avatar Jul 26 '21 12:07 webmaster128

Yeah. That is an important first step, as you said. Thank you for handling this issue.

youngjoon-lee avatar Jul 27 '21 05:07 youngjoon-lee