diff --git a/htlcswitch/link.go b/htlcswitch/link.go index 2cdba393..a29b4fd6 100644 --- a/htlcswitch/link.go +++ b/htlcswitch/link.go @@ -30,18 +30,6 @@ func init() { } const ( - // expiryGraceDelta is the minimum difference between the current block - // height and the CLTV we require on 1) an outgoing HTLC in order to - // forward as an intermediary hop, or 2) an incoming HTLC to reveal the - // preimage as the final hop. We'll reject any HTLC's who's timeout minus - // this value is less than or equal to the current block height. We require - // this in order to ensure that we have sufficient time to claim or - // timeout an HTLC on chain. - // - // This MUST be greater than the maximum BroadcastDelta of the - // ChannelArbitrator for the outbound channel. - expiryGraceDelta = 2 - // maxCltvExpiry is the maximum outgoing time lock that the node accepts // for forwarded payments. The value is relative to the current block // height. The reason to have a maximum is to prevent funds getting @@ -246,6 +234,18 @@ type ChannelLinkConfig struct { // fee rate. A random timeout will be selected between these values. MinFeeUpdateTimeout time.Duration MaxFeeUpdateTimeout time.Duration + + // ExpiryGraceDelta is the minimum difference between the current block + // height and the CLTV we require on 1) an outgoing HTLC in order to + // forward as an intermediary hop, or 2) an incoming HTLC to reveal the + // preimage as the final hop. We'll reject any HTLC's who's timeout minus + // this value is less than or equal to the current block height. We require + // this in order to ensure that we have sufficient time to claim or + // timeout an HTLC on chain. + // + // This MUST be greater than the maximum BroadcastDelta of the + // ChannelArbitrator for the outbound channel. + ExpiryGraceDelta uint32 } // channelLink is the service which drives a channel's commitment update @@ -2150,9 +2150,9 @@ func (l *channelLink) HtlcSatifiesPolicy(payHash [32]byte, } // We want to avoid offering an HTLC which will expire in the near - // future, so we'll reject an HTLC if the outgoing expiration time is - // too close to the current height. - if outgoingTimeout-expiryGraceDelta <= heightNow { + // future, so we'll reject an HTLC if the outgoing expiration time is too + // close to the current height. + if outgoingTimeout-l.cfg.ExpiryGraceDelta <= heightNow { l.errorf("htlc(%x) has an expiry that's too soon: "+ "outgoing_expiry=%v, best_height=%v", payHash[:], outgoingTimeout, heightNow) @@ -2675,7 +2675,7 @@ func (l *channelLink) processExitHop(pd *lnwallet.PaymentDescriptor, // First, we'll check the expiry of the HTLC itself against, the current // block height. If the timeout is too soon, then we'll reject the HTLC. - if pd.Timeout-expiryGraceDelta <= heightNow { + if pd.Timeout-l.cfg.ExpiryGraceDelta <= heightNow { log.Errorf("htlc(%x) has an expiry that's too soon: expiry=%v"+ ", best_height=%v", pd.RHash[:], pd.Timeout, heightNow) diff --git a/htlcswitch/link_test.go b/htlcswitch/link_test.go index 8d41cdd5..4d8143fa 100644 --- a/htlcswitch/link_test.go +++ b/htlcswitch/link_test.go @@ -1336,10 +1336,13 @@ func TestChannelLinkExpiryTooSoonExitNode(t *testing.T) { amount := lnwire.NewMSatFromSatoshis(btcutil.SatoshiPerBitcoin) - // We'll craft an HTLC packet, but set the starting height to 10 blocks - // before the current true height. + // We'll craft an HTLC packet, but set the final hop CLTV to 3 blocks + // after the current true height. This is less or equal to the expiry + // grace delta of 3, so we expect the incoming htlc to be failed by the + // exit hop. + lastHopDelta := n.firstBobChannelLink.cfg.FwrdingPolicy.TimeLockDelta htlcAmt, totalTimelock, hops := generateHops(amount, - startingHeight-10, n.firstBobChannelLink) + startingHeight+3-lastHopDelta, n.firstBobChannelLink) // Now we'll send out the payment from Alice to Bob. firstHop := n.firstBobChannelLink.ShortChanID() @@ -1395,11 +1398,14 @@ func TestChannelLinkExpiryTooSoonMidNode(t *testing.T) { amount := lnwire.NewMSatFromSatoshis(btcutil.SatoshiPerBitcoin) - // We'll craft an HTLC packet, but set the starting height to 10 blocks - // before the current true height. The final route will be three hops, - // so the middle hop should detect the issue. + // We'll craft an HTLC packet, but set the starting height to 3 blocks + // before the current true height. This means that the outgoing time + // lock of the middle hop will be at starting height + 3 blocks (channel + // policy time lock delta is 6 blocks). There is an expiry grace delta + // of 3 blocks relative to the current height, meaning that htlc will + // not be sent out by the middle hop. htlcAmt, totalTimelock, hops := generateHops(amount, - startingHeight-10, n.firstBobChannelLink, n.carolChannelLink) + startingHeight-3, n.firstBobChannelLink, n.carolChannelLink) // Now we'll send out the payment from Alice to Bob. firstHop := n.firstBobChannelLink.ShortChanID() diff --git a/htlcswitch/test_utils.go b/htlcswitch/test_utils.go index 053c99cf..19a6d19c 100644 --- a/htlcswitch/test_utils.go +++ b/htlcswitch/test_utils.go @@ -1019,6 +1019,7 @@ func (h *hopNetwork) createChannelLink(server, peer *mockServer, fwdPkgTimeout = 15 * time.Second minFeeUpdateTimeout = 30 * time.Minute maxFeeUpdateTimeout = 40 * time.Minute + expiryGraceDelta = 3 ) link := NewChannelLink( @@ -1048,6 +1049,7 @@ func (h *hopNetwork) createChannelLink(server, peer *mockServer, MinFeeUpdateTimeout: minFeeUpdateTimeout, MaxFeeUpdateTimeout: maxFeeUpdateTimeout, OnChannelFailure: func(lnwire.ChannelID, lnwire.ShortChannelID, LinkFailureError) {}, + ExpiryGraceDelta: expiryGraceDelta, }, channel, ) diff --git a/lnd_test.go b/lnd_test.go index 6d9a2114..be04bf8a 100644 --- a/lnd_test.go +++ b/lnd_test.go @@ -8164,6 +8164,14 @@ out: t.Fatalf("unable to generate carol invoice: %v", err) } + carolPayReq, err := carol.DecodePayReq(ctxb, + &lnrpc.PayReqString{ + PayReq: carolInvoice.PaymentRequest, + }) + if err != nil { + t.Fatalf("unable to decode generated payment request: %v", err) + } + // Before we send the payment, ensure that the announcement of the new // channel has been processed by Alice. ctxt, _ = context.WithTimeout(ctxb, defaultTimeout) @@ -8180,6 +8188,7 @@ out: PaymentHashString: hex.EncodeToString(makeFakePayHash(t)), DestString: hex.EncodeToString(carol.PubKey[:]), Amt: payAmt, + FinalCltvDelta: int32(carolPayReq.CltvExpiry), } resp, err := net.Alice.SendPaymentSync(ctxt, sendReq) if err != nil { @@ -8210,6 +8219,7 @@ out: PaymentHashString: hex.EncodeToString(carolInvoice.RHash), DestString: hex.EncodeToString(carol.PubKey[:]), Amt: int64(htlcAmt.ToSatoshis()), // 10k satoshis are expected. + FinalCltvDelta: int32(carolPayReq.CltvExpiry), } ctxt, _ = context.WithTimeout(ctxb, defaultTimeout) resp, err = net.Alice.SendPaymentSync(ctxt, sendReq) @@ -9652,9 +9662,12 @@ func testMultiHopReceiverChainClaim(net *lntest.NetworkHarness, t *harnessTest) defer shutdownAndAssert(net, t, carol) // With the network active, we'll now add a new invoice at Carol's end. + // Make sure the cltv expiry delta is large enough, otherwise Bob won't + // send out the outgoing htlc. const invoiceAmt = 100000 invoiceReq := &lnrpc.Invoice{ - Value: invoiceAmt, + Value: invoiceAmt, + CltvExpiry: 20, } ctxt, _ := context.WithTimeout(ctxb, defaultTimeout) carolInvoice, err := carol.AddInvoice(ctxt, invoiceReq) @@ -9698,7 +9711,7 @@ func testMultiHopReceiverChainClaim(net *lntest.NetworkHarness, t *harnessTest) // Now we'll mine enough blocks to prompt carol to actually go to the // chain in order to sweep her HTLC since the value is high enough. // TODO(roasbeef): modify once go to chain policy changes - numBlocks := uint32(defaultBitcoinTimeLockDelta - (2 * defaultBroadcastDelta)) + numBlocks := uint32(invoiceReq.CltvExpiry - defaultBroadcastDelta) if _, err := net.Miner.Node.Generate(numBlocks); err != nil { t.Fatalf("unable to generate blocks") } @@ -10382,7 +10395,8 @@ func testMultiHopHtlcLocalChainClaim(net *lntest.NetworkHarness, t *harnessTest) // With the network active, we'll now add a new invoice at Carol's end. invoiceReq := &lnrpc.Invoice{ - Value: 100000, + Value: 100000, + CltvExpiry: 20, } ctxt, _ := context.WithTimeout(ctxb, defaultTimeout) carolInvoice, err := carol.AddInvoice(ctxt, invoiceReq) @@ -10438,7 +10452,7 @@ func testMultiHopHtlcLocalChainClaim(net *lntest.NetworkHarness, t *harnessTest) // We'll now mine enough blocks so Carol decides that she needs to go // on-chain to claim the HTLC as Bob has been inactive. - numBlocks := uint32(defaultBitcoinTimeLockDelta - (2 * defaultBroadcastDelta)) + numBlocks := uint32(20 - defaultBroadcastDelta) if _, err := net.Miner.Node.Generate(numBlocks); err != nil { t.Fatalf("unable to generate blocks") } @@ -10722,7 +10736,8 @@ func testMultiHopHtlcRemoteChainClaim(net *lntest.NetworkHarness, t *harnessTest // With the network active, we'll now add a new invoice at Carol's end. const invoiceAmt = 100000 invoiceReq := &lnrpc.Invoice{ - Value: invoiceAmt, + Value: invoiceAmt, + CltvExpiry: 20, } ctxt, _ := context.WithTimeout(ctxb, defaultTimeout) carolInvoice, err := carol.AddInvoice(ctxt, invoiceReq) @@ -10792,8 +10807,7 @@ func testMultiHopHtlcRemoteChainClaim(net *lntest.NetworkHarness, t *harnessTest // We'll now mine enough blocks so Carol decides that she needs to go // on-chain to claim the HTLC as Bob has been inactive. - claimDelta := uint32(2 * defaultBroadcastDelta) - numBlocks := uint32(defaultBitcoinTimeLockDelta-claimDelta) - defaultCSV + numBlocks := uint32(20-defaultBroadcastDelta) - defaultCSV if _, err := net.Miner.Node.Generate(numBlocks); err != nil { t.Fatalf("unable to generate blocks") } diff --git a/peer.go b/peer.go index f1fe9490..16c1878c 100644 --- a/peer.go +++ b/peer.go @@ -58,6 +58,13 @@ const ( // messages to be sent across the wire, requested by objects outside // this struct. outgoingQueueLen = 50 + + // extraExpiryGraceDelta is the additional number of blocks required by + // the ExpiryGraceDelta of the forwarding policy beyond the maximum + // broadcast delta. This is the minimum number of blocks between the + // expiry on an accepted or offered HTLC and the block height at which + // we will go to chain. + extraExpiryGraceDelta = 3 ) // outgoingMsg packages an lnwire.Message to be sent out on the wire, along with @@ -199,6 +206,12 @@ type peer struct { // remote node. localFeatures *lnwire.RawFeatureVector + // expiryGraceDelta is the block time allowance for HTLCs offered and + // received on channels with this peer. The parameter is used to + // configure links with the peer. See ExpiryGraceDelta on + // ChannelLinkConfig for more information. + expiryGraceDelta uint32 + // remoteLocalFeatures is the local feature vector received from the // peer during the connection handshake. remoteLocalFeatures *lnwire.FeatureVector @@ -236,7 +249,8 @@ var _ lnpeer.Peer = (*peer)(nil) func newPeer(conn net.Conn, connReq *connmgr.ConnReq, server *server, addr *lnwire.NetAddress, inbound bool, localFeatures *lnwire.RawFeatureVector, - chanActiveTimeout time.Duration) (*peer, error) { + chanActiveTimeout time.Duration, expiryGraceDelta uint32) ( + *peer, error) { nodePub := addr.IdentityKey @@ -249,7 +263,8 @@ func newPeer(conn net.Conn, connReq *connmgr.ConnReq, server *server, server: server, - localFeatures: localFeatures, + localFeatures: localFeatures, + expiryGraceDelta: expiryGraceDelta, sendQueue: make(chan outgoingMsg), outgoingQueue: make(chan outgoingMsg), @@ -587,6 +602,7 @@ func (p *peer) addLink(chanPoint *wire.OutPoint, UnsafeReplay: cfg.UnsafeReplay, MinFeeUpdateTimeout: htlcswitch.DefaultMinLinkFeeUpdateTimeout, MaxFeeUpdateTimeout: htlcswitch.DefaultMaxLinkFeeUpdateTimeout, + ExpiryGraceDelta: p.expiryGraceDelta, } link := htlcswitch.NewChannelLink(linkCfg, lnChan) diff --git a/server.go b/server.go index d86f340c..b4f7d765 100644 --- a/server.go +++ b/server.go @@ -711,9 +711,7 @@ func newServer(listenAddrs []net.Addr, chanDB *channeldb.DB, cc *chainControl, contractBreaches := make(chan *ContractBreachEvent, 1) s.chainArb = contractcourt.NewChainArbitrator(contractcourt.ChainArbitratorConfig{ - ChainHash: *activeNetParams.GenesisHash, - // TODO(roasbeef): properly configure - // * needs to be << or specified final hop time delta + ChainHash: *activeNetParams.GenesisHash, BroadcastDelta: defaultBroadcastDelta, NewSweepAddr: func() ([]byte, error) { return newSweepPkScript(cc.wallet) @@ -2373,11 +2371,15 @@ func (s *server) peerConnected(conn net.Conn, connReq *connmgr.ConnReq, localFeatures.Set(lnwire.DataLossProtectRequired) localFeatures.Set(lnwire.GossipQueriesOptional) - // Now that we've established a connection, create a peer, and it to - // the set of currently active peers. + // Now that we've established a connection, create a peer, and it to the + // set of currently active peers. Configure the peer with a expiry grace + // delta greater than the broadcast delta, to prevent links from + // accepting htlcs that may trigger channel arbitrator force close the + // channel immediately. p, err := newPeer( conn, connReq, s, peerAddr, inbound, localFeatures, cfg.ChanEnableTimeout, + defaultBroadcastDelta+extraExpiryGraceDelta, ) if err != nil { srvrLog.Errorf("unable to create peer %v", err)