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

Update GasPriceOracle for Fjord upgrade #2189

Merged
merged 3 commits into from
Jul 5, 2024

Conversation

leomassazza
Copy link
Contributor

No description provided.

@leomassazza leomassazza self-assigned this Jul 2, 2024
@leomassazza leomassazza marked this pull request as ready for review July 3, 2024 00:31
@kaleb-keny
Copy link
Member

Do you rekon it's ok to drop the validation done here https://github.com/Synthetixio/synthetix-v3/pull/2189/files#diff-fa0f8cbc846f3613ed2f8c52aa36ec45ac6f8ddef59a22d0f2053eb36b07bde7L160 given that these are not longer used in fjord

        ovmGasPriceOracle.gasPrice();
        ovmGasPriceOracle.l1BaseFee();
        ovmGasPriceOracle.decimals();

In case optimism team decides to deprecate, we dont want our contracts bricked...

@leomassazza
Copy link
Contributor Author

Do you rekon it's ok to drop the validation done here https://github.com/Synthetixio/synthetix-v3/pull/2189/files#diff-fa0f8cbc846f3613ed2f8c52aa36ec45ac6f8ddef59a22d0f2053eb36b07bde7L160 given that these are not longer used in fjord

        ovmGasPriceOracle.gasPrice();
        ovmGasPriceOracle.l1BaseFee();
        ovmGasPriceOracle.decimals();

In case optimism team decides to deprecate, we dont want our contracts bricked...

They are not used, but still exists on fjord and is a way to confirm we are pointing to the right contract

@kaleb-keny
Copy link
Member

Do you rekon it's ok to drop the validation done here https://github.com/Synthetixio/synthetix-v3/pull/2189/files#diff-fa0f8cbc846f3613ed2f8c52aa36ec45ac6f8ddef59a22d0f2053eb36b07bde7L160 given that these are not longer used in fjord

        ovmGasPriceOracle.gasPrice();
        ovmGasPriceOracle.l1BaseFee();
        ovmGasPriceOracle.decimals();

In case optimism team decides to deprecate, we dont want our contracts bricked...

They are not used, but still exists on fjord and is a way to confirm we are pointing to the right contract

Hey yeah i understand, but they are marked by optimism as "legacy" since the team doesn't employ them anymore in their calculations... I guess we can keep them as is and drop them when we have our contracts bricked on sepolia, 😅 it's as good indicator as any of incoming changes. Alright @leomassazza is this ready for audit then? Or do you need to update few more tests?

@kaleb-keny kaleb-keny merged commit 459bd90 into main Jul 5, 2024
27 checks passed
@kaleb-keny kaleb-keny deleted the Gas-price-oracle-update-to-Fjord branch July 5, 2024 09:58
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

2 participants