From 7611743187b9ca19bf3905768d7534ea65e71e17 Mon Sep 17 00:00:00 2001 From: Austin Chandra Date: Wed, 19 Oct 2022 10:12:20 -0700 Subject: [PATCH 01/31] [WIP] EIP-712 Signature Refactor --- app/ante/ante_test.go | 30 +++ app/ante/utils_test.go | 150 ++++++++++++++ cmd/ethermintd/root.go | 3 + crypto/ethsecp256k1/ethsecp256k1.go | 31 ++- ethereum/eip712/eip712_test.go | 293 ++++++++++++++++++++++++++++ ethereum/eip712/encoding.go | 198 +++++++++++++++++++ 6 files changed, 698 insertions(+), 7 deletions(-) create mode 100644 ethereum/eip712/eip712_test.go create mode 100644 ethereum/eip712/encoding.go diff --git a/app/ante/ante_test.go b/app/ante/ante_test.go index 094fa80272..417a6f8917 100644 --- a/app/ante/ante_test.go +++ b/app/ante/ante_test.go @@ -19,6 +19,7 @@ import ( "github.com/evmos/ethermint/tests" evmtypes "github.com/evmos/ethermint/x/evm/types" + kmultisig "github.com/cosmos/cosmos-sdk/crypto/keys/multisig" banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" ) @@ -496,6 +497,35 @@ func (suite AnteTestSuite) TestAnteHandler() { return tx }, true, false, false, }, + { + "passes - EIP-712 multi-key", + func() sdk.Tx { + numKeys := 5 + privKeys, pubKeys := suite.GenerateMultipleKeys(numKeys) + pk := kmultisig.NewLegacyAminoPubKey(numKeys, pubKeys) + msg := banktypes.NewMsgSend( + sdk.AccAddress(pk.Address()), + addr[:], + sdk.NewCoins( + sdk.NewCoin( + "photon", + sdk.NewInt(1), + ), + ), + ) + + txBuilder := suite.CreateTestSignedMultisigTx( + privKeys, + pubKeys, + signing.SignMode_SIGN_MODE_DIRECT, + msg, + "ethermint_9000-1", + 2000000, + ) + + return txBuilder.GetTx() + }, false, false, true, + }, } for _, tc := range testCases { diff --git a/app/ante/utils_test.go b/app/ante/utils_test.go index c297820e98..1115a80440 100644 --- a/app/ante/utils_test.go +++ b/app/ante/utils_test.go @@ -25,7 +25,9 @@ import ( "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/client/tx" codectypes "github.com/cosmos/cosmos-sdk/codec/types" + kmultisig "github.com/cosmos/cosmos-sdk/crypto/keys/multisig" cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" + "github.com/cosmos/cosmos-sdk/crypto/types/multisig" "github.com/cosmos/cosmos-sdk/simapp" "github.com/cosmos/cosmos-sdk/testutil/testdata" sdk "github.com/cosmos/cosmos-sdk/types" @@ -35,6 +37,7 @@ import ( authtx "github.com/cosmos/cosmos-sdk/x/auth/tx" authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" cryptocodec "github.com/evmos/ethermint/crypto/codec" + "github.com/evmos/ethermint/crypto/ethsecp256k1" "github.com/cosmos/cosmos-sdk/crypto/keys/ed25519" evtypes "github.com/cosmos/cosmos-sdk/x/evidence/types" @@ -111,6 +114,7 @@ func (suite *AnteTestSuite) SetupTest() { encodingConfig := encoding.MakeConfig(app.ModuleBasics) // We're using TestMsg amino encoding in some tests, so register it here. encodingConfig.Amino.RegisterConcrete(&testdata.TestMsg{}, "testdata.TestMsg", nil) + eip712.SetEncodingConfig(encodingConfig) suite.clientCtx = client.Context{}.WithTxConfig(encodingConfig.TxConfig) @@ -448,6 +452,152 @@ func (suite *AnteTestSuite) CreateTestEIP712CosmosTxBuilder( return builder } +// Generate a set of pub/priv keys to be used in creating multi-keys +func (suite *AnteTestSuite) GenerateMultipleKeys(n int) ([]cryptotypes.PrivKey, []cryptotypes.PubKey) { + privKeys := make([]cryptotypes.PrivKey, n) + pubKeys := make([]cryptotypes.PubKey, n) + for i := 0; i < n; i++ { + privKey, err := ethsecp256k1.GenerateKey() + if err != nil { + panic("Could not generate ethsecp256k1 private key") + } + privKeys[i] = privKey + pubKeys[i] = privKey.PubKey() + } + return privKeys, pubKeys +} + +// Signs a set of messages using each private key within a given multi-key +func generateMultikeySignatures(signMode signing.SignMode, privKeys []cryptotypes.PrivKey, msgs [][]byte, mixSignatures bool) (signatures []signing.SignatureData) { + n := len(privKeys) + signatures = make([]signing.SignatureData, n) + + for i := 0; i < n; i++ { + privKey, err := ethsecp256k1.GenerateKey() + if err != nil { + panic("Could not generate ethsecp256k1 private key") + } + + msg, err := eip712.GetEIP712HashForMsg(msgs[i]) + if err != nil { + panic("Could not generate EIP712 hash for message") + } + + // If mixing signatures, sign standard payload for every other iteration + if mixSignatures && i%2 == 0 { + msg = msgs[i] + } + + sig, _ := privKey.Sign(msg) + signatures[i] = &signing.SingleSignatureData{ + SignMode: signMode, + Signature: sig, + } + } + + return signatures +} + +func (suite *AnteTestSuite) CreateTestSignedMultisigTx(privKeys []cryptotypes.PrivKey, pubKeys []cryptotypes.PubKey, signMode signing.SignMode, msg sdk.Msg, chainId string, gas uint64) client.TxBuilder { + if len(privKeys) != len(pubKeys) { + panic("length of private keys and public keys do not match") + } + + // Re-derive multikey + numKeys := len(privKeys) + pk := kmultisig.NewLegacyAminoPubKey(numKeys, pubKeys) + + // Create accounts for each pubkey + for _, pubKey := range pubKeys { + _ = suite.app.AccountKeeper.NewAccountWithAddress(suite.ctx, pubKey.Bytes()) + } + + // Create multi-key account + multiKeyAcc := suite.app.AccountKeeper.NewAccountWithAddress(suite.ctx, pk.Bytes()) + suite.Require().NoError(multiKeyAcc.SetSequence(1)) + suite.app.AccountKeeper.SetAccount(suite.ctx, multiKeyAcc) + + // Update balance for multikey account + suite.app.EvmKeeper.SetBalance(suite.ctx, common.BytesToAddress(pk.Address()), big.NewInt(10000000000)) + + // Set nonce to hard-coded value + nonce := 1 + + // Init builder + txBuilder := suite.clientCtx.TxConfig.NewTxBuilder() + + // Set fees + txBuilder.SetGasLimit(gas) + txBuilder.SetFeePayer(sdk.AccAddress(pk.Address())) + txBuilder.SetFeeAmount(sdk.NewCoins( + sdk.NewCoin( + "aphoton", + sdk.NewInt(10000), + ), + )) + + // Set message + err := txBuilder.SetMsgs([]sdk.Msg{msg}...) + if err != nil { + panic(fmt.Sprintf("could not set messages with error %v", err)) + } + + // Set memo and tip + txBuilder.SetMemo("") + + // Prepare signature field + sig := multisig.NewMultisig(len(pubKeys)) + txBuilder.SetSignatures(signing.SignatureV2{ + PubKey: pk, + Data: sig, + Sequence: uint64(nonce), + }) + + // Create signer infos + signerInfos := make([]authsigning.SignerData, numKeys) + for i, pubKey := range pubKeys { + signerInfos[i] = authsigning.SignerData{ + Address: sdk.AccAddress(pubKey.Address()).String(), + ChainID: chainId, + AccountNumber: uint64(i + 1), + Sequence: 1, + PubKey: pubKey, + } + } + + // Create signer bytes + signerBytes := make([][]byte, numKeys) + for i, signerInfo := range signerInfos { + bz, err := suite.clientCtx.TxConfig.SignModeHandler().GetSignBytes( + signMode, + signerInfo, + txBuilder.GetTx(), + ) + + if err != nil { + panic(fmt.Sprintf("could not get signer bytes with err %v\n", err)) + } + + signerBytes[i] = bz + } + + // Update signature field + sigs := generateMultikeySignatures(signMode, privKeys, signerBytes, true) + bitArray := cryptotypes.NewCompactBitArray(numKeys) + for i := 0; i < numKeys; i++ { + bitArray.SetIndex(i, true) + } + sig.BitArray = bitArray + sig.Signatures = sigs + txBuilder.SetSignatures(signing.SignatureV2{ + PubKey: pk, + Data: sig, + Sequence: uint64(nonce), + }) + + return txBuilder +} + func NextFn(ctx sdk.Context, _ sdk.Tx, _ bool) (sdk.Context, error) { return ctx, nil } diff --git a/cmd/ethermintd/root.go b/cmd/ethermintd/root.go index 5e17d8de06..aaba263207 100644 --- a/cmd/ethermintd/root.go +++ b/cmd/ethermintd/root.go @@ -37,6 +37,7 @@ import ( "github.com/evmos/ethermint/client/debug" "github.com/evmos/ethermint/crypto/hd" "github.com/evmos/ethermint/encoding" + "github.com/evmos/ethermint/ethereum/eip712" "github.com/evmos/ethermint/server" servercfg "github.com/evmos/ethermint/server/config" srvflags "github.com/evmos/ethermint/server/flags" @@ -61,6 +62,8 @@ func NewRootCmd() (*cobra.Command, params.EncodingConfig) { WithKeyringOptions(hd.EthSecp256k1Option()). WithViper(EnvPrefix) + eip712.SetEncodingConfig(encodingConfig) + rootCmd := &cobra.Command{ Use: "ethermintd", Short: "Ethermint Daemon", diff --git a/crypto/ethsecp256k1/ethsecp256k1.go b/crypto/ethsecp256k1/ethsecp256k1.go index 38f16c336a..98096f707a 100644 --- a/crypto/ethsecp256k1/ethsecp256k1.go +++ b/crypto/ethsecp256k1/ethsecp256k1.go @@ -10,7 +10,7 @@ import ( cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" "github.com/ethereum/go-ethereum/crypto" - + "github.com/evmos/ethermint/ethereum/eip712" tmcrypto "github.com/tendermint/tendermint/crypto" ) @@ -201,12 +201,20 @@ func (pubKey *PubKey) UnmarshalAminoJSON(bz []byte) error { return pubKey.UnmarshalAmino(bz) } -// VerifySignature verifies that the ECDSA public key created a given signature over -// the provided message. It will calculate the Keccak256 hash of the message -// prior to verification. -// -// CONTRACT: The signature should be in [R || S] format. -func (pubKey PubKey) VerifySignature(msg, sig []byte) bool { +// Verifies the signature as an EIP-712 signature by first converting the message payload +// to an EIP-712 hashed object, performing ECDSA verification on the hash. This is to support +// signing a Cosmos payload using EIP-712. +func (pubKey PubKey) verifySignatureAsEIP712(msg, sig []byte) bool { + eip712Hash, err := eip712.GetEIP712HashForMsg(msg) + if err != nil { + return false + } + + return pubKey.verifySignatureECDSA(eip712Hash, sig) +} + +// Perform standard ECDSA signature verification for the given raw bytes and signature. +func (pubKey PubKey) verifySignatureECDSA(msg, sig []byte) bool { if len(sig) == crypto.SignatureLength { // remove recovery ID (V) if contained in the signature sig = sig[:len(sig)-1] @@ -215,3 +223,12 @@ func (pubKey PubKey) VerifySignature(msg, sig []byte) bool { // the signature needs to be in [R || S] format when provided to VerifySignature return crypto.VerifySignature(pubKey.Key, crypto.Keccak256Hash(msg).Bytes(), sig) } + +// VerifySignature verifies that the ECDSA public key created a given signature over +// the provided message. It will calculate the Keccak256 hash of the message +// prior to verification. +// +// CONTRACT: The signature should be in [R || S] format. +func (pubKey PubKey) VerifySignature(msg, sig []byte) bool { + return pubKey.verifySignatureECDSA(msg, sig) || pubKey.verifySignatureAsEIP712(msg, sig) +} diff --git a/ethereum/eip712/eip712_test.go b/ethereum/eip712/eip712_test.go new file mode 100644 index 0000000000..c69ee1265e --- /dev/null +++ b/ethereum/eip712/eip712_test.go @@ -0,0 +1,293 @@ +package eip712_test + +import ( + "testing" + + "cosmossdk.io/math" + "github.com/stretchr/testify/require" + + "github.com/cosmos/cosmos-sdk/client" + "github.com/ethereum/go-ethereum/crypto" + "github.com/evmos/ethermint/ethereum/eip712" + + cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/evmos/ethermint/crypto/ethsecp256k1" + + banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" + "github.com/evmos/ethermint/app" + "github.com/evmos/ethermint/encoding" + + txtypes "github.com/cosmos/cosmos-sdk/types/tx" + "github.com/cosmos/cosmos-sdk/types/tx/signing" + authsigning "github.com/cosmos/cosmos-sdk/x/auth/signing" + + govtypes "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1" +) + +// Tests single-signer EIP-712 signature verification. Multi-signer verification tests are included +// in ante/integration test files. + +var config = encoding.MakeConfig(app.ModuleBasics) +var clientCtx = client.Context{}.WithTxConfig(config.TxConfig) + +// Set up test env to replicate prod. environment +func setupTestEnv(t *testing.T) { + t.Helper() + + sdk.GetConfig().SetBech32PrefixForAccount("ethm", "") + eip712.SetEncodingConfig(config) +} + +// Helper to create random test addresses for messages +func createTestAddress(t *testing.T) sdk.AccAddress { + t.Helper() + + privkey, _ := ethsecp256k1.GenerateKey() + key, err := privkey.ToECDSA() + require.NoError(t, err) + + addr := crypto.PubkeyToAddress(key.PublicKey) + + return addr.Bytes() +} + +// Helper to create random keypair for signing + verification +func createTestKeyPair(t *testing.T) (*ethsecp256k1.PrivKey, *ethsecp256k1.PubKey) { + t.Helper() + + privKey, err := ethsecp256k1.GenerateKey() + require.NoError(t, err) + + pubKey := ðsecp256k1.PubKey{ + Key: privKey.PubKey().Bytes(), + } + require.Implements(t, (*cryptotypes.PubKey)(nil), pubKey) + + return privKey, pubKey +} + +// Helper to create instance of sdk.Coins[] with single coin +func makeCoins(t *testing.T, denom string, amount math.Int) sdk.Coins { + t.Helper() + + return sdk.NewCoins( + sdk.NewCoin( + denom, + amount, + ), + ) +} + +func TestEIP712SignatureVerification(t *testing.T) { + setupTestEnv(t) + + signModes := []signing.SignMode{ + signing.SignMode_SIGN_MODE_DIRECT, + signing.SignMode_SIGN_MODE_LEGACY_AMINO_JSON, + } + + testCases := []struct { + title string + fee txtypes.Fee + memo string + msgs []sdk.Msg + accountNumber uint64 + sequence uint64 + tip txtypes.Tip + expectSuccess bool + }{ + { + title: "Standard MsgSend", + fee: txtypes.Fee{ + Amount: makeCoins(t, "aphoton", math.NewInt(2000)), + GasLimit: 20000, + Payer: sdk.MustBech32ifyAddressBytes("ethm", createTestAddress(t)), + }, + memo: "", + msgs: []sdk.Msg{ + banktypes.NewMsgSend( + createTestAddress(t), + createTestAddress(t), + makeCoins(t, "photon", math.NewInt(1)), + ), + }, + accountNumber: 8, + sequence: 5, + tip: txtypes.Tip{ + Amount: makeCoins(t, "aphoton", math.NewInt(20000)), + Tipper: sdk.MustBech32ifyAddressBytes("ethm", createTestAddress(t)), + }, + expectSuccess: true, + }, + { + title: "Standard MsgVote", + fee: txtypes.Fee{ + Amount: makeCoins(t, "aphoton", math.NewInt(2000)), + GasLimit: 20000, + Payer: sdk.MustBech32ifyAddressBytes("ethm", createTestAddress(t)), + }, + memo: "", + msgs: []sdk.Msg{ + govtypes.NewMsgVote( + createTestAddress(t), + 5, + govtypes.OptionNo, + ), + }, + accountNumber: 25, + sequence: 78, + tip: txtypes.Tip{ + Amount: makeCoins(t, "aphoton", math.NewInt(500000)), + Tipper: sdk.MustBech32ifyAddressBytes("ethm", createTestAddress(t)), + }, + expectSuccess: true, + }, + { + title: "Two MsgVotes", + fee: txtypes.Fee{ + Amount: makeCoins(t, "aphoton", math.NewInt(2000)), + GasLimit: 20000, + Payer: sdk.MustBech32ifyAddressBytes("ethm", createTestAddress(t)), + }, + memo: "", + msgs: []sdk.Msg{ + govtypes.NewMsgVote( + createTestAddress(t), + 5, + govtypes.OptionNo, + ), + govtypes.NewMsgVote( + createTestAddress(t), + 25, + govtypes.OptionAbstain, + ), + }, + accountNumber: 25, + sequence: 78, + tip: txtypes.Tip{ + Amount: makeCoins(t, "aphoton", math.NewInt(500000)), + Tipper: sdk.MustBech32ifyAddressBytes("ethm", createTestAddress(t)), + }, + expectSuccess: false, // Multiple messages (check for multiple signers in AnteHandler) + }, + { + title: "MsgSend + MsgVote", + fee: txtypes.Fee{ + Amount: makeCoins(t, "aphoton", math.NewInt(2000)), + GasLimit: 20000, + Payer: sdk.MustBech32ifyAddressBytes("ethm", createTestAddress(t)), + }, + memo: "", + msgs: []sdk.Msg{ + govtypes.NewMsgVote( + createTestAddress(t), + 5, + govtypes.OptionNo, + ), + banktypes.NewMsgSend( + createTestAddress(t), + createTestAddress(t), + makeCoins(t, "photon", math.NewInt(50)), + ), + }, + accountNumber: 25, + sequence: 78, + tip: txtypes.Tip{ + Amount: makeCoins(t, "aphoton", math.NewInt(500000)), + Tipper: sdk.MustBech32ifyAddressBytes("ethm", createTestAddress(t)), + }, + expectSuccess: false, // Multiple messages + }, + } + + for _, tc := range testCases { + for _, signMode := range signModes { + privKey, pubKey := createTestKeyPair(t) + + // Init tx builder + txBuilder := clientCtx.TxConfig.NewTxBuilder() + + // Set fees + txBuilder.SetGasLimit(tc.fee.GasLimit) + txBuilder.SetFeePayer(sdk.MustAccAddressFromBech32(tc.fee.Payer)) + txBuilder.SetFeeAmount(tc.fee.Amount) + + // Set messages + err := txBuilder.SetMsgs(tc.msgs...) + require.NoError(t, err) + + // Set memo and tip + txBuilder.SetMemo(tc.memo) + txBuilder.SetTip(&tc.tip) + + // Prepare signature field + txSigData := signing.SingleSignatureData{ + SignMode: signMode, + Signature: nil, + } + txSig := signing.SignatureV2{ + PubKey: pubKey, + Data: &txSigData, + Sequence: tc.sequence, + } + + err = txBuilder.SetSignatures([]signing.SignatureV2{ + txSig, + }...) + require.NoError(t, err) + + // Declare signerData + signerData := authsigning.SignerData{ + ChainID: "ethermint_9000-1", + AccountNumber: tc.accountNumber, + Sequence: tc.sequence, + PubKey: pubKey, + Address: sdk.MustBech32ifyAddressBytes("ethm", pubKey.Bytes()), + } + + bz, err := clientCtx.TxConfig.SignModeHandler().GetSignBytes( + signMode, + signerData, + txBuilder.GetTx(), + ) + require.NoError(t, err) + + verifyEIP712SignatureVerification(t, tc.expectSuccess, *privKey, *pubKey, bz) + } + } +} + +// Verify that the payload passes signature verification if signed as its EIP-712 representation. +func verifyEIP712SignatureVerification(t *testing.T, expectedSuccess bool, privKey ethsecp256k1.PrivKey, pubKey ethsecp256k1.PubKey, signBytes []byte) { + t.Helper() + + // Convert to EIP712 hash and sign + eip712Hash, err := eip712.GetEIP712HashForMsg(signBytes) + if expectedSuccess { + require.NoError(t, err) + } else { + // Expect failure generating EIP-712 hash + require.Error(t, err) + return + } + sigHash := crypto.Keccak256Hash(eip712Hash) + sig, err := privKey.Sign(sigHash.Bytes()) + require.NoError(t, err) + + // Verify against original payload bytes. This should pass, even though it is not + // the original message that was signed. + res := pubKey.VerifySignature(signBytes, sig) + require.True(t, res) + + // Verify against the signed EIP-712 bytes. This should pass, since it is the message signed. + res = pubKey.VerifySignature(eip712Hash, sig) + require.True(t, res) + + // Verify against random bytes to ensure it does not pass unexpectedly (sanity check). + randBytes := make([]byte, len(signBytes)) + copy(randBytes, signBytes) + randBytes[0] = (signBytes[0] + 10) % 128 + res = pubKey.VerifySignature(randBytes, sig) + require.False(t, res) +} diff --git a/ethereum/eip712/encoding.go b/ethereum/eip712/encoding.go new file mode 100644 index 0000000000..61c0375409 --- /dev/null +++ b/ethereum/eip712/encoding.go @@ -0,0 +1,198 @@ +package eip712 + +import ( + "errors" + "fmt" + + "github.com/cosmos/cosmos-sdk/simapp/params" + "github.com/cosmos/cosmos-sdk/x/auth/migrations/legacytx" + + cosmosTypes "github.com/cosmos/cosmos-sdk/types" + txTypes "github.com/cosmos/cosmos-sdk/types/tx" + + apitypes "github.com/ethereum/go-ethereum/signer/core/apitypes" + ethermint "github.com/evmos/ethermint/types" + + "github.com/cosmos/cosmos-sdk/codec" +) + +var ethermintProtoCodec codec.ProtoCodecMarshaler +var ethermintAminoCodec *codec.LegacyAmino + +func SetEncodingConfig(cfg params.EncodingConfig) { + ethermintAminoCodec = cfg.Amino + ethermintProtoCodec = codec.NewProtoCodec(cfg.InterfaceRegistry) +} + +func GetEIP712HashForMsg(signDocBytes []byte) ([]byte, error) { + var typedData apitypes.TypedData + + typedDataAmino, errAmino := decodeAminoSignDoc(signDocBytes) + typedDataProtobuf, errProtobuf := decodeProtobufSignDoc(signDocBytes) + + if errAmino == nil { + typedData = typedDataAmino + } else if errProtobuf == nil { + typedData = typedDataProtobuf + } else { + return make([]byte, 0), errors.New(fmt.Sprintf("could not decode sign doc as either Amino or Protobuf.\n amino: %v\n protobuf: %v\n", errAmino, errProtobuf)) + } + + return typedData.TypeHash("Tx"), nil +} + +// Attempt to decode the SignDoc bytes as an Amino SignDoc and return an error on failure +func decodeAminoSignDoc(signDocBytes []byte) (apitypes.TypedData, error) { + var ( + aminoDoc legacytx.StdSignDoc + err error + ) + + err = ethermintAminoCodec.UnmarshalJSON(signDocBytes, &aminoDoc) + if err != nil { + return apitypes.TypedData{}, err + } + + // Unwrap fees + var fees legacytx.StdFee + err = ethermintAminoCodec.UnmarshalJSON(aminoDoc.Fee, &fees) + if err != nil { + return apitypes.TypedData{}, err + } + + if len(aminoDoc.Msgs) != 1 { + return apitypes.TypedData{}, errors.New(fmt.Sprintf("Invalid number of messages in SignDoc, expected 1 but got %v\n", len(aminoDoc.Msgs))) + } + + var msg cosmosTypes.Msg + err = ethermintAminoCodec.UnmarshalJSON(aminoDoc.Msgs[0], &msg) + if err != nil { + fmt.Printf("Encountered err %v\n", err) + 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") + } + 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, + chainID.Uint64(), + msg, + signDocBytes, // Amino StdSignDocBytes + feeDelegation, + ) + + if err != nil { + return apitypes.TypedData{}, errors.New(fmt.Sprintf("Could not convert to EIP712 representation: %v\n", err)) + } + + return typedData, nil +} + +// Attempt to decode the SignDoc bytes as a Protobuf SignDoc and return an error on failure +func decodeProtobufSignDoc(signDocBytes []byte) (apitypes.TypedData, error) { + // Decode sign doc + signDoc := &txTypes.SignDoc{} + err := signDoc.Unmarshal(signDocBytes) + if err != nil { + return apitypes.TypedData{}, err + } + + // Decode auth info + authInfo := &txTypes.AuthInfo{} + err = authInfo.Unmarshal(signDoc.AuthInfoBytes) + if err != nil { + return apitypes.TypedData{}, err + } + + // Decode body + body := &txTypes.TxBody{} + err = body.Unmarshal(signDoc.BodyBytes) + if err != nil { + return apitypes.TypedData{}, err + } + + // Until support for these fields is added, throw an error at their presence + if body.TimeoutHeight != 0 || len(body.ExtensionOptions) != 0 || len(body.NonCriticalExtensionOptions) != 0 { + return apitypes.TypedData{}, errors.New("Body contains unsupported fields: TimeoutHeight, ExtensionOptions, or NonCriticalExtensionOptions") + } + + // Verify single message + if len(body.Messages) != 1 { + return apitypes.TypedData{}, errors.New(fmt.Sprintf("Invalid number of messages, expected 1 got %v\n", len(body.Messages))) + } + + // Verify single signature (single signer for now) + if len(authInfo.SignerInfos) != 1 { + return apitypes.TypedData{}, errors.New(fmt.Sprintf("Invalid number of signers, expected 1 got %v\n", len(authInfo.SignerInfos))) + } + + // Decode signer info (single signer for now) + signerInfo := authInfo.SignerInfos[0] + + // Parse ChainID + chainID, err := ethermint.ParseChainID(signDoc.ChainId) + if err != nil { + return apitypes.TypedData{}, errors.New("Invalid chain ID passed as argument") + } + + // Create StdFee + stdFee := &legacytx.StdFee{ + Amount: authInfo.Fee.Amount, + Gas: authInfo.Fee.GasLimit, + } + + // Parse Message (single message only) + var msg cosmosTypes.Msg + err = ethermintProtoCodec.UnpackAny(body.Messages[0], &msg) + if err != nil { + return apitypes.TypedData{}, errors.New(fmt.Sprintf("Could not unpack message object with error %v\n", err)) + } + + // Init fee payer + feePayer := msg.GetSigners()[0] + feeDelegation := &FeeDelegationOptions{ + FeePayer: feePayer, + } + + // Get tip + tip := authInfo.Tip + + // Create Legacy SignBytes (expected type for WrapTxToTypedData) + signBytes := legacytx.StdSignBytes( + signDoc.ChainId, + signDoc.AccountNumber, + signerInfo.Sequence, + body.TimeoutHeight, + *stdFee, + []cosmosTypes.Msg{msg}, + body.Memo, + tip, + ) + + typedData, err := WrapTxToTypedData( + ethermintProtoCodec, + chainID.Uint64(), + msg, + signBytes, + feeDelegation, + ) + if err != nil { + return apitypes.TypedData{}, err + } + + return typedData, nil +} From e4e865033d21fa0dbe2e1afadc9e5796e2ec681f Mon Sep 17 00:00:00 2001 From: Austin Chandra Date: Thu, 20 Oct 2022 16:15:11 -0700 Subject: [PATCH 02/31] Debug and add ante tests --- app/ante/ante_test.go | 38 +++++++++- app/ante/utils_test.go | 129 +++++++++++++++------------------ ethereum/eip712/eip712_test.go | 27 +------ ethereum/eip712/encoding.go | 13 +++- 4 files changed, 105 insertions(+), 102 deletions(-) diff --git a/app/ante/ante_test.go b/app/ante/ante_test.go index 0d9ecf3e91..92ab682be3 100644 --- a/app/ante/ante_test.go +++ b/app/ante/ante_test.go @@ -30,7 +30,6 @@ import ( "github.com/evmos/ethermint/tests" evmtypes "github.com/evmos/ethermint/x/evm/types" - kmultisig "github.com/cosmos/cosmos-sdk/crypto/keys/multisig" banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" ) @@ -514,6 +513,37 @@ func (suite AnteTestSuite) TestAnteHandler() { numKeys := 5 privKeys, pubKeys := suite.GenerateMultipleKeys(numKeys) pk := kmultisig.NewLegacyAminoPubKey(numKeys, pubKeys) + + msg := banktypes.NewMsgSend( + sdk.AccAddress(pk.Address()), + addr[:], + sdk.NewCoins( + sdk.NewCoin( + "photon", + sdk.NewInt(1), + ), + ), + ) + + txBuilder := suite.CreateTestSignedMultisigTx( + privKeys, + signing.SignMode_SIGN_MODE_LEGACY_AMINO_JSON, + msg, + "ethermint_9000-1", + 2000000, + "EIP-712", + ) + + return txBuilder.GetTx() + }, false, false, true, + }, + { + "passes - Mixed multi-key", + func() sdk.Tx { + numKeys := 5 + privKeys, pubKeys := suite.GenerateMultipleKeys(numKeys) + pk := kmultisig.NewLegacyAminoPubKey(numKeys, pubKeys) + msg := banktypes.NewMsgSend( sdk.AccAddress(pk.Address()), addr[:], @@ -527,11 +557,11 @@ func (suite AnteTestSuite) TestAnteHandler() { txBuilder := suite.CreateTestSignedMultisigTx( privKeys, - pubKeys, - signing.SignMode_SIGN_MODE_DIRECT, + signing.SignMode_SIGN_MODE_LEGACY_AMINO_JSON, msg, "ethermint_9000-1", 2000000, + "mixed", // Combine EIP-712 and standard signatures ) return txBuilder.GetTx() @@ -969,7 +999,7 @@ func (suite *AnteTestSuite) TestConsumeSignatureVerificationGas() { multisigKey1 := kmultisig.NewLegacyAminoPubKey(2, pkSet1) multisignature1 := multisig.NewMultisig(len(pkSet1)) expectedCost1 := expectedGasCostByKeys(pkSet1) - + for i := 0; i < len(pkSet1); i++ { stdSig := legacytx.StdSignature{PubKey: pkSet1[i], Signature: sigSet1[i]} sigV2, err := legacytx.StdSignatureToSignatureV2(cdc, stdSig) diff --git a/app/ante/utils_test.go b/app/ante/utils_test.go index 1115a80440..84d206bf13 100644 --- a/app/ante/utils_test.go +++ b/app/ante/utils_test.go @@ -33,6 +33,7 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" txtypes "github.com/cosmos/cosmos-sdk/types/tx" "github.com/cosmos/cosmos-sdk/types/tx/signing" + sdkante "github.com/cosmos/cosmos-sdk/x/auth/ante" authsigning "github.com/cosmos/cosmos-sdk/x/auth/signing" authtx "github.com/cosmos/cosmos-sdk/x/auth/tx" authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" @@ -468,67 +469,66 @@ func (suite *AnteTestSuite) GenerateMultipleKeys(n int) ([]cryptotypes.PrivKey, } // Signs a set of messages using each private key within a given multi-key -func generateMultikeySignatures(signMode signing.SignMode, privKeys []cryptotypes.PrivKey, msgs [][]byte, mixSignatures bool) (signatures []signing.SignatureData) { +func generateMultikeySignatures(signMode signing.SignMode, privKeys []cryptotypes.PrivKey, signDocBytes []byte, signType string) (signatures []signing.SignatureV2) { + var ( + msg []byte + err error + ) + n := len(privKeys) - signatures = make([]signing.SignatureData, n) + signatures = make([]signing.SignatureV2, n) for i := 0; i < n; i++ { - privKey, err := ethsecp256k1.GenerateKey() - if err != nil { - panic("Could not generate ethsecp256k1 private key") - } + privKey := privKeys[i] - msg, err := eip712.GetEIP712HashForMsg(msgs[i]) - if err != nil { - panic("Could not generate EIP712 hash for message") - } + msg = signDocBytes - // If mixing signatures, sign standard payload for every other iteration - if mixSignatures && i%2 == 0 { - msg = msgs[i] + if signType == "EIP-712" || (signType == "mixed" && i%2 == 0) { + msg, err = eip712.GetEIP712HashForMsg(signDocBytes) + if err != nil { + panic(fmt.Sprintf("Could not generate EIP712 hash for message: %v\n", err)) + } } - sig, _ := privKey.Sign(msg) - signatures[i] = &signing.SingleSignatureData{ + sigBytes, _ := privKey.Sign(msg) + sigData := &signing.SingleSignatureData{ SignMode: signMode, - Signature: sig, + Signature: sigBytes, + } + + signatures[i] = signing.SignatureV2{ + PubKey: privKey.PubKey(), + Data: sigData, } } return signatures } -func (suite *AnteTestSuite) CreateTestSignedMultisigTx(privKeys []cryptotypes.PrivKey, pubKeys []cryptotypes.PubKey, signMode signing.SignMode, msg sdk.Msg, chainId string, gas uint64) client.TxBuilder { - if len(privKeys) != len(pubKeys) { - panic("length of private keys and public keys do not match") +// Create and sign a multi-signed tx for the given message. `signType` indicates whether to use standard signing ("Standard"), +// EIP-712 signing ("EIP-712"), or a mix of the two ("mixed"). +func (suite *AnteTestSuite) CreateTestSignedMultisigTx(privKeys []cryptotypes.PrivKey, signMode signing.SignMode, msg sdk.Msg, chainId string, gas uint64, signType string) client.TxBuilder { + pubKeys := make([]cryptotypes.PubKey, len(privKeys)) + for i, privKey := range privKeys { + pubKeys[i] = privKey.PubKey() } // Re-derive multikey numKeys := len(privKeys) - pk := kmultisig.NewLegacyAminoPubKey(numKeys, pubKeys) - - // Create accounts for each pubkey - for _, pubKey := range pubKeys { - _ = suite.app.AccountKeeper.NewAccountWithAddress(suite.ctx, pubKey.Bytes()) - } + multiKey := kmultisig.NewLegacyAminoPubKey(numKeys, pubKeys) // Create multi-key account - multiKeyAcc := suite.app.AccountKeeper.NewAccountWithAddress(suite.ctx, pk.Bytes()) - suite.Require().NoError(multiKeyAcc.SetSequence(1)) + multiKeyAcc := suite.app.AccountKeeper.NewAccountWithAddress(suite.ctx, sdk.AccAddress(multiKey.Address())) suite.app.AccountKeeper.SetAccount(suite.ctx, multiKeyAcc) // Update balance for multikey account - suite.app.EvmKeeper.SetBalance(suite.ctx, common.BytesToAddress(pk.Address()), big.NewInt(10000000000)) - - // Set nonce to hard-coded value - nonce := 1 + suite.app.EvmKeeper.SetBalance(suite.ctx, common.BytesToAddress(multiKey.Address()), big.NewInt(10000000000)) // Init builder txBuilder := suite.clientCtx.TxConfig.NewTxBuilder() // Set fees txBuilder.SetGasLimit(gas) - txBuilder.SetFeePayer(sdk.AccAddress(pk.Address())) txBuilder.SetFeeAmount(sdk.NewCoins( sdk.NewCoin( "aphoton", @@ -538,9 +538,7 @@ func (suite *AnteTestSuite) CreateTestSignedMultisigTx(privKeys []cryptotypes.Pr // Set message err := txBuilder.SetMsgs([]sdk.Msg{msg}...) - if err != nil { - panic(fmt.Sprintf("could not set messages with error %v", err)) - } + suite.Require().NoError(err) // Set memo and tip txBuilder.SetMemo("") @@ -548,51 +546,38 @@ func (suite *AnteTestSuite) CreateTestSignedMultisigTx(privKeys []cryptotypes.Pr // Prepare signature field sig := multisig.NewMultisig(len(pubKeys)) txBuilder.SetSignatures(signing.SignatureV2{ - PubKey: pk, - Data: sig, - Sequence: uint64(nonce), + PubKey: multiKey, + Data: sig, }) - // Create signer infos - signerInfos := make([]authsigning.SignerData, numKeys) - for i, pubKey := range pubKeys { - signerInfos[i] = authsigning.SignerData{ - Address: sdk.AccAddress(pubKey.Address()).String(), - ChainID: chainId, - AccountNumber: uint64(i + 1), - Sequence: 1, - PubKey: pubKey, - } - } - // Create signer bytes - signerBytes := make([][]byte, numKeys) - for i, signerInfo := range signerInfos { - bz, err := suite.clientCtx.TxConfig.SignModeHandler().GetSignBytes( - signMode, - signerInfo, - txBuilder.GetTx(), - ) + acc, err := sdkante.GetSignerAcc(suite.ctx, suite.app.AccountKeeper, sdk.AccAddress(multiKey.Address())) + suite.Require().NoError(err) + signerInfo := authsigning.SignerData{ + Address: sdk.MustBech32ifyAddressBytes(sdk.GetConfig().GetBech32AccountAddrPrefix(), acc.GetAddress().Bytes()), + ChainID: chainId, + AccountNumber: acc.GetAccountNumber(), + Sequence: acc.GetSequence(), + PubKey: multiKey, + } - if err != nil { - panic(fmt.Sprintf("could not get signer bytes with err %v\n", err)) - } + signerBytes, err := suite.clientCtx.TxConfig.SignModeHandler().GetSignBytes( + signMode, + signerInfo, + txBuilder.GetTx(), + ) + suite.Require().NoError(err) - signerBytes[i] = bz + // Sign for each key and update signature field + sigs := generateMultikeySignatures(signMode, privKeys, signerBytes, signType) + for _, pkSig := range sigs { + err = multisig.AddSignatureV2(sig, pkSig, pubKeys) + suite.Require().NoError(err) } - // Update signature field - sigs := generateMultikeySignatures(signMode, privKeys, signerBytes, true) - bitArray := cryptotypes.NewCompactBitArray(numKeys) - for i := 0; i < numKeys; i++ { - bitArray.SetIndex(i, true) - } - sig.BitArray = bitArray - sig.Signatures = sigs txBuilder.SetSignatures(signing.SignatureV2{ - PubKey: pk, - Data: sig, - Sequence: uint64(nonce), + PubKey: multiKey, + Data: sig, }) return txBuilder diff --git a/ethereum/eip712/eip712_test.go b/ethereum/eip712/eip712_test.go index c69ee1265e..ccf70753fa 100644 --- a/ethereum/eip712/eip712_test.go +++ b/ethereum/eip712/eip712_test.go @@ -94,7 +94,6 @@ func TestEIP712SignatureVerification(t *testing.T) { msgs []sdk.Msg accountNumber uint64 sequence uint64 - tip txtypes.Tip expectSuccess bool }{ { @@ -102,7 +101,6 @@ func TestEIP712SignatureVerification(t *testing.T) { fee: txtypes.Fee{ Amount: makeCoins(t, "aphoton", math.NewInt(2000)), GasLimit: 20000, - Payer: sdk.MustBech32ifyAddressBytes("ethm", createTestAddress(t)), }, memo: "", msgs: []sdk.Msg{ @@ -114,10 +112,6 @@ func TestEIP712SignatureVerification(t *testing.T) { }, accountNumber: 8, sequence: 5, - tip: txtypes.Tip{ - Amount: makeCoins(t, "aphoton", math.NewInt(20000)), - Tipper: sdk.MustBech32ifyAddressBytes("ethm", createTestAddress(t)), - }, expectSuccess: true, }, { @@ -125,7 +119,6 @@ func TestEIP712SignatureVerification(t *testing.T) { fee: txtypes.Fee{ Amount: makeCoins(t, "aphoton", math.NewInt(2000)), GasLimit: 20000, - Payer: sdk.MustBech32ifyAddressBytes("ethm", createTestAddress(t)), }, memo: "", msgs: []sdk.Msg{ @@ -137,10 +130,6 @@ func TestEIP712SignatureVerification(t *testing.T) { }, accountNumber: 25, sequence: 78, - tip: txtypes.Tip{ - Amount: makeCoins(t, "aphoton", math.NewInt(500000)), - Tipper: sdk.MustBech32ifyAddressBytes("ethm", createTestAddress(t)), - }, expectSuccess: true, }, { @@ -148,7 +137,6 @@ func TestEIP712SignatureVerification(t *testing.T) { fee: txtypes.Fee{ Amount: makeCoins(t, "aphoton", math.NewInt(2000)), GasLimit: 20000, - Payer: sdk.MustBech32ifyAddressBytes("ethm", createTestAddress(t)), }, memo: "", msgs: []sdk.Msg{ @@ -165,10 +153,6 @@ func TestEIP712SignatureVerification(t *testing.T) { }, accountNumber: 25, sequence: 78, - tip: txtypes.Tip{ - Amount: makeCoins(t, "aphoton", math.NewInt(500000)), - Tipper: sdk.MustBech32ifyAddressBytes("ethm", createTestAddress(t)), - }, expectSuccess: false, // Multiple messages (check for multiple signers in AnteHandler) }, { @@ -176,7 +160,6 @@ func TestEIP712SignatureVerification(t *testing.T) { fee: txtypes.Fee{ Amount: makeCoins(t, "aphoton", math.NewInt(2000)), GasLimit: 20000, - Payer: sdk.MustBech32ifyAddressBytes("ethm", createTestAddress(t)), }, memo: "", msgs: []sdk.Msg{ @@ -193,10 +176,6 @@ func TestEIP712SignatureVerification(t *testing.T) { }, accountNumber: 25, sequence: 78, - tip: txtypes.Tip{ - Amount: makeCoins(t, "aphoton", math.NewInt(500000)), - Tipper: sdk.MustBech32ifyAddressBytes("ethm", createTestAddress(t)), - }, expectSuccess: false, // Multiple messages }, } @@ -208,18 +187,16 @@ func TestEIP712SignatureVerification(t *testing.T) { // Init tx builder txBuilder := clientCtx.TxConfig.NewTxBuilder() - // Set fees + // Set gas and fees txBuilder.SetGasLimit(tc.fee.GasLimit) - txBuilder.SetFeePayer(sdk.MustAccAddressFromBech32(tc.fee.Payer)) txBuilder.SetFeeAmount(tc.fee.Amount) // Set messages err := txBuilder.SetMsgs(tc.msgs...) require.NoError(t, err) - // Set memo and tip + // Set memo txBuilder.SetMemo(tc.memo) - txBuilder.SetTip(&tc.tip) // Prepare signature field txSigData := signing.SingleSignatureData{ diff --git a/ethereum/eip712/encoding.go b/ethereum/eip712/encoding.go index 61c0375409..4c4f7616ac 100644 --- a/ethereum/eip712/encoding.go +++ b/ethereum/eip712/encoding.go @@ -38,7 +38,18 @@ func GetEIP712HashForMsg(signDocBytes []byte) ([]byte, error) { return make([]byte, 0), errors.New(fmt.Sprintf("could not decode sign doc as either Amino or Protobuf.\n amino: %v\n protobuf: %v\n", errAmino, errProtobuf)) } - return typedData.TypeHash("Tx"), nil + domainSeparator, err := typedData.HashStruct("EIP712Domain", typedData.Domain.Map()) + if err != nil { + return nil, errors.New(fmt.Sprintf("could not hash EIP-712 domain: %v", err)) + } + typedDataHash, err := typedData.HashStruct(typedData.PrimaryType, typedData.Message) + + if err != nil { + return nil, errors.New(fmt.Sprintf("could not hash EIP-712 primary type: %v", err)) + } + rawData := []byte(fmt.Sprintf("\x19\x01%s%s", string(domainSeparator), string(typedDataHash))) + + return rawData, nil } // Attempt to decode the SignDoc bytes as an Amino SignDoc and return an error on failure From afc37f6dc75148cf572945853944b76f5bf932b2 Mon Sep 17 00:00:00 2001 From: Austin Chandra Date: Fri, 21 Oct 2022 10:21:30 -0700 Subject: [PATCH 03/31] Add tests for failure cases --- app/ante/ante_test.go | 116 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 116 insertions(+) diff --git a/app/ante/ante_test.go b/app/ante/ante_test.go index 92ab682be3..508a8175f6 100644 --- a/app/ante/ante_test.go +++ b/app/ante/ante_test.go @@ -31,6 +31,7 @@ import ( evmtypes "github.com/evmos/ethermint/x/evm/types" banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" + govtypes "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1" ) func (suite AnteTestSuite) TestAnteHandler() { @@ -567,6 +568,121 @@ func (suite AnteTestSuite) TestAnteHandler() { return txBuilder.GetTx() }, false, false, true, }, + { + "passes - Mixed multi-key with MsgVote", + func() sdk.Tx { + numKeys := 5 + privKeys, pubKeys := suite.GenerateMultipleKeys(numKeys) + pk := kmultisig.NewLegacyAminoPubKey(numKeys, pubKeys) + + msg := govtypes.NewMsgVote( + sdk.AccAddress(pk.Address()), + 1, + govtypes.OptionYes, + ) + + txBuilder := suite.CreateTestSignedMultisigTx( + privKeys, + signing.SignMode_SIGN_MODE_LEGACY_AMINO_JSON, + msg, + "ethermint_9000-1", + 2000000, + "mixed", // Combine EIP-712 and standard signatures + ) + + return txBuilder.GetTx() + }, false, false, true, + }, + { + "Fails - Multi-Key with incorrect Chain ID", + func() sdk.Tx { + numKeys := 5 + privKeys, pubKeys := suite.GenerateMultipleKeys(numKeys) + pk := kmultisig.NewLegacyAminoPubKey(numKeys, pubKeys) + + msg := banktypes.NewMsgSend( + sdk.AccAddress(pk.Address()), + addr[:], + sdk.NewCoins( + sdk.NewCoin( + "photon", + sdk.NewInt(1), + ), + ), + ) + + txBuilder := suite.CreateTestSignedMultisigTx( + privKeys, + signing.SignMode_SIGN_MODE_LEGACY_AMINO_JSON, + msg, + "ethermint_9005-1", + 2000000, + "mixed", + ) + + return txBuilder.GetTx() + }, false, false, false, + }, + { + "Fails - Multi-Key with incorrect sign mode", + func() sdk.Tx { + numKeys := 5 + privKeys, pubKeys := suite.GenerateMultipleKeys(numKeys) + pk := kmultisig.NewLegacyAminoPubKey(numKeys, pubKeys) + + msg := banktypes.NewMsgSend( + sdk.AccAddress(pk.Address()), + addr[:], + sdk.NewCoins( + sdk.NewCoin( + "photon", + sdk.NewInt(1), + ), + ), + ) + + txBuilder := suite.CreateTestSignedMultisigTx( + privKeys, + signing.SignMode_SIGN_MODE_DIRECT, + msg, + "ethermint_9000-1", + 2000000, + "mixed", + ) + + return txBuilder.GetTx() + }, false, false, false, + }, + { + "Fails - Multi-Key with too little gas", + func() sdk.Tx { + numKeys := 5 + privKeys, pubKeys := suite.GenerateMultipleKeys(numKeys) + pk := kmultisig.NewLegacyAminoPubKey(numKeys, pubKeys) + + msg := banktypes.NewMsgSend( + sdk.AccAddress(pk.Address()), + addr[:], + sdk.NewCoins( + sdk.NewCoin( + "photon", + sdk.NewInt(1), + ), + ), + ) + + txBuilder := suite.CreateTestSignedMultisigTx( + privKeys, + signing.SignMode_SIGN_MODE_DIRECT, + msg, + "ethermint_9000-1", + 2000, + "mixed", // Combine EIP-712 and standard signatures + ) + + return txBuilder.GetTx() + }, false, false, false, + }, } for _, tc := range testCases { From 57fe441e9561ce6470b646f41b540b1ce5ed111b Mon Sep 17 00:00:00 2001 From: Austin Chandra Date: Fri, 21 Oct 2022 10:32:04 -0700 Subject: [PATCH 04/31] Add changelog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index be40b4d92c..69c0b03a0b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -64,6 +64,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (cli) [#1360](https://github.com/evmos/ethermint/pull/1360) Introduce a new `grpc-only` flag, such that when enabled, will start the node in a query-only mode. Note, gRPC MUST be enabled with this flag. * (rpc) [#1378](https://github.com/evmos/ethermint/pull/1378) Add support for EVM RPC metrics * (ante) [#1390](https://github.com/evmos/ethermint/pull/1390) Added multisig tx support. +* (ante) [#1397](https://github.com/evmos/ethermint/pull/1397) Refactor EIP-712 signature verification to support EIP-712 multi-signing. ### Bug Fixes From 998987a6009fb2e8d36cdd34b3aa90ad3c262ab7 Mon Sep 17 00:00:00 2001 From: Austin Chandra Date: Mon, 24 Oct 2022 10:59:03 -0700 Subject: [PATCH 05/31] Code cleanup --- app/ante/utils_test.go | 12 ++++------ ethereum/eip712/eip712_test.go | 8 ++++--- ethereum/eip712/encoding.go | 42 ++++++++++++++++++++-------------- 3 files changed, 34 insertions(+), 28 deletions(-) diff --git a/app/ante/utils_test.go b/app/ante/utils_test.go index 84d206bf13..0fa66cb2f8 100644 --- a/app/ante/utils_test.go +++ b/app/ante/utils_test.go @@ -459,9 +459,7 @@ func (suite *AnteTestSuite) GenerateMultipleKeys(n int) ([]cryptotypes.PrivKey, pubKeys := make([]cryptotypes.PubKey, n) for i := 0; i < n; i++ { privKey, err := ethsecp256k1.GenerateKey() - if err != nil { - panic("Could not generate ethsecp256k1 private key") - } + suite.Require().NoError(err) privKeys[i] = privKey pubKeys[i] = privKey.PubKey() } @@ -469,7 +467,7 @@ func (suite *AnteTestSuite) GenerateMultipleKeys(n int) ([]cryptotypes.PrivKey, } // Signs a set of messages using each private key within a given multi-key -func generateMultikeySignatures(signMode signing.SignMode, privKeys []cryptotypes.PrivKey, signDocBytes []byte, signType string) (signatures []signing.SignatureV2) { +func (suite *AnteTestSuite) generateMultikeySignatures(signMode signing.SignMode, privKeys []cryptotypes.PrivKey, signDocBytes []byte, signType string) (signatures []signing.SignatureV2) { var ( msg []byte err error @@ -485,9 +483,7 @@ func generateMultikeySignatures(signMode signing.SignMode, privKeys []cryptotype if signType == "EIP-712" || (signType == "mixed" && i%2 == 0) { msg, err = eip712.GetEIP712HashForMsg(signDocBytes) - if err != nil { - panic(fmt.Sprintf("Could not generate EIP712 hash for message: %v\n", err)) - } + suite.Require().NoError(err) } sigBytes, _ := privKey.Sign(msg) @@ -569,7 +565,7 @@ func (suite *AnteTestSuite) CreateTestSignedMultisigTx(privKeys []cryptotypes.Pr suite.Require().NoError(err) // Sign for each key and update signature field - sigs := generateMultikeySignatures(signMode, privKeys, signerBytes, signType) + sigs := suite.generateMultikeySignatures(signMode, privKeys, signerBytes, signType) for _, pkSig := range sigs { err = multisig.AddSignatureV2(sig, pkSig, pubKeys) suite.Require().NoError(err) diff --git a/ethereum/eip712/eip712_test.go b/ethereum/eip712/eip712_test.go index ccf70753fa..be7cf2e028 100644 --- a/ethereum/eip712/eip712_test.go +++ b/ethereum/eip712/eip712_test.go @@ -241,13 +241,14 @@ func verifyEIP712SignatureVerification(t *testing.T, expectedSuccess bool, privK // Convert to EIP712 hash and sign eip712Hash, err := eip712.GetEIP712HashForMsg(signBytes) - if expectedSuccess { - require.NoError(t, err) - } else { + if !expectedSuccess { // Expect failure generating EIP-712 hash require.Error(t, err) return } + + require.NoError(t, err) + sigHash := crypto.Keccak256Hash(eip712Hash) sig, err := privKey.Sign(sigHash.Bytes()) require.NoError(t, err) @@ -264,6 +265,7 @@ func verifyEIP712SignatureVerification(t *testing.T, expectedSuccess bool, privK // Verify against random bytes to ensure it does not pass unexpectedly (sanity check). randBytes := make([]byte, len(signBytes)) copy(randBytes, signBytes) + // Change the first element of signBytes to a different value randBytes[0] = (signBytes[0] + 10) % 128 res = pubKey.VerifySignature(randBytes, sig) require.False(t, res) diff --git a/ethereum/eip712/encoding.go b/ethereum/eip712/encoding.go index 4c4f7616ac..a9f00572f2 100644 --- a/ethereum/eip712/encoding.go +++ b/ethereum/eip712/encoding.go @@ -19,33 +19,42 @@ import ( var ethermintProtoCodec codec.ProtoCodecMarshaler var ethermintAminoCodec *codec.LegacyAmino +// The process of unmarshaling SignDoc bytes into a SignDoc object requires having a codec +// 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) } +// Get the EIP-712 object hash for the given SignDoc bytes by first decoding the bytes into +// an EIP-712 object, then hashing the EIP-712 object to create the bytes to be signed. +// See https://eips.ethereum.org/EIPS/eip-712 for more. func GetEIP712HashForMsg(signDocBytes []byte) ([]byte, error) { var typedData apitypes.TypedData + // 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) typedDataProtobuf, errProtobuf := decodeProtobufSignDoc(signDocBytes) - if errAmino == nil { + switch { + case errAmino == nil: typedData = typedDataAmino - } else if errProtobuf == nil { + case errProtobuf == nil: typedData = typedDataProtobuf - } else { - return make([]byte, 0), errors.New(fmt.Sprintf("could not decode sign doc as either Amino or Protobuf.\n amino: %v\n protobuf: %v\n", errAmino, errProtobuf)) + default: + return make([]byte, 0), fmt.Errorf("could not decode sign doc as either Amino or Protobuf.\n amino: %v\n protobuf: %v\n", errAmino, errProtobuf) } domainSeparator, err := typedData.HashStruct("EIP712Domain", typedData.Domain.Map()) if err != nil { - return nil, errors.New(fmt.Sprintf("could not hash EIP-712 domain: %v", err)) + return nil, fmt.Errorf("could not hash EIP-712 domain: %w", err) } typedDataHash, err := typedData.HashStruct(typedData.PrimaryType, typedData.Message) if err != nil { - return nil, errors.New(fmt.Sprintf("could not hash EIP-712 primary type: %v", err)) + return nil, fmt.Errorf("could not hash EIP-712 primary type: %w", err) } rawData := []byte(fmt.Sprintf("\x19\x01%s%s", string(domainSeparator), string(typedDataHash))) @@ -72,20 +81,19 @@ func decodeAminoSignDoc(signDocBytes []byte) (apitypes.TypedData, error) { } if len(aminoDoc.Msgs) != 1 { - return apitypes.TypedData{}, errors.New(fmt.Sprintf("Invalid number of messages in SignDoc, expected 1 but got %v\n", len(aminoDoc.Msgs))) + return apitypes.TypedData{}, fmt.Errorf("Invalid number of messages in SignDoc, expected 1 but got %v\n", len(aminoDoc.Msgs)) } var msg cosmosTypes.Msg err = ethermintAminoCodec.UnmarshalJSON(aminoDoc.Msgs[0], &msg) if err != nil { - fmt.Printf("Encountered err %v\n", err) - return apitypes.TypedData{}, err + return apitypes.TypedData{}, fmt.Errorf("failed to unmarshal first message: %w", 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") + return apitypes.TypedData{}, errors.New("expected exactly one signer for message") } feePayer := msg.GetSigners()[0] feeDelegation := &FeeDelegationOptions{ @@ -95,7 +103,7 @@ func decodeAminoSignDoc(signDocBytes []byte) (apitypes.TypedData, error) { // Parse ChainID chainID, err := ethermint.ParseChainID(aminoDoc.ChainID) if err != nil { - return apitypes.TypedData{}, errors.New("Invalid chain ID passed as argument") + return apitypes.TypedData{}, errors.New("invalid chain ID passed as argument") } typedData, err := WrapTxToTypedData( @@ -107,7 +115,7 @@ func decodeAminoSignDoc(signDocBytes []byte) (apitypes.TypedData, error) { ) if err != nil { - return apitypes.TypedData{}, errors.New(fmt.Sprintf("Could not convert to EIP712 representation: %v\n", err)) + return apitypes.TypedData{}, fmt.Errorf("could not convert to EIP712 representation: %w\n", err) } return typedData, nil @@ -138,17 +146,17 @@ func decodeProtobufSignDoc(signDocBytes []byte) (apitypes.TypedData, error) { // Until support for these fields is added, throw an error at their presence if body.TimeoutHeight != 0 || len(body.ExtensionOptions) != 0 || len(body.NonCriticalExtensionOptions) != 0 { - return apitypes.TypedData{}, errors.New("Body contains unsupported fields: TimeoutHeight, ExtensionOptions, or NonCriticalExtensionOptions") + return apitypes.TypedData{}, errors.New("body contains unsupported fields: TimeoutHeight, ExtensionOptions, or NonCriticalExtensionOptions") } // Verify single message if len(body.Messages) != 1 { - return apitypes.TypedData{}, errors.New(fmt.Sprintf("Invalid number of messages, expected 1 got %v\n", len(body.Messages))) + return apitypes.TypedData{}, fmt.Errorf("invalid number of messages, expected 1 got %v\n", len(body.Messages)) } // Verify single signature (single signer for now) if len(authInfo.SignerInfos) != 1 { - return apitypes.TypedData{}, errors.New(fmt.Sprintf("Invalid number of signers, expected 1 got %v\n", len(authInfo.SignerInfos))) + return apitypes.TypedData{}, fmt.Errorf("invalid number of signers, expected 1 got %v\n", len(authInfo.SignerInfos)) } // Decode signer info (single signer for now) @@ -157,7 +165,7 @@ func decodeProtobufSignDoc(signDocBytes []byte) (apitypes.TypedData, error) { // Parse ChainID chainID, err := ethermint.ParseChainID(signDoc.ChainId) if err != nil { - return apitypes.TypedData{}, errors.New("Invalid chain ID passed as argument") + return apitypes.TypedData{}, fmt.Errorf("invalid chain ID passed as argument %v\n", chainID) } // Create StdFee @@ -170,7 +178,7 @@ func decodeProtobufSignDoc(signDocBytes []byte) (apitypes.TypedData, error) { var msg cosmosTypes.Msg err = ethermintProtoCodec.UnpackAny(body.Messages[0], &msg) if err != nil { - return apitypes.TypedData{}, errors.New(fmt.Sprintf("Could not unpack message object with error %v\n", err)) + return apitypes.TypedData{}, fmt.Errorf("could not unpack message object with error %w\n", err) } // Init fee payer From 578185ceb34b65e89e7ade4c68ff43fd2849b965 Mon Sep 17 00:00:00 2001 From: Austin Chandra Date: Tue, 25 Oct 2022 10:31:06 -0700 Subject: [PATCH 06/31] Add tests for MsgDelegate and MsgWithdrawDelegationReward --- ethereum/eip712/eip712_test.go | 37 ++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/ethereum/eip712/eip712_test.go b/ethereum/eip712/eip712_test.go index be7cf2e028..d8a3ca3436 100644 --- a/ethereum/eip712/eip712_test.go +++ b/ethereum/eip712/eip712_test.go @@ -22,7 +22,9 @@ import ( "github.com/cosmos/cosmos-sdk/types/tx/signing" authsigning "github.com/cosmos/cosmos-sdk/x/auth/signing" + distributiontypes "github.com/cosmos/cosmos-sdk/x/distribution/types" govtypes "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1" + stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" ) // Tests single-signer EIP-712 signature verification. Multi-signer verification tests are included @@ -132,6 +134,41 @@ func TestEIP712SignatureVerification(t *testing.T) { sequence: 78, expectSuccess: true, }, + { + title: "Standard MsgDelegate", + fee: txtypes.Fee{ + Amount: makeCoins(t, "aphoton", math.NewInt(2000)), + GasLimit: 20000, + }, + memo: "", + msgs: []sdk.Msg{ + stakingtypes.NewMsgDelegate( + createTestAddress(t), + sdk.ValAddress(createTestAddress(t)), + makeCoins(t, "photon", math.NewInt(1))[0], + ), + }, + accountNumber: 25, + sequence: 78, + expectSuccess: true, + }, + { + title: "Standard MsgWithdrawDelegationReward", + fee: txtypes.Fee{ + Amount: makeCoins(t, "aphoton", math.NewInt(2000)), + GasLimit: 20000, + }, + memo: "", + msgs: []sdk.Msg{ + distributiontypes.NewMsgWithdrawDelegatorReward( + createTestAddress(t), + sdk.ValAddress(createTestAddress(t)), + ), + }, + accountNumber: 25, + sequence: 78, + expectSuccess: true, + }, { title: "Two MsgVotes", fee: txtypes.Fee{ From e941c8ea9608e3f67c603386cd93c4b62e2a2126 Mon Sep 17 00:00:00 2001 From: Austin Chandra Date: Tue, 25 Oct 2022 10:34:22 -0700 Subject: [PATCH 07/31] Update ethereum/eip712/encoding.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com> --- ethereum/eip712/encoding.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ethereum/eip712/encoding.go b/ethereum/eip712/encoding.go index a9f00572f2..6f097ab20f 100644 --- a/ethereum/eip712/encoding.go +++ b/ethereum/eip712/encoding.go @@ -68,8 +68,7 @@ func decodeAminoSignDoc(signDocBytes []byte) (apitypes.TypedData, error) { err error ) - err = ethermintAminoCodec.UnmarshalJSON(signDocBytes, &aminoDoc) - if err != nil { + if err := ethermintAminoCodec.UnmarshalJSON(signDocBytes, &aminoDoc); err != nil { return apitypes.TypedData{}, err } From 14c4cd71c0f7d4c33bf874ec888fb884babdfc30 Mon Sep 17 00:00:00 2001 From: Austin Chandra Date: Tue, 25 Oct 2022 10:34:35 -0700 Subject: [PATCH 08/31] Update ethereum/eip712/encoding.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com> --- ethereum/eip712/encoding.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ethereum/eip712/encoding.go b/ethereum/eip712/encoding.go index 6f097ab20f..b4402c38fa 100644 --- a/ethereum/eip712/encoding.go +++ b/ethereum/eip712/encoding.go @@ -74,8 +74,7 @@ func decodeAminoSignDoc(signDocBytes []byte) (apitypes.TypedData, error) { // Unwrap fees var fees legacytx.StdFee - err = ethermintAminoCodec.UnmarshalJSON(aminoDoc.Fee, &fees) - if err != nil { + if err := ethermintAminoCodec.UnmarshalJSON(aminoDoc.Fee, &fees); err != nil { return apitypes.TypedData{}, err } From 84f061a963598deabf31ca346272c116c0eb8b01 Mon Sep 17 00:00:00 2001 From: Austin Chandra Date: Tue, 25 Oct 2022 10:34:46 -0700 Subject: [PATCH 09/31] Update ethereum/eip712/encoding.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com> --- ethereum/eip712/encoding.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ethereum/eip712/encoding.go b/ethereum/eip712/encoding.go index b4402c38fa..49a764021d 100644 --- a/ethereum/eip712/encoding.go +++ b/ethereum/eip712/encoding.go @@ -83,8 +83,7 @@ func decodeAminoSignDoc(signDocBytes []byte) (apitypes.TypedData, error) { } var msg cosmosTypes.Msg - err = ethermintAminoCodec.UnmarshalJSON(aminoDoc.Msgs[0], &msg) - if err != nil { + if err := ethermintAminoCodec.UnmarshalJSON(aminoDoc.Msgs[0], &msg); err != nil { return apitypes.TypedData{}, fmt.Errorf("failed to unmarshal first message: %w", err) } From e93c95e4cceb7588beec054e714032655c1cf849 Mon Sep 17 00:00:00 2001 From: Austin Chandra Date: Tue, 25 Oct 2022 10:34:59 -0700 Subject: [PATCH 10/31] Update ethereum/eip712/encoding.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com> --- ethereum/eip712/encoding.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ethereum/eip712/encoding.go b/ethereum/eip712/encoding.go index 49a764021d..567a2bd04d 100644 --- a/ethereum/eip712/encoding.go +++ b/ethereum/eip712/encoding.go @@ -122,8 +122,7 @@ func decodeAminoSignDoc(signDocBytes []byte) (apitypes.TypedData, error) { func decodeProtobufSignDoc(signDocBytes []byte) (apitypes.TypedData, error) { // Decode sign doc signDoc := &txTypes.SignDoc{} - err := signDoc.Unmarshal(signDocBytes) - if err != nil { + if err := signDoc.Unmarshal(signDocBytes); err != nil { return apitypes.TypedData{}, err } From e711f7757a1a339df1182074080bc405f3b53f38 Mon Sep 17 00:00:00 2001 From: Austin Chandra Date: Tue, 25 Oct 2022 10:35:15 -0700 Subject: [PATCH 11/31] Update ethereum/eip712/encoding.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com> --- ethereum/eip712/encoding.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ethereum/eip712/encoding.go b/ethereum/eip712/encoding.go index 567a2bd04d..72eaf15926 100644 --- a/ethereum/eip712/encoding.go +++ b/ethereum/eip712/encoding.go @@ -128,8 +128,7 @@ func decodeProtobufSignDoc(signDocBytes []byte) (apitypes.TypedData, error) { // Decode auth info authInfo := &txTypes.AuthInfo{} - err = authInfo.Unmarshal(signDoc.AuthInfoBytes) - if err != nil { + if err := authInfo.Unmarshal(signDoc.AuthInfoBytes); err != nil { return apitypes.TypedData{}, err } From e97298f3c324f3028501062abb58b957e2a90b0c Mon Sep 17 00:00:00 2001 From: Austin Chandra Date: Tue, 25 Oct 2022 10:35:29 -0700 Subject: [PATCH 12/31] Update ethereum/eip712/encoding.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com> --- ethereum/eip712/encoding.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ethereum/eip712/encoding.go b/ethereum/eip712/encoding.go index 72eaf15926..e7bf2c755a 100644 --- a/ethereum/eip712/encoding.go +++ b/ethereum/eip712/encoding.go @@ -171,8 +171,7 @@ func decodeProtobufSignDoc(signDocBytes []byte) (apitypes.TypedData, error) { // Parse Message (single message only) var msg cosmosTypes.Msg - err = ethermintProtoCodec.UnpackAny(body.Messages[0], &msg) - if err != nil { + if err = ethermintProtoCodec.UnpackAny(body.Messages[0], &msg); err != nil { return apitypes.TypedData{}, fmt.Errorf("could not unpack message object with error %w\n", err) } From 8f6f63bef26516fc02d92ee72705fbbcfd463031 Mon Sep 17 00:00:00 2001 From: Austin Chandra Date: Tue, 25 Oct 2022 10:37:33 -0700 Subject: [PATCH 13/31] Code cleanup --- ethereum/eip712/encoding.go | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/ethereum/eip712/encoding.go b/ethereum/eip712/encoding.go index e7bf2c755a..208409fd02 100644 --- a/ethereum/eip712/encoding.go +++ b/ethereum/eip712/encoding.go @@ -63,10 +63,7 @@ func GetEIP712HashForMsg(signDocBytes []byte) ([]byte, error) { // Attempt to decode the SignDoc bytes as an Amino SignDoc and return an error on failure func decodeAminoSignDoc(signDocBytes []byte) (apitypes.TypedData, error) { - var ( - aminoDoc legacytx.StdSignDoc - err error - ) + var aminoDoc legacytx.StdSignDoc if err := ethermintAminoCodec.UnmarshalJSON(signDocBytes, &aminoDoc); err != nil { return apitypes.TypedData{}, err @@ -134,8 +131,7 @@ func decodeProtobufSignDoc(signDocBytes []byte) (apitypes.TypedData, error) { // Decode body body := &txTypes.TxBody{} - err = body.Unmarshal(signDoc.BodyBytes) - if err != nil { + if err := body.Unmarshal(signDoc.BodyBytes); err != nil { return apitypes.TypedData{}, err } From 876876b0170de07b1de9827777d42c84d4f6dbe4 Mon Sep 17 00:00:00 2001 From: Austin Chandra Date: Wed, 26 Oct 2022 08:28:18 -0700 Subject: [PATCH 14/31] Update ethereum/eip712/encoding.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com> --- 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 208409fd02..ae953fa640 100644 --- a/ethereum/eip712/encoding.go +++ b/ethereum/eip712/encoding.go @@ -51,8 +51,8 @@ func GetEIP712HashForMsg(signDocBytes []byte) ([]byte, error) { if err != nil { return nil, fmt.Errorf("could not hash EIP-712 domain: %w", err) } - typedDataHash, err := typedData.HashStruct(typedData.PrimaryType, typedData.Message) + typedDataHash, err := typedData.HashStruct(typedData.PrimaryType, typedData.Message) if err != nil { return nil, fmt.Errorf("could not hash EIP-712 primary type: %w", err) } From 6b0481fce4d506228835e4ab54f807f57b122828 Mon Sep 17 00:00:00 2001 From: Austin Chandra Date: Wed, 26 Oct 2022 08:28:53 -0700 Subject: [PATCH 15/31] Minor codefix --- 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 ae953fa640..4fbbe00677 100644 --- a/ethereum/eip712/encoding.go +++ b/ethereum/eip712/encoding.go @@ -44,7 +44,7 @@ func GetEIP712HashForMsg(signDocBytes []byte) ([]byte, error) { case errProtobuf == nil: typedData = typedDataProtobuf default: - return make([]byte, 0), fmt.Errorf("could not decode sign doc as either Amino or Protobuf.\n amino: %v\n protobuf: %v\n", errAmino, errProtobuf) + return nil, fmt.Errorf("could not decode sign doc as either Amino or Protobuf.\n amino: %v\n protobuf: %v\n", errAmino, errProtobuf) } domainSeparator, err := typedData.HashStruct("EIP712Domain", typedData.Domain.Map()) From e08c6af58cad187f1bf9970edd330614d45616e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Federico=20Kunze=20K=C3=BCllmer?= <31522760+fedekunze@users.noreply.github.com> Date: Wed, 26 Oct 2022 17:39:30 +0200 Subject: [PATCH 16/31] Update ethereum/eip712/encoding.go --- 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 4fbbe00677..8ea90e9f89 100644 --- a/ethereum/eip712/encoding.go +++ b/ethereum/eip712/encoding.go @@ -156,7 +156,7 @@ func decodeProtobufSignDoc(signDocBytes []byte) (apitypes.TypedData, error) { // Parse ChainID chainID, err := ethermint.ParseChainID(signDoc.ChainId) if err != nil { - return apitypes.TypedData{}, fmt.Errorf("invalid chain ID passed as argument %v\n", chainID) + return apitypes.TypedData{}, fmt.Errorf("invalid chain ID passed as argument: %w", err) } // Create StdFee From 74548b74c8d033132f5c9ea49b708545170fff10 Mon Sep 17 00:00:00 2001 From: Austin Chandra Date: Fri, 28 Oct 2022 08:32:24 -0700 Subject: [PATCH 17/31] Minor code revision updates --- crypto/ethsecp256k1/ethsecp256k1.go | 18 +++++++++--------- ethereum/eip712/eip712_test.go | 4 +--- 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/crypto/ethsecp256k1/ethsecp256k1.go b/crypto/ethsecp256k1/ethsecp256k1.go index 98096f707a..a68f1d39c2 100644 --- a/crypto/ethsecp256k1/ethsecp256k1.go +++ b/crypto/ethsecp256k1/ethsecp256k1.go @@ -201,6 +201,15 @@ func (pubKey *PubKey) UnmarshalAminoJSON(bz []byte) error { return pubKey.UnmarshalAmino(bz) } +// VerifySignature verifies that the ECDSA public key created a given signature over +// the provided message. It will calculate the Keccak256 hash of the message +// prior to verification. +// +// CONTRACT: The signature should be in [R || S] format. +func (pubKey PubKey) VerifySignature(msg, sig []byte) bool { + return pubKey.verifySignatureECDSA(msg, sig) || pubKey.verifySignatureAsEIP712(msg, sig) +} + // Verifies the signature as an EIP-712 signature by first converting the message payload // to an EIP-712 hashed object, performing ECDSA verification on the hash. This is to support // signing a Cosmos payload using EIP-712. @@ -223,12 +232,3 @@ func (pubKey PubKey) verifySignatureECDSA(msg, sig []byte) bool { // the signature needs to be in [R || S] format when provided to VerifySignature return crypto.VerifySignature(pubKey.Key, crypto.Keccak256Hash(msg).Bytes(), sig) } - -// VerifySignature verifies that the ECDSA public key created a given signature over -// the provided message. It will calculate the Keccak256 hash of the message -// prior to verification. -// -// CONTRACT: The signature should be in [R || S] format. -func (pubKey PubKey) VerifySignature(msg, sig []byte) bool { - return pubKey.verifySignatureECDSA(msg, sig) || pubKey.verifySignatureAsEIP712(msg, sig) -} diff --git a/ethereum/eip712/eip712_test.go b/ethereum/eip712/eip712_test.go index d8a3ca3436..94aa8e47e2 100644 --- a/ethereum/eip712/eip712_test.go +++ b/ethereum/eip712/eip712_test.go @@ -246,9 +246,7 @@ func TestEIP712SignatureVerification(t *testing.T) { Sequence: tc.sequence, } - err = txBuilder.SetSignatures([]signing.SignatureV2{ - txSig, - }...) + err = txBuilder.SetSignatures([]signing.SignatureV2{txSig}...) require.NoError(t, err) // Declare signerData From 36ea28fcc074fd079831bddb2d8bbdb5cbefb6e2 Mon Sep 17 00:00:00 2001 From: Austin Chandra Date: Fri, 28 Oct 2022 12:43:49 -0700 Subject: [PATCH 18/31] Refactor EIP712 unit tests to use test suite --- ethereum/eip712/eip712_test.go | 204 ++++++++++++++++----------------- 1 file changed, 101 insertions(+), 103 deletions(-) diff --git a/ethereum/eip712/eip712_test.go b/ethereum/eip712/eip712_test.go index 94aa8e47e2..c929ab396b 100644 --- a/ethereum/eip712/eip712_test.go +++ b/ethereum/eip712/eip712_test.go @@ -1,12 +1,10 @@ package eip712_test import ( - "testing" - "cosmossdk.io/math" - "github.com/stretchr/testify/require" "github.com/cosmos/cosmos-sdk/client" + "github.com/cosmos/cosmos-sdk/simapp/params" "github.com/ethereum/go-ethereum/crypto" "github.com/evmos/ethermint/ethereum/eip712" @@ -25,29 +23,33 @@ import ( distributiontypes "github.com/cosmos/cosmos-sdk/x/distribution/types" govtypes "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1" stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" + "github.com/stretchr/testify/suite" ) -// Tests single-signer EIP-712 signature verification. Multi-signer verification tests are included -// in ante/integration test files. +// Unit tests for single-signer EIP-712 signature verification. Multi-signer verification tests are included +// in ante_test.go. + +type EIP712TestSuite struct { + suite.Suite -var config = encoding.MakeConfig(app.ModuleBasics) -var clientCtx = client.Context{}.WithTxConfig(config.TxConfig) + config params.EncodingConfig + clientCtx client.Context +} // Set up test env to replicate prod. environment -func setupTestEnv(t *testing.T) { - t.Helper() +func (suite *EIP712TestSuite) SetupTest() { + suite.config = encoding.MakeConfig(app.ModuleBasics) + suite.clientCtx = client.Context{}.WithTxConfig(suite.config.TxConfig) sdk.GetConfig().SetBech32PrefixForAccount("ethm", "") - eip712.SetEncodingConfig(config) + eip712.SetEncodingConfig(suite.config) } // Helper to create random test addresses for messages -func createTestAddress(t *testing.T) sdk.AccAddress { - t.Helper() - +func (suite *EIP712TestSuite) createTestAddress() sdk.AccAddress { privkey, _ := ethsecp256k1.GenerateKey() key, err := privkey.ToECDSA() - require.NoError(t, err) + suite.Require().NoError(err) addr := crypto.PubkeyToAddress(key.PublicKey) @@ -55,24 +57,20 @@ func createTestAddress(t *testing.T) sdk.AccAddress { } // Helper to create random keypair for signing + verification -func createTestKeyPair(t *testing.T) (*ethsecp256k1.PrivKey, *ethsecp256k1.PubKey) { - t.Helper() - +func (suite *EIP712TestSuite) createTestKeyPair() (*ethsecp256k1.PrivKey, *ethsecp256k1.PubKey) { privKey, err := ethsecp256k1.GenerateKey() - require.NoError(t, err) + suite.Require().NoError(err) pubKey := ðsecp256k1.PubKey{ Key: privKey.PubKey().Bytes(), } - require.Implements(t, (*cryptotypes.PubKey)(nil), pubKey) + suite.Require().Implements((*cryptotypes.PubKey)(nil), pubKey) return privKey, pubKey } // Helper to create instance of sdk.Coins[] with single coin -func makeCoins(t *testing.T, denom string, amount math.Int) sdk.Coins { - t.Helper() - +func (suite *EIP712TestSuite) makeCoins(denom string, amount math.Int) sdk.Coins { return sdk.NewCoins( sdk.NewCoin( denom, @@ -81,8 +79,8 @@ func makeCoins(t *testing.T, denom string, amount math.Int) sdk.Coins { ) } -func TestEIP712SignatureVerification(t *testing.T) { - setupTestEnv(t) +func (suite *EIP712TestSuite) TestEIP712SignatureVerification() { + suite.SetupTest() signModes := []signing.SignMode{ signing.SignMode_SIGN_MODE_DIRECT, @@ -101,15 +99,15 @@ func TestEIP712SignatureVerification(t *testing.T) { { title: "Standard MsgSend", fee: txtypes.Fee{ - Amount: makeCoins(t, "aphoton", math.NewInt(2000)), + Amount: suite.makeCoins("aphoton", math.NewInt(2000)), GasLimit: 20000, }, memo: "", msgs: []sdk.Msg{ banktypes.NewMsgSend( - createTestAddress(t), - createTestAddress(t), - makeCoins(t, "photon", math.NewInt(1)), + suite.createTestAddress(), + suite.createTestAddress(), + suite.makeCoins("photon", math.NewInt(1)), ), }, accountNumber: 8, @@ -119,13 +117,13 @@ func TestEIP712SignatureVerification(t *testing.T) { { title: "Standard MsgVote", fee: txtypes.Fee{ - Amount: makeCoins(t, "aphoton", math.NewInt(2000)), + Amount: suite.makeCoins("aphoton", math.NewInt(2000)), GasLimit: 20000, }, memo: "", msgs: []sdk.Msg{ govtypes.NewMsgVote( - createTestAddress(t), + suite.createTestAddress(), 5, govtypes.OptionNo, ), @@ -137,15 +135,15 @@ func TestEIP712SignatureVerification(t *testing.T) { { title: "Standard MsgDelegate", fee: txtypes.Fee{ - Amount: makeCoins(t, "aphoton", math.NewInt(2000)), + Amount: suite.makeCoins("aphoton", math.NewInt(2000)), GasLimit: 20000, }, memo: "", msgs: []sdk.Msg{ stakingtypes.NewMsgDelegate( - createTestAddress(t), - sdk.ValAddress(createTestAddress(t)), - makeCoins(t, "photon", math.NewInt(1))[0], + suite.createTestAddress(), + sdk.ValAddress(suite.createTestAddress()), + suite.makeCoins("photon", math.NewInt(1))[0], ), }, accountNumber: 25, @@ -155,14 +153,14 @@ func TestEIP712SignatureVerification(t *testing.T) { { title: "Standard MsgWithdrawDelegationReward", fee: txtypes.Fee{ - Amount: makeCoins(t, "aphoton", math.NewInt(2000)), + Amount: suite.makeCoins("aphoton", math.NewInt(2000)), GasLimit: 20000, }, memo: "", msgs: []sdk.Msg{ distributiontypes.NewMsgWithdrawDelegatorReward( - createTestAddress(t), - sdk.ValAddress(createTestAddress(t)), + suite.createTestAddress(), + sdk.ValAddress(suite.createTestAddress()), ), }, accountNumber: 25, @@ -172,130 +170,130 @@ func TestEIP712SignatureVerification(t *testing.T) { { title: "Two MsgVotes", fee: txtypes.Fee{ - Amount: makeCoins(t, "aphoton", math.NewInt(2000)), + Amount: suite.makeCoins("aphoton", math.NewInt(2000)), GasLimit: 20000, }, memo: "", msgs: []sdk.Msg{ govtypes.NewMsgVote( - createTestAddress(t), + suite.createTestAddress(), 5, govtypes.OptionNo, ), govtypes.NewMsgVote( - createTestAddress(t), + suite.createTestAddress(), 25, govtypes.OptionAbstain, ), }, accountNumber: 25, sequence: 78, - expectSuccess: false, // Multiple messages (check for multiple signers in AnteHandler) + expectSuccess: false, // Multiple messages are currently not allowed }, { title: "MsgSend + MsgVote", fee: txtypes.Fee{ - Amount: makeCoins(t, "aphoton", math.NewInt(2000)), + Amount: suite.makeCoins("aphoton", math.NewInt(2000)), GasLimit: 20000, }, memo: "", msgs: []sdk.Msg{ govtypes.NewMsgVote( - createTestAddress(t), + suite.createTestAddress(), 5, govtypes.OptionNo, ), banktypes.NewMsgSend( - createTestAddress(t), - createTestAddress(t), - makeCoins(t, "photon", math.NewInt(50)), + suite.createTestAddress(), + suite.createTestAddress(), + suite.makeCoins("photon", math.NewInt(50)), ), }, accountNumber: 25, sequence: 78, - expectSuccess: false, // Multiple messages + expectSuccess: false, }, } for _, tc := range testCases { for _, signMode := range signModes { - privKey, pubKey := createTestKeyPair(t) - - // Init tx builder - txBuilder := clientCtx.TxConfig.NewTxBuilder() - - // Set gas and fees - txBuilder.SetGasLimit(tc.fee.GasLimit) - txBuilder.SetFeeAmount(tc.fee.Amount) - - // Set messages - err := txBuilder.SetMsgs(tc.msgs...) - require.NoError(t, err) - - // Set memo - txBuilder.SetMemo(tc.memo) - - // Prepare signature field - txSigData := signing.SingleSignatureData{ - SignMode: signMode, - Signature: nil, - } - txSig := signing.SignatureV2{ - PubKey: pubKey, - Data: &txSigData, - Sequence: tc.sequence, - } - - err = txBuilder.SetSignatures([]signing.SignatureV2{txSig}...) - require.NoError(t, err) - - // Declare signerData - signerData := authsigning.SignerData{ - ChainID: "ethermint_9000-1", - AccountNumber: tc.accountNumber, - Sequence: tc.sequence, - PubKey: pubKey, - Address: sdk.MustBech32ifyAddressBytes("ethm", pubKey.Bytes()), - } - - bz, err := clientCtx.TxConfig.SignModeHandler().GetSignBytes( - signMode, - signerData, - txBuilder.GetTx(), - ) - require.NoError(t, err) - - verifyEIP712SignatureVerification(t, tc.expectSuccess, *privKey, *pubKey, bz) + suite.Run(tc.title, func() { + privKey, pubKey := suite.createTestKeyPair() + + // Init tx builder + txBuilder := suite.clientCtx.TxConfig.NewTxBuilder() + + // Set gas and fees + txBuilder.SetGasLimit(tc.fee.GasLimit) + txBuilder.SetFeeAmount(tc.fee.Amount) + + // Set messages + err := txBuilder.SetMsgs(tc.msgs...) + suite.Require().NoError(err) + + // Set memo + txBuilder.SetMemo(tc.memo) + + // Prepare signature field + txSigData := signing.SingleSignatureData{ + SignMode: signMode, + Signature: nil, + } + txSig := signing.SignatureV2{ + PubKey: pubKey, + Data: &txSigData, + Sequence: tc.sequence, + } + + err = txBuilder.SetSignatures([]signing.SignatureV2{txSig}...) + suite.Require().NoError(err) + + // Declare signerData + signerData := authsigning.SignerData{ + ChainID: "ethermint_9000-1", + AccountNumber: tc.accountNumber, + Sequence: tc.sequence, + PubKey: pubKey, + Address: sdk.MustBech32ifyAddressBytes("ethm", pubKey.Bytes()), + } + + bz, err := suite.clientCtx.TxConfig.SignModeHandler().GetSignBytes( + signMode, + signerData, + txBuilder.GetTx(), + ) + suite.Require().NoError(err) + + suite.verifyEIP712SignatureVerification(tc.expectSuccess, *privKey, *pubKey, bz) + }) } } } // Verify that the payload passes signature verification if signed as its EIP-712 representation. -func verifyEIP712SignatureVerification(t *testing.T, expectedSuccess bool, privKey ethsecp256k1.PrivKey, pubKey ethsecp256k1.PubKey, signBytes []byte) { - t.Helper() - +func (suite *EIP712TestSuite) verifyEIP712SignatureVerification(expectedSuccess bool, privKey ethsecp256k1.PrivKey, pubKey ethsecp256k1.PubKey, signBytes []byte) { // Convert to EIP712 hash and sign eip712Hash, err := eip712.GetEIP712HashForMsg(signBytes) if !expectedSuccess { // Expect failure generating EIP-712 hash - require.Error(t, err) + suite.Require().Error(err) return } - require.NoError(t, err) + suite.Require().NoError(err) sigHash := crypto.Keccak256Hash(eip712Hash) sig, err := privKey.Sign(sigHash.Bytes()) - require.NoError(t, err) + suite.Require().NoError(err) // Verify against original payload bytes. This should pass, even though it is not // the original message that was signed. res := pubKey.VerifySignature(signBytes, sig) - require.True(t, res) + suite.Require().True(res) // Verify against the signed EIP-712 bytes. This should pass, since it is the message signed. res = pubKey.VerifySignature(eip712Hash, sig) - require.True(t, res) + suite.Require().True(res) // Verify against random bytes to ensure it does not pass unexpectedly (sanity check). randBytes := make([]byte, len(signBytes)) @@ -303,5 +301,5 @@ func verifyEIP712SignatureVerification(t *testing.T, expectedSuccess bool, privK // Change the first element of signBytes to a different value randBytes[0] = (signBytes[0] + 10) % 128 res = pubKey.VerifySignature(randBytes, sig) - require.False(t, res) + suite.Require().False(res) } From 13e91ded1f29cb579da09a172ddb40bde4e3ccf0 Mon Sep 17 00:00:00 2001 From: Austin Chandra Date: Mon, 31 Oct 2022 13:58:03 -0700 Subject: [PATCH 19/31] Address import cycle and implement minor refactors --- crypto/ethsecp256k1/ethsecp256k1.go | 3 ++- ethereum/eip712/encoding.go | 34 +++++++++++++++++------------ types/validation_test.go | 13 ++++++----- 3 files changed, 29 insertions(+), 21 deletions(-) diff --git a/crypto/ethsecp256k1/ethsecp256k1.go b/crypto/ethsecp256k1/ethsecp256k1.go index a68f1d39c2..787e6d8e50 100644 --- a/crypto/ethsecp256k1/ethsecp256k1.go +++ b/crypto/ethsecp256k1/ethsecp256k1.go @@ -203,7 +203,8 @@ func (pubKey *PubKey) UnmarshalAminoJSON(bz []byte) error { // VerifySignature verifies that the ECDSA public key created a given signature over // the provided message. It will calculate the Keccak256 hash of the message -// prior to verification. +// prior to verification and approve verification if the signature can be verified +// from either the original message or its EIP-712 representation. // // CONTRACT: The signature should be in [R || S] format. func (pubKey PubKey) VerifySignature(msg, sig []byte) bool { diff --git a/ethereum/eip712/encoding.go b/ethereum/eip712/encoding.go index 8ea90e9f89..ffe72604f4 100644 --- a/ethereum/eip712/encoding.go +++ b/ethereum/eip712/encoding.go @@ -31,20 +31,9 @@ func SetEncodingConfig(cfg params.EncodingConfig) { // an EIP-712 object, then hashing the EIP-712 object to create the bytes to be signed. // See https://eips.ethereum.org/EIPS/eip-712 for more. func GetEIP712HashForMsg(signDocBytes []byte) ([]byte, error) { - var typedData apitypes.TypedData - - // 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) - typedDataProtobuf, errProtobuf := decodeProtobufSignDoc(signDocBytes) - - switch { - case errAmino == nil: - typedData = typedDataAmino - case errProtobuf == nil: - typedData = typedDataProtobuf - default: - return nil, fmt.Errorf("could not decode sign doc as either Amino or Protobuf.\n amino: %v\n protobuf: %v\n", errAmino, errProtobuf) + typedData, err := GetEIP712TypedDataForMsg(signDocBytes) + if err != nil { + return nil, err } domainSeparator, err := typedData.HashStruct("EIP712Domain", typedData.Domain.Map()) @@ -61,6 +50,23 @@ func GetEIP712HashForMsg(signDocBytes []byte) ([]byte, error) { return rawData, nil } +// Get the EIP-712 TypedData representation of the sign doc bytes for either Amino or +// Protobuf encodings. +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 { + return typedDataAmino, nil + } + typedDataProtobuf, errProtobuf := decodeProtobufSignDoc(signDocBytes) + if errProtobuf == nil { + return typedDataProtobuf, nil + } + + return apitypes.TypedData{}, fmt.Errorf("could not decode sign doc as either Amino or Protobuf.\n amino: %v\n protobuf: %v", errAmino, errProtobuf) +} + // Attempt to decode the SignDoc bytes as an Amino SignDoc and return an error on failure func decodeAminoSignDoc(signDocBytes []byte) (apitypes.TypedData, error) { var aminoDoc legacytx.StdSignDoc diff --git a/types/validation_test.go b/types/validation_test.go index d88cd7fc9e..d9b2ecedfc 100644 --- a/types/validation_test.go +++ b/types/validation_test.go @@ -1,10 +1,11 @@ -package types +package types_test import ( "testing" "github.com/ethereum/go-ethereum/common" "github.com/evmos/ethermint/tests" + "github.com/evmos/ethermint/types" "github.com/stretchr/testify/require" ) @@ -27,7 +28,7 @@ func TestIsEmptyHash(t *testing.T) { } for _, tc := range testCases { - require.Equal(t, tc.expEmpty, IsEmptyHash(tc.hash), tc.name) + require.Equal(t, tc.expEmpty, types.IsEmptyHash(tc.hash), tc.name) } } @@ -50,7 +51,7 @@ func TestIsZeroAddress(t *testing.T) { } for _, tc := range testCases { - require.Equal(t, tc.expEmpty, IsZeroAddress(tc.address), tc.name) + require.Equal(t, tc.expEmpty, types.IsZeroAddress(tc.address), tc.name) } } @@ -75,7 +76,7 @@ func TestValidateAddress(t *testing.T) { } for _, tc := range testCases { - err := ValidateAddress(tc.address) + err := types.ValidateAddress(tc.address) if tc.expError { require.Error(t, err, tc.name) @@ -106,7 +107,7 @@ func TestValidateNonZeroAddress(t *testing.T) { } for _, tc := range testCases { - err := ValidateNonZeroAddress(tc.address) + err := types.ValidateNonZeroAddress(tc.address) if tc.expError { require.Error(t, err, tc.name) @@ -131,7 +132,7 @@ func TestSafeInt64(t *testing.T) { } for _, tc := range testCases { - value, err := SafeInt64(tc.value) + value, err := types.SafeInt64(tc.value) if tc.expError { require.Error(t, err, tc.name) continue From 386a019c5b05fa68573e2181cd1b328e2caa8abb Mon Sep 17 00:00:00 2001 From: Austin Chandra Date: Mon, 31 Oct 2022 16:19:59 -0700 Subject: [PATCH 20/31] Fix lint issues --- ethereum/eip712/encoding.go | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/ethereum/eip712/encoding.go b/ethereum/eip712/encoding.go index ffe72604f4..47f5ea21a6 100644 --- a/ethereum/eip712/encoding.go +++ b/ethereum/eip712/encoding.go @@ -16,8 +16,10 @@ import ( "github.com/cosmos/cosmos-sdk/codec" ) -var ethermintProtoCodec codec.ProtoCodecMarshaler -var ethermintAminoCodec *codec.LegacyAmino +var ( + ethermintProtoCodec codec.ProtoCodecMarshaler + ethermintAminoCodec *codec.LegacyAmino +) // The process of unmarshaling SignDoc bytes into a SignDoc object requires having a codec // populated with all relevant message types. As a result, we must call this method on app @@ -82,7 +84,7 @@ func decodeAminoSignDoc(signDocBytes []byte) (apitypes.TypedData, error) { } if len(aminoDoc.Msgs) != 1 { - return apitypes.TypedData{}, fmt.Errorf("Invalid number of messages in SignDoc, expected 1 but got %v\n", len(aminoDoc.Msgs)) + return apitypes.TypedData{}, fmt.Errorf("invalid number of messages in SignDoc, expected 1 but got %v", len(aminoDoc.Msgs)) } var msg cosmosTypes.Msg @@ -113,9 +115,8 @@ func decodeAminoSignDoc(signDocBytes []byte) (apitypes.TypedData, error) { signDocBytes, // Amino StdSignDocBytes feeDelegation, ) - if err != nil { - return apitypes.TypedData{}, fmt.Errorf("could not convert to EIP712 representation: %w\n", err) + return apitypes.TypedData{}, fmt.Errorf("could not convert to EIP712 representation: %w", err) } return typedData, nil @@ -148,12 +149,12 @@ func decodeProtobufSignDoc(signDocBytes []byte) (apitypes.TypedData, error) { // Verify single message if len(body.Messages) != 1 { - return apitypes.TypedData{}, fmt.Errorf("invalid number of messages, expected 1 got %v\n", len(body.Messages)) + return apitypes.TypedData{}, fmt.Errorf("invalid number of messages, expected 1 got %v", len(body.Messages)) } // Verify single signature (single signer for now) if len(authInfo.SignerInfos) != 1 { - return apitypes.TypedData{}, fmt.Errorf("invalid number of signers, expected 1 got %v\n", len(authInfo.SignerInfos)) + return apitypes.TypedData{}, fmt.Errorf("invalid number of signers, expected 1 got %v", len(authInfo.SignerInfos)) } // Decode signer info (single signer for now) @@ -174,7 +175,7 @@ func decodeProtobufSignDoc(signDocBytes []byte) (apitypes.TypedData, error) { // 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\n", err) + return apitypes.TypedData{}, fmt.Errorf("could not unpack message object with error %w", err) } // Init fee payer From 3ef98c8de03c787c2f37740bc9d90868da3cd849 Mon Sep 17 00:00:00 2001 From: Austin Chandra Date: Mon, 31 Oct 2022 16:50:49 -0700 Subject: [PATCH 21/31] Add EIP712 unit suite test function --- ethereum/eip712/eip712_test.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/ethereum/eip712/eip712_test.go b/ethereum/eip712/eip712_test.go index c929ab396b..952bfc2a8f 100644 --- a/ethereum/eip712/eip712_test.go +++ b/ethereum/eip712/eip712_test.go @@ -1,6 +1,8 @@ package eip712_test import ( + "testing" + "cosmossdk.io/math" "github.com/cosmos/cosmos-sdk/client" @@ -36,6 +38,10 @@ type EIP712TestSuite struct { clientCtx client.Context } +func TestEIP712TestSuite(t *testing.T) { + suite.Run(t, &EIP712TestSuite{}) +} + // Set up test env to replicate prod. environment func (suite *EIP712TestSuite) SetupTest() { suite.config = encoding.MakeConfig(app.ModuleBasics) From 5003d8504e5e528148871e9922c19b2ac0d97123 Mon Sep 17 00:00:00 2001 From: Austin Chandra Date: Tue, 1 Nov 2022 11:31:09 -0700 Subject: [PATCH 22/31] Update ethereum/eip712/encoding.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com> --- 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 47f5ea21a6..ea48429d15 100644 --- a/ethereum/eip712/encoding.go +++ b/ethereum/eip712/encoding.go @@ -174,7 +174,7 @@ func decodeProtobufSignDoc(signDocBytes []byte) (apitypes.TypedData, error) { // Parse Message (single message only) var msg cosmosTypes.Msg - if err = ethermintProtoCodec.UnpackAny(body.Messages[0], &msg); err != nil { + if err := ethermintProtoCodec.UnpackAny(body.Messages[0], &msg); err != nil { return apitypes.TypedData{}, fmt.Errorf("could not unpack message object with error %w", err) } From 13a0f4d3a06e6a20cbe2af310e499e080027f492 Mon Sep 17 00:00:00 2001 From: Austin Chandra Date: Tue, 1 Nov 2022 11:31:26 -0700 Subject: [PATCH 23/31] Update ethereum/eip712/encoding.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com> --- ethereum/eip712/encoding.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ethereum/eip712/encoding.go b/ethereum/eip712/encoding.go index ea48429d15..07607264ce 100644 --- a/ethereum/eip712/encoding.go +++ b/ethereum/eip712/encoding.go @@ -52,8 +52,8 @@ func GetEIP712HashForMsg(signDocBytes []byte) ([]byte, error) { return rawData, nil } -// Get the EIP-712 TypedData representation of the sign doc bytes for either Amino or -// Protobuf encodings. +// GetEIP712TypedDataForMsg returns the EIP-712 TypedData representation for either +// Amino or Protobuf encoded signature doc bytes. 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. From cdba1ee43cde40806d1cf29d8bbafe4f6a86475f Mon Sep 17 00:00:00 2001 From: Austin Chandra Date: Tue, 1 Nov 2022 11:31:51 -0700 Subject: [PATCH 24/31] Update ethereum/eip712/encoding.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com> --- ethereum/eip712/encoding.go | 1 + 1 file changed, 1 insertion(+) diff --git a/ethereum/eip712/encoding.go b/ethereum/eip712/encoding.go index 07607264ce..882fa82005 100644 --- a/ethereum/eip712/encoding.go +++ b/ethereum/eip712/encoding.go @@ -21,6 +21,7 @@ var ( ethermintAminoCodec *codec.LegacyAmino ) +// SetEncodingConfig set the encoding config to the singleton codecs (Amino and Protobuf). // The process of unmarshaling SignDoc bytes into a SignDoc object requires having a codec // populated with all relevant message types. As a result, we must call this method on app // initialization with the app's encoding config. From f16265a1cd19da2f660cf1cb51dab3f6e9f6fb98 Mon Sep 17 00:00:00 2001 From: Austin Chandra Date: Tue, 1 Nov 2022 14:41:53 -0700 Subject: [PATCH 25/31] Add minor refactors; increase test coverage --- app/ante/ante_test.go | 42 +++++++++++++++ ethereum/eip712/eip712_test.go | 98 +++++++++++++++++++++++++++++++--- ethereum/eip712/encoding.go | 15 +++--- 3 files changed, 141 insertions(+), 14 deletions(-) diff --git a/app/ante/ante_test.go b/app/ante/ante_test.go index 508a8175f6..aeb6034661 100644 --- a/app/ante/ante_test.go +++ b/app/ante/ante_test.go @@ -680,6 +680,48 @@ func (suite AnteTestSuite) TestAnteHandler() { "mixed", // Combine EIP-712 and standard signatures ) + return txBuilder.GetTx() + }, false, false, false, + }, + { + "Fails - Multi-Key with multiple signers", + func() sdk.Tx { + numKeys := 5 + privKeys, pubKeys := suite.GenerateMultipleKeys(numKeys) + pk := kmultisig.NewLegacyAminoPubKey(numKeys, pubKeys) + + msg := banktypes.NewMsgMultiSend( + []banktypes.Input{ + banktypes.NewInput( + suite.createTestAddress(), + suite.makeCoins("photon", math.NewInt(50)), + ), + banktypes.NewInput( + suite.createTestAddress(), + suite.makeCoins("photon", math.NewInt(50)), + ), + }, + []banktypes.Output{ + banktypes.NewOutput( + suite.createTestAddress(), + suite.makeCoins("photon", math.NewInt(50)), + ), + banktypes.NewOutput( + suite.createTestAddress(), + suite.makeCoins("photon", math.NewInt(50)), + ), + }, + ) + + txBuilder := suite.CreateTestSignedMultisigTx( + privKeys, + signing.SignMode_SIGN_MODE_DIRECT, + msg, + "ethermint_9000-1", + 2000, + "mixed", // Combine EIP-712 and standard signatures + ) + return txBuilder.GetTx() }, false, false, false, }, diff --git a/ethereum/eip712/eip712_test.go b/ethereum/eip712/eip712_test.go index 952bfc2a8f..d2e933af4f 100644 --- a/ethereum/eip712/eip712_test.go +++ b/ethereum/eip712/eip712_test.go @@ -95,15 +95,17 @@ func (suite *EIP712TestSuite) TestEIP712SignatureVerification() { testCases := []struct { title string + chainId string fee txtypes.Fee memo string msgs []sdk.Msg accountNumber uint64 sequence uint64 + timeoutHeight uint64 expectSuccess bool }{ { - title: "Standard MsgSend", + title: "Succeeds - Standard MsgSend", fee: txtypes.Fee{ Amount: suite.makeCoins("aphoton", math.NewInt(2000)), GasLimit: 20000, @@ -121,7 +123,7 @@ func (suite *EIP712TestSuite) TestEIP712SignatureVerification() { expectSuccess: true, }, { - title: "Standard MsgVote", + title: "Succeeds - Standard MsgVote", fee: txtypes.Fee{ Amount: suite.makeCoins("aphoton", math.NewInt(2000)), GasLimit: 20000, @@ -139,7 +141,7 @@ func (suite *EIP712TestSuite) TestEIP712SignatureVerification() { expectSuccess: true, }, { - title: "Standard MsgDelegate", + title: "Succeeds - Standard MsgDelegate", fee: txtypes.Fee{ Amount: suite.makeCoins("aphoton", math.NewInt(2000)), GasLimit: 20000, @@ -157,7 +159,7 @@ func (suite *EIP712TestSuite) TestEIP712SignatureVerification() { expectSuccess: true, }, { - title: "Standard MsgWithdrawDelegationReward", + title: "Succeeds - Standard MsgWithdrawDelegationReward", fee: txtypes.Fee{ Amount: suite.makeCoins("aphoton", math.NewInt(2000)), GasLimit: 20000, @@ -174,7 +176,7 @@ func (suite *EIP712TestSuite) TestEIP712SignatureVerification() { expectSuccess: true, }, { - title: "Two MsgVotes", + title: "Fails - Two MsgVotes", fee: txtypes.Fee{ Amount: suite.makeCoins("aphoton", math.NewInt(2000)), GasLimit: 20000, @@ -197,7 +199,7 @@ func (suite *EIP712TestSuite) TestEIP712SignatureVerification() { expectSuccess: false, // Multiple messages are currently not allowed }, { - title: "MsgSend + MsgVote", + title: "Fails - MsgSend + MsgVote", fee: txtypes.Fee{ Amount: suite.makeCoins("aphoton", math.NewInt(2000)), GasLimit: 20000, @@ -219,6 +221,79 @@ func (suite *EIP712TestSuite) TestEIP712SignatureVerification() { sequence: 78, expectSuccess: false, }, + { + title: "Fails - Invalid ChainID", + chainId: "invalidchainid", + fee: txtypes.Fee{ + Amount: suite.makeCoins("aphoton", math.NewInt(2000)), + GasLimit: 20000, + }, + memo: "", + msgs: []sdk.Msg{ + govtypes.NewMsgVote( + suite.createTestAddress(), + 5, + govtypes.OptionNo, + ), + }, + accountNumber: 25, + sequence: 78, + expectSuccess: false, + }, + { + title: "Fails - Includes TimeoutHeight", + fee: txtypes.Fee{ + Amount: suite.makeCoins("aphoton", math.NewInt(2000)), + GasLimit: 20000, + }, + memo: "", + msgs: []sdk.Msg{ + govtypes.NewMsgVote( + suite.createTestAddress(), + 5, + govtypes.OptionNo, + ), + }, + accountNumber: 25, + sequence: 78, + timeoutHeight: 1000, + expectSuccess: false, + }, + { + title: "Fails - Single Message / Multi-Signer", + fee: txtypes.Fee{ + Amount: suite.makeCoins("aphoton", math.NewInt(2000)), + GasLimit: 20000, + }, + memo: "", + msgs: []sdk.Msg{ + banktypes.NewMsgMultiSend( + []banktypes.Input{ + banktypes.NewInput( + suite.createTestAddress(), + suite.makeCoins("photon", math.NewInt(50)), + ), + banktypes.NewInput( + suite.createTestAddress(), + suite.makeCoins("photon", math.NewInt(50)), + ), + }, + []banktypes.Output{ + banktypes.NewOutput( + suite.createTestAddress(), + suite.makeCoins("photon", math.NewInt(50)), + ), + banktypes.NewOutput( + suite.createTestAddress(), + suite.makeCoins("photon", math.NewInt(50)), + ), + }, + ), + }, + accountNumber: 25, + sequence: 78, + expectSuccess: false, + }, } for _, tc := range testCases { @@ -254,9 +329,18 @@ func (suite *EIP712TestSuite) TestEIP712SignatureVerification() { err = txBuilder.SetSignatures([]signing.SignatureV2{txSig}...) suite.Require().NoError(err) + chainId := "ethermint_9000-1" + if tc.chainId != "" { + chainId = tc.chainId + } + + if tc.timeoutHeight != 0 { + txBuilder.SetTimeoutHeight(tc.timeoutHeight) + } + // Declare signerData signerData := authsigning.SignerData{ - ChainID: "ethermint_9000-1", + ChainID: chainId, AccountNumber: tc.accountNumber, Sequence: tc.sequence, PubKey: pubKey, diff --git a/ethereum/eip712/encoding.go b/ethereum/eip712/encoding.go index 882fa82005..d4c90d4514 100644 --- a/ethereum/eip712/encoding.go +++ b/ethereum/eip712/encoding.go @@ -3,6 +3,7 @@ package eip712 import ( "errors" "fmt" + "reflect" "github.com/cosmos/cosmos-sdk/simapp/params" "github.com/cosmos/cosmos-sdk/x/auth/migrations/legacytx" @@ -59,11 +60,11 @@ 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 { + if !reflect.DeepEqual(typedDataAmino, apitypes.TypedData{}) && errAmino == nil { return typedDataAmino, nil } typedDataProtobuf, errProtobuf := decodeProtobufSignDoc(signDocBytes) - if errProtobuf == nil { + if !reflect.DeepEqual(typedDataProtobuf, apitypes.TypedData{}) && errProtobuf == nil { return typedDataProtobuf, nil } @@ -153,11 +154,6 @@ func decodeProtobufSignDoc(signDocBytes []byte) (apitypes.TypedData, error) { return apitypes.TypedData{}, fmt.Errorf("invalid number of messages, expected 1 got %v", len(body.Messages)) } - // Verify single signature (single signer for now) - if len(authInfo.SignerInfos) != 1 { - return apitypes.TypedData{}, fmt.Errorf("invalid number of signers, expected 1 got %v", len(authInfo.SignerInfos)) - } - // Decode signer info (single signer for now) signerInfo := authInfo.SignerInfos[0] @@ -179,6 +175,11 @@ func decodeProtobufSignDoc(signDocBytes []byte) (apitypes.TypedData, error) { 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{ From 3a5a52518f0f81d84b429565e58c3a8feae6df2c Mon Sep 17 00:00:00 2001 From: Austin Chandra Date: Tue, 1 Nov 2022 14:50:27 -0700 Subject: [PATCH 26/31] Correct ante_test for change in payload --- app/ante/ante_test.go | 37 ++++++++++++++----------------------- 1 file changed, 14 insertions(+), 23 deletions(-) diff --git a/app/ante/ante_test.go b/app/ante/ante_test.go index aeb6034661..ac87d27b30 100644 --- a/app/ante/ante_test.go +++ b/app/ante/ante_test.go @@ -684,33 +684,21 @@ func (suite AnteTestSuite) TestAnteHandler() { }, false, false, false, }, { - "Fails - Multi-Key with multiple signers", + "Fails - Multi-Key with different payload than one signed", func() sdk.Tx { - numKeys := 5 + numKeys := 1 privKeys, pubKeys := suite.GenerateMultipleKeys(numKeys) pk := kmultisig.NewLegacyAminoPubKey(numKeys, pubKeys) - msg := banktypes.NewMsgMultiSend( - []banktypes.Input{ - banktypes.NewInput( - suite.createTestAddress(), - suite.makeCoins("photon", math.NewInt(50)), - ), - banktypes.NewInput( - suite.createTestAddress(), - suite.makeCoins("photon", math.NewInt(50)), - ), - }, - []banktypes.Output{ - banktypes.NewOutput( - suite.createTestAddress(), - suite.makeCoins("photon", math.NewInt(50)), - ), - banktypes.NewOutput( - suite.createTestAddress(), - suite.makeCoins("photon", math.NewInt(50)), + msg := banktypes.NewMsgSend( + sdk.AccAddress(pk.Address()), + addr[:], + sdk.NewCoins( + sdk.NewCoin( + "photon", + sdk.NewInt(1), ), - }, + ), ) txBuilder := suite.CreateTestSignedMultisigTx( @@ -719,9 +707,12 @@ func (suite AnteTestSuite) TestAnteHandler() { msg, "ethermint_9000-1", 2000, - "mixed", // Combine EIP-712 and standard signatures + "EIP-712", ) + msg.Amount[0].Amount = sdk.NewInt(5) + txBuilder.SetMsgs(msg) + return txBuilder.GetTx() }, false, false, false, }, From e93b3dbac48d58c6ae87b82f5f1ed7fafd923ebe Mon Sep 17 00:00:00 2001 From: Austin Chandra Date: Tue, 1 Nov 2022 16:41:58 -0700 Subject: [PATCH 27/31] Add single-signer util and tests --- app/ante/ante_test.go | 87 +++++++++++++++++++++ app/ante/utils_test.go | 171 ++++++++++++++++++++++++++--------------- 2 files changed, 198 insertions(+), 60 deletions(-) diff --git a/app/ante/ante_test.go b/app/ante/ante_test.go index ac87d27b30..689f56cef4 100644 --- a/app/ante/ante_test.go +++ b/app/ante/ante_test.go @@ -508,6 +508,32 @@ func (suite AnteTestSuite) TestAnteHandler() { return tx }, true, false, false, }, + { + "passes - Single-signer EIP-712", + func() sdk.Tx { + msg := banktypes.NewMsgSend( + sdk.AccAddress(privKey.PubKey().Address()), + addr[:], + sdk.NewCoins( + sdk.NewCoin( + "photon", + sdk.NewInt(1), + ), + ), + ) + + txBuilder := suite.CreateTestSingleSignedTx( + privKey, + signing.SignMode_SIGN_MODE_LEGACY_AMINO_JSON, + msg, + "ethermint_9000-1", + 2000000, + "EIP-712", + ) + + return txBuilder.GetTx() + }, false, false, true, + }, { "passes - EIP-712 multi-key", func() sdk.Tx { @@ -713,6 +739,67 @@ func (suite AnteTestSuite) TestAnteHandler() { msg.Amount[0].Amount = sdk.NewInt(5) txBuilder.SetMsgs(msg) + return txBuilder.GetTx() + }, false, false, false, + }, + { + "Fails - Multi-Key with messages added after signing", + func() sdk.Tx { + numKeys := 1 + privKeys, pubKeys := suite.GenerateMultipleKeys(numKeys) + pk := kmultisig.NewLegacyAminoPubKey(numKeys, pubKeys) + + msg := banktypes.NewMsgSend( + sdk.AccAddress(pk.Address()), + addr[:], + sdk.NewCoins( + sdk.NewCoin( + "photon", + sdk.NewInt(1), + ), + ), + ) + + txBuilder := suite.CreateTestSignedMultisigTx( + privKeys, + signing.SignMode_SIGN_MODE_DIRECT, + msg, + "ethermint_9000-1", + 2000, + "EIP-712", + ) + + // Duplicate + txBuilder.SetMsgs(msg, msg) + + return txBuilder.GetTx() + }, false, false, false, + }, + { + "Fails - Single-Signer EIP-712 with messages added after signing", + func() sdk.Tx { + msg := banktypes.NewMsgSend( + sdk.AccAddress(privKey.PubKey().Address()), + addr[:], + sdk.NewCoins( + sdk.NewCoin( + "photon", + sdk.NewInt(1), + ), + ), + ) + + txBuilder := suite.CreateTestSingleSignedTx( + privKey, + signing.SignMode_SIGN_MODE_DIRECT, + msg, + "ethermint_9000-1", + 2000, + "EIP-712", + ) + + txBuilder.SetMsgs(msg, msg) + return txBuilder.GetTx() }, false, false, false, }, diff --git a/app/ante/utils_test.go b/app/ante/utils_test.go index 0fa66cb2f8..14b79b581c 100644 --- a/app/ante/utils_test.go +++ b/app/ante/utils_test.go @@ -466,79 +466,124 @@ func (suite *AnteTestSuite) GenerateMultipleKeys(n int) ([]cryptotypes.PrivKey, return privKeys, pubKeys } -// Signs a set of messages using each private key within a given multi-key -func (suite *AnteTestSuite) generateMultikeySignatures(signMode signing.SignMode, privKeys []cryptotypes.PrivKey, signDocBytes []byte, signType string) (signatures []signing.SignatureV2) { +// generateSingleSignature signs the given sign doc bytes using the given signType (EIP-712 or Standard) +func (suite *AnteTestSuite) generateSingleSignature(signMode signing.SignMode, privKey cryptotypes.PrivKey, signDocBytes []byte, signType string) (signature signing.SignatureV2) { var ( msg []byte err error ) + msg = signDocBytes + + if signType == "EIP-712" { + msg, err = eip712.GetEIP712HashForMsg(signDocBytes) + suite.Require().NoError(err) + } + + sigBytes, _ := privKey.Sign(msg) + sigData := &signing.SingleSignatureData{ + SignMode: signMode, + Signature: sigBytes, + } + + return signing.SignatureV2{ + PubKey: privKey.PubKey(), + Data: sigData, + } +} + +// generateMultikeySignatures signs a set of messages using each private key within a given multi-key +func (suite *AnteTestSuite) generateMultikeySignatures(signMode signing.SignMode, privKeys []cryptotypes.PrivKey, signDocBytes []byte, signType string) (signatures []signing.SignatureV2) { n := len(privKeys) signatures = make([]signing.SignatureV2, n) for i := 0; i < n; i++ { privKey := privKeys[i] - - msg = signDocBytes - - if signType == "EIP-712" || (signType == "mixed" && i%2 == 0) { - msg, err = eip712.GetEIP712HashForMsg(signDocBytes) - suite.Require().NoError(err) + currentType := signType + + // If mixed type, alternate signing type on each iteration + if signType == "mixed" { + if i%2 == 0 { + currentType = "EIP-712" + } else { + currentType = "Standard" + } } - sigBytes, _ := privKey.Sign(msg) - sigData := &signing.SingleSignatureData{ - SignMode: signMode, - Signature: sigBytes, - } - - signatures[i] = signing.SignatureV2{ - PubKey: privKey.PubKey(), - Data: sigData, - } + signatures[i] = suite.generateSingleSignature( + signMode, + privKey, + signDocBytes, + currentType, + ) } return signatures } -// Create and sign a multi-signed tx for the given message. `signType` indicates whether to use standard signing ("Standard"), -// EIP-712 signing ("EIP-712"), or a mix of the two ("mixed"). -func (suite *AnteTestSuite) CreateTestSignedMultisigTx(privKeys []cryptotypes.PrivKey, signMode signing.SignMode, msg sdk.Msg, chainId string, gas uint64, signType string) client.TxBuilder { - pubKeys := make([]cryptotypes.PubKey, len(privKeys)) - for i, privKey := range privKeys { - pubKeys[i] = privKey.PubKey() - } +// RegisterAccount creates an account with the keeper and populates the initial balance +func (suite *AnteTestSuite) RegisterAccount(pubKey cryptotypes.PubKey, balance *big.Int) { + acc := suite.app.AccountKeeper.NewAccountWithAddress(suite.ctx, sdk.AccAddress(pubKey.Address())) + suite.app.AccountKeeper.SetAccount(suite.ctx, acc) - // Re-derive multikey - numKeys := len(privKeys) - multiKey := kmultisig.NewLegacyAminoPubKey(numKeys, pubKeys) + suite.app.EvmKeeper.SetBalance(suite.ctx, common.BytesToAddress(pubKey.Address()), balance) +} - // Create multi-key account - multiKeyAcc := suite.app.AccountKeeper.NewAccountWithAddress(suite.ctx, sdk.AccAddress(multiKey.Address())) - suite.app.AccountKeeper.SetAccount(suite.ctx, multiKeyAcc) +// createSignerBytes generates sign doc bytes using the given parameters +func (suite *AnteTestSuite) createSignerBytes(chainId string, signMode signing.SignMode, pubKey cryptotypes.PubKey, txBuilder client.TxBuilder) []byte { + acc, err := sdkante.GetSignerAcc(suite.ctx, suite.app.AccountKeeper, sdk.AccAddress(pubKey.Address())) + suite.Require().NoError(err) + signerInfo := authsigning.SignerData{ + Address: sdk.MustBech32ifyAddressBytes(sdk.GetConfig().GetBech32AccountAddrPrefix(), acc.GetAddress().Bytes()), + ChainID: chainId, + AccountNumber: acc.GetAccountNumber(), + Sequence: acc.GetSequence(), + PubKey: pubKey, + } + + signerBytes, err := suite.clientCtx.TxConfig.SignModeHandler().GetSignBytes( + signMode, + signerInfo, + txBuilder.GetTx(), + ) + suite.Require().NoError(err) - // Update balance for multikey account - suite.app.EvmKeeper.SetBalance(suite.ctx, common.BytesToAddress(multiKey.Address()), big.NewInt(10000000000)) + return signerBytes +} - // Init builder +// createBaseTxBuilder creates a TxBuilder to be used for Single- or Multi-signing +func (suite *AnteTestSuite) createBaseTxBuilder(msg sdk.Msg, gas uint64) client.TxBuilder { txBuilder := suite.clientCtx.TxConfig.NewTxBuilder() - // Set fees txBuilder.SetGasLimit(gas) txBuilder.SetFeeAmount(sdk.NewCoins( - sdk.NewCoin( - "aphoton", - sdk.NewInt(10000), - ), + sdk.NewCoin("aphoton", sdk.NewInt(10000)), )) - // Set message - err := txBuilder.SetMsgs([]sdk.Msg{msg}...) + err := txBuilder.SetMsgs(msg) suite.Require().NoError(err) - // Set memo and tip txBuilder.SetMemo("") + return txBuilder +} + +// CreateTestSignedMultisigTx creates and sign a multi-signed tx for the given message. `signType` indicates whether to use standard signing ("Standard"), +// EIP-712 signing ("EIP-712"), or a mix of the two ("mixed"). +func (suite *AnteTestSuite) CreateTestSignedMultisigTx(privKeys []cryptotypes.PrivKey, signMode signing.SignMode, msg sdk.Msg, chainId string, gas uint64, signType string) client.TxBuilder { + pubKeys := make([]cryptotypes.PubKey, len(privKeys)) + for i, privKey := range privKeys { + pubKeys[i] = privKey.PubKey() + } + + // Re-derive multikey + numKeys := len(privKeys) + multiKey := kmultisig.NewLegacyAminoPubKey(numKeys, pubKeys) + + suite.RegisterAccount(multiKey, big.NewInt(10000000000)) + + txBuilder := suite.createBaseTxBuilder(msg, gas) + // Prepare signature field sig := multisig.NewMultisig(len(pubKeys)) txBuilder.SetSignatures(signing.SignatureV2{ @@ -546,28 +591,12 @@ func (suite *AnteTestSuite) CreateTestSignedMultisigTx(privKeys []cryptotypes.Pr Data: sig, }) - // Create signer bytes - acc, err := sdkante.GetSignerAcc(suite.ctx, suite.app.AccountKeeper, sdk.AccAddress(multiKey.Address())) - suite.Require().NoError(err) - signerInfo := authsigning.SignerData{ - Address: sdk.MustBech32ifyAddressBytes(sdk.GetConfig().GetBech32AccountAddrPrefix(), acc.GetAddress().Bytes()), - ChainID: chainId, - AccountNumber: acc.GetAccountNumber(), - Sequence: acc.GetSequence(), - PubKey: multiKey, - } - - signerBytes, err := suite.clientCtx.TxConfig.SignModeHandler().GetSignBytes( - signMode, - signerInfo, - txBuilder.GetTx(), - ) - suite.Require().NoError(err) + signerBytes := suite.createSignerBytes(chainId, signMode, multiKey, txBuilder) // Sign for each key and update signature field sigs := suite.generateMultikeySignatures(signMode, privKeys, signerBytes, signType) for _, pkSig := range sigs { - err = multisig.AddSignatureV2(sig, pkSig, pubKeys) + err := multisig.AddSignatureV2(sig, pkSig, pubKeys) suite.Require().NoError(err) } @@ -579,6 +608,28 @@ func (suite *AnteTestSuite) CreateTestSignedMultisigTx(privKeys []cryptotypes.Pr return txBuilder } +func (suite *AnteTestSuite) CreateTestSingleSignedTx(privKey cryptotypes.PrivKey, signMode signing.SignMode, msg sdk.Msg, chainId string, gas uint64, signType string) client.TxBuilder { + pubKey := privKey.PubKey() + + suite.RegisterAccount(pubKey, big.NewInt(10000000000)) + + txBuilder := suite.createBaseTxBuilder(msg, gas) + + // Prepare signature field + sig := signing.SingleSignatureData{} + txBuilder.SetSignatures(signing.SignatureV2{ + PubKey: pubKey, + Data: &sig, + }) + + signerBytes := suite.createSignerBytes(chainId, signMode, pubKey, txBuilder) + + sigData := suite.generateSingleSignature(signMode, privKey, signerBytes, signType) + txBuilder.SetSignatures(sigData) + + return txBuilder +} + func NextFn(ctx sdk.Context, _ sdk.Tx, _ bool) (sdk.Context, error) { return ctx, nil } From 47e224c8eff6249157b7d7c8612a278857789aef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Federico=20Kunze=20K=C3=BCllmer?= <31522760+fedekunze@users.noreply.github.com> Date: Wed, 2 Nov 2022 11:16:25 +0100 Subject: [PATCH 28/31] Update ethereum/eip712/encoding.go --- ethereum/eip712/encoding.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ethereum/eip712/encoding.go b/ethereum/eip712/encoding.go index d4c90d4514..5fd67377e0 100644 --- a/ethereum/eip712/encoding.go +++ b/ethereum/eip712/encoding.go @@ -60,11 +60,11 @@ 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 !reflect.DeepEqual(typedDataAmino, apitypes.TypedData{}) && errAmino == nil { + if errAmino == nil && !reflect.DeepEqual(typedDataAmino, apitypes.TypedData{}){ return typedDataAmino, nil } typedDataProtobuf, errProtobuf := decodeProtobufSignDoc(signDocBytes) - if !reflect.DeepEqual(typedDataProtobuf, apitypes.TypedData{}) && errProtobuf == nil { + if errProtobuf == nil && !reflect.DeepEqual(typedDataProtobuf, apitypes.TypedData{}){ return typedDataProtobuf, nil } From 1c075c87f38a6f52ae13b9ab20a988f3726baf6a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Federico=20Kunze=20K=C3=BCllmer?= <31522760+fedekunze@users.noreply.github.com> Date: Wed, 2 Nov 2022 11:17:18 +0100 Subject: [PATCH 29/31] Update ethereum/eip712/encoding.go --- 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 5fd67377e0..0c74aecc06 100644 --- a/ethereum/eip712/encoding.go +++ b/ethereum/eip712/encoding.go @@ -68,7 +68,7 @@ func GetEIP712TypedDataForMsg(signDocBytes []byte) (apitypes.TypedData, error) { return typedDataProtobuf, nil } - return apitypes.TypedData{}, fmt.Errorf("could not decode sign doc as either Amino or Protobuf.\n amino: %v\n protobuf: %v", errAmino, errProtobuf) + return apitypes.TypedData{}, fmt.Errorf("could not decode sign doc as either Amino or Protobuf.\n amino: %w\n protobuf: %w", errAmino, errProtobuf) } // Attempt to decode the SignDoc bytes as an Amino SignDoc and return an error on failure From 2b7486e92d0831fcdfad167042e9ff8faf13b12f Mon Sep 17 00:00:00 2001 From: Freddy Caceres Date: Wed, 2 Nov 2022 12:19:15 -0400 Subject: [PATCH 30/31] fix build --- ethereum/eip712/encoding.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ethereum/eip712/encoding.go b/ethereum/eip712/encoding.go index 0c74aecc06..0e76ec05b7 100644 --- a/ethereum/eip712/encoding.go +++ b/ethereum/eip712/encoding.go @@ -60,15 +60,15 @@ 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 && !reflect.DeepEqual(typedDataAmino, apitypes.TypedData{}){ + if errAmino == nil && !reflect.DeepEqual(typedDataAmino, apitypes.TypedData{}) { return typedDataAmino, nil } typedDataProtobuf, errProtobuf := decodeProtobufSignDoc(signDocBytes) - if errProtobuf == nil && !reflect.DeepEqual(typedDataProtobuf, apitypes.TypedData{}){ + if errProtobuf == nil && !reflect.DeepEqual(typedDataProtobuf, apitypes.TypedData{}) { return typedDataProtobuf, nil } - return apitypes.TypedData{}, fmt.Errorf("could not decode sign doc as either Amino or Protobuf.\n amino: %w\n protobuf: %w", errAmino, errProtobuf) + return apitypes.TypedData{}, fmt.Errorf("could not decode sign doc as either Amino or Protobuf. amino: %v protobuf: %v", errAmino, errProtobuf) } // Attempt to decode the SignDoc bytes as an Amino SignDoc and return an error on failure From a61c40fc04920e8de7db87223847d37882ad38b9 Mon Sep 17 00:00:00 2001 From: Austin Chandra Date: Thu, 3 Nov 2022 08:44:02 -0700 Subject: [PATCH 31/31] Remove reflect import --- ethereum/eip712/encoding.go | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/ethereum/eip712/encoding.go b/ethereum/eip712/encoding.go index d4c90d4514..fbd20f5a99 100644 --- a/ethereum/eip712/encoding.go +++ b/ethereum/eip712/encoding.go @@ -3,7 +3,6 @@ package eip712 import ( "errors" "fmt" - "reflect" "github.com/cosmos/cosmos-sdk/simapp/params" "github.com/cosmos/cosmos-sdk/x/auth/migrations/legacytx" @@ -60,17 +59,23 @@ 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 !reflect.DeepEqual(typedDataAmino, apitypes.TypedData{}) && errAmino == nil { + if errAmino == nil && verifyEIP712Payload(typedDataAmino) { return typedDataAmino, nil } typedDataProtobuf, errProtobuf := decodeProtobufSignDoc(signDocBytes) - if !reflect.DeepEqual(typedDataProtobuf, apitypes.TypedData{}) && errProtobuf == nil { + if errProtobuf == nil && verifyEIP712Payload(typedDataProtobuf) { return typedDataProtobuf, nil } 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 +// an improper initialization. +func verifyEIP712Payload(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 func decodeAminoSignDoc(signDocBytes []byte) (apitypes.TypedData, error) { var aminoDoc legacytx.StdSignDoc