From 8b3dd9415ef6b5f83505068231557d275b65db53 Mon Sep 17 00:00:00 2001 From: Roei Erez Date: Tue, 5 Nov 2019 09:57:38 +0200 Subject: [PATCH 1/2] channeldb: refresh channel state within RefreshShortChanID Refresh channel memory state whenever the short channel id is refreshed. This is to make the in-memory channel consistent with the disk data. Fixes #3765. --- channeldb/channel.go | 17 ++++---- channeldb/channel_test.go | 10 ++++- lntest/itest/lnd_test.go | 88 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 103 insertions(+), 12 deletions(-) diff --git a/channeldb/channel.go b/channeldb/channel.go index 1fdd97e9..c2eaba5a 100644 --- a/channeldb/channel.go +++ b/channeldb/channel.go @@ -594,13 +594,15 @@ func (c *OpenChannel) hasChanStatus(status ChannelStatus) bool { return c.chanStatus&status == status } -// RefreshShortChanID updates the in-memory short channel ID using the latest +// RefreshShortChanID updates the in-memory channel state using the latest // value observed on disk. +// TODO: the name of this function should be changed to reflect the fact that +// it is not only refreshing the short channel id but all the channel state. +// maybe Refresh/Reload? func (c *OpenChannel) RefreshShortChanID() error { c.Lock() defer c.Unlock() - var sid lnwire.ShortChannelID err := c.Db.View(func(tx *bbolt.Tx) error { chanBucket, err := fetchChanBucket( tx, c.IdentityPub, &c.FundingOutpoint, c.ChainHash, @@ -609,22 +611,17 @@ func (c *OpenChannel) RefreshShortChanID() error { return err } - channel, err := fetchOpenChannel(chanBucket, &c.FundingOutpoint) - if err != nil { - return err + // populating the in-memory channel with the info fetched from disk. + if err := fetchChanInfo(chanBucket, c); err != nil { + return fmt.Errorf("unable to fetch chan info: %v", err) } - sid = channel.ShortChannelID - return nil }) if err != nil { return err } - c.ShortChannelID = sid - c.Packager = NewChannelPackager(sid) - return nil } diff --git a/channeldb/channel_test.go b/channeldb/channel_test.go index 6d34c2f9..645b3030 100644 --- a/channeldb/channel_test.go +++ b/channeldb/channel_test.go @@ -939,8 +939,8 @@ func TestFetchWaitingCloseChannels(t *testing.T) { } // TestRefreshShortChanID asserts that RefreshShortChanID updates the in-memory -// short channel ID of another OpenChannel to reflect a preceding call to -// MarkOpen on a different OpenChannel. +// state of another OpenChannel to reflect a preceding call to MarkOpen on a +// different OpenChannel. func TestRefreshShortChanID(t *testing.T) { t.Parallel() @@ -1038,4 +1038,10 @@ func TestRefreshShortChanID(t *testing.T) { "got %v", chanOpenLoc, pendingChannel.Packager.(*ChannelPackager).source) } + + // Check to ensure that this channel is no longer pending and this field + // is up to date. + if pendingChannel.IsPending { + t.Fatalf("channel pending state wasn't updated: want false got true") + } } diff --git a/lntest/itest/lnd_test.go b/lntest/itest/lnd_test.go index 56517b82..5c011769 100644 --- a/lntest/itest/lnd_test.go +++ b/lntest/itest/lnd_test.go @@ -1219,6 +1219,90 @@ func testUnconfirmedChannelFunding(net *lntest.NetworkHarness, t *harnessTest) { closeChannelAndAssert(ctxt, t, net, carol, chanPoint, false) } +// testPaymentFollowingChannelOpen tests that the channel transition from +// 'pending' to 'open' state does not cause any inconsistencies within other +// subsystems trying to udpate the channel state in the db. We follow +// this transition with a payment that updates the commitment state and +// verify that the pending state is up to date. +func testPaymentFollowingChannelOpen(net *lntest.NetworkHarness, t *harnessTest) { + ctxb := context.Background() + + // We first establish a channel between Alice and Bob. + const paymentAmt = btcutil.Amount(100) + ctxt, _ := context.WithTimeout(ctxb, channelOpenTimeout) + channelCapacity := btcutil.Amount(paymentAmt * 1000) + pendingUpdate, err := net.OpenPendingChannel(ctxt, net.Alice, net.Bob, + channelCapacity, 0) + if err != nil { + t.Fatalf("unable to open channel: %v", err) + } + + // At this point, the channel's funding transaction will have + // been broadcast, but not confirmed. Alice and Bob's nodes + // should reflect this when queried via RPC. + ctxt, _ = context.WithTimeout(ctxb, defaultTimeout) + assertNumOpenChannelsPending(ctxt, t, net.Alice, net.Bob, 1) + + // We are restarting Bob's node to let the link be created for the pending + // channel. + if err := net.RestartNode(net.Bob, nil); err != nil { + t.Fatalf("Bob restart failed: %v", err) + } + + // We ensure that Bob reconnets to Alice. + ctxt, _ = context.WithTimeout(ctxb, defaultTimeout) + if err := net.EnsureConnected(ctxt, net.Bob, net.Alice); err != nil { + t.Fatalf("peers unable to reconnect after restart: %v", err) + } + + // We mine one block for the channel to be confirmed. + _ = mineBlocks(t, net, 6, 1)[0] + + // We verify that the chanel is open from both nodes point of view. + time.Sleep(time.Millisecond * 300) + ctxt, _ = context.WithTimeout(ctxb, defaultTimeout) + assertNumOpenChannelsPending(ctxt, t, net.Alice, net.Bob, 0) + + // With the channel open, we'll create invoices for Bob that Alice + // will pay to in order to advance the state of the channel. + bobPayReqs, _, _, err := createPayReqs( + net.Bob, paymentAmt, 1, + ) + if err != nil { + t.Fatalf("unable to create pay reqs: %v", err) + } + + // Send payment to Bob so there a chanel update to disk will be executed. + ctxt, _ = context.WithTimeout(ctxb, defaultTimeout) + _, err = net.Alice.SendPaymentSync(ctxt, + &lnrpc.SendRequest{PaymentRequest: bobPayReqs[0]}) + if err != nil { + t.Fatalf("unable to create payment stream for alice: %v", err) + } + + // At this point we want to make sure the channel is opened and not pending. + ctxt, _ = context.WithTimeout(ctxb, defaultTimeout) + res, err := net.Bob.ListChannels(ctxt, &lnrpc.ListChannelsRequest{}) + if err != nil { + t.Fatalf("unable to list bob channels: %v", err) + } + if len(res.Channels) == 0 { + t.Fatalf("bob list of channels is empty") + } + + // Finally, immediately close the channel. This function will also + // block until the channel is closed and will additionally assert the + // relevant channel closing post conditions. + chanPoint := &lnrpc.ChannelPoint{ + FundingTxid: &lnrpc.ChannelPoint_FundingTxidBytes{ + FundingTxidBytes: pendingUpdate.Txid, + }, + OutputIndex: pendingUpdate.OutputIndex, + } + ctxt, _ = context.WithTimeout(ctxb, channelCloseTimeout) + closeChannelAndAssert(ctxt, t, net, net.Alice, chanPoint, false) +} + // txStr returns the string representation of the channel's funding transaction. func txStr(chanPoint *lnrpc.ChannelPoint) string { fundingTxID, err := lnd.GetChanPointFundingTxid(chanPoint) @@ -14955,6 +15039,10 @@ var testsCases = []*testCase{ name: "macaroon authentication", test: testMacaroonAuthentication, }, + { + name: "immediate payment after channel opened", + test: testPaymentFollowingChannelOpen, + }, } // TestLightningNetworkDaemon performs a series of integration tests amongst a From 5bdb0d3d660abc29501e006417090ad1a5b35db6 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Wed, 27 Nov 2019 15:20:06 -0600 Subject: [PATCH 2/2] channeldb+lntest: code style fixes --- channeldb/channel.go | 4 ++- lntest/itest/lnd_test.go | 59 ++++++++++++++++++++++++---------------- 2 files changed, 39 insertions(+), 24 deletions(-) diff --git a/channeldb/channel.go b/channeldb/channel.go index c2eaba5a..9332f50e 100644 --- a/channeldb/channel.go +++ b/channeldb/channel.go @@ -596,6 +596,7 @@ func (c *OpenChannel) hasChanStatus(status ChannelStatus) bool { // RefreshShortChanID updates the in-memory channel state using the latest // value observed on disk. +// // TODO: the name of this function should be changed to reflect the fact that // it is not only refreshing the short channel id but all the channel state. // maybe Refresh/Reload? @@ -611,7 +612,8 @@ func (c *OpenChannel) RefreshShortChanID() error { return err } - // populating the in-memory channel with the info fetched from disk. + // We'll re-populating the in-memory channel with the info + // fetched from disk. if err := fetchChanInfo(chanBucket, c); err != nil { return fmt.Errorf("unable to fetch chan info: %v", err) } diff --git a/lntest/itest/lnd_test.go b/lntest/itest/lnd_test.go index 5c011769..07bca25b 100644 --- a/lntest/itest/lnd_test.go +++ b/lntest/itest/lnd_test.go @@ -1221,30 +1221,34 @@ func testUnconfirmedChannelFunding(net *lntest.NetworkHarness, t *harnessTest) { // testPaymentFollowingChannelOpen tests that the channel transition from // 'pending' to 'open' state does not cause any inconsistencies within other -// subsystems trying to udpate the channel state in the db. We follow -// this transition with a payment that updates the commitment state and -// verify that the pending state is up to date. +// subsystems trying to udpate the channel state in the db. We follow this +// transition with a payment that updates the commitment state and verify that +// the pending state is up to date. func testPaymentFollowingChannelOpen(net *lntest.NetworkHarness, t *harnessTest) { ctxb := context.Background() - // We first establish a channel between Alice and Bob. const paymentAmt = btcutil.Amount(100) - ctxt, _ := context.WithTimeout(ctxb, channelOpenTimeout) channelCapacity := btcutil.Amount(paymentAmt * 1000) - pendingUpdate, err := net.OpenPendingChannel(ctxt, net.Alice, net.Bob, - channelCapacity, 0) + + // We first establish a channel between Alice and Bob. + ctxt, cancel := context.WithTimeout(ctxb, channelOpenTimeout) + defer cancel() + pendingUpdate, err := net.OpenPendingChannel( + ctxt, net.Alice, net.Bob, channelCapacity, 0, + ) if err != nil { t.Fatalf("unable to open channel: %v", err) } - // At this point, the channel's funding transaction will have - // been broadcast, but not confirmed. Alice and Bob's nodes + // At this point, the channel's funding transaction will have been + // broadcast, but not confirmed. Alice and Bob's nodes // should reflect this when queried via RPC. - ctxt, _ = context.WithTimeout(ctxb, defaultTimeout) + ctxt, cancel = context.WithTimeout(ctxb, defaultTimeout) + defer cancel() assertNumOpenChannelsPending(ctxt, t, net.Alice, net.Bob, 1) - // We are restarting Bob's node to let the link be created for the pending - // channel. + // We are restarting Bob's node to let the link be created for the + // pending channel. if err := net.RestartNode(net.Bob, nil); err != nil { t.Fatalf("Bob restart failed: %v", err) } @@ -1259,12 +1263,12 @@ func testPaymentFollowingChannelOpen(net *lntest.NetworkHarness, t *harnessTest) _ = mineBlocks(t, net, 6, 1)[0] // We verify that the chanel is open from both nodes point of view. - time.Sleep(time.Millisecond * 300) - ctxt, _ = context.WithTimeout(ctxb, defaultTimeout) + ctxt, cancel = context.WithTimeout(ctxb, defaultTimeout) + defer cancel() assertNumOpenChannelsPending(ctxt, t, net.Alice, net.Bob, 0) - // With the channel open, we'll create invoices for Bob that Alice - // will pay to in order to advance the state of the channel. + // With the channel open, we'll create invoices for Bob that Alice will + // pay to in order to advance the state of the channel. bobPayReqs, _, _, err := createPayReqs( net.Bob, paymentAmt, 1, ) @@ -1272,16 +1276,24 @@ func testPaymentFollowingChannelOpen(net *lntest.NetworkHarness, t *harnessTest) t.Fatalf("unable to create pay reqs: %v", err) } - // Send payment to Bob so there a chanel update to disk will be executed. - ctxt, _ = context.WithTimeout(ctxb, defaultTimeout) - _, err = net.Alice.SendPaymentSync(ctxt, - &lnrpc.SendRequest{PaymentRequest: bobPayReqs[0]}) + // Send payment to Bob so there a chanel update to disk will be + // executed. + ctxt, cancel = context.WithTimeout(ctxb, defaultTimeout) + defer cancel() + _, err = net.Alice.SendPaymentSync( + ctxt, + &lnrpc.SendRequest{ + PaymentRequest: bobPayReqs[0], + }, + ) if err != nil { t.Fatalf("unable to create payment stream for alice: %v", err) } - // At this point we want to make sure the channel is opened and not pending. - ctxt, _ = context.WithTimeout(ctxb, defaultTimeout) + // At this point we want to make sure the channel is opened and not + // pending. + ctxt, cancel = context.WithTimeout(ctxb, defaultTimeout) + defer cancel() res, err := net.Bob.ListChannels(ctxt, &lnrpc.ListChannelsRequest{}) if err != nil { t.Fatalf("unable to list bob channels: %v", err) @@ -1299,7 +1311,8 @@ func testPaymentFollowingChannelOpen(net *lntest.NetworkHarness, t *harnessTest) }, OutputIndex: pendingUpdate.OutputIndex, } - ctxt, _ = context.WithTimeout(ctxb, channelCloseTimeout) + ctxt, cancel = context.WithTimeout(ctxb, channelCloseTimeout) + defer cancel() closeChannelAndAssert(ctxt, t, net, net.Alice, chanPoint, false) }