-
Notifications
You must be signed in to change notification settings - Fork 48
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
feat: ADR-005: Support multisig chain link #708
Merged
Merged
Changes from all commits
Commits
Show all changes
46 commits
Select commit
Hold shift + click to select a range
b476e11
Apply SignatureData in Proof
dadamu 11e307d
Fix types tests
dadamu 45b6567
Add signature testutil
dadamu b6df42d
changed how signature data should be provided inside the proof
RiccardoM 2fc266d
Register interface
dadamu cdeb8a5
Fix panic in test
dadamu bef25df
Remove signature nil checking since it causes panic
dadamu 73e6016
Add back gogoproto.equal
dadamu 73430a8
Update profiles types unit tests on multisig chainlink
dadamu e05f47a
Update keeper unit tests
dadamu 4772a51
Update cli unit tests
dadamu 6b35fe3
Update cli and v210 migration test
dadamu a9de5f0
Fix proof verify unpack issue
dadamu 3a84c6c
Fix lint
dadamu 7752995
Add invalid signature test for Proof
dadamu 144fac9
Move SignatureDataFromCosmosSignatureData to types
dadamu c82287e
Build multisig chain link json cli
dadamu 1bc9de5
Add unit test for chain link from multisign cli
dadamu 9c6981e
Remove unused config
dadamu 7d1bf62
Fix lint
dadamu 61753db
Fix SignatureDataFromCosmosSignatureData to handle error properly
dadamu 37cb980
Update create chain link json
dadamu 1e919a3
Update app/desmos/cmd/chainlink/create_json.go
dadamu d5c3dc9
Update app/desmos/cmd/chainlink/types/getter.go
dadamu 25eeaf2
Apply suggestions create chain link json cmd
dadamu 58675f3
Add doc for testutil
dadamu 69716b5
Merge branch 'paul/adr-005-multisig-chainlink-impl' of github.com:des…
dadamu 86b8b2c
Merge branch 'master' of github.com:desmos-labs/desmos into paul/adr-…
dadamu b04b997
Fix doc in the getter
dadamu fc4dd7d
Update changeset
dadamu b017cca
Revert generateChainLinkJSON
dadamu bd5bd67
Update app/desmos/cmd/chainlink/create_json.go
dadamu 2f0dae5
Fix the comment in cli test
dadamu 65417f4
Merge branch 'paul/adr-005-multisig-chainlink-impl' of github.com:des…
dadamu a364e24
Add test to check if signature is nil
dadamu 31854ac
Remove unused function
dadamu 60deaa2
Add more tests for multisig proof
dadamu 41c7496
improved the overall code organization
RiccardoM 40637f3
Merge branch 'paul/adr-005-multisig-chainlink-impl' of github.com:des…
RiccardoM 1769abb
improved the overall code organization
RiccardoM f198b21
Fix sign mode and test builder
dadamu ef18023
Rerange the files
dadamu 3f274d9
Update .changeset/entries/a492eff6c4ec45c98cb555b14de9f27ba50da5b147f…
dadamu 04b9181
small doc fixes
RiccardoM 9eb1093
Merge remote-tracking branch 'origin/paul/adr-005-multisig-chainlink-…
RiccardoM 78be05b
Merge branch 'master' into paul/adr-005-multisig-chainlink-impl
mergify[bot] File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
6 changes: 6 additions & 0 deletions
6
.changeset/entries/a492eff6c4ec45c98cb555b14de9f27ba50da5b147ff05b7a4cb1707af73655c.yaml
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
type: feat | ||
module: x/profiles | ||
pull_request: 708 | ||
description: Added support for multisig chain links | ||
backward_compatible: false | ||
date: 2022-01-12T03:49:42.2638125Z |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
package builder | ||
|
||
import ( | ||
"github.com/desmos-labs/desmos/v2/app/desmos/cmd/chainlink/types" | ||
"github.com/desmos-labs/desmos/v2/x/profiles/client/utils" | ||
) | ||
|
||
// ChainLinkJSONBuilder allows to build a ChainLinkJSON instance | ||
type ChainLinkJSONBuilder interface { | ||
BuildChainLinkJSON(chain types.Chain) (utils.ChainLinkJSON, error) | ||
} | ||
|
||
// ChainLinkJSONBuilderProvider allows to provide the provider ChainLinkJSONBuilder implementation based on whether | ||
// it should create the JSON chain link for single or | ||
type ChainLinkJSONBuilderProvider func(isSingleAccount bool) ChainLinkJSONBuilder |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,99 @@ | ||
package multi | ||
|
||
import ( | ||
"encoding/hex" | ||
"fmt" | ||
"io/ioutil" | ||
|
||
sdk "github.com/cosmos/cosmos-sdk/types" | ||
"github.com/cosmos/cosmos-sdk/types/tx/signing" | ||
authsigning "github.com/cosmos/cosmos-sdk/x/auth/signing" | ||
|
||
"github.com/desmos-labs/desmos/v2/app" | ||
"github.com/desmos-labs/desmos/v2/app/desmos/cmd/chainlink/getter" | ||
"github.com/desmos-labs/desmos/v2/app/desmos/cmd/chainlink/types" | ||
"github.com/desmos-labs/desmos/v2/x/profiles/client/utils" | ||
profilestypes "github.com/desmos-labs/desmos/v2/x/profiles/types" | ||
) | ||
|
||
// AccountChainLinkJSONBuilder implements the ChainLinkJSONBuilder for multi signature accounts | ||
type AccountChainLinkJSONBuilder struct { | ||
getter getter.MultiSignatureAccountReferenceGetter | ||
} | ||
|
||
// NewAccountChainLinkJSONBuilder returns a new AccountChainLinkJSONBuilder instance | ||
func NewAccountChainLinkJSONBuilder(getter getter.MultiSignatureAccountReferenceGetter) *AccountChainLinkJSONBuilder { | ||
return &AccountChainLinkJSONBuilder{ | ||
getter: getter, | ||
} | ||
} | ||
|
||
// BuildChainLinkJSON implements ChainLinkJSONBuilder | ||
func (b *AccountChainLinkJSONBuilder) BuildChainLinkJSON(chain types.Chain) (utils.ChainLinkJSON, error) { | ||
txFilePath, err := b.getter.GetMultiSignedTxFilePath() | ||
if err != nil { | ||
return utils.ChainLinkJSON{}, err | ||
} | ||
|
||
signedChainID, err := b.getter.GetSignedChainID() | ||
if err != nil { | ||
return utils.ChainLinkJSON{}, err | ||
} | ||
|
||
encodingConfig := app.MakeTestEncodingConfig() | ||
txCfg := encodingConfig.TxConfig | ||
|
||
// Read the transaction file | ||
bytes, err := ioutil.ReadFile(txFilePath) | ||
if err != nil { | ||
return utils.ChainLinkJSON{}, err | ||
} | ||
|
||
// Parse the transaction | ||
parsedTx, err := txCfg.TxJSONDecoder()(bytes) | ||
if err != nil { | ||
return utils.ChainLinkJSON{}, err | ||
} | ||
|
||
// Get the sign mode | ||
signMode := signing.SignMode_SIGN_MODE_LEGACY_AMINO_JSON | ||
|
||
// Wrap the transaction inside a builder to make it easier to get the signatures | ||
txBuilder, err := txCfg.WrapTxBuilder(parsedTx) | ||
if err != nil { | ||
return utils.ChainLinkJSON{}, err | ||
} | ||
|
||
sigs, err := txBuilder.GetTx().GetSignaturesV2() | ||
if err != nil { | ||
return utils.ChainLinkJSON{}, err | ||
} | ||
|
||
// Make sure there is only one signature for the multisig account | ||
if len(sigs) != 1 { | ||
return utils.ChainLinkJSON{}, fmt.Errorf("invalid number of signatures") | ||
} | ||
|
||
// Re-create the bytes that have been signed in order to produce the signature | ||
signingData := authsigning.SignerData{AccountNumber: 0, Sequence: 0, ChainID: signedChainID} | ||
value, err := txCfg.SignModeHandler().GetSignBytes(signMode, signingData, parsedTx) | ||
if err != nil { | ||
return utils.ChainLinkJSON{}, err | ||
} | ||
|
||
addr, err := sdk.Bech32ifyAddressBytes(chain.Prefix, sigs[0].PubKey.Address().Bytes()) | ||
if err != nil { | ||
return utils.ChainLinkJSON{}, err | ||
} | ||
|
||
sigData, err := profilestypes.SignatureDataFromCosmosSignatureData(sigs[0].Data) | ||
if err != nil { | ||
return utils.ChainLinkJSON{}, err | ||
} | ||
|
||
return utils.NewChainLinkJSON( | ||
profilestypes.NewBech32Address(addr, chain.Prefix), | ||
profilestypes.NewProof(sigs[0].PubKey, sigData, hex.EncodeToString(value)), | ||
profilestypes.NewChainConfig(chain.Name), | ||
), err | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,65 @@ | ||
package single | ||
|
||
import ( | ||
"encoding/hex" | ||
|
||
"github.com/cosmos/cosmos-sdk/crypto/hd" | ||
"github.com/cosmos/cosmos-sdk/crypto/keyring" | ||
sdk "github.com/cosmos/cosmos-sdk/types" | ||
"github.com/cosmos/cosmos-sdk/types/tx/signing" | ||
|
||
"github.com/desmos-labs/desmos/v2/app/desmos/cmd/chainlink/getter" | ||
"github.com/desmos-labs/desmos/v2/app/desmos/cmd/chainlink/types" | ||
"github.com/desmos-labs/desmos/v2/x/profiles/client/utils" | ||
profilestypes "github.com/desmos-labs/desmos/v2/x/profiles/types" | ||
) | ||
|
||
const ( | ||
KeyName = "desmos_chain_link_account" | ||
) | ||
|
||
// AccountChainLinkJSONBuilder implements the ChainLinkJSONBuilder for single signature accounts | ||
type AccountChainLinkJSONBuilder struct { | ||
getter getter.SingleSignatureAccountReferenceGetter | ||
} | ||
|
||
// NewAccountChainLinkJSONBuilder returns a new AccountChainLinkJSONBuilder instance | ||
func NewAccountChainLinkJSONBuilder(getter getter.SingleSignatureAccountReferenceGetter) *AccountChainLinkJSONBuilder { | ||
return &AccountChainLinkJSONBuilder{ | ||
getter: getter, | ||
} | ||
} | ||
|
||
// BuildChainLinkJSON implements ChainLinkJSONBuilder | ||
func (b *AccountChainLinkJSONBuilder) BuildChainLinkJSON(chain types.Chain) (utils.ChainLinkJSON, error) { | ||
mnemonic, err := b.getter.GetMnemonic() | ||
if err != nil { | ||
return utils.ChainLinkJSON{}, err | ||
} | ||
|
||
// Create an in-memory keybase for signing | ||
keyBase := keyring.NewInMemory() | ||
_, err = keyBase.NewAccount(KeyName, mnemonic, "", chain.DerivationPath, hd.Secp256k1) | ||
if err != nil { | ||
return utils.ChainLinkJSON{}, err | ||
} | ||
|
||
// Generate the proof signing it with the key | ||
key, _ := keyBase.Key(KeyName) | ||
addr, _ := sdk.Bech32ifyAddressBytes(chain.Prefix, key.GetAddress()) | ||
value := []byte(addr) | ||
sig, pubkey, err := keyBase.Sign(KeyName, value) | ||
if err != nil { | ||
return utils.ChainLinkJSON{}, err | ||
} | ||
sigData := &profilestypes.SingleSignatureData{ | ||
Mode: signing.SignMode_SIGN_MODE_DIRECT, | ||
Signature: sig, | ||
} | ||
|
||
return utils.NewChainLinkJSON( | ||
profilestypes.NewBech32Address(addr, chain.Prefix), | ||
profilestypes.NewProof(pubkey, sigData, hex.EncodeToString(value)), | ||
profilestypes.NewChainConfig(chain.Name), | ||
), nil | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One thing I don't understand if it's correct or not is this line: the transaction could be signed using
SIGN_MODE_DIRECT
as well from what I know. Isn't this valid for multi-sig transactions as well? Or are they all signed using theSIGN_MODE_LEGACY_AMINO_JSON
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They are all signed in
SIGN_MODE_LEGACY_AMINO_JSON
if it is the multi-sig tx as well as the ledger tx.Reference
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dadamu Perfect, thanks for the reference link as well! 🙏