Merge pull request #995 from hieblmi/listunspent-fix

staticaddr: filter unavailable deposits in `ListUnspentDeposits`

Author: hieblmi

loopd/swapclient_server.go

@@ -1546,9 +1546,65 @@ func (s *swapClientServer) ListUnspentDeposits(ctx context.Context,
 		return nil, err
 	}
 
-	// Prepare the list response.
+	// ListUnspentRaw returns the unspent wallet view of the backing lnd
+	// wallet. It might be that deposits show up there that are actually
+	// 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{})
+	)
+
+	// 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{}{}
+		}
+	}
+
+	// Check the spent status of the deposits by looking at their states.
+	deposits, err := s.depositManager.DepositsForOutpoints(ctx, outpoints)
+	if err != nil {
+		return nil, err
+	}
+	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.
+		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{}{}
+	}
+
+	// Prepare the list of unspent deposits for the rpc response.
 	var respUtxos []*looprpc.Utxo
 	for _, u := range utxos {
+		if _, ok := isUnspent[u.OutPoint]; !ok {
+			continue
+		}
+
 		utxo := &looprpc.Utxo{
 			StaticAddress: staticAddress.String(),
 			AmountSat:     int64(u.Value),

loopd/swapclient_server_test.go

@@ -8,15 +8,21 @@ import (
 
 	"github.com/btcsuite/btcd/btcutil"
 	"github.com/btcsuite/btcd/chaincfg"
+	"github.com/btcsuite/btcd/chaincfg/chainhash"
+	"github.com/btcsuite/btcd/wire"
 	"github.com/btcsuite/btclog/v2"
 	"github.com/lightninglabs/lndclient"
 	"github.com/lightninglabs/loop"
+	"github.com/lightninglabs/loop/fsm"
 	"github.com/lightninglabs/loop/labels"
 	"github.com/lightninglabs/loop/loopdb"
 	"github.com/lightninglabs/loop/looprpc"
+	"github.com/lightninglabs/loop/staticaddr/address"
+	"github.com/lightninglabs/loop/staticaddr/deposit"
 	"github.com/lightninglabs/loop/swap"
 	mock_lnd "github.com/lightninglabs/loop/test"
 	"github.com/lightningnetwork/lnd/lntypes"
+	"github.com/lightningnetwork/lnd/lnwallet"
 	"github.com/lightningnetwork/lnd/lnwire"
 	"github.com/lightningnetwork/lnd/routing/route"
 	"github.com/stretchr/testify/require"
@@ -891,3 +897,233 @@ func TestListSwapsFilterAndPagination(t *testing.T) {
 		})
 	}
 }
