Merge pull request lightninglabs#1096 from hieblmi/revert-dyn-conf

staticaddr: revert dynamic conf

Author: hieblmi

staticaddr/deposit/manager.go

@@ -20,7 +20,7 @@ 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 = 3
+	MinConfs = 6
 
 	// MaxConfs is unset since we don't require a max number of
 	// confirmations for deposits.

staticaddr/loopin/actions.go

@@ -30,7 +30,6 @@ import (
 	"github.com/lightningnetwork/lnd/lnwallet"
 	"github.com/lightningnetwork/lnd/lnwallet/chainfee"
 	"github.com/lightningnetwork/lnd/lnwire"
-	"google.golang.org/grpc/status"
 )
 
 const (
@@ -147,10 +146,6 @@ func (f *FSM) InitHtlcAction(ctx context.Context,
 		ctx, loopInReq,
 	)
 	if err != nil {
-		// Check if this is an insufficient confirmations error and log
-		// the details to help the user understand what's needed.
-		logInsufficientConfirmationsDetails(err)
-
 		err = fmt.Errorf("unable to initiate the loop-in with the "+
 			"server: %w", err)
 
@@ -919,30 +914,3 @@ func byteSliceTo66ByteSlice(b []byte) ([musig2.PubNonceSize]byte, error) {
 
 	return res, nil
 }
-
-// logInsufficientConfirmationsDetails extracts and logs the per-deposit
-// confirmation details from a gRPC error if present.
-func logInsufficientConfirmationsDetails(err error) {
-	st, ok := status.FromError(err)
-	if !ok {
-		return
-	}
-
-	for _, detail := range st.Details() {
-		confDetails, ok :=
-			detail.(*swapserverrpc.InsufficientConfirmationsDetails)
-
-		if !ok {
-			continue
-		}
-
-		log.Warnf("Insufficient deposit confirmations, max wait: %d blocks",
-			confDetails.MaxBlocksToWait)
-
-		for _, dep := range confDetails.Deposits {
-			log.Warnf("  Deposit %s: %d/%d confirmations (need %d more blocks)",
-				dep.Outpoint, dep.CurrentConfirmations,
-				dep.RequiredConfirmations, dep.BlocksToWait)
-		}
-	}
-}

staticaddr/loopin/manager.go

@@ -850,13 +850,11 @@ func (m *Manager) GetAllSwaps(ctx context.Context) ([]*StaticAddressLoopIn,
 	return swaps, nil
 }
 
-// SelectDeposits sorts the deposits to optimize for successful swaps with
-// dynamic confirmation requirements: 1) more confirmations first (higher chance
-// of server acceptance), 2) larger amounts first (to minimize number of deposits
-// used), 3) expiring sooner first (to use time-sensitive deposits). 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 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.
 func SelectDeposits(targetAmount btcutil.Amount,
 	unfilteredDeposits []*deposit.Deposit, csvExpiry uint32,
 	blockHeight uint32) ([]*deposit.Deposit, error) {
@@ -877,28 +875,17 @@ func SelectDeposits(targetAmount btcutil.Amount,
 		deposits = append(deposits, d)
 	}
 
-	// Sort deposits to optimize for successful swaps with dynamic
-	// confirmation requirements:
-	// 1. More confirmations first (higher chance of server acceptance)
-	// 2. Larger amounts first (to minimize number of deposits used)
+	// Sort the deposits by amount in descending order, then by
+	// blocks-until-expiry in ascending order.
 	sort.Slice(deposits, func(i, j int) bool {
-		// Primary: more confirmations first. Guard against the
-		// theoretical case where ConfirmationHeight > blockHeight
-		// (e.g. during a transient reorg inconsistency).
-		var iConfs, jConfs uint32
-		if blockHeight > uint32(deposits[i].ConfirmationHeight) {
-			iConfs = blockHeight -
-				uint32(deposits[i].ConfirmationHeight)
-		}
-		if blockHeight > uint32(deposits[j].ConfirmationHeight) {
-			jConfs = blockHeight -
-				uint32(deposits[j].ConfirmationHeight)
-		}
-		if iConfs != jConfs {
-			return iConfs > jConfs
-		}
+		if deposits[i].Value == deposits[j].Value {
+			iExp := uint32(deposits[i].ConfirmationHeight) +
+				csvExpiry - blockHeight
+			jExp := uint32(deposits[j].ConfirmationHeight) +
+				csvExpiry - blockHeight
 
-		// Secondary: larger amounts first.
+			return iExp < jExp
+		}
 		return deposits[i].Value > deposits[j].Value
 	})
 

staticaddr/loopin/manager_test.go

@@ -26,14 +26,10 @@ type testCase struct {
 
 // TestSelectDeposits tests the selectDeposits function, which selects
 // deposits that can cover a target value while respecting the dust limit.
-// Sorting priority: 1) more confirmations first, 2) larger amounts first,
-// 3) expiring sooner first.
 func TestSelectDeposits(t *testing.T) {
-	// Note: confirmations = blockHeight - ConfirmationHeight
-	// Lower ConfirmationHeight means more confirmations at a given block.
 	d1, d2, d3, d4 := &deposit.Deposit{
 		Value:              1_000_000,
-		ConfirmationHeight: 5_000, // most confs at height 5100
+		ConfirmationHeight: 5_000,
 	}, &deposit.Deposit{
 		Value:              2_000_000,
 		ConfirmationHeight: 5_001,
@@ -42,7 +38,7 @@ func TestSelectDeposits(t *testing.T) {
 		ConfirmationHeight: 5_002,
 	}, &deposit.Deposit{
 		Value:              3_000_000,
-		ConfirmationHeight: 5_003, // fewest confs at height 5100
+		ConfirmationHeight: 5_003,
 	}
 	d1.Hash = chainhash.Hash{1}
 	d1.Index = 0
@@ -53,108 +49,75 @@ func TestSelectDeposits(t *testing.T) {
 	d4.Hash = chainhash.Hash{4}
 	d4.Index = 0
 
-	// Use a realistic block height and csv expiry for all standard
-	// test cases. csvExpiry must be large enough that deposits remain
-	// swappable at this block height.
-	const (
-		testBlockHeight uint32 = 5_100
-		testCsvExpiry   uint32 = 2_500
-	)
-
 	testCases := []testCase{
 		{
 			name:        "single deposit exact target",
 			deposits:    []*deposit.Deposit{d1},
 			targetValue: 1_000_000,
-			csvExpiry:   testCsvExpiry,
-			blockHeight: testBlockHeight,
 			expected:    []*deposit.Deposit{d1},
 			expectedErr: "",
 		},
 		{
-			// d1 has more confirmations, so it's preferred even
-			// though d2 is larger.
-			name:        "prefer more confirmed deposit over larger",
+			name:        "prefer larger deposit when both cover",
 			deposits:    []*deposit.Deposit{d1, d2},
 			targetValue: 1_000_000,
-			csvExpiry:   testCsvExpiry,
-			blockHeight: testBlockHeight,
-			expected:    []*deposit.Deposit{d1},
+			expected:    []*deposit.Deposit{d2},
 			expectedErr: "",
 		},
 		{
-			// d1 has the most confirmations among d1, d2, d3.
-			name:        "prefer most confirmed among three",
+			name:        "prefer largest among three when one is enough",
 			deposits:    []*deposit.Deposit{d1, d2, d3},
 			targetValue: 1_000_000,
-			csvExpiry:   testCsvExpiry,
-			blockHeight: testBlockHeight,
-			expected:    []*deposit.Deposit{d1},
+			expected:    []*deposit.Deposit{d3},
 			expectedErr: "",
 		},
 		{
 			name:        "single deposit insufficient by 1",
 			deposits:    []*deposit.Deposit{d1},
 			targetValue: 1_000_001,
-			csvExpiry:   testCsvExpiry,
-			blockHeight: testBlockHeight,
 			expected:    []*deposit.Deposit{},
 			expectedErr: "not enough deposits to cover",
 		},
 		{
 			name:        "target leaves exact dust limit change",
 			deposits:    []*deposit.Deposit{d1},
 			targetValue: 1_000_000 - dustLimit,
-			csvExpiry:   testCsvExpiry,
-			blockHeight: testBlockHeight,
 			expected:    []*deposit.Deposit{d1},
 			expectedErr: "",
 		},
 		{
 			name:        "target leaves dust change (just over)",
 			deposits:    []*deposit.Deposit{d1},
 			targetValue: 1_000_000 - dustLimit + 1,
-			csvExpiry:   testCsvExpiry,
-			blockHeight: testBlockHeight,
 			expected:    []*deposit.Deposit{},
 			expectedErr: "not enough deposits to cover",
 		},
 		{
 			name:        "all deposits exactly match target",
 			deposits:    []*deposit.Deposit{d1, d2, d3},
 			targetValue: d1.Value + d2.Value + d3.Value,
-			csvExpiry:   testCsvExpiry,
-			blockHeight: testBlockHeight,
 			expected:    []*deposit.Deposit{d1, d2, d3},
 			expectedErr: "",
 		},
 		{
 			name:        "sum minus dust limit is allowed (change == dust)",
 			deposits:    []*deposit.Deposit{d1, d2, d3},
 			targetValue: d1.Value + d2.Value + d3.Value - dustLimit,
-			csvExpiry:   testCsvExpiry,
-			blockHeight: testBlockHeight,
 			expected:    []*deposit.Deposit{d1, d2, d3},
 			expectedErr: "",
 		},
 		{
 			name:        "sum minus dust limit plus 1 is not allowed (dust change)",
 			deposits:    []*deposit.Deposit{d1, d2, d3},
 			targetValue: d1.Value + d2.Value + d3.Value - dustLimit + 1,
-			csvExpiry:   testCsvExpiry,
-			blockHeight: testBlockHeight,
 			expected:    []*deposit.Deposit{},
 			expectedErr: "not enough deposits to cover",
 		},
 		{
-			// d3 and d4 have the same value but d3 has more
-			// confirmations (lower ConfirmationHeight), so it
-			// wins at the primary sort level.
-			name:        "same value, prefer more confirmed",
+			name:        "tie by value, prefer earlier expiry",
 			deposits:    []*deposit.Deposit{d3, d4},
-			targetValue: d4.Value - dustLimit,
-			csvExpiry:   testCsvExpiry,
-			blockHeight: testBlockHeight,
+			targetValue: d4.Value - dustLimit, // d3/d4 have the
+			// same value but different expiration.
 			expected:    []*deposit.Deposit{d3},
 			expectedErr: "",
 		},