Skip to content

Commit

Permalink
fix status field in rpc api (#505)
Browse files Browse the repository at this point in the history
fixes: #503
Every query was using latest params to compute wheter delegation is
active instead of params related to delegation version.

This could lead to not returnig certain results in case of updating
covenant quorum.
  • Loading branch information
KonradStaniec committed Feb 12, 2025
1 parent eff7248 commit 23f1d6e
Show file tree
Hide file tree
Showing 3 changed files with 132 additions and 6 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
### Bug fixes

- [#486](https://github.com/babylonlabs-io/babylon/pull/486) crypto: blinding base mult of nonce
- [#443](https://github.com/babylonlabs-io/babylon/pull/443) Fix swagger generation for incentive missing `v1` in path
- [#505](https://github.com/babylonlabs-io/babylon/pull/505) Fix `x/btcstaking`
delegation queries

## v1.0.0-rc5

Expand Down
15 changes: 9 additions & 6 deletions x/btcstaking/keeper/grpc_query.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,6 @@ func (k Keeper) BTCDelegations(ctx context.Context, req *types.QueryBTCDelegatio
return nil, status.Error(codes.InvalidArgument, "empty request")
}

covenantQuorum := k.GetParams(ctx).CovenantQuorum

// get current BTC height
btcTipHeight := k.btclcKeeper.GetTipInfo(ctx).Height

Expand All @@ -94,8 +92,10 @@ func (k Keeper) BTCDelegations(ctx context.Context, req *types.QueryBTCDelegatio
var btcDel types.BTCDelegation
k.cdc.MustUnmarshal(value, &btcDel)

params := k.GetParamsByVersion(ctx, btcDel.ParamsVersion)

// hit if the queried status is ANY or matches the BTC delegation status
status := btcDel.GetStatus(btcTipHeight, covenantQuorum)
status := btcDel.GetStatus(btcTipHeight, params.CovenantQuorum)
if req.Status == types.BTCDelegationStatus_ANY || status == req.Status {
if accumulate {
resp := types.NewBTCDelegationResponse(&btcDel, status)
Expand Down Expand Up @@ -136,7 +136,6 @@ func (k Keeper) FinalityProviderDelegations(ctx context.Context, req *types.Quer
btcDelStore := k.btcDelegatorFpStore(sdkCtx, fpPK)

btcHeight := k.btclcKeeper.GetTipInfo(ctx).Height
covenantQuorum := k.GetParams(ctx).CovenantQuorum

btcDels := []*types.BTCDelegatorDelegationsResponse{}
pageRes, err := query.Paginate(btcDelStore, req.Pagination, func(key, value []byte) error {
Expand All @@ -149,9 +148,11 @@ func (k Keeper) FinalityProviderDelegations(ctx context.Context, req *types.Quer

btcDelsResp := make([]*types.BTCDelegationResponse, len(curBTCDels.Dels))
for i, btcDel := range curBTCDels.Dels {
params := k.GetParamsByVersion(sdkCtx, btcDel.ParamsVersion)

status := btcDel.GetStatus(
btcHeight,
covenantQuorum,
params.CovenantQuorum,
)
btcDelsResp[i] = types.NewBTCDelegationResponse(btcDel, status)
}
Expand Down Expand Up @@ -186,9 +187,11 @@ func (k Keeper) BTCDelegation(ctx context.Context, req *types.QueryBTCDelegation
return nil, types.ErrBTCDelegationNotFound
}

params := k.GetParamsByVersion(ctx, btcDel.ParamsVersion)

status := btcDel.GetStatus(
k.btclcKeeper.GetTipInfo(ctx).Height,
k.GetParams(ctx).CovenantQuorum,
params.CovenantQuorum,
)

return &types.QueryBTCDelegationResponse{
Expand Down
120 changes: 120 additions & 0 deletions x/btcstaking/keeper/grpc_query_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"errors"
"math/rand"
"testing"
"time"

sdkmath "cosmossdk.io/math"

Expand Down Expand Up @@ -405,3 +406,122 @@ func AddFinalityProvider(t *testing.T, goCtx context.Context, k btcstakingkeeper
})
require.NoError(t, err)
}

func TestCorrectParamsVersionIsUsed(t *testing.T) {
r := rand.New(rand.NewSource(time.Now().UnixNano()))
ctrl := gomock.NewController(t)
defer ctrl.Finish()

btclcKeeper := types.NewMockBTCLightClientKeeper(ctrl)
btccKeeper := types.NewMockBtcCheckpointKeeper(ctrl)
btccKeeper.EXPECT().GetParams(gomock.Any()).Return(btcctypes.DefaultParams()).AnyTimes()
keeper, ctx := testkeeper.BTCStakingKeeper(t, btclcKeeper, btccKeeper, nil)

// covenant and slashing addr
covenantSKs, covenantPKs, covenantQuorum := datagen.GenCovenantCommittee(r)
slashingAddress, err := datagen.GenRandomBTCAddress(r, net)
require.NoError(t, err)
slashingPkScript, err := txscript.PayToAddrScript(slashingAddress)
require.NoError(t, err)
slashingChangeLockTime := uint16(101)

// Generate a slashing rate in the range [0.1, 0.50] i.e., 10-50%.
// NOTE - if the rate is higher or lower, it may produce slashing or change outputs
// with value below the dust threshold, causing test failure.
// Our goal is not to test failure due to such extreme cases here;
// this is already covered in FuzzGeneratingValidStakingSlashingTx
slashingRate := sdkmath.LegacyNewDecWithPrec(int64(datagen.RandomInt(r, 41)+10), 2)

fp, err := datagen.GenRandomFinalityProvider(r)
require.NoError(t, err)
AddFinalityProvider(t, ctx, *keeper, fp)

startHeight := uint32(datagen.RandomInt(r, 100)) + 1
btclcKeeper.EXPECT().GetTipInfo(gomock.Any()).Return(&btclctypes.BTCHeaderInfo{Height: startHeight}).AnyTimes()

endHeight := uint32(datagen.RandomInt(r, 1000)) + startHeight + btcctypes.DefaultParams().CheckpointFinalizationTimeout + 1
stakingTime := endHeight - startHeight
delSK, _, err := datagen.GenRandomBTCKeyPair(r)
require.NoError(t, err)
btcDel, err := datagen.GenRandomBTCDelegation(
r,
t,
net,
[]bbn.BIP340PubKey{*fp.BtcPk},
delSK,
covenantSKs,
covenantPKs,
covenantQuorum,
slashingPkScript,
stakingTime, startHeight, endHeight, 10000,
slashingRate,
slashingChangeLockTime,
)
require.NoError(t, err)

err = keeper.AddBTCDelegation(ctx, btcDel)
require.NoError(t, err)

// delegation is active as it have all covenant sigs
txHash := btcDel.MustGetStakingTxHash().String()
delView, err := keeper.BTCDelegation(ctx, &types.QueryBTCDelegationRequest{
StakingTxHashHex: txHash,
})
require.NoError(t, err)
require.NotNil(t, delView)

require.True(t, delView.BtcDelegation.Active)

dp := types.DefaultParams()

// Generate 3 new key pairs and increase keys and quorum size in params, this
// will mean new delegations will require more signatures to be active
_, pk1, err := datagen.GenRandomBTCKeyPair(r)
require.NoError(t, err)
_, pk2, err := datagen.GenRandomBTCKeyPair(r)
require.NoError(t, err)
_, pk3, err := datagen.GenRandomBTCKeyPair(r)
require.NoError(t, err)

// Convert public keys to BIP340 format
bip340pk1 := bbn.NewBIP340PubKeyFromBTCPK(pk1)
bip340pk2 := bbn.NewBIP340PubKeyFromBTCPK(pk2)
bip340pk3 := bbn.NewBIP340PubKeyFromBTCPK(pk3)

dp.BtcActivationHeight = 10
dp.CovenantPks = append(dp.CovenantPks, *bip340pk1, *bip340pk2, *bip340pk3)
dp.CovenantQuorum += 3

err = keeper.SetParams(ctx, dp)
require.NoError(t, err)

// check delegation is still active in every endpoint
delView, err = keeper.BTCDelegation(ctx, &types.QueryBTCDelegationRequest{
StakingTxHashHex: txHash,
})
require.NoError(t, err)
require.NotNil(t, delView)

require.True(t, delView.BtcDelegation.Active)

delegationsView, err := keeper.BTCDelegations(ctx, &types.QueryBTCDelegationsRequest{
Status: types.BTCDelegationStatus_ACTIVE,
})
require.NoError(t, err)
require.NotNil(t, delegationsView)
require.Len(t, delegationsView.BtcDelegations, 1)

pagination := constructRequestWithLimit(r, 10)
// Generate the initial query
req := types.QueryFinalityProviderDelegationsRequest{
FpBtcPkHex: fp.BtcPk.MarshalHex(),
Pagination: pagination,
}

fpView, err := keeper.FinalityProviderDelegations(ctx, &req)
require.NoError(t, err)
require.NotNil(t, fpView)
require.Len(t, fpView.BtcDelegatorDelegations, 1)
require.Len(t, fpView.BtcDelegatorDelegations[0].Dels, 1)
require.True(t, fpView.BtcDelegatorDelegations[0].Dels[0].Active)
}

0 comments on commit 23f1d6e

Please sign in to comment.