From 9db8bd5f2a5817218c5128d5e65af4391b7da7bf Mon Sep 17 00:00:00 2001 From: carla Date: Wed, 12 May 2021 14:09:28 +0200 Subject: [PATCH] loopout: do not reveal preimage if time to safe reveal has passed --- loopout.go | 42 +++++++++++++++++++++++++++++++++++++++--- loopout_test.go | 30 ++++++++++++++++-------------- 2 files changed, 55 insertions(+), 17 deletions(-) diff --git a/loopout.go b/loopout.go index b930354..a90365e 100644 --- a/loopout.go +++ b/loopout.go @@ -438,6 +438,12 @@ func (s *loopOutSwap) executeSwap(globalCtx context.Context) error { return err } + // If spend details are nil, we resolved the swap without waiting for + // its spend, so we can exit. + if spendDetails == nil { + return nil + } + // Inspect witness stack to see if it is a success transaction. We // don't just try to match with the hash of our sweep tx, because it // may be swept by a different (fee) sweep tx from a previous run. @@ -854,6 +860,14 @@ func (s *loopOutSwap) waitForHtlcSpendConfirmed(globalCtx context.Context, return nil, err } + // If the result of our spend func was that the swap + // has reached a final state, then we return nil spend + // details, because there is no further action required + // for this swap. + if s.state.Type() != loopdb.StateTypePending { + return nil, nil + } + // If our off chain payment is not yet complete, we // try to push our preimage to the server. if !paymentComplete { @@ -889,7 +903,9 @@ func (s *loopOutSwap) pushPreimage(ctx context.Context) { // sweep tries to sweep the given htlc to a destination address. It takes into // account the max miner fee and marks the preimage as revealed when it -// published the tx. +// published the tx. If the preimage has not yet been revealed, and the time +// during which we can safely reveal it has passed, the swap will be marked +// as failed, and the function will return. // // TODO: Use lnd sweeper? func (s *loopOutSwap) sweep(ctx context.Context, @@ -900,16 +916,36 @@ func (s *loopOutSwap) sweep(ctx context.Context, return s.htlc.GenSuccessWitness(sig, s.Preimage) } + remainingBlocks := s.CltvExpiry - s.height + blocksToLastReveal := remainingBlocks - MinLoopOutPreimageRevealDelta + preimageRevealed := s.state == loopdb.StatePreimageRevealed + + // If we have not revealed our preimage, and we don't have time left + // to sweep the swap, we abandon the swap because we can no longer + // sweep the success path (without potentially having to compete with + // the server's timeout sweep), and we have not had any coins pulled + // off-chain. + if blocksToLastReveal <= 0 && !preimageRevealed { + s.log.Infof("Preimage can no longer be safely revealed: "+ + "expires at: %v, current height: %v", s.CltvExpiry, + s.height) + + s.state = loopdb.StateFailTimeout + return nil + } + // Calculate the transaction fee based on the confirmation target // required to sweep the HTLC before the timeout. We'll use the // confirmation target provided by the client unless we've come too // close to the expiration height, in which case we'll use the default // if it is better than what the client provided. confTarget := s.SweepConfTarget - if s.CltvExpiry-s.height <= DefaultSweepConfTargetDelta && + if remainingBlocks <= DefaultSweepConfTargetDelta && confTarget > DefaultSweepConfTarget { + confTarget = DefaultSweepConfTarget } + fee, err := s.sweeper.GetSweepFee( ctx, s.htlc.AddSuccessToEstimator, s.DestAddr, confTarget, ) @@ -922,7 +958,7 @@ func (s *loopOutSwap) sweep(ctx context.Context, s.log.Warnf("Required fee %v exceeds max miner fee of %v", fee, s.MaxMinerFee) - if s.state == loopdb.StatePreimageRevealed { + if preimageRevealed { // The currently required fee exceeds the max, but we // already revealed the preimage. The best we can do now // is to republish with the max fee. diff --git a/loopout_test.go b/loopout_test.go index c3a189b..1efb9d6 100644 --- a/loopout_test.go +++ b/loopout_test.go @@ -583,8 +583,8 @@ func TestPreimagePush(t *testing.T) { } // TestExpiryBeforeReveal tests the case where the on-chain HTLC expires before -// we have revealed our preimage. This test demonstrates that the client will -// erroneously reveal the preimage even though we're nearing our timeout. +// we have revealed our preimage, demonstrating that we do not reveal our +// preimage once we've reached our expiry height. func TestExpiryBeforeReveal(t *testing.T) { defer test.Guard(t)() @@ -622,9 +622,8 @@ func TestExpiryBeforeReveal(t *testing.T) { } errChan := make(chan error) - cancelCtx, cancel := context.WithCancel(context.Background()) go func() { - err := swap.execute(cancelCtx, &executeConfig{ + err := swap.execute(context.Background(), &executeConfig{ statusChan: statusChan, blockEpochChan: blockEpochChan, timerFactory: timerFactory, @@ -653,7 +652,8 @@ func TestExpiryBeforeReveal(t *testing.T) { ctx.AssertRegisterConf(false, defaultConfirmations) // Advance the block height to get the HTLC confirmed. - blockEpochChan <- ctx.Lnd.Height + 1 + height := ctx.Lnd.Height + 1 + blockEpochChan <- height htlcTx := wire.NewMsgTx(2) htlcTx.AddTxOut(&wire.TxOut{ @@ -681,17 +681,19 @@ func TestExpiryBeforeReveal(t *testing.T) { // Advance the block height to the point where we would do timeout // instead of pushing the preimage. - blockEpochChan <- lnd.Height + testReq.Expiry + blockEpochChan <- testReq.Expiry + height - // Tick our expiry chan again, this time we expect the swap to - // publish our sweep timeout, despite expiry having passed, and the - // potential for a race with the server. + // Tick our expiry channel again to trigger another sweep attempt. expiryChan <- testTime - // Expect a signing request for the HTLC success transaction. - <-ctx.Lnd.SignOutputRawChannel + // We should see our swap marked as failed. + cfg.store.(*storeMock).assertLoopOutState( + loopdb.StateFailTimeout, + ) + status := <-statusChan + require.Equal( + t, status.State, loopdb.StateFailTimeout, + ) - // We just cancel the swap now rather than testing to completion. - cancel() - require.Equal(t, context.Canceled, <-errChan) + require.Nil(t, <-errChan) }