Taproot: Add taproot address and signing capabilities #792

guggero · 2022-02-14T16:28:37Z

~~Depends on lightningnetwork/lnd#6285 (for the TLV module).~~

Adds taproot key spend and script spend addresses.

Merge order for this saga of PRs is:

Roasbeef · 2022-02-15T00:41:32Z

Dependent PR has been merged.

wallet/txauthor/go.mod

Roasbeef · 2022-02-15T00:44:12Z

go.mod

+
+replace github.com/btcsuite/btcd/btcec/v2 => github.com/guggero/btcd/btcec/v2 v2.0.0-20220214155024-51fd177d32c0
+
+replace github.com/btcsuite/btcwallet/wallet/txauthor => ./wallet/txauthor


FWIW this was removed in a prior PR, but I can see how it's helpful to fully integrate things in one swoop. Otherwise, we can make a new PR that adds the new txscript stuff, then does a tag, then to have it integrated here ultimately.

wallet/createtx.go

wallet/psbt.go

waddrmgr/address.go

waddrmgr/manager_test.go

wallet/txauthor/author.go

go.mod

wallet/txauthor/go.mod

waddrmgr/address.go

guggero · 2022-02-22T16:00:32Z

The PR is now ready for review! I'll take a look at the linter and unit test failures in a bit.

Roasbeef

Excellent PR!

Completed an initial review pass through the latest set of commits. I like how we don't bind the caller/user into a particular way and allow them to either use a single script path or just import the entire tree for tracking/spending purposes. This'll really be useful once we start to implement more advanced multi-sig and other script operations using the wallet.

Ultimately, we may want to start (from an lnd perspective) also importing our scripts into the wallet, but possibly with a non-default account type. In any case, that's beyond the scope of this PR.

Only lingering design question IMO is if we really need to distinguish between pure-keyspend and a mixed variant for addresses. I guess as is, we assume usage of BIP 86, so anything that doesn't use that should use this new import API instead.

wallet/txauthor/author.go

waddrmgr/db.go

waddrmgr/scoped_manager.go

waddrmgr/address.go

waddrmgr/tapscript.go

waddrmgr/tlv.go

Roasbeef · 2022-02-23T19:21:45Z

@carlaKC

Roasbeef

Amazing PR!

The diff is looking really good from my PoV now. I think the only two things we'll want to do as follow up are:

Ensure that on the lnd side, we have proper recovery working, which may need some additional modifications to the recovery/rescan logic. I think since it works at mostly a higher level and lets the waddrmgr derive the addresses it should be ok. It can also make optimizations like not scan for taproot addrs if an address was created before a certain height, but that may generate some false positives.
(this one is a bonus imo) implement the PSBT parsing and assembly functionality.

waddrmgr/scoped_manager.go

As a preparation for when the identifier of a script isn't the hash of a script in every case anymore, we extract that part out of the importScriptAddress method and rename it. Pay-to-Taproot script addresses use the x-only coordinates of a tweaked public key as the address identifier.

To make integration of the upcoming taproot script address type a bit easier, we allow returning a ManagedScriptAddress interface type in newWitnessScriptAddress so we can re-use that function for the taproot script address too.

guggero · 2022-03-16T18:45:41Z

Rebased after the dependent PR has been merged.
Still need to verify the recovery of taproot addresses on the lnd side. But can always merge and create a bugfix later (will need a PR to remove the local replaces after pushing a tag anyway).

Roasbeef · 2022-03-17T22:56:09Z

Still need to verify the recovery of taproot addresses on the lnd side. But can always merge and create a bugfix later (will need a PR to remove the local replaces after pushing a tag anyway).

Definitely, I think we can easily parametrize our existing integration tests for that, as they already test p2wkh/np2wkh, etc.

guggero · 2022-03-18T17:53:02Z

Definitely, I think we can easily parametrize our existing integration tests for that, as they already test p2wkh/np2wkh, etc.

I added that integration test to lightningnetwork/lnd#6263, no issues found.

I also added a commit that exposes the ImportTaprootScript method on the wallet level (and not only on the scoped manager level). That method is now also used in a unit test in lnd.

I think with those two things tested end-to-end, this PR is ready to be merged.

positiveblue

Great job on this one @guggero, really excited to see this ready to be merged 🎉

carlaKC

Great PR! Did a high level santiy pass, looks great.

I think that the distinction between TaprootAddress and TaprootScript discussed here will be useful, the two different use cases make sense to me.

carlaKC · 2022-03-22T08:58:18Z

