Skip to content

[feature]: add specific change address for send coin RPCs #10271#10810

Open
murraystewart96 wants to merge 18 commits into
lightningnetwork:masterfrom
murraystewart96:LND-10271
Open

[feature]: add specific change address for send coin RPCs #10271#10810
murraystewart96 wants to merge 18 commits into
lightningnetwork:masterfrom
murraystewart96:LND-10271

Conversation

@murraystewart96
Copy link
Copy Markdown

@murraystewart96 murraystewart96 commented May 17, 2026

Change Description

#10271

Adds a change_address field to SendCoins, SendMany, and WalletKit.SendOutputs, allowing callers to pin the change output to a specific address rather than a wallet-derived one.

The field is mutually exclusive with send_all on SendCoins and the --sweepall flag on sendcoins.

Depends on btcwallet changes to support a custom change address at the transaction creation layer:

Issue: btcsuite/btcwallet#1234
PR: btcsuite/btcwallet#1235

Steps to Test

go test ./lnwallet/test/btcd/ -v -run "TestLightningWallet/btcwallet/btcd:create_simple_tx_change_addr"

NOTE: This will fail until the btcwallet PR has been merged and the new version has been brought into this repo

Pull Request Checklist

Testing

  • Your PR passes all CI checks.
  • Tests covering the positive and negative (error paths) are included.
  • Bug fixes contain tests triggering the bug to prevent regressions.

Code Style and Documentation

📝 Please see our Contribution Guidelines for further guidance.

@gemini-code-assist
Copy link
Copy Markdown

Summary of Changes

Hello, I'm Gemini Code Assist1! 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 introduces the ability to pin the change output of on-chain transactions to a specific address. By extending the RPC and internal wallet interfaces, callers can now explicitly define where change UTXOs should be sent. This feature includes robust validation to prevent invalid or dangerous address usage and ensures compatibility with existing wallet logic, such as the reserve requirement mechanism.

Highlights

  • Custom Change Address Support: Added a 'change_address' field to SendCoins, SendMany, and WalletKit.SendOutputs, allowing users to specify a destination for change UTXOs instead of using a wallet-derived address.
  • Validation and Safety: Implemented strict validation for the custom change address, including network compatibility checks and prevention of sending to raw public keys. Added mutual exclusivity checks to ensure 'change_address' cannot be used alongside 'sweepall' or 'send_all'.
  • Interface Updates: Updated the internal WalletController interface and its implementations (BtcWallet, RPCKeyRing, and mocks) to propagate the optional change address through the transaction creation pipeline.
  • Testing: Added a new integration test 'testCreateSimpleTxChangeAddr' to verify that transactions correctly route change to the user-provided address.
New Features

🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

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 the 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 counterproductive. 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.

Footnotes

  1. 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.

@github-actions github-actions Bot added the severity-critical Requires expert review - security/consensus critical label May 17, 2026
@github-actions
Copy link
Copy Markdown

🔴 PR Severity: CRITICAL

Automated classification | 11 files (non-generated, non-test) | ~275 lines changed

🔴 Critical (5 files)
  • lnwallet/btcwallet/btcwallet.go - wallet operations implementation in lnwallet/*
  • lnwallet/interface.go - wallet interface definition in lnwallet/*
  • lnwallet/mock.go - wallet mock in lnwallet/* package
  • lnwallet/rpcwallet/rpcwallet.go - remote signer wallet implementation in lnwallet/*
  • rpcserver.go - core server coordination
🟠 High (2 files)
  • lnrpc/walletrpc/walletkit_server.go - wallet RPC server implementation
  • lnrpc/lightning.swagger.json - API surface changes
🟡 Medium (3 files)
  • cmd/commands/commands.go - CLI client commands
  • lnrpc/lightning.proto - Lightning RPC API definition
  • lnrpc/walletrpc/walletkit.proto - Wallet RPC API definition
Excluded from counting (3 files)
  • lnrpc/lightning.pb.go - auto-generated (*.pb.go)
  • lnrpc/walletrpc/walletkit.pb.go - auto-generated (*.pb.go)
  • lntest/mock/walletcontroller.go - test file (lntest/*)

Analysis

This PR is classified as CRITICAL because it touches multiple files in lnwallet/* (wallet operations, channel funding, signing, commitment transactions) and rpcserver.go (core server coordination) — both of which are in the highest severity tier.

The changes appear to modify wallet interface signatures and their implementations across lnwallet/interface.go, lnwallet/btcwallet/btcwallet.go, and lnwallet/rpcwallet/rpcwallet.go, with corresponding updates propagated to the RPC layer (lnrpc/) and wired into the main rpcserver.go. Changes to wallet interface contracts and the core RPC server require expert review to ensure correctness of fund management, signing logic, and API compatibility.

No severity bump thresholds were triggered (11 non-generated/non-test files, ~275 lines changed — both below the bump thresholds of 20 files / 500 lines).


To override, add a severity-override-{critical,high,medium,low} label.
<!-- pr-severity-bot -->

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces the ability to specify a custom change address for on-chain transactions via the SendCoins and SendOutputs RPCs. The changes include updates to the protobuf definitions, CLI commands, and internal wallet logic to propagate and respect the optional change_address field. Feedback from the review highlights several areas for improvement: the change_address field is missing from the SendMany message despite being mentioned in the PR description, and there is duplicated address validation logic in walletkit_server.go that should be consolidated into a shared utility. Additionally, several style guide violations were noted, including the use of block comments instead of double-slashes in protobuf files, improper wrapping of function return values in lnwallet/interface.go, and a typo in a log message comment.

I am having trouble creating individual review comments. Click here to see my feedback.

lnrpc/lightning.proto (1337)

high

The pull request description mentions that change_address is added to SendMany, but the field is missing from the SendManyRequest message in this file. Please ensure the implementation is complete and consistent with the description.

lnrpc/lightning.proto (1333-1336)

medium

The project typically uses double-slash // for field comments to maintain consistency with the rest of the file. Additionally, per the style guide, comments should ideally follow the FieldName is ... pattern and explain the purpose or context.

    // An optional address to send change to. If not set, a wallet-derived
    // address is used.
References
  1. Always use the Golang code style described below in this document. readable code is the most important requirement. (link)

lnrpc/walletrpc/walletkit_server.go (828-838)

security-medium medium

This address validation logic is duplicated from the decodeAndValidateAddr helper introduced in rpcserver.go. Furthermore, it omits the check to prevent sending to raw public keys, which is present in the SendCoins implementation. It is recommended to move the validation logic to a shared utility package (e.g., lnwallet or a dedicated address package) to ensure consistency and security across all RPC entry points.

lnwallet/interface.go (376-377)

medium

The wrapping of the return values here violates the style guide. When a function declaration spans multiple lines, the closing parenthesis should not be on its own line if the return values follow, and the return values should ideally be on the same line as the closing parenthesis if they fit within the 80-character limit.

	) (*txauthor.AuthoredTx, error)
References
  1. Do not end a line with an open parenthesis if the function definition is not finished. If function arguments exceed the 80-character limit, maintain indentation on following lines. (link)

rpcserver.go (1452)

medium

Typo: 'interal' should be 'internal'.

		// Send all requires default internal wallet address to preserve
References
  1. Readable code is the most important requirement for any commit created. (link)

@murraystewart96 murraystewart96 changed the title [feature]: specific change address for SendCoinsRequest #10271 [feature]: add specific change address for send coin RPCs #10271 May 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

severity-critical Requires expert review - security/consensus critical

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant