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.

Sign up for GitHub

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

Jump to bottom

ADR-034: Add highest voted height to finality provider #304

Merged
merged 2 commits into from
Nov 28, 2024
Merged

ADR-034: Add highest voted height to finality provider #304

merged 2 commits into from
Nov 28, 2024

Conversation

gitferry
Copy link
Member

@gitferry gitferry commented Nov 26, 2024 •
edited
Loading

Implements node side of https://github.com/babylonlabs-io/pm/pull/134 by injecting HighestVotedHeight to finality provider

@gitferry gitferry marked this pull request as ready for review November 26, 2024 15:00
@gitferry gitferry requested a review from a team as a code owner November 26, 2024 15:00
@gitferry gitferry requested review from KonradStaniec, samricotta, SebastianElvis, RafilxTenfen and Lazar955 and removed request for a team November 26, 2024 15:00
RafilxTenfen
RafilxTenfen approved these changes Nov 27, 2024
View reviewed changes
Copy link
Contributor

@RafilxTenfen RafilxTenfen left a comment

Choose a reason for hiding this comment

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

All the changes looks good to me, can we also add a check in FuzzAddFinalitySig that verifies the HighestVotedHeight is being updated?

@gitferry
Copy link
Member Author

All the changes looks good to me, can we also add a check in FuzzAddFinalitySig that verifies the HighestVotedHeight is being updated?

Not sure how we can do it because UpdateFinalityProvider is an expected interface from btcstaking module. It is mocked in test. We have relevant test here

babylon/x/btcstaking/keeper/grpc_query_test.go

Line 135 in f93e8c1

require.Equal(t, v.HighestVotedHeight, resp.FinalityProvider.HighestVotedHeight)

@gitferry gitferry merged commit 457909d into main Nov 28, 2024
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RafilxTenfen RafilxTenfen RafilxTenfen approved these changes

@KonradStaniec KonradStaniec Awaiting requested review from KonradStaniec KonradStaniec was automatically assigned from babylonlabs-io/core-dev

@samricotta samricotta Awaiting requested review from samricotta samricotta was automatically assigned from babylonlabs-io/core-dev

@SebastianElvis SebastianElvis Awaiting requested review from SebastianElvis

@Lazar955 Lazar955 Awaiting requested review from Lazar955

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@gitferry @RafilxTenfen