From d4f836ecb3e6e6fe0b4e522373a252f9ad5d80e1 Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Wed, 13 Nov 2019 17:31:36 +0100 Subject: [PATCH 1/3] chanbackup: encode broadcast height in chan ID for unconfirmed channels --- chanbackup/single.go | 15 ++++++++++++++- chanbackup/single_test.go | 40 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 1 deletion(-) diff --git a/chanbackup/single.go b/chanbackup/single.go index 1bfb49ca..c989bda9 100644 --- a/chanbackup/single.go +++ b/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, diff --git a/chanbackup/single_test.go b/chanbackup/single_test.go index 776ce0bc..88e2f594 100644 --- a/chanbackup/single_test.go +++ b/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 From fb683f74a448808120ac6708f9c7ea7d082cd105 Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Mon, 18 Nov 2019 10:26:36 +0100 Subject: [PATCH 2/3] chanbackup: set funding broadcast height on restore --- chanrestore.go | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 71 insertions(+) diff --git a/chanrestore.go b/chanrestore.go index 5586ab92..ccc1b34a 100644 --- a/chanrestore.go +++ b/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)) From a124bb24da181550d752de614c9c455e7e36b368 Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Mon, 25 Nov 2019 11:34:23 +0100 Subject: [PATCH 3/3] lntest: add unconfirmed channel SCB test --- lntest/itest/lnd_test.go | 105 ++++++++++++++++++++++++++++++++------- 1 file changed, 87 insertions(+), 18 deletions(-) diff --git a/lntest/itest/lnd_test.go b/lntest/itest/lnd_test.go index 56517b82..14c4bd9e 100644 --- a/lntest/itest/lnd_test.go +++ b/lntest/itest/lnd_test.go @@ -13879,6 +13879,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 @@ -13943,25 +13947,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. - 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) + // We will either open a confirmed or unconfirmed channel, depending on + // the requirements of the test case. + switch { + case testCase.unconfirmed: + ctxt, _ = context.WithTimeout(ctxb, channelOpenTimeout) + _, err := net.OpenPendingChannel( + ctxt, from, to, chanAmt, pushAmt, + ) + 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 @@ -14320,6 +14339,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?