Merge pull request #751 from bhandras/negative-fees-fixup

loopout: fix negative reported fees

Author: bhandras

loopout.go

@@ -402,7 +402,7 @@ func (s *loopOutSwap) executeAndFinalize(globalCtx context.Context) error {
 		case result := <-s.swapPaymentChan:
 			s.swapPaymentChan = nil
 
-			err := s.handlePaymentResult(result)
+			err := s.handlePaymentResult(result, true)
 			if err != nil {
 				return err
 			}
@@ -418,7 +418,7 @@ func (s *loopOutSwap) executeAndFinalize(globalCtx context.Context) error {
 		case result := <-s.prePaymentChan:
 			s.prePaymentChan = nil
 
-			err := s.handlePaymentResult(result)
+			err := s.handlePaymentResult(result, false)
 			if err != nil {
 				return err
 			}
@@ -448,17 +448,32 @@ func (s *loopOutSwap) executeAndFinalize(globalCtx context.Context) error {
 	return s.persistState(globalCtx)
 }
 
-func (s *loopOutSwap) handlePaymentResult(result paymentResult) error {
+// handlePaymentResult processes the result of a payment attempt. If the
+// payment was successful and this is the main swap payment, the cost of the
+// swap is updated.
+func (s *loopOutSwap) handlePaymentResult(result paymentResult,
+	swapPayment bool) error {
+
 	switch {
 	// If our result has a non-nil error, our status will be nil. In this
 	// case the payment failed so we do not need to take any action.
 	case result.err != nil:
 		return nil
 
 	case result.status.State == lnrpc.Payment_SUCCEEDED:
-		s.cost.Server += result.status.Value.ToSatoshis() -
-			s.AmountRequested
-		s.cost.Offchain += result.status.Fee.ToSatoshis()
+		// Update the cost of the swap if this is the main swap payment.
+		if swapPayment {
+			// The client pays for the swap with the swap invoice,
+			// so we can calculate the total cost of the swap by
+			// subtracting the amount requested from the amount we
+			// actually paid.
+			s.cost.Server += result.status.Value.ToSatoshis() -
+				s.AmountRequested
+
+			// On top of the swap cost we also pay for routing which
+			// is reflected in the fee.
+			s.cost.Offchain += result.status.Fee.ToSatoshis()
+		}
 
 		return nil
 
@@ -917,7 +932,7 @@ func (s *loopOutSwap) waitForConfirmedHtlc(globalCtx context.Context) (
 			case result := <-s.swapPaymentChan:
 				s.swapPaymentChan = nil
 
-				err := s.handlePaymentResult(result)
+				err := s.handlePaymentResult(result, true)
 				if err != nil {
 					return nil, err
 				}
@@ -939,7 +954,7 @@ func (s *loopOutSwap) waitForConfirmedHtlc(globalCtx context.Context) (
 			case result := <-s.prePaymentChan:
 				s.prePaymentChan = nil
 
-				err := s.handlePaymentResult(result)
+				err := s.handlePaymentResult(result, false)
 				if err != nil {
 					return nil, err
 				}

sweepbatcher/sweep_batch.go

@@ -1158,9 +1158,8 @@ func (b *batch) monitorConfirmations(ctx context.Context) error {
 func getFeePortionForSweep(spendTx *wire.MsgTx, numSweeps int,
 	totalSweptAmt btcutil.Amount) (btcutil.Amount, btcutil.Amount) {
 
-	totalFee := spendTx.TxOut[0].Value - int64(totalSweptAmt)
-	feePortionPerSweep := (int64(totalSweptAmt) -
-		spendTx.TxOut[0].Value) / int64(numSweeps)
+	totalFee := int64(totalSweptAmt) - spendTx.TxOut[0].Value
+	feePortionPerSweep := totalFee / int64(numSweeps)
 	roundingDiff := totalFee - (int64(numSweeps) * feePortionPerSweep)
 
 	return btcutil.Amount(feePortionPerSweep), btcutil.Amount(roundingDiff)

sweepbatcher/sweep_batcher_test.go

@@ -984,3 +984,48 @@ func TestSweepBatcherComposite(t *testing.T) {
 	require.True(t, batcherStore.AssertSweepStored(sweepReq5.SwapHash))
 	require.True(t, batcherStore.AssertSweepStored(sweepReq6.SwapHash))
 }
+
+// makeTestTx creates a test transaction with a single output of the given
+// value.
+func makeTestTx(value int64) *wire.MsgTx {
+	tx := wire.NewMsgTx(wire.TxVersion)
+	tx.AddTxOut(wire.NewTxOut(value, nil))
+	return tx
+}
+
+// TestGetFeePortionForSweep tests that the fee portion for a sweep is correctly
+// calculated.
+func TestGetFeePortionForSweep(t *testing.T) {
+	tests := []struct {
+		name                 string
+		spendTxValue         int64
+		numSweeps            int
+		totalSweptAmt        btcutil.Amount
+		expectedFeePortion   btcutil.Amount
+		expectedRoundingDiff btcutil.Amount
+	}{
+		{
+			"Even Split",
+			100, 5, 200, 20, 0,
+		},
+		{
+			"Single Sweep",
+			100, 1, 200, 100, 0,
+		},
+		{
+			"With Rounding Diff",
+			200, 4, 350, 37, 2,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			spendTx := makeTestTx(tt.spendTxValue)
+			feePortion, roundingDiff := getFeePortionForSweep(
+				spendTx, tt.numSweeps, tt.totalSweptAmt,
+			)
+			require.Equal(t, tt.expectedFeePortion, feePortion)
+			require.Equal(t, tt.expectedRoundingDiff, roundingDiff)
+		})
+	}
+}