diff --git a/lnwallet/channel.go b/lnwallet/channel.go index d39dd84b..0c0f854f 100644 --- a/lnwallet/channel.go +++ b/lnwallet/channel.go @@ -2167,33 +2167,41 @@ func (lc *LightningChannel) fetchCommitmentView(remoteChain bool, keyRing *CommitmentKeyRing) (*commitment, error) { commitChain := lc.localCommitChain + dustLimit := lc.channelState.LocalChanCfg.DustLimit if remoteChain { commitChain = lc.remoteCommitChain + dustLimit = lc.channelState.RemoteChanCfg.DustLimit } 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. + // the balances on the commitment transaction accordingly. Note that + // these balances will be *before* taking a commitment fee from the + // initiator. htlcView := lc.fetchHTLCView(theirLogIndex, ourLogIndex) ourBalance, theirBalance, _, filteredHTLCView := lc.computeView( htlcView, remoteChain, true, ) feePerKw := filteredHTLCView.feePerKw - // 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.RemoteChanCfg.DustLimit - } else { - dustLimit = lc.channelState.LocalChanCfg.DustLimit + // Actually generate unsigned commitment transaction for this view. + commitTx, err := lc.commitBuilder.createUnsignedCommitmentTx( + ourBalance, theirBalance, !remoteChain, feePerKw, nextHeight, + filteredHTLCView, keyRing, + ) + if err != nil { + return nil, err } + // With the commitment view created, store the resulting balances and + // transaction with the other parameters for this height. c := &commitment{ - ourBalance: ourBalance, - theirBalance: theirBalance, + ourBalance: commitTx.ourBalance, + theirBalance: commitTx.theirBalance, + txn: commitTx.txn, + fee: commitTx.fee, ourMessageIndex: ourLogIndex, ourHtlcIndex: ourHtlcIndex, theirMessageIndex: theirLogIndex, @@ -2204,14 +2212,6 @@ func (lc *LightningChannel) fetchCommitmentView(remoteChain bool, isOurs: !remoteChain, } - // Actually generate unsigned commitment transaction for this view. - err := lc.commitBuilder.createCommitmentTx( - c, filteredHTLCView, keyRing, - ) - if err != nil { - return nil, err - } - // In order to ensure _none_ of the HTLC's associated with this new // commitment are mutated, we'll manually copy over each HTLC to its // respective slice. diff --git a/lnwallet/commitment.go b/lnwallet/commitment.go index e163fae0..c3abe2e1 100644 --- a/lnwallet/commitment.go +++ b/lnwallet/commitment.go @@ -9,6 +9,7 @@ import ( "github.com/btcsuite/btcutil" "github.com/lightningnetwork/lnd/channeldb" "github.com/lightningnetwork/lnd/input" + "github.com/lightningnetwork/lnd/lnwallet/chainfee" "github.com/lightningnetwork/lnd/lnwire" ) @@ -166,18 +167,41 @@ func createStateHintObfuscator(state *channeldb.OpenChannel) [StateHintSize]byte ) } -// createCommitmentTx generates the unsigned commitment transaction for a -// commitment view and assigns to txn field. -func (cb *CommitmentBuilder) createCommitmentTx(c *commitment, - filteredHTLCView *htlcView, keyRing *CommitmentKeyRing) error { +// unsignedCommitmentTx is the final commitment created from evaluating an HTLC +// view at a given height, along with some meta data. +type unsignedCommitmentTx struct { + // txn is the final, unsigned commitment transaction for this view. + txn *wire.MsgTx - ourBalance := c.ourBalance - theirBalance := c.theirBalance + // fee is the total fee of the commitment transaction. + fee btcutil.Amount + + // ourBalance|theirBalance is the balances of this commitment. This can + // be different than the balances before creating the commitment + // transaction as one party must pay the commitment fee. + ourBalance lnwire.MilliSatoshi + theirBalance lnwire.MilliSatoshi +} + +// createUnsignedCommitmentTx generates the unsigned commitment transaction for +// a commitment view and returns it as part of the unsignedCommitmentTx. The +// passed in balances should be balances *before* subtracting any commitment +// fees. +func (cb *CommitmentBuilder) createUnsignedCommitmentTx(ourBalance, + theirBalance lnwire.MilliSatoshi, isOurs bool, + feePerKw chainfee.SatPerKWeight, height uint64, + filteredHTLCView *htlcView, + keyRing *CommitmentKeyRing) (*unsignedCommitmentTx, error) { + + dustLimit := cb.chanState.LocalChanCfg.DustLimit + if !isOurs { + dustLimit = cb.chanState.RemoteChanCfg.DustLimit + } numHTLCs := int64(0) for _, htlc := range filteredHTLCView.ourUpdates { - if htlcIsDust(false, c.isOurs, c.feePerKw, - htlc.Amount.ToSatoshis(), c.dustLimit) { + if htlcIsDust(false, isOurs, feePerKw, + htlc.Amount.ToSatoshis(), dustLimit) { continue } @@ -185,8 +209,8 @@ func (cb *CommitmentBuilder) createCommitmentTx(c *commitment, numHTLCs++ } for _, htlc := range filteredHTLCView.theirUpdates { - if htlcIsDust(true, c.isOurs, c.feePerKw, - htlc.Amount.ToSatoshis(), c.dustLimit) { + if htlcIsDust(true, isOurs, feePerKw, + htlc.Amount.ToSatoshis(), dustLimit) { continue } @@ -202,7 +226,7 @@ func (cb *CommitmentBuilder) createCommitmentTx(c *commitment, // With the weight known, we can now calculate the commitment fee, // ensuring that we account for any dust outputs trimmed above. - commitFee := c.feePerKw.FeeForWeight(totalCommitWeight) + commitFee := feePerKw.FeeForWeight(totalCommitWeight) commitFeeMSat := lnwire.NewMSatFromSatoshis(commitFee) // Currently, within the protocol, the initiator always pays the fees. @@ -232,7 +256,7 @@ func (cb *CommitmentBuilder) createCommitmentTx(c *commitment, // CreateCommitTx with parameters matching the perspective, to generate // a new commitment transaction with all the latest unsettled/un-timed // out HTLCs. - if c.isOurs { + if isOurs { commitTx, err = CreateCommitTx( fundingTxIn(cb.chanState), keyRing, &cb.chanState.LocalChanCfg, &cb.chanState.RemoteChanCfg, ourBalance.ToSatoshis(), @@ -246,7 +270,7 @@ func (cb *CommitmentBuilder) createCommitmentTx(c *commitment, ) } if err != nil { - return err + return nil, err } // We'll now add all the HTLC outputs to the commitment transaction. @@ -260,26 +284,26 @@ func (cb *CommitmentBuilder) createCommitmentTx(c *commitment, // purposes of sorting. cltvs := make([]uint32, len(commitTx.TxOut)) for _, htlc := range filteredHTLCView.ourUpdates { - if htlcIsDust(false, c.isOurs, c.feePerKw, - htlc.Amount.ToSatoshis(), c.dustLimit) { + if htlcIsDust(false, isOurs, feePerKw, + htlc.Amount.ToSatoshis(), dustLimit) { continue } - err := addHTLC(commitTx, c.isOurs, false, htlc, keyRing) + err := addHTLC(commitTx, isOurs, false, htlc, keyRing) if err != nil { - return err + return nil, err } cltvs = append(cltvs, htlc.Timeout) } for _, htlc := range filteredHTLCView.theirUpdates { - if htlcIsDust(true, c.isOurs, c.feePerKw, - htlc.Amount.ToSatoshis(), c.dustLimit) { + if htlcIsDust(true, isOurs, feePerKw, + htlc.Amount.ToSatoshis(), dustLimit) { continue } - err := addHTLC(commitTx, c.isOurs, true, htlc, keyRing) + err := addHTLC(commitTx, isOurs, true, htlc, keyRing) if err != nil { - return err + return nil, err } cltvs = append(cltvs, htlc.Timeout) } @@ -287,9 +311,9 @@ func (cb *CommitmentBuilder) createCommitmentTx(c *commitment, // Set the state hint of the commitment transaction to facilitate // quickly recovering the necessary penalty state in the case of an // uncooperative broadcast. - err = SetStateNumHint(commitTx, c.height, cb.obfuscator) + err = SetStateNumHint(commitTx, height, cb.obfuscator) if err != nil { - return err + return nil, err } // Sort the transactions according to the agreed upon canonical @@ -301,7 +325,7 @@ func (cb *CommitmentBuilder) createCommitmentTx(c *commitment, // transaction which would be invalid by consensus. uTx := btcutil.NewTx(commitTx) if err := blockchain.CheckTransactionSanity(uTx); err != nil { - return err + return nil, err } // Finally, we'll assert that were not attempting to draw more out of @@ -311,17 +335,18 @@ func (cb *CommitmentBuilder) createCommitmentTx(c *commitment, totalOut += btcutil.Amount(txOut.Value) } if totalOut > cb.chanState.Capacity { - return fmt.Errorf("height=%v, for ChannelPoint(%v) attempts "+ - "to consume %v while channel capacity is %v", - c.height, cb.chanState.FundingOutpoint, + return nil, fmt.Errorf("height=%v, for ChannelPoint(%v) "+ + "attempts to consume %v while channel capacity is %v", + height, cb.chanState.FundingOutpoint, totalOut, cb.chanState.Capacity) } - c.txn = commitTx - c.fee = commitFee - c.ourBalance = ourBalance - c.theirBalance = theirBalance - return nil + return &unsignedCommitmentTx{ + txn: commitTx, + fee: commitFee, + ourBalance: ourBalance, + theirBalance: theirBalance, + }, nil } // CreateCommitTx creates a commitment transaction, spending from specified diff --git a/lnwallet/transactions_test.go b/lnwallet/transactions_test.go index 294f5f08..2ef86419 100644 --- a/lnwallet/transactions_test.go +++ b/lnwallet/transactions_test.go @@ -794,23 +794,32 @@ func TestCommitmentAndHTLCTransactions(t *testing.T) { } theHTLCView := htlcViewFromHTLCs(htlcs) + feePerKw := chainfee.SatPerKWeight(test.commitment.FeePerKw) + isOurs := true + height := test.commitment.CommitHeight + // Create unsigned commitment transaction. - commitmentView := &commitment{ - height: test.commitment.CommitHeight, - ourBalance: test.commitment.LocalBalance, - theirBalance: test.commitment.RemoteBalance, - feePerKw: chainfee.SatPerKWeight(test.commitment.FeePerKw), - dustLimit: tc.dustLimit, - isOurs: true, - } - err = channel.commitBuilder.createCommitmentTx( - commitmentView, theHTLCView, keys, + view, err := channel.commitBuilder.createUnsignedCommitmentTx( + test.commitment.LocalBalance, + test.commitment.RemoteBalance, isOurs, feePerKw, + height, theHTLCView, keys, ) if err != nil { t.Errorf("Case %d: Failed to create new commitment tx: %v", i, err) continue } + commitmentView := &commitment{ + ourBalance: view.ourBalance, + theirBalance: view.theirBalance, + txn: view.txn, + fee: view.fee, + height: height, + feePerKw: feePerKw, + dustLimit: tc.dustLimit, + isOurs: isOurs, + } + // Initialize LocalCommit, which is used in getSignedCommitTx. channelState.LocalCommitment = test.commitment channelState.LocalCommitment.Htlcs = htlcs