From 03eda01be72b0ecd0a087026f3ab7e658509b514 Mon Sep 17 00:00:00 2001 From: Slyghtning Date: Tue, 24 Mar 2026 17:31:53 +0100 Subject: [PATCH 1/9] deposit: track block height and derive confirmation from wallet UTXOs Replace the blocking RegisterConfirmationsNtfn-based getBlockHeight with a lightweight confirmationHeightForUtxo that derives the first confirmation height from the wallet UTXO confirmation count and the currently tracked block height. This removes the MinConfs gating from reconcileDeposits so that mempool deposits are detected immediately with ConfirmationHeight=0. Add updateDepositConfirmations to backfill confirmation heights once previously-unconfirmed deposits get mined. The manager now stores the current block height via an atomic and reconciles deposits on every new block. --- loopd/swapclient_server.go | 43 ++- loopd/swapclient_server_staticaddr_test.go | 211 +++++++++++++ staticaddr/deposit/deposit.go | 5 +- staticaddr/deposit/fsm.go | 12 + staticaddr/deposit/manager.go | 301 +++++++++++++++++-- staticaddr/deposit/manager_height_test.go | 69 +++++ staticaddr/deposit/manager_reconcile_test.go | 212 +++++++++++++ 7 files changed, 824 insertions(+), 29 deletions(-) create mode 100644 loopd/swapclient_server_staticaddr_test.go create mode 100644 staticaddr/deposit/manager_height_test.go create mode 100644 staticaddr/deposit/manager_reconcile_test.go diff --git a/loopd/swapclient_server.go b/loopd/swapclient_server.go index 62187d3e5..e69a6f071 100644 --- a/loopd/swapclient_server.go +++ b/loopd/swapclient_server.go @@ -976,15 +976,24 @@ func (s *swapClientServer) GetLoopInQuote(ctx context.Context, return nil, fmt.Errorf("expected %d deposits, got %d", len(req.DepositOutpoints), len(depositList.FilteredDeposits)) - } else { - numDeposits = len(depositList.FilteredDeposits) } + numDeposits = len(depositList.FilteredDeposits) // In case we quote for deposits, we send the server both the // selected value and the number of deposits. This is so the // server can probe the selected value and calculate the per // input fee. for _, deposit := range depositList.FilteredDeposits { + // ListStaticAddressDeposits only filters out deposits that are no + // longer visible to the user, such as Replaced records. For a manual + // quote we additionally require the current state to be Deposited so a + // stale client-side outpoint selection fails early instead of making it + // to swap initiation. + if deposit.State != looprpc.DepositState_DEPOSITED { + return nil, fmt.Errorf("deposit %s is not "+ + "currently available", deposit.Outpoint) + } + totalDepositAmount += btcutil.Amount( deposit.Value, ) @@ -1804,7 +1813,8 @@ func (s *swapClientServer) ListStaticAddressDeposits(ctx context.Context, var filteredDeposits []*looprpc.Deposit if len(outpoints) > 0 { f := func(d *deposit.Deposit) bool { - return slices.Contains(outpoints, d.OutPoint.String()) + return isVisibleDeposit(d) && + slices.Contains(outpoints, d.OutPoint.String()) } filteredDeposits = filter(allDeposits, f) @@ -1814,6 +1824,10 @@ func (s *swapClientServer) ListStaticAddressDeposits(ctx context.Context, } } else { f := func(d *deposit.Deposit) bool { + if !isVisibleDeposit(d) { + return false + } + if req.StateFilter == looprpc.DepositState_UNKNOWN_STATE { // Per default, we return deposits in all // states. @@ -1999,6 +2013,7 @@ func (s *swapClientServer) GetStaticAddressSummary(ctx context.Context, if err != nil { return nil, err } + allDeposits = filterDeposits(allDeposits, isVisibleDeposit) var ( totalNumDeposits = len(allDeposits) @@ -2206,6 +2221,28 @@ func (s *swapClientServer) StaticOpenChannel(ctx context.Context, type filterFunc func(deposits *deposit.Deposit) bool +func filterDeposits(deposits []*deposit.Deposit, + f filterFunc) []*deposit.Deposit { + + filtered := make([]*deposit.Deposit, 0, len(deposits)) + for _, deposit := range deposits { + if !f(deposit) { + continue + } + + filtered = append(filtered, deposit) + } + + return filtered +} + +func isVisibleDeposit(d *deposit.Deposit) bool { + // Replaced deposits are kept in the DB as history, but they should disappear + // from normal deposit listings and summary totals because the underlying + // outpoint is no longer present in the wallet and cannot be spent. + return d.GetState() != deposit.Replaced +} + func filter(deposits []*deposit.Deposit, f filterFunc) []*looprpc.Deposit { var clientDeposits []*looprpc.Deposit for _, d := range deposits { diff --git a/loopd/swapclient_server_staticaddr_test.go b/loopd/swapclient_server_staticaddr_test.go new file mode 100644 index 000000000..81c741926 --- /dev/null +++ b/loopd/swapclient_server_staticaddr_test.go @@ -0,0 +1,211 @@ +package loopd + +import ( + "context" + "testing" + + "github.com/btcsuite/btcd/btcutil" + "github.com/btcsuite/btcd/chaincfg/chainhash" + "github.com/btcsuite/btcd/wire" + "github.com/btcsuite/btclog/v2" + "github.com/lightninglabs/loop/looprpc" + "github.com/lightninglabs/loop/staticaddr/address" + "github.com/lightninglabs/loop/staticaddr/deposit" + mock_lnd "github.com/lightninglabs/loop/test" + "github.com/stretchr/testify/require" +) + +type staticAddrDepositStore struct { + allDeposits []*deposit.Deposit + byOutpoint map[string]*deposit.Deposit +} + +func (s *staticAddrDepositStore) CreateDeposit(context.Context, + *deposit.Deposit) error { + + return nil +} + +func (s *staticAddrDepositStore) UpdateDeposit(context.Context, + *deposit.Deposit) error { + + return nil +} + +func (s *staticAddrDepositStore) GetDeposit(context.Context, + deposit.ID) (*deposit.Deposit, error) { + + return nil, nil +} + +func (s *staticAddrDepositStore) DepositForOutpoint(_ context.Context, + outpoint string) (*deposit.Deposit, error) { + + if deposit, ok := s.byOutpoint[outpoint]; ok { + return deposit, nil + } + + return nil, deposit.ErrDepositNotFound +} + +func (s *staticAddrDepositStore) AllDeposits(context.Context) ( + []*deposit.Deposit, error) { + + return s.allDeposits, nil +} + +func newTestDepositManager( + deposits ...*deposit.Deposit) *deposit.Manager { + + byOutpoint := make(map[string]*deposit.Deposit, len(deposits)) + for _, deposit := range deposits { + byOutpoint[deposit.OutPoint.String()] = deposit + } + + return deposit.NewManager(&deposit.ManagerConfig{ + Store: &staticAddrDepositStore{ + allDeposits: deposits, + byOutpoint: byOutpoint, + }, + }) +} + +func newTestStaticAddressContext(t *testing.T) (*address.Manager, + *mock_lnd.LndMockServices) { + + t.Helper() + + mock := mock_lnd.NewMockLnd() + _, client := mock_lnd.CreateKey(1) + _, server := mock_lnd.CreateKey(2) + + addrStore := &mockAddressStore{ + params: []*address.Parameters{{ + ClientPubkey: client, + ServerPubkey: server, + Expiry: 10, + PkScript: []byte("pkscript"), + }}, + } + + addrMgr, err := address.NewManager(&address.ManagerConfig{ + Store: addrStore, + WalletKit: mock.WalletKit, + ChainParams: mock.ChainParams, + }, 1) + require.NoError(t, err) + + return addrMgr, mock +} + +func TestListStaticAddressDepositsHidesReplaced(t *testing.T) { + t.Parallel() + + replaced := &deposit.Deposit{ + OutPoint: wire.OutPoint{ + Hash: chainhash.Hash{1}, + Index: 1, + }, + } + replaced.SetState(deposit.Replaced) + + available := &deposit.Deposit{ + OutPoint: wire.OutPoint{ + Hash: chainhash.Hash{2}, + Index: 2, + }, + } + available.SetState(deposit.Deposited) + + addrMgr, lnd := newTestStaticAddressContext(t) + server := &swapClientServer{ + depositManager: newTestDepositManager(replaced, available), + staticAddressManager: addrMgr, + lnd: &lnd.LndServices, + } + + resp, err := server.ListStaticAddressDeposits( + context.Background(), &looprpc.ListStaticAddressDepositsRequest{}, + ) + require.NoError(t, err) + require.Len(t, resp.FilteredDeposits, 1) + require.Equal( + t, available.OutPoint.String(), + resp.FilteredDeposits[0].Outpoint, + ) +} + +func TestGetStaticAddressSummaryIgnoresReplaced(t *testing.T) { + t.Parallel() + + replaced := &deposit.Deposit{ + OutPoint: wire.OutPoint{ + Hash: chainhash.Hash{3}, + Index: 3, + }, + Value: btcutil.Amount(1_000), + } + replaced.SetState(deposit.Replaced) + + unconfirmed := &deposit.Deposit{ + OutPoint: wire.OutPoint{ + Hash: chainhash.Hash{4}, + Index: 4, + }, + Value: btcutil.Amount(2_000), + ConfirmationHeight: 0, + } + unconfirmed.SetState(deposit.Deposited) + + confirmed := &deposit.Deposit{ + OutPoint: wire.OutPoint{ + Hash: chainhash.Hash{5}, + Index: 5, + }, + Value: btcutil.Amount(3_000), + ConfirmationHeight: 123, + } + confirmed.SetState(deposit.Deposited) + + addrMgr, _ := newTestStaticAddressContext(t) + server := &swapClientServer{ + depositManager: newTestDepositManager( + replaced, unconfirmed, confirmed, + ), + staticAddressManager: addrMgr, + } + + resp, err := server.GetStaticAddressSummary( + context.Background(), &looprpc.StaticAddressSummaryRequest{}, + ) + require.NoError(t, err) + require.EqualValues(t, 2, resp.TotalNumDeposits) + require.EqualValues(t, 2_000, resp.ValueUnconfirmedSatoshis) + require.EqualValues(t, 3_000, resp.ValueDepositedSatoshis) +} + +func TestGetLoopInQuoteRejectsUnavailableSelectedDeposit(t *testing.T) { + t.Parallel() + setLogger(btclog.Disabled) + + locked := &deposit.Deposit{ + OutPoint: wire.OutPoint{ + Hash: chainhash.Hash{6}, + Index: 6, + }, + Value: btcutil.Amount(5_000), + } + locked.SetState(deposit.LoopingIn) + + addrMgr, lnd := newTestStaticAddressContext(t) + server := &swapClientServer{ + depositManager: newTestDepositManager(locked), + staticAddressManager: addrMgr, + lnd: &lnd.LndServices, + } + + _, err := server.GetLoopInQuote(context.Background(), &looprpc.QuoteRequest{ + DepositOutpoints: []string{locked.OutPoint.String()}, + }) + require.ErrorContains(t, err, "is not currently available") +} diff --git a/staticaddr/deposit/deposit.go b/staticaddr/deposit/deposit.go index 4cb64bc95..ec7655af7 100644 --- a/staticaddr/deposit/deposit.go +++ b/staticaddr/deposit/deposit.go @@ -69,9 +69,12 @@ func (d *Deposit) IsInFinalState() bool { d.Lock() defer d.Unlock() + // Replaced is inactive from the deposit FSM's point of view. The manager may + // still revive the same record if lnd reports the exact outpoint again after + // a transient wallet-view miss. return d.state == Expired || d.state == Withdrawn || d.state == LoopedIn || d.state == HtlcTimeoutSwept || - d.state == ChannelPublished + d.state == ChannelPublished || d.state == Replaced } func (d *Deposit) IsExpired(currentHeight, expiry uint32) bool { diff --git a/staticaddr/deposit/fsm.go b/staticaddr/deposit/fsm.go index 197bf2ee3..ae70e3809 100644 --- a/staticaddr/deposit/fsm.go +++ b/staticaddr/deposit/fsm.go @@ -46,6 +46,18 @@ var ( // confirmation height. Deposited = fsm.StateType("Deposited") + // Replaced signals that an unconfirmed deposit disappeared from the + // wallet view and can no longer be spent. + // + // The concrete case we need to handle is mempool replacement: a user can + // receive to the static address, we persist that unconfirmed outpoint, and + // then the funding transaction can be replaced or otherwise evicted before + // confirmation. Once that happens lnd stops returning the old outpoint from + // ListUnspent, but our DB would otherwise keep presenting it as selectable. + // Replaced lets us retain the historic record while making it clear that the + // original outpoint is no longer a live deposit. + Replaced = fsm.StateType("Replaced") + // Withdrawing signals that the withdrawal transaction has been // broadcast, awaiting sufficient confirmations. Withdrawing = fsm.StateType("Withdrawing") diff --git a/staticaddr/deposit/manager.go b/staticaddr/deposit/manager.go index af8820302..c28fecada 100644 --- a/staticaddr/deposit/manager.go +++ b/staticaddr/deposit/manager.go @@ -6,6 +6,7 @@ import ( "fmt" "sort" "sync" + "sync/atomic" "time" "github.com/btcsuite/btcd/txscript" @@ -33,6 +34,11 @@ const ( // PollInterval is the interval in which we poll for new deposits to our // static address. PollInterval = 10 * time.Second + + // vanishedUnconfirmedDepositThreshold is the number of consecutive wallet + // observations in which an unconfirmed deposit must be missing before we + // mark it replaced. + vanishedUnconfirmedDepositThreshold = 2 ) // ManagerConfig holds the configuration for the address manager. @@ -64,9 +70,17 @@ type Manager struct { // mu guards access to the activeDeposits map. mu sync.Mutex + // reconcileMu serializes deposit reconciliation so new deposits are + // discovered and retained exactly once per outpoint. + reconcileMu sync.Mutex + // activeDeposits contains all the active static address outputs. activeDeposits map[wire.OutPoint]*FSM + // missingUnconfirmedDeposits counts consecutive wallet observations in + // which an unconfirmed deposited outpoint was missing. + missingUnconfirmedDeposits map[wire.OutPoint]uint8 + // deposits contain all the deposits that have ever been made to the // static address. This field is used to store and recover deposits. It // also serves as a basis for reconciliation of newly detected deposits @@ -77,15 +91,19 @@ type Manager struct { // been finalized. The manager will adjust its internal state and flush // finalized deposits from its memory. finalizedDepositChan chan wire.OutPoint + + // currentHeight stores the currently best known block height. + currentHeight atomic.Uint32 } // NewManager creates a new deposit manager. func NewManager(cfg *ManagerConfig) *Manager { return &Manager{ - cfg: cfg, - activeDeposits: make(map[wire.OutPoint]*FSM), - deposits: make(map[wire.OutPoint]*Deposit), - finalizedDepositChan: make(chan wire.OutPoint), + cfg: cfg, + activeDeposits: make(map[wire.OutPoint]*FSM), + missingUnconfirmedDeposits: make(map[wire.OutPoint]uint8), + deposits: make(map[wire.OutPoint]*Deposit), + finalizedDepositChan: make(chan wire.OutPoint), } } @@ -98,6 +116,17 @@ func (m *Manager) Run(ctx context.Context, initChan chan struct{}) error { return err } + select { + case height := <-newBlockChan: + m.currentHeight.Store(uint32(height)) + + case err = <-newBlockErrChan: + return err + + case <-ctx.Done(): + return ctx.Err() + } + // Recover previous deposits and static address parameters from the DB. err = m.recoverDeposits(ctx) if err != nil { @@ -123,6 +152,13 @@ func (m *Manager) Run(ctx context.Context, initChan chan struct{}) error { for { select { case height := <-newBlockChan: + m.currentHeight.Store(uint32(height)) + + err := m.reconcileDeposits(ctx) + if err != nil { + log.Errorf("unable to reconcile deposits: %v", err) + } + // Inform all active deposits about a new block arrival. m.mu.Lock() activeDeposits := make([]*FSM, 0, len(m.activeDeposits)) @@ -207,8 +243,10 @@ func (m *Manager) recoverDeposits(ctx context.Context) error { return nil } -// pollDeposits polls new deposits to our static address and notifies the -// manager's event loop about them. +// pollDeposits periodically polls for new deposits to our static address. This +// complements the block-driven reconciliation in the main event loop: while new +// blocks trigger reconcileDeposits to promptly detect confirmations, the ticker +// here catches deposits that appear in the mempool between blocks. func (m *Manager) pollDeposits(ctx context.Context) { log.Debugf("Waiting for new static address deposits...") @@ -236,15 +274,40 @@ func (m *Manager) pollDeposits(ctx context.Context) { // far. It picks the newly identified deposits and starts a state machine per // deposit to track its progress. func (m *Manager) reconcileDeposits(ctx context.Context) error { + m.reconcileMu.Lock() + defer m.reconcileMu.Unlock() + log.Tracef("Reconciling new deposits...") utxos, err := m.cfg.AddressManager.ListUnspent( - ctx, MinConfs, MaxConfs, + ctx, 0, MaxConfs, ) if err != nil { return fmt.Errorf("unable to list new deposits: %w", err) } + err = m.updateDepositConfirmations(ctx, utxos) + if err != nil { + return fmt.Errorf("unable to update deposit "+ + "confirmations: %w", err) + } + + // If the same outpoint reappeared after a transient wallet-view miss, + // reactivate the existing record before we consider it new or vanished. + err = m.reviveReappearedDeposits(ctx, utxos) + if err != nil { + return fmt.Errorf("unable to revive reappeared deposits: %w", + err) + } + + // After handling reappearances, only still-missing outpoints contribute + // towards replacement detection. + err = m.invalidateVanishedUnconfirmedDeposits(ctx, utxos) + if err != nil { + return fmt.Errorf("unable to invalidate vanished "+ + "deposits: %w", err) + } + newDeposits := m.filterNewDeposits(utxos) if len(newDeposits) == 0 { log.Tracef("No new deposits...") @@ -274,7 +337,7 @@ func (m *Manager) reconcileDeposits(ctx context.Context) error { func (m *Manager) createNewDeposit(ctx context.Context, utxo *lnwallet.Utxo) (*Deposit, error) { - blockHeight, err := m.getBlockHeight(ctx, utxo) + confirmationHeight, err := m.confirmationHeightForUtxo(ctx, utxo) if err != nil { return nil, err } @@ -302,7 +365,7 @@ func (m *Manager) createNewDeposit(ctx context.Context, state: Deposited, OutPoint: utxo.OutPoint, Value: utxo.Value, - ConfirmationHeight: int64(blockHeight), + ConfirmationHeight: confirmationHeight, TimeOutSweepPkScript: timeoutSweepPkScript, } @@ -318,30 +381,32 @@ func (m *Manager) createNewDeposit(ctx context.Context, return deposit, nil } -// getBlockHeight retrieves the block height of a given utxo. -func (m *Manager) getBlockHeight(ctx context.Context, - utxo *lnwallet.Utxo) (uint32, error) { +// confirmationHeightForUtxo returns the first confirmation height of a UTXO. +// Unconfirmed UTXOs return 0. +func (m *Manager) confirmationHeightForUtxo(ctx context.Context, + utxo *lnwallet.Utxo) (int64, error) { - addressParams, err := m.cfg.AddressManager.GetStaticAddressParameters( - ctx, - ) + if utxo.Confirmations <= 0 { + return 0, nil + } + + params, err := m.cfg.AddressManager.GetStaticAddressParameters(ctx) if err != nil { - return 0, fmt.Errorf("couldn't get confirmation height for "+ - "deposit, %w", err) + return 0, fmt.Errorf("unable to get static address "+ + "parameters: %w", err) } - notifChan, errChan, err := - m.cfg.ChainNotifier.RegisterConfirmationsNtfn( - ctx, &utxo.OutPoint.Hash, addressParams.PkScript, - MinConfs, addressParams.InitiationHeight, - ) + confChan, errChan, err := m.cfg.ChainNotifier.RegisterConfirmationsNtfn( + ctx, &utxo.OutPoint.Hash, utxo.PkScript, 1, params.InitiationHeight, + ) if err != nil { - return 0, err + return 0, fmt.Errorf("unable to register confirmation "+ + "notification: %w", err) } select { - case tx := <-notifChan: - return tx.BlockHeight, nil + case conf := <-confChan: + return int64(conf.BlockHeight), nil case err := <-errChan: return 0, err @@ -351,6 +416,192 @@ func (m *Manager) getBlockHeight(ctx context.Context, } } +// updateDepositConfirmations backfills first confirmation heights for deposits +// that were previously detected unconfirmed. +func (m *Manager) updateDepositConfirmations(ctx context.Context, + utxos []*lnwallet.Utxo) error { + + for _, utxo := range utxos { + if utxo.Confirmations <= 0 { + continue + } + + m.mu.Lock() + deposit, ok := m.deposits[utxo.OutPoint] + m.mu.Unlock() + if !ok { + continue + } + + deposit.Lock() + if deposit.ConfirmationHeight > 0 { + deposit.Unlock() + continue + } + + confirmationHeight, err := m.confirmationHeightForUtxo(ctx, utxo) + if err != nil { + deposit.Unlock() + return err + } + + deposit.ConfirmationHeight = confirmationHeight + + err = m.cfg.Store.UpdateDeposit(ctx, deposit) + deposit.Unlock() + if err != nil { + return err + } + } + + return nil +} + +// reviveReappearedDeposits reactivates deposits that were previously marked as +// replaced if the exact same outpoint reappears in the wallet view. +// +// This is the inverse of invalidateVanishedUnconfirmedDeposits: it lets us +// recover from a transient ListUnspent gap without inventing a second record +// for the same outpoint. +func (m *Manager) reviveReappearedDeposits(ctx context.Context, + utxos []*lnwallet.Utxo) error { + + var candidates []*Deposit + + m.mu.Lock() + for _, utxo := range utxos { + delete(m.missingUnconfirmedDeposits, utxo.OutPoint) + + deposit, ok := m.deposits[utxo.OutPoint] + if !ok { + continue + } + + if _, active := m.activeDeposits[utxo.OutPoint]; active { + continue + } + + deposit.Lock() + isReplaced := deposit.IsInStateNoLock(Replaced) + deposit.Unlock() + if !isReplaced { + continue + } + + candidates = append(candidates, deposit) + } + m.mu.Unlock() + + for _, deposit := range candidates { + deposit.Lock() + if !deposit.IsInStateNoLock(Replaced) { + deposit.Unlock() + continue + } + + deposit.SetStateNoLock(Deposited) + err := m.cfg.Store.UpdateDeposit(ctx, deposit) + deposit.Unlock() + if err != nil { + return err + } + + log.Infof("Reactivated deposit %v after it reappeared in "+ + "wallet view", deposit.OutPoint) + + err = m.startDepositFsm(ctx, deposit) + if err != nil { + return err + } + } + + return nil +} + +// invalidateVanishedUnconfirmedDeposits marks unconfirmed Deposited outputs as +// replaced once lnd no longer reports the outpoint in multiple consecutive +// wallet observations. +// +// This closes the gap between wallet state and our DB state for mempool +// transactions. Before this check, an unconfirmed deposit could be persisted, +// later disappear because the transaction was RBF'd away, and still remain +// visible/selectable via deposit RPCs because those read from the store rather +// than from the wallet. We only invalidate deposits that are both: +// 1. still unconfirmed, and +// 2. still in the plain Deposited state. +// +// That keeps the scope narrow: confirmed deposits should not disappear due to a +// normal mempool replacement, and in-flight states like LoopingIn already have +// their own recovery/error handling. +func (m *Manager) invalidateVanishedUnconfirmedDeposits(ctx context.Context, + utxos []*lnwallet.Utxo) error { + + currentUtxos := make(map[wire.OutPoint]struct{}, len(utxos)) + for _, utxo := range utxos { + currentUtxos[utxo.OutPoint] = struct{}{} + } + + m.mu.Lock() + candidates := make([]*Deposit, 0, len(m.deposits)) + for outpoint, deposit := range m.deposits { + if _, ok := currentUtxos[outpoint]; ok { + delete(m.missingUnconfirmedDeposits, outpoint) + continue + } + + deposit.Lock() + isVanishedUnconfirmedDeposit := deposit.ConfirmationHeight <= 0 && + deposit.IsInStateNoLock(Deposited) + deposit.Unlock() + if !isVanishedUnconfirmedDeposit { + delete(m.missingUnconfirmedDeposits, outpoint) + continue + } + + m.missingUnconfirmedDeposits[outpoint]++ + if m.missingUnconfirmedDeposits[outpoint] < + vanishedUnconfirmedDepositThreshold { + + log.Debugf("Waiting for another wallet observation before "+ + "marking deposit %v replaced", outpoint) + + continue + } + + delete(m.missingUnconfirmedDeposits, outpoint) + candidates = append(candidates, deposit) + } + m.mu.Unlock() + + for _, deposit := range candidates { + deposit.Lock() + if deposit.ConfirmationHeight > 0 || + !deposit.IsInStateNoLock(Deposited) { + + deposit.Unlock() + continue + } + + // Persist the replacement marker before removing the deposit from the + // active set so restarted clients and RPC consumers see the same outcome. + deposit.SetStateNoLock(Replaced) + err := m.cfg.Store.UpdateDeposit(ctx, deposit) + deposit.Unlock() + if err != nil { + return err + } + + m.mu.Lock() + delete(m.activeDeposits, deposit.OutPoint) + m.mu.Unlock() + + log.Infof("Marked unconfirmed deposit %v as replaced", + deposit.OutPoint) + } + + return nil +} + // filterNewDeposits filters the given utxos for new deposits that we haven't // seen before. func (m *Manager) filterNewDeposits(utxos []*lnwallet.Utxo) []*lnwallet.Utxo { diff --git a/staticaddr/deposit/manager_height_test.go b/staticaddr/deposit/manager_height_test.go new file mode 100644 index 000000000..75e5b6967 --- /dev/null +++ b/staticaddr/deposit/manager_height_test.go @@ -0,0 +1,69 @@ +package deposit + +import ( + "context" + "testing" + + "github.com/btcsuite/btcd/chaincfg/chainhash" + "github.com/btcsuite/btcd/wire" + "github.com/lightninglabs/loop/staticaddr/address" + "github.com/lightningnetwork/lnd/chainntnfs" + "github.com/lightningnetwork/lnd/lnwallet" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +// TestConfirmationHeightForUtxo verifies first-confirmation height lookup for +// wallet UTXOs. +func TestConfirmationHeightForUtxo(t *testing.T) { + t.Run("unconfirmed", func(t *testing.T) { + manager := NewManager(&ManagerConfig{}) + + height, err := manager.confirmationHeightForUtxo( + context.Background(), &lnwallet.Utxo{}, + ) + require.NoError(t, err) + require.Zero(t, height) + }) + + t.Run("confirmed uses notifier height", func(t *testing.T) { + confChan := make(chan *chainntnfs.TxConfirmation, 1) + errChan := make(chan error, 1) + confChan <- &chainntnfs.TxConfirmation{ + BlockHeight: 101, + } + + mockAddressManager := new(mockAddressManager) + mockAddressManager.On( + "GetStaticAddressParameters", mock.Anything, + ).Return(&address.Parameters{ + InitiationHeight: 50, + }, nil) + + mockChainNotifier := new(MockChainNotifier) + txHash := chainhash.Hash{1} + mockChainNotifier.On( + "RegisterConfirmationsNtfn", mock.Anything, &txHash, + []byte("pkscript"), int32(1), int32(50), + ).Return(confChan, errChan, nil) + + manager := NewManager(&ManagerConfig{ + AddressManager: mockAddressManager, + ChainNotifier: mockChainNotifier, + }) + manager.currentHeight.Store(100) + + height, err := manager.confirmationHeightForUtxo( + context.Background(), &lnwallet.Utxo{ + Confirmations: 1, + PkScript: []byte("pkscript"), + OutPoint: wire.OutPoint{ + Hash: txHash, + Index: 1, + }, + }, + ) + require.NoError(t, err) + require.EqualValues(t, 101, height) + }) +} diff --git a/staticaddr/deposit/manager_reconcile_test.go b/staticaddr/deposit/manager_reconcile_test.go new file mode 100644 index 000000000..fd4178203 --- /dev/null +++ b/staticaddr/deposit/manager_reconcile_test.go @@ -0,0 +1,212 @@ +package deposit + +import ( + "context" + "errors" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/btcsuite/btcd/btcutil" + "github.com/btcsuite/btcd/chaincfg/chainhash" + "github.com/btcsuite/btcd/wire" + "github.com/lightninglabs/loop/staticaddr/address" + "github.com/lightninglabs/loop/staticaddr/script" + "github.com/lightninglabs/loop/staticaddr/version" + "github.com/lightninglabs/loop/test" + "github.com/lightningnetwork/lnd/lnwallet" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +func TestReconcileDepositsSerialized(t *testing.T) { + ctx := context.Background() + mockLnd := test.NewMockLnd() + utxo := &lnwallet.Utxo{ + AddressType: lnwallet.TaprootPubkey, + Value: btcutil.Amount(100_000), + Confirmations: 0, + OutPoint: wire.OutPoint{ + Hash: chainhash.Hash{1}, + Index: 1, + }, + } + + mockAddressManager := new(mockAddressManager) + mockAddressManager.On( + "ListUnspent", mock.Anything, int32(0), int32(MaxConfs), + ).Return([]*lnwallet.Utxo{utxo}, nil) + mockAddressManager.On( + "GetStaticAddressParameters", mock.Anything, + ).Return((*address.Parameters)(nil), errors.New("fsm init failed")) + + mockStore := new(mockStore) + var createCalls atomic.Int32 + createEntered := make(chan struct{}) + releaseCreate := make(chan struct{}) + mockStore.On( + "CreateDeposit", mock.Anything, mock.Anything, + ).Return(nil).Run(func(mock.Arguments) { + if createCalls.Add(1) == 1 { + close(createEntered) + } + + <-releaseCreate + }) + + manager := NewManager(&ManagerConfig{ + AddressManager: mockAddressManager, + Store: mockStore, + WalletKit: mockLnd.WalletKit, + Signer: mockLnd.Signer, + }) + + var wg sync.WaitGroup + wg.Add(2) + + errs := make(chan error, 2) + go func() { + defer wg.Done() + errs <- manager.reconcileDeposits(ctx) + }() + + <-createEntered + + go func() { + defer wg.Done() + errs <- manager.reconcileDeposits(ctx) + }() + + time.Sleep(100 * time.Millisecond) + close(releaseCreate) + wg.Wait() + close(errs) + + var gotErrs []error + for err := range errs { + gotErrs = append(gotErrs, err) + } + + require.EqualValues(t, 1, createCalls.Load()) + require.Len(t, manager.deposits, 1) + require.Empty(t, manager.activeDeposits) + require.Len(t, gotErrs, 2) + + var errCount int + for _, err := range gotErrs { + if err == nil { + continue + } + + errCount++ + require.ErrorContains(t, err, "unable to start new deposit FSM") + } + require.Equal(t, 1, errCount) +} + +// TestReconcileDepositsInvalidatesVanishedUnconfirmedDeposit verifies that a +// single missing ListUnspent observation is reversible, but repeated misses +// still mark the deposit as replaced. +func TestReconcileDepositsInvalidatesVanishedUnconfirmedDeposit(t *testing.T) { + ctx := context.Background() + outpoint := wire.OutPoint{ + Hash: chainhash.Hash{2}, + Index: 7, + } + + deposit := &Deposit{ + OutPoint: outpoint, + } + deposit.SetState(Deposited) + + mockAddressManager := new(mockAddressManager) + mockAddressManager.On( + "ListUnspent", mock.Anything, int32(0), int32(MaxConfs), + ).Return([]*lnwallet.Utxo{}, nil) + + mockStore := new(mockStore) + var updateCalls atomic.Int32 + mockStore.On( + "UpdateDeposit", mock.Anything, mock.Anything, + ).Return(nil).Run(func(args mock.Arguments) { + updateCalls.Add(1) + updatedDeposit := args.Get(1).(*Deposit) + require.True(t, updatedDeposit.IsInStateNoLock(Replaced)) + }) + + manager := NewManager(&ManagerConfig{ + AddressManager: mockAddressManager, + Store: mockStore, + }) + manager.deposits[outpoint] = deposit + manager.activeDeposits[outpoint] = &FSM{} + + // The first miss only increments the consecutive-miss counter. + require.NoError(t, manager.reconcileDeposits(ctx)) + require.EqualValues(t, 0, updateCalls.Load()) + require.Equal(t, Deposited, deposit.GetState()) + require.Len(t, manager.activeDeposits, 1) + + // The second consecutive miss is strong enough evidence to finalize the + // record as replaced. + require.NoError(t, manager.reconcileDeposits(ctx)) + require.EqualValues(t, 1, updateCalls.Load()) + require.Equal(t, Replaced, deposit.GetState()) + require.Empty(t, manager.activeDeposits) +} + +// TestReconcileDepositsReactivatesReappearedReplacedDeposit verifies that the +// same outpoint can be revived if lnd reports it again after being marked +// replaced. +func TestReconcileDepositsReactivatesReappearedReplacedDeposit(t *testing.T) { + ctx := context.Background() + outpoint := wire.OutPoint{ + Hash: chainhash.Hash{3}, + Index: 5, + } + + deposit := &Deposit{ + OutPoint: outpoint, + Value: btcutil.Amount(100_000), + } + deposit.SetState(Replaced) + + utxo := &lnwallet.Utxo{ + OutPoint: outpoint, + Value: deposit.Value, + } + + mockAddressManager := new(mockAddressManager) + mockAddressManager.On( + "ListUnspent", mock.Anything, int32(0), int32(MaxConfs), + ).Return([]*lnwallet.Utxo{utxo}, nil) + mockAddressManager.On( + "GetStaticAddressParameters", mock.Anything, + ).Return(&address.Parameters{ + ProtocolVersion: version.ProtocolVersion_V0, + }, nil) + mockAddressManager.On( + "GetStaticAddress", mock.Anything, + ).Return((*script.StaticAddress)(nil), nil) + + mockStore := new(mockStore) + mockStore.On( + "UpdateDeposit", mock.Anything, mock.Anything, + ).Return(nil).Run(func(args mock.Arguments) { + updatedDeposit := args.Get(1).(*Deposit) + require.True(t, updatedDeposit.IsInStateNoLock(Deposited)) + }) + + manager := NewManager(&ManagerConfig{ + AddressManager: mockAddressManager, + Store: mockStore, + }) + manager.deposits[outpoint] = deposit + + // Reconciliation should revive the existing record instead of creating a + // second deposit entry for the same outpoint. + require.NoError(t, manager.reconcileDeposits(ctx)) + require.Equal(t, Deposited, deposit.GetState()) + require.Len(t, manager.activeDeposits, 1) +} From 9e66340c63c0483825be76b16de5717fc3d5369e Mon Sep 17 00:00:00 2001 From: Slyghtning Date: Tue, 24 Mar 2026 17:32:10 +0100 Subject: [PATCH 2/9] deposit: guard unconfirmed deposits against expiry Unconfirmed deposits (ConfirmationHeight <= 0) cannot have started their CSV timer, so IsExpired now returns false for them. Update the Deposited state and OnStart event documentation to reflect that deposits are detected from the mempool rather than after reaching a confirmation threshold. --- staticaddr/deposit/deposit.go | 4 ++++ staticaddr/deposit/deposit_test.go | 15 +++++++++++++++ staticaddr/deposit/fsm.go | 8 ++++---- 3 files changed, 23 insertions(+), 4 deletions(-) create mode 100644 staticaddr/deposit/deposit_test.go diff --git a/staticaddr/deposit/deposit.go b/staticaddr/deposit/deposit.go index ec7655af7..35d5c2c43 100644 --- a/staticaddr/deposit/deposit.go +++ b/staticaddr/deposit/deposit.go @@ -81,6 +81,10 @@ func (d *Deposit) IsExpired(currentHeight, expiry uint32) bool { d.Lock() defer d.Unlock() + if d.ConfirmationHeight <= 0 { + return false + } + return currentHeight >= uint32(d.ConfirmationHeight)+expiry } diff --git a/staticaddr/deposit/deposit_test.go b/staticaddr/deposit/deposit_test.go new file mode 100644 index 000000000..ddfef6df3 --- /dev/null +++ b/staticaddr/deposit/deposit_test.go @@ -0,0 +1,15 @@ +package deposit + +import "testing" + +// TestIsExpiredUnconfirmed checks that unconfirmed deposits don't start their +// expiry timer. +func TestIsExpiredUnconfirmed(t *testing.T) { + deposit := &Deposit{ + ConfirmationHeight: 0, + } + + if deposit.IsExpired(500, 100) { + t.Fatal("unconfirmed deposit should not be expired") + } +} diff --git a/staticaddr/deposit/fsm.go b/staticaddr/deposit/fsm.go index ae70e3809..a26eea0ce 100644 --- a/staticaddr/deposit/fsm.go +++ b/staticaddr/deposit/fsm.go @@ -42,8 +42,8 @@ var ( // States. var ( - // Deposited signals that funds at a static address have reached the - // confirmation height. + // Deposited signals that funds at a static address have been detected + // and are available to the client. Deposited = fsm.StateType("Deposited") // Replaced signals that an unconfirmed deposit disappeared from the @@ -105,8 +105,8 @@ var ( // Events. var ( // OnStart is sent to the fsm once the deposit outpoint has been - // sufficiently confirmed. It transitions the fsm into the Deposited - // state from where we can trigger a withdrawal, a loopin or an expiry. + // detected. It transitions the fsm into the Deposited state from where + // we can trigger a withdrawal, a loopin or an expiry. OnStart = fsm.EventType("OnStart") // OnWithdrawInitiated is sent to the fsm when a withdrawal has been From a77b0794920b50f808b70236a7aaeb6355540dc4 Mon Sep 17 00:00:00 2001 From: Slyghtning Date: Tue, 24 Mar 2026 17:32:50 +0100 Subject: [PATCH 3/9] loopd: simplify ListUnspentDeposits to use deposit state Remove the MinConfs-based bifurcation from ListUnspentDeposits. All wallet UTXOs are now checked against the deposit store: known deposits in the Deposited state are available, unknown outpoints are new deposits and also available, and all other known states are filtered out. The mock deposit store now returns ErrDepositNotFound for missing outpoints to match the real store behavior, and tests are updated to verify availability by deposit state rather than confirmation depth. --- loopd/swapclient_server.go | 41 +++++++++--------------- loopd/swapclient_server_test.go | 55 +++++++++++++++++---------------- 2 files changed, 44 insertions(+), 52 deletions(-) diff --git a/loopd/swapclient_server.go b/loopd/swapclient_server.go index e69a6f071..d0c3b0a37 100644 --- a/loopd/swapclient_server.go +++ b/loopd/swapclient_server.go @@ -1671,54 +1671,43 @@ func (s *swapClientServer) ListUnspentDeposits(ctx context.Context, // not spendable because they already have been used but not yet spent // by the server. We filter out such deposits here. var ( - outpoints []string - isUnspent = make(map[wire.OutPoint]struct{}) + outpoints []string + isUnspent = make(map[wire.OutPoint]struct{}) + knownUtxos = make(map[wire.OutPoint]struct{}) ) - // Keep track of confirmed outpoints that we need to check against our - // database. - confirmedToCheck := make(map[wire.OutPoint]struct{}) - for _, utxo := range utxos { - if utxo.Confirmations < deposit.MinConfs { - // Unconfirmed deposits are always available. - isUnspent[utxo.OutPoint] = struct{}{} - } else { - // Confirmed deposits need to be checked. - outpoints = append(outpoints, utxo.OutPoint.String()) - confirmedToCheck[utxo.OutPoint] = struct{}{} - } + outpoints = append(outpoints, utxo.OutPoint.String()) + knownUtxos[utxo.OutPoint] = struct{}{} } // Check the spent status of the deposits by looking at their states. - ignoreUnknownOutpoints := false + ignoreUnknownOutpoints := true deposits, err := s.depositManager.DepositsForOutpoints( ctx, outpoints, ignoreUnknownOutpoints, ) if err != nil { return nil, err } + + knownDeposits := make(map[wire.OutPoint]struct{}, len(deposits)) for _, d := range deposits { - // A nil deposit means we don't have a record for it. We'll - // handle this case after the loop. if d == nil { continue } - // If the deposit is in the "Deposited" state, it's available. + knownDeposits[d.OutPoint] = struct{}{} if d.IsInState(deposit.Deposited) { isUnspent[d.OutPoint] = struct{}{} } - - // We have a record for this deposit, so we no longer need to - // check it. - delete(confirmedToCheck, d.OutPoint) } - // Any remaining outpoints in confirmedToCheck are ones that lnd knows - // about but we don't. These are new, unspent deposits. - for op := range confirmedToCheck { - isUnspent[op] = struct{}{} + // Any wallet outpoints that are unknown to the deposit store are new + // deposits and therefore still available. + for op := range knownUtxos { + if _, ok := knownDeposits[op]; !ok { + isUnspent[op] = struct{}{} + } } // Prepare the list of unspent deposits for the rpc response. diff --git a/loopd/swapclient_server_test.go b/loopd/swapclient_server_test.go index a3f29443f..e3ed8cde0 100644 --- a/loopd/swapclient_server_test.go +++ b/loopd/swapclient_server_test.go @@ -1002,7 +1002,7 @@ func (s *mockDepositStore) DepositForOutpoint(_ context.Context, if d, ok := s.byOutpoint[outpoint]; ok { return d, nil } - return nil, nil + return nil, deposit.ErrDepositNotFound } func (s *mockDepositStore) AllDeposits(_ context.Context) ([]*deposit.Deposit, error) { @@ -1051,11 +1051,11 @@ func TestListUnspentDeposits(t *testing.T) { } } - minConfs := int64(deposit.MinConfs) - utxoBelow := makeUtxo(0, minConfs-1) // always included - utxoAt := makeUtxo(1, minConfs) // included only if Deposited - utxoAbove1 := makeUtxo(2, minConfs+1) - utxoAbove2 := makeUtxo(3, minConfs+2) + utxoUnknown := makeUtxo(0, 0) + utxoDeposited := makeUtxo(1, 1) + utxoWithdrawn := makeUtxo(2, 2) + utxoLoopingIn := makeUtxo(3, 5) + utxoConfirmedUnknown := makeUtxo(4, 3) // Helper to build the deposit manager with specific states. buildDepositMgr := func( @@ -1073,17 +1073,19 @@ func TestListUnspentDeposits(t *testing.T) { return deposit.NewManager(&deposit.ManagerConfig{Store: store}) } - // Include below-min-conf and >=min with Deposited; exclude others. - t.Run("below min conf always, Deposited included, others excluded", + // Unknown deposits are available, Deposited is available and known + // non-Deposited states are excluded. + t.Run("unknown and Deposited included, locked states excluded", func(t *testing.T) { mock.SetListUnspent([]*lnwallet.Utxo{ - utxoBelow, utxoAt, utxoAbove1, utxoAbove2, + utxoUnknown, utxoDeposited, utxoWithdrawn, + utxoLoopingIn, }) depMgr := buildDepositMgr(map[wire.OutPoint]fsm.StateType{ - utxoAt.OutPoint: deposit.Deposited, - utxoAbove1.OutPoint: deposit.Withdrawn, - utxoAbove2.OutPoint: deposit.LoopingIn, + utxoDeposited.OutPoint: deposit.Deposited, + utxoWithdrawn.OutPoint: deposit.Withdrawn, + utxoLoopingIn.OutPoint: deposit.LoopingIn, }) server := &swapClientServer{ @@ -1096,7 +1098,7 @@ func TestListUnspentDeposits(t *testing.T) { ) require.NoError(t, err) - // Expect utxoBelow and utxoAt only. + // Expect the unknown utxo and the Deposited utxo only. require.Len(t, resp.Utxos, 2) got := map[string]struct{}{} for _, u := range resp.Utxos { @@ -1105,25 +1107,25 @@ func TestListUnspentDeposits(t *testing.T) { // same across utxos. require.NotEmpty(t, u.StaticAddress) } - _, ok1 := got[utxoBelow.OutPoint.String()] - _, ok2 := got[utxoAt.OutPoint.String()] + _, ok1 := got[utxoUnknown.OutPoint.String()] + _, ok2 := got[utxoDeposited.OutPoint.String()] require.True(t, ok1) require.True(t, ok2) }) - // Swap states, now include utxoBelow and utxoAbove1. - t.Run("Deposited on >=min included; non-Deposited excluded", + // Confirmation depth no longer changes availability; state does. + t.Run("availability ignores conf depth once deposit state is known", func(t *testing.T) { mock.SetListUnspent( []*lnwallet.Utxo{ - utxoBelow, utxoAt, utxoAbove1, - utxoAbove2, + utxoUnknown, utxoDeposited, + utxoWithdrawn, utxoLoopingIn, }) depMgr := buildDepositMgr(map[wire.OutPoint]fsm.StateType{ - utxoAt.OutPoint: deposit.Withdrawn, - utxoAbove1.OutPoint: deposit.Deposited, - utxoAbove2.OutPoint: deposit.Withdrawn, + utxoDeposited.OutPoint: deposit.Deposited, + utxoWithdrawn.OutPoint: deposit.Withdrawn, + utxoLoopingIn.OutPoint: deposit.LoopingIn, }) server := &swapClientServer{ @@ -1141,8 +1143,8 @@ func TestListUnspentDeposits(t *testing.T) { for _, u := range resp.Utxos { got[u.Outpoint] = struct{}{} } - _, ok1 := got[utxoBelow.OutPoint.String()] - _, ok2 := got[utxoAbove1.OutPoint.String()] + _, ok1 := got[utxoUnknown.OutPoint.String()] + _, ok2 := got[utxoDeposited.OutPoint.String()] require.True(t, ok1) require.True(t, ok2) }) @@ -1151,7 +1153,7 @@ func TestListUnspentDeposits(t *testing.T) { t.Run("confirmed utxo not in store is included", func(t *testing.T) { // Only return a confirmed UTXO from lnd and make sure the // deposit manager/store doesn't know about it. - mock.SetListUnspent([]*lnwallet.Utxo{utxoAbove2}) + mock.SetListUnspent([]*lnwallet.Utxo{utxoConfirmedUnknown}) // Empty store (no states for any outpoint). depMgr := buildDepositMgr(map[wire.OutPoint]fsm.StateType{}) @@ -1170,7 +1172,8 @@ func TestListUnspentDeposits(t *testing.T) { // doesn't exist in the store yet. require.Len(t, resp.Utxos, 1) require.Equal( - t, utxoAbove2.OutPoint.String(), resp.Utxos[0].Outpoint, + t, utxoConfirmedUnknown.OutPoint.String(), + resp.Utxos[0].Outpoint, ) require.NotEmpty(t, resp.Utxos[0].StaticAddress) }) From 7a3d8210b07506c524fa0f9614c15b74572a37ed Mon Sep 17 00:00:00 2001 From: Slyghtning Date: Tue, 24 Mar 2026 17:33:57 +0100 Subject: [PATCH 4/9] loopd: handle unconfirmed deposits in RPC responses Extract depositBlocksUntilExpiry helper that returns the full CSV value for unconfirmed deposits (ConfirmationHeight <= 0) since their CSV has not started. Use it in ListStaticAddressSwaps and populateBlocksUntilExpiry. In GetStaticAddressSummary, derive the unconfirmed value from deposits in the Deposited state with no confirmation height instead of issuing a separate wallet ListUnspent call. --- loopd/swapclient_server.go | 62 +++++++++++++++++-------- loopd/swapclient_server_deposit_test.go | 21 +++++++++ 2 files changed, 64 insertions(+), 19 deletions(-) create mode 100644 loopd/swapclient_server_deposit_test.go diff --git a/loopd/swapclient_server.go b/loopd/swapclient_server.go index d0c3b0a37..f5ad12682 100644 --- a/loopd/swapclient_server.go +++ b/loopd/swapclient_server.go @@ -1778,6 +1778,22 @@ func (s *swapClientServer) WithdrawDeposits(ctx context.Context, }, err } +// confirmedDeposits filters the given deposits and returns only those that have +// a positive confirmation height, i.e. deposits that have been confirmed +// on-chain. +func confirmedDeposits(deposits []*deposit.Deposit) []*deposit.Deposit { + confirmed := make([]*deposit.Deposit, 0, len(deposits)) + for _, d := range deposits { + if d.ConfirmationHeight <= 0 { + continue + } + + confirmed = append(confirmed, d) + } + + return confirmed +} + // ListStaticAddressDeposits returns a list of all sufficiently confirmed // deposits behind the static address and displays properties like value, // state or blocks til expiry. @@ -1951,9 +1967,10 @@ func (s *swapClientServer) ListStaticAddressSwaps(ctx context.Context, protoDeposits = make([]*looprpc.Deposit, 0, len(ds)) for _, d := range ds { state := toClientDepositState(d.GetState()) - blocksUntilExpiry := d.ConfirmationHeight + - int64(addrParams.Expiry) - - int64(lndInfo.BlockHeight) + blocksUntilExpiry := depositBlocksUntilExpiry( + d.ConfirmationHeight, addrParams.Expiry, + int64(lndInfo.BlockHeight), + ) pd := &looprpc.Deposit{ Id: d.ID[:], @@ -2015,23 +2032,16 @@ func (s *swapClientServer) GetStaticAddressSummary(ctx context.Context, htlcTimeoutSwept int64 ) - // Value unconfirmed. - utxos, err := s.staticAddressManager.ListUnspent( - ctx, 0, deposit.MinConfs-1, - ) - if err != nil { - return nil, err - } - for _, u := range utxos { - valueUnconfirmed += int64(u.Value) - } - - // Confirmed total values by category. + // Total values by category. for _, d := range allDeposits { value := int64(d.Value) switch d.GetState() { case deposit.Deposited: - valueDeposited += value + if d.ConfirmationHeight <= 0 { + valueUnconfirmed += value + } else { + valueDeposited += value + } case deposit.Expired: valueExpired += value @@ -2174,13 +2184,27 @@ func (s *swapClientServer) populateBlocksUntilExpiry(ctx context.Context, return err } for i := range len(deposits) { - deposits[i].BlocksUntilExpiry = - deposits[i].ConfirmationHeight + - int64(params.Expiry) - bestBlockHeight + deposits[i].BlocksUntilExpiry = depositBlocksUntilExpiry( + deposits[i].ConfirmationHeight, params.Expiry, + bestBlockHeight, + ) } return nil } +// depositBlocksUntilExpiry returns the remaining blocks until a deposit +// expires. Unconfirmed deposits return the full CSV value because the timeout +// has not started yet. +func depositBlocksUntilExpiry(confirmationHeight int64, expiry uint32, + bestBlockHeight int64) int64 { + + if confirmationHeight <= 0 { + return int64(expiry) + } + + return confirmationHeight + int64(expiry) - bestBlockHeight +} + // StaticOpenChannel initiates an open channel request using static address // deposits. func (s *swapClientServer) StaticOpenChannel(ctx context.Context, diff --git a/loopd/swapclient_server_deposit_test.go b/loopd/swapclient_server_deposit_test.go new file mode 100644 index 000000000..bded2da99 --- /dev/null +++ b/loopd/swapclient_server_deposit_test.go @@ -0,0 +1,21 @@ +package loopd + +import "testing" + +// TestDepositBlocksUntilExpiry checks blocks-until-expiry handling for +// confirmed and unconfirmed deposits. +func TestDepositBlocksUntilExpiry(t *testing.T) { + t.Run("unconfirmed", func(t *testing.T) { + if blocks := depositBlocksUntilExpiry(0, 144, 500); blocks != 144 { + t.Fatalf("expected 144 blocks for unconfirmed deposit, got %d", + blocks) + } + }) + + t.Run("confirmed", func(t *testing.T) { + if blocks := depositBlocksUntilExpiry(450, 144, 500); blocks != 94 { + t.Fatalf("expected 94 blocks until expiry, got %d", + blocks) + } + }) +} From 8e1154d8237ca3f4f9ce66744933d85e45ab2f3c Mon Sep 17 00:00:00 2001 From: Slyghtning Date: Tue, 24 Mar 2026 17:34:13 +0100 Subject: [PATCH 5/9] loopin: handle unconfirmed deposits in swap selection Unconfirmed deposits (ConfirmationHeight == 0) are considered swappable because their CSV timeout has not started yet. Extract blocksUntilDepositExpiry helper that returns MaxUint32 for unconfirmed deposits and use it in SelectDeposits sorting and IsSwappable. --- staticaddr/deposit/manager_test.go | 4 ++ staticaddr/loopin/manager.go | 63 +++++++++++++++++++++--------- staticaddr/loopin/manager_test.go | 27 +++++++++++++ 3 files changed, 75 insertions(+), 19 deletions(-) diff --git a/staticaddr/deposit/manager_test.go b/staticaddr/deposit/manager_test.go index ab8aaa7a8..d29c3ffe0 100644 --- a/staticaddr/deposit/manager_test.go +++ b/staticaddr/deposit/manager_test.go @@ -234,6 +234,10 @@ func TestManager(t *testing.T) { runErrChan <- testContext.manager.Run(ctx, initChan) }() + // Send an initial block so the manager can proceed past its startup + // block wait. + testContext.blockChan <- int32(defaultDepositConfirmations) + // Ensure that the manager has been initialized. select { case <-initChan: diff --git a/staticaddr/loopin/manager.go b/staticaddr/loopin/manager.go index 444ab5856..5efd4d59d 100644 --- a/staticaddr/loopin/manager.go +++ b/staticaddr/loopin/manager.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "fmt" + "math" "slices" "sort" "sync/atomic" @@ -850,11 +851,11 @@ func (m *Manager) GetAllSwaps(ctx context.Context) ([]*StaticAddressLoopIn, return swaps, nil } -// SelectDeposits sorts the deposits by amount in descending order, then by -// blocks-until-expiry in ascending order. It then selects the deposits that -// are needed to cover the amount requested without leaving a dust change. It -// returns an error if the sum of deposits minus dust is less than the requested -// amount. +// SelectDeposits sorts deposits by confirmation status first, then by amount in +// descending order, then by blocks-until-expiry in ascending order. It then +// selects the deposits that are needed to cover the amount requested without +// leaving a dust change. It returns an error if the sum of deposits minus dust +// is less than the requested amount. func SelectDeposits(targetAmount btcutil.Amount, unfilteredDeposits []*deposit.Deposit, csvExpiry uint32, blockHeight uint32) ([]*deposit.Deposit, error) { @@ -875,14 +876,25 @@ func SelectDeposits(targetAmount btcutil.Amount, deposits = append(deposits, d) } - // Sort the deposits by amount in descending order, then by - // blocks-until-expiry in ascending order. + // Sort confirmed deposits ahead of unconfirmed ones so auto-selection + // prefers deposits the server can accept immediately. Within each group + // we prefer larger deposits, then earlier expiries. sort.Slice(deposits, func(i, j int) bool { + iConfirmed := deposits[i].ConfirmationHeight > 0 + jConfirmed := deposits[j].ConfirmationHeight > 0 + if iConfirmed != jConfirmed { + return iConfirmed + } + if deposits[i].Value == deposits[j].Value { - iExp := uint32(deposits[i].ConfirmationHeight) + - csvExpiry - blockHeight - jExp := uint32(deposits[j].ConfirmationHeight) + - csvExpiry - blockHeight + iExp := blocksUntilDepositExpiry( + uint32(deposits[i].ConfirmationHeight), + blockHeight, csvExpiry, + ) + jExp := blocksUntilDepositExpiry( + uint32(deposits[j].ConfirmationHeight), + blockHeight, csvExpiry, + ) return iExp < jExp } @@ -914,20 +926,33 @@ func SelectDeposits(targetAmount btcutil.Amount, // IsSwappable checks if a deposit is swappable. It returns true if the deposit // is not expired and the htlc is not too close to expiry. func IsSwappable(confirmationHeight, blockHeight, csvExpiry uint32) bool { + if confirmationHeight == 0 { + return true + } + // The deposit expiry height is the confirmation height plus the csv // expiry. - depositExpiryHeight := confirmationHeight + csvExpiry + return blocksUntilDepositExpiry( + confirmationHeight, blockHeight, csvExpiry, + ) >= DefaultLoopInOnChainCltvDelta+DepositHtlcDelta +} - // The htlc expiry height is the current height plus the htlc - // cltv delta. - htlcExpiryHeight := blockHeight + DefaultLoopInOnChainCltvDelta +// blocksUntilDepositExpiry returns the remaining number of blocks until a +// deposit expires. Unconfirmed deposits return MaxUint32 because their CSV has +// not started yet. +func blocksUntilDepositExpiry(confirmationHeight, blockHeight, + csvExpiry uint32) uint32 { - // Ensure that the deposit doesn't expire before the htlc. - if depositExpiryHeight < htlcExpiryHeight+DepositHtlcDelta { - return false + if confirmationHeight == 0 { + return math.MaxUint32 + } + + depositExpiryHeight := confirmationHeight + csvExpiry + if depositExpiryHeight <= blockHeight { + return 0 } - return true + return depositExpiryHeight - blockHeight } // DeduceSwapAmount calculates the swap amount based on the selected amount and diff --git a/staticaddr/loopin/manager_test.go b/staticaddr/loopin/manager_test.go index d908a9e16..e4a197076 100644 --- a/staticaddr/loopin/manager_test.go +++ b/staticaddr/loopin/manager_test.go @@ -71,6 +71,27 @@ func TestSelectDeposits(t *testing.T) { expected: []*deposit.Deposit{d3}, expectedErr: "", }, + { + name: "prefer confirmed deposit over larger unconfirmed one", + deposits: []*deposit.Deposit{ + { + Value: 2_000_000, + ConfirmationHeight: 0, + }, + { + Value: 1_500_000, + ConfirmationHeight: 5_004, + }, + }, + targetValue: 1_000_000, + expected: []*deposit.Deposit{ + { + Value: 1_500_000, + ConfirmationHeight: 5_004, + }, + }, + expectedErr: "", + }, { name: "single deposit insufficient by 1", deposits: []*deposit.Deposit{d1}, @@ -176,6 +197,12 @@ func TestSelectDeposits(t *testing.T) { } } +// TestIsSwappableUnconfirmed checks that an unconfirmed deposit is considered +// swappable because its CSV timeout has not started yet. +func TestIsSwappableUnconfirmed(t *testing.T) { + require.True(t, IsSwappable(0, 5000, 1000)) +} + // mockDepositManager implements DepositManager for tests. type mockDepositManager struct { byOutpoint map[string]*deposit.Deposit From 9cf4bad8f6cabb26578dc367c0d9b07aa7bcbb1a Mon Sep 17 00:00:00 2001 From: Slyghtning Date: Tue, 24 Mar 2026 17:34:13 +0100 Subject: [PATCH 6/9] staticaddr: cancel invoice if init htlc fails --- staticaddr/loopin/actions.go | 84 ++++++++++++----- staticaddr/loopin/actions_test.go | 146 ++++++++++++++++++++++++++++++ 2 files changed, 209 insertions(+), 21 deletions(-) diff --git a/staticaddr/loopin/actions.go b/staticaddr/loopin/actions.go index 70a27811f..da6882f9b 100644 --- a/staticaddr/loopin/actions.go +++ b/staticaddr/loopin/actions.go @@ -36,6 +36,8 @@ const ( defaultConfTarget = 3 DefaultPaymentTimeoutSeconds = 60 + + defaultInvoiceCleanupTimeout = 5 * time.Second ) var ( @@ -57,6 +59,24 @@ var ( func (f *FSM) InitHtlcAction(ctx context.Context, _ fsm.EventContext) fsm.EventType { + var event fsm.EventType + invoiceNeedsCleanup := false + defer func() { + // If we created the private invoice but failed before persisting the + // swap, cancel it so retries do not accumulate orphan invoices. + if !invoiceNeedsCleanup || event != fsm.OnError { + return + } + + f.cancelSwapInvoice(ctx) + }() + + returnError := func(err error) fsm.EventType { + event = f.HandleError(err) + + return event + } + // Lock the deposits and transition them to the LoopingIn state. err := f.cfg.DepositManager.TransitionDeposits( ctx, f.loopIn.Deposits, deposit.OnLoopInInitiated, @@ -65,7 +85,7 @@ func (f *FSM) InitHtlcAction(ctx context.Context, if err != nil { err = fmt.Errorf("unable to loop-in deposits: %w", err) - return f.HandleError(err) + return returnError(err) } // Calculate the swap invoice amount. The server needs to pay us the @@ -88,7 +108,7 @@ func (f *FSM) InitHtlcAction(ctx context.Context, err = fmt.Errorf("unable to create random swap preimage: %w", err) - return f.HandleError(err) + return returnError(err) } f.loopIn.SwapPreimage = swapPreimage f.loopIn.SwapHash = swapPreimage.Hash() @@ -100,7 +120,7 @@ func (f *FSM) InitHtlcAction(ctx context.Context, if err != nil { err = fmt.Errorf("unable to derive client htlc key: %w", err) - return f.HandleError(err) + return returnError(err) } f.loopIn.ClientPubkey = keyDesc.PubKey f.loopIn.HtlcKeyLocator = keyDesc.KeyLocator @@ -119,10 +139,14 @@ func (f *FSM) InitHtlcAction(ctx context.Context, if err != nil { err = fmt.Errorf("unable to create swap invoice: %w", err) - return f.HandleError(err) + return returnError(err) } f.loopIn.SwapInvoice = swapInvoice + // From here until CreateLoopIn succeeds, any error path would otherwise + // leave behind a live invoice with no persisted swap to recover it. + invoiceNeedsCleanup = true + f.loopIn.ProtocolVersion = version.AddressProtocolVersion( version.CurrentRPCProtocolVersion(), ) @@ -149,7 +173,7 @@ func (f *FSM) InitHtlcAction(ctx context.Context, err = fmt.Errorf("unable to initiate the loop-in with the "+ "server: %w", err) - return f.HandleError(err) + return returnError(err) } // Pushing empty sigs signals the server that we abandoned the swap @@ -171,7 +195,7 @@ func (f *FSM) InitHtlcAction(ctx context.Context, pushEmptySigs() err = fmt.Errorf("unable to parse server pubkey: %w", err) - return f.HandleError(err) + return returnError(err) } f.loopIn.ServerPubkey = serverPubkey @@ -185,7 +209,7 @@ func (f *FSM) InitHtlcAction(ctx context.Context, err = fmt.Errorf("server response parameters are outside "+ "our allowed range: %w", err) - return f.HandleError(err) + return returnError(err) } f.loopIn.HtlcCltvExpiry = loopInResp.HtlcExpiry @@ -194,7 +218,7 @@ func (f *FSM) InitHtlcAction(ctx context.Context, pushEmptySigs() err = fmt.Errorf("unable to convert server nonces: %w", err) - return f.HandleError(err) + return returnError(err) } f.htlcServerNoncesHighFee, err = toNonces( loopInResp.HighFeeHtlcInfo.Nonces, @@ -202,7 +226,7 @@ func (f *FSM) InitHtlcAction(ctx context.Context, if err != nil { pushEmptySigs() - return f.HandleError(err) + return returnError(err) } f.htlcServerNoncesExtremelyHighFee, err = toNonces( loopInResp.ExtremeFeeHtlcInfo.Nonces, @@ -210,7 +234,7 @@ func (f *FSM) InitHtlcAction(ctx context.Context, if err != nil { pushEmptySigs() - return f.HandleError(err) + return returnError(err) } // We need to defend against the server setting high fees for the htlc @@ -232,7 +256,7 @@ func (f *FSM) InitHtlcAction(ctx context.Context, log.Errorf("server htlc tx fee is higher than the configured "+ "allowed maximum: %v > %v", fee, maxHtlcTxFee) - return f.HandleError(ErrFeeTooHigh) + return returnError(ErrFeeTooHigh) } f.loopIn.HtlcTxFeeRate = feeRate @@ -246,7 +270,7 @@ func (f *FSM) InitHtlcAction(ctx context.Context, "configured allowed maximum: %v > %v", fee, maxHtlcTxBackupFee) - return f.HandleError(ErrFeeTooHigh) + return returnError(ErrFeeTooHigh) } f.loopIn.HtlcTxHighFeeRate = highFeeRate @@ -262,7 +286,7 @@ func (f *FSM) InitHtlcAction(ctx context.Context, "configured allowed maximum: %v > %v", fee, maxHtlcTxBackupFee) - return f.HandleError(ErrFeeTooHigh) + return returnError(ErrFeeTooHigh) } f.loopIn.HtlcTxExtremelyHighFeeRate = extremelyHighFeeRate @@ -276,7 +300,7 @@ func (f *FSM) InitHtlcAction(ctx context.Context, err = fmt.Errorf("unable to derive htlc timeout sweep "+ "address: %w", err) - return f.HandleError(err) + return returnError(err) } f.loopIn.HtlcTimeoutSweepAddress = sweepAddress @@ -286,10 +310,30 @@ func (f *FSM) InitHtlcAction(ctx context.Context, pushEmptySigs() err = fmt.Errorf("unable to store loop-in in db: %w", err) - return f.HandleError(err) + return returnError(err) } - return OnHtlcInitiated + // Once the swap is stored, restart/recovery code owns invoice lifecycle. + invoiceNeedsCleanup = false + + event = OnHtlcInitiated + + return event +} + +// cancelSwapInvoice best-effort cancels the current swap invoice using a +// detached timeout-limited context so cleanup still runs even if the caller's +// context is already done. +func (f *FSM) cancelSwapInvoice(ctx context.Context) { + cleanupCtx, cancel := context.WithTimeout( + context.WithoutCancel(ctx), defaultInvoiceCleanupTimeout, + ) + defer cancel() + + err := f.cfg.InvoicesClient.CancelInvoice(cleanupCtx, f.loopIn.SwapHash) + if err != nil { + f.Warnf("unable to cancel invoice: %v", err) + } } // SignHtlcTxAction is called if the htlc was initialized and the server @@ -557,11 +601,9 @@ func (f *FSM) MonitorInvoiceAndHtlcTxAction(ctx context.Context, // Cancel the lndclient invoice subscription. cancelInvoiceSubscription() - err = f.cfg.InvoicesClient.CancelInvoice(ctx, f.loopIn.SwapHash) - if err != nil { - f.Warnf("unable to cancel invoice "+ - "for swap hash: %v", err) - } + // Reuse the same helper as InitHtlcAction so timeout cleanup follows + // the same detached-context path as early-init cleanup. + f.cancelSwapInvoice(ctx) } for { diff --git a/staticaddr/loopin/actions_test.go b/staticaddr/loopin/actions_test.go index 646e9898b..ebc804deb 100644 --- a/staticaddr/loopin/actions_test.go +++ b/staticaddr/loopin/actions_test.go @@ -13,10 +13,12 @@ import ( "github.com/lightninglabs/loop/staticaddr/deposit" "github.com/lightninglabs/loop/staticaddr/script" "github.com/lightninglabs/loop/staticaddr/version" + "github.com/lightninglabs/loop/swapserverrpc" "github.com/lightninglabs/loop/test" "github.com/lightningnetwork/lnd/invoices" "github.com/lightningnetwork/lnd/lntypes" "github.com/stretchr/testify/require" + "google.golang.org/grpc" ) // TestMonitorInvoiceAndHtlcTxReRegistersOnConfErr ensures that an error from @@ -123,6 +125,124 @@ func TestMonitorInvoiceAndHtlcTxReRegistersOnConfErr(t *testing.T) { } } +// TestInitHtlcActionCancelsInvoiceOnServerError verifies that an invoice +// created before a server-side rejection is canceled immediately. +func TestInitHtlcActionCancelsInvoiceOnServerError(t *testing.T) { + ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second) + defer cancel() + + mockLnd := test.NewMockLnd() + + loopIn := &StaticAddressLoopIn{ + Deposits: []*deposit.Deposit{{ + Value: 200_000, + }}, + InitiationHeight: uint32(mockLnd.Height), + InitiationTime: time.Now(), + PaymentTimeoutSeconds: DefaultPaymentTimeoutSeconds, + ProtocolVersion: version.ProtocolVersion_V0, + } + + cfg := &Config{ + AddressManager: &mockAddressManager{ + params: &address.Parameters{ + ProtocolVersion: version.ProtocolVersion_V0, + }, + }, + DepositManager: &noopDepositManager{}, + WalletKit: mockLnd.WalletKit, + LndClient: mockLnd.Client, + InvoicesClient: mockLnd.LndServices.Invoices, + Server: &initHtlcTestServer{ + loopInErr: errors.New("server rejected swap"), + }, + } + + f, err := NewFSM(ctx, loopIn, cfg, false) + require.NoError(t, err) + + // The init step should fail and synchronously trigger deferred invoice + // cleanup. + event := f.InitHtlcAction(ctx, nil) + require.Equal(t, fsm.OnError, event) + + select { + case hash := <-mockLnd.FailInvoiceChannel: + require.Equal(t, loopIn.SwapHash, hash) + + case <-ctx.Done(): + t.Fatalf("invoice was not canceled: %v", ctx.Err()) + } +} + +// TestInitHtlcActionCancelsInvoiceOnFeeGuardFailure verifies that the early +// fee guard also cancels the pre-created invoice before returning an error. +func TestInitHtlcActionCancelsInvoiceOnFeeGuardFailure(t *testing.T) { + ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second) + defer cancel() + + mockLnd := test.NewMockLnd() + serverKey, err := btcec.NewPrivateKey() + require.NoError(t, err) + + loopIn := &StaticAddressLoopIn{ + Deposits: []*deposit.Deposit{{ + Value: 200_000, + }}, + InitiationHeight: uint32(mockLnd.Height), + InitiationTime: time.Now(), + PaymentTimeoutSeconds: DefaultPaymentTimeoutSeconds, + ProtocolVersion: version.ProtocolVersion_V0, + } + + cfg := &Config{ + AddressManager: &mockAddressManager{ + params: &address.Parameters{ + ProtocolVersion: version.ProtocolVersion_V0, + }, + }, + DepositManager: &noopDepositManager{}, + WalletKit: mockLnd.WalletKit, + LndClient: mockLnd.Client, + InvoicesClient: mockLnd.LndServices.Invoices, + Server: &initHtlcTestServer{ + loopInResp: &swapserverrpc.ServerStaticAddressLoopInResponse{ + HtlcServerPubKey: serverKey.PubKey(). + SerializeCompressed(), + HtlcExpiry: mockLnd.Height + + DefaultLoopInOnChainCltvDelta, + StandardHtlcInfo: &swapserverrpc.ServerHtlcSigningInfo{ + FeeRate: 1_000_000, + }, + HighFeeHtlcInfo: &swapserverrpc.ServerHtlcSigningInfo{}, + ExtremeFeeHtlcInfo: &swapserverrpc. + ServerHtlcSigningInfo{}, + }, + }, + ValidateLoopInContract: func(int32, int32) error { + return nil + }, + MaxStaticAddrHtlcFeePercentage: 0, + MaxStaticAddrHtlcBackupFeePercentage: 1, + } + + f, err := NewFSM(ctx, loopIn, cfg, false) + require.NoError(t, err) + + // The fee guard runs before persistence, so the deferred cleanup must + // cancel the invoice on this error path as well. + event := f.InitHtlcAction(ctx, nil) + require.Equal(t, fsm.OnError, event) + + select { + case hash := <-mockLnd.FailInvoiceChannel: + require.Equal(t, loopIn.SwapHash, hash) + + case <-ctx.Done(): + t.Fatalf("invoice was not canceled: %v", ctx.Err()) + } +} + // mockAddressManager is a minimal AddressManager implementation used by the // test FSM setup. type mockAddressManager struct { @@ -180,3 +300,29 @@ func (n *noopDepositManager) GetActiveDepositsInState(fsm.StateType) ( return nil, nil } + +// initHtlcTestServer lets InitHtlcAction tests inject a deterministic server +// response without standing up the full gRPC client. +type initHtlcTestServer struct { + swapserverrpc.StaticAddressServerClient + + loopInResp *swapserverrpc.ServerStaticAddressLoopInResponse + loopInErr error +} + +// ServerStaticAddressLoopIn returns the canned response configured by the test. +func (s *initHtlcTestServer) ServerStaticAddressLoopIn(context.Context, + *swapserverrpc.ServerStaticAddressLoopInRequest, ...grpc.CallOption, +) (*swapserverrpc.ServerStaticAddressLoopInResponse, error) { + + return s.loopInResp, s.loopInErr +} + +// PushStaticAddressHtlcSigs accepts the abandonment signal used by error-path +// tests without adding additional assertions. +func (s *initHtlcTestServer) PushStaticAddressHtlcSigs(context.Context, + *swapserverrpc.PushStaticAddressHtlcSigsRequest, ...grpc.CallOption, +) (*swapserverrpc.PushStaticAddressHtlcSigsResponse, error) { + + return &swapserverrpc.PushStaticAddressHtlcSigsResponse{}, nil +} From 5f25def51173b4f9343ccddee78ee3ecf482a879 Mon Sep 17 00:00:00 2001 From: Slyghtning Date: Tue, 24 Mar 2026 17:34:26 +0100 Subject: [PATCH 7/9] staticaddr: guard channel open and withdraw against unconfirmed deposits Now that Deposited includes mempool outputs, channel opens and withdrawals must explicitly reject unconfirmed deposits (ConfirmationHeight <= 0) since both operations require confirmed inputs. --- loopd/swapclient_server.go | 2 +- loopd/swapclient_server_deposit_test.go | 27 +++++- staticaddr/openchannel/manager.go | 28 ++++++ staticaddr/openchannel/manager_test.go | 108 ++++++++++++++++++++++-- staticaddr/withdraw/manager.go | 9 ++ 5 files changed, 166 insertions(+), 8 deletions(-) diff --git a/loopd/swapclient_server.go b/loopd/swapclient_server.go index f5ad12682..104d696e3 100644 --- a/loopd/swapclient_server.go +++ b/loopd/swapclient_server.go @@ -1754,7 +1754,7 @@ func (s *swapClientServer) WithdrawDeposits(ctx context.Context, return nil, err } - for _, d := range deposits { + for _, d := range confirmedDeposits(deposits) { outpoints = append(outpoints, d.OutPoint) } diff --git a/loopd/swapclient_server_deposit_test.go b/loopd/swapclient_server_deposit_test.go index bded2da99..b49906241 100644 --- a/loopd/swapclient_server_deposit_test.go +++ b/loopd/swapclient_server_deposit_test.go @@ -1,6 +1,10 @@ package loopd -import "testing" +import ( + "testing" + + "github.com/lightninglabs/loop/staticaddr/deposit" +) // TestDepositBlocksUntilExpiry checks blocks-until-expiry handling for // confirmed and unconfirmed deposits. @@ -19,3 +23,24 @@ func TestDepositBlocksUntilExpiry(t *testing.T) { } }) } + +// TestConfirmedDeposits checks that helpers for bulk operations only keep +// deposits that can actually be spent on-chain. +func TestConfirmedDeposits(t *testing.T) { + t.Run("filters unconfirmed", func(t *testing.T) { + deposits := []*deposit.Deposit{ + {}, + { + ConfirmationHeight: 123, + }, + } + + filtered := confirmedDeposits(deposits) + if len(filtered) != 1 { + t.Fatalf("expected 1 confirmed deposit, got %d", len(filtered)) + } + if filtered[0].ConfirmationHeight != 123 { + t.Fatal("expected confirmed deposit to remain") + } + }) +} diff --git a/staticaddr/openchannel/manager.go b/staticaddr/openchannel/manager.go index 2274ce506..d2937506d 100644 --- a/staticaddr/openchannel/manager.go +++ b/staticaddr/openchannel/manager.go @@ -310,6 +310,10 @@ func (m *Manager) OpenChannel(ctx context.Context, return nil, err } + // Automatic channel funding must ignore mempool deposits because + // they cannot yet be used as funding inputs. + deposits = filterConfirmedDeposits(deposits) + if req.LocalFundingAmount != 0 { deposits, err = staticutil.SelectDeposits( deposits, req.LocalFundingAmount, @@ -325,6 +329,14 @@ func (m *Manager) OpenChannel(ctx context.Context, } } + for _, d := range deposits { + // Deposited now includes mempool outputs for static loop-ins, but + // channel opens still require the deposit input to be confirmed. + if d.ConfirmationHeight <= 0 { + return nil, ErrOpeningChannelUnavailableDeposits + } + } + // Pre-check: calculate the channel funding amount and the optional // change before locking deposits. This ensures the selected deposits // can cover the funding amount plus fees. @@ -399,6 +411,22 @@ func (m *Manager) OpenChannel(ctx context.Context, return nil, err } +// filterConfirmedDeposits filters the given deposits and returns only those +// that have a positive confirmation height, i.e. deposits that have been +// confirmed on-chain. +func filterConfirmedDeposits(deposits []*deposit.Deposit) []*deposit.Deposit { + confirmed := make([]*deposit.Deposit, 0, len(deposits)) + for _, d := range deposits { + if d.ConfirmationHeight <= 0 { + continue + } + + confirmed = append(confirmed, d) + } + + return confirmed +} + // openChannelPsbt starts an interactive channel open protocol that uses a // partially signed bitcoin transaction (PSBT) to fund the channel output. The // protocol involves several steps between the loop client and the server: diff --git a/staticaddr/openchannel/manager_test.go b/staticaddr/openchannel/manager_test.go index f408da169..e76e6a1b1 100644 --- a/staticaddr/openchannel/manager_test.go +++ b/staticaddr/openchannel/manager_test.go @@ -29,6 +29,7 @@ type transitionCall struct { } type mockDepositManager struct { + activeDeposits []*deposit.Deposit openingDeposits []*deposit.Deposit getErr error transitionErrs map[fsm.EventType]error @@ -44,15 +45,19 @@ func (m *mockDepositManager) AllOutpointsActiveDeposits([]wire.OutPoint, func (m *mockDepositManager) GetActiveDepositsInState(stateFilter fsm.StateType) ( []*deposit.Deposit, error) { - if stateFilter != deposit.OpeningChannel { - return nil, nil - } + switch stateFilter { + case deposit.Deposited: + return m.activeDeposits, nil + + case deposit.OpeningChannel: + if m.getErr != nil { + return nil, m.getErr + } - if m.getErr != nil { - return nil, m.getErr + return m.openingDeposits, nil } - return m.openingDeposits, nil + return nil, nil } func (m *mockDepositManager) TransitionDeposits(_ context.Context, @@ -464,6 +469,97 @@ func TestOpenChannelDuplicateOutpoints(t *testing.T) { require.ErrorContains(t, err, "duplicate outpoint") } +// TestOpenChannelSkipsUnconfirmedAutoSelection verifies that automatic coin +// selection ignores mempool deposits and keeps using confirmed ones. +func TestOpenChannelSkipsUnconfirmedAutoSelection(t *testing.T) { + t.Parallel() + + confirmedA := &deposit.Deposit{ + OutPoint: testOutPoint(1), + Value: 160_000, + ConfirmationHeight: 10, + } + confirmedB := &deposit.Deposit{ + OutPoint: testOutPoint(2), + Value: 140_000, + ConfirmationHeight: 11, + } + unconfirmed := &deposit.Deposit{ + OutPoint: testOutPoint(3), + Value: 500_000, + } + + depositManager := &mockDepositManager{ + activeDeposits: []*deposit.Deposit{ + unconfirmed, confirmedA, confirmedB, + }, + transitionErrs: map[fsm.EventType]error{ + deposit.OnOpeningChannel: errors.New("stop after selection"), + }, + } + manager := &Manager{ + cfg: &Config{ + DepositManager: depositManager, + }, + } + + req := &lnrpc.OpenChannelRequest{ + NodePubkey: make([]byte, 33), + LocalFundingAmount: 100_000, + SatPerVbyte: 10, + } + + _, err := manager.OpenChannel(context.Background(), req) + require.ErrorContains(t, err, "stop after selection") + require.Len(t, depositManager.calls, 1) + require.Equal(t, deposit.OnOpeningChannel, depositManager.calls[0].event) + require.NotContains(t, depositManager.calls[0].outpoints, unconfirmed.OutPoint) +} + +// TestOpenChannelFundMaxSkipsUnconfirmed verifies that fundmax only locks +// confirmed deposits. +func TestOpenChannelFundMaxSkipsUnconfirmed(t *testing.T) { + t.Parallel() + + confirmed := &deposit.Deposit{ + OutPoint: testOutPoint(1), + Value: 200_000, + ConfirmationHeight: 10, + } + unconfirmed := &deposit.Deposit{ + OutPoint: testOutPoint(2), + Value: 300_000, + } + + depositManager := &mockDepositManager{ + activeDeposits: []*deposit.Deposit{ + unconfirmed, confirmed, + }, + transitionErrs: map[fsm.EventType]error{ + deposit.OnOpeningChannel: errors.New("stop after selection"), + }, + } + manager := &Manager{ + cfg: &Config{ + DepositManager: depositManager, + }, + } + + req := &lnrpc.OpenChannelRequest{ + NodePubkey: make([]byte, 33), + FundMax: true, + SatPerVbyte: 10, + } + + _, err := manager.OpenChannel(context.Background(), req) + require.ErrorContains(t, err, "stop after selection") + require.Len(t, depositManager.calls, 1) + require.Equal( + t, []wire.OutPoint{confirmed.OutPoint}, + depositManager.calls[0].outpoints, + ) +} + // TestValidateInitialPsbtFlags verifies that request fields incompatible with // PSBT funding are rejected early, before any deposits are locked. func TestValidateInitialPsbtFlags(t *testing.T) { diff --git a/staticaddr/withdraw/manager.go b/staticaddr/withdraw/manager.go index 99fddd267..f43986881 100644 --- a/staticaddr/withdraw/manager.go +++ b/staticaddr/withdraw/manager.go @@ -381,6 +381,15 @@ func (m *Manager) WithdrawDeposits(ctx context.Context, } } + for _, d := range deposits { + // Deposited now includes mempool outputs for static loop-ins, but + // withdrawals still require the deposit input to be confirmed. + if d.ConfirmationHeight <= 0 { + return "", "", fmt.Errorf("can't withdraw, " + + "unconfirmed deposits can't be withdrawn") + } + } + var ( withdrawalAddress btcutil.Address err error From 9fdf7c4bde51eb3d0f0db801ef8e3f44c5e48a11 Mon Sep 17 00:00:00 2001 From: Slyghtning Date: Tue, 24 Mar 2026 17:34:57 +0100 Subject: [PATCH 8/9] cmd/loop: simplify no-deposits error and remove deposit.MinConfs Update the loop-in CLI error message to no longer reference a minimum confirmation count now that mempool deposits are surfaced immediately. Remove the unused MinConfs constant from the deposit package. --- cmd/loop/staticaddr.go | 7 +------ staticaddr/deposit/manager.go | 5 ----- 2 files changed, 1 insertion(+), 11 deletions(-) diff --git a/cmd/loop/staticaddr.go b/cmd/loop/staticaddr.go index fc36597e4..683cc5496 100644 --- a/cmd/loop/staticaddr.go +++ b/cmd/loop/staticaddr.go @@ -7,7 +7,6 @@ import ( "github.com/lightninglabs/loop/labels" "github.com/lightninglabs/loop/looprpc" - "github.com/lightninglabs/loop/staticaddr/deposit" "github.com/lightninglabs/loop/staticaddr/loopin" "github.com/lightninglabs/loop/swapserverrpc" lndcommands "github.com/lightningnetwork/lnd/cmd/commands" @@ -553,11 +552,7 @@ func staticAddressLoopIn(ctx context.Context, cmd *cli.Command) error { allDeposits := depositList.FilteredDeposits if len(allDeposits) == 0 { - errString := fmt.Sprintf("no confirmed deposits available, "+ - "deposits need at least %v confirmations", - deposit.MinConfs) - - return errors.New(errString) + return errors.New("no deposited outputs available") } var depositOutpoints []string diff --git a/staticaddr/deposit/manager.go b/staticaddr/deposit/manager.go index c28fecada..8148d8b91 100644 --- a/staticaddr/deposit/manager.go +++ b/staticaddr/deposit/manager.go @@ -18,11 +18,6 @@ import ( ) const ( - // MinConfs is the minimum number of confirmations we require for a - // deposit to be considered available for loop-ins, coop-spends and - // timeouts. - MinConfs = 6 - // MaxConfs is unset since we don't require a max number of // confirmations for deposits. MaxConfs = 0 From 6babc2d81af5b1344e12f2ed9c6cc070d1642b90 Mon Sep 17 00:00:00 2001 From: Slyghtning Date: Thu, 26 Mar 2026 14:34:08 +0100 Subject: [PATCH 9/9] cmd/loop: warn user about low-confirmation deposits before loop-in Display a warning when selected deposits have fewer than 6 confirmations, since the swap payment for those won't be received immediately. Works for both manually selected and auto-selected deposits by deriving confirmation count from the CSV expiry and blocks-until-expiry fields. --- cmd/loop/staticaddr.go | 98 +++++++++++++++++++++++++++++++++++++ cmd/loop/staticaddr_test.go | 57 +++++++++++++++++++++ 2 files changed, 155 insertions(+) create mode 100644 cmd/loop/staticaddr_test.go diff --git a/cmd/loop/staticaddr.go b/cmd/loop/staticaddr.go index 683cc5496..434b1ccb0 100644 --- a/cmd/loop/staticaddr.go +++ b/cmd/loop/staticaddr.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "strings" "github.com/lightninglabs/loop/labels" "github.com/lightninglabs/loop/looprpc" @@ -555,6 +556,13 @@ func staticAddressLoopIn(ctx context.Context, cmd *cli.Command) error { return errors.New("no deposited outputs available") } + summary, err := client.GetStaticAddressSummary( + ctx, &looprpc.StaticAddressSummaryRequest{}, + ) + if err != nil { + return err + } + var depositOutpoints []string switch { case isAllSelected && isUtxoSelected: @@ -609,6 +617,22 @@ func staticAddressLoopIn(ctx context.Context, cmd *cli.Command) error { return err } + // Warn the user if any selected deposits have fewer than 6 + // confirmations, as the swap payment won't be received immediately + // for those. + depositsToCheck := depositOutpoints + if autoSelectDepositsForQuote { + // When auto-selecting, any deposit could be chosen. + depositsToCheck = depositsToOutpoints(allDeposits) + } + warning := lowConfDepositWarning( + allDeposits, depositsToCheck, + int64(summary.RelativeExpiryBlocks), + ) + if warning != "" { + fmt.Println(warning) + } + if !(cmd.Bool("force") || cmd.Bool("f")) { err = displayInDetails(quoteReq, quote, cmd.Bool("verbose")) if err != nil { @@ -664,6 +688,80 @@ func depositsToOutpoints(deposits []*looprpc.Deposit) []string { return outpoints } +// conservativeWarningConfs is the highest default confirmation tier used by +// the server's dynamic confirmation-risk policy. +// +// The CLI does not currently know the server's exact policy, so we use this +// conservative threshold for warnings without promising immediate execution. +const conservativeWarningConfs = 6 + +// lowConfDepositWarning checks the selected deposits against a conservative +// confirmation threshold and returns a warning string if any are found. +func lowConfDepositWarning(allDeposits []*looprpc.Deposit, + selectedOutpoints []string, csvExpiry int64) string { + + depositMap := make(map[string]*looprpc.Deposit, len(allDeposits)) + for _, d := range allDeposits { + depositMap[d.Outpoint] = d + } + + var lowConfEntries []string + for _, op := range selectedOutpoints { + d, ok := depositMap[op] + if !ok { + continue + } + + var confs int64 + switch { + case d.ConfirmationHeight <= 0: + confs = 0 + + case csvExpiry > 0: + // For confirmed deposits we can compute + // confirmations as CSVExpiry - BlocksUntilExpiry + 1. + confs = csvExpiry - d.BlocksUntilExpiry + 1 + + default: + // Can't determine confirmations without the CSV expiry. + continue + } + + if confs >= conservativeWarningConfs { + continue + } + + if confs == 0 { + lowConfEntries = append( + lowConfEntries, + fmt.Sprintf(" - %s (unconfirmed)", op), + ) + } else { + lowConfEntries = append( + lowConfEntries, + fmt.Sprintf( + " - %s (%d confirmations)", op, + confs, + ), + ) + } + } + + if len(lowConfEntries) == 0 { + return "" + } + + return fmt.Sprintf( + "\nWARNING: The following deposits are below the "+ + "conservative %d-confirmation threshold:\n%s\n"+ + "The swap payment for these deposits may wait for "+ + "more confirmations depending on the server's "+ + "confirmation-risk policy.\n", + conservativeWarningConfs, + strings.Join(lowConfEntries, "\n"), + ) +} + func displayNewAddressWarning() error { fmt.Printf("\nWARNING: Be aware that loosing your l402.token file in " + ".loop under your home directory will take your ability to " + diff --git a/cmd/loop/staticaddr_test.go b/cmd/loop/staticaddr_test.go new file mode 100644 index 000000000..996d9e424 --- /dev/null +++ b/cmd/loop/staticaddr_test.go @@ -0,0 +1,57 @@ +package main + +import ( + "strings" + "testing" + + "github.com/lightninglabs/loop/looprpc" + "github.com/stretchr/testify/require" +) + +func TestLowConfDepositWarningConfirmedOnly(t *testing.T) { + t.Parallel() + + deposits := []*looprpc.Deposit{ + { + Outpoint: "confirmed-low", + ConfirmationHeight: 100, + BlocksUntilExpiry: 140, + }, + { + Outpoint: "confirmed-high", + ConfirmationHeight: 95, + BlocksUntilExpiry: 139, + }, + } + + warning := lowConfDepositWarning( + deposits, []string{"confirmed-low", "confirmed-high"}, 144, + ) + + require.Contains(t, warning, "confirmed-low (5 confirmations)") + require.NotContains(t, warning, "confirmed-high") +} + +func TestLowConfDepositWarningUnconfirmed(t *testing.T) { + t.Parallel() + + deposits := []*looprpc.Deposit{ + { + Outpoint: "mempool", + ConfirmationHeight: 0, + BlocksUntilExpiry: 144, + }, + } + + warning := lowConfDepositWarning(deposits, []string{"mempool"}, 144) + + require.Contains(t, warning, "mempool (unconfirmed)") + require.True( + t, + strings.Contains( + warning, + "conservative 6-confirmation threshold", + ), + ) + require.NotContains(t, warning, "executed immediately") +}