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

Use Polygon gas oracle #2965

Merged
merged 9 commits into from
Nov 23, 2023
Merged

Use Polygon gas oracle #2965

merged 9 commits into from
Nov 23, 2023

Conversation

tkporter
Copy link
Collaborator

@tkporter tkporter commented Nov 22, 2023

Description

After investigating #2959, I found the following

this is a known problem with Polygon, explanation here ethers-io/ethers.js#2828 (comment)

Fun fact Asa looked into this once #771

Here's a discussion it in Foundry, which I found hoping that ethers-rs folks had ran into this before
foundry-rs/foundry#1703

Foundry fixed this by using the Polygon gas station oracle which seems to be recommended path https://github.com/foundry-rs/foundry/pull/3368/files#diff-c89a4bbf7a90da118dcf00c5fe70eba78f8e5d95662bb5f039a353113e95042bR205

There's actually a polygon ethers middleware for this https://docs.rs/ethers/latest/ethers/middleware/gas_oracle/polygon/struct.Polygon.html

So I (originally) borrowed this code from Foundry https://github.com/foundry-rs/foundry/blob/master/crates/common/src/provider.rs#L254-L290

Changed to use Middlewares

This also means we can remove our existing janky Polygon logic

Drive-by changes

Related issues

Backward compatibility

Testing

Copy link

changeset-bot bot commented Nov 22, 2023

⚠️ No Changeset found

Latest commit: dec8238

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link
Member

@yorhodes yorhodes left a comment

Choose a reason for hiding this comment

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

nice work
would be good to just import as a middleware and add to the stack

Base automatically changed from trevor/new-featv3-cosmos-oct-28 to main November 23, 2023 10:23
@tkporter tkporter force-pushed the trevor/polygon-gas-oracle branch from 4197c85 to 51fde3d Compare November 23, 2023 11:27
@tkporter
Copy link
Collaborator Author

Changed to use middlewares - note this was a bit more painful / ugly than you'd probably expect, which is a result of the Middleware trait not being object safe. So conditional usage of a middleware gets really messy (see gakonst/ethers-rs#592)

The GasOracleMiddleware will apply the provided GasOracle regardless of what chain it is, so we'd need to either:
a) conditionally use the GasOracleMiddleware, or
b) because GasOracle is object safe, just always use a GasOracleMiddleware but use a different GasOracle depending on the chain

With (a), we end up with a bunch of really ugly conditional paths because we always need a concrete type for the provider/middleware object

And with (b), we need to have a GasOracle that will signal to the GasOracleMiddleware to not use a gas oracle at all - they have ProviderOracle, which lets you use the provider's oracle instead. This is fine, but the downside is to make this work we have to make the provider Arc<M> bc ProviderOracle requires ownership of the provider

@tkporter tkporter enabled auto-merge (squash) November 23, 2023 15:11
@tkporter tkporter merged commit 1cebc39 into main Nov 23, 2023
17 checks passed
@tkporter tkporter deleted the trevor/polygon-gas-oracle branch November 23, 2023 15:20
nambrot pushed a commit that referenced this pull request Nov 24, 2023
After investigating #2959, I found the following

this is a known problem with Polygon, explanation here
ethers-io/ethers.js#2828 (comment)

Fun fact Asa looked into this once
#771

Here's a discussion it in Foundry, which I found hoping that ethers-rs
folks had ran into this before
foundry-rs/foundry#1703

Foundry fixed this by using the Polygon gas station oracle which seems
to be recommended path
https://github.com/foundry-rs/foundry/pull/3368/files#diff-c89a4bbf7a90da118dcf00c5fe70eba78f8e5d95662bb5f039a353113e95042bR205

There's actually a polygon ethers middleware for this
https://docs.rs/ethers/latest/ethers/middleware/gas_oracle/polygon/struct.Polygon.html

So I (originally) borrowed this code from Foundry
https://github.com/foundry-rs/foundry/blob/master/crates/common/src/provider.rs#L254-L290

Changed to use Middlewares

This also means we can remove our existing janky Polygon logic

<!--
Are there any minor or drive-by changes also included?
-->

<!--
- Fixes #[issue number here]
-->

<!--
Are these changes backward compatible? Are there any infrastructure
implications, e.g. changes that would prohibit deploying older commits
using this infra tooling?

Yes/No
-->

<!--
What kind of testing have these changes undergone?

None/Manual/Unit Tests
-->
tkporter added a commit that referenced this pull request Nov 28, 2023
### Description

using image from
#2965

### Drive-by changes

<!--
Are there any minor or drive-by changes also included?
-->

### Related issues

<!--
- Fixes #[issue number here]
-->

### Backward compatibility

<!--
Are these changes backward compatible? Are there any infrastructure
implications, e.g. changes that would prohibit deploying older commits
using this infra tooling?

Yes/No
-->

### Testing

<!--
What kind of testing have these changes undergone?

None/Manual/Unit Tests
-->
nambrot pushed a commit that referenced this pull request Dec 1, 2023
After investigating #2959, I found the following

this is a known problem with Polygon, explanation here
ethers-io/ethers.js#2828 (comment)

Fun fact Asa looked into this once
#771

Here's a discussion it in Foundry, which I found hoping that ethers-rs
folks had ran into this before
foundry-rs/foundry#1703

Foundry fixed this by using the Polygon gas station oracle which seems
to be recommended path
https://github.com/foundry-rs/foundry/pull/3368/files#diff-c89a4bbf7a90da118dcf00c5fe70eba78f8e5d95662bb5f039a353113e95042bR205

There's actually a polygon ethers middleware for this
https://docs.rs/ethers/latest/ethers/middleware/gas_oracle/polygon/struct.Polygon.html

So I (originally) borrowed this code from Foundry
https://github.com/foundry-rs/foundry/blob/master/crates/common/src/provider.rs#L254-L290

Changed to use Middlewares

This also means we can remove our existing janky Polygon logic

<!--
Are there any minor or drive-by changes also included?
-->

<!--
- Fixes #[issue number here]
-->

<!--
Are these changes backward compatible? Are there any infrastructure
implications, e.g. changes that would prohibit deploying older commits
using this infra tooling?

Yes/No
-->

<!--
What kind of testing have these changes undergone?

None/Manual/Unit Tests
-->
tkporter added a commit that referenced this pull request Dec 4, 2023
Cherry-pick from #2965

---------

Co-authored-by: Trevor Porter <[email protected]>
daniel-savu pushed a commit that referenced this pull request Jun 5, 2024
Cherry-pick from #2965

---------

Co-authored-by: Trevor Porter <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants