Skip to content

Conversation

@Marketen
Copy link
Contributor

@Marketen Marketen commented Sep 19, 2025

Adds a SQLite storage adapter and related persistence layer to record validator epoch status, block proposals, and sync committee participation; also introduces sync committee notifications and associated beacon API call.

  • Add ValidatorStoragePort for SQLite adapter
  • Fetch sync committees participation in each epoch
  • Updates proposal tracking to store slot of proposals.

@Marketen Marketen requested a review from Copilot September 19, 2025 11:46
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Adds a SQLite storage adapter and related persistence layer to record validator epoch status, block proposals, and sync committee participation; also introduces sync committee notifications and associated beacon API call.

  • Introduces ValidatorStoragePort and a SQLite implementation (schema migrations + upsert methods)
  • Adds sync committee retrieval, notification type, and persistence of validator status per epoch
  • Updates proposal tracking to store richer duty data and sends new committee notifications

Reviewed Changes

Copilot reviewed 10 out of 11 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
internal/application/services/dutieschecker_service.go Adds storage persistence for proposals and epoch status; integrates sync committee checks and notification logic.
internal/application/ports/validatorstorage.go Defines new storage port interface for validator metadata, epoch status, and proposals.
internal/application/ports/notifier.go Extends notifier port with committee notification method.
internal/application/ports/beaconchain.go Extends beacon adapter interface with GetSyncCommittee.
internal/application/domain/notification.go Adds Committee notification category.
internal/adapters/sqlite/sqlite_storage.go Implements SQLite storage adapter with schema migrations and upsert methods.
internal/adapters/notifier/notifier.go Implements committee notification payload builder/send method.
internal/adapters/beacon/beacon.go Adds GetSyncCommittee implementation.
go.mod Adds sqlite3 driver dependency.
cmd/main.go Wires new SQLite storage into application and assigns to DutiesChecker.
Comments suppressed due to low confidence (1)

go.mod:3

  • The go directive specifies version 1.24.3, which does not (yet) exist in released Go toolchains and may break builds; replace with the latest supported stable version (e.g., 1.22 or 1.23) that is actually required by the code (slices requires >=1.21).
go 1.24.3

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +58 to +62
for _, q := range queries {
if _, err := db.Exec(q); err != nil {
return err
}
}
Copy link

Copilot AI Sep 19, 2025

Choose a reason for hiding this comment

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

[nitpick] Migration statements are executed individually without a transaction, so a mid-sequence failure can leave the schema in a partially applied state. Wrap the loop in a BEGIN/COMMIT transaction (rolling back on error) to ensure atomic schema setup.

Suggested change
for _, q := range queries {
if _, err := db.Exec(q); err != nil {
return err
}
}
tx, err := db.Begin()
if err != nil {
return err
}
for _, q := range queries {
if _, err := tx.Exec(q); err != nil {
tx.Rollback()
return err
}
}
if err := tx.Commit(); err != nil {
return err
}

Copilot uses AI. Check for mistakes.
@Marketen Marketen requested a review from Copilot September 19, 2025 12:00
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 10 out of 11 changed files in this pull request and generated 4 comments.

Comments suppressed due to low confidence (1)

go.mod:3

  • Go version 1.24.3 does not exist. The latest Go version as of January 2025 was 1.23.x. Please use a valid Go version.
go 1.24.3

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +327 to +331
syncCommitteeMap := make(map[domain.ValidatorIndex]bool)
for _, idx := range syncCommittee.Data.Validators {
syncCommitteeMap[domain.ValidatorIndex(idx)] = true
}
return syncCommitteeMap, nil
Copy link

Copilot AI Sep 19, 2025

Choose a reason for hiding this comment

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

The function creates a map of all sync committee members but doesn't filter for the requested validator indices. This returns membership status for all validators in the sync committee instead of just the ones being tracked. The function should only return results for validators in the indices parameter.

Suggested change
syncCommitteeMap := make(map[domain.ValidatorIndex]bool)
for _, idx := range syncCommittee.Data.Validators {
syncCommitteeMap[domain.ValidatorIndex(idx)] = true
}
return syncCommitteeMap, nil
// Build a set of sync committee members for fast lookup
syncCommitteeSet := make(map[phase0.ValidatorIndex]struct{}, len(syncCommittee.Data.Validators))
for _, idx := range syncCommittee.Data.Validators {
syncCommitteeSet[idx] = struct{}{}
}
// Build result map for only the requested indices
result := make(map[domain.ValidatorIndex]bool, len(indices))
for _, idx := range indices {
_, exists := syncCommitteeSet[phase0.ValidatorIndex(idx)]
result[idx] = exists
}
return result, nil

Copilot uses AI. Check for mistakes.
Comment on lines +205 to +220
var liveness *bool
isLive := slices.Contains(online, idx)
liveness = new(bool)
*liveness = isLive

var inSyncCommittee *bool
if syncCommitteeMap != nil {
val := syncCommitteeMap[idx]
inSyncCommittee = new(bool)
*inSyncCommittee = val
}

var slashedFlag *bool
isSlashed := slices.Contains(slashed, idx)
slashedFlag = new(bool)
*slashedFlag = isSlashed
Copy link

Copilot AI Sep 19, 2025

Choose a reason for hiding this comment

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

This pointer allocation pattern is unnecessarily verbose. Consider using liveness := &isLive instead of the three-line approach with new(bool) and dereferencing.

Suggested change
var liveness *bool
isLive := slices.Contains(online, idx)
liveness = new(bool)
*liveness = isLive
var inSyncCommittee *bool
if syncCommitteeMap != nil {
val := syncCommitteeMap[idx]
inSyncCommittee = new(bool)
*inSyncCommittee = val
}
var slashedFlag *bool
isSlashed := slices.Contains(slashed, idx)
slashedFlag = new(bool)
*slashedFlag = isSlashed
isLive := slices.Contains(online, idx)
liveness := &isLive
var inSyncCommittee *bool
if syncCommitteeMap != nil {
val := syncCommitteeMap[idx]
inSyncCommittee = &val
}
isSlashed := slices.Contains(slashed, idx)
slashedFlag := &isSlashed

Copilot uses AI. Check for mistakes.
Comment on lines +217 to +220
var slashedFlag *bool
isSlashed := slices.Contains(slashed, idx)
slashedFlag = new(bool)
*slashedFlag = isSlashed
Copy link

Copilot AI Sep 19, 2025

Choose a reason for hiding this comment

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

This pointer allocation pattern is unnecessarily verbose. Consider using slashedFlag := &isSlashed instead of the three-line approach with new(bool) and dereferencing.

Suggested change
var slashedFlag *bool
isSlashed := slices.Contains(slashed, idx)
slashedFlag = new(bool)
*slashedFlag = isSlashed
isSlashed := slices.Contains(slashed, idx)
slashedFlag := &isSlashed

Copilot uses AI. Check for mistakes.
Comment on lines +205 to +220
var liveness *bool
isLive := slices.Contains(online, idx)
liveness = new(bool)
*liveness = isLive

var inSyncCommittee *bool
if syncCommitteeMap != nil {
val := syncCommitteeMap[idx]
inSyncCommittee = new(bool)
*inSyncCommittee = val
}

var slashedFlag *bool
isSlashed := slices.Contains(slashed, idx)
slashedFlag = new(bool)
*slashedFlag = isSlashed
Copy link

Copilot AI Sep 19, 2025

Choose a reason for hiding this comment

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

This pointer allocation pattern is unnecessarily verbose. Consider using val := syncCommitteeMap[idx]; inSyncCommittee := &val instead of the allocation with new(bool) and dereferencing.

Suggested change
var liveness *bool
isLive := slices.Contains(online, idx)
liveness = new(bool)
*liveness = isLive
var inSyncCommittee *bool
if syncCommitteeMap != nil {
val := syncCommitteeMap[idx]
inSyncCommittee = new(bool)
*inSyncCommittee = val
}
var slashedFlag *bool
isSlashed := slices.Contains(slashed, idx)
slashedFlag = new(bool)
*slashedFlag = isSlashed
isLive := slices.Contains(online, idx)
liveness := &isLive
var inSyncCommittee *bool
if syncCommitteeMap != nil {
val := syncCommitteeMap[idx]
inSyncCommittee = &val
}
isSlashed := slices.Contains(slashed, idx)
slashedFlag := &isSlashed

Copilot uses AI. Check for mistakes.
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.

2 participants