Skip to content

Commit

Permalink
VSCPackets should have timeout on provider (#422)
Browse files Browse the repository at this point in the history 
* add provider-based timeout params

* add InitTimeoutTimestamp to store

* add init timeout logic

* params boilerplate code & making tests pass

* add TestInitTimeout* e2e tests

* improve e2e tests; add test case to TestUndelegationDuringInit

* remove VSC timeout

* remove VSC timeout param

* add testcase to TestValidateParams

* handle StopConsumerChain error & gofmt

* add VSC timeout period param

* Fix init timeout conflicts (#409)

* Importable e2e tests (#401)

* fixes

* add comment to GetInitTimeoutTimestamp

* add VscTimeoutTimestamp key and tests

* change VSCTimeoutTimestamp key

* fix e2e tests

* add e2e test

* remove useless code

* improve comment

* copy -> append in provider key definitions (#426)

Update keys.go

* Update tests/e2e/unbonding.go

Co-authored-by: Shawn Marshall-Spitzbart <[email protected]>

* Update x/ccv/provider/keeper/keeper_test.go

Co-authored-by: Shawn Marshall-Spitzbart <[email protected]>

* update comment

* replace removedChainIds w/ chainIdsToRemove

* changing keys from (chainID, ts) to (chainID, vscID)

* make UnbondingOpIndexKey consistent with VscSendingTimestampKey

Co-authored-by: Shawn Marshall-Spitzbart <[email protected]>
  • Loading branch information
@mpoke @shaspitz
mpoke and shaspitz authored Nov 4, 2022
1 parent 9241d1f commit 6c0d718
Show file tree
Hide file tree
Showing 16 changed files with 681 additions and 292 deletions.
2 changes: 1 addition & 1 deletion docs/quality_assurance.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ The main concern addressed in this section is the correctness of the provider ch
| 4.02 | Liveness of redelegations <br> - redelegations entries are eventually removed from `Redelegations` | `Scheduled` | `NA` | `Scheduled` | `Scheduled` | `Scheduled` | `NA` |
| 4.03 | Liveness of validator unbondings <br> - unbonding validators with no delegations are eventually removed from `Validators` | `Scheduled` | `NA` | `NA` | `Done` | `Scheduled` | `NA` |
| 4.04 | Unbonding operations (undelegations, redelegations, validator unbondings) should eventually complete even if the CCV channel is never established (due to error) <br> - expected outcome: the channel initialization sub-protocol eventually times out, which leads to the consumer chain removal | `Scheduled` | `NA` | `Done` [TestUndelegationDuringInit](../tests/e2e/unbonding_test.go#145) | `Future work` | `Scheduled` | `Done` |
| 4.05 | Unbonding operations (undelegations, redelegations, validator unbondings) should eventually complete even if one of the clients expire <br> - expected outcome: the pending VSC packets eventually timeout, which leads to the consumer chain removal <br> - requires https://github.com/cosmos/interchain-security/issues/283 | `Scheduled` | `NA` | `Scheduled` | `Future work` | `Scheduled` | `NA` |
| 4.05 | Unbonding operations (undelegations, redelegations, validator unbondings) should eventually complete even if one of the clients expire <br> - expected outcome: the pending VSC packets eventually timeout, which leads to the consumer chain removal | `Scheduled` | `NA` | `Done` [TestUndelegationVscTimeout](../tests/e2e/unbonding.go#127) | `Future work` | `Scheduled` | `NA` |
| 4.06 | A validator cannot get slashed more than once for double signing, regardless of how many times it double signs on different chains (consumers or provider) | `Scheduled` | `NA` |`Done` <br> [TestHandleSlashPacketErrors](../tests/e2e/slashing_test.go#L317) | `Done` | `Scheduled` | `NA` |
| 4.07 | A validator cannot get slashed multiple times for downtime on the same consumer chain without requesting to `Unjail` itself on the provider chain in between | `Scheduled` | `NA` | `Partial coverage` <br> [TestSendSlashPacket](../tests/e2e/slashing_test.go#L648) | `Partial coverage` | `Scheduled` | `NA` |
| 4.08 | A validator can be slashed multiple times for downtime on different chains | `Scheduled` | `NA` | `Future work` | `NA` | `Scheduled` | `NA` |
Expand Down
6 changes: 6 additions & 0 deletions proto/interchain_security/ccv/provider/v1/provider.proto
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,12 @@ message Params {
// The channel initialization (IBC channel opening handshake) will timeout after this duration
google.protobuf.Duration init_timeout_period = 4
[(gogoproto.nullable) = false, (gogoproto.stdduration) = true];
// The VSC packets sent by the provider will timeout after this duration.
// Note that unlike ccv_timeout_period which is an IBC param,
// the vsc_timeout_period is a provider-side param that enables the provider
// to timeout VSC packets even when a consumer chain is not live.
google.protobuf.Duration vsc_timeout_period = 5
[(gogoproto.nullable) = false, (gogoproto.stdduration) = true];
}

message HandshakeMetadata {
Expand Down
220 changes: 136 additions & 84 deletions tests/e2e/unbonding.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,102 +9,135 @@ import (
ccv "github.com/cosmos/interchain-security/x/ccv/types"
)

// TestUndelegationProviderFirst checks that an unbonding operation completes
// when the unbonding period elapses first on the provider chain
func (s *CCVTestSuite) TestUndelegationProviderFirst() {
s.SetupCCVChannel()
s.SetupTransferChannel()
// TestUndelegationNormalOperation tests that undelegations complete after
// the unbonding period elapses on both the consumer and provider, without
// VSC packets timing out.
func (s *CCVTestSuite) TestUndelegationNormalOperation() {
unbondConsumer := func(expectedPackets int) {
// relay 1 VSC packet from provider to consumer
relayAllCommittedPackets(s, s.providerChain, s.path, ccv.ProviderPortID, s.path.EndpointB.ChannelID, expectedPackets)
// increment time so that the unbonding period ends on the consumer
incrementTimeByUnbondingPeriod(s, Consumer)
// relay 1 VSCMatured packet from consumer to provider
relayAllCommittedPackets(s, s.consumerChain, s.path, ccv.ConsumerPortID, s.path.EndpointA.ChannelID, expectedPackets)
}

// delegate bondAmt and undelegate 1/2 of it
bondAmt := sdk.NewInt(10000000)
delAddr := s.providerChain.SenderAccount.GetAddress()
initBalance, valsetUpdateID := delegateAndUndelegate(s, delAddr, bondAmt, 2)
// - check that staking unbonding op was created and onHold is true
checkStakingUnbondingOps(s, 1, true, true)
// - check that CCV unbonding op was created
checkCCVUnbondingOp(s, s.providerCtx(), s.consumerChain.ChainID, valsetUpdateID, true)
testCases := []struct {
name string
shareDiv int64
unbond func(expBalance, balance sdk.Int)
}{
{
"provider unbonding period elapses first", 2, func(expBalance, balance sdk.Int) {
// increment time so that the unbonding period ends on the provider
incrementTimeByUnbondingPeriod(s, Provider)

// call NextBlock on the provider (which increments the height)
s.providerChain.NextBlock()
// check that onHold is true
checkStakingUnbondingOps(s, 1, true, true, "unbonding should be on hold")

// relay 1 VSC packet from provider to consumer
relayAllCommittedPackets(s, s.providerChain, s.path, ccv.ProviderPortID, s.path.EndpointB.ChannelID, 1)
// check that the unbonding is not complete
s.Require().Equal(expBalance, balance, "unexpected balance after provider unbonding")

// increment time so that the unbonding period ends on the provider
incrementTimeByUnbondingPeriod(s, Provider)
// undelegation complete on consumer
unbondConsumer(1)
},
},
{
"consumer unbonding period elapses first", 2, func(expBalance, balance sdk.Int) {
// undelegation complete on consumer
unbondConsumer(1)

// check that onHold is true
checkStakingUnbondingOps(s, 1, true, true)
// check that the unbonding is not complete
s.Require().True(getBalance(s, s.providerCtx(), delAddr).Equal(initBalance.Sub(bondAmt)))
// check that onHold is false
checkStakingUnbondingOps(s, 1, true, false, "unbonding should be not be on hold")

// increment time so that the unbonding period ends on the consumer
incrementTimeByUnbondingPeriod(s, Consumer)
// check that the unbonding is not complete
s.Require().Equal(expBalance, balance, "unexpected balance after consumer unbonding")

// relay 1 VSCMatured packet from consumer to provider
relayAllCommittedPackets(s, s.consumerChain, s.path, ccv.ConsumerPortID, s.path.EndpointA.ChannelID, 1)
// increment time so that the unbonding period ends on the provider
incrementTimeByUnbondingPeriod(s, Provider)
},
},
{
"no valset changes", 1, func(expBalance, balance sdk.Int) {
// undelegation complete on consumer
unbondConsumer(1)

// check that the unbonding operation completed
// - check that ccv unbonding op has been deleted
checkCCVUnbondingOp(s, s.providerCtx(), s.consumerChain.ChainID, valsetUpdateID, false)
// - check that staking unbonding op has been deleted
checkStakingUnbondingOps(s, valsetUpdateID, false, false)
// - check that half the delegated coins have been returned
s.Require().True(getBalance(s, s.providerCtx(), delAddr).Equal(initBalance.Sub(bondAmt.Quo(sdk.NewInt(2)))))
}
// check that onHold is false
checkStakingUnbondingOps(s, 1, true, false, "unbonding should be not be on hold")

// TestUndelegationConsumerFirst checks that an unbonding operation completes
// when the unbonding period elapses first on the consumer chain
func (s *CCVTestSuite) TestUndelegationConsumerFirst() {
s.SetupCCVChannel()
s.SetupTransferChannel()
// check that the unbonding is not complete
s.Require().Equal(expBalance, balance, "unexpected balance after consumer unbonding")

// delegate bondAmt and undelegate 1/2 of it
bondAmt := sdk.NewInt(10000000)
delAddr := s.providerChain.SenderAccount.GetAddress()
initBalance, valsetUpdateID := delegateAndUndelegate(s, delAddr, bondAmt, 2)
// - check that staking unbonding op was created and onHold is true
checkStakingUnbondingOps(s, 1, true, true)
// - check that CCV unbonding op was created
checkCCVUnbondingOp(s, s.providerCtx(), s.consumerChain.ChainID, valsetUpdateID, true)
// increment time so that the unbonding period ends on the provider
incrementTimeByUnbondingPeriod(s, Provider)
},
},
}

// call NextBlock on the provider (which increments the height)
s.providerChain.NextBlock()
for i, tc := range testCases {
providerKeeper := s.providerApp.GetProviderKeeper()
consumerKeeper := s.consumerApp.GetConsumerKeeper()
stakingKeeper := s.providerApp.GetE2eStakingKeeper()

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

// increment time so that the unbonding period ends on the consumer
incrementTimeByUnbondingPeriod(s, Consumer)
// set VSC timeout period to not trigger the removal of the consumer chain
providerUnbondingPeriod := stakingKeeper.UnbondingTime(s.providerCtx())
consumerUnbondingPeriod := consumerKeeper.GetUnbondingPeriod(s.consumerCtx())
providerKeeper.SetVscTimeoutPeriod(s.providerCtx(), providerUnbondingPeriod+consumerUnbondingPeriod+24*time.Hour)

// relay 1 VSCMatured packet from consumer to provider
relayAllCommittedPackets(s, s.consumerChain, s.path, ccv.ConsumerPortID, s.path.EndpointA.ChannelID, 1)
// delegate bondAmt and undelegate tc.shareDiv of it
bondAmt := sdk.NewInt(10000000)
delAddr := s.providerChain.SenderAccount.GetAddress()
initBalance, valsetUpdateID := delegateAndUndelegate(s, delAddr, bondAmt, tc.shareDiv)
// - check that staking unbonding op was created and onHold is true
checkStakingUnbondingOps(s, 1, true, true, "test: "+tc.name)
// - check that CCV unbonding op was created
checkCCVUnbondingOp(s, s.providerCtx(), s.consumerChain.ChainID, valsetUpdateID, true, "test: "+tc.name)

// check that the unbonding is not complete
s.Require().True(getBalance(s, s.providerCtx(), delAddr).Equal(initBalance.Sub(bondAmt)))
// call NextBlock on the provider (which increments the height)
s.providerChain.NextBlock()

// increment time so that the unbonding period ends on the provider
incrementTimeByUnbondingPeriod(s, Provider)
// unbond both on provider and consumer and check that
// the balance remains unchanged in between
tc.unbond(initBalance.Sub(bondAmt), getBalance(s, s.providerCtx(), delAddr))

// check that the unbonding operation completed
// - check that ccv unbonding op has been deleted
checkCCVUnbondingOp(s, s.providerCtx(), s.consumerChain.ChainID, valsetUpdateID, false, "test: "+tc.name)
// - check that staking unbonding op has been deleted
checkStakingUnbondingOps(s, valsetUpdateID, false, false, "test: "+tc.name)
// - check that necessary delegated coins have been returned
unbondAmt := bondAmt.Sub(bondAmt.Quo(sdk.NewInt(tc.shareDiv)))
s.Require().Equal(
initBalance.Sub(unbondAmt),
getBalance(s, s.providerCtx(), delAddr),
"unexpected initial balance after unbonding; test: %s", tc.name,
)

// check that the unbonding operation completed
// - check that ccv unbonding op has been deleted
checkCCVUnbondingOp(s, s.providerCtx(), s.consumerChain.ChainID, valsetUpdateID, false)
// - check that staking unbonding op has been deleted
checkStakingUnbondingOps(s, valsetUpdateID, false, false)
// - check that half the delegated coins have been returned
s.Require().True(getBalance(s, s.providerCtx(), delAddr).Equal(initBalance.Sub(bondAmt.Quo(sdk.NewInt(2)))))
if i+1 < len(testCases) {
// reset suite to reset provider client
s.SetupTest()
}
}
}

// TestUndelegationNoValsetChange checks that an unbonding operation completes
// even when the validator set is not changed
func (s *CCVTestSuite) TestUndelegationNoValsetChange() {
// TestUndelegationVscTimeout tests that an undelegation
// completes after vscTimeoutPeriod even if it does not
// reach maturity on the consumer chain. In this case,
// the consumer chain is removed.
func (s *CCVTestSuite) TestUndelegationVscTimeout() {
providerKeeper := s.providerApp.GetProviderKeeper()

s.SetupCCVChannel()
s.SetupTransferChannel()

// delegate bondAmt and undelegate all of it
// set VSC timeout period to trigger the removal of the consumer chain
vscTimeout := providerKeeper.GetVscTimeoutPeriod(s.providerCtx())

// delegate bondAmt and undelegate 1/2 of it
bondAmt := sdk.NewInt(10000000)
delAddr := s.providerChain.SenderAccount.GetAddress()
initBalance, valsetUpdateID := delegateAndUndelegate(s, delAddr, bondAmt, 1)
initBalance, valsetUpdateID := delegateAndUndelegate(s, delAddr, bondAmt, 2)
// - check that staking unbonding op was created and onHold is true
checkStakingUnbondingOps(s, 1, true, true)
// - check that CCV unbonding op was created
Expand All @@ -113,28 +146,41 @@ func (s *CCVTestSuite) TestUndelegationNoValsetChange() {
// call NextBlock on the provider (which increments the height)
s.providerChain.NextBlock()

// 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)

// check that onHold is true
checkStakingUnbondingOps(s, 1, true, true, "unbonding should be on hold")

// check that the unbonding is not complete
s.Require().True(getBalance(s, s.providerCtx(), delAddr).Equal(initBalance.Sub(bondAmt)))
s.Require().Equal(
initBalance.Sub(bondAmt),
getBalance(s, s.providerCtx(), delAddr),
"unexpected balance after provider unbonding")

// increment time so that the unbonding period ends on the consumer
incrementTimeByUnbondingPeriod(s, Consumer)
// increment time
incrementTimeBy(s, vscTimeout)

// relay 1 VSCMatured packet from consumer to provider
relayAllCommittedPackets(s, s.consumerChain, s.path, ccv.ConsumerPortID, s.path.EndpointA.ChannelID, 1)
// check whether the chain was removed
chainID := s.consumerChain.ChainID
_, found := providerKeeper.GetConsumerClientId(s.providerCtx(), chainID)
s.Require().Equal(false, found, "consumer chain was not removed")

// increment time so that the unbonding period ends on the provider
incrementTimeByUnbondingPeriod(s, Provider)
// check if the chain was properly removed
s.checkConsumerChainIsRemoved(chainID, false, true)

// check that the unbonding operation completed
// - check that ccv unbonding op has been deleted
checkCCVUnbondingOp(s, s.providerCtx(), s.consumerChain.ChainID, valsetUpdateID, false)
// - check that staking unbonding op has been deleted
checkStakingUnbondingOps(s, valsetUpdateID, false, false)
// - check that all the delegated coins have been returned
s.Require().True(getBalance(s, s.providerCtx(), delAddr).Equal(initBalance))
// - check that necessary delegated coins have been returned
unbondAmt := bondAmt.Sub(bondAmt.Quo(sdk.NewInt(2)))
s.Require().Equal(
initBalance.Sub(unbondAmt),
getBalance(s, s.providerCtx(), delAddr),
"unexpected initial balance after VSC timeout",
)
}

// TestUndelegationDuringInit checks that before the CCV channel is established
Expand Down Expand Up @@ -337,8 +383,14 @@ func (s *CCVTestSuite) TestRedelegationProviderFirst() {
s.SetupTransferChannel()

providerKeeper := s.providerApp.GetProviderKeeper()
consumerKeeper := s.consumerApp.GetConsumerKeeper()
stakingKeeper := s.providerApp.GetE2eStakingKeeper()

// set VSC timeout period to not trigger the removal of the consumer chain
providerUnbondingPeriod := stakingKeeper.UnbondingTime(s.providerCtx())
consumerUnbondingPeriod := consumerKeeper.GetUnbondingPeriod(s.consumerCtx())
providerKeeper.SetVscTimeoutPeriod(s.providerCtx(), providerUnbondingPeriod+consumerUnbondingPeriod+24*time.Hour)

// Setup delegator, bond amount, and src/dst validators
bondAmt := sdk.NewInt(10000000)
delAddr := s.providerChain.SenderAccount.GetAddress()
Expand Down
64 changes: 55 additions & 9 deletions x/ccv/provider/keeper/keeper.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"encoding/binary"
"encoding/json"
"fmt"
"time"

"github.com/cosmos/cosmos-sdk/codec"
sdk "github.com/cosmos/cosmos-sdk/types"
Expand Down Expand Up @@ -359,18 +360,15 @@ func (k Keeper) SetUnbondingOpIndex(ctx sdk.Context, chainID string, valsetUpdat
// IterateOverUnbondingOpIndex iterates over the unbonding indexes for a given chain id.
func (k Keeper) IterateOverUnbondingOpIndex(ctx sdk.Context, chainID string, cb func(vscID uint64, ubdIndex []uint64) bool) {
store := ctx.KVStore(k.storeKey)
iterationPrefix := append([]byte{types.UnbondingOpIndexBytePrefix}, types.HashString(chainID)...)
iterator := sdk.KVStorePrefixIterator(store, iterationPrefix)

iterator := sdk.KVStorePrefixIterator(store, types.ChainIdWithLenKey(types.UnbondingOpIndexBytePrefix, chainID))
defer iterator.Close()

for ; iterator.Valid(); iterator.Next() {
// parse key to get the current VSC ID
var vscID uint64
vscBytes, err := types.ParseUnbondingOpIndexKey(iterator.Key())
_, vscID, err := types.ParseUnbondingOpIndexKey(iterator.Key())
if err != nil {
panic(err)
panic(fmt.Errorf("failed to parse UnbondingOpIndexKey: %w", err))
}
vscID = binary.BigEndian.Uint64(vscBytes)

var index ccv.UnbondingOpsIndex
if err = index.Unmarshal(iterator.Value()); err != nil {
Expand Down Expand Up @@ -784,8 +782,6 @@ func (k Keeper) DeleteConsumerClientId(ctx sdk.Context, chainID string) {
store.Delete(types.ChainToClientKey(chainID))
}

// ------

// SetInitTimeoutTimestamp sets the init timeout timestamp for the given chain ID
func (k Keeper) SetInitTimeoutTimestamp(ctx sdk.Context, chainID string, ts uint64) {
store := ctx.KVStore(k.storeKey)
Expand Down Expand Up @@ -825,3 +821,53 @@ func (k Keeper) IterateInitTimeoutTimestamp(ctx sdk.Context, cb func(chainID str
}
}
}

// SetVscSendTimestamp sets the VSC send timestamp
// for a VSCPacket with ID vscID sent to a chain with ID chainID
func (k Keeper) SetVscSendTimestamp(
ctx sdk.Context,
chainID string,
vscID uint64,
timestamp time.Time,
) {
store := ctx.KVStore(k.storeKey)

// Convert timestamp into bytes for storage
timeBz := sdk.FormatTimeBytes(timestamp)

store.Set(types.VscSendingTimestampKey(chainID, vscID), timeBz)
}

// DeleteVscSendTimestamp removes from the store a specific VSC send timestamp
// for the given chainID and vscID.
func (k Keeper) DeleteVscSendTimestamp(ctx sdk.Context, chainID string, vscID uint64) {
store := ctx.KVStore(k.storeKey)
store.Delete(types.VscSendingTimestampKey(chainID, vscID))
}

// IterateVscSendTimestamps iterates in order (lowest first)
// over the vsc send timestamps of the given chainID.
func (k Keeper) IterateVscSendTimestamps(
ctx sdk.Context,
chainID string,
cb func(vscID uint64, ts time.Time) bool,
) {
store := ctx.KVStore(k.storeKey)
iterator := sdk.KVStorePrefixIterator(store, types.ChainIdWithLenKey(types.VscSendTimestampBytePrefix, chainID))
defer iterator.Close()

for ; iterator.Valid(); iterator.Next() {
key := iterator.Key()
_, vscID, err := types.ParseVscSendingTimestampKey(key)
if err != nil {
panic(fmt.Errorf("failed to parse VscSendTimestampKey: %w", err))
}
ts, err := sdk.ParseTimeBytes(iterator.Value())
if err != nil {
panic(fmt.Errorf("failed to parse timestamp value: %w", err))
}
if !cb(vscID, ts) {
return
}
}
}
Loading

0 comments on commit 6c0d718

Please sign in to comment.