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: add finality activation height #101

Merged
merged 14 commits into from
Oct 24, 2024

Conversation

RafilxTenfen
Copy link
Contributor

@RafilxTenfen RafilxTenfen commented Oct 19, 2024

  • Add checks for finality activation height
  • Pub rand commit start height at activation height or last commited height whatever is higher
  • Finality vote, only vote in blocks after the finality activation height
  • Update babylon to v014

@RafilxTenfen RafilxTenfen changed the title chore: add to client controller get param activation block height fin… chore: add finality activation height Oct 23, 2024
@RafilxTenfen RafilxTenfen self-assigned this Oct 23, 2024
@RafilxTenfen RafilxTenfen marked this pull request as ready for review October 23, 2024 15:01
Copy link

@KonradStaniec KonradStaniec left a comment

Choose a reason for hiding this comment

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

In general looks good 👍 though lets wait for @gitferry review

itest/test_manager.go Outdated Show resolved Hide resolved
Copy link
Member

@gitferry gitferry left a comment

Choose a reason for hiding this comment

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

Maybe I was missing something, but shouldn't we add a check to prevent the fp sending finality sig before activation height?

finality-provider/service/fastsync.go Outdated Show resolved Hide resolved
@@ -395,6 +389,18 @@ func (fp *FinalityProviderInstance) tryFastSync(targetBlock *types.BlockInfo) (*
return fp.FastSync(startHeight, targetBlock.Height)
}

func (fp *FinalityProviderInstance) fastSyncStartHeight(lastFinalizedHeight uint64) (uint64, error) {
Copy link
Member

Choose a reason for hiding this comment

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

fastSyncStartHeight is called after we have known that there's finalized blocks, indicating that activation is already started, so maybe we don't need to check activation height here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fastSyncStartHeight returns the height that fast sync should start right?
I probably can remove the check inside the fastSync than

Copy link
Member

@Lazar955 Lazar955 left a comment

Choose a reason for hiding this comment

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

Lgtm! Minor nits:

finality-provider/cmd/cmd_test.go Show resolved Hide resolved
util/math.go Outdated Show resolved Hide resolved
@RafilxTenfen
Copy link
Contributor Author

Maybe I was missing something, but shouldn't we add a check to prevent the fp sending finality sig before activation height?

Yeap, you are right, there was another loop that sends finality, check added at 7175eba

Copy link
Member

@gitferry gitferry left a comment

Choose a reason for hiding this comment

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

Thanks for the changes! Lgtm!

@RafilxTenfen RafilxTenfen merged commit 2dbba5a into main Oct 24, 2024
12 checks passed
@Lazar955 Lazar955 deleted the rafilx/btc-staking-activation-height branch December 23, 2024 16:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants