Merge pull request lightningnetwork#1937 from halseth/data-loss-prote…

…ct-resending

Data loss protect resending

channeldb/channel.go

@@ -3,6 +3,7 @@ package channeldb
 import (
 	"bytes"
 	"encoding/binary"
+	"errors"
 	"fmt"
 	"io"
 	"net"
@@ -1832,6 +1833,10 @@ type ChannelCloseSummary struct {
 
 	// LocalChanCfg is the channel configuration for the local node.
 	LocalChanConfig ChannelConfig
+
+	// LastChanSyncMsg is the ChannelReestablish message for this channel
+	// for the state at the point where it was closed.
+	LastChanSyncMsg *lnwire.ChannelReestablish
 }
 
 // CloseChannel closes a previously active Lightning channel. Closing a channel
@@ -2059,7 +2064,12 @@ func serializeChannelCloseSummary(w io.Writer, cs *ChannelCloseSummary) error {
 	// If this is a close channel summary created before the addition of
 	// the new fields, then we can exit here.
 	if cs.RemoteCurrentRevocation == nil {
-		return nil
+		return WriteElements(w, false)
+	}
+
+	// If fields are present, write boolean to indicate this, and continue.
+	if err := WriteElements(w, true); err != nil {
+		return err
 	}
 
 	if err := WriteElements(w, cs.RemoteCurrentRevocation); err != nil {
@@ -2070,14 +2080,34 @@ func serializeChannelCloseSummary(w io.Writer, cs *ChannelCloseSummary) error {
 		return err
 	}
 
-	// We'll write this field last, as it's possible for a channel to be
-	// closed before we learn of the next unrevoked revocation point for
-	// the remote party.
-	if cs.RemoteNextRevocation == nil {
-		return nil
+	// The RemoteNextRevocation field is optional, as it's possible for a
+	// channel to be closed before we learn of the next unrevoked
+	// revocation point for the remote party. Write a boolen indicating
+	// whether this field is present or not.
+	if err := WriteElements(w, cs.RemoteNextRevocation != nil); err != nil {
+		return err
+	}
+
+	// Write the field, if present.
+	if cs.RemoteNextRevocation != nil {
+		if err = WriteElements(w, cs.RemoteNextRevocation); err != nil {
+			return err
+		}
+	}
+
+	// Write whether the channel sync message is present.
+	if err := WriteElements(w, cs.LastChanSyncMsg != nil); err != nil {
+		return err
+	}
+
+	// Write the channel sync message, if present.
+	if cs.LastChanSyncMsg != nil {
+		if err := WriteElements(w, cs.LastChanSyncMsg); err != nil {
+			return err
+		}
 	}
 
-	return WriteElements(w, cs.RemoteNextRevocation)
+	return nil
 }
 
 func fetchChannelCloseSummary(tx *bolt.Tx,
@@ -2111,15 +2141,19 @@ func deserializeCloseChannelSummary(r io.Reader) (*ChannelCloseSummary, error) {
 
 	// We'll now check to see if the channel close summary was encoded with
 	// any of the additional optional fields.
-	err = ReadElements(r, &c.RemoteCurrentRevocation)
-	switch {
-	case err == io.EOF:
+	var hasNewFields bool
+	err = ReadElements(r, &hasNewFields)
+	if err != nil {
+		return nil, err
+	}
+
+	// If fields are not present, we can return.
+	if !hasNewFields {
 		return c, nil
+	}
 
-	// If we got a non-eof error, then we know there's an actually issue.
-	// Otherwise, it may have been the case that this summary didn't have
-	// the set of optional fields.
-	case err != nil:
+	// Otherwise read the new fields.
+	if err := ReadElements(r, &c.RemoteCurrentRevocation); err != nil {
 		return nil, err
 	}
 
@@ -2129,17 +2163,48 @@ func deserializeCloseChannelSummary(r io.Reader) (*ChannelCloseSummary, error) {
 
 	// Finally, we'll attempt to read the next unrevoked commitment point
 	// for the remote party. If we closed the channel before receiving a
-	// funding locked message, then this can be nil. As a result, we'll use
-	// the same technique to read the field, only if there's still data
-	// left in the buffer.
-	err = ReadElements(r, &c.RemoteNextRevocation)
-	if err != nil && err != io.EOF {
-		// If we got a non-eof error, then we know there's an actually
-		// issue. Otherwise, it may have been the case that this
-		// summary didn't have the set of optional fields.
+	// funding locked message then this might not be present. A boolean
+	// indicating whether the field is present will come first.
+	var hasRemoteNextRevocation bool
+	err = ReadElements(r, &hasRemoteNextRevocation)
+	if err != nil {
+		return nil, err
+	}
+
+	// If this field was written, read it.
+	if hasRemoteNextRevocation {
+		err = ReadElements(r, &c.RemoteNextRevocation)
+		if err != nil {
+			return nil, err
+		}
+	}
+
+	// Check if we have a channel sync message to read.
+	var hasChanSyncMsg bool
+	err = ReadElements(r, &hasChanSyncMsg)
+	if err == io.EOF {
+		return c, nil
+	} else if err != nil {
 		return nil, err
 	}
 
+	// If a chan sync message is present, read it.
+	if hasChanSyncMsg {
+		// We must pass in reference to a lnwire.Message for the codec
+		// to support it.
+		var msg lnwire.Message
+		if err := ReadElements(r, &msg); err != nil {
+			return nil, err
+		}
+
+		chanSync, ok := msg.(*lnwire.ChannelReestablish)
+		if !ok {
+			return nil, errors.New("unable cast db Message to " +
+				"ChannelReestablish")
+		}
+		c.LastChanSyncMsg = chanSync
+	}
+
 	return c, nil
 }
 

channeldb/channel_test.go

@@ -248,10 +248,10 @@ func TestOpenChannelPutGetDelete(t *testing.T) {
 	t.Parallel()
 
 	cdb, cleanUp, err := makeTestDB()
-	defer cleanUp()
 	if err != nil {
 		t.Fatalf("unable to make test database: %v", err)
 	}
+	defer cleanUp()
 
 	// Create the test channel state, then add an additional fake HTLC
 	// before syncing to disk.
@@ -368,10 +368,10 @@ func TestChannelStateTransition(t *testing.T) {
 	t.Parallel()
 
 	cdb, cleanUp, err := makeTestDB()
-	defer cleanUp()
 	if err != nil {
 		t.Fatalf("unable to make test database: %v", err)
 	}
+	defer cleanUp()
 
 	// First create a minimal channel, then perform a full sync in order to
 	// persist the data.

channeldb/db.go

@@ -12,6 +12,7 @@ import (
 	"github.com/btcsuite/btcd/wire"
 	"github.com/coreos/bbolt"
 	"github.com/go-errors/errors"
+	"github.com/lightningnetwork/lnd/lnwire"
 )
 
 const (
@@ -80,6 +81,13 @@ var (
 			number:    6,
 			migration: migratePruneEdgeUpdateIndex,
 		},
+		{
+			// The DB version that migrates the ChannelCloseSummary
+			// to a format where optional fields are indicated with
+			// boolean flags.
+			number:    7,
+			migration: migrateOptionalChannelCloseSummaryFields,
+		},
 	}
 
 	// Big endian is the preferred byte order, due to cursor scans over
@@ -609,6 +617,54 @@ func (d *DB) FetchClosedChannel(chanID *wire.OutPoint) (*ChannelCloseSummary, er
 	return chanSummary, nil
 }
 
+// FetchClosedChannelForID queries for a channel close summary using the
+// channel ID of the channel in question.
+func (d *DB) FetchClosedChannelForID(cid lnwire.ChannelID) (
+	*ChannelCloseSummary, error) {
+
+	var chanSummary *ChannelCloseSummary
+	if err := d.View(func(tx *bolt.Tx) error {
+		closeBucket := tx.Bucket(closedChannelBucket)
+		if closeBucket == nil {
+			return ErrClosedChannelNotFound
+		}
+
+		// The first 30 bytes of the channel ID and outpoint will be
+		// equal.
+		cursor := closeBucket.Cursor()
+		op, c := cursor.Seek(cid[:30])
+
+		// We scan over all possible candidates for this channel ID.
+		for ; op != nil && bytes.Compare(cid[:30], op[:30]) <= 0; op, c = cursor.Next() {
+			var outPoint wire.OutPoint
+			err := readOutpoint(bytes.NewReader(op), &outPoint)
+			if err != nil {
+				return err
+			}
+
+			// If the found outpoint does not correspond to this
+			// channel ID, we continue.
+			if !cid.IsChanPoint(&outPoint) {
+				continue
+			}
+
+			// Deserialize the close summary and return.
+			r := bytes.NewReader(c)
+			chanSummary, err = deserializeCloseChannelSummary(r)
+			if err != nil {
+				return err
+			}
+
+			return nil
+		}
+		return ErrClosedChannelNotFound
+	}); err != nil {
+		return nil, err
+	}
+
+	return chanSummary, nil
+}
+
 // MarkChanFullyClosed marks a channel as fully closed within the database. A
 // channel should be marked as fully closed if the channel was initially
 // cooperatively closed and it's reached a single confirmation, or after all

channeldb/db_test.go

@@ -5,6 +5,9 @@ import (
 	"os"
 	"path/filepath"
 	"testing"
+
+	"github.com/btcsuite/btcutil"
+	"github.com/lightningnetwork/lnd/lnwire"
 )
 
 func TestOpenWithCreate(t *testing.T) {
@@ -71,3 +74,76 @@ func TestWipe(t *testing.T) {
 			ErrNoClosedChannels, err)
 	}
 }
+
+// TestFetchClosedChannelForID tests that we are able to properly retrieve a
+// ChannelCloseSummary from the DB given a ChannelID.
+func TestFetchClosedChannelForID(t *testing.T) {
+	t.Parallel()
+
+	const numChans = 101
+
+	cdb, cleanUp, err := makeTestDB()
+	if err != nil {
+		t.Fatalf("unable to make test database: %v", err)
+	}
+	defer cleanUp()
+
+	// Create the test channel state, that we will mutate the index of the
+	// funding point.
+	state, err := createTestChannelState(cdb)
+	if err != nil {
+		t.Fatalf("unable to create channel state: %v", err)
+	}
+
+	// Now run through the number of channels, and modify the outpoint index
+	// to create new channel IDs.
+	for i := uint32(0); i < numChans; i++ {
+		// Save the open channel to disk.
+		state.FundingOutpoint.Index = i
+		if err := state.FullSync(); err != nil {
+			t.Fatalf("unable to save and serialize channel "+
+				"state: %v", err)
+		}
+
+		// Close the channel. To make sure we retrieve the correct
+		// summary later, we make them differ in the SettledBalance.
+		closeSummary := &ChannelCloseSummary{
+			ChanPoint:      state.FundingOutpoint,
+			RemotePub:      state.IdentityPub,
+			SettledBalance: btcutil.Amount(500 + i),
+		}
+		if err := state.CloseChannel(closeSummary); err != nil {
+			t.Fatalf("unable to close channel: %v", err)
+		}
+	}
+
+	// Now run though them all again and make sure we are able to retrieve
+	// summaries from the DB.
+	for i := uint32(0); i < numChans; i++ {
+		state.FundingOutpoint.Index = i
+
+		// We calculate the ChannelID and use it to fetch the summary.
+		cid := lnwire.NewChanIDFromOutPoint(&state.FundingOutpoint)
+		fetchedSummary, err := cdb.FetchClosedChannelForID(cid)
+		if err != nil {
+			t.Fatalf("unable to fetch close summary: %v", err)
+		}
+
+		// Make sure we retrieved the correct one by checking the
+		// SettledBalance.
+		if fetchedSummary.SettledBalance != btcutil.Amount(500+i) {
+			t.Fatalf("summaries don't match: expected %v got %v",
+				btcutil.Amount(500+i),
+				fetchedSummary.SettledBalance)
+		}
+	}
+
+	// As a final test we make sure that we get ErrClosedChannelNotFound
+	// for a ChannelID we didn't add to the DB.
+	state.FundingOutpoint.Index++
+	cid := lnwire.NewChanIDFromOutPoint(&state.FundingOutpoint)
+	_, err = cdb.FetchClosedChannelForID(cid)
+	if err != ErrClosedChannelNotFound {
+		t.Fatalf("expected ErrClosedChannelNotFound, instead got: %v", err)
+	}
+}

channeldb/legacy_serialization.go

@@ -0,0 +1,53 @@
+package channeldb
+
+import "io"
+
+// deserializeCloseChannelSummaryV6 reads the v6 database format for
+// ChannelCloseSummary.
+//
+// NOTE: deprecated, only for migration.
+func deserializeCloseChannelSummaryV6(r io.Reader) (*ChannelCloseSummary, error) {
+	c := &ChannelCloseSummary{}
+
+	err := ReadElements(r,
+		&c.ChanPoint, &c.ShortChanID, &c.ChainHash, &c.ClosingTXID,
+		&c.CloseHeight, &c.RemotePub, &c.Capacity, &c.SettledBalance,
+		&c.TimeLockedBalance, &c.CloseType, &c.IsPending,
+	)
+	if err != nil {
+		return nil, err
+	}
+
+	// We'll now check to see if the channel close summary was encoded with
+	// any of the additional optional fields.
+	err = ReadElements(r, &c.RemoteCurrentRevocation)
+	switch {
+	case err == io.EOF:
+		return c, nil
+
+	// If we got a non-eof error, then we know there's an actually issue.
+	// Otherwise, it may have been the case that this summary didn't have
+	// the set of optional fields.
+	case err != nil:
+		return nil, err
+	}
+
+	if err := readChanConfig(r, &c.LocalChanConfig); err != nil {
+		return nil, err
+	}
+
+	// Finally, we'll attempt to read the next unrevoked commitment point
+	// for the remote party. If we closed the channel before receiving a
+	// funding locked message, then this can be nil. As a result, we'll use
+	// the same technique to read the field, only if there's still data
+	// left in the buffer.
+	err = ReadElements(r, &c.RemoteNextRevocation)
+	if err != nil && err != io.EOF {
+		// If we got a non-eof error, then we know there's an actually
+		// issue. Otherwise, it may have been the case that this
+		// summary didn't have the set of optional fields.
+		return nil, err
+	}
+
+	return c, nil
+}