Fix unbonding hook and add unit test

module/app/app.go

@@ -363,12 +363,25 @@ func NewGravityApp(
 		authtypes.NewModuleAddress(govtypes.ModuleName).String(),
 	)
 
+	app.gravityKeeper = keeper.NewKeeper(
+		appCodec,
+		keys[gravitytypes.StoreKey],
+		app.GetSubspace(gravitytypes.ModuleName),
+		app.accountKeeper,
+		stakingKeeper,
+		app.bankKeeper,
+		app.slashingKeeper,
+		app.distrKeeper,
+		sdk.DefaultPowerReduction,
+		app.ModuleAccountAddressesToNames([]string{}),
+		app.ModuleAccountAddressesToNames([]string{distrtypes.ModuleName}),
+	)
+
 	app.stakingKeeper = *stakingKeeper.SetHooks(
 		stakingtypes.NewMultiStakingHooks(
 			app.distrKeeper.Hooks(),
 			app.slashingKeeper.Hooks(),
-			//app.gravityKeeper.Hooks(), TODO(bolten): this hook is broken, do not set it, to be fixed
-			// (Collin) is this still true?
+			app.gravityKeeper.Hooks(),
 		),
 	)
 
@@ -401,20 +414,6 @@ func NewGravityApp(
 	)
 	app.evidenceKeeper = *evidenceKeeper
 
-	app.gravityKeeper = keeper.NewKeeper(
-		appCodec,
-		keys[gravitytypes.StoreKey],
-		app.GetSubspace(gravitytypes.ModuleName),
-		app.accountKeeper,
-		stakingKeeper,
-		app.bankKeeper,
-		app.slashingKeeper,
-		app.distrKeeper,
-		sdk.DefaultPowerReduction,
-		app.ModuleAccountAddressesToNames([]string{}),
-		app.ModuleAccountAddressesToNames([]string{distrtypes.ModuleName}),
-	)
-
 	govRouter := govtypesv1beta1.NewRouter()
 	govRouter.AddRoute(govtypes.RouterKey, govtypesv1beta1.ProposalHandler).
 		AddRoute(paramsproposal.RouterKey, params.NewParamChangeProposalHandler(app.paramsKeeper)).

module/x/gravity/abci_test.go

