Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use custom struct for Orchestrator - Node jsonrpc communication #26

Open
RedaOps opened this issue Jan 27, 2024 · 1 comment
Open

Use custom struct for Orchestrator - Node jsonrpc communication #26

RedaOps opened this issue Jan 27, 2024 · 1 comment

Comments

@RedaOps
Copy link
Contributor

RedaOps commented Jan 27, 2024

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
Copy link
Contributor

mimoo commented Jan 28, 2024

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 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants