Skip to content

Commit

Permalink
Validate gas to weight conversion
Browse files Browse the repository at this point in the history
  • Loading branch information
tgmichel committed Apr 19, 2023
1 parent 3ace0af commit 6deaacb
Show file tree
Hide file tree
Showing 15 changed files with 62 additions and 40 deletions.
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ bn = { package = "substrate-bn", version = "0.6", default-features = false }
environmental = { version = "1.1.3", default-features = false }
ethereum = { version = "0.14.0", default-features = false }
ethereum-types = { version = "0.14.1", default-features = false }
evm = { version = "0.37.0", default-features = false, features = [ "with-substrate" ] }
evm = { version = "0.37.0", default-features = false }
impl-serde = { version = "0.4.0", default-features = false }
jsonrpsee = "0.16.2"
kvdb-rocksdb = "0.17.0"
Expand Down
1 change: 1 addition & 0 deletions frame/ethereum/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -81,3 +81,4 @@ try-runtime = [
"pallet-evm/try-runtime",
]
forbid-evm-reentrancy = ["pallet-evm/forbid-evm-reentrancy"]
evm-with-weight-limit = ["evm/with-substrate"]
18 changes: 18 additions & 0 deletions frame/ethereum/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,24 @@ where
len: usize,
) -> Option<TransactionValidity> {
if let Call::transact { transaction } = self {
// Validate submitted gas limit to weight conversion
let gas_limit = match transaction {
Transaction::Legacy(t) => t.gas_limit,
Transaction::EIP2930(t) => t.gas_limit,
Transaction::EIP1559(t) => t.gas_limit,
};
let without_base_extrinsic_weight = true;
let submitted_weight = T::GasWeightMapping::gas_to_weight(

This comment has been minimized.

Copy link
@librelois

librelois Apr 19, 2023

Contributor

It seems's that the GasWeightMapping trait should be change to modify or add a method that take the encoded transaction size, or a reference to the whole transaction (to compute the encoded size in the impl)

This comment has been minimized.

Copy link
@tgmichel

tgmichel Apr 19, 2023

Author Contributor

Thank you Elois, yes for sure

gas_limit.unique_saturated_into(),
without_base_extrinsic_weight
);
if submitted_weight != dispatch_info.weight {
return Some(Err(
TransactionValidityError::Invalid(InvalidTransaction::Custom(255))
));
}

// CheckWeight
if let Err(e) = CheckWeight::<T>::do_validate(dispatch_info, len) {
return Some(Err(e));
}
Expand Down
1 change: 1 addition & 0 deletions frame/evm/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -76,3 +76,4 @@ try-runtime = [
"pallet-timestamp/try-runtime",
]
forbid-evm-reentrancy = ["dep:environmental"]
evm-with-weight-limit = ["evm/with-substrate"]
1 change: 1 addition & 0 deletions frame/evm/precompile/blake2/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,4 @@ std = [
# Frontier
"fp-evm/std",
]
evm-with-weight-limit = ["pallet-evm-test-vector-support/evm-with-weight-limit"]
1 change: 1 addition & 0 deletions frame/evm/precompile/bn128/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,4 @@ std = [
# Frontier
"fp-evm/std",
]
evm-with-weight-limit = ["pallet-evm-test-vector-support/evm-with-weight-limit"]
2 changes: 2 additions & 0 deletions frame/evm/precompile/dispatch/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ where

let origin = T::AddressMapping::into_account_id(context.caller);

#[cfg(feature = "evm-with-weight-limit")]
handle.record_external_cost(
Some(info.weight.ref_time()),
Some(info.weight.proof_size()),
Expand All @@ -92,6 +93,7 @@ where
let actual_weight = post_info.actual_weight.unwrap_or(info.weight);
let cost = T::GasWeightMapping::weight_to_gas(actual_weight);
handle.record_cost(cost)?;
#[cfg(feature = "evm-with-weight-limit")]
handle.refund_external_cost(
Some(info.weight.ref_time().saturating_sub(actual_weight.ref_time())),
Some(info.weight.proof_size().saturating_sub(actual_weight.proof_size())),
Expand Down
1 change: 1 addition & 0 deletions frame/evm/precompile/modexp/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,4 @@ std = [
# Frontier
"fp-evm/std",
]
evm-with-weight-limit = ["pallet-evm-test-vector-support/evm-with-weight-limit"]
1 change: 1 addition & 0 deletions frame/evm/precompile/simple/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,4 @@ std = [
# Frontier
"fp-evm/std",
]
evm-with-weight-limit = ["pallet-evm-test-vector-support/evm-with-weight-limit"]
3 changes: 3 additions & 0 deletions frame/evm/src/runner/stack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -863,6 +863,7 @@ where
}

// TODO this impl behind a feature flag, otherwise no-op
#[cfg(feature = "evm-with-weight-limit")]
fn record_external_opcode_cost(&mut self, opcode: Opcode, gas_cost: GasCost, target: evm::gasometer::StorageTarget) -> Result<(), ExitError> {
// Return if external costs are disabled
let mut weight_info = if let Some(weight_info) = self.weight_info {
Expand Down Expand Up @@ -970,6 +971,7 @@ where
self.record_external_cost(None, Some(opcode_proof_size.low_u64()))
}

#[cfg(feature = "evm-with-weight-limit")]
fn record_external_cost(&mut self, ref_time: Option<u64>, proof_size: Option<u64>) -> Result<(), ExitError> {
let mut weight_info = if let Some(weight_info) = self.weight_info {
weight_info
Expand All @@ -987,6 +989,7 @@ where
Ok(())
}

#[cfg(feature = "evm-with-weight-limit")]
fn refund_external_cost(&mut self, ref_time: Option<u64>, proof_size: Option<u64>) {
if let Some(mut weight_info) = self.weight_info {
if let Some(amount) = ref_time {
Expand Down
3 changes: 3 additions & 0 deletions frame/evm/test-vector-support/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,6 @@ serde_json = { workspace = true }
sp-core = { workspace = true, features = ["default"] }
# Frontier
fp-evm = { workspace = true, features = ["default"] }

[features]
evm-with-weight-limit = ["evm/with-substrate"]
2 changes: 2 additions & 0 deletions frame/evm/test-vector-support/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,10 +72,12 @@ impl PrecompileHandle for MockHandle {
Ok(())
}

#[cfg(feature = "evm-with-weight-limit")]
fn record_external_cost(&mut self, _: Option<u64>, _: Option<u64>) -> Result<(), ExitError> {
Ok(())
}

#[cfg(feature = "evm-with-weight-limit")]
fn refund_external_cost(&mut self, _: Option<u64>, _: Option<u64>) {}

fn log(&mut self, _: H160, _: Vec<H256>, _: Vec<u8>) -> Result<(), ExitError> {
Expand Down
1 change: 1 addition & 0 deletions template/node/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -93,3 +93,4 @@ runtime-benchmarks = [
"sc-service/runtime-benchmarks",
"frontier-template-runtime/runtime-benchmarks",
]
evm-with-weight-limit = ["frontier-template-runtime/evm-with-weight-limit"]
4 changes: 4 additions & 0 deletions template/runtime/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -122,3 +122,7 @@ runtime-benchmarks = [
"pallet-evm/runtime-benchmarks",
"pallet-hotfix-sufficients/runtime-benchmarks",
]
evm-with-weight-limit = [
"pallet-evm/evm-with-weight-limit",
"pallet-ethereum/evm-with-weight-limit"
]
61 changes: 22 additions & 39 deletions template/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -539,32 +539,6 @@ impl fp_self_contained::SelfContainedCall for RuntimeCall {
}
}

trait EthereumWeigherT {
fn weight_limit(gas_limit: U256, encoded_len: usize) -> Weight;
}

// Something that holds a runtime specific formula that converts Gas limit to Weight limit,
// and that is used for both dispatching and validating.
struct EthereumWeigher;
impl EthereumWeigherT for EthereumWeigher {
// TODO
fn weight_limit(gas_limit: U256, encoded_len: usize) -> Weight {
let gas_to_proof_size_limit = |_gas_limit: U256| -> u64 {
u64::MAX
};

let without_base_extrinsic_weight = true;
let mut weight_limit = <Runtime as pallet_evm::Config>::GasWeightMapping::gas_to_weight({
gas_limit.unique_saturated_into()
}, without_base_extrinsic_weight);

let proof_size_limit = gas_to_proof_size_limit(gas_limit).saturating_add(encoded_len as u64);
*weight_limit.proof_size_mut() = proof_size_limit;

weight_limit
}
}

#[cfg(feature = "runtime-benchmarks")]
#[macro_use]
extern crate frame_benchmarking;
Expand Down Expand Up @@ -799,20 +773,29 @@ impl_runtime_apis! {

impl fp_rpc::ConvertTransactionRuntimeApi<Block> for Runtime {
fn convert_transaction(transaction: EthereumTransaction) -> <Block as BlockT>::Extrinsic {
// TODO Weight v2 type 16 bytes? check scale compact stuff
let encoded_len = transaction.encode().len() + 16usize;

let gas_limit = match &transaction {
EthereumTransaction::Legacy(t) => t.gas_limit,
EthereumTransaction::EIP2930(t) => t.gas_limit,
EthereumTransaction::EIP1559(t) => t.gas_limit,
};

let weight_limit = EthereumWeigher::weight_limit(gas_limit, encoded_len);
let transact_with_weight_limit = pallet_ethereum::Call::<Runtime>::transact_with_weight_limit { transaction, weight_limit };

#[cfg(feature = "evm-with-weight-limit")]
{
// TODO Weight v2 type 16 bytes? check scale compact stuff
let encoded_len = transaction.encode().len() + 16usize;

let gas_limit = match &transaction {
EthereumTransaction::Legacy(t) => t.gas_limit,
EthereumTransaction::EIP2930(t) => t.gas_limit,
EthereumTransaction::EIP1559(t) => t.gas_limit,
};
let weight_limit = <Runtime as pallet_evm::Config>::GasWeightMapping::gas_to_weight(
gas_limit.unique_saturated_into(),
true,
);
let transact_with_weight_limit = pallet_ethereum::Call::<Runtime>::transact_with_weight_limit { transaction, weight_limit };

UncheckedExtrinsic::new_unsigned(
transact_with_weight_limit.into(),
)
}
#[cfg(not(feature = "evm-with-weight-limit"))]
UncheckedExtrinsic::new_unsigned(
transact_with_weight_limit.into(),
pallet_ethereum::Call::<Runtime>::transact { transaction }.into(),
)
}
}
Expand Down

0 comments on commit 6deaacb

Please sign in to comment.