staticaddr: psbt withdrawals #1043

hieblmi · 2025-11-12T13:56:50Z

This PR a new rpc for withdrawals via psbts,

rpc ServerPsbtWithdrawDeposits (ServerPsbtWithdrawRequest)
        returns (ServerPsbtWithdrawResponse);

gemini-code-assist · 2025-11-12T13:57:30Z

Summary of Changes

Hello @hieblmi, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request enhances the static address withdrawal mechanism by integrating Partially Signed Bitcoin Transactions (PSBTs). It introduces a new RPC for PSBT-based withdrawals and refactors several core utility functions into a dedicated package, streamlining the process of cooperative transaction signing and improving modularity within the codebase.

Highlights

New RPC for PSBT Withdrawals: A new RPC endpoint, ServerPsbtWithdrawDeposits, has been introduced to facilitate withdrawals using Partially Signed Bitcoin Transactions (PSBTs). This allows the client to construct the withdrawal transaction and the server to provide its signature.
Code Refactoring to staticutil Package: Common utility functions related to Musig2 session creation, toPrevOuts conversion, and withdrawal transaction value calculations have been refactored into a new staticaddr/staticutil package. This improves code organization and reusability across the static address components.
PSBT Integration in Withdrawal Process: The withdrawal transaction creation and signing flow has been updated to leverage PSBTs. The client now sends an unsigned PSBT to the server, which then signs it and returns the necessary signing information, allowing for a more flexible and standardized multi-party signing process.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request introduces a new RPC endpoint for PSBT-based withdrawals, ServerPsbtWithdrawDeposits, which enhances the flexibility and security of the withdrawal process by allowing the client to construct the transaction and the server to provide only the necessary signatures. The changes involve significant refactoring, moving common utility functions to a new staticutil package, and updating the withdrawal manager to use the new PSBT flow. The introduction of staticutil improves code organization and reusability. Overall, the changes are well-structured and align with best practices for handling Bitcoin transactions.

gemini-code-assist · 2025-11-12T14:00:53Z

staticaddr/withdraw/manager.go

+	// Do some sanity checks.
+	txHash := withdrawalTx.TxHash()
+	if !bytes.Equal(txHash.CloneBytes(), sigResp.Txid) {
+		return nil, nil, errors.New("txid doesn't match")
 	}

