053.md

Brilliant Umber Eagle

Medium

Jailed validators can lose reward for earlier block

Summary

Jailed validators can lose eariler accumulated rewards as well because incorrect reward distribution mechanism.

Root Cause

Current reward accumulation from block begin, code flow: incentive/abci.go#BeginBlocker -> intercept_fee_collector.go#HandleCoinsInFeeCollector -> btc_staking_gauge#accumulateBTCStakingReward -> SetBTCStakingGauge, so it accumulate the previous block rewards then store to StakingGaugeStore, also can see here.

Also the finality module will update all active/jailed/slashed fps in the block begin, the dc update code flow: finality/keeper.go#BeginBlocker -> power_dist_change.go#UpdatePowerDist -> power_dist_change.go#recordVotingPowerAndCache -> power_table.go#ApplyActiveFinalityProviders, the dc will sort all the fps in desc order by vote power and all the jailed fps vote power would be treated as zero:

// SortFinalityProvidersWithZeroedVotingPower sorts the finality providers slice,
// from higher to lower voting power. In the following cases, the voting power
// is treated as zero:
// 1. IsTimestamped is false
// 2. IsJailed is true
func SortFinalityProvidersWithZeroedVotingPower(fps []*FinalityProviderDistInfo) {
	sort.SliceStable(fps, func(i, j int) bool {
		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
		}

		if !iShouldBeZeroed && jShouldBeZeroed {
			return true
		}

		iPkHex, jPkHex := fps[i].BtcPk.MarshalHex(), fps[j].BtcPk.MarshalHex()

		if iShouldBeZeroed && jShouldBeZeroed {
			// Both have zeroed voting power, compare BTC public keys
			return iPkHex < jPkHex
		}

		// both voting power the same, compare BTC public keys
		if fps[i].TotalBondedSat == fps[j].TotalBondedSat {
			return iPkHex < jPkHex
		}

		return fps[i].TotalBondedSat > fps[j].TotalBondedSat
	})
}

then the dc.NumActiveFps will count all non-zero vote power fps so the jailed fps can be excluded from active fps:

func (dc *VotingPowerDistCache) ApplyActiveFinalityProviders(maxActiveFPs uint32) {
    for _, fp := range dc.FinalityProviders {
        if numActiveFPs == maxActiveFPs {
            break
        }
        if fp.TotalBondedSat == 0 {
            break
        }
        if !fp.IsTimestamped {
            break
        }
        if fp.IsJailed {
            break
        }
        if fp.IsSlashed {
            break
        }

        numActiveFPs++
    }

    totalVotingPower := uint64(0)

    for i := uint32(0); i < numActiveFPs; i++ {
        totalVotingPower += dc.FinalityProviders[i].TotalBondedSat
    }

    dc.TotalVotingPower = totalVotingPower
    dc.NumActiveFps = numActiveFPs
}

It then distribute the rewards based on the dc.NumActiveFps and execute during block end, code flow: finality/abci.go#EndBlocker -> rewarding.go#HandleRewarding -> rewardBTCStaking -> btc_staking_gauge.go#RewardBTCStaking:

func (k Keeper) RewardBTCStaking(ctx context.Context, height uint64, dc *ftypes.VotingPowerDistCache, voters map[string]struct{}) {
    ...
    for i, fp := range dc.FinalityProviders {
        if i >= int(dc.NumActiveFps) {
            break
        }

        if _, ok := voters[fp.BtcPk.MarshalHex()]; !ok {
            continue
        }

        // calculate the portion of a finality provider's voting power out of the total voting power of the voters
        fpPortion := sdkmath.LegacyNewDec(int64(fp.TotalBondedSat)).
            QuoTruncate(sdkmath.LegacyNewDec(int64(totalVotingPowerOfVoters)))
        coinsForFpsAndDels := gauge.GetCoinsPortion(fpPortion)

        // reward the finality provider with commission
        coinsForCommission := types.GetCoinsPortion(coinsForFpsAndDels, *fp.Commission)
        k.accumulateRewardGauge(ctx, types.FinalityProviderType, fp.GetAddress(), coinsForCommission)

        // 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(fmt.Errorf("failed to add fp rewards for btc delegation %s at height %d: %w", fp.GetAddress().String(), height, err))
        }
    }
}

So only active fps at current block can get rewards and jailed fps will lose the rewards, but it's unfair to the new jailed fps and its delegators:

1. FP-01 works as normal at block N-1.
2. The fp get jailed at block N before/during BeginBlocker so it's excluded from the active fps.
3. The block N-1 rewards are distributed during EndBlocker only to normal fps, so the jailed fps will lose the block N-1 rewards.

Internal Pre-conditions

None.

External Pre-conditions

None.

Attack Path

Impact

Jailed fps can lose the previous block rewards, which is unfair to the new jailed fps and its delegators.

PoC

Mitigation

Restribute the previous block rewards even if the fp is jailed at block N.