Skip to content
This repository has been archived by the owner on Apr 4, 2024. It is now read-only.

Refactor EIP-712 signature verification #1397

Merged
merged 40 commits into from
Nov 7, 2022
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
7611743
[WIP] EIP-712 Signature Refactor
0a1c Oct 19, 2022
aeda1cf
Merge branch 'main' of https://github.com/evmos/ethermint into austin…
0a1c Oct 20, 2022
e4e8650
Debug and add ante tests
0a1c Oct 20, 2022
afc37f6
Add tests for failure cases
0a1c Oct 21, 2022
57fe441
Add changelog entry
0a1c Oct 21, 2022
998987a
Code cleanup
0a1c Oct 24, 2022
578185c
Add tests for MsgDelegate and MsgWithdrawDelegationReward
0a1c Oct 25, 2022
e941c8e
Update ethereum/eip712/encoding.go
0a1c Oct 25, 2022
14c4cd7
Update ethereum/eip712/encoding.go
0a1c Oct 25, 2022
84f061a
Update ethereum/eip712/encoding.go
0a1c Oct 25, 2022
e93c95e
Update ethereum/eip712/encoding.go
0a1c Oct 25, 2022
e711f77
Update ethereum/eip712/encoding.go
0a1c Oct 25, 2022
e97298f
Update ethereum/eip712/encoding.go
0a1c Oct 25, 2022
8f6f63b
Code cleanup
0a1c Oct 25, 2022
876876b
Update ethereum/eip712/encoding.go
0a1c Oct 26, 2022
6b0481f
Minor codefix
0a1c Oct 26, 2022
b8cb5a4
Merge branch 'main' into austin/ENG-891
0a1c Oct 26, 2022
e08c6af
Update ethereum/eip712/encoding.go
fedekunze Oct 26, 2022
74548b7
Minor code revision updates
0a1c Oct 28, 2022
36ea28f
Refactor EIP712 unit tests to use test suite
0a1c Oct 28, 2022
6a7cd65
Merge branch 'main' into austin/ENG-891
facs95 Oct 31, 2022
d91ea3a
update branch
facs95 Oct 31, 2022
13e91de
Address import cycle and implement minor refactors
0a1c Oct 31, 2022
386a019
Fix lint issues
0a1c Oct 31, 2022
3ef98c8
Add EIP712 unit suite test function
0a1c Oct 31, 2022
5003d85
Update ethereum/eip712/encoding.go
0a1c Nov 1, 2022
13a0f4d
Update ethereum/eip712/encoding.go
0a1c Nov 1, 2022
cdba1ee
Update ethereum/eip712/encoding.go
0a1c Nov 1, 2022
f16265a
Add minor refactors; increase test coverage
0a1c Nov 1, 2022
3a5a525
Correct ante_test for change in payload
0a1c Nov 1, 2022
e93b3db
Add single-signer util and tests
0a1c Nov 1, 2022
47e224c
Update ethereum/eip712/encoding.go
fedekunze Nov 2, 2022
1c075c8
Update ethereum/eip712/encoding.go
fedekunze Nov 2, 2022
f795487
Merge branch 'main' into austin/ENG-891
fedekunze Nov 2, 2022
2b7486e
fix build
facs95 Nov 2, 2022
e4795a2
Merge branch 'main' into austin/ENG-891
facs95 Nov 2, 2022
c265a66
Merge branch 'main' into austin/ENG-891
facs95 Nov 3, 2022
a61c40f
Remove reflect import
0a1c Nov 3, 2022
658ed26
Merge branch 'austin/ENG-891' of https://github.com/evmos/ethermint i…
0a1c Nov 3, 2022
be34c8c
Merge branch 'main' into austin/ENG-891
fedekunze Nov 7, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

### Bug Fixes

Expand Down
178 changes: 177 additions & 1 deletion app/ante/ante_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -507,6 +508,181 @@ 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,
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,
fedekunze marked this conversation as resolved.
Show resolved Hide resolved
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 {
Expand Down Expand Up @@ -939,7 +1115,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)
Expand Down
131 changes: 131 additions & 0 deletions app/ante/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -448,6 +453,132 @@ 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
}

// 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) {
var (
msg []byte
err error
)

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)
}

sigBytes, _ := privKey.Sign(msg)
sigData := &signing.SingleSignatureData{
SignMode: signMode,
Signature: sigBytes,
}

signatures[i] = signing.SignatureV2{
PubKey: privKey.PubKey(),
Data: sigData,
}
}

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()
}

// Re-derive multikey
numKeys := len(privKeys)
multiKey := kmultisig.NewLegacyAminoPubKey(numKeys, pubKeys)

// Create multi-key account
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(multiKey.Address()), big.NewInt(10000000000))

// Init builder
txBuilder := suite.clientCtx.TxConfig.NewTxBuilder()

// Set fees
txBuilder.SetGasLimit(gas)
txBuilder.SetFeeAmount(sdk.NewCoins(
sdk.NewCoin(
"aphoton",
sdk.NewInt(10000),
),
))

// Set message
err := txBuilder.SetMsgs([]sdk.Msg{msg}...)
suite.Require().NoError(err)

// Set memo and tip
txBuilder.SetMemo("")

// Prepare signature field
sig := multisig.NewMultisig(len(pubKeys))
txBuilder.SetSignatures(signing.SignatureV2{
PubKey: multiKey,
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)

// 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 NextFn(ctx sdk.Context, _ sdk.Tx, _ bool) (sdk.Context, error) {
return ctx, nil
}
Expand Down
3 changes: 3 additions & 0 deletions cmd/ethermintd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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",
Expand Down
Loading