-
Notifications
You must be signed in to change notification settings - Fork 0
add sqlite adapter #29
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this 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.
| for _, q := range queries { | ||
| if _, err := db.Exec(q); err != nil { | ||
| return err | ||
| } | ||
| } |
Copilot
AI
Sep 19, 2025
There was a problem hiding this comment.
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.
| 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 | |
| } |
There was a problem hiding this 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.
| syncCommitteeMap := make(map[domain.ValidatorIndex]bool) | ||
| for _, idx := range syncCommittee.Data.Validators { | ||
| syncCommitteeMap[domain.ValidatorIndex(idx)] = true | ||
| } | ||
| return syncCommitteeMap, nil |
Copilot
AI
Sep 19, 2025
There was a problem hiding this comment.
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.
| 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 |
| 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 |
Copilot
AI
Sep 19, 2025
There was a problem hiding this comment.
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.
| 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 |
| var slashedFlag *bool | ||
| isSlashed := slices.Contains(slashed, idx) | ||
| slashedFlag = new(bool) | ||
| *slashedFlag = isSlashed |
Copilot
AI
Sep 19, 2025
There was a problem hiding this comment.
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.
| var slashedFlag *bool | |
| isSlashed := slices.Contains(slashed, idx) | |
| slashedFlag = new(bool) | |
| *slashedFlag = isSlashed | |
| isSlashed := slices.Contains(slashed, idx) | |
| slashedFlag := &isSlashed |
| 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 |
Copilot
AI
Sep 19, 2025
There was a problem hiding this comment.
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.
| 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 |
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.