Skip to content

staticaddr: dynamic deposit conf target #1097

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
hieblmi wants to merge 2 commits into
base: master
Choose a base branch
from
+408 −102
Open

staticaddr: dynamic deposit conf target #1097

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0d8fb74
swapserverrpc: add proto messages for confirmation error details
hieblmi Jan 9, 2026
d22be64
staticaddr: support dynamic deposit confirmation requirements
hieblmi Jan 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion staticaddr/deposit/manager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 = 6
MinConfs = 3
Copy link
Copy Markdown
Collaborator

@starius starius Apr 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to hold this PR until we deploy the server side. Otherwise a new client might send its deposits too early to an old server.


// MaxConfs is unset since we don't require a max number of
// confirmations for deposits.
Expand Down
32 changes: 32 additions & 0 deletions staticaddr/loopin/actions.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import (
"github.com/lightningnetwork/lnd/lnwallet"
"github.com/lightningnetwork/lnd/lnwallet/chainfee"
"github.com/lightningnetwork/lnd/lnwire"
"google.golang.org/grpc/status"
)

const (
Expand Down Expand Up @@ -146,6 +147,10 @@ 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)
Copy link
Copy Markdown
Collaborator

@starius starius Apr 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new normal insufficient-confirmation path leaks an invoice. InitHtlcAction creates the swap invoice before it calls ServerStaticAddressLoopIn in staticaddr/loopin/actions.go:109-148. If the server rejects the request, the new branch only logs the structured details and returns HandleError in staticaddr/loopin/actions.go:149-157.

The only invoice cancel path I found is much later in the payment-timeout monitor in staticaddr/loopin/actions.go:558-569, which is never reached for this early failure.

Early ServerStaticAddressLoopIn failures, and the follow-on fee-validation failures in staticaddr/loopin/actions.go:231-270, should cancel the freshly created invoice before unlocking the deposits.


err = fmt.Errorf("unable to initiate the loop-in with the "+
"server: %w", err)

Expand Down Expand Up @@ -914,3 +919,30 @@ 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)
}
}
}
53 changes: 39 additions & 14 deletions staticaddr/loopin/manager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -850,11 +850,14 @@ 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 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 (prioritize time-sensitive deposits).
Copy link
Copy Markdown
Collaborator

@starius starius Apr 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SelectDeposits can now turn a valid amount-only swap into a more expensive or failing one because it switched to a confirmations-first greedy prefix instead of actually optimizing the chosen subset. The new logic sorts primarily by confirmations and then keeps taking the first deposits until the target is covered in staticaddr/loopin/manager.go:853-930.

That means a set like 500k@10 conf, 500k@9 conf, 1,000k@8 conf for a 1,000k target now selects two inputs instead of the exact one-input match. This is not just a UX issue: the chosen input count feeds the quote path in staticaddr/loopin/manager.go:729-742, and the later HTLC fee guards scale with f.loopIn.htlcWeight(hasChange) in staticaddr/loopin/actions.go:221-270. So the new order can produce avoidable ErrSwapFeeTooHigh or ErrFeeTooHigh failures, or simply worse fees, even when a slightly younger single deposit would already have been acceptable to the server.

If the client wants to prefer older deposits, they need a subset selection strategy that still minimizes input count and fee impact, or at least a second pass that prefers exact or lower-input covers before falling back to the confirmations-first prefix.

// Deposits that are too close to expiry are filtered out before sorting.
// 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) {
Expand All @@ -875,18 +878,40 @@ 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 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 (prioritize time-sensitive deposits).
sort.Slice(deposits, func(i, j int) bool {
if deposits[i].Value == deposits[j].Value {
iExp := uint32(deposits[i].ConfirmationHeight) +
csvExpiry - blockHeight
jExp := uint32(deposits[j].ConfirmationHeight) +
csvExpiry - blockHeight
// 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
}

return iExp < jExp
// Secondary: larger amounts first.
if deposits[i].Value != deposits[j].Value {
return deposits[i].Value > deposits[j].Value
}
return deposits[i].Value > deposits[j].Value

// Tertiary: expiring sooner first.
iExp := uint32(deposits[i].ConfirmationHeight) +
csvExpiry - blockHeight
jExp := uint32(deposits[j].ConfirmationHeight) +
csvExpiry - blockHeight

return iExp < jExp
})
Comment thread
hieblmi marked this conversation as resolved.

// Select the deposits that are needed to cover the swap amount without
Expand Down
55 changes: 46 additions & 9 deletions staticaddr/loopin/manager_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,14 @@ 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,
ConfirmationHeight: 5_000, // most confs at height 5100
}, &deposit.Deposit{
Value: 2_000_000,
ConfirmationHeight: 5_001,
Expand All @@ -38,7 +42,7 @@ func TestSelectDeposits(t *testing.T) {
ConfirmationHeight: 5_002,
}, &deposit.Deposit{
Value: 3_000_000,
ConfirmationHeight: 5_003,
ConfirmationHeight: 5_003, // fewest confs at height 5100
}
d1.Hash = chainhash.Hash{1}
d1.Index = 0
Expand All @@ -49,75 +53,108 @@ 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: "",
},
{
name: "prefer larger deposit when both cover",
// d1 has more confirmations, so it's preferred even
// though d2 is larger.
name: "prefer more confirmed deposit over larger",
deposits: []*deposit.Deposit{d1, d2},
targetValue: 1_000_000,
expected: []*deposit.Deposit{d2},
csvExpiry: testCsvExpiry,
blockHeight: testBlockHeight,
expected: []*deposit.Deposit{d1},
expectedErr: "",
},
{
name: "prefer largest among three when one is enough",
// d1 has the most confirmations among d1, d2, d3.
name: "prefer most confirmed among three",
deposits: []*deposit.Deposit{d1, d2, d3},
targetValue: 1_000_000,
expected: []*deposit.Deposit{d3},
csvExpiry: testCsvExpiry,
blockHeight: testBlockHeight,
expected: []*deposit.Deposit{d1},
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",
},
{
name: "tie by value, prefer earlier expiry",
// 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",
deposits: []*deposit.Deposit{d3, d4},
targetValue: d4.Value - dustLimit, // d3/d4 have the
// same value but different expiration.
targetValue: d4.Value - dustLimit,
csvExpiry: testCsvExpiry,
blockHeight: testBlockHeight,
expected: []*deposit.Deposit{d3},
expectedErr: "",
},
Expand Down
Loading
Loading