Skip to content

chore: fix move of repository#1

Closed
gregorydemay wants to merge 34 commits intomainfrom
gdemay/XC-420-fix-move
Closed

chore: fix move of repository#1
gregorydemay wants to merge 34 commits intomainfrom
gdemay/XC-420-fix-move

Conversation

@gregorydemay
Copy link
Contributor

@gregorydemay gregorydemay commented Jul 9, 2025

The library canhttp that was initially in the EVM RPC canister repository was moved as is from commit 3995dbd to this repository as commit d41568a while preserving the Git history (using git filter-repo --subdirectory-filter canhttp as described here). Additionally, the Git history was rewritten to preserve the links to the original PRs with

git filter-repo --message-callback 'return re.sub(br"#(\d{3})", br"dfinity/evm-rpc-canister#\1", message)'

so that commit messages like this one that were pointed to PRs such as #364 are now pointing to dfinity/evm-rpc-canister#364.

This broke various things that this PR addresses:

  1. Symlinks for license and notice that were pointing to files outside the moved crate.
  2. Re-create a Cargo workspace.
  3. Add Github CI pipeline.

gregorydemay and others added 30 commits February 13, 2025 16:51
Starts extracting the logic to handle HTTPs outcalls into a separate
library, named `canhttp`, that temporarily lives in the same repository.
The main idea is to start from a low-level client implementing the
[`tower::Service`](https://docs.rs/tower/latest/tower/trait.Service.html)
trait that can be extended by additional middlewares. This PR focusses
on this low-level client and a first middleware handling cycles
accounting.

Left for future PRs:
1. Add a mapping layer so that requests are of type `http::Request<>`
and responses are of type `http::Response<>`. This will allow to be able
to use the `Request` builder from the
[`http`](https://crates.io/crates/http) crate as well as various
HTTP-specific middlewares such as the ones from the
[`tower-http`](https://crates.io/crates/tower-http) crate. In
particular, the
[`TraceLayer`](https://docs.rs/tower-http/latest/tower_http/trace/index.html)
will allow to react on requests/responses to add the appropriates
metrics and logs.
2. Add an additional mapping layer for JSON-RPC applications so that
requests are of type `http::Request<JsonRpcRequest<>>` and responses are
of type `http::Response<JsonRpcResponse<>>` to easily make JSON-RPC
requests.

---------

Co-authored-by: Louis Pahlavi <louis.pahlavi@gmail.com>
Follow-up on #364 to add a new observability layer to take care of
logging and metrics. Unfortunately,
[`tower_http::trace`](https://docs.rs/tower-http/latest/tower_http/trace/index.html)
cannot be used in a canister environment because it measures call
latency with `Instant::now`. The proposed layer, while inspired by
[`tower_http::trace`](https://docs.rs/tower-http/latest/tower_http/trace/index.html),
is also simpler because it does not have to deal with streaming
responses.

---------

Co-authored-by: Louis Pahlavi <louis.pahlavi@gmail.com>
Follow-up on #370 and #364 to add a conversion layer between request and
responses so that the caller only has to deal with types from the Rust
[`http`](https://crates.io/crates/http) crate. This has several
advantages from the caller's point of view:

1. Can re-use existing types like `Request::builder` or `StatusCode`.
2. Requests are automatically sanitized and canonicalized (e.g. header
names are validated and lower cased).
3. Can re-use existing middlewares, like from the
`[tower-http`](https://crates.io/crates/tower-http) crate, since many of
them require a client that uses the
[`http`](https://crates.io/crates/http) crate.
Follow-up on #370, #364, and #374 to add a conversion layer between
request and responses to handle [JSON
RPC](https://www.jsonrpc.org/specification) over HTTP.

Concretely:
1. Refactor existing infrastructure to handle errors explicitly, instead
of having to downcast `BoxError`. This is beneficial since errors may
happen at different layers and they all need to be handled by the above
observability layer, e.g., add the corresponding metric in case the
response has a non successful status.
2. Refactor existing infrastructure to transform request and response to
use a single `Convert` trait to convert one type into another with some
potential error.
3. Add a layer for `http::Request` and `http::Response` to transform
their body into a JSON RPC type. This uses the above `Convert` trait.
This conversion layer is behind a feature flag (`json`) since it
requires `serde_json` to handle the serialization/deserialization
4. Add a new layer to filter out non-successful HTTP responses.
Follow-up on #370, #364, #374, and #375 to automatically retry requests
by doubling its `max_response_bytes` when the response was too big.
…specification (#386)

Refactor the JSON-RPC request and response types to follow the
[specification](https://www.jsonrpc.org/specification) more closely:

1. `jsonrpc` must have the value `"2.0"`.
2. `id` must be a String, a number or a null value.

Left for a future PR: it should be ensured that the response ID matches
the one from the request. This will be added as another tower
middleware.
Ensure that the ID in a JSON-RPC response matches the ID of the JSON-RPC
request that lead to that response as specified in the JSON-RPC
[specification](https://www.jsonrpc.org/specification).

Most of the changes in that PR are due to updating the integration tests
which were not following the JSON-RPC specification.
Use a `tower::Service` to issue multiple requests in parallel which is
used to query multiple JSON-RPC providers and avoid a single point of
failure.

Remarks:

1. The type `MultiCallResults` was renamed to `MultiResults` and moved
to `canhttp::multi`. The new type is a genralization of the previous one
to deal with arbitrary keys, values and errors.
2. The logic for reduction (by equality or threshold) was also moved to
`canhttp::multi`.
3. The corresponding unit tests were also moved to `canhttp::multi`.
Add a `map` method to `canhttp::http::json::response::JsonRpcResponse`.
Make `JsonRpcResponse` map able to return a different type instead of
being restricted to the original result type.
Add a wrapper around a JSON-RPC request ID to ensure that the amount of
bytes used when serialized to JSON is constant.

This is necessary to ensure that two HTTPs outcalls for two JSON-RPC
requests differing only by their IDs will have an identical cycle cost.
See also dfinity/sol-rpc-canister#62.
1. Update Rust toolchain to 1.87.0
2. Update Pocket IC to 9.0.1
3. Update all libraries via `cargo update`, excepted for `ethnum` which
introduced a breaking change between 1.5.0 and 1.5.1 (see
nlordell/ethnum-rs#50)
Update changelogs and bump version in preparation to the v2.4.0 release.
Follow-up on #422 to add missing the `repository` field in
`canhttp/Cargo.toml`.
Remove the `ic_cdk` as a dependency of `evm_rpc_types` to avoid callers
of that library to have a transitive dependency on the `ic_cdk`. This
will help in callers upgrading to `ic_cdk > 0.17` which is incompatible
with previous versions and was the main motivation for #416.

**BREAKING CHANGE**: although the interface of the EVM RPC canister did
**not** change, this PR introduces breaking changes for Rust clients of
the following libraries:
1. `canhttp`: the `IcError` type now uses `ic_error_types::RejectCode`
instead of `ic_cdk::api::call::RejectionCode`
2. `evm_rpc_types`: the variant `HttpOutcallError::IcError` now uses a
local copy of `ic_cdk::api::call::RejectionCode`, renamed to
`LegacyRejectionCode`. The Candid serialization/deserialization of that
type should remain identical.
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 <louis.pahlavi@gmail.com>
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)
@gregorydemay gregorydemay marked this pull request as ready for review July 9, 2025 14:38
@gregorydemay gregorydemay requested a review from lpahlavi July 9, 2025 14:40
Copy link
Contributor

@lpahlavi lpahlavi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for creating the repository @gregorydemay, I think it's much cleaner this way to have canhttp completely decoupled from the EVM RPC canister! LGTM!

env:
RUSTDOCFLAGS: "--deny warnings"

unit-tests:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is out of the scope of this PR, but it would be good to add some integration tests eventually. The most logical thing to do is probably to create a simple testing canister for this purpose.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes totally! In particular we want to always make sure that the code can be used in a canister. I created XC-427 to track this.

@gregorydemay
Copy link
Contributor Author

Was automatically closed by Github due to the force pushed on main to rewrite commit messages to point to the original PRs. Rebased as #2.

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

Successfully merging this pull request may close these issues.

3 participants