backport: Fix invalid minUnbondingTime for verifying inclusion proof (#289)

CHANGELOG.md

@@ -37,6 +37,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 
 ## Unreleased
 
+### Bug fixes
+
+- [#289](https://github.com/babylonlabs-io/babylon/pull/289) hotfix: Invalid minUnbondingTime for verifying inclusion proof
+
 ## v0.17.0
 
 ### State Breaking

testutil/btcstaking-helper/keeper.go

@@ -159,7 +159,7 @@ func (h *Helper) GenAndApplyCustomParams(
 		CovenantQuorum:            3,
 		MinStakingValueSat:        1000,
 		MaxStakingValueSat:        int64(4 * 10e8),
-		MinStakingTimeBlocks:      10,
+		MinStakingTimeBlocks:      400,
 		MaxStakingTimeBlocks:      10000,
 		SlashingPkScript:          slashingPkScript,
 		MinSlashingTxFeeSat:       10,

testutil/datagen/btc_transaction.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"encoding/binary"
 	"encoding/hex"
+	"fmt"
 	"math"
 	"math/rand"
 	"runtime"
@@ -331,7 +332,7 @@ func CreateBlockWithTransaction(
 	proof, err := btcctypes.SpvProofFromHeaderAndTransactions(&headerBytes, txBytes, 1)
 
 	if err != nil {
-		panic("could not calculate proof")
+		panic(fmt.Errorf("could not calculate proof: %w", err))
 	}
 
 	return &BtcHeaderWithProof{
@@ -340,6 +341,32 @@ func CreateBlockWithTransaction(
 	}
 }
 
+func CreateDummyTx() *wire.MsgTx {
+	// Create a witness transaction instead of empty transaction
+	msgTx := wire.NewMsgTx(wire.TxVersion)
+
+	// Add a dummy input
+	prevOut := wire.NewOutPoint(&chainhash.Hash{}, 0)
+	txIn := wire.NewTxIn(prevOut, nil, [][]byte{
+		{0x01}, // Simple witness script
+	})
+	msgTx.AddTxIn(txIn)
+
+	// Add a dummy output
+	pkScript := []byte{
+		0x00,                         // Version 0 witness program
+		0x14,                         // Push 20 bytes
+		0x01, 0x02, 0x03, 0x04, 0x05, // Dummy public key hash (20 bytes)
+		0x06, 0x07, 0x08, 0x09, 0x0a,
+		0x0b, 0x0c, 0x0d, 0x0e, 0x0f,
+		0x10, 0x11, 0x12, 0x13, 0x14,
+	}
+	txOut := wire.NewTxOut(100000, pkScript)
+	msgTx.AddTxOut(txOut)
+
+	return msgTx
+}
+
 func GenRandomTx(r *rand.Rand) *wire.MsgTx {
 	// structure of the below tx is from https://github.com/btcsuite/btcd/blob/master/wire/msgtx_test.go
 	tx := &wire.MsgTx{

x/btcstaking/keeper/btc_delegations.go

@@ -71,7 +71,7 @@ func (k Keeper) AddBTCDelegation(
 			panic(fmt.Errorf("failed to emit EventBTCDelegationInclusionProofReceived for the new pending BTC delegation: %w", err))
 		}
 
-		// record event that the BTC delegation will become unbonded at endHeight-w
+		// record event that the BTC delegation will become unbonded at EndHeight-w
 		// This event will be generated to subscribers as block event, when the
 		// btc light client block height will reach btcDel.EndHeight-wValue
 		unbondedEvent := types.NewEventPowerDistUpdateWithBTCDel(&types.EventBTCDelegationStateUpdate{

x/btcstaking/keeper/genesis_test.go

@@ -81,7 +81,7 @@ func TestExportGenesis(t *testing.T) {
 				DelBtcPk: del.BtcPk,
 			}
 
-			// record event that the BTC delegation will become unbonded at endHeight-w
+			// record event that the BTC delegation will become unbonded at EndHeight-w
 			unbondedEvent := types.NewEventPowerDistUpdateWithBTCDel(&types.EventBTCDelegationStateUpdate{
 				StakingTxHash: stakingTxHash.String(),
 				NewState:      types.BTCDelegationStatus_UNBONDED,

x/btcstaking/keeper/inclusion_proof.go

@@ -10,21 +10,23 @@ import (
 	"github.com/babylonlabs-io/babylon/x/btcstaking/types"
 )
 
-type delegationTimeRangeInfo struct {
-	startHeight uint32
-	endHeight   uint32
+type DelegationTimeRangeInfo struct {
+	StartHeight uint32
+	EndHeight   uint32
 }
 
 // VerifyInclusionProofAndGetHeight verifies the inclusion proof of the given staking tx
 // and returns the start height and end height
+// Note: the `minUnbondingTime` passed here should be from the corresponding params
+// of the staking tx
 func (k Keeper) VerifyInclusionProofAndGetHeight(
 	ctx sdk.Context,
 	stakingTx *btcutil.Tx,
 	confirmationDepth uint32,
 	stakingTime uint32,
 	minUnbondingTime uint32,
 	inclusionProof *types.ParsedProofOfInclusion,
-) (*delegationTimeRangeInfo, error) {
+) (*DelegationTimeRangeInfo, error) {
 	// Check:
 	// - timelock of staking tx
 	// - staking tx is k-deep
@@ -58,13 +60,15 @@ func (k Keeper) VerifyInclusionProofAndGetHeight(
 	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 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 nil, types.ErrInvalidStakingTx.
+			Wrapf("staking tx's timelock has no more than unbonding(=%d) blocks left", minUnbondingTime)
 	}
 
-	return &delegationTimeRangeInfo{
-		startHeight: startHeight,
-		endHeight:   endHeight,
+	return &DelegationTimeRangeInfo{
+		StartHeight: startHeight,
+		EndHeight:   endHeight,
 	}, nil
 }

x/btcstaking/keeper/inclusion_proof_test.go

@@ -0,0 +1,187 @@
+package keeper_test
+
+import (
+	"math/rand"
+	"testing"
+
+	"github.com/btcsuite/btcd/btcutil"
+	"github.com/golang/mock/gomock"
+	"github.com/stretchr/testify/require"
+
+	testutil "github.com/babylonlabs-io/babylon/testutil/btcstaking-helper"
+	"github.com/babylonlabs-io/babylon/testutil/datagen"
+	bbntypes "github.com/babylonlabs-io/babylon/types"
+	btclctypes "github.com/babylonlabs-io/babylon/x/btclightclient/types"
+	"github.com/babylonlabs-io/babylon/x/btcstaking/types"
+)
+
+func FuzzVerifyInclusionProofAndGetHeight(f *testing.F) {
+	datagen.AddRandomSeedsToFuzzer(f, 100)
+
+	f.Fuzz(func(t *testing.T, seed int64) {
+		r := rand.New(rand.NewSource(seed))
+		ctrl := gomock.NewController(t)
+		defer ctrl.Finish()
+
+		// mock BTC light client and BTC checkpoint modules
+		btclcKeeper := types.NewMockBTCLightClientKeeper(ctrl)
+		btccKeeper := types.NewMockBtcCheckpointKeeper(ctrl)
+		h := testutil.NewHelper(t, btclcKeeper, btccKeeper)
+
+		// set all parameters
+		h.GenAndApplyParams(r)
+
+		// generate dummy staking tx data
+		msgTx := datagen.CreateDummyTx()
+		stakingTx := btcutil.NewTx(msgTx)
+		confirmationDepth := uint32(6)
+		stakingTime := uint32(1000)
+
+		params := h.BTCStakingKeeper.GetParams(h.Ctx)
+
+		// generate common merkle proof and inclusion header
+		prevBlock, _ := datagen.GenRandomBtcdBlock(r, 0, nil)
+		btcHeaderWithProof := datagen.CreateBlockWithTransaction(r, &prevBlock.Header, msgTx)
+		headerHash := btcHeaderWithProof.HeaderBytes.Hash()
+		headerBytes, err := bbntypes.NewBTCHeaderBytesFromBytes(btcHeaderWithProof.HeaderBytes)
+		require.NoError(t, err)
+		inclusionHeight := uint32(datagen.RandomInt(r, 1000) + 1)
+		inclusionHeader := &btclctypes.BTCHeaderInfo{
+			Header: &headerBytes,
+			Height: inclusionHeight,
+		}
+
+		// create inclusion proof
+		proof := &types.ParsedProofOfInclusion{
+			HeaderHash: headerHash,
+			Proof:      btcHeaderWithProof.SpvProof.MerkleNodes,
+			Index:      btcHeaderWithProof.SpvProof.BtcTransactionIndex,
+		}
+
+		// indicates the staking tx has room for unbonding
+		maxValidTipHeight := inclusionHeight + stakingTime - params.MinUnbondingTimeBlocks - 1
+		// indicates the staking tx is k-deep
+		minValidTipHeight := inclusionHeight + confirmationDepth
+
+		t.Run("successful verification", func(t *testing.T) {
+			// Set the tip height to be in the range of valid min and max tip height
+			tipHeight := datagen.RandomInt(r, int(maxValidTipHeight)-int(minValidTipHeight)+1) + uint64(minValidTipHeight)
+			mockTipHeaderInfo := &btclctypes.BTCHeaderInfo{Height: uint32(tipHeight)}
+
+			btclcKeeper.EXPECT().GetHeaderByHash(gomock.Any(), headerHash).Return(inclusionHeader).Times(1)
+			btclcKeeper.EXPECT().GetTipInfo(gomock.Any()).Return(mockTipHeaderInfo).Times(1)
+
+			// Verify inclusion proof
+			timeRange, err := h.BTCStakingKeeper.VerifyInclusionProofAndGetHeight(
+				h.Ctx,
+				stakingTx,
+				confirmationDepth,
+				stakingTime,
+				params.MinUnbondingTimeBlocks,
+				proof,
+			)
+
+			require.NoError(t, err)
+			require.Equal(t, inclusionHeader.Height, timeRange.StartHeight)
+			require.Equal(t, inclusionHeader.Height+stakingTime, timeRange.EndHeight)
+		})
+
+		t.Run("nil inclusion header", func(t *testing.T) {
+			// set the returned inclusion header as nil
+			btclcKeeper.EXPECT().GetHeaderByHash(gomock.Any(), headerHash).Return(nil).Times(1)
+
+			// Verify inclusion proof
+			_, err := h.BTCStakingKeeper.VerifyInclusionProofAndGetHeight(
+				h.Ctx,
+				stakingTx,
+				confirmationDepth,
+				stakingTime,
+				params.MinUnbondingTimeBlocks,
+				proof,
+			)
+
+			require.ErrorContains(t, err, "header that includes the staking tx is not found")
+		})
+
+		t.Run("invalid proof", func(t *testing.T) {
+			btclcKeeper.EXPECT().GetHeaderByHash(gomock.Any(), headerHash).Return(inclusionHeader).Times(1)
+
+			copyProof := *proof
+			// make the proof invalid by setting the index to a different value
+			copyProof.Index = proof.Index + 1
+			_, err = h.BTCStakingKeeper.VerifyInclusionProofAndGetHeight(
+				h.Ctx,
+				stakingTx,
+				confirmationDepth,
+				stakingTime,
+				params.MinUnbondingTimeBlocks,
+				&copyProof,
+			)
+
+			require.ErrorContains(t, err, "not included in the Bitcoin chain")
+		})
+
+		t.Run("insufficient confirmation depth", func(t *testing.T) {
+			tipHeight := inclusionHeight + uint32(datagen.RandomInt(r, int(confirmationDepth)))
+			mockTipHeaderInfo := &btclctypes.BTCHeaderInfo{Height: tipHeight}
+
+			btclcKeeper.EXPECT().GetHeaderByHash(gomock.Any(), headerHash).Return(inclusionHeader).Times(1)
+			btclcKeeper.EXPECT().GetTipInfo(gomock.Any()).Return(mockTipHeaderInfo).Times(1)
+
+			// Verify inclusion proof
+			_, err = h.BTCStakingKeeper.VerifyInclusionProofAndGetHeight(
+				h.Ctx,
+				stakingTx,
+				confirmationDepth,
+				stakingTime,
+				params.MinUnbondingTimeBlocks,
+				proof,
+			)
+
+			require.ErrorContains(t, err, "not k-deep")
+		})
+
+		t.Run("insufficient unbonding time", func(t *testing.T) {
+			tipHeight := datagen.RandomInt(r, 1000) + uint64(maxValidTipHeight) + 1
+			mockTipHeaderInfo := &btclctypes.BTCHeaderInfo{Height: uint32(tipHeight)}
+
+			btclcKeeper.EXPECT().GetHeaderByHash(gomock.Any(), headerHash).Return(inclusionHeader).Times(1)
+			btclcKeeper.EXPECT().GetTipInfo(gomock.Any()).Return(mockTipHeaderInfo).Times(1)
+
+			// Verify inclusion proof
+			_, err = h.BTCStakingKeeper.VerifyInclusionProofAndGetHeight(
+				h.Ctx,
+				stakingTx,
+				confirmationDepth,
+				stakingTime,
+				params.MinUnbondingTimeBlocks,
+				proof,
+			)
+
+			require.ErrorContains(t, err, "staking tx's timelock has no more than unbonding")
+		})
+
+		t.Run("invalid min unbonding time", func(t *testing.T) {
+			// Set the tip height to be in the range of valid min and max tip height
+			tipHeight := datagen.RandomInt(r, int(maxValidTipHeight)-int(minValidTipHeight)+1) + uint64(minValidTipHeight)
+			mockTipHeaderInfo := &btclctypes.BTCHeaderInfo{Height: uint32(tipHeight)}
+
+			btclcKeeper.EXPECT().GetHeaderByHash(gomock.Any(), headerHash).Return(inclusionHeader).Times(1)
+			btclcKeeper.EXPECT().GetTipInfo(gomock.Any()).Return(mockTipHeaderInfo).Times(1)
+
+			// an invalid min_unbonding_time should be >= end_height - tip_height
+			invalidMinUnbondingTime := uint32(datagen.RandomInt(r, 1000)) + inclusionHeight + stakingTime - uint32(tipHeight)
+
+			_, err = h.BTCStakingKeeper.VerifyInclusionProofAndGetHeight(
+				h.Ctx,
+				stakingTx,
+				confirmationDepth,
+				stakingTime,
+				invalidMinUnbondingTime,
+				proof,
+			)
+
+			require.ErrorContains(t, err, "staking tx's timelock has no more than unbonding")
+		})
+	})
+}

x/btcstaking/keeper/msg_server.go

@@ -215,14 +215,14 @@ func (ms msgServer) CreateBTCDelegation(goCtx context.Context, req *types.MsgCre
 			btcutil.NewTx(parsedMsg.StakingTx.Transaction),
 			btccParams.BtcConfirmationDepth,
 			uint32(parsedMsg.StakingTime),
-			vp.Params.MinStakingTimeBlocks,
+			vp.Params.MinUnbondingTimeBlocks,
 			parsedMsg.StakingTxProofOfInclusion)
 		if err != nil {
 			return nil, fmt.Errorf("invalid inclusion proof: %w", err)
 		}
 
-		startHeight = timeInfo.startHeight
-		endHeight = timeInfo.endHeight
+		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
@@ -324,8 +324,8 @@ func (ms msgServer) AddBTCDelegationInclusionProof(
 	}
 
 	// 6. set start height and end height and save it to db
-	btcDel.StartHeight = timeInfo.startHeight
-	btcDel.EndHeight = timeInfo.endHeight
+	btcDel.StartHeight = timeInfo.StartHeight
+	btcDel.EndHeight = timeInfo.EndHeight
 	ms.setBTCDelegation(ctx, btcDel)
 
 	// 7. emit events
@@ -351,7 +351,7 @@ func (ms msgServer) AddBTCDelegationInclusionProof(
 	btcTip := ms.btclcKeeper.GetTipInfo(ctx)
 	ms.addPowerDistUpdateEvent(ctx, btcTip.Height, activeEvent)
 
-	// record event that the BTC delegation will become unbonded at endHeight-w
+	// record event that the BTC delegation will become unbonded at EndHeight-w
 	unbondedEvent := types.NewEventPowerDistUpdateWithBTCDel(&types.EventBTCDelegationStateUpdate{
 		StakingTxHash: req.StakingTxHash,
 		NewState:      types.BTCDelegationStatus_UNBONDED,