Introduce a SignatoryManager service. #509

crodas · 2024-12-20T00:29:12Z

Description

The SignatoryManager manager provides an API to interact with keysets, private keys, and all key-related operations, offering segregation between the mint and the most sensible part of the mind: the private keys.

Although the default signatory runs in memory, it is completely isolated from the rest of the system and can only be communicated through the interface offered by the signatory manager. Only messages can be sent from the mintd to the Signatory trait through the Signatory Manager.

This pull request sets the foundation for eventually being able to run the Signatory and all the key-related operations in a separate service, possibly in a foreign service, to offload risks, as described in #476.

The Signatory manager is concurrent and deferred any mechanism needed to handle concurrency to the Signatory trait.

TODO

Add standalone binary to spawn a GRPC server inside cdk-signatory
Add GRPC client on cdk
Add tests
Add cdk-signatory to the pipeline

Notes to the reviewers

Maybe a new terminology, other than Signatory and SignatoryManager can be used, but the idea I believe is solid.

Suggested CHANGELOG Updates

CHANGED

ADDED

REMOVED

FIXED

Checklist

I followed the code style guidelines
I ran just final-check before committing

callebtc · 2024-12-23T09:11:29Z

crates/cdk/src/mint/signatory/memory.rs

+        if let Ok(secret) =
+            <&crate::secret::Secret as TryInto<crate::nuts::nut10::Secret>>::try_into(&proof.secret)
+        {
+            // Checks and verifes known secret kinds.
+            // If it is an unknown secret kind it will be treated as a normal secret.
+            // Spending conditions will **not** be check. It is up to the wallet to ensure
+            // only supported secret kinds are used as there is no way for the mint to
+            // enforce only signing supported secrets as they are blinded at
+            // that point.
+            match secret.kind {
+                Kind::P2PK => {
+                    proof.verify_p2pk()?;
+                }
+                Kind::HTLC => {
+                    proof.verify_htlc()?;
+                }
+            }


Functionality like this should happen in the core mint instead of here. The message should be sent to the blind signer only if all checks are successful, including scripts etc.

callebtc · 2024-12-23T09:12:34Z

crates/cdk/src/mint/signatory/memory.rs

+                    derivation_path,
+                    Some(0),
+                    unit.clone(),
+                    max_order,


Possibly out of scope but it would be nice if this parameter would accept a slice of integers (all possible amounts) instead of a single exponent (max order).

crodas · 2025-01-21T03:47:59Z

@thesimplekid @callebtc Does it look better?

Basically so far I have:

Move the signatory to their own repo. The same code can be embedded or run in a its own binary and connect through an URL
Regardless of embedded or remote, the signatory works in their own tokio thread, and they communicate with the mintd through messages
A lot of code that can be merged before in its own PR, which aims to have fewer dependency and reduce the duplicate code, such as the Database instantiation

If this makes sense, I'll do the preparations (like preparing 3), fix merging issues, and add some comments. 3 was only required if the signatory binary is not in the same crate as mintd, but it makes sense to perhaps remove it for now and revisit it later #550, only rebasing is missing if it makes sense.

thesimplekid

Move the signatory to their own repo.

You mean crate right?

This is a really good start and a structure that makes sense. I think there are still some open questions around what should be handled by the signatory and what by the mint. For example the proof verification I don't think the checking of p2pk and htlc conditions should be done by the signatory and it should only handle stuff that actually required the mint private keys. However, we do want to provide enough information that the signatory can be more then just a blind signer, doing ie rate limiting and we don't have a strong picture of what that will look like yet.

thesimplekid · 2025-01-21T10:42:19Z

crates/cdk-signatory/src/lib.rs

+        Ok(blinded_signature)
+    }
+
+    async fn verify_proof(&self, proof: Proof) -> Result<(), Error> {


I don't think the full verification of the proof should be in the signatory only the verification of the signature.

thesimplekid · 2025-01-21T10:43:54Z

crates/cdk-signatory/Cargo.toml

+tonic = "0.12.3"
+prost = "0.13.4"


These break MSRV, I'll figure that out on the management-rpc branch and report back

I'll fix this, making this optional (only to compile the binary)

I this fixed now @thesimplekid ?

crates/cdk-mintd/src/config.rs

The SignatoryManager manager provides an API to interact with keysets, private keys, and all key-related operations, offering segregation between the mint and the most sensible part of the mind: the private keys. Although the default signatory runs in memory, it is completely isolated from the rest of the system and can only be communicated through the interface offered by the signatory manager. Only messages can be sent from the mintd to the Signatory trait through the Signatory Manager. This pull request sets the foundation for eventually being able to run the Signatory and all the key-related operations in a separate service, possibly in a foreign service, to offload risks, as described in cashubtc#476. The Signatory manager is concurrent and deferred any mechanism needed to handle concurrency to the Signatory trait.

callebtc reviewed Dec 23, 2024

View reviewed changes

crodas mentioned this pull request Dec 31, 2024

Introduce cashu to host the shared types, traits and other common code #519

Merged

2 tasks

crodas force-pushed the proto/independent-signatory branch 2 times, most recently from 45d8352 to a11988b Compare January 14, 2025 02:35

crodas mentioned this pull request Jan 14, 2025

Possibly out of scope but it would be nice if this parameter would accept a slice of integers (all possible amounts) instead of a single exponent (max order). #533

Open

crodas force-pushed the proto/independent-signatory branch from a11988b to a8c289e Compare January 14, 2025 02:41

crodas mentioned this pull request Jan 14, 2025

Functionality like this should happen in the core mint instead of here. The message should be sent to the blind signer only if all checks are successful, including scripts etc. #534

Open

crodas mentioned this pull request Jan 21, 2025

Introduce cdk-database #550

Closed

2 tasks

thesimplekid reviewed Jan 21, 2025

View reviewed changes

crodas force-pushed the proto/independent-signatory branch from b8e13e4 to 77ead1b Compare January 21, 2025 16:23

thesimplekid added enhancement New feature or request mint labels Jan 22, 2025

crodas force-pushed the proto/independent-signatory branch 2 times, most recently from 6178232 to 5c71573 Compare January 23, 2025 16:13

crodas force-pushed the proto/independent-signatory branch 4 times, most recently from c9197b3 to 7fe0c8e Compare February 2, 2025 23:40

crodas mentioned this pull request Feb 2, 2025

make Keyset Id's member variables public #564

Open

crodas force-pushed the proto/independent-signatory branch 10 times, most recently from 55713c6 to e06ec34 Compare February 5, 2025 02:04

crodas force-pushed the proto/independent-signatory branch 3 times, most recently from c052ba1 to a8fbfde Compare February 5, 2025 12:46

crodas force-pushed the proto/independent-signatory branch from a8fbfde to 43227d3 Compare February 5, 2025 13:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce a SignatoryManager service. #509

Introduce a SignatoryManager service. #509

crodas commented Dec 20, 2024 •

edited

Loading

callebtc Dec 23, 2024

callebtc Dec 23, 2024

crodas commented Jan 21, 2025 •

edited

Loading

thesimplekid left a comment

thesimplekid Jan 21, 2025

thesimplekid Jan 21, 2025

crodas Jan 21, 2025

crodas Jan 21, 2025

		tonic = "0.12.3"
		prost = "0.13.4"

Introduce a SignatoryManager service. #509

Are you sure you want to change the base?

Introduce a SignatoryManager service. #509

Conversation

crodas commented Dec 20, 2024 • edited Loading

Description

TODO

Notes to the reviewers

Suggested CHANGELOG Updates

CHANGED

ADDED

REMOVED

FIXED

Checklist

callebtc Dec 23, 2024

Choose a reason for hiding this comment

callebtc Dec 23, 2024

Choose a reason for hiding this comment

crodas commented Jan 21, 2025 • edited Loading

thesimplekid left a comment

Choose a reason for hiding this comment

thesimplekid Jan 21, 2025

Choose a reason for hiding this comment

thesimplekid Jan 21, 2025

Choose a reason for hiding this comment

crodas Jan 21, 2025

Choose a reason for hiding this comment

crodas Jan 21, 2025

Choose a reason for hiding this comment

crodas commented Dec 20, 2024 •

edited

Loading

crodas commented Jan 21, 2025 •

edited

Loading