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

Commit

Permalink
Refactor EIP-712 signature verification (#1397)
Browse files Browse the repository at this point in the history 
* [WIP] EIP-712 Signature Refactor

* Debug and add ante tests

* Add tests for failure cases

* Add changelog entry

* Code cleanup

* Add tests for MsgDelegate and MsgWithdrawDelegationReward

* Update ethereum/eip712/encoding.go

Co-authored-by: Federico Kunze Küllmer <[email protected]>

* Update ethereum/eip712/encoding.go

Co-authored-by: Federico Kunze Küllmer <[email protected]>

* Update ethereum/eip712/encoding.go

Co-authored-by: Federico Kunze Küllmer <[email protected]>

* Update ethereum/eip712/encoding.go

Co-authored-by: Federico Kunze Küllmer <[email protected]>

* Update ethereum/eip712/encoding.go

Co-authored-by: Federico Kunze Küllmer <[email protected]>

* Update ethereum/eip712/encoding.go

Co-authored-by: Federico Kunze Küllmer <[email protected]>

* Code cleanup

* Update ethereum/eip712/encoding.go

Co-authored-by: Federico Kunze Küllmer <[email protected]>

* Minor codefix

* Update ethereum/eip712/encoding.go

* Minor code revision updates

* Refactor EIP712 unit tests to use test suite

* Address import cycle and implement minor refactors

* Fix lint issues

* Add EIP712 unit suite test function

* Update ethereum/eip712/encoding.go

Co-authored-by: Federico Kunze Küllmer <[email protected]>

* Update ethereum/eip712/encoding.go

Co-authored-by: Federico Kunze Küllmer <[email protected]>

* Update ethereum/eip712/encoding.go

Co-authored-by: Federico Kunze Küllmer <[email protected]>

* Add minor refactors; increase test coverage

* Correct ante_test for change in payload

* Add single-signer util and tests

* Update ethereum/eip712/encoding.go

* Update ethereum/eip712/encoding.go

* fix build

Co-authored-by: Federico Kunze Küllmer <[email protected]>
Co-authored-by: Freddy Caceres <[email protected]>
  • Loading branch information
@0a1c @fedekunze @facs95
3 people authored Nov 7, 2022
1 parent 41425fc commit cb632c6
Show file tree
Hide file tree
Showing 8 changed files with 1,126 additions and 9 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
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.
* (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.

Expand Down
298 changes: 297 additions & 1 deletion app/ante/ante_test.go
View file
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,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 {
Expand Down Expand Up @@ -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)
Expand Down
Loading

0 comments on commit cb632c6

Please sign in to comment.