Use pallet_ethereum to calculate proof_size_base_cost< 10000 /bdi> #3279
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR replaces custom transaction weight and proof size calculation logic with the pallet_ethereum implementation and updates the runtime dependencies accordingly.
- Replaces legacy gas weight mapping with the new pallet_ethereum::Pallet transaction_weight API.
- Removes redundant estimated transaction length calculations in favor of a unified transaction_data creation.
- Updates Cargo.toml to include pallet-ethereum in dependencies, std, and runtime benchmarks.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| runtime/common/src/apis.rs | Replaces custom gas to weight logic with pallet_ethereum’s transaction_weight computation. |
| runtime/common/Cargo.toml | Adds pallet-ethereum as a dependency across workspace, std, and runtime benchmarks. |
Comments suppressed due to low confidence (4)
runtime/common/src/apis.rs:356
- [nitpick] Ensure that the new transaction_weight computation is covered by tests, particularly for edge cases involving proof_size_base_cost.
let (weight_limit, proof_size_base_cost) = pallet_ethereum::Pallet::<Runtime>::transaction_weight(&transaction_data);
runtime/common/src/apis.rs:474
- [nitpick] Verify that changes replacing the manual computation with transaction_weight are properly validated with unit and integration tests.
let (weight_limit, proof_size_base_cost) = pallet_ethereum::Pallet::<Runtime>::transaction_weight(&transaction_data);
runtime/common/src/apis.rs:530
- [nitpick] Ensure that the behavior of the updated transaction weight and proof cost calculations is thoroughly tested to catch any regressions.
let (weight_limit, proof_size_base_cost) = pallet_ethereum::Pallet::<Runtime>::transaction_weight(&transaction_data);
runtime/common/Cargo.toml:22
- [nitpick] Confirm that the addition of pallet-ethereum as a workspace dependency aligns with the project configuration and versioning strategies.
pallet-ethereum = { workspace = true }
WASM runtime size check:Compared to target branchMoonbase runtime: 2428 KB (no changes) 🚨 Moonbeam runtime: 2412 KB (no changes) 🚨 Moonriver runtime: 2408 KB (no changes) 🚨 Compared to latest release (runtime-3600)Moonbase runtime: 2428 KB (+472 KB compared to latest release) 🚨 Moonbeam runtime: 2412 KB (+468 KB compared to latest release) 🚨 Moonriver runtime: 2408 KB (+468 KB compared to latest release) 🚨 |
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the calculation of proof_size_base_cost by replacing the manual estimation logic and the GasWeightMapping call with the new pallet_ethereum::Pallet transaction_weight API. In addition, it updates the Cargo.toml to include the pallet-ethereum dependency along with required features.
- Replaces manual encoded transaction length estimation with a new TransactionData-based computation.
- Updates GasWeightMapping usage to rely on pallet_ethereum::Pallet::::transaction_weight.
- Introduces the pallet-ethereum dependency with the "forbid-evm-reentrancy" feature in Cargo.toml.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| runtime/common/src/apis.rs | Refactored proof_size_base_cost logic to utilize pallet_ethereum API. |
| runtime/common/Cargo.toml | Added pallet-ethereum dependency and updated feature/std listings. |
Coverage Report@@ Coverage Diff @@
## master tarekkma/improve-base-cost-calc +/- ##
===================================================================
+ Coverage 74.76% 74.79% +0.03%
Files 400 400
- Lines 101272 101212 -60
===================================================================
- Hits 75712 75696 -16
- Misses 25560 25516 -44
|
|
To be honest, I don’t like the design of TransactionData, as it’s less accurate than using the TransactionV2 enum directly. It will overestimate the PoV base cost when you submit a Legacy or EIP‑2930 transaction. Fixing that would require substantial changes upstream in Frontier. However, it’s fine for new blocks, because nowadays everyone uses the EIP‑1559 transaction type, so we can merge it as is. |
Use
pallet_ethereum'sTransactionDatastruct andtransaction_weight(...)function to calculateproof_size_base_costinstead of using hardcoded numbers.