Merge pull request #6186 from yyforyongyu/itest-fix-pending-ticker

htlcswitch: fix PendingCommitTicker and make it configurable

config.go

@@ -168,14 +168,24 @@ const (
 	// TODO(halseth): find a more scientific choice of value.
 	defaultMaxLocalCSVDelay = 10000
 
-	// defaultChannelCommitInterval is the default maximum time between receiving a
-	// channel state update and signing a new commitment.
+	// defaultChannelCommitInterval is the default maximum time between
+	// receiving a channel state update and signing a new commitment.
 	defaultChannelCommitInterval = 50 * time.Millisecond
 
 	// maxChannelCommitInterval is the maximum time the commit interval can
 	// be configured to.
 	maxChannelCommitInterval = time.Hour
 
+	// defaultPendingCommitInterval specifies the default timeout value
+	// while waiting for the remote party to revoke a locally initiated
+	// commitment state.
+	defaultPendingCommitInterval = 1 * time.Minute
+
+	// maxPendingCommitInterval specifies the max allowed duration when
+	// waiting for the remote party to revoke a locally initiated
+	// commitment state.
+	maxPendingCommitInterval = 5 * time.Minute
+
 	// defaultChannelCommitBatchSize is the default maximum number of
 	// channel state updates that is accumulated before signing a new
 	// commitment.
@@ -340,8 +350,11 @@ type Config struct {
 	MaxChanSize                   int64         `long:"maxchansize" description:"The largest channel size (in satoshis) that we should accept. Incoming channels larger than this will be rejected"`
 	CoopCloseTargetConfs          uint32        `long:"coop-close-target-confs" description:"The target number of blocks that a cooperative channel close transaction should confirm in. This is used to estimate the fee to use as the lower bound during fee negotiation for the channel closure."`
 
-	ChannelCommitInterval  time.Duration `long:"channel-commit-interval" description:"The maximum time that is allowed to pass between receiving a channel state update and signing the next commitment. Setting this to a longer duration allows for more efficient channel operations at the cost of latency."`
-	ChannelCommitBatchSize uint32        `long:"channel-commit-batch-size" description:"The maximum number of channel state updates that is accumulated before signing a new commitment."`
+	ChannelCommitInterval time.Duration `long:"channel-commit-interval" description:"The maximum time that is allowed to pass between receiving a channel state update and signing the next commitment. Setting this to a longer duration allows for more efficient channel operations at the cost of latency."`
+
+	PendingCommitInterval time.Duration `long:"pending-commit-interval" description:"The maximum time that is allowed to pass while waiting for the remote party to revoke a locally initiated commitment state. Setting this to a longer duration if a slow response is expected from the remote party or large number of payments are attempted at the same time."`
+
+	ChannelCommitBatchSize uint32 `long:"channel-commit-batch-size" description:"The maximum number of channel state updates that is accumulated before signing a new commitment."`
 
 	DefaultRemoteMaxHtlcs uint16 `long:"default-remote-max-htlcs" description:"The default max_htlc applied when opening or accepting channels. This value limits the number of concurrent HTLCs that the remote party can add to the commitment. The maximum possible value is 483."`
 
@@ -601,6 +614,7 @@ func DefaultConfig() Config {
 		registeredChains:        chainreg.NewChainRegistry(),
 		ActiveNetParams:         chainreg.BitcoinTestNetParams,
 		ChannelCommitInterval:   defaultChannelCommitInterval,
+		PendingCommitInterval:   defaultPendingCommitInterval,
 		ChannelCommitBatchSize:  defaultChannelCommitBatchSize,
 		CoinSelectionStrategy:   defaultCoinSelectionStrategy,
 		RemoteSigner: &lncfg.RemoteSigner{
@@ -1578,6 +1592,14 @@ func ValidateConfig(cfg Config, interceptor signal.Interceptor, fileParser,
 			maxChannelCommitInterval)
 	}
 
+	// Limit PendingCommitInterval so we don't wait too long for the remote
+	// party to send back a revoke.
+	if cfg.PendingCommitInterval > maxPendingCommitInterval {
+		return nil, mkErr("pending-commit-interval (%v) must be less "+
+			"than %v", cfg.PendingCommitInterval,
+			maxPendingCommitInterval)
+	}
+
 	if err := cfg.Gossip.Parse(); err != nil {
 		return nil, mkErr("error parsing gossip syncer: %v", err)
 	}

docs/release-notes/release-notes-0.15.0.md

@@ -101,10 +101,14 @@
 * [Make etcd leader election session
   TTL](https://github.com/lightningnetwork/lnd/pull/6342) configurable.
 
-
 * [Fix race condition in the htlc interceptor unit
   test](https://github.com/lightningnetwork/lnd/pull/6353).
 
+* [A new config option, `pending-commit-interval` is
+  added](https://github.com/lightningnetwork/lnd/pull/6186). This value
+  specifies the maximum duration it allows for a remote peer to respond to a
+  locally initiated commitment update.
+
 ## RPC Server
 
 * [Add value to the field

htlcswitch/link.go

@@ -1084,8 +1084,13 @@ func (l *channelLink) htlcManager() {
 		// batch is empty.
 		if l.channel.PendingLocalUpdateCount() > 0 {
 			l.cfg.BatchTicker.Resume()
+			l.log.Tracef("BatchTicker resumed, "+
+				"PendingLocalUpdateCount=%d",
+				l.channel.PendingLocalUpdateCount())
 		} else {
 			l.cfg.BatchTicker.Pause()
+			l.log.Trace("BatchTicker paused due to zero " +
+				"PendingLocalUpdateCount")
 		}
 
 		select {
@@ -2102,6 +2107,7 @@ func (l *channelLink) updateCommitTx() error {
 	theirCommitSig, htlcSigs, pendingHTLCs, err := l.channel.SignNextCommitment()
 	if err == lnwallet.ErrNoWindow {
 		l.cfg.PendingCommitTicker.Resume()
+		l.log.Trace("PendingCommitTicker resumed")
 
 		l.log.Tracef("revocation window exhausted, unable to send: "+
 			"%v, pend_updates=%v, dangling_closes%v",
@@ -2123,6 +2129,7 @@ func (l *channelLink) updateCommitTx() error {
 	}
 
 	l.cfg.PendingCommitTicker.Pause()
+	l.log.Trace("PendingCommitTicker paused after ackDownStreamPackets")
 
 	// The remote party now has a new pending commitment, so we'll update
 	// the contract court to be aware of this new set (the prior old remote

lnwallet/channel.go

@@ -3530,7 +3530,10 @@ func (lc *LightningChannel) SignNextCommitment() (lnwire.Sig, []lnwire.Sig,
 	// party, then we're unable to create new states. Each time we create a
 	// new state, we consume a prior revocation point.
 	commitPoint := lc.channelState.RemoteNextRevocation
-	if lc.remoteCommitChain.hasUnackedCommitment() || commitPoint == nil {
+	unacked := lc.remoteCommitChain.hasUnackedCommitment()
+	if unacked || commitPoint == nil {
+		lc.log.Tracef("waiting for remote ack=%v, nil "+
+			"RemoteNextRevocation: %v", unacked, commitPoint == nil)
 		return sig, htlcSigs, nil, ErrNoWindow
 	}
 

peer/brontide.go

@@ -318,6 +318,13 @@ type Config struct {
 	// operations at the cost of latency.
 	ChannelCommitInterval time.Duration
 
+	// PendingCommitInterval is the maximum time that is allowed to pass
+	// while waiting for the remote party to revoke a locally initiated
+	// commitment state. Setting this to a longer duration if a slow
+	// response is expected from the remote party or large number of
+	// payments are attempted at the same time.
+	PendingCommitInterval time.Duration
+
 	// ChannelCommitBatchSize is the maximum number of channel state updates
 	// that is accumulated before signing a new commitment.
 	ChannelCommitBatchSize uint32
@@ -832,26 +839,28 @@ func (p *Brontide) addLink(chanPoint *wire.OutPoint,
 	}
 
 	linkCfg := htlcswitch.ChannelLinkConfig{
-		Peer:                    p,
-		DecodeHopIterators:      p.cfg.Sphinx.DecodeHopIterators,
-		ExtractErrorEncrypter:   p.cfg.Sphinx.ExtractErrorEncrypter,
-		FetchLastChannelUpdate:  p.cfg.FetchLastChanUpdate,
-		HodlMask:                p.cfg.Hodl.Mask(),
-		Registry:                p.cfg.Invoices,
-		BestHeight:              p.cfg.Switch.BestHeight,
-		Circuits:                p.cfg.Switch.CircuitModifier(),
-		ForwardPackets:          p.cfg.InterceptSwitch.ForwardPackets,
-		FwrdingPolicy:           *forwardingPolicy,
-		FeeEstimator:            p.cfg.FeeEstimator,
-		PreimageCache:           p.cfg.WitnessBeacon,
-		ChainEvents:             chainEvents,
-		UpdateContractSignals:   updateContractSignals,
-		NotifyContractUpdate:    notifyContractUpdate,
-		OnChannelFailure:        onChannelFailure,
-		SyncStates:              syncStates,
-		BatchTicker:             ticker.New(p.cfg.ChannelCommitInterval),
-		FwdPkgGCTicker:          ticker.New(time.Hour),
-		PendingCommitTicker:     ticker.New(time.Minute),
+		Peer:                   p,
+		DecodeHopIterators:     p.cfg.Sphinx.DecodeHopIterators,
+		ExtractErrorEncrypter:  p.cfg.Sphinx.ExtractErrorEncrypter,
+		FetchLastChannelUpdate: p.cfg.FetchLastChanUpdate,
+		HodlMask:               p.cfg.Hodl.Mask(),
+		Registry:               p.cfg.Invoices,
+		BestHeight:             p.cfg.Switch.BestHeight,
+		Circuits:               p.cfg.Switch.CircuitModifier(),
+		ForwardPackets:         p.cfg.InterceptSwitch.ForwardPackets,
+		FwrdingPolicy:          *forwardingPolicy,
+		FeeEstimator:           p.cfg.FeeEstimator,
+		PreimageCache:          p.cfg.WitnessBeacon,
+		ChainEvents:            chainEvents,
+		UpdateContractSignals:  updateContractSignals,
+		NotifyContractUpdate:   notifyContractUpdate,
+		OnChannelFailure:       onChannelFailure,
+		SyncStates:             syncStates,
+		BatchTicker:            ticker.New(p.cfg.ChannelCommitInterval),
+		FwdPkgGCTicker:         ticker.New(time.Hour),
+		PendingCommitTicker: ticker.New(
+			p.cfg.PendingCommitInterval,
+		),
 		BatchSize:               p.cfg.ChannelCommitBatchSize,
 		UnsafeReplay:            p.cfg.UnsafeReplay,
 		MinFeeUpdateTimeout:     htlcswitch.DefaultMinLinkFeeUpdateTimeout,

sample-lnd.conf

@@ -297,6 +297,13 @@
 ; capped at 1 hour. The default is 50 milliseconds.
 ; channel-commit-interval=1h
 
+; The maximum time that is allowed to pass while waiting for the remote party
+; to revoke a locally initiated commitment state. Setting this to a longer
+; duration if a slow response is expected from the remote party or large
+; number of payments are attempted at the same time. 
+; (default: 1m0s)
+; pending-commit-interval=5m
+
 ; The maximum number of channel state updates that is accumulated before signing
 ; a new commitment.
 ; channel-commit-batch-size=10

server.go

@@ -3526,6 +3526,7 @@ func (s *server) peerConnected(conn net.Conn, connReq *connmgr.ConnReq,
 		MaxAnchorsCommitFeeRate: chainfee.SatPerKVByte(
 			s.cfg.MaxCommitFeeRateAnchors * 1000).FeePerKWeight(),
 		ChannelCommitInterval:  s.cfg.ChannelCommitInterval,
+		PendingCommitInterval:  s.cfg.PendingCommitInterval,
 		ChannelCommitBatchSize: s.cfg.ChannelCommitBatchSize,
 		HandleCustomMessage:    s.handleCustomMessage,
 		Quit:                   s.quit,