Fix removing voting power during expiry

CHANGELOG.md

@@ -41,6 +41,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 
 * [#228](https://github.com/babylonlabs-io/babylon/pull/228) Add inclusion height to early unbonding event
 
+### State Machine Breaking
+
+* [#218](https://github.com/babylonlabs-io/babylon/pull/218) Fix removing voting
+power during expiry
+
 ## v0.14.0
 
 ### State Machine Breaking

testutil/helper/helper.go

@@ -307,8 +307,8 @@ func (h *Helper) CheckDelegator(delegator sdk.AccAddress, val sdk.ValAddress, fo
 	require.Equal(h.t, ok, found)
 }
 
-func (h *Helper) AddDelegation(del *btcstakingtypes.BTCDelegation) {
-	err := h.App.BTCStakingKeeper.AddBTCDelegation(h.Ctx, del)
+func (h *Helper) AddDelegation(del *btcstakingtypes.BTCDelegation, minUnbondingTime uint32) {
+	err := h.App.BTCStakingKeeper.AddBTCDelegation(h.Ctx, del, minUnbondingTime)
 	h.NoError(err)
 }
 

x/btcstaking/keeper/btc_delegations.go

@@ -18,7 +18,11 @@ import (
 // - indexing the given BTC delegation in the BTC delegator store,
 // - saving it under BTC delegation store, and
 // - emit events about this BTC delegation.
-func (k Keeper) AddBTCDelegation(ctx sdk.Context, btcDel *types.BTCDelegation) error {
+func (k Keeper) AddBTCDelegation(
+	ctx sdk.Context,
+	btcDel *types.BTCDelegation,
+	minUnbondingTime uint32,
+) error {
 	if err := btcDel.ValidateBasic(); err != nil {
 		return err
 	}
@@ -75,9 +79,9 @@ func (k Keeper) AddBTCDelegation(ctx sdk.Context, btcDel *types.BTCDelegation) e
 			StakingTxHash: stakingTxHash.String(),
 			NewState:      types.BTCDelegationStatus_UNBONDED,
 		})
-		// NOTE: we should have verified that EndHeight > btcTip.Height + CheckpointFinalizationTimeout
-		wValue := k.btccKeeper.GetParams(ctx).CheckpointFinalizationTimeout
-		k.addPowerDistUpdateEvent(ctx, btcDel.EndHeight-wValue, unbondedEvent)
+
+		// NOTE: we should have verified that EndHeight > btcTip.Height + max(w, min_unbonding_time)
+		k.addPowerDistUpdateEvent(ctx, btcDel.EndHeight-minUnbondingTime, unbondedEvent)
 	}
 
 	return nil

x/btcstaking/keeper/genesis_test.go

@@ -20,7 +20,9 @@ func TestExportGenesis(t *testing.T) {
 
 	fps := datagen.CreateNFinalityProviders(r, t, numFps)
 	params := k.GetParams(ctx)
-	wValue := btcCheckK.GetParams(ctx).CheckpointFinalizationTimeout
+	btcckptParams := btcCheckK.GetParams(ctx)
+
+	minUnbondingTime := types.MinimumUnbondingTime(&params, &btcckptParams)
 
 	chainsHeight := make([]*types.BlockHeightBbnToBtc, 0)
 	// creates the first as it starts already with an chain height from the helper.
@@ -70,7 +72,7 @@ func TestExportGenesis(t *testing.T) {
 			totalDelegations++
 
 			// sets delegations
-			h.AddDelegation(del)
+			h.AddDelegation(del, minUnbondingTime)
 			btcDelegations = append(btcDelegations, del)
 
 			// BTC delegators idx
@@ -99,7 +101,7 @@ func TestExportGenesis(t *testing.T) {
 			idxEvent := uint64(totalDelegations - 1)
 			eventsIdx[idxEvent] = &types.EventIndex{
 				Idx:            idxEvent,
-				BlockHeightBtc: del.EndHeight - wValue,
+				BlockHeightBtc: del.EndHeight - minUnbondingTime,
 				Event:          unbondedEvent,
 			}
 		}

x/btcstaking/keeper/grpc_query_test.go

@@ -238,7 +238,7 @@ func FuzzPendingBTCDelegations(f *testing.F) {
 					btcDel.CovenantSigs = nil
 					pendingBtcDelsMap[btcDel.BtcPk.MarshalHex()] = btcDel
 				}
-				err = keeper.AddBTCDelegation(ctx, btcDel)
+				err = keeper.AddBTCDelegation(ctx, btcDel, btcDel.UnbondingTime-1)
 				require.NoError(t, err)
 
 				txHash := btcDel.MustGetStakingTxHash().String()
@@ -443,7 +443,7 @@ func FuzzActiveFinalityProvidersAtHeight(f *testing.F) {
 					slashingChangeLockTime,
 				)
 				require.NoError(t, err)
-				err = keeper.AddBTCDelegation(ctx, btcDel)
+				err = keeper.AddBTCDelegation(ctx, btcDel, btcDel.UnbondingTime-1)
 				require.NoError(t, err)
 				totalVotingPower += btcDel.TotalSat
 			}
@@ -556,7 +556,7 @@ func FuzzFinalityProviderDelegations(f *testing.F) {
 			)
 			require.NoError(t, err)
 			expectedBtcDelsMap[btcDel.BtcPk.MarshalHex()] = btcDel
-			err = keeper.AddBTCDelegation(ctx, btcDel)
+			err = keeper.AddBTCDelegation(ctx, btcDel, btcDel.UnbondingTime-1)
 			require.NoError(t, err)
 		}
 

x/btcstaking/keeper/inclusion_proof.go

@@ -10,23 +10,29 @@ import (
 	"github.com/babylonlabs-io/babylon/x/btcstaking/types"
 )
 
+type delegationTimeRangeInfo struct {
+	startHeight uint32
+	endHeight   uint32
+}
+
 // VerifyInclusionProofAndGetHeight verifies the inclusion proof of the given staking tx
-// and returns the inclusion height
+// and returns the start height and end height
 func (k Keeper) VerifyInclusionProofAndGetHeight(
 	ctx sdk.Context,
 	stakingTx *btcutil.Tx,
+	confirmationDepth uint32,
 	stakingTime uint32,
+	minUnbondingTime uint32,
 	inclusionProof *types.ParsedProofOfInclusion,
-) (uint32, error) {
-	btccParams := k.btccKeeper.GetParams(ctx)
+) (*delegationTimeRangeInfo, error) {
 	// Check:
 	// - timelock of staking tx
 	// - staking tx is k-deep
 	// - staking tx inclusion proof
 	stakingTxHeader := k.btclcKeeper.GetHeaderByHash(ctx, inclusionProof.HeaderHash)
 
 	if stakingTxHeader == nil {
-		return 0, fmt.Errorf("header that includes the staking tx is not found")
+		return nil, fmt.Errorf("header that includes the staking tx is not found")
 	}
 
 	// no need to do more validations to the btc header as it was already
@@ -41,21 +47,24 @@ func (k Keeper) VerifyInclusionProofAndGetHeight(
 	)
 
 	if !proofValid {
-		return 0, types.ErrInvalidStakingTx.Wrapf("not included in the Bitcoin chain")
+		return nil, types.ErrInvalidStakingTx.Wrapf("not included in the Bitcoin chain")
 	}
 
 	startHeight := stakingTxHeader.Height
 	endHeight := stakingTxHeader.Height + stakingTime
 
 	btcTip := k.btclcKeeper.GetTipInfo(ctx)
 	stakingTxDepth := btcTip.Height - stakingTxHeader.Height
-	if stakingTxDepth < btccParams.BtcConfirmationDepth {
-		return 0, types.ErrInvalidStakingTx.Wrapf("not k-deep: k=%d; depth=%d", btccParams.BtcConfirmationDepth, stakingTxDepth)
+	if stakingTxDepth < confirmationDepth {
+		return nil, types.ErrInvalidStakingTx.Wrapf("not k-deep: k=%d; depth=%d", confirmationDepth, stakingTxDepth)
 	}
-	// ensure staking tx's timelock has more than w BTC blocks left
-	if btcTip.Height+btccParams.CheckpointFinalizationTimeout >= endHeight {
-		return 0, types.ErrInvalidStakingTx.Wrapf("staking tx's timelock has no more than w(=%d) blocks left", btccParams.CheckpointFinalizationTimeout)
+	// ensure staking tx's timelock has more than unbonding BTC blocks left
+	if btcTip.Height+minUnbondingTime >= endHeight {
+		return nil, types.ErrInvalidStakingTx.Wrapf("staking tx's timelock has no more than unbonding(=%d) blocks left", minUnbondingTime)
 	}
 
-	return startHeight, nil
+	return &delegationTimeRangeInfo{
+		startHeight: startHeight,
+		endHeight:   endHeight,
+	}, nil
 }

x/btcstaking/keeper/msg_server.go

@@ -187,17 +187,19 @@ func (ms msgServer) CreateBTCDelegation(goCtx context.Context, req *types.MsgCre
 	// and set start height and end height
 	var startHeight, endHeight uint32
 	if parsedMsg.StakingTxProofOfInclusion != nil {
-		inclusionHeight, err := ms.VerifyInclusionProofAndGetHeight(
+		timeInfo, err := ms.VerifyInclusionProofAndGetHeight(
 			ctx,
 			btcutil.NewTx(parsedMsg.StakingTx.Transaction),
+			btccParams.BtcConfirmationDepth,
 			uint32(parsedMsg.StakingTime),
+			paramsValidationResult.MinUnbondingTime,
 			parsedMsg.StakingTxProofOfInclusion)
 		if err != nil {
 			return nil, fmt.Errorf("invalid inclusion proof: %w", err)
 		}
 
-		startHeight = inclusionHeight
-		endHeight = startHeight + uint32(parsedMsg.StakingTime)
+		startHeight = timeInfo.startHeight
+		endHeight = timeInfo.endHeight
 	} else {
 		// NOTE: here we consume more gas to protect Babylon chain and covenant members against spamming
 		// i.e creating delegation that will never reach BTC
@@ -234,7 +236,7 @@ func (ms msgServer) CreateBTCDelegation(goCtx context.Context, req *types.MsgCre
 	}
 
 	// add this BTC delegation, and emit corresponding events
-	if err := ms.AddBTCDelegation(ctx, newBTCDel); err != nil {
+	if err := ms.AddBTCDelegation(ctx, newBTCDel, paramsValidationResult.MinUnbondingTime); err != nil {
 		panic(fmt.Errorf("failed to add BTC delegation that has passed verification: %w", err))
 	}
 
@@ -280,10 +282,17 @@ func (ms msgServer) AddBTCDelegationInclusionProof(
 	if err != nil {
 		return nil, err
 	}
-	inclusionHeight, err := ms.VerifyInclusionProofAndGetHeight(
+
+	btccParams := ms.btccKeeper.GetParams(ctx)
+
+	minUnbondingTime := types.MinimumUnbondingTime(params, &btccParams)
+
+	timeInfo, err := ms.VerifyInclusionProofAndGetHeight(
 		ctx,
 		btcutil.NewTx(stakingTx),
+		btccParams.BtcConfirmationDepth,
 		btcDel.StakingTime,
+		minUnbondingTime,
 		parsedInclusionProof,
 	)
 
@@ -292,8 +301,8 @@ func (ms msgServer) AddBTCDelegationInclusionProof(
 	}
 
 	// 6. set start height and end height and save it to db
-	btcDel.StartHeight = inclusionHeight
-	btcDel.EndHeight = btcDel.StartHeight + btcDel.StakingTime
+	btcDel.StartHeight = timeInfo.startHeight
+	btcDel.EndHeight = timeInfo.endHeight
 	ms.setBTCDelegation(ctx, btcDel)
 
 	// 7. emit events
@@ -324,8 +333,9 @@ func (ms msgServer) AddBTCDelegationInclusionProof(
 		StakingTxHash: req.StakingTxHash,
 		NewState:      types.BTCDelegationStatus_UNBONDED,
 	})
-	wValue := ms.btccKeeper.GetParams(ctx).CheckpointFinalizationTimeout
-	ms.addPowerDistUpdateEvent(ctx, btcDel.EndHeight-wValue, unbondedEvent)
+
+	// NOTE: we should have verified that EndHeight > btcTip.Height + max(w, min_unbonding_time)
+	ms.addPowerDistUpdateEvent(ctx, btcDel.EndHeight-minUnbondingTime, unbondedEvent)
 
 	// at this point, the BTC delegation inclusion proof is verified and is not duplicated
 	// thus, we can safely consider this message as refundable

x/btcstaking/keeper/msg_server_test.go

@@ -259,7 +259,7 @@ func TestProperVersionInDelegation(t *testing.T) {
 		fpPK,
 		changeAddress.EncodeAddress(),
 		stakingValue,
-		1000,
+		10000,
 		stakingValue-1000,
 		uint16(customMinUnbondingTime)+1,
 		false,
@@ -976,9 +976,12 @@ func FuzzDeterminismBtcstakingBeginBlocker(f *testing.F) {
 		require.Equal(t, appHash1, appHash2)
 
 		// Default params are the same in both apps
-		covQuorum := h.App.BTCStakingKeeper.GetParams(h.Ctx).CovenantQuorum
-		maxFinalityProviders := int32(h.App.BTCStakingKeeper.GetParams(h.Ctx).MaxActiveFinalityProviders)
+		stakingParams := h.App.BTCStakingKeeper.GetParams(h.Ctx)
+		covQuorum := stakingParams.CovenantQuorum
+		maxFinalityProviders := int32(stakingParams.MaxActiveFinalityProviders)
+		btcckptParams := h.App.BtcCheckpointKeeper.GetParams(h.Ctx)
 
+		minUnbondingTime := types.MinimumUnbondingTime(&stakingParams, &btcckptParams)
 		// Number of finality providers from 10 to maxFinalityProviders + 10
 		numFinalityProviders := int(r.Int31n(maxFinalityProviders) + 10)
 
@@ -1012,8 +1015,8 @@ func FuzzDeterminismBtcstakingBeginBlocker(f *testing.F) {
 			)
 
 			for _, del := range delegations {
-				h.AddDelegation(del)
-				h1.AddDelegation(del)
+				h.AddDelegation(del, minUnbondingTime)
+				h1.AddDelegation(del, minUnbondingTime)
 			}
 		}
 

x/btcstaking/keeper/power_dist_change_test.go

@@ -459,8 +459,14 @@ func FuzzBTCDelegationEvents_NoPreApproval(f *testing.F) {
 		btcTip := btclcKeeper.GetTipInfo(h.Ctx)
 		events := h.BTCStakingKeeper.GetAllPowerDistUpdateEvents(h.Ctx, btcTip.Height, btcTip.Height)
 		require.Len(t, events, 0)
-		// the BTC delegation will be unbonded at end height - w
-		unbondedHeight := actualDel.EndHeight - btccKeeper.GetParams(h.Ctx).CheckpointFinalizationTimeout
+
+		btckptParams := btccKeeper.GetParams(h.Ctx)
+		stakingParams := h.BTCStakingKeeper.GetParamsWithVersion(h.Ctx).Params
+
+		minUnbondingTime := types.MinimumUnbondingTime(&stakingParams, &btckptParams)
+
+		// the BTC delegation will be unbonded at end height - max(w, min_unbonding_time)
+		unbondedHeight := actualDel.EndHeight - minUnbondingTime
 		events = h.BTCStakingKeeper.GetAllPowerDistUpdateEvents(h.Ctx, unbondedHeight, unbondedHeight)
 		require.Len(t, events, 1)
 		btcDelStateUpdate := events[0].GetBtcDelStateUpdate()
@@ -612,8 +618,14 @@ func FuzzBTCDelegationEvents_WithPreApproval(f *testing.F) {
 		require.NotNil(t, btcDelStateUpdate)
 		require.Equal(t, stakingTxHash, btcDelStateUpdate.StakingTxHash)
 		require.Equal(t, types.BTCDelegationStatus_ACTIVE, btcDelStateUpdate.NewState)
-		// there exists 1 event that the BTC delegation becomes unbonded at end height - w
-		unbondedHeight := activatedDel.EndHeight - btccKeeper.GetParams(h.Ctx).CheckpointFinalizationTimeout
+
+		btckptParams := btccKeeper.GetParams(h.Ctx)
+		stakingParams := h.BTCStakingKeeper.GetParamsWithVersion(h.Ctx).Params
+
+		minUnbondingTime := types.MinimumUnbondingTime(&stakingParams, &btckptParams)
+
+		// the BTC delegation will be unbonded at end height - max(w, min_unbonding_time)
+		unbondedHeight := activatedDel.EndHeight - minUnbondingTime
 		events = h.BTCStakingKeeper.GetAllPowerDistUpdateEvents(h.Ctx, unbondedHeight, unbondedHeight)
 		require.Len(t, events, 1)
 		btcDelStateUpdate = events[0].GetBtcDelStateUpdate()
@@ -704,8 +716,14 @@ func TestDoNotGenerateDuplicateEventsAfterHavingCovenantQuorum(t *testing.T) {
 	btcTip := btclcKeeper.GetTipInfo(h.Ctx)
 	events := h.BTCStakingKeeper.GetAllPowerDistUpdateEvents(h.Ctx, btcTip.Height, btcTip.Height)
 	require.Len(t, events, 0)
-	// the BTC delegation will be unbonded at end height - w
-	unbondedHeight := actualDel.EndHeight - btccKeeper.GetParams(h.Ctx).CheckpointFinalizationTimeout
+
+	btckptParams := btccKeeper.GetParams(h.Ctx)
+	stakingParams := h.BTCStakingKeeper.GetParamsWithVersion(h.Ctx).Params
+
+	minUnbondingTime := types.MinimumUnbondingTime(&stakingParams, &btckptParams)
+
+	// the BTC delegation will be unbonded at end height - max(w, min_unbonding_time)
+	unbondedHeight := actualDel.EndHeight - minUnbondingTime
 	events = h.BTCStakingKeeper.GetAllPowerDistUpdateEvents(h.Ctx, unbondedHeight, unbondedHeight)
 	require.Len(t, events, 1)
 	btcDelStateUpdate := events[0].GetBtcDelStateUpdate()

x/btcstaking/types/validate_parsed_message.go

@@ -12,6 +12,7 @@ import (
 type ParamsValidationResult struct {
 	StakingOutputIdx   uint32
 	UnbondingOutputIdx uint32
+	MinUnbondingTime   uint32
 }
 
 // ValidateParsedMessageAgainstTheParams validates parsed message against parameters
@@ -196,5 +197,6 @@ func ValidateParsedMessageAgainstTheParams(
 	return &ParamsValidationResult{
 		StakingOutputIdx:   stakingOutputIdx,
 		UnbondingOutputIdx: unbondingOutputIdx,
+		MinUnbondingTime:   minUnbondingTime,
 	}, nil
 }

x/btcstaking/types/validate_parsed_message_test.go

@@ -42,8 +42,8 @@ func testStakingParams(
 		CovenantQuorum:             3,
 		MinStakingValueSat:         100000,
 		MaxStakingValueSat:         int64(4 * 10e8),
-		MinStakingTimeBlocks:       10,
-		MaxStakingTimeBlocks:       10000,
+		MinStakingTimeBlocks:       1000,
+		MaxStakingTimeBlocks:       100000,
 		SlashingPkScript:           slashingPkScript,
 		MinSlashingTxFeeSat:        1000,
 		MinCommissionRate:          sdkmath.LegacyMustNewDecFromStr("0.01"),
@@ -782,6 +782,9 @@ func TestValidateParsedMessageAgainstTheParams(t *testing.T) {
 			} else {
 				require.NoError(t, err)
 				require.NotNil(t, got)
+
+				minUnbondingTime := types.MinimumUnbondingTime(params, checkpointParams)
+				require.Equal(t, minUnbondingTime, got.MinUnbondingTime)
 			}
 
 		})