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

feat!: remove vscmatured packets #2372

Merged
merged 13 commits into from
Nov 7, 2024
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- `[x/provider]` Allow consumer chains to specify a list of priority validators that are included in the validator set before other validators are considered
([\#2101](https://github.com/cosmos/interchain-security/pull/2101))
2 changes: 2 additions & 0 deletions .changelog/unreleased/features/2372-vscmatured-packets.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- `[x/consumer]` Remove `VSCMaturedPackets`. Consumer-side changes for [ADR 018](https://cosmos.github.io/interchain-security/adrs/adr-018-remove-vscmatured#consumer-changes-r2).
([\#2372](https://github.com/cosmos/interchain-security/pull/2372))
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
- Allow the chain id of a consumer chain to be updated before the chain
- `[x/provider]` Allow the chain id of a consumer chain to be updated before the chain
launches. ([\#2378](https://github.com/cosmos/interchain-security/pull/2378))

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
- Allow consumer chains to specify a list of priority validators that are included in the validator set before other validators are considered
- `[x/provider]` Allow consumer chains to specify a list of priority validators that are included in the validator set before other validators are considered
([\#2101](https://github.com/cosmos/interchain-security/pull/2101))
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- `[x/consumer]` Remove `VSCMaturedPackets`. Consumer-side changes for [ADR 018](https://cosmos.github.io/interchain-security/adrs/adr-018-remove-vscmatured#consumer-changes-r2).
([\#2372](https://github.com/cosmos/interchain-security/pull/2372))
30 changes: 12 additions & 18 deletions proto/interchain_security/ccv/consumer/v1/genesis.proto
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
syntax = "proto3";

Check failure on line 1 in proto/interchain_security/ccv/consumer/v1/genesis.proto

View workflow job for this annotation

GitHub Actions / break-check

Previously present message "MaturingVSCPacket" was deleted from file.

package interchain_security.ccv.consumer.v1;

Expand All @@ -19,7 +19,19 @@
//
// Note: this type is only used on consumer side and references shared types with
// provider
message GenesisState {

Check failure on line 22 in proto/interchain_security/ccv/consumer/v1/genesis.proto

View workflow job for this annotation

GitHub Actions / break-check

Previously present field "5" with name "provider_client_state" on message "GenesisState" was deleted.

Check failure on line 22 in proto/interchain_security/ccv/consumer/v1/genesis.proto

View workflow job for this annotation

GitHub Actions / break-check

Previously present field "6" with name "provider_consensus_state" on message "GenesisState" was deleted.

Check failure on line 22 in proto/interchain_security/ccv/consumer/v1/genesis.proto

View workflow job for this annotation

GitHub Actions / break-check

Previously present field "7" with name "maturing_packets" on message "GenesisState" was deleted.

Check failure on line 22 in proto/interchain_security/ccv/consumer/v1/genesis.proto

View workflow job for this annotation

GitHub Actions / break-check

Previously present field "8" with name "initial_val_set" on message "GenesisState" was deleted.
// Reserve 5th slot for removed provider_client_state field
reserved 5;

// Reserve 6th slot for removed provider_consensus_state field
reserved 6;

// Reserve 7th slot for removed maturing_packets field
reserved 7;

// Reserve 8th slot for removed initial_val_set field
reserved 8;

// ConsumerParams is a shared type with provider module
interchain_security.ccv.v1.ConsumerParams params = 1
[ (gogoproto.nullable) = false ];
Expand All @@ -29,15 +41,6 @@
string provider_channel_id = 3;
// true for new chain, false for chain restart.
bool new_chain = 4;
// !!! DEPRECATED !!! ProviderClientState is deprecated. Use provider.client_state instead
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing this requires that we have yet another consumer genesis migration CLI command.
Could we avoid this change and simply mark it as no-op?

Or is it so that the provider hasn't even returned these fields for a while now?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The provider doesn't return these fields for a while.

  • provider_client_state, provider_consensus_state and initial_val_set are part of the ProviderInfo
  • maturing_packets is not return by the provider as it's empty for new consumer modules

ibc.lightclients.tendermint.v1.ClientState provider_client_state = 5 [ deprecated = true];
// !!! DEPRECATED !!! ProviderConsensusState is deprecated. Use provider.consensus_state instead
ibc.lightclients.tendermint.v1.ConsensusState provider_consensus_state = 6 [ deprecated = true];
// MaturingPackets nil on new chain, filled in on restart.
repeated MaturingVSCPacket maturing_packets = 7
[ (gogoproto.nullable) = false ];
// !!! DEPRECATED !!!! InitialValset is deprecated. Use provider.initial_val_set instead
repeated .tendermint.abci.ValidatorUpdate initial_val_set = 8 [ (gogoproto.nullable) = false, deprecated = true ];
// HeightToValsetUpdateId nil on new chain, filled in on restart.
repeated HeightToValsetUpdateID height_to_valset_update_id = 9
[ (gogoproto.nullable) = false ];
Expand Down Expand Up @@ -73,15 +76,6 @@
// to the consumer CCV module.
message LastTransmissionBlockHeight { int64 height = 1; }

// MaturingVSCPacket represents a vsc packet that is maturing internal to the
// consumer CCV module, where the consumer has not yet relayed a VSCMatured
// packet back to the provider.
message MaturingVSCPacket {
uint64 vscId = 1;
google.protobuf.Timestamp maturity_time = 2
[ (gogoproto.stdtime) = true, (gogoproto.nullable) = false ];
}

// ConsumerPacketDataList is a list of consumer packet data packets.
//
// Note this type is used internally to the consumer CCV module
Expand Down
5 changes: 2 additions & 3 deletions scripts/test_doc/test_documentation.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
| Function | Short Description |
|----------|-------------------|
[TestVSCPacketSendExpiredClient](../../tests/integration/expired_client.go#L29) | TestVSCPacketSendExpiredClient tests queueing of VSCPackets when the consumer client is expired.<details><summary>Details</summary>* Set up a CCV channel and expire the client on consumer chain.<br>* Bond tokens to provider, send CCV packet to consumer and check pending packets.<br>* While the consumer client is expired (or inactive for some reason) all packets will be queued.<br>* The packet sending and checks are then repeated.<br>* More tokens are bonded on provider to change validator powers.<br>* Upgrade expired client to the consumer and all packets are cleared once the consumer client is established.</details> |
[TestConsumerPacketSendExpiredClient](../../tests/integration/expired_client.go#L99) | TestConsumerPacketSendExpiredClient tests the consumer sending packets when the provider client is expired.<details><summary>Details</summary>* Set up a CCV channel and bond tokens on provider.<br>* Send CCV packet to consumer and rebond tokens on provider.<br>* Check for pending VSC packets and relay all VSC packets to consumer.<br>* The provider client is then expired.<br>* Confirm that while the provider client is expired all packets will be queued and then cleared<br>once the provider client is upgraded.</details> |
[TestConsumerPacketSendExpiredClient](../../tests/integration/expired_client.go#L95) | TestConsumerPacketSendExpiredClient tests the consumer sending packets when the provider client is expired.<details><summary>Details</summary>* Set up a CCV channel and bond tokens on provider.<br>* Send CCV packet to consumer and rebond tokens on provider.<br>* Check for pending VSC packets and relay all VSC packets to consumer.<br>* The provider client is then expired.<br>* Confirm that while the provider client is expired all packets will be queued and then cleared<br>once the provider client is upgraded.</details> |
</details>

# [key_assignment.go](../../tests/integration/key_assignment.go)
Expand Down Expand Up @@ -136,7 +136,6 @@

| Function | Short Description |
|----------|-------------------|
[TestPacketRoundtrip](../../tests/integration/valset_update.go#L23) | TestPacketRoundtrip tests a CCV packet roundtrip when tokens are bonded on the provider.<details><summary>Details</summary>* Set up CCV and transfer channels.<br>* Bond some tokens on the provider side in order to change validator power.<br>* Relay a packet from the provider chain to the consumer chain.<br>* Relays a matured packet from the consumer chain back to the provider chain.</details> |
[TestQueueAndSendVSCMaturedPackets](../../tests/integration/valset_update.go#L59) | TestQueueAndSendVSCMaturedPackets tests the behavior of EndBlock QueueVSCMaturedPackets call and its integration with SendPackets call.<details><summary>Details</summary>* Set up CCV channel.<br>* Create and simulate the sending of three VSC packets from the provider chain to the consumer chain at different times.<br>* Send the first packet and validate its processing.<br>* Simulate the passage of one hour.<br>* Send the second packet and validate its processing.<br>* Simulate the passage of 24 more hours.<br>* Send the third packet and validate its processing.<br>* Retrieve all packet maturity times from the consumer, and use this to check the maturity status of the packets sent earlier.<br>* Advance the time so that the first two packets reach their unbonding period, while the third packet does not.<br>* Ensure first two packets are unbonded, their maturity times are deleted, and that VSCMatured packets are queued.<br>* The third packet is still in the store and has not yet been processed for unbonding.<br>* Checks that the packet commitments for the processed packets are correctly reflected in the consumer chain's state.</details> |
[TestPacketRoundtrip](../../tests/integration/valset_update.go#L15) | TestPacketRoundtrip tests a CCV packet roundtrip when tokens are bonded on the provider.<details><summary>Details</summary>* Set up CCV and transfer channels.<br>* Bond some tokens on the provider side in order to change validator power.<br>* Relay a packet from the provider chain to the consumer chain.<br>* Relays a matured packet from the consumer chain back to the provider chain.</details> |
</details>

21 changes: 6 additions & 15 deletions tests/integration/expired_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,10 +82,6 @@ func (s *CCVTestSuite) TestVSCPacketSendExpiredClient() {
s.nextEpoch()
// - relay all VSC packet from provider to consumer
relayAllCommittedPackets(s, s.providerChain, s.path, ccv.ProviderPortID, s.path.EndpointB.ChannelID, 3)
// - increment time so that the unbonding period ends on the consumer
incrementTimeByUnbondingPeriod(s, Consumer)
// - relay all VSCMatured packet from consumer to provider
relayAllCommittedPackets(s, s.consumerChain, s.path, ccv.ConsumerPortID, s.path.EndpointA.ChannelID, 3)
}

// TestConsumerPacketSendExpiredClient tests the consumer sending packets when the provider client is expired.
Expand Down Expand Up @@ -134,10 +130,9 @@ func (s *CCVTestSuite) TestConsumerPacketSendExpiredClient() {
consumerUnbondingPeriod := s.consumerApp.GetConsumerKeeper().GetUnbondingPeriod(s.consumerCtx())
incrementTimeWithoutUpdate(s, consumerUnbondingPeriod+time.Hour, Provider)

// check that the packets were added to the list of pending data packets
// check that no packets were added to the list of pending data packets
consumerPackets := consumerKeeper.GetPendingPackets(s.consumerCtx())
s.Require().NotEmpty(consumerPackets)
s.Require().Len(consumerPackets, 2, "unexpected number of pending data packets")
s.Require().Empty(consumerPackets)

// try to send slash packet for downtime infraction
addr := ed25519.GenPrivKey().PubKey().Address()
Expand All @@ -151,21 +146,21 @@ func (s *CCVTestSuite) TestConsumerPacketSendExpiredClient() {
// check that the packets were added to the list of pending data packets
consumerPackets = consumerKeeper.GetPendingPackets(s.consumerCtx())
s.Require().NotEmpty(consumerPackets)
// At this point we expect 4 packets, two vsc matured packets and two trailing slash packets
s.Require().Len(consumerPackets, 4, "unexpected number of pending data packets")
// At this point we expect two trailing slash packets
s.Require().Len(consumerPackets, 2, "unexpected number of pending data packets")

// upgrade expired client to the consumer
upgradeExpiredClient(s, Provider)

// go to next block to trigger SendPendingPackets
s.consumerChain.NextBlock()

// Check that the leading vsc matured packets were sent and no longer pending
// Check that both slash packets are still pending
consumerPackets = consumerKeeper.GetPendingPackets(s.consumerCtx())
s.Require().Len(consumerPackets, 2, "unexpected number of pending data packets")

// relay committed packets from consumer to provider, first slash packet should be committed
relayAllCommittedPackets(s, s.consumerChain, s.path, ccv.ConsumerPortID, s.path.EndpointA.ChannelID, 3) // two vsc matured + one slash
relayAllCommittedPackets(s, s.consumerChain, s.path, ccv.ConsumerPortID, s.path.EndpointA.ChannelID, 1) // one slash

// First slash has been acked, now only the second slash packet should remain as pending
consumerPackets = consumerKeeper.GetPendingPackets(s.consumerCtx())
Expand All @@ -187,10 +182,6 @@ func (s *CCVTestSuite) TestConsumerPacketSendExpiredClient() {
s.nextEpoch()
// - relay 1 VSC packet from provider to consumer
relayAllCommittedPackets(s, s.providerChain, s.path, ccv.ProviderPortID, s.path.EndpointB.ChannelID, 1)
// - increment time so that the unbonding period ends on the provider
incrementTimeByUnbondingPeriod(s, Consumer)
// - relay 1 VSCMatured packet from consumer to provider
relayAllCommittedPackets(s, s.consumerChain, s.path, ccv.ConsumerPortID, s.path.EndpointA.ChannelID, 1)
}

// expireClient expires the client to the `clientTo` chain
Expand Down
132 changes: 0 additions & 132 deletions tests/integration/valset_update.go
Original file line number Diff line number Diff line change
@@ -1,16 +1,8 @@
package integration

import (
"time"

clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types"

"cosmossdk.io/math"

cryptocodec "github.com/cosmos/cosmos-sdk/crypto/codec"

abci "github.com/cometbft/cometbft/abci/types"

ccv "github.com/cosmos/interchain-security/v6/x/ccv/types"
)

Expand All @@ -34,128 +26,4 @@ func (s *CCVTestSuite) TestPacketRoundtrip() {

// Relay 1 VSC packet from provider to consumer
relayAllCommittedPackets(s, s.providerChain, s.path, ccv.ProviderPortID, s.path.EndpointB.ChannelID, 1)

// Increment time so that the unbonding period ends on the provider
incrementTimeByUnbondingPeriod(s, Provider)

// Relay 1 VSCMatured packet from consumer to provider
relayAllCommittedPackets(s, s.consumerChain, s.path, ccv.ConsumerPortID, s.path.EndpointA.ChannelID, 1)
}

// TestQueueAndSendVSCMaturedPackets tests the behavior of EndBlock QueueVSCMaturedPackets call and its integration with SendPackets call.
// @Long Description@
// * Set up CCV channel.
// * Create and simulate the sending of three VSC packets from the provider chain to the consumer chain at different times.
// * Send the first packet and validate its processing.
// * Simulate the passage of one hour.
// * Send the second packet and validate its processing.
// * Simulate the passage of 24 more hours.
// * Send the third packet and validate its processing.
// * Retrieve all packet maturity times from the consumer, and use this to check the maturity status of the packets sent earlier.
// * Advance the time so that the first two packets reach their unbonding period, while the third packet does not.
// * Ensure first two packets are unbonded, their maturity times are deleted, and that VSCMatured packets are queued.
// * The third packet is still in the store and has not yet been processed for unbonding.
// * Checks that the packet commitments for the processed packets are correctly reflected in the consumer chain's state.
func (suite *CCVTestSuite) TestQueueAndSendVSCMaturedPackets() {
consumerKeeper := suite.consumerApp.GetConsumerKeeper()

// setup CCV channel
suite.SetupCCVChannel(suite.path)

// send 3 packets to consumer chain at different times
pk, err := cryptocodec.FromCmtPubKeyInterface(suite.providerChain.Vals.Validators[0].PubKey)
suite.Require().NoError(err)
pk1, err := cryptocodec.ToCmtProtoPublicKey(pk)
suite.Require().NoError(err)
pk, err = cryptocodec.FromCmtPubKeyInterface(suite.providerChain.Vals.Validators[1].PubKey)
suite.Require().NoError(err)
pk2, err := cryptocodec.ToCmtProtoPublicKey(pk)
suite.Require().NoError(err)

pd := ccv.NewValidatorSetChangePacketData(
[]abci.ValidatorUpdate{
{
PubKey: pk1,
Power: 30,
},
{
PubKey: pk2,
Power: 20,
},
},
1,
nil,
)

// send first packet
packet := suite.newPacketFromProvider(pd.GetBytes(), 1, suite.path, clienttypes.NewHeight(1, 0), 0)
err = consumerKeeper.OnRecvVSCPacket(suite.consumerChain.GetContext(), packet, pd)
suite.Require().Nil(err, "OnRecvVSCPacket did return non-nil error")

// increase time
incrementTime(suite, time.Hour)

// update time and send second packet
pd.ValidatorUpdates[0].Power = 15
pd.ValsetUpdateId = 2
packet.Data = pd.GetBytes()
packet.Sequence = 2
err = consumerKeeper.OnRecvVSCPacket(suite.consumerChain.GetContext(), packet, pd)
suite.Require().Nil(err, "OnRecvVSCPacket did return non-nil error")

// increase time
incrementTime(suite, 24*time.Hour)

// update time and send third packet
pd.ValidatorUpdates[1].Power = 40
pd.ValsetUpdateId = 3
packet.Data = pd.GetBytes()
packet.Sequence = 3
err = consumerKeeper.OnRecvVSCPacket(suite.consumerChain.GetContext(), packet, pd)
suite.Require().Nil(err, "OnRecvVSCPacket did return non-nil error")

packetMaturities := consumerKeeper.GetAllPacketMaturityTimes(suite.consumerChain.GetContext())

// increase time such that first two packets are unbonded but third is not.
unbondingPeriod := consumerKeeper.GetUnbondingPeriod(suite.consumerChain.GetContext())
// increase time
incrementTime(suite, unbondingPeriod-time.Hour)

// ensure first two packets are unbonded and VSCMatured packets are queued
// unbonded time is deleted
suite.Require().False(
consumerKeeper.PacketMaturityTimeExists(
suite.consumerChain.GetContext(),
packetMaturities[0].VscId,
packetMaturities[0].MaturityTime,
),
"maturity time not deleted for mature packet 1",
)
suite.Require().False(
consumerKeeper.PacketMaturityTimeExists(
suite.consumerChain.GetContext(),
packetMaturities[1].VscId,
packetMaturities[1].MaturityTime,
),
"maturity time not deleted for mature packet 2",
)
// ensure that third packet did not get unbonded and is still in store
suite.Require().True(
consumerKeeper.PacketMaturityTimeExists(
suite.consumerChain.GetContext(),
packetMaturities[2].VscId,
packetMaturities[2].MaturityTime,
),
"maturity time for packet 3 is not after current time",
)

// check that the packets are committed in state
commitments := suite.consumerApp.GetIBCKeeper().ChannelKeeper.GetAllPacketCommitmentsAtChannel(
suite.consumerChain.GetContext(),
ccv.ConsumerPortID,
suite.path.EndpointA.ChannelID,
)
suite.Require().Equal(2, len(commitments), "did not find packet commitments")
suite.Require().Equal(uint64(1), commitments[0].Sequence, "did not send VSCMatured packet for VSC packet 1")
suite.Require().Equal(uint64(2), commitments[1].Sequence, "did not send VSCMatured packet for VSC packet 2")
}
6 changes: 0 additions & 6 deletions x/ccv/consumer/keeper/genesis.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,10 +73,6 @@ func (k Keeper) InitGenesis(ctx sdk.Context, state *types.GenesisState) []abci.V
if state.ProviderChannelId != "" {
// set provider channel ID
k.SetProviderChannel(ctx, state.ProviderChannelId)
// set all unbonding sequences
for _, mp := range state.MaturingPackets {
k.SetPacketMaturityTime(ctx, mp.VscId, mp.MaturityTime)
}
// set outstanding downtime slashing requests
for _, od := range state.OutstandingDowntimeSlashing {
consAddr, err := sdk.ConsAddressFromBech32(od.ValidatorConsensusAddress)
Expand Down Expand Up @@ -141,7 +137,6 @@ func (k Keeper) ExportGenesis(ctx sdk.Context) (genesis *types.GenesisState) {
genesis = types.NewRestartGenesisState(
clientID,
channelID,
k.GetAllPacketMaturityTimes(ctx),
valset,
k.GetAllHeightToValsetUpdateIDs(ctx),
pendingPacketsDepreciated,
Expand All @@ -161,7 +156,6 @@ func (k Keeper) ExportGenesis(ctx sdk.Context) (genesis *types.GenesisState) {
genesis = types.NewRestartGenesisState(
clientID,
"",
nil,
valset,
k.GetAllHeightToValsetUpdateIDs(ctx),
pendingPacketsDepreciated,
Expand Down
Loading
Loading