diff --git a/CHANGELOG.md b/CHANGELOG.md index d7721e07ce..956f7a95cf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -65,6 +65,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (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. * (test) [#1396](https://github.com/evmos/ethermint/pull/1396) Increase test coverage for the EVM module `keeper` +* (ante) [#1397](https://github.com/evmos/ethermint/pull/1397) Refactor EIP-712 signature verification to support EIP-712 multi-signing. * (deps) [#1416](https://github.com/evmos/ethermint/pull/1416) Bump Go version to `1.19` * (cmd) [\#1417](https://github.com/evmos/ethermint/pull/1417) Apply Google CLI Syntax for required and optional args. diff --git a/app/ante/ante_test.go b/app/ante/ante_test.go index 9056ddaae3..689f56cef4 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() { @@ -507,6 +508,301 @@ 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 { + 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[:], + sdk.NewCoins( + sdk.NewCoin( + "photon", + sdk.NewInt(1), + ), + ), + ) + + 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, + }, + { + "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, + }, + { + "Fails - Multi-Key with different payload than one signed", + 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", + ) + + 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, + }, } for _, tc := range testCases { @@ -939,7 +1235,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 c297820e98..14b79b581c 100644 --- a/app/ante/utils_test.go +++ b/app/ante/utils_test.go @@ -25,16 +25,20 @@ 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" 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" 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 +115,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 +453,183 @@ 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() + suite.Require().NoError(err) + privKeys[i] = privKey + pubKeys[i] = privKey.PubKey() + } + return privKeys, pubKeys +} + +// 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] + currentType := signType + + // If mixed type, alternate signing type on each iteration + if signType == "mixed" { + if i%2 == 0 { + currentType = "EIP-712" + } else { + currentType = "Standard" + } + } + + signatures[i] = suite.generateSingleSignature( + signMode, + privKey, + signDocBytes, + currentType, + ) + } + + return signatures +} + +// 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) + + suite.app.EvmKeeper.SetBalance(suite.ctx, common.BytesToAddress(pubKey.Address()), balance) +} + +// 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) + + return signerBytes +} + +// 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() + + txBuilder.SetGasLimit(gas) + txBuilder.SetFeeAmount(sdk.NewCoins( + sdk.NewCoin("aphoton", sdk.NewInt(10000)), + )) + + err := txBuilder.SetMsgs(msg) + suite.Require().NoError(err) + + 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{ + PubKey: multiKey, + Data: sig, + }) + + 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) + suite.Require().NoError(err) + } + + txBuilder.SetSignatures(signing.SignatureV2{ + PubKey: multiKey, + Data: sig, + }) + + 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 } diff --git a/cmd/ethermintd/root.go b/cmd/ethermintd/root.go index d833fb3476..104e416eee 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..787e6d8e50 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" ) @@ -203,10 +203,28 @@ 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 { + 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. +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] diff --git a/ethereum/eip712/eip712_test.go b/ethereum/eip712/eip712_test.go new file mode 100644 index 0000000000..d2e933af4f --- /dev/null +++ b/ethereum/eip712/eip712_test.go @@ -0,0 +1,395 @@ +package eip712_test + +import ( + "testing" + + "cosmossdk.io/math" + + "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" + + 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" + + 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" +) + +// Unit tests for single-signer EIP-712 signature verification. Multi-signer verification tests are included +// in ante_test.go. + +type EIP712TestSuite struct { + suite.Suite + + config params.EncodingConfig + 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) + suite.clientCtx = client.Context{}.WithTxConfig(suite.config.TxConfig) + + sdk.GetConfig().SetBech32PrefixForAccount("ethm", "") + eip712.SetEncodingConfig(suite.config) +} + +// Helper to create random test addresses for messages +func (suite *EIP712TestSuite) createTestAddress() sdk.AccAddress { + privkey, _ := ethsecp256k1.GenerateKey() + key, err := privkey.ToECDSA() + suite.Require().NoError(err) + + addr := crypto.PubkeyToAddress(key.PublicKey) + + return addr.Bytes() +} + +// Helper to create random keypair for signing + verification +func (suite *EIP712TestSuite) createTestKeyPair() (*ethsecp256k1.PrivKey, *ethsecp256k1.PubKey) { + privKey, err := ethsecp256k1.GenerateKey() + suite.Require().NoError(err) + + pubKey := ðsecp256k1.PubKey{ + Key: privKey.PubKey().Bytes(), + } + suite.Require().Implements((*cryptotypes.PubKey)(nil), pubKey) + + return privKey, pubKey +} + +// Helper to create instance of sdk.Coins[] with single coin +func (suite *EIP712TestSuite) makeCoins(denom string, amount math.Int) sdk.Coins { + return sdk.NewCoins( + sdk.NewCoin( + denom, + amount, + ), + ) +} + +func (suite *EIP712TestSuite) TestEIP712SignatureVerification() { + suite.SetupTest() + + signModes := []signing.SignMode{ + signing.SignMode_SIGN_MODE_DIRECT, + signing.SignMode_SIGN_MODE_LEGACY_AMINO_JSON, + } + + testCases := []struct { + title string + chainId string + fee txtypes.Fee + memo string + msgs []sdk.Msg + accountNumber uint64 + sequence uint64 + timeoutHeight uint64 + expectSuccess bool + }{ + { + title: "Succeeds - Standard MsgSend", + fee: txtypes.Fee{ + Amount: suite.makeCoins("aphoton", math.NewInt(2000)), + GasLimit: 20000, + }, + memo: "", + msgs: []sdk.Msg{ + banktypes.NewMsgSend( + suite.createTestAddress(), + suite.createTestAddress(), + suite.makeCoins("photon", math.NewInt(1)), + ), + }, + accountNumber: 8, + sequence: 5, + expectSuccess: true, + }, + { + title: "Succeeds - Standard MsgVote", + 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: true, + }, + { + title: "Succeeds - Standard MsgDelegate", + fee: txtypes.Fee{ + Amount: suite.makeCoins("aphoton", math.NewInt(2000)), + GasLimit: 20000, + }, + memo: "", + msgs: []sdk.Msg{ + stakingtypes.NewMsgDelegate( + suite.createTestAddress(), + sdk.ValAddress(suite.createTestAddress()), + suite.makeCoins("photon", math.NewInt(1))[0], + ), + }, + accountNumber: 25, + sequence: 78, + expectSuccess: true, + }, + { + title: "Succeeds - Standard MsgWithdrawDelegationReward", + fee: txtypes.Fee{ + Amount: suite.makeCoins("aphoton", math.NewInt(2000)), + GasLimit: 20000, + }, + memo: "", + msgs: []sdk.Msg{ + distributiontypes.NewMsgWithdrawDelegatorReward( + suite.createTestAddress(), + sdk.ValAddress(suite.createTestAddress()), + ), + }, + accountNumber: 25, + sequence: 78, + expectSuccess: true, + }, + { + title: "Fails - Two MsgVotes", + fee: txtypes.Fee{ + Amount: suite.makeCoins("aphoton", math.NewInt(2000)), + GasLimit: 20000, + }, + memo: "", + msgs: []sdk.Msg{ + govtypes.NewMsgVote( + suite.createTestAddress(), + 5, + govtypes.OptionNo, + ), + govtypes.NewMsgVote( + suite.createTestAddress(), + 25, + govtypes.OptionAbstain, + ), + }, + accountNumber: 25, + sequence: 78, + expectSuccess: false, // Multiple messages are currently not allowed + }, + { + title: "Fails - MsgSend + MsgVote", + fee: txtypes.Fee{ + Amount: suite.makeCoins("aphoton", math.NewInt(2000)), + GasLimit: 20000, + }, + memo: "", + msgs: []sdk.Msg{ + govtypes.NewMsgVote( + suite.createTestAddress(), + 5, + govtypes.OptionNo, + ), + banktypes.NewMsgSend( + suite.createTestAddress(), + suite.createTestAddress(), + suite.makeCoins("photon", math.NewInt(50)), + ), + }, + accountNumber: 25, + 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 { + for _, signMode := range signModes { + 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) + + chainId := "ethermint_9000-1" + if tc.chainId != "" { + chainId = tc.chainId + } + + if tc.timeoutHeight != 0 { + txBuilder.SetTimeoutHeight(tc.timeoutHeight) + } + + // Declare signerData + signerData := authsigning.SignerData{ + ChainID: chainId, + 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 (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 + suite.Require().Error(err) + return + } + + suite.Require().NoError(err) + + sigHash := crypto.Keccak256Hash(eip712Hash) + sig, err := privKey.Sign(sigHash.Bytes()) + 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) + 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) + suite.Require().True(res) + + // 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) + suite.Require().False(res) +} diff --git a/ethereum/eip712/encoding.go b/ethereum/eip712/encoding.go new file mode 100644 index 0000000000..0bb1c1ab44 --- /dev/null +++ b/ethereum/eip712/encoding.go @@ -0,0 +1,221 @@ +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 + 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. +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) { + typedData, err := GetEIP712TypedDataForMsg(signDocBytes) + if err != nil { + return nil, err + } + + domainSeparator, err := typedData.HashStruct("EIP712Domain", typedData.Domain.Map()) + if err != nil { + 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, fmt.Errorf("could not hash EIP-712 primary type: %w", err) + } + rawData := []byte(fmt.Sprintf("\x19\x01%s%s", string(domainSeparator), string(typedDataHash))) + + return rawData, nil +} + +// 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. + typedDataAmino, errAmino := decodeAminoSignDoc(signDocBytes) + if errAmino == nil && verifyEIP712Payload(typedDataAmino) { + return typedDataAmino, nil + } + typedDataProtobuf, errProtobuf := decodeProtobufSignDoc(signDocBytes) + if errProtobuf == nil && verifyEIP712Payload(typedDataProtobuf) { + return typedDataProtobuf, nil + } + + return apitypes.TypedData{}, fmt.Errorf("could not decode sign doc as either Amino or Protobuf. amino: %v protobuf: %v", errAmino, errProtobuf) +} + +// 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 + + if err := ethermintAminoCodec.UnmarshalJSON(signDocBytes, &aminoDoc); err != nil { + return apitypes.TypedData{}, err + } + + // Unwrap fees + var fees legacytx.StdFee + if err := ethermintAminoCodec.UnmarshalJSON(aminoDoc.Fee, &fees); err != nil { + return apitypes.TypedData{}, err + } + + if len(aminoDoc.Msgs) != 1 { + return apitypes.TypedData{}, fmt.Errorf("invalid number of messages in SignDoc, expected 1 but got %v", len(aminoDoc.Msgs)) + } + + var msg cosmosTypes.Msg + if err := ethermintAminoCodec.UnmarshalJSON(aminoDoc.Msgs[0], &msg); err != nil { + return apitypes.TypedData{}, fmt.Errorf("failed to unmarshal first message: %w", err) + } + + // 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{}, fmt.Errorf("could not convert to EIP712 representation: %w", 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{} + if err := signDoc.Unmarshal(signDocBytes); err != nil { + return apitypes.TypedData{}, err + } + + // Decode auth info + authInfo := &txTypes.AuthInfo{} + if err := authInfo.Unmarshal(signDoc.AuthInfoBytes); err != nil { + return apitypes.TypedData{}, err + } + + // Decode body + body := &txTypes.TxBody{} + if err := body.Unmarshal(signDoc.BodyBytes); err != nil { + return apitypes.TypedData{}, err + } + + // 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{}, fmt.Errorf("invalid number of messages, expected 1 got %v", len(body.Messages)) + } + + // 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{}, fmt.Errorf("invalid chain ID passed as argument: %w", err) + } + + // Create StdFee + stdFee := &legacytx.StdFee{ + Amount: authInfo.Fee.Amount, + Gas: authInfo.Fee.GasLimit, + } + + // Parse Message (single message only) + var msg cosmosTypes.Msg + if err := ethermintProtoCodec.UnpackAny(body.Messages[0], &msg); err != nil { + return apitypes.TypedData{}, fmt.Errorf("could not unpack message object with error %w", err) + } + + // Verify single signer (single signer for now) + if len(msg.GetSigners()) != 1 { + return apitypes.TypedData{}, fmt.Errorf("invalid number of signers, expected 1 got %v", len(authInfo.SignerInfos)) + } + + // Init fee payer + feePayer := msg.GetSigners()[0] + feeDelegation := &FeeDelegationOptions{ + FeePayer: feePayer, + } + + // Get tip + tip := authInfo.Tip + + // Create Legacy SignBytes (expected type for WrapTxToTypedData) + 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 +} 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