diff --git a/staticaddr/loopin/autoloop_dp.go b/staticaddr/loopin/autoloop_dp.go index c605c43e..d8abde50 100644 --- a/staticaddr/loopin/autoloop_dp.go +++ b/staticaddr/loopin/autoloop_dp.go @@ -246,8 +246,10 @@ func filterAutoloopCandidateDeposits(maxAmount btcutil.Amount, continue } - residualLife := candidateDeposit.ConfirmationHeight + - int64(csvExpiry) - int64(blockHeight) + residualLife := int64(blocksUntilDepositExpiry( + uint32(candidateDeposit.ConfirmationHeight), + blockHeight, csvExpiry, + )) eligibleDeposits = append( eligibleDeposits, autoloopCandidateDeposit{ diff --git a/staticaddr/loopin/autoloop_dp_test.go b/staticaddr/loopin/autoloop_dp_test.go index c1253670..bf197ad7 100644 --- a/staticaddr/loopin/autoloop_dp_test.go +++ b/staticaddr/loopin/autoloop_dp_test.go @@ -80,6 +80,28 @@ func TestSelectNoChangeDepositsWithMemoryBudget(t *testing.T) { } } +// TestSelectNoChangeDepositsPrefersConfirmedTie verifies unconfirmed deposits +// are not treated as earlier-expiring than confirmed deposits. Their CSV timer +// has not started yet, so a same-value confirmed deposit should win the expiry +// tie-break. +func TestSelectNoChangeDepositsPrefersConfirmedTie(t *testing.T) { + t.Parallel() + + unconfirmed := makeDeposit(34, 0, 5_000, 0) + confirmed := makeDeposit(35, 0, 5_000, 200) + + deposits, err := selectNoChangeDeposits( + 5_000, 5_000, []*deposit.Deposit{ + unconfirmed, confirmed, + }, 1_000, 100, nil, + ) + require.NoError(t, err) + require.Equal( + t, []string{confirmed.OutPoint.String()}, + depositOutpoints(deposits), + ) +} + // TestAutoloopDPSizing verifies the bucket sizing math. These cases are easier // to understand directly than by inferring the step from a larger selector // behavior test. diff --git a/staticaddr/loopin/manager.go b/staticaddr/loopin/manager.go index a76cbba3..a78adfe4 100644 --- a/staticaddr/loopin/manager.go +++ b/staticaddr/loopin/manager.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "fmt" + "math" "slices" "sort" "sync/atomic" @@ -845,11 +846,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) { @@ -870,14 +871,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 } @@ -909,20 +921,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 { - // The deposit expiry height is the confirmation height plus the csv - // expiry. - depositExpiryHeight := confirmationHeight + csvExpiry - - // The htlc expiry height is the current height plus the htlc - // cltv delta. - htlcExpiryHeight := blockHeight + DefaultLoopInOnChainCltvDelta - - // Ensure that the deposit doesn't expire before the htlc. - if depositExpiryHeight < htlcExpiryHeight+DepositHtlcDelta { - return false + if confirmationHeight == 0 { + return true } - return true + // The deposit expiry height is the confirmation height plus the csv + // expiry. + return blocksUntilDepositExpiry( + confirmationHeight, blockHeight, csvExpiry, + ) >= DefaultLoopInOnChainCltvDelta+DepositHtlcDelta +} + +// 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 { + + if confirmationHeight == 0 { + return math.MaxUint32 + } + + depositExpiryHeight := confirmationHeight + csvExpiry + if depositExpiryHeight <= blockHeight { + return 0 + } + + 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 85178a05..58549747 100644 --- a/staticaddr/loopin/manager_test.go +++ b/staticaddr/loopin/manager_test.go @@ -81,6 +81,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}, @@ -298,6 +319,12 @@ func TestHandleLoopInSweepReqRejectsInvalidServerNonce(t *testing.T) { require.ErrorContains(t, err, depOutpoint) } +// 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 { // activeDeposits is the set returned by GetActiveDepositsInState.