sigma0-xyz / zkbitcoin

zkBitcoin: zero-knowledge proofs on Bitcoin!
MIT License
161 stars 31 forks source link

Use custom struct for Orchestrator - Node jsonrpc communication #26

Open RedaOps opened 6 months ago

RedaOps commented 6 months ago

Right now, the Orchestrator uses bitcoincore_rpc::jsonrpc::Response struct for deserialising responses from MPC nodes.

#[derive(Debug, Clone, Deserialize, Serialize)]
/// A JSONRPC response object.
pub struct Response {
    /// A result if there is one, or [`None`].
    pub result: Option<Box<RawValue>>,
    /// An error if there is one, or [`None`].
    pub error: Option<error::RpcError>,
    /// Identifier for this Request, which should match that of the request.
    pub id: serde_json::Value,
    /// jsonrpc field, MUST be "2.0".
    pub jsonrpc: Option<String>,
}

This is working as expected, since the structure follows the standardised jsonrpc response, and nodes send replies using jsonrpsee_core::RpcResult. However, we should use the same package/structure on both sides.

On top of that, I suggest using the jsonrpsee client for jsonrpc requests, instead of the reqwest library. (examples)

I suggest using jsonrpsee's Response struct for deserialization in the following places:

https://github.com/sigma0-xyz/zkbitcoin/blob/601b6a4ad94db02dff358c2f7baf92912c78e687/src/committee/orchestrator.rs#L174-L185

https://github.com/sigma0-xyz/zkbitcoin/blob/601b6a4ad94db02dff358c2f7baf92912c78e687/src/committee/orchestrator.rs#L344-L356

https://github.com/sigma0-xyz/zkbitcoin/blob/601b6a4ad94db02dff358c2f7baf92912c78e687/src/committee/orchestrator.rs#L407-L420

mimoo commented 6 months ago

I can't remember why I didn't use jsonrpsee, I think I couldn't set some timeout or I had some issue customizing something... but in general if we can replace the code we have with library functions I will welcome a PR :)