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

FeeOracleV2 #1951

Open
kevinhalliday opened this issue Sep 23, 2024 · 1 comment · Fixed by #2291
Open

FeeOracleV2 #1951

kevinhalliday opened this issue Sep 23, 2024 · 1 comment · Fixed by #2291
Assignees

Comments

@kevinhalliday
Copy link
Contributor

kevinhalliday commented Sep 23, 2024

Problem to Solve

Our current fee oracles do effectively keep us "in the green" (charging more in fees than our relayer spends on gas). They could, however, use some improvements.

Data / non-evm pricing

FeeOracleV1 uses a "postsTo" mechanism to deteremine the data gas price for an xmsg. It uses the configured gas price of the "postsTo" chain to charge data fees for an xcall. This means that calls to L2s use the execution gas price of L1. Most L2s now post tx data as blobs, and blobs have an independent fee market. We need to update the fee oracles and xfeemngr to configure and charge xcall fees based on blob gas prices.

Additionally, our pricing is pretty evm specific. We should consider what it would look like to include a non-evm chain. We may well be able to "translate" non-evm chains into our exisiting evm-esque parameters. But we should consider what this would look like.

Overheads

Sigma prime suggested some "overheads" - for data costs / gas prices.

Requires active management

The current fee orocle contracts and monitor/xfeemngr do a lot of on chain transactions - can we reduce that cost? We want to remain "in the green". We also do not want to overcharge fees too much.

Proposed Solution

FeeOracleV2 was intrduced, but is unused. This version uses explicit dataGasPrice, rather than postsTo mechanism.

This may be all we need. Though it may not. The owner of this task should read the sigma prime report, as well as our responses to their suggestions. They should also look as mainnet / omega metrics.

Then, design, propose and implement both the

  • new fee oracle contracts
  • new xfeemngr
  • a process for upgrading live networks (omega & mainnet) to the new oracles
@kevinhalliday kevinhalliday self-assigned this Sep 23, 2024
@kevinhalliday kevinhalliday changed the title xmsg data fees do not use blob gas prices xcall data fees do not use blob gas prices Sep 23, 2024
@kevinhalliday kevinhalliday added this to the 1 - Mainnet Beta milestone Sep 23, 2024
kevinhalliday added a commit that referenced this issue Sep 27, 2024
Add FeeOracleV2 contract.

- fee oracle v2 uses explicit data & exec gas prices per chain
- add version() view on fee oracles


issue: #1951
@kevinhalliday kevinhalliday removed their assignment Oct 21, 2024
@kevinhalliday kevinhalliday changed the title xcall data fees do not use blob gas prices FeeOracleV2 Oct 21, 2024
Zodomo added a commit that referenced this issue Oct 28, 2024
…ts (#2291)

Optimized all variables to reduce contract storage slots from 4 to 2, and the struct's required SLOAD/SREADs from 5 to 2. This leads to a ~45% gas reduction in the bulkSetFeeParams() function and ~8% in the user-facing feeFor() function.

issue: #1951
@Zodomo Zodomo reopened this Oct 28, 2024
@Zodomo
Copy link
Contributor

Zodomo commented Oct 28, 2024

I was able to significantly decrease gas consumption in our bulkSetFeeParams() function in FeeOracleV2 by ~45% in #2291. This will make it substantially cheaper for us to operate it. However, I am reopening this issue as there might still need to be some discussion about whether the change to also targeting the appropriate L1 fee market (be it blobs or calldata) is sufficient, or if we have any alternative strategies worth pursuing.

I do think that execution and DA costs are the only things necessary for a fee oracle. As we can uniquely configure it for each route, we can handle differences between EVMs and non-EVMs. There isn't a way for a L2 smart contract to know if its L2 is posting to L1 using calldata or blobs, so it is on us to reconfigure the oracle's prices if such switches happen. To date, I don't know of any L2s that have switched between calldata and blobs frequently. However, this may change when the blob market is saturated.

@Zodomo Zodomo self-assigned this Oct 31, 2024
Zodomo added a commit that referenced this issue Nov 14, 2024
Created a FeeOracleV2 deployment routine that uses Create3. We cannot
upgrade FeeOracleV1 as V2 has a different storage layout. This routine
does not currently include upgrade logic.

issue: #1951
Zodomo added a commit that referenced this issue Nov 15, 2024
FeeOracleV2 Create3 salt was missing.

issue: #1951
Zodomo added a commit that referenced this issue Nov 19, 2024
Generation of FeeParams for L2 Sepolia testnets was failing as Sepolia
isn't in the manifests for staging or omega. Default gas prices to 1
gwei for non-mainnet manifests.

issue: #1951
Zodomo added a commit that referenced this issue Nov 29, 2024
Refactored FeeOracleV2 such that data cost fee updates can be made once 
and update all execution layer pricing for all chains that post to any given 
data layer. Also, conversion rates are set independently, which updates pricing
for all routes that rely on them.

issue: #1951
Zodomo added a commit that referenced this issue Dec 2, 2024
This adds an admin command to upgrade the `feeOracle` address on the
OmniPortal contracts to the FeeOracleV2 address.

issue: #1951
chmllr pushed a commit that referenced this issue Dec 3, 2024
Refactored FeeOracleV2 such that data cost fee updates can be made once 
and update all execution layer pricing for all chains that post to any given 
data layer. Also, conversion rates are set independently, which updates pricing
for all routes that rely on them.

issue: #1951
chmllr pushed a commit that referenced this issue Dec 3, 2024
This adds an admin command to upgrade the `feeOracle` address on the
OmniPortal contracts to the FeeOracleV2 address.

issue: #1951
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 a pull request may close this issue.

4 participants