assets swap deposits [4/N]: add deposit kit encapsulating TAP deposit interactions #984

bhandras · 2025-07-15T15:03:30Z

This PR adds a high level deposit Kit type which includes the necessary functions to create and spend deposits to HTLC or through success, timeout or cooperative MuSig2 sweeps.

Pull Request Checklist

Update release_notes.md if your PR contains major features, breaking changes or bugfixes

gemini-code-assist

Summary of Changes

Hello @bhandras, 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 introduces a comprehensive framework for managing Taproot Asset deposits, which are crucial for enabling asset-based swaps. It provides a dedicated DepositKit for handling the lifecycle of these deposits, from creation to various sweep mechanisms like HTLCs, MuSig2 cooperative sweeps, and timeout paths. Complementary RPC definitions and client-side utilities are added to support these new functionalities.

Highlights

New Deposit Kit: Introduced a high-level Kit type within assets/deposit that encapsulates the logic for creating and managing 2-of-2 MuSig2 asset deposits. This includes key derivation, address generation, and proof verification for these specialized deposits.
Enhanced Tapd Client Functionality: Extended the TapdClient with new methods for interacting with the Taproot Assets wallet. This includes funding and signing virtual PSBTs, preparing and committing virtual PSBTs for on-chain anchoring, logging and publishing asset transfers, querying asset balances, deriving new keys, and importing asset proofs. It also adds new event subscription capabilities for asset receive and send events.
HTLC and Sweep Utilities for Taproot Assets: Added new packages (assets/htlc) providing foundational utilities for constructing and managing Taproot Asset HTLCs. This includes functions for generating success and timeout path scripts, creating OP_TRUE taproot leaves, and a SwapKit for handling asset-to-bitcoin atomic swap mechanics, including witness and control block generation for various sweep scenarios (preimage, timeout).
New RPC Definitions for Asset Deposits: Defined a new gRPC service (AssetDepositService) and associated messages in swapserverrpc. These new RPCs enable the creation of new asset deposits, withdrawal of deposits, and the pushing of partial MuSig2 signatures and private keys for deposit spending, laying the groundwork for asset-based loop-in swaps.

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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

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 issue 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 is currently in preview and 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 to provide feedback.

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 deposit.Kit to encapsulate interactions with Taproot Asset Protocol (TAP) deposits for asset swaps. This is a significant feature addition, introducing new logic for creating and spending deposits via HTLCs, timeouts, or MuSig2 sweeps. The changes are spread across several new files (assets/deposit/kit.go, assets/htlc/script.go, assets/htlc/swapkit.go, assets/tapkit.go) and define new RPCs in swapserverrpc/asset_deposit.proto.

My review focuses on correctness, maintainability, and potential issues. I've identified a critical issue with a panic in a goroutine, some code duplication that should be refactored, and several medium-severity issues including leftover debug statements, commented-out code, and typos in protobuf comments. Addressing these points will improve the robustness and clarity of the new functionality.

gemini-code-assist · 2025-07-15T15:06:50Z

assets/client.go

+			select {
+			case <-receiveEventsClient.Context().Done():
+				panic(receiveEventsClient.Context().Err())
+			default:
+			}


This select block introduces a panic on context cancellation, which is dangerous and can crash the application. The receiveEventsClient.Recv() call below will already return an error when its context is canceled, which is handled correctly. This check is both redundant and unsafe.

Please remove this select block to avoid the panic and rely on the error handling for Recv().

gemini-code-assist · 2025-07-15T15:06:50Z

assets/deposit/kit.go

