htlcswitch+lnwallet+channeldb: invalid sig fix

channeldb/channel.go

@@ -66,6 +66,11 @@ var (
 	// party.
 	chanCommitmentKey = []byte("chan-commitment-key")
 
+	// unsignedAckedUpdatesKey is an entry in the channel bucket that
+	// contains the remote updates that we have acked, but not yet signed
+	// for in one of our remote commits.
+	unsignedAckedUpdatesKey = []byte("unsigned-acked-updates-key")
+
 	// revocationStateKey stores their current revocation hash, our
 	// preimage producer and their preimage store.
 	revocationStateKey = []byte("revocation-state-key")
@@ -1242,9 +1247,14 @@ func syncNewChannel(tx *bbolt.Tx, c *OpenChannel, addrs []net.Addr) error {
 // UpdateCommitment updates the local commitment state. It locks in the pending
 // local updates that were received by us from the remote party. The commitment
 // state completely describes the balance state at this point in the commitment
-// chain. This method its to be called when we revoke our prior commitment
+// chain. In addition to that, it persists all the remote log updates that we
+// have acked, but not signed a remote commitment for yet. These need to be
+// persisted to be able to produce a valid commit signature if a restart would
+// occur. This method its to be called when we revoke our prior commitment
 // state.
-func (c *OpenChannel) UpdateCommitment(newCommitment *ChannelCommitment) error {
+func (c *OpenChannel) UpdateCommitment(newCommitment *ChannelCommitment,
+	unsignedAckedUpdates []LogUpdate) error {
+
 	c.Lock()
 	defer c.Unlock()
 
@@ -1287,6 +1297,20 @@ func (c *OpenChannel) UpdateCommitment(newCommitment *ChannelCommitment) error {
 				"revocations: %v", err)
 		}
 
+		// Persist unsigned but acked remote updates that need to be
+		// restored after a restart.
+		var b bytes.Buffer
+		err = serializeLogUpdates(&b, unsignedAckedUpdates)
+		if err != nil {
+			return err
+		}
+
+		err = chanBucket.Put(unsignedAckedUpdatesKey, b.Bytes())
+		if err != nil {
+			return fmt.Errorf("unable to store dangline remote "+
+				"updates: %v", err)
+		}
+
 		return nil
 	})
 	if err != nil {
@@ -1751,6 +1775,14 @@ func (c *OpenChannel) AppendRemoteCommitChain(diff *CommitDiff) error {
 			return err
 		}
 
+		// Clear unsigned acked remote updates. We are signing now for
+		// all that we've got.
+		err = chanBucket.Delete(unsignedAckedUpdatesKey)
+		if err != nil {
+			return fmt.Errorf("unable to clear dangling remote "+
+				"updates: %v", err)
+		}
+
 		// TODO(roasbeef): use seqno to derive key for later LCP
 
 		// With the bucket retrieved, we'll now serialize the commit
@@ -1804,6 +1836,38 @@ func (c *OpenChannel) RemoteCommitChainTip() (*CommitDiff, error) {
 	return cd, err
 }
 
+// UnsignedAckedUpdates retrieves the persisted unsigned acked remote log
+// updates that still need to be signed for.
+func (c *OpenChannel) UnsignedAckedUpdates() ([]LogUpdate, error) {
+	var updates []LogUpdate
+	err := c.Db.View(func(tx *bbolt.Tx) error {
+		chanBucket, err := fetchChanBucket(
+			tx, c.IdentityPub, &c.FundingOutpoint, c.ChainHash,
+		)
+		switch err {
+		case nil:
+		case ErrNoChanDBExists, ErrNoActiveChannels, ErrChannelNotFound:
+			return nil
+		default:
+			return err
+		}
+
+		updateBytes := chanBucket.Get(unsignedAckedUpdatesKey)
+		if updateBytes == nil {
+			return nil
+		}
+
+		r := bytes.NewReader(updateBytes)
+		updates, err = deserializeLogUpdates(r)
+		return err
+	})
+	if err != nil {
+		return nil, err
+	}
+
+	return updates, nil
+}
+
 // InsertNextRevocation inserts the _next_ commitment point (revocation) into
 // the database, and also modifies the internal RemoteNextRevocation attribute
 // to point to the passed key. This method is to be using during final channel

channeldb/channel_test.go

@@ -524,10 +524,34 @@ func TestChannelStateTransition(t *testing.T) {
 	// First update the local node's broadcastable state and also add a
 	// CommitDiff remote node's as well in order to simulate a proper state
 	// transition.
-	if err := channel.UpdateCommitment(&commitment); err != nil {
+	unsignedAckedUpdates := []LogUpdate{
+		{
+			LogIndex: 2,
+			UpdateMsg: &lnwire.UpdateAddHTLC{
+				ChanID: lnwire.ChannelID{1, 2, 3},
+			},
+		},
+	}
+
+	err = channel.UpdateCommitment(&commitment, unsignedAckedUpdates)
+	if err != nil {
 		t.Fatalf("unable to update commitment: %v", err)
 	}
 
+	// Assert that update is correctly written to the database.
+	dbUnsignedAckedUpdates, err := channel.UnsignedAckedUpdates()
+	if err != nil {
+		t.Fatalf("unable to fetch dangling remote updates: %v", err)
+	}
+	if len(dbUnsignedAckedUpdates) != 1 {
+		t.Fatalf("unexpected number of dangling remote updates")
+	}
+	if !reflect.DeepEqual(
+		dbUnsignedAckedUpdates[0], unsignedAckedUpdates[0],
+	) {
+		t.Fatalf("unexpected update")
+	}
+
 	// The balances, new update, the HTLCs and the changes to the fake
 	// commitment transaction along with the modified signature should all
 	// have been updated.

channeldb/db_test.go

@@ -403,7 +403,7 @@ func TestRestoreChannelShells(t *testing.T) {
 	// Ensure that it isn't possible to modify the commitment state machine
 	// of this restored channel.
 	channel := nodeChans[0]
-	err = channel.UpdateCommitment(nil)
+	err = channel.UpdateCommitment(nil, nil)
 	if err != ErrNoRestoredChannelMutation {
 		t.Fatalf("able to mutate restored channel")
 	}