feat: replace l2 extra fees with price overriders #460
Conversation
WalkthroughReplaces network-specific extra-fee services with a pluggable PriceParamsHandler; adds Optimism handler and test mock; removes legacy L2/optimism extra-fee modules; updates PriceCalculator API and RelayerTransactionFactory wiring; and removes L2 fee data from gas price cache. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant RT as RelayerTransactionFactory
participant PC as PriceCalculator
participant PPH as PriceParamsHandler
participant EP as EVM Provider
RT->>PC: new(gas_price_service, price_params_handler?)
RT->>PC: calculate_price(tx)
alt Handler present
PC->>PPH: handle_price_params(tx, PriceParams)
PPH->>EP: (Optimism) oracle calls (parallel reads)
EP-->>PPH: fee data (U256s)
PPH-->>PC: overridden PriceParams (extra_fee/total_cost)
else No handler
PC-->>RT: baseline PriceParams (no overrides)
end
PC-->>RT: final PriceParams
sequenceDiagram
autonumber
participant PC as PriceCalculator
participant PPH as PriceParamsHandler
PC->>PC: calculate_bumped_gas_price(...)
alt Handler present
PC->>PPH: handle_price_params(tx, bumped PriceParams)
PPH-->>PC: possibly overridden params
else No handler
PC-->>PC: use legacy bump logic
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks (2 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ Finishing Touches
🧪 Generate unit tests
Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #460 +/- ##
=======================================
+ Coverage 92.7% 92.9% +0.1%
=======================================
Files 219 219
Lines 74599 74884 +285
=======================================
+ Hits 69205 69600 +395
+ Misses 5394 5284 -110
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (6)
src/services/gas/handlers/test_mock.rs (1)
14-22: Align mock behavior with production path (recompute total, don’t mutate by +42)Keep semantics consistent with Optimism handler: set extra_fee then recompute total via calculate_total_cost.
- original_params.extra_fee = Some(U256::from(42u128)); - original_params.total_cost = original_params.total_cost + U256::from(42u128); + original_params.extra_fee = Some(U256::from(42u128)); + // Recompute like production handlers do + let gas_limit = _tx.gas_limit.unwrap_or(crate::constants::DEFAULT_GAS_LIMIT); + let is_eip1559 = original_params.max_fee_per_gas.is_some(); + original_params.total_cost = + original_params.calculate_total_cost(is_eip1559, gas_limit, _tx.value);src/services/gas/handlers/optimism.rs (1)
227-231: Unit test expectation should reflect ceil vs floor once confirmedIf ceil-div is correct, this test should assert the ceiled value.
src/domain/transaction/evm/price_calculator.rs (4)
133-151: Prefer Self over explicit type in forwarding callsUse Self::… to avoid repeating generics and improve readability.
- PriceCalculator::<G>::get_transaction_price_params(self, tx_data, relayer).await + Self::get_transaction_price_params(self, tx_data, relayer).await … - PriceCalculator::<G>::calculate_bumped_gas_price(self, tx_data, relayer).await + Self::calculate_bumped_gas_price(self, tx_data, relayer).await
230-233: Minor: avoid redundant U256::from for U256 valuetx_data.value is already U256.
- U256::from(tx_data.value), + tx_data.value,
621-623: Comment says “<” but code enforces “≤”Nit: align comment with behavior (≤), since equal is valid per clients/EIP-1559.
- // Ensure maxPriorityFeePerGas < maxFeePerGas to avoid client errors + // Ensure maxPriorityFeePerGas ≤ maxFeePerGas to avoid client errors
15-31: Docs example is stale (mentions provider param not in API anymore)Update the example to reflect PriceCalculator::get_transaction_price_params(&self, …) without a provider argument.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
src/domain/transaction/evm/price_calculator.rs(24 hunks)src/domain/transaction/mod.rs(3 hunks)src/models/network/evm/network.rs(1 hunks)src/services/gas/cache.rs(2 hunks)src/services/gas/handlers/mod.rs(1 hunks)src/services/gas/handlers/optimism.rs(1 hunks)src/services/gas/handlers/test_mock.rs(1 hunks)src/services/gas/mod.rs(1 hunks)src/services/gas/network_extra_fee.rs(0 hunks)src/services/gas/optimism_extra_fee.rs(0 hunks)src/services/gas/price_params_handler.rs(1 hunks)
💤 Files with no reviewable changes (2)
- src/services/gas/network_extra_fee.rs
- src/services/gas/optimism_extra_fee.rs
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: LuisUrrutia
PR: OpenZeppelin/openzeppelin-relayer#422
File: src/services/gas/optimism_extra_fee.rs:106-120
Timestamp: 2025-09-01T11:31:36.133Z
Learning: The Optimism Ecotone L1 fee calculation formula officially includes a factor of 16 in the weighted gas price calculation: `weighted_gas_price = 16*base_fee_scalar*base_fee + blob_base_fee_scalar*blob_base_fee`. This factor of 16 compensates for the division by 16 in the compressed transaction size calculation and is part of the official Optimism fee specification.
📚 Learning: 2025-09-01T11:31:36.133Z
Learnt from: LuisUrrutia
PR: OpenZeppelin/openzeppelin-relayer#422
File: src/services/gas/optimism_extra_fee.rs:106-120
Timestamp: 2025-09-01T11:31:36.133Z
Learning: The Optimism Ecotone L1 fee calculation formula officially includes a factor of 16 in the weighted gas price calculation: `weighted_gas_price = 16*base_fee_scalar*base_fee + blob_base_fee_scalar*blob_base_fee`. This factor of 16 compensates for the division by 16 in the compressed transaction size calculation and is part of the official Optimism fee specification.
Applied to files:
src/services/gas/handlers/optimism.rs
🧬 Code graph analysis (5)
src/services/gas/handlers/test_mock.rs (2)
src/services/gas/handlers/optimism.rs (1)
handle_price_params(123-144)src/services/gas/price_params_handler.rs (1)
handle_price_params(41-55)
src/domain/transaction/mod.rs (2)
src/services/gas/price_params_handler.rs (1)
for_network(27-35)src/domain/transaction/evm/price_calculator.rs (1)
new(158-163)
src/services/gas/handlers/optimism.rs (2)
src/services/gas/handlers/test_mock.rs (2)
new(10-12)handle_price_params(14-22)src/services/gas/price_params_handler.rs (1)
handle_price_params(41-55)
src/domain/transaction/evm/price_calculator.rs (4)
src/services/gas/handlers/test_mock.rs (1)
new(10-12)src/services/gas/evm_gas_price.rs (3)
new(160-170)default(30-37)default(49-55)src/models/transaction/repository.rs (4)
from(1046-1053)default(376-393)default(398-416)default(457-459)src/utils/mocks.rs (1)
create_mock_relayer(48-69)
src/services/gas/price_params_handler.rs (3)
src/domain/transaction/evm/price_calculator.rs (1)
new(158-163)src/services/gas/handlers/optimism.rs (2)
new(31-36)handle_price_params(123-144)src/services/gas/handlers/test_mock.rs (2)
new(10-12)handle_price_params(14-22)
🪛 Gitleaks (8.27.2)
src/services/gas/price_params_handler.rs
[high] 84-84: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Redirect rules - openzeppelin-relayer
- GitHub Check: Header rules - openzeppelin-relayer
- GitHub Check: Pages changed - openzeppelin-relayer
- GitHub Check: boostsecurity - boostsecurityio/semgrep-pro
- GitHub Check: clippy
- GitHub Check: test
- GitHub Check: msrv
- GitHub Check: Analyze (rust)
🔇 Additional comments (11)
src/services/gas/cache.rs (1)
32-60: Removal of L2 fee data from cache looks goodConstructor and entry shape now focus on core gas components only; aligns with new overrider flow.
src/models/network/evm/network.rs (1)
3-5: Formatting-only import changeNo functional impact; safe to merge.
src/services/gas/mod.rs (1)
4-5: Public surface reshaped toward handlers + overriderDeletions and additions align with the new PriceParamsHandler design.
src/services/gas/handlers/mod.rs (1)
1-9: Clean handler module boundaryExplicit re-exports for Optimism and test mock are clear and scoped.
src/domain/transaction/mod.rs (3)
26-28: Import switch to PriceParamsHandler is consistent with PR intentNo issues spotted.
418-420: Network-aware price overrider wiring looks correctCreating the handler with the concrete provider and letting it be optional per-network is the right shape.
437-439: PriceCalculator::new signature validated All invocations across the codebase take two parameters (gas_price_service and an Option), so the constructor call here is correct.src/services/gas/price_params_handler.rs (1)
27-35: for_network decision is minimal and predictableReturning Some only for Optimism-based networks is appropriate for this iteration.
src/services/gas/handlers/optimism.rs (1)
72-83: Keep floor division per official spec
The Ecotone spec defines
tx_compressed_size = (count_zero_bytes4 + count_non_zero_bytes16) / 16
using integer (floor) division and postpones the division for precision, so no ceil adjustment is needed (docs.optimism.io, specs.optimism.io)Likely an incorrect or invalid review comment.
src/domain/transaction/evm/price_calculator.rs (2)
127-131: Good generic simplification of PriceCalculatorDropping the extra generic and introducing an overrider hook keeps the type surface smaller without losing flexibility.
158-163: Constructor wiring looks correctConstructor now cleanly injects the optional overrider.
| // Use price params overrider if available for custom network pricing | ||
| if let Some(handler) = &self.price_params_handler { | ||
| let req = Self::build_request_from(tx_data, &final_params); | ||
| let extra_fee = svc.get_extra_fee(&req).await?; | ||
| final_params.extra_fee = Some(extra_fee); | ||
| final_params = handler.handle_price_params(&req, final_params).await?; | ||
| } else { | ||
| // Default behavior: calculate total cost | ||
| final_params.total_cost = final_params.calculate_total_cost( | ||
| tx_data.is_eip1559(), | ||
| tx_data.gas_limit.unwrap_or(DEFAULT_GAS_LIMIT), // Use default gas limit if not provided | ||
| U256::from(tx_data.value), | ||
| ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Cap can be bypassed after handler; also ensure total_cost fallback
If a handler mutates gas_price/max_fee fields, those mutations currently skip re-capping. Also, relying on the handler to always set total_cost is brittle.
Apply caps again after handler and compute total_cost if unset:
- // Use price params overrider if available for custom network pricing
- if let Some(handler) = &self.price_params_handler {
- let req = Self::build_request_from(tx_data, &final_params);
- final_params = handler.handle_price_params(&req, final_params).await?;
- } else {
- // Default behavior: calculate total cost
- final_params.total_cost = final_params.calculate_total_cost(
- tx_data.is_eip1559(),
- tx_data.gas_limit.unwrap_or(DEFAULT_GAS_LIMIT), // Use default gas limit if not provided
- U256::from(tx_data.value),
- );
- }
+ // Apply network-specific overrides (if any)
+ if let Some(handler) = &self.price_params_handler {
+ let req = Self::build_request_from(tx_data, &final_params);
+ final_params = handler.handle_price_params(&req, final_params).await?;
+
+ // Safety: re-apply relayer cap in case handler changed fee fields
+ let (gp, mfp, mp) = self.apply_gas_price_cap(
+ final_params.gas_price.unwrap_or_default(),
+ final_params.max_fee_per_gas,
+ final_params.max_priority_fee_per_gas,
+ relayer,
+ )?;
+ final_params.gas_price = gp;
+ final_params.max_fee_per_gas = mfp;
+ final_params.max_priority_fee_per_gas = mp;
+ }
+
+ // Ensure total_cost is set (handlers may leave it unset)
+ if final_params.total_cost == U256::ZERO {
+ final_params.total_cost = final_params.calculate_total_cost(
+ tx_data.is_eip1559(),
+ tx_data.gas_limit.unwrap_or(DEFAULT_GAS_LIMIT),
+ tx_data.value,
+ );
+ }Also consider documenting the handler contract (which fields it may change) to make this invariant explicit.
🤖 Prompt for AI Agents
In src/domain/transaction/evm/price_calculator.rs around lines 223–234, the
handler-returned price params may mutate gas_price/max_fee and currently escape
the capping logic and may not set total_cost; after calling
handler.handle_price_params(...) re-apply the same caps/limits you apply for the
default path to ensure gas_price/max_fee are bounded, and if
final_params.total_cost is not set (or zero), compute and populate it using
final_params.calculate_total_cost(tx_data.is_eip1559(),
tx_data.gas_limit.unwrap_or(DEFAULT_GAS_LIMIT), U256::from(tx_data.value)); keep
the same cap logic and gas_limit fallback so invariants hold regardless of
handler mutations.
💡 Verification agent
🧩 Analysis chain
Optimism handler: confirm Ecotone factor-of-16 in fee math
Per prior learnings, Optimism’s weighted gas price includes a 16× factor. Ensure your Optimism handler applies that correctly.
Based on our retrieved learnings; if you want, I can prep a web check for the latest spec.
🌐 Web query:
Optimism Ecotone L1 fee calculation includes factor 16 in weighted gas price; confirm current spec and example formula.
💡 Result:
Yes — Ecotone's L1 data-fee math includes a factor 16 multiplying the L1 base-fee term. Current spec (Ecotone) and an equivalent example:
-
Compressed-size (conceptual):
tx_compressed_size = (zero_bytes4 + nonzero_bytes16) / 16. [1] -
Weighted gas price (note the 16 factor on the base-fee term):
weighted_gas_price = 16 * base_fee_scalar * l1BaseFee + l1BlobBaseFeeScalar * l1BlobBaseFee. [1][2] -
L1 data fee:
l1_data_fee = tx_compressed_size * weighted_gas_price. (Implementation uses integer-preserving form: (zeroes4 + ones16) * (16l1BaseFeel1BaseFeeScalar + l1BlobBaseFee*l1BlobBaseFeeScalar) / 16e6). [2]
Sources: Optimism developer docs (Ecotone section). [1] and the ethereum-optimism spec on GitHub (Ecotone L1-cost formula). [2]
References
[1] Optimism Docs — Transaction Fees (Ecotone section).
[2] ethereum-optimism/specs — exec-engine.md (Ecotone L1-cost formula).
Add Ecotone 16× multiplier to Optimism L1 fee math
Ensure your Optimism Ecotone handler applies the 16× factor on the L1 base‐fee term:
weighted_gas_price = 16 * base_fee_scalar * l1BaseFee + blob_base_fee_scalar * l1BlobBaseFee
Update the implementation in src/services/gas/optimism_extra_fee.rs to include this multiplier.
🤖 Prompt for AI Agents
In src/domain/transaction/evm/price_calculator.rs around lines 223-234, the
Optimism Ecotone L1 fee multiplier is missing; update the Optimism handler
implementation in src/services/gas/optimism_extra_fee.rs so the L1 base-fee term
is multiplied by 16. Change the weighted_gas_price computation to use:
weighted_gas_price = 16 * base_fee_scalar * l1BaseFee + blob_base_fee_scalar *
l1BlobBaseFee (preserving existing scalars and types), and ensure the handler
returns the adjusted total_cost consistently with the rest of the price params
flow and update any affected tests.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/services/gas/price_params_handler.rs (1)
80-84: Hardcoded API-key-looking value trips secret scanners (Gitleaks)Use a clearly dummy token; also, consider setting env once to avoid cross-test races.
fn setup_test_env() { - env::set_var("API_KEY", "7EF1CB7C-5003-4696-B384-C72AF8C3E15D"); + // Use a clearly dummy value to avoid secret scanners + env::set_var("API_KEY", "dummy-api-key"); env::set_var("REDIS_URL", "redis://localhost:6379"); env::set_var("RPC_TIMEOUT_MS", "5000"); }
🧹 Nitpick comments (2)
src/services/gas/price_params_handler.rs (2)
38-52: Add lightweight tracing to surface which handler appliedA single debug event helps ops understand which network override path ran.
pub async fn handle_price_params( &self, tx: &EvmTransactionRequest, original_params: PriceParams, ) -> Result<PriceParams, TransactionError> { match self { - PriceParamsHandler::Optimism(handler) => { + PriceParamsHandler::Optimism(handler) => { + #[cfg(feature = "trace")] + tracing::debug!(target = "gas", handler = "optimism", "applying price overrides"); handler.handle_price_params(tx, original_params).await } #[cfg(test)] PriceParamsHandler::Mock(handler) => { handler.handle_price_params(tx, original_params).await } } }
8-12: Follow-up: avoid unwrap in OptimismPriceHandler::new()In handlers/optimism.rs, new() parses the oracle address with unwrap(). Prefer a compile-time address (e.g., via hex literal) or map parse errors to TransactionError to remove a potential panic path.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/services/gas/price_params_handler.rs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/services/gas/price_params_handler.rs (3)
src/services/gas/handlers/optimism.rs (2)
new(31-36)handle_price_params(123-144)src/services/gas/handlers/test_mock.rs (2)
new(10-12)handle_price_params(14-22)src/domain/transaction/evm/price_calculator.rs (1)
new(158-163)
🪛 Gitleaks (8.27.2)
src/services/gas/price_params_handler.rs
[high] 81-81: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: boostsecurity - boostsecurityio/semgrep-pro
- GitHub Check: clippy
- GitHub Check: test
- GitHub Check: Redirect rules - openzeppelin-relayer
- GitHub Check: Header rules - openzeppelin-relayer
- GitHub Check: Pages changed - openzeppelin-relayer
- GitHub Check: Analyze (rust)
🔇 Additional comments (6)
src/services/gas/price_params_handler.rs (6)
55-78: Test helper LGTMSynthetic network builder is clear and self-contained.
100-108: Non-L2 path test LGTM
110-139: Mock variant behavior test LGTMCovers override of extra_fee and total_cost as intended.
13-18: Retain Clone derive on PriceParamsHandler
Both variants are Clone:OptimismPriceHandler<P>derives Clone forP: CloneandEvmProvideris Clone, andMockPriceHandleris Clone. The enum’s#[derive(Clone)]is valid.Likely an incorrect or invalid review comment.
24-32: No changes needed: OP Stack networks covered by is_optimism()
EvmNetwork::is_optimism() (src/models/network/evm/network.rs:103–106) already checks for both OPTIMISM_BASED_TAG and OPTIMISM_TAG, so Base/Zora‐like chains tagged accordingly will be routed to the Optimism handler.
86-98: EvmProvider::new is lazy; no network I/O — it only validates URLs and builds the selector (no dialing of endpoints occurs), so the tests remain fully hermetic.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
src/domain/transaction/evm/price_calculator.rs (3)
223-254: Good: Re-applying caps after handler prevents bypass.The implementation correctly re-applies gas price caps after the handler modifies parameters, preventing malicious or buggy handlers from bypassing relayer limits. The total_cost fallback logic when the handler leaves it unset is also properly implemented.
332-360: LGTM! Consistent handling in bump path.The bump path correctly mirrors the initial pricing path with the same post-handler capping and total_cost fallback logic.
1560-1625: Add tests for post-handler capping behavior.While the current tests verify the mock handler functionality, consider adding specific tests that verify:
- A malicious handler attempting to set gas_price/max_fee above the relayer cap gets properly capped
- The total_cost fallback when a handler doesn't set it
Add test cases for cap enforcement:
#[tokio::test] async fn test_handler_gas_price_cap_enforcement() { // Create a malicious handler that tries to bypass caps struct MaliciousHandler; #[async_trait::async_trait] impl PriceParamsHandlerTrait for MaliciousHandler { async fn handle_price_params( &self, _req: &EvmTransactionRequest, mut params: PriceParams, ) -> Result<PriceParams, TransactionError> { // Try to set gas prices way above what should be allowed params.gas_price = Some(1_000_000_000_000); // 1000 Gwei params.max_fee_per_gas = Some(1_000_000_000_000); params.max_priority_fee_per_gas = Some(500_000_000_000); Ok(params) } } let mut mock_service = MockEvmGasPriceServiceTrait::new(); // ... setup mocks ... let handler = Some(PriceParamsHandler::Mock(Box::new(MaliciousHandler))); let pc = PriceCalculator::new(mock_service, handler); let mut relayer = create_mock_relayer(); relayer.policies = RelayerNetworkPolicy::Evm(RelayerEvmPolicy { gas_price_cap: Some(100_000_000_000), // 100 Gwei cap ..Default::default() }); let tx_data = EvmTransactionData { gas_price: Some(20_000_000_000), ..Default::default() }; let params = pc.get_transaction_price_params(&tx_data, &relayer).await.unwrap(); // Verify that caps were enforced despite handler's attempt assert_eq!(params.gas_price, Some(100_000_000_000)); } #[tokio::test] async fn test_handler_total_cost_fallback() { // Create a handler that sets extra_fee but not total_cost struct PartialHandler; #[async_trait::async_trait] impl PriceParamsHandlerTrait for PartialHandler { async fn handle_price_params( &self, _req: &EvmTransactionRequest, mut params: PriceParams, ) -> Result<PriceParams, TransactionError> { params.extra_fee = Some(U256::from(1000)); // Intentionally leave total_cost as zero params.total_cost = U256::ZERO; Ok(params) } } // ... test that total_cost gets calculated ... }
🧹 Nitpick comments (1)
src/domain/transaction/evm/price_calculator.rs (1)
127-130: Consider documenting the PriceParamsHandler contract.The struct now uses
PriceParamsHandlerfor custom network pricing overrides. Consider adding documentation comments explaining what fields the handler may modify and what invariants it must maintain.Add documentation for the
price_params_handlerfield:pub struct PriceCalculator<G: EvmGasPriceServiceTrait> { gas_price_service: G, + /// Optional handler for custom network-specific price parameter overrides. + /// Handlers may modify gas_price, max_fee_per_gas, max_priority_fee_per_gas, + /// extra_fee, and total_cost fields. If total_cost is left as zero after + /// handler processing, it will be automatically calculated. price_params_handler: Option<PriceParamsHandler>, }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/domain/transaction/evm/price_calculator.rs(25 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-01T11:31:36.133Z
Learnt from: LuisUrrutia
PR: OpenZeppelin/openzeppelin-relayer#422
File: src/services/gas/optimism_extra_fee.rs:106-120
Timestamp: 2025-09-01T11:31:36.133Z
Learning: The Optimism Ecotone L1 fee calculation formula officially includes a factor of 16 in the weighted gas price calculation: `weighted_gas_price = 16*base_fee_scalar*base_fee + blob_base_fee_scalar*blob_base_fee`. This factor of 16 compensates for the division by 16 in the compressed transaction size calculation and is part of the official Optimism fee specification.
Applied to files:
src/domain/transaction/evm/price_calculator.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: clippy
- GitHub Check: test
- GitHub Check: msrv
- GitHub Check: boostsecurity - boostsecurityio/semgrep-pro
- GitHub Check: Redirect rules - openzeppelin-relayer
- GitHub Check: Header rules - openzeppelin-relayer
- GitHub Check: Pages changed - openzeppelin-relayer
- GitHub Check: Analyze (rust)
- GitHub Check: semgrep/ci
🔇 Additional comments (4)
src/domain/transaction/evm/price_calculator.rs (4)
45-45: LGTM!The import change from
NetworkExtraFeeCalculatorServicetoPriceParamsHandleraligns with the PR's objective of replacing the L2 extra-fees mechanism with price overriders.
1471-1513: Good test coverage for total_cost recomputation.The test verifies that
total_costis correctly recomputed when no overrider is present, using the final gas_price and provided gas_limit.
1515-1558: Good test coverage for overrider's total_cost preservation.The test verifies that when an overrider sets a non-zero
total_cost, the calculator correctly preserves it without recomputation.
1628-1655: LGTM! Comprehensive test coverage.The test properly verifies that
total_costis recomputed based on the final gas_price and gas_limit when no overrider is present.
| final_params = handler.handle_price_params(&req, final_params).await?; | ||
|
|
||
| let (gas_price_capped, max_fee_per_gas_capped, max_priority_fee_per_gas_capped) = self | ||
| .apply_gas_price_cap( | ||
| final_params.gas_price.unwrap_or_default(), | ||
| final_params.max_fee_per_gas, | ||
| final_params.max_priority_fee_per_gas, | ||
| relayer, | ||
| )?; | ||
| final_params.gas_price = gas_price_capped; | ||
| final_params.max_fee_per_gas = max_fee_per_gas_capped; | ||
| final_params.max_priority_fee_per_gas = max_priority_fee_per_gas_capped; | ||
|
|
||
| recompute_total_cost = final_params.total_cost == U256::ZERO; |
There was a problem hiding this comment.
I see this is duplicated, can we extra to function and reuse it
There was a problem hiding this comment.
I will handle that in a different PR, as this one is related to another change. I created this one to make it easier to review.
Summary
The previous implementation supported extra fees from L2 networks. However, some networks adjust gas estimation values via RPC calls to improve accuracy, which was not supported. To address this, we decided to add support for price overrides.
Testing Process
Checklist
Summary by CodeRabbit