channeldb: ensure all state is deleted for a channel by CloseChannel

This commit addresses some lingering TODO’s which ensure that related
state to a channel is properly deleted by the CloseChannel method.
Previously the values for the respective dust-limits of either side,
the on-disk HTLC’s, and any entries the revocation log for the channel
weren’t being properly deleted.

Additionally, we now modify the checks within the unit tests to ensure
that we can still read the channel from disk w/o running into an error
(thought the slice will be blank), and also the the revocation log is
properly garbage collected.

channeldb/channel.go

@@ -569,7 +569,6 @@ func (c *OpenChannel) FindPreviousState(updateNum uint64) (*ChannelDelta, error)
 // entails deleting all saved state within the database concerning this
 // channel, as well as created a small channel summary for record keeping
 // purposes.
-// TODO(roasbeef): delete on-disk set of HTLCs
 func (c *OpenChannel) CloseChannel() error {
 	return c.Db.Update(func(tx *bolt.Tx) error {
 		// First fetch the top level bucket which stores all data
@@ -616,10 +615,20 @@ func (c *OpenChannel) CloseChannel() error {
 		// Now that the index to this channel has been deleted, purge
 		// the remaining channel metadata from the database.
 		if err := deleteOpenChannel(chanBucket, nodeChanBucket,
-			outPointBytes); err != nil {
+			outPointBytes, c.ChanID); err != nil {
 			return err
 		}
 
+		// With the base channel data deleted, attempt to delte the
+		// information stored within the revocation log.
+		logBucket := nodeChanBucket.Bucket(channelLogBucket)
+		if logBucket != nil {
+			err := wipeChannelLogEntries(logBucket, c.ChanID)
+			if err != nil {
+				return err
+			}
+		}
+
 		// Finally, create a summary of this channel in the closed
 		// channel bucket for this node.
 		return putClosedChannelSummary(tx, outPointBytes)
@@ -779,6 +788,7 @@ func fetchOpenChannel(openChanBucket *bolt.Bucket, nodeChanBucket *bolt.Bucket,
 	// With the existence of an open channel bucket with this node verified,
 	// perform a full read of the entire struct. Starting with the prefixed
 	// fields residing in the parent bucket.
+	// TODO(roasbeef): combine the below into channel config like key
 	if err = fetchChanCapacity(openChanBucket, channel); err != nil {
 		return nil, err
 	}
@@ -802,7 +812,7 @@ func fetchOpenChannel(openChanBucket *bolt.Bucket, nodeChanBucket *bolt.Bucket,
 }
 
 func deleteOpenChannel(openChanBucket *bolt.Bucket, nodeChanBucket *bolt.Bucket,
-	channelID []byte) error {
+	channelID []byte, o *wire.OutPoint) error {
 
 	// First we'll delete all the "common" top level items stored outside
 	// the node's channel bucket.
@@ -818,6 +828,12 @@ func deleteOpenChannel(openChanBucket *bolt.Bucket, nodeChanBucket *bolt.Bucket,
 	if err := deleteChanAmountsTransferred(openChanBucket, channelID); err != nil {
 		return err
 	}
+	if err := deleteChanTheirDustLimit(openChanBucket, channelID); err != nil {
+		return err
+	}
+	if err := deleteChanOurDustLimit(openChanBucket, channelID); err != nil {
+		return err
+	}
 
 	// Finally, delete all the fields directly within the node's channel
 	// bucket.
@@ -839,6 +855,9 @@ func deleteOpenChannel(openChanBucket *bolt.Bucket, nodeChanBucket *bolt.Bucket,
 	if err := deleteChanDeliveryScripts(nodeChanBucket, channelID); err != nil {
 		return err
 	}
+	if err := deleteCurrentHtlcs(nodeChanBucket, o); err != nil {
+		return err
+	}
 
 	return nil
 }
@@ -1004,6 +1023,14 @@ func fetchChanTheirDustLimit(openChanBucket *bolt.Bucket, channel *OpenChannel)
 	return nil
 }
 
+func deleteChanTheirDustLimit(openChanBucket *bolt.Bucket, chanID []byte) error {
+	theirDustKey := make([]byte, 3+len(chanID))
+	copy(theirDustKey, theirDustLimitPrefix)
+	copy(theirDustKey[3:], chanID)
+
+	return openChanBucket.Delete(theirDustKey)
+}
+
 func fetchChanOurDustLimit(openChanBucket *bolt.Bucket, channel *OpenChannel) error {
 	var b bytes.Buffer
 	if err := writeOutpoint(&b, channel.ChanID); err != nil {
@@ -1020,6 +1047,14 @@ func fetchChanOurDustLimit(openChanBucket *bolt.Bucket, channel *OpenChannel) er
 	return nil
 }
 
+func deleteChanOurDustLimit(openChanBucket *bolt.Bucket, chanID []byte) error {
+	ourDustKey := make([]byte, 3+len(chanID))
+	copy(ourDustKey, ourDustLimitPrefix)
+	copy(ourDustKey[3:], chanID)
+
+	return openChanBucket.Delete(ourDustKey)
+}
+
 func putChanNumUpdates(openChanBucket *bolt.Bucket, channel *OpenChannel) error {
 	scratch := make([]byte, 8)
 	byteOrder.PutUint64(scratch, channel.NumUpdates)
@@ -1708,6 +1743,11 @@ func fetchCurrentHtlcs(nodeChanBucket *bolt.Bucket,
 	return htlcs, nil
 }
 
+func deleteCurrentHtlcs(nodeChanBucket *bolt.Bucket, o *wire.OutPoint) error {
+	htlcKey := makeHtlcKey(o)
+	return nodeChanBucket.Delete(htlcKey[:])
+}
+
 func serializeChannelDelta(w io.Writer, delta *ChannelDelta) error {
 	// TODO(roasbeef): could use compression here to reduce on-disk space.
 	var scratch [8]byte
@@ -1822,6 +1862,35 @@ func fetchChannelLogEntry(log *bolt.Bucket, chanPoint *wire.OutPoint,
 	return deserializeChannelDelta(deltaReader)
 }
 
+func wipeChannelLogEntries(log *bolt.Bucket, o *wire.OutPoint) error {
+	var (
+		n         int
+		logPrefix [32 + 4]byte
+		scratch   [4]byte
+	)
+
+	// First we'll construct a key prefix that we'll use to scan through
+	// and delete all the log entries related to this channel. The format
+	// for log entries within the database is: txid || index || update_num.
+	// We'll construct a prefix key with the first two thirds of the full
+	// key to scan with and delete all entries.
+	n += copy(logPrefix[:], o.Hash[:])
+	byteOrder.PutUint32(scratch[:], o.Index)
+	copy(logPrefix[n:], scratch[:])
+
+	// With the prefix constructed, scan through the log bucket from the
+	// starting point of the log entries for this channel. We'll keep
+	// deleting keys until the prefix no longer matches.
+	logCursor := log.Cursor()
+	for logKey, _ := logCursor.Seek(logPrefix[:]); bytes.HasPrefix(logKey, logPrefix[:]); logKey, _ = logCursor.Next() {
+		if err := log.Delete(logKey); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
 func writeOutpoint(w io.Writer, o *wire.OutPoint) error {
 	// TODO(roasbeef): make all scratch buffers on the stack
 	scratch := make([]byte, 4)

channeldb/channel_test.go

@@ -279,6 +279,7 @@ func TestOpenChannelPutGetDelete(t *testing.T) {
 		t.Fatalf("redeem script doesn't match")
 	}
 
+	// The local and remote delivery scripts should be identical.
 	if !bytes.Equal(state.OurDeliveryScript, newState.OurDeliveryScript) {
 		t.Fatalf("our delivery address doesn't match")
 	}
@@ -354,8 +355,16 @@ func TestOpenChannelPutGetDelete(t *testing.T) {
 	if err != nil {
 		t.Fatalf("unable to fetch open channels: %v", err)
 	}
+	if len(openChans) != 0 {
+		t.Fatalf("all channels not deleted, found %v", len(openChans))
+	}
 
-	// TODO(roasbeef): need to assert much more
+	// Additionally, attempting to fetch all the open channels globally
+	// should yield no results.
+	openChans, err = cdb.FetchAllChannels()
+	if err != nil {
+		t.Fatalf("unable to fetch all open chans")
+	}
 	if len(openChans) != 0 {
 		t.Fatalf("all channels not deleted, found %v", len(openChans))
 	}
@@ -498,6 +507,7 @@ func TestChannelStateTransition(t *testing.T) {
 				spew.Sdump(diskHTLC))
 		}
 	}
+
 	// The revocation state stored on-disk should now also be identical.
 	updatedChannel, err = cdb.FetchOpenChannels(channel.IdentityPub)
 	if err != nil {
@@ -507,4 +517,27 @@ func TestChannelStateTransition(t *testing.T) {
 		newRevocation) {
 		t.Fatalf("revocation state wasn't synced!")
 	}
+
+	// Now attempt to delete the channel from the database.
+	if err := updatedChannel[0].CloseChannel(); err != nil {
+		t.Fatalf("unable to delete updated channel: %v", err)
+	}
+
+	// If we attempt to fetch the target channel again, it shouldn't be
+	// found.
+	channels, err := cdb.FetchOpenChannels(channel.IdentityPub)
+	if err != nil {
+		t.Fatalf("unable to fetch updated channels: %v", err)
+	}
+	if len(channels) != 0 {
+		t.Fatalf("%v channels, found, but none should be",
+			len(channels))
+	}
+
+	// Attempting to find previous states on the channel should fail as the
+	// revocation log has been deleted.
+	_, err = updatedChannel[0].FindPreviousState(uint64(delta.UpdateNum))
+	if err == nil {
+		t.Fatalf("revocation log search should've failed")
+	}
 }