Merge pull request #3719 from guggero/scb-unconfirmed-channels

chanbackup: encode broadcast height in chan ID for unconfirmed channels

chanbackup/single.go

@@ -66,6 +66,9 @@ type Single struct {
 	// ShortChannelID encodes the exact location in the chain in which the
 	// channel was initially confirmed. This includes: the block height,
 	// transaction index, and the output within the target transaction.
+	// Channels that were not confirmed at the time of backup creation will
+	// have the funding TX broadcast height set as their block height in
+	// the ShortChannelID.
 	ShortChannelID lnwire.ShortChannelID
 
 	// RemoteNodePub is the identity public key of the remote node this
@@ -126,11 +129,21 @@ func NewSingle(channel *channeldb.OpenChannel,
 	// key.
 	_, shaChainPoint := btcec.PrivKeyFromBytes(btcec.S256(), b.Bytes())
 
+	// If a channel is unconfirmed, the block height of the ShortChannelID
+	// is zero. This will lead to problems when trying to restore that
+	// channel as the spend notifier would get a height hint of zero.
+	// To work around that problem, we add the channel broadcast height
+	// to the channel ID so we can use that as height hint on restore.
+	chanID := channel.ShortChanID()
+	if chanID.BlockHeight == 0 {
+		chanID.BlockHeight = channel.FundingBroadcastHeight
+	}
+
 	single := Single{
 		IsInitiator:     channel.IsInitiator,
 		ChainHash:       channel.ChainHash,
 		FundingOutpoint: channel.FundingOutpoint,
-		ShortChannelID:  channel.ShortChannelID,
+		ShortChannelID:  chanID,
 		RemoteNodePub:   channel.IdentityPub,
 		Addresses:       nodeAddrs,
 		Capacity:        channel.Capacity,

chanbackup/single_test.go

@@ -410,4 +410,44 @@ func TestSinglePackStaticChanBackups(t *testing.T) {
 	}
 }
 
+// TestSingleUnconfirmedChannel tests that unconfirmed channels get serialized
+// correctly by encoding the funding broadcast height as block height of the
+// short channel ID.
+func TestSingleUnconfirmedChannel(t *testing.T) {
+	t.Parallel()
+
+	var fundingBroadcastHeight = uint32(1234)
+
+	// Let's create an open channel shell that contains all the information
+	// we need to create a static channel backup but simulate an
+	// unconfirmed channel by setting the block height to 0.
+	channel, err := genRandomOpenChannelShell()
+	if err != nil {
+		t.Fatalf("unable to gen open channel: %v", err)
+	}
+	channel.ShortChannelID.BlockHeight = 0
+	channel.FundingBroadcastHeight = fundingBroadcastHeight
+
+	singleChanBackup := NewSingle(channel, []net.Addr{addr1, addr2})
+	keyRing := &mockKeyRing{}
+
+	// Pack it and then unpack it again to make sure everything is written
+	// correctly, then check that the block height of the unpacked
+	// is the funding broadcast height we set before.
+	var b bytes.Buffer
+	if err := singleChanBackup.PackToWriter(&b, keyRing); err != nil {
+		t.Fatalf("unable to pack single: %v", err)
+	}
+	var unpackedSingle Single
+	err = unpackedSingle.UnpackFromReader(&b, keyRing)
+	if err != nil {
+		t.Fatalf("unable to unpack single: %v", err)
+	}
+	if unpackedSingle.ShortChannelID.BlockHeight != fundingBroadcastHeight {
+		t.Fatalf("invalid block height. got %d expected %d.",
+			unpackedSingle.ShortChannelID.BlockHeight,
+			fundingBroadcastHeight)
+	}
+}
+
 // TODO(roasbsef): fuzz parsing

chanrestore.go

@@ -2,9 +2,11 @@ package lnd
 
 import (
 	"fmt"
+	"math"
 	"net"
 
 	"github.com/btcsuite/btcd/btcec"
+	"github.com/btcsuite/btcd/chaincfg"
 	"github.com/btcsuite/btcd/chaincfg/chainhash"
 	"github.com/lightningnetwork/lnd/chanbackup"
 	"github.com/lightningnetwork/lnd/channeldb"
@@ -14,6 +16,18 @@ import (
 	"github.com/lightningnetwork/lnd/shachain"
 )
 
+const (
+	// mainnetSCBLaunchBlock is the approximate block height of the bitcoin
+	// mainnet chain of the date when SCBs first were released in lnd
+	// (v0.6.0-beta). The block date is 4/15/2019, 10:54 PM UTC.
+	mainnetSCBLaunchBlock = 571800
+
+	// testnetSCBLaunchBlock is the approximate block height of the bitcoin
+	// testnet3 chain of the date when SCBs first were released in lnd
+	// (v0.6.0-beta). The block date is 4/16/2019, 08:04 AM UTC.
+	testnetSCBLaunchBlock = 1489300
+)
+
 // chanDBRestorer is an implementation of the chanbackup.ChannelRestorer
 // interface that is able to properly map a Single backup, into a
 // channeldb.ChannelShell which is required to fully restore a channel. We also
@@ -126,15 +140,72 @@ func (c *chanDBRestorer) openChannelShell(backup chanbackup.Single) (
 // NOTE: Part of the chanbackup.ChannelRestorer interface.
 func (c *chanDBRestorer) RestoreChansFromSingles(backups ...chanbackup.Single) error {
 	channelShells := make([]*channeldb.ChannelShell, 0, len(backups))
+	firstChanHeight := uint32(math.MaxUint32)
 	for _, backup := range backups {
 		chanShell, err := c.openChannelShell(backup)
 		if err != nil {
 			return err
 		}
 
+		// Find the block height of the earliest channel in this backup.
+		chanHeight := chanShell.Chan.ShortChanID().BlockHeight
+		if chanHeight != 0 && chanHeight < firstChanHeight {
+			firstChanHeight = chanHeight
+		}
+
 		channelShells = append(channelShells, chanShell)
 	}
 
+	// In case there were only unconfirmed channels, we will have to scan
+	// the chain beginning from the launch date of SCBs.
+	if firstChanHeight == math.MaxUint32 {
+		chainHash := channelShells[0].Chan.ChainHash
+		switch {
+		case chainHash.IsEqual(chaincfg.MainNetParams.GenesisHash):
+			firstChanHeight = mainnetSCBLaunchBlock
+
+		case chainHash.IsEqual(chaincfg.TestNet3Params.GenesisHash):
+			firstChanHeight = testnetSCBLaunchBlock
+
+		default:
+			// Worst case: We have no height hint and start at
+			// block 1. Should only happen for SCBs in regtest,
+			// simnet and litecoin.
+			firstChanHeight = 1
+		}
+	}
+
+	// If there were channels in the backup that were not confirmed at the
+	// time of the backup creation, they won't have a block height in the
+	// ShortChanID which would lead to an error in the chain watcher.
+	// We want to at least set the funding broadcast height that the chain
+	// watcher can use instead. We have two possible fallback values for
+	// the broadcast height that we are going to try here.
+	for _, chanShell := range channelShells {
+		channel := chanShell.Chan
+
+		switch {
+		// Fallback case 1: It is extremely unlikely at this point that
+		// a channel we are trying to restore has a coinbase funding TX.
+		// Therefore we can be quite certain that if the TxIndex is
+		// zero, it was an unconfirmed channel where we used the
+		// BlockHeight to encode the funding TX broadcast height. To not
+		// end up with an invalid short channel ID that looks valid, we
+		// restore the "original" unconfirmed one here.
+		case channel.ShortChannelID.TxIndex == 0:
+			broadcastHeight := channel.ShortChannelID.BlockHeight
+			channel.FundingBroadcastHeight = broadcastHeight
+			channel.ShortChannelID.BlockHeight = 0
+
+		// Fallback case 2: This is an unconfirmed channel from an old
+		// backup file where we didn't have any workaround in place.
+		// Best we can do here is set the funding broadcast height to a
+		// reasonable value that we determined earlier.
+		case channel.ShortChanID().BlockHeight == 0:
+			channel.FundingBroadcastHeight = firstChanHeight
+		}
+	}
+
 	ltndLog.Infof("Inserting %v SCB channel shells into DB",
 		len(channelShells))
 

lntest/itest/lnd_test.go

@@ -13976,6 +13976,10 @@ type chanRestoreTestCase struct {
 	// private or not.
 	private bool
 
+	// unconfirmed signals if the channel from Dave to Carol should be
+	// confirmed or not.
+	unconfirmed bool
+
 	// restoreMethod takes an old node, then returns a function
 	// closure that'll return the same node, but with its state
 	// restored via a custom method. We use this to abstract away
@@ -14040,25 +14044,40 @@ func testChanRestoreScenario(t *harnessTest, net *lntest.NetworkHarness,
 	if err := net.ConnectNodes(ctxt, dave, carol); err != nil {
 		t.Fatalf("unable to connect dave to carol: %v", err)
 	}
-	ctxt, _ = context.WithTimeout(ctxb, channelOpenTimeout)
-	chanPoint := openChannelAndAssert(
-		ctxt, t, net, from, to,
-		lntest.OpenChannelParams{
-			Amt:     chanAmt,
-			PushAmt: pushAmt,
-			Private: testCase.private,
-		},
-	)
 
-	// Wait for both sides to see the opened channel.
+	// We will either open a confirmed or unconfirmed channel, depending on
-	ctxt, _ = context.WithTimeout(ctxb, defaultTimeout)
+	// the requirements of the test case.
-	err = dave.WaitForNetworkChannelOpen(ctxt, chanPoint)
+	switch {
-	if err != nil {
+	case testCase.unconfirmed:
-		t.Fatalf("dave didn't report channel: %v", err)
+		ctxt, _ = context.WithTimeout(ctxb, channelOpenTimeout)
-	}
+		_, err := net.OpenPendingChannel(
-	err = carol.WaitForNetworkChannelOpen(ctxt, chanPoint)
+			ctxt, from, to, chanAmt, pushAmt,
-	if err != nil {
+		)
-		t.Fatalf("carol didn't report channel: %v", err)
+		if err != nil {
+			t.Fatalf("couldn't open pending channel: %v", err)
+		}
+
+	default:
+		ctxt, _ = context.WithTimeout(ctxb, channelOpenTimeout)
+		chanPoint := openChannelAndAssert(
+			ctxt, t, net, from, to,
+			lntest.OpenChannelParams{
+				Amt:     chanAmt,
+				PushAmt: pushAmt,
+				Private: testCase.private,
+			},
+		)
+
+		// Wait for both sides to see the opened channel.
+		ctxt, _ = context.WithTimeout(ctxb, defaultTimeout)
+		err = dave.WaitForNetworkChannelOpen(ctxt, chanPoint)
+		if err != nil {
+			t.Fatalf("dave didn't report channel: %v", err)
+		}
+		err = carol.WaitForNetworkChannelOpen(ctxt, chanPoint)
+		if err != nil {
+			t.Fatalf("carol didn't report channel: %v", err)
+		}
 	}
 
 	// If both parties should start with existing channel updates, then
@@ -14417,6 +14436,56 @@ func testChannelBackupRestore(net *lntest.NetworkHarness, t *harnessTest) {
 				}, nil
 			},
 		},
+
+		// Create a backup from an unconfirmed channel and make sure
+		// recovery works as well.
+		{
+			name:            "restore unconfirmed channel",
+			channelsUpdated: false,
+			initiator:       true,
+			private:         false,
+			unconfirmed:     true,
+			restoreMethod: func(oldNode *lntest.HarnessNode,
+				backupFilePath string,
+				mnemonic []string) (nodeRestorer, error) {
+
+				// For this restoration method, we'll grab the
+				// current multi-channel backup from the old
+				// node. The channel should be included, even if
+				// it is not confirmed yet.
+				req := &lnrpc.ChanBackupExportRequest{}
+				chanBackup, err := oldNode.ExportAllChannelBackups(
+					ctxb, req,
+				)
+				if err != nil {
+					return nil, fmt.Errorf("unable to obtain "+
+						"channel backup: %v", err)
+				}
+				chanPoints := chanBackup.MultiChanBackup.ChanPoints
+				if len(chanPoints) == 0 {
+					return nil, fmt.Errorf("unconfirmed " +
+						"channel not included in backup")
+				}
+
+				// Let's assume time passes, the channel
+				// confirms in the meantime but for some reason
+				// the backup we made while it was still
+				// unconfirmed is the only backup we have. We
+				// should still be able to restore it. To
+				// simulate time passing, we mine some blocks
+				// to get the channel confirmed _after_ we saved
+				// the backup.
+				mineBlocks(t, net, 6, 1)
+
+				// In our nodeRestorer function, we'll restore
+				// the node from seed, then manually recover
+				// the channel backup.
+				multi := chanBackup.MultiChanBackup.MultiChanBackup
+				return chanRestoreViaRPC(
+					net, password, mnemonic, multi,
+				)
+			},
+		},
 	}
 
 	// TODO(roasbeef): online vs offline close?