From f98f4525287d74fe76e6b46f8d4c6115a11920fc Mon Sep 17 00:00:00 2001
From: "Johan T. Halseth" <johanth@gmail.com>
Date: Wed, 12 Jun 2019 14:50:59 +0200
Subject: [PATCH] channeldb: avoid modifying bucket during cursor traversal

This is not safe according to bbolt documentation.
---
 channeldb/channel.go | 17 -----------------
 channeldb/graph.go   | 21 ++++++++++++++++++---
 2 files changed, 18 insertions(+), 20 deletions(-)

diff --git a/channeldb/channel.go b/channeldb/channel.go
index 9d7fccc0..341a8698 100644
--- a/channeldb/channel.go
+++ b/channeldb/channel.go
@@ -2087,10 +2087,6 @@ func (c *OpenChannel) CloseChannel(summary *ChannelCloseSummary) error {
 		// information stored within the revocation log.
 		logBucket := chanBucket.Bucket(revocationLogBucket)
 		if logBucket != nil {
-			err := wipeChannelLogEntries(logBucket)
-			if err != nil {
-				return err
-			}
 			err = chanBucket.DeleteBucket(revocationLogBucket)
 			if err != nil {
 				return err
@@ -2713,16 +2709,3 @@ func fetchChannelLogEntry(log *bbolt.Bucket,
 	commitReader := bytes.NewReader(commitBytes)
 	return deserializeChanCommit(commitReader)
 }
-
-func wipeChannelLogEntries(log *bbolt.Bucket) error {
-	// TODO(roasbeef): comment
-
-	logCursor := log.Cursor()
-	for k, _ := logCursor.First(); k != nil; k, _ = logCursor.Next() {
-		if err := logCursor.Delete(); err != nil {
-			return err
-		}
-	}
-
-	return nil
-}
diff --git a/channeldb/graph.go b/channeldb/graph.go
index 79258397..8cf14985 100644
--- a/channeldb/graph.go
+++ b/channeldb/graph.go
@@ -1061,6 +1061,9 @@ func (c *ChannelGraph) DisconnectBlockAtHeight(height uint32) ([]*ChannelEdgeInf
 
 		// Scan from chanIDStart to chanIDEnd, deleting every
 		// found edge.
+		// NOTE: we must delete the edges after the cursor loop, since
+		// modifying the bucket while traversing is not safe.
+		var keys [][]byte
 		cursor := edgeIndex.Cursor()
 		for k, v := cursor.Seek(chanIDStart[:]); k != nil &&
 			bytes.Compare(k, chanIDEnd[:]) <= 0; k, v = cursor.Next() {
@@ -1070,6 +1073,12 @@ func (c *ChannelGraph) DisconnectBlockAtHeight(height uint32) ([]*ChannelEdgeInf
 			if err != nil {
 				return err
 			}
+
+			keys = append(keys, k)
+			removedChans = append(removedChans, &edgeInfo)
+		}
+
+		for _, k := range keys {
 			err = delChannelEdge(
 				edges, edgeIndex, chanIndex, zombieIndex, nodes,
 				k, false,
@@ -1077,8 +1086,6 @@ func (c *ChannelGraph) DisconnectBlockAtHeight(height uint32) ([]*ChannelEdgeInf
 			if err != nil && err != ErrEdgeNotFound {
 				return err
 			}
-
-			removedChans = append(removedChans, &edgeInfo)
 		}
 
 		// Delete all the entries in the prune log having a height
@@ -1099,10 +1106,18 @@ func (c *ChannelGraph) DisconnectBlockAtHeight(height uint32) ([]*ChannelEdgeInf
 		var pruneKeyEnd [4]byte
 		byteOrder.PutUint32(pruneKeyEnd[:], math.MaxUint32)
 
+		// To avoid modifying the bucket while traversing, we delete
+		// the keys in a second loop.
+		var pruneKeys [][]byte
 		pruneCursor := pruneBucket.Cursor()
 		for k, _ := pruneCursor.Seek(pruneKeyStart[:]); k != nil &&
 			bytes.Compare(k, pruneKeyEnd[:]) <= 0; k, _ = pruneCursor.Next() {
-			if err := pruneCursor.Delete(); err != nil {
+
+			pruneKeys = append(pruneKeys, k)
+		}
+
+		for _, k := range pruneKeys {
+			if err := pruneBucket.Delete(k); err != nil {
 				return err
 			}
 		}