+func (d *Kit) IsMatchingAddr(addr string) (bool, error) {
+	tap, err := address.DecodeAddress(addr, d.chainParams)
+	if err != nil {
+		log.Debugf("unable to decode address %v: %v", addr, err)


The log package is used here but is not imported in the file, which will cause a compilation error. Please add the necessary import for your logging package.

gemini-code-assist · 2025-07-15T15:06:50Z

assets/client.go

+func handleReceiveEvent(event *taprpc.ReceiveEvent,
+	resChan chan<- TapReceiveEvent) (bool, error) {
+
+	fmt.Printf("!!! Received event: %v\n", event.Status)


This fmt.Printf appears to be a leftover debug statement. It should be removed or replaced with a proper logger call (e.g., log.Debugf) before merging.

gemini-code-assist · 2025-07-15T15:06:51Z

assets/htlc/swapkit.go

+
+// GenTaprootAssetRootFromProof generates the taproot asset root from the proof
+// of the swap.
+func GenTaprootAssetRootFromProof(proof *proof.Proof) ([]byte, error) {


This function GenTaprootAssetRootFromProof is also defined in assets/tapkit.go. To avoid code duplication and improve maintainability, this function should only exist in one place. Since assets/tapkit.go seems to be a utility file for tap-related operations, it would be a good candidate to house this function. Please remove this duplicate implementation.

gemini-code-assist · 2025-07-15T15:06:51Z

assets/tapkit.go

+	/*
+		addressRecvVpkt, err := tappsbt.FromAddresses([]*address.Tap{addr}, 0)
+		if err != nil {
+			return nil, err
+		}
+
+		sweepVpkt.Outputs = addressRecvVpkt.Outputs
+	*/


This block of code is commented out. It should be removed before merging to keep the codebase clean.

gemini-code-assist · 2025-07-15T15:06:51Z

assets/tapkit.go

+		// TODO(bhandras): set this to the actual internal key derived
+		// from the sender node, otherwise they'll lose the 1000 sats
+		// of the tombstone output.
+		AnchorOutputInternalKey: asset.NUMSPubKey,


This TODO comment indicates that there's a potential issue with losing sats if the internal key is not set correctly. This should be addressed before merging, or at least be tracked with a ticket to avoid accumulating technical debt.

gemini-code-assist · 2025-07-15T15:06:51Z

swapserverrpc/asset_deposit.proto

+        returns (WithdrawAssetDepositsServerRes);
+
+    // PushHtlcSigs pushes a MuSig2 partial signatures to the server spending
+    // one ore more deposits to a zero fee HTLC.


There's a typo in the comment. It should be "one or more".

// one or more deposits to a zero fee HTLC.

gemini-code-assist · 2025-07-15T15:06:51Z

swapserverrpc/asset_deposit.proto

+    // PushKeys pushes (ie reveals) the private keys of one ore more deposits to
+    // the server.


There's a typo in the comment. It should be "one or more".

// PushKeys pushes (ie reveals) the private keys of one or more deposits to // the server.

gemini-code-assist · 2025-07-15T15:06:51Z

swapserverrpc/asset_deposit.proto

+    // deposit_keys is a map wich maps deposit_id to deposit internal private
+    // key.


There's a typo in the comment. It should be "which".

// deposit_keys is a map which maps deposit_id to deposit internal private // key.

With this commit we add high level helpers along with scripts to create asset HTLCs.

This commit enables package relayed HTLCs by making the CSV check in the success path optional.

This commit adds additional scaffolding to our tapd client, along with new high-level helpers in the assets package, which will be used later for swaps and deposits.

This commit adds a high level deposit Kit type which includes the necessary functions to create and spend deposits to HTLC or through success, timeout or cooperative MuSig2 sweeps.

gemini-code-assist bot reviewed Jul 15, 2025

View reviewed changes

bhandras force-pushed the asset-swap-deposits-4 branch 8 times, most recently from f506369 to df755fb Compare July 17, 2025 13:27

levmi assigned bhandras Jul 24, 2025

levmi added loop in Assets labels Jul 24, 2025

bhandras added 5 commits July 25, 2025 18:27

assets: add asset HTLC helpers

53aa089

With this commit we add high level helpers along with scripts to create asset HTLCs.

assets: add no-csv option to the asset HTLC to support package relay

83c66fd

This commit enables package relayed HTLCs by making the CSV check in the success path optional.

assets: extend the tapd client and add high-level TAP helpers

0e7e413

This commit adds additional scaffolding to our tapd client, along with new high-level helpers in the assets package, which will be used later for swaps and deposits.

swapserverrpc: add proto definitions for server asset deposit RPCs

070b2a9

assets: add deposit kit encapsulating TAP deposit interactions

9420ee6

This commit adds a high level deposit Kit type which includes the necessary functions to create and spend deposits to HTLC or through success, timeout or cooperative MuSig2 sweeps.

bhandras force-pushed the asset-swap-deposits-4 branch from df755fb to 9420ee6 Compare July 25, 2025 16:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

assets swap deposits [4/N]: add deposit kit encapsulating TAP deposit interactions #984

assets swap deposits [4/N]: add deposit kit encapsulating TAP deposit interactions #984

Uh oh!

bhandras commented Jul 15, 2025

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

gemini-code-assist bot Jul 15, 2025

Uh oh!

gemini-code-assist bot Jul 15, 2025

Uh oh!

gemini-code-assist bot Jul 15, 2025

Uh oh!

gemini-code-assist bot Jul 15, 2025

Uh oh!

gemini-code-assist bot Jul 15, 2025

Uh oh!

gemini-code-assist bot Jul 15, 2025

Uh oh!

gemini-code-assist bot Jul 15, 2025

Uh oh!

gemini-code-assist bot Jul 15, 2025

Uh oh!

gemini-code-assist bot Jul 15, 2025

Uh oh!

Uh oh!

		// PushKeys pushes (ie reveals) the private keys of one ore more deposits to
		// the server.

		// deposit_keys is a map wich maps deposit_id to deposit internal private
		// key.

assets swap deposits [4/N]: add deposit kit encapsulating TAP deposit interactions #984

Are you sure you want to change the base?

assets swap deposits [4/N]: add deposit kit encapsulating TAP deposit interactions #984

Uh oh!

Conversation

bhandras commented Jul 15, 2025

Pull Request Checklist

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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 Jul 15, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jul 15, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jul 15, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jul 15, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jul 15, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jul 15, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jul 15, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jul 15, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jul 15, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!