Merge pull request #277 from terra-money/feat/smartaccounts/multisig

feat: handle multi signatures

x/smartaccount/ante/auth.go

@@ -82,14 +82,16 @@ func (sad SmartAccountAuthDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simu
 		return next(ctx, tx, simulate)
 	}
 
-	// Current smartaccount only supports one signer (TODO review in the future)
+	// Current smartaccount only supports one signer
+	// Multiple signers here means that the transaction perform state changes on multiple wallets
+	// (Not the same as a multi-sig transaction)
 	if len(signers) != 1 {
 		return ctx, sdkerrors.ErrorInvalidSigner.Wrap("only one account is supported (sigTx.GetSigners()!= 1)")
 	}
 
 	// Sender here is the account that signed the transaction
 	// Could be different from the account above (confusingly named signer)
-	senderAddr, signaturesBs, signedBytes, err := sad.GetParamsForCustomAuthVerification(ctx, sigTx, account)
+	senderAddrs, signaturesBs, signedBytes, err := sad.GetParamsForCustomAuthVerification(ctx, sigTx, account)
 	if err != nil {
 		return ctx, err
 	}
@@ -99,7 +101,7 @@ func (sad SmartAccountAuthDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simu
 		success, err := sad.CustomAuthVerify(
 			ctx,
 			setting.Authorization,
-			[]string{senderAddr.String()},
+			senderAddrs,
 			accountStr,
 			signaturesBs,
 			signedBytes,
@@ -111,7 +113,7 @@ func (sad SmartAccountAuthDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simu
 		if success {
 			return next(ctx, tx, simulate)
 		} else if !setting.Fallback {
-			return ctx, sdkerrors.ErrUnauthorized.Wrap("authorization failed")
+			return ctx, sdkerrors.ErrUnauthorized.Wrap(fmt.Sprintf("authorization failed: %s", err))
 		}
 	}
 
@@ -130,9 +132,9 @@ func (sad SmartAccountAuthDecorator) GetParamsForCustomAuthVerification(
 	sigTx authsigning.SigVerifiableTx,
 	account sdk.AccAddress,
 ) (
-	senderAddr sdk.AccAddress,
-	signatureBz [][]byte,
-	signedBytes [][]byte,
+	senderAddrs []string,
+	signatureBzs [][]byte,
+	signedBzs [][]byte,
 	err error,
 ) {
 	signatures, err := sigTx.GetSignaturesV2()
@@ -141,63 +143,70 @@ func (sad SmartAccountAuthDecorator) GetParamsForCustomAuthVerification(
 	}
 	if len(signatures) == 0 {
 		return nil, nil, nil, sdkerrors.ErrNoSignatures.Wrap("no signatures found")
-	} else if len(signatures) > 1 {
-		// TODO: remove when support multi sig auth
-		return nil, nil, nil, sdkerrors.ErrUnauthorized.Wrap("multiple signatures not supported")
 	}
 
-	signature := signatures[0]
-	signaturesBs := [][]byte{}
+	for _, signature := range signatures {
+		// This is to get the list of signers that contributed to the signatures
+		sas, err := GetSignerAddrStrings(signature.PubKey)
+		if err != nil {
+			return nil, nil, nil, err
+		}
+		senderAddrs = append(senderAddrs, sas...)
 
-	senderAddr, err = sdk.AccAddressFromHexUnsafe(signature.PubKey.Address().String())
-	if err != nil {
-		return nil, nil, nil, err
-	}
+		// This is to get the address of the signer (either a multisig or a single sig)
+		// For multisig, the address to generated from the json encoded list of signers
+		// See: https://github.com/cosmos/cosmos-sdk/blob/v0.47.10/crypto/keys/multisig/multisig.go
+		senderAddr, err := sdk.AccAddressFromHexUnsafe(signature.PubKey.Address().String())
+		if err != nil {
+			return nil, nil, nil, err
+		}
 
-	senderAcc, err := authante.GetSignerAcc(ctx, sad.accountKeeper, senderAddr)
-	if err != nil {
-		return nil, nil, nil, err
-	}
-	var senderAccNum uint64
-	if ctx.BlockHeight() != 0 {
-		senderAccNum = senderAcc.GetAccountNumber()
-	}
+		senderAcc, err := authante.GetSignerAcc(ctx, sad.accountKeeper, senderAddr)
+		if err != nil {
+			return nil, nil, nil, err
+		}
+		var senderAccNum uint64
+		if ctx.BlockHeight() != 0 {
+			senderAccNum = senderAcc.GetAccountNumber()
+		}
 
-	signerData := authsigning.SignerData{
-		Address:       senderAddr.String(),
-		ChainID:       ctx.ChainID(),
-		AccountNumber: senderAccNum,
-		Sequence:      senderAcc.GetSequence(),
-		PubKey:        signature.PubKey,
-	}
+		signerData := authsigning.SignerData{
+			Address:       senderAddr.String(),
+			ChainID:       ctx.ChainID(),
+			AccountNumber: senderAccNum,
+			Sequence:      senderAcc.GetSequence(),
+			PubKey:        signature.PubKey,
+		}
 
-	signatureBz, err = signatureDataToBz(signature.Data)
-	if err != nil {
-		return nil, nil, nil, err
-	}
-	signedBytes, err = GetSignBytesArr(signature.PubKey, signerData, signature.Data, sad.signModeHandler, sigTx)
-	if err != nil {
-		return nil, nil, nil, err
+		signatureBz, err := signatureDataToBz(signature.Data)
+		if err != nil {
+			return nil, nil, nil, err
+		}
+		signedBytes, err := GetSignBytesArr(signature.PubKey, signerData, signature.Data, sad.signModeHandler, sigTx)
+		signedBzs = append(signedBzs, signedBytes...)
+		if err != nil {
+			return nil, nil, nil, err
+		}
+		signatureBzs = append(signatureBzs, signatureBz...)
 	}
-	signaturesBs = append(signaturesBs, signatureBz...)
-	return senderAddr, signaturesBs, signedBytes, nil
+	return senderAddrs, signatureBzs, signedBzs, nil
 }
 
 func (sad SmartAccountAuthDecorator) CustomAuthVerify(
 	ctx sdk.Context,
 	authMsgs []*types.AuthorizationMsg,
-	sender []string,
+	senders []string,
 	account string,
 	signatures,
 	signedBytes [][]byte,
 	data []byte,
-) (success bool, err error) {
-	success = false
+) (bool, error) {
+	success := false
+	var errs []error
 	for _, auth := range authMsgs {
 		authMsg := types.Authorization{
-			Senders: sender,
-			Account: account,
-			// TODO: add in future when needed
+			Senders:     senders,
+			Account:     account,
 			Signatures:  signatures,
 			SignedBytes: signedBytes,
 			Data:        data,
@@ -216,9 +225,15 @@ func (sad SmartAccountAuthDecorator) CustomAuthVerify(
 		if err == nil {
 			success = true
 			break
+		} else {
+			errs = append(errs, err)
 		}
 	}
-	return success, nil
+	if success {
+		return success, nil
+	} else {
+		return success, fmt.Errorf("%v", errs)
+	}
 }
 
 // signatureDataToBz converts a SignatureData into raw bytes signature.
@@ -267,12 +282,6 @@ func GetSignBytesArr(pubKey cryptotypes.PubKey, signerData authsigning.SignerDat
 		if err != nil {
 			return nil, err
 		}
-		// TODO: should this be removed?
-		// this works right now because its secp256k1
-		// if verification is done only in wasm, then this probably would not work
-		// if !pubKey.VerifySignature(signBytes, data.Signature) {
-		// 	return nil, fmt.Errorf("unable to verify single signer signature")
-		// }
 		return [][]byte{signBytes}, nil
 
 	case *signing.MultiSignatureData:
@@ -333,3 +342,21 @@ func GetMultiSigSignBytes(multiPK multisig.PubKey, sig *signing.MultiSignatureDa
 	}
 	return signersBytes, nil
 }
+
+func GetSignerAddrStrings(pubKey cryptotypes.PubKey) ([]string, error) {
+	switch pubKey := pubKey.(type) {
+	case multisig.PubKey:
+		pubKeys := pubKey.GetPubKeys()
+		var addrStrings []string
+		for _, pk := range pubKeys {
+			as, err := GetSignerAddrStrings(pk)
+			if err != nil {
+				return nil, err
+			}
+			addrStrings = append(addrStrings, as...)
+		}
+		return addrStrings, nil
+	default:
+		return []string{pubKey.Address().String()}, nil
+	}
+}

x/smartaccount/ante/tests/auth_test.go

@@ -96,6 +96,79 @@ func (s *AnteTestSuite) TestAuthAnteHandler() {
 	require.NoError(s.T(), err)
 }
 
+func (s *AnteTestSuite) TestAuthAnteHandlerWithMultisig() {
+	s.Setup()
+
+	type MultiSigSetting struct {
+		Signers    []string `json:"signers"`
+		PublicKeys []string `json:"public_keys"`
+		Threshold  uint8    `json:"threshold"`
+	}
+
+	acc1 := s.TestAccs[1]
+	acc2 := s.TestAccs[2]
+	pubKey1 := s.TestAccPrivs[1].PubKey()
+	pubKey2 := s.TestAccPrivs[2].PubKey()
+	pkEncoded1 := []byte(base64.StdEncoding.EncodeToString(pubKey1.Bytes()))
+	pkEncoded2 := []byte(base64.StdEncoding.EncodeToString(pubKey2.Bytes()))
+
+	codeId, _, err := s.WasmKeeper.Create(s.Ctx, acc1, test_helpers.SmartMultiSigWasm, nil)
+	require.NoError(s.T(), err)
+	contractAddr, _, err := s.WasmKeeper.Instantiate(s.Ctx, codeId, acc1, acc1, []byte("{}"), "auth", sdk.NewCoins())
+	require.NoError(s.T(), err)
+
+	setting := MultiSigSetting{
+		Signers:    []string{acc1.String(), acc2.String()},
+		PublicKeys: []string{string(pkEncoded1), string(pkEncoded2)},
+		Threshold:  2,
+	}
+	settingBs, err := json.Marshal(setting)
+	require.NoError(s.T(), err)
+
+	// create initMsg
+	initMsg := smartaccounttypes.Initialization{
+		Account: acc1.String(),
+		Msg:     settingBs,
+	}
+	sudoInitMsg := smartaccounttypes.SudoMsg{Initialization: &initMsg}
+	sudoInitMsgBs, err := json.Marshal(sudoInitMsg)
+	require.NoError(s.T(), err)
+
+	_, err = s.WasmKeeper.Sudo(s.Ctx, contractAddr, sudoInitMsgBs)
+	require.NoError(s.T(), err)
+
+	// set settings
+	authMsg := &smartaccounttypes.AuthorizationMsg{
+		ContractAddress: contractAddr.String(),
+		InitMsg:         sudoInitMsg.Initialization,
+	}
+	err = s.SmartAccountKeeper.SetSetting(s.Ctx, smartaccounttypes.Setting{
+		Owner:         acc1.String(),
+		Fallback:      false,
+		Authorization: []*smartaccounttypes.AuthorizationMsg{authMsg},
+	})
+	require.NoError(s.T(), err)
+
+	// signing with multisig should pass
+	txBuilder := s.BuildDefaultMultiSigMsgTx([]int{1, 2}, &types.MsgSend{
+		FromAddress: acc1.String(),
+		ToAddress:   acc1.String(),
+		Amount:      sdk.NewCoins(sdk.NewInt64Coin("uluna", 1)),
+	})
+	_, err = s.AuthDecorator.AnteHandle(s.Ctx, txBuilder.GetTx(), false, sdk.ChainAnteDecorators(sdk.Terminator{}))
+	require.NoError(s.T(), err)
+
+	// signing with multisig with only 1 signer should fail
+	txBuilder = s.BuildDefaultMultiSigMsgTx([]int{1}, &types.MsgSend{
+		FromAddress: acc1.String(),
+		ToAddress:   acc1.String(),
+		Amount:      sdk.NewCoins(sdk.NewInt64Coin("uluna", 1)),
+	})
+	_, err = s.AuthDecorator.AnteHandle(s.Ctx, txBuilder.GetTx(), false, sdk.ChainAnteDecorators(sdk.Terminator{}))
+	require.Error(s.T(), err)
+	require.ErrorContainsf(s.T(), err, "required: 2, found: 1", "")
+}
+
 func (s *AnteTestSuite) BuildDefaultMsgTx(accountIndex int, msgs ...sdk.Msg) client.TxBuilder {
 	pk := s.TestAccPrivs[accountIndex]
 	sender := s.TestAccs[accountIndex]
@@ -142,3 +215,74 @@ func (s *AnteTestSuite) BuildDefaultMsgTx(accountIndex int, msgs ...sdk.Msg) cli
 
 	return txBuilder
 }
+
+func (s *AnteTestSuite) BuildDefaultMultiSigMsgTx(accountIndices []int, msgs ...sdk.Msg) client.TxBuilder {
+
+	txBuilder := s.EncodingConfig.TxConfig.NewTxBuilder()
+	err := txBuilder.SetMsgs(
+		msgs...,
+	)
+	require.NoError(s.T(), err)
+
+	var emptySigs []signing.SignatureV2
+
+	for _, accountIndex := range accountIndices {
+		sender := s.TestAccs[accountIndex]
+		senderAcc := s.App.Keepers.AccountKeeper.GetAccount(s.Ctx, sender)
+		senderSeq := senderAcc.GetSequence()
+		pk := s.TestAccPrivs[accountIndex]
+
+		signer := authsigning.SignerData{
+			Address:       sender.String(),
+			ChainID:       "test",
+			AccountNumber: senderAcc.GetAccountNumber(),
+			Sequence:      senderSeq,
+			PubKey:        pk.PubKey(),
+		}
+
+		emptySig := signing.SignatureV2{
+			PubKey: signer.PubKey,
+			Data: &signing.SingleSignatureData{
+				SignMode:  s.EncodingConfig.TxConfig.SignModeHandler().DefaultMode(),
+				Signature: nil,
+			},
+			Sequence: signer.Sequence,
+		}
+
+		emptySigs = append(emptySigs, emptySig)
+	}
+	err = txBuilder.SetSignatures(emptySigs...)
+
+	require.NoError(s.T(), err)
+
+	var signed []signing.SignatureV2
+	for _, accountIndex := range accountIndices {
+		sender := s.TestAccs[accountIndex]
+		senderAcc := s.App.Keepers.AccountKeeper.GetAccount(s.Ctx, sender)
+		senderSeq := senderAcc.GetSequence()
+		pk := s.TestAccPrivs[accountIndex]
+
+		signer := authsigning.SignerData{
+			Address:       sender.String(),
+			ChainID:       "test",
+			AccountNumber: senderAcc.GetAccountNumber(),
+			Sequence:      senderSeq,
+			PubKey:        pk.PubKey(),
+		}
+		sigV2, err := tx.SignWithPrivKey(
+			s.EncodingConfig.TxConfig.SignModeHandler().DefaultMode(),
+			signer,
+			txBuilder,
+			pk,
+			s.EncodingConfig.TxConfig,
+			senderSeq,
+		)
+		require.NoError(s.T(), err)
+		signed = append(signed, sigV2)
+	}
+
+	err = txBuilder.SetSignatures(signed...)
+	require.NoError(s.T(), err)
+
+	return txBuilder
+}

x/smartaccount/test_helpers/test_data.go

@@ -7,3 +7,6 @@ var LimitSendOnlyHookWasm []byte
 
 //go:embed test_data/smart_auth_contract.wasm
 var SmartAuthContractWasm []byte
+
+//go:embed test_data/smart_auth_multisig.wasm
+var SmartMultiSigWasm []byte