Introduce a SignatoryManager service.

[crodas]

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]

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]

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]

@thesimplekid @callebtc Does it look better?

Basically so far I have:

1. 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
2. Regardless of embedded or remote, the signatory works in their own tokio thread, and they communicate with the mintd through messages
3. 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]

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

[davidcaseria]

Should the functions also take a batch of messages/proofs to limit the amount of network calls?

[thesimplekid]

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

[crodas]

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

[crodas]

I this fixed now @thesimplekid ?

[ok300]

Suggested change

	#[cfg(feature = "grpc")]
	println!("cargo:rerun-if-changed=src/proto/signatory.proto");
	#[cfg(feature = "grpc")]

This will make sure cargo only recompiles when the proto file changes.

[crodas]

Thank you! I'll add it shortly, I'm rebasing first.

[thesimplekid]

What do you think about this being a binary within the cdk-signatory crate?

[thesimplekid]

can we week serde at 1? I know we changed uuid bc of the wasm build issue

[thesimplekid]

these should all be version 0.7.1

[thesimplekid]

maybe instead of a verify_proof fn we have a verify signature fn to make it clear we only verify the signature on the proof.

[thesimplekid]

What do you think if implementing start and stop on MwmorySignatory instead of this fn

cdk/crates/cdk-mint-rpc/src/proto/server.rs

Lines 50 to 120 in e1458b0

	impl MintRPCServer {
	pub fn new(addr: &str, port: u16, mint: Arc<Mint>) -> Result<Self, Error> {
	Ok(Self {
	socket_addr: format!("{addr}:{port}").parse()?,
	mint,
	shutdown: Arc::new(Notify::new()),
	handle: None,
	})
	}
	pub async fn start(&mut self, tls_dir: Option<PathBuf>) -> Result<(), Error> {
	tracing::info!("Starting RPC server {}", self.socket_addr);
	let server = match tls_dir {
	Some(tls_dir) => {
	tracing::info!("TLS configuration found, starting secure server");
	let cert = std::fs::read_to_string(tls_dir.join("server.pem"))?;
	let key = std::fs::read_to_string(tls_dir.join("server.key"))?;
	let client_ca_cert = std::fs::read_to_string(tls_dir.join("ca.pem"))?;
	let client_ca_cert = Certificate::from_pem(client_ca_cert);
	let server_identity = Identity::from_pem(cert, key);
	let tls_config = ServerTlsConfig::new()
	.identity(server_identity)
	.client_ca_root(client_ca_cert);
	Server::builder()
	.tls_config(tls_config)?
	.add_service(CdkMintServer::new(self.clone()))
	}
	None => {
	tracing::warn!("No valid TLS configuration found, starting insecure server");
	Server::builder().add_service(CdkMintServer::new(self.clone()))
	}
	};
	let shutdown = self.shutdown.clone();
	let addr = self.socket_addr;
	self.handle = Some(Arc::new(tokio::spawn(async move {
	let server = server.serve_with_shutdown(addr, async {
	shutdown.notified().await;
	});
	server.await?;
	Ok(())
	})));
	Ok(())
	}
	pub async fn stop(&self) -> Result<(), Error> {
	self.shutdown.notify_one();
	if let Some(handle) = &self.handle {
	while !handle.is_finished() {
	tracing::info!("Waitning for mint rpc server to stop");
	tokio::time::sleep(Duration::from_millis(100)).await;
	}
	}
	tracing::info!("Mint rpc server stopped");
	Ok(())
	}
	}
	impl Drop for MintRPCServer {
	fn drop(&mut self) {
	tracing::debug!("Dropping mint rpc server");
	self.shutdown.notify_one();
	}
	}

[thesimplekid]

We still want to commit to MSRV with grpc enabled. The required dep version on used the cdk-mint-rpc. I think you are using them maybe we just remove the comment.

[thesimplekid]

This should be moved up under features?

[thesimplekid]

Why do we need a with_signatory and with_remote_signatory can't we just use with_signatory and its something that implements the trait and we don't care what?

[crodas]

Good point, I'll refactor this.

[thesimplekid]

Do we need a grpc feature? If the signatory is just something that impl the Signatory trait does cdk need to know if its rpc or not?

[crodas]

It was all due the MSRV, I'll revisit before having this PR ready for review

[crodas]

After addressing the MSRV, (as you described in #509 (comment)) I will drop the GRPC feature

[thesimplekid]

I think this just got put back in on a rebase we shouldn't need it now that this info is stored in db.