From 7e7516137d5a433eb4ec69012023bb5980a2e953 Mon Sep 17 00:00:00 2001 From: nsa Date: Wed, 29 Nov 2017 14:16:36 +0100 Subject: [PATCH 01/16] lnwallet: validate commited channel constraints --- lnwallet/reservation.go | 48 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/lnwallet/reservation.go b/lnwallet/reservation.go index d856dfd1..ef9739f8 100644 --- a/lnwallet/reservation.go +++ b/lnwallet/reservation.go @@ -282,6 +282,54 @@ func (r *ChannelReservation) CommitConstraints(csvDelay, maxHtlcs uint16, r.Lock() defer r.Unlock() + // Fail if csvDelay is excessively large. + if csvDelay > 10000 { + return fmt.Errorf("csvDelay is too large: %d", csvDelay) + } + + // Fail if the channel reserve is set to greater than 20% of the + // channel capacity. + maxChanReserve := (r.partialState.Capacity + 4) / 5 + if chanReserve > maxChanReserve { + return fmt.Errorf("chanReserve is too large: %g", + chanReserve.ToBTC()) + } + + // Fail if the dust limit is lower than the DefaultDustLimit() + if r.ourContribution.DustLimit < DefaultDustLimit() { + return fmt.Errorf("dust limit is too small: %g", + r.ourContribution.DustLimit.ToBTC()) + } + + // Fail if maxHtlcs is above the maximum allowed number of 483. + if maxHtlcs > uint16(MaxHTLCNumber/2) { + return fmt.Errorf("maxHtlcs is too large: %d", maxHtlcs) + } + + // Fail if maxHtlcs is too small. + if maxHtlcs < 5 { + return fmt.Errorf("maxHtlcs is too small: %d", maxHtlcs) + } + + // Fail if maxValueInFlight is too large. + if maxValueInFlight > lnwire.NewMSatFromSatoshis( + r.partialState.Capacity-chanReserve) { + return fmt.Errorf("maxValueInFlight is too large: %g", + maxValueInFlight.ToBTC()) + } + + // Fail if maxValueInFlight is too small. + if maxValueInFlight < r.ourContribution.MinHTLC { + return fmt.Errorf("maxValueInFlight is too small: %g", + maxValueInFlight.ToBTC()) + } + + // Fail if the minimum HTLC value is too large + if r.ourContribution.MinHTLC > maxValueInFlight { + return fmt.Errorf("minimum HTLC value is too large: %g", + r.ourContribution.MinHTLC.ToBTC()) + } + r.ourContribution.ChannelConfig.CsvDelay = csvDelay r.ourContribution.ChannelConfig.ChanReserve = chanReserve r.ourContribution.ChannelConfig.MaxAcceptedHtlcs = maxHtlcs From 7e84892c21821a851bec0daa35264ec3b9d90d67 Mon Sep 17 00:00:00 2001 From: nsa Date: Wed, 29 Nov 2017 14:26:48 +0100 Subject: [PATCH 02/16] htlcswitch: account for channel reserve in Bandwidth --- htlcswitch/link.go | 3 ++- htlcswitch/test_utils.go | 28 ++++++++++++++++++++-------- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/htlcswitch/link.go b/htlcswitch/link.go index 4f4e0b52..4c94b44d 100644 --- a/htlcswitch/link.go +++ b/htlcswitch/link.go @@ -1264,8 +1264,9 @@ func (l *channelLink) Bandwidth() lnwire.MilliSatoshi { // TODO(roasbeef): subtract reserve channelBandwidth := l.channel.AvailableBalance() overflowBandwidth := l.overflowQueue.TotalHtlcAmount() + reserve := lnwire.NewMSatFromSatoshis(l.channel.GetReserve()) - return channelBandwidth - overflowBandwidth + return channelBandwidth - overflowBandwidth - reserve } // policyUpdate is a message sent to a channel link when an outside sub-system diff --git a/htlcswitch/test_utils.go b/htlcswitch/test_utils.go index 7595a6d7..23a9776c 100644 --- a/htlcswitch/test_utils.go +++ b/htlcswitch/test_utils.go @@ -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: 0, + MinHTLC: 0, + MaxAcceptedHtlcs: lnwallet.MaxHTLCNumber / 2, + } + + bobConstraints := &channeldb.ChannelConstraints{ + DustLimit: btcutil.Amount(800), + MaxPendingAmount: lnwire.NewMSatFromSatoshis( + channelCapacity), + ChanReserve: 0, + 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, From d924fcfdf475fa0a49b0fd4b896b10f17f3189d5 Mon Sep 17 00:00:00 2001 From: nsa Date: Wed, 29 Nov 2017 14:39:28 +0100 Subject: [PATCH 03/16] funding test: add default constraints to wallet config --- fundingmanager_test.go | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/fundingmanager_test.go b/fundingmanager_test.go index e00dd72f..3ea5d233 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 From 65723387fa7e613fa5a854524d7de58977394c0b Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 29 Nov 2017 13:41:45 +0100 Subject: [PATCH 04/16] channeldb: update ChannelConstraints godoc This commit changes the definition of the constraints in the ChannelConstraints struct to specify that these are all constraints the *owner* of the set of constraints must *never violate*. This is done to make it easier to check that a particular node is not violating any constraint for a gien update, as before it could violate constraints found both in its local and the remote contraints. --- channeldb/channel.go | 37 +++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 18 deletions(-) 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 From 50f495fae10ca69076677d160fba8ca20d624fcc Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 29 Nov 2017 13:50:44 +0100 Subject: [PATCH 05/16] lnwallet: cleanup of reservation.CommitConstraints, and move RemoteChanConstraints. This commit adds some more comments and checks to reservation.CommitConstraints, including making MinHTLC value one of the passed constraints. RemoteChanConstraints is also moved out of reservation. --- lnwallet/interface_test.go | 10 ++--- lnwallet/reservation.go | 76 +++++++++++++------------------------- 2 files changed, 30 insertions(+), 56 deletions(-) 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 ef9739f8..0465531b 100644 --- a/lnwallet/reservation.go +++ b/lnwallet/reservation.go @@ -277,92 +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 csvDelay is excessively large. + // 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 the channel reserve is set to greater than 20% of the + // 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 + 4) / 5 + maxChanReserve := r.partialState.Capacity / 5 if chanReserve > maxChanReserve { return fmt.Errorf("chanReserve is too large: %g", chanReserve.ToBTC()) } - // Fail if the dust limit is lower than the DefaultDustLimit() - if r.ourContribution.DustLimit < DefaultDustLimit() { - return fmt.Errorf("dust limit is too small: %g", - r.ourContribution.DustLimit.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 maxHtlcs is too small. - if maxHtlcs < 5 { + // 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 maxValueInFlight is too large. - if maxValueInFlight > lnwire.NewMSatFromSatoshis( - r.partialState.Capacity-chanReserve) { - return fmt.Errorf("maxValueInFlight is too large: %g", - maxValueInFlight.ToBTC()) - } - - // Fail if maxValueInFlight is too small. - if maxValueInFlight < r.ourContribution.MinHTLC { + // 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()) } - // Fail if the minimum HTLC value is too large - if r.ourContribution.MinHTLC > maxValueInFlight { - return fmt.Errorf("minimum HTLC value is too large: %g", - r.ourContribution.MinHTLC.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. From e6f7a46d905ebc2d75b0783eb0702be888fe3648 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 29 Nov 2017 13:57:41 +0100 Subject: [PATCH 06/16] htlcswitch: don't return negative value from Bandwidth() This commits prevents the Bandwith() method from returning a negative value if the channel reserve is larger than the actual available balance. --- htlcswitch/link.go | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/htlcswitch/link.go b/htlcswitch/link.go index 4c94b44d..e78eab0f 100644 --- a/htlcswitch/link.go +++ b/htlcswitch/link.go @@ -1261,12 +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() - reserve := lnwire.NewMSatFromSatoshis(l.channel.GetReserve()) + linkBandwidth := channelBandwidth - overflowBandwidth + reserve := lnwire.NewMSatFromSatoshis(l.channel.LocalChanReserve()) - return channelBandwidth - overflowBandwidth - reserve + // 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 From 509adce2adc00f11ef11be20839fd8a599a87007 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 7 Feb 2018 19:45:19 -0500 Subject: [PATCH 07/16] htlcswitch test: add TestChannelLinkBandwidthChanReserve --- htlcswitch/link_test.go | 140 +++++++++++++++++++++++++++++++++++++-- htlcswitch/test_utils.go | 16 +++-- 2 files changed, 144 insertions(+), 12 deletions(-) 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 23a9776c..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) { @@ -104,7 +104,7 @@ func createTestChannel(alicePrivKey, bobPrivKey []byte, DustLimit: btcutil.Amount(200), MaxPendingAmount: lnwire.NewMSatFromSatoshis( channelCapacity), - ChanReserve: 0, + ChanReserve: aliceReserve, MinHTLC: 0, MaxAcceptedHtlcs: lnwallet.MaxHTLCNumber / 2, } @@ -113,7 +113,7 @@ func createTestChannel(alicePrivKey, bobPrivKey []byte, DustLimit: btcutil.Amount(800), MaxPendingAmount: lnwire.NewMSatFromSatoshis( channelCapacity), - ChanReserve: 0, + ChanReserve: bobReserve, MinHTLC: 0, MaxAcceptedHtlcs: lnwallet.MaxHTLCNumber / 2, } @@ -668,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 "+ From 997b1ea8eeb35317078fce14136586f514a0053c Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 29 Nov 2017 13:59:27 +0100 Subject: [PATCH 08/16] fundingmanager: channel constraints closures This commit makes more channel constraints available via closures part of the fundingConfig, moving them from the reservation.RemoteChanConstraints method. --- fundingmanager.go | 32 +++++++++++++++++++++++++++----- fundingmanager_test.go | 10 ++++++++++ 2 files changed, 37 insertions(+), 5 deletions(-) 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 3ea5d233..972e0205 100644 --- a/fundingmanager_test.go +++ b/fundingmanager_test.go @@ -278,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 From a3a0efbeb07321090139f8afc7f1f63bc3d0bdc9 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 29 Nov 2017 14:03:01 +0100 Subject: [PATCH 09/16] lnd: populate channel constraints closures in fundingConfig. --- lnd.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) 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 From 78514acd495b6bf79f1ae3923dfce828b39d7c1b Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 29 Nov 2017 14:03:50 +0100 Subject: [PATCH 10/16] integration tests: accoount for channel reserve when sending payments. --- lnd_test.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) 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 From 98d28611e4d7803ac4afc9a5b81a5ead683cf369 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Tue, 6 Feb 2018 14:02:01 -0500 Subject: [PATCH 11/16] lnwallet/channel: let evaluateHTLCView take mutate boolean This commit adds a new boolean parameter mutateState to evalueteHTLCView, that let us call it without neccessarily mutating the addHeight/removeHeight of the HTLCs, which is useful when evaluating the commitment validity without mutating the state. --- lnwallet/channel.go | 34 +++++++++++++++++++++++----------- 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/lnwallet/channel.go b/lnwallet/channel.go index 36859952..7d81ca55 100644 --- a/lnwallet/channel.go +++ b/lnwallet/channel.go @@ -2253,8 +2253,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 +2284,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 +2293,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 +2304,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 +2313,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 +2326,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 +2336,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 +2348,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 +2376,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 +2386,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 +2426,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 From 1873fe1381f9bf0d6fe8706a7ebdf84ec98bfbd6 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Tue, 9 Jan 2018 16:42:07 +0100 Subject: [PATCH 12/16] lnwallet/channel: move common calculation of balance into computeView This commit moves common logic used to calculate the state of a commitment after applying a set of HTLC updates, into the new method computeView. This method can be used when calculating the available balance, validating the sanity of a commitment after applying a set of updates, and also when creating a new commitment, reducing the duplication of this logic. --- lnwallet/channel.go | 277 +++++++++++++++++++------------------------- 1 file changed, 117 insertions(+), 160 deletions(-) diff --git a/lnwallet/channel.go b/lnwallet/channel.go index 7d81ca55..88c76c4d 100644 --- a/lnwallet/channel.go +++ b/lnwallet/channel.go @@ -2023,62 +2023,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. @@ -3072,6 +3024,111 @@ func (lc *LightningChannel) ChanSyncMsg() (*lnwire.ChannelReestablish, error) { }, nil } +// 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) { + + commitChain := lc.localCommitChain + dustLimit := lc.localChanCfg.DustLimit + if remoteChain { + commitChain = lc.remoteCommitChain + dustLimit = lc.remoteChanCfg.DustLimit + } + + // 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 + + // 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 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 @@ -4975,124 +5032,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 From 7b9f098fe6ac5f53eef157366c34d24c0add08e4 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 29 Nov 2017 14:20:02 +0100 Subject: [PATCH 13/16] lnwallet: fully validate all channel constraints in validateCommitmentSanity This commit introduces changes to the validateCommitmentSanity function to fully validate all channel constraints. validateCommitmentSanity now validates that the MaxPendingAmount, ChanReserve, MinHTLC, & MaxAcceptedHTLCs limits are all adhered to during the lifetime of a channel. When applying a set of updates, the channel constraints are validated from the point-of-view of either the local or the remote node, to make sure the updates will be accepted. Co-authored-by: nsa --- lnwallet/channel.go | 242 +++++++++++++++++++++++++------------------- 1 file changed, 139 insertions(+), 103 deletions(-) diff --git a/lnwallet/channel.go b/lnwallet/channel.go index 88c76c4d..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, @@ -2665,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 } @@ -3129,69 +3144,118 @@ func (lc *LightningChannel) computeView(view *htlcView, remoteChain bool, return ourBalance, theirBalance, totalCommitWeight, filteredHTLCView, feePerKw } -// 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. +// 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, prediction bool, local bool, remote bool) error { - - // 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++ - } - - // TODO(roasbeef): call availableBalance in here re-using htlcView - - // Run through all the HTLCs that will be covered by this transaction - // in order to calculate theirs count. + 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 @@ -3392,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 } @@ -3411,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, @@ -3737,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), @@ -3784,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 @@ -3801,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), @@ -5343,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 +} From f9701cde63913b5c5cf937258d3f74a57451d16b Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 7 Feb 2018 21:05:53 -0500 Subject: [PATCH 14/16] lnwallet tests: set channel reserve during channel tests --- lnwallet/channel_test.go | 50 +++++++++++++++++++++++++++------------- 1 file changed, 34 insertions(+), 16 deletions(-) diff --git a/lnwallet/channel_test.go b/lnwallet/channel_test.go index be8407ee..a7970538 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) } From 82dc8e0794a657905ce9c71e075c6de848f8dd33 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 7 Feb 2018 20:55:34 -0500 Subject: [PATCH 15/16] lnwallet test: add channel constraints tests This commit adds the tests TestMaxAcceptedHTLCs, TestMaxPendingAmount, TestChanReserve and TestMinHTLC. --- lnwallet/channel_test.go | 356 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 356 insertions(+) diff --git a/lnwallet/channel_test.go b/lnwallet/channel_test.go index a7970538..d809c4a0 100644 --- a/lnwallet/channel_test.go +++ b/lnwallet/channel_test.go @@ -4378,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) + } +} From f6d357e122443654824cc633e9b48361fdff76cb Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 7 Feb 2018 22:36:34 -0500 Subject: [PATCH 16/16] breacharbiter tests: set MinHtlc=0 during tests --- breacharbiter_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) 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),