Add low-level taproot helpers #3086

sstone · 2025-05-15T13:26:36Z

We add a new commitment format for simple taproot channels, and implement creating and verifying musig2 and taproot signatures for these channels.
Wire format and channel creation/update workflows have not been modified, this will be done in another PR.

t-bast

I think there's a bit more work to do around taproot scripts and witnesses to make it cleaner by introducing better types and encapsulation of script trees and their corresponding witnesses. My main comment that requires more work is #3086 (comment), the rest is mostly nits for now.

eclair-core/src/main/scala/fr/acinq/eclair/transactions/Transactions.scala

eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/ErrorHandlers.scala

t-bast

This is looking much better than the attempts we made before the transactions architecture refactoring, that was worth it! I think we're almost there, but can still simplify it a bit.

eclair-core/src/main/scala/fr/acinq/eclair/channel/Commitments.scala

eclair-core/src/test/scala/fr/acinq/eclair/transactions/TransactionsSpec.scala

eclair-core/src/main/scala/fr/acinq/eclair/transactions/Scripts.scala

eclair-core/src/main/scala/fr/acinq/eclair/transactions/Transactions.scala

t-bast · 2025-05-22T08:02:06Z

Needs rebase!

There are no functional changes, we just define a new format that is not used anywhere yet, with dummy "not implemented" blocks when needed.

We can create, sign, spend commit and htlc transactions for the new simple taproot channels commitment format. Wire messages and channel creation/update workflow have not been modified.

We only either single leaves or branches with 2 single leaves (a timeout and a success branch for example), having specific types for them makes to code easier to read.

t-bast · 2025-05-22T09:22:38Z

eclair-core/src/test/scala/fr/acinq/eclair/transactions/TransactionsSpec.scala

+  def assertWeightMatches(actualWeight: Int, expectedWeight: Int, commitmentFormat: CommitmentFormat): Unit = commitmentFormat match {
+    case DefaultCommitmentFormat | _: AnchorOutputsCommitmentFormat => assert(Math.abs(actualWeight - expectedWeight) < 20)
+    case SimpleTaprootChannelCommitmentFormat => assert(actualWeight == expectedWeight)
+  }


This unused function still hasn't been removed?

done in c2d5efe

t-bast · 2025-05-22T09:26:35Z

eclair-core/src/test/scala/fr/acinq/eclair/transactions/TransactionsSpec.scala

    val walletPriv = randomKey()
    val walletPub = walletPriv.publicKey
    val finalPubKeyScript = Script.write(Script.pay2wpkh(walletPub))
-    val commitInput = Funding.makeFundingInputInfo(randomTxId(), 0, Btc(1), localFundingPriv.publicKey, remoteFundingPriv.publicKey, UnsafeLegacyAnchorOutputsCommitmentFormat)
+    // funding tx sends to musig2 aggregate of local and remote funding keys


This comment is wrong for non-taproot cases, we should probably just remove it?

done in c2d5efe

t-bast · 2025-05-22T09:34:33Z

eclair-core/src/test/scala/fr/acinq/eclair/transactions/TransactionsSpec.scala

+            localPartialSig <- txInfo.partialSign(localFundingPriv, remoteFundingPriv.publicKey, Map.empty, LocalNonce(secretLocalNonce, publicLocalNonce), publicNonces)
+            remotePartialSig <- txInfo.partialSign(remoteFundingPriv, localFundingPriv.publicKey, Map.empty, LocalNonce(secretRemoteNonce, publicRemoteNonce), publicNonces)
+            _ = assert(txInfo.checkRemotePartialSignature(localFundingPriv.publicKey, remoteFundingPriv.publicKey, remotePartialSig, publicLocalNonce))
+            invalidRemotePartialSig = remotePartialSig.copy(remotePartialSig.partialSig.reverse, remotePartialSig.nonce)


nit: this call to copy is a bit weird, you're not specifying what you're copying?

Suggested change

invalidRemotePartialSig = remotePartialSig.copy(remotePartialSig.partialSig.reverse, remotePartialSig.nonce)

invalidRemotePartialSig = remotePartialSig.copy(partialSig = remotePartialSig.partialSig.reverse)

Or maybe:

Suggested change

invalidRemotePartialSig = remotePartialSig.copy(remotePartialSig.partialSig.reverse, remotePartialSig.nonce)

invalidRemotePartialSig = ChannelSpendSignature.PartialSignatureWithNonce(randomBytes32(), remotePartialSig.nonce)

done in c2d5efe

t-bast · 2025-05-22T09:48:34Z

eclair-core/src/main/scala/fr/acinq/eclair/transactions/Transactions.scala

@@ -148,6 +149,29 @@ object Transactions {
   */
  case object ZeroFeeHtlcTxAnchorOutputsCommitmentFormat extends AnchorOutputsCommitmentFormat

+  sealed trait TaprootCommitmentFormat extends CommitmentFormat
+
+  case object SimpleTaprootChannelCommitmentFormat extends TaprootCommitmentFormat {


This should probably become a trait since we want two support a variation of the official commitment format where HTLC txs pay the same feerate as the commit tx, like what we do for anchor outputs (for mobile wallets until we're able to move to v3).

done in 6ec1d0c

t-bast · 2025-05-22T09:49:51Z

eclair-core/src/main/scala/fr/acinq/eclair/transactions/Transactions.scala

@@ -183,8 +210,10 @@ object Transactions {
     * @param leafHash    hash of the leaf script we're spending (must belong to the tree).
     */
    case class TaprootScriptPath(internalKey: XonlyPublicKey, scriptTree: ScriptTree, leafHash: ByteVector32) extends Taproot {
-      require(Option(scriptTree.findScript(KotlinUtils.scala2kmp(leafHash))).nonEmpty, "script tree must contain the provided leaf")
-      val redeemScript: ByteVector = KotlinUtils.kmp2scala(scriptTree.findScript(KotlinUtils.scala2kmp(leafHash)).getScript)
+      import KotlinUtils._


We import KotlinUtils 11 times in this file, why not do it a single time for the whole file in the main imports?

done in c2d5efe

t-bast · 2025-05-22T09:55:21Z

eclair-core/src/main/scala/fr/acinq/eclair/transactions/Transactions.scala

+        RedeemInfo.P2wsh(redeemScript)
+      case SimpleTaprootChannelCommitmentFormat =>
+        val receivedHtlcTree = Taproot.receivedHtlcScriptTree(commitKeys, paymentHash, htlcExpiry)
+        RedeemInfo.TaprootScriptPath(commitKeys.revocationPublicKey.xOnly, receivedHtlcTree.scriptTree, KotlinUtils.kmp2scala(receivedHtlcTree.success.hash()))


nit: can remove the explicit KotlinUtils.kmp2scala if we import KotlinUtils at the file root. Same for other similar calls in this file.

done in c2d5efe

t-bast · 2025-05-22T10:08:54Z

eclair-core/src/main/scala/fr/acinq/eclair/transactions/Transactions.scala

+      case SimpleTaprootChannelCommitmentFormat =>
+        if (toLocalAmount >= dustLimit || hasHtlcs) {
+          val redeemInfo = RedeemInfo.TaprootKeyPath(commitmentKeys.localDelayedPaymentPublicKey.xOnly, Some(Taproot.anchorScriptTree))
+          outputs.append(ToLocalAnchor(TxOut(AnchorOutputsCommitmentFormat.anchorAmount, redeemInfo.pubkeyScript)))
+        }
+        if (toRemoteAmount >= dustLimit || hasHtlcs) {
+          val redeemInfo = RedeemInfo.TaprootKeyPath(commitmentKeys.remotePaymentPublicKey.xOnly, Some(Taproot.anchorScriptTree))
+
+          outputs.append(ToRemoteAnchor(TxOut(AnchorOutputsCommitmentFormat.anchorAmount, redeemInfo.pubkeyScript)))


Can't we group AnchorOutputsCommitmentFormat and SimpleTaprootChannelCommitmentFormat? Most of the logic is the same, it's only the redeemInfo construction that changes, but we already have redeemInfo helpers exposed on the ClaimAnchorOutputTx?

done in c2d5efe

sstone requested a review from t-bast May 15, 2025 13:26

sstone marked this pull request as ready for review May 15, 2025 13:26

sstone mentioned this pull request May 15, 2025

Add low-level tooling support for taproot channels #3079

Closed

t-bast reviewed May 16, 2025

View reviewed changes

t-bast reviewed May 19, 2025

View reviewed changes

sstone added 8 commits May 22, 2025 10:10

Add a new commiment format for simple taproot channels

8016502

There are no functional changes, we just define a new format that is not used anywhere yet, with dummy "not implemented" blocks when needed.

Implement low-level tooling support for simple taproot channels

444f152

We can create, sign, spend commit and htlc transactions for the new simple taproot channels commitment format. Wire messages and channel creation/update workflow have not been modified.

Fixup: address review comments

b53328d

Fixup: Add new types to simplify how we create script trees

0ca8e38

We only either single leaves or branches with 2 single leaves (a timeout and a success branch for example), having specific types for them makes to code easier to read.

Fixup: remove unused import

0be491c

Simplify and extend transaction tests

a9c411c

Fixup: Simplify makeCommitTxOutputs()

7520a9f

Use Musig2.verifyTaprootSignature from bitcoin-lib 0.39

8fcc0f9

sstone force-pushed the add-taproot-helpers branch from 668da46 to 8fcc0f9 Compare May 22, 2025 08:30

t-bast reviewed May 22, 2025

View reviewed changes

sstone added 2 commits May 22, 2025 17:21

Fixup: cleanup + simplify makeCommitTxOutputs

c2d5efe

Add commitment formats for Legacy/ZeroFee Htlc taproot channels

6ec1d0c

	invalidRemotePartialSig = remotePartialSig.copy(remotePartialSig.partialSig.reverse, remotePartialSig.nonce)
	invalidRemotePartialSig = remotePartialSig.copy(partialSig = remotePartialSig.partialSig.reverse)

	invalidRemotePartialSig = remotePartialSig.copy(remotePartialSig.partialSig.reverse, remotePartialSig.nonce)
	invalidRemotePartialSig = ChannelSpendSignature.PartialSignatureWithNonce(randomBytes32(), remotePartialSig.nonce)

Add low-level taproot helpers #3086

Are you sure you want to change the base?

Add low-level taproot helpers #3086

Uh oh!

Conversation

sstone commented May 15, 2025

Uh oh!

t-bast left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

t-bast left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

t-bast commented May 22, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

t-bast left a comment •

edited

Loading