[group key addrs 7/7]: send and receive support for V2 addresses #1587

guggero · 2025-06-06T16:58:25Z

This PR introduces a new V2 address that:

Allows grouped assets to be sent.
Allows the amount to be defined as zero by the recipient, allowing the sender to choose the amount.
Can be re-used indefinitely without any privacy downsides.
Use a new proof courier type to transfer an encrypted send fragment from the sender to the receiver using a mailbox system (but a more robust mailbox than the previous hashmailbox implementation).

fn/memory.go

address/book.go

Roasbeef · 2025-06-17T01:45:07Z

proof/send.go

+	// ScriptKeyDerivationUniquePedersen means the script key is derived
+	// using the address's recipient ID key and a single leaf that contains
+	// an un-spendable Pedersen commitment key
+	// (OP_CHECKSIG <NUMS_key + asset_id * G>). This can be used to


Ah ok, so this is just to ensure unique script keys for each unique asset ID in a send fragment, but we still plan on inserting that new asset TLV containing the shared secret hash to make all asset commitments for a given addr unique?

Correct. Will add the odd TLV value to make sure identical sends (e.g. same amounts and same asset IDs) will still produce a different on-chain output for better privacy.

proof/send.go

Roasbeef · 2025-06-17T01:48:44Z

proof/send.go

+	// TxProof is the proof of the transaction that contains the asset
+	// outputs that are being sent. This is used as proof-of work to show
+	// to the auth mailbox server.
+	TxProof TxProof


Looks like we're duplicating the block header + height here across TxProof and SendFragment. Or is the idea that the encoding is smart enough to only encode once and copy the fields over as needed?

The envelope itself isn't encoded, only the SendFragment within. So the TxProof here is is what's required to send the fragment to the server.
Perhaps "envelope" isn't the best analogy here, as that's also sent along with a letter/message. Going to rename this to SendManifest instead, perhaps that makes more sense.

Roasbeef · 2025-06-17T01:53:42Z

proof/courier.go

+		Proof: &mboxrpc.SendMessageRequest_TxProof{
+			TxProof: rpcProof,
+		},
+		// TODO(guggero): what value?


Good question...I think will depend on what we end up using as the max block height/expiry value for the server.

Went with 210k blocks for now, can perhaps make it configurable later on. And currently we don't delete messages yet. Not sure if we want to add message cleanup (both after user ACK and expiry) in this PR or whether I should create an issue for it.

Roasbeef · 2025-06-17T01:55:20Z

rpcserver.go

 			if err != nil {
 				return nil, fmt.Errorf("unable to decode "+
 					"addr: %w", err)
 			}
+
+			// TODO(guggero): Need to add send fragment envelopes to


Can spin out into a sub-issue.

Do the vPSBTs actually need to be updated though? IIUC, we just need to specify the new proof courier semantics.

Actually, I think we should be able to support the vPSBT flow by just adding the TAP address as a new field to the vOutput. Otherwise we'd have to add safety checks to make sure users don't use the vPSBT flow with V2 addresses and then fail at the proof transfer. So adding vPSBT support is probably the easier way.
Added it as a TODO that I'm going to address before re-requesting review.

Roasbeef · 2025-06-17T01:57:49Z

tapfreighter/chain_porter.go

+				"service handle: %w", err)
+		}
+
+		err = courier.DeliverFragment(ctx, envelope)


Any reason to not fold this into the loop below where we'll attempt to send out the proofs/fragments in parallel?

Good point. The cardinality is different for fragments vs. proofs. But I made it so that we can always try again to send the same message, so we just try in the same loop. Also makes the whole backoff and re-try logic work seamlessly.

tapfreighter/parcel.go

Roasbeef · 2025-06-17T02:03:18Z

taprpc/taprootassets.proto

+    // required for V2 addresses that don't specify an amount (amount of zero).
+    // If an address does specify an amount, then it cannot be overridden in
+    // this map.
+    map<string, uint64> address_amounts = 5;


Hmm, for parsing, what if we allowed tap_addrs to be blank if this is specified, then we only read out address_amounts. This way a user doesn't need to specify an addr twice if it's a v2 addr.

Another alternative would be to upgrade tap_addrs to be a new message type, that can specify the addr, then also a custom amount.

Mainly thinking about how we can streamline the process of a new user potentially unknowingly using a re-useable addr.

