Skip to content

Commit

Permalink
backport: Fix invalid minUnbondingTime for verifying inclusion proof (#…
Browse files Browse the repository at this point in the history
…289) (#290)

The bug is caused due to setting `params.MinStakingTime` to the
btcstaking module's keeper method `VerifyInclusionProofAndGetHeight`
other than `params.MinUnbondingTime`. In the fix, we remove
`minUnbondingTime` from the parameter list of
`VerifyInclusionProofAndGetHeight` as it should retrieve the parameter
within the method.

This PR also added comprehensive tests for
`VerifyInclusionProofAndGetHeight`
  • Loading branch information
gitferry authored Nov 22, 2024
1 parent 7048993 commit 8339e55
Show file tree
Hide file tree
Showing 10 changed files with 263 additions and 27 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion testutil/btcstaking-helper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
29 changes: 28 additions & 1 deletion testutil/datagen/btc_transaction.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"bytes"
"encoding/binary"
"encoding/hex"
"fmt"
"math"
"math/rand"
"runtime"
Expand Down Expand Up @@ -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{
Expand All @@ -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{
Expand Down
2 changes: 1 addition & 1 deletion x/btcstaking/keeper/btc_delegations.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
2 changes: 1 addition & 1 deletion x/btcstaking/keeper/genesis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
20 changes: 12 additions & 8 deletions x/btcstaking/keeper/inclusion_proof.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}
187 changes: 187 additions & 0 deletions x/btcstaking/keeper/inclusion_proof_test.go
Original file line number Diff line number Diff line change
@@ -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")
})
})
}
12 changes: 6 additions & 6 deletions x/btcstaking/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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,
Expand Down
Loading

0 comments on commit 8339e55

Please sign in to comment.