staticaddr: fractional swap amount #887

hieblmi · 2025-02-20T14:47:14Z

the first commits are from the previous PR

This PR adds an amount flag to static loop-in swaps that allows the caller to only swap a fraction of the value of the selected deposits. If only amount is selected, the client selects deposits automatically to cover for the swaps amount.

To ensure backwards UX compatibility, omitting the new flag (--amount=0) selects the total deposit amount for swapping.

This PR also adds a method SelectDeposits to the loop-in manager that is used to coin-select deposits in case the user
only provides a static swap amount.

TODO

Sign sweepless sweep and htlc tx with change output if needed
integration test

hieblmi · 2025-04-30T10:51:28Z

Rebased

hieblmi · 2025-09-05T09:37:32Z

/gemini review

gemini-code-assist

Code Review

This pull request introduces a significant feature: fractional loop-in swaps for static addresses, including automatic coin selection. The changes are extensive, touching the CLI, RPC layer, database, and core swap logic. The implementation appears robust, with new logic for coin selection, handling of change outputs in HTLCs, and a database migration for backward compatibility. The accompanying tests are comprehensive. I've identified a couple of minor issues for your consideration.

loopd/swapclient_server.go

hieblmi · 2025-09-10T07:10:47Z

Address gemini comments and rebased.

starius · 2025-09-10T19:07:57Z

first two commits are from the previous PR

Which PR includes commit "sqlc: add amount column to static swaps"?

hieblmi · 2025-09-10T20:00:13Z

It is included in this PR. I changed the PR description.

starius

LGTM! Great work! 💾
I left several comments.

starius · 2025-09-10T19:25:08Z

looprpc/client.proto

+    will be selected from the static address deposits. This field cannot be used
+    in conjunction with deposit_outpoints.
+    */
+    bool select_deposits = 10;


This logic is hard to understand.
IIUC, we have 3 cases how this RPC is used with loop-ins:

legacy loop-in (deposit_outpoints=[], select_deposits=false)

static loop-in, deposits autoselected (deposit_outpoints=[], select_deposits=true)

static loop-in, deposits selected manually (deposit_outpoints is provided, select_deposits=false)

Also in both static loop-in cases depending on amt value it can result in partial static loop-in (have change).

Can we provide oneof with these 3 cases of loop-in to make it more explicit:

oneof loopin { LegacyIn legacy = 10; StaticInAutoselect static_autoselect = 11; StaticInManualSelection static_manual_selection = 12; }

~~Great suggestion! I've added an enum FundingOption that covers these cases. It is evaluated in GetLoopInQuote.~~

I've renamed select_deposits to auto_select_deposits and added clarifying comments on the usage of it. If we introduce a enum type instead of the bool we will have to evaluate enums against paramters set in deposit_outpoints and amt, which I find confusing. So I'd rely on the clarifying comments, unless there is a strong opinion to change it.

starius · 2025-09-10T19:35:19Z

staticaddr/loopin/loopin.go


 	msgTx.AddTxOut(sweepOutput)

+	// We expect change to be sent back to our static address output script.


We expect change to be sent back to our static address output script.

I guess in the multi address world this will be fresh static address, right?

I propose to prepare a bit: add a field holding change pkscript (and for now just assign it to l.AddressParams.PkScript). We should use it everywhere and pass instead of hasChange to htlcWeight. Then we don't have to assume there that is is P2TR and we can use method weightEstimator.AddOutput.

I guess in the multi address world this will be fresh static address, right?

It will be, yes. I am working on this on an offline PR.

I propose to prepare a bit: add a field holding change pkscript (and for now just assign it to l.AddressParams.PkScript). We should use it everywhere and pass instead of hasChange to htlcWeight. Then we don't have to assume there that is is P2TR and we can use method weightEstimator.AddOutput.

I will leave this for the future PR, just so that we don't overload this already full PR.

starius · 2025-09-10T19:40:32Z

staticaddr/loopin/manager.go

+			if out.Value == int64(changeAmt) &&
+				bytes.Equal(out.PkScript, changePkScript) {
+
+				foundChange = true


What is two swaps have equal change outputs? IIRC, we need to check all swaps taking part in the transaction to make sure that each of them has a separate change output.

I will address this in a follow-up PR.

I've added method checkChange that ensures that for all inputs that associate a swap there is the expected change output per swap. Since there is only one static address at the moment we can sum up all change of associated swaps and check if there is a consolidated change output for us.

staticaddr/loopin/manager.go

staticaddr/loopin/manager_test.go

loopd/swapclient_server.go

staticaddr/loopin/manager.go

cmd/loop/staticaddr.go

bhandras

Awesome work! LGTM 🎉

bhandras · 2025-09-11T15:19:39Z

staticaddr/loopin/selected_amount_migration.go

+func MigrateSelectedSwapAmount(ctx context.Context, db loopdb.SwapStore,
+	depositStore *deposit.SqlStore, swapStore *SqlStore) error {
+
+	migrationDone, err := db.HasMigration(ctx, selectedAmountMigrationID)


So it's possible that we crash before we call db.SetMigration(), but now double checked and iiuc it's harmess as we'd just set the selected_amount again to sum of used deposits, so should be good 👍

lightninglabs-deploy · 2025-09-18T18:29:21Z

@hieblmi, remember to re-request review from reviewers when ready

starius

LGTM! 💾
I checked two last commits.
I propose to adjust the code so it works both now and in multi-address world.

starius · 2025-09-22T21:22:40Z

sweepbatcher/sweep_batcher.go

+	if len(spendTx.TxOut) < 1 {
+		return fmt.Errorf("expected at least one output in batch")


starius · 2025-09-22T21:25:55Z

staticaddr/loopin/manager.go

+		if out.Value == int64(expectedChange) &&
+			bytes.Equal(out.PkScript, changeAddr.PkScript) {


This will be updated in multi-address world, right? There might be multiple change outputs.

We can write the code already with multi-address in mind. We just need to collect change addresses and change amounts from all the loopIn variables in the previous for loop and verify that each of them is present. In single address world this would work identically to the current implementation.

Yes I had that in mind and agree, however, this requires some refactoring around loading the address parameters per swap which is introduced in the multi-address PRs. So I'd like to defer that.

The QuoteRequest adds a field auto_select_deposits to signal that the specified quote amount should be coin-selected from the client's deposits to determine the number of deposits to quote for. The StaticAddressLoopInRequest gets a new amount field that either indicates that the swap amount should be allocated from the passed deposits or coin-selected from all available deposits.

In this commit we add a new function SelectDeposits to the loop-in manager. It coin-selects deposits that meet an arbitrary swap amount provided by the client. We have to ensure that the server creates the correct change outputs for the htlc- and sweepless sweep transactions.

If a quote request contains an amount and flag SelectDeposits set to true the quoting coin- selects the required deposits to meet the swap amount in order to quote for the number of deposits.

The selected_amount column of all previous swaps is filled with the total value of deposits that partook in these swaps.

hieblmi added the static-loop-in label Feb 20, 2025

hieblmi self-assigned this Feb 20, 2025

hieblmi marked this pull request as draft February 20, 2025 14:47

hieblmi force-pushed the static-arb-amount-swap branch 3 times, most recently from fd6cc75 to f54115f Compare February 21, 2025 15:23

hieblmi force-pushed the static-arb-amount-swap branch 6 times, most recently from 67612da to b3522b1 Compare March 19, 2025 11:27

hieblmi force-pushed the static-arb-amount-swap branch 6 times, most recently from 8b33f65 to 21fb9ac Compare April 2, 2025 13:48

hieblmi requested a review from Copilot April 2, 2025 13:52

hieblmi marked this pull request as ready for review April 2, 2025 13:52

This comment was marked as outdated.

Sign in to view

hieblmi force-pushed the static-arb-amount-swap branch 3 times, most recently from 4b1058d to 5625af3 Compare April 3, 2025 15:02

hieblmi requested review from bhandras, sputn1ck and starius April 4, 2025 12:09

hieblmi force-pushed the static-arb-amount-swap branch from 5625af3 to 569359f Compare April 4, 2025 12:09

hieblmi force-pushed the static-arb-amount-swap branch from 569359f to 8be2c65 Compare April 30, 2025 10:51

hieblmi force-pushed the static-arb-amount-swap branch 3 times, most recently from 9deb39b to bc1f2f5 Compare August 28, 2025 17:10

gemini-code-assist bot reviewed Sep 5, 2025

View reviewed changes

loopd/swapclient_server.go Outdated Show resolved Hide resolved

loopd/swapclient_server.go Outdated Show resolved Hide resolved

hieblmi force-pushed the static-arb-amount-swap branch 2 times, most recently from b60695f to 967c121 Compare September 10, 2025 07:10

starius approved these changes Sep 10, 2025

View reviewed changes

hieblmi force-pushed the static-arb-amount-swap branch from 967c121 to 61a7bc6 Compare September 11, 2025 11:56

bhandras approved these changes Sep 11, 2025

View reviewed changes

hieblmi force-pushed the static-arb-amount-swap branch from 61a7bc6 to ba239a9 Compare September 15, 2025 12:12

hieblmi force-pushed the static-arb-amount-swap branch 2 times, most recently from ca7c14a to b9852cb Compare September 22, 2025 08:37

starius approved these changes Sep 22, 2025

View reviewed changes

hieblmi added 9 commits September 25, 2025 11:13

sqlc: add amount column to static swaps

22ac716

loopd: deposit selection for GetLoopInQuote

95a4567

If a quote request contains an amount and flag SelectDeposits set to true the quoting coin- selects the required deposits to meet the swap amount in order to quote for the number of deposits.

staticaddr: fix swap amount in listswaps command

23f6c24

staticaddr: selected swap amount migration

2ee772b

The selected_amount column of all previous swaps is filled with the total value of deposits that partook in these swaps.

loopd: fractional static address swap amount

a012430

sweepbatcher: relax output constrained in monitorConfAndNotify

221372b

staticaddr: checkChange method for sweep signing

a877cfd

hieblmi force-pushed the static-arb-amount-swap branch from b9852cb to a877cfd Compare September 25, 2025 09:14

hieblmi merged commit 7c41ec5 into lightninglabs:master Sep 25, 2025
4 checks passed

hieblmi deleted the static-arb-amount-swap branch September 25, 2025 09:32


		msgTx.AddTxOut(sweepOutput)

		// We expect change to be sent back to our static address output script.

		if len(spendTx.TxOut) < 1 {
		return fmt.Errorf("expected at least one output in batch")

		if out.Value == int64(expectedChange) &&
		bytes.Equal(out.PkScript, changeAddr.PkScript) {

staticaddr: fractional swap amount #887

staticaddr: fractional swap amount #887

Uh oh!

Conversation

hieblmi commented Feb 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

hieblmi commented Apr 30, 2025

Uh oh!

hieblmi commented Sep 5, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

hieblmi commented Sep 10, 2025

Uh oh!

starius commented Sep 10, 2025

Uh oh!

hieblmi commented Sep 10, 2025

Uh oh!

starius left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hieblmi Sep 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bhandras left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lightninglabs-deploy commented Sep 18, 2025

Uh oh!

starius left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

hieblmi commented Feb 20, 2025 •

edited

Loading

hieblmi Sep 11, 2025 •

edited

Loading