chore: unbonding w e2e#471
chore: unbonding w e2e#471samricotta wants to merge 8 commits intofeature/monitor-activation-unbondingfrom
Conversation
samricotta
changed the base branch from
main
to
feature/monitor-activation-unbonding
There was a problem hiding this comment.
Pull Request Overview
This PR implements an activation and unbonding timing monitor system (unbonding w e2e) for the vigilante service. The monitor tracks BTC delegations through their lifecycle, measuring activation delays and detecting timeouts.
Key changes include:
- Addition of activation/unbonding monitoring functionality with timing metrics
- Integration of the monitor into the main vigilante service
- Comprehensive test coverage including e2e tests
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| monitor/monitor.go | Integrates activation unbonding monitor into main monitor service |
| monitor/activation_unbonding_monitor.go | Core monitoring logic for tracking delegation timing |
| monitor/expected_btcstaking_client.go | Client adapter for Babylon blockchain interactions |
| monitor/mock_btcstaking_client.go | Mock implementations for testing |
| monitor/activation_unbonding_monitor_test.go | Unit tests for monitor functionality |
| monitor/activation_unbonding_monitor_metrics_test.go | Tests for metrics collection |
| metrics/activation_unbonding.go | Prometheus metrics definitions |
| config/monitor.go | Configuration options for timing thresholds |
| cmd/vigilante/cmd/monitor.go | CLI integration for the monitor |
| e2etest/activation_unbonding_e2e_test.go | End-to-end tests for monitor behavior |
| e2etest/test_manager.go | Test infrastructure improvements with deterministic randomization |
| e2etest/test_manager_btcstaking.go | Updated to use deterministic random source |
| e2etest/slasher_e2e_test.go | Fixed goroutine management and random source |
| e2etest/monitor_e2e_test.go | Updated monitor instantiation |
| go.mod | Dependency management updates |
| btcstaking-tracker/stakingeventwatcher/stakingeventwatcher_test.go | Fixed typo in method name |
| btcstaking-tracker/stakingeventwatcher/mock_babylon_client.go | Fixed typo in method name |
| Makefile | Added mock generation for new interfaces |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Lazar955
left a comment
Lazar955
left a comment
There was a problem hiding this comment.
looks good, just address the slasher e2e change
e2etest/test_manager.go
Outdated
|
|
||
| h := fnv.New64a() | ||
| h.Write([]byte(t.Name())) | ||
| seed := int64(h.Sum64()) |
There was a problem hiding this comment.
why not just used timestamp for source of randomness?
There was a problem hiding this comment.
smart one! i changed
e2etest/slasher_e2e_test.go
Outdated
| err := bsTracker.Bootstrap(0) | ||
| require.NoError(t, err) | ||
| }() | ||
| err = bsTracker.Bootstrap(0) |
There was a problem hiding this comment.
why is this no longer in go routine?
There was a problem hiding this comment.
The goroutine doesnt wait, so if Start was called after (which it should be), it would race with Bootstrap. Also require in a goroutine only kills that goroutine, not the test(from what i understand).
Made it sync so Bootstrap finishes before Start runs
There was a problem hiding this comment.
yeah I think in that test we don;t want it to wait
There was a problem hiding this comment.
ok then ive removed the error then completely since it will fail regardless as we have require.Eventually
No description provided.