From 32965fd4bef388c3cbff1e35d0fa287fdd4ca4bd Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Wed, 13 Nov 2019 16:09:20 -0800 Subject: [PATCH] rpc: update AbandonChannel to remove state from cnct, nursery and graph In this commit, we update the `AbandonChannel` method to also remove the state from the countract court as well as the channel graph. Abandoning a channel is now a three step process: remove from the open channel state, remove from the graph, remove from the contract court. Between any step it's possible that the users restarts the process all over again. As a result, each of the steps below are intended to be idempotent. We also update the integration test to assert that no channel is found in the graph any longer. Before this commit, this test would fail as the channel was still found in the graph, which can cause other issues for an operational daemon. Fixes #3716. --- lntest/itest/lnd_test.go | 55 ++++++++++++++++++--- rpcserver.go | 104 +++++++++++++++++++++++++++++---------- 2 files changed, 125 insertions(+), 34 deletions(-) diff --git a/lntest/itest/lnd_test.go b/lntest/itest/lnd_test.go index 5817e85d..a6a76099 100644 --- a/lntest/itest/lnd_test.go +++ b/lntest/itest/lnd_test.go @@ -13155,11 +13155,17 @@ func testAbandonChannel(net *lntest.NetworkHarness, t *harnessTest) { ctxt, _ := context.WithTimeout(ctxb, channelOpenTimeout) chanPoint := openChannelAndAssert( - ctxt, t, net, net.Alice, net.Bob, channelParam) + ctxt, t, net, net.Alice, net.Bob, channelParam, + ) + txid, err := lnd.GetChanPointFundingTxid(chanPoint) + if err != nil { + t.Fatalf("unable to get txid: %v", err) + } + chanPointStr := fmt.Sprintf("%v:%v", txid, chanPoint.OutputIndex) // Wait for channel to be confirmed open. ctxt, _ = context.WithTimeout(ctxb, defaultTimeout) - err := net.Alice.WaitForNetworkChannelOpen(ctxt, chanPoint) + err = net.Alice.WaitForNetworkChannelOpen(ctxt, chanPoint) if err != nil { t.Fatalf("alice didn't report channel: %v", err) } @@ -13168,6 +13174,25 @@ func testAbandonChannel(net *lntest.NetworkHarness, t *harnessTest) { t.Fatalf("bob didn't report channel: %v", err) } + // Now that the channel is open, we'll obtain its channel ID real quick + // so we can use it to query the graph below. + listReq := &lnrpc.ListChannelsRequest{} + ctxt, _ = context.WithTimeout(ctxb, defaultTimeout) + aliceChannelList, err := net.Alice.ListChannels(ctxt, listReq) + if err != nil { + t.Fatalf("unable to fetch alice's channels: %v", err) + } + var chanID uint64 + for _, channel := range aliceChannelList.Channels { + if channel.ChannelPoint == chanPointStr { + chanID = channel.ChanId + } + } + + if chanID == 0 { + t.Fatalf("unable to find channel") + } + // Send request to abandon channel. abandonChannelRequest := &lnrpc.AbandonChannelRequest{ ChannelPoint: chanPoint, @@ -13180,9 +13205,8 @@ func testAbandonChannel(net *lntest.NetworkHarness, t *harnessTest) { } // Assert that channel in no longer open. - listReq := &lnrpc.ListChannelsRequest{} ctxt, _ = context.WithTimeout(ctxb, defaultTimeout) - aliceChannelList, err := net.Alice.ListChannels(ctxt, listReq) + aliceChannelList, err = net.Alice.ListChannels(ctxt, listReq) if err != nil { t.Fatalf("unable to list channels: %v", err) } @@ -13230,9 +13254,26 @@ func testAbandonChannel(net *lntest.NetworkHarness, t *harnessTest) { len(aliceClosedList.Channels)) } - // Now that we're done with the test, the channel can be closed. This is - // necessary to avoid unexpected outcomes of other tests that use Bob's - // lnd instance. + // Ensure that the channel can no longer be found in the channel graph. + _, err = net.Alice.GetChanInfo(ctxb, &lnrpc.ChanInfoRequest{ + ChanId: chanID, + }) + if !strings.Contains(err.Error(), "marked as zombie") { + t.Fatalf("channel shouldn't be found in the channel " + + "graph!") + } + + // Calling AbandonChannel again, should result in no new errors, as the + // channel has already been removed. + ctxt, _ = context.WithTimeout(ctxb, defaultTimeout) + _, err = net.Alice.AbandonChannel(ctxt, abandonChannelRequest) + if err != nil { + t.Fatalf("unable to abandon channel a second time: %v", err) + } + + // Now that we're done with the test, the channel can be closed. This + // is necessary to avoid unexpected outcomes of other tests that use + // Bob's lnd instance. ctxt, _ = context.WithTimeout(ctxb, channelCloseTimeout) closeChannelAndAssert(ctxt, t, net, net.Bob, chanPoint, true) diff --git a/rpcserver.go b/rpcserver.go index 828ce7cf..837c2cae 100644 --- a/rpcserver.go +++ b/rpcserver.go @@ -1947,6 +1947,28 @@ func createRPCCloseUpdate(update interface{}) ( return nil, errors.New("unknown close status update") } +// abandonChanFromGraph attempts to remove a channel from the channel graph. If +// we can't find the chanID in the graph, then we assume it has already been +// removed, and will return a nop. +func abandonChanFromGraph(chanGraph *channeldb.ChannelGraph, + chanPoint *wire.OutPoint) error { + + // First, we'll obtain the channel ID. If we can't locate this, then + // it's the case that the channel may have already been removed from + // the graph, so we'll return a nil error. + chanID, err := chanGraph.ChannelID(chanPoint) + switch { + case err == channeldb.ErrEdgeNotFound: + return nil + case err != nil: + return err + } + + // If the channel ID is still in the graph, then that means the channel + // is still open, so we'll now move to purge it from the graph. + return chanGraph.DeleteChannelEdges(chanID) +} + // AbandonChannel removes all channel state from the database except for a // close summary. This method can be used to get rid of permanently unusable // channels due to bugs fixed in newer versions of lnd. @@ -1970,42 +1992,70 @@ func (r *rpcServer) AbandonChannel(ctx context.Context, index := in.ChannelPoint.OutputIndex chanPoint := wire.NewOutPoint(txid, index) - // With the chanPoint constructed, we'll attempt to find the target - // channel in the database. If we can't find the channel, then we'll - // return the error back to the caller. - dbChan, err := r.server.chanDB.FetchChannel(*chanPoint) - if err != nil { - return nil, err - } - - // Now that we've found the channel, we'll populate a close summary for - // the channel, so we can store as much information for this abounded - // channel as possible. We also ensure that we set Pending to false, to - // indicate that this channel has been "fully" closed. + // When we remove the channel from the database, we need to set a close + // height, so we'll just use the current best known height. _, bestHeight, err := r.server.cc.chainIO.GetBestBlock() if err != nil { return nil, err } - summary := &channeldb.ChannelCloseSummary{ - CloseType: channeldb.Abandoned, - ChanPoint: *chanPoint, - ChainHash: dbChan.ChainHash, - CloseHeight: uint32(bestHeight), - RemotePub: dbChan.IdentityPub, - Capacity: dbChan.Capacity, - SettledBalance: dbChan.LocalCommitment.LocalBalance.ToSatoshis(), - ShortChanID: dbChan.ShortChanID(), - RemoteCurrentRevocation: dbChan.RemoteCurrentRevocation, - RemoteNextRevocation: dbChan.RemoteNextRevocation, - LocalChanConfig: dbChan.LocalChanCfg, + + dbChan, err := r.server.chanDB.FetchChannel(*chanPoint) + switch { + // If the channel isn't found in the set of open channels, then we can + // continue on as it can't be loaded into the link/peer. + case err == channeldb.ErrChannelNotFound: + break + + // If the channel is still known to be open, then before we modify any + // on-disk state, we'll remove the channel from the switch and peer + // state if it's been loaded in. + case err == nil: + // We'll mark the channel as borked before we remove the state + // from the switch/peer so it won't be loaded back in if the + // peer reconnects. + if err := dbChan.MarkBorked(); err != nil { + return nil, err + } + remotePub := dbChan.IdentityPub + if peer, err := r.server.FindPeer(remotePub); err == nil { + if err := peer.WipeChannel(chanPoint); err != nil { + return nil, fmt.Errorf("unable to wipe "+ + "channel state: %v", err) + } + } + + default: + return nil, err } - // Finally, we'll close the channel in the DB, and return back to the - // caller. - err = dbChan.CloseChannel(summary) + // Abandoning a channel is a three step process: remove from the open + // channel state, remove from the graph, remove from the contract + // court. Between any step it's possible that the users restarts the + // process all over again. As a result, each of the steps below are + // intended to be idempotent. + err = r.server.chanDB.AbandonChannel(chanPoint, uint32(bestHeight)) if err != nil { return nil, err } + err = abandonChanFromGraph( + r.server.chanDB.ChannelGraph(), chanPoint, + ) + if err != nil { + return nil, err + } + err = r.server.chainArb.ResolveContract(*chanPoint) + if err != nil { + return nil, err + } + + // If this channel was in the process of being closed, but didn't fully + // close, then it's possible that the nursery is hanging on to some + // state. To err on the side of caution, we'll now attempt to wipe any + // state for this channel from the nursery. + err = r.server.utxoNursery.cfg.Store.RemoveChannel(chanPoint) + if err != nil && err != ErrContractNotFound { + return nil, err + } return &lnrpc.AbandonChannelResponse{}, nil }