-
Notifications
You must be signed in to change notification settings - Fork 20
refactor: remove default/non-default providers #436
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
Conversation
lpahlavi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR @gregorydemay, LGTM!
ninegua
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks
gregorydemay
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Introduce a new data structure `TimedSizedVec<T>` to store a limited number of values, evicting older and expired entries. This is similar to [`TimedSizedCache`](https://docs.rs/cached/0.55.1/cached/stores/struct.TimedSizedCache.html) except that it's time agnostic to ensure that it works in a canister (no `Instant::now`). In a later PR, this data structure will be used to register timestamps of JSON-RPC ok responses from each supported provider. This is the second step towards dynamically ranking supported providers according to the number of their successful responses, which is done in the following 3 PRs: 1. #436 2. #434 (this one) 3. #435 --------- Co-authored-by: Louis Pahlavi <[email protected]>
Dynamically select supported JSON-RPC providers according to the number of successful JSON-RPC responses in the last 20 minutes, where providers with a higher number will be prioritized over ones with lower numbers. This is a heuristic that aims at selecting only currently functioning providers. The idea is that since JSON-RPC errors cannot be relied upon, as they are not specified by the Ethereum JSON-RPC API and so different EVM clients or JSON-RPC providers do use different errors, we only count instead the number of successful responses. Obviously, this is still not bulletproof, since it won't help in case the provider returns a successful JSON-RPC response with incorrect data. Looking back at previous incidents with Ethereum JSON-RPC providers for the EVM RPC canister or the ckETH/ckERC20 minter, we can see that having this heuristic would have helped in most cases, excepted when a provider returns successful responses with incorrect data (1 out of 4 incidents): 1. 😌 `Cloudflare` always return an error (proposal [136701](https://dashboard.internetcomputer.org/proposal/136701)) 2. 😌 `LlamaNodes` is down (proposal [132474](https://dashboard.internetcomputer.org/proposal/132474)) 3. 😌 `Ankr` dropped IPv6 connectivity ([132415](https://dashboard.internetcomputer.org/proposal/132415) 4. 😱`Cloudflare` returns successful responses with wrong results (proposal [128365](https://dashboard.internetcomputer.org/proposal/128365)) This is the last step towards dynamically ranking supported providers according to the number of their successful responses, which is done in the following 3 PRs: 1. #436 5. #434 6. #435 (this one)
…ster#434) Introduce a new data structure `TimedSizedVec<T>` to store a limited number of values, evicting older and expired entries. This is similar to [`TimedSizedCache`](https://docs.rs/cached/0.55.1/cached/stores/struct.TimedSizedCache.html) except that it's time agnostic to ensure that it works in a canister (no `Instant::now`). In a later PR, this data structure will be used to register timestamps of JSON-RPC ok responses from each supported provider. This is the second step towards dynamically ranking supported providers according to the number of their successful responses, which is done in the following 3 PRs: 1. dfinity/evm-rpc-canister#436 2. dfinity/evm-rpc-canister#434 (this one) 3. dfinity/evm-rpc-canister#435 --------- Co-authored-by: Louis Pahlavi <[email protected]>
…ty/evm-rpc-canister#435) Dynamically select supported JSON-RPC providers according to the number of successful JSON-RPC responses in the last 20 minutes, where providers with a higher number will be prioritized over ones with lower numbers. This is a heuristic that aims at selecting only currently functioning providers. The idea is that since JSON-RPC errors cannot be relied upon, as they are not specified by the Ethereum JSON-RPC API and so different EVM clients or JSON-RPC providers do use different errors, we only count instead the number of successful responses. Obviously, this is still not bulletproof, since it won't help in case the provider returns a successful JSON-RPC response with incorrect data. Looking back at previous incidents with Ethereum JSON-RPC providers for the EVM RPC canister or the ckETH/ckERC20 minter, we can see that having this heuristic would have helped in most cases, excepted when a provider returns successful responses with incorrect data (1 out of 4 incidents): 1. 😌 `Cloudflare` always return an error (proposal [136701](https://dashboard.internetcomputer.org/proposal/136701)) 2. 😌 `LlamaNodes` is down (proposal [132474](https://dashboard.internetcomputer.org/proposal/132474)) 3. 😌 `Ankr` dropped IPv6 connectivity ([132415](https://dashboard.internetcomputer.org/proposal/132415) 4. 😱`Cloudflare` returns successful responses with wrong results (proposal [128365](https://dashboard.internetcomputer.org/proposal/128365)) This is the last step towards dynamically ranking supported providers according to the number of their successful responses, which is done in the following 3 PRs: 1. dfinity/evm-rpc-canister#436 5. dfinity/evm-rpc-canister#434 6. dfinity/evm-rpc-canister#435 (this one)
The notion of default/non-default providers was only useful when Equality was chosen as consensus strategy and no providers was explicitly specified, in which case the "default" providers would be selected. This PR removes the notion of default/non-default providers by replacing with an ordered list of providers for a given EVM chain, corresponding to the supported providers. The previous notion of "default" providers simply correspond to the first three providers in that list.
This is similar to the refactoring done in dfinity/sol-rpc-canister#122.
Differentiating between supported and custom providers is the first step towards dynamically ranking supported providers according to the number of their successful responses, which is done in the following 3 PRs: