Skip to content
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

Support Alchemy in get_logs #180

Merged
merged 6 commits into from
Mar 9, 2022
Merged

Support Alchemy in get_logs #180

merged 6 commits into from
Mar 9, 2022

Conversation

Mirko-von-Leipzig
Copy link
Contributor

@Mirko-von-Leipzig Mirko-von-Leipzig commented Mar 8, 2022

This PR adds support for the Alchemy Ethereum endpoint.

Background

Querying for Ethereum logs can result in a filter range which is too large i.e. the query would return too many values. Generally, the Ethereum endpoint returns the LimitExceeded RPC error, however it turns out the Alchemy uses InvalidParams to indicate this instead.

In addition, Alchemy also throws custom errors when

  • querying a block range that exceeds the current head of chain, and
  • a query exceeds the time limit

This PR includes supporting those.

Other included work

Our CI currently only run Ethereum tests against the Infura endpoint specified in our Github secrets. I've added secrets for Alchemy and renamed the secrets to be less general. The CI now runs the Ethereum tests twice, once each for Infura and Alchemy endpoints.

Other comments

Overall, this Ethereum "interface" could use a larger refactor including a CI overhaul to make testing these specific endpoints more obvious. And adding dependency injection for things like get_logs and other related, more complicated things so that we can test scenarios without relying on an actual Ethereum endpoint directly.

As part of this overhaul we should also somehow test / imitate other likely Ethereum node types directly if possible.

Closes #42.

From `ethereum::tests` to `ethereum`.
Alchemy uses a different error format.
Currently supports Infura and Alchemy services.
Alchemy gives custom server errors for:
- query exceeded timeout, and
- query block range includes an unknown block number
@Mirko-von-Leipzig Mirko-von-Leipzig marked this pull request as ready for review March 8, 2022 10:23
@Mirko-von-Leipzig Mirko-von-Leipzig requested review from CHr15F0x, koivunej and kkovaacs and removed request for koivunej March 8, 2022 10:23
Copy link
Member

@CHr15F0x CHr15F0x left a comment

Choose a reason for hiding this comment

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

LGTM 👍 ,maybe add some more trace!s?

Err(web3::Error::Rpc(err)) if err.code.code() == RpcErrorCode::LimitExceeded.code() => {
return Err(GetLogsError::QueryLimit)
Err(Rpc(err)) if err.code.code() == LimitExceeded.code() => {
return Err(GetLogsError::QueryLimit);
Copy link
Member

Choose a reason for hiding this comment

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

Maybe it would be useful to trace! when we hit the limit on infura & alchemy too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wasn't sure since its a valid, expected response. And it does occur quite regularly. If anything I guess one would add these traces in the calling function?

Copy link
Member

Choose a reason for hiding this comment

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

Oh,if it happens often then yes, definitely.

@Mirko-von-Leipzig Mirko-von-Leipzig merged commit d0ca729 into main Mar 9, 2022
@Mirko-von-Leipzig Mirko-von-Leipzig deleted the alchemy_get_logs branch March 9, 2022 07:50
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.

Add alchemy CI as ethereum endpoint target
2 participants