lnwallet: replace hard-coded fees and adjust tests accordingly

This commit replaces the hard-coded 5000 satoshi fees with calls to the
FeeEstimator interface. This should provide a way to cleanly plug in
additional fee calculation algorithms in the future. This change
affected quite a few tests. When possible, the tests were changed to
assert amounts sent rather than balances so that fees wouldn't need to
be taken into account. There were several tests for which this wasn't
possible, so calls to the static fee calculator were made.

lnd_test.go

@@ -283,6 +283,20 @@ func assertNumConnections(ctxt context.Context, t *harnessTest,
 	}
 }
 
+// calcStaticFee calculates appropriate fees for commitment transactions.  This
+// function provides a simple way to allow test balance assertions to take fee
+// calculations into account.
+// TODO(bvu): Refactor when dynamic fee estimation is added.
+func calcStaticFee(numHTLCs int) btcutil.Amount {
+	const (
+		commitWeight = btcutil.Amount(724)
+		htlcWeight   = 172
+		feePerByte   = btcutil.Amount(250 * 4)
+	)
+	return feePerByte * (commitWeight +
+		btcutil.Amount(htlcWeight*numHTLCs)) / 1000
+}
+
 // testBasicChannelFunding performs a test exercising expected behavior from a
 // basic funding workflow. The test creates a new channel between Alice and
 // Bob, then immediately closes the channel after asserting some expected post
@@ -322,9 +336,9 @@ func testBasicChannelFunding(net *networkHarness, t *harnessTest) {
 	if err != nil {
 		t.Fatalf("unable to get bobs's balance: %v", err)
 	}
-	if aliceBal.Balance != int64(chanAmt-pushAmt) {
+	if aliceBal.Balance != int64(chanAmt-pushAmt-calcStaticFee(0)) {
 		t.Fatalf("alice's balance is incorrect: expected %v got %v",
-			chanAmt-pushAmt, aliceBal)
+			chanAmt-pushAmt-calcStaticFee(0), aliceBal)
 	}
 	if bobBal.Balance != int64(pushAmt) {
 		t.Fatalf("bob's balance is incorrect: expected %v got %v",
@@ -631,7 +645,7 @@ func testChannelBalance(net *networkHarness, t *harnessTest) {
 
 	// As this is a single funder channel, Alice's balance should be
 	// exactly 0.5 BTC since now state transitions have taken place yet.
-	checkChannelBalance(net.Alice, amount)
+	checkChannelBalance(net.Alice, amount-calcStaticFee(0))
 
 	// Ensure Bob currently has no available balance within the channel.
 	checkChannelBalance(net.Bob, 0)
@@ -1864,6 +1878,7 @@ func testHtlcErrorPropagation(net *networkHarness, t *harnessTest) {
 		t.Fatalf("channel not seen by alice before timeout: %v", err)
 	}
 
+	commitFee := calcStaticFee(0)
 	assertBaseBalance := func() {
 		balReq := &lnrpc.ChannelBalanceRequest{}
 		aliceBal, err := net.Alice.ChannelBalance(ctxb, balReq)
@@ -1874,13 +1889,13 @@ func testHtlcErrorPropagation(net *networkHarness, t *harnessTest) {
 		if err != nil {
 			t.Fatalf("unable to get channel balance: %v", err)
 		}
-		if aliceBal.Balance != int64(chanAmt) {
+		if aliceBal.Balance != int64(chanAmt-commitFee) {
 			t.Fatalf("alice has an incorrect balance: expected %v got %v",
-				int64(chanAmt), aliceBal)
+				int64(chanAmt-commitFee), aliceBal)
 		}
-		if bobBal.Balance != int64(chanAmt) {
+		if bobBal.Balance != int64(chanAmt-commitFee) {
 			t.Fatalf("bob has an incorrect balance: expected %v got %v",
-				int64(chanAmt), bobBal)
+				int64(chanAmt-commitFee), bobBal)
 		}
 	}
 

lnwallet/channel.go

@@ -683,8 +683,7 @@ type LightningChannel struct {
 // automatically persist pertinent state to the database in an efficient
 // manner.
 func NewLightningChannel(signer Signer, events chainntnfs.ChainNotifier,
-	fe FeeEstimator, state *channeldb.OpenChannel) (*LightningChannel,
+	fe FeeEstimator, state *channeldb.OpenChannel) (*LightningChannel, error) {
-	error) {
 
 	lc := &LightningChannel{
 		signer:                signer,
@@ -719,6 +718,7 @@ func NewLightningChannel(signer Signer, events chainntnfs.ChainNotifier,
 		ourMessageIndex:   0,
 		theirBalance:      state.TheirBalance,
 		theirMessageIndex: 0,
+		fee:               state.CommitFee,
 	})
 	walletLog.Debugf("ChannelPoint(%v), starting local commitment: %v",
 		state.ChanID, newLogClosure(func() string {
@@ -739,6 +739,7 @@ func NewLightningChannel(signer Signer, events chainntnfs.ChainNotifier,
 		ourMessageIndex:   0,
 		theirBalance:      state.TheirBalance,
 		theirMessageIndex: 0,
+		fee:               state.CommitFee,
 	}
 	if logTail == nil {
 		remoteCommitment.height = 0
@@ -1193,8 +1194,6 @@ func (lc *LightningChannel) restoreStateLogs() error {
 	lc.localCommitChain.tail().theirMessageIndex = theirCounter
 	lc.remoteCommitChain.tail().ourMessageIndex = ourCounter
 	lc.remoteCommitChain.tail().theirMessageIndex = theirCounter
-	lc.localCommitChain.tail().fee = lc.channelState.CommitFee
-	lc.remoteCommitChain.tail().fee = lc.channelState.CommitFee
 
 	return nil
 }
@@ -1262,6 +1261,10 @@ func (lc *LightningChannel) fetchCommitmentView(remoteChain bool,
 	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 = ourBalance + commitChain.tip().fee
 	} else if !lc.channelState.IsInitiator {
@@ -1278,24 +1281,54 @@ func (lc *LightningChannel) fetchCommitmentView(remoteChain bool,
 	filteredHTLCView := lc.evaluateHTLCView(htlcView, &ourBalance, &theirBalance,
 		nextHeight, remoteChain)
 
+	// Determine how many current HTLCs are over the dust limit, and should
+	// be counted for the purpose of fee calculation.
+	var dustLimit btcutil.Amount
+	if remoteChain {
+		dustLimit = lc.channelState.TheirDustLimit
+	} else {
+		dustLimit = lc.channelState.OurDustLimit
+	}
+	numHTLCs := 0
+	for _, htlc := range filteredHTLCView.ourUpdates {
+		if htlc.Amount < dustLimit {
+			continue
+		}
+		numHTLCs++
+	}
+	for _, htlc := range filteredHTLCView.theirUpdates {
+		if htlc.Amount < dustLimit {
+			continue
+		}
+		numHTLCs++
+	}
+
+	// Calculate the fee for the commitment transaction based on its size.
+	commitFee := btcutil.Amount(lc.feeEstimator.EstimateFeePerWeight(1)) *
+		(commitWeight + btcutil.Amount(htlcWeight*numHTLCs)) / 1000
+
+	if lc.channelState.IsInitiator {
+		ourBalance = ourBalance - commitFee
+	} else if !lc.channelState.IsInitiator {
+		theirBalance = theirBalance - commitFee
+	}
+
 	var selfKey *btcec.PublicKey
 	var remoteKey *btcec.PublicKey
 	var delay uint32
-	var delayBalance, p2wkhBalance, dustLimit btcutil.Amount
+	var delayBalance, p2wkhBalance btcutil.Amount
 	if remoteChain {
 		selfKey = lc.channelState.TheirCommitKey
 		remoteKey = lc.channelState.OurCommitKey
 		delay = lc.channelState.RemoteCsvDelay
 		delayBalance = theirBalance
 		p2wkhBalance = ourBalance
-		dustLimit = lc.channelState.TheirDustLimit
 	} else {
 		selfKey = lc.channelState.OurCommitKey
 		remoteKey = lc.channelState.TheirCommitKey
 		delay = lc.channelState.LocalCsvDelay
 		delayBalance = ourBalance
 		p2wkhBalance = theirBalance
-		dustLimit = lc.channelState.OurDustLimit
 	}
 
 	// Generate a new commitment transaction with all the latest
@@ -1352,6 +1385,7 @@ func (lc *LightningChannel) fetchCommitmentView(remoteChain bool,
 		ourMessageIndex:   ourLogIndex,
 		theirMessageIndex: theirLogIndex,
 		theirBalance:      theirBalance,
+		fee:               commitFee,
 	}
 
 	// In order to ensure _none_ of the HTLC's associated with this new
@@ -2502,6 +2536,17 @@ func (lc *LightningChannel) InitCooperativeClose() ([]byte, *chainhash.Hash, err
 		return nil, nil, ErrChanClosing
 	}
 
+	// Calculate the fee for the commitment transaction based on its size.
+	// For a cooperative close, there should be no HTLCs.
+	commitFee := btcutil.Amount(lc.feeEstimator.EstimateFeePerWeight(1)) *
+		commitWeight / 1000
+
+	if lc.channelState.IsInitiator {
+		lc.channelState.OurBalance = lc.channelState.OurBalance - commitFee
+	} else {
+		lc.channelState.TheirBalance = lc.channelState.TheirBalance - commitFee
+	}
+
 	closeTx := CreateCooperativeCloseTx(lc.fundingTxIn,
 		lc.channelState.OurDustLimit, lc.channelState.TheirDustLimit,
 		lc.channelState.OurBalance, lc.channelState.TheirBalance,
@@ -2552,6 +2597,17 @@ func (lc *LightningChannel) CompleteCooperativeClose(remoteSig []byte) (*wire.Ms
 		return nil, ErrChanClosing
 	}
 
+	// Calculate the fee for the commitment transaction based on its size.
+	// For a cooperative close, there should be no HTLCs.
+	commitFee := btcutil.Amount(lc.feeEstimator.EstimateFeePerWeight(1)) *
+		commitWeight / 1000
+
+	if lc.channelState.IsInitiator {
+		lc.channelState.OurBalance = lc.channelState.OurBalance - commitFee
+	} else {
+		lc.channelState.TheirBalance = lc.channelState.TheirBalance - commitFee
+	}
+
 	// Create the transaction used to return the current settled balance
 	// on this active channel back to both parties. In this current model,
 	// the initiator pays full fees for the cooperative close transaction.
@@ -2695,15 +2751,6 @@ func CreateCooperativeCloseTx(fundingTxIn *wire.TxIn,
 	closeTx := wire.NewMsgTx(2)
 	closeTx.AddTxIn(fundingTxIn)
 
-	// The initiator of a cooperative closure pays the fee in entirety.
-	// Determine if we're the initiator so we can compute fees properly.
-	if initiator {
-		// TODO(roasbeef): take sat/byte here instead of properly calc
-		ourBalance -= 5000
-	} else {
-		theirBalance -= 5000
-	}
-
 	// Create both cooperative closure outputs, properly respecting the
 	// dust limits of both parties.
 	if ourBalance >= localDust {

lnwallet/channel_test.go

@@ -329,6 +329,20 @@ func createTestChannels(revocationWindow int) (*LightningChannel, *LightningChan
 	return channelAlice, channelBob, cleanUpFunc, nil
 }
 
+// calcStaticFee calculates appropriate fees for commitment transactions.  This
+// function provides a simple way to allow test balance assertions to take fee
+// calculations into account.
+// TODO(bvu): Refactor when dynamic fee estimation is added.
+func calcStaticFee(numHTLCs int) btcutil.Amount {
+	const (
+		commitWeight = btcutil.Amount(724)
+		htlcWeight   = 172
+		feePerByte   = btcutil.Amount(50 * 4)
+	)
+	return feePerByte * (commitWeight +
+		btcutil.Amount(htlcWeight*numHTLCs)) / 1000
+}
+
 // TestSimpleAddSettleWorkflow tests a simple channel scenario wherein the
 // local node (Alice in this case) creates a new outgoing HTLC to bob, commits
 // this change, then bob immediately commits a settlement of the HTLC after the
@@ -429,25 +443,27 @@ func TestSimpleAddSettleWorkflow(t *testing.T) {
 			"forward %v", len(htlcs))
 	}
 
-	// At this point, both sides should have the proper balance, and
+	// At this point, both sides should have the proper number of satoshis
-	// commitment height updated within their local channel state.
+	// sent, and commitment height updated within their local channel
-	aliceBalance := btcutil.Amount(4 * 1e8)
+	// state.
-	bobBalance := btcutil.Amount(5 * 1e8)
+	aliceSent := uint64(0)
-	if aliceChannel.channelState.OurBalance != aliceBalance {
+	bobSent := uint64(0)
-		t.Fatalf("alice has incorrect local balance %v vs %v",
+
-			aliceChannel.channelState.OurBalance, aliceBalance)
+	if aliceChannel.channelState.TotalSatoshisSent != aliceSent {
+		t.Fatalf("alice has incorrect satoshis sent: %v vs %v",
+			aliceChannel.channelState.TotalSatoshisSent, aliceSent)
 	}
-	if aliceChannel.channelState.TheirBalance != bobBalance {
+	if aliceChannel.channelState.TotalSatoshisReceived != bobSent {
-		t.Fatalf("alice has incorrect remote balance %v vs %v",
+		t.Fatalf("alice has incorrect satoshis received %v vs %v",
-			aliceChannel.channelState.TheirBalance, bobBalance)
+			aliceChannel.channelState.TotalSatoshisReceived, bobSent)
 	}
-	if bobChannel.channelState.OurBalance != bobBalance {
+	if bobChannel.channelState.TotalSatoshisSent != bobSent {
-		t.Fatalf("bob has incorrect local balance %v vs %v",
+		t.Fatalf("bob has incorrect satoshis sent %v vs %v",
-			bobChannel.channelState.OurBalance, bobBalance)
+			bobChannel.channelState.TotalSatoshisSent, bobSent)
 	}
-	if bobChannel.channelState.TheirBalance != aliceBalance {
+	if bobChannel.channelState.TotalSatoshisReceived != aliceSent {
-		t.Fatalf("bob has incorrect remote balance %v vs %v",
+		t.Fatalf("bob has incorrect satoshis received %v vs %v",
-			bobChannel.channelState.TheirBalance, aliceBalance)
+			bobChannel.channelState.TotalSatoshisReceived, aliceSent)
 	}
 	if bobChannel.currentHeight != 1 {
 		t.Fatalf("bob has incorrect commitment height, %v vs %v",
@@ -526,25 +542,7 @@ func TestSimpleAddSettleWorkflow(t *testing.T) {
 	// 4 BTC. Alice's channel should show 1 BTC sent and Bob's channel should
 	// show 1 BTC received. They should also be at commitment height two,
 	// with the revocation window extended by by 1 (5).
-	aliceSettleBalance := btcutil.Amount(4 * 1e8)
-	bobSettleBalance := btcutil.Amount(6 * 1e8)
 	satoshisTransferred := uint64(100000000)
-	if aliceChannel.channelState.OurBalance != aliceSettleBalance {
-		t.Fatalf("alice has incorrect local balance %v vs %v",
-			aliceChannel.channelState.OurBalance, aliceSettleBalance)
-	}
-	if aliceChannel.channelState.TheirBalance != bobSettleBalance {
-		t.Fatalf("alice has incorrect remote balance %v vs %v",
-			aliceChannel.channelState.TheirBalance, bobSettleBalance)
-	}
-	if bobChannel.channelState.OurBalance != bobSettleBalance {
-		t.Fatalf("bob has incorrect local balance %v vs %v",
-			bobChannel.channelState.OurBalance, bobSettleBalance)
-	}
-	if bobChannel.channelState.TheirBalance != aliceSettleBalance {
-		t.Fatalf("bob has incorrect remote balance %v vs %v",
-			bobChannel.channelState.TheirBalance, aliceSettleBalance)
-	}
 	if aliceChannel.channelState.TotalSatoshisSent != satoshisTransferred {
 		t.Fatalf("alice satoshis sent incorrect %v vs %v expected",
 			aliceChannel.channelState.TotalSatoshisSent,
@@ -1060,6 +1058,42 @@ func TestCheckDustLimit(t *testing.T) {
 	}
 	defer cleanUp()
 
+	assertChannelState := func(chanA, chanB *LightningChannel, numOutsA,
+		numOutsB int, amountSentA, amountSentB uint64) {
+
+		commitment := chanA.localCommitChain.tip()
+		if len(commitment.txn.TxOut) != numOutsA {
+			t.Fatalf("incorrect # of outputs: expected %v, got %v",
+				numOutsA, len(commitment.txn.TxOut))
+		}
+		commitment = chanB.localCommitChain.tip()
+		if len(commitment.txn.TxOut) != numOutsB {
+			t.Fatalf("Incorrect # of outputs: expected %v, got %v",
+				numOutsB, len(commitment.txn.TxOut))
+		}
+
+		if chanA.channelState.TotalSatoshisSent != amountSentA {
+			t.Fatalf("alice satoshis sent incorrect: expected %v, "+
+				"got %v", amountSentA,
+				chanA.channelState.TotalSatoshisSent)
+		}
+		if chanA.channelState.TotalSatoshisReceived != amountSentB {
+			t.Fatalf("alice satoshis received incorrect: expected"+
+				"%v, got %v", amountSentB,
+				chanA.channelState.TotalSatoshisReceived)
+		}
+		if chanB.channelState.TotalSatoshisSent != amountSentB {
+			t.Fatalf("bob satoshis sent incorrect: expected %v, "+
+				" got %v", amountSentB,
+				chanB.channelState.TotalSatoshisSent)
+		}
+		if chanB.channelState.TotalSatoshisReceived != amountSentA {
+			t.Fatalf("bob satoshis received incorrect: expected "+
+				"%v, got %v", amountSentA,
+				chanB.channelState.TotalSatoshisReceived)
+		}
+	}
+
 	aliceDustLimit := aliceChannel.channelState.OurDustLimit
 	bobDustLimit := bobChannel.channelState.OurDustLimit
 	htlcAmount := btcutil.Amount(500)
@@ -1070,9 +1104,6 @@ func TestCheckDustLimit(t *testing.T) {
 			bobDustLimit)
 	}
 
-	aliceAmount := aliceChannel.channelState.OurBalance
-	bobAmount := bobChannel.channelState.OurBalance
-
 	htlc, preimage := createHTLC(0, htlcAmount)
 	if _, err := aliceChannel.AddHTLC(htlc); err != nil {
 		t.Fatalf("alice unable to add htlc: %v", err)
@@ -1090,34 +1121,8 @@ func TestCheckDustLimit(t *testing.T) {
 	// will be settled.
 
 	// From Alice point of view HTLC's amount is bigger than dust limit.
-	commitment := aliceChannel.localCommitChain.tip()
-	if len(commitment.txn.TxOut) != 3 {
-		t.Fatalf("incorrect number of outputs: expected %v, got %v",
-			3, len(commitment.txn.TxOut))
-	}
-	if commitment.ourBalance != aliceAmount-htlcAmount {
-		t.Fatalf("Alice has wrong balance: expected %v, got %v",
-			aliceAmount-htlcAmount, commitment.ourBalance)
-	}
-	if commitment.theirBalance != bobAmount {
-		t.Fatalf("Alice has wrong balance for Bob: expected %v, got %v",
-			bobAmount, commitment.theirBalance)
-	}
-
 	// From Bob point of view HTLC's amount is lower then dust limit.
-	commitment = bobChannel.localCommitChain.tip()
+	assertChannelState(aliceChannel, bobChannel, 3, 2, 0, 0)
-	if len(commitment.txn.TxOut) != 2 {
-		t.Fatalf("Incorrect number of outputs: expected %v, got %v",
-			2, len(commitment.txn.TxOut))
-	}
-	if commitment.theirBalance != aliceAmount-htlcAmount {
-		t.Fatalf("Bob has wrong balance for Alice: expected %v, got %v",
-			aliceAmount-htlcAmount, commitment.theirBalance)
-	}
-	if commitment.ourBalance != bobAmount {
-		t.Fatalf("Bob has wrong balance: expected %v, got %v",
-			bobAmount, commitment.ourBalance)
-	}
 
 	// Settle HTLC and sign new commitment.
 	settleIndex, err := bobChannel.SettleHTLC(preimage)
@@ -1132,41 +1137,15 @@ func TestCheckDustLimit(t *testing.T) {
 		t.Fatalf("state transition error: %v", err)
 	}
 
-	commitment = aliceChannel.localCommitChain.tip()
+	assertChannelState(aliceChannel, bobChannel, 2, 2, uint64(htlcAmount), 0)
-	if len(commitment.txn.TxOut) != 2 {
-		t.Fatalf("wrong number of outputs: expected %v, got %v",
-			2, len(commitment.txn.TxOut))
-	}
-	if commitment.ourBalance != aliceAmount-htlcAmount {
-		t.Fatalf("Alice has wrong balance: expected %v, got %v",
-			aliceAmount-htlcAmount, commitment.ourBalance)
-	}
-	if commitment.theirBalance != bobAmount+htlcAmount {
-		t.Fatalf("Alice has wrong balance for Bob: expected %v, got %v",
-			bobAmount, commitment.theirBalance)
-	}
-
-	commitment = bobChannel.localCommitChain.tip()
-	if len(commitment.txn.TxOut) != 2 {
-		t.Fatalf("wrong number of outputs: expected %v, got %v",
-			2, len(commitment.txn.TxOut))
-	}
-	if commitment.ourBalance != bobAmount+htlcAmount {
-		t.Fatalf("Bob has wrong balance: expected %v, got %v",
-			bobAmount+htlcAmount, commitment.ourBalance)
-	}
-	if commitment.theirBalance != aliceAmount-htlcAmount {
-		t.Fatalf("Bob has wrong balance for Alice: expected %v, got %v",
-			aliceAmount-htlcAmount, commitment.theirBalance)
-	}
 
 	// Next we will test when a non-HTLC output in the commitment
 	// transaction is below the dust limit.  We create an HTLC that will
 	// only leave a small enough amount to Alice such that Bob will
 	// consider it a dust output.
-	aliceAmount = aliceChannel.channelState.OurBalance
+
-	bobAmount = bobChannel.channelState.OurBalance
+	aliceAmount := btcutil.Amount(5e8)
-	htlcAmount2 := aliceAmount - htlcAmount
+	htlcAmount2 := aliceAmount - btcutil.Amount(1000)
 	htlc, preimage = createHTLC(0, htlcAmount2)
 	if _, err := aliceChannel.AddHTLC(htlc); err != nil {
 		t.Fatalf("alice unable to add htlc: %v", err)
@@ -1179,34 +1158,8 @@ func TestCheckDustLimit(t *testing.T) {
 	}
 
 	// From Alice's point of view, her output is bigger than the dust limit
-	commitment = aliceChannel.localCommitChain.tip()
-	if len(commitment.txn.TxOut) != 3 {
-		t.Fatalf("incorrect number of outputs in commitment transaction "+
-			"expected %v, got %v", 3, len(commitment.txn.TxOut))
-	}
-	if commitment.ourBalance != aliceAmount-htlcAmount2 {
-		t.Fatalf("Alice has wrong balance: expected %v, got %v",
-			aliceAmount-htlcAmount, commitment.ourBalance)
-	}
-	if commitment.theirBalance != bobAmount {
-		t.Fatalf("Alice has wrong balance for Bob: expected %v, got %v",
-			bobAmount, commitment.theirBalance)
-	}
-
 	// From Bob's point of view, Alice's output is lower than the dust limit
-	commitment = bobChannel.localCommitChain.tip()
+	assertChannelState(aliceChannel, bobChannel, 3, 2, uint64(htlcAmount), 0)
-	if len(commitment.txn.TxOut) != 2 {
-		t.Fatalf("incorrect number of outputs in commitment transaction "+
-			"expected %v, got %v", 2, len(commitment.txn.TxOut))
-	}
-	if commitment.theirBalance != aliceAmount-htlcAmount2 {
-		t.Fatalf("Bob has wrong balance for Alice: expected %v, got %v",
-			aliceAmount-htlcAmount, commitment.theirBalance)
-	}
-	if commitment.ourBalance != bobAmount {
-		t.Fatalf("Bob has wrong balance: expected %v, got %v",
-			bobAmount+htlcAmount, commitment.ourBalance)
-	}
 
 	// Settle HTLC and sign new commitment.
 	settleIndex, err = bobChannel.SettleHTLC(preimage)
@@ -1221,33 +1174,8 @@ func TestCheckDustLimit(t *testing.T) {
 		t.Fatalf("state transition error: %v", err)
 	}
 
-	commitment = aliceChannel.localCommitChain.tip()
+	assertChannelState(aliceChannel, bobChannel, 2, 1,
-	if len(commitment.txn.TxOut) != 2 {
+		uint64(htlcAmount+htlcAmount2), 0)
-		t.Fatalf("incorrect number of outputs in commitment transaction, "+
-			"expected %v got %v", 2, len(commitment.txn.TxOut))
-	}
-	if commitment.ourBalance != aliceAmount-htlcAmount2 {
-		t.Fatalf("Alice has wrong balance: expected %v, got %v",
-			aliceAmount-htlcAmount, commitment.ourBalance)
-	}
-	if commitment.theirBalance != bobAmount+htlcAmount2 {
-		t.Fatalf("Alice has wrong balance for Bob: expected %v, got %v",
-			bobAmount, commitment.theirBalance)
-	}
-
-	commitment = bobChannel.localCommitChain.tip()
-	if len(commitment.txn.TxOut) != 1 {
-		t.Fatalf("incorrect number of outputs in commitment transaction, "+
-			"expected %v got %v", 1, len(commitment.txn.TxOut))
-	}
-	if commitment.ourBalance != bobAmount+htlcAmount2 {
-		t.Fatalf("Bob has wrong balance: expected %v, got %v",
-			bobAmount+htlcAmount, commitment.ourBalance)
-	}
-	if commitment.theirBalance != aliceAmount-htlcAmount2 {
-		t.Fatalf("Bob has wrong balance for Alice: expected %v, got %v",
-			aliceAmount-htlcAmount, commitment.theirBalance)
-	}
 }
 
 func TestStateUpdatePersistence(t *testing.T) {
@@ -1267,9 +1195,6 @@ func TestStateUpdatePersistence(t *testing.T) {
 		t.Fatalf("unable to sync bob's channel: %v", err)
 	}
 
-	aliceStartingBalance := aliceChannel.channelState.OurBalance
-	bobStartingBalance := bobChannel.channelState.OurBalance
-
 	const numHtlcs = 4
 
 	// Alice adds 3 HTLCs to the update log, while Bob adds a single HTLC.
@@ -1281,7 +1206,7 @@ func TestStateUpdatePersistence(t *testing.T) {
 		rHash := sha256.Sum256(alicePreimage[:])
 		h := &lnwire.UpdateAddHTLC{
 			PaymentHash: rHash,
-			Amount:      btcutil.Amount(500),
+			Amount:      btcutil.Amount(5000),
 			Expiry:      uint32(10),
 		}
 
@@ -1295,7 +1220,7 @@ func TestStateUpdatePersistence(t *testing.T) {
 	rHash := sha256.Sum256(bobPreimage[:])
 	bobh := &lnwire.UpdateAddHTLC{
 		PaymentHash: rHash,
-		Amount:      btcutil.Amount(500),
+		Amount:      btcutil.Amount(5000),
 		Expiry:      uint32(10),
 	}
 	if _, err := bobChannel.AddHTLC(bobh); err != nil {
@@ -1319,20 +1244,6 @@ func TestStateUpdatePersistence(t *testing.T) {
 		t.Fatalf("unable to complete bob's state transition: %v", err)
 	}
 
-	// The balances of both channels should be updated accordingly.
-	aliceBalance := aliceChannel.channelState.OurBalance
-	expectedAliceBalance := aliceStartingBalance - btcutil.Amount(1500)
-	bobBalance := bobChannel.channelState.OurBalance
-	expectedBobBalance := bobStartingBalance - btcutil.Amount(500)
-	if aliceBalance != expectedAliceBalance {
-		t.Fatalf("expected %v alice balance, got %v", int64(expectedAliceBalance),
-			int64(aliceBalance))
-	}
-	if bobBalance != expectedBobBalance {
-		t.Fatalf("expected %v bob balance, got %v", int64(expectedBobBalance),
-			int64(bobBalance))
-	}
-
 	// The latest commitment from both sides should have all the HTLCs.
 	numAliceOutgoing := aliceChannel.localCommitChain.tail().outgoingHTLCs
 	numAliceIncoming := aliceChannel.localCommitChain.tail().incomingHTLCs
@@ -1419,7 +1330,7 @@ func TestStateUpdatePersistence(t *testing.T) {
 	if bobChannel.localUpdateLog.Len() !=
 		bobChannelNew.localUpdateLog.Len() {
 		t.Fatalf("bob log len: expected %v, got %v",
-			bobChannelNew.localUpdateLog.Len(),
+			bobChannel.localUpdateLog.Len(),
 			bobChannelNew.localUpdateLog.Len())
 	}
 	if bobChannel.remoteUpdateLog.Len() !=
@@ -1511,37 +1422,23 @@ func TestStateUpdatePersistence(t *testing.T) {
 		t.Fatalf("unable to update commitments: %v", err)
 	}
 
-	// The balances of both sides should have been updated accordingly.
-	aliceBalance = aliceChannelNew.channelState.OurBalance
-	expectedAliceBalance = aliceStartingBalance - btcutil.Amount(1000)
-	bobBalance = bobChannelNew.channelState.OurBalance
-	expectedBobBalance = bobStartingBalance + btcutil.Amount(1000)
-	if aliceBalance != expectedAliceBalance {
-		t.Fatalf("expected %v alice balance, got %v", expectedAliceBalance,
-			aliceBalance)
-	}
-	if bobBalance != expectedBobBalance {
-		t.Fatalf("expected %v bob balance, got %v", expectedBobBalance,
-			bobBalance)
-	}
-
 	// The amounts transferred should been updated as per the amounts in
 	// the HTLCs
-	if aliceChannelNew.channelState.TotalSatoshisSent != 1500 {
+	if aliceChannelNew.channelState.TotalSatoshisSent != 15000 {
 		t.Fatalf("expected %v alice satoshis sent, got %v",
-			3000, aliceChannelNew.channelState.TotalSatoshisSent)
+			15000, aliceChannelNew.channelState.TotalSatoshisSent)
 	}
-	if aliceChannelNew.channelState.TotalSatoshisReceived != 500 {
+	if aliceChannelNew.channelState.TotalSatoshisReceived != 5000 {
 		t.Fatalf("expected %v alice satoshis received, got %v",
-			1000, aliceChannelNew.channelState.TotalSatoshisReceived)
+			5000, aliceChannelNew.channelState.TotalSatoshisReceived)
 	}
-	if bobChannelNew.channelState.TotalSatoshisSent != 500 {
+	if bobChannelNew.channelState.TotalSatoshisSent != 5000 {
 		t.Fatalf("expected %v bob satoshis sent, got %v",
-			1000, bobChannel.channelState.TotalSatoshisSent)
+			5000, bobChannel.channelState.TotalSatoshisSent)
 	}
-	if bobChannelNew.channelState.TotalSatoshisReceived != 1500 {
+	if bobChannelNew.channelState.TotalSatoshisReceived != 15000 {
 		t.Fatalf("expected %v bob satoshis sent, got %v",
-			3000, bobChannel.channelState.TotalSatoshisSent)
+			15000, bobChannel.channelState.TotalSatoshisSent)
 	}
 }
 
@@ -1580,7 +1477,8 @@ func TestCancelHTLC(t *testing.T) {
 
 	// With the HTLC committed, Alice's balance should reflect the clearing
 	// of the new HTLC.
-	aliceExpectedBalance := btcutil.Amount(btcutil.SatoshiPerBitcoin * 4)
+	aliceExpectedBalance := btcutil.Amount(btcutil.SatoshiPerBitcoin*4) -
+		calcStaticFee(1)
 	if aliceChannel.channelState.OurBalance != aliceExpectedBalance {
 		t.Fatalf("Alice's balance is wrong: expected %v, got %v",
 			aliceExpectedBalance, aliceChannel.channelState.OurBalance)
@@ -1622,9 +1520,10 @@ func TestCancelHTLC(t *testing.T) {
 	}
 
 	expectedBalance := btcutil.Amount(btcutil.SatoshiPerBitcoin * 5)
-	if aliceChannel.channelState.OurBalance != expectedBalance {
+	if aliceChannel.channelState.OurBalance != expectedBalance-calcStaticFee(0) {
 		t.Fatalf("balance is wrong: expected %v, got %v",
-			aliceChannel.channelState.OurBalance, expectedBalance)
+			aliceChannel.channelState.OurBalance, expectedBalance-
+				calcStaticFee(0))
 	}
 	if aliceChannel.channelState.TheirBalance != expectedBalance {
 		t.Fatalf("balance is wrong: expected %v, got %v",
@@ -1634,9 +1533,10 @@ func TestCancelHTLC(t *testing.T) {
 		t.Fatalf("balance is wrong: expected %v, got %v",
 			bobChannel.channelState.OurBalance, expectedBalance)
 	}
-	if bobChannel.channelState.TheirBalance != expectedBalance {
+	if bobChannel.channelState.TheirBalance != expectedBalance-calcStaticFee(0) {
 		t.Fatalf("balance is wrong: expected %v, got %v",
-			bobChannel.channelState.TheirBalance, expectedBalance)
+			bobChannel.channelState.TheirBalance,
+			expectedBalance-calcStaticFee(0))
 	}
 }
 
@@ -1699,7 +1599,7 @@ func TestCooperativeCloseDustAdherence(t *testing.T) {
 	// Next we'll modify the current balances and dust limits such that
 	// Bob's current balance is above _below_ his dust limit.
 	aliceBal := btcutil.Amount(btcutil.SatoshiPerBitcoin)
-	bobBal := btcutil.Amount(500)
+	bobBal := btcutil.Amount(250)
 	setBalances(aliceBal, bobBal)
 
 	// Attempt another cooperative channel closure. It should succeed
@@ -1716,14 +1616,13 @@ func TestCooperativeCloseDustAdherence(t *testing.T) {
 
 	// The closure transaction should only have a single output, and that
 	// output should be Alice's balance.
-	// TODO(roasbeef): remove -5000 after fees are no longer hard coded
 	if len(closeTx.TxOut) != 1 {
 		t.Fatalf("close tx has wrong number of outputs: expected %v "+
 			"got %v", 1, len(closeTx.TxOut))
 	}
-	if closeTx.TxOut[0].Value != int64(aliceBal-5000) {
+	if closeTx.TxOut[0].Value != int64(aliceBal-calcStaticFee(0)) {
 		t.Fatalf("alice's balance is incorrect: expected %v, got %v",
-			aliceBal-5000, closeTx.TxOut[0].Value)
+			aliceBal-calcStaticFee(0), closeTx.TxOut[0].Value)
 	}
 
 	// Finally, we'll modify the current balances and dust limits such that
@@ -1745,7 +1644,6 @@ func TestCooperativeCloseDustAdherence(t *testing.T) {
 
 	// The closure transaction should only have a single output, and that
 	// output should be Bob's balance.
-	// TODO(roasbeef): remove -5000 after fees are no longer hard coded
 	if len(closeTx.TxOut) != 1 {
 		t.Fatalf("close tx has wrong number of outputs: expected %v "+
 			"got %v", 1, len(closeTx.TxOut))

lnwallet/interface_test.go

@@ -101,6 +101,20 @@ func assertReservationDeleted(res *lnwallet.ChannelReservation, t *testing.T) {
 	}
 }
 
+// calcStaticFee calculates appropriate fees for commitment transactions.  This
+// function provides a simple way to allow test balance assertions to take fee
+// calculations into account.
+// TODO(bvu): Refactor when dynamic fee estimation is added.
+func calcStaticFee(numHTLCs int) btcutil.Amount {
+	const (
+		commitWeight = btcutil.Amount(724)
+		htlcWeight   = 172
+		feePerByte   = btcutil.Amount(250 * 4)
+	)
+	return feePerByte * (commitWeight +
+		btcutil.Amount(htlcWeight*numHTLCs)) / 1000
+}
+
 // bobNode represents the other party involved as a node within LN. Bob is our
 // only "default-route", we have a direct connection with him.
 type bobNode struct {
@@ -454,7 +468,7 @@ func testDualFundingReservationWorkflow(miner *rpctest.Harness, wallet *lnwallet
 
 	// TODO(roasbeef): account for current hard-coded commit fee,
 	// need to remove bob all together
-	chanCapacity := int64(10e8 + 5000)
+	chanCapacity := int64(10e8)
 	// Alice responds with her output, change addr, multi-sig key and signatures.
 	// Bob then responds with his signatures.
 	bobsSigs, err := bobNode.signFundingTx(fundingTx)
@@ -572,9 +586,10 @@ func testCancelNonExistantReservation(miner *rpctest.Harness,
 
 	t.Log("Running cancel reservation tests")
 
+	feeRate := btcutil.Amount(wallet.FeeEstimator.EstimateFeePerWeight(1))
 	// Create our own reservation, give it some ID.
-	res := lnwallet.NewChannelReservation(1000, 1000, 5000, wallet, 22,
+	res := lnwallet.NewChannelReservation(1000, 1000, feeRate, wallet,
-		numReqConfs, 10)
+		22, numReqConfs, 10)
 
 	// Attempt to cancel this reservation. This should fail, we know
 	// nothing of it.
@@ -691,7 +706,7 @@ func testSingleFunderReservationWorkflowInitiator(miner *rpctest.Harness,
 		chanReservation.FundingRedeemScript(),
 		// TODO(roasbeef): account for current hard-coded fee, need to
 		// remove bobNode entirely
-		int64(fundingAmt)+5000)
+		int64(fundingAmt))
 	if err != nil {
 		t.Fatalf("bob is unable to sign alice's commit tx: %v", err)
 	}
@@ -781,6 +796,7 @@ func testSingleFunderReservationWorkflowResponder(miner *rpctest.Harness,
 	// include any inputs or change addresses, as only Bob will construct
 	// the funding transaction.
 	bobContribution := bobNode.Contribution(ourContribution.CommitKey)
+	bobContribution.DustLimit = lnwallet.DefaultDustLimit()
 	if err := chanReservation.ProcessSingleContribution(bobContribution); err != nil {
 		t.Fatalf("unable to process bob's contribution: %v", err)
 	}
@@ -815,7 +831,7 @@ func testSingleFunderReservationWorkflowResponder(miner *rpctest.Harness,
 		ourContribution.MultiSigKey.SerializeCompressed(),
 		bobContribution.MultiSigKey.SerializeCompressed(),
 		// TODO(roasbeef): account for hard-coded fee, remove bob node
-		int64(capacity)+5000)
+		int64(capacity))
 	if err != nil {
 		t.Fatalf("unable to generate multi-sig output: %v", err)
 	}
@@ -843,10 +859,11 @@ func testSingleFunderReservationWorkflowResponder(miner *rpctest.Harness,
 	// Next, manually create Alice's commitment transaction, signing the
 	// fully sorted and state hinted transaction.
 	fundingTxIn := wire.NewTxIn(fundingOutpoint, nil, nil)
+
 	aliceCommitTx, err := lnwallet.CreateCommitTx(fundingTxIn,
 		ourContribution.CommitKey, bobContribution.CommitKey,
 		ourContribution.RevocationKey, ourContribution.CsvDelay, 0,
-		capacity, lnwallet.DefaultDustLimit())
+		capacity-calcStaticFee(0), lnwallet.DefaultDustLimit())
 	if err != nil {
 		t.Fatalf("unable to create alice's commit tx: %v", err)
 	}
@@ -857,7 +874,7 @@ func testSingleFunderReservationWorkflowResponder(miner *rpctest.Harness,
 	}
 	bobCommitSig, err := bobNode.signCommitTx(aliceCommitTx,
 		// TODO(roasbeef): account for hard-coded fee, remove bob node
-		fundingRedeemScript, int64(capacity)+5000)
+		fundingRedeemScript, int64(capacity))
 	if err != nil {
 		t.Fatalf("unable to sign alice's commit tx: %v", err)
 	}

lnwallet/reservation.go

@@ -155,6 +155,8 @@ func NewChannelReservation(capacity, fundingAmt btcutil.Amount, minFeeRate btcut
 		initiator    bool
 	)
 
+	commitFee := minFeeRate * commitWeight / 1000
+
 	// If we're the responder to a single-funder reservation, then we have
 	// no initial balance in the channel unless the remote party is pushing
 	// some funds to us within the first commitment state.
@@ -166,20 +168,22 @@ func NewChannelReservation(capacity, fundingAmt btcutil.Amount, minFeeRate btcut
 		// TODO(roasbeef): need to rework fee structure in general and
 		// also when we "unlock" dual funder within the daemon
 
-		if capacity == fundingAmt+commitFee {
+		if capacity == fundingAmt {
 			// If we're initiating a single funder workflow, then
 			// we pay all the initial fees within the commitment
 			// transaction. We also deduct our balance by the
 			// amount pushed as part of the initial state.
 			ourBalance = capacity - commitFee - pushSat
+			theirBalance = capacity - fundingAmt + pushSat
 		} else {
 			// Otherwise, this is a dual funder workflow where both
 			// slides split the amount funded and the commitment
 			// fee.
-			ourBalance = fundingAmt - commitFee
+			ourBalance = fundingAmt - (commitFee / 2)
+			theirBalance = capacity - fundingAmt -
+				(commitFee / 2) + pushSat
 		}
 
-		theirBalance = capacity - fundingAmt - commitFee + pushSat
 		initiator = true
 	}
 

lnwallet/wallet.go

@@ -37,7 +37,8 @@ const (
 	// rotations, etc.
 	identityKeyIndex = hdkeychain.HardenedKeyStart + 2
 
-	commitFee = 5000
+	commitWeight = btcutil.Amount(724)
+	htlcWeight   = 172
 )
 
 var (
@@ -526,8 +527,7 @@ func (l *LightningWallet) handleFundingReserveRequest(req *initFundingReserveMsg
 	}
 
 	id := atomic.AddUint64(&l.nextFundingID, 1)
-	totalCapacity := req.capacity + commitFee
+	reservation := NewChannelReservation(req.capacity, req.fundingAmount,
-	reservation := NewChannelReservation(totalCapacity, req.fundingAmount,
 		req.minFeeRate, l, id, req.numConfs, req.pushSat)
 
 	// Grab the mutex on the ChannelReservation to ensure thread-safety
@@ -551,9 +551,8 @@ func (l *LightningWallet) handleFundingReserveRequest(req *initFundingReserveMsg
 	if req.fundingAmount != 0 {
 		// TODO(roasbeef): consult model for proper fee rate on funding
 		// tx
-		feeRate := uint64(10)
+		err := l.selectCoinsAndChange(uint64(req.minFeeRate),
-		amt := req.fundingAmount + commitFee
+			req.fundingAmount, ourContribution)
-		err := l.selectCoinsAndChange(feeRate, amt, ourContribution)
 		if err != nil {
 			req.err <- err
 			req.resp <- nil

routing/notifications_test.go

@@ -367,7 +367,7 @@ func TestEdgeUpdateNotification(t *testing.T) {
 		}
 		// TODO(roasbeef): this is a hack, needs to be removed
 		// after commitment fees are dynamic.
-		if edgeUpdate.Capacity != chanValue-5000 {
+		if edgeUpdate.Capacity != chanValue {
 			t.Fatalf("capacity of edge doesn't match: "+
 				"expected %v, got %v", chanValue, edgeUpdate.Capacity)
 		}
@@ -682,7 +682,7 @@ func TestChannelCloseNotification(t *testing.T) {
 		}
 		// TODO(roasbeef): this is a hack, needs to be removed
 		// after commitment fees are dynamic.
-		if closedChan.Capacity != chanValue-5000 {
+		if closedChan.Capacity != chanValue {
 			t.Fatalf("capacity of closed channel doesn't match: "+
 				"expected %v, got %v", chanValue, closedChan.Capacity)
 		}

routing/router.go

@@ -622,7 +622,7 @@ func (r *ChannelRouter) processUpdate(msg interface{}) error {
 
 		// TODO(roasbeef): this is a hack, needs to be removed
 		// after commitment fees are dynamic.
-		msg.Capacity = btcutil.Amount(chanUtxo.Value) - btcutil.Amount(5000)
+		msg.Capacity = btcutil.Amount(chanUtxo.Value)
 		msg.ChannelPoint = *fundingPoint
 		if err := r.cfg.Graph.AddChannelEdge(msg); err != nil {
 			return errors.Errorf("unable to add edge: %v", err)