Skip to content

loop: wait for chain notifier server to start #377

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Apr 29, 2021
Merged

loop: wait for chain notifier server to start #377

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 32 additions & 4 deletions executor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package loop
import (
"context"
"fmt"
"strings"
"sync"
"sync/atomic"
"time"
Expand Down Expand Up @@ -52,10 +53,37 @@ func newExecutor(cfg *executorConfig) *executor {
func (s *executor) run(mainCtx context.Context,
statusChan chan<- SwapInfo) error {

blockEpochChan, blockErrorChan, err :=
s.lnd.ChainNotifier.RegisterBlockEpochNtfn(mainCtx)
if err != nil {
return err
var (
err error
blockEpochChan <-chan int32
blockErrorChan <-chan error
)

for {
blockEpochChan, blockErrorChan, err =
s.lnd.ChainNotifier.RegisterBlockEpochNtfn(mainCtx)
if err != nil {
if strings.Contains(err.Error(),
"in the process of starting") {

log.Warnf("LND chain notifier server not " +
Copy link
Contributor

@carlaKC carlaKC Apr 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: could just be Warn, but don't push again for this!

"ready yet, retrying with delay")

// Give chain notifier some time to start and
// try to re-attempt block epoch subscription.
select {
case <-time.After(500 * time.Millisecond):
continue
Copy link
Contributor

@carlaKC carlaKC Apr 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps add a log here that we're waiting for the chain notifier? Otherwise loop will just be hanging and user may not know why?

Copy link
Contributor

@guggero guggero Apr 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Logging sounds good to me. And maybe increase the delay to 500ms to not spam lnd too much? It's got enough on its plate at this moment of startup anyway 😂

Copy link
Member Author

@bhandras bhandras Apr 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah the 50ms was a bit overkill :) Log added.


case <-mainCtx.Done():
return err
}
}

return err
}

break
Comment on lines +83 to +86
Copy link
Contributor

@carlaKC carlaKC Apr 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to break? Couldn't we just return err here? Either it's nil or it's a non-"process of starting" error.

Copy link
Contributor

@guggero guggero Apr 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function continues below. So getting here means we've succeeded with the connection.

}

// Before starting, make sure we have an up to date block height.
Expand Down