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.

Sign up for GitHub

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

Fix estimate gas for old runtimes #1409

Merged
merged 5 commits into from
May 13, 2024
Merged

Fix estimate gas for old runtimes #1409

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
53eba3e
max_fee_per_gas should be set to None for old runtimes
librelois Apr 29, 2024
25aaaf3
estimate max transaction length for proof size base cost
librelois Apr 30, 2024
ec8b9ab
import GasWeightMapping trait
librelois Apr 30, 2024
0f56abe
fix compilation errors
librelois Apr 30, 2024
7bb579d
remove unused imports
librelois Apr 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 3 additions & 7 deletions client/rpc/src/eth/execute.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,12 +100,7 @@ where
let details = fee_details(gas_price, max_fee_per_gas, max_priority_fee_per_gas)?;
(
details.gas_price,
// Old runtimes require max_fee_per_gas to be None for non transactional calls.
if details.max_fee_per_gas == Some(U256::zero()) {
None
} else {
details.max_fee_per_gas
},
details.max_fee_per_gas,
details.max_priority_fee_per_gas,
)
};
Expand Down Expand Up @@ -1051,7 +1046,8 @@ fn fee_details(
// Default to EIP-1559 transaction
_ => Ok(FeeDetails {
gas_price: None,
max_fee_per_gas: Some(U256::zero()),
// Old runtimes require max_fee_per_gas to be None for non transactional calls.
max_fee_per_gas: None,
max_priority_fee_per_gas: Some(U256::zero()),
fee_cap: U256::zero(),
}),
Expand Down
114 changes: 83 additions & 31 deletions template/runtime/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,7 @@ use pallet_transaction_payment::{ConstFeeMultiplier, CurrencyAdapter};
use fp_account::EthereumSignature;
use fp_evm::weight_per_gas;
use fp_rpc::TransactionStatus;
use pallet_ethereum::{
Call::transact, PostLogContent, Transaction as EthereumTransaction, TransactionAction,
TransactionData,
};
use pallet_ethereum::{Call::transact, PostLogContent, Transaction as EthereumTransaction};
use pallet_evm::{
Account as EVMAccount, EnsureAccountId20, FeeCalculator, IdentityAddressMapping, Runner,
};
Expand Down Expand Up @@ -791,6 +788,8 @@ impl_runtime_apis! {
estimate: bool,
access_list: Option<Vec<(H160, Vec<H256>)>>,
) -> Result<pallet_evm::CallInfo, sp_runtime::DispatchError> {
use pallet_evm::GasWeightMapping as _;

let config = if estimate {
let mut config = <Runtime as pallet_evm::Config>::config().clone();
config.estimate = true;
Expand All @@ -799,20 +798,45 @@ impl_runtime_apis! {
None
};

let gas_limit = gas_limit.min(u64::MAX.into());
let transaction_data = TransactionData::new(
TransactionAction::Call(to),
data.clone(),
nonce.unwrap_or_default(),
gas_limit,
None,
max_fee_per_gas,
max_priority_fee_per_gas,
value,
Some(<Runtime as pallet_evm::Config>::ChainId::get()),
access_list.clone().unwrap_or_default(),
);
Copy link
Collaborator

@boundless-forest boundless-forest May 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the delay in replying(just come back from holiday). I come up with another idea is that we can pass default value to the TransactionData::new(xxx) for these field with Option<T> type, such as max_fee_per_gas and max_priority_fee_per_gas, instead of None. This way can also meet our needs and looks cleaner. What do you think? @librelois

let (weight_limit, proof_size_base_cost) = pallet_ethereum::Pallet::<Runtime>::transaction_weight(&transaction_data);
// Estimated encoded transaction size must be based on the heaviest transaction
// type (EIP1559Transaction) to be compatible with all transaction types.
let mut estimated_transaction_len = data.len() +
// pallet ethereum index: 1
// transact call index: 1
// Transaction enum variant: 1
// chain_id 8 bytes
// nonce: 32
// max_priority_fee_per_gas: 32
// max_fee_per_gas: 32
// gas_limit: 32
// action: 21 (enum varianrt + call address)
// value: 32
// access_list: 1 (empty vec size)
// 65 bytes signature
258;

if access_list.is_some() {
estimated_transaction_len += access_list.encoded_size();
}


let gas_limit = if gas_limit > U256::from(u64::MAX) {
u64::MAX
} else {
gas_limit.low_u64()
};
let without_base_extrinsic_weight = true;

let (weight_limit, proof_size_base_cost) =
match <Runtime as pallet_evm::Config>::GasWeightMapping::gas_to_weight(
gas_limit,
without_base_extrinsic_weight
) {
weight_limit if weight_limit.proof_size() > 0 => {
(Some(weight_limit), Some(estimated_transaction_len as u64))
}
_ => (None, None),
};

<Runtime as pallet_evm::Config>::Runner::call(
from,
Expand Down Expand Up @@ -843,6 +867,8 @@ impl_runtime_apis! {
estimate: bool,
access_list: Option<Vec<(H160, Vec<H256>)>>,
) -> Result<pallet_evm::CreateInfo, sp_runtime::DispatchError> {
use pallet_evm::GasWeightMapping as _;

let config = if estimate {
let mut config = <Runtime as pallet_evm::Config>::config().clone();
config.estimate = true;
Expand All @@ -851,19 +877,45 @@ impl_runtime_apis! {
None
};

let transaction_data = TransactionData::new(
TransactionAction::Create,
data.clone(),
nonce.unwrap_or_default(),
gas_limit,
None,
max_fee_per_gas,
max_priority_fee_per_gas,
value,
Some(<Runtime as pallet_evm::Config>::ChainId::get()),
access_list.clone().unwrap_or_default(),
);
let (weight_limit, proof_size_base_cost) = pallet_ethereum::Pallet::<Runtime>::transaction_weight(&transaction_data);

let mut estimated_transaction_len = data.len() +
// from: 20
// value: 32
// gas_limit: 32
// nonce: 32
// 1 byte transaction action variant
// chain id 8 bytes
// 65 bytes signature
190;

if max_fee_per_gas.is_some() {
estimated_transaction_len += 32;
}
if max_priority_fee_per_gas.is_some() {
estimated_transaction_len += 32;
}
if access_list.is_some() {
estimated_transaction_len += access_list.encoded_size();
}


let gas_limit = if gas_limit > U256::from(u64::MAX) {
u64::MAX
} else {
gas_limit.low_u64()
};
let without_base_extrinsic_weight = true;

let (weight_limit, proof_size_base_cost) =
match <Runtime as pallet_evm::Config>::GasWeightMapping::gas_to_weight(
gas_limit,
without_base_extrinsic_weight
) {
weight_limit if weight_limit.proof_size() > 0 => {
(Some(weight_limit), Some(estimated_transaction_len as u64))
}
_ => (None, None),
};

<Runtime as pallet_evm::Config>::Runner::create(
from,
Expand Down