babylonlabs-io / babylon-staking-indexer

Other
1 stars 2 forks source link

feat: unbonding state transitions #27

Closed gusin13 closed 1 month ago

gusin13 commented 1 month ago

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

jrwbabylonlab commented 1 month ago

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.

gusin13 commented 1 month ago

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

jrwbabylonlab commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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