Good point here. I updated the RPC to have two fields, the old tap_addrs and new addresses_with_amounts list, both being mutually exclusive. So we can deprecate the older one eventually.

ZZiigguurraatt · 2025-07-23T13:28:44Z

Not sure if you want to address everything in this PR, but do we want to make a "meta" issue like #1446 to track changes to bip-tap.mediawiki that need to be addressed later? Like I feel like there is probably a lot of new stuff related to this PR that is not documented anywhere clearly?

guggero · 2025-07-23T14:19:15Z

Not sure if you want to address everything in this PR, but do we want to make a "meta" issue like #1446 to track changes to bip-tap.mediawiki that need to be addressed later? Like I feel like there is probably a lot of new stuff related to this PR that is not documented anywhere clearly?

Added that here: #293

guggero · 2025-07-23T16:09:13Z

I've addressed all comments and also added a commit that addresses a user experience issue: For a v2 address with a group key, we don't really want to show an asset ID. So it's zeroed out.

I think that introduced a problem in one of the itests, which I'll fix tomorrow. I'll also add more unit test coverage for the receive flow to the asset custodian.

But even with the above two TODOs, I still think this is ready for another round of review.

To make sure we support future recovery cases, we don't want to delete any messages for outpoints that haven't been spent. So we rename and clarify the meaning of the expiry block delta. No cleanup mechanism is currently implemented anyway, so this just serves as documentation for when we do add that feature in the future.

This commit adds a new proof courier protocol that's based on the universe RPC protocol but also adds the auth mailbox capability. We assume that all our official universes will be upgraded to run an auth mailbox compatible universe, so we change the protocol for all our default instances.

This is a preparatory commit that deals with all the database changes required to allow a single address receive event to have multiple asset outputs, and with that multiple proofs associated with it.

As a preparation for creating events from wire transactions that aren't notified by the lnd wallet (and therefore don't come through the lndclient calls), we make the arguments to GetOrCreateEvents a bit more explicit.

We refactor the proof handling to allow multiple outputs to be processed and proofs for them to be fetched.

Adds the new --addrs_with_amounts flag that allows to specify the amount to send to an address that allows sending user-defined amounts (address V2).

For v2 addresses we want the address to only show the group key if a group key is used. The value of the asset ID becomes meaningless for an address that supports group keys. Unfortunately we can't really _remove_ the value from the address (it's always encoded). But we can set it to all zeroes so it's more apparent to the user that it's not used.

guggero · 2025-07-24T09:26:01Z

Okay, I've addressed all TODOs. And with the litd itest fixed, this should now also be testable on the tapd-main-branch of litd, cc @ZZiigguurraatt.

ffranr

I've reviewed until commit multi: don't use lndclient transaction for event

All looks good so far!

ffranr · 2025-07-24T22:57:13Z

taprpc/authmailboxrpc/mailbox.proto

+    // A user-defined expiry block height delta for the message. Once the
+    // outpoint claimed by the proof provided by this message has been spent,
+    // then after this number of blocks after the spending transaction height
+    // the message can be considered expired and may be deleted.
+    uint32 expiry_block_delta = 4;


Why is this value user-configurable? I would have expected it to be a constant defined by the authmailbox service operator.

If this is indeed intended to be set by the client, should we enforce an upper bound on the field? Even if we’re not performing cleanup yet, it still might makes sense to cap the value.

ffranr · 2025-07-24T23:20:07Z

tapdb/addrs_test.go

