diff --git a/x/smartaccount/ante/auth.go b/x/smartaccount/ante/auth.go index c7899b22..ffa9b625 100644 --- a/x/smartaccount/ante/auth.go +++ b/x/smartaccount/ante/auth.go @@ -81,14 +81,16 @@ func (sad SmartAccountAuthDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simu return ctx, err } - // 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 } @@ -101,7 +103,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, @@ -113,7 +115,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)) } } @@ -132,9 +134,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() @@ -143,63 +145,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, @@ -218,9 +227,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. @@ -269,12 +284,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: @@ -335,3 +344,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 + } +} diff --git a/x/smartaccount/ante/tests/auth_test.go b/x/smartaccount/ante/tests/auth_test.go index 51b5c62b..56a0ffa8 100644 --- a/x/smartaccount/ante/tests/auth_test.go +++ b/x/smartaccount/ante/tests/auth_test.go @@ -95,6 +95,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] @@ -141,3 +214,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 +} diff --git a/x/smartaccount/test_helpers/test_data.go b/x/smartaccount/test_helpers/test_data.go index 8ef8b151..bee6e5e5 100644 --- a/x/smartaccount/test_helpers/test_data.go +++ b/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 diff --git a/x/smartaccount/test_helpers/test_data/smart_auth_multisig.wasm b/x/smartaccount/test_helpers/test_data/smart_auth_multisig.wasm new file mode 100644 index 00000000..e471f635 Binary files /dev/null and b/x/smartaccount/test_helpers/test_data/smart_auth_multisig.wasm differ