Reference implementation of CT descriptors #31

apoelstra · 2022-10-06T18:36:42Z

I'm aware that there are already a basic form of CT descriptors in this library, and this does not remove them, it just adds a new module which should act as a reference implementation for ElementsProject/ELIPs#1

WIP because it needs a new rust-elements release to get tagged hashes in bitcoin_hashes.

apoelstra · 2022-10-06T18:54:20Z

Oops, missed some files. Added and force-pushed. I squashed everything because the individual commits weren't split out right; let me know if I should split them up again.

sanket1729

Should we remove the Blinded descriptors now? Should this struct also be called Descriptor?

sanket1729 · 2022-10-07T01:08:41Z

src/confidential/mod.rs

+    /// The first public key is included outside of the `Vec` to reduce allocations
+    /// and to enforce that the list of keys is not empty; but it is not special in
+    /// any way.
+    HashToSk(Pk, Vec<Pk>),


This makes creating the Key really annoying. Can we just have a Vec instead? To ensure that there is at least one element, we should wrap this in a struct and add constructors.

IMO, we should really make things convenient for users by adding some extra code and allocations here.

lol, okay, agreed

sanket1729 · 2022-10-07T01:10:52Z

src/confidential/mod.rs

+        let spk = self.descriptor.script_pubkey();
+        self.descriptor
+            .blinded_address(self.key.to_public_key(secp, &spk), params)
+    }


We should add other descriptor APIs later.

sanket1729 · 2022-10-07T01:33:18Z

src/confidential/slip77.rs

+        let mut addr = elements::Address::from_str("2dpWh6jbhAowNsQ5agtFzi7j6nKscj6UnEr").unwrap();
+        addr.blinding_pubkey = Some(mbk.blinding_key(&secp, &spk));
+        assert_eq!(addr.to_string(), "CTEkf75DFff5ReB7juTg2oehrj41aMj21kvvJaQdWsEAQohz1EDhu7Ayh6goxpz3GZRVKidTtaXaXYEJ");
+    }


I did not know this code, but I tested this with elements 22.0 and it works as expected. Perhaps, you can also add this test?

#[test] fn local_test_elements_22_0() { // Local test on elements 22.0 let mbk = MasterBlindingKey::from_hex( "64269a8de756da06ebe35d26dccb4dd46bddcf858b54eeaae315490cfe6cacc0", ) .unwrap(); let addr = elements::Address::from_str( "el1qqg2pz79c0reryhr6hzxrzueju9m2asllwydrhexs6vj854cvwlen4tryh4thsdt2a26rte3fe87rf3my9t90wt78pcqrxv733", ) .unwrap(); let derived_blinding_key = mbk.blinding_private_key(&addr.script_pubkey()); assert_eq!( derived_blinding_key, secp256k1_zkp::SecretKey::from_slice(&unhex( "791a1081ae2ad98a5ad603737c648247f19d3c26e2beb54617638172edb230e7" )) .unwrap() ); }

apoelstra · 2022-10-07T21:09:47Z

@sanket1729 regarding "should we remove the Blinded descriptors now", yes, and re "should this be called Descriptor, I think yes, the goal is to use it as confidential::Descirptor. We can also re-export it under a new name.

sanket1729 · 2022-10-07T22:31:10Z

@apoelstra, agreed on both things

apoelstra · 2022-10-18T14:25:10Z

So, where I'm at with this is that I need

a way to encode internal descriptors without checksums, which requires
feature request: # flag should turn off checksum when formatting descriptors rust-bitcoin/rust-miniscript#477 and then
a new rust-miniscript release, whose use requires
an update of the rust-miniscript dependency here, which requires
an update of the rust-bitcoin and rust-elements dependencies here, which require
Introduce Sequence and LockTime from rust-bitcoin rust-elements#152

sanket1729 · 2022-10-18T18:12:04Z

Thanks for the enumerated list. Left some feedback on the rust-elements repo.

sanket1729 · 2023-01-06T08:47:21Z

@apoelstra, what is the status of this project/PR?

apoelstra · 2023-01-06T15:28:24Z

@sanket1729 I intend to finish this and merge it. I think I've got everything I need now? Will try to do it over the weekend or early next week.

apoelstra · 2023-01-06T15:36:43Z

Rebase only.

apoelstra · 2023-01-31T21:27:21Z

cc @sanket1729 I have finally finished this PR, with test vectors and everything.

No rush, but ready for review.

LeoComandini · 2023-03-15T11:17:36Z

It would be nice to have elements_miniscript::confidential::Descriptor::parse_descriptor implemented so we can use descriptors strings with (extended) private keys. Now with from_str we can only parse descriptors with public keys. It would allow to create test vectors with private keys.

