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(monitor): bootstrap monitor from db #34

Merged
merged 17 commits into from
Sep 10, 2024

Conversation

Lazar955
Copy link
Member

@Lazar955 Lazar955 commented Sep 9, 2024

This PR enables the monitor to start from the last processed confirmed header, we also save the epoch

References issue.

@Lazar955 Lazar955 marked this pull request as ready for review September 9, 2024 14:25
Comment on lines 137 to 146
// get the height from db if it exists otherwise use the baseHeight provided from genesis
var startHeight uint64
dbHeight, exists, err := m.store.LatestHeight()
if err != nil {
panic(fmt.Errorf("error getting latest height from db %w", err))
} else if !exists {
startHeight = baseHeight
} else {
startHeight = dbHeight
}
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion to extract this into a func

Copy link
Contributor

@RafilxTenfen RafilxTenfen left a comment

Choose a reason for hiding this comment

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

Have you considered using the KVStore to be similar to what is being used in the Babylon code?

besides that, only a few suggestions that are no impediments to merge! 🎉

@Lazar955
Copy link
Member Author

Have you considered using the KVStore to be similar to what is being used in the Babylon code?

besides that, only a few suggestions that are no impediments to merge! 🎉

Nice suggestion, but I've seen that we use this in our btc-staker and indexer, Konrad suggested to go with it

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.

Great work! Some minor comments:

monitor/monitor.go Outdated Show resolved Hide resolved
monitor/monitor.go Outdated Show resolved Hide resolved
monitor/monitor.go Outdated Show resolved Hide resolved
e2etest/test_manager.go Outdated Show resolved Hide resolved
@@ -34,6 +34,7 @@ var (
eventuallyWaitTimeOut = 40 * time.Second
eventuallyPollTime = 1 * time.Second
regtestParams = &chaincfg.RegressionNetParams
defaultEpochInterval = uint(400) //nolint:unused
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
defaultEpochInterval = uint(400) //nolint:unused
defaultEpochInterval = uint(400)

e2etest/monitor_e2e_test.go Outdated Show resolved Hide resolved
@Lazar955 Lazar955 merged commit 3d6f6b6 into main Sep 10, 2024
8 checks passed
@Lazar955 Lazar955 deleted the lazar/monitor-stored-bootstrap branch September 10, 2024 12:00
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