contractcourt: when creating resolveContract closure don't bind to loop variable

In this commit, we fix a long standing bug where at times a co-op
channel closure wouldn't be properly marked as fully closed in the
database. The culprit was a re-occurring code flaw we've seen many times
in the codebase: a closure variable that closes over a loop iterator
variable. Before this instance, I assumed that this could only pop up
when goroutines bind to the loop iterator within a  closure. However,
this instance is the exact same issue, but within a regular closure that
has _delayed_ execution. As the closure doesn't execute until long after
the loop has finished executing, it may still be holding onto the _last_
item the loop iterator variable was assigned to.

The fix for this issue is very simple: re-assign the channel point
before creating the closure. Without this fix, we would go to call
db.MarkChanFullyClosed on a channel that may not have yet actually be in
the pending close state, causing all executions to fail.

Fixes #1054.
Fixes #1056.
Fixes #1075.
This commit is contained in:
Olaoluwa Osuntokun 2018-04-12 18:53:22 -07:00
parent f052f18312
commit aaa8fa33b1
No known key found for this signature in database
GPG Key ID: 964EA263DD637C21
2 changed files with 11 additions and 5 deletions

@ -327,19 +327,22 @@ func (c *ChainArbitrator) Start() error {
// For each open channel, we'll configure then launch a corresponding
// ChannelArbitrator.
for _, channel := range openChannels {
chanPoint := channel.FundingOutpoint
// First, we'll create an active chainWatcher for this channel
// to ensure that we detect any relevant on chain events.
chainWatcher, err := newChainWatcher(
channel, c.cfg.Notifier, c.cfg.PreimageDB, c.cfg.Signer,
c.cfg.IsOurAddress, func() error {
return c.resolveContract(channel.FundingOutpoint, nil)
// TODO(roasbeef): also need to pass in log?
return c.resolveContract(chanPoint, nil)
},
)
if err != nil {
return err
}
c.activeWatchers[channel.FundingOutpoint] = chainWatcher
c.activeWatchers[chanPoint] = chainWatcher
channelArb, err := newActiveChannelArbitrator(
channel, c, chainWatcher.SubscribeChannelEvents(false),
)
@ -347,7 +350,7 @@ func (c *ChainArbitrator) Start() error {
return err
}
c.activeChannels[channel.FundingOutpoint] = channelArb
c.activeChannels[chanPoint] = channelArb
}
// In addition to the channels that we know to be open, we'll also

@ -782,11 +782,14 @@ func (c *CooperativeCloseCtx) LogPotentialClose(potentialClose *channeldb.Channe
// pending closed in the database, then launch a goroutine to mark the channel
// fully closed upon confirmation.
func (c *CooperativeCloseCtx) Finalize(preferredClose *channeldb.ChannelCloseSummary) error {
log.Infof("Finalizing chan close for ChannelPoint(%v)",
c.watcher.chanState.FundingOutpoint)
chanPoint := c.watcher.chanState.FundingOutpoint
log.Infof("Finalizing chan close for ChannelPoint(%v)", chanPoint)
err := c.watcher.chanState.CloseChannel(preferredClose)
if err != nil {
log.Errorf("closeCtx: unable to close ChannelPoint(%v): %v",
chanPoint, err)
return err
}