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

fix: BTCDelgationResponse missing staking_time #197

Merged
merged 2 commits into from
Oct 15, 2024

Conversation

gitferry
Copy link
Member

In pre-approval flow, the delegation has zero value for both startHeight and endHeight. Therefore, the staking time should not be inferred from endHeight - startHeight. Instead, it should be stored seprately. This PR adds stakingTime to BTCDelegationResponse

@gitferry gitferry requested a review from a team as a code owner October 15, 2024 07:43
@gitferry gitferry requested review from Lazar955 and RafilxTenfen and removed request for a team October 15, 2024 07:43
@gitferry gitferry added bug Something isn't working api breaking breaks grpc api in non-compatible way backport release/v0.12.x labels Oct 15, 2024
@gitferry gitferry requested review from KonradStaniec and removed request for Lazar955 and RafilxTenfen October 15, 2024 07:43
@gitferry gitferry changed the title fix: Add staking time to delegation response fix: BTCDelgationResponse missing staking_time Oct 15, 2024
@gitferry gitferry merged commit 6f1808b into main Oct 15, 2024
20 checks passed
@gitferry gitferry deleted the fix/delegation-response-staking-time branch October 15, 2024 09:25
@RafilxTenfen
Copy link
Contributor

This will need a new release at btc-staker, because it broke the proto for query QueryBTCDelegationResponse

@gitferry
Copy link
Member Author

gitferry commented Oct 16, 2024

This will need a new release at btc-staker, because it broke the proto for query QueryBTCDelegationResponse

Yep, this will go to the next devnet release as backporting to v0.12.x causes a lot of conflicts due to block height type change

@@ -251,40 +251,42 @@ message BTCDelegationResponse {
// fp_btc_pk_list is the list of BIP-340 PKs of the finality providers that
// this BTC delegation delegates to
repeated bytes fp_btc_pk_list = 3 [ (gogoproto.customtype) = "github.com/babylonlabs-io/babylon/types.BIP340PubKey" ];
// staking_time is the number of blocks for which the delegation is locked on BTC chain
uint32 staking_time = 4;
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: To try to be more backwards compatible, you can use a new label here, i.e.:
uint32 staking_time = 17;

That way the older versions will be able to handle all the other fields.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api breaking breaks grpc api in non-compatible way bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants