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

feat: handle unexpected unbonding tx #102

Conversation

gusin13
Copy link
Collaborator

@gusin13 gusin13 commented Dec 29, 2024

This pr

  1. refactors/optimizes the spend notification logic
  2. intercepts the unknown unbonding tx case as discussed in this slack thread, this will enable the API to transition such delegations to Unbonding (this will be the final state for such case and such delegations will be stuck at Unbonding, as we don't know if it can be withdrawn)

Note - metrics will be added in separate PR

@gusin13 gusin13 marked this pull request as ready for review January 1, 2025 12:57
@jrwbabylonlab
Copy link
Collaborator

IMO, we shall get help from core team to investigate the root case first. Otherwise we are adding unnecessary logic and entry in db.

@gusin13
Copy link
Collaborator Author

gusin13 commented Jan 2, 2025

@jrwbabylonlab
We still need indexer to handle this case, this is something that Babylon does too i.e when receiving such tx it marks the delegation as "Unbonded" in its internal db. - code link

Copy link
Member

@gitferry gitferry left a comment

Choose a reason for hiding this comment

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

I think unexpected unbonding txs will be caught by IsValidUnbondingTx. Probably no need to consume the event from Babylon. It also makes think that maybe we can just do the same way used in Babylon (link) to identify unexpected unbonding tx (checking whether the hash matches to the unbonding tx in the delegation). We can do it now because now we have enforced that unbonding tx must be the same as the one in eoi

@gusin13 gusin13 marked this pull request as draft January 9, 2025 10:33
@gusin13 gusin13 force-pushed the 100-spend-notifications-on-transitioned-delegations-from-phase1-fails branch from 42701ef to 1ea1ea3 Compare January 13, 2025 09:54
@gusin13 gusin13 marked this pull request as ready for review January 13, 2025 15:49
@gusin13
Copy link
Collaborator Author

gusin13 commented Jan 13, 2025

@gitferry I have made the above discussed changes, can you please re-review

I deployed and tested this pr on devnet and the concerned delegation in devnet 91f0466163870397682e0cbc76cc132bc8734ede295e2d6950ba5f945533b5ab
which is discussed in this slack thread
is now being marked as Unbonding. Logs link - https://grafana.tooling.babylonlabs.io/goto/Jj1qjxvNR?orgId=1

Screenshot 2025-01-13 at 9 22 16 PM

@gusin13 gusin13 changed the title feat: handle unexpected unbonding tx event feat: handle unexpected unbonding tx Jan 13, 2025
@jrwbabylonlab jrwbabylonlab requested a review from kirugan January 14, 2025 06:41
internal/services/watch_btc_events.go Outdated Show resolved Hide resolved
internal/services/watch_btc_events.go Outdated Show resolved Hide resolved
internal/services/watch_btc_events.go Outdated Show resolved Hide resolved
internal/services/watch_btc_events.go Outdated Show resolved Hide resolved
@gusin13
Copy link
Collaborator Author

gusin13 commented Jan 15, 2025

@gitferry i have made further changes in pr as per our call.

  • removes the isValidUnbondingTx and uses multiple smaller functions instead isSpendingStakingTxUnbondingPath, validateUnbondingTxOutput
  • also updates the state (we discussed to do this in separate pr but the scope was very overlapping so while coding i realized it might be better to add now)

regarding the improvement of slashing - it will be done in separate pr

Copy link
Member

@gitferry gitferry left a comment

Choose a reason for hiding this comment

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

Thanks for the change. Great work!

internal/services/watch_btc_events.go Outdated Show resolved Hide resolved
internal/services/watch_btc_events.go Show resolved Hide resolved
@gusin13 gusin13 force-pushed the 100-spend-notifications-on-transitioned-delegations-from-phase1-fails branch from c773fca to aa989f3 Compare January 16, 2025 05:45
@gusin13 gusin13 merged commit d226846 into main Jan 16, 2025
11 checks passed
@gusin13 gusin13 deleted the 100-spend-notifications-on-transitioned-delegations-from-phase1-fails branch January 16, 2025 05:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants