fix!: avoid panicking on CancelUnbondingDelegation (#977)

* handle CancelUnbondingDelegation message

* tests: add cancel-unbond e2e test

* fix: appease linter

* chore: Hardcode golangci-lint version (#990)

* Hardcode golangci-lint version

* Hardcode version in CI config

---------

Co-authored-by: MSalopek <[email protected]>
Co-authored-by: Philip Offtermatt <[email protected]>

tests/e2e/actions.go

@@ -1417,6 +1417,74 @@ func (tr TestRun) unbondTokens(
 	tr.waitBlocks(action.chain, 1, 10*time.Second)
 }
 
+type cancelUnbondTokensAction struct {
+	chain     chainID
+	delegator validatorID
+	validator validatorID
+	amount    uint
+}
+
+func (tr TestRun) cancelUnbondTokens(
+	action cancelUnbondTokensAction,
+	verbose bool,
+) {
+	validator := tr.validatorConfigs[action.validator].valoperAddress
+	if tr.validatorConfigs[action.validator].useConsumerKey {
+		validator = tr.validatorConfigs[action.validator].consumerValoperAddress
+	}
+
+	// get creation-height from state
+	//#nosec G204 -- Bypass linter warning for spawning subprocess with cmd arguments.
+	cmd := exec.Command("docker", "exec", tr.containerConfig.instanceName, tr.chainConfigs[action.chain].binaryName,
+		"q", "staking", "unbonding-delegation",
+		tr.validatorConfigs[action.delegator].delAddress,
+		validator,
+		`--home`, tr.getValidatorHome(action.chain, action.delegator),
+		`--node`, tr.getValidatorNode(action.chain, action.delegator),
+		`-o`, `json`,
+	)
+	if verbose {
+		fmt.Println("get unbonding delegations cmd:", cmd.String())
+	}
+
+	bz, err := cmd.CombinedOutput()
+	if err != nil {
+		log.Fatal(err, "\n", string(bz))
+	}
+	creationHeight := gjson.Get(string(bz), "entries.0.creation_height").Int()
+	if creationHeight == 0 {
+		log.Fatal("invalid creation height")
+	}
+
+	//#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", "cancel-unbond",
+		validator,
+		fmt.Sprint(action.amount)+`stake`,
+		fmt.Sprint(creationHeight),
+		`--from`, `validator`+fmt.Sprint(action.delegator),
+		`--chain-id`, string(tr.chainConfigs[action.chain].chainId),
+		`--home`, tr.getValidatorHome(action.chain, action.delegator),
+		`--node`, tr.getValidatorNode(action.chain, action.delegator),
+		`--gas`, "900000",
+		`--keyring-backend`, `test`,
+		`-o`, `json`,
+		`-y`,
+	)
+
+	if verbose {
+		fmt.Println("unbond cmd:", cmd.String())
+	}
+
+	bz, err = cmd.CombinedOutput()
+	if err != nil {
+		log.Fatal(err, "\n", string(bz))
+	}
+
+	// wait for inclusion in a block -> '--broadcast-mode block' is deprecated
+	tr.waitBlocks(action.chain, 2, 20*time.Second)
+}
+
 type redelegateTokensAction struct {
 	chain    chainID
 	src      validatorID

tests/e2e/main.go

@@ -155,6 +155,8 @@ func (tr *TestRun) runStep(step Step, verbose bool) {
 		tr.delegateTokens(action, verbose)
 	case unbondTokensAction:
 		tr.unbondTokens(action, verbose)
+	case cancelUnbondTokensAction:
+		tr.cancelUnbondTokens(action, verbose)
 	case redelegateTokensAction:
 		tr.redelegateTokens(action, verbose)
 	case downtimeSlashAction:

tests/e2e/step_delegation.go

@@ -129,6 +129,95 @@ func stepsUnbond(consumerName string) []Step {
 	}
 }
 
+// stepsCancelUnbond canceling unbonding operation for delegator and validator combination
+// the steps perform a full unbonding where the unbonding delegation is removed from the unbonding queue
+func stepsCancelUnbond(consumerName string) []Step {
+	return []Step{
+		{
+			action: unbondTokensAction{
+				chain:      chainID("provi"),
+				unbondFrom: validatorID("alice"),
+				sender:     validatorID("alice"),
+				amount:     1000000,
+			},
+			state: State{
+				chainID("provi"): ChainState{
+					ValPowers: &map[validatorID]uint{
+						validatorID("alice"): 509,
+						validatorID("bob"):   500,
+						validatorID("carol"): 500,
+					},
+				},
+				chainID("consu"): ChainState{
+					ValPowers: &map[validatorID]uint{
+						// Voting power on consumer should not be affected yet
+						validatorID("alice"): 510,
+						validatorID("bob"):   500,
+						validatorID("carol"): 500,
+					},
+				},
+			},
+		},
+		{
+			action: relayPacketsAction{
+				chain:   chainID("provi"),
+				port:    "provider",
+				channel: 0,
+			},
+			state: State{
+				chainID("consu"): ChainState{
+					ValPowers: &map[validatorID]uint{
+						validatorID("alice"): 509,
+						validatorID("bob"):   500,
+						validatorID("carol"): 500,
+					},
+				},
+			},
+		},
+		{
+			action: cancelUnbondTokensAction{
+				chain:     chainID("provi"),
+				delegator: validatorID("alice"),
+				validator: validatorID("alice"),
+				amount:    1000000, // cancel unbonding the full amount
+			},
+			state: State{
+				chainID("provi"): ChainState{
+					ValPowers: &map[validatorID]uint{
+						validatorID("alice"): 510, // power restored
+						validatorID("bob"):   500,
+						validatorID("carol"): 500,
+					},
+				},
+				chainID("consu"): ChainState{
+					ValPowers: &map[validatorID]uint{
+						// Voting power on consumer should not be affected yet
+						validatorID("alice"): 509,
+						validatorID("bob"):   500,
+						validatorID("carol"): 500,
+					},
+				},
+			},
+		},
+		{
+			action: relayPacketsAction{
+				chain:   chainID("provi"),
+				port:    "provider",
+				channel: 0,
+			},
+			state: State{
+				chainID("consu"): ChainState{
+					ValPowers: &map[validatorID]uint{
+						validatorID("alice"): 510, // power restored on consumer
+						validatorID("bob"):   500,
+						validatorID("carol"): 500,
+					},
+				},
+			},
+		},
+	}
+}
+
 // stepsRedelegateForOptOut tests redelegation, and sets up voting powers s.t
 // alice will have less than 5% of the total voting power. This is needed to
 // test opt-out functionality.

tests/e2e/steps.go

@@ -18,6 +18,7 @@ var happyPathSteps = concatSteps(
 	stepsDelegate("consu"),
 	stepsAssignConsumerKeyOnStartedChain("consu", "bob"),
 	stepsUnbond("consu"),
+	stepsCancelUnbond("consu"),
 	stepsRedelegateForOptOut("consu"),
 	stepsDowntimeWithOptOut("consu"),
 	stepsRedelegate("consu"),

x/ccv/provider/keeper/relay.go

@@ -114,8 +114,18 @@ func (k Keeper) completeMaturedUnbondingOps(ctx sdk.Context) {
 		// Attempt to complete unbonding in staking module
 		err := k.stakingKeeper.UnbondingCanComplete(ctx, id)
 		if err != nil {
-			// UnbondingCanComplete fails if the unbonding operation was not found,
-			// which means that the state of the x/staking module of cosmos-sdk is invalid.
+			if stakingtypes.ErrUnbondingNotFound.Is(err) {
+				// The unbonding was not found.
+				unbondingType, found := k.stakingKeeper.GetUnbondingType(ctx, id)
+				if found && unbondingType == stakingtypes.UnbondingType_UnbondingDelegation {
+					// If this is an unbonding delegation, it may have been removed
+					// after through a CancelUnbondingDelegation message
+					k.Logger(ctx).Debug("unbonding delegation was already removed:", "unbondingID", id)
+					continue
+				}
+			}
+			// UnbondingCanComplete failing means that the state of the x/staking module
+			// of cosmos-sdk is invalid. An exception is the case handled above
 			panic(fmt.Sprintf("could not complete unbonding op: %s", err.Error()))
 		}
 		k.Logger(ctx).Debug("unbonding operation matured on all consumers", "opID", id)

x/ccv/types/expected_keepers.go

@@ -44,6 +44,7 @@ type StakingKeeper interface {
 	GetLastTotalPower(ctx sdk.Context) sdk.Int
 	GetLastValidators(ctx sdk.Context) (validators []stakingtypes.Validator)
 	BondDenom(ctx sdk.Context) (res string)
+	GetUnbondingType(ctx sdk.Context, id uint64) (unbondingType stakingtypes.UnbondingType, found bool)
 }
 
 type EvidenceKeeper interface {