Skip to content

Commit

Permalink
fix!: Validation of SlashAcks fails due to marshaling to Bech32 (#1570)
Browse files Browse the repository at this point in the history
* add different Bech32Prefix for consumer and provider

* separate app encoding and params

* remove ConsumerValPubKey from ValidatorConfig

* update addresses in tests

* make SlashAcks consistent across chains

* add comments for clarity

* Regenerate traces

* Fix argument order

* set bech32prefix for provider to cosmos

* add changelog entries

* add consumer-double-downtime e2e test

* update nightly-e2e workflow

* fix typo

* add consumer-double-downtime to testConfigs

* remove changes on provider

* skip invalid SlashAcks

* seal the config

* clear the outstanding downtime flag for new vals

* add info on upgrading to v4.0.0

* fix upgrade handler

* fix changeover e2e test

* Update tests/e2e/config.go

Co-authored-by: Philip Offtermatt <57488781+p-offtermatt@users.noreply.github.com>

* Update tests/e2e/config.go

Co-authored-by: Philip Offtermatt <57488781+p-offtermatt@users.noreply.github.com>

* add AccountPrefix to ChainConfig

* fix docstrings

* update AccountAddressPrefix in app.go

* fix consumer-misb e2e test

---------

Co-authored-by: Philip Offtermatt <p.offtermatt@gmail.com>
Co-authored-by: Simon Noetzlin <simon.ntz@gmail.com>
Co-authored-by: Philip Offtermatt <57488781+p-offtermatt@users.noreply.github.com>
  • Loading branch information
4 people committed Jan 19, 2024
1 parent dda626b commit 8604692
Show file tree
Hide file tree
Showing 42 changed files with 756 additions and 500 deletions.
2 changes: 2 additions & 0 deletions .changelog/unreleased/bug-fixes/1570-slashack-bech32.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- Fix the validation of VSCPackets to not fail due to marshaling to string using Bech32.
([\#1570](https://github.com/cosmos/interchain-security/pull/1570))
2 changes: 2 additions & 0 deletions .changelog/unreleased/state-breaking/1570-slashack-bech32.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- Fix the validation of VSCPackets to not fail due to marshaling to string using Bech32.
([\#1570](https://github.com/cosmos/interchain-security/pull/1570))
103 changes: 0 additions & 103 deletions .github/workflows/manual-e2e.yml

This file was deleted.

55 changes: 52 additions & 3 deletions .github/workflows/nightly-e2e.yml
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
# Run integration tests nightly on main

# !! Relevant changes to this file should be propagated manual-integration.yml

name: nightly-e2e-main
on:
workflow_dispatch:
schedule:
# run every day at 03:00 UTC
# ┌───────────── minute (0 - 59)
Expand Down Expand Up @@ -114,6 +112,54 @@ jobs:
go-version: "1.21" # The Go version to download (if necessary) and use.
- name: E2E multi-consumer tests
run: go run ./tests/e2e/... --tc multiconsumer
consumer-misbehaviour-test:
runs-on: ubuntu-latest
timeout-minutes: 20
steps:
- uses: actions/setup-go@v5
with:
go-version: "1.21"
- uses: actions/checkout@v4
- name: Checkout LFS objects
run: git lfs checkout
- name: Setup Go
uses: actions/setup-go@v5
with:
go-version: "1.21" # The Go version to download (if necessary) and use.
- name: E2E consumer-misbehaviour tests
run: go run ./tests/e2e/... --tc consumer-misbehaviour
consumer-double-sign-test:
runs-on: ubuntu-latest
timeout-minutes: 20
steps:
- uses: actions/setup-go@v5
with:
go-version: "1.21"
- uses: actions/checkout@v4
- name: Checkout LFS objects
run: git lfs checkout
- name: Setup Go
uses: actions/setup-go@v5
with:
go-version: "1.21" # The Go version to download (if necessary) and use.
- name: E2E consumer-double-sign tests
run: go run ./tests/e2e/... --tc consumer-double-sign
consumer-double-downtime-test:
runs-on: ubuntu-latest
timeout-minutes: 20
steps:
- uses: actions/setup-go@v5
with:
go-version: "1.21"
- uses: actions/checkout@v4
- name: Checkout LFS objects
run: git lfs checkout
- name: Setup Go
uses: actions/setup-go@v5
with:
go-version: "1.21" # The Go version to download (if necessary) and use.
- name: E2E consumer-double-downtime tests
run: go run ./tests/e2e/... --tc consumer-double-downtime

nightly-test-fail:
needs:
Expand All @@ -123,6 +169,9 @@ jobs:
- democracy-test
- slash-throttle-test
- multiconsumer-test
- consumer-misbehaviour-test
- consumer-double-sign-test
- consumer-double-downtime-test
if: ${{ failure() }}
runs-on: ubuntu-latest
steps:
Expand Down
22 changes: 21 additions & 1 deletion UPGRADING.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,29 @@ Upgrading a provider from `v3.3.0` to `v4.0.0` will require state migrations, se

### Consumer

***Note that consumer chains can upgrade directly from `v3.1.0` to `v4.0.0`.***

Upgrading a consumer from `v3.2.0` to `v4.0.0` will not require state migration, however, upgrading directly from `v3.1.0` to `v4.0.0` will require state migrations, see https://github.com/cosmos/interchain-security/blob/release/v4.0.x/x/ccv/consumer/keeper/migrations.go#L22.

Note that consumer chains can upgrade directly from `v3.1.0` to `v4.0.0`.
In addition, the following migration needs to be added to the upgrade handler of the consumer chain:
```golang
func migrateICSOutstandingDowntime(ctx sdk.Context, keepers *upgrades.UpgradeKeepers) error {
ctx.Logger().Info("Migrating ICS oustanding downtime...")

downtimes := keepers.ConsumerKeeper.GetAllOutstandingDowntimes(ctx)
for _, od := range downtimes {
consAddr, err := sdk.ConsAddressFromBech32(od.ValidatorConsensusAddress)
if err != nil {
return err
}
keepers.ConsumerKeeper.DeleteOutstandingDowntime(ctx, consAddr)
}

ctx.Logger().Info("Finished ICS oustanding downtime")

return nil
}
```

## [v3.3.x](https://github.com/cosmos/interchain-security/tree/release/v3.2.x)

Expand Down
12 changes: 6 additions & 6 deletions app/consumer-democracy/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ import (
"github.com/cometbft/cometbft/libs/log"
tmos "github.com/cometbft/cometbft/libs/os"

appparams "github.com/cosmos/interchain-security/v4/app/params"
appencoding "github.com/cosmos/interchain-security/v4/app/encoding"
testutil "github.com/cosmos/interchain-security/v4/testutil/integration"
consumer "github.com/cosmos/interchain-security/v4/x/ccv/consumer"
consumerkeeper "github.com/cosmos/interchain-security/v4/x/ccv/consumer/keeper"
Expand All @@ -116,7 +116,7 @@ import (
const (
AppName = "interchain-security-cd"
upgradeName = "sovereign-changeover" // arbitrary name, define your own appropriately named upgrade
AccountAddressPrefix = "cosmos"
AccountAddressPrefix = "consumer"
)

var (
Expand Down Expand Up @@ -1005,17 +1005,17 @@ func initParamsKeeper(appCodec codec.BinaryCodec, legacyAmino *codec.LegacyAmino
// should be used only in tests or when creating a new app instance (NewApp*()).
// App user shouldn't create new codecs - use the app.AppCodec instead.
// [DEPRECATED]
func MakeTestEncodingConfig() appparams.EncodingConfig {
encodingConfig := appparams.MakeTestEncodingConfig()
func MakeTestEncodingConfig() appencoding.EncodingConfig {
encodingConfig := appencoding.MakeTestEncodingConfig()
std.RegisterLegacyAminoCodec(encodingConfig.Amino)
std.RegisterInterfaces(encodingConfig.InterfaceRegistry)
ModuleBasics.RegisterLegacyAminoCodec(encodingConfig.Amino)
ModuleBasics.RegisterInterfaces(encodingConfig.InterfaceRegistry)
return encodingConfig
}

func makeEncodingConfig() appparams.EncodingConfig {
encodingConfig := appparams.MakeTestEncodingConfig()
func makeEncodingConfig() appencoding.EncodingConfig {
encodingConfig := appencoding.MakeTestEncodingConfig()
std.RegisterLegacyAminoCodec(encodingConfig.Amino)
std.RegisterInterfaces(encodingConfig.InterfaceRegistry)
ModuleBasics.RegisterLegacyAminoCodec(encodingConfig.Amino)
Expand Down
4 changes: 2 additions & 2 deletions app/consumer/ante/disabled_modules_ante_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@ import (
slashingtypes "github.com/cosmos/cosmos-sdk/x/slashing/types"

"github.com/cosmos/interchain-security/v4/app/consumer/ante"
"github.com/cosmos/interchain-security/v4/app/params"
appencoding "github.com/cosmos/interchain-security/v4/app/encoding"
)

func TestDisabledModulesDecorator(t *testing.T) {
txCfg := params.MakeTestEncodingConfig().TxConfig
txCfg := appencoding.MakeTestEncodingConfig().TxConfig
authzMsgExecSlashing := authz.NewMsgExec(sdk.AccAddress{}, []sdk.Msg{&slashingtypes.MsgUnjail{}})
authzMsgExecEvidence := authz.NewMsgExec(sdk.AccAddress{}, []sdk.Msg{&evidencetypes.MsgSubmitEvidence{}})
nestedAuthzMsgExecSlashing := authz.NewMsgExec(sdk.AccAddress{}, []sdk.Msg{&authzMsgExecSlashing})
Expand Down
4 changes: 2 additions & 2 deletions app/consumer/ante/msg_filter_ante_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (
banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"

"github.com/cosmos/interchain-security/v4/app/consumer/ante"
"github.com/cosmos/interchain-security/v4/app/params"
appencoding "github.com/cosmos/interchain-security/v4/app/encoding"
)

type consumerKeeper struct {
Expand All @@ -28,7 +28,7 @@ func noOpAnteDecorator() sdk.AnteHandler {
}

func TestMsgFilterDecorator(t *testing.T) {
txCfg := params.MakeTestEncodingConfig().TxConfig
txCfg := appencoding.MakeTestEncodingConfig().TxConfig

testCases := []struct {
name string
Expand Down
12 changes: 6 additions & 6 deletions app/consumer/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ import (
"github.com/cometbft/cometbft/libs/log"
tmos "github.com/cometbft/cometbft/libs/os"

appparams "github.com/cosmos/interchain-security/v4/app/params"
appencoding "github.com/cosmos/interchain-security/v4/app/encoding"
testutil "github.com/cosmos/interchain-security/v4/testutil/integration"
ibcconsumer "github.com/cosmos/interchain-security/v4/x/ccv/consumer"
ibcconsumerkeeper "github.com/cosmos/interchain-security/v4/x/ccv/consumer/keeper"
Expand All @@ -97,7 +97,7 @@ import (
const (
AppName = "interchain-security-c"
upgradeName = "ics-v1-to-v2"
AccountAddressPrefix = "cosmos"
AccountAddressPrefix = "consumer"
)

var (
Expand Down Expand Up @@ -830,17 +830,17 @@ func initParamsKeeper(appCodec codec.BinaryCodec, legacyAmino *codec.LegacyAmino
// return encodingConfig
// }

func MakeTestEncodingConfig() appparams.EncodingConfig {
encodingConfig := appparams.MakeTestEncodingConfig()
func MakeTestEncodingConfig() appencoding.EncodingConfig {
encodingConfig := appencoding.MakeTestEncodingConfig()
std.RegisterLegacyAminoCodec(encodingConfig.Amino)
std.RegisterInterfaces(encodingConfig.InterfaceRegistry)
ModuleBasics.RegisterLegacyAminoCodec(encodingConfig.Amino)
ModuleBasics.RegisterInterfaces(encodingConfig.InterfaceRegistry)
return encodingConfig
}

func makeEncodingConfig() appparams.EncodingConfig {
encodingConfig := appparams.MakeTestEncodingConfig()
func makeEncodingConfig() appencoding.EncodingConfig {
encodingConfig := appencoding.MakeTestEncodingConfig()
std.RegisterLegacyAminoCodec(encodingConfig.Amino)
std.RegisterInterfaces(encodingConfig.InterfaceRegistry)
ModuleBasics.RegisterLegacyAminoCodec(encodingConfig.Amino)
Expand Down
12 changes: 11 additions & 1 deletion app/params/proto.go → app/encoding/encoding.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,21 @@
package params
package encoding

import (
"github.com/cosmos/cosmos-sdk/client"
"github.com/cosmos/cosmos-sdk/codec"
"github.com/cosmos/cosmos-sdk/codec/types"
"github.com/cosmos/cosmos-sdk/x/auth/tx"
)

// EncodingConfig specifies the concrete encoding types to use for a given app.
// This is provided for compatibility between protobuf and amino implementations.
type EncodingConfig struct {
InterfaceRegistry types.InterfaceRegistry
Codec codec.Codec
TxConfig client.TxConfig
Amino *codec.LegacyAmino
}

// MakeTestEncodingConfig creates an EncodingConfig for an amino based test configuration.
func MakeTestEncodingConfig() EncodingConfig {
amino := codec.NewLegacyAmino()
Expand Down
Loading

0 comments on commit 8604692

Please sign in to comment.