@@ -155,11 +155,14 @@ func TestAddressInsertion(t *testing.T) {

 	// Make a series of new addrs, then insert them into the DB.
 	const numAddrs = 5
-	proofCourierAddr := address.RandProofCourierAddr(t)
+	addrVersion := test.RandFlip(address.V0, address.V1)


Should we include V2 in this random selection? Other similar statements below.

Could potentially gain V2 address test coverage.

ffranr · 2025-07-24T23:31:26Z

tapdb/addrs.go

+		var assetID asset.ID
+		copy(assetID[:], dbOutput.AssetID)


could use asset.NewIDFromBytes here

ffranr · 2025-07-24T23:47:18Z

tapgarden/custodian.go

+			// This code path is only for V0 and V1
+			// addresses, which only have a single output.


sanity check the addr version here and below in mapToTapAddr?

ffranr · 2025-07-25T00:16:01Z

rpcserver.go

+		// Ensure all addrs are of the same asset specifier, as the
+		// wallet only supports funding for one asset or group at a
+		// time.
+		//
+		// TODO(guggero): Support creating multiple virtual packets, one
+		// for each asset ID when the user wants to send multiple asset
+		// IDs at the same time without going through the PSBT flow.
+		firstSpec := asset.NewSpecifierOptionalGroupPubKey(
+			tapAddrs[0].AssetID, tapAddrs[0].GroupKey,
+		)
+		if idx > 0 {
+			spec := asset.NewSpecifierOptionalGroupPubKey(
+				tapAddrs[idx].AssetID, tapAddrs[idx].GroupKey,
+			)
+			if spec != firstSpec {
+				return nil, fmt.Errorf("all addrs must be of "+
+					"the same asset ID or group key %v",
+					firstSpec)
+			}
+		}


hmm the equality check may not work correctly if the user specifies an asset group for one of the addresses and asset IDs from that group for the others. But probably not a big deal.

ffranr · 2025-07-25T00:17:24Z

tapfreighter/chain_porter.go

+const (
+	// DefaultSendFragmentExpiryDelta is the default number of blocks that
+	// we expect a send fragment to be valid for after its claimed outpoint
+	// has been spent. This is roughly equivalent to 90 days.
+	DefaultSendFragmentExpiryDelta = 12_960
+)


👍, why let the user specify, we can just use this const everywhere perhaps

ffranr · 2025-07-25T00:23:25Z

tapfreighter/wallet.go

 	}

 	return fundedVPkt, nil
 }

+// createSendFragments creates the send fragments for the given funded virtual


this creates the send manifests? so maybe we should rename this function and modify the doc comment?

ffranr · 2025-07-25T00:28:51Z

tapfreighter/wallet.go

