From 13ade7e9cee5937653f11817ba9eef3131e89d52 Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Fri, 21 Jan 2022 07:35:56 +0800 Subject: [PATCH 1/3] htlcswitch+lnwallet: add trace logs for tickers --- htlcswitch/link.go | 7 +++++++ lnwallet/channel.go | 5 ++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/htlcswitch/link.go b/htlcswitch/link.go index a867f6b00..99b758ce9 100644 --- a/htlcswitch/link.go +++ b/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 diff --git a/lnwallet/channel.go b/lnwallet/channel.go index 489265d9a..22a53defc 100644 --- a/lnwallet/channel.go +++ b/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 } From 1c8c0b4fa0e3e912ec543399f8d47bec0f3fda89 Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Fri, 21 Jan 2022 07:44:57 +0800 Subject: [PATCH 2/3] multi: make PendingCommitInterval configurable --- config.go | 30 +++++++++++++++++++++++++---- peer/brontide.go | 49 ++++++++++++++++++++++++++++-------------------- sample-lnd.conf | 7 +++++++ server.go | 1 + 4 files changed, 63 insertions(+), 24 deletions(-) diff --git a/config.go b/config.go index 6f7a66b98..b2f5dfbd0 100644 --- a/config.go +++ b/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) } diff --git a/peer/brontide.go b/peer/brontide.go index 978234c02..8c2664246 100644 --- a/peer/brontide.go +++ b/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, diff --git a/sample-lnd.conf b/sample-lnd.conf index 7740734b9..72bb189a2 100644 --- a/sample-lnd.conf +++ b/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 diff --git a/server.go b/server.go index 28ac51807..4accc6e97 100644 --- a/server.go +++ b/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, From 54ea71bd0ade75ce985082396173c21f944a587b Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Fri, 21 Jan 2022 07:53:37 +0800 Subject: [PATCH 3/3] docs: add release note for PendingCommitTicker --- docs/release-notes/release-notes-0.15.0.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/docs/release-notes/release-notes-0.15.0.md b/docs/release-notes/release-notes-0.15.0.md index 91a8fbae2..047d8f5b3 100644 --- a/docs/release-notes/release-notes-0.15.0.md +++ b/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