apoelstra · 2023-03-16T16:49:54Z

On @LeoComandini out-of-band advice we may modify this scheme so that the "bare pubkey" variant p2c-commits to the scriptpubkey. Conveniently, even when the scriptpubkey is exposed alongside the resulting key, nobody can "undo" the p2c and learn the original public key. So we get very good on-chain privacy here, even when one participant in a multiparty setup is reusing keys.

Will update after some more thought. Writing here so I don't forget.

sanket1729 · 2023-06-22T08:22:31Z

Can now be rebased

In a previous incarnation of CT descriptors we considered "musig" and "slip77" expressions to constitute keys, allowing them to be parsed in contexts where normally only keys would be allowed (and _not_ expanding them as expression trees which would then need to be reconstructed). This is still plausibly the direction we want to go in for MuSig keys, but it never made a lot of sense for SLIP77, and indeed, we now want to go a different direction, where SLIP77 expressions are allowed only as the first argument of a CT descriptor.

apoelstra · 2023-06-23T00:19:14Z

Nice. I need to add some xpub-based test vectors but this is otherwise ready for review.

Need to re-think the API for this; but sufficient to produce a test vector.

apoelstra · 2023-06-29T22:15:37Z

cc @sanket1729 ready for review

sanket1729

ACK 725f450.

Checked against the spec. Any reason why don't have wildcards in BlindedKey?

sanket1729 · 2023-07-01T00:43:38Z

src/confidential/bare.rs

+            "d12a140aca856fbb917b931f263c42f064608985e2ce17ae5157daa17c55e8d9",
+        );
+
+        // And hash of 100 bytes


nit: 80 bytes

apoelstra · 2023-07-03T01:08:05Z

Checked against the spec. Any reason why don't have wildcards in BlindedKey?

There isn't a type called BlindedKey, but I guess you mean, "why not have wildcards in the blinding key". The reason is a bit dumb -- it's because we have a generic keytype for the main descriptor, but for the blinding key we have either (a) a generic type, or (b) a DescriptorSecretKey.

Generic key types don't, in general, have wildcards. Normally, wildcards are handled by people taking a generic descriptor, using DescriptorPublicKey with it, and then using some convenience methods around Descriptor::translate_pk to map every pubkey to a DefiniteDescriptorKey at a particular index. The descriptor itself doesn't understand anything about indices.

So, when the descriptor has an extra "blinding key" which needs to be updated in tandem with the the rest of the keys, our generic-key model breaks down. I think the right way to handle this would be to link DescriptorPublicKey and DescriptorSecretKey somehow, so that we could say that CT view descriptors had a blinding key which was Pk::SecretKey or something rather than a concrete key type. Then for the specific case of DescriptorPublicKey, we could support deriving public and secret keys at the same time.

Does this make sense? Basically the issue is that we're mixing a generic keytype and a concrete one, and our API does not handle this gracefully.

sanket1729 · 2023-07-06T20:55:45Z

@apoelstra, I get the issues with DescriptorSecretKey. One of the main benefits of decsriptors is backing up and recovery.