@@ -35,15 +35,57 @@ func TestSignerSetTxCreationUponUnbonding(t *testing.T) {
 	gravityKeeper := input.GravityKeeper
 	gravityKeeper.CreateSignerSetTx(ctx)
 
-	input.Context = ctx.WithBlockHeight(ctx.BlockHeight() + 1)
-	// begin unbonding
-	input.StakingKeeper.Undelegate(input.Context, sdk.AccAddress(keeper.ValAddrs[0]), keeper.ValAddrs[0], sdk.NewDec(keeper.StakingAmount.Int64()))
+	input.Context = input.Context.WithBlockHeight(input.Context.BlockHeight() + 1)
+
+	smallValAddr := keeper.ValAddrs[4]
+	smallVal, _ := input.StakingKeeper.GetValidator(input.Context, smallValAddr)
+	unbondAmount := sdk.NewDec(smallVal.GetBondedTokens().Int64())
+	_, err := input.StakingKeeper.Undelegate(
+		input.Context,
+		sdk.AccAddress(smallValAddr),
+		smallValAddr,
+		unbondAmount,
+	)
+	require.NoError(t, err)
 
 	// Run the staking endblocker to ensure signer set tx is set in state
 	staking.EndBlocker(input.Context, input.StakingKeeper)
+
+	// power diff should be less than 5%
+	latestSignerSetTx := input.GravityKeeper.GetLatestSignerSetTx(input.Context)
+	powerDiff := types.EthereumSigners(input.GravityKeeper.CurrentSignerSet(input.Context)).PowerDiff(latestSignerSetTx.Signers)
+	require.Less(t, powerDiff, 0.05)
+
+	// last unbonding height should be the current block
+	lastUnbondingHeight := input.GravityKeeper.GetLastUnbondingBlockHeight(input.Context)
+	require.Equal(t, uint64(input.Context.BlockHeight()), lastUnbondingHeight)
+
+	// should create a new signer set
 	gravity.BeginBlocker(input.Context, gravityKeeper)
+	require.EqualValues(t, 2, gravityKeeper.GetLatestSignerSetTxNonce(input.Context))
 
-	require.EqualValues(t, 2, gravityKeeper.GetLatestSignerSetTxNonce(ctx))
+	// create signer set due to >5% power diff
+
+	input.Context = input.Context.WithBlockHeight(input.Context.BlockHeight() + 1)
+
+	undelegateAmount := sdk.NewDec(keeper.StakingAmount.Quo(sdk.NewInt(3)).Int64())
+	_, err = input.StakingKeeper.Undelegate(
+		input.Context,
+		sdk.AccAddress(keeper.ValAddrs[0]),
+		keeper.ValAddrs[0],
+		undelegateAmount,
+	)
+	require.NoError(t, err)
+
+	staking.EndBlocker(input.Context, input.StakingKeeper)
+
+	// last unbonding height should not be the current block
+	lastUnbondingHeight = input.GravityKeeper.GetLastUnbondingBlockHeight(input.Context)
+	require.NotEqual(t, uint64(input.Context.BlockHeight()), lastUnbondingHeight)
+
+	// signer set was created
+	gravity.BeginBlocker(input.Context, gravityKeeper)
+	require.EqualValues(t, 3, gravityKeeper.GetLatestSignerSetTxNonce(input.Context))
 }
 
 func TestSignerSetTxSlashing_SignerSetTxCreated_Before_ValidatorBonded(t *testing.T) {

module/x/gravity/keeper/hooks.go

@@ -15,14 +15,40 @@ var _ stakingtypes.StakingHooks = Hooks{}
 // Hooks Create new gravity hooks
 func (k Keeper) Hooks() Hooks { return Hooks{k} }
 
-func (h Hooks) AfterValidatorBeginUnbonding(ctx sdk.Context, _ sdk.ConsAddress, _ sdk.ValAddress) error {
+func (h Hooks) AfterValidatorBeginUnbonding(ctx sdk.Context, _ sdk.ConsAddress, valAddress sdk.ValAddress) error {
 
-	// When Validator starts Unbonding, Persist the block height in the store
-	// Later in endblocker, check if there is at least one validator who started unbonding and create a valset request.
-	// The reason for creating valset requests in endblock is to create only one valset request per block,
+	// When Validator starts Unbonding, Persist the block height in the store if their power is greater
+	// than 1% of the total power.
+	// Later in endblocker, check if this persisted block height is the current one and create a signer set tx if it is.
+	// The reason for creating signer set txs in endblock is to create only one valset request per block,
 	// if multiple validators starts unbonding at same block.
 
-	h.k.setLastUnbondingBlockHeight(ctx, uint64(ctx.BlockHeight()))
+	lastUnbondingBlockHeight := h.k.GetLastUnbondingBlockHeight(ctx)
+	if lastUnbondingBlockHeight == uint64(ctx.BlockHeight()) {
+		return nil
+	}
+
+	latestSignerSet := h.k.GetLatestSignerSetTx(ctx)
+	ethAddress := h.k.GetValidatorEthereumAddress(ctx, valAddress).Hex()
+	power := uint64(0)
+	totalPower := uint64(0)
+	for _, s := range latestSignerSet.Signers {
+		if s.EthereumAddress == ethAddress {
+			power = s.Power
+			break
+		}
+
+		totalPower += s.Power
+	}
+
+	if totalPower == 0 {
+		return nil
+	}
+
+	proportion := float64(power) / float64(totalPower)
+	if proportion > 0.01 {
+		h.k.setLastUnbondingBlockHeight(ctx, uint64(ctx.BlockHeight()))
+	}
 
 	return nil
 }

module/x/gravity/keeper/test_common.go

@@ -261,8 +261,14 @@ func SetupFiveValChain(t *testing.T) (TestInput, sdk.Context) {
 		input.AccountKeeper.SetAccount(input.Context, acc)
 
 		// Create a validator for that account using some of the tokens in the account
-		// and the staking handler
-		sh.CreateValidator(ValAddrs[i], AccPubKeys[i], StakingAmount, true)
+		// and the staking handler. Give the 5th validator a smaller stake so we can
+		// test unbonding hooks.
+		if i == 4 {
+			amt := sdk.TokensFromConsensusPower(1, sdk.DefaultPowerReduction)
+			sh.CreateValidator(ValAddrs[i], AccPubKeys[i], amt, true)
+		} else {
+			sh.CreateValidator(ValAddrs[i], AccPubKeys[i], StakingAmount, true)
+		}
 	}
 
 	// Run the staking endblocker to ensure valset is correct in state