ChainSafe / ChainSafe/gossamer

Reconsider methods defined by the Subsystem interface

Open
#3,989 0 comments 0 reactions 0 assignees View on GitHub
S-subsystems-overseer T-question T-refactor
Dominant language
Go
Stars
454
Forks
144
PR merge metrics
No merged PRs in 30d

Description

## Issue summary

This is an issue to consider weather the Subsystem interface should be refactored since some of the methods defined there are not always used.

The Subsystem interface https://github.com/ChainSafe/gossamer/blob/ff28c9f58dbb021f79e7c9d00c117dddfb9fd026/dot/parachain/types/types.go#L660 defines methods:
- `ProcessActiveLeavesUpdateSignal(ActiveLeavesUpdateSignal) error`
- `ProcessBlockFinalizedSignal(BlockFinalizedSignal) error`
however some of the subsystems are not required to do anything regarding these signals, so perhaps we should reconsider if these methods should be included in the interface. As we design more subsystems it may become more evident what methods are required for a subsystem.

Also defined in the interface is `Run(ctx context.Context, OverseerToSubSystem chan any, SubSystemToOverseer chan any)` however many of the subsystems do not utilize the `OverseerToSubsystem` or the `SubsystemToOverseer` parameters in this method. So, perhaps the parameters of this interface should also be reconsidered.

## Other information and links

- https://github.com/ChainSafe/gossamer/pull/3916#discussion_r1610854502

Contributor guide

Open the contributing guide

Research direction

Start with the Subsystem interface in dot/parachain/types/types.go around line 660, then read the linked discussion in pull request 3916. Review which subsystems use the signal methods and Run parameters before proposing an interface design. Done means the required interface changes and their affected usages are agreed and consistently updated.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
distributed-systems
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.