diff --git a/watchtower/wtclient/client_test.go b/watchtower/wtclient/client_test.go index dc1d87842..7be616d77 100644 --- a/watchtower/wtclient/client_test.go +++ b/watchtower/wtclient/client_test.go @@ -2014,14 +2014,12 @@ var clientTests = []clientTest{ }, }, { - // Demonstrate that the client is unable to upload state updates - // to a tower if the client deletes its database after already - // having created and started to use a session with a tower. - // This happens because the session key is generated - // deterministically and will only be unique for new sessions - // if the same DB is used. The server therefore rejects these - // updates with the StateUpdateCodeClientBehind error. - name: "demonstrate the StateUpdateCodeClientBehind error", + // Demonstrate that the client is unable to recover after + // deleting its database by skipping through key indices until + // it gets to one that does not result in the + // CreateSessionCodeAlreadyExists error code being returned from + // the server. + name: "continue after client database deletion", cfg: harnessCfg{ localBalance: localBalance, remoteBalance: remoteBalance, @@ -2063,9 +2061,8 @@ var clientTests = []clientTest{ // Attempt to back up the remaining tasks. h.backupStates(chanID, numUpdates/2, numUpdates, nil) - // Show that the server does not get the remaining - // updates. - h.waitServerUpdates(nil, waitTime) + // Show that the server does get the remaining updates. + h.waitServerUpdates(hints[numUpdates/2:], waitTime) }, }, } diff --git a/watchtower/wtclient/errors.go b/watchtower/wtclient/errors.go index c6884bb35..2cb070ac4 100644 --- a/watchtower/wtclient/errors.go +++ b/watchtower/wtclient/errors.go @@ -34,4 +34,9 @@ var ( // revoked state because the channel had not been previously registered // with the client. ErrUnregisteredChannel = errors.New("channel is not registered") + + // ErrSessionKeyAlreadyUsed indicates that the client attempted to + // create a new session with a tower with a session key that has already + // been used in the past. + ErrSessionKeyAlreadyUsed = errors.New("session key already used") ) diff --git a/watchtower/wtclient/session_negotiator.go b/watchtower/wtclient/session_negotiator.go index aafc44b2a..d137d45e1 100644 --- a/watchtower/wtclient/session_negotiator.go +++ b/watchtower/wtclient/session_negotiator.go @@ -1,6 +1,7 @@ package wtclient import ( + "errors" "fmt" "sync" "time" @@ -272,6 +273,7 @@ retryWithBackoff: } } +tryNextCandidate: for { select { case <-n.quit: @@ -302,28 +304,39 @@ retryWithBackoff: n.log.Debugf("Attempting session negotiation with tower=%x", towerPub) - // Before proceeding, we will reserve a session key index to use - // with this specific tower. If one is already reserved, the - // existing index will be returned. - keyIndex, err := n.cfg.DB.NextSessionKeyIndex( - tower.ID, n.cfg.Policy.BlobType, false, - ) - if err != nil { - n.log.Debugf("Unable to reserve session key index "+ - "for tower=%x: %v", towerPub, err) - continue - } + var forceNextKey bool + for { + // Before proceeding, we will reserve a session key + // index to use with this specific tower. If one is + // already reserved, the existing index will be + // returned. + keyIndex, err := n.cfg.DB.NextSessionKeyIndex( + tower.ID, n.cfg.Policy.BlobType, forceNextKey, + ) + if err != nil { + n.log.Debugf("Unable to reserve session key "+ + "index for tower=%x: %v", towerPub, err) - // We'll now attempt the CreateSession dance with the tower to - // get a new session, trying all addresses if necessary. - err = n.createSession(tower, keyIndex) - if err != nil { - // An unexpected error occurred, updpate our backoff. + goto tryNextCandidate + } + + // We'll now attempt the CreateSession dance with the + // tower to get a new session, trying all addresses if + // necessary. + err = n.createSession(tower, keyIndex) + if err == nil { + return + } else if errors.Is(err, ErrSessionKeyAlreadyUsed) { + forceNextKey = true + continue + } + + // An unexpected error occurred, update our backoff. updateBackoff() n.log.Debugf("Session negotiation with tower=%x "+ - "failed, trying again -- reason: %v", - tower.IdentityKey.SerializeCompressed(), err) + "failed, trying again -- reason: %v", towerPub, + err) goto retryWithBackoff } @@ -360,7 +373,10 @@ func (n *sessionNegotiator) createSession(tower *Tower, keyIndex uint32) error { err = n.tryAddress(sessionKey, keyIndex, tower, lnAddr) tower.Addresses.ReleaseLock(addr) switch { - case err == ErrPermanentTowerFailure: + case errors.Is(err, ErrSessionKeyAlreadyUsed): + return err + + case errors.Is(err, ErrPermanentTowerFailure): // TODO(conner): report to iterator? can then be reset // with restart fallthrough @@ -454,12 +470,7 @@ func (n *sessionNegotiator) tryAddress(sessionKey keychain.SingleKeyECDH, } switch createSessionReply.Code { - case wtwire.CodeOK, wtwire.CreateSessionCodeAlreadyExists: - - // TODO(conner): add last-applied to create session reply to - // handle case where we lose state, session already exists, and - // we want to possibly resume using the session - + case wtwire.CodeOK: // TODO(conner): validate reward address rewardPkScript := createSessionReply.Data @@ -500,6 +511,16 @@ func (n *sessionNegotiator) tryAddress(sessionKey keychain.SingleKeyECDH, return ErrNegotiatorExiting } + case wtwire.CreateSessionCodeAlreadyExists: + // TODO(conner): use the last-applied in the create session + // reply to handle case where we lose state, session already + // exists, and we want to possibly resume using the session. + // NOTE that this should not be done until the server code + // has been adapted to first check that the CreateSession + // request is for the same blob-type as the initial session. + + return ErrSessionKeyAlreadyUsed + // TODO(conner): handle error codes properly case wtwire.CreateSessionCodeRejectBlobType: return fmt.Errorf("tower rejected blob type: %v",