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) #2974

Merged
merged 7 commits into from
Dec 4, 2023
Merged

Use Polygon gas oracle (#2965) #2974

merged 7 commits into from
Dec 4, 2023

Conversation

nambrot
Copy link
Contributor

@nambrot nambrot commented Nov 24, 2023

Cherry-pick from #2965

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
-->
Copy link

changeset-bot bot commented Nov 24, 2023

⚠️ No Changeset found

Latest commit: 30e8c92

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.

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

@tkporter tkporter enabled auto-merge (squash) November 28, 2023 10:46
nambrot and others added 2 commits November 28, 2023 16:55
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
-->
@nambrot
Copy link
Contributor Author

nambrot commented Dec 1, 2023

@tkporter cherry-picked #3000 onto here (not sure why the commit message doesnt reflect that), that should be all right?

@tkporter
Copy link
Collaborator

tkporter commented Dec 2, 2023

@nambrot I think so - I just got paged that Polygon v2 is having issues bc of the same replacement transaction thing, so gonna ship with this image and test it works

@tkporter
Copy link
Collaborator

tkporter commented Dec 2, 2023

Actually nvm we need the ethers-rs tag 2023-11-29-02, i'll change

@tkporter tkporter merged commit 125a197 into v2 Dec 4, 2023
5 checks passed
@tkporter tkporter deleted the nambrot/polygon-v2-fix branch December 4, 2023 11:36
tkporter added a commit that referenced this pull request Dec 5, 2023
### Description

Deploys to include
#2974

### 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
-->
daniel-savu pushed a commit that referenced this pull request Jun 5, 2024
Cherry-pick from #2965

---------

Co-authored-by: Trevor Porter <trkporter@ucdavis.edu>
daniel-savu pushed a commit that referenced this pull request Jun 5, 2024
### Description

Deploys to include
#2974

### 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
-->
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.

None yet

2 participants