feat: add retry strategy to client

[lpahlavi]

Add the option of configuring a retry strategy to the EvmRpcClient in order to double the amount of cycles attached to a call until it succeeds.

[lpahlavi]

The Clone bounds are needed here to be able to clone the request when retrying.

[lpahlavi]

I hesitated here to make this a trait instead of an enum, so users can provide their own retry implementation. In the end, I felt like it might have been over-engineered though and an enum might be sufficient. WDYT?

[gregorydemay]

One problem I see is that if we need some additional trait bounds for a new variant in EvmRpcRetryStrategy, then this will trickle down to all methods, such as try_execute_request, even if the caller would not use this particular variant. For that reason, my impression is that having a trait here would be useful. Something like in the spirit of tower::retry::Policy. One benefit is that the trait EvmRpcResult would also I think no longer be necessary, because the bound would be on the strategy implementing RetryPolicy. Example

pub trait RetryPolicy<Config, Params, CandidOutput, Output> {
    fn retry(
        &mut self, //can mutate self in case the strategy is stateful
        request: Request<Config, Params, CandidOutput, Output>, //take ownership, no separate_method `clone_request
        result: &mut Result<Output, IcError>, //strategy can decide to change result
    ) -> Option<Request<Config, Params, CandidOutput, Output>>;
}

WDYT?

[lpahlavi]

I actually originally had something very similar, and then moved to an enum... But I think you make some good points and in the end the more complicated solution is indeed probably better.

Note that I did add a clone_request method, otherwise, even with a NoRetry policy, requests would still need to be cloneable. This means we effectively have something extremely similar to tower::retry::Policy, and so I'm wondering if in the end it might not make more sense to just use that? We could use tower for the whole thing which would just require adding a 'static bound on the Config and Params types. WDYT?

[gregorydemay]

Thanks for the changes, the trait looks very good to me!

and so I'm wondering if in the end it might not make more sense to just use that?

For now I would not, for the following reasons:

1. less dependencies
2. simpler, because the current retry policy does not need to be async.

[gregorydemay]

Thanks for the refactoring @lpahlavi ! Some minor comments but otherwise looks very good to me!

[gregorydemay]

Thanks for the changes, the trait looks very good to me!

and so I'm wondering if in the end it might not make more sense to just use that?

For now I would not, for the following reasons:

1. less dependencies
2. simpler, because the current retry policy does not need to be async.

[lpahlavi]

Thanks a lot for the review and input @gregorydemay! This should be ready for another round.