feat: allow to override provider url upon installation #346
Conversation
| @@ -1,3 +1,4 @@ | |||
| use crate::memory::get_override_provider; | |||
| use crate::{ | |||
There was a problem hiding this comment.
nit: you could merge this import with the one below.
| evm_rpc_types::LogFilter::HidePattern(regex) => LogFilter::HidePattern(regex.into()), | ||
| } | ||
| evm_rpc_types::LogFilter::ShowPattern(regex) => { | ||
| LogFilter::ShowPattern(RegexString::try_from(regex)?) |
There was a problem hiding this comment.
nit: I guess this is really just a taste difference, but maybe .try_into()??
| match &self.override_url { | ||
| None => Ok(api), | ||
| Some(substitution) => { | ||
| let regex = substitution.pattern.compile()?; |
There was a problem hiding this comment.
Suggestion: Maybe I'm missing something here, but it seems we could cache the result of the pattern.compile() call by e.g. having the compiled pattern in RegexSubstitution instead of a RegexString since it's a relatively expensive operation. I guess this is currently only really relevant for local development since in prod the substitution is empty anyways, so it's not a big deal either way.
| } | ||
| } | ||
|
|
||
| fn arb_regex() -> impl Strategy<Value = RegexString> { |
There was a problem hiding this comment.
Suggestion: Would it make sense to try some actual valid regexes here since, statistically speaking, the probability that this is actually a regex with a valid pattern is quite low... I would even suggest using prop_oneof! and testing both cases of a purely random string and a somewhat random but well-formed pattern.
Another risk of course is that a random string might not be a well-formed regex and might lead to test flakiness...
|
Overall LGTM! Just have one or two minor comments/suggestions. |
Allow to override the URL that will be queried by the EVM RPC canister via a regex specified upon canister installation. This is allows testing against a local Ethereum development setup (such as foundry) without changing the client calling the EVM RPC canister.