rpc: add change_address parameter to SendCoins

[anumukul]

Summary

Adds change_address parameter to SendCoins RPC and sendcoins CLI command, allowing users to drain addresses by controlling where change is sent.

Closes #10271

Changes

• Added change_address field to SendCoinsRequest (lightning.proto) and SendOutputsRequest (walletkit.proto)
• Implemented custom change address logic in btcwallet
• Updated WalletController.SendOutputs interface to accept change address parameter
• Added --change_address flag to sendcoins CLI command
• Updated all implementations and call sites

Usage

lncli sendcoins --utxo txid:vout --amount XXX --addr target_address \
  --change_address address_of_txid:vout

[gemini-code-assist[bot]]

Summary of Changes

Hello @anumukul, I'm Gemini Code Assist[^1]! 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 a significant enhancement to the SendCoins functionality by allowing users to explicitly define a change address for their transactions. This feature provides greater control over UTXO management, particularly useful for scenarios requiring the systematic draining of funds from specific addresses. The changes involve updates to the RPC definitions, core wallet logic, and the command-line interface, along with necessary dependency upgrades.

Highlights

• New RPC Parameter: Added a change_address parameter to the SendCoins RPC and the sendcoins CLI command, allowing users to specify where change outputs should be sent.
• Address Draining Capability: This new parameter enables users to drain funds from specific addresses by repeatedly sending transactions and directing the change back to the same address.
• Protocol Buffer Updates: The change_address field was added to SendCoinsRequest in lightning.proto and SendOutputsRequest in walletkit.proto.
• Wallet Logic Implementation: Custom logic for handling the specified change address was implemented within btcwallet, including a new sendOutputsWithCustomChange function.
• Interface and CLI Updates: The WalletController.SendOutputs interface was updated to accept the new change address parameter, and the sendcoins CLI command now includes a --change_address flag.
• Dependency Updates: Several Go module dependencies were updated, including golang.org/x/crypto, google.golang.org/grpc, and google.golang.org/protobuf, leading to regenerated gRPC stubs.

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 :thumbsup: and :thumbsdown: 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.

