beacon node: renaming functions and services to better reflect what they do #14643
Conversation
| @@ -240,7 +240,6 @@ func NewService(ctx context.Context, cfg *Config) *Service { | |||
| GenesisFetcher: s.cfg.GenesisFetcher, | |||
| FinalizationFetcher: s.cfg.FinalizationFetcher, | |||
| TimeFetcher: s.cfg.GenesisTimeFetcher, | |||
| BlockFetcher: s.cfg.ExecutionChainService, | |||
There was a problem hiding this comment.
We had 2 registrations of Block Fetcher, one as block fetcher and one as Eth1BlockFetcher
| @@ -222,7 +222,7 @@ func (s *Service) validateAggregatedAtt(ctx context.Context, signed ethpb.Signed | |||
| return s.validateWithBatchVerifier(ctx, "aggregate", set) | |||
| } | |||
|
|
|||
| func (s *Service) validateBlockInAttestation(ctx context.Context, satt ethpb.SignedAggregateAttAndProof) bool { | |||
| func (s *Service) validateBlockPresenceOrQueueAttestation(ctx context.Context, satt ethpb.SignedAggregateAttAndProof) bool { | |||
There was a problem hiding this comment.
Shorter name + documentation would be better IMO
There was a problem hiding this comment.
honestly it feels like there's a deeper issue here why is the save like hacked into this lol the function does 2 things.
couldn't think of a better name, I didn't think validateBlockInAttestation represented what it did
There was a problem hiding this comment.
In that case, should we not separate it into two different functions?
There was a problem hiding this comment.
I was afraid to do any major changes on this without fully understanding why it was merged in the first place
|
I would rename |
| @@ -222,7 +222,7 @@ func (s *Service) validateAggregatedAtt(ctx context.Context, signed ethpb.Signed | |||
| return s.validateWithBatchVerifier(ctx, "aggregate", set) | |||
| } | |||
|
|
|||
| func (s *Service) validateBlockInAttestation(ctx context.Context, satt ethpb.SignedAggregateAttAndProof) bool { | |||
| func (s *Service) validateBlockPresenceOrQueueAttestation(ctx context.Context, satt ethpb.SignedAggregateAttAndProof) bool { | |||
There was a problem hiding this comment.
In that case, should we not separate it into two different functions?
Co-authored-by: Sammy Rosso <[email protected]>
What type of PR is this?
Other
What does this PR do? Why is it needed?
Which issues(s) does this PR fix?
Fixes #14639
Other notes for review
Acknowledgements