Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Allow unbonding time to be min unbonding value #278

Merged
merged 6 commits into from
Nov 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ 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

Expand Down
17 changes: 9 additions & 8 deletions test/e2e/btc_staking_e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -985,7 +986,7 @@ func (s *BTCStakingTestSuite) BTCStakingUnbondSlashInfo(
unbondingValue,
params.SlashingPkScript,
params.SlashingRate,
unbondingTime,
uint16(unbondingTime),
)

stakingSlashingPathInfo, err := testStakingInfo.StakingInfo.SlashingPathSpendInfo()
Expand Down
8 changes: 4 additions & 4 deletions test/e2e/btc_staking_pre_approval_e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -584,7 +584,7 @@ func (s *BTCStakingPreApprovalTestSuite) BTCStakingUnbondSlashInfo(
unbondingValue,
params.SlashingPkScript,
params.SlashingRate,
unbondingTime,
uint16(unbondingTime),
)

stakingSlashingPathInfo, err := testStakingInfo.StakingInfo.SlashingPathSpendInfo()
Expand Down
5 changes: 3 additions & 2 deletions testutil/btcstaking-helper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
}
Expand Down
9 changes: 8 additions & 1 deletion x/btccheckpoint/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand Down Expand Up @@ -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 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we add a comment here on why it can't be changed?

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
}
Expand Down
30 changes: 27 additions & 3 deletions x/btccheckpoint/keeper/msg_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
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 @@ -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)
}

Expand Down
8 changes: 4 additions & 4 deletions x/btcstaking/keeper/genesis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
22 changes: 15 additions & 7 deletions x/btcstaking/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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))
}

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down
Loading