diff --git a/docs/release-notes/release-notes-next.md b/docs/release-notes/release-notes-next.md index 8ea78eedd..27a5c4e9e 100644 --- a/docs/release-notes/release-notes-next.md +++ b/docs/release-notes/release-notes-next.md @@ -15,6 +15,12 @@ already using, and a failed lock no longer resets deposits that a withdrawal or a channel open has taken. +* The static address Loop In timeout sweep now spends the exact HTLC output + confirmed on chain, including server-published backup fee variants. + Previously it always targeted the standard fee variant, so a confirmed high + fee or extremely high fee HTLC transaction could not be swept back after the + HTLC timed out. Sweeps that would leave a dust output are no longer signed. + #### Maintenance #### Contributors (Alphabetical Order) diff --git a/staticaddr/loopin/actions.go b/staticaddr/loopin/actions.go index 09aead3fb..6c478c075 100644 --- a/staticaddr/loopin/actions.go +++ b/staticaddr/loopin/actions.go @@ -1518,10 +1518,18 @@ func (f *FSM) MonitorInvoiceAndHtlcTxAction(ctx context.Context, } } -// htlcTimeoutSweepRetryDelay is the delay between retries when publishing the -// htlc timeout sweep transaction fails. +// htlcTimeoutSweepRetryDelay is the delay between retries when looking up the +// htlc tx or creating or publishing the htlc timeout sweep transaction fails. const htlcTimeoutSweepRetryDelay = time.Hour +// defaultHtlcTxLookupTimeout is the default maximum time to wait for lnd to +// report a spend when looking up the htlc tx to sweep, or the tx that already +// spent the htlc output. The wait ends as soon as a confirmed spend is +// reported, which is right away when the spend is already confirmed. After the +// timeout, lnd's wallet is checked for a confirmed spend that lnd didn't +// report. +const defaultHtlcTxLookupTimeout = 30 * time.Minute + // SweepHtlcTimeoutAction is called if the server published the htlc tx without // paying the invoice. We wait for the timeout path to open up and sweep the // funds back to us. @@ -1534,6 +1542,13 @@ func (f *FSM) SweepHtlcTimeoutAction(ctx context.Context, return OnHtlcTimeoutSweepPublished } + // Keep the current state on shutdown so recovery resumes the + // sweep after restart. + if ctx.Err() != nil { + return fsm.NoOp + } + + f.LastActionError = err f.Errorf("unable to create and publish htlc timeout sweep "+ "tx: %v, retrying in %v", err, htlcTimeoutSweepRetryDelay) @@ -1673,6 +1688,13 @@ func (f *FSM) unlockDeposits(ctx context.Context) error { // createAndPublishHtlcTimeoutSweepTx creates and publishes the htlc timeout // sweep transaction. func (f *FSM) createAndPublishHtlcTimeoutSweepTx(ctx context.Context) error { + // Look up the htlc tx first. This can take a while, so the fee rate + // and the block height are only fetched once it is known. + htlcTx, err := f.fetchHtlcTx(ctx) + if err != nil { + return err + } + // Get a fee rate. feeRate, err := f.cfg.WalletKit.EstimateFeeRate(ctx, defaultConfTarget) if err != nil { @@ -1686,9 +1708,8 @@ func (f *FSM) createAndPublishHtlcTimeoutSweepTx(ctx context.Context) error { // Create htlc timeout transaction. timeoutTx, err := f.loopIn.createHtlcSweepTx( - ctx, f.cfg.Signer, f.loopIn.HtlcTimeoutSweepAddress, feeRate, - f.cfg.ChainParams, getInfo.BlockHeight, - f.cfg.MaxStaticAddrHtlcFeePercentage, + ctx, f.cfg.Signer, htlcTx, f.loopIn.HtlcTimeoutSweepAddress, + feeRate, f.cfg.ChainParams, getInfo.BlockHeight, ) if err != nil { return fmt.Errorf("unable to create htlc timeout sweep tx: %w", @@ -1710,17 +1731,165 @@ func (f *FSM) createAndPublishHtlcTimeoutSweepTx(ctx context.Context) error { f.LastActionError = err return err } - } else { - f.Debugf("published htlc timeout sweep with txid: %v", - timeoutTx.TxHash()) - hash := timeoutTx.TxHash() + // lnd also reports missing inputs and mempool conflicts this + // way. With the invoice canceled, only our sweeps can spend the + // htlc output, so monitor the confirmed spender. + spendingTx, spendErr := f.htlcSpendingTx( + ctx, timeoutTx.TxIn[0].PreviousOutPoint, + ) + if spendErr != nil { + return fmt.Errorf("%v: %w, and no confirmed spend of the "+ + "htlc output was found: %w", txLabel, err, spendErr) + } + + // The monitor only sees txs paying the sweep address. + hash := spendingTx.TxHash() + paysSweepAddr, _ := input.FindScriptOutputIndex( + spendingTx, timeoutTx.TxOut[0].PkScript, + ) + if !paysSweepAddr { + return fmt.Errorf("%v: htlc output spent by tx %v, which "+ + "doesn't pay the sweep address", txLabel, hash) + } + + f.Infof("htlc output already spent by sweep tx %v", hash) f.loopIn.HtlcTimeoutSweepTxHash = &hash + + return nil } + f.Debugf("published htlc timeout sweep with txid: %v", + timeoutTx.TxHash()) + + hash := timeoutTx.TxHash() + f.loopIn.HtlcTimeoutSweepTxHash = &hash + return nil } +// htlcSpendingTx waits for the spend of the given htlc output and returns the +// spending tx. +func (f *FSM) htlcSpendingTx(ctx context.Context, + htlcOutpoint wire.OutPoint) (*wire.MsgTx, error) { + + htlc, err := f.loopIn.getHtlc(f.cfg.ChainParams) + if err != nil { + return nil, err + } + + return f.waitForSpend( + ctx, htlcOutpoint, htlc.PkScript, "htlc output", "sweep tx", + ) +} + +// fetchHtlcTx waits for the spend of the swap's first deposit and returns the +// spending htlc tx. The server may publish any of the htlc tx variants the +// client co-signed, and only the standard fee rate is persisted, so the htlc +// tx that is swept is taken from the chain instead of being rebuilt. +func (f *FSM) fetchHtlcTx(ctx context.Context) (*wire.MsgTx, error) { + outpoints := f.loopIn.Outpoints() + if len(outpoints) == 0 { + return nil, errors.New("loop-in has no deposits") + } + + if f.loopIn.AddressParams == nil { + return nil, errors.New("loop-in has no static address " + + "parameters") + } + + return f.waitForSpend( + ctx, outpoints[0], f.loopIn.AddressParams.PkScript, "deposit", + "htlc tx", + ) +} + +// waitForSpend returns the confirmed tx that spends the outpoint. If lnd +// doesn't report the spend within the htlc tx lookup timeout, lnd's wallet is +// checked before giving up. The labels are for logs and errors. +func (f *FSM) waitForSpend(ctx context.Context, outpoint wire.OutPoint, + pkScript []byte, description, spender string) (*wire.MsgTx, error) { + + lookupTimeout := f.cfg.HtlcTxLookupTimeout + if lookupTimeout == 0 { + lookupTimeout = defaultHtlcTxLookupTimeout + } + + spendCtx, cancel := context.WithTimeout(ctx, lookupTimeout) + defer cancel() + + spendChan, errChan, err := f.cfg.ChainNotifier.RegisterSpendNtfn( + spendCtx, &outpoint, pkScript, int32(f.loopIn.InitiationHeight), + ) + if err != nil { + return nil, fmt.Errorf("unable to register for the spend of "+ + "%s %v: %w", description, outpoint, err) + } + + f.Infof("waiting for the %s that spends %s %v, height hint %v", + spender, description, outpoint, f.loopIn.InitiationHeight) + + select { + case spend := <-spendChan: + if spend == nil || spend.SpendingTx == nil { + return nil, fmt.Errorf("spend of %s %v has no spending "+ + "tx", description, outpoint) + } + + f.Infof("%s %v spent by %s %v", description, outpoint, spender, + spend.SpendingTx.TxHash()) + + return spend.SpendingTx, nil + + case err := <-errChan: + return nil, fmt.Errorf("unable to watch the spend of %s %v: %w", + description, outpoint, err) + + case <-spendCtx.Done(): + if ctx.Err() != nil { + return nil, ctx.Err() + } + + // If lnd's historical spend scan fails, lnd neither reports + // the error nor starts a new scan when we register again, so a + // past spend is only found after lnd restarts. See + // lightningnetwork/lnd#11234. Check lnd's wallet instead. + tx := f.walletSpendingTx(ctx, outpoint, description) + if tx != nil { + f.Infof("%s %v spent by %s %v, found in the wallet", + description, outpoint, spender, tx.TxHash()) + + return tx, nil + } + + return nil, fmt.Errorf("%s %v was not spent within %v", + description, outpoint, lookupTimeout) + } +} + +// walletSpendingTx returns a confirmed tx from lnd's wallet that spends the +// outpoint, or nil if the wallet lists none or the lookup fails. +func (f *FSM) walletSpendingTx(ctx context.Context, outpoint wire.OutPoint, + description string) *wire.MsgTx { + + if f.cfg.TxOutChecker == nil { + return nil + } + + tx, err := f.cfg.TxOutChecker.ConfirmedSpendingTx( + ctx, outpoint, + htlcTxSearchStartHeight(f.loopIn.InitiationHeight), + ) + if err != nil { + f.Warnf("unable to look up the spend of %s %v in the "+ + "wallet: %v", description, outpoint, err) + + return nil + } + + return tx +} + // toNonces converts a byte slice to a 66 byte slice. func toNonces(nonces [][]byte) ([][musig2.PubNonceSize]byte, error) { res := make([][musig2.PubNonceSize]byte, 0, len(nonces)) diff --git a/staticaddr/loopin/actions_test.go b/staticaddr/loopin/actions_test.go index 198deac34..1187cba2f 100644 --- a/staticaddr/loopin/actions_test.go +++ b/staticaddr/loopin/actions_test.go @@ -3863,6 +3863,14 @@ type recordingTxOutChecker struct { spendOutpoints [][]wire.OutPoint spendPkScripts [][]byte spendStartHeights []int32 + + // spendingTxs is what ConfirmedSpendingTx returns per outpoint. + spendingTxs map[wire.OutPoint]*wire.MsgTx + + // spendingTxOutpoints and spendingTxStartHeights record what + // ConfirmedSpendingTx was asked about. + spendingTxOutpoints []wire.OutPoint + spendingTxStartHeights []int32 } // GetTxOuts records the request and returns the configured available outputs. @@ -3897,6 +3905,27 @@ func (r *recordingTxOutChecker) SpentByTxPayingTo(_ context.Context, return r.htlcSpent, nil } +// ConfirmedSpendingTx records the outpoint and start height and returns the +// configured spending tx, or an error if the context is done. +func (r *recordingTxOutChecker) ConfirmedSpendingTx(ctx context.Context, + outpoint wire.OutPoint, startHeight int32) (*wire.MsgTx, error) { + + r.spendingTxOutpoints = append(r.spendingTxOutpoints, outpoint) + r.spendingTxStartHeights = append( + r.spendingTxStartHeights, startHeight, + ) + if r.err != nil { + return nil, r.err + } + + // Like the lnd RPC, fail on a done context. + if err := ctx.Err(); err != nil { + return nil, err + } + + return r.spendingTxs[outpoint], nil +} + // initHtlcTestServer lets InitHtlcAction tests inject a deterministic server // response without standing up the full gRPC client. type initHtlcTestServer struct { diff --git a/staticaddr/loopin/htlc_timeout_sweep_test.go b/staticaddr/loopin/htlc_timeout_sweep_test.go new file mode 100644 index 000000000..9a0ca2e46 --- /dev/null +++ b/staticaddr/loopin/htlc_timeout_sweep_test.go @@ -0,0 +1,1017 @@ +package loopin + +import ( + "context" + "errors" + "testing" + "time" + + "github.com/btcsuite/btcd/btcec/v2" + "github.com/btcsuite/btcd/btcutil" + "github.com/btcsuite/btcd/chaincfg/chainhash" + "github.com/btcsuite/btcd/txscript" + "github.com/btcsuite/btcd/wire" + "github.com/lightninglabs/lndclient" + "github.com/lightninglabs/loop/fsm" + "github.com/lightninglabs/loop/staticaddr/deposit" + "github.com/lightninglabs/loop/staticaddr/script" + "github.com/lightninglabs/loop/staticaddr/version" + "github.com/lightninglabs/loop/test" + "github.com/lightningnetwork/lnd/chainntnfs" + "github.com/lightningnetwork/lnd/input" + "github.com/lightningnetwork/lnd/lntypes" + "github.com/lightningnetwork/lnd/lnwallet" + "github.com/lightningnetwork/lnd/lnwallet/chainfee" + "github.com/stretchr/testify/require" +) + +const ( + testStandardHtlcFeeRate = chainfee.SatPerKWeight(200) + testHighHtlcFeeRate = chainfee.SatPerKWeight(1_000) + testExtremeHtlcFeeRate = chainfee.SatPerKWeight(4_000) + testMaxHtlcFeePct = 0.2 + testMaxHtlcBackupFeePct = 0.5 +) + +// notSpentErr is part of the error of a spend lookup that timed out. +const notSpentErr = "was not spent within" + +// recordingSweepSigner records the sign descriptors of the htlc timeout sweep +// and returns dummy signatures. +type recordingSweepSigner struct { + lndclient.SignerClient + + descs chan *lndclient.SignDescriptor +} + +// SignOutputRaw records the sign descriptors and returns dummy 64-byte +// signatures. +func (s *recordingSweepSigner) SignOutputRaw(_ context.Context, _ *wire.MsgTx, + descs []*lndclient.SignDescriptor, _ []*wire.TxOut) ([][]byte, error) { + + sigs := make([][]byte, len(descs)) + for i, desc := range descs { + sigs[i] = make([]byte, 64) + + select { + case s.descs <- desc: + default: + } + } + + return sigs, nil +} + +// newHtlcVariantTestLoopIn returns a loop-in over two deposits with a change +// output and the three htlc tx variants the client co-signs for it. +func newHtlcVariantTestLoopIn(t *testing.T, + mockLnd *test.LndMockServices) (*StaticAddressLoopIn, + map[chainfee.SatPerKWeight]*wire.MsgTx) { + + t.Helper() + + clientKey, err := btcec.NewPrivateKey() + require.NoError(t, err) + serverKey, err := btcec.NewPrivateKey() + require.NoError(t, err) + + staticAddr, err := newStaticAddress( + clientKey.PubKey(), serverKey.PubKey(), 4032, + ) + require.NoError(t, err) + + pkScript, err := staticAddr.StaticAddressScript() + require.NoError(t, err) + + deposits := []*deposit.Deposit{ + { + Hash: chainhash.Hash{0x11}, + Index: 1, + Value: btcutil.Amount(600_000), + }, + { + Hash: chainhash.Hash{0x22}, + Index: 0, + Value: btcutil.Amount(400_000), + }, + } + + sweepAddr, err := btcutil.NewAddressTaproot( + make([]byte, 32), mockLnd.ChainParams, + ) + require.NoError(t, err) + + loopIn := &StaticAddressLoopIn{ + SwapHash: lntypes.Hash{7, 7, 7}, + HtlcCltvExpiry: mockLnd.Height + 10, + InitiationHeight: uint32(mockLnd.Height), + InitiationTime: time.Now(), + ProtocolVersion: version.ProtocolVersion_V0, + ClientPubkey: clientKey.PubKey(), + ServerPubkey: serverKey.PubKey(), + Deposits: deposits, + SelectedAmount: btcutil.Amount(800_000), + AddressParams: &script.Parameters{ + ClientPubkey: clientKey.PubKey(), + ServerPubkey: serverKey.PubKey(), + PkScript: pkScript, + Expiry: 4032, + ProtocolVersion: version.ProtocolVersion_V0, + }, + HtlcTxFeeRate: testStandardHtlcFeeRate, + HtlcTxHighFeeRate: testHighHtlcFeeRate, + HtlcTxExtremelyHighFeeRate: testExtremeHtlcFeeRate, + HtlcTimeoutSweepAddress: sweepAddr, + PaymentTimeoutSeconds: 3_600, + } + + variants := make(map[chainfee.SatPerKWeight]*wire.MsgTx, 3) + for _, rate := range []chainfee.SatPerKWeight{ + testStandardHtlcFeeRate, testHighHtlcFeeRate, + testExtremeHtlcFeeRate, + } { + maxFeePct := testMaxHtlcBackupFeePct + if rate == testStandardHtlcFeeRate { + maxFeePct = testMaxHtlcFeePct + } + + htlcTx, err := loopIn.createHtlcTx( + mockLnd.ChainParams, rate, maxFeePct, + ) + require.NoError(t, err) + + variants[rate] = htlcTx + } + + return loopIn, variants +} + +// TestHtlcTimeoutSweepSpendsPublishedVariant verifies that the htlc timeout +// sweep spends the htlc output of the htlc tx variant that spent the deposits +// on chain. The client co-signs a standard, a high fee and an extremely high +// fee variant and the server may publish any of them. Only the standard fee +// rate is persisted, so the sweep must not depend on the backup fee rates +// either. +func TestHtlcTimeoutSweepSpendsPublishedVariant(t *testing.T) { + tests := []struct { + name string + rate chainfee.SatPerKWeight + }{ + { + name: "standard fee variant", + rate: testStandardHtlcFeeRate, + }, + { + name: "high fee variant", + rate: testHighHtlcFeeRate, + }, + { + name: "extremely high fee variant", + rate: testExtremeHtlcFeeRate, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + testHtlcTimeoutSweepSpendsVariant(t, tc.rate) + }) + } +} + +// testHtlcTimeoutSweepSpendsVariant publishes the htlc timeout sweep of a +// recovered loop-in after the htlc tx variant with the given fee rate spent +// the deposits. +func testHtlcTimeoutSweepSpendsVariant(t *testing.T, + publishedRate chainfee.SatPerKWeight) { + + ctx, cancel := context.WithTimeout(t.Context(), 10*time.Second) + defer cancel() + + mockLnd := test.NewMockLnd() + loopIn, variants := newHtlcVariantTestLoopIn(t, mockLnd) + + // The three variants are distinct transactions. + require.NotEqual( + t, variants[testStandardHtlcFeeRate].TxHash(), + variants[testHighHtlcFeeRate].TxHash(), + ) + require.NotEqual( + t, variants[testHighHtlcFeeRate].TxHash(), + variants[testExtremeHtlcFeeRate].TxHash(), + ) + + publishedTx := variants[publishedRate] + + // Simulate a restart. The backup fee rates are not persisted, so a + // recovered loop-in only knows the standard fee rate. + loopIn.HtlcTxHighFeeRate = 0 + loopIn.HtlcTxExtremelyHighFeeRate = 0 + + signer := &recordingSweepSigner{ + descs: make(chan *lndclient.SignDescriptor, 1), + } + + f := newHtlcSweepTestFSM(t, ctx, mockLnd, loopIn, signer) + + resultChan := make(chan fsm.EventType, 1) + go func() { + resultChan <- f.SweepHtlcTimeoutAction(ctx, nil) + }() + + // The sweep looks up the tx that spent the first deposit. + spendReg := receiveDepositSpendReg(t, ctx, mockLnd, loopIn) + require.Equal(t, loopIn.AddressParams.PkScript, spendReg.PkScript) + require.Equal(t, int32(loopIn.InitiationHeight), spendReg.HeightHint) + + deliverDepositSpend(t, ctx, spendReg, publishedTx) + + var sweepTx *wire.MsgTx + select { + case sweepTx = <-mockLnd.TxPublishChannel: + case <-ctx.Done(): + t.Fatalf("htlc timeout sweep not published: %v", ctx.Err()) + } + + select { + case event := <-resultChan: + require.Equal(t, OnHtlcTimeoutSweepPublished, event) + + case <-ctx.Done(): + t.Fatalf("sweep action did not finish: %v", ctx.Err()) + } + + htlc, err := loopIn.getHtlc(mockLnd.ChainParams) + require.NoError(t, err) + + // The htlc output is at index 0 and the change output at index 1. + require.Len(t, publishedTx.TxOut, 2) + require.Equal(t, htlc.PkScript, publishedTx.TxOut[0].PkScript) + + require.Len(t, sweepTx.TxIn, 1) + require.Equal(t, wire.OutPoint{ + Hash: publishedTx.TxHash(), + Index: 0, + }, sweepTx.TxIn[0].PreviousOutPoint) + + sweepHash := sweepTx.TxHash() + require.Equal(t, &sweepHash, loopIn.HtlcTimeoutSweepTxHash) + + // The sweep is signed for the value of the published htlc output. + select { + case desc := <-signer.descs: + require.Equal(t, publishedTx.TxOut[0].Value, desc.Output.Value) + require.Equal(t, htlc.PkScript, desc.Output.PkScript) + + default: + t.Fatalf("htlc timeout sweep was not signed") + } + require.Less(t, sweepTx.TxOut[0].Value, publishedTx.TxOut[0].Value) +} + +// TestHtlcOutput verifies that only a tx that spends exactly the swap's +// deposits and pays the htlc once is accepted as htlc tx. +func TestHtlcOutput(t *testing.T) { + t.Parallel() + + mockLnd := test.NewMockLnd() + loopIn, variants := newHtlcVariantTestLoopIn(t, mockLnd) + + htlc, err := loopIn.getHtlc(mockLnd.ChainParams) + require.NoError(t, err) + + tests := []struct { + name string + modify func(tx *wire.MsgTx) *wire.MsgTx + htlcIndex uint32 + err string + }{ + { + name: "published variant", + modify: func(tx *wire.MsgTx) *wire.MsgTx { + return tx + }, + }, + { + name: "deposits in a different order", + modify: func(tx *wire.MsgTx) *wire.MsgTx { + tx.TxIn[0], tx.TxIn[1] = tx.TxIn[1], tx.TxIn[0] + return tx + }, + }, + { + name: "htlc output after the change output", + modify: func(tx *wire.MsgTx) *wire.MsgTx { + out := tx.TxOut + out[0], out[1] = out[1], out[0] + + return tx + }, + htlcIndex: 1, + }, + { + name: "no tx", + modify: func(*wire.MsgTx) *wire.MsgTx { + return nil + }, + err: "no htlc tx provided", + }, + { + name: "missing deposit", + modify: func(tx *wire.MsgTx) *wire.MsgTx { + tx.TxIn = tx.TxIn[:1] + return tx + }, + err: "expected the 2 deposits", + }, + { + name: "same deposit twice", + modify: func(tx *wire.MsgTx) *wire.MsgTx { + tx.TxIn[1].PreviousOutPoint = + tx.TxIn[0].PreviousOutPoint + + return tx + }, + err: "not a remaining deposit", + }, + { + name: "foreign input", + modify: func(tx *wire.MsgTx) *wire.MsgTx { + tx.TxIn[1].PreviousOutPoint.Index++ + return tx + }, + err: "not a remaining deposit", + }, + { + name: "no htlc output", + modify: func(tx *wire.MsgTx) *wire.MsgTx { + tx.TxOut = tx.TxOut[1:] + return tx + }, + err: "has no htlc output", + }, + { + name: "two htlc outputs", + modify: func(tx *wire.MsgTx) *wire.MsgTx { + tx.TxOut[1].PkScript = htlc.PkScript + return tx + }, + err: "more than one htlc output", + }, + { + name: "htlc output without value", + modify: func(tx *wire.MsgTx) *wire.MsgTx { + tx.TxOut[0].Value = 0 + return tx + }, + err: "without value", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + htlcTx := tc.modify( + variants[testHighHtlcFeeRate].Copy(), + ) + + outpoint, txOut, err := loopIn.htlcOutput( + htlcTx, htlc.PkScript, + ) + if tc.err != "" { + require.ErrorContains(t, err, tc.err) + return + } + + require.NoError(t, err) + require.Equal(t, wire.OutPoint{ + Hash: htlcTx.TxHash(), + Index: tc.htlcIndex, + }, outpoint) + require.Equal(t, htlcTx.TxOut[tc.htlcIndex], txOut) + }) + } +} + +// TestCreateHtlcSweepTxRejectsDust verifies that the sweep is only signed if +// the fee leaves an output at or above the dust limit. +func TestCreateHtlcSweepTxRejectsDust(t *testing.T) { + t.Parallel() + + const feeRate = chainfee.SatPerKWeight(50_000) + + mockLnd := test.NewMockLnd() + loopIn, variants := newHtlcVariantTestLoopIn(t, mockLnd) + + htlc, err := loopIn.getHtlc(mockLnd.ChainParams) + require.NoError(t, err) + + var weightEstimator input.TxWeightEstimator + weightEstimator.AddP2TROutput() + require.NoError(t, htlc.AddSuccessToEstimator(&weightEstimator)) + + fee := int64(feeRate.FeeForWeight(weightEstimator.Weight())) + dustLimit := int64(lnwallet.DustLimitForSize(input.P2TRSize)) + + tests := []struct { + name string + htlcValue int64 + err string + }{ + { + name: "output at dust limit", + htlcValue: fee + dustLimit, + }, + { + name: "output below dust limit", + htlcValue: fee + dustLimit - 1, + err: "too small to pay the sweep fee", + }, + { + name: "fee above htlc value", + htlcValue: fee - 1, + err: "too small to pay the sweep fee", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + htlcTx := variants[testExtremeHtlcFeeRate].Copy() + htlcTx.TxOut[0].Value = tc.htlcValue + + sweepTx, err := loopIn.createHtlcSweepTx( + t.Context(), &noopSigner{}, htlcTx, + loopIn.HtlcTimeoutSweepAddress, feeRate, + mockLnd.ChainParams, + uint32(loopIn.HtlcCltvExpiry), + ) + if tc.err != "" { + require.ErrorContains(t, err, tc.err) + return + } + + require.NoError(t, err) + require.Equal( + t, dustLimit, sweepTx.TxOut[0].Value, + ) + }) + } +} + +// newHtlcSweepTestFSM returns a loop-in FSM that can publish the htlc timeout +// sweep of the given loop-in against the mock lnd. +func newHtlcSweepTestFSM(t *testing.T, ctx context.Context, + mockLnd *test.LndMockServices, loopIn *StaticAddressLoopIn, + signer lndclient.SignerClient) *FSM { + + t.Helper() + + cfg := &Config{ + AddressManager: &mockAddressManager{ + params: loopIn.AddressParams, + }, + ChainNotifier: mockLnd.ChainNotifier, + DepositManager: &noopDepositManager{}, + LndClient: mockLnd.Client, + ChainParams: mockLnd.ChainParams, + WalletKit: mockLnd.WalletKit, + Signer: signer, + MaxStaticAddrHtlcFeePercentage: testMaxHtlcFeePct, + MaxStaticAddrHtlcBackupFeePercentage: testMaxHtlcBackupFeePct, + } + + f, err := NewFSM(ctx, loopIn, cfg, false) + require.NoError(t, err) + + return f +} + +// receiveDepositSpendReg waits for the spend registration of the first +// deposit of the loop-in. +func receiveDepositSpendReg(t *testing.T, ctx context.Context, + mockLnd *test.LndMockServices, + loopIn *StaticAddressLoopIn) *test.SpendRegistration { + + t.Helper() + + select { + case spendReg := <-mockLnd.RegisterSpendChannel: + require.Equal( + t, loopIn.Deposits[0].OutPoint, *spendReg.Outpoint, + ) + + return spendReg + + case <-ctx.Done(): + t.Fatalf("deposit spend registration not received: %v", + ctx.Err()) + + return nil + } +} + +// deliverDepositSpend reports the given tx as the spend of the registered +// deposit. +func deliverDepositSpend(t *testing.T, ctx context.Context, + spendReg *test.SpendRegistration, spendingTx *wire.MsgTx) { + + t.Helper() + + select { + case spendReg.SpendChannel <- &chainntnfs.SpendDetail{ + SpentOutPoint: spendReg.Outpoint, + SpendingTx: spendingTx, + }: + + case <-ctx.Done(): + t.Fatalf("unable to deliver deposit spend: %v", ctx.Err()) + } +} + +// TestHtlcTimeoutSweepFeeRateAfterLookup verifies that the sweep fee rate is +// estimated after the htlc tx was found, not before waiting for it. +func TestHtlcTimeoutSweepFeeRateAfterLookup(t *testing.T) { + ctx, cancel := context.WithTimeout(t.Context(), 10*time.Second) + defer cancel() + + const ( + staleRate = chainfee.SatPerKWeight(253) + freshRate = chainfee.SatPerKWeight(5_000) + ) + + mockLnd := test.NewMockLnd() + mockLnd.SetFeeEstimate(defaultConfTarget, staleRate) + + loopIn, variants := newHtlcVariantTestLoopIn(t, mockLnd) + htlcTx := variants[testHighHtlcFeeRate] + f := newHtlcSweepTestFSM(t, ctx, mockLnd, loopIn, &noopSigner{}) + + resultChan := make(chan fsm.EventType, 1) + go func() { + resultChan <- f.SweepHtlcTimeoutAction(ctx, nil) + }() + + spendReg := receiveDepositSpendReg(t, ctx, mockLnd, loopIn) + + // Fees rise while the sweep waits for the htlc tx. + mockLnd.SetFeeEstimate(defaultConfTarget, freshRate) + deliverDepositSpend(t, ctx, spendReg, htlcTx) + + var sweepTx *wire.MsgTx + select { + case sweepTx = <-mockLnd.TxPublishChannel: + case <-ctx.Done(): + t.Fatalf("htlc timeout sweep not published: %v", ctx.Err()) + } + require.Equal(t, OnHtlcTimeoutSweepPublished, <-resultChan) + + htlc, err := loopIn.getHtlc(mockLnd.ChainParams) + require.NoError(t, err) + + var weightEstimator input.TxWeightEstimator + weightEstimator.AddP2TROutput() + require.NoError(t, htlc.AddSuccessToEstimator(&weightEstimator)) + + fee := htlcTx.TxOut[0].Value - sweepTx.TxOut[0].Value + require.Equal( + t, int64(freshRate.FeeForWeight(weightEstimator.Weight())), + fee, + ) +} + +// TestFetchHtlcTxFailures verifies that the htlc tx lookup fails the sweep +// attempt instead of waiting forever or sweeping without an htlc tx. +func TestFetchHtlcTxFailures(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + lookupTimeout time.Duration + deliver func(ctx context.Context, shutdown func(), + spendReg *test.SpendRegistration) bool + err string + errorIs error + }{ + { + name: "notifier error", + deliver: func(ctx context.Context, _ func(), + spendReg *test.SpendRegistration) bool { + + select { + case spendReg.ErrChan <- errors.New("closed"): + return true + + case <-ctx.Done(): + return false + } + }, + err: "closed", + }, + { + name: "spend without tx", + deliver: func(ctx context.Context, _ func(), + spendReg *test.SpendRegistration) bool { + + spend := &chainntnfs.SpendDetail{} + select { + case spendReg.SpendChannel <- spend: + return true + + case <-ctx.Done(): + return false + } + }, + err: "has no spending tx", + }, + { + name: "deposit not spent", + lookupTimeout: 100 * time.Millisecond, + deliver: func(context.Context, func(), + *test.SpendRegistration) bool { + + return true + }, + err: notSpentErr, + }, + { + name: "shutdown", + deliver: func(_ context.Context, shutdown func(), + _ *test.SpendRegistration) bool { + + shutdown() + return true + }, + errorIs: context.Canceled, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + ctx, cancel := context.WithTimeout( + t.Context(), 10*time.Second, + ) + defer cancel() + + mockLnd := test.NewMockLnd() + loopIn, _ := newHtlcVariantTestLoopIn(t, mockLnd) + f := newHtlcSweepTestFSM( + t, ctx, mockLnd, loopIn, &noopSigner{}, + ) + f.cfg.HtlcTxLookupTimeout = tc.lookupTimeout + + lookupCtx, shutdown := context.WithCancel(ctx) + defer shutdown() + + errChan := make(chan error, 1) + go func() { + _, err := f.fetchHtlcTx(lookupCtx) + errChan <- err + }() + + spendReg := receiveDepositSpendReg( + t, ctx, mockLnd, loopIn, + ) + require.True(t, tc.deliver(ctx, shutdown, spendReg)) + + select { + case err := <-errChan: + if tc.errorIs != nil { + require.ErrorIs(t, err, tc.errorIs) + return + } + + require.ErrorContains(t, err, tc.err) + + case <-ctx.Done(): + t.Fatalf("htlc tx lookup did not fail: %v", + ctx.Err()) + } + }) + } +} + +// TestHtlcTimeoutSweepOutputAlreadySpent verifies that an "output already +// spent" publish error only counts as a published sweep if the htlc output is +// spent by a tx that pays the sweep address, and that this tx is the one that +// is monitored. +func TestHtlcTimeoutSweepOutputAlreadySpent(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + + // spent is set if a tx spends the htlc output. + spent bool + + // paysSweepAddress is set if that tx pays the sweep address. + paysSweepAddress bool + + // inWallet is set if the spend is only found in lnd's wallet, + // because lnd's spend notification never comes. + inWallet bool + + err string + }{ + { + name: "spent by an earlier sweep", + spent: true, + paysSweepAddress: true, + }, + { + name: "spent by a tx paying elsewhere", + spent: true, + err: "doesn't pay the sweep address", + }, + { + name: "earlier sweep only found in the " + + "wallet", + spent: true, + paysSweepAddress: true, + inWallet: true, + }, + { + name: "tx paying elsewhere only found in " + + "the wallet", + spent: true, + inWallet: true, + err: "doesn't pay the sweep address", + }, + { + name: "not spent", + err: "no confirmed spend of the htlc output", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + ctx, cancel := context.WithTimeout( + t.Context(), 10*time.Second, + ) + defer cancel() + + mockLnd := test.NewMockLnd() + mockLnd.PublishHandler = func(context.Context, + *wire.MsgTx, string) error { + + return lnwallet.ErrDoubleSpend + } + + loopIn, variants := newHtlcVariantTestLoopIn(t, mockLnd) + htlcTx := variants[testHighHtlcFeeRate] + f := newHtlcSweepTestFSM( + t, ctx, mockLnd, loopIn, &noopSigner{}, + ) + f.cfg.HtlcTxLookupTimeout = time.Second + + htlc, err := loopIn.getHtlc(mockLnd.ChainParams) + require.NoError(t, err) + htlcOutpoint, _, err := loopIn.htlcOutput( + htlcTx, htlc.PkScript, + ) + require.NoError(t, err) + + // An earlier sweep with another fee, or a tx paying + // elsewhere, spent the output. + sweepPkScript, err := txscript.PayToAddrScript( + loopIn.HtlcTimeoutSweepAddress, + ) + require.NoError(t, err) + pkScript := sweepPkScript + if !tc.paysSweepAddress { + pkScript = []byte{0x00, 0x14, 0x01} + } + + spendingTx := wire.NewMsgTx(2) + spendingTx.AddTxIn(&wire.TxIn{ + PreviousOutPoint: htlcOutpoint, + }) + spendingTx.AddTxOut(&wire.TxOut{ + Value: 1_000, + PkScript: pkScript, + }) + spendingTx.LockTime = 1 + + if tc.inWallet { + walletTxs := map[wire.OutPoint]*wire.MsgTx{ + htlcOutpoint: spendingTx, + } + f.cfg.TxOutChecker = &recordingTxOutChecker{ + spendingTxs: walletTxs, + } + } + + errChan := make(chan error, 1) + go func() { + errChan <- f.createAndPublishHtlcTimeoutSweepTx(ctx) + }() + + deliverDepositSpend( + t, ctx, receiveDepositSpendReg( + t, ctx, mockLnd, loopIn, + ), htlcTx, + ) + + // The sweep checks whether the htlc output is spent. + var spendReg *test.SpendRegistration + select { + case spendReg = <-mockLnd.RegisterSpendChannel: + case <-ctx.Done(): + t.Fatalf("htlc output spend registration not "+ + "received: %v", ctx.Err()) + } + require.Equal(t, htlcOutpoint, *spendReg.Outpoint) + require.Equal(t, htlc.PkScript, spendReg.PkScript) + + if tc.spent && !tc.inWallet { + select { + case spendReg.SpendChannel <- &chainntnfs.SpendDetail{ + SpentOutPoint: spendReg.Outpoint, + SpendingTx: spendingTx, + }: + + case <-ctx.Done(): + t.Fatalf("unable to deliver htlc output "+ + "spend: %v", ctx.Err()) + } + } + + select { + case err = <-errChan: + case <-ctx.Done(): + t.Fatalf("sweep attempt did not return: %v", + ctx.Err()) + } + + if tc.err != "" { + require.ErrorContains(t, err, tc.err) + require.Nil(t, loopIn.HtlcTimeoutSweepTxHash) + + return + } + + require.NoError(t, err) + require.Equal( + t, spendingTx.TxHash(), + *loopIn.HtlcTimeoutSweepTxHash, + ) + }) + } +} + +// TestSweepHtlcTimeoutActionReportsLookupError checks that a failed htlc tx +// lookup is reported in LastActionError before the action waits to retry. +func TestSweepHtlcTimeoutActionReportsLookupError(t *testing.T) { + t.Parallel() + + ctx, cancel := context.WithCancel(t.Context()) + defer cancel() + + mockLnd := test.NewMockLnd() + f := &FSM{ + StateMachine: &fsm.StateMachine{}, + cfg: &Config{ + LndClient: mockLnd.Client, + WalletKit: mockLnd.WalletKit, + }, + loopIn: &StaticAddressLoopIn{}, + } + + // The lookup fails right away because the swap has no deposits. Stop + // the action while it waits to retry. + stop := time.AfterFunc(100*time.Millisecond, cancel) + defer stop.Stop() + + event := f.SweepHtlcTimeoutAction(ctx, nil) + require.Equal(t, fsm.NoOp, event) + require.ErrorContains(t, f.LastActionError, "loop-in has no deposits") +} + +// TestHtlcTimeoutSweepWalletFallback verifies that the sweep takes the htlc tx +// from lnd's wallet when lnd never reports the deposit spend, which happens +// when lnd's historical spend scan fails. +func TestHtlcTimeoutSweepWalletFallback(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + + // inWallet is set if lnd's wallet lists the htlc tx. + inWallet bool + + // walletErr is the error of the wallet lookup. + walletErr error + + err string + }{ + { + name: "htlc tx found in the wallet", + inWallet: true, + }, + { + name: "htlc tx not in the wallet", + err: notSpentErr, + }, + { + name: "wallet lookup fails", + walletErr: errors.New("list transactions failed"), + err: notSpentErr, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + ctx, cancel := context.WithTimeout( + t.Context(), 10*time.Second, + ) + defer cancel() + + mockLnd := test.NewMockLnd() + loopIn, variants := newHtlcVariantTestLoopIn(t, mockLnd) + htlcTx := variants[testHighHtlcFeeRate] + depositOutpoint := loopIn.Deposits[0].OutPoint + + checker := &recordingTxOutChecker{err: tc.walletErr} + if tc.inWallet { + walletTxs := map[wire.OutPoint]*wire.MsgTx{ + depositOutpoint: htlcTx, + } + checker.spendingTxs = walletTxs + } + + f := newHtlcSweepTestFSM( + t, ctx, mockLnd, loopIn, &noopSigner{}, + ) + f.cfg.HtlcTxLookupTimeout = 100 * time.Millisecond + f.cfg.TxOutChecker = checker + + errChan := make(chan error, 1) + go func() { + err := f.createAndPublishHtlcTimeoutSweepTx(ctx) + errChan <- err + }() + + // lnd accepts the registration but never reports the + // spend. + receiveDepositSpendReg(t, ctx, mockLnd, loopIn) + + // The mock publish blocks until the tx is read. + var sweepTx *wire.MsgTx + if tc.err == "" { + select { + case sweepTx = <-mockLnd.TxPublishChannel: + case <-ctx.Done(): + t.Fatalf("htlc timeout sweep not "+ + "published: %v", ctx.Err()) + } + } + + var err error + select { + case err = <-errChan: + case <-ctx.Done(): + t.Fatalf("sweep attempt did not return: %v", + ctx.Err()) + } + + require.Equal( + t, []wire.OutPoint{depositOutpoint}, + checker.spendingTxOutpoints, + ) + require.Equal( + t, []int32{htlcTxSearchStartHeight( + loopIn.InitiationHeight, + )}, + checker.spendingTxStartHeights, + ) + + if tc.err != "" { + require.ErrorContains(t, err, tc.err) + require.Nil(t, loopIn.HtlcTimeoutSweepTxHash) + + return + } + + require.NoError(t, err) + + htlc, err := loopIn.getHtlc(mockLnd.ChainParams) + require.NoError(t, err) + htlcOutpoint, _, err := loopIn.htlcOutput( + htlcTx, htlc.PkScript, + ) + require.NoError(t, err) + require.Len(t, sweepTx.TxIn, 1) + require.Equal( + t, htlcOutpoint, + sweepTx.TxIn[0].PreviousOutPoint, + ) + }) + } +} diff --git a/staticaddr/loopin/interface.go b/staticaddr/loopin/interface.go index 17a119347..c375b441c 100644 --- a/staticaddr/loopin/interface.go +++ b/staticaddr/loopin/interface.go @@ -141,6 +141,12 @@ type TxOutChecker interface { // startHeight are considered. SpentByTxPayingTo(ctx context.Context, outpoints []wire.OutPoint, pkScript []byte, startHeight int32) (bool, error) + + // ConfirmedSpendingTx returns a confirmed wallet transaction that + // spends the outpoint, or nil if there is none. Only transactions + // confirmed at or above startHeight are considered. + ConfirmedSpendingTx(ctx context.Context, outpoint wire.OutPoint, + startHeight int32) (*wire.MsgTx, error) } type NotificationManager interface { diff --git a/staticaddr/loopin/loopin.go b/staticaddr/loopin/loopin.go index ba2217f59..eafd6b35f 100644 --- a/staticaddr/loopin/loopin.go +++ b/staticaddr/loopin/loopin.go @@ -26,6 +26,7 @@ import ( "github.com/lightningnetwork/lnd/input" "github.com/lightningnetwork/lnd/keychain" "github.com/lightningnetwork/lnd/lntypes" + "github.com/lightningnetwork/lnd/lnwallet" "github.com/lightningnetwork/lnd/lnwallet/chainfee" "github.com/lightningnetwork/lnd/routing/route" "github.com/lightningnetwork/lnd/zpay32" @@ -321,8 +322,7 @@ func (l *StaticAddressLoopIn) createHtlcTx(chainParams *chaincfg.Params, return nil, err } - // Create the sweep output. NOTE: The HTLC output must be added at - // index 0. createHtlcSweepTx relies on this layout invariant. + // Create the sweep output. The HTLC output is added at index 0. sweepOutput := &wire.TxOut{ Value: int64(swapAmt - fee), PkScript: pkscript, @@ -366,12 +366,95 @@ func (l *StaticAddressLoopIn) htlcWeight(hasChange bool) lntypes.WeightUnit { return weightEstimator.Weight() } +// htlcOutput returns the outpoint and the output of the htlc in the given htlc +// tx. The client co-signs a standard, a high fee and an extremely high fee +// variant of the htlc tx, and the server may publish any of them. The timeout +// sweep therefore has to spend the htlc output of the tx that actually spent +// the deposits instead of rebuilding one of the variants. The tx is only +// accepted if it spends exactly the deposits of this swap and has a single +// output to the given htlc pkScript. The input order is not checked because +// recovery can load the deposits in a different order than they were signed +// in, and only the co-signed htlc tx both spends the deposits and pays to the +// htlc pkScript. +func (l *StaticAddressLoopIn) htlcOutput(htlcTx *wire.MsgTx, + htlcPkScript []byte) (wire.OutPoint, *wire.TxOut, error) { + + if htlcTx == nil { + return wire.OutPoint{}, nil, errors.New("no htlc tx provided") + } + + outpoints := l.Outpoints() + if len(outpoints) == 0 { + return wire.OutPoint{}, nil, errors.New("loop-in has no " + + "deposits") + } + + txHash := htlcTx.TxHash() + if len(htlcTx.TxIn) != len(outpoints) { + return wire.OutPoint{}, nil, fmt.Errorf("tx %v spends %d "+ + "inputs, expected the %d deposits of the swap", txHash, + len(htlcTx.TxIn), len(outpoints)) + } + + unspent := make(map[wire.OutPoint]struct{}, len(outpoints)) + for _, outpoint := range outpoints { + unspent[outpoint] = struct{}{} + } + + for i, txIn := range htlcTx.TxIn { + if _, ok := unspent[txIn.PreviousOutPoint]; !ok { + return wire.OutPoint{}, nil, fmt.Errorf("tx %v input "+ + "%d spends %v, which is not a remaining "+ + "deposit of the swap", txHash, i, + txIn.PreviousOutPoint) + } + + delete(unspent, txIn.PreviousOutPoint) + } + + var ( + htlcIndex = -1 + htlcOut *wire.TxOut + ) + for i, txOut := range htlcTx.TxOut { + if !bytes.Equal(txOut.PkScript, htlcPkScript) { + continue + } + + if htlcOut != nil { + return wire.OutPoint{}, nil, fmt.Errorf("tx %v has "+ + "more than one htlc output", txHash) + } + + htlcIndex = i + htlcOut = txOut + } + + if htlcOut == nil { + return wire.OutPoint{}, nil, fmt.Errorf("tx %v has no htlc "+ + "output", txHash) + } + + if htlcOut.Value <= 0 { + return wire.OutPoint{}, nil, fmt.Errorf("tx %v has an htlc "+ + "output without value", txHash) + } + + htlcOutpoint := wire.OutPoint{ + Hash: txHash, + Index: uint32(htlcIndex), + } + + return htlcOutpoint, htlcOut, nil +} + // createHtlcSweepTx creates the htlc sweep transaction for the timeout path of -// the loop-in htlc. +// the loop-in htlc. The htlcTx is the htlc tx variant that spent the deposits +// on chain. func (l *StaticAddressLoopIn) createHtlcSweepTx(ctx context.Context, - signer lndclient.SignerClient, sweepAddress btcutil.Address, - feeRate chainfee.SatPerKWeight, network *chaincfg.Params, - blockHeight uint32, maxFeePercentage float64) (*wire.MsgTx, error) { + signer lndclient.SignerClient, htlcTx *wire.MsgTx, + sweepAddress btcutil.Address, feeRate chainfee.SatPerKWeight, + network *chaincfg.Params, blockHeight uint32) (*wire.MsgTx, error) { if network == nil { return nil, errors.New("no network provided") @@ -382,6 +465,11 @@ func (l *StaticAddressLoopIn) createHtlcSweepTx(ctx context.Context, return nil, err } + htlcOutpoint, htlcOut, err := l.htlcOutput(htlcTx, htlc.PkScript) + if err != nil { + return nil, err + } + // Create the sweep transaction. sweepTx := wire.NewMsgTx(2) sweepTx.LockTime = blockHeight @@ -394,36 +482,11 @@ func (l *StaticAddressLoopIn) createHtlcSweepTx(ctx context.Context, return nil, err } - htlcTx, err := l.createHtlcTx( - network, l.HtlcTxFeeRate, maxFeePercentage, - ) - if err != nil { - return nil, err - } - - // The HTLC output is always at index 0 (createHtlcTx adds it first). - // If there is a change output, it is at index 1. Verify this invariant - // so we fail fast if createHtlcTx's layout ever changes. - const htlcInputIndex = uint32(0) - if len(htlcTx.TxOut) == 2 { - if bytes.Equal( - htlcTx.TxOut[0].PkScript, l.AddressParams.PkScript, - ) { - - return nil, fmt.Errorf("htlc tx output layout " + - "invariant violated: expected HTLC output " + - "at index 0, got change output") - } - } - // Add the htlc input. sweepTx.AddTxIn(&wire.TxIn{ - PreviousOutPoint: wire.OutPoint{ - Hash: htlcTx.TxHash(), - Index: htlcInputIndex, - }, - SignatureScript: htlc.SigScript, - Sequence: htlc.SuccessSequence(), + PreviousOutPoint: htlcOutpoint, + SignatureScript: htlc.SigScript, + Sequence: htlc.SuccessSequence(), }) // Add the sweep output. @@ -434,7 +497,16 @@ func (l *StaticAddressLoopIn) createHtlcSweepTx(ctx context.Context, fee := feeRate.FeeForWeight(weightEstimator.Weight()) - htlcOutValue := htlcTx.TxOut[htlcInputIndex].Value + // The backup htlc tx variants leave a smaller htlc output, so make + // sure the sweep fee still leaves a spendable output. + htlcOutValue := htlcOut.Value + dustLimit := lnwallet.DustLimitForSize(input.P2TRSize) + if htlcOutValue-int64(fee) < int64(dustLimit) { + return nil, fmt.Errorf("htlc output %v of %v is too small to "+ + "pay the sweep fee %v at fee rate %v", htlcOutpoint, + btcutil.Amount(htlcOutValue), fee, feeRate) + } + output := &wire.TxOut{ Value: htlcOutValue - int64(fee), PkScript: sweepPkScript, diff --git a/staticaddr/loopin/loopin_test.go b/staticaddr/loopin/loopin_test.go index 8b0892e66..8f4b588cc 100644 --- a/staticaddr/loopin/loopin_test.go +++ b/staticaddr/loopin/loopin_test.go @@ -130,9 +130,8 @@ func TestCreateHtlcSweepTxSweepValue(t *testing.T) { // Call createHtlcSweepTx and verify that the sweep output is derived // from the HTLC value, not the change. sweepTx, err := loopIn.createHtlcSweepTx( - t.Context(), signer, sweepAddr, feeRate, + t.Context(), signer, htlcTx, sweepAddr, feeRate, network, uint32(loopIn.HtlcCltvExpiry)+1, - maxFeePercentage, ) require.NoError(t, err) require.Len(t, sweepTx.TxOut, 1) diff --git a/staticaddr/loopin/manager.go b/staticaddr/loopin/manager.go index 3cb3d6502..dcd797bbe 100644 --- a/staticaddr/loopin/manager.go +++ b/staticaddr/loopin/manager.go @@ -66,8 +66,9 @@ type Config struct { // TxOutChecker checks the selected deposits against lnd's wallet // transactions. It is used before the client gives the server HTLC - // signatures, and after a risk rejection to find out whether the htlc - // tx spent the deposits. + // signatures, after a risk rejection to find out whether the htlc tx + // spent the deposits, and by the htlc timeout sweep when lnd doesn't + // report a spend. TxOutChecker TxOutChecker // InvoicesClient is used to subscribe to invoice settlements and @@ -125,6 +126,14 @@ type Config struct { // MaxStaticAddrHtlcFeePercentage since it serves the server as backup // transaction in case of fee spikes. MaxStaticAddrHtlcBackupFeePercentage float64 + + // HtlcTxLookupTimeout is the maximum time to wait for lnd to report a + // confirmed spend when looking up the htlc tx to sweep, or the tx that + // already spent the htlc output. After it, lnd's wallet is checked. If + // the wallet lists no spend either, the sweep attempt fails so that it + // is retried and reported like any other failure. Zero means + // defaultHtlcTxLookupTimeout. + HtlcTxLookupTimeout time.Duration } // newSwapRequest is used to send a loop-in request to the manager main loop. diff --git a/staticaddr/loopin/txout_checker.go b/staticaddr/loopin/txout_checker.go index 3157fd373..fa7a3993f 100644 --- a/staticaddr/loopin/txout_checker.go +++ b/staticaddr/loopin/txout_checker.go @@ -114,6 +114,34 @@ func (c *lndTxOutChecker) SpentByTxPayingTo(ctx context.Context, return false, nil } +// ConfirmedSpendingTx returns a confirmed transaction from lnd's wallet that +// spends the outpoint, or nil if there is none. The wallet only lists +// transactions that spend wallet outputs or pay to the wallet. Only +// transactions confirmed at or above startHeight are listed, so the lookup +// doesn't decode the whole wallet history. +func (c *lndTxOutChecker) ConfirmedSpendingTx(ctx context.Context, + outpoint wire.OutPoint, startHeight int32) (*wire.MsgTx, error) { + + txs, err := c.client.ListTransactions(ctx, startHeight, -1) + if err != nil { + return nil, err + } + + for _, tx := range txs { + if tx.Tx == nil || tx.Confirmations <= 0 { + continue + } + + for _, txIn := range tx.Tx.TxIn { + if txIn.PreviousOutPoint == outpoint { + return tx.Tx, nil + } + } + } + + return nil, nil +} + // paysTo reports whether the transaction has an output with the pkScript. func paysTo(tx *wire.MsgTx, pkScript []byte) bool { for _, txOut := range tx.TxOut { diff --git a/staticaddr/loopin/txout_checker_test.go b/staticaddr/loopin/txout_checker_test.go index b3cdfe981..e9d6b8312 100644 --- a/staticaddr/loopin/txout_checker_test.go +++ b/staticaddr/loopin/txout_checker_test.go @@ -196,6 +196,93 @@ func TestLndTxOutCheckerSpentByTxPayingTo(t *testing.T) { }) } +// TestLndTxOutCheckerConfirmedSpendingTx checks that only a confirmed wallet +// transaction that spends the exact outpoint is returned. +func TestLndTxOutCheckerConfirmedSpendingTx(t *testing.T) { + outpoint := wire.OutPoint{Hash: [32]byte{1}, Index: 1} + sameHashOutpoint := wire.OutPoint{Hash: [32]byte{1}, Index: 0} + otherOutpoint := wire.OutPoint{Hash: [32]byte{2}, Index: 1} + + const startHeight int32 = 500 + + spendingTx := func(prevs ...wire.OutPoint) *wire.MsgTx { + tx := wire.NewMsgTx(2) + for _, prev := range prevs { + tx.AddTxIn(wire.NewTxIn(&prev, nil, nil)) + } + tx.AddTxOut(wire.NewTxOut(1000, []byte{0x51, 0x20, 0x01})) + + return tx + } + + spend := spendingTx(otherOutpoint, outpoint) + + tests := []struct { + name string + txs []lndclient.Transaction + want *wire.MsgTx + }{{ + name: "confirmed spend in a later input", + txs: []lndclient.Transaction{{ + Tx: spendingTx(otherOutpoint), + Confirmations: 3, + }, { + Tx: spend, + Confirmations: 1, + }}, + want: spend, + }, { + name: "unconfirmed spend", + txs: []lndclient.Transaction{{ + Tx: spendingTx(outpoint), + }}, + }, { + name: "spend of another output of the same tx", + txs: []lndclient.Transaction{{ + Tx: spendingTx(sameHashOutpoint), + Confirmations: 1, + }}, + }, { + name: "no wallet transactions", + }, { + name: "transaction without raw tx", + txs: []lndclient.Transaction{{ + Confirmations: 1, + PreviousOutpoints: []*lnrpc.PreviousOutPoint{{ + Outpoint: outpoint.String(), + }}, + }}, + }} + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + client := &mockTxListLightningClient{txs: tc.txs} + checker := NewLndTxOutChecker(client) + + tx, err := checker.ConfirmedSpendingTx( + t.Context(), outpoint, startHeight, + ) + require.NoError(t, err) + require.Equal(t, tc.want, tx) + require.Equal(t, []txListCall{{ + startHeight: startHeight, + endHeight: -1, + }}, client.calls) + }) + } + + t.Run("returns error", func(t *testing.T) { + expectedErr := errors.New("list transactions failed") + client := &mockTxListLightningClient{err: expectedErr} + checker := NewLndTxOutChecker(client) + + _, err := checker.ConfirmedSpendingTx( + t.Context(), outpoint, startHeight, + ) + require.ErrorIs(t, err, expectedErr) + }) +} + type txListCall struct { startHeight int32 endHeight int32