I guess this is more a question of a spec. Perhaps, I am misunderstanding something here, if ct(xpub/*,wsh(ms)), would the backups not grow linearly instead of just a single copy?

apoelstra · 2023-07-07T12:47:28Z

@sanket1729 I don't understand what you're saying. The spec allows wildcards. The reference implementation just doesn't support it because I didn't want to do the rearchitecture work required for it.

delta1

2 minor comment suggestions

delta1 · 2023-08-08T09:47:01Z

src/confidential/slip77.rs

+impl MasterBlindingKey {
+    /// Compute a master blinding key from a seed
+    ///
+    /// The recommended in (SLIP-39) source of this seed is to obtain the


Suggested change

/// The recommended in (SLIP-39) source of this seed is to obtain the

/// The recommended source of this seed (in SLIP-39) is to obtain the

delta1 · 2023-08-08T09:57:47Z

src/confidential/mod.rs

+            test.check(&secp);
+        }
+        // Uncomment to regenerate test vectors; to see the output, run
+        // cargo test confidential::tests:;confidential_descriptor -- --nocapture


Suggested change

// cargo test confidential::tests:;confidential_descriptor -- --nocapture

// cargo test confidential::tests::confidential_descriptor -- --nocapture

apoelstra · 2023-08-23T17:19:46Z

Going to merge this; can address comment nits in a followup PR.

apoelstra force-pushed the 2022-10--ct-descriptors branch from 6b678bb to 76ca5be Compare October 6, 2022 18:53

sanket1729 reviewed Oct 7, 2022

View reviewed changes

apoelstra force-pushed the 2022-10--ct-descriptors branch from 7b35e15 to 47eee43 Compare January 6, 2023 15:36

apoelstra force-pushed the 2022-10--ct-descriptors branch 3 times, most recently from 440aaa6 to c3b356c Compare January 31, 2023 21:26

apoelstra marked this pull request as ready for review January 31, 2023 21:26

apoelstra added 11 commits June 23, 2023 00:17

descriptor: fix Display impl for legacy CSFS covenants

b11c818

descriptor: expose checksum function to whole crate

67e9422

delete descriptor::Blinded

5d2755d

CT descriptors: block in module

277b1a5

CT descriptors: add slip77 module

f5b2fe5

CT descriptors: add bare module

e9ac0bc

CT descriptors: add test vectors

604c7e3

CT descriptors: generate ELIP test vectors

2b5bd55

CT descriptors: add fuzz test

694628f

re-export confidential::Descirptor as ConfidentialDescirptor

a332208

apoelstra force-pushed the 2022-10--ct-descriptors branch from c3b356c to a332208 Compare June 23, 2023 00:18

apoelstra added 2 commits June 29, 2023 21:16

confidential: switch test vectors to use xpubs rather than bare keys

6359f03

confidential: hack in view descriptor support

725f450

Need to re-think the API for this; but sufficient to produce a test vector.

apoelstra mentioned this pull request Jun 30, 2023

new ELIP: Confidential Transaction descriptors ElementsProject/ELIPs#2

Merged

sanket1729 approved these changes Jul 2, 2023

View reviewed changes

delta1 reviewed Aug 8, 2023

View reviewed changes

sanket1729 approved these changes Aug 23, 2023 •

edited

Loading

View reviewed changes

apoelstra merged commit f887126 into ElementsProject:master Aug 23, 2023

apoelstra deleted the 2022-10--ct-descriptors branch August 23, 2023 17:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reference implementation of CT descriptors #31

Reference implementation of CT descriptors #31

apoelstra commented Oct 6, 2022

apoelstra commented Oct 6, 2022

sanket1729 left a comment

sanket1729 Oct 7, 2022

sanket1729 Oct 7, 2022

apoelstra Oct 7, 2022

sanket1729 Oct 7, 2022

sanket1729 Oct 7, 2022

apoelstra commented Oct 7, 2022

sanket1729 commented Oct 7, 2022

apoelstra commented Oct 18, 2022

sanket1729 commented Oct 18, 2022 •

edited

Loading

sanket1729 commented Jan 6, 2023

apoelstra commented Jan 6, 2023

apoelstra commented Jan 6, 2023

apoelstra commented Jan 31, 2023

LeoComandini commented Mar 15, 2023

apoelstra commented Mar 16, 2023

sanket1729 commented Jun 22, 2023

apoelstra commented Jun 23, 2023

apoelstra commented Jun 29, 2023

sanket1729 left a comment

sanket1729 Jul 1, 2023

apoelstra commented Jul 3, 2023

sanket1729 commented Jul 6, 2023 •

edited

Loading

apoelstra commented Jul 7, 2023

delta1 left a comment

delta1 Aug 8, 2023

delta1 Aug 8, 2023

apoelstra commented Aug 23, 2023

	/// The recommended in (SLIP-39) source of this seed is to obtain the
	/// The recommended source of this seed (in SLIP-39) is to obtain the

	// cargo test confidential::tests:;confidential_descriptor -- --nocapture
	// cargo test confidential::tests::confidential_descriptor -- --nocapture

Reference implementation of CT descriptors #31

Reference implementation of CT descriptors #31

Conversation

apoelstra commented Oct 6, 2022

apoelstra commented Oct 6, 2022

sanket1729 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

apoelstra commented Oct 7, 2022

sanket1729 commented Oct 7, 2022

apoelstra commented Oct 18, 2022

sanket1729 commented Oct 18, 2022 • edited Loading

sanket1729 commented Jan 6, 2023

apoelstra commented Jan 6, 2023

apoelstra commented Jan 6, 2023

apoelstra commented Jan 31, 2023

LeoComandini commented Mar 15, 2023

apoelstra commented Mar 16, 2023

sanket1729 commented Jun 22, 2023

apoelstra commented Jun 23, 2023

apoelstra commented Jun 29, 2023

sanket1729 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

apoelstra commented Jul 3, 2023

sanket1729 commented Jul 6, 2023 • edited Loading

apoelstra commented Jul 7, 2023

delta1 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

apoelstra commented Aug 23, 2023

sanket1729 commented Oct 18, 2022 •

edited

Loading

sanket1729 commented Jul 6, 2023 •

edited

Loading