+
+// mockAddressStore is a minimal in-memory store for address parameters.
+type mockAddressStore struct {
+	params []*address.Parameters
+}
+
+func (s *mockAddressStore) CreateStaticAddress(_ context.Context,
+	p *address.Parameters) error {
+
+	s.params = append(s.params, p)
+	return nil
+}
+
+func (s *mockAddressStore) GetStaticAddress(_ context.Context, _ []byte) (
+	*address.Parameters, error) {
+
+	if len(s.params) == 0 {
+		return nil, nil
+	}
+
+	return s.params[0], nil
+}
+
+func (s *mockAddressStore) GetAllStaticAddresses(_ context.Context) (
+	[]*address.Parameters, error) {
+
+	return s.params, nil
+}
+
+// mockDepositStore implements deposit.Store minimally for DepositsForOutpoints.
+type mockDepositStore struct {
+	byOutpoint map[string]*deposit.Deposit
+}
+
+func (s *mockDepositStore) CreateDeposit(_ context.Context,
+	_ *deposit.Deposit) error {
+
+	return nil
+}
+
+func (s *mockDepositStore) UpdateDeposit(_ context.Context,
+	_ *deposit.Deposit) error {
+
+	return nil
+}
+
+func (s *mockDepositStore) GetDeposit(_ context.Context,
+	_ deposit.ID) (*deposit.Deposit, error) {
+
+	return nil, nil
+}
+
+func (s *mockDepositStore) DepositForOutpoint(_ context.Context,
+	outpoint string) (*deposit.Deposit, error) {
+
+	if d, ok := s.byOutpoint[outpoint]; ok {
+		return d, nil
+	}
+	return nil, nil
+}
+func (s *mockDepositStore) AllDeposits(_ context.Context) ([]*deposit.Deposit,
+	error) {
+
+	return nil, nil
+}
+
+// TestListUnspentDeposits tests filtering behavior of ListUnspentDeposits.
+func TestListUnspentDeposits(t *testing.T) {
+	ctx := context.Background()
+	mock := mock_lnd.NewMockLnd()
+
+	// Prepare a single static address parameter set.
+	_, client := mock_lnd.CreateKey(1)
+	_, server := mock_lnd.CreateKey(2)
+	pkScript := []byte("pkscript")
+	addrParams := &address.Parameters{
+		ClientPubkey: client,
+		ServerPubkey: server,
+		Expiry:       10,
+		PkScript:     pkScript,
+	}
+
+	addrStore := &mockAddressStore{params: []*address.Parameters{addrParams}}
+
+	// Build an address manager using our mock lnd and fake address store.
+	addrMgr := address.NewManager(&address.ManagerConfig{
+		Store:       addrStore,
+		WalletKit:   mock.WalletKit,
+		ChainParams: mock.ChainParams,
+		// ChainNotifier and AddressClient are not needed for this test.
+	}, 0)
+
+	// Construct several UTXOs with different confirmation counts.
+	makeUtxo := func(idx uint32, confs int64) *lnwallet.Utxo {
+		return &lnwallet.Utxo{
+			AddressType:   lnwallet.TaprootPubkey,
+			Value:         btcutil.Amount(250_000 + int64(idx)),
+			Confirmations: confs,
+			PkScript:      pkScript,
+			OutPoint: wire.OutPoint{
+				Hash:  chainhash.Hash{byte(idx + 1)},
+				Index: idx,
+			},
+		}
+	}
+
+	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)
+
+	// Helper to build the deposit manager with specific states.
+	buildDepositMgr := func(
+		states map[wire.OutPoint]fsm.StateType) *deposit.Manager {
+
+		store := &mockDepositStore{
+			byOutpoint: make(map[string]*deposit.Deposit),
+		}
+		for op, state := range states {
+			d := &deposit.Deposit{OutPoint: op}
+			d.SetState(state)
+			store.byOutpoint[op.String()] = d
+		}
+
+		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",
+		func(t *testing.T) {
+			mock.SetListUnspent([]*lnwallet.Utxo{
+				utxoBelow, utxoAt, utxoAbove1, utxoAbove2,
+			})
+
+			depMgr := buildDepositMgr(map[wire.OutPoint]fsm.StateType{
+				utxoAt.OutPoint:     deposit.Deposited,
+				utxoAbove1.OutPoint: deposit.Withdrawn,
+				utxoAbove2.OutPoint: deposit.LoopingIn,
+			})
+
+			server := &swapClientServer{
+				staticAddressManager: addrMgr,
+				depositManager:       depMgr,
+			}
+
+			resp, err := server.ListUnspentDeposits(
+				ctx, &looprpc.ListUnspentDepositsRequest{},
+			)
+			require.NoError(t, err)
+
+			// Expect utxoBelow and utxoAt only.
+			require.Len(t, resp.Utxos, 2)
+			got := map[string]struct{}{}
+			for _, u := range resp.Utxos {
+				got[u.Outpoint] = struct{}{}
+				// Confirm address string is non-empty and the
+				// same across utxos.
+				require.NotEmpty(t, u.StaticAddress)
+			}
+			_, ok1 := got[utxoBelow.OutPoint.String()]
+			_, ok2 := got[utxoAt.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",
+		func(t *testing.T) {
+			mock.SetListUnspent(
+				[]*lnwallet.Utxo{
+					utxoBelow, utxoAt, utxoAbove1,
+					utxoAbove2,
+				})
+
+			depMgr := buildDepositMgr(map[wire.OutPoint]fsm.StateType{
+				utxoAt.OutPoint:     deposit.Withdrawn,
+				utxoAbove1.OutPoint: deposit.Deposited,
+				utxoAbove2.OutPoint: deposit.Withdrawn,
+			})
+
+			server := &swapClientServer{
+				staticAddressManager: addrMgr,
+				depositManager:       depMgr,
+			}
+
+			resp, err := server.ListUnspentDeposits(
+				ctx, &looprpc.ListUnspentDepositsRequest{},
+			)
+			require.NoError(t, err)
+
+			require.Len(t, resp.Utxos, 2)
+			got := map[string]struct{}{}
+			for _, u := range resp.Utxos {
+				got[u.Outpoint] = struct{}{}
+			}
+			_, ok1 := got[utxoBelow.OutPoint.String()]
+			_, ok2 := got[utxoAbove1.OutPoint.String()]
+			require.True(t, ok1)
+			require.True(t, ok2)
+		})
+
+	// Confirmed UTXO not present in store should be included.
+	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})
+
+		// Empty store (no states for any outpoint).
+		depMgr := buildDepositMgr(map[wire.OutPoint]fsm.StateType{})
+
+		server := &swapClientServer{
+			staticAddressManager: addrMgr,
+			depositManager:       depMgr,
+		}
+
+		resp, err := server.ListUnspentDeposits(
+			ctx, &looprpc.ListUnspentDepositsRequest{},
+		)
+		require.NoError(t, err)
+
+		// We expect the confirmed UTXO to be included even though it
+		// doesn't exist in the store yet.
+		require.Len(t, resp.Utxos, 1)
+		require.Equal(
+			t, utxoAbove2.OutPoint.String(), resp.Utxos[0].Outpoint,
+		)
+		require.NotEmpty(t, resp.Utxos[0].StaticAddress)
+	})
+}

test/lnd_services_mock.go

@@ -290,3 +290,9 @@ func (s *LndMockServices) SetFeeEstimate(confTarget int32,
 func (s *LndMockServices) SetMinRelayFee(feeEstimate chainfee.SatPerKWeight) {
 	s.LndServices.WalletKit.(*mockWalletKit).setMinRelayFee(feeEstimate)
 }
+
+// SetListUnspent sets the list of UTXOs returned by the mock's WalletKit
+// ListUnspent call.
+func (s *LndMockServices) SetListUnspent(utxos []*lnwallet.Utxo) {
+	s.LndServices.WalletKit.(*mockWalletKit).setListUnspent(utxos)
+}

test/walletkit_mock.go

@@ -36,6 +36,9 @@ type mockWalletKit struct {
 	feeEstimateLock sync.Mutex
 	feeEstimates    map[int32]chainfee.SatPerKWeight
 	minRelayFee     chainfee.SatPerKWeight
+
+	// listUnspent holds test UTXOs to be returned by ListUnspent.
+	listUnspent []*lnwallet.Utxo
 }
 
 var _ lndclient.WalletKitClient = (*mockWalletKit)(nil)
@@ -51,7 +54,7 @@ func (m *mockWalletKit) ListUnspent(ctx context.Context, minConfs,
 	maxConfs int32, opts ...lndclient.ListUnspentOption) (
 	[]*lnwallet.Utxo, error) {
 
-	return nil, nil
+	return m.listUnspent, nil
 }
 
 func (m *mockWalletKit) ListLeases(
@@ -184,6 +187,11 @@ func (m *mockWalletKit) setMinRelayFee(fee chainfee.SatPerKWeight) {
 	m.minRelayFee = fee
 }
 
+// setListUnspent sets the list of UTXOs returned by ListUnspent.
+func (m *mockWalletKit) setListUnspent(utxos []*lnwallet.Utxo) {
+	m.listUnspent = utxos
+}
+
 // MinRelayFee returns the current minimum relay fee based on our chain backend
 // in sat/kw. It can be set with setMinRelayFee.
 func (m *mockWalletKit) MinRelayFee(