From 240318befe936a2f5cfd99875923bb69baa0b9b4 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 26 Sep 2018 11:12:57 +0200 Subject: [PATCH 1/5] peer: return error on newChanReq Previously the errors weren't returned back to the sender of the new channel, making it impossible to tell whether it suceeded or not. --- peer.go | 34 ++++++++++++++++++++++------------ 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/peer.go b/peer.go index d438a470..3dfa639f 100644 --- a/peer.go +++ b/peer.go @@ -64,7 +64,7 @@ type outgoingMsg struct { // has been confirmed and the channel creation process completed. type newChannelMsg struct { channel *lnwallet.LightningChannel - done chan struct{} + err chan error } // closeMsgs is a wrapper struct around any wire messages that deal with the @@ -1533,7 +1533,7 @@ out: "ignoring.", chanPoint) p.activeChanMtx.Unlock() - close(newChanReq.done) + close(newChanReq.err) newChanReq.channel.Stop() // If we're being sent a new channel, and our @@ -1574,15 +1574,22 @@ out: // TODO(roasbeef): panic on below? _, currentHeight, err := p.server.cc.chainIO.GetBestBlock() if err != nil { - peerLog.Errorf("unable to get best block: %v", err) + err := fmt.Errorf("unable to get best "+ + "block: %v", err) + peerLog.Errorf(err.Error()) + + newChanReq.err <- err continue } chainEvents, err := p.server.chainArb.SubscribeChannelEvents( *chanPoint, ) if err != nil { - peerLog.Errorf("unable to subscribe to chain "+ - "events: %v", err) + err := fmt.Errorf("unable to subscribe to "+ + "chain events: %v", err) + peerLog.Errorf(err.Error()) + + newChanReq.err <- err continue } @@ -1606,12 +1613,16 @@ out: chainEvents, currentHeight, false, ) if err != nil { - peerLog.Errorf("can't register new channel "+ + err := fmt.Errorf("can't register new channel "+ "link(%v) with NodeKey(%x)", chanPoint, p.PubKey()) + peerLog.Errorf(err.Error()) + + newChanReq.err <- err + continue } - close(newChanReq.done) + close(newChanReq.err) // We've just received a local request to close an active // channel. If will either kick of a cooperative channel @@ -2174,10 +2185,10 @@ func (p *peer) Address() net.Addr { func (p *peer) AddNewChannel(channel *lnwallet.LightningChannel, cancel <-chan struct{}) error { - newChanDone := make(chan struct{}) + errChan := make(chan error, 1) newChanMsg := &newChannelMsg{ channel: channel, - done: newChanDone, + err: errChan, } select { @@ -2191,12 +2202,11 @@ func (p *peer) AddNewChannel(channel *lnwallet.LightningChannel, // We pause here to wait for the peer to recognize the new channel // before we close the channel barrier corresponding to the channel. select { - case <-newChanDone: + case err := <-errChan: + return err case <-p.quit: return ErrPeerExiting } - - return nil } // StartTime returns the time at which the connection was established if the From addb4aed89478b53d594bde9e7f6d843da33edc0 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 26 Sep 2018 11:12:57 +0200 Subject: [PATCH 2/5] funding+server: make FindChannel return OpenChannel instead of LightningChannel --- fundingmanager.go | 11 ++++------- server.go | 9 ++++----- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/fundingmanager.go b/fundingmanager.go index 49b23ecd..55622e40 100644 --- a/fundingmanager.go +++ b/fundingmanager.go @@ -267,7 +267,7 @@ type fundingConfig struct { // FindChannel queries the database for the channel with the given // channel ID. - FindChannel func(chanID lnwire.ChannelID) (*lnwallet.LightningChannel, error) + FindChannel func(chanID lnwire.ChannelID) (*channeldb.OpenChannel, error) // TempChanIDSeed is a cryptographically random string of bytes that's // used as a seed to generate pending channel ID's. @@ -2323,10 +2323,9 @@ func (f *fundingManager) handleFundingLocked(fmsg *fundingLockedMsg) { // If the RemoteNextRevocation is non-nil, it means that we have // already processed fundingLocked for this channel, so ignore. - if channel.RemoteNextRevocation() != nil { + if channel.RemoteNextRevocation != nil { fndgLog.Infof("Received duplicate fundingLocked for "+ "ChannelID(%v), ignoring.", chanID) - channel.Stop() return } @@ -2334,10 +2333,9 @@ func (f *fundingManager) handleFundingLocked(fmsg *fundingLockedMsg) { // need to create the next commitment state for the remote party. So // we'll insert that into the channel now before passing it along to // other sub-systems. - err = channel.InitNextRevocation(fmsg.msg.NextPerCommitmentPoint) + err = channel.InsertNextRevocation(fmsg.msg.NextPerCommitmentPoint) if err != nil { fndgLog.Errorf("unable to insert next commitment point: %v", err) - channel.Stop() return } @@ -2361,8 +2359,7 @@ func (f *fundingManager) handleFundingLocked(fmsg *fundingLockedMsg) { if err := fmsg.peer.AddNewChannel(channel, f.quit); err != nil { fndgLog.Errorf("Unable to add new channel %v with peer %x: %v", fmsg.peer.IdentityKey().SerializeCompressed(), - *channel.ChanPoint, err) - channel.Stop() + channel.FundingOutpoint, err) } } diff --git a/server.go b/server.go index 9fb002bb..b6e8af2e 100644 --- a/server.go +++ b/server.go @@ -743,7 +743,9 @@ func newServer(listenAddrs []net.Addr, chanDB *channeldb.DB, cc *chainControl, }, NotifyWhenOnline: s.NotifyWhenOnline, TempChanIDSeed: chanIDSeed, - FindChannel: func(chanID lnwire.ChannelID) (*lnwallet.LightningChannel, error) { + FindChannel: func(chanID lnwire.ChannelID) ( + *channeldb.OpenChannel, error) { + dbChannels, err := chanDB.FetchAllChannels() if err != nil { return nil, err @@ -751,10 +753,7 @@ func newServer(listenAddrs []net.Addr, chanDB *channeldb.DB, cc *chainControl, for _, channel := range dbChannels { if chanID.IsChanPoint(&channel.FundingOutpoint) { - return lnwallet.NewLightningChannel( - cc.signer, s.witnessBeacon, - channel, - ) + return channel, nil } } From b712b861f89de2a6c4ca8c7ea5c68fcf1961fe85 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 26 Sep 2018 11:12:57 +0200 Subject: [PATCH 3/5] peer: make AddNewChannel take OpenChannel This commit makes the AddNewChannel expect a OpenChannel instead of a LightningChannel struct. This moves the responsibility for starting the LightningChannel from the fundingmanager to the peer, and we can defer the channel restoration until we know that the channel is not already active. --- lnpeer/peer.go | 4 ++-- peer.go | 40 ++++++++++++++++++++++++++++------------ 2 files changed, 30 insertions(+), 14 deletions(-) diff --git a/lnpeer/peer.go b/lnpeer/peer.go index fb0e743c..34675aa5 100644 --- a/lnpeer/peer.go +++ b/lnpeer/peer.go @@ -5,7 +5,7 @@ import ( "github.com/btcsuite/btcd/btcec" "github.com/btcsuite/btcd/wire" - "github.com/lightningnetwork/lnd/lnwallet" + "github.com/lightningnetwork/lnd/channeldb" "github.com/lightningnetwork/lnd/lnwire" ) @@ -19,7 +19,7 @@ type Peer interface { // AddNewChannel adds a new channel to the peer. The channel should fail // to be added if the cancel channel is closed. - AddNewChannel(channel *lnwallet.LightningChannel, cancel <-chan struct{}) error + AddNewChannel(channel *channeldb.OpenChannel, cancel <-chan struct{}) error // WipeChannel removes the channel uniquely identified by its channel // point from all indexes associated with the peer. diff --git a/peer.go b/peer.go index 3dfa639f..563bb789 100644 --- a/peer.go +++ b/peer.go @@ -59,11 +59,11 @@ type outgoingMsg struct { errChan chan error // MUST be buffered. } -// newChannelMsg packages an lnwallet.LightningChannel with a channel that -// allows the receiver of the request to report when the funding transaction -// has been confirmed and the channel creation process completed. +// newChannelMsg packages a channeldb.OpenChannel with a channel that allows +// the receiver of the request to report when the funding transaction has been +// confirmed and the channel creation process completed. type newChannelMsg struct { - channel *lnwallet.LightningChannel + channel *channeldb.OpenChannel err chan error } @@ -1522,9 +1522,9 @@ out: // funding workflow. We'll initialize the necessary local // state, and notify the htlc switch of a new link. case newChanReq := <-p.newChannels: - chanPoint := newChanReq.channel.ChannelPoint() - chanID := lnwire.NewChanIDFromOutPoint(chanPoint) newChan := newChanReq.channel + chanPoint := &newChan.FundingOutpoint + chanID := lnwire.NewChanIDFromOutPoint(chanPoint) // Make sure this channel is not already active. p.activeChanMtx.Lock() @@ -1534,7 +1534,6 @@ out: p.activeChanMtx.Unlock() close(newChanReq.err) - newChanReq.channel.Stop() // If we're being sent a new channel, and our // existing channel doesn't have the next @@ -1548,7 +1547,7 @@ out: "FundingLocked for ChannelPoint(%v)", chanPoint) - nextRevoke := newChan.RemoteNextRevocation() + nextRevoke := newChan.RemoteNextRevocation err := currentChan.InitNextRevocation(nextRevoke) if err != nil { peerLog.Errorf("unable to init chan "+ @@ -1562,7 +1561,21 @@ out: // If not already active, we'll add this channel to the // set of active channels, so we can look it up later // easily according to its channel ID. - p.activeChannels[chanID] = newChan + lnChan, err := lnwallet.NewLightningChannel( + p.server.cc.signer, p.server.witnessBeacon, + newChan, + ) + if err != nil { + p.activeChanMtx.Unlock() + err := fmt.Errorf("unable to create "+ + "LightningChannel: %v", err) + peerLog.Errorf(err.Error()) + + newChanReq.err <- err + continue + } + + p.activeChannels[chanID] = lnChan p.activeChanMtx.Unlock() peerLog.Infof("New channel active ChannelPoint(%v) "+ @@ -1578,6 +1591,7 @@ out: "block: %v", err) peerLog.Errorf(err.Error()) + lnChan.Stop() newChanReq.err <- err continue } @@ -1589,6 +1603,7 @@ out: "chain events: %v", err) peerLog.Errorf(err.Error()) + lnChan.Stop() newChanReq.err <- err continue } @@ -1598,7 +1613,7 @@ out: // forwarded. For fees we'll use the default values, as // they currently are always set to the default values // at initial channel creation. - fwdMinHtlc := newChan.FwdMinHtlc() + fwdMinHtlc := lnChan.FwdMinHtlc() defaultPolicy := p.server.cc.routingPolicy forwardingPolicy := &htlcswitch.ForwardingPolicy{ MinHTLC: fwdMinHtlc, @@ -1609,7 +1624,7 @@ out: // Create the link and add it to the switch. err = p.addLink( - chanPoint, newChan, forwardingPolicy, + chanPoint, lnChan, forwardingPolicy, chainEvents, currentHeight, false, ) if err != nil { @@ -1618,6 +1633,7 @@ out: p.PubKey()) peerLog.Errorf(err.Error()) + lnChan.Stop() newChanReq.err <- err continue } @@ -2182,7 +2198,7 @@ func (p *peer) Address() net.Addr { // added if the cancel channel is closed. // // NOTE: Part of the lnpeer.Peer interface. -func (p *peer) AddNewChannel(channel *lnwallet.LightningChannel, +func (p *peer) AddNewChannel(channel *channeldb.OpenChannel, cancel <-chan struct{}) error { errChan := make(chan error, 1) From a433e7057553292fa6ebb234eaa4dabb6d6521f4 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 26 Sep 2018 11:12:57 +0200 Subject: [PATCH 4/5] fundingmanager test: satisfy new interfaces --- fundingmanager_test.go | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/fundingmanager_test.go b/fundingmanager_test.go index bc1f7750..c1421575 100644 --- a/fundingmanager_test.go +++ b/fundingmanager_test.go @@ -178,13 +178,13 @@ func (n *testNode) QuitSignal() <-chan struct{} { return n.shutdownChannel } -func (n *testNode) AddNewChannel(channel *lnwallet.LightningChannel, +func (n *testNode) AddNewChannel(channel *channeldb.OpenChannel, quit <-chan struct{}) error { - done := make(chan struct{}) + errChan := make(chan error) msg := &newChannelMsg{ channel: channel, - done: done, + err: errChan, } select { @@ -194,12 +194,11 @@ func (n *testNode) AddNewChannel(channel *lnwallet.LightningChannel, } select { - case <-done: + case err := <-errChan: + return err case <-quit: return ErrFundingManagerShuttingDown } - - return nil } func init() { @@ -304,7 +303,8 @@ func createTestFundingManager(t *testing.T, privKey *btcec.PrivateKey, return lnwire.NodeAnnouncement{}, nil }, TempChanIDSeed: chanIDSeed, - FindChannel: func(chanID lnwire.ChannelID) (*lnwallet.LightningChannel, error) { + FindChannel: func(chanID lnwire.ChannelID) ( + *channeldb.OpenChannel, error) { dbChannels, err := cdb.FetchAllChannels() if err != nil { return nil, err @@ -312,10 +312,7 @@ func createTestFundingManager(t *testing.T, privKey *btcec.PrivateKey, for _, channel := range dbChannels { if chanID.IsChanPoint(&channel.FundingOutpoint) { - return lnwallet.NewLightningChannel( - signer, - nil, - channel) + return channel, nil } } @@ -992,14 +989,14 @@ func assertHandleFundingLocked(t *testing.T, alice, bob *testNode) { // They should both send the new channel state to their peer. select { case c := <-alice.newChannels: - close(c.done) + close(c.err) case <-time.After(time.Second * 15): t.Fatalf("alice did not send new channel to peer") } select { case c := <-bob.newChannels: - close(c.done) + close(c.err) case <-time.After(time.Second * 15): t.Fatalf("bob did not send new channel to peer") } From 54a608d09d826c7a6122f9d4d8dd10728b2eb238 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 26 Sep 2018 11:12:58 +0200 Subject: [PATCH 5/5] htlcswitch+discovery mock: adhere to new lnpeer interface --- discovery/gossiper_test.go | 3 +-- htlcswitch/link_test.go | 3 ++- htlcswitch/mock.go | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/discovery/gossiper_test.go b/discovery/gossiper_test.go index 3e8c5f22..56012525 100644 --- a/discovery/gossiper_test.go +++ b/discovery/gossiper_test.go @@ -26,7 +26,6 @@ import ( "github.com/lightningnetwork/lnd/chainntnfs" "github.com/lightningnetwork/lnd/channeldb" "github.com/lightningnetwork/lnd/lnpeer" - "github.com/lightningnetwork/lnd/lnwallet" "github.com/lightningnetwork/lnd/lnwire" "github.com/lightningnetwork/lnd/routing" ) @@ -2315,7 +2314,7 @@ func (p *mockPeer) SendMessage(_ bool, msgs ...lnwire.Message) error { return nil } -func (p *mockPeer) AddNewChannel(_ *lnwallet.LightningChannel, _ <-chan struct{}) error { +func (p *mockPeer) AddNewChannel(_ *channeldb.OpenChannel, _ <-chan struct{}) error { return nil } func (p *mockPeer) WipeChannel(_ *wire.OutPoint) error { return nil } diff --git a/htlcswitch/link_test.go b/htlcswitch/link_test.go index 9e85f163..142ee8c7 100644 --- a/htlcswitch/link_test.go +++ b/htlcswitch/link_test.go @@ -1477,7 +1477,8 @@ func (m *mockPeer) SendMessage(sync bool, msgs ...lnwire.Message) error { } return nil } -func (m *mockPeer) AddNewChannel(_ *lnwallet.LightningChannel, _ <-chan struct{}) error { +func (m *mockPeer) AddNewChannel(_ *channeldb.OpenChannel, + _ <-chan struct{}) error { return nil } func (m *mockPeer) WipeChannel(*wire.OutPoint) error { diff --git a/htlcswitch/mock.go b/htlcswitch/mock.go index 617229b6..58ccc8b1 100644 --- a/htlcswitch/mock.go +++ b/htlcswitch/mock.go @@ -543,7 +543,7 @@ func (s *mockServer) Address() net.Addr { return nil } -func (s *mockServer) AddNewChannel(channel *lnwallet.LightningChannel, +func (s *mockServer) AddNewChannel(channel *channeldb.OpenChannel, cancel <-chan struct{}) error { return nil