-
Notifications
You must be signed in to change notification settings - Fork 123
assets swap deposits [1/N]: add asset HTLC helpers #979
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary of Changes
Hello @bhandras, 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 lays the groundwork for atomic asset-to-Bitcoin swaps by introducing a dedicated SwapKit
and a suite of helper functions. These additions streamline the creation, management, and spending of HTLCs, providing robust support for script generation, Taproot integration, and witness construction, which are essential components for facilitating secure and efficient cross-chain asset transfers.
Highlights
- New HTLC Helper Struct: Introduced
SwapKit
, a central helper struct designed to manage and construct Hash Time-Locked Contracts (HTLCs) specifically for atomic asset-to-Bitcoin swaps. This struct encapsulates all necessary parameters for a swap. - HTLC Script Generation: Added utility functions (
GenSuccessPathScript
,GenTimeoutPathScript
) to programmatically generate the Bitcoin scripts for both the success (preimage) and timeout paths of an HTLC, with optionalCHECKSEQUENCEVERIFY
support for pinning attack prevention. - Taproot Integration for HTLCs: Implemented comprehensive Taproot support for HTLCs, including functions to create Taproot leaves (
GetTimeOutLeaf
,GetSuccessLeaf
), compute aggregate MuSig2 keys (GetAggregateKey
), generate Tapscript sibling preimages (GetSiblingPreimage
), and construct Bitcoin control blocks for both success and timeout spending paths (GenSuccessBtcControlBlock
,GenTimeoutBtcControlBlock
). AnOP_TRUE
Taproot leaf helper (CreateOpTrueLeaf
) was also added. - Witness and PSBT Generation: Provided methods within
SwapKit
to facilitate the creation of Bitcoin transaction witnesses for both the success (CreatePreimageWitness
) and timeout (CreateTimeoutWitness
) paths of an HTLC, integrating with anlndclient.SignerClient
for signing and supporting PSBT generation. - Asset Commitment Utilities: Included helper functions (
GenTaprootAssetRootFromProof
,GetPkScriptFromAsset
) to derive the Taproot asset root from a proof and to generate the top-level Bitcoin script for an asset, crucial for integrating asset commitments into the Bitcoin blockchain.
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.
Footnotes
-
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. ↩
There was a problem hiding this 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 SwapKit
struct to facilitate atomic asset to bitcoin swaps using HTLCs. The code includes functions for generating scripts, taproot control blocks, witness creation, and PSBT generation. The review focuses on type safety, documentation, and ensuring the correctness of signature generation.
signDesc := &lndclient.SignDescriptor{ | ||
KeyDesc: keychain.KeyDescriptor{ | ||
KeyLocator: keyLocator, | ||
}, | ||
SignMethod: input.TaprootScriptSpendSignMethod, | ||
WitnessScript: successScript, | ||
Output: assetTxOut, | ||
InputIndex: 0, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The SignDescriptor
struct is being populated with WitnessScript: successScript
, but the Output
field is being populated with assetTxOut
. It's important to ensure that the Output
field is the correct output being signed, and that it corresponds to the WitnessScript
being used. Otherwise, the signature will be invalid.
// Ensure that the Output field corresponds to the WitnessScript being used.
Output: assetTxOut,
signDesc := &lndclient.SignDescriptor{ | ||
KeyDesc: keychain.KeyDescriptor{ | ||
KeyLocator: keyLocator, | ||
}, | ||
SignMethod: input.TaprootScriptSpendSignMethod, | ||
WitnessScript: timeoutScript, | ||
Output: assetTxOut, | ||
InputIndex: 0, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The SignDescriptor
struct is being populated with WitnessScript: timeoutScript
, but the Output
field is being populated with assetTxOut
. It's important to ensure that the Output
field is the correct output being signed, and that it corresponds to the WitnessScript
being used. Otherwise, the signature will be invalid.
// Ensure that the Output field corresponds to the WitnessScript being used.
Output: assetTxOut,
// we use btcutil.Amount here for simplicity, but the actual amount | ||
// is in the asset's native unit. | ||
Amount btcutil.Amount |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider using the asset's Amount
type directly instead of btcutil.Amount
to avoid potential confusion and ensure type safety. This might require changes in other parts of the code that interact with this field.
// we use btcutil.Amount here for simplicity, but the actual amount | |
// is in the asset's native unit. | |
Amount btcutil.Amount | |
// Amount is the amount of the asset that will be swapped. | |
Amount asset.Amount |
assets/htlc/swapkit.go
Outdated
// todo(sputn1ck) assetversion | ||
AssetVersion: asset.Version(1), | ||
Amount: uint64(s.Amount), | ||
Interactive: true, | ||
AnchorOutputIndex: 1, | ||
ScriptKey: asset.NewScriptKey( | ||
tapScriptKey.PubKey, | ||
), | ||
AnchorOutputInternalKey: btcInternalKey, | ||
AnchorOutputTapscriptSibling: &siblingPreimage, | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding a comment explaining why asset.Version(1)
is being used here. Is this a default value, or is there a specific reason for using version 1?
// todo(sputn1ck) assetversion | |
AssetVersion: asset.Version(1), | |
Amount: uint64(s.Amount), | |
Interactive: true, | |
AnchorOutputIndex: 1, | |
ScriptKey: asset.NewScriptKey( | |
tapScriptKey.PubKey, | |
), | |
AnchorOutputInternalKey: btcInternalKey, | |
AnchorOutputTapscriptSibling: &siblingPreimage, | |
}) | |
// AssetVersion: asset.Version(1) // TODO: Determine the correct asset version to use. |
0e3567b
to
13cd0f4
Compare
With this commit we add high level helpers along with scripts to create asset HTLCs.
13cd0f4
to
f561ea9
Compare
This commit enables package relayed HTLCs by making the CSV check in the success path optional.
f561ea9
to
7638d8b
Compare
This PR introduces
SwapKit
, a helper struct for constructing and managing HTLCs used in atomic asset to bitcoin swaps. It includes support for generating success/timeout scripts, taproot control blocks, witness creation, and psbt generation with MuSig2 aggregate keys.Pull Request Checklist
release_notes.md
if your PR contains major features, breaking changes or bugfixes