Implement a common aggregator client - Phase 1

[jpraynaud]

Why

We have some code that is duplicated for the aggregator client in all the crates that need to communicate with an aggregator (aggregator, signer, client). We want to remove the duplicated code for easier maintenance.

What

Create a new internal crate for supporting the communications with an aggregator and publish it.

How

• Create a new internal mithril-aggregator-client crate
• Abstract the requests to be executed by the client with a trait instead of the existing enums (as what has been done in the Query trait)
• Define new queries for all existing request on the duplicated clients (not for the Mithril client library yet)
• Deprecate the with_aggregator_client function in client builder

[Alenar]

Existing features across clients

Feature	Mithril-client (lib)	Mithril-Aggregator	Mithril-Signer
custom header	✔️	❌	❌
proxy	❌	✔️	✔️
aggregator url sanitation	✔️	✔️	✔️
timeout	❌	✔️	✔️
api version mismatch warn	✔️	✔️	✔️
custom 550 error ¹	❌	✔️	✔️
error root cause analysis	❌	✔️	✔️

¹ registration round not opened error

Specific behavior to keep

• Aggregator: set a custom aggregator-node-version with value env!("CARGO_PKG_VERSION")
• Signer: set a custom signer-node-version with value env!("CARGO_PKG_VERSION")

Note

The custom node version for the aggregator and signer can be handled with by enabling to set additional headers to the shared aggregator client (probably in the builder AggregatorClient::builder().additional_header("signer-node-version", env!("CARGO_PKG_VERSION")).build()).

[Alenar]

Mithril-client lib case

The mithril-client library use a api that's not straight compatible with the one in the signer and the aggregator. Here are the differences:

• The signer and aggregator both defines a AggregatorClient trait with one method per route that can be called (i.e. retrieve_epoch_settings, register_signer)
  • the de-serialization of the response body is done in the client, the caller get type answer
  • if a new endpoint is needed another method is added to the trait
• The client library define a AggregatorClient which have two methods that try to handle all cases: get_content, post_content
  • they returns the response body string, it's the caller that handle the de-serialization
  • if a new endpoint is needed an entry is added to the public AggregatorRequest enum

This is an issue since the proposed AggregatorClient api returns typed answer, de-serialized at the client level like the aggregator and the signer.
Implementing the current mithril_client::aggregator_client::AggregatorClient trait on the new api would be inefficient as this would basically requires to ... add a compatibility layer that convert to json strings the answer from the new api so the client caller can de-serialize them as usual.

Proposals

1 - Breaking refactoring of the mithril-client

Refactor the mithril-client AggregatorClient trait following the same pattern than in mithril-aggregator in mithril-signer, with a method for each endpoint that needs to be called.

Pros: unified patterns with the other crates, typed api, best performance (direct de-serialization without intermediate string)
Cons: breaking change

Tip

We could define multiples traits instead of one, scoped by entity types, but this would pose some issue in the client builder

2 - Inefficient compatibility layer

Implement the current mithril-client AggregatorClient trait and convert all shared client response to json strings.

Pros: no breaking change
Cons: performance cost with an conversion from the response byte to a target type then to a string then to the target type again.

Note

This could be a first step before going to the first proposal

3 - keeping mithril-client as is

Don't do anything in mithril-client, this means keeping some duplicated code with the client used by the signer and aggregator.

Pros: no breaking change
Cons: the Aggregator client code remains partially duplicated