From 52b56b8cf295ba1af953b5826ac6c4c419daab9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christopher=20J=C3=A4mthagen?= Date: Mon, 23 Jan 2017 14:56:00 +0100 Subject: [PATCH] lnwallet+test: no dust outputs in commitment transaction + tests Currently non-HTLC outputs will be accepted in the commitment transaction as long as it is non-zero. We change this by not allowing outputs with a value lower than the dust limit. The value of such an output will go towards transaction fees. --- lnwallet/channel.go | 10 ++--- lnwallet/channel_test.go | 85 ++++++++++++++++++++++++++++++++++- lnwallet/interface_test.go | 2 +- lnwallet/script_utils_test.go | 2 +- lnwallet/wallet.go | 9 ++-- 5 files changed, 95 insertions(+), 13 deletions(-) diff --git a/lnwallet/channel.go b/lnwallet/channel.go index 64e128d9..11382342 100644 --- a/lnwallet/channel.go +++ b/lnwallet/channel.go @@ -1095,7 +1095,7 @@ func (lc *LightningChannel) fetchCommitmentView(remoteChain bool, // unsettled/un-timed out HTLCs. ourCommitTx := !remoteChain commitTx, err := CreateCommitTx(lc.fundingTxIn, selfKey, remoteKey, - revocationKey, delay, delayBalance, p2wkhBalance) + revocationKey, delay, delayBalance, p2wkhBalance, dustLimit) if err != nil { return nil, err } @@ -2361,7 +2361,7 @@ func (lc *LightningChannel) StateSnapshot() *channeldb.ChannelSnapshot { // counterparty within the channel, which can be spent immediately. func CreateCommitTx(fundingOutput *wire.TxIn, selfKey, theirKey *btcec.PublicKey, revokeKey *btcec.PublicKey, csvTimeout uint32, amountToSelf, - amountToThem btcutil.Amount) (*wire.MsgTx, error) { + amountToThem, dustLimit btcutil.Amount) (*wire.MsgTx, error) { // First, we create the script for the delayed "pay-to-self" output. // This output has 2 main redemption clauses: either we can redeem the @@ -2391,11 +2391,11 @@ func CreateCommitTx(fundingOutput *wire.TxIn, selfKey, theirKey *btcec.PublicKey commitTx := wire.NewMsgTx(2) commitTx.AddTxIn(fundingOutput) - // Avoid creating zero value outputs within the commitment transaction. - if amountToSelf != 0 { + // Avoid creating dust outputs within the commitment transaction. + if amountToSelf >= dustLimit { commitTx.AddTxOut(wire.NewTxOut(int64(amountToSelf), payToUsScriptHash)) } - if amountToThem != 0 { + if amountToThem >= dustLimit { commitTx.AddTxOut(wire.NewTxOut(int64(amountToThem), theirWitnessKeyHash)) } diff --git a/lnwallet/channel_test.go b/lnwallet/channel_test.go index 1684f5b0..3e439c90 100644 --- a/lnwallet/channel_test.go +++ b/lnwallet/channel_test.go @@ -219,12 +219,12 @@ func createTestChannels(revocationWindow int) (*LightningChannel, *LightningChan aliceRevokeKey := DeriveRevocationPubkey(bobKeyPub, aliceFirstRevoke[:]) aliceCommitTx, err := CreateCommitTx(fundingTxIn, aliceKeyPub, - bobKeyPub, aliceRevokeKey, csvTimeoutAlice, channelBal, channelBal) + bobKeyPub, aliceRevokeKey, csvTimeoutAlice, channelBal, channelBal, aliceDustLimit) if err != nil { return nil, nil, nil, err } bobCommitTx, err := CreateCommitTx(fundingTxIn, bobKeyPub, - aliceKeyPub, bobRevokeKey, csvTimeoutBob, channelBal, channelBal) + aliceKeyPub, bobRevokeKey, csvTimeoutBob, channelBal, channelBal, bobDustLimit) if err != nil { return nil, nil, nil, err } @@ -956,6 +956,87 @@ func TestCheckDustLimit(t *testing.T) { if commitment.theirBalance != aliceAmount-htlcAmount { t.Fatal("their balance wasn't updated") } + + // 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 + htlc, preimage = createHTLC(0, htlcAmount2) + if _, err := aliceChannel.AddHTLC(htlc); err != nil { + t.Fatalf("alice unable to add htlc: %v", err) + } + if _, err := bobChannel.ReceiveHTLC(htlc); err != nil { + t.Fatalf("bob unable to receive htlc: %v", err) + } + if err := forceStateTransition(aliceChannel, bobChannel); err != nil { + t.Fatalf("Can't update the channel state: %v", err) + } + + // From Alices' point of view, her output is bigger than the dust limit + commitment = aliceChannel.localCommitChain.tip() + if len(commitment.txn.TxOut) != 3 { + t.Fatal("incorrect number of outputs in commitment transaction "+ + "expected %v, got %v", 3, commitment.txn.TxOut) + } + if commitment.ourBalance != aliceAmount-htlcAmount2 { + t.Fatal("our balance wasn't updated") + } + if commitment.theirBalance != bobAmount { + t.Fatal("their balance was updated") + } + + // From Bobs' point of view, Alice's output is lower than the dust limit + commitment = bobChannel.localCommitChain.tip() + if len(commitment.txn.TxOut) != 2 { + t.Fatal("incorrect number of outputs in commitment transaction "+ + "expected %v, got %v", 2, commitment.txn.TxOut) + } + if commitment.theirBalance != aliceAmount-htlcAmount2 { + t.Fatal("their balance wasn't updated") + } + if commitment.ourBalance != bobAmount { + t.Fatal("our balance was updated") + } + + // Settle HTLC and sign new commitment. + settleIndex, err = bobChannel.SettleHTLC(preimage) + if err != nil { + t.Fatalf("bob unable to settle inbound htlc: %v", err) + } + err = aliceChannel.ReceiveHTLCSettle(preimage, settleIndex) + if err != nil { + t.Fatalf("alice unable to accept settle of outbound htlc: %v", err) + } + if err := forceStateTransition(bobChannel, aliceChannel); err != nil { + t.Fatalf("Can't update the channel state: %v", err) + } + + commitment = aliceChannel.localCommitChain.tip() + if len(commitment.txn.TxOut) != 2 { + t.Fatal("incorrect number of outputs in commitment transaction, "+ + "expected %v got %v", 2, len(commitment.txn.TxOut)) + } + if commitment.ourBalance != aliceAmount-htlcAmount2 { + t.Fatal("our balance wasn't updated") + } + if commitment.theirBalance != bobAmount+htlcAmount2 { + t.Fatal("their balance wasn't updated") + } + + commitment = bobChannel.localCommitChain.tip() + if len(commitment.txn.TxOut) != 1 { + t.Fatal("incorrect number of outputs in commitment transaction, "+ + "expected %v got %v", 1, len(commitment.txn.TxOut)) + } + if commitment.ourBalance != bobAmount+htlcAmount2 { + t.Fatal("our balance wasn't updated") + } + if commitment.theirBalance != aliceAmount-htlcAmount2 { + t.Fatal("their balance wasn't updated") + } + } func TestStateUpdatePersistence(t *testing.T) { diff --git a/lnwallet/interface_test.go b/lnwallet/interface_test.go index bb443736..b17e1715 100644 --- a/lnwallet/interface_test.go +++ b/lnwallet/interface_test.go @@ -843,7 +843,7 @@ func testSingleFunderReservationWorkflowResponder(miner *rpctest.Harness, aliceCommitTx, err := lnwallet.CreateCommitTx(fundingTxIn, ourContribution.CommitKey, bobContribution.CommitKey, ourContribution.RevocationKey, ourContribution.CsvDelay, 0, - capacity) + capacity, 540) if err != nil { t.Fatalf("unable to create alice's commit tx: %v", err) } diff --git a/lnwallet/script_utils_test.go b/lnwallet/script_utils_test.go index fabdcfc0..b9832297 100644 --- a/lnwallet/script_utils_test.go +++ b/lnwallet/script_utils_test.go @@ -54,7 +54,7 @@ func TestCommitmentSpendValidation(t *testing.T) { // of 5 blocks before sweeping the output, while bob can spend // immediately with either the revocation key, or his regular key. commitmentTx, err := CreateCommitTx(fakeFundingTxIn, aliceKeyPub, - bobKeyPub, revokePubKey, csvTimeout, channelBalance, channelBalance) + bobKeyPub, revokePubKey, csvTimeout, channelBalance, channelBalance, 540) if err != nil { t.Fatalf("unable to create commitment transaction: %v", nil) } diff --git a/lnwallet/wallet.go b/lnwallet/wallet.go index dfac64d2..992b8c35 100644 --- a/lnwallet/wallet.go +++ b/lnwallet/wallet.go @@ -783,14 +783,14 @@ func (l *LightningWallet) handleContributionMsg(req *addContributionMsg) { ourCommitKey := ourContribution.CommitKey ourCommitTx, err := CreateCommitTx(fundingTxIn, ourCommitKey, theirCommitKey, ourRevokeKey, ourContribution.CsvDelay, - ourBalance, theirBalance) + ourBalance, theirBalance, pendingReservation.partialState.OurDustLimit) if err != nil { req.err <- err return } theirCommitTx, err := CreateCommitTx(fundingTxIn, theirCommitKey, ourCommitKey, theirContribution.RevocationKey, theirContribution.CsvDelay, - theirBalance, ourBalance) + theirBalance, ourBalance, pendingReservation.partialState.TheirDustLimit) if err != nil { req.err <- err return @@ -1100,14 +1100,15 @@ func (l *LightningWallet) handleSingleFunderSigs(req *addSingleFunderSigsMsg) { theirBalance := pendingReservation.partialState.TheirBalance ourCommitTx, err := CreateCommitTx(fundingTxIn, ourCommitKey, theirCommitKey, pendingReservation.ourContribution.RevocationKey, - pendingReservation.ourContribution.CsvDelay, ourBalance, theirBalance) + pendingReservation.ourContribution.CsvDelay, ourBalance, theirBalance, + pendingReservation.partialState.OurDustLimit) if err != nil { req.err <- err return } theirCommitTx, err := CreateCommitTx(fundingTxIn, theirCommitKey, ourCommitKey, req.revokeKey, pendingReservation.theirContribution.CsvDelay, - theirBalance, ourBalance) + theirBalance, ourBalance, pendingReservation.partialState.TheirDustLimit) if err != nil { req.err <- err return