diff --git a/breacharbiter_test.go b/breacharbiter_test.go index eebc9875..7502415a 100644 --- a/breacharbiter_test.go +++ b/breacharbiter_test.go @@ -1268,8 +1268,8 @@ func createInitChannels(revocationWindow int) (*lnwallet.LightningChannel, *lnwa ChannelConstraints: channeldb.ChannelConstraints{ DustLimit: aliceDustLimit, MaxPendingAmount: lnwire.MilliSatoshi(rand.Int63()), - ChanReserve: btcutil.Amount(rand.Int63()), - MinHTLC: lnwire.MilliSatoshi(rand.Int63()), + ChanReserve: 0, + MinHTLC: 0, MaxAcceptedHtlcs: uint16(rand.Int31()), }, CsvDelay: uint16(csvTimeoutAlice), @@ -1283,8 +1283,8 @@ func createInitChannels(revocationWindow int) (*lnwallet.LightningChannel, *lnwa ChannelConstraints: channeldb.ChannelConstraints{ DustLimit: bobDustLimit, MaxPendingAmount: lnwire.MilliSatoshi(rand.Int63()), - ChanReserve: btcutil.Amount(rand.Int63()), - MinHTLC: lnwire.MilliSatoshi(rand.Int63()), + ChanReserve: 0, + MinHTLC: 0, MaxAcceptedHtlcs: uint16(rand.Int31()), }, CsvDelay: uint16(csvTimeoutBob), diff --git a/channeldb/channel.go b/channeldb/channel.go index ed7376e8..151feabb 100644 --- a/channeldb/channel.go +++ b/channeldb/channel.go @@ -109,7 +109,7 @@ const ( ) // ChannelConstraints represents a set of constraints meant to allow a node to -// limit their exposure, enact flow control and ensure that all HTLC's are +// limit their exposure, enact flow control and ensure that all HTLCs are // economically relevant This struct will be mirrored for both sides of the // channel, as each side will enforce various constraints that MUST be adhered // to for the life time of the channel. The parameters for each of these @@ -121,27 +121,28 @@ type ChannelConstraints struct { // as an actual output, but is instead burned to miner's fees. DustLimit btcutil.Amount - // MaxPendingAmount is the maximum pending HTLC value that can be - // present within the channel at a particular time. This value is set - // by the initiator of the channel and must be upheld at all times. - MaxPendingAmount lnwire.MilliSatoshi - - // ChanReserve is an absolute reservation on the channel for this - // particular node. This means that the current settled balance for - // this node CANNOT dip below the reservation amount. This acts as a - // defense against costless attacks when either side no longer has any - // skin in the game. + // ChanReserve is an absolute reservation on the channel for the + // owner of this set of constraints. This means that the current + // settled balance for this node CANNOT dip below the reservation + // amount. This acts as a defense against costless attacks when + // either side no longer has any skin in the game. ChanReserve btcutil.Amount - // MinHTLC is the minimum HTLC accepted for a direction of the channel. - // If any HTLC's below this amount are offered, then the HTLC will be - // rejected. This, in tandem with the dust limit allows a node to - // regulate the smallest HTLC that it deems economically relevant. + // MaxPendingAmount is the maximum pending HTLC value that the + // owner of these constraints can offer the remote node at a + // particular time. + MaxPendingAmount lnwire.MilliSatoshi + + // MinHTLC is the minimum HTLC value that the the owner of these + // constraints can offer the remote node. If any HTLCs below this + // amount are offered, then the HTLC will be rejected. This, in + // tandem with the dust limit allows a node to regulate the + // smallest HTLC that it deems economically relevant. MinHTLC lnwire.MilliSatoshi - // MaxAcceptedHtlcs is the maximum amount of HTLC's that are to be - // accepted by the owner of this set of constraints. This allows each - // node to limit their over all exposure to HTLC's that may need to be + // MaxAcceptedHtlcs is the maximum number of HTLCs that the owner of + // this set of constraints can offer the remote node. This allows each + // node to limit their over all exposure to HTLCs that may need to be // acted upon in the case of a unilateral channel closure or a contract // breach. MaxAcceptedHtlcs uint16 diff --git a/fundingmanager.go b/fundingmanager.go index 434e7711..50e47714 100644 --- a/fundingmanager.go +++ b/fundingmanager.go @@ -236,6 +236,22 @@ type fundingConfig struct { // contract breach. RequiredRemoteDelay func(btcutil.Amount) uint16 + // RequiredRemoteChanReserve is a function closure that, given the + // channel capacity, will return an appropriate amount for the remote + // peer's required channel reserve that is to be adhered to at all + // times. + RequiredRemoteChanReserve func(btcutil.Amount) btcutil.Amount + + // RequiredRemoteMaxValue is a function closure that, given the + // channel capacity, returns the amount of MilliSatoshis that our + // remote peer can have in total outstanding HTLCs with us. + RequiredRemoteMaxValue func(btcutil.Amount) lnwire.MilliSatoshi + + // RequiredRemoteMaxHTLCs is a function closure that, given the + // channel capacity, returns the number of maximum HTLCs the remote + // peer can offer us. + RequiredRemoteMaxHTLCs func(btcutil.Amount) uint16 + // WatchNewChannel is to be called once a new channel enters the final // funding stage: waiting for on-chain confirmation. This method sends // the channel to the ChainArbitrator so it can watch for any on-chain @@ -868,7 +884,7 @@ func (f *fundingManager) handleFundingOpen(fmsg *fundingOpenMsg) { // party is attempting to dictate for our commitment transaction. err = reservation.CommitConstraints( uint16(msg.CsvDelay), msg.MaxAcceptedHTLCs, - msg.MaxValueInFlight, msg.ChannelReserve, + msg.MaxValueInFlight, msg.HtlcMinimum, msg.ChannelReserve, ) if err != nil { f.failFundingFlow( @@ -904,7 +920,9 @@ func (f *fundingManager) handleFundingOpen(fmsg *fundingOpenMsg) { remoteCsvDelay := f.cfg.RequiredRemoteDelay(amt) // We'll also generate our required constraints for the remote party, - chanReserve, maxValue, maxHtlcs := reservation.RemoteChanConstraints() + chanReserve := f.cfg.RequiredRemoteChanReserve(amt) + maxValue := f.cfg.RequiredRemoteMaxValue(amt) + maxHtlcs := f.cfg.RequiredRemoteMaxHTLCs(amt) // With our parameters set, we'll now process their contribution so we // can move the funding workflow ahead. @@ -1004,7 +1022,7 @@ func (f *fundingManager) handleFundingAccept(fmsg *fundingAcceptMsg) { resCtx.reservation.SetNumConfsRequired(uint16(msg.MinAcceptDepth)) err = resCtx.reservation.CommitConstraints( uint16(msg.CsvDelay), msg.MaxAcceptedHTLCs, - msg.MaxValueInFlight, msg.ChannelReserve, + msg.MaxValueInFlight, msg.HtlcMinimum, msg.ChannelReserve, ) if err != nil { f.failFundingFlow( @@ -1018,7 +1036,9 @@ func (f *fundingManager) handleFundingAccept(fmsg *fundingAcceptMsg) { // As they've accepted our channel constraints, we'll regenerate them // here so we can properly commit their accepted constraints to the // reservation. - chanReserve, maxValue, maxHtlcs := resCtx.reservation.RemoteChanConstraints() + chanReserve := f.cfg.RequiredRemoteChanReserve(resCtx.chanAmt) + maxValue := f.cfg.RequiredRemoteMaxValue(resCtx.chanAmt) + maxHtlcs := f.cfg.RequiredRemoteMaxHTLCs(resCtx.chanAmt) // The remote node has responded with their portion of the channel // contribution. At this point, we can process their contribution which @@ -2389,7 +2409,9 @@ func (f *fundingManager) handleInitFundingMsg(msg *initFundingMsg) { // Finally, we'll use the current value of the channels and our default // policy to determine of required commitment constraints for the // remote party. - chanReserve, maxValue, maxHtlcs := reservation.RemoteChanConstraints() + chanReserve := f.cfg.RequiredRemoteChanReserve(capacity) + maxValue := f.cfg.RequiredRemoteMaxValue(capacity) + maxHtlcs := f.cfg.RequiredRemoteMaxHTLCs(capacity) fndgLog.Infof("Starting funding workflow with %v for pendingID(%x)", msg.peerAddress.Address, chanID) diff --git a/fundingmanager_test.go b/fundingmanager_test.go index e00dd72f..972e0205 100644 --- a/fundingmanager_test.go +++ b/fundingmanager_test.go @@ -154,13 +154,14 @@ func createTestWallet(cdb *channeldb.DB, netParams *chaincfg.Params, estimator lnwallet.FeeEstimator) (*lnwallet.LightningWallet, error) { wallet, err := lnwallet.NewLightningWallet(lnwallet.Config{ - Database: cdb, - Notifier: notifier, - WalletController: wc, - Signer: signer, - ChainIO: bio, - FeeEstimator: estimator, - NetParams: *netParams, + Database: cdb, + Notifier: notifier, + WalletController: wc, + Signer: signer, + ChainIO: bio, + FeeEstimator: estimator, + NetParams: *netParams, + DefaultConstraints: defaultChannelConstraints, }) if err != nil { return nil, err @@ -277,6 +278,16 @@ func createTestFundingManager(t *testing.T, privKey *btcec.PrivateKey, RequiredRemoteDelay: func(amt btcutil.Amount) uint16 { return 4 }, + RequiredRemoteChanReserve: func(chanAmt btcutil.Amount) btcutil.Amount { + return chanAmt / 100 + }, + RequiredRemoteMaxValue: func(chanAmt btcutil.Amount) lnwire.MilliSatoshi { + reserve := lnwire.NewMSatFromSatoshis(chanAmt / 100) + return lnwire.NewMSatFromSatoshis(chanAmt) - reserve + }, + RequiredRemoteMaxHTLCs: func(chanAmt btcutil.Amount) uint16 { + return uint16(lnwallet.MaxHTLCNumber / 2) + }, ArbiterChan: arbiterChan, WatchNewChannel: func(*channeldb.OpenChannel) error { return nil diff --git a/htlcswitch/link.go b/htlcswitch/link.go index 4f4e0b52..e78eab0f 100644 --- a/htlcswitch/link.go +++ b/htlcswitch/link.go @@ -1261,11 +1261,21 @@ type getBandwidthCmd struct { // // NOTE: Part of the ChannelLink interface. func (l *channelLink) Bandwidth() lnwire.MilliSatoshi { - // TODO(roasbeef): subtract reserve channelBandwidth := l.channel.AvailableBalance() overflowBandwidth := l.overflowQueue.TotalHtlcAmount() + linkBandwidth := channelBandwidth - overflowBandwidth + reserve := lnwire.NewMSatFromSatoshis(l.channel.LocalChanReserve()) - return channelBandwidth - overflowBandwidth + // If the channel reserve is greater than the total available + // balance of the link, just return 0. + if linkBandwidth < reserve { + return 0 + } + + // Else the amount that is available to flow through the link at + // this point is the available balance minus the reserve amount + // we are required to keep as collateral. + return linkBandwidth - reserve } // policyUpdate is a message sent to a channel link when an outside sub-system diff --git a/htlcswitch/link_test.go b/htlcswitch/link_test.go index 17ee42d4..ce1491c1 100644 --- a/htlcswitch/link_test.go +++ b/htlcswitch/link_test.go @@ -1405,8 +1405,8 @@ func (m *mockPeer) Disconnect(reason error) { var _ Peer = (*mockPeer)(nil) -func newSingleLinkTestHarness(chanAmt btcutil.Amount) (ChannelLink, - *lnwallet.LightningChannel, chan time.Time, func(), error) { +func newSingleLinkTestHarness(chanAmt, chanReserve btcutil.Amount) ( + ChannelLink, *lnwallet.LightningChannel, chan time.Time, func(), error) { globalEpoch := &chainntnfs.BlockEpochEvent{ Epochs: make(chan *chainntnfs.BlockEpoch), Cancel: func() { @@ -1415,7 +1415,8 @@ func newSingleLinkTestHarness(chanAmt btcutil.Amount) (ChannelLink, chanID := lnwire.NewShortChanIDFromInt(4) aliceChannel, bobChannel, fCleanUp, _, err := createTestChannel( - alicePrivKey, bobPrivKey, chanAmt, chanAmt, chanID, + alicePrivKey, bobPrivKey, chanAmt, chanAmt, + chanReserve, chanReserve, chanID, ) if err != nil { return nil, nil, nil, nil, err @@ -1659,7 +1660,7 @@ func TestChannelLinkBandwidthConsistency(t *testing.T) { // We'll start the test by creating a single instance of const chanAmt = btcutil.SatoshiPerBitcoin * 5 - link, bobChannel, tmr, cleanUp, err := newSingleLinkTestHarness(chanAmt) + link, bobChannel, tmr, cleanUp, err := newSingleLinkTestHarness(chanAmt, 0) if err != nil { t.Fatalf("unable to create link: %v", err) } @@ -1992,7 +1993,8 @@ func TestChannelLinkBandwidthConsistencyOverflow(t *testing.T) { var mockBlob [lnwire.OnionPacketSize]byte const chanAmt = btcutil.SatoshiPerBitcoin * 5 - aliceLink, bobChannel, batchTick, cleanUp, err := newSingleLinkTestHarness(chanAmt) + aliceLink, bobChannel, batchTick, cleanUp, err := + newSingleLinkTestHarness(chanAmt, 0) if err != nil { t.Fatalf("unable to create link: %v", err) } @@ -2191,6 +2193,134 @@ func TestChannelLinkBandwidthConsistencyOverflow(t *testing.T) { } } +// TestChannelLinkBandwidthChanReserve checks that the bandwidth available +// on the channel link reflects the channel reserve that must be kept +// at all times. +func TestChannelLinkBandwidthChanReserve(t *testing.T) { + t.Parallel() + + // First start a link that has a balance greater than it's + // channel reserve. + const chanAmt = btcutil.SatoshiPerBitcoin * 5 + const chanReserve = btcutil.SatoshiPerBitcoin * 1 + aliceLink, bobChannel, batchTimer, cleanUp, err := + newSingleLinkTestHarness(chanAmt, chanReserve) + if err != nil { + t.Fatalf("unable to create link: %v", err) + } + defer cleanUp() + + var ( + mockBlob [lnwire.OnionPacketSize]byte + coreLink = aliceLink.(*channelLink) + coreChan = coreLink.channel + defaultCommitFee = coreChan.StateSnapshot().CommitFee + aliceStartingBandwidth = aliceLink.Bandwidth() + aliceMsgs = coreLink.cfg.Peer.(*mockPeer).sentMsgs + ) + + estimator := &lnwallet.StaticFeeEstimator{ + FeeRate: 24, + } + feePerWeight, err := estimator.EstimateFeePerWeight(1) + if err != nil { + t.Fatalf("unable to query fee estimator: %v", err) + } + feePerKw := feePerWeight * 1000 + htlcFee := lnwire.NewMSatFromSatoshis( + btcutil.Amount((int64(feePerKw) * lnwallet.HtlcWeight) / 1000), + ) + + // The starting bandwidth of the channel should be exactly the amount + // that we created the channel between her and Bob, minus the channel + // reserve. + expectedBandwidth := lnwire.NewMSatFromSatoshis( + chanAmt - defaultCommitFee - chanReserve) + assertLinkBandwidth(t, aliceLink, expectedBandwidth) + + // Next, we'll create an HTLC worth 3 BTC, and send it into the link as + // a switch initiated payment. The resulting bandwidth should + // now be decremented to reflect the new HTLC. + htlcAmt := lnwire.NewMSatFromSatoshis(3 * btcutil.SatoshiPerBitcoin) + invoice, htlc, err := generatePayment(htlcAmt, htlcAmt, 5, mockBlob) + if err != nil { + t.Fatalf("unable to create payment: %v", err) + } + addPkt := htlcPacket{ + htlc: htlc, + } + aliceLink.HandleSwitchPacket(&addPkt) + time.Sleep(time.Millisecond * 100) + assertLinkBandwidth(t, aliceLink, aliceStartingBandwidth-htlcAmt-htlcFee) + + // Alice should send the HTLC to Bob. + var msg lnwire.Message + select { + case msg = <-aliceMsgs: + case <-time.After(2 * time.Second): + t.Fatalf("did not receive message") + } + + addHtlc, ok := msg.(*lnwire.UpdateAddHTLC) + if !ok { + t.Fatalf("expected UpdateAddHTLC, got %T", msg) + } + + bobIndex, err := bobChannel.ReceiveHTLC(addHtlc) + if err != nil { + t.Fatalf("bob failed receiving htlc: %v", err) + } + + // Lock in the HTLC. + if err := updateState(batchTimer, coreLink, bobChannel, true); err != nil { + t.Fatalf("unable to update state: %v", err) + } + + assertLinkBandwidth(t, aliceLink, aliceStartingBandwidth-htlcAmt-htlcFee) + + // If we now send in a valid HTLC settle for the prior HTLC we added, + // then the bandwidth should remain unchanged as the remote party will + // gain additional channel balance. + err = bobChannel.SettleHTLC(invoice.Terms.PaymentPreimage, bobIndex) + if err != nil { + t.Fatalf("unable to settle htlc: %v", err) + } + htlcSettle := &lnwire.UpdateFulfillHTLC{ + ID: bobIndex, + PaymentPreimage: invoice.Terms.PaymentPreimage, + } + aliceLink.HandleChannelUpdate(htlcSettle) + time.Sleep(time.Millisecond * 500) + + // Since the settle is not locked in yet, Alice's bandwidth should still + // reflect that she has to pay the fee. + assertLinkBandwidth(t, aliceLink, aliceStartingBandwidth-htlcAmt-htlcFee) + + // Lock in the settle. + if err := updateState(batchTimer, coreLink, bobChannel, false); err != nil { + t.Fatalf("unable to update state: %v", err) + } + + time.Sleep(time.Millisecond * 100) + assertLinkBandwidth(t, aliceLink, aliceStartingBandwidth-htlcAmt) + + // Now we create a channel that has a channel reserve that is + // greater than it's balance. In these case only payments can + // be received on this channel, not sent. The available bandwidth + // should therefore be 0. + const bobChanAmt = btcutil.SatoshiPerBitcoin * 1 + const bobChanReserve = btcutil.SatoshiPerBitcoin * 1.5 + bobLink, _, _, bobCleanUp, err := newSingleLinkTestHarness(bobChanAmt, + bobChanReserve) + if err != nil { + t.Fatalf("unable to create link: %v", err) + } + defer bobCleanUp() + + // Make sure bandwidth is reported as 0. + assertLinkBandwidth(t, bobLink, 0) +} + // TestChannelRetransmission tests the ability of the channel links to // synchronize theirs states after abrupt disconnect. func TestChannelRetransmission(t *testing.T) { diff --git a/htlcswitch/test_utils.go b/htlcswitch/test_utils.go index 7595a6d7..bc3b0191 100644 --- a/htlcswitch/test_utils.go +++ b/htlcswitch/test_utils.go @@ -88,7 +88,7 @@ func generateRandomBytes(n int) ([]byte, error) { // // TODO(roasbeef): need to factor out, similar func re-used in many parts of codebase func createTestChannel(alicePrivKey, bobPrivKey []byte, - aliceAmount, bobAmount btcutil.Amount, + aliceAmount, bobAmount, aliceReserve, bobReserve btcutil.Amount, chanID lnwire.ShortChannelID) (*lnwallet.LightningChannel, *lnwallet.LightningChannel, func(), func() (*lnwallet.LightningChannel, *lnwallet.LightningChannel, error), error) { @@ -97,11 +97,27 @@ func createTestChannel(alicePrivKey, bobPrivKey []byte, bobKeyPriv, bobKeyPub := btcec.PrivKeyFromBytes(btcec.S256(), bobPrivKey) channelCapacity := aliceAmount + bobAmount - aliceDustLimit := btcutil.Amount(200) - bobDustLimit := btcutil.Amount(800) csvTimeoutAlice := uint32(5) csvTimeoutBob := uint32(4) + aliceConstraints := &channeldb.ChannelConstraints{ + DustLimit: btcutil.Amount(200), + MaxPendingAmount: lnwire.NewMSatFromSatoshis( + channelCapacity), + ChanReserve: aliceReserve, + MinHTLC: 0, + MaxAcceptedHtlcs: lnwallet.MaxHTLCNumber / 2, + } + + bobConstraints := &channeldb.ChannelConstraints{ + DustLimit: btcutil.Amount(800), + MaxPendingAmount: lnwire.NewMSatFromSatoshis( + channelCapacity), + ChanReserve: bobReserve, + MinHTLC: 0, + MaxAcceptedHtlcs: lnwallet.MaxHTLCNumber / 2, + } + var hash [sha256.Size]byte randomSeed, err := generateRandomBytes(sha256.Size) if err != nil { @@ -116,9 +132,7 @@ func createTestChannel(alicePrivKey, bobPrivKey []byte, fundingTxIn := wire.NewTxIn(prevOut, nil, nil) aliceCfg := channeldb.ChannelConfig{ - ChannelConstraints: channeldb.ChannelConstraints{ - DustLimit: aliceDustLimit, - }, + ChannelConstraints: *aliceConstraints, CsvDelay: uint16(csvTimeoutAlice), MultiSigKey: aliceKeyPub, RevocationBasePoint: aliceKeyPub, @@ -127,9 +141,7 @@ func createTestChannel(alicePrivKey, bobPrivKey []byte, HtlcBasePoint: aliceKeyPub, } bobCfg := channeldb.ChannelConfig{ - ChannelConstraints: channeldb.ChannelConstraints{ - DustLimit: bobDustLimit, - }, + ChannelConstraints: *bobConstraints, CsvDelay: uint16(csvTimeoutBob), MultiSigKey: bobKeyPub, RevocationBasePoint: bobKeyPub, @@ -656,15 +668,17 @@ func createClusterChannels(aliceToBob, bobToCarol btcutil.Amount) ( secondChanID := lnwire.NewShortChanIDFromInt(5) // Create lightning channels between Alice<->Bob and Bob<->Carol - aliceChannel, firstBobChannel, cleanAliceBob, restoreAliceBob, err := createTestChannel( - alicePrivKey, bobPrivKey, aliceToBob, aliceToBob, firstChanID) + aliceChannel, firstBobChannel, cleanAliceBob, restoreAliceBob, err := + createTestChannel(alicePrivKey, bobPrivKey, aliceToBob, + aliceToBob, 0, 0, firstChanID) if err != nil { return nil, nil, nil, errors.Errorf("unable to create "+ "alice<->bob channel: %v", err) } - secondBobChannel, carolChannel, cleanBobCarol, restoreBobCarol, err := createTestChannel( - bobPrivKey, carolPrivKey, bobToCarol, bobToCarol, secondChanID) + secondBobChannel, carolChannel, cleanBobCarol, restoreBobCarol, err := + createTestChannel(bobPrivKey, carolPrivKey, bobToCarol, + bobToCarol, 0, 0, secondChanID) if err != nil { cleanAliceBob() return nil, nil, nil, errors.Errorf("unable to create "+ diff --git a/lnd.go b/lnd.go index 2d27b6b4..09532fb6 100644 --- a/lnd.go +++ b/lnd.go @@ -388,6 +388,24 @@ func lndMain() error { cid := lnwire.NewChanIDFromOutPoint(&chanPoint) return server.htlcSwitch.UpdateShortChanID(cid, sid) }, + RequiredRemoteChanReserve: func(chanAmt btcutil.Amount) btcutil.Amount { + // By default, we'll require the remote peer to maintain + // at least 1% of the total channel capacity at all + // times. + return chanAmt / 100 + }, + RequiredRemoteMaxValue: func(chanAmt btcutil.Amount) lnwire.MilliSatoshi { + // By default, we'll allow the remote peer to fully + // utilize the full bandwidth of the channel, minus our + // required reserve. + reserve := lnwire.NewMSatFromSatoshis(chanAmt / 100) + return lnwire.NewMSatFromSatoshis(chanAmt) - reserve + }, + RequiredRemoteMaxHTLCs: func(chanAmt btcutil.Amount) uint16 { + // By default, we'll permit them to utilize the full + // channel bandwidth. + return uint16(lnwallet.MaxHTLCNumber / 2) + }, }) if err != nil { return err diff --git a/lnd_test.go b/lnd_test.go index fcaa3231..15bb0294 100644 --- a/lnd_test.go +++ b/lnd_test.go @@ -4103,7 +4103,8 @@ out: // Alice's side, leaving on 10k satoshis of available balance for bob. // There's a max payment amount, so we'll have to do this // incrementally. - amtToSend := int64(chanAmt) - 20000 + chanReserve := int64(chanAmt / 100) + amtToSend := int64(chanAmt) - chanReserve - 20000 amtSent := int64(0) for amtSent != amtToSend { // We'll send in chunks of the max payment amount. If we're @@ -4642,8 +4643,10 @@ func testAsyncPayments(net *lntest.NetworkHarness, t *harnessTest) { t.Fatalf("unable to get alice channel info: %v", err) } - // Calculate the number of invoices. - numInvoices := int(info.LocalBalance / paymentAmt) + // Calculate the number of invoices. We will deplete the channel + // all the way down to the channel reserve. + chanReserve := info.LocalBalance / 100 + numInvoices := int((info.LocalBalance - chanReserve) / paymentAmt) bobAmt := int64(numInvoices * paymentAmt) aliceAmt := info.LocalBalance - bobAmt diff --git a/lnwallet/channel.go b/lnwallet/channel.go index 36859952..1f45805c 100644 --- a/lnwallet/channel.go +++ b/lnwallet/channel.go @@ -47,9 +47,22 @@ var ( ErrMaxHTLCNumber = fmt.Errorf("commitment transaction exceed max " + "htlc number") - // ErrInsufficientBalance is returned when a proposed HTLC would - // exceed the available balance. - ErrInsufficientBalance = fmt.Errorf("insufficient local balance") + // ErrMaxPendingAmount is returned when a proposed HTLC would exceed + // the overall maximum pending value of all HTLCs if committed in a + // state transition. + ErrMaxPendingAmount = fmt.Errorf("commitment transaction exceed max" + + "overall pending htlc value") + + // ErrBelowChanReserve is returned when a proposed HTLC would cause + // one of the peer's funds to dip below the channel reserve limit. + ErrBelowChanReserve = fmt.Errorf("commitment transaction dips peer " + + "below chan reserve") + + // ErrBelowMinHTLC is returned when a proposed HTLC has a value that + // is below the minimum HTLC value constraint for either us or our + // peer depending on which flags are set. + ErrBelowMinHTLC = fmt.Errorf("proposed HTLC value is below minimum " + + "allowed HTLC value") // ErrCannotSyncCommitChains is returned if, upon receiving a ChanSync // message, the state machine deems that is unable to properly @@ -324,6 +337,8 @@ type commitment struct { // within the commitment chain. This balance is computed by properly // evaluating all the add/remove/settle log entries before the listed // indexes. + // + // NOTE: This is the balance *before* subtracting any commitment fee. ourBalance lnwire.MilliSatoshi theirBalance lnwire.MilliSatoshi @@ -1928,7 +1943,7 @@ func htlcSuccessFee(feePerKw btcutil.Amount) btcutil.Amount { // htlcIsDust determines if an HTLC output is dust or not depending on two // bits: if the HTLC is incoming and if the HTLC will be placed on our // commitment transaction, or theirs. These two pieces of information are -// require as we currently used second-level HTLC transactions ass off-chain +// require as we currently used second-level HTLC transactions as off-chain // covenants. Depending on the two bits, we'll either be using a timeout or // success transaction which have different weights. func htlcIsDust(incoming, ourCommit bool, @@ -2023,62 +2038,14 @@ func (lc *LightningChannel) fetchCommitmentView(remoteChain bool, commitChain = lc.remoteCommitChain } - ourBalance := commitChain.tip().ourBalance - theirBalance := commitChain.tip().theirBalance - - // Add the fee from the previous commitment state back to the - // initiator's balance, so that the fee can be recalculated and - // re-applied in case fee estimation parameters have changed or the - // number of outstanding HTLCs has changed. - if lc.channelState.IsInitiator { - ourBalance += lnwire.NewMSatFromSatoshis(commitChain.tip().fee) - } else if !lc.channelState.IsInitiator { - theirBalance += lnwire.NewMSatFromSatoshis(commitChain.tip().fee) - } - nextHeight := commitChain.tip().height + 1 // Run through all the HTLCs that will be covered by this transaction // in order to update their commitment addition height, and to adjust // the balances on the commitment transaction accordingly. htlcView := lc.fetchHTLCView(theirLogIndex, ourLogIndex) - filteredHTLCView := lc.evaluateHTLCView(htlcView, &ourBalance, - &theirBalance, nextHeight, remoteChain) - - // Initiate feePerKw to the last committed fee for this chain as we'll - // need this to determine which HTLC's are dust, and also the final fee - // rate. - feePerKw := commitChain.tail().feePerKw - - // Check if any fee updates have taken place since that last - // commitment. - if lc.channelState.IsInitiator { - switch { - // We've sent an update_fee message since our last commitment, - // and now are now creating a commitment that reflects the new - // fee update. - case remoteChain && lc.pendingFeeUpdate != nil: - feePerKw = *lc.pendingFeeUpdate - - // We've created a new commitment for the remote chain that - // includes a fee update, and have not received a commitment - // after the fee update has been ACKed. - case !remoteChain && lc.pendingAckFeeUpdate != nil: - feePerKw = *lc.pendingAckFeeUpdate - } - } else { - switch { - // We've received a fee update since the last local commitment, - // so we'll include the fee update in the current view. - case !remoteChain && lc.pendingFeeUpdate != nil: - feePerKw = *lc.pendingFeeUpdate - - // Earlier we received a commitment that signed an earlier fee - // update, and now we must ACK that update. - case remoteChain && lc.pendingAckFeeUpdate != nil: - feePerKw = *lc.pendingAckFeeUpdate - } - } + ourBalance, theirBalance, _, filteredHTLCView, feePerKw := + lc.computeView(htlcView, remoteChain, true) // Determine how many current HTLCs are over the dust limit, and should // be counted for the purpose of fee calculation. @@ -2253,8 +2220,16 @@ func (lc *LightningChannel) createCommitmentTx(c *commitment, // settles, and timeouts found in both logs. The resulting view returned // reflects the current state of HTLCs within the remote or local commitment // chain. +// +// If mutateState is set to true, then the add height of all added HTLCs +// will be set to nextHeight, and the remove height of all removed HTLCs +// will be set to nextHeight. This should therefore only be set to true +// once for each height, and only in concert with signing a new commitment. +// TODO(halseth): return htlcs to mutate instead of mutating inside +// method. func (lc *LightningChannel) evaluateHTLCView(view *htlcView, ourBalance, - theirBalance *lnwire.MilliSatoshi, nextHeight uint64, remoteChain bool) *htlcView { + theirBalance *lnwire.MilliSatoshi, nextHeight uint64, + remoteChain, mutateState bool) *htlcView { newView := &htlcView{} @@ -2276,7 +2251,7 @@ func (lc *LightningChannel) evaluateHTLCView(view *htlcView, ourBalance, // If we're settling an inbound HTLC, and it hasn't been // processed yet, then increment our state tracking the total // number of satoshis we've received within the channel. - if entry.EntryType == Settle && !remoteChain && + if mutateState && entry.EntryType == Settle && !remoteChain && entry.removeCommitHeightLocal == 0 { lc.channelState.TotalMSatReceived += entry.Amount } @@ -2285,7 +2260,7 @@ func (lc *LightningChannel) evaluateHTLCView(view *htlcView, ourBalance, skipThem[addEntry.HtlcIndex] = struct{}{} processRemoveEntry(entry, ourBalance, theirBalance, - nextHeight, remoteChain, true) + nextHeight, remoteChain, true, mutateState) } for _, entry := range view.theirUpdates { if entry.EntryType == Add { @@ -2296,7 +2271,7 @@ func (lc *LightningChannel) evaluateHTLCView(view *htlcView, ourBalance, // and it hasn't been processed, yet, the increment our state // tracking the total number of satoshis we've sent within the // channel. - if entry.EntryType == Settle && !remoteChain && + if mutateState && entry.EntryType == Settle && !remoteChain && entry.removeCommitHeightLocal == 0 { lc.channelState.TotalMSatSent += entry.Amount } @@ -2305,7 +2280,7 @@ func (lc *LightningChannel) evaluateHTLCView(view *htlcView, ourBalance, skipUs[addEntry.HtlcIndex] = struct{}{} processRemoveEntry(entry, ourBalance, theirBalance, - nextHeight, remoteChain, false) + nextHeight, remoteChain, false, mutateState) } // Next we take a second pass through all the log entries, skipping any @@ -2318,7 +2293,7 @@ func (lc *LightningChannel) evaluateHTLCView(view *htlcView, ourBalance, } processAddEntry(entry, ourBalance, theirBalance, nextHeight, - remoteChain, false) + remoteChain, false, mutateState) newView.ourUpdates = append(newView.ourUpdates, entry) } for _, entry := range view.theirUpdates { @@ -2328,7 +2303,7 @@ func (lc *LightningChannel) evaluateHTLCView(view *htlcView, ourBalance, } processAddEntry(entry, ourBalance, theirBalance, nextHeight, - remoteChain, true) + remoteChain, true, mutateState) newView.theirUpdates = append(newView.theirUpdates, entry) } @@ -2340,7 +2315,7 @@ func (lc *LightningChannel) evaluateHTLCView(view *htlcView, ourBalance, // was committed is updated. Keeping track of this inclusion height allows us to // later compact the log once the change is fully committed in both chains. func processAddEntry(htlc *PaymentDescriptor, ourBalance, theirBalance *lnwire.MilliSatoshi, - nextHeight uint64, remoteChain bool, isIncoming bool) { + nextHeight uint64, remoteChain bool, isIncoming, mutateState bool) { // If we're evaluating this entry for the remote chain (to create/view // a new commitment), then we'll may be updating the height this entry @@ -2368,7 +2343,9 @@ func processAddEntry(htlc *PaymentDescriptor, ourBalance, theirBalance *lnwire.M *ourBalance -= htlc.Amount } - *addHeight = nextHeight + if mutateState { + *addHeight = nextHeight + } } // processRemoveEntry processes a log entry which settles or times out a @@ -2376,7 +2353,7 @@ func processAddEntry(htlc *PaymentDescriptor, ourBalance, theirBalance *lnwire.M // is skipped. func processRemoveEntry(htlc *PaymentDescriptor, ourBalance, theirBalance *lnwire.MilliSatoshi, nextHeight uint64, - remoteChain bool, isIncoming bool) { + remoteChain bool, isIncoming, mutateState bool) { var removeHeight *uint64 if remoteChain { @@ -2416,7 +2393,9 @@ func processRemoveEntry(htlc *PaymentDescriptor, ourBalance, *ourBalance += htlc.Amount } - *removeHeight = nextHeight + if mutateState { + *removeHeight = nextHeight + } } // generateRemoteHtlcSigJobs generates a series of HTLC signature jobs for the @@ -2701,7 +2680,7 @@ func (lc *LightningChannel) SignNextCommitment() (lnwire.Sig, []lnwire.Sig, erro // party set up when we initially set up the channel. If we are, then // we'll abort this state transition. err := lc.validateCommitmentSanity(remoteACKedIndex, - lc.localUpdateLog.logIndex, false, true, true) + lc.localUpdateLog.logIndex, true, nil) if err != nil { return sig, htlcSigs, err } @@ -3060,69 +3039,223 @@ func (lc *LightningChannel) ChanSyncMsg() (*lnwire.ChannelReestablish, error) { }, nil } -// validateCommitmentSanity is used to validate that on current state the commitment -// transaction is valid in terms of propagating it over Bitcoin network, and -// also that all outputs are meet Bitcoin spec requirements and they are -// spendable. -func (lc *LightningChannel) validateCommitmentSanity(theirLogCounter, - ourLogCounter uint64, prediction bool, local bool, remote bool) error { +// computeView takes the given htlcView, and calculates the balances, +// filtered view (settling unsettled HTLCs), commitment weight and +// feePerKw, after applying the HTLCs to the latest commitment. The +// returned balanced are the balances *before* subtracting the +// commitment fee from the initiator's balance. +// +// If the updateState boolean is set true, the add and remove heights +// of the HTLCs will be set to the next commitment height. +func (lc *LightningChannel) computeView(view *htlcView, remoteChain bool, + updateState bool) (lnwire.MilliSatoshi, lnwire.MilliSatoshi, int64, + *htlcView, btcutil.Amount) { - // TODO(roasbeef): verify remaining sanity requirements - htlcCount := 0 - - // If we adding or receiving the htlc we increase the number of htlcs - // by one in order to not overflow the commitment transaction by - // insertion. - if prediction { - htlcCount++ + commitChain := lc.localCommitChain + dustLimit := lc.localChanCfg.DustLimit + if remoteChain { + commitChain = lc.remoteCommitChain + dustLimit = lc.remoteChanCfg.DustLimit } - // TODO(roasbeef): call availableBalance in here re-using htlcView + // Since the fetched htlc view will include all updates added + // after the last committed state, we start with the balances + // reflecting that state. + ourBalance := commitChain.tip().ourBalance + theirBalance := commitChain.tip().theirBalance - // Run through all the HTLCs that will be covered by this transaction - // in order to calculate theirs count. + // Add the fee from the previous commitment state back to the + // initiator's balance, so that the fee can be recalculated and + // re-applied in case fee estimation parameters have changed or + // the number of outstanding HTLCs has changed. + if lc.channelState.IsInitiator { + ourBalance += lnwire.NewMSatFromSatoshis( + commitChain.tip().fee) + } else if !lc.channelState.IsInitiator { + theirBalance += lnwire.NewMSatFromSatoshis( + commitChain.tip().fee) + } + nextHeight := commitChain.tip().height + 1 + + // We evaluate the view at this stage, meaning settled and + // failed HTLCs will remove their corresponding added HTLCs. + // The resulting filtered view will only have Add entries left, + // making it easy to compare the channel constraints to the + // final commitment state. + filteredHTLCView := lc.evaluateHTLCView(view, &ourBalance, + &theirBalance, nextHeight, remoteChain, updateState) + + // Initiate feePerKw to the last committed fee for this chain as we'll + // need this to determine which HTLCs are dust, and also the final fee + // rate. + feePerKw := commitChain.tip().feePerKw + + // Check if any fee updates have taken place since that last + // commitment. + if lc.channelState.IsInitiator { + switch { + // We've sent an update_fee message since our last commitment, + // and now are now creating a commitment that reflects the new + // fee update. + case remoteChain && lc.pendingFeeUpdate != nil: + feePerKw = *lc.pendingFeeUpdate + + // We've created a new commitment for the remote chain that + // includes a fee update, and have not received a commitment + // after the fee update has been ACKed. + case !remoteChain && lc.pendingAckFeeUpdate != nil: + feePerKw = *lc.pendingAckFeeUpdate + } + } else { + switch { + // We've received a fee update since the last local commitment, + // so we'll include the fee update in the current view. + case !remoteChain && lc.pendingFeeUpdate != nil: + feePerKw = *lc.pendingFeeUpdate + + // Earlier we received a commitment that signed an earlier fee + // update, and now we must ACK that update. + case remoteChain && lc.pendingAckFeeUpdate != nil: + feePerKw = *lc.pendingAckFeeUpdate + } + } + + // Now go through all HTLCs at this stage, to calculate the total + // weight, needed to calculate the transaction fee. + var totalHtlcWeight int64 + for _, htlc := range filteredHTLCView.ourUpdates { + if htlcIsDust(remoteChain, !remoteChain, feePerKw, + htlc.Amount.ToSatoshis(), dustLimit) { + continue + } + + totalHtlcWeight += HtlcWeight + } + for _, htlc := range filteredHTLCView.theirUpdates { + if htlcIsDust(!remoteChain, !remoteChain, feePerKw, + htlc.Amount.ToSatoshis(), dustLimit) { + continue + } + + totalHtlcWeight += HtlcWeight + } + + totalCommitWeight := CommitWeight + totalHtlcWeight + return ourBalance, theirBalance, totalCommitWeight, filteredHTLCView, feePerKw +} + +// validateCommitmentSanity is used to validate the current state of the +// commitment transaction in terms of the ChannelConstraints that we and our +// remote peer agreed upon during the funding workflow. The predictAdded +// parameter should be set to a valid PaymentDescriptor if we are validating +// in the state when adding a new HTLC, or nil otherwise. +func (lc *LightningChannel) validateCommitmentSanity(theirLogCounter, + ourLogCounter uint64, remoteChain bool, + predictAdded *PaymentDescriptor) error { + // Fetch all updates not committed. view := lc.fetchHTLCView(theirLogCounter, ourLogCounter) - if remote { - for _, entry := range view.theirUpdates { - if entry.EntryType == Add { - htlcCount++ - } - } - for _, entry := range view.ourUpdates { - if entry.EntryType != Add { - htlcCount-- - } - } + // If we are checking if we can add a new HTLC, we add this to the + // update log, in order to validate the sanity of the commitment + // resulting from _actually adding_ this HTLC to the state. + if predictAdded != nil { + // If we are adding an HTLC, this will be an Add to the + // local update log. + view.ourUpdates = append(view.ourUpdates, predictAdded) } - if local { - for _, entry := range view.ourUpdates { - if entry.EntryType == Add { - htlcCount++ - } - } - for _, entry := range view.theirUpdates { - if entry.EntryType != Add { - htlcCount-- - } - } + commitChain := lc.localCommitChain + if remoteChain { + commitChain = lc.remoteCommitChain } + ourInitialBalance := commitChain.tip().ourBalance + theirInitialBalance := commitChain.tip().theirBalance - // If we're validating the commitment sanity for HTLC _log_ update by a - // particular side, then we'll only consider half of the available HTLC - // bandwidth. However, if we're validating the _creation_ of a new - // commitment state, then we'll use the full value as the sum of the - // contribution of both sides shouldn't exceed the max number. - var maxHTLCNumber int - if local && remote { - maxHTLCNumber = MaxHTLCNumber + ourBalance, theirBalance, commitWeight, filteredView, feePerKw := + lc.computeView(view, remoteChain, false) + + // Calculate the commitment fee, and subtract it from the + // initiator's balance. + commitFee := btcutil.Amount((int64(feePerKw) * commitWeight) / 1000) + if lc.channelState.IsInitiator { + ourBalance -= lnwire.NewMSatFromSatoshis(commitFee) } else { - maxHTLCNumber = MaxHTLCNumber / 2 + theirBalance -= lnwire.NewMSatFromSatoshis(commitFee) } - if htlcCount > maxHTLCNumber { - return ErrMaxHTLCNumber + // If the added HTLCs will decrease the balance, make sure + // they won't dip the local and remote balances below the + // channel reserves. + if ourBalance < ourInitialBalance && + ourBalance < lnwire.NewMSatFromSatoshis( + lc.localChanCfg.ChanReserve) { + return ErrBelowChanReserve + } + + if theirBalance < theirInitialBalance && + theirBalance < lnwire.NewMSatFromSatoshis( + lc.remoteChanCfg.ChanReserve) { + return ErrBelowChanReserve + } + + // validateUpdates take a set of updates, and validates them + // against the passed channel constraints. + validateUpdates := func(updates []*PaymentDescriptor, + constraints *channeldb.ChannelConfig) error { + + // We keep track of the number of HTLCs in flight for + // the commitment, and the amount in flight. + var numInFlight uint16 + var amtInFlight lnwire.MilliSatoshi + + // Go through all updates, checking that they don't + // violate the channel constraints. + for _, entry := range updates { + if entry.EntryType == Add { + // An HTLC is being added, this will + // add to the number and amount in + // flight. + amtInFlight += entry.Amount + numInFlight++ + + // Check that the value of the HTLC they + // added is above our minimum. + if entry.Amount < constraints.MinHTLC { + return ErrBelowMinHTLC + } + } + } + + // Now that we know the total value of added HTLCs, + // we check that this satisfy the MaxPendingAmont + // contraint. + if amtInFlight > constraints.MaxPendingAmount { + return ErrMaxPendingAmount + } + + // In this step, we verify that the total number of + // active HTLCs does not exceed the constraint of the + // maximum number of HTLCs in flight. + if numInFlight > constraints.MaxAcceptedHtlcs { + return ErrMaxHTLCNumber + } + return nil + } + + // First check that the remote updates won't violate it's + // channel constraints. + err := validateUpdates(filteredView.theirUpdates, + lc.remoteChanCfg) + if err != nil { + return err + } + + // Secondly check that our updates won't violate our + // channel constraints. + err = validateUpdates(filteredView.ourUpdates, + lc.localChanCfg) + if err != nil { + return err } return nil @@ -3323,7 +3456,7 @@ func (lc *LightningChannel) ReceiveNewCommitment(commitSig lnwire.Sig, // the constraints we specified during initial channel setup. If not, // then we'll abort the channel as they've violated our constraints. err := lc.validateCommitmentSanity(lc.remoteUpdateLog.logIndex, - localACKedIndex, false, true, true) + localACKedIndex, false, nil) if err != nil { return err } @@ -3342,8 +3475,9 @@ func (lc *LightningChannel) ReceiveNewCommitment(commitSig lnwire.Sig, lc.remoteChanCfg) // With the current commitment point re-calculated, construct the new - // commitment view which includes all the entries we know of in their - // HTLC log, and up to ourLogIndex in our HTLC log. + // commitment view which includes all the entries (pending or committed) + // we know of in the remote node's HTLC log, but only our local changes + // up to the last change the remote node has ACK'd. localCommitmentView, err := lc.fetchCommitmentView( false, localACKedIndex, localHtlcIndex, lc.remoteUpdateLog.logIndex, lc.remoteUpdateLog.htlcCounter, @@ -3668,43 +3802,6 @@ func (lc *LightningChannel) AddHTLC(htlc *lnwire.UpdateAddHTLC) (uint64, error) lc.Lock() defer lc.Unlock() - if err := lc.validateCommitmentSanity(lc.remoteUpdateLog.logIndex, - lc.localUpdateLog.logIndex, true, true, false); err != nil { - return 0, err - } - - // To ensure that we can actually fully accept this new HTLC, we'll - // calculate the current available bandwidth, and subtract the value of - // the HTLC from it. - initialBalance, _ := lc.availableBalance() - availableBalance := initialBalance - availableBalance -= htlc.Amount - - feePerKw := lc.channelState.LocalCommitment.FeePerKw - dustLimit := lc.channelState.LocalChanCfg.DustLimit - htlcIsDust := htlcIsDust( - false, true, feePerKw, htlc.Amount.ToSatoshis(), dustLimit, - ) - - // If this HTLC is not dust, and we're the initiator, then we'll also - // subtract the amount we'll need to pay in fees for this HTLC. - if !htlcIsDust && lc.channelState.IsInitiator { - htlcFee := lnwire.NewMSatFromSatoshis( - btcutil.Amount((int64(feePerKw) * HtlcWeight) / 1000), - ) - availableBalance -= htlcFee - } - - // If this value is negative, then we can't accept the HTLC, so we'll - // reject it with an error. - if availableBalance < 0 { - // TODO(roasbeef): also needs to respect reservation - // * expand to add context err msg - walletLog.Errorf("Unable to carry added HTLC: amt=%v, bal=%v", - htlc.Amount, availableBalance) - return 0, ErrInsufficientBalance - } - pd := &PaymentDescriptor{ EntryType: Add, RHash: PaymentHash(htlc.PaymentHash), @@ -3715,6 +3812,14 @@ func (lc *LightningChannel) AddHTLC(htlc *lnwire.UpdateAddHTLC) (uint64, error) OnionBlob: htlc.OnionBlob[:], } + // Make sure adding this HTLC won't violate any of the constrainst + // we must keep on our commitment transaction. + remoteACKedIndex := lc.localCommitChain.tail().theirMessageIndex + if err := lc.validateCommitmentSanity(remoteACKedIndex, + lc.localUpdateLog.logIndex, true, pd); err != nil { + return 0, err + } + lc.localUpdateLog.appendHtlc(pd) return pd.HtlcIndex, nil @@ -3732,11 +3837,6 @@ func (lc *LightningChannel) ReceiveHTLC(htlc *lnwire.UpdateAddHTLC) (uint64, err "ID %d", htlc.ID, lc.remoteUpdateLog.htlcCounter) } - if err := lc.validateCommitmentSanity(lc.remoteUpdateLog.logIndex, - lc.localUpdateLog.logIndex, true, false, true); err != nil { - return 0, err - } - pd := &PaymentDescriptor{ EntryType: Add, RHash: PaymentHash(htlc.PaymentHash), @@ -4963,124 +5063,24 @@ func (lc *LightningChannel) AvailableBalance() lnwire.MilliSatoshi { // this method. Additionally, the total weight of the next to be created // commitment is returned for accounting purposes. func (lc *LightningChannel) availableBalance() (lnwire.MilliSatoshi, int64) { - // First, we'll grab the current local balance. If we're the initiator - // of the channel then we paid the fees on the last commitment state, - // so we'll re-apply those. - settledBalance := lc.channelState.LocalCommitment.LocalBalance - if lc.channelState.IsInitiator { - settledBalance += lnwire.NewMSatFromSatoshis( - lc.localCommitChain.tip().fee, - ) - } - - // Next we'll grab the current set of log updates that are still active - // and haven't been garbage collected. + // We'll grab the current set of log updates that the remote has + // ACKed. remoteACKedIndex := lc.localCommitChain.tip().theirMessageIndex htlcView := lc.fetchHTLCView(remoteACKedIndex, lc.localUpdateLog.logIndex) - feePerKw := lc.channelState.LocalCommitment.FeePerKw - dustLimit := lc.channelState.LocalChanCfg.DustLimit - // We'll now re-compute the current weight of all the active HTLC's. We - // make sure to skip any HTLC's that would be dust on our version of - // the commitment transaction. - var totalHtlcWeight int64 - for _, htlc := range lc.channelState.LocalCommitment.Htlcs { - if htlcIsDust(false, true, feePerKw, htlc.Amt.ToSatoshis(), - dustLimit) { - continue - } + // Then compute our current balance for that view. + ourBalance, _, commitWeight, _, feePerKw := + lc.computeView(htlcView, false, false) - totalHtlcWeight += HtlcWeight - } - - // Next we'll run through our set of updates and modify the - // settledBalance and totalHtlcWeight fields accordingly. - for _, entry := range htlcView.ourUpdates { - switch { - - // For any new HTLC's added as a part of this state, we'll - // subtract the total balance, and tally the weight increase if - // it isn't dust. - case entry.EntryType == Add && entry.addCommitHeightLocal == 0: - settledBalance -= entry.Amount - - if htlcIsDust(false, true, feePerKw, entry.Amount.ToSatoshis(), - dustLimit) { - continue - - } - - totalHtlcWeight += HtlcWeight - - // For any new HTLC's we newly settled as part of this state, - // we'll subtract the HTLC weight and increase our balance - // accordingly. - case entry.EntryType == Settle && entry.removeCommitHeightLocal == 0: - totalHtlcWeight -= HtlcWeight - - settledBalance += entry.Amount - - // For any new fails added as a part of this state, we'll - // subtract the weight of the HTLC we're failing. - case entry.EntryType == Fail && entry.removeCommitHeightLocal == 0: - fallthrough - case entry.EntryType == MalformedFail && entry.removeCommitHeightLocal == 0: - totalHtlcWeight -= HtlcWeight - } - } - for _, entry := range htlcView.theirUpdates { - switch { - // If the remote party has an HTLC that will be included as - // part of this state, then we'll account for the additional - // weight of the HTLC. - case entry.EntryType == Add && entry.addCommitHeightLocal == 0: - if htlcIsDust(true, true, feePerKw, entry.Amount.ToSatoshis(), - dustLimit) { - continue - - } - - totalHtlcWeight += HtlcWeight - - // If the remote party is settling one of our HTLC's for the - // first time as part of this state, then we'll subtract the - // weight of the HTLC. - case entry.EntryType == Settle && entry.removeCommitHeightLocal == 0: - totalHtlcWeight -= HtlcWeight - - // For any HTLC's that they're failing as a part of the next, - // state, we'll subtract the weight of the HTLC and also credit - // ourselves back the value of the HTLC. - case entry.EntryType == Fail && entry.removeCommitHeightLocal == 0: - fallthrough - case entry.EntryType == MalformedFail && entry.removeCommitHeightLocal == 0: - totalHtlcWeight -= HtlcWeight - - settledBalance += entry.Amount - } - - } - - // If we subtracted dust HTLC's, then we'll need to reset the weight of - // the HTLCs back to zero. - if totalHtlcWeight < 0 { - totalHtlcWeight = 0 - } - - // If we're the initiator then we need to pay fees for this state, so - // taking into account the number of active HTLC's we'll calculate the - // fee that must be paid. - totalCommitWeight := CommitWeight + totalHtlcWeight + // If we are the channel initiator, we must remember to subtract the + // commitment fee from our available balance. + commitFee := btcutil.Amount((int64(feePerKw) * commitWeight) / 1000) if lc.channelState.IsInitiator { - additionalFee := lnwire.NewMSatFromSatoshis( - btcutil.Amount((int64(feePerKw) * totalCommitWeight) / 1000), - ) - - settledBalance -= additionalFee + ourBalance -= lnwire.NewMSatFromSatoshis(commitFee) } - return settledBalance, totalCommitWeight + return ourBalance, commitWeight } // StateSnapshot returns a snapshot of the current fully committed state within @@ -5374,3 +5374,8 @@ func (lc *LightningChannel) ActiveHtlcs() []channeldb.HTLC { return activeHtlcs } + +// LocalChanReserve returns our local ChanReserve requirement for the remote party. +func (lc *LightningChannel) LocalChanReserve() btcutil.Amount { + return lc.localChanCfg.ChanReserve +} diff --git a/lnwallet/channel_test.go b/lnwallet/channel_test.go index be8407ee..d809c4a0 100644 --- a/lnwallet/channel_test.go +++ b/lnwallet/channel_test.go @@ -4,7 +4,7 @@ import ( "bytes" "crypto/sha256" "io/ioutil" - "math/rand" + "os" "reflect" "runtime" @@ -163,10 +163,10 @@ func createTestChannels(revocationWindow int) (*LightningChannel, aliceCfg := channeldb.ChannelConfig{ ChannelConstraints: channeldb.ChannelConstraints{ DustLimit: aliceDustLimit, - MaxPendingAmount: lnwire.MilliSatoshi(rand.Int63()), + MaxPendingAmount: lnwire.NewMSatFromSatoshis(channelCapacity), ChanReserve: channelCapacity / 100, - MinHTLC: lnwire.MilliSatoshi(rand.Int63()), - MaxAcceptedHtlcs: uint16(rand.Int31()), + MinHTLC: 0, + MaxAcceptedHtlcs: MaxHTLCNumber / 2, }, CsvDelay: uint16(csvTimeoutAlice), MultiSigKey: aliceKeys[0].PubKey(), @@ -178,10 +178,10 @@ func createTestChannels(revocationWindow int) (*LightningChannel, bobCfg := channeldb.ChannelConfig{ ChannelConstraints: channeldb.ChannelConstraints{ DustLimit: bobDustLimit, - MaxPendingAmount: lnwire.MilliSatoshi(rand.Int63()), + MaxPendingAmount: lnwire.NewMSatFromSatoshis(channelCapacity), ChanReserve: channelCapacity / 100, - MinHTLC: lnwire.MilliSatoshi(rand.Int63()), - MaxAcceptedHtlcs: uint16(rand.Int31()), + MinHTLC: 0, + MaxAcceptedHtlcs: MaxHTLCNumber / 2, }, CsvDelay: uint16(csvTimeoutBob), MultiSigKey: bobKeys[0].PubKey(), @@ -1100,6 +1100,14 @@ func TestForceCloseDustOutput(t *testing.T) { } defer cleanUp() + // We set both node's channel reserves to 0, to make sure + // they can create small dust ouputs without going under + // their channel reserves. + aliceChannel.localChanCfg.ChanReserve = 0 + bobChannel.localChanCfg.ChanReserve = 0 + aliceChannel.remoteChanCfg.ChanReserve = 0 + bobChannel.remoteChanCfg.ChanReserve = 0 + htlcAmount := lnwire.NewMSatFromSatoshis(500) aliceAmount := aliceChannel.channelState.LocalCommitment.LocalBalance @@ -1372,6 +1380,11 @@ func TestChannelBalanceDustLimit(t *testing.T) { } defer cleanUp() + // To allow Alice's balance to get beneath her dust limit, set the + // channel reserve to be 0. + aliceChannel.localChanCfg.ChanReserve = 0 + bobChannel.remoteChanCfg.ChanReserve = 0 + // This amount should leave an amount larger than Alice's dust limit // once fees have been subtracted, but smaller than Bob's dust limit. // We account in fees for the HTLC we will be adding. @@ -2426,8 +2439,12 @@ func TestAddHTLCNegativeBalance(t *testing.T) { } defer cleanUp() - // First, we'll add 5 HTLCs of 1 BTC each to Alice's commitment. - const numHTLCs = 4 + // We set the channel reserve to 0, such that we can add HTLCs + // all the way to a negative balance. + aliceChannel.localChanCfg.ChanReserve = 0 + + // First, we'll add 3 HTLCs of 1 BTC each to Alice's commitment. + const numHTLCs = 3 htlcAmt := lnwire.NewMSatFromSatoshis(btcutil.SatoshiPerBitcoin) for i := 0; i < numHTLCs; i++ { htlc, _ := createHTLC(i, htlcAmt) @@ -2436,13 +2453,14 @@ func TestAddHTLCNegativeBalance(t *testing.T) { } } - // We'll then craft another HTLC with 2 BTC to add to Alice's channel. - // This attempt should put Alice in the negative, meaning she should - // reject the HTLC. - htlc, _ := createHTLC(numHTLCs+1, htlcAmt*2) + // Alice now has an available balance of 2 BTC. We'll add a new HTLC + // of value 2 BTC, which should make Alice's balance negative (since + // (she has to pay a commitment fee). + htlcAmt = lnwire.NewMSatFromSatoshis(2 * btcutil.SatoshiPerBitcoin) + htlc, _ := createHTLC(numHTLCs+1, htlcAmt) _, err = aliceChannel.AddHTLC(htlc) - if err != ErrInsufficientBalance { - t.Fatalf("expected insufficient balance, instead got: %v", err) + if err != ErrBelowChanReserve { + t.Fatalf("expected balance below channel reserve, instead got: %v", err) } } @@ -4344,7 +4362,7 @@ func TestDesyncHTLCs(t *testing.T) { // because the balance is unavailable. htlcAmt = lnwire.NewMSatFromSatoshis(1 * btcutil.SatoshiPerBitcoin) htlc, _ = createHTLC(1, htlcAmt) - if _, err = aliceChannel.AddHTLC(htlc); err != ErrInsufficientBalance { + if _, err = aliceChannel.AddHTLC(htlc); err != ErrBelowChanReserve { t.Fatalf("expected ErrInsufficientBalance, instead received: %v", err) } @@ -4360,3 +4378,359 @@ func TestDesyncHTLCs(t *testing.T) { } // TODO(roasbeef): testing.Quick test case for retrans!!! + +// TestMaxAcceptedHTLCs tests that the correct error message (ErrMaxHTLCNumber) +// is thrown when a node tries to accept more than MaxAcceptedHTLCs in a channel. +func TestMaxAcceptedHTLCs(t *testing.T) { + t.Parallel() + + // We'll kick off the test by creating our channels which both are + // loaded with 5 BTC each. + aliceChannel, bobChannel, cleanUp, err := createTestChannels(1) + if err != nil { + t.Fatalf("unable to create test channels: %v", err) + } + defer cleanUp() + + // One over the maximum number of HTLCs that either can accept. + const numHTLCs = 20 + const numHTLCsReceived = 12 + + // Set the remote's required MaxAcceptedHtlcs. This means that alice + // can only offer the remote up to numHTLCs HTLCs. + aliceChannel.localChanCfg.MaxAcceptedHtlcs = numHTLCs + bobChannel.remoteChanCfg.MaxAcceptedHtlcs = numHTLCs + + // Similarly, set the remote config's MaxAcceptedHtlcs. This means + // that the remote will be aware that Alice will only accept up to + // numHTLCsRecevied at a time. + aliceChannel.remoteChanCfg.MaxAcceptedHtlcs = numHTLCsReceived + bobChannel.localChanCfg.MaxAcceptedHtlcs = numHTLCsReceived + + // Each HTLC amount is 0.1 BTC. + htlcAmt := lnwire.NewMSatFromSatoshis(0.1 * btcutil.SatoshiPerBitcoin) + + // Send the maximum allowed number of HTLCs. + for i := 0; i < numHTLCs; i++ { + htlc, _ := createHTLC(i, htlcAmt) + if _, err := aliceChannel.AddHTLC(htlc); err != nil { + t.Fatalf("unable to add htlc: %v", err) + } + if _, err := bobChannel.ReceiveHTLC(htlc); err != nil { + t.Fatalf("unable to recv htlc: %v", err) + } + } + + // The next HTLC should fail with ErrMaxHTLCNumber. + htlc, _ := createHTLC(numHTLCs, htlcAmt) + _, err = aliceChannel.AddHTLC(htlc) + if err != ErrMaxHTLCNumber { + t.Fatalf("expected ErrMaxHTLCNumber, instead received: %v", err) + } + + // After receiving the next HTLC, next state transition should fail + // with ErrMaxHTLCNumber. + if _, err := bobChannel.ReceiveHTLC(htlc); err != nil { + t.Fatalf("unable to recv htlc: %v", err) + } + err = forceStateTransition(aliceChannel, bobChannel) + if err != ErrMaxHTLCNumber { + t.Fatalf("expected ErrMaxHTLCNumber, instead received: %v", err) + } +} + +// TestMaxPendingAmount tests that the maximum overall pending HTLC value is met +// given several HTLCs that, combined, exceed this value. An ErrMaxPendingAmount +// error should be returned. +func TestMaxPendingAmount(t *testing.T) { + t.Parallel() + + // We'll kick off the test by creating our channels which both are + // loaded with 5 BTC each. + aliceChannel, bobChannel, cleanUp, err := createTestChannels(1) + if err != nil { + t.Fatalf("unable to create test channels: %v", err) + } + defer cleanUp() + + // We set the remote required MaxPendingAmount to 3 BTC. We will + // attempt to overflow this value and see if it gives us the + // ErrMaxPendingAmount error. + maxPending := lnwire.NewMSatFromSatoshis(btcutil.SatoshiPerBitcoin * 3) + + // We set the max pending amount of Alice's config. This mean that she + // cannot offer Bob HTLCs with a total value above this limit at a given + // time. + aliceChannel.localChanCfg.MaxPendingAmount = maxPending + bobChannel.remoteChanCfg.MaxPendingAmount = maxPending + + // First, we'll add 2 HTLCs of 1.5 BTC each to Alice's commitment. + // This won't trigger Alice's ErrMaxPendingAmount error. + const numHTLCs = 2 + htlcAmt := lnwire.NewMSatFromSatoshis(1.5 * btcutil.SatoshiPerBitcoin) + for i := 0; i < numHTLCs; i++ { + htlc, _ := createHTLC(i, htlcAmt) + if _, err := aliceChannel.AddHTLC(htlc); err != nil { + t.Fatalf("unable to add htlc: %v", err) + } + if _, err := bobChannel.ReceiveHTLC(htlc); err != nil { + t.Fatalf("unable to recv htlc: %v", err) + } + } + + // We finally add one more HTLC of 0.1 BTC to Alice's commitment. This + // SHOULD trigger Alice's ErrMaxPendingAmount error. + htlcAmt = lnwire.NewMSatFromSatoshis(0.1 * btcutil.SatoshiPerBitcoin) + htlc, _ := createHTLC(numHTLCs, htlcAmt) + _, err = aliceChannel.AddHTLC(htlc) + if err != ErrMaxPendingAmount { + t.Fatalf("expected ErrMaxPendingAmount, instead received: %v", err) + } + + // And also Bob shouldn't be accepting this HTLC in the next state + // transition. + if _, err := bobChannel.ReceiveHTLC(htlc); err != nil { + t.Fatalf("unable to recv htlc: %v", err) + } + err = forceStateTransition(aliceChannel, bobChannel) + if err != ErrMaxPendingAmount { + t.Fatalf("expected ErrMaxPendingAmount, instead received: %v", err) + } +} + +// TestChanReserve tests that the ErrBelowChanReserve error is thrown when +// an HTLC is added that causes a node's balance to dip below its channel +// reserve limit. +func TestChanReserve(t *testing.T) { + t.Parallel() + + setupChannels := func() (*LightningChannel, *LightningChannel, func()) { + // We'll kick off the test by creating our channels which both are + // loaded with 5 BTC each. + aliceChannel, bobChannel, cleanUp, err := createTestChannels(1) + if err != nil { + t.Fatalf("unable to create test channels: %v", err) + } + + // We set the remote required ChanReserve to 0.5 BTC. We will + // attempt to cause Alice's balance to dip below this amount and test + // whether it triggers the ErrBelowChanReserve error. + aliceMinReserve := btcutil.Amount(0.5 * btcutil.SatoshiPerBitcoin) + + // Alice will need to keep her reserve above aliceMinReserve, so + // set this limit to here local config. + aliceChannel.localChanCfg.ChanReserve = aliceMinReserve + + // During channel opening Bob will also get to know Alice's minimum + // reserve, and this will be found in his remote config. + bobChannel.remoteChanCfg.ChanReserve = aliceMinReserve + + // We set Bob's channel reserve to a value that is larger than his + // current balance in the channel. This will ensure that after a + // channel is first opened, Bob can still receive HTLCs + // even though his balance is less than his channel reserve. + bobMinReserve := btcutil.Amount(6 * btcutil.SatoshiPerBitcoin) + bobChannel.localChanCfg.ChanReserve = bobMinReserve + aliceChannel.remoteChanCfg.ChanReserve = bobMinReserve + + return aliceChannel, bobChannel, cleanUp + } + aliceChannel, bobChannel, cleanUp := setupChannels() + defer cleanUp() + + aliceIndex := 0 + bobIndex := 0 + + // Add an HTLC that will increase Bob's balance. This should + // succeed, since Alice stays above her channel reserve, and + // Bob increases his balance (while still being below his + // channel reserve). + // Resulting balances: + // Alice: 4.5 + // Bob: 5.5 + htlcAmt := lnwire.NewMSatFromSatoshis(0.5 * btcutil.SatoshiPerBitcoin) + htlc, _ := createHTLC(aliceIndex, htlcAmt) + aliceIndex++ + if _, err := aliceChannel.AddHTLC(htlc); err != nil { + t.Fatalf("unable to add htlc: %v", err) + } + if _, err := bobChannel.ReceiveHTLC(htlc); err != nil { + t.Fatalf("unable to recv htlc: %v", err) + } + + // Force a state transation, making sure this HTLC is considered + // valid even though the channel reserves are not met. + if err := forceStateTransition(aliceChannel, bobChannel); err != nil { + t.Fatalf("unable to complete state update: %v", err) + } + + // Now let Bob try to add an HTLC. This should fail, since it + // will decrease his balance, which is already below the channel + // reserve. + // Resulting balances: + // Alice: 4.5 + // Bob: 5.5 + htlc, _ = createHTLC(bobIndex, htlcAmt) + bobIndex++ + _, err := bobChannel.AddHTLC(htlc) + if err != ErrBelowChanReserve { + t.Fatalf("expected ErrBelowChanReserve, instead received: %v", err) + } + + // Alice will reject this htlc when a state transition is attempted. + if _, err := aliceChannel.ReceiveHTLC(htlc); err != nil { + t.Fatalf("unable to recv htlc: %v", err) + } + err = forceStateTransition(aliceChannel, bobChannel) + if err != ErrBelowChanReserve { + t.Fatalf("expected ErrBelowChanReserve, instead received: %v", err) + } + + // We must setup the channels again, since a violation of the channel + // constraints leads to channel shutdown. + aliceChannel, bobChannel, cleanUp = setupChannels() + defer cleanUp() + + aliceIndex = 0 + bobIndex = 0 + + // Now we'll add HTLC of 3.5 BTC to Alice's commitment, this should + // put Alice's balance at 1.5 BTC. + // Resulting balances: + // Alice: 1.5 + // Bob: 9.5 + htlcAmt = lnwire.NewMSatFromSatoshis(3.5 * btcutil.SatoshiPerBitcoin) + + // The first HTLC should successfully be sent. + htlc, _ = createHTLC(aliceIndex, htlcAmt) + aliceIndex++ + if _, err := aliceChannel.AddHTLC(htlc); err != nil { + t.Fatalf("unable to add htlc: %v", err) + } + if _, err := bobChannel.ReceiveHTLC(htlc); err != nil { + t.Fatalf("unable to recv htlc: %v", err) + } + + // Add a second HTLC of 1 BTC. This should fail because it will take + // Alice's balance all the way down to her channel reserve, but + // since she is the initiator the additional transaction fee makes + // her balance dip below. + htlcAmt = lnwire.NewMSatFromSatoshis(1 * btcutil.SatoshiPerBitcoin) + htlc, _ = createHTLC(aliceIndex, htlcAmt) + aliceIndex++ + _, err = aliceChannel.AddHTLC(htlc) + if err != ErrBelowChanReserve { + t.Fatalf("expected ErrBelowChanReserve, instead received: %v", err) + } + + // Likewise, Bob will reject a state transition after this htlc is + // received, of the same reason. + if _, err := bobChannel.ReceiveHTLC(htlc); err != nil { + t.Fatalf("unable to recv htlc: %v", err) + } + err = forceStateTransition(aliceChannel, bobChannel) + if err != ErrBelowChanReserve { + t.Fatalf("expected ErrBelowChanReserve, instead received: %v", err) + } + + // We must setup the channels again, since a violation of the channel + // constraints leads to channel shutdown. + aliceChannel, bobChannel, cleanUp = setupChannels() + defer cleanUp() + + aliceIndex = 0 + bobIndex = 0 + + // Add a HTLC of 2 BTC to Alice, and the settle it. + // Resulting balances: + // Alice: 3.0 + // Bob: 7.0 + htlcAmt = lnwire.NewMSatFromSatoshis(2 * btcutil.SatoshiPerBitcoin) + htlc, preimage := createHTLC(aliceIndex, htlcAmt) + aliceIndex++ + aliceHtlcIndex, err := aliceChannel.AddHTLC(htlc) + if err != nil { + t.Fatalf("unable to add htlc: %v", err) + } + bobHtlcIndex, err := bobChannel.ReceiveHTLC(htlc) + if err != nil { + t.Fatalf("unable to recv htlc: %v", err) + } + if err := bobChannel.SettleHTLC(preimage, bobHtlcIndex); err != nil { + t.Fatalf("bob unable to settle inbound htlc: %v", err) + } + if err := aliceChannel.ReceiveHTLCSettle(preimage, aliceHtlcIndex); err != nil { + t.Fatalf("alice unable to accept settle of outbound htlc: %v", err) + } + + // And now let Bob add an HTLC of 1 BTC. This will take Bob's balance + // all the way down to his channel reserve, but since he is not paying the + // fee this is okay. + htlcAmt = lnwire.NewMSatFromSatoshis(1 * btcutil.SatoshiPerBitcoin) + htlc, _ = createHTLC(bobIndex, htlcAmt) + bobIndex++ + if _, err := bobChannel.AddHTLC(htlc); err != nil { + t.Fatalf("unable to add htlc: %v", err) + } + if _, err := aliceChannel.ReceiveHTLC(htlc); err != nil { + t.Fatalf("unable to recv htlc: %v", err) + } + // Do a last state transition, which should succeed. + if err := forceStateTransition(aliceChannel, bobChannel); err != nil { + t.Fatalf("unable to complete state update: %v", err) + } +} + +// TestMinHTLC tests that the ErrBelowMinHTLC error is thrown if an HTLC is added +// that is below the minimm allowed value for HTLCs. +func TestMinHTLC(t *testing.T) { + t.Parallel() + + // We'll kick off the test by creating our channels which both are + // loaded with 5 BTC each. + aliceChannel, bobChannel, cleanUp, err := createTestChannels(1) + if err != nil { + t.Fatalf("unable to create test channels: %v", err) + } + defer cleanUp() + + // We set Alice's MinHTLC to 0.1 BTC. We will attempt to send an + // HTLC BELOW this value to trigger the ErrBelowMinHTLC error. + minValue := lnwire.NewMSatFromSatoshis(0.1 * btcutil.SatoshiPerBitcoin) + + // Setting the min value in Alice's local config means that the + // remote will not accept any HTLCs of value less than specified. + aliceChannel.localChanCfg.MinHTLC = minValue + bobChannel.remoteChanCfg.MinHTLC = minValue + + // First, we will add an HTLC of 0.5 BTC. This will not trigger + // ErrBelowMinHTLC. + htlcAmt := lnwire.NewMSatFromSatoshis(0.5 * btcutil.SatoshiPerBitcoin) + htlc, _ := createHTLC(0, htlcAmt) + if _, err := aliceChannel.AddHTLC(htlc); err != nil { + t.Fatalf("unable to add htlc: %v", err) + } + if _, err := bobChannel.ReceiveHTLC(htlc); err != nil { + t.Fatalf("unable to recv htlc: %v", err) + } + + // We add an HTLC below the min value, this should result in + // an ErrBelowMinHTLC error. + amt := minValue - 100 + htlc, _ = createHTLC(1, amt) + _, err = aliceChannel.AddHTLC(htlc) + if err != ErrBelowMinHTLC { + t.Fatalf("expected ErrBelowMinHTLC, instead received: %v", err) + } + + // Bob will receive this HTLC, but reject the next state update, since + // the htlc is too small. + _, err = bobChannel.ReceiveHTLC(htlc) + if err != nil { + t.Fatalf("error receiving htlc: %v", err) + } + err = forceStateTransition(aliceChannel, bobChannel) + if err != ErrBelowMinHTLC { + t.Fatalf("expected ErrBelowMinHTLC, instead received: %v", err) + } +} diff --git a/lnwallet/interface_test.go b/lnwallet/interface_test.go index 600ade35..612be9c7 100644 --- a/lnwallet/interface_test.go +++ b/lnwallet/interface_test.go @@ -296,7 +296,7 @@ func testDualFundingReservationWorkflow(miner *rpctest.Harness, } aliceChanReservation.SetNumConfsRequired(numReqConfs) aliceChanReservation.CommitConstraints(csvDelay, lnwallet.MaxHTLCNumber/2, - lnwire.NewMSatFromSatoshis(fundingAmount), 10) + lnwire.NewMSatFromSatoshis(fundingAmount), 1, 10) // The channel reservation should now be populated with a multi-sig key // from our HD chain, a change output with 3 BTC, and 2 outputs @@ -319,7 +319,7 @@ func testDualFundingReservationWorkflow(miner *rpctest.Harness, t.Fatalf("bob unable to init channel reservation: %v", err) } bobChanReservation.CommitConstraints(csvDelay, lnwallet.MaxHTLCNumber/2, - lnwire.NewMSatFromSatoshis(fundingAmount), 10) + lnwire.NewMSatFromSatoshis(fundingAmount), 1, 10) bobChanReservation.SetNumConfsRequired(numReqConfs) assertContributionInitPopulated(t, bobChanReservation.OurContribution()) @@ -649,7 +649,7 @@ func testSingleFunderReservationWorkflow(miner *rpctest.Harness, } aliceChanReservation.SetNumConfsRequired(numReqConfs) aliceChanReservation.CommitConstraints(csvDelay, lnwallet.MaxHTLCNumber/2, - lnwire.NewMSatFromSatoshis(fundingAmt), 10) + lnwire.NewMSatFromSatoshis(fundingAmt), 1, 10) // Verify all contribution fields have been set properly. aliceContribution := aliceChanReservation.OurContribution() @@ -661,7 +661,6 @@ func testSingleFunderReservationWorkflow(miner *rpctest.Harness, t.Fatalf("coin selection failed, should have one change outputs, "+ "instead have: %v", len(aliceContribution.ChangeOutputs)) } - aliceContribution.CsvDelay = csvDelay assertContributionInitPopulated(t, aliceContribution) // Next, Bob receives the initial request, generates a corresponding @@ -673,12 +672,11 @@ func testSingleFunderReservationWorkflow(miner *rpctest.Harness, t.Fatalf("unable to create bob reservation: %v", err) } bobChanReservation.CommitConstraints(csvDelay, lnwallet.MaxHTLCNumber/2, - lnwire.NewMSatFromSatoshis(fundingAmt), 10) + lnwire.NewMSatFromSatoshis(fundingAmt), 1, 10) bobChanReservation.SetNumConfsRequired(numReqConfs) // We'll ensure that Bob's contribution also gets generated properly. bobContribution := bobChanReservation.OurContribution() - bobContribution.CsvDelay = csvDelay assertContributionInitPopulated(t, bobContribution) // With his contribution generated, he can now process Alice's diff --git a/lnwallet/reservation.go b/lnwallet/reservation.go index d856dfd1..0465531b 100644 --- a/lnwallet/reservation.go +++ b/lnwallet/reservation.go @@ -277,44 +277,68 @@ func (r *ChannelReservation) RegisterMinHTLC(minHTLC lnwire.MilliSatoshi) { // will also attempt to verify the constraints for sanity, returning an error // if the parameters are seemed unsound. func (r *ChannelReservation) CommitConstraints(csvDelay, maxHtlcs uint16, - maxValueInFlight lnwire.MilliSatoshi, chanReserve btcutil.Amount) error { + maxValueInFlight, minHtlc lnwire.MilliSatoshi, + chanReserve btcutil.Amount) error { r.Lock() defer r.Unlock() + // Fail if we consider csvDelay excessively large. + // TODO(halseth): find a more scientific choice of value. + if csvDelay > 10000 { + return fmt.Errorf("csvDelay is too large: %d", csvDelay) + } + + // Fail if we consider the channel reserve to be too large. + // We currently fail if it is greater than 20% of the + // channel capacity. + maxChanReserve := r.partialState.Capacity / 5 + if chanReserve > maxChanReserve { + return fmt.Errorf("chanReserve is too large: %g", + chanReserve.ToBTC()) + } + + // Fail if the minimum HTLC value is too large. If this is + // too large, the channel won't be useful for sending small + // payments. This limit is currently set to maxValueInFlight, + // effictively letting the remote setting this as large as + // it wants. + // TODO(halseth): set a reasonable/dynamic value. + if minHtlc > maxValueInFlight { + return fmt.Errorf("minimum HTLC value is too large: %g", + r.ourContribution.MinHTLC.ToBTC()) + } + + // Fail if maxHtlcs is above the maximum allowed number of 483. + // This number is specified in BOLT-02. + if maxHtlcs > uint16(MaxHTLCNumber/2) { + return fmt.Errorf("maxHtlcs is too large: %d", maxHtlcs) + } + + // Fail if we consider maxHtlcs too small. If this is too small + // we cannot offer many HTLCs to the remote. + const minNumHtlc = 5 + if maxHtlcs < minNumHtlc { + return fmt.Errorf("maxHtlcs is too small: %d", maxHtlcs) + } + + // Fail if we consider maxValueInFlight too small. We currently + // require the remote to at least allow minNumHtlc * minHtlc + // in flight. + if maxValueInFlight < minNumHtlc*minHtlc { + return fmt.Errorf("maxValueInFlight is too small: %g", + maxValueInFlight.ToBTC()) + } + r.ourContribution.ChannelConfig.CsvDelay = csvDelay r.ourContribution.ChannelConfig.ChanReserve = chanReserve r.ourContribution.ChannelConfig.MaxAcceptedHtlcs = maxHtlcs r.ourContribution.ChannelConfig.MaxPendingAmount = maxValueInFlight + r.ourContribution.ChannelConfig.MinHTLC = minHtlc return nil } -// RemoteChanConstraints returns our desired parameters which constraint the -// type of commitment transactions that the remote party can extend for our -// current state. In order to ensure that we only accept sane states, we'll -// specify: the required reserve the remote party must uphold, the max value in -// flight, and the maximum number of HTLC's that can propose in a state. -func (r *ChannelReservation) RemoteChanConstraints() (btcutil.Amount, lnwire.MilliSatoshi, uint16) { - chanCapacity := r.partialState.Capacity - - // TODO(roasbeef): move csv delay calculation into func? - - // By default, we'll require them to maintain at least 1% of the total - // channel capacity at all times. This is the absolute amount the - // settled balance of the remote party must be above at *all* times. - chanReserve := (chanCapacity) / 100 - - // We'll allow them to fully utilize the full bandwidth of the channel, - // minus our required reserve. - maxValue := lnwire.NewMSatFromSatoshis(chanCapacity - chanReserve) - - // Finally, we'll permit them to utilize the full channel bandwidth - maxHTLCs := uint16(MaxHTLCNumber / 2) - - return chanReserve, maxValue, maxHTLCs -} - // OurContribution returns the wallet's fully populated contribution to the // pending payment channel. See 'ChannelContribution' for further details // regarding the contents of a contribution.