Skip to content

Commit

Permalink
chore: Remove last processed height (#167)
Browse files Browse the repository at this point in the history
The `lastProcessedHeight` was introduced for fast catch up blocks for
which the fp does not have voting power. However, this case rarely
happens and is unharmful. More discussions can be found
[here](#120 (comment)).
  • Loading branch information
gitferry authored Nov 27, 2024
1 parent 0fca54e commit b4cd6ac
Show file tree
Hide file tree
Showing 11 changed files with 172 additions and 266 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
* [#148](https://github.com/babylonlabs-io/finality-provider/pull/148) Allow command `eotsd keys add` to use
empty HD path to derive new key and use master private key.
* [#154](https://github.com/babylonlabs-io/finality-provider/pull/154) Use sign schnorr instead of getting private key from EOTS manager
* [#167](https://github.com/babylonlabs-io/finality-provider/pull/167) Remove last processed height

### v0.12.1

Expand Down
293 changes: 140 additions & 153 deletions finality-provider/proto/finality_providers.pb.go

Large diffs are not rendered by default.

7 changes: 2 additions & 5 deletions finality-provider/proto/finality_providers.proto
Original file line number Diff line number Diff line change
Expand Up @@ -158,11 +158,8 @@ message FinalityProvider {
string chain_id = 7;
// last_voted_height defines the height of the last voted chain block
uint64 last_voted_height = 8;
// last_processed_height defines the height of the last successfully processed block
// even though the vote is not cast
uint64 last_processed_height = 9;
// status defines the current finality provider status
FinalityProviderStatus status = 10;
FinalityProviderStatus status = 9;
}

// FinalityProviderInfo is the basic information of a finality provider mainly for external usage
Expand Down Expand Up @@ -271,4 +268,4 @@ message EditFinalityProviderRequest {
}

// Define an empty response message
message EmptyResponse {}
message EmptyResponse {}
39 changes: 13 additions & 26 deletions finality-provider/service/fp_instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@ import (
"go.uber.org/atomic"
"go.uber.org/zap"

fppath "github.com/babylonlabs-io/finality-provider/lib/math"

"github.com/babylonlabs-io/finality-provider/clientcontroller"
"github.com/babylonlabs-io/finality-provider/eotsmanager"
fpcfg "github.com/babylonlabs-io/finality-provider/finality-provider/config"
Expand Down Expand Up @@ -230,8 +228,13 @@ func (fp *FinalityProviderInstance) getAllBlocksFromChan() []*types.BlockInfo {
}

func (fp *FinalityProviderInstance) shouldProcessBlock(b *types.BlockInfo) (bool, error) {
// check whether the block has been processed before
if fp.hasProcessed(b) {
if b.Height <= fp.GetLastVotedHeight() {
fp.logger.Debug(
"the block height is lower than last processed height",
zap.String("pk", fp.GetBtcPkHex()),
zap.Uint64("block_height", b.Height),
zap.Uint64("last_voted_height", fp.GetLastVotedHeight()),
)
return false, nil
}

Expand All @@ -243,7 +246,6 @@ func (fp *FinalityProviderInstance) shouldProcessBlock(b *types.BlockInfo) (bool
if !hasVp {
// the finality provider does not have voting power
// and it will never will at this block
fp.MustSetLastProcessedHeight(b.Height)
fp.metrics.IncrementFpTotalBlocksWithoutVotingPower(fp.GetBtcPkHex())
return false, nil
}
Expand Down Expand Up @@ -287,20 +289,6 @@ func (fp *FinalityProviderInstance) randomnessCommitmentLoop() {
}
}

func (fp *FinalityProviderInstance) hasProcessed(b *types.BlockInfo) bool {
if b.Height <= fp.GetLastProcessedHeight() {
fp.logger.Debug(
"the block has been processed before, skip processing",
zap.String("pk", fp.GetBtcPkHex()),
zap.Uint64("block_height", b.Height),
zap.Uint64("last_processed_height", fp.GetLastProcessedHeight()),
)
return true
}

return false
}

func (fp *FinalityProviderInstance) hasVotingPower(b *types.BlockInfo) (bool, error) {
power, err := fp.GetVotingPowerWithRetry(b.Height)
if err != nil {
Expand Down Expand Up @@ -500,7 +488,7 @@ func (fp *FinalityProviderInstance) CommitPubRand(tipHeight uint64) (*types.TxRe

// make sure that the start height is at least the finality activation height
// and updated to generate the list with the same as the committed height.
startHeight = fppath.MaxUint64(startHeight, activationBlkHeight)
startHeight = max(startHeight, activationBlkHeight)
// generate a list of Schnorr randomness pairs
// NOTE: currently, calling this will create and save a list of randomness
// in case of failure, randomness that has been created will be overwritten
Expand Down Expand Up @@ -651,11 +639,11 @@ func (fp *FinalityProviderInstance) TestSubmitFinalitySignatureAndExtractPrivKey
}

// getPollerStartingHeight gets the starting height of the poller with
// max(lastProcessedHeight+1, lastFinalizedHeight+1, params.FinalityActivationHeight)
// max(lastVotedHeight+1, lastFinalizedHeight+1, params.FinalityActivationHeight)
// this ensures that:
// (1) the fp will not vote for a height lower than params.FinalityActivationHeight
// (2) the fp will not miss for any non-finalized blocks
// (3) the fp will not process any blocks that have been already processed
// (3) the fp will not process any blocks that have been already voted
// Note: if the fp starting from the last finalized height with a gap to the last
// processed height, the fp might miss some rewards due to not sending the votes
// depending on the consumer chain's reward distribution mechanism
Expand All @@ -682,12 +670,11 @@ func (fp *FinalityProviderInstance) getPollerStartingHeight() (uint64, error) {

// if we have finalized blocks, consider the height after the latest finalized block
if len(latestFinalisedBlocks) > 0 {
startHeight = fppath.MaxUint64(startHeight, latestFinalisedBlocks[0].Height+1)
startHeight = max(startHeight, latestFinalisedBlocks[0].Height+1)
}

// consider the height after the last processed block
lastProcessedHeight := fp.GetLastProcessedHeight()
startHeight = fppath.MaxUint64(startHeight, lastProcessedHeight+1)
// consider the height after the last voted height
startHeight = max(startHeight, fp.GetLastVotedHeight()+1)

return startHeight, nil
}
Expand Down
1 change: 0 additions & 1 deletion finality-provider/service/fp_instance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,6 @@ func FuzzSubmitFinalitySigs(f *testing.F) {

// check the last_voted_height
require.Equal(t, nextBlock.Height, fpIns.GetLastVotedHeight())
require.Equal(t, nextBlock.Height, fpIns.GetLastProcessedHeight())
})
}

Expand Down
28 changes: 2 additions & 26 deletions finality-provider/service/fp_store_adapter.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,17 +87,9 @@ func (fps *fpState) setStatus(s proto.FinalityProviderStatus) error {
return fps.s.SetFpStatus(fps.fp.BtcPk, s)
}

func (fps *fpState) setLastProcessedHeight(height uint64) error {
fps.mu.Lock()
fps.fp.LastProcessedHeight = height
fps.mu.Unlock()
return fps.s.SetFpLastProcessedHeight(fps.fp.BtcPk, height)
}

func (fps *fpState) setLastProcessedAndVotedHeight(height uint64) error {
func (fps *fpState) setLastVotedHeight(height uint64) error {
fps.mu.Lock()
fps.fp.LastVotedHeight = height
fps.fp.LastProcessedHeight = height
fps.mu.Unlock()
return fps.s.SetFpLastVotedHeight(fps.fp.BtcPk, height)
}
Expand Down Expand Up @@ -126,10 +118,6 @@ func (fp *FinalityProviderInstance) GetLastVotedHeight() uint64 {
return fp.fpState.getStoreFinalityProvider().LastVotedHeight
}

func (fp *FinalityProviderInstance) GetLastProcessedHeight() uint64 {
return fp.fpState.getStoreFinalityProvider().LastProcessedHeight
}

func (fp *FinalityProviderInstance) GetChainID() []byte {
return []byte(fp.fpState.getStoreFinalityProvider().ChainID)
}
Expand All @@ -145,20 +133,8 @@ func (fp *FinalityProviderInstance) MustSetStatus(s proto.FinalityProviderStatus
}
}

func (fp *FinalityProviderInstance) SetLastProcessedHeight(height uint64) error {
return fp.fpState.setLastProcessedHeight(height)
}

func (fp *FinalityProviderInstance) MustSetLastProcessedHeight(height uint64) {
if err := fp.SetLastProcessedHeight(height); err != nil {
fp.logger.Fatal("failed to set last processed height",
zap.String("pk", fp.GetBtcPkHex()), zap.Uint64("last_processed_height", height))
}
fp.metrics.RecordFpLastProcessedHeight(fp.GetBtcPkHex(), height)
}

func (fp *FinalityProviderInstance) updateStateAfterFinalitySigSubmission(height uint64) error {
return fp.fpState.setLastProcessedAndVotedHeight(height)
return fp.fpState.setLastVotedHeight(height)
}

func (fp *FinalityProviderInstance) MustUpdateStateAfterFinalitySigSubmission(height uint64) {
Expand Down
17 changes: 0 additions & 17 deletions finality-provider/store/fpstore.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,30 +148,13 @@ func (s *FinalityProviderStore) SetFpLastVotedHeight(btcPk *btcec.PublicKey, las
if fp.LastVotedHeight < lastVotedHeight {
fp.LastVotedHeight = lastVotedHeight
}
if fp.LastProcessedHeight < lastVotedHeight {
fp.LastProcessedHeight = lastVotedHeight
}

return nil
}

return s.setFinalityProviderState(btcPk, setFpLastVotedHeight)
}

// SetFpLastProcessedHeight sets the last processed height to the stored last processed height
// only if it is larger than the stored one. This is to ensure the stored state to increase monotonically
func (s *FinalityProviderStore) SetFpLastProcessedHeight(btcPk *btcec.PublicKey, lastProcessedHeight uint64) error {
setFpLastProcessedHeight := func(fp *proto.FinalityProvider) error {
if fp.LastProcessedHeight < lastProcessedHeight {
fp.LastProcessedHeight = lastProcessedHeight
}

return nil
}

return s.setFinalityProviderState(btcPk, setFpLastProcessedHeight)
}

func (s *FinalityProviderStore) setFinalityProviderState(
btcPk *btcec.PublicKey,
stateTransitionFn func(provider *proto.FinalityProvider) error,
Expand Down
28 changes: 13 additions & 15 deletions finality-provider/store/storedfp.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,15 @@ import (
)

type StoredFinalityProvider struct {
FPAddr string
BtcPk *btcec.PublicKey
Description *stakingtypes.Description
Commission *sdkmath.LegacyDec
Pop *proto.ProofOfPossession
KeyName string
ChainID string
LastVotedHeight uint64
LastProcessedHeight uint64
Status proto.FinalityProviderStatus
FPAddr string
BtcPk *btcec.PublicKey
Description *stakingtypes.Description
Commission *sdkmath.LegacyDec
Pop *proto.ProofOfPossession
KeyName string
ChainID string
LastVotedHeight uint64
Status proto.FinalityProviderStatus
}

func protoFpToStoredFinalityProvider(fp *proto.FinalityProvider) (*StoredFinalityProvider, error) {
Expand All @@ -49,11 +48,10 @@ func protoFpToStoredFinalityProvider(fp *proto.FinalityProvider) (*StoredFinalit
Pop: &proto.ProofOfPossession{
BtcSig: fp.Pop.BtcSig,
},
KeyName: fp.KeyName,
ChainID: fp.ChainId,
LastVotedHeight: fp.LastVotedHeight,
LastProcessedHeight: fp.LastProcessedHeight,
Status: fp.Status,
KeyName: fp.KeyName,
ChainID: fp.ChainId,
LastVotedHeight: fp.LastVotedHeight,
Status: fp.Status,
}, nil
}

Expand Down
2 changes: 1 addition & 1 deletion itest/test_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -689,7 +689,7 @@ func (tm *TestManager) InsertBTCDelegation(t *testing.T, fpPks []*btcec.PublicKe
delBtcPrivKey, delBtcPubKey, err := datagen.GenRandomBTCKeyPair(r)
require.NoError(t, err)

unbondingTime := uint16(tm.StakingParams.MinimumUnbondingTime()) + 1
unbondingTime := uint16(tm.StakingParams.MinUnbondingTime)
testStakingInfo := datagen.GenBTCStakingSlashingInfo(
r,
t,
Expand Down
12 changes: 0 additions & 12 deletions lib/math/math.go

This file was deleted.

10 changes: 0 additions & 10 deletions types/stakingparams.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,3 @@ type StakingParams struct {
// The minimum time for unbonding transaction timelock in BTC blocks
MinUnbondingTime uint32
}

// MinimumUnbondingTime returns the minimum unbonding time. It is the bigger value from:
// - MinUnbondingTime
// - CheckpointFinalizationTimeout
func (p *StakingParams) MinimumUnbondingTime() uint32 {
return sdkmath.Max[uint32](
p.MinUnbondingTime,
p.FinalizationTimeoutBlocks,
)
}

0 comments on commit b4cd6ac

Please sign in to comment.