Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support more signature algorithm and specifications inside chain links proofs #871

Closed
RiccardoM opened this issue May 19, 2022 · 0 comments · Fixed by #968
Closed

Support more signature algorithm and specifications inside chain links proofs #871

RiccardoM opened this issue May 19, 2022 · 0 comments · Fixed by #968
Assignees
Labels
kind/new-feature Propose the addition of a new feature that does not yet exist

Comments

@RiccardoM
Copy link
Contributor

Feature description

Currently Desmos only supports a couple of signature specifications when creating a chain link: Cosmos single signatures and Cosmos multi signature. Although this allows to connect any Cosmos-based chain, it might happen that for some chains it's not possible to connect an address using a third party wallet because such wallet only supports custom signature specifications. An example of this can be MetaMask, which supports the personal_sign specification that generated the signature as follows:

sign(keccak256("\x19Ethereum Signed Message:\n" + len(message) + message)))

Implementation proposal

In order to support more signature specification, we need to extend the current set of supported signatures. The ones we should look at first are the ones that are supported by the most used third party wallets of the ecosystems that we want to support (eg. MetaMask for Ethereum, Phantom/Sollet for Solana)

@RiccardoM RiccardoM added the kind/new-feature Propose the addition of a new feature that does not yet exist label May 19, 2022
@RiccardoM RiccardoM self-assigned this May 19, 2022
mergify bot pushed a commit that referenced this issue Jul 14, 2022
## Description

This ADR contains the specification for new signature type support that we should add when creating chain links in order to allow such creation though commonly used third party wallets (eg. MetaMask) as described inside #871.



---

### Author Checklist

*All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.*

I have...

- [x] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] added `!` to the type prefix if API or client breaking change
- [x] targeted the correct branch (see [PR Targeting](https://github.com/desmos-labs/desmos/blob/master/CONTRIBUTING.md#pr-targeting))
- [ ] provided a link to the relevant issue or specification
- [ ] followed the guidelines for [building modules](https://docs.cosmos.network/v0.44/building-modules/intro.html)
- [ ] included the necessary unit and integration [tests](https://github.com/desmos-labs/desmos/blob/master/CONTRIBUTING.md#testing)
- [ ] added a changelog entry to `CHANGELOG.md`
- [ ] included comments for [documenting Go code](https://blog.golang.org/godoc)
- [ ] updated the relevant documentation or specification
- [x] reviewed "Files changed" and left comments if necessary
- [x] confirmed all CI checks have passed

### Reviewers Checklist

*All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.*

I have...

- [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] confirmed `!` in the type prefix if API or client breaking change
- [ ] confirmed all author checklist items have been addressed
- [ ] reviewed state machine logic
- [ ] reviewed API design and naming
- [ ] reviewed documentation is accurate
- [ ] reviewed tests and test coverage
- [ ] manually tested (if applicable)
@mergify mergify bot closed this as completed in #968 Jul 21, 2022
mergify bot pushed a commit that referenced this issue Jul 21, 2022
## Description
This PR adds support for different signature algorithms (namely EVM-compatible `personal_sign`) when creating a chain link, as per [ADR-015](https://github.com/desmos-labs/desmos/blob/master/docs/architecture/adr-015-improve-chain-link-signature-support.md). 

`personal_sign` has been chosen to be the first EVM-compatible signature algorithm supported due to the fact that most wallets support this and it allows to specify an arbitrary message to be signed. 

Closes: #871



---

### Author Checklist

*All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.*

I have...

- [x] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [x] added `!` to the type prefix if API or client breaking change
- [x] targeted the correct branch (see [PR Targeting](https://github.com/desmos-labs/desmos/blob/master/CONTRIBUTING.md#pr-targeting))
- [x] provided a link to the relevant issue or specification
- [ ] followed the guidelines for [building modules](https://docs.cosmos.network/v0.44/building-modules/intro.html)
- [x] included the necessary unit and integration [tests](https://github.com/desmos-labs/desmos/blob/master/CONTRIBUTING.md#testing)
- [x] added a changelog entry to `CHANGELOG.md`
- [x] included comments for [documenting Go code](https://blog.golang.org/godoc)
- [x] updated the relevant documentation or specification
- [x] reviewed "Files changed" and left comments if necessary
- [x] confirmed all CI checks have passed

### Reviewers Checklist

*All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.*

I have...

- [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] confirmed `!` in the type prefix if API or client breaking change
- [ ] confirmed all author checklist items have been addressed
- [ ] reviewed state machine logic
- [ ] reviewed API design and naming
- [ ] reviewed documentation is accurate
- [ ] reviewed tests and test coverage
- [ ] manually tested (if applicable)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/new-feature Propose the addition of a new feature that does not yet exist
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant