feat(wallet-connect): impl BTC (UTxO) for swaps v2 #2617
+221
−90
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Collaborator
Author
|
|
…ash type this makes it so we return the bare p2sh siganture (bare as in, no redeem or unlocking script were added to the signature. i.e. the signature is exactly the one walletconnect returned with no modifications). this also adds one extra configuration parameter to change the sighash type of the transaction. this is to be used later when signing with SIGHASH_SINGLE vs SIGHASH_ALL in the TPU.
remove the unneeded `pub` for some methods (until they are needed in the future) and refactor sign_p2sh into two different functions. one function that returns a complete finalized transaction without the bare signature (used in swaps v1 since these are simple, non-cooperative p2sh spends) one function that returns the bare p2sh signature (i.e. no modifications to the signature what so ever). this function doesn't require an unlocking_script since that's only needed to finalize a p2sh transaction and we are not trying to finalize it here
was due to the wrap inside push_data and also that an extra sighash types byte was being added
the sighash_types should be sighashTypes for walletconnect to understand it also the doc comment was updated based on the returned error message by walletconnect. the returned error message state that sighashTypes field is a whitelist of possible sighash_types to allow in the psbt signing process. such thing is not mentioned explicitly in reown's docs (though i would say reown's docs is the vauge one here, esp since they take sighashTypes as an array). reown's docs about sighashTypes (weird because: 1- it's a list, 2- this field already exists in the psbt): sighashTypes : Integer[] - (Optional) Specifies which part(s) of the transaction the signature commits to. Default is [1]. walletconnect error message about missing sighashTypes (happens when you try to sign using SIGHASH_SINGLE while a missing sighashTypes defaults to [SIGHASH_ALL]): Sighash type is not allowed. Retry the sign method passing the sighashTypes array of whitelisted types. Sighash type: SIGHASH_SINGLE
mariocynicys
force-pushed
the
btc-walletconnect-for-swaps-v2
branch
from
f2726c3 to
51ac7e0
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for swaps v2 (TPU) via WalletConnect for UTxO-based cryptocurrencies. It refactors the existing WalletConnect swap functionality to handle both v1 and v2 swap protocols by introducing a new P2SHSigner abstraction that enables cooperative signing for funding and payment spend transactions.
- Introduces
P2SHSignerenum to abstract between KeyPair and WalletConnect signing methods - Adds new WalletConnect signing functions for v2 swap cooperative signing
- Updates swap transaction generation and signing functions to use the new signer abstraction
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| wallet_connect_tests.rs | Parameterizes test function to support both v1 and v2 swap protocols |
| wallet_connect.rs | Adds v2-specific WalletConnect signing functions with bare signature extraction |
| utxo_standard.rs | Updates v2 swap operations to use new P2SHSigner abstraction |
| utxo_common.rs | Refactors signing logic throughout swap functions to support both KeyPair and WalletConnect signers |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Comment on lines
1540
to
+1546
| ) -> Result<UtxoTx, TransactionErr> { | ||
| let secret_hash = dhash160(secret); | ||
| let redeem_script = swap_proto_v2_scripts::taker_payment_script( | ||
| try_tx_s!(gen_args.time_lock.try_into()), | ||
| secret_hash.as_slice(), | ||
| gen_args.taker_pub, | ||
| htlc_keypair.public(), | ||
| gen_args.maker_pub, |
There was a problem hiding this comment.
This line appears to be a bug fix - changing from htlc_keypair.public() to gen_args.maker_pub. This should be verified as it changes the public key being used in the script generation.
|
@dimxy @shamardy @onur-ozkan |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds support for swaps v2 (TPU) via WalletConnect for UTxO.
The only difference in transaction signing between v1 and v2 swaps are just the co-operative signing step in:
funding spend (aka taker payment)taker payment spend (aka finalizing tx)In these two, each side performs pretty much the same P2SH signing call as before but only stores (and sends) the bare signature returned from WalletConnect. This is analogous to calling
calc_and_sign_sighashusing aKeyPair.And then the same process as before goes on: signatures are later combined to create the complete
script_sig, etc...A manual test is added for this feat:
test_walletconnect_swap_v2, and this: #2566 (comment) is how to run it.