cosmos / cosmos-sdk

:chains: A Framework for Building High Value Public Blockchains :sparkles:
https://cosmos.network/
Apache License 2.0
6.31k stars 3.64k forks source link

[Feature]: Remove multisigners #18759

Closed tac0turtle closed 7 months ago

tac0turtle commented 11 months ago

Summary

In the past we discussed removing multisigners for a single tx at the global level. The idea around the removal was to reduce complexity in the code and assumptions the state machine needs to make. Im reopening the discussion to understand if anyone is using this feature, in what capacity. In previous issues people have complained but could not cite a use case in the wild.

Secondly, multisigner can be done at a local message level if a module author would like.

This issue proposes removing:

This issue does not propose removing multimessages with a single signer.

We would like to understand who is using this feature and in what capacity. Due the high amount of complexity this introduces into refactors/rewrites at the core level we are evaluating removing it.

If you are using multiple signers in a message or transaction, please comment the code that does so. This will allow us to better understand how this is being used and how to best proceed.

If you are not using this feature and would still like to have this feature, please comment how you will use it.

Problem Definition

Multiple signers for a single message introduces higher amounts of complexity and maintenance in the long run.

Many clients do not support multiple signers in a single transaction today.

Proposed Feature

Remove the concept of many signers to a single message and transaction.

Secondly, this feature can be supported on a local level. This means module authors can support multiple signers in a message. They would need to handle signature verification, but this should not cause any problems as other modules (x/accounts) is handling its own verification as well.

webmaster128 commented 11 months ago

TL;DR: Full support for this initiative.


Every time you go from 1 to n in software development, you open a new dimension. It may not look like a big deal but is expensive for long term maintenance of verifier as well as all implementations of signers (CosmJS, Keplr, Ledger, cosmos-rs, Java, Dart, Swift, ....). And there is cognitive overhead for people reasoning about the correctness of the signing.

Multiple signatures for signing was implemented in CosmJS before the Stargate upgrade. When we built the Stargate client, it was not implemented and no user missed it in 3 years now. Implementing this correctly in a production system across different implementation is much harder than in this test. Especially the lack of a strictly standardized export format for unsigned and partially signed signatures make it hard for implementations other than the reference implementation.

The following use cases remain possible after the change:

  1. Classic threshold signatures: those translate in a single signature of a type that contains multiple signatures internally
  2. Threshold signing at elliptic curve level
  3. On-chain authorization aggregation (e.g. groups module, multisig contracts, DAO implementation)

Especially with 3. being widely accessible and cheap to do in Cosmos app chains, I see less of an argument for creating off-chain infrastructure to collect signatures from multiple parties for a single transaction.

eledra89 commented 11 months ago

Hey guys, we are actually using it. And in a real life use case as well.

We have a third party payment gateway accepting cash payment. Due to complex regulation, we can't do on-ramp crypto service like you normally see in Kado, Onramp, etc.

An user wants to make a micro transaction on a dapp (e.g. a NFT marketplace), but he doesn't have token. On ramp is expensive and not for micro payment. He doesn't want to overspent for (say a cup of coffee NFT or a simple faire ticket NFT). The process happens as follow

  1. an user pre-sign a transaction mint which require token he doesn't have.
  2. He pays fiat to the payment gateway, the gateway acknowledges the payment and its sign a transaction that send an equivalent amount of token to the user wallet.
  3. Client now bundle both the send token message and the mint message in 1 single transaction. The user is still the one who mint the NFT and the marketplace still receive the fund.

I'm not against the proposal of removing the multisigners feature if it is beneficial to the SDK maintainer. But this is something we actually use and probably a lot of more creative use cases can be born from it.

alexanderbez commented 11 months ago

@eledra89 why not just feegrants or something similar?

tac0turtle commented 11 months ago

thanks for your write up @eledra89. It seems you are using this global feature for a single type of message, or it many types of messages? Bundling is a valid use case, if the goal is to avoid fees for users that dont have funds there are other simpler ways to do this. But it seems that one signer = one message would be okay. the issue is one signer = one transaction. We can go step by step and start with one signer = one message, then go another step later on.

Feegrant as alex mentioned seems to fit your needs much better than multiple signers as well. With x/accounts module landing in the next few weeks bundling and custom accounts will be possible. This means that you could write a new account type for these users and then have a off chain bundler of these txs to pay for fees. This feature is built with single signer in mind.

eledra89 commented 11 months ago

Hi @alexanderbez, @tac0turtle, I'm aware of feegrant and we also use feegrant on a lot of features.

However, feegrant does have a lot of limitation, especially when you are on earlier version of Cosmos SDK. You can't do very fine grained feegrant for separated actions simultaneously. Even if you do, the desired behavior should be doing 2 actions in 1 transaction like I just described.

Again, I'm not arguing to keep the feature. I'm just presenting how we are benefited from using it since 0.45 until now.

tac0turtle commented 11 months ago

As a first step we will implement a multisigner acocunt in x/accounts that can be used as an alternative to the global design. Once we have it we will share so teams have an idea of how to do this. If its accepted we will move towards that design for multiple signers

sunnya97 commented 11 months ago

I agree with @eledra89. I can foresee usecases for multiple signers per message.

I personally wanted to create an "it's on me" msg type, that is a no-op other than requiring a specific signer, and thus can be added as the first msg of tx in order to change the fee payer. I know that feegrants exist, but imo, this is a simpler far more easy to integrate solution for one-time fee covering.

I am personally okay with removing multisigner for single messages, although I'm not sure if that meaningfully decreases complexity?

tac0turtle commented 11 months ago

I agree with @eledra89. I can foresee usecases for multiple signers per message.

do you mean per tx? the above sentence contradicts the last one. Signing complexity increased dramatically due to this feature. Btw, you can still do this feature without it being a global feature. We will prepare an example that we will host in the sdk allowing people to see and also use going forward. The example you mention is covered in the bundler design, there wont be a need to add a new message type.

There seems to be agreement on removing per messge multiple signers but not per tx multiple signers. This is a good first step because it already reduces complexity in our code.

webmaster128 commented 11 months ago

There seems to be agreement on removing per messge multiple signers but not per tx multiple signers. This is a good first step because it already reduces complexity in our code.

👍 This is already going to be very helpful for what Authentication Abstraction that Osmosis folks are worlking on here.

Would it be exactly one signer per message or 0-1 signers?

tac0turtle commented 11 months ago

👍 This is already going to be very helpful for what Authentication Abstraction that Osmosis folks are worlking on here.

Would it be exactly one signer per message or 0-1 signers?

0-1 signers. The AA stuff osmosis people are working on is the accounts module, we have implemented already. We are already adding support for 0 signers and skipping basic authentication

webmaster128 commented 11 months ago

Oh, interesting. Do you have an example of a message type with 0 required signers?

tac0turtle commented 10 months ago

Oh, interesting. Do you have an example of a message type with 0 required signers?

@testinginprod would be the nest person to explain it. He is working on it

SpicyLemon commented 9 months ago

Provenance blockchain's x/metadata module makes heavy use of messages that require multiple signatures. The module allows creation and maintenance of on-chain data where different aspects of that data often requires sign-off from multiple parties. These signature requirements are dynamic (different requirements for different specific data entries) and can end up being very complex. These signature requirements are one of the key features of the module.

The SDK's x/groups module is insufficient for the needs of the x/metadata module because of the complexity of the signer requirements. E.g. there might be one party that's required to sign off plus a second signature is required from one of three specific other parties. We also often need to transfer a permission from one party to another (leaving the others intact). Additionally, the module needs to specifically know which parties/signers are taking the action, which you can't get from the x/groups module. Group policies just don't have enough capabilities to be a replacement for what we've got. To allow more use of the x/groups module, we are considering extending it to support off chain signature aggregation and single message execution against a policy; that would require multi-signer support still, though.

The x/authz module has helped quite a bit, but doesn't fully remove the need to allow multiple signatures. We have some custom authorizations that help make these multi-signature actions easier on users. The x/authz Exec endpoint is insufficient for our needs, though (since it only works for single-signer things), so we made alterations in our x/metadata module to check with x/authz for applicable grants as needed. The result is that users can issue grants through x/authz that allow grantees to use the x/metadata endpoints directly on their behalf. One issue with this approach, though, is that the scope of an x/authz grant is often too large, i.e. the grant should only be usable for a subset of the data the granter has control of. It's just too complex to try to house in a custom authorization type. So, while the x/authz module greatly improves user interaction with the x/metadata module, it can't fully remove the need to have multiple signers.

We also need to be able to have multiple signers on a Tx. The primary reason is to handle those multi-signer Msgs. A second reason is to allow multiple Msgs with different signer requirements to be in a single Tx; so that several pieces of data are created/updated together (or fail together).

Ultimately, there would be no way that our x/metadata module could work if a Msg could only have a single signer. By extension, we also need the ability to have multiple signers on Txs.

tac0turtle commented 9 months ago

We might keep it for this release but mark it as deprecated for the next release. Some modules will disable it already though

SpicyLemon commented 9 months ago

To be clear, if Cosmos SDK stops supporting multiple signers on a message, then Provenance Blockchain can no longer be based on Cosmos SDK.

We have messages that require multiple signers, and we need to know exactly which accounts are doing that signing.

tac0turtle commented 9 months ago

we will keep it then. Its a poor design i guess we have to live with. It complicates designs for other users and modules but since its here and you guys use it then i guess we dont have a choice. Also as we talked on the accounts working group call your design could be simplified and made more elegant with the accounts module

SpicyLemon commented 9 months ago

For these endpoints, it's not enough to know that all of the accounts listed in the Msg actually signed the Msg/Tx. We also have to ensure that that set of signing accounts satisfies some other requirements (as currently defined in state) using other pieces of data (as currently set in state).

Multi-sig accounts (based only on their name and what's been said in this issue) hold some promise, but I fear we'd end up polluting the accounts module with a large quantity of one-offs. Ultimately, I won't know how well the accounts module will help us orchestrate these signing requirements until I sit down and really dig into it.

Also as we talked on the accounts working group call your design could be simplified and made more elegant with the accounts module

I'd be interested in hearing your ideas, but I fear you're underestimating the sheer complexity of these signing requirements.

testinginprod commented 9 months ago

The issue arises from the complexity introduced by allowing a message to be signed by multiple signers, leading to uncertainty about the message's sender. When a message involves multiple signers, it's unclear who the actual sender is—is it signer 1, signer 2, or signer 3? This ambiguity creates numerous exceptions within the system. The Cosmos SDK aims to be generic, but the current handling of multisigners, which requires the sender of a message (representing a state transition) to be identified as an array of sdk.AccAddress rather than a single sdk.AccAddress, does not align well with this goal.

Such an approach is problematic for intermodule communications, where modules exchange messages. Introducing an authorization system complicates matters further, as it becomes difficult to determine how to apply authorization policies to messages with multiple signers and decide whether to execute them. Initially, there was a consideration to prohibit MultiSigner messages from intermodule communications, which would be yet another exception.

This design leads to inconsistency across the system, particularly when considering other fundamental components like CosmWasm, which expects a single sender for a message.

From what I've observed, the use case for Provenance seems to be enabling a message sender to carry transaction-scoped authorizations from other users to execute the message. While MultiSigners can facilitate this, it introduces incorrect assumptions into the stack.

For now, my recommendation is to avoid disrupting nodes based on baseapp, allowing them to continue supporting multiple signers messages. Instead, we should adjust the serverv2 APIs to adopt a one-message-one-signer model. The module that would handle this "switch based on runtime complexity" is x/tx, which manages signer information and differentiates between single and multiple signers.

testinginprod commented 9 months ago

To be clear, if Cosmos SDK stops supporting multiple signers on a message, then Provenance Blockchain can no longer be based on Cosmos SDK.

I'd be curious to know if there's any blockchain SDK that allows multiple signers in the same message and even multiple messages in the same TX!

AFAIK: substrate allows multiple signers through multi sig, but the sender's identity is the multisig, which is yet agian different from what provenance seems to need.

iramiller commented 9 months ago

CosmWasm, which expects a single sender for a message.

we also found this to be extremely limiting ... not knowing who the original sender of a request was that caused a contract to dispatch subsequent messages. I have read the Cosmwasm design doc and noted that they were quite proud of not including this state and only including the contract address as the source "to prevent confusion". The answer here in my opinion was to be explicit and complete, not to redact information.

It seems from the complaints above that the issue isn't that there are multiple signers so much as there isn't a standardized primary signer on message. The current system takes fees from the first signer and uses this seemingly by convention.

tac0turtle commented 7 months ago

closing this as txv1 will continue in the current landscape. Once v2 of the core sdk is completed we will begin txv2 in which single signer txs will be enforced. Apps will be able to use both