Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consumer handles expired client to provider #452

Closed
Closed
Show file tree
Hide file tree
Changes from 32 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
d0e0175
handle expired client when sending packets
mpoke Nov 7, 2022
a179f0f
add e2e test
mpoke Nov 7, 2022
e46c46b
add upgradeExpiredClient to e2e tests
mpoke Nov 8, 2022
4f0c74e
improve incrementTime... functions
mpoke Nov 8, 2022
317c9d1
fix golangci-lint error
mpoke Nov 8, 2022
2d1c04a
add client expired check
mpoke Nov 8, 2022
a9b0c18
Merge branch 'main' into marius/435-client-expired
mpoke Nov 8, 2022
795bde6
code for pending data packets
mpoke Nov 8, 2022
a97a952
replace pending SlashRequests w/ peding DataPackets
mpoke Nov 8, 2022
e46cacf
remove pending slash requests from genesis
mpoke Nov 8, 2022
e7abb70
remove SlashRequest from proto
mpoke Nov 8, 2022
bfdbfa5
Merge branch 'main' into marius/435-client-expired
mpoke Nov 9, 2022
6494e5d
add e2e test TestConsumerPacketSendExpiredClient
mpoke Nov 9, 2022
87a8bbe
update QA plan
mpoke Nov 9, 2022
e32ca19
replace incrementTimeBy w/ incrementTime
mpoke Nov 9, 2022
2ca59af
replace AppendPendingVSC w/ AppendPendingVSCs
mpoke Nov 9, 2022
a711dd7
simplify logic of sendValidatorUpdates
mpoke Nov 9, 2022
aaab443
separate PrepareIBCPacketSend from SendIBCPacket
mpoke Nov 9, 2022
1bf3a13
fix merge conflict
mpoke Nov 9, 2022
b63b490
use PrepareIBCPacketSend pattern on consumer
mpoke Nov 9, 2022
65d708b
error handling on SendPacket
mpoke Nov 9, 2022
ea26d53
resolve merge conflicts
mpoke Nov 9, 2022
4aef799
error handling on SendPacket
mpoke Nov 9, 2022
d069a3c
export pending VSC packets
mpoke Nov 9, 2022
b8c2216
avoid trying to send on expired client
mpoke Nov 9, 2022
4aaa6f0
improve comments
mpoke Nov 9, 2022
c39f393
use k.GetCCVTimeoutPeriod
mpoke Nov 9, 2022
ae12b9d
remove GetUpgradeKeeper
mpoke Nov 9, 2022
ad76153
Merge branch 'marius/435-client-expired' into marius/435-client-expir…
mpoke Nov 9, 2022
82770fb
AppendPendingVSCs: use variadic function
mpoke Nov 10, 2022
0fb21c0
Merge branch 'marius/435-client-expired' into marius/435-client-expir…
mpoke Nov 11, 2022
ea86998
fix typo
mpoke Nov 11, 2022
d5fa612
remove unnecessary if
mpoke Nov 11, 2022
5e92047
remove unnecessary if
mpoke Nov 11, 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
6 changes: 3 additions & 3 deletions docs/quality_assurance.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,10 @@ IBC packets:
| -- | ------- | ----------- | ------------ | ----------- | ------------- | ------- |
| 2.01 | Create IBC clients | `Scheduled` (ibc-go) | `Done` [TestCreateConsumerClient](../x/ccv/provider/keeper/proposal_test.go#117), [TestInitGenesis](../x/ccv/consumer/keeper/genesis_test.go#26) | `Done` [SetupTest](../tests/e2e/setup_test.go#39), [TestConsumerGenesis](../tests/e2e/channel_init_test.go#21) | `Future work` | `Scheduled` |
| 2.02 | Create CCV channel (handshake) | `Scheduled` (ibc-go) | `Done` [provider/ibc_module_test.go](../x/ccv/provider/ibc_module_test.go), [consumer/ibc_module_test.go](../x/ccv/consumer/ibc_module_test.go) | `Done` [SetupCCVChannel](../tests/e2e/setup_test.go#125) | `Future work` | `Scheduled` |
| 2.03 | Sending IBC packets <br> [SendIBCPacket](../x/ccv/utils/utils.go#40) | `Scheduled` (ibc-go) | `NA` | `Done` [TestSendVSCMaturedPackets](../tests/e2e/valset_update_test.go#39), [TestSendSlashPacket](../tests/e2e/slashing_test.go#648) | `Done` | `Scheduled` |
| 2.03 | Sending IBC packets | `Scheduled` (ibc-go) | `NA` | `Done` [TestSendVSCMaturedPackets](../tests/e2e/valset_update_test.go#39), [TestSendSlashPacket](../tests/e2e/slashing_test.go#648) | `Done` | `Scheduled` |
| 2.04 | Handling acknowledgments | `Scheduled` (ibc-go) | [Scheduled](https://github.com/cosmos/interchain-security/issues/362) | `Partial coverage` [TestOnAcknowledgementPacket](../x/ccv/consumer/keeper/relay_test.go#152), [TestSlashPacketAcknowldgement](../tests/e2e/slashing_test.go#258) | `Done` | `Scheduled` |
| 2.05 | Handling timeouts | `Scheduled` (ibc-go) | [Scheduled](https://github.com/cosmos/interchain-security/issues/362) |`NA` | `Future work` | `Scheduled` |
| 2.06 | Handling IBC client expiration <br> - high priority| `Scheduled` (ibc-go) | `NA` | `NA` | `Future work` | `Scheduled` |
| 2.06 | Handling IBC client expiration | `Scheduled` (ibc-go) | `NA` | `Done` [expired_client.go](../tests/e2e/expired_client.go) | `Future work` | `Scheduled` |
| 2.07 | ICS-20 channel creation | `Scheduled` (ibc-go) | `NA` | `Done` [SetupTransferChannel](../tests/e2e/setup_test.go#152) |`Future work` | `Scheduled` |
| 2.08 | ICS-20 transfer | `Scheduled` (ibc-go) | `NA` | `Done` [TestRewardsDistribution](../tests/e2e/distribution_test.go#17) | `NA` | `Scheduled` |
| 2.09 | Changes in IBC-GO testing suite | `Scheduled` (ibc-go) | `NA` | `NA` | `Partial coverage` | `NA` |
Expand All @@ -63,7 +63,7 @@ IBC packets:

| ID | Concern | Code Review | Unit Testing | E2E Testing | Diff. Testing | Testnet |
| -- | ------- | ----------- | ------------ | ----------- | ------------- | ------- |
| 3.01 | Changes to staking module | `Done` | `Done` (Cosmos-SDK side) | `Partial coverage` <br> [unbonding_test.go](../tests/e2e/unbonding_test.go) <br> redelegation could be expanded, validator unbonding missing | `Partial coverage` | `Scheduled` |
| 3.01 | Changes to staking module | `Done` | `Done` [unbonding_test.go](https://github.com/cosmos/cosmos-sdk/blob/interchain-security-rebase.0.45.6/x/staking/keeper/unbonding_test.go) | `Partial coverage` <br> [unbonding_test.go](../tests/e2e/unbonding_test.go) <br> redelegation could be expanded, validator unbonding missing | `Partial coverage` | `Scheduled` |
| 3.02 | Changes to slashing module | `Done` | `NA` | `Done` <br> [TestValidatorDowntime](../tests/e2e/slashing_test.go#L502) <br> | `Partial coverage` | `Scheduled` |
| 3.03 | Changes to evidence module | `Done` | `NA` | `Done` <br> [TestValidatorDoubleSigning](../tests/e2e/slashing_test.go#L584) <br> | `NA` | `Scheduled` |

Expand Down
27 changes: 19 additions & 8 deletions proto/interchain_security/ccv/consumer/v1/consumer.proto
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import "interchain_security/ccv/v1/ccv.proto";
option go_package = "github.com/cosmos/interchain-security/x/ccv/consumer/types";

import "google/protobuf/any.proto";
import "cosmos/staking/v1beta1/staking.proto";
import "gogoproto/gogo.proto";
import "cosmos_proto/cosmos.proto";
import "google/protobuf/duration.proto";
Expand Down Expand Up @@ -69,14 +68,26 @@ message CrossChainValidator {
];
}

// SlashRequest defines a slashing request for CCV consumer module
message SlashRequest {
interchain_security.ccv.v1.SlashPacketData packet = 1;
cosmos.staking.v1beta1.InfractionType infraction = 2;
// ConsumerPacketType indicates the consumer packet type.
enum ConsumerPacketType {
option (gogoproto.goproto_enum_prefix) = false;

// UNSPECIFIED packet type
CONSUMER_PACKET_TYPE_UNSPECIFIED = 0 [(gogoproto.enumvalue_customname) = "UnspecifiedPacket"];
// Slash packet
CONSUMER_PACKET_TYPE_SLASH = 1 [(gogoproto.enumvalue_customname) = "SlashPacket"];
// VSCMatured packet
CONSUMER_PACKET_TYPE_VSCM = 2 [(gogoproto.enumvalue_customname) = "VscMaturedPacket"];
}

// DataPacket is a data packet for a specific type of packet
message DataPacket {
ConsumerPacketType type = 1;
bytes data = 2;
}

// SlashRequests is a list of slash requests for CCV consumer module
message SlashRequests {
repeated SlashRequest requests = 1
// DataPackets is a list of data packets
message DataPackets {
repeated DataPacket list = 1
[ (gogoproto.nullable) = false ];
}
3 changes: 0 additions & 3 deletions proto/interchain_security/ccv/consumer/v1/genesis.proto
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,6 @@ message GenesisState {
// OutstandingDowntimes nil on new chain, filled on restart.
repeated OutstandingDowntime outstanding_downtime_slashing = 10
[ (gogoproto.nullable) = false ];
// PendingSlashRequests filled in on new chain, nil on restart.
interchain_security.ccv.consumer.v1.SlashRequests pending_slash_requests = 11
[ (gogoproto.nullable) = false ];
}

// MaturingVSCPacket defines the genesis information for the
Expand Down
4 changes: 2 additions & 2 deletions tests/difference/core/driver/core_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -381,8 +381,8 @@ func (s *CoreSuite) TestAssumptions() {
s.T().Fatal(FAIL_MSG)
}

// Consumer has no slash requests
s.Require().Empty(s.consumerKeeper().GetPendingSlashRequests(s.ctx(C)))
// Consumer has no pending data packets
s.Require().Empty(s.consumerKeeper().GetPendingDataPackets(s.ctx(C)))

// Consumer has no maturities
s.consumerKeeper().IteratePacketMaturityTime(s.ctx(C),
Expand Down
2 changes: 1 addition & 1 deletion tests/difference/core/driver/setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -482,7 +482,7 @@ func (b *Builder) createConsumerGenesis(tmConfig *ibctesting.TendermintConfig) *
consumertypes.DefaultHistoricalEntries,
consumertypes.DefaultConsumerUnbondingPeriod,
)
return consumertypes.NewInitialGenesisState(providerClient, providerConsState, valUpdates, consumertypes.SlashRequests{}, params)
return consumertypes.NewInitialGenesisState(providerClient, providerConsState, valUpdates, params)
}

func (b *Builder) createLink() {
Expand Down
1 change: 1 addition & 0 deletions tests/e2e/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ E2e tests are categorized into files as follows:
- `distribution.go` - e2e tests for the _Reward Distribution_ sub-protocol
- `stop_consumer.go` - e2e tests for the _Consumer Chain Removal_ sub-protocol
- `normal_operations.go` - e2e tests for _normal operations_ of ICS enabled chains
- `expired_client.go` - e2e tests for testing expired clients
- `instance_test.go` - ties the e2e test structure into golang's standard test mechanism, with appropriate definitions for concrete app types and setup callback

To run the e2e tests defined in this repo on any arbitrary consumer and provider implementation, copy the pattern exemplified in `instance_test.go`
2 changes: 1 addition & 1 deletion tests/e2e/channel_init.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ func (suite *CCVTestSuite) TestInitTimeout() {
suite.providerChain.NextBlock()

// increment time
incrementTimeBy(suite, initTimeout)
incrementTime(suite, initTimeout)

// check whether the chain was removed
_, found = providerKeeper.GetConsumerClientId(suite.providerCtx(), chainID)
Expand Down
96 changes: 66 additions & 30 deletions tests/e2e/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
sdk "github.com/cosmos/cosmos-sdk/types"
stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
"github.com/cosmos/interchain-security/testutil/e2e"
consumertypes "github.com/cosmos/interchain-security/x/ccv/consumer/types"
providertypes "github.com/cosmos/interchain-security/x/ccv/provider/types"
ccv "github.com/cosmos/interchain-security/x/ccv/types"
"github.com/stretchr/testify/require"
Expand All @@ -17,6 +16,7 @@ import (
channeltypes "github.com/cosmos/ibc-go/v3/modules/core/04-channel/types"
commitmenttypes "github.com/cosmos/ibc-go/v3/modules/core/23-commitment/types"
"github.com/cosmos/ibc-go/v3/modules/core/exported"
ibctm "github.com/cosmos/ibc-go/v3/modules/light-clients/07-tendermint/types"
ibctmtypes "github.com/cosmos/ibc-go/v3/modules/light-clients/07-tendermint/types"
ibctesting "github.com/cosmos/ibc-go/v3/testing"
)
Expand Down Expand Up @@ -212,29 +212,16 @@ func relayAllCommittedPackets(
// Note that it is expected for the provider unbonding period
// to be one day larger than the consumer unbonding period.
func incrementTimeByUnbondingPeriod(s *CCVTestSuite, chainType ChainType) {
// Get unboding period from staking keeper
// Get unboding periods
providerUnbondingPeriod := s.providerApp.GetStakingKeeper().UnbondingTime(s.providerCtx())
consumerUnbondingPeriod := s.consumerApp.GetConsumerKeeper().GetUnbondingPeriod(s.consumerCtx())
// Note: the assertions below are not strictly necessary, and rely on default values
s.Require().Equal(consumertypes.DefaultConsumerUnbondingPeriod+24*time.Hour, providerUnbondingPeriod, "unexpected provider unbonding period")
s.Require().Equal(consumertypes.DefaultConsumerUnbondingPeriod, consumerUnbondingPeriod, "unexpected consumer unbonding period")
var jumpPeriod time.Duration
if chainType == Provider {
jumpPeriod = providerUnbondingPeriod
} else {
jumpPeriod = consumerUnbondingPeriod
}
// Make sure the clients do not expire
jumpPeriod = jumpPeriod/4 + time.Hour
for i := 0; i < 4; i++ {
s.coordinator.IncrementTimeBy(jumpPeriod)
// Update the provider client on the consumer
err := s.path.EndpointA.UpdateClient()
s.Require().NoError(err)
// Update the consumer client on the provider
err = s.path.EndpointB.UpdateClient()
s.Require().NoError(err)
}
incrementTime(s, jumpPeriod)
}

func checkStakingUnbondingOps(s *CCVTestSuite, id uint64, found bool, onHold bool, msgAndArgs ...interface{}) {
Expand Down Expand Up @@ -350,25 +337,74 @@ func (suite *CCVTestSuite) commitSlashPacket(ctx sdk.Context, packetData ccv.Sla
return channeltypes.CommitPacket(suite.consumerChain.App.AppCodec(), packet)
}

// incrementTimeBy increments the overall time by jumpPeriod
func incrementTimeBy(s *CCVTestSuite, jumpPeriod time.Duration) {
// Get unboding period from staking keeper
consumerUnbondingPeriod := s.consumerApp.GetConsumerKeeper().GetUnbondingPeriod(s.consumerChain.GetContext())
split := 1
trustingPeriodFraction := s.providerApp.GetProviderKeeper().GetTrustingPeriodFraction(s.providerCtx())
if jumpPeriod > consumerUnbondingPeriod/time.Duration(trustingPeriodFraction) {
// Make sure the clients do not expire
split = 4
jumpPeriod = jumpPeriod / 4
// incrementTime increments the overall time by jumpPeriod
// while updating to not expire the clients
func incrementTime(s *CCVTestSuite, jumpPeriod time.Duration) {
// get trusting period of client on provider endpoint
cs, ok := s.providerApp.GetIBCKeeper().ClientKeeper.GetClientState(s.providerCtx(), s.path.EndpointB.ClientID)
s.Require().True(ok)
providerEndpointTP := cs.(*ibctm.ClientState).TrustingPeriod
// get trusting period of client on consumer endpoint
cs, ok = s.consumerApp.GetIBCKeeper().ClientKeeper.GetClientState(s.consumerCtx(), s.path.EndpointA.ClientID)
s.Require().True(ok)
consumerEndpointTP := cs.(*ibctm.ClientState).TrustingPeriod
// find the minimum trusting period
var minTP time.Duration
if providerEndpointTP < consumerEndpointTP {
minTP = providerEndpointTP
} else {
minTP = consumerEndpointTP
}
for i := 0; i < split; i++ {
s.coordinator.IncrementTimeBy(jumpPeriod)
// Update the provider client on the consumer
// jumpStep is the maximum interval at which both clients are updated
jumpStep := minTP / 2
for jumpPeriod > 0 {
var step time.Duration
if jumpPeriod < jumpStep {
step = jumpPeriod
} else {
step = jumpStep
}
s.coordinator.IncrementTimeBy(step)
// update the provider client on the consumer
err := s.path.EndpointA.UpdateClient()
s.Require().NoError(err)
// Update the consumer client on the provider
// update the consumer client on the provider
err = s.path.EndpointB.UpdateClient()
s.Require().NoError(err)
jumpPeriod -= step
}
}

// incrementTimeWithoutUpdate increments the overall time by jumpPeriod
// without updating the client to the `noUpdate` chain
func incrementTimeWithoutUpdate(s *CCVTestSuite, jumpPeriod time.Duration, noUpdate ChainType) {
var trustingPeriod time.Duration
var endpointToUpdate *ibctesting.Endpoint
if noUpdate == Consumer {
cs, ok := s.consumerApp.GetIBCKeeper().ClientKeeper.GetClientState(s.consumerCtx(), s.path.EndpointA.ClientID)
s.Require().True(ok)
trustingPeriod = cs.(*ibctm.ClientState).TrustingPeriod
endpointToUpdate = s.path.EndpointA
} else {
cs, ok := s.providerApp.GetIBCKeeper().ClientKeeper.GetClientState(s.providerCtx(), s.path.EndpointB.ClientID)
s.Require().True(ok)
trustingPeriod = cs.(*ibctm.ClientState).TrustingPeriod
endpointToUpdate = s.path.EndpointB
}
// jumpStep is the maximum interval at which the client on endpointToUpdate is updated
jumpStep := trustingPeriod / 2
for jumpPeriod > 0 {
var step time.Duration
if jumpPeriod < jumpStep {
step = jumpPeriod
} else {
step = jumpStep
}
s.coordinator.IncrementTimeBy(step)
// update the client
err := endpointToUpdate.UpdateClient()
s.Require().NoError(err)
jumpPeriod -= step
}
}

Expand Down
Loading