Skip to content

Commit

Permalink
Merge pull request #3425 from autonomys/revert-evm-fee-estimates
Browse files Browse the repository at this point in the history 
Revert incorrect EVM gas fee estimates
  • Loading branch information
@teor2345
teor2345 authored Mar 10, 2025
2 parents 554249d + 3615829 commit 7f51ed8
Show file tree
Hide file tree
Showing 4 changed files with 168 additions and 193 deletions.
2 changes: 1 addition & 1 deletion domains/client/domain-operator/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,5 +80,5 @@ sp-state-machine.workspace = true
subspace-core-primitives.workspace = true
subspace-test-runtime.workspace = true
subspace-test-service.workspace = true
subspace-test-primitives.workspace = true
subspace-test-primitives = { workspace = true, features = ["std"] }
tempfile.workspace = true
158 changes: 146 additions & 12 deletions domains/client/domain-operator/src/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ use sp_domains::merkle_tree::MerkleTree;
use sp_domains::test_ethereum::{generate_legacy_tx, max_extrinsic_gas};
use sp_domains::test_ethereum_tx::{address_build, contract_address, AccountInfo};
use sp_domains::{
Bundle, BundleValidity, ChainId, ChannelId, DomainsApi, HeaderHashingFor, InboxedBundle,
InvalidBundleType, PermissionedActionAllowedBy, Transfers,
BlockFees, Bundle, BundleValidity, ChainId, ChannelId, DomainsApi, HeaderHashingFor,
InboxedBundle, InvalidBundleType, PermissionedActionAllowedBy, Transfers,
};
use sp_domains_fraud_proof::fraud_proof::{
ApplyExtrinsicMismatch, ExecutionPhase, FinalizeBlockMismatch, FraudProofVariant,
Expand Down Expand Up @@ -1264,10 +1264,10 @@ async fn test_evm_domain_create_contracts_with_allow_list_multiple() {

#[tokio::test(flavor = "multi_thread")]
async fn test_evm_domain_gas_estimates() {
let (_directory, _ferdie, alice, account_infos) =
let (_directory, mut ferdie, mut alice, account_infos) =
setup_evm_test_accounts(Sr25519Alice, false, None).await;

let test_estimate_gas = |evm_call| {
let test_estimate_gas = |evm_call, is_estimate| {
let <TestRuntime as frame_system::Config>::RuntimeCall::EVM(evm_call) = evm_call else {
panic!("Unexpected RuntimeCall type");
};
Expand Down Expand Up @@ -1295,8 +1295,8 @@ async fn test_evm_domain_gas_estimates() {
Some(max_fee_per_gas),
max_priority_fee_per_gas,
nonce,
// Estimate gas
true,
// Do we want to estimate gas, or run the call?
is_estimate,
Some(access_list.clone()),
)
.expect("test EVM Create runtime call must succeed")
Expand All @@ -1308,7 +1308,7 @@ async fn test_evm_domain_gas_estimates() {
create_info.used_gas.effective,
),
// The exact estimate is not important, but we want to know if it changes
(4_327_174.into(), 4_327_174.into()),
(4_326_280.into(), 4_326_280.into()),
"Incorrect EVM Create gas estimate: {:?} {:?}",
evm_call,
create_info,
Expand Down Expand Up @@ -1338,8 +1338,8 @@ async fn test_evm_domain_gas_estimates() {
Some(max_fee_per_gas),
max_priority_fee_per_gas,
nonce,
// Estimate gas
true,
// Do we want to estimate gas, or run the call?
is_estimate,
Some(access_list.clone()),
)
.expect("test EVM Call runtime call must succeed")
Expand All @@ -1348,7 +1348,7 @@ async fn test_evm_domain_gas_estimates() {
assert_eq!(
(call_info.used_gas.standard, call_info.used_gas.effective),
// The exact estimate is not important, but we want to know if it changes
(22_258.into(), 22_258.into()),
(21_400.into(), 21_400.into()),
"Incorrect EVM Call gas estimate: {:?} {:?}",
evm_call,
call_info,
Expand All @@ -1364,6 +1364,7 @@ async fn test_evm_domain_gas_estimates() {
.gas_price(alice.client.info().best_hash)
.unwrap();

// Estimates
let evm_nonce = alice
.client
.runtime_api()
Expand All @@ -1377,7 +1378,7 @@ async fn test_evm_domain_gas_estimates() {
evm_nonce,
gas_price,
);
test_estimate_gas(evm_create);
test_estimate_gas(evm_create, true);

let evm_nonce = alice
.client
Expand All @@ -1392,7 +1393,140 @@ async fn test_evm_domain_gas_estimates() {
evm_nonce,
gas_price,
);
test_estimate_gas(evm_call);
test_estimate_gas(evm_call, true);

// Really do it, using runtime API calls
let evm_nonce = alice
.client
.runtime_api()
.account_basic(alice.client.info().best_hash, account_infos[0].address)
.unwrap()
.nonce;
let evm_create = generate_evm_domain_call(
account_infos[0].clone(),
ethereum::TransactionAction::Create,
0,
evm_nonce,
gas_price,
);
test_estimate_gas(evm_create, false);

let evm_contract_address = contract_address(account_infos[0].address, evm_nonce.as_u64());

produce_blocks!(ferdie, alice, 3).await.unwrap();

let evm_nonce = alice
.client
.runtime_api()
.account_basic(alice.client.info().best_hash, account_infos[0].address)
.unwrap()
.nonce;
let evm_call = generate_evm_domain_call(
account_infos[0].clone(),
ethereum::TransactionAction::Call(evm_contract_address),
0,
evm_nonce,
gas_price,
);
test_estimate_gas(evm_call, false);

produce_blocks!(ferdie, alice, 3).await.unwrap();

// Really do it, using construct_and_send_extrinsic()
let evm_nonce = alice
.client
.runtime_api()
.account_basic(alice.client.info().best_hash, account_infos[0].address)
.unwrap()
.nonce;
let evm_tx = generate_evm_domain_call(
account_infos[0].clone(),
ethereum::TransactionAction::Create,
0,
evm_nonce,
gas_price,
);
let evm_contract_address = contract_address(account_infos[0].address, evm_nonce.as_u64());

// Use alice's account ID, which does the signing in construct_and_send_extrinsic()
let result = alice.construct_and_send_extrinsic(evm_tx).await;
assert_matches!(
result,
Ok(_),
"Unexpectedly failed to send signed extrinsic"
);

// Produce a bundle that contains just the sent extrinsics
let (slot, bundle) = ferdie.produce_slot_and_wait_for_bundle_submission().await;
assert_eq!(bundle.extrinsics.len(), 1);
produce_block_with!(ferdie.produce_block_with_slot(slot), alice)
.await
.unwrap();
let consensus_block_hash = ferdie.client.info().best_hash;

// Produce one more bundle, this bundle should contain the ER of the previous bundle
let (_, bundle) = ferdie.produce_slot_and_wait_for_bundle_submission().await;
let receipt = bundle.into_receipt();
assert_eq!(receipt.consensus_block_hash, consensus_block_hash);
assert_eq!(
receipt.block_fees,
// Check the actual block fees for an EVM contract create
BlockFees {
consensus_storage_fee: 789,
domain_execution_fee: 10_815_700_000_631,
burned_balance: 0,
chain_rewards: [].into(),
}
);

produce_blocks!(ferdie, alice, 3).await.unwrap();

let evm_nonce = alice
.client
.runtime_api()
.account_basic(alice.client.info().best_hash, account_infos[0].address)
.unwrap()
.nonce;
let evm_tx = generate_evm_domain_call(
account_infos[0].clone(),
ethereum::TransactionAction::Call(evm_contract_address),
0,
evm_nonce,
gas_price,
);

// Use alice's account ID, which does the signing in construct_and_send_extrinsic()
let result = alice.construct_and_send_extrinsic(evm_tx).await;
assert_matches!(
result,
Ok(_),
"Unexpectedly failed to send signed extrinsic"
);

// Produce a bundle that contains just the sent extrinsics
let (slot, bundle) = ferdie.produce_slot_and_wait_for_bundle_submission().await;
assert_eq!(bundle.extrinsics.len(), 1);
produce_block_with!(ferdie.produce_block_with_slot(slot), alice)
.await
.unwrap();
let consensus_block_hash = ferdie.client.info().best_hash;

// Produce one more bundle, this bundle should contain the ER of the previous bundle
let (_, bundle) = ferdie.produce_slot_and_wait_for_bundle_submission().await;
let receipt = bundle.into_receipt();
assert_eq!(receipt.consensus_block_hash, consensus_block_hash);
assert_eq!(
receipt.block_fees,
// Check the actual block fees for an EVM contract call
BlockFees {
consensus_storage_fee: 849,
domain_execution_fee: 10_815_700_000_651,
burned_balance: 0,
chain_rewards: [].into(),
}
);

produce_blocks!(ferdie, alice, 3).await.unwrap();
}

#[tokio::test(flavor = "multi_thread")]
Expand Down
89 changes: 9 additions & 80 deletions domains/runtime/evm/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ use sp_runtime::transaction_validity::{
};
use sp_runtime::type_with_default::TypeWithDefault;
use sp_runtime::{
generic, impl_opaque_keys, ApplyExtrinsicResult, ArithmeticError, ConsensusEngineId, Digest,
generic, impl_opaque_keys, ApplyExtrinsicResult, ConsensusEngineId, Digest,
ExtrinsicInclusionMode,
};
pub use sp_runtime::{MultiAddress, Perbill, Permill};
Expand Down Expand Up @@ -1593,56 +1593,22 @@ impl_runtime_apis! {

let (weight_limit, proof_size_base_cost) = pallet_ethereum::Pallet::<Runtime>::transaction_weight(&transaction_data);

let mut call_info = <Runtime as pallet_evm::Config>::Runner::call(
<Runtime as pallet_evm::Config>::Runner::call(
from,
to,
data.clone(),
data,
value,
gas_limit.unique_saturated_into(),
max_fee_per_gas,
max_priority_fee_per_gas,
nonce,
access_list.clone().unwrap_or_default(),
access_list.unwrap_or_default(),
is_transactional,
validate,
weight_limit,
proof_size_base_cost,
evm_config,
).map_err(|err| err.error)?;

// Add the storage fee to the estimated gas cost
// (in the actual call, this is handled by OnChargeEVMTransaction)
if estimate {
// It doesn't matter if we use pallet_evm or pallet_ethereum calls here, because
// they will be roughly the same size.
// TODO: try all possibilities, using all 3 ethereum formats, and choose the
// largest as the estimate
let xt = UncheckedExtrinsic::new_bare(
pallet_evm::Call::call {
source: from,
target: to,
input: data,
value,
gas_limit: gas_limit.unique_saturated_into(),
// TODO: use the actual default here (but that shouldn't change the
// extrinsic size)
max_fee_per_gas: max_fee_per_gas.unwrap_or_default(),
max_priority_fee_per_gas,
nonce,
access_list: access_list.unwrap_or_default(),
}.into()
);

let len = xt.encoded_size();
let consensus_storage_fee = consensus_storage_fee(len).map_err(|_| ArithmeticError::Overflow)?;

// TODO: handle the effective gas ratio correctly:
// <https://docs.chain.t3rn.io/fp_evm/struct.UsedGas.html#structfield.effective>
call_info.used_gas.standard += consensus_storage_fee.into();
call_info.used_gas.effective += consensus_storage_fee.into();
}

Ok(call_info)
).map_err(|err| err.error.into())
}

fn create(
Expand All @@ -1669,58 +1635,21 @@ impl_runtime_apis! {
let weight_limit = None;
let proof_size_base_cost = None;
let evm_config = config.as_ref().unwrap_or(<Runtime as pallet_evm::Config>::config());

let mut create_info = <Runtime as pallet_evm::Config>::Runner::create(
<Runtime as pallet_evm::Config>::Runner::create(
from,
data.clone(),
data,
value,
gas_limit.unique_saturated_into(),
max_fee_per_gas,
max_priority_fee_per_gas,
nonce,
access_list.clone().unwrap_or_default(),
access_list.unwrap_or_default(),
is_transactional,
validate,
weight_limit,
proof_size_base_cost,
evm_config,
).map_err(|err| err.error)?;

// Add the storage fee to the estimated gas cost
// (in the actual call, this is handled by OnChargeEVMTransaction)
if estimate {
// It doesn't matter if we use pallet_evm or pallet_ethereum, or create or create2,
// because they will be roughly the same size.
// TODO: try all possibilities, using create/create2 and all 3 ethereum formats,
// and choose the largest as the estimate
let xt = UncheckedExtrinsic::new_bare(
pallet_evm::Call::create2 {
source: from,
init: data,
// TODO: use an actual salt here (but that shouldn't change the extrinsic
// size)
salt: H256::zero(),
value,
gas_limit: gas_limit.unique_saturated_into(),
// TODO: use the actual default here (but that shouldn't change the
// extrinsic size)
max_fee_per_gas: max_fee_per_gas.unwrap_or_default(),
max_priority_fee_per_gas,
nonce,
access_list: access_list.unwrap_or_default(),
}.into()
);

let len = xt.encoded_size();
let consensus_storage_fee = consensus_storage_fee(len).map_err(|_| ArithmeticError::Overflow)?;

// TODO: handle the effective gas ratio correctly:
// <https://docs.chain.t3rn.io/fp_evm/struct.UsedGas.html#structfield.effective>
create_info.used_gas.standard += consensus_storage_fee.into();
create_info.used_gas.effective += consensus_storage_fee.into();
}

Ok(create_info)
).map_err(|err| err.error.into())
}

fn current_transaction_statuses() -> Option<Vec<TransactionStatus>> {
Expand Down
Loading

0 comments on commit 7f51ed8

Please sign in to comment.