[^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.

[hieblmi]

Thanks for your PR @anumukul!

Could you please place your changes into different commits to ease review. The commit structure usually follows the pattern <package>: <summary of what the changes to the package achieve>

Please also check the review that gemini code assist gave.

[saubyk]

@anumukul please go through the code contribution guidelines: https://github.com/lightningnetwork/lnd/blob/master/docs/code_contribution_guidelines.md

[anumukul]

Thanks for the detailed reviews! I've addressed all the feedback:

Changes Made:

Fixed critical bug in sendOutputsWithCustomChange:

• Removed the problematic double CreateSimpleTx call
• New implementation: creates tx once to determine change amount, then uses btcwallet's SendOutputs/SendOutputsWithInput with final output set including custom change
• This avoids re-running coin selection and ensures correct fee calculation

Added function comment:

• Added proper documentation for sendCoinsOnChain function per LND style guide

Split into separate commits:

• Reorganized into 6 logical commits following the <package>: <summary> pattern
• Each commit is focused and self-contained

Fixed build error:

• Added missing nil argument to SendOutputs call in walletkit_server.go

Ran make fmt:

• All code formatted according to project standards

Make rpc:

• Attempted but requires Docker which I don't have locally
• Proto files are manually updated and compile successfully

Tests: I'd like to add tests but would appreciate guidance on:

• Should I add unit tests for sendOutputsWithCustomChange in btcwallet?
• Integration tests for the full SendCoins flow?
• What's the preferred testing approach for wallet functionality in LND?

Backward compatibility: The interface change adds a new optional parameter at the end. All existing callers pass nil to maintain current behavior. Is this approach sufficient?

Ready to iterate based on your feedback!

[NishantBansal2003]

• Attempted but requires Docker which I don't have locally

I think it would be better if you compile using make rpc

• Should I add unit tests for sendOutputsWithCustomChange in btcwallet?
• Integration tests for the full SendCoins flow?
• What's the preferred testing approach for wallet functionality in LND?

Preferably both. I haven’t fully gone through the concept of the PR yet, but you can check the existing *_test.go files for unit tests and the itest/ directory for integration tests to see which approach fits best in your case.

Backward compatibility: The interface change adds a new optional parameter at the end. All existing callers pass nil to maintain current behavior. Is this approach sufficient?

I don’t think this is how backward compatibility works. It might help to read more about it to understand the concept better.

Also, I recommend going through the https://github.com/lightningnetwork/lnd/blob/master/docs/development_guidelines.md as there are still a few conventions that need to be followed, for eg.- the 80 character line length.

[anumukul]

• Attempted but requires Docker which I don't have locally

I think it would be better if you compile using make rpc

• Should I add unit tests for sendOutputsWithCustomChange in btcwallet?
• Integration tests for the full SendCoins flow?
• What's the preferred testing approach for wallet functionality in LND?

Preferably both. I haven’t fully gone through the concept of the PR yet, but you can check the existing *_test.go files for unit tests and the itest/ directory for integration tests to see which approach fits best in your case.

Backward compatibility: The interface change adds a new optional parameter at the end. All existing callers pass nil to maintain current behavior. Is this approach sufficient?

I don’t think this is how backward compatibility works. It might help to read more about it to understand the concept better.

Also, I recommend going through the https://github.com/lightningnetwork/lnd/blob/master/docs/development_guidelines.md as there are still a few conventions that need to be followed, for eg.- the 80 character line length.

Thanks for the detailed feedback! I've addressed all the issues:

Fixed Backward Compatibility

The original approach broke backward compatibility by changing the interface signature. I've now:

• Kept the original SendOutputs method unchanged for backward compatibility
• Added a new SendOutputsWithChangeAddr method that accepts the optional change address parameter
• Updated all implementations (BtcWallet, RPCKeyRing, mocks) to provide both methods
• The old SendOutputs now delegates to the new method with nil for change address

This ensures any external code using the WalletController interface continues to work without changes.

Ran make rpc with Docker

Successfully ran make rpc using Docker to regenerate all protobuf files. All generated files are now up to date.

Code Formatting

Ran make fmt - all code follows LND style guidelines. No line length violations in hand-written code (only in auto-generated proto files, which is expected).

Organized into Logical Commits

Split changes into 13 focused commits following the <package>: <summary> pattern:

1. Interface changes for backward compatibility
2. Implementation in btcwallet
3. Mock updates
4. RPC wallet updates
5. RPC server changes
6. Walletkit proto updates
7. Proto regeneration

Still TODO: Tests

I understand tests are required. I'd appreciate guidance on:

• Unit tests: Should I test sendOutputsWithCustomChange in lnwallet/btcwallet/btcwallet_test.go?
• Integration tests: Should I add a test in itest/ for the full SendCoins flow with custom change?
• What's the preferred mocking/setup approach for wallet tests in LND?

[mohamedawnallah]

• Attempted but requires Docker which I don't have locally

I think it would be better if you compile using make rpc

• Should I add unit tests for sendOutputsWithCustomChange in btcwallet?
• Integration tests for the full SendCoins flow?
• What's the preferred testing approach for wallet functionality in LND?

Preferably both. I haven’t fully gone through the concept of the PR yet, but you can check the existing *_test.go files for unit tests and the itest/ directory for integration tests to see which approach fits best in your case.

Backward compatibility: The interface change adds a new optional parameter at the end. All existing callers pass nil to maintain current behavior. Is this approach sufficient?

I don’t think this is how backward compatibility works. It might help to read more about it to understand the concept better. Also, I recommend going through the https://github.com/lightningnetwork/lnd/blob/master/docs/development_guidelines.md as there are still a few conventions that need to be followed, for eg.- the 80 character line length.

Thanks for the detailed feedback! I've addressed all the issues:

Fixed Backward Compatibility

The original approach broke backward compatibility by changing the interface signature. I've now:

• Kept the original SendOutputs method unchanged for backward compatibility
• Added a new SendOutputsWithChangeAddr method that accepts the optional change address parameter
• Updated all implementations (BtcWallet, RPCKeyRing, mocks) to provide both methods
• The old SendOutputs now delegates to the new method with nil for change address

This ensures any external code using the WalletController interface continues to work without changes.

Ran make rpc with Docker

Successfully ran make rpc using Docker to regenerate all protobuf files. All generated files are now up to date.

Code Formatting

Ran make fmt - all code follows LND style guidelines. No line length violations in hand-written code (only in auto-generated proto files, which is expected).

Organized into Logical Commits

Split changes into 13 focused commits following the <package>: <summary> pattern:

1. Interface changes for backward compatibility
2. Implementation in btcwallet
3. Mock updates
4. RPC wallet updates
5. RPC server changes
6. Walletkit proto updates
7. Proto regeneration

@anumukul – This may came across as unsolicited advice but please try to write responses in your own words instead of using Large Language Model. That way it would feel the OP responses more humanized and to the point

[lightninglabs-deploy]

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