feat(wallet-connect): impl BTC (UTxO) basic signing and swaps v1 #2566
Conversation
this method will be needed in swaps as we can't use sendTransfer for unlocking HTLCs (i.e. needed for swaps v1 and v2)
which forces us to return Public instead of &Public from this function, and a whole wave of ripple effects all over
no watchers, just plain swaps v1 methods
the caller needs to provide a map of utxo transactions for the p2pkh inputs, otherwise signing will fail
soley contained in geenrate_and_send_tx
|
There are two pending fixmes that i will resolve in this PR. The PR is still reviewable at its current state and won't change much. |
|
Are any docs updates required for this PR? |
nope
p2sh support doesn't mean spending p2sh utxos that the wallet own, but rather spending p2sh utxos that are byproduct of the swap in intermediate steps. |
|
this wallet should be used for testing. |
…s in utxo_common so kdf won't panic cuz we don't have walletconnect we better remove these panics tho note that not all these methods are needed for swap v1. but i did them nontheless
|
added a utility test function in my last commit that performs a basic swap (will figure out how to test for the refund case without having to wait eternity, but for now this tests the swap going the pretty way). to run the test basically you wanna have two walletconnect capable wallets that support utxo (i.e. this boy) and have them perform trades with each other. the test trades tbtc against tbtc, as the test wallet above doesn't support other utxo coins that could be traded. in the test we instantiate two coins tbtc-1 and tbtc-2 but they are essentially the same coin with the same blockchain and same electrums. this is how to run the test:
cc/ @smk762 |
requires manual testing with walletconnect capable wallets by the test step by step
mariocynicys
force-pushed
the
btc-walletconnect-for-swaps
branch
from
daaf758 to
c3e6b08
Compare
There is a custom time lock config option for that. |
idk why we sent the watcher message one time then moved to the next step and created a handle to keep sending it periodically. this commit makes it so we don't do the initial send but only create the handler to keep sending it periodically as before (this part didin't change, but the log message was put in the appropriate place)
this is to be handled in a different PR since it seems it might be complext. we need to sign the watcher msg with the htlc privkey which means we will ask walletconnect for one more signature. this might get entangled with p2p crate code
via reverting back to use derive_htlc_key_pair using the unique swap data
regarding whether we should bake fork_id into the sighash_type or would the signer already do it for us
shamardy
left a comment
shamardy
left a comment
There was a problem hiding this comment.
LGTM! Only one non-blocker nit!
mm2src/coins/utxo/utxo_common.rs
Outdated
| pub fn derive_htlc_pubkey(coin: &UtxoCoinFields, swap_unique_data: &[u8]) -> [u8; 33] { | ||
| match coin.priv_key_policy { | ||
| PrivKeyPolicy::WalletConnect { public_key, .. } => public_key.0, | ||
| _ => derive_htlc_key_pair(coin, swap_unique_data) |
There was a problem hiding this comment.
Don't like the wildcard pattern, please use an exhaustive list.
in derive_htlc_pubkey just like in derive_htlc_key_pair
Are there any related docs updates needed? |
api wise, there are no new methods or updated endpoints. we can write some docs (more of user docs) about the fact that popups that will show up in the user's walletconnnect-capable wallet to ask for address/pubkey sharing, msg & transaction signing. if that makes sense. |
it wasn't like that before, so leaving it as is to avoid unforeseen errors
KDF WASM Playground Previews
|
|
fyi, the wasm problem was induced by this commit (51905d4) that adds |
but not yet for wasm! since taproot signing uses rust-bitcoin (to no re-invent the wheel), adding rust-bitcoin dependency on wasm will break linking just like what happened in #2566 since #262 already fixes this issue by converging all versions of secp (and thus rust-bitcoin) and enabling them on wasm environment, let's defer wasm support till that PR is merged
This PR adds support for basic P2PKH, P2WPKH, and single-input P2SH signing via WalletConnect.
These basic signing operations are the ones needed for swaps v1, which is implemented here as well for WalletConnect.
P2(W)PKH signing is used in
taker fee/maker payment/taker paymentP2SH signing is used in
taker payment spend/maker payment spend/taker payment refund/maker payment refundswaps v2 will use the existing WalletConnect signing functions + some new methods for the co-operative signing step.
Advised to review commit by commit to rule out the bulky chore changes.