contractcourt+switch: keep channels with timed-out initiated htlcs.

This commit enables the user to specify he is not interested in
automatically close channels with pending payments that their
corresponding htlcs have timed-out.
By requiring a configurable grace period uptime of our node
before closing such channels, we give a chance to the other node to
properly cancel the htlc and avoid unnecessary on-chain transaction.
In mobile it is very important for the user experience as otherwise
channels will be force closed more frequently.

config.go

@@ -58,15 +58,16 @@ const (
 	// pending channels permitted per peer.
 	DefaultMaxPendingChannels = 1
 
-	defaultNoSeedBackup             = false
-	defaultTrickleDelay             = 90 * 1000
-	defaultChanStatusSampleInterval = time.Minute
-	defaultChanEnableTimeout        = 19 * time.Minute
-	defaultChanDisableTimeout       = 20 * time.Minute
-	defaultMaxLogFiles              = 3
-	defaultMaxLogFileSize           = 10
-	defaultMinBackoff               = time.Second
-	defaultMaxBackoff               = time.Hour
+	defaultNoSeedBackup                  = false
+	defaultPaymentsExpirationGracePeriod = time.Duration(0)
+	defaultTrickleDelay                  = 90 * 1000
+	defaultChanStatusSampleInterval      = time.Minute
+	defaultChanEnableTimeout             = 19 * time.Minute
+	defaultChanDisableTimeout            = 20 * time.Minute
+	defaultMaxLogFiles                   = 3
+	defaultMaxLogFileSize                = 10
+	defaultMinBackoff                    = time.Second
+	defaultMaxBackoff                    = time.Hour
 
 	defaultTorSOCKSPort            = 9050
 	defaultTorDNSHost              = "soa.nodes.lightning.directory"
@@ -298,10 +299,11 @@ type config struct {
 
 	NoSeedBackup bool `long:"noseedbackup" description:"If true, NO SEED WILL BE EXPOSED AND THE WALLET WILL BE ENCRYPTED USING THE DEFAULT PASSPHRASE -- EVER. THIS FLAG IS ONLY FOR TESTING AND IS BEING DEPRECATED."`
 
-	TrickleDelay             int           `long:"trickledelay" description:"Time in milliseconds between each release of announcements to the network"`
-	ChanEnableTimeout        time.Duration `long:"chan-enable-timeout" description:"The duration that a peer connection must be stable before attempting to send a channel update to reenable or cancel a pending disables of the peer's channels on the network (default: 19m)."`
-	ChanDisableTimeout       time.Duration `long:"chan-disable-timeout" description:"The duration that must elapse after first detecting that an already active channel is actually inactive and sending channel update disabling it to the network. The pending disable can be canceled if the peer reconnects and becomes stable for chan-enable-timeout before the disable update is sent. (default: 20m)"`
-	ChanStatusSampleInterval time.Duration `long:"chan-status-sample-interval" description:"The polling interval between attempts to detect if an active channel has become inactive due to its peer going offline. (default: 1m)"`
+	PaymentsExpirationGracePeriod time.Duration `long:"payments-expiration-grace-period" description:"A period to wait before force closing channels with outgoing htlcs that have timed-out and are a result of this node initiated payments."`
+	TrickleDelay                  int           `long:"trickledelay" description:"Time in milliseconds between each release of announcements to the network"`
+	ChanEnableTimeout             time.Duration `long:"chan-enable-timeout" description:"The duration that a peer connection must be stable before attempting to send a channel update to reenable or cancel a pending disables of the peer's channels on the network (default: 19m)."`
+	ChanDisableTimeout            time.Duration `long:"chan-disable-timeout" description:"The duration that must elapse after first detecting that an already active channel is actually inactive and sending channel update disabling it to the network. The pending disable can be canceled if the peer reconnects and becomes stable for chan-enable-timeout before the disable update is sent. (default: 20m)"`
+	ChanStatusSampleInterval      time.Duration `long:"chan-status-sample-interval" description:"The polling interval between attempts to detect if an active channel has become inactive due to its peer going offline. (default: 1m)"`
 
 	Alias       string `long:"alias" description:"The node alias. Used as a moniker by peers and intelligence services"`
 	Color       string `long:"color" description:"The color of the node in hex format (i.e. '#3399FF'). Used to customize node appearance in intelligence services"`
@@ -416,15 +418,16 @@ func loadConfig() (*config, error) {
 				"preferential": 1.0,
 			},
 		},
-		TrickleDelay:             defaultTrickleDelay,
-		ChanStatusSampleInterval: defaultChanStatusSampleInterval,
-		ChanEnableTimeout:        defaultChanEnableTimeout,
-		ChanDisableTimeout:       defaultChanDisableTimeout,
-		Alias:                    defaultAlias,
-		Color:                    defaultColor,
-		MinChanSize:              int64(minChanFundingSize),
-		NumGraphSyncPeers:        defaultMinPeers,
-		HistoricalSyncInterval:   discovery.DefaultHistoricalSyncInterval,
+		PaymentsExpirationGracePeriod: defaultPaymentsExpirationGracePeriod,
+		TrickleDelay:                  defaultTrickleDelay,
+		ChanStatusSampleInterval:      defaultChanStatusSampleInterval,
+		ChanEnableTimeout:             defaultChanEnableTimeout,
+		ChanDisableTimeout:            defaultChanDisableTimeout,
+		Alias:                         defaultAlias,
+		Color:                         defaultColor,
+		MinChanSize:                   int64(minChanFundingSize),
+		NumGraphSyncPeers:             defaultMinPeers,
+		HistoricalSyncInterval:        discovery.DefaultHistoricalSyncInterval,
 		Tor: &torConfig{
 			SOCKS:   defaultTorSOCKS,
 			DNS:     defaultTorDNS,

contractcourt/chain_arbitrator.go

@@ -5,12 +5,14 @@ import (
 	"fmt"
 	"sync"
 	"sync/atomic"
+	"time"
 
 	"github.com/btcsuite/btcd/chaincfg/chainhash"
 	"github.com/btcsuite/btcd/wire"
 	"github.com/btcsuite/btcutil"
 	"github.com/lightningnetwork/lnd/chainntnfs"
 	"github.com/lightningnetwork/lnd/channeldb"
+	"github.com/lightningnetwork/lnd/clock"
 	"github.com/lightningnetwork/lnd/input"
 	"github.com/lightningnetwork/lnd/lnwallet"
 	"github.com/lightningnetwork/lnd/lnwallet/chainfee"
@@ -153,6 +155,19 @@ type ChainArbitratorConfig struct {
 	// OnionProcessor is used to decode onion payloads for on-chain
 	// resolution.
 	OnionProcessor OnionProcessor
+
+	// PaymentsExpirationGracePeriod indicates is a time window we let the
+	// other node to cancel an outgoing htlc that our node has initiated and
+	// has timed out.
+	PaymentsExpirationGracePeriod time.Duration
+
+	// IsForwardedHTLC checks for a given htlc, identified by channel id and
+	// htlcIndex, if it is a forwarded one.
+	IsForwardedHTLC func(chanID lnwire.ShortChannelID, htlcIndex uint64) bool
+
+	// Clock is the clock implementation that ChannelArbitrator uses.
+	// It is useful for testing.
+	Clock clock.Clock
 }
 
 // ChainArbitrator is a sub-system that oversees the on-chain resolution of all

contractcourt/chain_arbitrator_test.go

@@ -9,6 +9,7 @@ import (
 	"github.com/btcsuite/btcd/chaincfg/chainhash"
 	"github.com/btcsuite/btcd/wire"
 	"github.com/lightningnetwork/lnd/channeldb"
+	"github.com/lightningnetwork/lnd/clock"
 	"github.com/lightningnetwork/lnd/lnwallet"
 )
 
@@ -83,6 +84,7 @@ func TestChainArbitratorRepublishCloses(t *testing.T) {
 			published[tx.TxHash()]++
 			return nil
 		},
+		Clock: clock.NewDefaultClock(),
 	}
 	chainArb := NewChainArbitrator(
 		chainArbCfg, db,
@@ -171,6 +173,7 @@ func TestResolveContract(t *testing.T) {
 		PublishTx: func(tx *wire.MsgTx) error {
 			return nil
 		},
+		Clock: clock.NewDefaultClock(),
 	}
 	chainArb := NewChainArbitrator(
 		chainArbCfg, db,

contractcourt/channel_arbitrator.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"sync"
 	"sync/atomic"
+	"time"
 
 	"github.com/btcsuite/btcd/wire"
 	"github.com/btcsuite/btcutil"
@@ -260,6 +261,9 @@ type ChannelArbitrator struct {
 	started int32 // To be used atomically.
 	stopped int32 // To be used atomically.
 
+	// startTimestamp is the time when this ChannelArbitrator was started.
+	startTimestamp time.Time
+
 	// log is a persistent log that the attendant will use to checkpoint
 	// its next action, and the state of any unresolved contracts.
 	log ArbitratorLog
@@ -328,6 +332,7 @@ func (c *ChannelArbitrator) Start() error {
 	if !atomic.CompareAndSwapInt32(&c.started, 0, 1) {
 		return nil
 	}
+	c.startTimestamp = c.cfg.Clock.Now()
 
 	var (
 		err error
@@ -1132,7 +1137,29 @@ func (c *ChannelArbitrator) shouldGoOnChain(htlc channeldb.HTLC,
 
 	// We should on-chain for this HTLC, iff we're within out broadcast
 	// cutoff window.
-	return currentHeight >= broadcastCutOff
+	if currentHeight < broadcastCutOff {
+		return false
+	}
+
+	// In case of incoming htlc we should go to chain.
+	if htlc.Incoming {
+		return true
+	}
+
+	// For htlcs that are result of our initiated payments we give some grace
+	// period before force closing the channel. During this time we expect
+	// both nodes to connect and give a chance to the other node to send its
+	// updates and cancel the htlc.
+	// This shouldn't add any security risk as there is no incoming htlc to
+	// fulfill at this case and the expectation is that when the channel is
+	// active the other node will send update_fail_htlc to remove the htlc
+	// without closing the channel. It is up to the user to force close the
+	// channel if the peer misbehaves and doesn't send the update_fail_htlc.
+	// It is useful when this node is most of the time not online and is
+	// likely to miss the time slot where the htlc may be cancelled.
+	isForwarded := c.cfg.IsForwardedHTLC(c.cfg.ShortChanID, htlc.HtlcIndex)
+	upTime := c.cfg.Clock.Now().Sub(c.startTimestamp)
+	return isForwarded || upTime > c.cfg.PaymentsExpirationGracePeriod
 }
 
 // checkCommitChainActions is called for each new block connected to the end of

contractcourt/channel_arbitrator_test.go

@@ -15,6 +15,7 @@ import (
 	"github.com/coreos/bbolt"
 	"github.com/lightningnetwork/lnd/chainntnfs"
 	"github.com/lightningnetwork/lnd/channeldb"
+	"github.com/lightningnetwork/lnd/clock"
 	"github.com/lightningnetwork/lnd/input"
 	"github.com/lightningnetwork/lnd/lnwallet"
 	"github.com/lightningnetwork/lnd/lnwire"
@@ -309,6 +310,12 @@ func createTestChannelArbitrator(t *testing.T, log ArbitratorLog) (*chanArbTestC
 			return nil
 		},
 		OnionProcessor: &mockOnionProcessor{},
+		IsForwardedHTLC: func(chanID lnwire.ShortChannelID,
+			htlcIndex uint64) bool {
+
+			return true
+		},
+		Clock: clock.NewDefaultClock(),
 	}
 
 	// We'll use the resolvedChan to synchronize on call to
@@ -1811,3 +1818,87 @@ func TestChannelArbitratorDanglingCommitForceClose(t *testing.T) {
 		})
 	}
 }
+
+// TestChannelArbitratorPendingExpiredHTLC tests that if we have pending htlc
+// that is expired we will only go to chain if we are running at least the
+// time defined in PaymentsExpirationGracePeriod.
+// During this time the remote party is expected to send his updates and cancel
+// The htlc.
+func TestChannelArbitratorPendingExpiredHTLC(t *testing.T) {
+	t.Parallel()
+
+	// We'll create the arbitrator and its backing log in a default state.
+	log := &mockArbitratorLog{
+		state:     StateDefault,
+		newStates: make(chan ArbitratorState, 5),
+		resolvers: make(map[ContractResolver]struct{}),
+	}
+	chanArbCtx, err := createTestChannelArbitrator(t, log)
+	if err != nil {
+		t.Fatalf("unable to create ChannelArbitrator: %v", err)
+	}
+	chanArb := chanArbCtx.chanArb
+
+	// We'll inject a test clock implementation so we can control the uptime.
+	startTime := time.Date(2020, time.February, 3, 13, 0, 0, 0, time.UTC)
+	testClock := clock.NewTestClock(startTime)
+	chanArb.cfg.Clock = testClock
+
+	// We also configure the grace period and the IsForwardedHTLC to identify
+	// the htlc as our initiated payment.
+	chanArb.cfg.PaymentsExpirationGracePeriod = time.Second * 15
+	chanArb.cfg.IsForwardedHTLC = func(chanID lnwire.ShortChannelID,
+		htlcIndex uint64) bool {
+
+		return false
+	}
+
+	if err := chanArb.Start(); err != nil {
+		t.Fatalf("unable to start ChannelArbitrator: %v", err)
+	}
+	defer func() {
+		if err := chanArb.Stop(); err != nil {
+			t.Fatalf("unable to stop chan arb: %v", err)
+		}
+	}()
+
+	// Now that our channel arb has started, we'll set up
+	// its contract signals channel so we can send it
+	// various HTLC updates for this test.
+	htlcUpdates := make(chan *ContractUpdate)
+	signals := &ContractSignals{
+		HtlcUpdates: htlcUpdates,
+		ShortChanID: lnwire.ShortChannelID{},
+	}
+	chanArb.UpdateContractSignals(signals)
+
+	// Next, we'll send it a new HTLC that is set to expire
+	// in 10 blocks.
+	htlcIndex := uint64(99)
+	htlcExpiry := uint32(10)
+	pendingHTLC := channeldb.HTLC{
+		Incoming:      false,
+		Amt:           10000,
+		HtlcIndex:     htlcIndex,
+		RefundTimeout: htlcExpiry,
+	}
+	htlcUpdates <- &ContractUpdate{
+		HtlcKey: RemoteHtlcSet,
+		Htlcs:   []channeldb.HTLC{pendingHTLC},
+	}
+
+	// We will advance the uptime to 10 seconds which should be still within
+	// the grace period and should not trigger going to chain.
+	testClock.SetTime(startTime.Add(time.Second * 10))
+	chanArbCtx.blockEpochs <- &chainntnfs.BlockEpoch{Height: 5}
+	chanArbCtx.AssertState(StateDefault)
+
+	// We will advance the uptime to 16 seconds which should trigger going
+	// to chain.
+	testClock.SetTime(startTime.Add(time.Second * 16))
+	chanArbCtx.blockEpochs <- &chainntnfs.BlockEpoch{Height: 6}
+	chanArbCtx.AssertStateTransitions(
+		StateBroadcastCommit,
+		StateCommitmentBroadcasted,
+	)
+}

htlcswitch/switch.go

@@ -460,6 +460,18 @@ func (s *Switch) UpdateForwardingPolicies(
 	s.indexMtx.RUnlock()
 }
 
+// IsForwardedHTLC checks for a given channel and htlc index if it is related
+// to an opened circuit that represents a forwarded payment.
+func (s *Switch) IsForwardedHTLC(chanID lnwire.ShortChannelID,
+	htlcIndex uint64) bool {
+
+	circuit := s.circuits.LookupOpenCircuit(channeldb.CircuitKey{
+		ChanID: chanID,
+		HtlcID: htlcIndex,
+	})
+	return circuit != nil && circuit.Incoming.ChanID != hop.Source
+}
+
 // forward is used in order to find next channel link and apply htlc update.
 // Also this function is used by channel links itself in order to forward the
 // update after it has been included in the channel.

htlcswitch/switch_test.go

@@ -322,6 +322,10 @@ func TestSwitchForward(t *testing.T) {
 		t.Fatal("wrong amount of circuits")
 	}
 
+	if !s.IsForwardedHTLC(bobChannelLink.ShortChanID(), 0) {
+		t.Fatal("htlc should be identified as forwarded")
+	}
+
 	// Create settle request pretending that bob link handled the add htlc
 	// request and sent the htlc settle request back. This request should
 	// be forwarder back to Alice link.
@@ -1892,6 +1896,9 @@ func TestSwitchSendPayment(t *testing.T) {
 		t.Fatalf("unable obfuscate failure: %v", err)
 	}
 
+	if s.IsForwardedHTLC(aliceChannelLink.ShortChanID(), update.ID) {
+		t.Fatal("htlc should be identified as not forwarded")
+	}
 	packet := &htlcPacket{
 		outgoingChanID: aliceChannelLink.ShortChanID(),
 		outgoingHTLCID: 0,

server.go

@@ -915,11 +915,14 @@ func newServer(listenAddrs []net.Addr, chanDB *channeldb.DB,
 				return ErrServerShuttingDown
 			}
 		},
-		DisableChannel:      s.chanStatusMgr.RequestDisable,
-		Sweeper:             s.sweeper,
-		Registry:            s.invoices,
-		NotifyClosedChannel: s.channelNotifier.NotifyClosedChannelEvent,
-		OnionProcessor:      s.sphinx,
+		DisableChannel:                s.chanStatusMgr.RequestDisable,
+		Sweeper:                       s.sweeper,
+		Registry:                      s.invoices,
+		NotifyClosedChannel:           s.channelNotifier.NotifyClosedChannelEvent,
+		OnionProcessor:                s.sphinx,
+		PaymentsExpirationGracePeriod: cfg.PaymentsExpirationGracePeriod,
+		IsForwardedHTLC:               s.htlcSwitch.IsForwardedHTLC,
+		Clock:                         clock.NewDefaultClock(),
 	}, chanDB)
 
 	s.breachArbiter = newBreachArbiter(&BreachConfig{

test_utils.go

@@ -17,6 +17,7 @@ import (
 	"github.com/btcsuite/btcutil"
 	"github.com/lightningnetwork/lnd/chainntnfs"
 	"github.com/lightningnetwork/lnd/channeldb"
+	"github.com/lightningnetwork/lnd/clock"
 	"github.com/lightningnetwork/lnd/contractcourt"
 	"github.com/lightningnetwork/lnd/htlcswitch"
 	"github.com/lightningnetwork/lnd/input"
@@ -360,6 +361,12 @@ func createTestPeer(notifier chainntnfs.ChainNotifier, publTx chan *wire.MsgTx,
 		contractcourt.ChainArbitratorConfig{
 			Notifier: notifier,
 			ChainIO:  chainIO,
+			IsForwardedHTLC: func(chanID lnwire.ShortChannelID,
+				htlcIndex uint64) bool {
+
+				return true
+			},
+			Clock: clock.NewDefaultClock(),
 		}, dbAlice,
 	)
 	chainArb.WatchNewChannel(aliceChannelState)