-	// Next we'll get our sweep tx signatures.
-	prevOutFetcher := txscript.NewMultiPrevOutFetcher(prevOuts)
-	_, err = m.signMusig2Tx(
-		ctx, prevOutFetcher, outpoints, m.cfg.Signer, withdrawalTx,
-		withdrawalSessions, coopServerNonces,
-	)
-	if err != nil {
-		return nil, err
+	if len(sigResp.SigningInfo) != len(deposits) {
+		return nil, nil, errors.New("invalid number of " +
+			"deposit signatures")


Adding sanity checks for the returned txid and the length of SigningInfo from the server is a critical security and correctness measure. It ensures that the server's response matches the client's expectations and prevents potential manipulation or errors.

What is your point? The checks are in place. Do you suggest additional checks?

I propose to check not only the length, but also that they have the same elements (1:1 matching). That each deposit got one record in sigResp.SigningInfo.

Please also add a test checking this situation.

hieblmi · 2025-11-20T16:26:56Z

swapserverrpc/staticaddr.proto


+message ServerPsbtWithdrawRequest {
+    // The withdrawal psbt.
+    bytes withdrawal_psbt = 1;


nit: comment that txscript.SigHashDefault will be enforced by default.

…eposits

lightninglabs-deploy · 2025-11-27T17:07:41Z

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

starius

LGTM! 🚀
Added several comments.

starius · 2025-11-28T17:00:43Z

staticaddr/withdraw/manager.go

+	// Do some sanity checks.
+	txHash := withdrawalTx.TxHash()
+	if !bytes.Equal(txHash.CloneBytes(), sigResp.Txid) {
+		return nil, nil, errors.New("txid doesn't match")
 	}

-	// Next we'll get our sweep tx signatures.
-	prevOutFetcher := txscript.NewMultiPrevOutFetcher(prevOuts)
-	_, err = m.signMusig2Tx(
-		ctx, prevOutFetcher, outpoints, m.cfg.Signer, withdrawalTx,
-		withdrawalSessions, coopServerNonces,
-	)
-	if err != nil {
-		return nil, err
+	if len(sigResp.SigningInfo) != len(deposits) {
+		return nil, nil, errors.New("invalid number of " +
+			"deposit signatures")


I propose to check not only the length, but also that they have the same elements (1:1 matching). That each deposit got one record in sigResp.SigningInfo.

Please also add a test checking this situation.

starius · 2025-11-28T17:04:26Z

staticaddr/staticutil/outpoints.go

+
+// ToWireOutpoints converts lnrpc.OutPoint protos into wire.OutPoint structs so
+// they can be consumed by lower level transaction building code.
+func ToWireOutpoints(outpoints []*lnrpc.OutPoint) ([]wire.OutPoint, error) {


This function is not used and not tested.

starius · 2025-11-28T17:07:27Z

staticaddr/staticutil/utils.go

+// 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(deposits []*deposit.Deposit, amount int64) (


The function is not used and not tested.

starius · 2025-11-28T17:12:25Z

staticaddr/withdraw/manager.go

+	addrParams, err := m.cfg.AddressManager.GetStaticAddressParameters(ctx)
+	if err != nil {
+		return nil, nil, err
+	}
+
+	staticAddress, err := m.cfg.AddressManager.GetStaticAddress(ctx)
+	if err != nil {
+		return nil, nil, err
+	}


This makes two DB queries. It could be one: staticAddress can be built from addrParams. We can factor out a function from address.Manager.GetStaticAddress which converts params to address. Or just merge two methods GetStaticAddressParameters and GetStaticAddress together and return both things at once.

I totally agree. These methods will be removed in the follow-up PR for multi addresses so i think we can leave them for now.

starius · 2025-11-28T17:17:29Z

staticaddr/staticutil/utils.go

+
+// CreateMusig2Session creates a musig2 session for the deposit.
+func CreateMusig2Session(ctx context.Context,
+	signer lndclient.SignerClient, addrParams *address.Parameters,


Idea: can we move address.Parameters to script package and reuse it and this function.

starius · 2025-11-28T17:22:40Z

staticaddr/withdraw/manager.go

 // transaction.
 func (m *Manager) signMusig2Tx(ctx context.Context,
-	prevOutFetcher *txscript.MultiPrevOutFetcher, outpoints []wire.OutPoint,
+	prevOutFetcher *txscript.MultiPrevOutFetcher,


Add tests for signMusig2Tx.

When sigInfo is missing an entry for one of the deposits, the call should error (no partial return). Build a dummy tx with two inputs, provide sessions for both, but only one SigningInfo entry; expect error.

When sigInfo has all expected keys but one maps to the wrong index (mismatched depositsToIdx), ensure it errors.

starius · 2025-11-28T17:24:55Z

staticaddr/withdraw/manager.go


-	case *btcutil.AddressTaproot:
-		weightEstimator.AddP2TROutput()
+func CalculateWithdrawalTxValaues(deposits []*deposit.Deposit,


typo: "Valaues"

starius · 2025-11-28T17:26:40Z

staticaddr/withdraw/manager.go

+func CalculateWithdrawalTxValaues(deposits []*deposit.Deposit,
+	localAmount btcutil.Amount, feeRate chainfee.SatPerKWeight,
+	withdrawalAddress btcutil.Address,
+	commitmentType lnrpc.CommitmentType) (btcutil.Amount, btcutil.Amount,


Add a table test for CalculateWithdrawalTxValues covering: insufficient funds after dust/fee, negative change, and min-channel-size guard.

hieblmi marked this pull request as draft November 12, 2025 13:56

gemini-code-assist bot reviewed Nov 12, 2025

View reviewed changes

hieblmi force-pushed the withdraw-psbt branch 4 times, most recently from 94c5f92 to e387361 Compare November 17, 2025 07:01

hieblmi self-assigned this Nov 17, 2025

hieblmi added the static-loop-in label Nov 17, 2025

hieblmi marked this pull request as ready for review November 17, 2025 10:03

hieblmi requested review from bhandras, sputn1ck and starius November 17, 2025 13:11

hieblmi force-pushed the withdraw-psbt branch 7 times, most recently from edc5090 to 97d8733 Compare November 19, 2025 07:28

hieblmi commented Nov 20, 2025

View reviewed changes

hieblmi force-pushed the withdraw-psbt branch from 97d8733 to d9531ac Compare November 25, 2025 14:35

hieblmi added 4 commits November 27, 2025 11:07

swapserverrpc: psbt-base ServerWithdraw endpoint

29836e7

swapserverrpc: deprecate ServerWithdrawDeposits

2104a38

staticutil: refactor methods into utils

8fa488d

staticaddr: replace ServerWithdrawDeposits rpc /w ServerPsbtWithdrawD…

bacabe1

…eposits

hieblmi force-pushed the withdraw-psbt branch from d9531ac to bacabe1 Compare November 27, 2025 10:12

starius approved these changes Nov 28, 2025

View reviewed changes

staticaddr: psbt withdrawals #1043

Are you sure you want to change the base?

staticaddr: psbt withdrawals #1043

Conversation

hieblmi commented Nov 12, 2025

Uh oh!

gemini-code-assist bot commented Nov 12, 2025

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Nov 12, 2025

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!

hieblmi Nov 20, 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!

lightninglabs-deploy commented Nov 27, 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!

starius Nov 28, 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!

starius Nov 28, 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

hieblmi Nov 20, 2025 •

edited

Loading

starius Nov 28, 2025 •

edited

Loading

starius Nov 28, 2025 •

edited

Loading