feat(signer): Add Dirk support #206
Conversation
ltitanb
left a comment
ltitanb
left a comment
There was a problem hiding this comment.
Left a few comments but overall looks good! Would ideally have a few more logging on the signer side, also we could have a step by step guide on how to setup dirk for local development in the "Develop" section of the docs
| let channel = match connect(&host, &config.client_cert, &config.cert_auth).await { | ||
| Ok(channel) => channel, | ||
| Err(e) => { | ||
| warn!("Failed to connect to Dirk host {}: {e}", host.url); |
There was a problem hiding this comment.
should we error and exit here?
There was a problem hiding this comment.
With distributed wallets, you may have some down servers whilst still be able to reach the needed amount of signatures from working ones. It's true that if using simple accounts, that ones won't be available on down servers, but I think that's not the main use case.
Let me know what do you think
| } | ||
|
|
||
| /// Aggregate partial signatures into a master signature | ||
| fn aggregate_partial_signatures(partials: &[(BlsSignature, u32)]) -> eyre::Result<BlsSignature> { |
There was a problem hiding this comment.
is there a reference implementation / spec for this? we should also add unit tests
| let res = tokio::join!(signature, proxy_signature_bls, proxy_signature_ecdsa); | ||
| (res.0?, res.1?, res.2?) | ||
| let (signature, proxy_signature_bls) = { | ||
| let res = tokio::join!(signature, proxy_signature_bls); |
There was a problem hiding this comment.
let's add signature verification here?
Add support for Dirk remote signer by mapping requests received by the signer module to a Dirk gateway.
A new signer option (
Dirk) was added to theLocalandRemoteones. When this config is used, the signer module starts acting as an interface between Dirk and the client, in order to keep the API.As Dirk doesn't support ECDSA signatures, requests involving that schema return a HTTP 400 (Bad request) error. Also because of this, the
DA_COMMITexample module was slightly modified to be able to avoid ECDSA operations using the TOML config.It's worth to note that Dirk has a small bug where signing passing the public key instead of an account name is impossible. In order to test this (and later use it) changes on PR#75 on Dirk repo should be applied.
Close #17