Skip to content

Commit

Permalink
fix!: Remove unused fields verifyingContract & salt from EIP712 domai…
Browse files Browse the repository at this point in the history
…n separator (#75)

verifyingContract field is validated by MetaMask to be an address, but was expected to be a string. This field is unused so it is removed.
  • Loading branch information
drklee3 authored Sep 19, 2024
1 parent 5869608 commit 65384e0
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 18 deletions.
6 changes: 4 additions & 2 deletions .github/workflows/lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,10 @@ jobs:
go.sum
- uses: golangci/golangci-lint-action@v3.3.1
with:
# Required: the version of golangci-lint is required and must be specified without patch version: we always use the latest patch version.
version: latest
# Required: the version of golangci-lint is required and must be
# specified without patch version: Newer versions are currently not
# passing.
version: v1.59
args: --timeout 10m
github-token: ${{ secrets.github_token }}
# Check only if there are differences in the source code
Expand Down
4 changes: 1 addition & 3 deletions ethereum/eip712/msg_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,7 @@ func TestExtractMsgTypes(t *testing.T) {
"EIP712Domain": [
{ "name": "name", "type": "string" },
{ "name": "version", "type": "string" },
{ "name": "chainId", "type": "uint256" },
{ "name": "verifyingContract", "type": "string" },
{ "name": "salt", "type": "string" }
{ "name": "chainId", "type": "uint256" }
],
"Fee": [
{ "name": "amount", "type": "Coin[]" },
Expand Down
32 changes: 19 additions & 13 deletions ethereum/eip712/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,25 @@ import (

func getTypedDataDomain(chainID uint64) apitypes.TypedDataDomain {
return apitypes.TypedDataDomain{
Name: "Kava Cosmos",
Version: "1.0.0",
ChainId: math.NewHexOrDecimal256(int64(chainID)),
VerifyingContract: "kavaCosmos",
Salt: "0",
Name: "Kava Cosmos",
Version: "1.0.0",
ChainId: math.NewHexOrDecimal256(int64(chainID)),

// Fields below are not used signature verification so they are
// explicitly set empty to exclude them from the hash to be signed.

// Salt in most cases is not used, other chains sometimes set the
// chainID as the salt instead of using the chainId field and not
// together.
// Discussion on salt usage:
// https://github.com/OpenZeppelin/openzeppelin-contracts/issues/4318
Salt: "",

// VerifyingContract is empty as there is no contract that is verifying
// the signature. Signature verification is done in the ante handler.
// Smart contracts that handle EIP712 signatures will include their own
// address in the domain separator.
VerifyingContract: "",
}
}

Expand All @@ -30,14 +44,6 @@ func getRootTypes() apitypes.Types {
Name: "chainId",
Type: "uint256",
},
{
Name: "verifyingContract",
Type: "string",
},
{
Name: "salt",
Type: "string",
},
},
"Tx": {
{Name: "account_number", Type: "string"},
Expand Down
27 changes: 27 additions & 0 deletions ethereum/eip712/types_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package eip712

import (
"testing"

"github.com/stretchr/testify/require"
)

func TestTypedDataDomain(t *testing.T) {
domain := getTypedDataDomain(1234)

domainMap := domain.Map()

// Verify both len and expected contents in order to assert that no other
// fields are present
require.Len(t, domainMap, 3)
require.Contains(t, domainMap, "chainId")
require.Contains(t, domainMap, "name")
require.Contains(t, domainMap, "version")

// Extra check to ensure that the fields that are not used for signature
// verification are not present in the map. Should be in conjunction with
// the checks above to ensure there isn't a different variant of these
// fields present, e.g. different casing.
require.NotContains(t, domainMap, "verifyingContract")
require.NotContains(t, domainMap, "salt")
}

0 comments on commit 65384e0

Please sign in to comment.