Skip to content

Commit

Permalink
docs(adr): ADR-005: Support multisig chain link (#634)
Browse files Browse the repository at this point in the history
## Description



This PR introduces the draft of the 6th ADR related to Support multisig chain link in #633.

---

### 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...

- [ ] 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
- [ ] 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
- [ ] reviewed "Files changed" and left comments if necessary
- [ ] 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)
  • Loading branch information
dadamu authored Dec 21, 2021
1 parent ec02782 commit a4de414
Showing 1 changed file with 143 additions and 0 deletions.
143 changes: 143 additions & 0 deletions docs/architecture/adr-005-support-multisig-chain-link.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,143 @@
# ADR 006: Support multisig chain link

## Changelog

- September 29th 2021: Initial draft
- October 21th, 2021: Proposed
- October 21th, 2021: First Review
- October 22th, 2021: Second Review
- November 15th, 2021: Third Review

## Status

PROPOSED

## Abstract

Currently, it is not possible to create a chain link using a multisig account. Since many validators MIGHT use multisig accounts, we SHOULD change the `Proof` type and its `Verify` function in order to support this kind of account as well.

## Context

Currently, the `x/profiles` module gives users the possibility to link their profile to different external accounts.
In particular, to link other blockchains accounts to a profile, we follow this process:
1. the user signs a message with their own private key;
2. the signature and the signed value are placed inside a `Proof` object;
3. Desmos verifies the `Proof` object to guarantee that the user really owns such account and thus it can be linked successfully to their Desmos profile.

Currently, this process works properly for single-signature accounts, but it does not support multi-signature accounts. This is due to the fact that the `Proof` type only supports single-account's signatures, and its `Verify` function is only able to verify such signature type.

## Decision

We propose to change the `Proof#Signature` field to be of type [`SignatureDescriptor_Data`](https://github.com/cosmos/cosmos-sdk/blob/master/proto/cosmos/tx/signing/v1beta1/signing.proto#L57).

### Proof implementation

`SignatureDescriptor_Data` supports storing both single and multi-sig signatures. Moreover, the Cosmos SDK provides a [function](https://github.com/cosmos/cosmos-sdk/blob/master/types/tx/signing/signature.go#L65) to convert such type into the corresponding interface ([`SignatureData`](https://github.com/cosmos/cosmos-sdk/blob/master/types/tx/signing/signature_data.go#L10)), which is required from the `Verify` function in order to validate such signature.

The verification process can be implemented as follows:
1. If it's a `SingleSignatureData`, make sure the account public key is a `cryptotypes.PubKey` and then use the `VerifySignature` method to verify the signature.
2. If it's a `MultiSignatureData`, make sure the account public key is a `multisig.PubKey` and then use the `VerifyMultisignature` method to verify the signature.

### CLI implementation

We propose to use an interactive prompt to create a new chain-link JSON for both single signature and multi signature account. For the single signature account, the process will remain the same as before. For the multi signature account, we will add the new feature beyond the current ones.

To simplify things for multisig accounts, we propose to create a chain link JSON starting from a transaction signed with a multisig account. Inside the Cosmos SDK, multisig account transactions signing depends on the `tx sign` and `tx multisign` commands. To send a transaction to the node, the following process takes place:
1. the multisig account owners create a raw transaction file;
2. the threshold number of signers sign it to using the `tx sign` command;
3. all individual signatures are gathered and used as input from the `tx multisign` to generate the final signed transaction.

The multisign file includes not only all public keys and threshold of the multisig account, but also the required number of signatures. Thanks to this, we can leverage all the information stored inside such file to create a proper chain link JSON starting from a multisigned transaction.

The whole process in code is presented below:
```go
// getChainLinkJSONFromMultiSign generates the chain-link JSON from the multisign file and its raw transaction file
func getChainLinkJSONFromMultiSign(
cmd *cobra.Command,
txFile string,
chainID string,
chain chainlinktypes.Chain,
) (profilescliutils.ChainLinkJSON, error) {
clientCtx, err := client.GetClientTxContext(cmd)
if err != nil {
return profilescliutils.ChainLinkJSON{}, err
}

parsedTx, err := authclient.ReadTxFromFile(clientCtx, txFile)
if err != nil {
return profilescliutils.ChainLinkJSON{}, err
}

txCfg := clientCtx.TxConfig
txBuilder, err := txCfg.WrapTxBuilder(parsedTx)
if err != nil {
return profilescliutils.ChainLinkJSON{}, err
}

txFactory := tx.NewFactoryCLI(clientCtx, cmd.Flags())
if txFactory.SignMode() == signingtypes.SignMode_SIGN_MODE_UNSPECIFIED {
txFactory = txFactory.WithSignMode(signingtypes.SignMode_SIGN_MODE_LEGACY_AMINO_JSON)
}

sigs, err := txBuilder.GetTx().GetSignaturesV2()
if len(sigs) != 1 {
return profilescliutils.ChainLinkJSON{}, fmt.Errorf("invalid number of signatures")
}
multisigSig := sigs[0]

signingData := authSigning.SignerData{
ChainID: chainID,
AccountNumber: txFactory.AccountNumber(),
Sequence: txFactory.Sequence(),
}
// the bytes of plain text
value, err := txCfg.SignModeHandler().GetSignBytes(txFactory.SignMode(), signingData, txBuilder.GetTx())
if err != nil {
return profilescliutils.ChainLinkJSON{}, err
}

addr, _ := sdk.Bech32ifyAddressBytes(chain.Prefix, multisigSig.PubKey.Address().Bytes())
return profilescliutils.NewChainLinkJSON(
profilestypes.NewBech32Address(addr, chain.Prefix),
profilestypes.NewProof(multisigSig.PubKey, signingtypes.SignatureDataToProto(multisigSig.Data), hex.EncodeToString(value)),
profilestypes.NewChainConfig(chain.Name),
), nil
}
```

## Consequences

### Backwards Compatibility

Since we change the `Proof#Signature` field from a hex-encoded string into a `SignatureDescriptor_Data`, this feature is not backwards compatible. For this reason, it will be needed to migrate the old chain link signatures to `SignatureDescriptor_Data`.

### Positive

- Give the possibility to link a multisig account to a Desmos profile

### Negative

- Raise the complexity to generate and verify the signature

### Neutral

(none known)

## Further Discussions

## Test Cases [optional]

The following tests cases MUST to be present:
- Verify `Proof` including wrong format signature returns error
- Verify `Proof` including non-matched single signature and pubkey returns error
- Verify `Proof` including non-matched multi signatures and pubkeys returns error
- Verify `Proof` including proper single signature and pubkey returns no error
- Verify `Proof` including proper multi signatures and pubkeys returns no error


## References

- Issue [#633](https://github.com/desmos-labs/desmos/issues/633)
- [SignatureData](https://github.com/cosmos/cosmos-sdk/blob/master/types/tx/signing/signature_data.go)
- [Signature](https://github.com/cosmos/cosmos-sdk/blob/master/types/tx/signing/signature.go)
- [Multisig](https://github.com/cosmos/cosmos-sdk/blob/master/crypto/keys/multisig/multisig.go)

0 comments on commit a4de414

Please sign in to comment.