New external staking events #131

KonradStaniec · 2024-10-04T14:11:33Z

Implements events generation as described in https://babylonlabs.atlassian.net/wiki/spaces/BABYLON/pages/31195227/API+asks+for+BBN+node

Few caveats:

for in delegation creation event we have mulitple finality providers keys to avoid breaking changes for phase-3
generation of event responsible for expiry happens in BeginBlocker so that unbonding early and unbonding through expiry have are retrieved in different way

This pr handles only staking events, finality provider events will be added in separate pr.

proto/babylon/btcstaking/v1/events.proto

SebastianElvis · 2024-10-07T07:02:40Z

x/btcstaking/keeper/btc_delegations.go

@@ -57,11 +57,28 @@ func (k Keeper) AddBTCDelegation(ctx sdk.Context, btcDel *types.BTCDelegation) e
 		panic(fmt.Errorf("failed to emit EventBTCDelegationStateUpdate for the new pending BTC delegation: %w", err))
 	}

+	if err := ctx.EventManager().EmitTypedEvent(types.NewBtcDelCreationEvent(


Can L51-58 be removed?

hmm we should emit both events, but it can be simplified by using EmitTypedEvents(tevs ...proto.Message) . Will do that.

Why is it the case? in my mind the former event only contains a subset of the latter event and they share the same type of consumers

there are at least two advantages:

consumers that do not want to process the "fat" event can only follow the state state updates events

consumers know the initial state of created delegation i.e that is is PENDING. Without it they would need to either assume some state or we would need to add state field to EventBTCDelegationCreated

Hmm I'm not sure if there is a use case for the "slim" event, as the main consumer of this API is backend and other components do not seem to be interested in a newly pending delegation. Also EventBTCDelegationCreated seems to imply this BTC delegation is pending, and if not subsequent events will be emitted. That's why I would still prefer to keep each state transition to have a unique type of event.

Another thing is that the EventBTCDelegationStateUpdate event now has two use cases, one for voting power update and the other for serving API consumers. This is not a perfect idea as EventBTCDelegationStateUpdate is in fact a "DB" object. Now it seems to be a good time to fix this

Another thing is that the EventBTCDelegationStateUpdate event now has two use cases, one for voting power update and the other for serving API consumers. This is not a perfect idea as EventBTCDelegationStateUpdate is in fact a "DB" object. Now it seems to be a good time to fix this

This is good point, we should not expose db objects for external consumers. Either way, I would not tangle this into this pr which is about new events for api but create separate one for this purpose

Being totally honest I do not have strong preference here and imo both approaches have its merits. @jrwbabylonlab what would api prefer:

Some special events, like delegation_creation + state transition events (i.e one event showing what is current state of delegation)

Or set of different events like: delegation_creation, proof_of_inclusion_provided, unbonded_early, expired and covenant_quorum_reached ?

Option 2 offers more flexibility for the backend to power the UI. In addition to this, would it be possible to emit the key state transition event alongside the Option 2 events? For example, an isActive: true/false flag could be emitted when receiving the proof_of_inclusion_provided event (as an example).

That said, it’s not a major issue or blocker if this isn’t feasible.

I would avoid the flags as much as possible.

We could attach new state to every specific event if this would help the api ?

lets go option 2 then and add state field to each event that will reflect current state of the delegation

commit e9f16f6, implements option 2.

Now:

we have separate external and internal events

for external events every event have different type

every external event except EventCovenantSignatureRecevied carries info about state. EventCovenantSignatureRecevied does not carry state as there is separate event for reaching quorum EventCovenantQuroumReached which changes state

there are two separate events for EventBTCDelgationUnbondedEarly and EventBTCDelegationExpired both are block events.

x/btcstaking/keeper/btc_delegations.go

SebastianElvis · 2024-10-07T07:05:15Z

x/btcstaking/keeper/btc_delegations.go

@@ -111,6 +136,15 @@ func (k Keeper) addCovenantSigsToBTCDelegation(
 		activeEvent := types.NewEventPowerDistUpdateWithBTCDel(event)
 		btcTip := k.btclcKeeper.GetTipInfo(ctx)
 		k.addPowerDistUpdateEvent(ctx, btcTip.Height, activeEvent)
+	} else if btcDel.HasCovenantQuorums(params.CovenantQuorum) && !btcDel.HasInclusionProof() {


If conditions in L125 and L139 could be consolidated somehow, e.g.,

if btcDel.HasCovenantQuorums(params.CovenantQuorum) { if btcDel.HasInclusionProof() { } else { } }

hmm this is probably matter of preference but I try do avoid deeply nested if/else blocks.

Though I will experiment with this a bit to check the readability.

hmm it looks good. I can leave with one level of nesting in this case 😅

SebastianElvis · 2024-10-07T07:05:52Z

x/btcstaking/keeper/msg_server.go

+
+	stakingTxHash := btcDel.MustGetStakingTxHash()
+
+	if err := ctx.EventManager().EmitTypedEvent(types.NewInclusionProofEvent(


can L290-296 be removed?

will switch to EmitTypedEvents(tevs ...proto.Message) 👍

x/btcstaking/keeper/power_dist_change.go

CHANGELOG.md

gitferry

Great work!

gitferry · 2024-10-07T09:40:27Z

proto/babylon/btcstaking/v1/events.proto

+  // timelock is the timelock of the staking tx specified in the BTC script
+  uint32 timelock = 5;


Suggested change

// timelock is the timelock of the staking tx specified in the BTC script

uint32 timelock = 5;

// staking_time is the timelock of the staking tx specified in the BTC script

uint32 staking_time = 5;

Seems we have been using staking_time everywhere in the staking library other than timelock

good point lets be consistent then across the code base.

gitferry · 2024-10-07T09:42:05Z

proto/babylon/btcstaking/v1/events.proto

+  uint64 staking_amount = 6;
+  // unbonding_time is the time is timelock on unbonding tx chosen by the staker
+  uint32 unbonding_time = 7;
+  // unbonding_tx is hex encoded bytes of the unbonding tx


Suggested change

// unbonding_tx is hex encoded bytes of the unbonding tx

// unbonding_tx is hex encoded bytes of the unsigned unbonding tx

gitferry · 2024-10-07T09:43:05Z

proto/babylon/btcstaking/v1/events.proto

+  string covenant_btc_pk_hex = 2;
+  // covenant_unbonding_signature_hex is the hex str of the BIP340 Schnorr
+  // signature of the covenant committee on the unbonding tx
+  string covenant_unbonding_signature_hex = 3;


why do we need to contain signature in the event?

I imagine api will process those events, retrieve signatures from them and then staker will be able to unbond without consulting Babylon anymore

gitferry · 2024-10-07T09:44:17Z

proto/babylon/btcstaking/v1/events.proto

+  // end_height is the end height of the BTC delegation
+  // it is calculated by end_height = start_height + staking_time
+  uint64 end_height = 3;


can end_height be calculated on the subscriber's sider?

probably, but tbh 4bytes is small price to pay and we already have all the information on hand when generating this event.

KonradStaniec · 2024-10-07T14:12:01Z

@SebastianElvis I have applied all the comment so I will merge it now. If there will be some missing things I will improve them in follow up.

…nge (#144) This is a follow-up PR of #131. This PR implements events generation related to finality providers as described in https://babylonlabs.atlassian.net/wiki/spaces/BABYLON/pages/31195227/API+asks+for+BBN+node. In particular, this PR 1. adds events for fp creation and editing, which are emitted within corresponding message handlers 2. adds events for fp status update, i.e., active, inactive, jailed, slashed, which are emitted during voting power table update per BeginBlock

KonradStaniec added 2 commits October 4, 2024 12:44

Add new type of events

ee1dabe

Generate new events

Loading
Loading status checks…

c8be21d

KonradStaniec requested a review from a team as a code owner October 4, 2024 14:11

KonradStaniec requested review from gitferry, Lazar955 and jrwbabylonlab and removed request for a team October 4, 2024 14:11

KonradStaniec added 3 commits October 4, 2024 16:33

Add changelog entry

Loading
Loading status checks…

54bef8e

Merge branch 'main' into konradstaniec/external-staking-events

7b55cf9

fix entry

Loading
Loading status checks…

4c52a89

SebastianElvis reviewed Oct 6, 2024

View reviewed changes

proto/babylon/btcstaking/v1/events.proto Show resolved Hide resolved

SebastianElvis reviewed Oct 7, 2024

View reviewed changes

KonradStaniec added 3 commits October 7, 2024 09:54

pr comments

Loading
Loading status checks…

2091dfa

Merge branch 'main' into konradstaniec/external-staking-events

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

GPG key ID: B5690EEEBB952194

Verified
Learn about vigilant mode

9e0b0de

fix changelog

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

GPG key ID: B5690EEEBB952194

Verified
Learn about vigilant mode

Loading
Loading status checks…

47a071b

gitferry approved these changes Oct 7, 2024

View reviewed changes

KonradStaniec added 3 commits October 7, 2024 13:14

protos for new events

faa9101

re-name state

95756b1

separate internal events and external ones

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

GPG key ID: B5690EEEBB952194

Verified
Learn about vigilant mode

Loading
Loading status checks…

e9f16f6

KonradStaniec merged commit efeb745 into main Oct 7, 2024
18 checks passed

KonradStaniec deleted the konradstaniec/external-staking-events branch October 7, 2024 14:12

gitferry mentioned this pull request Oct 8, 2024

feat: Add new external events related to finality provider status change #144

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New external staking events #131

New external staking events #131

KonradStaniec commented Oct 4, 2024 •

edited

Loading

SebastianElvis Oct 7, 2024

KonradStaniec Oct 7, 2024

SebastianElvis Oct 7, 2024

KonradStaniec Oct 7, 2024

SebastianElvis Oct 7, 2024

KonradStaniec Oct 7, 2024

jrwbabylonlab Oct 7, 2024

KonradStaniec Oct 7, 2024

KonradStaniec Oct 7, 2024

KonradStaniec Oct 7, 2024

SebastianElvis Oct 7, 2024

KonradStaniec Oct 7, 2024

KonradStaniec Oct 7, 2024

SebastianElvis Oct 7, 2024

KonradStaniec Oct 7, 2024

gitferry left a comment

gitferry Oct 7, 2024

KonradStaniec Oct 7, 2024

gitferry Oct 7, 2024

gitferry Oct 7, 2024

KonradStaniec Oct 7, 2024

gitferry Oct 7, 2024

KonradStaniec Oct 7, 2024

KonradStaniec commented Oct 7, 2024


		stakingTxHash := btcDel.MustGetStakingTxHash()

		if err := ctx.EventManager().EmitTypedEvent(types.NewInclusionProofEvent(

		// timelock is the timelock of the staking tx specified in the BTC script
		uint32 timelock = 5;

	// unbonding_tx is hex encoded bytes of the unbonding tx
	// unbonding_tx is hex encoded bytes of the unsigned unbonding tx

New external staking events #131

New external staking events #131

Conversation

KonradStaniec commented Oct 4, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gitferry left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KonradStaniec commented Oct 7, 2024

KonradStaniec commented Oct 4, 2024 •

edited

Loading