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

[nice to have] multisig preventing signing with wrong key #286

Merged
merged 5 commits into from
Jun 14, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
43 changes: 37 additions & 6 deletions x/auth/client/cli/tx_sign.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,14 @@
"fmt"
"os"

"github.com/cosmos/cosmos-sdk/crypto/keyring"
"github.com/spf13/cobra"

"github.com/cosmos/cosmos-sdk/client"
"github.com/cosmos/cosmos-sdk/client/flags"
"github.com/cosmos/cosmos-sdk/client/tx"
sdk "github.com/cosmos/cosmos-sdk/types"
kmultisig "github.com/cosmos/cosmos-sdk/crypto/keys/multisig"
"github.com/cosmos/cosmos-sdk/types/errors"
authclient "github.com/cosmos/cosmos-sdk/x/auth/client"
)

Expand Down Expand Up @@ -215,7 +217,7 @@
return err
}

txFactory := tx.NewFactoryCLI(clientCtx, cmd.Flags())

Check failure on line 220 in x/auth/client/cli/tx_sign.go

View workflow job for this annotation

GitHub Actions / tests (07)

txFactory declared but not used

Check failure on line 220 in x/auth/client/cli/tx_sign.go

View workflow job for this annotation

GitHub Actions / tests (05)

txFactory declared but not used

Check failure on line 220 in x/auth/client/cli/tx_sign.go

View workflow job for this annotation

GitHub Actions / tests (05)

txFactory declared but not used

Check failure on line 220 in x/auth/client/cli/tx_sign.go

View workflow job for this annotation

GitHub Actions / tests (06)

txFactory declared but not used

Check failure on line 220 in x/auth/client/cli/tx_sign.go

View workflow job for this annotation

GitHub Actions / tests (06)

txFactory declared but not used

Check failure on line 220 in x/auth/client/cli/tx_sign.go

View workflow job for this annotation

GitHub Actions / tests (01)

txFactory declared but not used

Check failure on line 220 in x/auth/client/cli/tx_sign.go

View workflow job for this annotation

GitHub Actions / tests (09)

txFactory declared but not used

Check failure on line 220 in x/auth/client/cli/tx_sign.go

View workflow job for this annotation

GitHub Actions / tests (09)

txFactory declared but not used

Check failure on line 220 in x/auth/client/cli/tx_sign.go

View workflow job for this annotation

GitHub Actions / tests (08)

txFactory declared but not used

Check failure on line 220 in x/auth/client/cli/tx_sign.go

View workflow job for this annotation

GitHub Actions / tests (08)

txFactory declared but not used

Check failure on line 220 in x/auth/client/cli/tx_sign.go

View workflow job for this annotation

GitHub Actions / tests (08)

txFactory declared but not used

Check failure on line 220 in x/auth/client/cli/tx_sign.go

View workflow job for this annotation

GitHub Actions / tests (03)

txFactory declared but not used

Check failure on line 220 in x/auth/client/cli/tx_sign.go

View workflow job for this annotation

GitHub Actions / tests (03)

txFactory declared but not used

Check failure on line 220 in x/auth/client/cli/tx_sign.go

View workflow job for this annotation

GitHub Actions / tests (03)

txFactory declared but not used

Check failure on line 220 in x/auth/client/cli/tx_sign.go

View workflow job for this annotation

GitHub Actions / tests (03)

txFactory declared but not used

Check failure on line 220 in x/auth/client/cli/tx_sign.go

View workflow job for this annotation

GitHub Actions / tests (00)

txFactory declared but not used

Check failure on line 220 in x/auth/client/cli/tx_sign.go

View workflow job for this annotation

GitHub Actions / tests (00)

txFactory declared but not used

Check failure on line 220 in x/auth/client/cli/tx_sign.go

View workflow job for this annotation

GitHub Actions / tests (00)

txFactory declared but not used

Check failure on line 220 in x/auth/client/cli/tx_sign.go

View workflow job for this annotation

GitHub Actions / tests (00)

txFactory declared but not used

Check failure on line 220 in x/auth/client/cli/tx_sign.go

View workflow job for this annotation

GitHub Actions / tests (00)

txFactory declared but not used

Check failure on line 220 in x/auth/client/cli/tx_sign.go

View workflow job for this annotation

GitHub Actions / tests (00)

txFactory declared but not used

Check failure on line 220 in x/auth/client/cli/tx_sign.go

View workflow job for this annotation

GitHub Actions / tests (00)

txFactory declared but not used

Check failure on line 220 in x/auth/client/cli/tx_sign.go

View workflow job for this annotation

GitHub Actions / tests (00)

txFactory declared but not used

Check failure on line 220 in x/auth/client/cli/tx_sign.go

View workflow job for this annotation

GitHub Actions / tests (00)

txFactory declared but not used

Check failure on line 220 in x/auth/client/cli/tx_sign.go

View workflow job for this annotation

GitHub Actions / tests (02)

txFactory declared but not used

Check failure on line 220 in x/auth/client/cli/tx_sign.go

View workflow job for this annotation

GitHub Actions / tests (02)

txFactory declared but not used

Check failure on line 220 in x/auth/client/cli/tx_sign.go

View workflow job for this annotation

GitHub Actions / tests (04)

txFactory declared but not used

Check failure on line 220 in x/auth/client/cli/tx_sign.go

View workflow job for this annotation

GitHub Actions / tests (04)

txFactory declared but not used

Check failure on line 220 in x/auth/client/cli/tx_sign.go

View workflow job for this annotation

GitHub Actions / tests (04)

txFactory declared but not used
txCfg := clientCtx.TxConfig
txBuilder, err := txCfg.WrapTxBuilder(newTx)
if err != nil {
Expand All @@ -235,14 +237,33 @@

overwrite, _ := f.GetBool(flagOverwrite)
if multisig != "" {
multisigAddr, err := sdk.AccAddressFromBech32(multisig)
// Bech32 decode error, maybe it's a name, we try to fetch from keyring
multisigAddr, multisigName, _, err := client.GetFromFields(txF.Keybase(), multisig, clientCtx.GenerateOnly)
if err != nil {
// Bech32 decode error, maybe it's a name, we try to fetch from keyring
multisigAddr, _, _, err = client.GetFromFields(txFactory.Keybase(), multisig, clientCtx.GenerateOnly)
if err != nil {
return fmt.Errorf("error getting account from keybase: %w", err)
return fmt.Errorf("error getting account from keybase: %w", err)
}
multisigkey, err := getMultisigRecord(clientCtx, multisigName)
if err != nil {
return err
}
multisigPubKey := multisigkey.GetPubKey()
multisigLegacyPub := multisigPubKey.(*kmultisig.LegacyAminoPubKey)

fromRecord, err := clientCtx.Keyring.Key(fromName)
if err != nil {
return fmt.Errorf("error getting account from keybase: %w", err)
}
fromPubKey := fromRecord.GetPubKey()

var found bool
for _, pubkey := range multisigLegacyPub.GetPubKeys() {
if pubkey.Equals(fromPubKey) {
found = true
}
}
if !found {
return fmt.Errorf("signing key is not a part of multisig key")
}
err = authclient.SignTxWithSignerAddress(
txF, clientCtx, multisigAddr, fromName, txBuilder, clientCtx.Offline, overwrite)
if err != nil {
Expand Down Expand Up @@ -316,3 +337,13 @@

return txConfig.TxJSONEncoder()(parsedTx)
}

func getMultisigRecord(clientCtx client.Context, name string) (keyring.Info, error) {
kb := clientCtx.Keyring
multisigRecord, err := kb.Key(name)
if err != nil {
return nil, errors.Wrap(err, "error getting keybase multisig account")
}

return multisigRecord, nil
}
17 changes: 16 additions & 1 deletion x/auth/client/testutil/suite.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,10 @@ func (s *IntegrationTestSuite) SetupSuite() {
account2, _, err := kb.NewMnemonic("newAccount2", keyring.English, sdk.FullFundraiserPath, keyring.DefaultBIP39Passphrase, hd.Secp256k1)
s.Require().NoError(err)

// Create a dummy account for testing purpose
_, _, err = kb.NewMnemonic("dummyAccount", keyring.English, sdk.FullFundraiserPath, keyring.DefaultBIP39Passphrase, hd.Secp256k1)
s.Require().NoError(err)

multi := kmultisig.NewLegacyAminoPubKey(2, []cryptotypes.PubKey{account1.GetPubKey(), account2.GetPubKey()})
_, err = kb.SaveMultisig("multi", multi)
s.Require().NoError(err)
Expand Down Expand Up @@ -737,6 +741,10 @@ func (s *IntegrationTestSuite) TestCLIMultisignSortSignatures() {
multisigInfo, err := val1.ClientCtx.Keyring.Key("multi")
s.Require().NoError(err)

// Generate dummy account which is not a part of multisig.
dummyAcc, err := val1.ClientCtx.Keyring.Key("dummyAccount")
s.Require().NoError(err)

resp, err := bankcli.QueryBalancesExec(val1.ClientCtx, multisigInfo.GetAddress())
s.Require().NoError(err)

Expand Down Expand Up @@ -795,6 +803,13 @@ func (s *IntegrationTestSuite) TestCLIMultisignSortSignatures() {

sign2File := testutil.WriteToNewTempFile(s.T(), account2Signature.String())

// Sign with dummy account
dummyAddr := dummyAcc.GetAddress()
s.Require().NoError(err)
_, err = TxSignExec(val1.ClientCtx, dummyAddr, multiGeneratedTxFile.Name(), "--multisig", addr.String())
s.Require().Error(err)
s.Require().Contains(err.Error(), "signing key is not a part of multisig key")

multiSigWith2Signatures, err := TxMultiSignExec(val1.ClientCtx, multisigInfo.GetName(), multiGeneratedTxFile.Name(), sign1File.Name(), sign2File.Name())
s.Require().NoError(err)

Expand Down Expand Up @@ -849,7 +864,7 @@ func (s *IntegrationTestSuite) TestSignWithMultisig() {
// as the main point of this test is to test the `--multisig` flag with an address
// that is not in the keyring.
_, err = TxSignExec(val1.ClientCtx, addr1, multiGeneratedTx2File.Name(), "--multisig", multisigAddr.String())
s.Require().Contains(err.Error(), "tx intended signer does not match the given signer")
s.Require().Contains(err.Error(), "error getting account from keybase")
}

func (s *IntegrationTestSuite) TestCLIMultisign() {
Expand Down