waddrmgr/scoped_manager.go

 	)
 }

 // importScriptAddress imports a new pay-to-script or pay-to-witness-script
 // address.
 func (s *ScopedKeyManager) importScriptAddress(ns walletdb.ReadWriteBucket,
-	script []byte, bs *BlockStamp, addrType AddressType,
+	identity Identity, script []byte, bs *BlockStamp, addrType AddressType,


Why do we need to pass the identity closure in here vs just []byte?

See comment here: #792 (comment)
Gives us the ability to use nicely named, pre-defined identity functions.

waddrmgr/address.go

Taproot: Add taproot address and signing capabilities

guggero mentioned this pull request Feb 14, 2022

Taproot: integrate btcec/v2 and btcwallet changes to support Taproot key spend paths in lnd's wallet lightningnetwork/lnd#6263

Merged

This was referenced Feb 15, 2022

multi: update signing RPCs to expose new tapscript+taproot sig types lightningnetwork/lnd#6267

Closed

multi: permit sending to and receiving from P2TR addresses lightningnetwork/lnd#6266

Closed

Roasbeef reviewed Feb 15, 2022

View reviewed changes

guggero force-pushed the schnorr-taproot branch from 4ec92b0 to 203df47 Compare February 15, 2022 16:14

3nprob reviewed Feb 16, 2022

View reviewed changes

go.mod Outdated Show resolved Hide resolved

3nprob reviewed Feb 16, 2022

View reviewed changes

wallet/txauthor/go.mod Outdated Show resolved Hide resolved

guggero force-pushed the schnorr-taproot branch 2 times, most recently from 6e96598 to aca9822 Compare February 18, 2022 18:44

3nprob reviewed Feb 21, 2022

View reviewed changes

waddrmgr/address.go Show resolved Hide resolved

guggero force-pushed the schnorr-taproot branch 2 times, most recently from da559a8 to 6a78190 Compare February 21, 2022 16:42

Roasbeef mentioned this pull request Feb 21, 2022

multi: taproot mega tracking issue btcsuite/btcd#1735

Closed

15 tasks

guggero force-pushed the schnorr-taproot branch from 6a78190 to ff7eeb1 Compare February 22, 2022 15:58

guggero marked this pull request as ready for review February 22, 2022 15:59

guggero force-pushed the schnorr-taproot branch 2 times, most recently from c253f14 to 6d1c60c Compare February 22, 2022 20:16

guggero requested a review from Roasbeef February 22, 2022 20:31

Roasbeef requested changes Feb 23, 2022

View reviewed changes

guggero force-pushed the schnorr-taproot branch from 6d1c60c to b42468a Compare February 23, 2022 08:42

guggero force-pushed the schnorr-taproot branch 2 times, most recently from f656436 to 2b6e334 Compare March 7, 2022 19:05

Roasbeef approved these changes Mar 9, 2022

View reviewed changes

waddrmgr/scoped_manager.go Show resolved Hide resolved

guggero force-pushed the schnorr-taproot branch 3 times, most recently from 75369b4 to cb37aec Compare March 9, 2022 19:01

txauthor: bump to latest txscript package

adeb823

guggero added 7 commits March 16, 2022 19:43

multi: bump btcec and btcutil to new versions

57d44d3

waddrmgr: add taproot pubkey address type

03e8e89

wallet: add taproot pubkey signing to signer

5eb1a9b

txauthor: add taproot pubkey address type

a80b80e

waddrmgr: store taproot addresses as witness v1 addrs

6c445ed

guggero force-pushed the schnorr-taproot branch from cb37aec to 89167fe Compare March 16, 2022 18:43

guggero force-pushed the schnorr-taproot branch from 89167fe to 85b35fa Compare March 16, 2022 19:29

positiveblue approved these changes Mar 21, 2022

View reviewed changes

carlaKC approved these changes Mar 22, 2022

View reviewed changes

guggero added 2 commits March 22, 2022 10:38

mod+waddrmgr: add taproot script address

f68a548

wallet: add ImportTaprootScript method

15b5f8a

guggero force-pushed the schnorr-taproot branch from 8de604d to 15b5f8a Compare March 22, 2022 09:38

Roasbeef merged commit b0001c2 into btcsuite:master Mar 22, 2022

guggero deleted the schnorr-taproot branch March 22, 2022 18:30

buck54321 pushed a commit to buck54321/btcwallet that referenced this pull request Apr 21, 2024

Merge pull request btcsuite#792 from guggero/schnorr-taproot

e7ba21c

Taproot: Add taproot address and signing capabilities

buck54321 pushed a commit to buck54321/btcwallet that referenced this pull request Apr 21, 2024

Merge pull request btcsuite#792 from guggero/schnorr-taproot

35b9664

Taproot: Add taproot address and signing capabilities

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Taproot: Add taproot address and signing capabilities #792

Taproot: Add taproot address and signing capabilities #792

guggero commented Feb 14, 2022 •

edited

Roasbeef commented Feb 15, 2022

Roasbeef Feb 15, 2022

guggero commented Feb 22, 2022

Roasbeef left a comment

Roasbeef commented Feb 23, 2022

Roasbeef left a comment

guggero commented Mar 16, 2022

Roasbeef commented Mar 17, 2022

guggero commented Mar 18, 2022

positiveblue left a comment

carlaKC left a comment

carlaKC Mar 22, 2022

guggero Mar 22, 2022


		replace github.com/btcsuite/btcd/btcec/v2 => github.com/guggero/btcd/btcec/v2 v2.0.0-20220214155024-51fd177d32c0

		replace github.com/btcsuite/btcwallet/wallet/txauthor => ./wallet/txauthor

Taproot: Add taproot address and signing capabilities #792

Taproot: Add taproot address and signing capabilities #792

Conversation

guggero commented Feb 14, 2022 • edited

Roasbeef commented Feb 15, 2022

Roasbeef Feb 15, 2022

Choose a reason for hiding this comment

guggero commented Feb 22, 2022

Roasbeef left a comment

Choose a reason for hiding this comment

Roasbeef commented Feb 23, 2022

Roasbeef left a comment

Choose a reason for hiding this comment

guggero commented Mar 16, 2022

Roasbeef commented Mar 17, 2022

guggero commented Mar 18, 2022

positiveblue left a comment

Choose a reason for hiding this comment

carlaKC left a comment

Choose a reason for hiding this comment

carlaKC Mar 22, 2022

Choose a reason for hiding this comment

guggero Mar 22, 2022

Choose a reason for hiding this comment

guggero commented Feb 14, 2022 •

edited