diff --git a/lnd_test.go b/lnd_test.go index 27da0729..8c0fc59c 100644 --- a/lnd_test.go +++ b/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) } } diff --git a/lnwallet/channel.go b/lnwallet/channel.go index ad433a24..22fe00b7 100644 --- a/lnwallet/channel.go +++ b/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, - error) { + fe FeeEstimator, state *channeldb.OpenChannel) (*LightningChannel, 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 { diff --git a/lnwallet/channel_test.go b/lnwallet/channel_test.go index 1a5d0c6e..fa4dc075 100644 --- a/lnwallet/channel_test.go +++ b/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 - // commitment height updated within their local channel state. - aliceBalance := btcutil.Amount(4 * 1e8) - bobBalance := btcutil.Amount(5 * 1e8) - if aliceChannel.channelState.OurBalance != aliceBalance { - t.Fatalf("alice has incorrect local balance %v vs %v", - aliceChannel.channelState.OurBalance, aliceBalance) + // At this point, both sides should have the proper number of satoshis + // sent, and commitment height updated within their local channel + // state. + aliceSent := uint64(0) + bobSent := uint64(0) + + if aliceChannel.channelState.TotalSatoshisSent != aliceSent { + t.Fatalf("alice has incorrect satoshis sent: %v vs %v", + aliceChannel.channelState.TotalSatoshisSent, aliceSent) } - if aliceChannel.channelState.TheirBalance != bobBalance { - t.Fatalf("alice has incorrect remote balance %v vs %v", - aliceChannel.channelState.TheirBalance, bobBalance) + if aliceChannel.channelState.TotalSatoshisReceived != bobSent { + t.Fatalf("alice has incorrect satoshis received %v vs %v", + aliceChannel.channelState.TotalSatoshisReceived, bobSent) } - if bobChannel.channelState.OurBalance != bobBalance { - t.Fatalf("bob has incorrect local balance %v vs %v", - bobChannel.channelState.OurBalance, bobBalance) + if bobChannel.channelState.TotalSatoshisSent != bobSent { + t.Fatalf("bob has incorrect satoshis sent %v vs %v", + bobChannel.channelState.TotalSatoshisSent, bobSent) } - if bobChannel.channelState.TheirBalance != aliceBalance { - t.Fatalf("bob has incorrect remote balance %v vs %v", - bobChannel.channelState.TheirBalance, aliceBalance) + if bobChannel.channelState.TotalSatoshisReceived != aliceSent { + t.Fatalf("bob has incorrect satoshis received %v vs %v", + 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() - 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) - } + assertChannelState(aliceChannel, bobChannel, 3, 2, 0, 0) // 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() - 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) - } + assertChannelState(aliceChannel, bobChannel, 2, 2, uint64(htlcAmount), 0) // 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 - htlcAmount2 := aliceAmount - htlcAmount + + aliceAmount := btcutil.Amount(5e8) + 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() - 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) - } + assertChannelState(aliceChannel, bobChannel, 3, 2, uint64(htlcAmount), 0) // 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() - 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.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) - } + assertChannelState(aliceChannel, bobChannel, 2, 1, + uint64(htlcAmount+htlcAmount2), 0) } 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)) diff --git a/lnwallet/interface_test.go b/lnwallet/interface_test.go index f6bfe7eb..e6ff1a3f 100644 --- a/lnwallet/interface_test.go +++ b/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, - numReqConfs, 10) + res := lnwallet.NewChannelReservation(1000, 1000, feeRate, wallet, + 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) } diff --git a/lnwallet/reservation.go b/lnwallet/reservation.go index 74c13c8e..9968c3e0 100644 --- a/lnwallet/reservation.go +++ b/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 } diff --git a/lnwallet/wallet.go b/lnwallet/wallet.go index e415e6de..455b9d28 100644 --- a/lnwallet/wallet.go +++ b/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(totalCapacity, req.fundingAmount, + reservation := NewChannelReservation(req.capacity, 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) - amt := req.fundingAmount + commitFee - err := l.selectCoinsAndChange(feeRate, amt, ourContribution) + err := l.selectCoinsAndChange(uint64(req.minFeeRate), + req.fundingAmount, ourContribution) if err != nil { req.err <- err req.resp <- nil diff --git a/routing/notifications_test.go b/routing/notifications_test.go index 21ac129a..cbbf238a 100644 --- a/routing/notifications_test.go +++ b/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) } diff --git a/routing/router.go b/routing/router.go index 8b88d3fc..d24e4882 100644 --- a/routing/router.go +++ b/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)