Merge pull request #443 from halseth/chanConstraints

Validate channel constraints

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()),
+			ChanReserve:      0,
-			MinHTLC:          lnwire.MilliSatoshi(rand.Int63()),
+			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()),
+			ChanReserve:      0,
-			MinHTLC:          lnwire.MilliSatoshi(rand.Int63()),
+			MinHTLC:          0,
 			MaxAcceptedHtlcs: uint16(rand.Int31()),
 		},
 		CsvDelay:            uint16(csvTimeoutBob),

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
+	// ChanReserve is an absolute reservation on the channel for the
-	// present within the channel at a particular time. This value is set
+	// owner of this set of constraints. This means that the current
-	// by the initiator of the channel and must be upheld at all times.
+	// settled balance for this node CANNOT dip below the reservation
-	MaxPendingAmount lnwire.MilliSatoshi
+	// 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 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 btcutil.Amount
 
-	// MinHTLC is the minimum HTLC accepted for a direction of the channel.
+	// MaxPendingAmount is the maximum pending HTLC value that the
-	// If any HTLC's below this amount are offered, then the HTLC will be
+	// owner of these constraints can offer the remote node at a
-	// rejected. This, in tandem with the dust limit allows a node to
+	// particular time.
-	// regulate the smallest HTLC that it deems economically relevant.
+	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
+	// MaxAcceptedHtlcs is the maximum number of HTLCs that the owner of
-	// accepted by the owner of this set of constraints. This allows each
+	// this set of constraints can offer the remote node. This allows each
-	// node to limit their over all exposure to HTLC's that may need to be
+	// 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

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)

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,
+		Database:           cdb,
-		Notifier:         notifier,
+		Notifier:           notifier,
-		WalletController: wc,
+		WalletController:   wc,
-		Signer:           signer,
+		Signer:             signer,
-		ChainIO:          bio,
+		ChainIO:            bio,
-		FeeEstimator:     estimator,
+		FeeEstimator:       estimator,
-		NetParams:        *netParams,
+		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

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

htlcswitch/link_test.go

@@ -1405,8 +1405,8 @@ func (m *mockPeer) Disconnect(reason error) {
 
 var _ Peer = (*mockPeer)(nil)
 
-func newSingleLinkTestHarness(chanAmt btcutil.Amount) (ChannelLink,
+func newSingleLinkTestHarness(chanAmt, chanReserve btcutil.Amount) (
-	*lnwallet.LightningChannel, chan time.Time, func(), error) {
+	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) {

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{
+		ChannelConstraints:  *aliceConstraints,
-			DustLimit: aliceDustLimit,
-		},
 		CsvDelay:            uint16(csvTimeoutAlice),
 		MultiSigKey:         aliceKeyPub,
 		RevocationBasePoint: aliceKeyPub,
@@ -127,9 +141,7 @@ func createTestChannel(alicePrivKey, bobPrivKey []byte,
 		HtlcBasePoint:       aliceKeyPub,
 	}
 	bobCfg := channeldb.ChannelConfig{
-		ChannelConstraints: channeldb.ChannelConstraints{
+		ChannelConstraints:  *bobConstraints,
-			DustLimit: bobDustLimit,
-		},
 		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(
+	aliceChannel, firstBobChannel, cleanAliceBob, restoreAliceBob, err :=
-		alicePrivKey, bobPrivKey, aliceToBob, aliceToBob, firstChanID)
+		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(
+	secondBobChannel, carolChannel, cleanBobCarol, restoreBobCarol, err :=
-		bobPrivKey, carolPrivKey, bobToCarol, bobToCarol, secondChanID)
+		createTestChannel(bobPrivKey, carolPrivKey, bobToCarol,
+			bobToCarol, 0, 0, secondChanID)
 	if err != nil {
 		cleanAliceBob()
 		return nil, nil, nil, errors.Errorf("unable to create "+

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

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.
+	// Calculate the number of invoices. We will deplete the channel
-	numInvoices := int(info.LocalBalance / paymentAmt)
+	// 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
 

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
+	// ErrMaxPendingAmount is returned when a proposed HTLC would exceed
-	// exceed the available balance.
+	// the overall maximum pending value of all HTLCs if committed in a
-	ErrInsufficientBalance = fmt.Errorf("insufficient local balance")
+	// 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,
+	ourBalance, theirBalance, _, filteredHTLCView, feePerKw :=
-		&theirBalance, nextHeight, remoteChain)
+		lc.computeView(htlcView, remoteChain, true)
-
-	// 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
-		}
-	}
 
 	// 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
+// computeView takes the given htlcView, and calculates the balances,
-// transaction is valid in terms of propagating it over Bitcoin network, and
+// filtered view (settling unsettled HTLCs), commitment weight and
-// also that all outputs are meet Bitcoin spec requirements and they are
+// feePerKw, after applying the HTLCs to the latest commitment. The
-// spendable.
+// returned balanced are the balances *before* subtracting the
-func (lc *LightningChannel) validateCommitmentSanity(theirLogCounter,
+// commitment fee from the initiator's balance.
-	ourLogCounter uint64, prediction bool, local bool, remote bool) error {
+//
+// 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
+	commitChain := lc.localCommitChain
-	htlcCount := 0
+	dustLimit := lc.localChanCfg.DustLimit
-
+	if remoteChain {
-	// If we adding or receiving the htlc we increase the number of htlcs
+		commitChain = lc.remoteCommitChain
-	// by one in order to not overflow the commitment transaction by
+		dustLimit = lc.remoteChanCfg.DustLimit
-	// insertion.
-	if prediction {
-		htlcCount++
 	}
 
-	// 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
+	// Add the fee from the previous commitment state back to the
-	// in order to calculate theirs count.
+	// 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 {
+	// If we are checking if we can add a new HTLC, we add this to the
-		for _, entry := range view.theirUpdates {
+	// update log, in order to validate the sanity of the commitment
-			if entry.EntryType == Add {
+	// resulting from _actually adding_ this HTLC to the state.
-				htlcCount++
+	if predictAdded != nil {
-			}
+		// If we are adding an HTLC, this will be an Add to the
-		}
+		// local update log.
-		for _, entry := range view.ourUpdates {
+		view.ourUpdates = append(view.ourUpdates, predictAdded)
-			if entry.EntryType != Add {
-				htlcCount--
-			}
-		}
 	}
 
-	if local {
+	commitChain := lc.localCommitChain
-		for _, entry := range view.ourUpdates {
+	if remoteChain {
-			if entry.EntryType == Add {
+		commitChain = lc.remoteCommitChain
-				htlcCount++
-			}
-		}
-		for _, entry := range view.theirUpdates {
-			if entry.EntryType != Add {
-				htlcCount--
-			}
-		}
 	}
+	ourInitialBalance := commitChain.tip().ourBalance
+	theirInitialBalance := commitChain.tip().theirBalance
 
-	// If we're validating the commitment sanity for HTLC _log_ update by a
+	ourBalance, theirBalance, commitWeight, filteredView, feePerKw :=
-	// particular side, then we'll only consider half of the available HTLC
+		lc.computeView(view, remoteChain, false)
-	// 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
+	// Calculate the commitment fee, and subtract it from the
-	// contribution of both sides shouldn't exceed the max number.
+	// initiator's balance.
-	var maxHTLCNumber int
+	commitFee := btcutil.Amount((int64(feePerKw) * commitWeight) / 1000)
-	if local && remote {
+	if lc.channelState.IsInitiator {
-		maxHTLCNumber = MaxHTLCNumber
+		ourBalance -= lnwire.NewMSatFromSatoshis(commitFee)
 	} else {
-		maxHTLCNumber = MaxHTLCNumber / 2
+		theirBalance -= lnwire.NewMSatFromSatoshis(commitFee)
 	}
 
-	if htlcCount > maxHTLCNumber {
+	// If the added HTLCs will decrease the balance, make sure
-		return ErrMaxHTLCNumber
+	// 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
+	// commitment view which includes all the entries (pending or committed)
-	// HTLC log, and up to ourLogIndex in our HTLC log.
+	// 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
+	// We'll grab the current set of log updates that the remote has
-	// of the channel then we paid the fees on the last commitment state,
+	// ACKed.
-	// 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.
 	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
+	// Then compute our current balance for that view.
-	// make sure to skip any HTLC's that would be dust on our version of
+	ourBalance, _, commitWeight, _, feePerKw :=
-	// the commitment transaction.
+		lc.computeView(htlcView, false, false)
-	var totalHtlcWeight int64
-	for _, htlc := range lc.channelState.LocalCommitment.Htlcs {
-		if htlcIsDust(false, true, feePerKw, htlc.Amt.ToSatoshis(),
-			dustLimit) {
-			continue
-		}
 
-		totalHtlcWeight += HtlcWeight
+	// 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)
-	// 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 lc.channelState.IsInitiator {
-		additionalFee := lnwire.NewMSatFromSatoshis(
+		ourBalance -= lnwire.NewMSatFromSatoshis(commitFee)
-			btcutil.Amount((int64(feePerKw) * totalCommitWeight) / 1000),
-		)
-
-		settledBalance -= additionalFee
 	}
 
-	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
+}

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()),
+			MinHTLC:          0,
-			MaxAcceptedHtlcs: uint16(rand.Int31()),
+			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()),
+			MinHTLC:          0,
-			MaxAcceptedHtlcs: uint16(rand.Int31()),
+			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.
+	// We set the channel reserve to 0, such that we can add HTLCs
-	const numHTLCs = 4
+	// 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.
+	// Alice now has an available balance of 2 BTC. We'll add a new HTLC
-	// This attempt should put Alice in the negative, meaning she should
+	// of value 2 BTC, which should make Alice's balance negative (since
-	// reject the HTLC.
+	// (she has to pay a commitment fee).
-	htlc, _ := createHTLC(numHTLCs+1, htlcAmt*2)
+	htlcAmt = lnwire.NewMSatFromSatoshis(2 * btcutil.SatoshiPerBitcoin)
+	htlc, _ := createHTLC(numHTLCs+1, htlcAmt)
 	_, err = aliceChannel.AddHTLC(htlc)
-	if err != ErrInsufficientBalance {
+	if err != ErrBelowChanReserve {
-		t.Fatalf("expected insufficient balance, instead got: %v", err)
+		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)
+	}
+}

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

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.