search

Synthetixio / synthetix-v3

MIT License
114 stars 56 forks source link

Update GasPriceOracle for Fjord upgrade #2189

Closed leomassazza closed 2 months ago

kaleb-keny commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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?