From dbc7d9a56ecb3aa403390b7e5fcf8f2fc0945e38 Mon Sep 17 00:00:00 2001 From: Austin Chandra Date: Mon, 21 Nov 2022 12:35:58 -0800 Subject: [PATCH 1/7] Add EIP-712 encoding for multiple messages of the same type --- ethereum/eip712/eip712_test.go | 34 ++++++- ethereum/eip712/encoding.go | 174 +++++++++++++++++++++++---------- 2 files changed, 150 insertions(+), 58 deletions(-) diff --git a/ethereum/eip712/eip712_test.go b/ethereum/eip712/eip712_test.go index d2e933af4f..4b4cdee7e3 100644 --- a/ethereum/eip712/eip712_test.go +++ b/ethereum/eip712/eip712_test.go @@ -93,6 +93,9 @@ func (suite *EIP712TestSuite) TestEIP712SignatureVerification() { signing.SignMode_SIGN_MODE_LEGACY_AMINO_JSON, } + // Fixed test address + testAddress := suite.createTestAddress() + testCases := []struct { title string chainId string @@ -176,7 +179,30 @@ func (suite *EIP712TestSuite) TestEIP712SignatureVerification() { expectSuccess: true, }, { - title: "Fails - Two MsgVotes", + title: "Succeeds - Two Single-Signer MsgDelegate", + fee: txtypes.Fee{ + Amount: suite.makeCoins("aphoton", math.NewInt(2000)), + GasLimit: 20000, + }, + memo: "", + msgs: []sdk.Msg{ + stakingtypes.NewMsgDelegate( + testAddress, + sdk.ValAddress(suite.createTestAddress()), + suite.makeCoins("photon", math.NewInt(1))[0], + ), + stakingtypes.NewMsgDelegate( + testAddress, + sdk.ValAddress(suite.createTestAddress()), + suite.makeCoins("photon", math.NewInt(5))[0], + ), + }, + accountNumber: 25, + sequence: 78, + expectSuccess: true, + }, + { + title: "Fails - Two MsgVotes with Different Signers", fee: txtypes.Fee{ Amount: suite.makeCoins("aphoton", math.NewInt(2000)), GasLimit: 20000, @@ -199,7 +225,7 @@ func (suite *EIP712TestSuite) TestEIP712SignatureVerification() { expectSuccess: false, // Multiple messages are currently not allowed }, { - title: "Fails - MsgSend + MsgVote", + title: "Fails - Single-Signer MsgSend + MsgVote", fee: txtypes.Fee{ Amount: suite.makeCoins("aphoton", math.NewInt(2000)), GasLimit: 20000, @@ -207,12 +233,12 @@ func (suite *EIP712TestSuite) TestEIP712SignatureVerification() { memo: "", msgs: []sdk.Msg{ govtypes.NewMsgVote( - suite.createTestAddress(), + testAddress, 5, govtypes.OptionNo, ), banktypes.NewMsgSend( - suite.createTestAddress(), + testAddress, suite.createTestAddress(), suite.makeCoins("photon", math.NewInt(50)), ), diff --git a/ethereum/eip712/encoding.go b/ethereum/eip712/encoding.go index 0bb1c1ab44..a57d776446 100644 --- a/ethereum/eip712/encoding.go +++ b/ethereum/eip712/encoding.go @@ -1,13 +1,15 @@ package eip712 import ( + "encoding/json" "errors" "fmt" + "strings" "github.com/cosmos/cosmos-sdk/simapp/params" "github.com/cosmos/cosmos-sdk/x/auth/migrations/legacytx" - cosmosTypes "github.com/cosmos/cosmos-sdk/types" + sdk "github.com/cosmos/cosmos-sdk/types" txTypes "github.com/cosmos/cosmos-sdk/types/tx" apitypes "github.com/ethereum/go-ethereum/signer/core/apitypes" @@ -17,8 +19,8 @@ import ( ) var ( - ethermintProtoCodec codec.ProtoCodecMarshaler - ethermintAminoCodec *codec.LegacyAmino + protoCodec codec.ProtoCodecMarshaler + aminoCodec *codec.LegacyAmino ) // SetEncodingConfig set the encoding config to the singleton codecs (Amino and Protobuf). @@ -26,8 +28,8 @@ var ( // populated with all relevant message types. As a result, we must call this method on app // initialization with the app's encoding config. func SetEncodingConfig(cfg params.EncodingConfig) { - ethermintAminoCodec = cfg.Amino - ethermintProtoCodec = codec.NewProtoCodec(cfg.InterfaceRegistry) + aminoCodec = cfg.Amino + protoCodec = codec.NewProtoCodec(cfg.InterfaceRegistry) } // Get the EIP-712 object hash for the given SignDoc bytes by first decoding the bytes into @@ -59,67 +61,69 @@ func GetEIP712TypedDataForMsg(signDocBytes []byte) (apitypes.TypedData, error) { // Attempt to decode as both Amino and Protobuf since the message format is unknown. // If either decode works, we can move forward with the corresponding typed data. typedDataAmino, errAmino := decodeAminoSignDoc(signDocBytes) - if errAmino == nil && verifyEIP712Payload(typedDataAmino) { + if errAmino == nil && isValidEIP712Payload(typedDataAmino) { return typedDataAmino, nil } typedDataProtobuf, errProtobuf := decodeProtobufSignDoc(signDocBytes) - if errProtobuf == nil && verifyEIP712Payload(typedDataProtobuf) { + if errProtobuf == nil && isValidEIP712Payload(typedDataProtobuf) { return typedDataProtobuf, nil } - return apitypes.TypedData{}, fmt.Errorf("could not decode sign doc as either Amino or Protobuf. amino: %v protobuf: %v", errAmino, errProtobuf) + return apitypes.TypedData{}, fmt.Errorf("could not decode sign doc as either Amino or Protobuf.\n amino: %v\n protobuf: %v", errAmino, errProtobuf) } -// verifyEIP712Payload ensures that the given TypedData does not contain empty fields from +// isValidEIP712Payload ensures that the given TypedData does not contain empty fields from // an improper initialization. -func verifyEIP712Payload(typedData apitypes.TypedData) bool { +func isValidEIP712Payload(typedData apitypes.TypedData) bool { return len(typedData.Message) != 0 && len(typedData.Types) != 0 && typedData.PrimaryType != "" && typedData.Domain != apitypes.TypedDataDomain{} } -// Attempt to decode the SignDoc bytes as an Amino SignDoc and return an error on failure +// decodeAminoSignDoc attempts to decode the provided sign doc (bytes) as an Amino payload +// and returns a signable EIP-712 TypedData object. func decodeAminoSignDoc(signDocBytes []byte) (apitypes.TypedData, error) { var aminoDoc legacytx.StdSignDoc - - if err := ethermintAminoCodec.UnmarshalJSON(signDocBytes, &aminoDoc); err != nil { + if err := aminoCodec.UnmarshalJSON(signDocBytes, &aminoDoc); err != nil { return apitypes.TypedData{}, err } - // Unwrap fees var fees legacytx.StdFee - if err := ethermintAminoCodec.UnmarshalJSON(aminoDoc.Fee, &fees); err != nil { + if err := aminoCodec.UnmarshalJSON(aminoDoc.Fee, &fees); err != nil { return apitypes.TypedData{}, err } - if len(aminoDoc.Msgs) != 1 { - return apitypes.TypedData{}, fmt.Errorf("invalid number of messages in SignDoc, expected 1 but got %v", len(aminoDoc.Msgs)) + // Validate payload messages + var msgs []sdk.Msg + for _, jsonMsg := range aminoDoc.Msgs { + var m sdk.Msg + if err := aminoCodec.UnmarshalJSON(jsonMsg, &m); err != nil { + return apitypes.TypedData{}, fmt.Errorf("failed to unmarshal sign doc message: %w", err) + } + msgs = append(msgs, m) } - var msg cosmosTypes.Msg - if err := ethermintAminoCodec.UnmarshalJSON(aminoDoc.Msgs[0], &msg); err != nil { - return apitypes.TypedData{}, fmt.Errorf("failed to unmarshal first message: %w", err) + if err := validatePayloadMessages(msgs); err != nil { + return apitypes.TypedData{}, err } - // By default, use first address in list of signers to cover fee - // Currently, support only one signer - if len(msg.GetSigners()) != 1 { - return apitypes.TypedData{}, errors.New("expected exactly one signer for message") - } + // Use first message for fee payer and type inference + msg := msgs[0] + + // By convention, the fee payer is the first address in the list of signers. feePayer := msg.GetSigners()[0] feeDelegation := &FeeDelegationOptions{ FeePayer: feePayer, } - // Parse ChainID chainID, err := ethermint.ParseChainID(aminoDoc.ChainID) if err != nil { return apitypes.TypedData{}, errors.New("invalid chain ID passed as argument") } typedData, err := WrapTxToTypedData( - ethermintProtoCodec, + protoCodec, chainID.Uint64(), msg, - signDocBytes, // Amino StdSignDocBytes + signDocBytes, feeDelegation, ) if err != nil { @@ -129,21 +133,19 @@ func decodeAminoSignDoc(signDocBytes []byte) (apitypes.TypedData, error) { return typedData, nil } -// Attempt to decode the SignDoc bytes as a Protobuf SignDoc and return an error on failure +// decodeProtobufSignDoc attempts to decode the provided sign doc (bytes) as a Protobuf payload +// and returns a signable EIP-712 TypedData object. func decodeProtobufSignDoc(signDocBytes []byte) (apitypes.TypedData, error) { - // Decode sign doc signDoc := &txTypes.SignDoc{} if err := signDoc.Unmarshal(signDocBytes); err != nil { return apitypes.TypedData{}, err } - // Decode auth info authInfo := &txTypes.AuthInfo{} if err := authInfo.Unmarshal(signDoc.AuthInfoBytes); err != nil { return apitypes.TypedData{}, err } - // Decode body body := &txTypes.TxBody{} if err := body.Unmarshal(signDoc.BodyBytes); err != nil { return apitypes.TypedData{}, err @@ -154,60 +156,60 @@ func decodeProtobufSignDoc(signDocBytes []byte) (apitypes.TypedData, error) { return apitypes.TypedData{}, errors.New("body contains unsupported fields: TimeoutHeight, ExtensionOptions, or NonCriticalExtensionOptions") } - // Verify single message - if len(body.Messages) != 1 { - return apitypes.TypedData{}, fmt.Errorf("invalid number of messages, expected 1 got %v", len(body.Messages)) + if len(authInfo.SignerInfos) != 1 { + return apitypes.TypedData{}, fmt.Errorf("invalid number of signer infos provided, expected 1 got %v", len(authInfo.SignerInfos)) + } + + // Validate payload messages + var msgs []sdk.Msg + for _, protoMsg := range body.Messages { + var m sdk.Msg + if err := protoCodec.UnpackAny(protoMsg, &m); err != nil { + return apitypes.TypedData{}, fmt.Errorf("could not unpack message object with error %w", err) + } + msgs = append(msgs, m) } - // Decode signer info (single signer for now) + if err := validatePayloadMessages(msgs); err != nil { + return apitypes.TypedData{}, err + } + + // Use first message for fee payer and type inference + msg := msgs[0] + signerInfo := authInfo.SignerInfos[0] - // Parse ChainID chainID, err := ethermint.ParseChainID(signDoc.ChainId) if err != nil { return apitypes.TypedData{}, fmt.Errorf("invalid chain ID passed as argument: %w", err) } - // Create StdFee stdFee := &legacytx.StdFee{ Amount: authInfo.Fee.Amount, Gas: authInfo.Fee.GasLimit, } - // Parse Message (single message only) - var msg cosmosTypes.Msg - if err := ethermintProtoCodec.UnpackAny(body.Messages[0], &msg); err != nil { - return apitypes.TypedData{}, fmt.Errorf("could not unpack message object with error %w", err) - } - - // Verify single signer (single signer for now) - if len(msg.GetSigners()) != 1 { - return apitypes.TypedData{}, fmt.Errorf("invalid number of signers, expected 1 got %v", len(authInfo.SignerInfos)) - } - - // Init fee payer feePayer := msg.GetSigners()[0] feeDelegation := &FeeDelegationOptions{ FeePayer: feePayer, } - // Get tip tip := authInfo.Tip - // Create Legacy SignBytes (expected type for WrapTxToTypedData) + // WrapTxToTypedData expects the payload as an Amino Sign Doc signBytes := legacytx.StdSignBytes( signDoc.ChainId, signDoc.AccountNumber, signerInfo.Sequence, body.TimeoutHeight, *stdFee, - []cosmosTypes.Msg{msg}, + []sdk.Msg{msg}, body.Memo, tip, ) typedData, err := WrapTxToTypedData( - ethermintProtoCodec, + protoCodec, chainID.Uint64(), msg, signBytes, @@ -219,3 +221,67 @@ func decodeProtobufSignDoc(signDocBytes []byte) (apitypes.TypedData, error) { return typedData, nil } + +// validatePayloadMessages ensures that the transaction messages can be represented in an EIP-712 +// encoding by checking that messages exist, are of the same type, and share a single signer. +func validatePayloadMessages(msgs []sdk.Msg) error { + if len(msgs) == 0 { + return errors.New("unable to build EIP-712 payload: transaction does contain any messages") + } + + var msgType string + var msgSigner sdk.AccAddress + + for i, m := range msgs { + t, err := getMsgType(m) + if err != nil { + return err + } + + if len(m.GetSigners()) != 1 { + return errors.New("unable to build EIP-712 payload: expect exactly 1 signer") + } + + if i == 0 { + msgType = t + msgSigner = m.GetSigners()[0] + continue + } + + if strings.Compare(t, msgType) != 0 { + return errors.New("unable to build EIP-712 payload: different types of messages detected") + } + + if !msgSigner.Equals(m.GetSigners()[0]) { + return errors.New("unable to build EIP-712 payload: multiple signers detected") + } + } + + return nil +} + +type aminoMessage struct { + Type string `json:"type"` + Value interface{} `json:"value"` +} + +// getMsgType returns the message type prefix for the given Cosmos SDK Msg +func getMsgType(msg sdk.Msg) (string, error) { + jsonBytes, err := aminoCodec.MarshalJSON(msg) + if err != nil { + return "", err + } + + var jsonMsg aminoMessage + err = json.Unmarshal(jsonBytes, &jsonMsg) + if err != nil { + return "", err + } + + // Verify Type was successfully filled in + if jsonMsg.Type == "" { + return "", errors.New("could not decode message: type is missing") + } + + return jsonMsg.Type, nil +} From 137840e435138e9c053523019658b4bee4b30114 Mon Sep 17 00:00:00 2001 From: Austin Chandra Date: Mon, 21 Nov 2022 12:42:48 -0800 Subject: [PATCH 2/7] Fix Protobuf encoding bug --- ethereum/eip712/encoding.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ethereum/eip712/encoding.go b/ethereum/eip712/encoding.go index a57d776446..67ff634a48 100644 --- a/ethereum/eip712/encoding.go +++ b/ethereum/eip712/encoding.go @@ -203,7 +203,7 @@ func decodeProtobufSignDoc(signDocBytes []byte) (apitypes.TypedData, error) { signerInfo.Sequence, body.TimeoutHeight, *stdFee, - []sdk.Msg{msg}, + msgs, body.Memo, tip, ) From de0af4e22a0487d21571f075a596aa4c154a5752 Mon Sep 17 00:00:00 2001 From: Austin Chandra Date: Mon, 21 Nov 2022 13:42:00 -0800 Subject: [PATCH 3/7] Add ante tests --- app/ante/ante_test.go | 24 +++++++++++++++++++- app/ante/utils_test.go | 50 +++++++++++++++++++++++++++++------------- 2 files changed, 58 insertions(+), 16 deletions(-) diff --git a/app/ante/ante_test.go b/app/ante/ante_test.go index 6baf11a3b9..a058745e1b 100644 --- a/app/ante/ante_test.go +++ b/app/ante/ante_test.go @@ -387,7 +387,7 @@ func (suite AnteTestSuite) TestAnteHandler() { from, grantee, &banktypes.SendAuthorization{SpendLimit: gasAmount}, &expiresAt, ) suite.Require().NoError(err) - return suite.CreateTestEIP712CosmosTxBuilder(from, privKey, "ethermint_9000-1", gas, gasAmount, msg).GetTx() + return suite.CreateTestSingleMessageEIP712TxBuilder(from, privKey, "ethermint_9000-1", gas, gasAmount, msg).GetTx() }, false, false, true, }, @@ -457,6 +457,28 @@ func (suite AnteTestSuite) TestAnteHandler() { return txBuilder.GetTx() }, false, false, true, }, + { + "success- DeliverTx EIP712 Multiple MsgSend", + func() sdk.Tx { + from := acc.GetAddress() + coinAmount := sdk.NewCoin(evmtypes.DefaultEVMDenom, sdk.NewInt(20)) + amount := sdk.NewCoins(coinAmount) + gas := uint64(200000) + txBuilder := suite.CreateTestEIP712MultipleMsgSend(from, privKey, "ethermint_9000-1", gas, amount) + return txBuilder.GetTx() + }, false, false, true, + }, + { + "fails - DeliverTx EIP712 Multiple Signers", + func() sdk.Tx { + from := acc.GetAddress() + coinAmount := sdk.NewCoin(evmtypes.DefaultEVMDenom, sdk.NewInt(20)) + amount := sdk.NewCoins(coinAmount) + gas := uint64(200000) + txBuilder := suite.CreateTestEIP712MultipleSignerMsgs(from, privKey, "ethermint_9000-1", gas, amount) + return txBuilder.GetTx() + }, false, false, false, + }, { "fails - DeliverTx EIP712 signed Cosmos Tx with wrong Chain ID", func() sdk.Tx { diff --git a/app/ante/utils_test.go b/app/ante/utils_test.go index 5da28a3af3..c9047f4076 100644 --- a/app/ante/utils_test.go +++ b/app/ante/utils_test.go @@ -272,7 +272,7 @@ func (suite *AnteTestSuite) CreateTestEIP712TxBuilderMsgSend(from sdk.AccAddress // Build MsgSend recipient := sdk.AccAddress(common.Address{}.Bytes()) msgSend := types2.NewMsgSend(from, recipient, sdk.NewCoins(sdk.NewCoin(evmtypes.DefaultEVMDenom, sdkmath.NewInt(1)))) - return suite.CreateTestEIP712CosmosTxBuilder(from, priv, chainId, gas, gasAmount, msgSend) + return suite.CreateTestSingleMessageEIP712TxBuilder(from, priv, chainId, gas, gasAmount, msgSend) } func (suite *AnteTestSuite) CreateTestEIP712TxBuilderMsgDelegate(from sdk.AccAddress, priv cryptotypes.PrivKey, chainId string, gas uint64, gasAmount sdk.Coins) client.TxBuilder { @@ -280,7 +280,7 @@ func (suite *AnteTestSuite) CreateTestEIP712TxBuilderMsgDelegate(from sdk.AccAdd valEthAddr := tests.GenerateAddress() valAddr := sdk.ValAddress(valEthAddr.Bytes()) msgSend := types3.NewMsgDelegate(from, valAddr, sdk.NewCoin(evmtypes.DefaultEVMDenom, sdkmath.NewInt(20))) - return suite.CreateTestEIP712CosmosTxBuilder(from, priv, chainId, gas, gasAmount, msgSend) + return suite.CreateTestSingleMessageEIP712TxBuilder(from, priv, chainId, gas, gasAmount, msgSend) } func (suite *AnteTestSuite) CreateTestEIP712MsgCreateValidator(from sdk.AccAddress, priv cryptotypes.PrivKey, chainId string, gas uint64, gasAmount sdk.Coins) client.TxBuilder { @@ -296,7 +296,7 @@ func (suite *AnteTestSuite) CreateTestEIP712MsgCreateValidator(from sdk.AccAddre sdk.OneInt(), ) suite.Require().NoError(err) - return suite.CreateTestEIP712CosmosTxBuilder(from, priv, chainId, gas, gasAmount, msgCreate) + return suite.CreateTestSingleMessageEIP712TxBuilder(from, priv, chainId, gas, gasAmount, msgCreate) } func (suite *AnteTestSuite) CreateTestEIP712MsgCreateValidator2(from sdk.AccAddress, priv cryptotypes.PrivKey, chainId string, gas uint64, gasAmount sdk.Coins) client.TxBuilder { @@ -313,7 +313,7 @@ func (suite *AnteTestSuite) CreateTestEIP712MsgCreateValidator2(from sdk.AccAddr sdk.OneInt(), ) suite.Require().NoError(err) - return suite.CreateTestEIP712CosmosTxBuilder(from, priv, chainId, gas, gasAmount, msgCreate) + return suite.CreateTestSingleMessageEIP712TxBuilder(from, priv, chainId, gas, gasAmount, msgCreate) } func (suite *AnteTestSuite) CreateTestEIP712SubmitProposal(from sdk.AccAddress, priv cryptotypes.PrivKey, chainId string, gas uint64, gasAmount sdk.Coins, deposit sdk.Coins) client.TxBuilder { @@ -321,7 +321,7 @@ func (suite *AnteTestSuite) CreateTestEIP712SubmitProposal(from sdk.AccAddress, suite.Require().True(ok) msgSubmit, err := types5.NewMsgSubmitProposal(proposal, deposit, from) suite.Require().NoError(err) - return suite.CreateTestEIP712CosmosTxBuilder(from, priv, chainId, gas, gasAmount, msgSubmit) + return suite.CreateTestSingleMessageEIP712TxBuilder(from, priv, chainId, gas, gasAmount, msgSubmit) } func (suite *AnteTestSuite) CreateTestEIP712GrantAllowance(from sdk.AccAddress, priv cryptotypes.PrivKey, chainId string, gas uint64, gasAmount sdk.Coins) client.TxBuilder { @@ -335,7 +335,7 @@ func (suite *AnteTestSuite) CreateTestEIP712GrantAllowance(from sdk.AccAddress, grantedAddr := suite.app.AccountKeeper.NewAccountWithAddress(suite.ctx, granted.Bytes()) msgGrant, err := feegrant.NewMsgGrantAllowance(basic, from, grantedAddr.GetAddress()) suite.Require().NoError(err) - return suite.CreateTestEIP712CosmosTxBuilder(from, priv, chainId, gas, gasAmount, msgGrant) + return suite.CreateTestSingleMessageEIP712TxBuilder(from, priv, chainId, gas, gasAmount, msgGrant) } func (suite *AnteTestSuite) CreateTestEIP712MsgEditValidator(from sdk.AccAddress, priv cryptotypes.PrivKey, chainId string, gas uint64, gasAmount sdk.Coins) client.TxBuilder { @@ -346,7 +346,7 @@ func (suite *AnteTestSuite) CreateTestEIP712MsgEditValidator(from sdk.AccAddress nil, nil, ) - return suite.CreateTestEIP712CosmosTxBuilder(from, priv, chainId, gas, gasAmount, msgEdit) + return suite.CreateTestSingleMessageEIP712TxBuilder(from, priv, chainId, gas, gasAmount, msgEdit) } func (suite *AnteTestSuite) CreateTestEIP712MsgSubmitEvidence(from sdk.AccAddress, priv cryptotypes.PrivKey, chainId string, gas uint64, gasAmount sdk.Coins) client.TxBuilder { @@ -359,12 +359,12 @@ func (suite *AnteTestSuite) CreateTestEIP712MsgSubmitEvidence(from sdk.AccAddres }) suite.Require().NoError(err) - return suite.CreateTestEIP712CosmosTxBuilder(from, priv, chainId, gas, gasAmount, msgEvidence) + return suite.CreateTestSingleMessageEIP712TxBuilder(from, priv, chainId, gas, gasAmount, msgEvidence) } func (suite *AnteTestSuite) CreateTestEIP712MsgVoteV1(from sdk.AccAddress, priv cryptotypes.PrivKey, chainId string, gas uint64, gasAmount sdk.Coins) client.TxBuilder { msgVote := govtypes.NewMsgVote(from, 1, govtypes.VoteOption_VOTE_OPTION_YES, "") - return suite.CreateTestEIP712CosmosTxBuilder(from, priv, chainId, gas, gasAmount, msgVote) + return suite.CreateTestSingleMessageEIP712TxBuilder(from, priv, chainId, gas, gasAmount, msgVote) } func (suite *AnteTestSuite) CreateTestEIP712SubmitProposalV1(from sdk.AccAddress, priv cryptotypes.PrivKey, chainId string, gas uint64, gasAmount sdk.Coins) client.TxBuilder { @@ -403,14 +403,28 @@ func (suite *AnteTestSuite) CreateTestEIP712SubmitProposalV1(from sdk.AccAddress suite.Require().NoError(err) - return suite.CreateTestEIP712CosmosTxBuilder(from, priv, chainId, gas, gasAmount, msgProposal) + return suite.CreateTestSingleMessageEIP712TxBuilder(from, priv, chainId, gas, gasAmount, msgProposal) } func (suite *AnteTestSuite) CreateTestEIP712MsgExec(from sdk.AccAddress, priv cryptotypes.PrivKey, chainId string, gas uint64, gasAmount sdk.Coins) client.TxBuilder { recipient := sdk.AccAddress(common.Address{}.Bytes()) msgSend := types2.NewMsgSend(from, recipient, sdk.NewCoins(sdk.NewCoin(evmtypes.DefaultEVMDenom, sdkmath.NewInt(1)))) msgExec := authz.NewMsgExec(from, []sdk.Msg{msgSend}) - return suite.CreateTestEIP712CosmosTxBuilder(from, priv, chainId, gas, gasAmount, &msgExec) + return suite.CreateTestSingleMessageEIP712TxBuilder(from, priv, chainId, gas, gasAmount, &msgExec) +} + +func (suite *AnteTestSuite) CreateTestEIP712MultipleMsgSend(from sdk.AccAddress, priv cryptotypes.PrivKey, chainId string, gas uint64, gasAmount sdk.Coins) client.TxBuilder { + recipient := sdk.AccAddress(common.Address{}.Bytes()) + msgSend := types2.NewMsgSend(from, recipient, sdk.NewCoins(sdk.NewCoin(evmtypes.DefaultEVMDenom, sdkmath.NewInt(1)))) + return suite.CreateTestEIP712CosmosTxBuilder(from, priv, chainId, gas, gasAmount, []sdk.Msg{msgSend, msgSend, msgSend}) +} + +// Fails +func (suite *AnteTestSuite) CreateTestEIP712MultipleSignerMsgs(from sdk.AccAddress, priv cryptotypes.PrivKey, chainId string, gas uint64, gasAmount sdk.Coins) client.TxBuilder { + recipient := sdk.AccAddress(common.Address{}.Bytes()) + msgSend1 := types2.NewMsgSend(from, recipient, sdk.NewCoins(sdk.NewCoin(evmtypes.DefaultEVMDenom, sdkmath.NewInt(1)))) + msgSend2 := types2.NewMsgSend(recipient, from, sdk.NewCoins(sdk.NewCoin(evmtypes.DefaultEVMDenom, sdkmath.NewInt(1)))) + return suite.CreateTestEIP712CosmosTxBuilder(from, priv, chainId, gas, gasAmount, []sdk.Msg{msgSend1, msgSend2}) } // StdSignBytes returns the bytes to sign for a transaction. @@ -451,8 +465,14 @@ func StdSignBytes(cdc *codec.LegacyAmino, chainID string, accnum uint64, sequenc return sdk.MustSortJSON(bz) } -func (suite *AnteTestSuite) CreateTestEIP712CosmosTxBuilder( +func (suite *AnteTestSuite) CreateTestSingleMessageEIP712TxBuilder( from sdk.AccAddress, priv cryptotypes.PrivKey, chainId string, gas uint64, gasAmount sdk.Coins, msg sdk.Msg, +) client.TxBuilder { + return suite.CreateTestEIP712CosmosTxBuilder(from, priv, chainId, gas, gasAmount, []sdk.Msg{msg}) +} + +func (suite *AnteTestSuite) CreateTestEIP712CosmosTxBuilder( + from sdk.AccAddress, priv cryptotypes.PrivKey, chainId string, gas uint64, gasAmount sdk.Coins, msgs []sdk.Msg, ) client.TxBuilder { var err error @@ -473,8 +493,8 @@ func (suite *AnteTestSuite) CreateTestEIP712CosmosTxBuilder( fee := legacytx.NewStdFee(gas, gasAmount) accNumber := suite.app.AccountKeeper.GetAccount(suite.ctx, from).GetAccountNumber() - data := legacytx.StdSignBytes(chainId, accNumber, nonce, 0, fee, []sdk.Msg{msg}, "", nil) - typedData, err := eip712.WrapTxToTypedData(ethermintCodec, ethChainId, msg, data, &eip712.FeeDelegationOptions{ + data := legacytx.StdSignBytes(chainId, accNumber, nonce, 0, fee, msgs, "", nil) + typedData, err := eip712.WrapTxToTypedData(ethermintCodec, ethChainId, msgs[0], data, &eip712.FeeDelegationOptions{ FeePayer: from, }) suite.Require().NoError(err) @@ -517,7 +537,7 @@ func (suite *AnteTestSuite) CreateTestEIP712CosmosTxBuilder( err = builder.SetSignatures(sigsV2) suite.Require().NoError(err) - err = builder.SetMsgs(msg) + err = builder.SetMsgs(msgs...) suite.Require().NoError(err) return builder From 080cfaeeab2adccb09fc01b62ebab6c528f3c1ce Mon Sep 17 00:00:00 2001 From: Austin Chandra Date: Mon, 21 Nov 2022 14:31:05 -0800 Subject: [PATCH 4/7] Refactor naming and minor implementation details --- app/ante/ante_test.go | 2 +- app/ante/utils_test.go | 24 ++++++++++++------------ ethereum/eip712/encoding.go | 12 ++++++------ 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/app/ante/ante_test.go b/app/ante/ante_test.go index a058745e1b..15cc95338e 100644 --- a/app/ante/ante_test.go +++ b/app/ante/ante_test.go @@ -387,7 +387,7 @@ func (suite AnteTestSuite) TestAnteHandler() { from, grantee, &banktypes.SendAuthorization{SpendLimit: gasAmount}, &expiresAt, ) suite.Require().NoError(err) - return suite.CreateTestSingleMessageEIP712TxBuilder(from, privKey, "ethermint_9000-1", gas, gasAmount, msg).GetTx() + return suite.CreateTestEIP712SingleMessageTxBuilder(from, privKey, "ethermint_9000-1", gas, gasAmount, msg).GetTx() }, false, false, true, }, diff --git a/app/ante/utils_test.go b/app/ante/utils_test.go index c9047f4076..6c670849f7 100644 --- a/app/ante/utils_test.go +++ b/app/ante/utils_test.go @@ -272,7 +272,7 @@ func (suite *AnteTestSuite) CreateTestEIP712TxBuilderMsgSend(from sdk.AccAddress // Build MsgSend recipient := sdk.AccAddress(common.Address{}.Bytes()) msgSend := types2.NewMsgSend(from, recipient, sdk.NewCoins(sdk.NewCoin(evmtypes.DefaultEVMDenom, sdkmath.NewInt(1)))) - return suite.CreateTestSingleMessageEIP712TxBuilder(from, priv, chainId, gas, gasAmount, msgSend) + return suite.CreateTestEIP712SingleMessageTxBuilder(from, priv, chainId, gas, gasAmount, msgSend) } func (suite *AnteTestSuite) CreateTestEIP712TxBuilderMsgDelegate(from sdk.AccAddress, priv cryptotypes.PrivKey, chainId string, gas uint64, gasAmount sdk.Coins) client.TxBuilder { @@ -280,7 +280,7 @@ func (suite *AnteTestSuite) CreateTestEIP712TxBuilderMsgDelegate(from sdk.AccAdd valEthAddr := tests.GenerateAddress() valAddr := sdk.ValAddress(valEthAddr.Bytes()) msgSend := types3.NewMsgDelegate(from, valAddr, sdk.NewCoin(evmtypes.DefaultEVMDenom, sdkmath.NewInt(20))) - return suite.CreateTestSingleMessageEIP712TxBuilder(from, priv, chainId, gas, gasAmount, msgSend) + return suite.CreateTestEIP712SingleMessageTxBuilder(from, priv, chainId, gas, gasAmount, msgSend) } func (suite *AnteTestSuite) CreateTestEIP712MsgCreateValidator(from sdk.AccAddress, priv cryptotypes.PrivKey, chainId string, gas uint64, gasAmount sdk.Coins) client.TxBuilder { @@ -296,7 +296,7 @@ func (suite *AnteTestSuite) CreateTestEIP712MsgCreateValidator(from sdk.AccAddre sdk.OneInt(), ) suite.Require().NoError(err) - return suite.CreateTestSingleMessageEIP712TxBuilder(from, priv, chainId, gas, gasAmount, msgCreate) + return suite.CreateTestEIP712SingleMessageTxBuilder(from, priv, chainId, gas, gasAmount, msgCreate) } func (suite *AnteTestSuite) CreateTestEIP712MsgCreateValidator2(from sdk.AccAddress, priv cryptotypes.PrivKey, chainId string, gas uint64, gasAmount sdk.Coins) client.TxBuilder { @@ -313,7 +313,7 @@ func (suite *AnteTestSuite) CreateTestEIP712MsgCreateValidator2(from sdk.AccAddr sdk.OneInt(), ) suite.Require().NoError(err) - return suite.CreateTestSingleMessageEIP712TxBuilder(from, priv, chainId, gas, gasAmount, msgCreate) + return suite.CreateTestEIP712SingleMessageTxBuilder(from, priv, chainId, gas, gasAmount, msgCreate) } func (suite *AnteTestSuite) CreateTestEIP712SubmitProposal(from sdk.AccAddress, priv cryptotypes.PrivKey, chainId string, gas uint64, gasAmount sdk.Coins, deposit sdk.Coins) client.TxBuilder { @@ -321,7 +321,7 @@ func (suite *AnteTestSuite) CreateTestEIP712SubmitProposal(from sdk.AccAddress, suite.Require().True(ok) msgSubmit, err := types5.NewMsgSubmitProposal(proposal, deposit, from) suite.Require().NoError(err) - return suite.CreateTestSingleMessageEIP712TxBuilder(from, priv, chainId, gas, gasAmount, msgSubmit) + return suite.CreateTestEIP712SingleMessageTxBuilder(from, priv, chainId, gas, gasAmount, msgSubmit) } func (suite *AnteTestSuite) CreateTestEIP712GrantAllowance(from sdk.AccAddress, priv cryptotypes.PrivKey, chainId string, gas uint64, gasAmount sdk.Coins) client.TxBuilder { @@ -335,7 +335,7 @@ func (suite *AnteTestSuite) CreateTestEIP712GrantAllowance(from sdk.AccAddress, grantedAddr := suite.app.AccountKeeper.NewAccountWithAddress(suite.ctx, granted.Bytes()) msgGrant, err := feegrant.NewMsgGrantAllowance(basic, from, grantedAddr.GetAddress()) suite.Require().NoError(err) - return suite.CreateTestSingleMessageEIP712TxBuilder(from, priv, chainId, gas, gasAmount, msgGrant) + return suite.CreateTestEIP712SingleMessageTxBuilder(from, priv, chainId, gas, gasAmount, msgGrant) } func (suite *AnteTestSuite) CreateTestEIP712MsgEditValidator(from sdk.AccAddress, priv cryptotypes.PrivKey, chainId string, gas uint64, gasAmount sdk.Coins) client.TxBuilder { @@ -346,7 +346,7 @@ func (suite *AnteTestSuite) CreateTestEIP712MsgEditValidator(from sdk.AccAddress nil, nil, ) - return suite.CreateTestSingleMessageEIP712TxBuilder(from, priv, chainId, gas, gasAmount, msgEdit) + return suite.CreateTestEIP712SingleMessageTxBuilder(from, priv, chainId, gas, gasAmount, msgEdit) } func (suite *AnteTestSuite) CreateTestEIP712MsgSubmitEvidence(from sdk.AccAddress, priv cryptotypes.PrivKey, chainId string, gas uint64, gasAmount sdk.Coins) client.TxBuilder { @@ -359,12 +359,12 @@ func (suite *AnteTestSuite) CreateTestEIP712MsgSubmitEvidence(from sdk.AccAddres }) suite.Require().NoError(err) - return suite.CreateTestSingleMessageEIP712TxBuilder(from, priv, chainId, gas, gasAmount, msgEvidence) + return suite.CreateTestEIP712SingleMessageTxBuilder(from, priv, chainId, gas, gasAmount, msgEvidence) } func (suite *AnteTestSuite) CreateTestEIP712MsgVoteV1(from sdk.AccAddress, priv cryptotypes.PrivKey, chainId string, gas uint64, gasAmount sdk.Coins) client.TxBuilder { msgVote := govtypes.NewMsgVote(from, 1, govtypes.VoteOption_VOTE_OPTION_YES, "") - return suite.CreateTestSingleMessageEIP712TxBuilder(from, priv, chainId, gas, gasAmount, msgVote) + return suite.CreateTestEIP712SingleMessageTxBuilder(from, priv, chainId, gas, gasAmount, msgVote) } func (suite *AnteTestSuite) CreateTestEIP712SubmitProposalV1(from sdk.AccAddress, priv cryptotypes.PrivKey, chainId string, gas uint64, gasAmount sdk.Coins) client.TxBuilder { @@ -403,14 +403,14 @@ func (suite *AnteTestSuite) CreateTestEIP712SubmitProposalV1(from sdk.AccAddress suite.Require().NoError(err) - return suite.CreateTestSingleMessageEIP712TxBuilder(from, priv, chainId, gas, gasAmount, msgProposal) + return suite.CreateTestEIP712SingleMessageTxBuilder(from, priv, chainId, gas, gasAmount, msgProposal) } func (suite *AnteTestSuite) CreateTestEIP712MsgExec(from sdk.AccAddress, priv cryptotypes.PrivKey, chainId string, gas uint64, gasAmount sdk.Coins) client.TxBuilder { recipient := sdk.AccAddress(common.Address{}.Bytes()) msgSend := types2.NewMsgSend(from, recipient, sdk.NewCoins(sdk.NewCoin(evmtypes.DefaultEVMDenom, sdkmath.NewInt(1)))) msgExec := authz.NewMsgExec(from, []sdk.Msg{msgSend}) - return suite.CreateTestSingleMessageEIP712TxBuilder(from, priv, chainId, gas, gasAmount, &msgExec) + return suite.CreateTestEIP712SingleMessageTxBuilder(from, priv, chainId, gas, gasAmount, &msgExec) } func (suite *AnteTestSuite) CreateTestEIP712MultipleMsgSend(from sdk.AccAddress, priv cryptotypes.PrivKey, chainId string, gas uint64, gasAmount sdk.Coins) client.TxBuilder { @@ -465,7 +465,7 @@ func StdSignBytes(cdc *codec.LegacyAmino, chainID string, accnum uint64, sequenc return sdk.MustSortJSON(bz) } -func (suite *AnteTestSuite) CreateTestSingleMessageEIP712TxBuilder( +func (suite *AnteTestSuite) CreateTestEIP712SingleMessageTxBuilder( from sdk.AccAddress, priv cryptotypes.PrivKey, chainId string, gas uint64, gasAmount sdk.Coins, msg sdk.Msg, ) client.TxBuilder { return suite.CreateTestEIP712CosmosTxBuilder(from, priv, chainId, gas, gasAmount, []sdk.Msg{msg}) diff --git a/ethereum/eip712/encoding.go b/ethereum/eip712/encoding.go index 67ff634a48..24e9bb8cac 100644 --- a/ethereum/eip712/encoding.go +++ b/ethereum/eip712/encoding.go @@ -92,13 +92,13 @@ func decodeAminoSignDoc(signDocBytes []byte) (apitypes.TypedData, error) { } // Validate payload messages - var msgs []sdk.Msg - for _, jsonMsg := range aminoDoc.Msgs { + msgs := make([]sdk.Msg, len(aminoDoc.Msgs)) + for i, jsonMsg := range aminoDoc.Msgs { var m sdk.Msg if err := aminoCodec.UnmarshalJSON(jsonMsg, &m); err != nil { return apitypes.TypedData{}, fmt.Errorf("failed to unmarshal sign doc message: %w", err) } - msgs = append(msgs, m) + msgs[i] = m } if err := validatePayloadMessages(msgs); err != nil { @@ -161,13 +161,13 @@ func decodeProtobufSignDoc(signDocBytes []byte) (apitypes.TypedData, error) { } // Validate payload messages - var msgs []sdk.Msg - for _, protoMsg := range body.Messages { + msgs := make([]sdk.Msg, len(body.Messages)) + for i, protoMsg := range body.Messages { var m sdk.Msg if err := protoCodec.UnpackAny(protoMsg, &m); err != nil { return apitypes.TypedData{}, fmt.Errorf("could not unpack message object with error %w", err) } - msgs = append(msgs, m) + msgs[i] = m } if err := validatePayloadMessages(msgs); err != nil { From 3c7ddee01c4920a5daf0e9fe5ec1ca70a16330c0 Mon Sep 17 00:00:00 2001 From: Austin Chandra Date: Mon, 21 Nov 2022 15:01:34 -0800 Subject: [PATCH 5/7] Test empty transaction coverage --- ethereum/eip712/eip712_test.go | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/ethereum/eip712/eip712_test.go b/ethereum/eip712/eip712_test.go index 4b4cdee7e3..46b906e33d 100644 --- a/ethereum/eip712/eip712_test.go +++ b/ethereum/eip712/eip712_test.go @@ -222,7 +222,19 @@ func (suite *EIP712TestSuite) TestEIP712SignatureVerification() { }, accountNumber: 25, sequence: 78, - expectSuccess: false, // Multiple messages are currently not allowed + expectSuccess: false, + }, + { + title: "Fails - Empty transaction", + fee: txtypes.Fee{ + Amount: suite.makeCoins("aphoton", math.NewInt(2000)), + GasLimit: 20000, + }, + memo: "", + msgs: []sdk.Msg{}, + accountNumber: 25, + sequence: 78, + expectSuccess: false, }, { title: "Fails - Single-Signer MsgSend + MsgVote", From bafd0007c2e033bfc44f7d63ea2e494b5770f543 Mon Sep 17 00:00:00 2001 From: Austin Chandra Date: Tue, 22 Nov 2022 20:30:58 -0600 Subject: [PATCH 6/7] Address revisions for code clarity --- ethereum/eip712/encoding.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/ethereum/eip712/encoding.go b/ethereum/eip712/encoding.go index 24e9bb8cac..3ee267af80 100644 --- a/ethereum/eip712/encoding.go +++ b/ethereum/eip712/encoding.go @@ -4,7 +4,6 @@ import ( "encoding/json" "errors" "fmt" - "strings" "github.com/cosmos/cosmos-sdk/simapp/params" "github.com/cosmos/cosmos-sdk/x/auth/migrations/legacytx" @@ -248,7 +247,7 @@ func validatePayloadMessages(msgs []sdk.Msg) error { continue } - if strings.Compare(t, msgType) != 0 { + if t != msgType { return errors.New("unable to build EIP-712 payload: different types of messages detected") } @@ -273,8 +272,7 @@ func getMsgType(msg sdk.Msg) (string, error) { } var jsonMsg aminoMessage - err = json.Unmarshal(jsonBytes, &jsonMsg) - if err != nil { + if err := json.Unmarshal(jsonBytes, &jsonMsg); err != nil { return "", err } From 7259ef03dcaf033ba5cda798c47932a95ba376d8 Mon Sep 17 00:00:00 2001 From: Austin Chandra Date: Tue, 22 Nov 2022 20:31:37 -0600 Subject: [PATCH 7/7] Move aminoMessage type definition --- ethereum/eip712/encoding.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/ethereum/eip712/encoding.go b/ethereum/eip712/encoding.go index 3ee267af80..fa744ad74a 100644 --- a/ethereum/eip712/encoding.go +++ b/ethereum/eip712/encoding.go @@ -17,6 +17,11 @@ import ( "github.com/cosmos/cosmos-sdk/codec" ) +type aminoMessage struct { + Type string `json:"type"` + Value interface{} `json:"value"` +} + var ( protoCodec codec.ProtoCodecMarshaler aminoCodec *codec.LegacyAmino @@ -259,11 +264,6 @@ func validatePayloadMessages(msgs []sdk.Msg) error { return nil } -type aminoMessage struct { - Type string `json:"type"` - Value interface{} `json:"value"` -} - // getMsgType returns the message type prefix for the given Cosmos SDK Msg func getMsgType(msg sdk.Msg) (string, error) { jsonBytes, err := aminoCodec.MarshalJSON(msg)