Redelegation integration tests (#312)

* rebase

* just e2e test left

* small

* Update valset_update_test.go

* fix redelegate function per comment

* move to unbonding_test

* check redeg entry is gone

* add redelegation test

* small

Co-authored-by: Jehan <[email protected]>

Dockerfile

@@ -23,7 +23,7 @@ RUN go mod tidy
 RUN make install
 
 # Get Hermes build
-FROM informalsystems/hermes:1.0.0-rc.1 AS hermes-builder
+FROM informalsystems/hermes:1.0.0 AS hermes-builder
 
 FROM --platform=linux/amd64 fedora:36
 RUN dnf update -y

e2e-tests/common_test.go

@@ -77,6 +77,43 @@ func delegateAndUndelegate(s *ProviderTestSuite, delAddr sdk.AccAddress, bondAmt
 	return initBalance, valsetUpdateId
 }
 
+// Delegates "amount" to a source validator, then redelegates that same amount to a dest validator,
+// with related state assertions along the way.
+//
+// Note: This function advances blocks in-between operations, where validator powers are
+// not checked, since they are checked in integration tests.
+func delegateAndRedelegate(s *ProviderTestSuite, delAddr sdk.AccAddress,
+	srcValAddr sdk.ValAddress, dstValAddr sdk.ValAddress, amount sdk.Int) {
+
+	stakingKeeper := s.providerChain.App.(*appProvider.App).StakingKeeper
+
+	// Delegate to src validator
+	srcValTokensBefore := stakingKeeper.Validator(s.providerCtx(), srcValAddr).GetBondedTokens()
+	_, sharesDelegated, _ := delegate(s, delAddr, amount)
+
+	// Assert expected amount was bonded to src validator
+	srcValTokensAfter := stakingKeeper.Validator(s.providerCtx(), srcValAddr).GetBondedTokens()
+	s.Require().Equal(srcValTokensAfter.Sub(srcValTokensBefore), amount)
+
+	s.providerChain.NextBlock()
+
+	dstValTokensBefore := stakingKeeper.Validator(s.providerCtx(), dstValAddr).GetBondedTokens()
+
+	// redelegate shares from src to dst validators
+	redelegate(s, delAddr,
+		srcValAddr,
+		dstValAddr,
+		sharesDelegated,
+	)
+
+	// Assert expected amount was delegated to dst val
+	dstValTokensAfter := stakingKeeper.Validator(s.providerCtx(), dstValAddr).GetBondedTokens()
+	s.Require().Equal(dstValTokensAfter.Sub(dstValTokensBefore), amount)
+
+	// Assert delegated tokens amount returned to original value for src validator
+	s.Require().Equal(srcValTokensBefore, stakingKeeper.Validator(s.providerCtx(), srcValAddr).GetBondedTokens())
+}
+
 // delegate delegates bondAmt to the first validator
 func delegate(s *ProviderTestSuite, delAddr sdk.AccAddress, bondAmt sdk.Int) (initBalance sdk.Int, shares sdk.Dec, valAddr sdk.ValAddress) {
 	initBalance = getBalance(s, s.providerCtx(), delAddr)
@@ -108,6 +145,38 @@ func undelegate(s *ProviderTestSuite, delAddr sdk.AccAddress, valAddr sdk.ValAdd
 	return valsetUpdateID
 }
 
+// Executes a BeginRedelegation (unbonding and redelegation) operation
+// on the provider chain using delegated funds from delAddr
+func redelegate(s *ProviderTestSuite, delAddr sdk.AccAddress, valSrcAddr sdk.ValAddress,
+	ValDstAddr sdk.ValAddress, sharesAmount sdk.Dec) {
+	ctx := s.providerCtx()
+
+	// delegate bondAmt tokens on provider to change validator powers
+	completionTime, err := s.providerChain.App.(*appProvider.App).StakingKeeper.BeginRedelegation(
+		ctx,
+		delAddr,
+		valSrcAddr,
+		ValDstAddr,
+		sharesAmount,
+	)
+	s.Require().NoError(err)
+
+	stakingKeeper := s.providerChain.App.(*appProvider.App).StakingKeeper
+	providerUnbondingPeriod := stakingKeeper.UnbondingTime(ctx)
+
+	valSrc, found := stakingKeeper.GetValidator(ctx, valSrcAddr)
+	s.Require().True(found)
+
+	// Completion time of redelegation operation will be after unbonding period if source val is bonded
+	if valSrc.IsBonded() {
+		s.Require().Equal(ctx.BlockHeader().Time.Add(providerUnbondingPeriod), completionTime)
+	}
+	// Completion time of redelegation operation will be at unbonding time of validator if it's unbonding
+	if valSrc.IsUnbonding() {
+		s.Require().Equal(valSrc.UnbondingTime, completionTime)
+	}
+}
+
 // relayAllCommittedPackets relays all committed packets from `srcChain` on `path`
 func relayAllCommittedPackets(
 	s *ProviderTestSuite,
@@ -123,7 +192,8 @@ func relayAllCommittedPackets(
 		portID,
 		channelID,
 	)
-	s.Require().Equal(expectedPackets, len(commitments), "did not find packet commitments")
+	s.Require().Equal(expectedPackets, len(commitments),
+		"actual number of packet commitments does not match expectation")
 
 	// relay all packets from srcChain to counterparty
 	for _, commitment := range commitments {
@@ -137,8 +207,8 @@ func relayAllCommittedPackets(
 }
 
 // incrementTimeByUnbondingPeriod increments the overall time by
-//   - if provider == true, the unbonding period on the provider;
-//   - otherwise, the unbonding period on the consumer.
+//  - if chainType == Provider, the unbonding period on the provider.
+//  - otherwise, the unbonding period on the consumer.
 //
 // Note that it is expected for the provider unbonding period
 // to be one day larger than the consumer unbonding period.
@@ -185,6 +255,24 @@ func checkCCVUnbondingOp(s *ProviderTestSuite, providerCtx sdk.Context, chainID
 	}
 }
 
+// Checks that an expected amount of redelegations exist for a delegator
+// via the staking keeper, then returns those redelegations.
+func checkRedelegations(s *ProviderTestSuite, delAddr sdk.AccAddress,
+	expect uint16) []stakingtypes.Redelegation {
+
+	redelegations := s.providerChain.App.(*appProvider.App).StakingKeeper.
+		GetRedelegations(s.providerCtx(), delAddr, 2)
+
+	s.Require().Len(redelegations, int(expect))
+	return redelegations
+}
+
+// Checks that a redelegation entry has a completion time equal to an expected time
+func checkRedelegationEntryCompletionTime(
+	s *ProviderTestSuite, entry stakingtypes.RedelegationEntry, expectedCompletion time.Time) {
+	s.Require().Equal(expectedCompletion, entry.CompletionTime)
+}
+
 func getStakingUnbondingDelegationEntry(ctx sdk.Context, k stakingkeeper.Keeper, id uint64) (stakingUnbondingOp stakingtypes.UnbondingDelegationEntry, found bool) {
 	stakingUbd, found := k.GetUnbondingDelegationByUnbondingId(ctx, id)
 

e2e-tests/unbonding_test.go

@@ -214,7 +214,7 @@ func (s *ProviderTestSuite) TestUnbondingNoConsumer() {
 	checkCCVUnbondingOp(s, s.providerCtx(), s.consumerChain.ChainID, valsetUpdateID, false)
 
 	// increment time so that the unbonding period ends on the provider;
-	// cannot use incrementTimeByProviderUnbondingPeriod() since it tries
+	// cannot use incrementTimeByUnbondingPeriod() since it tries
 	// to also update the provider's client on the consumer
 	providerUnbondingPeriod := s.providerChain.App.GetStakingKeeper().UnbondingTime(s.providerCtx())
 	s.coordinator.IncrementTimeBy(providerUnbondingPeriod + time.Hour)
@@ -228,3 +228,119 @@ func (s *ProviderTestSuite) TestUnbondingNoConsumer() {
 	// - check that half the coins have been returned
 	s.Require().True(getBalance(s, s.providerCtx(), delAddr).Equal(initBalance.Sub(bondAmt.Quo(sdk.NewInt(2)))))
 }
+
+// TestRedelegationNoConsumer tests a redelegate transaction
+// submitted on a provider chain with no consumers
+func (s *ProviderTestSuite) TestRedelegationNoConsumer() {
+
+	providerKeeper := s.providerChain.App.(*appProvider.App).ProviderKeeper
+	stakingKeeper := s.providerChain.App.(*appProvider.App).StakingKeeper
+
+	// remove the consumer chain, which was already registered during setup
+	providerKeeper.DeleteConsumerClientId(s.providerCtx(), s.consumerChain.ChainID)
+
+	// Setup delegator, bond amount, and src/dst validators
+	bondAmt := sdk.NewInt(10000000)
+	delAddr := s.providerChain.SenderAccount.GetAddress()
+	_, srcVal := s.getVal(0)
+	_, dstVal := s.getVal(1)
+
+	delegateAndRedelegate(
+		s,
+		delAddr,
+		srcVal,
+		dstVal,
+		bondAmt,
+	)
+
+	// 1 redelegation record should exist for original delegator
+	redelegations := checkRedelegations(s, delAddr, 1)
+
+	// Check that the only entry has appropriate maturation time, the unbonding period from now
+	checkRedelegationEntryCompletionTime(
+		s,
+		redelegations[0].Entries[0],
+		s.providerCtx().BlockTime().Add(stakingKeeper.UnbondingTime(s.providerCtx())),
+	)
+
+	// Increment time so that the unbonding period passes on the provider
+	incrementTimeByUnbondingPeriod(s, Provider)
+
+	// Call NextBlock on the provider (which increments the height)
+	s.providerChain.NextBlock()
+
+	// No redelegation records should exist for original delegator anymore
+	checkRedelegations(s, delAddr, 0)
+}
+
+// TestRedelegationWithConsumer tests a redelegate transaction submitted on a provider chain
+// when the unbonding period elapses first on the provider chain
+func (s *ProviderTestSuite) TestRedelegationProviderFirst() {
+	s.SetupCCVChannel()
+
+	stakingKeeper := s.providerChain.App.(*appProvider.App).StakingKeeper
+	providerKeeper := s.providerChain.App.(*appProvider.App).ProviderKeeper
+
+	// Setup delegator, bond amount, and src/dst validators
+	bondAmt := sdk.NewInt(10000000)
+	delAddr := s.providerChain.SenderAccount.GetAddress()
+	_, srcVal := s.getVal(0)
+	_, dstVal := s.getVal(1)
+
+	delegateAndRedelegate(
+		s,
+		delAddr,
+		srcVal,
+		dstVal,
+		bondAmt,
+	)
+
+	// 1 redelegation record should exist for original delegator
+	redelegations := checkRedelegations(s, delAddr, 1)
+
+	// Check that the only entry has appropriate maturation time, the unbonding period from now
+	checkRedelegationEntryCompletionTime(
+		s,
+		redelegations[0].Entries[0],
+		s.providerCtx().BlockTime().Add(stakingKeeper.UnbondingTime(s.providerCtx())),
+	)
+
+	// Save the current valset update ID
+	valsetUpdateID := providerKeeper.GetValidatorSetUpdateId(s.providerCtx())
+
+	// Check that CCV unbonding op was created from AfterUnbondingInitiated hook
+	checkCCVUnbondingOp(s, s.providerCtx(), s.consumerChain.ChainID, valsetUpdateID, true)
+
+	// Call NextBlock on the provider (which increments the height)
+	s.providerChain.NextBlock()
+
+	// Relay 2 VSC packets from provider to consumer (original delegation, and redelegation)
+	relayAllCommittedPackets(s, s.providerChain, s.path,
+		ccv.ProviderPortID, s.path.EndpointB.ChannelID, 2)
+
+	// Increment time so that the unbonding period ends on the provider
+	incrementTimeByUnbondingPeriod(s, Provider)
+
+	// 1 redelegation record should still exist for original delegator on provider
+	checkRedelegations(s, delAddr, 1)
+
+	// CCV unbonding op should also still exist
+	checkCCVUnbondingOp(s, s.providerCtx(), s.consumerChain.ChainID, valsetUpdateID, true)
+
+	// Increment time so that the unbonding period ends on the consumer
+	incrementTimeByUnbondingPeriod(s, Consumer)
+
+	// Relay 2 VSCMatured packets from consumer to provider (original delegation and redelegation)
+	relayAllCommittedPackets(s, s.consumerChain,
+		s.path, ccv.ConsumerPortID, s.path.EndpointA.ChannelID, 2)
+
+	//
+	// Check that the redelegation operation has now completed on provider
+	//
+
+	// Redelegation record should be deleted for original delegator
+	checkRedelegations(s, delAddr, 0)
+
+	// Check that ccv unbonding op has been deleted
+	checkCCVUnbondingOp(s, s.providerCtx(), s.consumerChain.ChainID, valsetUpdateID, false)
+}

integration-tests/actions.go

@@ -616,3 +616,42 @@ func (tr TestRun) unbondTokens(
 		log.Fatal(err, "\n", string(bz))
 	}
 }
+
+type RedelegateTokensAction struct {
+	chain    chainID
+	src      validatorID
+	dst      validatorID
+	txSender validatorID
+	amount   uint
+}
+
+func (tr TestRun) redelegateTokens(action RedelegateTokensAction, verbose bool) {
+	//#nosec G204 -- Bypass linter warning for spawning subprocess with cmd arguments.
+	cmd := exec.Command("docker", "exec",
+		tr.containerConfig.instanceName,
+		tr.chainConfigs[action.chain].binaryName,
+
+		"tx", "staking", "redelegate",
+		tr.validatorConfigs[action.src].valoperAddress,
+		tr.validatorConfigs[action.dst].valoperAddress,
+		fmt.Sprint(action.amount)+`stake`,
+		`--from`, `validator`+fmt.Sprint(action.txSender),
+		`--chain-id`, string(tr.chainConfigs[action.chain].chainId),
+		`--home`, tr.getValidatorHome(action.chain, action.txSender),
+		`--node`, tr.getValidatorNode(action.chain, action.txSender),
+		// Need to manually set gas limit past default (200000), since redelegate has a lot of operations
+		`--gas`, "900000",
+		`--keyring-backend`, `test`,
+		`-b`, `block`,
+		`-y`,
+	)
+
+	if verbose {
+		fmt.Println("redelegate cmd:", cmd.String())
+	}
+
+	bz, err := cmd.CombinedOutput()
+	if err != nil {
+		log.Fatal(err, "\n", string(bz))
+	}
+}

integration-tests/main.go

@@ -54,6 +54,8 @@ func (tr TestRun) runStep(step Step, verbose bool) {
 		tr.delegateTokens(action, verbose)
 	case UnbondTokensAction:
 		tr.unbondTokens(action, verbose)
+	case RedelegateTokensAction:
+		tr.redelegateTokens(action, verbose)
 	default:
 		log.Fatalf(fmt.Sprintf(`unknown action: %#v`, action))
 	}

integration-tests/steps.go

@@ -276,5 +276,49 @@ var happyPathSteps = []Step{
 			},
 		},
 	},
-	// TODO: Test full unbonding functionality, considering liquidity after unbonding period, etc.
+	{
+		action: RedelegateTokensAction{
+			chain:    chainID("provi"),
+			src:      validatorID("bob"),
+			dst:      validatorID("carol"),
+			txSender: validatorID("bob"),
+			amount:   11000000,
+		},
+		state: State{
+			chainID("provi"): ChainState{
+				ValPowers: &map[validatorID]uint{
+					validatorID("alice"): 500,
+					validatorID("bob"):   489,
+					validatorID("carol"): 511,
+				},
+			},
+			chainID("consu"): ChainState{
+				ValPowers: &map[validatorID]uint{
+					// Voting power changes not seen by consumer yet
+					validatorID("alice"): 500,
+					validatorID("bob"):   500,
+					validatorID("carol"): 500,
+				},
+			},
+		},
+	},
+	{
+		action: RelayPacketsAction{
+			chain:   chainID("provi"),
+			port:    "provider",
+			channel: 0,
+		},
+		state: State{
+			chainID("consu"): ChainState{
+				ValPowers: &map[validatorID]uint{
+					// Now power changes are seen by consumer
+					validatorID("alice"): 500,
+					validatorID("bob"):   489,
+					validatorID("carol"): 511,
+				},
+			},
+		},
+	},
+
+	// TODO: Test full unbonding functionality, tracked as: https://github.com/cosmos/interchain-security/issues/311
 }

integration-tests/testnet-scripts/start-chain.sh

@@ -164,7 +164,7 @@ do
     GRPC_ADDRESS="--grpc.address $CHAIN_IP_PREFIX.$VAL_IP_SUFFIX:9091"
     LISTEN_ADDRESS="--address tcp://$CHAIN_IP_PREFIX.$VAL_IP_SUFFIX:26655"
     P2P_ADDRESS="--p2p.laddr tcp://$CHAIN_IP_PREFIX.$VAL_IP_SUFFIX:26656"
-    LOG_LEVEL="--log_level info"
+    LOG_LEVEL="--log_level trace" # Keep as "trace" to see panic messages
     ENABLE_WEBGRPC="--grpc-web.enable=false"
 
     PERSISTENT_PEERS=""