+				"for output %d: %w", outIdx, err)
+		}
+
+		if addr.Version < address.V2 {


if addr.Version != address.V2 { here? so we can be explicit about V3, but V3 shouldn't get to this point anyway, maybe we'll forget to update this part of the code or something

Maybe we should add a method to address.Tap. Something like: SupportsSendManifest() bool? So we leak the version switch less.

ffranr · 2025-07-25T00:34:02Z

tapsend/send.go

@@ -252,6 +252,12 @@ func DescribeAddrs(addrs []*address.Tap) (*FundingDescriptor, error) {
 		firstAddr.AssetID, firstAddr.GroupKey,
 	)

+	if firstAddr.Version == address.V2 && firstAddr.GroupKey != nil {


instead of if firstAddr.Version == address.V2 here maybe a method like firstAddr.SupportsGroupKey

levmi assigned guggero Jun 12, 2025

levmi added this to Taproot-Assets Project Board Jun 12, 2025

github-project-automation bot moved this to 🆕 New in Taproot-Assets Project Board Jun 12, 2025

levmi moved this from 🆕 New to 🏗 In progress in Taproot-Assets Project Board Jun 12, 2025

Roasbeef reviewed Jun 17, 2025

View reviewed changes

guggero force-pushed the group-key-addr branch from 937db82 to 79a3ec7 Compare June 18, 2025 13:52

guggero force-pushed the group-key-addr-part-2 branch from a5c2fd0 to b482ea3 Compare June 19, 2025 11:48

guggero changed the title ~~[group key addrs 3/?]: send and receive support for V2 addresses~~ [group key addrs 4/4]: send and receive support for V2 addresses Jun 19, 2025

guggero mentioned this pull request Jun 19, 2025

[group key addrs 3/5]: misc refactor #1611

Merged

guggero changed the title ~~[group key addrs 4/4]: send and receive support for V2 addresses~~ [group key addrs 5/5]: send and receive support for V2 addresses Jun 23, 2025

guggero force-pushed the group-key-addr-part-2 branch from b482ea3 to 8f8e164 Compare June 23, 2025 14:55

guggero changed the base branch from group-key-addr to add-mailbox-server June 23, 2025 14:55

guggero mentioned this pull request Jun 23, 2025

[group key addrs 4/5]: implement mailbox server and message store database #1614

Merged

guggero force-pushed the add-mailbox-server branch from 094b31e to 3d1a126 Compare June 24, 2025 08:27

guggero force-pushed the group-key-addr-part-2 branch from 8f8e164 to 060159b Compare June 24, 2025 08:27

guggero force-pushed the add-mailbox-server branch 3 times, most recently from dc426c6 to 74953ff Compare June 25, 2025 08:49

guggero force-pushed the group-key-addr-part-2 branch 2 times, most recently from db7f36c to a3dbdd3 Compare June 26, 2025 09:14

guggero changed the title ~~[group key addrs 5/5]: send and receive support for V2 addresses~~ [group key addrs 6/6]: send and receive support for V2 addresses Jun 26, 2025

guggero mentioned this pull request Jun 26, 2025

[group key addrs 5/6]: Add Pedersen unique script key type #1621

Merged

guggero changed the base branch from add-mailbox-server to pedersen-keys-refactor June 26, 2025 09:17

guggero force-pushed the pedersen-keys-refactor branch from 5687720 to 46d9159 Compare June 26, 2025 10:18

guggero force-pushed the group-key-addr-part-2 branch from a3dbdd3 to ad2c782 Compare June 26, 2025 10:19

guggero force-pushed the pedersen-keys-refactor branch 2 times, most recently from e5fdf7e to eea552e Compare June 27, 2025 10:56

guggero force-pushed the group-key-addr-part-2 branch 2 times, most recently from ae1aefb to cd9b477 Compare June 30, 2025 15:33

guggero force-pushed the pedersen-keys-refactor branch from eea552e to 555c711 Compare June 30, 2025 15:34

guggero changed the base branch from more-group-key-refactors to main July 22, 2025 11:47

guggero force-pushed the group-key-addr-part-2 branch from feee5ca to a74f887 Compare July 22, 2025 11:47

ZZiigguurraatt mentioned this pull request Jul 23, 2025

Add Multi-RFQ Send #1613

Open

guggero force-pushed the group-key-addr-part-2 branch from a74f887 to 805d197 Compare July 23, 2025 14:15

guggero force-pushed the group-key-addr-part-2 branch from 805d197 to b8ae454 Compare July 23, 2025 16:07

guggero requested review from ffranr and Roasbeef July 23, 2025 16:09

ZZiigguurraatt mentioned this pull request Jul 23, 2025

[bug]: tapcli addrs new -h missing a lot of details #1664

Open

guggero added 15 commits July 24, 2025 11:22

multi: support creating V2 addresses

77b6a4a

address+tapdb: addr event can have multiple outputs

69846c4

This is a preparatory commit that deals with all the database changes required to allow a single address receive event to have multiple asset outputs, and with that multiple proofs associated with it.

multi: add address to transfer output

d550e87

multi: add addr v2 send support

2b81b77

multi: don't use lndclient transaction for event

c1357ed

As a preparation for creating events from wire transactions that aren't notified by the lnd wallet (and therefore don't come through the lndclient calls), we make the arguments to GetOrCreateEvents a bit more explicit.

tapgarden: allow address to have multiple outputs

33352ba

We refactor the proof handling to allow multiple outputs to be processed and proofs for them to be fetched.

multi: add addr v2 receive support

640d57f

authmailbox: export mock server, fix comments

7b03e20

address+tapgarden: add address v2 unit tests to custodian

332c817

itest: add address v2 integration test

aa0e923

address+cmd/commands: allow specifying addrs with amounts

c2b9ffc

Adds the new --addrs_with_amounts flag that allows to specify the amount to send to an address that allows sending user-defined amounts (address V2).

docs: add release notes

601bb7e

guggero force-pushed the group-key-addr-part-2 branch from b8ae454 to 601bb7e Compare July 24, 2025 09:23

itest: fix later itests by not changing global variable

ad9dd7f

ffranr reviewed Jul 25, 2025

View reviewed changes

		// This code path is only for V0 and V1
		// addresses, which only have a single output.

[group key addrs 7/7]: send and receive support for V2 addresses #1587

Are you sure you want to change the base?

[group key addrs 7/7]: send and receive support for V2 addresses #1587

Uh oh!

Conversation

guggero commented Jun 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ZZiigguurraatt commented Jul 23, 2025

Uh oh!

guggero commented Jul 23, 2025

Uh oh!

guggero commented Jul 23, 2025

Uh oh!

guggero commented Jul 24, 2025

Uh oh!

ffranr left a comment

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!

guggero commented Jun 6, 2025 •

edited

Loading