From 89efc0c59847a42632fb4856bda09302b65c5afd Mon Sep 17 00:00:00 2001 From: drklee3 Date: Tue, 10 Sep 2024 12:22:34 -0700 Subject: [PATCH 1/6] fix: Remove VerifyingContract field from EIP712 domain separator --- ethereum/eip712/types.go | 26 ++++++++++++++++++++------ ethereum/eip712/types_test.go | 27 +++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 6 deletions(-) create mode 100644 ethereum/eip712/types_test.go diff --git a/ethereum/eip712/types.go b/ethereum/eip712/types.go index 2cef4539a1..59d82799a0 100644 --- a/ethereum/eip712/types.go +++ b/ethereum/eip712/types.go @@ -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: "", } } @@ -32,7 +46,7 @@ func getRootTypes() apitypes.Types { }, { Name: "verifyingContract", - Type: "string", + Type: "address", }, { Name: "salt", diff --git a/ethereum/eip712/types_test.go b/ethereum/eip712/types_test.go new file mode 100644 index 0000000000..750456a6c4 --- /dev/null +++ b/ethereum/eip712/types_test.go @@ -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") +} From 314ef3c12c9390756a89aefe70e11e826e9f3529 Mon Sep 17 00:00:00 2001 From: drklee3 Date: Tue, 10 Sep 2024 12:41:19 -0700 Subject: [PATCH 2/6] Remove unused EIP712Domain types --- ethereum/eip712/types.go | 8 -------- 1 file changed, 8 deletions(-) diff --git a/ethereum/eip712/types.go b/ethereum/eip712/types.go index 59d82799a0..db3825814f 100644 --- a/ethereum/eip712/types.go +++ b/ethereum/eip712/types.go @@ -44,14 +44,6 @@ func getRootTypes() apitypes.Types { Name: "chainId", Type: "uint256", }, - { - Name: "verifyingContract", - Type: "address", - }, - { - Name: "salt", - Type: "string", - }, }, "Tx": { {Name: "account_number", Type: "string"}, From 4546e35f49f72887714c174718873c12a3ae0884 Mon Sep 17 00:00:00 2001 From: drklee3 Date: Tue, 10 Sep 2024 12:46:52 -0700 Subject: [PATCH 3/6] test: Remove verifyingContract & salt from expected msg types --- ethereum/eip712/msg_test.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/ethereum/eip712/msg_test.go b/ethereum/eip712/msg_test.go index 00b40dbe9a..aa9f217332 100644 --- a/ethereum/eip712/msg_test.go +++ b/ethereum/eip712/msg_test.go @@ -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[]" }, From 1b4cd7b323781e700c78244663f410953607875b Mon Sep 17 00:00:00 2001 From: drklee3 Date: Tue, 10 Sep 2024 16:06:12 -0700 Subject: [PATCH 4/6] ci: Pin golangci-lint to v1.59 --- .github/workflows/lint.yml | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index bf7bb9977e..00dd6be9be 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -7,6 +7,13 @@ on: push: branches: - main +permissions: + # Required: allow read access to the content for analysis. + contents: read + # Optional: allow read access to pull request. Use with `only-new-issues` option. + pull-requests: read + # Optional: allow write access to checks to allow the action to annotate code in the PR. + checks: write jobs: golangci: name: Run golangci-lint @@ -19,7 +26,6 @@ jobs: go-version: '1.21' check-latest: true - uses: actions/checkout@v3 - - uses: technote-space/get-diff-action@v6.1.2 with: PATTERNS: | **/**.go @@ -27,8 +33,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 From 0937314ba85f3155a2601b5080389419dc9eceb2 Mon Sep 17 00:00:00 2001 From: drklee3 Date: Tue, 10 Sep 2024 16:12:16 -0700 Subject: [PATCH 5/6] ci: Remove ci accidentally included permissions --- .github/workflows/lint.yml | 7 ------- 1 file changed, 7 deletions(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 00dd6be9be..c9a9512e20 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -7,13 +7,6 @@ on: push: branches: - main -permissions: - # Required: allow read access to the content for analysis. - contents: read - # Optional: allow read access to pull request. Use with `only-new-issues` option. - pull-requests: read - # Optional: allow write access to checks to allow the action to annotate code in the PR. - checks: write jobs: golangci: name: Run golangci-lint From a9b525ab00df083993441c189b6a7b892284dcf2 Mon Sep 17 00:00:00 2001 From: drklee3 Date: Tue, 10 Sep 2024 16:34:53 -0700 Subject: [PATCH 6/6] ci: Re-add get-diff-action somehow deleted this on accident --- .github/workflows/lint.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index c9a9512e20..16e110d9f7 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -19,6 +19,7 @@ jobs: go-version: '1.21' check-latest: true - uses: actions/checkout@v3 + - uses: technote-space/get-diff-action@v6.1.2 with: PATTERNS: | **/**.go