ADR036: Arbitrary signature

[fdymylja]

Description

ADR for arbitrary message signature to allow cosmos-sdk users to create and verify messags, where by message we intend arbitrary bytes (json files, text, etc).

Tentative implementation: #7896

---

Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

• Targeted PR against correct branch (see CONTRIBUTING.md)
• Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
• Code follows the module structure standards.
• Wrote unit and integration tests
• Updated relevant documentation (docs/) or specification (x/<module>/spec/)
• Added relevant godoc comments.
• Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
• Re-reviewed Files changed in the Github PR explorer
• Review Codecov Report in the comment section below once CI passes

[alessio]

CC'ing @antoineherzog

[aaronc]

Is this just for cli users? Or would nodes do this verification too?

[ethanfrey]

Interesting, so the signers / clients wouldn't have to change at all - this is just a standardized spec for how to make an invalid tx that serves an an off-chain signed message. I like it.

Only question is why this is a module and not just a proto definition.

[ethanfrey]

👍

[ethanfrey]

Huh? This is a module? I thought this was just a format specification.

Can't you just define a format as above, and add a protobuf spec fo MsgSendText (and legacy amino compatibility for it)? The chain never needs to process this, as existing decorators would be sure to reject it.

[amaury1093]

Agreed, it could go in x/auth.

[ethanfrey]

I think the main users would not be cli tools, but rather keplr and other web-based signers. As one of the most common uses of signed messages in ethereum is to authenticate yourself to a webapp. (Please correct me if I am wrong, and ideally expand on this on context - give 1 or 2 concrete workflows that would be enabled)

[romeo4934]

Great to see a signed message spec coming into the cosmos world 👍
I agree with Ethan, I believe this spec is only about the format specification. The spec maybe could provide an example of a signed message. Up to the client to implement this spec. Ideally we want to have the major clients of the cosmos ecosystem like KeplR and other web-based clients to implement it.

Signed message are usefull to login into a Cosmos Dapp. They are also usefull for external provider like KYB providers which wants to create certificates in the cosmos ecosystem with a cosmos key for corporate use cases. CLI client can be still be interesting for theses kind of providers who wants to be able to sign a message from a command line interface.

[moshemalawach]

Implemented in :

• https://github.com/aleph-im/pyaleph
• https://github.com/aleph-im/aleph-js
• https://github.com/aleph-im/aleph-client

Example use with a cosmos account:
https://aleph-im.github.io/aleph-js/guide/getting-started.html#cosmos-cosmos-sdk

[aaronc]

Why do we need a new Msg type? Could we just overload memo? Do we even need to make this a Tx?

[zmanian]

@aaronc this needs to look sufficiently like a transaction to be signable on a ledger or other existing custody solution.

Overloading memo is bad because we want to sign structured data.

[alexanderbez]

I feel like a lot is missing from this. What does the structure look like? How is it encoded? How will an HSM sign it and display it? What kinda of formats/characters are acceptable?

[fdymylja]

I've addressed the changes highlighted. Added the Msg definition and an example.

How is it encoded?

If by encoded you mean serialized, then shouldn't we be able to support both JSON and Protobuf?

How will an HSM sign it and display it?

Since this is an sdk.Msg, shouldn't what we have at our disposal (tx subcommand for module supports ledger already, for example) work already?

[tac0turtle]

I am afraid we can't continue doing this work because there's ostensibly a lack of priorities alignment across the various teams that work on the Cosmos SDK.

I would say this statement is false. No one has taken the initiative to move this forward.

Anyways, we are here to collaborate. Let's figure out a way we can move forward with this. We can include the discussion points in an upcoming sdk architecture call. And coordinate a call with the necessary parties if they are unable to attend the sdk calls.

[zmanian]

So I think really the only open issue is how to handle stateful validation criteria that might be introduced into the SDK by parallel contemporaneous work.

Here is what I would propose.

The Verifier should provide an optional validation context to the signer.

"validation_context":"chain-id", "height"

The signer should sign over the validation context and provide correct credentials.

The verifier will need to check the signed validation context is as expected and then pull the account state from the chain-id and height to verify.

[github-actions]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[zmanian]

Sommelier has a near term usecase for this authorization mechanism.

If there isn't support for it, we will do it out of tree

[alessio]

I am afraid we can't continue doing this work because there's ostensibly a lack of priorities alignment across the various teams that work on the Cosmos SDK.

I would say this statement is false. No one has taken the initiative to move this forward.

@fdymylja has. He opened a PR that implemented this ADR.

I want to add that unlike other ADRs, whose implementations were merged even before their respective ADRs had been approved, the implementation PR got blocked because the ADR committee failed to reach a consensus.

I'm deliberately not pointing my finger at anyone. I believe that no individual developers are at fault here. This ADR is just the brightest example of the damage that the utter lack of clear governance can do to an ever-growing and complex open source project like Cosmos SDK.

My 2p

[alessio]

If there isn't support for it, we will do it out of tree

I would welcome such an initiative. Please do reach out, we're very happy to collaborate on an out-of-tree solution.

[moshemalawach]

Sommelier has a near term usecase for this authorization mechanism.

If there isn't support for it, we will do it out of tree

At aleph.im we are already using it in production according to what has been implemented in first proposal.

[robert-zaremba]

I want to add that unlike other ADRs, whose implementations were merged even before their respective ADRs had been approved, the implementation PR got blocked because the ADR committee failed to reach a consensus.

Is there still anything blocking this ADR? If yest, let's add it to the Friday Architecture Call agenda. This is the easiest way to unblock ADRs.

[tac0turtle]

