Skip to content

Commit 19f09f6

Browse files
ViktorT-11ViktorT-11
authored
Merge pull request #1353 from bitromortac/2607-itest-flake
terminal: gate wallet-ready status on lnd's actual RPC readiness
2 parents 8efa0e0 + 9370a6a commit 19f09f6

5 files changed

Lines changed: 231 additions & 8 deletions

File tree

‎docs/release-notes/release-notes-0.17.0.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,13 @@
3333
expiration date would overwrite it to 0 (never expires). It now correctly
3434
defaults to -1 (no change).
3535

36+
* [Gate wallet-ready status on lnd's actual RPC
37+
readiness](https://github.com/lightninglabs/lightning-terminal/pull/1353):
38+
Fixed a startup race where litd could report the LND sub-server as "Wallet
39+
Ready" before lnd's RPC interceptor had actually left its
40+
`WAITING_TO_START` state, so the very next call could still fail with
41+
`rpc error: ... waiting to start`.
42+
3643
### Functional Changes/Additions
3744

3845
* [Support for SQL database

‎itest/litd_node.go‎

Lines changed: 33 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -899,31 +899,56 @@ func (hn *HarnessNode) WaitUntilStarted(conn grpc.ClientConnInterface,
899899
return err
900900
}
901901

902+
// LiT itself only reports Running once it has finished baking
903+
// and writing its default macaroons to disk, so waiting for
904+
// this closes the race between that and callers that read
905+
// LitMacPath straight off disk right after we return.
906+
litStatus, ok := states.SubServers[subservers.LIT]
907+
if !ok || !litStatus.Running {
908+
return fmt.Errorf("LiT has not yet started")
909+
}
910+
902911
if faradayMode != terminal.ModeDisable {
903912
faraday, ok := states.SubServers[subservers.FARADAY]
904-
if !ok || !faraday.Running {
905-
return fmt.Errorf("faraday has not yet started")
913+
if !ok {
914+
return fmt.Errorf("faraday status not found")
915+
}
916+
if faraday.Error != "" {
917+
return fmt.Errorf("faraday failed to "+
918+
"start: %s", faraday.Error)
906919
}
907920
}
908921

909922
if loopMode != terminal.ModeDisable {
910923
loop, ok := states.SubServers[subservers.LOOP]
911-
if !ok || !loop.Running {
912-
return fmt.Errorf("loop has not yet started")
924+
if !ok {
925+
return fmt.Errorf("loop status not found")
926+
}
927+
if loop.Error != "" {
928+
return fmt.Errorf("loop failed to "+
929+
"start: %s", loop.Error)
913930
}
914931
}
915932

916933
if poolMode != terminal.ModeDisable {
917934
pool, ok := states.SubServers[subservers.POOL]
918-
if !ok || !pool.Running {
919-
return fmt.Errorf("pool has not yet started")
935+
if !ok {
936+
return fmt.Errorf("pool status not found")
937+
}
938+
if pool.Error != "" {
939+
return fmt.Errorf("pool failed to "+
940+
"start: %s", pool.Error)
920941
}
921942
}
922943

923944
if tapMode != terminal.ModeDisable {
924945
tap, ok := states.SubServers[subservers.TAP]
925-
if !ok || !tap.Running {
926-
return fmt.Errorf("tap has not yet started")
946+
if !ok {
947+
return fmt.Errorf("tap status not found")
948+
}
949+
if tap.Error != "" {
950+
return fmt.Errorf("tap failed to "+
951+
"start: %s", tap.Error)
927952
}
928953
}
929954

‎lnd_ready.go‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
package terminal
2+
3+
import (
4+
"context"
5+
"fmt"
6+
"time"
7+
8+
"github.com/lightningnetwork/lnd/lnrpc"
9+
)
10+
11+
// waitForLndRPCReady polls lnd's StateService until lnd's RPC interceptor has
12+
// left the WAITING_TO_START state, or timeout elapses. Unlike every other
13+
// lnd RPC, the StateService is exempt from both lnd's macaroon check and its
14+
// RPC-readiness check (see lnd's rpcperms.InterceptorChain), so it can be
15+
// queried on a fresh connection before the wallet is unlocked and before any
16+
// macaroon exists. This lets us distinguish lnd's gRPC listener merely being
17+
// bound (which is all readyChan/unlockChan guarantee) from lnd actually being
18+
// able to service non-State RPCs.
19+
func waitForLndRPCReady(ctx context.Context, stateClient lnrpc.StateClient,
20+
timeout time.Duration) error {
21+
22+
timer := time.NewTimer(timeout)
23+
defer timer.Stop()
24+
25+
var lastErr error
26+
27+
for {
28+
resp, err := stateClient.GetState(ctx, &lnrpc.GetStateRequest{})
29+
switch {
30+
case err != nil:
31+
lastErr = err
32+
33+
case resp.State != lnrpc.WalletState_WAITING_TO_START:
34+
return nil
35+
}
36+
37+
select {
38+
case <-time.After(stateServicePollInterval):
39+
40+
case <-timer.C:
41+
if lastErr != nil {
42+
return fmt.Errorf("lnd's RPC interceptor "+
43+
"did not leave WAITING_TO_START "+
44+
"within %v: %w", timeout, lastErr)
45+
}
46+
47+
return fmt.Errorf("lnd's RPC interceptor did not "+
48+
"leave WAITING_TO_START within %v", timeout)
49+
50+
case <-ctx.Done():
51+
return ctx.Err()
52+
}
53+
}
54+
}

‎lnd_ready_test.go‎

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
package terminal
2+
3+
import (
4+
"context"
5+
"errors"
6+
"testing"
7+
"time"
8+
9+
"github.com/lightningnetwork/lnd/lnrpc"
10+
"github.com/stretchr/testify/require"
11+
"google.golang.org/grpc"
12+
)
13+
14+
// fakeStateClient is a test-only lnrpc.StateClient that replays a canned
15+
// sequence of GetState responses/errors, repeating the last entry once the
16+
// sequence is exhausted.
17+
type fakeStateClient struct {
18+
lnrpc.StateClient
19+
20+
responses []*lnrpc.GetStateResponse
21+
errs []error
22+
calls int
23+
}
24+
25+
func (f *fakeStateClient) GetState(_ context.Context,
26+
_ *lnrpc.GetStateRequest, _ ...grpc.CallOption) (
27+
*lnrpc.GetStateResponse, error) {
28+
29+
idx := f.calls
30+
if idx >= len(f.responses) {
31+
idx = len(f.responses) - 1
32+
}
33+
f.calls++
34+
35+
return f.responses[idx], f.errs[idx]
36+
}
37+
38+
// TestWaitForLndRPCReady asserts that waitForLndRPCReady correctly polls
39+
// lnd's StateService until it reports a state other than WAITING_TO_START,
40+
// and that it times out with an error if that never happens.
41+
func TestWaitForLndRPCReady(t *testing.T) {
42+
t.Parallel()
43+
44+
activeResp := &lnrpc.GetStateResponse{
45+
State: lnrpc.WalletState_RPC_ACTIVE,
46+
}
47+
waitingResp := &lnrpc.GetStateResponse{
48+
State: lnrpc.WalletState_WAITING_TO_START,
49+
}
50+
51+
testCases := []struct {
52+
name string
53+
client *fakeStateClient
54+
timeout time.Duration
55+
expectError bool
56+
}{
57+
{
58+
name: "ready immediately",
59+
client: &fakeStateClient{
60+
responses: []*lnrpc.GetStateResponse{
61+
activeResp,
62+
},
63+
errs: []error{nil},
64+
},
65+
timeout: time.Second,
66+
},
67+
{
68+
name: "ready after N polls",
69+
client: &fakeStateClient{
70+
responses: []*lnrpc.GetStateResponse{
71+
waitingResp, waitingResp, activeResp,
72+
},
73+
errs: []error{nil, nil, nil},
74+
},
75+
timeout: time.Second,
76+
},
77+
{
78+
name: "timeout without transition",
79+
client: &fakeStateClient{
80+
responses: []*lnrpc.GetStateResponse{
81+
waitingResp,
82+
},
83+
errs: []error{nil},
84+
},
85+
timeout: 300 * time.Millisecond,
86+
expectError: true,
87+
},
88+
{
89+
name: "timeout while erroring",
90+
client: &fakeStateClient{
91+
responses: []*lnrpc.GetStateResponse{nil},
92+
errs: []error{
93+
errors.New("connection refused"),
94+
},
95+
},
96+
timeout: 300 * time.Millisecond,
97+
expectError: true,
98+
},
99+
}
100+
101+
for _, tc := range testCases {
102+
tc := tc
103+
t.Run(tc.name, func(t *testing.T) {
104+
t.Parallel()
105+
106+
err := waitForLndRPCReady(
107+
context.Background(), tc.client, tc.timeout,
108+
)
109+
110+
if tc.expectError {
111+
require.Error(t, err)
112+
return
113+
}
114+
require.NoError(t, err)
115+
})
116+
}
117+
}

‎terminal.go‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,10 @@ const (
8787
defaultRPCTimeout = 3 * time.Minute
8888
minimumRPCTimeout = 30 * time.Second
8989
defaultStartupTimeout = 5 * time.Second
90+
91+
// stateServicePollInterval is how often we poll lnd's StateService
92+
// while waiting for its RPC interceptor to leave WAITING_TO_START.
93+
stateServicePollInterval = 200 * time.Millisecond
9094
)
9195

9296
// restRegistration is a function type that represents a REST proxy
@@ -673,6 +677,22 @@ func (g *LightningTerminal) start(ctx context.Context) error {
673677
err)
674678
}
675679

680+
// The unlockChan/readyChan signal we waited on above only guarantees
681+
// that lnd's gRPC listener socket is bound, not that lnd's RPC
682+
// interceptor has advanced far enough to service non-State RPCs. Wait
683+
// for that here so that the "Wallet Ready" status set below is not
684+
// observed before it's actually true.
685+
lndStateClient := lnrpc.NewStateClient(g.lndConn)
686+
if err := waitForLndRPCReady(
687+
ctx, lndStateClient, defaultConnectTimeout,
688+
); err != nil {
689+
g.statusMgr.SetErrored(
690+
subservers.LND, "lnd RPC not ready: %v", err,
691+
)
692+
693+
return fmt.Errorf("lnd RPC not ready: %v", err)
694+
}
695+
676696
// We now set a custom status for the LND sub-server to indicate that
677697
// the wallet is ready.
678698
// This is done _before_ we have set up the lnd clients so that the

0 commit comments

Comments
 (0)