diff --git a/lnwallet/channel.go b/lnwallet/channel.go index 88c76c4d..1f45805c 100644 --- a/lnwallet/channel.go +++ b/lnwallet/channel.go @@ -47,9 +47,22 @@ var ( ErrMaxHTLCNumber = fmt.Errorf("commitment transaction exceed max " + "htlc number") - // ErrInsufficientBalance is returned when a proposed HTLC would - // exceed the available balance. - ErrInsufficientBalance = fmt.Errorf("insufficient local balance") + // ErrMaxPendingAmount is returned when a proposed HTLC would exceed + // the overall maximum pending value of all HTLCs if committed in a + // state transition. + ErrMaxPendingAmount = fmt.Errorf("commitment transaction exceed max" + + "overall pending htlc value") + + // ErrBelowChanReserve is returned when a proposed HTLC would cause + // one of the peer's funds to dip below the channel reserve limit. + ErrBelowChanReserve = fmt.Errorf("commitment transaction dips peer " + + "below chan reserve") + + // ErrBelowMinHTLC is returned when a proposed HTLC has a value that + // is below the minimum HTLC value constraint for either us or our + // peer depending on which flags are set. + ErrBelowMinHTLC = fmt.Errorf("proposed HTLC value is below minimum " + + "allowed HTLC value") // ErrCannotSyncCommitChains is returned if, upon receiving a ChanSync // message, the state machine deems that is unable to properly @@ -324,6 +337,8 @@ type commitment struct { // within the commitment chain. This balance is computed by properly // evaluating all the add/remove/settle log entries before the listed // indexes. + // + // NOTE: This is the balance *before* subtracting any commitment fee. ourBalance lnwire.MilliSatoshi theirBalance lnwire.MilliSatoshi @@ -1928,7 +1943,7 @@ func htlcSuccessFee(feePerKw btcutil.Amount) btcutil.Amount { // htlcIsDust determines if an HTLC output is dust or not depending on two // bits: if the HTLC is incoming and if the HTLC will be placed on our // commitment transaction, or theirs. These two pieces of information are -// require as we currently used second-level HTLC transactions ass off-chain +// require as we currently used second-level HTLC transactions as off-chain // covenants. Depending on the two bits, we'll either be using a timeout or // success transaction which have different weights. func htlcIsDust(incoming, ourCommit bool, @@ -2665,7 +2680,7 @@ func (lc *LightningChannel) SignNextCommitment() (lnwire.Sig, []lnwire.Sig, erro // party set up when we initially set up the channel. If we are, then // we'll abort this state transition. err := lc.validateCommitmentSanity(remoteACKedIndex, - lc.localUpdateLog.logIndex, false, true, true) + lc.localUpdateLog.logIndex, true, nil) if err != nil { return sig, htlcSigs, err } @@ -3129,69 +3144,118 @@ func (lc *LightningChannel) computeView(view *htlcView, remoteChain bool, return ourBalance, theirBalance, totalCommitWeight, filteredHTLCView, feePerKw } -// validateCommitmentSanity is used to validate that on current state the commitment -// transaction is valid in terms of propagating it over Bitcoin network, and -// also that all outputs are meet Bitcoin spec requirements and they are -// spendable. +// validateCommitmentSanity is used to validate the current state of the +// commitment transaction in terms of the ChannelConstraints that we and our +// remote peer agreed upon during the funding workflow. The predictAdded +// parameter should be set to a valid PaymentDescriptor if we are validating +// in the state when adding a new HTLC, or nil otherwise. func (lc *LightningChannel) validateCommitmentSanity(theirLogCounter, - ourLogCounter uint64, prediction bool, local bool, remote bool) error { - - // TODO(roasbeef): verify remaining sanity requirements - htlcCount := 0 - - // If we adding or receiving the htlc we increase the number of htlcs - // by one in order to not overflow the commitment transaction by - // insertion. - if prediction { - htlcCount++ - } - - // TODO(roasbeef): call availableBalance in here re-using htlcView - - // Run through all the HTLCs that will be covered by this transaction - // in order to calculate theirs count. + ourLogCounter uint64, remoteChain bool, + predictAdded *PaymentDescriptor) error { + // Fetch all updates not committed. view := lc.fetchHTLCView(theirLogCounter, ourLogCounter) - if remote { - for _, entry := range view.theirUpdates { - if entry.EntryType == Add { - htlcCount++ - } - } - for _, entry := range view.ourUpdates { - if entry.EntryType != Add { - htlcCount-- - } - } + // If we are checking if we can add a new HTLC, we add this to the + // update log, in order to validate the sanity of the commitment + // resulting from _actually adding_ this HTLC to the state. + if predictAdded != nil { + // If we are adding an HTLC, this will be an Add to the + // local update log. + view.ourUpdates = append(view.ourUpdates, predictAdded) } - if local { - for _, entry := range view.ourUpdates { - if entry.EntryType == Add { - htlcCount++ - } - } - for _, entry := range view.theirUpdates { - if entry.EntryType != Add { - htlcCount-- - } - } + commitChain := lc.localCommitChain + if remoteChain { + commitChain = lc.remoteCommitChain } + ourInitialBalance := commitChain.tip().ourBalance + theirInitialBalance := commitChain.tip().theirBalance - // If we're validating the commitment sanity for HTLC _log_ update by a - // particular side, then we'll only consider half of the available HTLC - // bandwidth. However, if we're validating the _creation_ of a new - // commitment state, then we'll use the full value as the sum of the - // contribution of both sides shouldn't exceed the max number. - var maxHTLCNumber int - if local && remote { - maxHTLCNumber = MaxHTLCNumber + ourBalance, theirBalance, commitWeight, filteredView, feePerKw := + lc.computeView(view, remoteChain, false) + + // Calculate the commitment fee, and subtract it from the + // initiator's balance. + commitFee := btcutil.Amount((int64(feePerKw) * commitWeight) / 1000) + if lc.channelState.IsInitiator { + ourBalance -= lnwire.NewMSatFromSatoshis(commitFee) } else { - maxHTLCNumber = MaxHTLCNumber / 2 + theirBalance -= lnwire.NewMSatFromSatoshis(commitFee) } - if htlcCount > maxHTLCNumber { - return ErrMaxHTLCNumber + // If the added HTLCs will decrease the balance, make sure + // they won't dip the local and remote balances below the + // channel reserves. + if ourBalance < ourInitialBalance && + ourBalance < lnwire.NewMSatFromSatoshis( + lc.localChanCfg.ChanReserve) { + return ErrBelowChanReserve + } + + if theirBalance < theirInitialBalance && + theirBalance < lnwire.NewMSatFromSatoshis( + lc.remoteChanCfg.ChanReserve) { + return ErrBelowChanReserve + } + + // validateUpdates take a set of updates, and validates them + // against the passed channel constraints. + validateUpdates := func(updates []*PaymentDescriptor, + constraints *channeldb.ChannelConfig) error { + + // We keep track of the number of HTLCs in flight for + // the commitment, and the amount in flight. + var numInFlight uint16 + var amtInFlight lnwire.MilliSatoshi + + // Go through all updates, checking that they don't + // violate the channel constraints. + for _, entry := range updates { + if entry.EntryType == Add { + // An HTLC is being added, this will + // add to the number and amount in + // flight. + amtInFlight += entry.Amount + numInFlight++ + + // Check that the value of the HTLC they + // added is above our minimum. + if entry.Amount < constraints.MinHTLC { + return ErrBelowMinHTLC + } + } + } + + // Now that we know the total value of added HTLCs, + // we check that this satisfy the MaxPendingAmont + // contraint. + if amtInFlight > constraints.MaxPendingAmount { + return ErrMaxPendingAmount + } + + // In this step, we verify that the total number of + // active HTLCs does not exceed the constraint of the + // maximum number of HTLCs in flight. + if numInFlight > constraints.MaxAcceptedHtlcs { + return ErrMaxHTLCNumber + } + return nil + } + + // First check that the remote updates won't violate it's + // channel constraints. + err := validateUpdates(filteredView.theirUpdates, + lc.remoteChanCfg) + if err != nil { + return err + } + + // Secondly check that our updates won't violate our + // channel constraints. + err = validateUpdates(filteredView.ourUpdates, + lc.localChanCfg) + if err != nil { + return err } return nil @@ -3392,7 +3456,7 @@ func (lc *LightningChannel) ReceiveNewCommitment(commitSig lnwire.Sig, // the constraints we specified during initial channel setup. If not, // then we'll abort the channel as they've violated our constraints. err := lc.validateCommitmentSanity(lc.remoteUpdateLog.logIndex, - localACKedIndex, false, true, true) + localACKedIndex, false, nil) if err != nil { return err } @@ -3411,8 +3475,9 @@ func (lc *LightningChannel) ReceiveNewCommitment(commitSig lnwire.Sig, lc.remoteChanCfg) // With the current commitment point re-calculated, construct the new - // commitment view which includes all the entries we know of in their - // HTLC log, and up to ourLogIndex in our HTLC log. + // commitment view which includes all the entries (pending or committed) + // we know of in the remote node's HTLC log, but only our local changes + // up to the last change the remote node has ACK'd. localCommitmentView, err := lc.fetchCommitmentView( false, localACKedIndex, localHtlcIndex, lc.remoteUpdateLog.logIndex, lc.remoteUpdateLog.htlcCounter, @@ -3737,43 +3802,6 @@ func (lc *LightningChannel) AddHTLC(htlc *lnwire.UpdateAddHTLC) (uint64, error) lc.Lock() defer lc.Unlock() - if err := lc.validateCommitmentSanity(lc.remoteUpdateLog.logIndex, - lc.localUpdateLog.logIndex, true, true, false); err != nil { - return 0, err - } - - // To ensure that we can actually fully accept this new HTLC, we'll - // calculate the current available bandwidth, and subtract the value of - // the HTLC from it. - initialBalance, _ := lc.availableBalance() - availableBalance := initialBalance - availableBalance -= htlc.Amount - - feePerKw := lc.channelState.LocalCommitment.FeePerKw - dustLimit := lc.channelState.LocalChanCfg.DustLimit - htlcIsDust := htlcIsDust( - false, true, feePerKw, htlc.Amount.ToSatoshis(), dustLimit, - ) - - // If this HTLC is not dust, and we're the initiator, then we'll also - // subtract the amount we'll need to pay in fees for this HTLC. - if !htlcIsDust && lc.channelState.IsInitiator { - htlcFee := lnwire.NewMSatFromSatoshis( - btcutil.Amount((int64(feePerKw) * HtlcWeight) / 1000), - ) - availableBalance -= htlcFee - } - - // If this value is negative, then we can't accept the HTLC, so we'll - // reject it with an error. - if availableBalance < 0 { - // TODO(roasbeef): also needs to respect reservation - // * expand to add context err msg - walletLog.Errorf("Unable to carry added HTLC: amt=%v, bal=%v", - htlc.Amount, availableBalance) - return 0, ErrInsufficientBalance - } - pd := &PaymentDescriptor{ EntryType: Add, RHash: PaymentHash(htlc.PaymentHash), @@ -3784,6 +3812,14 @@ func (lc *LightningChannel) AddHTLC(htlc *lnwire.UpdateAddHTLC) (uint64, error) OnionBlob: htlc.OnionBlob[:], } + // Make sure adding this HTLC won't violate any of the constrainst + // we must keep on our commitment transaction. + remoteACKedIndex := lc.localCommitChain.tail().theirMessageIndex + if err := lc.validateCommitmentSanity(remoteACKedIndex, + lc.localUpdateLog.logIndex, true, pd); err != nil { + return 0, err + } + lc.localUpdateLog.appendHtlc(pd) return pd.HtlcIndex, nil @@ -3801,11 +3837,6 @@ func (lc *LightningChannel) ReceiveHTLC(htlc *lnwire.UpdateAddHTLC) (uint64, err "ID %d", htlc.ID, lc.remoteUpdateLog.htlcCounter) } - if err := lc.validateCommitmentSanity(lc.remoteUpdateLog.logIndex, - lc.localUpdateLog.logIndex, true, false, true); err != nil { - return 0, err - } - pd := &PaymentDescriptor{ EntryType: Add, RHash: PaymentHash(htlc.PaymentHash), @@ -5343,3 +5374,8 @@ func (lc *LightningChannel) ActiveHtlcs() []channeldb.HTLC { return activeHtlcs } + +// LocalChanReserve returns our local ChanReserve requirement for the remote party. +func (lc *LightningChannel) LocalChanReserve() btcutil.Amount { + return lc.localChanCfg.ChanReserve +}