BTCPrivate
/
lnd
mirror of https://github.com/BTCPrivate/lnd.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Merge pull request #779 from Roasbeef/unsigned-msat-and-fee-fixes 

lnwire+lnwallet+htlcswitch: modify lnwire.MilliSatoshi to be unsigned, fix fee related bugs that emerged
This commit is contained in:
Olaoluwa Osuntokun 2018-03-08 22:28:16 -05:00 committed by GitHub
parent 30c92ea7f1 8d35ea381e
commit 855e85511e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 169 additions and 118 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
htlcswitch/link.go
View File

@ -1729,15 +1729,18 @@ func (l *channelLink) processLockedInHtlcs(
fwdInfo.AmountToForward,
)
// If the amount of the incoming HTLC, minus
// our expected fee isn't equal to the
// forwarding instructions, then either the
// values have been tampered with, or the send
// used incorrect/dated information to
// If the actual fee is less than our expected
// fee, then we'll reject this HTLC as it
// didn't provide a sufficient amount of fees,
// or the values have been tampered with, or
// the send used incorrect/dated information to
// construct the forwarding information for
// this hop. In any case, we'll cancel this
// HTLC.
if pd.Amount-expectedFee < fwdInfo.AmountToForward {
actualFee := pd.Amount - fwdInfo.AmountToForward
if pd.Amount < fwdInfo.AmountToForward ||
actualFee < expectedFee {
log.Errorf("Incoming htlc(%x) has "+
"insufficient fee: expected "+
"%v, got %v", pd.RHash[:],

4
htlcswitch/link_test.go
View File

@ -777,7 +777,7 @@ func TestUpdateForwardingPolicy(t *testing.T) {
testStartingHeight,
n.firstBobChannelLink, n.carolChannelLink)
// First, send this 1 BTC payment over the three hops, the payment
// First, send this 10 mSAT payment over the three hops, the payment
// should succeed, and all balances should be updated accordingly.
payResp, err := n.makePayment(n.aliceServer, n.carolServer,
n.bobServer.PubKey(), hops, amountNoFee, htlcAmt,
@ -827,7 +827,7 @@ func TestUpdateForwardingPolicy(t *testing.T) {
n.firstBobChannelLink.UpdateForwardingPolicy(newPolicy)
// Next, we'll send the payment again, using the exact same per-hop
// payload for each node. This payment should fail as it wont' factor
// payload for each node. This payment should fail as it won't factor
// in Bob's new fee policy.
_, err = n.makePayment(n.aliceServer, n.carolServer,
n.bobServer.PubKey(), hops, amountNoFee, htlcAmt,

159
lnwallet/channel.go
View File

@ -2139,14 +2139,24 @@ func (lc *LightningChannel) 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)
commitFeeMSat := lnwire.NewMSatFromSatoshis(commitFee)
// Currently, within the protocol, the initiator always pays the fees.
// So we'll subtract the fee amount from the balance of the current
// initiator.
if lc.channelState.IsInitiator {
ourBalance -= lnwire.NewMSatFromSatoshis(commitFee)
} else if !lc.channelState.IsInitiator {
theirBalance -= lnwire.NewMSatFromSatoshis(commitFee)
// initiator. If the initiator is unable to pay the fee fully, then
// their entire output is consumed.
switch {
case lc.channelState.IsInitiator && commitFee > ourBalance.ToSatoshis():
ourBalance = 0
case lc.channelState.IsInitiator:
ourBalance -= commitFeeMSat
case !lc.channelState.IsInitiator && commitFee > theirBalance.ToSatoshis():
theirBalance = 0
case !lc.channelState.IsInitiator:
theirBalance -= commitFeeMSat
}
var (
@ -3042,14 +3052,14 @@ func (lc *LightningChannel) ChanSyncMsg() (*lnwire.ChannelReestablish, error) {
}, nil
}
// computeView takes the given htlcView, and calculates the balances,
// filtered view (settling unsettled HTLCs), commitment weight and
// feePerKw, after applying the HTLCs to the latest commitment. The
// returned balanced are the balances *before* subtracting the
// commitment fee from the initiator's balance.
// computeView takes the given htlcView, and calculates the balances, filtered
// view (settling unsettled HTLCs), commitment weight and feePerKw, after
// applying the HTLCs to the latest commitment. The returned balances are the
// balances *before* subtracting the commitment fee from the initiator's
// balance.
//
// If the updateState boolean is set true, the add and remove heights
// of the HTLCs will be set to the next commitment height.
// If the updateState boolean is set true, the add and remove heights of the
// HTLCs will be set to the next commitment height.
func (lc *LightningChannel) computeView(view *htlcView, remoteChain bool,
updateState bool) (lnwire.MilliSatoshi, lnwire.MilliSatoshi, int64,
*htlcView, SatPerKWeight) {
@ -3061,16 +3071,16 @@ func (lc *LightningChannel) computeView(view *htlcView, remoteChain bool,
dustLimit = lc.remoteChanCfg.DustLimit
}
// Since the fetched htlc view will include all updates added
// after the last committed state, we start with the balances
// reflecting that state.
// Since the fetched htlc view will include all updates added after the
// last committed state, we start with the balances reflecting that
// state.
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.
// re-applied in case fee estimation parameters have changed or the
// number of outstanding HTLCs has changed.
if lc.channelState.IsInitiator {
ourBalance += lnwire.NewMSatFromSatoshis(
commitChain.tip().fee)
@ -3080,11 +3090,10 @@ func (lc *LightningChannel) computeView(view *htlcView, remoteChain bool,
}
nextHeight := commitChain.tip().height + 1
// We evaluate the view at this stage, meaning settled and
// failed HTLCs will remove their corresponding added HTLCs.
// The resulting filtered view will only have Add entries left,
// making it easy to compare the channel constraints to the
// final commitment state.
// We evaluate the view at this stage, meaning settled and failed HTLCs
// will remove their corresponding added HTLCs. The resulting filtered
// view will only have Add entries left, making it easy to compare the
// channel constraints to the final commitment state.
filteredHTLCView := lc.evaluateHTLCView(view, &ourBalance,
&theirBalance, nextHeight, remoteChain, updateState)
@ -3155,6 +3164,7 @@ func (lc *LightningChannel) computeView(view *htlcView, remoteChain bool,
func (lc *LightningChannel) validateCommitmentSanity(theirLogCounter,
ourLogCounter uint64, remoteChain bool,
predictAdded *PaymentDescriptor) error {
// Fetch all updates not committed.
view := lc.fetchHTLCView(theirLogCounter, ourLogCounter)
@ -3162,8 +3172,8 @@ func (lc *LightningChannel) validateCommitmentSanity(theirLogCounter,
// 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.
// If we are adding an HTLC, this will be an Add to the local
// update log.
view.ourUpdates = append(view.ourUpdates, predictAdded)
}
@ -3174,21 +3184,33 @@ func (lc *LightningChannel) validateCommitmentSanity(theirLogCounter,
ourInitialBalance := commitChain.tip().ourBalance
theirInitialBalance := commitChain.tip().theirBalance
ourBalance, theirBalance, commitWeight, filteredView, feePerKw :=
lc.computeView(view, remoteChain, false)
ourBalance, theirBalance, commitWeight, filteredView, feePerKw := lc.computeView(
view, remoteChain, false,
)
// Calculate the commitment fee, and subtract it from the
// initiator's balance.
// Calculate the commitment fee, and subtract it from the initiator's
// balance.
commitFee := feePerKw.FeeForWeight(commitWeight)
commitFeeMsat := lnwire.NewMSatFromSatoshis(commitFee)
if lc.channelState.IsInitiator {
ourBalance -= lnwire.NewMSatFromSatoshis(commitFee)
ourBalance -= commitFeeMsat
} else {
theirBalance -= lnwire.NewMSatFromSatoshis(commitFee)
theirBalance -= commitFeeMsat
}
// If the added HTLCs will decrease the balance, make sure
// they won't dip the local and remote balances below the
// channel reserves.
// As a quick sanity check, we'll ensure that if we interpret the
// balances as signed integers, they haven't dipped down below zero. If
// they have, then this indicates that a party doesn't have sufficient
// balance to satisfy the final evaluated HTLC's.
switch {
case int64(ourBalance) < 0:
return ErrBelowChanReserve
case int64(theirBalance) < 0:
return ErrBelowChanReserve
}
// 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) {
@ -3201,62 +3223,63 @@ func (lc *LightningChannel) validateCommitmentSanity(theirLogCounter,
return ErrBelowChanReserve
}
// validateUpdates take a set of updates, and validates them
// against the passed channel constraints.
// 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.
// 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.
// 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.
// 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.
// 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.
// 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.
// 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)
// 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)
// Secondly check that our updates won't violate our channel
// constraints.
err = validateUpdates(
filteredView.ourUpdates, lc.localChanCfg,
)
if err != nil {
return err
}
@ -3815,11 +3838,13 @@ 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.
// Make sure adding this HTLC won't violate any of the constraints we
// must keep on our commitment transaction.
remoteACKedIndex := lc.localCommitChain.tail().theirMessageIndex
if err := lc.validateCommitmentSanity(remoteACKedIndex,
lc.localUpdateLog.logIndex, true, pd); err != nil {
err := lc.validateCommitmentSanity(
remoteACKedIndex, lc.localUpdateLog.logIndex, true, pd,
)
if err != nil {
return 0, err
}
@ -5112,10 +5137,18 @@ func (lc *LightningChannel) validateFeeRate(feePerKw SatPerKWeight) error {
newFee := lnwire.NewMSatFromSatoshis(
feePerKw.FeeForWeight(txWeight),
)
balanceAfterFee := availableBalance - newFee
// If the total fee exceeds our available balance, then we'll reject
// this update as it would mean we need to trim our entire output.
if newFee > availableBalance {
return fmt.Errorf("cannot apply fee_update=%v sat/kw, new fee "+
"of %v is greater than balance of %v", int64(feePerKw),
newFee, availableBalance)
}
// If this new balance is below our reserve, then we can't accommodate
// the fee change, so we'll reject it.
balanceAfterFee := availableBalance - newFee
if balanceAfterFee.ToSatoshis() < lc.channelState.LocalChanCfg.ChanReserve {
return fmt.Errorf("cannot apply fee_update=%v sat/kw, "+
"insufficient balance: start=%v, end=%v",

37
lnwallet/channel_test.go
View File

@ -2462,8 +2462,8 @@ func TestAddHTLCNegativeBalance(t *testing.T) {
}
defer cleanUp()
// We set the channel reserve to 0, such that we can add HTLCs
// all the way to a negative balance.
// We set the channel reserve to 0, such that we can add HTLCs all the
// way to a negative balance.
aliceChannel.localChanCfg.ChanReserve = 0
// First, we'll add 3 HTLCs of 1 BTC each to Alice's commitment.
@ -2476,14 +2476,15 @@ func TestAddHTLCNegativeBalance(t *testing.T) {
}
}
// Alice now has an available balance of 2 BTC. We'll add a new HTLC
// of value 2 BTC, which should make Alice's balance negative (since
// (she has to pay a commitment fee).
// Alice now has an available balance of 2 BTC. We'll add a new HTLC of
// value 2 BTC, which should make Alice's balance negative (since she
// has to pay a commitment fee).
htlcAmt = lnwire.NewMSatFromSatoshis(2 * btcutil.SatoshiPerBitcoin)
htlc, _ := createHTLC(numHTLCs+1, htlcAmt)
_, err = aliceChannel.AddHTLC(htlc)
if err != ErrBelowChanReserve {
t.Fatalf("expected balance below channel reserve, instead got: %v", err)
t.Fatalf("expected balance below channel reserve, instead "+
"got: %v", err)
}
}
@ -4375,23 +4376,22 @@ func TestDesyncHTLCs(t *testing.T) {
t.Fatalf("unable to recv htlc cancel: %v", err)
}
// Alice now has gotten all here original balance (5 BTC) back,
// however, adding a new HTLC at this point SHOULD fail, since
// if she add the HTLC and sign the next state, Bob cannot assume
// she received the FailHTLC, and must assume she doesn't have
// the necessary balance available.
// Alice now has gotten all her original balance (5 BTC) back, however,
// adding a new HTLC at this point SHOULD fail, since if she adds the
// HTLC and signs the next state, Bob cannot assume she received the
// FailHTLC, and must assume she doesn't have the necessary balance
// available.
//
// We try adding an HTLC of value 1 BTC, which should fail
// because the balance is unavailable.
// We try adding an HTLC of value 1 BTC, which should fail because the
// balance is unavailable.
htlcAmt = lnwire.NewMSatFromSatoshis(1 * btcutil.SatoshiPerBitcoin)
htlc, _ = createHTLC(1, htlcAmt)
if _, err = aliceChannel.AddHTLC(htlc); err != ErrBelowChanReserve {
t.Fatalf("expected ErrInsufficientBalance, instead received: %v",
err)
t.Fatalf("expected ErrBelowChanReserve, instead received: %v", err)
}
// Now do a state transition, which will ACK the FailHTLC, making
// Alice able to add the new HTLC.
// Now do a state transition, which will ACK the FailHTLC, making Alice
// able to add the new HTLC.
if err := forceStateTransition(aliceChannel, bobChannel); err != nil {
t.Fatalf("unable to complete state update: %v", err)
}
@ -4403,7 +4403,8 @@ func TestDesyncHTLCs(t *testing.T) {
// TODO(roasbeef): testing.Quick test case for retrans!!!
// TestMaxAcceptedHTLCs tests that the correct error message (ErrMaxHTLCNumber)
// is thrown when a node tries to accept more than MaxAcceptedHTLCs in a channel.
// is thrown when a node tries to accept more than MaxAcceptedHTLCs in a
// channel.
func TestMaxAcceptedHTLCs(t *testing.T) {
t.Parallel()

46
lnwallet/reservation.go
View File

@ -157,6 +157,15 @@ func NewChannelReservation(capacity, fundingAmt btcutil.Amount,
ourBalance = pushMSat
theirBalance = capacityMSat - feeMSat - pushMSat
initiator = false
// If the responder doesn't have enough funds to actually pay
// the fees, then we'll bail our early.
if int64(theirBalance) < 0 {
return nil, ErrFunderBalanceDust(
int64(commitFee), int64(theirBalance.ToSatoshis()),
int64(2*DefaultDustLimit()),
)
}
} else {
// TODO(roasbeef): need to rework fee structure in general and
// also when we "unlock" dual funder within the daemon
@ -177,6 +186,15 @@ func NewChannelReservation(capacity, fundingAmt btcutil.Amount,
}
initiator = true
// If we, the initiator don't have enough funds to actually pay
// the fees, then we'll exit with an error.
if int64(ourBalance) < 0 {
return nil, ErrFunderBalanceDust(
int64(commitFee), int64(ourBalance),
int64(2*DefaultDustLimit()),
)
}
}
// If we're the initiator and our starting balance within the channel
@ -288,40 +306,36 @@ func (r *ChannelReservation) CommitConstraints(csvDelay, maxHtlcs uint16,
return ErrCsvDelayTooLarge(csvDelay, maxDelay)
}
// Fail if we consider the channel reserve to be too large.
// We currently fail if it is greater than 20% of the
// channel capacity.
// Fail if we consider the channel reserve to be too large. We
// currently fail if it is greater than 20% of the channel capacity.
maxChanReserve := r.partialState.Capacity / 5
if chanReserve > maxChanReserve {
return ErrChanReserveTooLarge(chanReserve, maxChanReserve)
}
// Fail if the minimum HTLC value is too large. If this is
// too large, the channel won't be useful for sending small
// payments. This limit is currently set to maxValueInFlight,
// effictively letting the remote setting this as large as
// it wants.
// TODO(halseth): set a reasonable/dynamic value.
// Fail if the minimum HTLC value is too large. If this is too large,
// the channel won't be useful for sending small payments. This limit
// is currently set to maxValueInFlight, effectively letting the remote
// setting this as large as it wants.
if minHtlc > maxValueInFlight {
return ErrMinHtlcTooLarge(minHtlc, maxValueInFlight)
}
// Fail if maxHtlcs is above the maximum allowed number of 483.
// This number is specified in BOLT-02.
// Fail if maxHtlcs is above the maximum allowed number of 483. This
// number is specified in BOLT-02.
if maxHtlcs > uint16(MaxHTLCNumber/2) {
return ErrMaxHtlcNumTooLarge(maxHtlcs, uint16(MaxHTLCNumber/2))
}
// Fail if we consider maxHtlcs too small. If this is too small
// we cannot offer many HTLCs to the remote.
// Fail if we consider maxHtlcs too small. If this is too small we
// cannot offer many HTLCs to the remote.
const minNumHtlc = 5
if maxHtlcs < minNumHtlc {
return ErrMaxHtlcNumTooSmall(maxHtlcs, minNumHtlc)
}
// Fail if we consider maxValueInFlight too small. We currently
// require the remote to at least allow minNumHtlc * minHtlc
// in flight.
// Fail if we consider maxValueInFlight too small. We currently require
// the remote to at least allow minNumHtlc * minHtlc in flight.
if maxValueInFlight < minNumHtlc*minHtlc {
return ErrMaxValueInFlightTooSmall(maxValueInFlight,
minNumHtlc*minHtlc)

10
lnwallet/transactions_test.go
View File

@ -6,6 +6,7 @@ import (
"fmt"
"testing"
"github.com/davecgh/go-spew/spew"
"github.com/lightningnetwork/lnd/channeldb"
"github.com/lightningnetwork/lnd/keychain"
"github.com/lightningnetwork/lnd/lnwire"
@ -806,8 +807,9 @@ func TestCommitmentAndHTLCTransactions(t *testing.T) {
dustLimit: tc.dustLimit,
isOurs: true,
}
err = channel.createCommitmentTx(commitmentView, theHTLCView,
keys)
err = channel.createCommitmentTx(
commitmentView, theHTLCView, keys,
)
if err != nil {
t.Errorf("Case %d: Failed to create new commitment tx: %v", i, err)
continue
@ -836,8 +838,8 @@ func TestCommitmentAndHTLCTransactions(t *testing.T) {
// Check that commitment transaction was created correctly.
if commitTx.WitnessHash() != *expectedCommitmentTx.WitnessHash() {
t.Errorf("Case %d: Generated unexpected commitment tx: "+
"expected %s, got %s", i, expectedCommitmentTx.WitnessHash(),
commitTx.WitnessHash())
"expected %s, got %s", i, spew.Sdump(expectedCommitmentTx),
spew.Sdump(commitTx))
continue
}

12
lnwire/msat.go
View File

@ -8,7 +8,7 @@ import (
// mSatScale is a value that's used to scale satoshis to milli-satoshis, and
// the other way around.
const mSatScale int64 = 1000
const mSatScale uint64 = 1000
// MilliSatoshi are the native unit of the Lightning Network. A milli-satoshi
// is simply 1/1000th of a satoshi. There are 1000 milli-satoshis in a single
@ -16,12 +16,12 @@ const mSatScale int64 = 1000
// milli-satoshis. As milli-satoshis aren't deliverable on the native
// blockchain, before settling to broadcasting, the values are rounded down to
// the nearest satoshi.
type MilliSatoshi int64
type MilliSatoshi uint64
// NewMSatFromSatoshis creates a new MilliSatoshi instance from a target amount
// of satoshis.
func NewMSatFromSatoshis(sat btcutil.Amount) MilliSatoshi {
return MilliSatoshi(int64(sat) * mSatScale)
return MilliSatoshi(uint64(sat) * mSatScale)
}
// ToBTC converts the target MilliSatoshi amount to its corresponding value
@ -34,10 +34,12 @@ func (m MilliSatoshi) ToBTC() float64 {
// ToSatoshis converts the target MilliSatoshi amount to satoshis. Simply, this
// sheds a factor of 1000 from the mSAT amount in order to convert it to SAT.
func (m MilliSatoshi) ToSatoshis() btcutil.Amount {
return btcutil.Amount(int64(m) / mSatScale)
return btcutil.Amount(uint64(m) / mSatScale)
}
// String returns the string representation of the mSAT amount.
func (m MilliSatoshi) String() string {
return fmt.Sprintf("%v mSAT", int64(m))
return fmt.Sprintf("%v mSAT", uint64(m))
}
// TODO(roasbeef): extend with arithmetic operations?

4
zpay32/invoice_internal_test.go
View File

@ -149,10 +149,6 @@ func TestEncodeAmount(t *testing.T) {
valid bool
result string
}{
{
msat: -10, // mSat
valid: false, // negative amount
},
{
msat: 1, // mSat
valid: true,