Merge branch 'main' into gai/gov-jail-fp

CHANGELOG.md

@@ -37,11 +37,16 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 
 ## Unreleased
 
+### State Breaking
+
+- [278](https://github.com/babylonlabs-io/babylon/pull/278) Allow unbonding time to be min unbonding value
 
 ### Bug fixes
 
 - [#270](https://github.com/babylonlabs-io/babylon/pull/270) Validate there is only
 one finality provider key in the staking request
+- [#270](https://github.com/babylonlabs-io/babylon/pull/277) Panic due to possible
+nil params response
 
 ## v0.16.1
 

client/query/finality.go

@@ -3,9 +3,10 @@ package query
 import (
 	"context"
 
-	finalitytypes "github.com/babylonlabs-io/babylon/x/finality/types"
 	"github.com/cosmos/cosmos-sdk/client"
 	sdkquerytypes "github.com/cosmos/cosmos-sdk/types/query"
+
+	finalitytypes "github.com/babylonlabs-io/babylon/x/finality/types"
 )
 
 // QueryFinality queries the Finality module of the Babylon node according to the given function
@@ -65,7 +66,7 @@ func (c *QueryClient) ActivatedHeight() (*finalitytypes.QueryActivatedHeightResp
 }
 
 // FinalityParams queries the finality module parameters
-func (c *QueryClient) FinalityParams() (*finalitytypes.Params, error) {
+func (c *QueryClient) FinalityParams() (*finalitytypes.QueryParamsResponse, error) {
 	var resp *finalitytypes.QueryParamsResponse
 	err := c.QueryFinality(func(ctx context.Context, queryClient finalitytypes.QueryClient) error {
 		var err error
@@ -74,7 +75,7 @@ func (c *QueryClient) FinalityParams() (*finalitytypes.Params, error) {
 		return err
 	})
 
-	return &resp.Params, err
+	return resp, err
 }
 
 // VotesAtHeight queries the Finality module to get signature set at a given babylon block height

test/e2e/btc_staking_e2e_test.go

@@ -17,9 +17,10 @@ import (
 
 	sdkmath "cosmossdk.io/math"
 	feegrantcli "cosmossdk.io/x/feegrant/client/cli"
-	appparams "github.com/babylonlabs-io/babylon/app/params"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 
+	appparams "github.com/babylonlabs-io/babylon/app/params"
+
 	"github.com/babylonlabs-io/babylon/crypto/eots"
 	"github.com/babylonlabs-io/babylon/test/e2e/configurer"
 	"github.com/babylonlabs-io/babylon/test/e2e/configurer/chain"
@@ -93,7 +94,7 @@ func (s *BTCStakingTestSuite) Test1CreateFinalityProviderAndDelegation() {
 	params := nonValidatorNode.QueryBTCStakingParams()
 
 	// minimal required unbonding time
-	unbondingTime := uint16(initialization.BabylonBtcFinalizationPeriod) + 1
+	unbondingTime := params.MinUnbondingTimeBlocks
 
 	// NOTE: we use the node's address for the BTC delegation
 	stakerAddr := sdk.MustAccAddressFromBech32(nonValidatorNode.PublicAddress)
@@ -508,7 +509,7 @@ func (s *BTCStakingTestSuite) Test6MultisigBTCDelegation() {
 	params := nonValidatorNode.QueryBTCStakingParams()
 
 	// minimal required unbonding time
-	unbondingTime := uint16(initialization.BabylonBtcFinalizationPeriod) + 1
+	unbondingTime := params.MinUnbondingTimeBlocks
 
 	// NOTE: we use the multisig address for the BTC delegation
 	multisigStakerAddr := sdk.MustAccAddressFromBech32(multisigAddr)
@@ -579,7 +580,7 @@ func (s *BTCStakingTestSuite) Test7BTCDelegationFeeGrant() {
 	btcStkParams := nonValidatorNode.QueryBTCStakingParams()
 
 	// minimal required unbonding time
-	unbondingTime := uint16(initialization.BabylonBtcFinalizationPeriod) + 1
+	unbondingTime := btcStkParams.MinUnbondingTimeBlocks
 
 	// NOTE: we use the grantee staker address for the BTC delegation PoP
 	pop, err := bstypes.NewPoPBTC(granteeStakerAddr, s.delBTCSK)
@@ -670,7 +671,7 @@ func (s *BTCStakingTestSuite) Test8BTCDelegationFeeGrantTyped() {
 	btcStkParams := node.QueryBTCStakingParams()
 
 	// minimal required unbonding time
-	unbondingTime := uint16(initialization.BabylonBtcFinalizationPeriod) + 1
+	unbondingTime := btcStkParams.MinUnbondingTimeBlocks
 
 	// NOTE: we use the grantee staker address for the BTC delegation PoP
 	pop, err := bstypes.NewPoPBTC(granteeStakerAddr, s.delBTCSK)
@@ -936,7 +937,7 @@ func (s *BTCStakingTestSuite) BTCStakingUnbondSlashInfo(
 ) {
 	covenantBTCPKs := CovenantBTCPKs(params)
 	// minimal required unbonding time
-	unbondingTime := uint16(initialization.BabylonBtcFinalizationPeriod) + 1
+	unbondingTime := params.MinUnbondingTimeBlocks
 
 	testStakingInfo = datagen.GenBTCStakingSlashingInfo(
 		s.r,
@@ -950,7 +951,7 @@ func (s *BTCStakingTestSuite) BTCStakingUnbondSlashInfo(
 		s.stakingValue,
 		params.SlashingPkScript,
 		params.SlashingRate,
-		unbondingTime,
+		uint16(unbondingTime),
 	)
 
 	// submit staking tx to Bitcoin and get inclusion proof
@@ -985,7 +986,7 @@ func (s *BTCStakingTestSuite) BTCStakingUnbondSlashInfo(
 		unbondingValue,
 		params.SlashingPkScript,
 		params.SlashingRate,
-		unbondingTime,
+		uint16(unbondingTime),
 	)
 
 	stakingSlashingPathInfo, err := testStakingInfo.StakingInfo.SlashingPathSpendInfo()

test/e2e/btc_staking_pre_approval_e2e_test.go

@@ -92,7 +92,7 @@ func (s *BTCStakingPreApprovalTestSuite) Test1CreateFinalityProviderAndDelegatio
 	params := nonValidatorNode.QueryBTCStakingParams()
 
 	// minimal required unbonding time
-	unbondingTime := uint16(initialization.BabylonBtcFinalizationPeriod) + 1
+	unbondingTime := params.MinUnbondingTimeBlocks
 
 	// NOTE: we use the node's address for the BTC delegation
 	stakerAddr := sdk.MustAccAddressFromBech32(nonValidatorNode.PublicAddress)
@@ -535,7 +535,7 @@ func (s *BTCStakingPreApprovalTestSuite) BTCStakingUnbondSlashInfo(
 ) {
 	covenantBTCPKs := CovenantBTCPKs(params)
 	// minimal required unbonding time
-	unbondingTime := uint16(initialization.BabylonBtcFinalizationPeriod) + 1
+	unbondingTime := params.MinUnbondingTimeBlocks
 
 	testStakingInfo = datagen.GenBTCStakingSlashingInfo(
 		s.r,
@@ -549,7 +549,7 @@ func (s *BTCStakingPreApprovalTestSuite) BTCStakingUnbondSlashInfo(
 		s.stakingValue,
 		params.SlashingPkScript,
 		params.SlashingRate,
-		unbondingTime,
+		uint16(unbondingTime),
 	)
 
 	// submit staking tx to Bitcoin and get inclusion proof
@@ -584,7 +584,7 @@ func (s *BTCStakingPreApprovalTestSuite) BTCStakingUnbondSlashInfo(
 		unbondingValue,
 		params.SlashingPkScript,
 		params.SlashingRate,
-		unbondingTime,
+		uint16(unbondingTime),
 	)
 
 	stakingSlashingPathInfo, err := testStakingInfo.StakingInfo.SlashingPathSpendInfo()

testutil/btcstaking-helper/keeper.go

@@ -124,7 +124,8 @@ func (h *Helper) BeginBlocker() {
 }
 
 func (h *Helper) GenAndApplyParams(r *rand.Rand) ([]*btcec.PrivateKey, []*btcec.PublicKey) {
-	return h.GenAndApplyCustomParams(r, 100, 0, 0)
+	// ensure that minUnbondingTime is larger than finalizationTimeout
+	return h.GenAndApplyCustomParams(r, 100, 200, 0)
 }
 
 func (h *Helper) SetCtxHeight(height uint64) {
@@ -228,7 +229,7 @@ func (h *Helper) CreateDelegation(
 	if unbondingValue == 0 {
 		unbondingValue = defaultUnbondingValue
 	}
-	defaultUnbondingTime := types.MinimumUnbondingTime(&bsParams, &bcParams) + 1
+	defaultUnbondingTime := bsParams.MinUnbondingTimeBlocks
 	if unbondingTime == 0 {
 		unbondingTime = uint16(defaultUnbondingTime)
 	}

x/btccheckpoint/keeper/msg_server.go

@@ -4,9 +4,10 @@ import (
 	"context"
 
 	errorsmod "cosmossdk.io/errors"
-	"github.com/babylonlabs-io/babylon/x/btccheckpoint/types"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	govtypes "github.com/cosmos/cosmos-sdk/x/gov/types"
+
+	"github.com/babylonlabs-io/babylon/x/btccheckpoint/types"
 )
 
 var _ types.MsgServer = msgServer{}
@@ -109,7 +110,13 @@ func (ms msgServer) UpdateParams(goCtx context.Context, req *types.MsgUpdatePara
 		return nil, govtypes.ErrInvalidProposalMsg.Wrapf("invalid parameter: %v", err)
 	}
 
+	// CheckpointFinalizationTimeout must remain immutable as changing it
+	// breaks a lot of system assumption
 	ctx := sdk.UnwrapSDKContext(goCtx)
+	if req.Params.CheckpointFinalizationTimeout != ms.k.GetParams(ctx).CheckpointFinalizationTimeout {
+		return nil, govtypes.ErrInvalidProposalMsg.Wrapf("the checkpoint finalization timeout cannot be changed")
+	}
+
 	if err := ms.k.SetParams(ctx, req.Params); err != nil {
 		return nil, err
 	}

x/btccheckpoint/keeper/msg_server_test.go

@@ -8,14 +8,17 @@ import (
 	"testing"
 	"time"
 
+	"github.com/btcsuite/btcd/chaincfg"
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
+	govtypes "github.com/cosmos/cosmos-sdk/x/gov/types"
+	"github.com/stretchr/testify/require"
+
 	dg "github.com/babylonlabs-io/babylon/testutil/datagen"
 	keepertest "github.com/babylonlabs-io/babylon/testutil/keeper"
 	bbn "github.com/babylonlabs-io/babylon/types"
 	bkeeper "github.com/babylonlabs-io/babylon/x/btccheckpoint/keeper"
 	btcctypes "github.com/babylonlabs-io/babylon/x/btccheckpoint/types"
-	"github.com/btcsuite/btcd/chaincfg"
-	sdk "github.com/cosmos/cosmos-sdk/types"
-	"github.com/stretchr/testify/require"
 )
 
 type TestKeepers struct {
@@ -78,6 +81,27 @@ func (k *TestKeepers) onTipChange() {
 	k.BTCCheckpoint.OnTipChange(k.SdkCtx)
 }
 
+func TestUpdateParams(t *testing.T) {
+	tk := InitTestKeepers(t)
+
+	// Try to update params with a different checkpoint finalization timeout
+	msg := &btcctypes.MsgUpdateParams{
+		Authority: authtypes.NewModuleAddress(govtypes.ModuleName).String(),
+		Params: btcctypes.Params{
+			CheckpointFinalizationTimeout: btcctypes.DefaultParams().CheckpointFinalizationTimeout + 1,
+		},
+	}
+
+	_, err := tk.MsgSrv.UpdateParams(tk.Ctx, msg)
+	require.ErrorIs(t, err, govtypes.ErrInvalidProposalMsg,
+		"should not be able to change CheckpointFinalizationTimeout parameter")
+
+	// Verify params were not changed
+	params := tk.BTCCheckpoint.GetParams(tk.SdkCtx)
+	require.Equal(t, btcctypes.DefaultParams().CheckpointFinalizationTimeout, params.CheckpointFinalizationTimeout,
+		"minUnbondingTime should remain unchanged")
+}
+
 func TestRejectDuplicatedSubmission(t *testing.T) {
 	r := rand.New(rand.NewSource(time.Now().Unix()))
 	epoch := uint64(1)

x/btcstaking/keeper/btc_delegations.go

@@ -79,7 +79,7 @@ func (k Keeper) AddBTCDelegation(
 			NewState:      types.BTCDelegationStatus_UNBONDED,
 		})
 
-		// NOTE: we should have verified that EndHeight > btcTip.Height + max(w, min_unbonding_time)
+		// NOTE: we should have verified that EndHeight > btcTip.Height + min_unbonding_time
 		k.addPowerDistUpdateEvent(ctx, btcDel.EndHeight-minUnbondingTime, unbondedEvent)
 	}
 

x/btcstaking/keeper/genesis_test.go

@@ -6,23 +6,23 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/stretchr/testify/require"
+
 	"github.com/babylonlabs-io/babylon/testutil/datagen"
 	"github.com/babylonlabs-io/babylon/testutil/helper"
 	btclightclientt "github.com/babylonlabs-io/babylon/x/btclightclient/types"
 	"github.com/babylonlabs-io/babylon/x/btcstaking/types"
-	"github.com/stretchr/testify/require"
 )
 
 func TestExportGenesis(t *testing.T) {
 	r, h := rand.New(rand.NewSource(11)), helper.NewHelper(t)
-	k, btclcK, btcCheckK, ctx := h.App.BTCStakingKeeper, h.App.BTCLightClientKeeper, h.App.BtcCheckpointKeeper, h.Ctx
+	k, btclcK, ctx := h.App.BTCStakingKeeper, h.App.BTCLightClientKeeper, h.Ctx
 	numFps := 3
 
 	fps := datagen.CreateNFinalityProviders(r, t, numFps)
 	params := k.GetParams(ctx)
-	btcckptParams := btcCheckK.GetParams(ctx)
 
-	minUnbondingTime := types.MinimumUnbondingTime(&params, &btcckptParams)
+	minUnbondingTime := params.MinUnbondingTimeBlocks
 
 	chainsHeight := make([]*types.BlockHeightBbnToBtc, 0)
 	// creates the first as it starts already with an chain height from the helper.

x/btcstaking/keeper/msg_server.go

@@ -46,7 +46,16 @@ func (ms msgServer) UpdateParams(goCtx context.Context, req *types.MsgUpdatePara
 		return nil, govtypes.ErrInvalidProposalMsg.Wrapf("invalid parameter: %v", err)
 	}
 
+	// ensure the min unbonding time is always larger than the checkpoint finalization timeout
 	ctx := sdk.UnwrapSDKContext(goCtx)
+	ckptFinalizationTime := ms.btccKeeper.GetParams(ctx).CheckpointFinalizationTimeout
+	minUnbondingTime := req.Params.MinUnbondingTimeBlocks
+	if minUnbondingTime <= ckptFinalizationTime {
+		return nil, govtypes.ErrInvalidProposalMsg.
+			Wrapf("the min unbonding time %d must be larger than the checkpoint finalization timeout %d",
+				minUnbondingTime, ckptFinalizationTime)
+	}
+
 	if err := ms.SetParams(ctx, req.Params); err != nil {
 		return nil, err
 	}
@@ -190,24 +199,23 @@ func (ms msgServer) CreateBTCDelegation(goCtx context.Context, req *types.MsgCre
 	}
 
 	// 6. Validate parsed message against parameters
-	btccParams := ms.btccKeeper.GetParams(ctx)
-
-	paramsValidationResult, err := types.ValidateParsedMessageAgainstTheParams(parsedMsg, &vp.Params, &btccParams, ms.btcNet)
+	paramsValidationResult, err := types.ValidateParsedMessageAgainstTheParams(parsedMsg, &vp.Params, ms.btcNet)
 
 	if err != nil {
 		return nil, err
 	}
 
 	// 7. If the delegation contains the inclusion proof, we need to verify the proof
 	// and set start height and end height
+	btccParams := ms.btccKeeper.GetParams(ctx)
 	var startHeight, endHeight uint32
 	if parsedMsg.StakingTxProofOfInclusion != nil {
 		timeInfo, err := ms.VerifyInclusionProofAndGetHeight(
 			ctx,
 			btcutil.NewTx(parsedMsg.StakingTx.Transaction),
 			btccParams.BtcConfirmationDepth,
 			uint32(parsedMsg.StakingTime),
-			paramsValidationResult.MinUnbondingTime,
+			vp.Params.MinStakingTimeBlocks,
 			parsedMsg.StakingTxProofOfInclusion)
 		if err != nil {
 			return nil, fmt.Errorf("invalid inclusion proof: %w", err)
@@ -251,7 +259,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, paramsValidationResult.MinUnbondingTime); err != nil {
+	if err := ms.AddBTCDelegation(ctx, newBTCDel, vp.Params.MinUnbondingTimeBlocks); err != nil {
 		panic(fmt.Errorf("failed to add BTC delegation that has passed verification: %w", err))
 	}
 
@@ -300,7 +308,7 @@ func (ms msgServer) AddBTCDelegationInclusionProof(
 
 	btccParams := ms.btccKeeper.GetParams(ctx)
 
-	minUnbondingTime := types.MinimumUnbondingTime(params, &btccParams)
+	minUnbondingTime := params.MinUnbondingTimeBlocks
 
 	timeInfo, err := ms.VerifyInclusionProofAndGetHeight(
 		ctx,
@@ -349,7 +357,7 @@ func (ms msgServer) AddBTCDelegationInclusionProof(
 		NewState:      types.BTCDelegationStatus_UNBONDED,
 	})
 
-	// NOTE: we should have verified that EndHeight > btcTip.Height + max(w, min_unbonding_time)
+	// NOTE: we should have verified that EndHeight > btcTip.Height + min_unbonding_time
 	ms.addPowerDistUpdateEvent(ctx, btcDel.EndHeight-minUnbondingTime, unbondedEvent)
 
 	// at this point, the BTC delegation inclusion proof is verified and is not duplicated