We have now reached consensus. We have the three approvals needed and then some. Let's merge this PR.

[aaronc]

I want to add that unlike other ADRs, whose implementations were merged even before their respective ADRs had been approved, the implementation PR got blocked because the ADR committee failed to reach a consensus.

I'm deliberately not pointing my finger at anyone. I believe that no individual developers are at fault here. This ADR is just the brightest example of the damage that the utter lack of clear governance can do to an ever-growing and complex open source project like Cosmos SDK.

Wow, I'm sensing you and possibly your team are pretty frustrated by the process here, and I'm sorry to hear that. This is a complex project in complex times and I definitely appreciate the challenges in working on a multi-entity, multi-stakeholder open source project. I also apologize for any way that I or my team may have contributed to that frustration, either directly or indirectly.

That said, I feel it is important to note that as far as I can tell @marbar3778's original observation that no one picked up this work seems pretty accurate. From reviewing the commit log and also the meeting notes where this ADR was discussed, it seems pretty clear that nobody worked on this or #7896 after Nov 16th, 2020. I did find references in meeting notes from Nov 20th and Feb 26th indicating that your team intended to spend more time on this with the aim to reconcile it with ADR 034, but I have not seen any progress on that. I understand there are things in this development process that can be upsetting. It seems like the back and forth on Nov 13th got a little tense even though it ended alright. All of that aside, I do not think it is accurate or fair to say that the ADR committee blocked this or the implementation because of a failure to reach consensus. There were and still are outstanding issues including compatibility with ADR 034, and now that this has been merged they still will need to be addressed. If your team needed to move this forward quickly as sort of a "preview" feature, I and I believe the rest of my team would have been open to that. From my perspective, we had given the prelimary concept ACK both here and in architecture meetings and were waiting for your team to take the next steps.

Without a doubt we need improve the way we do things as a complex, multi-stakeholder project. At the same time we are all a part of the process. If you’re feeling blocked, if things aren’t going well in a discussion thread, or what not, please reach out or bring it up on a call. We’re here to collaborate and improve things and we want make this project work for all stakeholders.

[alessio]

Without a doubt we need improve the way we do things as a complex, multi-stakeholder project. At the same time we are all a part of the process. If you’re feeling blocked, if things aren’t going well in a discussion thread, or what not, please reach out or bring it up on a call. We’re here to collaborate and improve things and we want make this project work for all stakeholders.

I believe there are still governance issues here that have not yet been resolved (and neither of our respective teams are part of the problem here). I tried to contribute to the conversations and advised on the subject multiple times. Nothing happened afterward.

That said, I appreciate your response wholeheartedly. We remain committed to supporting the Cosmos SDK development on a best-effort basis.

Thanks, and keep up the great work!

[webmaster128]

Nice work. Just to double check: do we agree that having multiple messages is supported by this spec? This is somewhat implied by not having a message restriction in the dummy tx.

[robert-zaremba]

@webmaster128 this is not for signing transaction - we don't want to do it here. This is for signing messages which are not transactions.

[webmaster128]

@robert-zaremba sure, but if you wrap a special message type in an msg array of a (fake) cosmos-sdk/StdTx, we should either support multiple messages or add an explicit restriction that msg must contain exactly one element in order to be valid.

[webmaster128]

In cosmos/cosmjs#847 you find is a CosmJS implementation of this spec. Pretty straight forward if Amino signing tooling is present.

The main open question I have is how to encode the signed data, i.e. the exchange format from signer to verifier. I see the followiging options:

1. The pair (data, raw_secp256k1_signature). This is how Ethereuum's web3.eth.sign works.
2. The pair (data, signature as StdSignature). Like 1. but is open for Non-Secp256k1 signature algorithms.
3. The whole StdTx structure as shown in this ADR with top level "type": "cosmos-sdk/StdTx", "value": ….
4. The content of StdTx's value field. This is currently used in my implementation.

Option 3 and 4 allow using multiple datas through multiple messages (see cosmos/cosmjs@06da6b8). I'm not saying this is useful but seems to work without violating the spec.

[fdymylja]

In cosmos/cosmjs#847 you find is a CosmJS implementation of this spec. Pretty straight forward if Amino signing tooling is present.

The main open question I have is how to encode the signed data, i.e. the exchange format from signer to verifier. I see the followiging options:

1. The pair (data, raw_secp256k1_signature). This is how Ethereuum's web3.eth.sign works.
2. The pair (data, signature as StdSignature). Like 1. but is open for Non-Secp256k1 signature algorithms.
3. The whole StdTx structure as shown in this ADR with top level "type": "cosmos-sdk/StdTx", "value": ….
4. The content of StdTx's value field. This is currently used in my implementation.

Option 3 and 4 allow using multiple datas through multiple messages (see cosmos/cosmjs@06da6b8). I'm not saying this is useful but seems to work without violating the spec.

This spec is not updated, and needs to be updated, we've had a call weeks ago, and we decide to generalize the purpose of this ADR further.

In the sense that ADR-036 should not aim to provide a predefined Message such as MsgSignData, the only aim for this ADR is to provide a spec that can be understood and used by existing tooling (cosmjs, ledger etc), such that an offchain transaction (which is a composition of messages) looks exactly like a normal cosmos-sdk TX with the only property of being always invalid if run on a live chain.

This is the reference to the call we've had: #7896 (comment)

[assafmo]

For reference, this is what we ended up doing on Secret Network:

• Implementation: scrtlabs/cosmos-sdk@v0.9.1-scrt...v0.9.2-scrt
• Keplr & CLI usage examples: SNIP-24 - Query Permits scrtlabs/snip20-reference-impl#22