diff --git a/config.go b/config.go index 66d096a2..7923b7a0 100644 --- a/config.go +++ b/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, diff --git a/contractcourt/chain_arbitrator.go b/contractcourt/chain_arbitrator.go index add69c0f..552549f4 100644 --- a/contractcourt/chain_arbitrator.go +++ b/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 diff --git a/contractcourt/chain_arbitrator_test.go b/contractcourt/chain_arbitrator_test.go index 07c31b28..5710b14a 100644 --- a/contractcourt/chain_arbitrator_test.go +++ b/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, diff --git a/contractcourt/channel_arbitrator.go b/contractcourt/channel_arbitrator.go index 8b6896e1..b11c81aa 100644 --- a/contractcourt/channel_arbitrator.go +++ b/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 diff --git a/contractcourt/channel_arbitrator_test.go b/contractcourt/channel_arbitrator_test.go index af4681a3..9375d895 100644 --- a/contractcourt/channel_arbitrator_test.go +++ b/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, + ) +} diff --git a/htlcswitch/switch.go b/htlcswitch/switch.go index b78f43b1..bdd4d446 100644 --- a/htlcswitch/switch.go +++ b/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. diff --git a/htlcswitch/switch_test.go b/htlcswitch/switch_test.go index 28493c83..5a8ace21 100644 --- a/htlcswitch/switch_test.go +++ b/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, diff --git a/server.go b/server.go index a5538483..9ce272f2 100644 --- a/server.go +++ b/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{ diff --git a/test_utils.go b/test_utils.go index afb16cd5..11c025e2 100644 --- a/test_utils.go +++ b/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)