-
Notifications
You must be signed in to change notification settings - Fork 51
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
fix(relayer): ensure submission order #2198
Conversation
a87a10b
to
e2a0127
Compare
e2a0127
to
e30203a
Compare
e30203a
to
b330cd7
Compare
returnErr := func(err error) chan error { | ||
resp := make(chan error, 1) | ||
resp <- 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.
we should close resp channel
close(resp) |
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.
it is buffered, and we either add err
or nil
so I do not think we need to close it
} | ||
asyncResp := make(chan error, 1) // Actual async response populated by goroutine below. | ||
go func() { | ||
tx, rec, err := s.txMgr.Send(ctx, candidate) |
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.
we should close asyncResp channel, to prevent routine leak
tx, rec, err := s.txMgr.Send(ctx, candidate) | |
defer close(asyncResp) |
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.
no goroutines to leak
@@ -187,7 +187,7 @@ func newMsgStreamMapper(network netconf.Network) msgStreamMapper { | |||
|
|||
func (w *Worker) newCallback( | |||
msgFilter *msgCursorFilter, | |||
sender SendFunc, | |||
sendBuffer func(context.Context, xchain.Submission) error, |
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.
sendBuffer func(context.Context, xchain.Submission) error, | |
mempoolSubmit func(context.Context, xchain.Submission) error, |
nit: using sender was a bit confusing when first looking at the code
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.
Nicely done 🚀
Co-authored-by: Khalil Claybon <[email protected]>
2a60769
to
1a58425
Compare
Fixes race condition in relayer that resulted in unordered submissions sometimes.
This refactors the
Sender
to reserve nonces synchronously, but do the actual sending asynchronously. Active buffer refactored to reserves nonces synchronously ensuring strictly ordered submissions.issue: https://github.com/omni-network/ops/issues/533