funding+htlcswitch: dynamically update short chan id of existing link

In this commit, we fix an existing bug that would result in some
payments getting “stuck”. This would happen if one side restarted
before the channel was fully locked in. In this case, since upon
re-connection, the link will get added to the switch with a *short
channel ID of zero*. If A then tries to make a multi-hop payment
through B, B will fail to forward the payment, as it’ll mistakenly
think that the payment originated from a local-subsystem as the channel
ID is zero. A short channel ID of zero is used to map local payments
back to their caller.

With fix this by allowing the funding manager to dynamically update the
short channel ID of a link after it discovers the short channel ID.

In this commit, we fix a second instance of reported “stuck” payments
by users.
This commit is contained in:
Olaoluwa Osuntokun 2018-02-03 18:14:09 -08:00
parent 71a837630a
commit 30dbbd69a0
No known key found for this signature in database
GPG Key ID: 964EA263DD637C21
7 changed files with 145 additions and 14 deletions

View File

@ -240,6 +240,11 @@ type fundingConfig struct {
// the channel to the ChainArbitrator so it can watch for any on-chain
// events related to the channel.
WatchNewChannel func(*channeldb.OpenChannel) error
// ReportShortChanID allows the funding manager to report the newly
// discovered short channel ID of a formerly pending channel to outside
// sub-systems.
ReportShortChanID func(wire.OutPoint, lnwire.ShortChannelID) error
}
// fundingManager acts as an orchestrator/bridge between the wallet's
@ -457,7 +462,8 @@ func (f *fundingManager) Start() error {
&channel.FundingOutpoint)
if err == ErrChannelNotFound {
// Channel not in fundingManager's opening database,
// meaning it was successully announced to the network.
// meaning it was successfully announced to the
// network.
continue
} else if err != nil {
return err
@ -1612,9 +1618,9 @@ func (f *fundingManager) waitForFundingConfirmation(completeChan *channeldb.Open
// should be abstracted
// The funding transaction now being confirmed, we add this channel to
// the fundingManager's internal persistant state machine that we use
// to track the remaining process of the channel opening. This is useful
// to resume the opening process in case of restarts.
// the fundingManager's internal persistent state machine that we use
// to track the remaining process of the channel opening. This is
// useful to resume the opening process in case of restarts.
//
// TODO(halseth): make the two db transactions (MarkChannelAsOpen and
// saveChannelOpeningState) atomic by doing them in the same transaction.
@ -1627,6 +1633,13 @@ func (f *fundingManager) waitForFundingConfirmation(completeChan *channeldb.Open
return
}
// As there might already be an active link in the switch with an
// outdated short chan ID, we'll update it now.
err = f.cfg.ReportShortChanID(fundingPoint, shortChanID)
if err != nil {
fndgLog.Errorf("unable to report short chan id: %v", err)
}
select {
case confChan <- &shortChanID:
case <-f.quit:

View File

@ -273,6 +273,9 @@ func createTestFundingManager(t *testing.T, privKey *btcec.PrivateKey,
WatchNewChannel: func(*channeldb.OpenChannel) error {
return nil
},
ReportShortChanID: func(wire.OutPoint, lnwire.ShortChannelID) error {
return nil
},
})
if err != nil {
t.Fatalf("failed creating fundingManager: %v", err)

View File

@ -65,6 +65,12 @@ type ChannelLink interface {
// the original funding output can be found.
ShortChanID() lnwire.ShortChannelID
// UpdateShortChanID updates the short channel ID for a link. This may
// be required in the event that a link is created before the short
// chan ID for it is known, or a re-org occurs, and the funding
// transacton changes location within the chain.
UpdateShortChanID(lnwire.ShortChannelID)
// UpdateForwardingPolicy updates the forwarding policy for the target
// ChannelLink. Once updated, the link will use the new forwarding
// policy to govern if it an incoming HTLC should be forwarded or not.

View File

@ -196,6 +196,9 @@ type channelLink struct {
// updates.
channel *lnwallet.LightningChannel
// shortChanID is the most up to date short channel ID for the link.
shortChanID lnwire.ShortChannelID
// cfg is a structure which carries all dependable fields/handlers
// which may affect behaviour of the service.
cfg ChannelLinkConfig
@ -236,6 +239,8 @@ type channelLink struct {
logCommitTimer *time.Timer
logCommitTick <-chan time.Time
sync.RWMutex
wg sync.WaitGroup
quit chan struct{}
}
@ -248,6 +253,7 @@ func NewChannelLink(cfg ChannelLinkConfig, channel *lnwallet.LightningChannel,
link := &channelLink{
cfg: cfg,
channel: channel,
shortChanID: channel.ShortChanID(),
mailBox: newMemoryMailBox(),
linkControl: make(chan interface{}),
// TODO(roasbeef): just do reserve here?
@ -1090,6 +1096,8 @@ func (l *channelLink) handleUpstreamMsg(msg lnwire.Message) {
l.channel.ChannelPoint(), len(htlcsToForward))
for _, packet := range htlcsToForward {
if err := l.cfg.Switch.forward(packet); err != nil {
// TODO(roasbeef): cancel back htlc
// under certain conditions?
log.Errorf("channel link(%v): "+
"unhandled error while forwarding "+
"htlc packet over htlc "+
@ -1161,7 +1169,37 @@ func (l *channelLink) Peer() Peer {
//
// NOTE: Part of the ChannelLink interface.
func (l *channelLink) ShortChanID() lnwire.ShortChannelID {
return l.channel.ShortChanID()
l.RLock()
defer l.RUnlock()
return l.shortChanID
}
// UpdateShortChanID updates the short channel ID for a link. This may be
// required in the event that a link is created before the short chan ID for it
// is known, or a re-org occurs, and the funding transaction changes location
// within the chain.
//
// NOTE: Part of the ChannelLink interface.
func (l *channelLink) UpdateShortChanID(sid lnwire.ShortChannelID) {
l.Lock()
defer l.Unlock()
log.Infof("Updating short chan ID for ChannelPoint(%v)", l)
l.shortChanID = sid
go func() {
err := l.cfg.UpdateContractSignals(&contractcourt.ContractSignals{
HtlcUpdates: l.htlcUpdates,
ShortChanID: l.channel.ShortChanID(),
})
if err != nil {
log.Errorf("Unable to update signals for "+
"ChannelLink(%v)", l)
}
}()
return
}
// ChanID returns the channel ID for the channel link. The channel ID is a more
@ -1806,5 +1844,5 @@ func (l *channelLink) sendMalformedHTLCError(htlcIndex uint64,
func (l *channelLink) fail(format string, a ...interface{}) {
reason := errors.Errorf(format, a...)
log.Error(reason)
l.cfg.Peer.Disconnect(reason)
go l.cfg.Peer.Disconnect(reason)
}

View File

@ -476,13 +476,14 @@ func (f *mockChannelLink) Stats() (uint64, lnwire.MilliSatoshi, lnwire.MilliSato
return 0, 0, 0
}
func (f *mockChannelLink) ChanID() lnwire.ChannelID { return f.chanID }
func (f *mockChannelLink) ShortChanID() lnwire.ShortChannelID { return f.shortChanID }
func (f *mockChannelLink) Bandwidth() lnwire.MilliSatoshi { return 99999999 }
func (f *mockChannelLink) Peer() Peer { return f.peer }
func (f *mockChannelLink) Start() error { return nil }
func (f *mockChannelLink) Stop() {}
func (f *mockChannelLink) EligibleToForward() bool { return f.eligible }
func (f *mockChannelLink) ChanID() lnwire.ChannelID { return f.chanID }
func (f *mockChannelLink) ShortChanID() lnwire.ShortChannelID { return f.shortChanID }
func (f *mockChannelLink) UpdateShortChanID(sid lnwire.ShortChannelID) { f.shortChanID = sid }
func (f *mockChannelLink) Bandwidth() lnwire.MilliSatoshi { return 99999999 }
func (f *mockChannelLink) Peer() Peer { return f.peer }
func (f *mockChannelLink) Start() error { return nil }
func (f *mockChannelLink) Stop() {}
func (f *mockChannelLink) EligibleToForward() bool { return f.eligible }
var _ ChannelLink = (*mockChannelLink)(nil)

View File

@ -941,6 +941,10 @@ func (s *Switch) htlcForwarder() {
links, err := s.getLinks(cmd.peer)
cmd.done <- links
cmd.err <- err
case *updateForwardingIndexCmd:
cmd.err <- s.updateShortChanID(
cmd.chanID, cmd.shortChanID,
)
}
case <-s.quit:
@ -1006,6 +1010,8 @@ func (s *Switch) AddLink(link ChannelLink) error {
// addLink is used to add the newly created channel link and start use it to
// handle the channel updates.
func (s *Switch) addLink(link ChannelLink) error {
// TODO(roasbeef): reject if link already tehre?
// First we'll add the link to the linkIndex which lets us quickly look
// up a channel when we need to close or register it, and the
// forwarding index which'll be used when forwarding HTLC's in the
@ -1097,7 +1103,8 @@ func (s *Switch) RemoveLink(chanID lnwire.ChannelID) error {
case s.linkControl <- command:
return <-command.err
case <-s.quit:
return errors.New("unable to remove link htlc switch was stopped")
return errors.New("unable to remove link htlc switch was " +
"stopped")
}
}
@ -1123,6 +1130,62 @@ func (s *Switch) removeLink(chanID lnwire.ChannelID) error {
return nil
}
// updateForwardingIndexCmd is a command sent by outside sub-systems to update
// the forwarding index of the switch in the event that the short channel ID of
// a particular link changes.
type updateForwardingIndexCmd struct {
chanID lnwire.ChannelID
shortChanID lnwire.ShortChannelID
err chan error
}
// UpdateShortChanID updates the short chan ID for an existing channel. This is
// required in the case of a re-org and re-confirmation or a channel, or in the
// case that a link was added to the switch before its short chan ID was known.
func (s *Switch) UpdateShortChanID(chanID lnwire.ChannelID,
shortChanID lnwire.ShortChannelID) error {
command := &updateForwardingIndexCmd{
chanID: chanID,
shortChanID: shortChanID,
err: make(chan error, 1),
}
select {
case s.linkControl <- command:
return <-command.err
case <-s.quit:
return errors.New("unable to remove link htlc switch was " +
"stopped")
}
}
// updateShortChanID updates the short chan ID of an existing link.
func (s *Switch) updateShortChanID(chanID lnwire.ChannelID,
shortChanID lnwire.ShortChannelID) error {
// First, we'll extract the current link as is from the link link
// index. If the link isn't even in the index, then we'll return an
// error.
link, ok := s.linkIndex[chanID]
if !ok {
return fmt.Errorf("link %v not found", chanID)
}
log.Infof("Updating short_chan_id for ChannelLink(%v): old=%v, new=%v",
chanID, link.ShortChanID(), shortChanID)
// At this point the link is actually active, so we'll update the
// forwarding index with the next short channel ID.
s.forwardingIndex[shortChanID] = link
// Finally, we'll notify the link of its new short channel ID.
link.UpdateShortChanID(shortChanID)
return nil
}
// getLinksCmd is a get links command wrapper, it is used to propagate handler
// parameters and return handler error.
type getLinksCmd struct {

7
lnd.go
View File

@ -42,6 +42,7 @@ import (
"github.com/lightningnetwork/lnd/macaroons"
"github.com/lightningnetwork/lnd/walletunlocker"
"github.com/roasbeef/btcd/btcec"
"github.com/roasbeef/btcd/wire"
"github.com/roasbeef/btcutil"
)
@ -370,6 +371,12 @@ func lndMain() error {
return delay
},
WatchNewChannel: server.chainArb.WatchNewChannel,
ReportShortChanID: func(chanPoint wire.OutPoint,
sid lnwire.ShortChannelID) error {
cid := lnwire.NewChanIDFromOutPoint(&chanPoint)
return server.htlcSwitch.UpdateShortChanID(cid, sid)
},
})
if err != nil {
return err