Merge pull request #676 from hieblmi/sweep-timeout-on-invalid-amt

loopin: sweep incorrect amount on timeout

loopd/swapclient_server.go

@@ -306,6 +306,9 @@ func (s *swapClientServer) marshallSwap(loopSwap *loop.SwapInfo) (
 	case loopdb.StateFailInsufficientConfirmedBalance:
 		failureReason = clientrpc.FailureReason_FAILURE_REASON_INSUFFICIENT_CONFIRMED_BALANCE
 
+	case loopdb.StateFailIncorrectHtlcAmtSwept:
+		failureReason = clientrpc.FailureReason_FAILURE_REASON_INCORRECT_HTLC_AMT_SWEPT
+
 	default:
 		return nil, fmt.Errorf("unknown swap state: %v", loopSwap.State)
 	}

loopdb/swapstate.go

@@ -72,6 +72,11 @@ const (
 	// StateFailInsufficientConfirmedBalance indicates that the swap wasn't
 	// published due to insufficient confirmed balance.
 	StateFailInsufficientConfirmedBalance SwapState = 12
+
+	// StateFailIncorrectHtlcAmtSwept indicates that the amount of an
+	// externally published loop in htlc that didn't match the swap amount
+	// has been swept back to the user after the htlc timeout period.
+	StateFailIncorrectHtlcAmtSwept SwapState = 13
 )
 
 // SwapStateType defines the types of swap states that exist. Every swap state
@@ -92,10 +97,7 @@ const (
 
 // Type returns the type of the SwapState it is called on.
 func (s SwapState) Type() SwapStateType {
-	if s == StateInitiated || s == StateHtlcPublished ||
-		s == StatePreimageRevealed || s == StateFailTemporary ||
-		s == StateInvoiceSettled {
-
+	if s.IsPending() {
 		return StateTypePending
 	}
 
@@ -110,7 +112,7 @@ func (s SwapState) Type() SwapStateType {
 func (s SwapState) IsPending() bool {
 	return s == StateInitiated || s == StateHtlcPublished ||
 		s == StatePreimageRevealed || s == StateFailTemporary ||
-		s == StateInvoiceSettled
+		s == StateInvoiceSettled || s == StateFailIncorrectHtlcAmt
 }
 
 // IsFinal returns true if the swap is in a final state.
@@ -160,6 +162,9 @@ func (s SwapState) String() string {
 	case StateFailInsufficientConfirmedBalance:
 		return "InsufficientConfirmedBalance"
 
+	case StateFailIncorrectHtlcAmtSwept:
+		return "StateFailIncorrectHtlcAmtSwept"
+
 	default:
 		return "Unknown"
 	}

loopin.go

@@ -623,10 +623,18 @@ func (s *loopInSwap) executeSwap(globalCtx context.Context) error {
 	}
 
 	// Verify that the confirmed (external) htlc value matches the swap
-	// amount. Otherwise, fail the swap immediately.
-	if htlcValue != s.LoopInContract.AmountRequested {
+	// amount. If the amounts mismatch we update the swap state to indicate
+	// this, but end processing the swap. Instead, we continue to wait for
+	// the htlc to expire and publish a timeout tx to reclaim the funds. We
+	// skip this part if the swap was recovered from this state.
+	if s.state != loopdb.StateFailIncorrectHtlcAmt &&
+		htlcValue != s.LoopInContract.AmountRequested {
+
 		s.setState(loopdb.StateFailIncorrectHtlcAmt)
-		return s.persistAndAnnounceState(globalCtx)
+		err = s.persistAndAnnounceState(globalCtx)
+		if err != nil {
+			log.Errorf("Error persisting state: %v", err)
+		}
 	}
 
 	// The server is expected to see the htlc on-chain and know that it can
@@ -1032,7 +1040,16 @@ func (s *loopInSwap) processHtlcSpend(ctx context.Context,
 		// We needed another on chain tx to sweep the timeout clause,
 		// which we now include in our costs.
 		s.cost.Onchain += sweepFee
-		s.setState(loopdb.StateFailTimeout)
+
+		// If the swap is in state StateFailIncorrectHtlcAmt we know
+		// that the deposited htlc amount wasn't equal to the contract
+		// amount. We can finalize the swap by setting an appropriate
+		// state.
+		if s.state == loopdb.StateFailIncorrectHtlcAmt {
+			s.setState(loopdb.StateFailIncorrectHtlcAmtSwept)
+		} else {
+			s.setState(loopdb.StateFailTimeout)
+		}
 
 		// Now that the timeout tx confirmed, we can safely cancel the
 		// swap invoice. We still need to query the final invoice state.

loopin_test.go

@@ -277,14 +277,19 @@ func testLoopInTimeout(t *testing.T, externalValue int64) {
 		Tx: &htlcTx,
 	}
 
-	// Assert that the swap is failed in case of an invalid amount.
-	invalidAmt := externalValue != 0 && externalValue != int64(req.Amount)
-	if invalidAmt {
-		ctx.assertState(loopdb.StateFailIncorrectHtlcAmt)
-		ctx.store.AssertLoopInState(loopdb.StateFailIncorrectHtlcAmt)
+	isInvalidAmt := externalValue != 0 && externalValue != int64(req.Amount)
+	handleHtlcExpiry(
+		t, ctx, inSwap, htlcTx, cost, errChan, isInvalidAmt,
+	)
+}
 
-		require.NoError(t, <-errChan)
-		return
+func handleHtlcExpiry(t *testing.T, ctx *loopInTestContext, inSwap *loopInSwap,
+	htlcTx wire.MsgTx, cost loopdb.SwapCost, errChan chan error,
+	isInvalidAmount bool) {
+
+	if isInvalidAmount {
+		ctx.store.AssertLoopInState(loopdb.StateFailIncorrectHtlcAmt)
+		ctx.assertState(loopdb.StateFailIncorrectHtlcAmt)
 	}
 
 	// Client starts listening for spend of htlc.
@@ -329,8 +334,16 @@ func testLoopInTimeout(t *testing.T, externalValue int64) {
 	// Signal that the invoice was canceled.
 	ctx.updateInvoiceState(0, invpkg.ContractCanceled)
 
-	ctx.assertState(loopdb.StateFailTimeout)
-	state := ctx.store.AssertLoopInState(loopdb.StateFailTimeout)
+	var state loopdb.SwapStateData
+	if isInvalidAmount {
+		state = ctx.store.AssertLoopInState(
+			loopdb.StateFailIncorrectHtlcAmtSwept,
+		)
+		ctx.assertState(loopdb.StateFailIncorrectHtlcAmtSwept)
+	} else {
+		ctx.assertState(loopdb.StateFailTimeout)
+		state = ctx.store.AssertLoopInState(loopdb.StateFailTimeout)
+	}
 	require.Equal(t, cost, state.Cost)
 
 	require.NoError(t, <-errChan)