Process headers on broadcast feed connect - #883
Conversation
On initial feed connection, client sends feed client version and requested message sequence number. Server validates that feed version number is accepted, and does not send any cached messages before requiested sequence number. Server responds to initial connection with feed server version and the chain id. Client validates server version and chain id, quitting with an error if unexpected values are returned.
Codecov Report
@@ Coverage Diff @@
## master #883 +/- ##
==========================================
- Coverage 51.81% 47.78% -4.04%
==========================================
Files 228 217 -11
Lines 28530 24585 -3945
Branches 502 502
==========================================
- Hits 14784 11747 -3037
+ Misses 12199 11275 -924
- Partials 1547 1563 +16 |
| func (b *SequenceNumberCatchupBuffer) OnDoBroadcast(bmi interface{}) error { | ||
| broadcastMessage, ok := bmi.(BroadcastMessage) | ||
| if !ok { | ||
| log.Crit("Requested to broadcast messasge of unknown type") |
There was a problem hiding this comment.
Let's do it in a separate PR as it's unrelated, but it seems like we could use generics around here and in the client manager to add type safety
| t *testing.T, | ||
| ctx context.Context, | ||
| isSequencer bool, | ||
| feedErrChan chan error, |
There was a problem hiding this comment.
Instead of passing the feedErrChan around everywhere, maybe CreateTestNode* could start a background goroutine that selects on feedErrChan and context cancellation, and fails the test if feedErrChan gets an error.
There was a problem hiding this comment.
testBlockValidatorSimple calls both CreateTestNodeOnL1WithConfig and Create2ndNodeWithConfig, want to make sure test aborts if either one fails.
There was a problem hiding this comment.
My thinking was that we could call t.Fail in a background goroutine which reads from feedErrChan. It's a bit inconvenient to need to pass around feedErrChan as-is.
Ensure that database is properly initialized when txTransaction is created so that no errors can occur
|
Mostly LGTM but has merge conflicts. I would also appreciate it if we didn't have to explicitly handle |
Reducing |
In most cases code can depend on ctx cancellation instead of passing around feedErrChan. Other places already have a reasonable timeout. There might be a few corner cases where feed errors are not properly reported during unit tests, but production code is fine.
2 participants
On initial feed connection: