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

feat(ethers-contract): Make ethers-providers optional #2536

Merged
merged 6 commits into from Aug 18, 2023

Conversation

oblique
Copy link
Contributor

@oblique oblique commented Aug 5, 2023

Motivation

I'm working in a very restricted WASM environment (see #2532), which I can not use ethers-providers crate. However I would like to use ethers_contract::EthCall derive functions.

Solution

With this PR, I introduced the providers flag in ethers-contract, ethers-contract-abigen, and ethers-contract-derive. Anything related to ethers-providers is now enabled with the flag.

providers flag was added in the default features of ethers-contract.

If a user adds ethers or ethers-middleware crates, then providers is always enabled, even if user disabled the default flags. This is because they have ethers-providers as a dependency and there is no reason to make the flag optional.

If a user adds ethers-contract and disables default features, then they need to add providers feature to get the same functionality as before. - Is this edge case a major breaking change?

If you have any other ideas how this can be better, let me know.

PR Checklist

  • Added Tests
  • Added Documentation
  • Breaking changes

Copy link
Contributor Author

@oblique oblique Aug 5, 2023

Choose a reason for hiding this comment

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

Content from ethers-contract/src/multicall/mod.rs was moved here. Nothing else was changed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Content from ethers-contract/src/event.rs was moved here. Nothing else was changed.

@oblique
Copy link
Contributor Author

oblique commented Aug 5, 2023

The tests that are failing are not related to my changes and they failing before the changes on my machine.

@oblique
Copy link
Contributor Author

oblique commented Aug 5, 2023

I found that foundry was responsible for the test failures. The tests are passing with version nightly-e05b9c75b4501d5880764948b61db787f3dd7fe0 and they fail with version nightly-2d87c0c2fcc47088feecc72721c46d8e07e3c220

@oblique
Copy link
Contributor Author

oblique commented Aug 7, 2023

With the latest foundry nightly the tests passed, so this PR is ready. Any comments are welcome.

Copy link
Collaborator

@DaniPopes DaniPopes left a comment

Choose a reason for hiding this comment

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

This is great! Just a few things

ethers-contract/Cargo.toml Show resolved Hide resolved
@oblique oblique requested a review from DaniPopes August 18, 2023 07:57
@DaniPopes DaniPopes merged commit 179891d into gakonst:master Aug 18, 2023
19 checks passed
zvolin added a commit to zvolin/ethereum-canister that referenced this pull request Aug 18, 2023
oblique pushed a commit to eigerco/ethereum-canister that referenced this pull request Aug 21, 2023
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.

None yet

3 participants