Taproot swaps#10756
Conversation
SomberNight
left a comment
There was a problem hiding this comment.
Taproot-swaps: pretty cool feature.
This is a lot of code. It is fairly clear you generated it.
It is so much that it is a daunting task to review.
You basically need to get buy-in from a maintainer to review it.
but also, before asking someone else to review it, you need to simplify, de-dupe, and review it yourself. Did you review it yourself?!
Also, what's with all the Tor stuff? It seems completely unrelated. The larger you make the PR, the more likely it will never get merged. Split out unrelated stuff for later.
Even stuff you don't postpone for later, see if you can split into logically separate small commits of up to a few hundred lines each (not counting tests).
(The electrum-ecc PR needs to come first anyway though.)
Keep in mind someone needs to maintain master. Debugging code is much harder than writing it, and that was stated 50 years ago (Kernighan's Law), before clankers could write it for you.
|
Hi @SomberNight, thanks for taking a look. This wasn't ready for review yet, my bad for not labeling it as draft (fixed now). I opened it early to discuss the concept and validate the approach as a PoC before polishing the code, but I should have made that explicit. Your three code comments are all valid catches (the On the concept, which is what I'd like to settle first. The swapserver feature is very cool because it easily enables a P2P LN swap market for many Electrum users. That's why I want to build on top of it rather than start something from scratch (I reached out to @ecdsa a few weeks ago and he showed interest in Taproot swaps). Three things I'd like to improve:
The first two already work on this branch: I have live regtest E2E tests where the client discovers the offer on a real Nostr relay, negotiates taproot-v1, and settles the swap (cooperative MuSig2 claim and script-path claim) against bitcoind + Lightning. The web client is in progress. My plan is to get the whole thing running on signet to learn the limitations, then settle the protocol and do a clean, reviewable implementation. That we can split into small PRs or commits, however you see fit. What do you think about the concept? Is there anything you would change or do differently? Is this concept something you'd want in Electrum? |
|
@m0wer some thoughts regarding your comment:
another approach might be using onion messages, Electrum already supports them but it might be non-trivial to get onion messages working in a web app (though maybe some LDK crates could be used with wasm).
Note there is already such a project ongoing, https://www.whales.exchange/. |
|
Hi @f321x, thanks for the pointers. Onion messages: I looked at
whales.exchange limitations (#10611, #10650): thanks, I wasn't aware of these. Both apply to this branch as-is, since taproot-v1 only changes the on-chain contract and keeps the LN flows unchanged. A recap of the flows for context: Forward swaps (client wants to send onchain and receive through LN):
Reverse swaps (client wants to send through LN and receive onchain): are already structurally the same in Electrum and Boltz, and third-party friendly: the client generates the preimage, the hold invoice lives server-side only, and the client claims on-chain with the preimage (cooperative key path under taproot). The remaining wart is the miner-fee prepay, which protects the server from a mining-fee DoS (user never claims after the server broadcasts the lockup; see ThomasV's bundled payments post). Electrum protects the client via bundled payments, which is an Electrum-only invoice extension, so third-party wallets risk losing the prepay. Boltz's taproot API dropped the prepay entirely ( Since this branch already implements the cooperative key-path refund ( |
Where does the client fetch the offers from? Is that still nostr?
With taproot swaps, does it matter if the nostr relay can "time" swaps? What does that mean exactly? What do they compare the timing against? With current swaps I understand they can compare it against blockchain/mempool events because they see the scripts, but that does not apply to cooperative successful taproot swaps. Regarding selectively dropping messages, what exactly do you have in mind? We already send/receive direct messages across all configured relays, not just one. Do you mean all relays could form a cartel and DDOS the ones that refuse to join the cartel - or what is your threat model?
If the client talks to the swap server over a clearnet URL, that too reveals the IPs of the counterparties to each other. (just like the onion message direct peer connection fallback) As it would be much simpler to just keep using nostr for everything, the extra complexity really needs to be justified. |
I agree. it looks like you are trying to smuggle features we do not necessarily want.
why? there is a reason why I removed the old flow. With a central server, it was possible to assume that the server was (relatively) well maintained and did have liquidity to send the payment. We cannot make that assumption in a decentralized marketplace. So, reverting to the "client goes on-chain first" flow is out of question.
I did express interest in taproot swaps. but not in the two other items in your list. As mentioned by @SomberNight, those other things are orthogonal from taproot, and should not be bundled to this pull request. |
|
Understood. To be clear, there was no intent to smuggle features; I opened the draft to discuss the broader concept and listed those features explicitly. That said, the scope decision is clear, and I agree the transport and web work are orthogonal. I will not add the broader work to this PR. I’ll preserve it separately and reduce the Electrum work to MuSig2 first, followed by a Taproot-only implementation that keeps the existing transport and hold-invoice flow. The Tor transport, web client, and standard-invoice flow will remain outside that scope. |
9fe3c6a to
ddec196
Compare
ddec196 to
9bd2f90
Compare
|
Added some bug fixes in this branch but did not test: https://github.com/1440000bytes/electrum/commits/taproot-swaps-review-fixes/ |
This adds Taproot-based submarine swaps, including script-path signing and MuSig2 support.
Tests and release notes are included. Needs spesmilo/electrum-ecc#17. Assumes
electrum-ecc>=0.0.8, which does not exist and is not bumped in the electrum-ecc PR as requested.Here are the minimal changes needed to make the complete feature work, split across atomic commits. These could be split into smaller PRs if needed.
Breakdown:
Add tapscript transaction signing
Adds BIP342 signature hashing and script-path signing.
Tests cover control blocks, annexes, code separators, leaf versions, and all supported sighash modes.
Commits:
Add Taproot swap primitives
Adds two-leaf Taproot swap contracts and MuSig2 session primitives.
This contains contract and signing logic only. It does not change the swap protocol or server behavior.
Commits:
Add Taproot submarine swaps
Adds optional
taproot-v1support to Electrum's existing submarine swap flow.It preserves the current hold-invoice flow, reverse prepayment, transports, and legacy P2WSH fallback. Taproot swaps use script-path claims and refunds.
Commits:
Add cooperative Taproot swap settlement
Adds optional
taproot-cooperative-v1settlement using MuSig2 key-path spends.MuSig2 sessions are bounded, memory-only, and single-use. Cooperative failures fall back to the existing Taproot script paths.
Commits: