feat: unbonding state transitions #27

gusin13 · 2024-10-22T19:28:15Z

This PR

Sets up internal expiry checker service which reads the timelock_queue table and mark the delegations as Withdrawable

Note -
ran into this issue - ref tl;dr - this pr only marks delegations which are naturally expired as Unbonding -> Withdrawable.
for manual unbonding some extra work is needed as logged in above issue

i couldn't properly test the expiry checker as i can't access btc node locally, have pinged devops

…ING, later expiry checker will mark unbonded by logic

internal/db/timelock.go

jrwbabylonlab · 2024-10-23T02:17:59Z

This PR

Sets up internal expiry checker service which reads the timelock_queue table and mark the delegations as Withdrawable

Note - ran into this issue - ref tl;dr - this pr only marks delegations which are naturally expired as Unbonding -> Withdrawable. for manual unbonding some extra work is needed as logged in above issue

i couldn't properly test the expiry checker as i can't access btc node locally, have pinged devops

@gusin13 "marks delegations which are naturally expired as Unbonding -> Withdrawable." this statement seems wrong to me. The BBN will emit EventBTCDelegationExpired when the BTC timelock is expired. That means we do not need to derive the withdrawable state in indexer for natural expired delegation. it should be marked to withdrawable as soon as we received the EventBTCDelegationExpired event.

I was referring to this comment in the babylon code

// EventBTCDelegationExpired is the event emitted when a BTC delegation
// is unbonded by expiration of the staking tx timelock

If this is correct then my above assumption should hold. But if not, then core team need to fix this comment and explicity point out the EventBTCDelegationExpired are emitted at the time of VP lost, rather than timelock expired.

jrwbabylonlab · 2024-10-23T02:28:06Z

internal/services/delegation.go

 	if err := s.db.UpdateBTCDelegationState(
-		ctx, unbondedEarlyEvent.StakingTxHash, types.DelegationState(unbondedEarlyEvent.NewState),
+		ctx, unbondedEarlyEvent.StakingTxHash, types.StateUnbonding,


could we have a method in the utils to map the BBN state into web state? i.e types.DelegationState(unbondedEarlyEvent.NewState) seems ok to me, but not sure why it's removed?

the state sent in Babylon event is Unbonded, types.DelegationState(unbondedEarlyEvent.NewState) would give us Unbonded

but the indexer needs to treat this as Unbonding

internal/services/delegation.go

jrwbabylonlab · 2024-10-23T02:33:38Z

internal/services/expiry-checker.go

+		}
+
+		// previous state should be unbonding
+		if delegation.State != types.StateUnbonding {


I think this validation is too strong which will leads to issues when we have duplicated entries in db upon re-bootstrap of the service.

We have this set of methods in the phase-1 which i think we can re-use in phase-2 https://github.com/babylonlabs-io/staking-api-service/blob/main/internal/shared/utils/state_transition.go#L34
i.e ignore if the state is already beyond the withdrawable state

i see, sure i have removed the check.
this might required lot of change in code for the outdated/qualified state transitions, would prefer to do in separate pr

#29

jrwbabylonlab

Approved with some comments

gusin13 · 2024-10-23T03:47:06Z

This PR
Sets up internal expiry checker service which reads the timelock_queue table and mark the delegations as Withdrawable
Note - ran into this issue - ref tl;dr - this pr only marks delegations which are naturally expired as Unbonding -> Withdrawable. for manual unbonding some extra work is needed as logged in above issue
i couldn't properly test the expiry checker as i can't access btc node locally, have pinged devops

@gusin13 "marks delegations which are naturally expired as Unbonding -> Withdrawable." this statement seems wrong to me. The BBN will emit EventBTCDelegationExpired when the BTC timelock is expired. That means we do not need to derive the withdrawable state in indexer for natural expired delegation. it should be marked to withdrawable as soon as we received the EventBTCDelegationExpired event.

I was referring to this comment in the babylon code
// EventBTCDelegationExpired is the event emitted when a BTC delegation
// is unbonded by expiration of the staking tx timelock
If this is correct then my above assumption should hold. But if not, then core team need to fix this comment and explicity point out the EventBTCDelegationExpired are emitted at the time of VP lost, rather than timelock expired.

@jrwbabylonlab

That means we do not need to derive the withdrawable state in indexer for natural expired delegation

https://github.com/babylonlabs-io/babylon/blob/6a0ccacc41435a249316a77fe6e1e06aeb654d13/x/btcstaking/keeper/msg_server.go#L323-L332

https://github.com/babylonlabs-io/babylon/blob/6a0ccacc41435a249316a77fe6e1e06aeb654d13/x/btcstaking/types/btc_delegation.go#L122-L126

In Babylon this event is sent at btcDel.EndHeight-wValue which is basically when the VP is lost, but for our case we need to make sure
current btc tip > btcDel.EndHeight to mark it as Withdrawable.
Maybe comments can be improved in that event

gitferry

Great work! Some nits and questions:

internal/config/poller.go

internal/db/model/setup.go

internal/services/delegation.go

internal/db/timelock.go

internal/services/expiry-checker.go

jrwbabylonlab · 2024-10-23T12:05:11Z

This PR
Sets up internal expiry checker service which reads the timelock_queue table and mark the delegations as Withdrawable
Note - ran into this issue - ref tl;dr - this pr only marks delegations which are naturally expired as Unbonding -> Withdrawable. for manual unbonding some extra work is needed as logged in above issue
i couldn't properly test the expiry checker as i can't access btc node locally, have pinged devops

@gusin13 "marks delegations which are naturally expired as Unbonding -> Withdrawable." this statement seems wrong to me. The BBN will emit EventBTCDelegationExpired when the BTC timelock is expired. That means we do not need to derive the withdrawable state in indexer for natural expired delegation. it should be marked to withdrawable as soon as we received the EventBTCDelegationExpired event.
I was referring to this comment in the babylon code
// EventBTCDelegationExpired is the event emitted when a BTC delegation
// is unbonded by expiration of the staking tx timelock
If this is correct then my above assumption should hold. But if not, then core team need to fix this comment and explicity point out the EventBTCDelegationExpired are emitted at the time of VP lost, rather than timelock expired.
@jrwbabylonlab

That means we do not need to derive the withdrawable state in indexer for natural expired delegation

https://github.com/babylonlabs-io/babylon/blob/6a0ccacc41435a249316a77fe6e1e06aeb654d13/x/btcstaking/keeper/msg_server.go#L323-L332

https://github.com/babylonlabs-io/babylon/blob/6a0ccacc41435a249316a77fe6e1e06aeb654d13/x/btcstaking/types/btc_delegation.go#L122-L126

In Babylon this event is sent at btcDel.EndHeight-wValue which is basically when the VP is lost, but for our case we need to make sure current btc tip > btcDel.EndHeight to mark it as Withdrawable. Maybe comments can be improved in that event

@gusin13 gotcha. In that case, yeah, make sense to have the natural unbonded event to be tracked in the expiry process so we can transition to the withdrawable state once timelock expires.
However, i do not think we need an unbonding state for the natural expire delegation. There is no such thing from BTC staking point of view.

gusin13 · 2024-10-23T16:04:26Z

However, i do not think we need an unbonding state for the natural expire delegation. There is no such thing from BTC staking point of view.

@jrwbabylonlab i found some comments here which explains this case. Basically after the VP is lost, the delegation is neither active nor withdrawable, there is a middle state which can be referred as unbonding

jrwbabylonlab · 2024-10-24T04:51:07Z

However, i do not think we need an unbonding state for the natural expire delegation. There is no such thing from BTC staking point of view.

@jrwbabylonlab i found some comments here which explains this case. Basically after the VP is lost, the delegation is neither active nor withdrawable, there is a middle state which can be referred as unbonding

let's move this conversation to slack

gusin13 added 9 commits October 22, 2024 13:25

save in timelock when inclusion proof received

b5fa82b

if early unbonding or natural expired - indexer should mark as UNBOND…

3e45e15

…ING, later expiry checker will mark unbonded by logic

change unbondong to withdrawable

03891d7

timelock insertion should trigger from bbn expired event

9ccb345

add todo

35f1e5d

start expiry

8e7e0b1

update cfg

bc529cd

update mock interface

b649dc6

expire service

8d3ee72

gusin13 marked this pull request as ready for review October 22, 2024 22:16

gusin13 requested review from jrwbabylonlab, gitferry and jeremy-babylonlabs October 22, 2024 22:16

jrwbabylonlab reviewed Oct 23, 2024

View reviewed changes

internal/db/timelock.go Outdated Show resolved Hide resolved

jrwbabylonlab reviewed Oct 23, 2024

View reviewed changes

internal/services/delegation.go Outdated Show resolved Hide resolved

jrwbabylonlab reviewed Oct 23, 2024

View reviewed changes

jrwbabylonlab approved these changes Oct 23, 2024

View reviewed changes

gitferry approved these changes Oct 23, 2024

View reviewed changes

gusin13 added 3 commits October 23, 2024 11:39

pr comment - expired del limit set from cfg & fix cfg

2d25c1b

pr comment: fix dberr

ecc2b8e

pr comment: fix timelock db name and dberr

12ccb48

gusin13 added 2 commits October 23, 2024 12:06

fix cfg

132d6bd

add todo

0d73e74

gusin13 mentioned this pull request Oct 23, 2024

Use proper elibility/outdation criteria during state transitions #29

Closed

fix todo

2bf39e1

gusin13 merged commit 22ed334 into main Oct 23, 2024
11 checks passed

gusin13 deleted the gusin13/unbonding-transition branch October 23, 2024 16:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: unbonding state transitions #27

feat: unbonding state transitions #27

gusin13 commented Oct 22, 2024 •

edited

Loading

jrwbabylonlab commented Oct 23, 2024 •

edited

Loading

jrwbabylonlab Oct 23, 2024

gusin13 Oct 23, 2024 •

edited

Loading

jrwbabylonlab Oct 23, 2024

gusin13 Oct 23, 2024

jrwbabylonlab left a comment

gusin13 commented Oct 23, 2024 •

edited

Loading

gitferry left a comment

jrwbabylonlab commented Oct 23, 2024

gusin13 commented Oct 23, 2024

jrwbabylonlab commented Oct 24, 2024

feat: unbonding state transitions #27

feat: unbonding state transitions #27

Conversation

gusin13 commented Oct 22, 2024 • edited Loading

jrwbabylonlab commented Oct 23, 2024 • edited Loading

jrwbabylonlab Oct 23, 2024

Choose a reason for hiding this comment

gusin13 Oct 23, 2024 • edited Loading

Choose a reason for hiding this comment

jrwbabylonlab Oct 23, 2024

Choose a reason for hiding this comment

gusin13 Oct 23, 2024

Choose a reason for hiding this comment

jrwbabylonlab left a comment

Choose a reason for hiding this comment

gusin13 commented Oct 23, 2024 • edited Loading

gitferry left a comment

Choose a reason for hiding this comment

jrwbabylonlab commented Oct 23, 2024

gusin13 commented Oct 23, 2024

jrwbabylonlab commented Oct 24, 2024

gusin13 commented Oct 22, 2024 •

edited

Loading

jrwbabylonlab commented Oct 23, 2024 •

edited

Loading

gusin13 Oct 23, 2024 •

edited

Loading

gusin13 commented Oct 23, 2024 •

edited

Loading