Copy orchestrator implementation from Celestia-app #50
Conversation
| type I interface { | ||
| Start(ctx context.Context) | ||
| StartNewEventsListener(ctx context.Context, queue chan<- uint64, signalChan <-chan struct{}) error | ||
| EnqueueMissingEvents(ctx context.Context, queue chan<- uint64, signalChan <-chan struct{}) error | ||
| ProcessNonces(ctx context.Context, noncesQueue <-chan uint64, signalChan chan<- struct{}) error | ||
| Process(ctx context.Context, nonce uint64) error | ||
| ProcessValsetEvent(ctx context.Context, valset celestiatypes.Valset) error | ||
| ProcessDataCommitmentEvent(ctx context.Context, dc celestiatypes.DataCommitment) error | ||
| } |
There was a problem hiding this comment.
naming an interface I feels very non-standard lol, but I think I see where this is coming from given that everything is named orchestrator.
While we certainly don't have to handle in this PR, I think re-evaluating this, and other, interfaces would prove valuable . IIRC, we originally added the interface to make some unit tests easier, but then changes the design. if that's the case, then we should think about creating a different mock that can do something very similar. In general, I'm typically skeptical of large interfaces, especially when we only plan on supporting a single implementation.
speaking of which, do we also not have any unit tests for these? if not, mind creating an issue or handling here? thanks!
There was a problem hiding this comment.
then we should think about creating a different mock that can do something very similar
Do you have some suggestion or an example of using different mocks without having to create a general interface? Would love to take a look at it and improve on this
do we also not have any unit tests for these?
Good catch #51
There was a problem hiding this comment.
well, we could use generated mocks, but I'm not the biggest fan of those either, or mocking when not absolutely necessary. One thing might be possible is to only mock the app queries. We could also use a testnode. Those can have fast 200ms blocktimes or so, and only take a few seconds to run simple tests
There was a problem hiding this comment.
We could also use a testnode
I like this, created issue: #55
So, we just leave the current interface as it is for now? until we have a testnode?
Overview
Closes #11
Checklist