Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

loopin: sweep incorrect amount on timeout #676

Merged
merged 4 commits into from
Feb 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions loopd/swapclient_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
15 changes: 10 additions & 5 deletions loopdb/swapstate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
}

Expand All @@ -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.
Expand Down Expand Up @@ -160,6 +162,9 @@ func (s SwapState) String() string {
case StateFailInsufficientConfirmedBalance:
return "InsufficientConfirmedBalance"

case StateFailIncorrectHtlcAmtSwept:
return "StateFailIncorrectHtlcAmtSwept"

default:
return "Unknown"
}
Expand Down
25 changes: 21 additions & 4 deletions loopin.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can leave this part in, but just not return here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would then show the client in the cli that something is wrong and it's waiting to expire (maybe we should add expiry to the text and actually show the expiry in the clie)

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
Expand Down Expand Up @@ -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.
Expand Down
31 changes: 22 additions & 9 deletions loopin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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)
Expand Down
Loading
Loading