From c7c673370aa11c1db75b83ee30cdd5f6af2270b2 Mon Sep 17 00:00:00 2001 From: RafilxTenfen Date: Tue, 24 Dec 2024 12:08:21 -0300 Subject: [PATCH] fix: slashed fp and BTC rewards tracker (#371) `EventPowerDistUpdate_SlashedFp` and [`RewardBTCStaking`](https://github.com/babylonlabs-io/babylon/blob/b132d16483d2e12ac68be74a524a4b7121edac65/x/incentive/keeper/btc_staking_gauge.go#L19) are called at different blocks. The rewards are being called a few blocks behind and if the slashed fp is pruned in BTC reward tracker data, it fails to find and panics On babylond-deployments an fp is being slashed at block height 46, the call of `RewardBTCStaking` happens at Babylon block 48 but is called to distribute rewards of voting power table from block 44 (which didn't had the FP slashed yet) which then checks for fp rewards for the FP that was pruned at the slashing processed event. ``` 6:04PM INF finalizing commit of block hash=153E869F7B0056A9CD8208301F2019FFACA2BEFBBB35AEADF70DCA14C15F3972 height=47 module=consensus num_txs=2 root=62B5F025459DEEBE2EE6F5D869C5BCBE903299FF80825FB9DE62551E6B725D52 ERR CONSENSUS FAILURE!!! err="failed to add fp rewards for btc delegation bbn1wwkaq6z3kdkekm7ltwxng4ftvzux8gzp2xjx8d at height 44: finality provider current rewards not found ``` ```shell $~ babylond q btcstaking finality-providers -o json | jq ... { "description": { "moniker": "Finality Provider 0", "identity": "", "website": "", "security_contact": "", "details": "" }, "commission": "0.050000000000000000", "addr": "bbn1wwkaq6z3kdkekm7ltwxng4ftvzux8gzp2xjx8d", "btc_pk": "6ba387c315dc6105ec02b50684593505103a9f13452d3a597c16ac2f22b924dc", "pop": { "btc_sig_type": "BIP340", "btc_sig": "Q3PcfiKT0fwWOVx95rjV+mQvI/juTbYIV84Hfz2NAmQByiHRXz8OWnFbyugJgOrd/TrqR+6BOgtr10bZUFDFbA==" }, "slashed_babylon_height": "46", "slashed_btc_height": 131, "height": "46", "jailed": false, "highest_voted_height": 45 }, ``` Basically, this means we are processing the rewards a few blocks down the road after the slash and then it is not possible to prune the data for reward tracker until that fp slashed height is processed. The fix for the time being: do nothing at the btc reward tracker structures if some fp gets slashed just continue to store that data. In the future during the issue https://github.com/babylonlabs-io/babylon/issues/362 we can properly handle the pruning of slashed finality provider data --- CHANGELOG.md | 9 +- test/e2e/btc_rewards_distribution_e2e_test.go | 139 +++++++--- testutil/btcstaking-helper/keeper.go | 4 +- testutil/incentives-helper/keeper.go | 4 - x/btcstaking/keeper/bench_test.go | 3 - x/btcstaking/keeper/msg_server_test.go | 53 +--- x/finality/keeper/liveness_test.go | 4 - x/finality/keeper/power_dist_change.go | 24 +- x/finality/keeper/power_dist_change_test.go | 238 ++++++++++++++---- x/finality/keeper/power_table_test.go | 17 +- x/finality/types/expected_keepers.go | 1 - x/finality/types/power_table.go | 8 +- x/incentive/keeper/btc_staking_gauge.go | 3 +- x/incentive/keeper/reward_tracker_test.go | 8 +- 14 files changed, 342 insertions(+), 173 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c64a819ea..3e1abb85f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,7 +37,14 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) ## Unreleased -## v1.0.0-rc1 +## v1.0.0-rc2 + +### Bug fixes + +- [#371](https://github.com/babylonlabs-io/babylon/pull/371) Do not prune BTC +reward tracker structures at the slash of finality provider. + +## v1.0.0-rc.1 ### Improvements diff --git a/test/e2e/btc_rewards_distribution_e2e_test.go b/test/e2e/btc_rewards_distribution_e2e_test.go index df4848a4e..751867922 100644 --- a/test/e2e/btc_rewards_distribution_e2e_test.go +++ b/test/e2e/btc_rewards_distribution_e2e_test.go @@ -16,6 +16,7 @@ import ( "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" + "github.com/babylonlabs-io/babylon/crypto/eots" "github.com/babylonlabs-io/babylon/test/e2e/configurer" "github.com/babylonlabs-io/babylon/test/e2e/configurer/chain" "github.com/babylonlabs-io/babylon/testutil/coins" @@ -117,7 +118,7 @@ func (s *BtcRewardsDistribution) SetupSuite() { // Test1CreateFinalityProviders creates all finality providers func (s *BtcRewardsDistribution) Test1CreateFinalityProviders() { chainA := s.configurer.GetChainConfig(0) - chainA.WaitUntilHeight(1) + chainA.WaitUntilHeight(2) n1, err := chainA.GetNodeAtIndex(1) s.NoError(err) @@ -207,9 +208,6 @@ func (s *BtcRewardsDistribution) Test3SubmitCovenantSignature() { n1.WaitForNextBlock() } - // wait for a block so that above txs take effect - n1.WaitForNextBlock() - // ensure the BTC delegation has covenant sigs now activeDelsSet := n1.QueryFinalityProvidersDelegations(s.fp1.BtcPk.MarshalHex(), s.fp2.BtcPk.MarshalHex()) s.Len(activeDelsSet, 3) @@ -228,7 +226,7 @@ func (s *BtcRewardsDistribution) Test4CommitPublicRandomnessAndSealed() { s.NoError(err) // commit public randomness list - commitStartHeight := uint64(1) + commitStartHeight := uint64(5) fp1RandListInfo, fp1CommitPubRandList, err := datagen.GenRandomMsgCommitPubRandList(s.r, s.fp1BTCSK, commitStartHeight, numPubRand) s.NoError(err) @@ -293,10 +291,12 @@ func (s *BtcRewardsDistribution) Test4CommitPublicRandomnessAndSealed() { s.Equal(s.finalityBlockHeightVoted, finalizedBlocks[0].Height) s.Equal(appHash.Bytes(), finalizedBlocks[0].AppHash) s.T().Logf("the block %d is finalized", s.finalityBlockHeightVoted) - s.AddFinalityVoteUntilCurrentHeight() } // Test5CheckRewardsFirstDelegations verifies the rewards independent of mint amounts +// There might be a difference in rewards if the BTC delegations were included in different blocks +// that is the reason to get the difference in rewards between a block range to assert +// the reward difference between fps and delegators. func (s *BtcRewardsDistribution) Test5CheckRewardsFirstDelegations() { n2, err := s.configurer.GetChainConfig(0).GetNodeAtIndex(2) s.NoError(err) @@ -312,21 +312,33 @@ func (s *BtcRewardsDistribution) Test5CheckRewardsFirstDelegations() { // (del1) => 4_00000000 // (del2) => 4_00000000 + // verifies that everyone is active and not slashed + fps := n2.QueryFinalityProviders() + s.Len(fps, 2) + s.Equal(fps[0].Commission.String(), fps[1].Commission.String()) + for _, fp := range fps { + s.Equal(fp.SlashedBabylonHeight, uint64(0)) + s.Equal(fp.SlashedBtcHeight, uint32(0)) + } + + dels := n2.QueryFinalityProvidersDelegations(s.fp1.BtcPk.MarshalHex(), s.fp2.BtcPk.MarshalHex()) + s.Len(dels, 3) + for _, del := range dels { + s.True(del.Active) + } + // The rewards distributed for the finality providers should be fp1 => 3x, fp2 => 1x - fp1LastRewardGauge, fp2LastRewardGauge, btcDel1LastRewardGauge, btcDel2LastRewardGauge := s.QueryRewardGauges(n2) + fp1DiffRewards, fp2DiffRewards, del1DiffRewards, del2DiffRewards := s.QueryRewardGauges(n2) + s.AddFinalityVoteUntilCurrentHeight() - // fp1 ~2674ubbn - // fp2 ~891ubbn coins.RequireCoinsDiffInPointOnePercentMargin( s.T(), - fp2LastRewardGauge.Coins.MulInt(sdkmath.NewIntFromUint64(3)), // ~2673ubbn - fp1LastRewardGauge.Coins, + fp2DiffRewards.Coins.MulInt(sdkmath.NewIntFromUint64(3)), + fp1DiffRewards.Coins, ) // The rewards distributed to the delegators should be the same for each delegator - // del1 ~7130ubbn - // del2 ~7130ubbn - coins.RequireCoinsDiffInPointOnePercentMargin(s.T(), btcDel1LastRewardGauge.Coins, btcDel2LastRewardGauge.Coins) + coins.RequireCoinsDiffInPointOnePercentMargin(s.T(), del1DiffRewards.Coins, del2DiffRewards.Coins) CheckWithdrawReward(s.T(), n2, wDel2, s.del2Addr) @@ -399,27 +411,9 @@ func (s *BtcRewardsDistribution) Test7CheckRewards() { // (fp2) => 8_00000000 // (del1) => 4_00000000 // (del2) => 10_00000000 - fp1RewardGaugePrev, fp2RewardGaugePrev, btcDel1RewardGaugePrev, btcDel2RewardGaugePrev := s.QueryRewardGauges(n2) - // wait a few block of rewards to calculate the difference - n2.WaitForNextBlocks(2) - s.AddFinalityVoteUntilCurrentHeight() - n2.WaitForNextBlocks(2) - s.AddFinalityVoteUntilCurrentHeight() - n2.WaitForNextBlocks(2) - s.AddFinalityVoteUntilCurrentHeight() - n2.WaitForNextBlocks(2) - fp1RewardGauge, fp2RewardGauge, btcDel1RewardGauge, btcDel2RewardGauge := s.QueryRewardGauges(n2) - - // since varius block were created, it is needed to get the difference - // from a certain point where all the delegations were active to properly - // calculate the distribution with the voting power structure with 4 BTC delegations active - // Note: if a new block is mined during the query of reward gauges, the calculation might be a - // bit off by some ubbn - fp1DiffRewards := fp1RewardGauge.Coins.Sub(fp1RewardGaugePrev.Coins...) - fp2DiffRewards := fp2RewardGauge.Coins.Sub(fp2RewardGaugePrev.Coins...) - del1DiffRewards := btcDel1RewardGauge.Coins.Sub(btcDel1RewardGaugePrev.Coins...) - del2DiffRewards := btcDel2RewardGauge.Coins.Sub(btcDel2RewardGaugePrev.Coins...) + // gets the difference in rewards in 8 blocks range + fp1DiffRewards, fp2DiffRewards, del1DiffRewards, del2DiffRewards := s.GetRewardDifferences(8) // Check the difference in the finality providers // fp1 should receive ~75% of the rewards received by fp2 @@ -442,7 +436,82 @@ func (s *BtcRewardsDistribution) Test7CheckRewards() { s.NotEmpty(del2DiffRewardsStr) } -// TODO(rafilx): Slash a FP and expect rewards to be withdraw. +// Test8SlashFp slashes the finality provider, but should continue to produce blocks +func (s *BtcRewardsDistribution) Test8SlashFp() { + chainA := s.configurer.GetChainConfig(0) + n2, err := chainA.GetNodeAtIndex(2) + s.NoError(err) + + badBlockHeightToVote := s.finalityBlockHeightVoted + 1 + + blockToVote, err := n2.QueryBlock(int64(badBlockHeightToVote)) + s.NoError(err) + appHash := blockToVote.AppHash + + // generate bad EOTS signature with a diff block height to vote + msgToSign := append(sdk.Uint64ToBigEndian(s.finalityBlockHeightVoted), appHash...) + fp1Sig, err := eots.Sign(s.fp2BTCSK, s.fp2RandListInfo.SRList[s.finalityIdx], msgToSign) + s.NoError(err) + + finalitySig := bbn.NewSchnorrEOTSSigFromModNScalar(fp1Sig) + + // submit finality signature to slash + n2.AddFinalitySigFromVal( + s.fp2.BtcPk, + s.finalityBlockHeightVoted, + &s.fp2RandListInfo.PRList[s.finalityIdx], + *s.fp2RandListInfo.ProofList[s.finalityIdx].ToProto(), + appHash, + finalitySig, + ) + + n2.WaitForNextBlocks(2) + + fps := n2.QueryFinalityProviders() + require.Len(s.T(), fps, 2) + for _, fp := range fps { + if strings.EqualFold(fp.Addr, s.fp1Addr) { + require.Zero(s.T(), fp.SlashedBabylonHeight) + continue + } + require.NotZero(s.T(), fp.SlashedBabylonHeight) + } + + // wait a few blocks to check if it doesn't panic when rewards are being produced + n2.WaitForNextBlocks(5) +} + +func (s *BtcRewardsDistribution) GetRewardDifferences(blocksDiff uint64) ( + fp1DiffRewards, fp2DiffRewards, del1DiffRewards, del2DiffRewards sdk.Coins, +) { + chainA := s.configurer.GetChainConfig(0) + n2, err := chainA.GetNodeAtIndex(2) + s.NoError(err) + + fp1RewardGaugePrev, fp2RewardGaugePrev, btcDel1RewardGaugePrev, btcDel2RewardGaugePrev := s.QueryRewardGauges(n2) + // wait a few block of rewards to calculate the difference + for i := 1; i <= int(blocksDiff); i++ { + if i%2 == 0 { + s.AddFinalityVoteUntilCurrentHeight() + } + n2.WaitForNextBlock() + } + + fp1RewardGauge, fp2RewardGauge, btcDel1RewardGauge, btcDel2RewardGauge := s.QueryRewardGauges(n2) + + // since varius block were created, it is needed to get the difference + // from a certain point where all the delegations were active to properly + // calculate the distribution with the voting power structure with 4 BTC delegations active + // Note: if a new block is mined during the query of reward gauges, the calculation might be a + // bit off by some ubbn + fp1DiffRewards = fp1RewardGauge.Coins.Sub(fp1RewardGaugePrev.Coins...) + fp2DiffRewards = fp2RewardGauge.Coins.Sub(fp2RewardGaugePrev.Coins...) + del1DiffRewards = btcDel1RewardGauge.Coins.Sub(btcDel1RewardGaugePrev.Coins...) + del2DiffRewards = btcDel2RewardGauge.Coins.Sub(btcDel2RewardGaugePrev.Coins...) + + s.AddFinalityVoteUntilCurrentHeight() + return fp1DiffRewards, fp2DiffRewards, del1DiffRewards, del2DiffRewards +} func (s *BtcRewardsDistribution) AddFinalityVoteUntilCurrentHeight() { chainA := s.configurer.GetChainConfig(0) diff --git a/testutil/btcstaking-helper/keeper.go b/testutil/btcstaking-helper/keeper.go index ebb1a5bef..f224c5e5c 100644 --- a/testutil/btcstaking-helper/keeper.go +++ b/testutil/btcstaking-helper/keeper.go @@ -230,7 +230,6 @@ func (h *Helper) CreateDelegation( r *rand.Rand, delSK *btcec.PrivateKey, fpPK *btcec.PublicKey, - changeAddress string, stakingValue int64, stakingTime uint16, unbondingValue int64, @@ -239,7 +238,7 @@ func (h *Helper) CreateDelegation( addToAllowList bool, ) (string, *types.MsgCreateBTCDelegation, *types.BTCDelegation, *btclctypes.BTCHeaderInfo, *types.InclusionProof, *UnbondingTxInfo, error) { return h.CreateDelegationWithBtcBlockHeight( - r, delSK, fpPK, changeAddress, stakingValue, + r, delSK, fpPK, stakingValue, stakingTime, unbondingValue, unbondingTime, usePreApproval, addToAllowList, 10, 10, ) @@ -249,7 +248,6 @@ func (h *Helper) CreateDelegationWithBtcBlockHeight( r *rand.Rand, delSK *btcec.PrivateKey, fpPK *btcec.PublicKey, - changeAddress string, stakingValue int64, stakingTime uint16, unbondingValue int64, diff --git a/testutil/incentives-helper/keeper.go b/testutil/incentives-helper/keeper.go index 9f2012e77..6334bfdad 100644 --- a/testutil/incentives-helper/keeper.go +++ b/testutil/incentives-helper/keeper.go @@ -69,9 +69,6 @@ func (h *IncentiveHelper) CreateBtcDelegation( ) ( stakingTxHash string, msgCreateBTCDel *bstypes.MsgCreateBTCDelegation, actualDel *bstypes.BTCDelegation, unbondingInfo *btcstkhelper.UnbondingTxInfo, ) { - changeAddress, err := datagen.GenRandomBTCAddress(r, h.Net) - h.NoError(err) - delSK, _, err := datagen.GenRandomBTCKeyPair(r) h.NoError(err) @@ -79,7 +76,6 @@ func (h *IncentiveHelper) CreateBtcDelegation( r, delSK, fpPK, - changeAddress.EncodeAddress(), stakingValue, stakingTime, 0, diff --git a/x/btcstaking/keeper/bench_test.go b/x/btcstaking/keeper/bench_test.go index a07083ad9..fe7496308 100644 --- a/x/btcstaking/keeper/bench_test.go +++ b/x/btcstaking/keeper/bench_test.go @@ -28,8 +28,6 @@ func benchBeginBlock(b *testing.B, numFPs int, numDelsUnderFP int) { h := testutil.NewHelper(b, btclcKeeper, btccKeeper) // set all parameters covenantSKs, _ := h.GenAndApplyParams(r) - changeAddress, err := datagen.GenRandomBTCAddress(r, h.Net) - h.NoError(err) // generate new finality providers fps := []*types.FinalityProvider{} @@ -60,7 +58,6 @@ func benchBeginBlock(b *testing.B, numFPs int, numDelsUnderFP int) { r, delSK, fp.BtcPk.MustToBTCPK(), - changeAddress.EncodeAddress(), stakingValue, 1000, 0, diff --git a/x/btcstaking/keeper/msg_server_test.go b/x/btcstaking/keeper/msg_server_test.go index 2ce6051a6..331a665f0 100644 --- a/x/btcstaking/keeper/msg_server_test.go +++ b/x/btcstaking/keeper/msg_server_test.go @@ -193,9 +193,6 @@ func FuzzCreateBTCDelegation(f *testing.F) { // set all parameters h.GenAndApplyParams(r) - changeAddress, err := datagen.GenRandomBTCAddress(r, h.Net) - require.NoError(t, err) - // generate and insert new finality provider _, fpPK, _ := h.CreateFinalityProvider(r) @@ -213,7 +210,6 @@ func FuzzCreateBTCDelegation(f *testing.F) { r, delSK, fpPK, - changeAddress.EncodeAddress(), stakingValue, 1000, 0, @@ -229,7 +225,6 @@ func FuzzCreateBTCDelegation(f *testing.F) { r, delSK, fpPK, - changeAddress.EncodeAddress(), stakingValue, 1000, 0, @@ -305,9 +300,6 @@ func FuzzCreateBTCDelegationWithParamsFromBtcHeight(f *testing.F) { require.Equal(t, version, expectedParamsVersion) // creates one BTC delegation with BTC block height between expectedParamsBlockHeight and 500 - changeAddress, err := datagen.GenRandomBTCAddress(r, h.Net) - h.NoError(err) - // generate and insert new finality provider _, fpPK, _ := h.CreateFinalityProvider(r) @@ -320,7 +312,6 @@ func FuzzCreateBTCDelegationWithParamsFromBtcHeight(f *testing.F) { r, delSK, fpPK, - changeAddress.EncodeAddress(), stakingValue, 1000, 0, @@ -348,9 +339,6 @@ func TestProperVersionInDelegation(t *testing.T) { // set all parameters h.GenAndApplyParams(r) - changeAddress, err := datagen.GenRandomBTCAddress(r, h.Net) - require.NoError(t, err) - // generate and insert new finality provider _, fpPK, _ := h.CreateFinalityProvider(r) @@ -362,7 +350,6 @@ func TestProperVersionInDelegation(t *testing.T) { r, delSK, fpPK, - changeAddress.EncodeAddress(), stakingValue, 1000, 0, @@ -396,7 +383,6 @@ func TestProperVersionInDelegation(t *testing.T) { r, delSK, fpPK, - changeAddress.EncodeAddress(), stakingValue, 10000, stakingValue-1000, @@ -428,9 +414,6 @@ func TestRejectActivationThatShouldNotUsePreApprovalFlow(t *testing.T) { // set all parameters covenantSKs, _ := h.GenAndApplyParams(r) - changeAddress, err := datagen.GenRandomBTCAddress(r, h.Net) - require.NoError(t, err) - // generate and insert new finality provider _, fpPK, _ := h.CreateFinalityProvider(r) @@ -439,7 +422,7 @@ func TestRejectActivationThatShouldNotUsePreApprovalFlow(t *testing.T) { // params will be activate at block height 2 currentParams.BtcActivationHeight++ // Update new params - err = h.BTCStakingKeeper.SetParams(h.Ctx, currentParams) + err := h.BTCStakingKeeper.SetParams(h.Ctx, currentParams) require.NoError(t, err) // generate and insert new BTC delegation @@ -450,7 +433,6 @@ func TestRejectActivationThatShouldNotUsePreApprovalFlow(t *testing.T) { r, delSK, fpPK, - changeAddress.EncodeAddress(), stakingValue, 1000, 0, @@ -520,9 +502,6 @@ func FuzzAddCovenantSigs(f *testing.F) { // set all parameters covenantSKs, _ := h.GenAndApplyParams(r) - changeAddress, err := datagen.GenRandomBTCAddress(r, h.Net) - require.NoError(t, err) - // generate and insert new finality provider _, fpPK, _ := h.CreateFinalityProvider(r) @@ -540,7 +519,6 @@ func FuzzAddCovenantSigs(f *testing.F) { r, delSK, fpPK, - changeAddress.EncodeAddress(), stakingValue, 1000, 0, @@ -610,8 +588,6 @@ func FuzzAddBTCDelegationInclusionProof(f *testing.F) { // set all parameters covenantSKs, _ := h.GenAndApplyParams(r) - changeAddress, err := datagen.GenRandomBTCAddress(r, h.Net) - require.NoError(t, err) // generate and insert new finality provider _, fpPK, _ := h.CreateFinalityProvider(r) @@ -624,7 +600,6 @@ func FuzzAddBTCDelegationInclusionProof(f *testing.F) { r, delSK, fpPK, - changeAddress.EncodeAddress(), stakingValue, 1000, 0, @@ -685,9 +660,6 @@ func FuzzBTCUndelegate(f *testing.F) { bsParams := h.BTCStakingKeeper.GetParams(h.Ctx) - changeAddress, err := datagen.GenRandomBTCAddress(r, h.Net) - require.NoError(t, err) - // generate and insert new finality provider _, fpPK, _ := h.CreateFinalityProvider(r) @@ -699,7 +671,6 @@ func FuzzBTCUndelegate(f *testing.F) { r, delSK, fpPK, - changeAddress.EncodeAddress(), stakingValue, 1000, 0, @@ -767,9 +738,6 @@ func FuzzBTCUndelegateExpired(f *testing.F) { bsParams := h.BTCStakingKeeper.GetParams(h.Ctx) - changeAddress, err := datagen.GenRandomBTCAddress(r, h.Net) - require.NoError(t, err) - // generate and insert new finality provider _, fpPK, _ := h.CreateFinalityProvider(r) @@ -781,7 +749,6 @@ func FuzzBTCUndelegateExpired(f *testing.F) { r, delSK, fpPK, - changeAddress.EncodeAddress(), stakingValue, 1000, 0, @@ -836,9 +803,6 @@ func FuzzSelectiveSlashing(f *testing.F) { covenantSKs, _ := h.GenAndApplyParams(r) bsParams := h.BTCStakingKeeper.GetParams(h.Ctx) - changeAddress, err := datagen.GenRandomBTCAddress(r, h.Net) - require.NoError(t, err) - // generate and insert new finality provider fpSK, fpPK, _ := h.CreateFinalityProvider(r) fpBtcPk := bbn.NewBIP340PubKeyFromBTCPK(fpPK) @@ -851,7 +815,6 @@ func FuzzSelectiveSlashing(f *testing.F) { r, delSK, fpPK, - changeAddress.EncodeAddress(), stakingValue, 1000, 0, @@ -916,9 +879,6 @@ func FuzzSelectiveSlashing_StakingTx(f *testing.F) { covenantSKs, _ := h.GenAndApplyParams(r) bsParams := h.BTCStakingKeeper.GetParams(h.Ctx) - changeAddress, err := datagen.GenRandomBTCAddress(r, h.Net) - require.NoError(t, err) - // generate and insert new finality provider fpSK, fpPK, _ := h.CreateFinalityProvider(r) fpBtcPk := bbn.NewBIP340PubKeyFromBTCPK(fpPK) @@ -931,7 +891,6 @@ func FuzzSelectiveSlashing_StakingTx(f *testing.F) { r, delSK, fpPK, - changeAddress.EncodeAddress(), stakingValue, 1000, 0, @@ -1166,9 +1125,6 @@ func TestCorrectUnbondingTimeInDelegation(t *testing.T) { // set all parameters _, _ = h.GenAndApplyCustomParams(r, tt.finalizationTimeout, tt.unbondingTimeInParams, 0) - changeAddress, err := datagen.GenRandomBTCAddress(r, h.Net) - require.NoError(t, err) - // generate and insert new finality provider _, fpPK, _ := h.CreateFinalityProvider(r) @@ -1180,7 +1136,6 @@ func TestCorrectUnbondingTimeInDelegation(t *testing.T) { r, delSK, fpPK, - changeAddress.EncodeAddress(), stakingValue, 1000, stakingValue-1000, @@ -1216,9 +1171,6 @@ func TestAllowList(t *testing.T) { // set all parameters, use the allow list h.GenAndApplyCustomParams(r, 100, 0, allowListExpirationHeight) - changeAddress, err := datagen.GenRandomBTCAddress(r, h.Net) - require.NoError(t, err) - // generate and insert new finality provider _, fpPK, _ := h.CreateFinalityProvider(r) @@ -1232,7 +1184,6 @@ func TestAllowList(t *testing.T) { r, delSK, fpPK, - changeAddress.EncodeAddress(), stakingValue, 1000, 0, @@ -1252,7 +1203,6 @@ func TestAllowList(t *testing.T) { r, delSK1, fpPK, - changeAddress.EncodeAddress(), stakingValue, 1000, 0, @@ -1275,7 +1225,6 @@ func TestAllowList(t *testing.T) { r, delSK2, fpPK, - changeAddress.EncodeAddress(), stakingValue, 1000, 0, diff --git a/x/finality/keeper/liveness_test.go b/x/finality/keeper/liveness_test.go index c4e01c70c..2ebed01f5 100644 --- a/x/finality/keeper/liveness_test.go +++ b/x/finality/keeper/liveness_test.go @@ -103,8 +103,6 @@ func FuzzHandleLivenessDeterminism(f *testing.F) { params := h1.BTCStakingKeeper.GetParams(h1.Ctx) err := h2.BTCStakingKeeper.SetParams(h2.Ctx, params) require.NoError(t, err) - changeAddress, err := datagen.GenRandomBTCAddress(r, h1.Net) - require.NoError(t, err) // Generate multiple finality providers numFPs := 5 // Can be adjusted or randomized @@ -125,7 +123,6 @@ func FuzzHandleLivenessDeterminism(f *testing.F) { r, delSK, fpPK, - changeAddress.EncodeAddress(), stakingValue, 1000, 0, @@ -140,7 +137,6 @@ func FuzzHandleLivenessDeterminism(f *testing.F) { r, delSK, fpPK, - changeAddress.EncodeAddress(), stakingValue, 1000, 0, diff --git a/x/finality/keeper/power_dist_change.go b/x/finality/keeper/power_dist_change.go index 581d415a0..75e602163 100644 --- a/x/finality/keeper/power_dist_change.go +++ b/x/finality/keeper/power_dist_change.go @@ -233,10 +233,13 @@ func (k Keeper) ProcessAllPowerDistUpdateEvents( types.EmitSlashedFPEvent(sdkCtx, typedEvent.SlashedFp.Pk) fpBTCPKHex := typedEvent.SlashedFp.Pk.MarshalHex() slashedFPs[fpBTCPKHex] = struct{}{} - fp := k.loadFP(ctx, fpByBtcPkHex, fpBTCPKHex) - if err := k.IncentiveKeeper.FpSlashed(ctx, fp.Address()); err != nil { - panic(err) - } + // TODO(rafilx): handle slashed fps prunning + // It is not possible to slash fp and delete all of his data at the + // babylon block height that is being processed, because + // the function RewardBTCStaking is called a few blocks behind. + // If the data is deleted at the slash event, when slashed fps are + // receveing rewards from a few blocks behind HandleRewarding + // verifies the next block height to be rewarded. case *types.EventPowerDistUpdate_JailedFp: // record jailed fps types.EmitJailedFPEvent(sdkCtx, typedEvent.JailedFp.Pk) @@ -320,6 +323,11 @@ func (k Keeper) ProcessAllPowerDistUpdateEvents( // sort new finality providers in activeBTCDels to ensure determinism fpActiveBtcPkHexList := make([]string, 0, len(activedSatsByFpBtcPk)) for fpBTCPKHex := range activedSatsByFpBtcPk { + // if the fp was slashed, should not even be added to the list + _, isSlashed := slashedFPs[fpBTCPKHex] + if isSlashed { + continue + } fpActiveBtcPkHexList = append(fpActiveBtcPkHexList, fpBTCPKHex) } sort.SliceStable(fpActiveBtcPkHexList, func(i, j int) bool { @@ -332,6 +340,14 @@ func (k Keeper) ProcessAllPowerDistUpdateEvents( newFP := k.loadFP(ctx, fpByBtcPkHex, fpBTCPKHex) fpDistInfo := ftypes.NewFinalityProviderDistInfo(newFP) + // check for jailing cases + if _, ok := jailedFPs[fpBTCPKHex]; ok { + fpDistInfo.IsJailed = true + } + if _, ok := unjailedFPs[fpBTCPKHex]; ok { + fpDistInfo.IsJailed = false + } + // add each BTC delegation fpActiveSats := activedSatsByFpBtcPk[fpBTCPKHex] for _, activatedSats := range fpActiveSats { diff --git a/x/finality/keeper/power_dist_change_test.go b/x/finality/keeper/power_dist_change_test.go index fa5a81c40..a5c2d4a27 100644 --- a/x/finality/keeper/power_dist_change_test.go +++ b/x/finality/keeper/power_dist_change_test.go @@ -6,6 +6,7 @@ import ( "time" "github.com/btcsuite/btcd/btcec/v2" + "github.com/decred/dcrd/dcrec/secp256k1/v4" "github.com/golang/mock/gomock" "github.com/stretchr/testify/require" @@ -31,8 +32,6 @@ func FuzzProcessAllPowerDistUpdateEvents_Determinism(f *testing.F) { // set all parameters h.GenAndApplyParams(r) - changeAddress, err := datagen.GenRandomBTCAddress(r, h.Net) - require.NoError(t, err) // generate and insert a number of new finality providers fpPKs := []*btcec.PublicKey{} @@ -56,7 +55,6 @@ func FuzzProcessAllPowerDistUpdateEvents_Determinism(f *testing.F) { r, delSK, fpPK, - changeAddress.EncodeAddress(), stakingValue, 1000, 0, @@ -83,59 +81,226 @@ func FuzzProcessAllPowerDistUpdateEvents_Determinism(f *testing.F) { }) } +func CreateFpAndBtcDel( + t *testing.T, + r *rand.Rand, +) ( + h *testutil.Helper, + del *types.BTCDelegation, + covenantSKs []*secp256k1.PrivateKey, +) { + 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 + covenantSKs, _ = h.GenAndApplyParams(r) + + _, fpPK, _ := h.CreateFinalityProvider(r) + + delSK, _, err := datagen.GenRandomBTCKeyPair(r) + h.NoError(err) + _, _, del, _, _, _, err = h.CreateDelegationWithBtcBlockHeight( + r, + delSK, + fpPK, + int64(2*10e8), + 1000, + 0, + 0, + false, + false, + 10, + 30, + ) + h.NoError(err) + return h, del, covenantSKs +} + func FuzzProcessAllPowerDistUpdateEvents_ActiveAndUnbondTogether(f *testing.F) { datagen.AddRandomSeedsToFuzzer(f, 10) f.Fuzz(func(t *testing.T, seed int64) { r := rand.New(rand.NewSource(seed)) - ctrl := gomock.NewController(t) - defer ctrl.Finish() + h, del, _ := CreateFpAndBtcDel(t, r) - // mock BTC light client and BTC checkpoint modules - btclcKeeper := types.NewMockBTCLightClientKeeper(ctrl) - btccKeeper := types.NewMockBtcCheckpointKeeper(ctrl) - h := testutil.NewHelper(t, btclcKeeper, btccKeeper) + eventActive := types.NewEventPowerDistUpdateWithBTCDel(&types.EventBTCDelegationStateUpdate{ + StakingTxHash: del.MustGetStakingTxHash().String(), + NewState: types.BTCDelegationStatus_ACTIVE, + }) + eventUnbond := types.NewEventPowerDistUpdateWithBTCDel(&types.EventBTCDelegationStateUpdate{ + StakingTxHash: del.MustGetStakingTxHash().String(), + NewState: types.BTCDelegationStatus_UNBONDED, + }) + events := []*types.EventPowerDistUpdate{eventActive, eventUnbond} - // set all parameters - h.GenAndApplyParams(r) - changeAddress, err := datagen.GenRandomBTCAddress(r, h.Net) - require.NoError(t, err) + newDc := h.FinalityKeeper.ProcessAllPowerDistUpdateEvents(h.Ctx, ftypes.NewVotingPowerDistCache(), events) + require.Len(t, newDc.FinalityProviders, 0) + }) +} + +func FuzzProcessAllPowerDistUpdateEvents_ActiveAndSlashTogether(f *testing.F) { + datagen.AddRandomSeedsToFuzzer(f, 10) + + f.Fuzz(func(t *testing.T, seed int64) { + r := rand.New(rand.NewSource(seed)) + h, del, _ := CreateFpAndBtcDel(t, r) + + eventActive := types.NewEventPowerDistUpdateWithBTCDel(&types.EventBTCDelegationStateUpdate{ + StakingTxHash: del.MustGetStakingTxHash().String(), + NewState: types.BTCDelegationStatus_ACTIVE, + }) + eventSlash := types.NewEventPowerDistUpdateWithSlashedFP(&del.FpBtcPkList[0]) + events := []*types.EventPowerDistUpdate{eventActive, eventSlash} - // empty dist cache dc := ftypes.NewVotingPowerDistCache() + newDc := h.FinalityKeeper.ProcessAllPowerDistUpdateEvents(h.Ctx, dc, events) + require.Len(t, newDc.FinalityProviders, 0) + }) +} - _, fpPK, _ := h.CreateFinalityProvider(r) +func FuzzProcessAllPowerDistUpdateEvents_PreApprovalWithSlahedFP(f *testing.F) { + datagen.AddRandomSeedsToFuzzer(f, 10) - delSK, _, err := datagen.GenRandomBTCKeyPair(r) + f.Fuzz(func(t *testing.T, seed int64) { + r := rand.New(rand.NewSource(seed)) + h, delNoPreApproval, covenantSKs := CreateFpAndBtcDel(t, r) + + // activates one delegation to the finality provider without preapproval + eventActive := types.NewEventPowerDistUpdateWithBTCDel(&types.EventBTCDelegationStateUpdate{ + StakingTxHash: delNoPreApproval.MustGetStakingTxHash().String(), + NewState: types.BTCDelegationStatus_ACTIVE, + }) + + newDc := h.FinalityKeeper.ProcessAllPowerDistUpdateEvents(h.Ctx, ftypes.NewVotingPowerDistCache(), []*types.EventPowerDistUpdate{eventActive}) + // updates as if that fp is timestamping + for _, fp := range newDc.FinalityProviders { + fp.IsTimestamped = true + } + // FP is active and has voting power. + newDc.ApplyActiveFinalityProviders(100) + require.Len(t, newDc.FinalityProviders, 1) + require.Equal(t, newDc.TotalVotingPower, delNoPreApproval.TotalSat) + + // simulating a new BTC delegation with preapproval coming + delSKPreApproval, _, err := datagen.GenRandomBTCKeyPair(r) h.NoError(err) - _, _, del, _, _, _, err := h.CreateDelegationWithBtcBlockHeight( + + stakingTxHash, msgCreateBTCDelPreApproval, delPreApproval, btcHeaderInfo, inclusionProof, _, err := h.CreateDelegationWithBtcBlockHeight( r, - delSK, - fpPK, - changeAddress.EncodeAddress(), + delSKPreApproval, + delNoPreApproval.FpBtcPkList[0].MustToBTCPK(), int64(2*10e8), 1000, 0, 0, - false, + true, false, 10, - 30, + 10, ) h.NoError(err) + // should not modify the amount of voting power + newDc.ApplyActiveFinalityProviders(100) + require.Len(t, newDc.FinalityProviders, 1) + require.Equal(t, newDc.TotalVotingPower, delPreApproval.TotalSat) + + // slash the fp + slashEvent := types.NewEventPowerDistUpdateWithSlashedFP(&delPreApproval.FpBtcPkList[0]) + newDc = h.FinalityKeeper.ProcessAllPowerDistUpdateEvents(h.Ctx, newDc, []*types.EventPowerDistUpdate{slashEvent}) + + // fp should have be erased from the list + newDc.ApplyActiveFinalityProviders(100) + require.Len(t, newDc.FinalityProviders, 0) + require.Zero(t, newDc.TotalVotingPower) + + // activates the preapproval delegation + btcTip := btclctypes.BTCHeaderInfo{Height: 30} + + h.CreateCovenantSigs(r, covenantSKs, msgCreateBTCDelPreApproval, delPreApproval, btcTip.Height) + h.AddInclusionProof(stakingTxHash, btcHeaderInfo, inclusionProof, btcTip.Height) + + activatedDel, err := h.BTCStakingKeeper.GetBTCDelegation(h.Ctx, stakingTxHash) + h.NoError(err) + h.Equal(activatedDel.TotalSat, uint64(msgCreateBTCDelPreApproval.StakingValue)) + + // simulates the del tx getting activated + eventActive = types.NewEventPowerDistUpdateWithBTCDel(&types.EventBTCDelegationStateUpdate{ + StakingTxHash: delPreApproval.MustGetStakingTxHash().String(), + NewState: types.BTCDelegationStatus_ACTIVE, + }) + // it will get included in the new vp dist, but will not have voting power after ApplyActiveFinalityProviders + newDc = h.FinalityKeeper.ProcessAllPowerDistUpdateEvents(h.Ctx, newDc, []*types.EventPowerDistUpdate{eventActive}) + require.Len(t, newDc.FinalityProviders, 1) + + for _, fp := range newDc.FinalityProviders { + fp.IsTimestamped = true + fp.IsSlashed = true + } + newDc.ApplyActiveFinalityProviders(100) + require.Equal(t, newDc.TotalVotingPower, uint64(0)) + require.Equal(t, newDc.NumActiveFps, uint32(0)) + }) +} + +func FuzzProcessAllPowerDistUpdateEvents_ActiveAndJailTogether(f *testing.F) { + datagen.AddRandomSeedsToFuzzer(f, 10) + + f.Fuzz(func(t *testing.T, seed int64) { + r := rand.New(rand.NewSource(seed)) + h, del, _ := CreateFpAndBtcDel(t, r) + eventActive := types.NewEventPowerDistUpdateWithBTCDel(&types.EventBTCDelegationStateUpdate{ StakingTxHash: del.MustGetStakingTxHash().String(), NewState: types.BTCDelegationStatus_ACTIVE, }) - eventUnbond := types.NewEventPowerDistUpdateWithBTCDel(&types.EventBTCDelegationStateUpdate{ + eventJailed := types.NewEventPowerDistUpdateWithJailedFP(&del.FpBtcPkList[0]) + events := []*types.EventPowerDistUpdate{eventActive, eventJailed} + + newDc := h.FinalityKeeper.ProcessAllPowerDistUpdateEvents(h.Ctx, ftypes.NewVotingPowerDistCache(), events) + for _, fp := range newDc.FinalityProviders { + fp.IsTimestamped = true + } + newDc.ApplyActiveFinalityProviders(100) + require.Len(t, newDc.FinalityProviders, 1) + require.Zero(t, newDc.TotalVotingPower) + }) +} + +func FuzzProcessAllPowerDistUpdateEvents_SlashActiveFp(f *testing.F) { + datagen.AddRandomSeedsToFuzzer(f, 10) + + f.Fuzz(func(t *testing.T, seed int64) { + t.Parallel() + r := rand.New(rand.NewSource(seed)) + h, del, _ := CreateFpAndBtcDel(t, r) + + eventActive := types.NewEventPowerDistUpdateWithBTCDel(&types.EventBTCDelegationStateUpdate{ StakingTxHash: del.MustGetStakingTxHash().String(), - NewState: types.BTCDelegationStatus_UNBONDED, + NewState: types.BTCDelegationStatus_ACTIVE, }) - events := []*types.EventPowerDistUpdate{eventActive, eventUnbond} + events := []*types.EventPowerDistUpdate{eventActive} - newDc := h.FinalityKeeper.ProcessAllPowerDistUpdateEvents(h.Ctx, dc, events) - require.Zero(t, newDc.TotalVotingPower) + newDc := h.FinalityKeeper.ProcessAllPowerDistUpdateEvents(h.Ctx, ftypes.NewVotingPowerDistCache(), events) + for _, fp := range newDc.FinalityProviders { + fp.IsTimestamped = true + } + newDc.ApplyActiveFinalityProviders(100) + require.Equal(t, newDc.TotalVotingPower, del.TotalSat) + + // afer the fp has some active voting power slash it + eventSlash := types.NewEventPowerDistUpdateWithSlashedFP(&del.FpBtcPkList[0]) + events = []*types.EventPowerDistUpdate{eventSlash} + + newDc = h.FinalityKeeper.ProcessAllPowerDistUpdateEvents(h.Ctx, newDc, events) + newDc.ApplyActiveFinalityProviders(100) + require.Len(t, newDc.FinalityProviders, 0) + require.Equal(t, newDc.TotalVotingPower, uint64(0)) }) } @@ -154,8 +319,6 @@ func FuzzSlashFinalityProviderEvent(f *testing.F) { // set all parameters covenantSKs, _ := h.GenAndApplyParams(r) - changeAddress, err := datagen.GenRandomBTCAddress(r, h.Net) - require.NoError(t, err) // generate and insert new finality provider fpSK, fpPK, fp := h.CreateFinalityProvider(r) @@ -172,7 +335,6 @@ func FuzzSlashFinalityProviderEvent(f *testing.F) { r, delSK, fpPK, - changeAddress.EncodeAddress(), stakingValue, 1000, 0, @@ -243,8 +405,6 @@ func FuzzJailFinalityProviderEvents(f *testing.F) { // set all parameters covenantSKs, _ := h.GenAndApplyParams(r) - changeAddress, err := datagen.GenRandomBTCAddress(r, h.Net) - require.NoError(t, err) // generate and insert new finality provider fpSK, fpPK, fp := h.CreateFinalityProvider(r) @@ -261,7 +421,6 @@ func FuzzJailFinalityProviderEvents(f *testing.F) { r, delSK, fpPK, - changeAddress.EncodeAddress(), stakingValue, 1000, 0, @@ -334,7 +493,6 @@ func FuzzJailFinalityProviderEvents(f *testing.F) { r, delSK2, fpPK, - changeAddress.EncodeAddress(), stakingValue, 1000, 0, @@ -380,8 +538,6 @@ func FuzzUnjailFinalityProviderEvents(f *testing.F) { // set all parameters covenantSKs, _ := h.GenAndApplyParams(r) - changeAddress, err := datagen.GenRandomBTCAddress(r, h.Net) - require.NoError(t, err) // generate and insert new finality provider fpSK, fpPK, fp := h.CreateFinalityProvider(r) @@ -398,7 +554,6 @@ func FuzzUnjailFinalityProviderEvents(f *testing.F) { r, delSK, fpPK, - changeAddress.EncodeAddress(), stakingValue, 1000, 0, @@ -489,8 +644,6 @@ func FuzzBTCDelegationEvents_NoPreApproval(f *testing.F) { // set all parameters covenantSKs, _ := h.GenAndApplyParams(r) - changeAddress, err := datagen.GenRandomBTCAddress(r, h.Net) - require.NoError(t, err) // generate and insert new finality provider fpSK, fpPK, fp := h.CreateFinalityProvider(r) @@ -505,7 +658,6 @@ func FuzzBTCDelegationEvents_NoPreApproval(f *testing.F) { r, delSK, fpPK, - changeAddress.EncodeAddress(), stakingValue, 1000, 0, @@ -607,8 +759,6 @@ func FuzzBTCDelegationEvents_WithPreApproval(f *testing.F) { // set all parameters covenantSKs, _ := h.GenAndApplyParams(r) - changeAddress, err := datagen.GenRandomBTCAddress(r, h.Net) - require.NoError(t, err) // generate and insert new finality provider fpSK, fpPK, fp := h.CreateFinalityProvider(r) @@ -621,7 +771,6 @@ func FuzzBTCDelegationEvents_WithPreApproval(f *testing.F) { r, delSK, fpPK, - changeAddress.EncodeAddress(), stakingValue, 1000, 0, @@ -733,8 +882,6 @@ func TestDoNotGenerateDuplicateEventsAfterHavingCovenantQuorum(t *testing.T) { // set all parameters covenantSKs, _ := h.GenAndApplyParams(r) - changeAddress, err := datagen.GenRandomBTCAddress(r, h.Net) - require.NoError(t, err) // generate and insert new finality provider _, fpPK, fp := h.CreateFinalityProvider(r) @@ -748,7 +895,6 @@ func TestDoNotGenerateDuplicateEventsAfterHavingCovenantQuorum(t *testing.T) { r, delSK, fpPK, - changeAddress.EncodeAddress(), stakingValue, 1000, 0, diff --git a/x/finality/keeper/power_table_test.go b/x/finality/keeper/power_table_test.go index 2b2d14520..bde87933a 100644 --- a/x/finality/keeper/power_table_test.go +++ b/x/finality/keeper/power_table_test.go @@ -32,8 +32,6 @@ func FuzzVotingPowerTable(f *testing.F) { // set all parameters covenantSKs, _ := h.GenAndApplyParams(r) - changeAddress, err := datagen.GenRandomBTCAddress(r, h.Net) - h.NoError(err) // generate a random batch of finality providers, and commit pub rand list with timestamp fps := []*types.FinalityProvider{} @@ -56,7 +54,6 @@ func FuzzVotingPowerTable(f *testing.F) { r, delSK, fps[i].BtcPk.MustToBTCPK(), - changeAddress.EncodeAddress(), int64(stakingValue), 1000, 0, @@ -78,7 +75,7 @@ func FuzzVotingPowerTable(f *testing.F) { babylonHeight := datagen.RandomInt(r, 10) + 1 h.SetCtxHeight(babylonHeight) h.BTCLightClientKeeper.EXPECT().GetTipInfo(gomock.Eq(h.Ctx)).Return(&btclctypes.BTCHeaderInfo{Height: 30}).AnyTimes() - err = h.BTCStakingKeeper.BeginBlocker(h.Ctx) + err := h.BTCStakingKeeper.BeginBlocker(h.Ctx) require.NoError(t, err) err = h.FinalityKeeper.BeginBlocker(h.Ctx) require.NoError(t, err) @@ -185,8 +182,6 @@ func FuzzRecordVotingPowerDistCache(f *testing.F) { // set all parameters covenantSKs, _ := h.GenAndApplyParams(r) - changeAddress, err := datagen.GenRandomBTCAddress(r, h.Net) - h.NoError(err) // generate a random batch of finality providers, and commit // pub rand list with timestamp @@ -214,7 +209,6 @@ func FuzzRecordVotingPowerDistCache(f *testing.F) { r, delSK, fp.BtcPk.MustToBTCPK(), - changeAddress.EncodeAddress(), int64(stakingValue), 1000, 0, @@ -265,8 +259,6 @@ func FuzzVotingPowerTable_ActiveFinalityProviders(f *testing.F) { // set all parameters covenantSKs, _ := h.GenAndApplyParams(r) - changeAddress, err := datagen.GenRandomBTCAddress(r, h.Net) - h.NoError(err) // generate a random batch of finality providers, each with a BTC delegation with random power fpsWithMeta := []*ftypes.FinalityProviderDistInfo{} @@ -284,7 +276,6 @@ func FuzzVotingPowerTable_ActiveFinalityProviders(f *testing.F) { r, delSK, fp.BtcPk.MustToBTCPK(), - changeAddress.EncodeAddress(), int64(stakingValue), 1000, 0, @@ -376,9 +367,6 @@ func FuzzVotingPowerTable_ActiveFinalityProviderRotation(f *testing.F) { fParams.MaxActiveFinalityProviders = uint32(datagen.RandomInt(r, 20) + 10) err := h.FinalityKeeper.SetParams(h.Ctx, fParams) h.NoError(err) - // change address - changeAddress, err := datagen.GenRandomBTCAddress(r, h.Net) - h.NoError(err) numFps := datagen.RandomInt(r, 20) + 10 numActiveFPs := int(min(numFps, uint64(fParams.MaxActiveFinalityProviders))) @@ -403,7 +391,6 @@ func FuzzVotingPowerTable_ActiveFinalityProviderRotation(f *testing.F) { r, delSK, fpPK, - changeAddress.EncodeAddress(), int64(stakingValue), 1000, 0, @@ -462,7 +449,6 @@ func FuzzVotingPowerTable_ActiveFinalityProviderRotation(f *testing.F) { r, delSK, fpBTCPK.MustToBTCPK(), - changeAddress.EncodeAddress(), int64(stakingValue), 1000, 0, @@ -499,7 +485,6 @@ func FuzzVotingPowerTable_ActiveFinalityProviderRotation(f *testing.F) { r, delSK, fpPK, - changeAddress.EncodeAddress(), int64(stakingValue), 1000, 0, diff --git a/x/finality/types/expected_keepers.go b/x/finality/types/expected_keepers.go index bc7eebea3..6c9d9da4d 100644 --- a/x/finality/types/expected_keepers.go +++ b/x/finality/types/expected_keepers.go @@ -37,5 +37,4 @@ type IncentiveKeeper interface { IndexRefundableMsg(ctx context.Context, msg sdk.Msg) BtcDelegationActivated(ctx context.Context, fp, del sdk.AccAddress, sat uint64) error BtcDelegationUnbonded(ctx context.Context, fp, del sdk.AccAddress, sat uint64) error - FpSlashed(ctx context.Context, fp sdk.AccAddress) error } diff --git a/x/finality/types/power_table.go b/x/finality/types/power_table.go index 6cd20bcef..cab2f559f 100644 --- a/x/finality/types/power_table.go +++ b/x/finality/types/power_table.go @@ -87,6 +87,10 @@ func (dc *VotingPowerDistCache) ApplyActiveFinalityProviders(maxActiveFPs uint32 if fp.IsJailed { break } + if fp.IsSlashed { + break + } + numActiveFPs++ } @@ -170,8 +174,8 @@ func (v *FinalityProviderDistInfo) GetBTCDelPortion(totalSatDelegation uint64) s // 2. IsJailed is true func SortFinalityProvidersWithZeroedVotingPower(fps []*FinalityProviderDistInfo) { sort.SliceStable(fps, func(i, j int) bool { - iShouldBeZeroed := fps[i].IsJailed || !fps[i].IsTimestamped - jShouldBeZeroed := fps[j].IsJailed || !fps[j].IsTimestamped + iShouldBeZeroed := fps[i].IsJailed || !fps[i].IsTimestamped || fps[i].IsSlashed + jShouldBeZeroed := fps[j].IsJailed || !fps[j].IsTimestamped || fps[j].IsSlashed if iShouldBeZeroed && !jShouldBeZeroed { return false diff --git a/x/incentive/keeper/btc_staking_gauge.go b/x/incentive/keeper/btc_staking_gauge.go index e47b6dca8..837903407 100644 --- a/x/incentive/keeper/btc_staking_gauge.go +++ b/x/incentive/keeper/btc_staking_gauge.go @@ -2,6 +2,7 @@ package keeper import ( "context" + "fmt" sdkmath "cosmossdk.io/math" "cosmossdk.io/store/prefix" @@ -55,7 +56,7 @@ func (k Keeper) RewardBTCStaking(ctx context.Context, height uint64, dc *ftypes. // reward the rest of coins to each BTC delegation proportional to its voting power portion coinsForBTCDels := coinsForFpsAndDels.Sub(coinsForCommission...) if err := k.AddFinalityProviderRewardsForBtcDelegations(ctx, fp.GetAddress(), coinsForBTCDels); err != nil { - panic(err) + panic(fmt.Errorf("failed to add fp rewards for btc delegation %s at height %d: %w", fp.GetAddress().String(), height, err)) } } // TODO: prune unnecessary state (delete BTCStakingGauge after the amount is used) diff --git a/x/incentive/keeper/reward_tracker_test.go b/x/incentive/keeper/reward_tracker_test.go index d81a911df..90e43a2d1 100644 --- a/x/incentive/keeper/reward_tracker_test.go +++ b/x/incentive/keeper/reward_tracker_test.go @@ -64,7 +64,7 @@ func FuzzCheckFpSlashed(f *testing.F) { del2RwdGauge := k.GetRewardGauge(ctx, types.BTCDelegationType, del2) coins.RequireCoinsDiffInPointOnePercentMargin(t, del2Fp1Rwds, del2RwdGauge.Coins) - // verifies that everything was deleted + // verifies that everything was deleted for fp1 _, err = k.GetBTCDelegationRewardsTracker(ctx, fp1, del1) require.EqualError(t, err, types.ErrBTCDelegationRewardsTrackerNotFound.Error()) _, err = k.GetBTCDelegationRewardsTracker(ctx, fp1, del2) @@ -74,6 +74,12 @@ func FuzzCheckFpSlashed(f *testing.F) { _, err = k.GetFinalityProviderHistoricalRewards(ctx, fp1, 1) require.EqualError(t, err, types.ErrFPHistoricalRewardsNotFound.Error()) + // verifies that for fp2 is all there + _, err = k.GetFinalityProviderCurrentRewards(ctx, fp2) + require.NoError(t, err) + _, err = k.GetFinalityProviderHistoricalRewards(ctx, fp2, 1) + require.NoError(t, err) + count := 0 err = k.iterBtcDelegationsByDelegator(ctx, del1, func(del, fp sdk.AccAddress) error { count++