Skip to content
Merged
51 changes: 47 additions & 4 deletions client/rpc/src/cache/lru_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,13 +66,19 @@ impl<K: Eq + core::hash::Hash, V: Encode> LRUCacheByteLimited<K, V> {
}
}
pub fn put(&mut self, k: K, v: V) {
// Handle size limit
// If the key is already present, remove it first so its old size is no longer
// counted. Without this, re-inserting an existing key inflates `self.size` and
// causes the eviction loop to throw out unrelated entries unnecessarily, making
// the effective cache capacity much smaller than intended.
if let Some(old_v) = self.cache.remove(&k) {
self.size = self.size.saturating_sub(old_v.encoded_size() as u64);
}

self.size += v.encoded_size() as u64;

while self.size > self.max_size {
if let Some((_, v)) = self.cache.pop_oldest() {
let v_size = v.encoded_size() as u64;
self.size -= v_size;
if let Some((_, evicted_v)) = self.cache.pop_oldest() {
self.size = self.size.saturating_sub(evicted_v.encoded_size() as u64);
} else {
break;
}
Expand Down Expand Up @@ -143,4 +149,41 @@ mod tests {
cache.put(3, "lmn");
assert!(cache.get(&3).is_some());
}

/// Regression test: re-inserting the same key must not inflate `size` and must not
/// evict entries that would still fit within `max_size`.
///
/// Before the fix, `put(k, v2)` when `k` was already cached with `v1` would add
/// `encoded_size(v2)` to `self.size` without subtracting `encoded_size(v1)`. Over
/// time this caused `self.size` to drift far above the actual data in the cache,
/// leading to excessive LRU evictions and a degraded (nearly empty) cache.
///
/// We must replace the *newest* key (key 1). Replacing the oldest (key 0) would
/// evict key 0 first, which accidentally "corrects" size; replacing key 1 evicts
/// key 0 incorrectly when the bug exists.
#[test]
fn test_replace_same_key_does_not_inflate_size() {
// max_size = 10 bytes, "abcd" = 4 bytes, "efgh" = 4 bytes.
let mut cache = LRUCacheByteLimited::new("name", 10, None);

// Fill with two entries that together fit. Order: 0=oldest, 1=newest.
cache.put(0u32, "abcd"); // 4 bytes
cache.put(1u32, "efgh"); // 4 bytes
assert!(cache.get(&0).is_some());
assert!(cache.get(&1).is_some());

// Replace key 1 (newest) with a same-sized value. Before the fix, size grows
// to 12, we evict oldest (key 0), and key 0 is incorrectly gone even though
// both entries still fit.
cache.put(1u32, "wxyz"); // 4 bytes replaces 4 bytes
assert_eq!(
cache.get(&1),
Some(&"wxyz"),
"key 1 should hold the new value"
);
assert!(
cache.get(&0).is_some(),
"key 0 must not be evicted: replacing a same-size value should not grow the cache"
);
}
}
5 changes: 4 additions & 1 deletion client/rpc/src/eth/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,10 @@ impl Geth {
VError::InvalidChainId => "invalid chain id".into(),
VError::InvalidSignature => "invalid sender".into(),
VError::GasLimitTooLow => "intrinsic gas too low".into(),
VError::GasLimitTooHigh => "exceeds block gas limit".into(),
VError::GasLimitExceedsBlockLimit => "exceeds block gas limit".into(),
VError::TransactionGasLimitExceedsCap => {
"exceeds transaction gas limit cap".into()
}
VError::GasPriceTooLow => "gas price less than block base fee".into(),
VError::PriorityFeeTooHigh => {
"max priority fee per gas higher than max fee per gas".into()
Expand Down
66 changes: 46 additions & 20 deletions frame/ethereum/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,8 @@ pub mod pallet {
type PostLogContent: Get<PostLogContent>;
/// The maximum length of the extra data in the Executed event.
type ExtraDataLength: Get<u32>;
/// Whether transactional ethereum calls accept legacy transactions without EIP-155 chain id.
type AllowUnprotectedTxs: Get<bool>;
}

pub mod config_preludes {
Expand All @@ -225,13 +227,15 @@ pub mod pallet {

parameter_types! {
pub const PostBlockAndTxnHashes: PostLogContent = PostLogContent::BlockAndTxnHashes;
pub const AllowUnprotectedTxs: bool = false;
}

#[register_default_impl(TestDefaultConfig)]
impl DefaultConfig for TestDefaultConfig {
type StateRoot = IntermediateStateRoot<Self::Version>;
type PostLogContent = PostBlockAndTxnHashes;
type ExtraDataLength = ConstU32<30>;
type AllowUnprotectedTxs = AllowUnprotectedTxs;
}
}

Expand All @@ -241,7 +245,10 @@ pub mod pallet {
<Pallet<T>>::store_block(
match fp_consensus::find_pre_log(&frame_system::Pallet::<T>::digest()) {
Ok(_) => None,
Err(_) => Some(T::PostLogContent::get()),
Err(fp_consensus::FindLogError::NotFound) => Some(T::PostLogContent::get()),
Err(fp_consensus::FindLogError::MultipleLogs) => {
panic!("multiple pre-runtime Frontier logs; block is invalid")
}
},
U256::from(UniqueSaturatedInto::<u128>::unique_saturated_into(
frame_system::Pallet::<T>::block_number(),
Expand All @@ -263,23 +270,31 @@ pub mod pallet {
fn on_initialize(_: BlockNumberFor<T>) -> Weight {
let mut weight = T::SystemWeightInfo::kill_storage(1);

// If the digest contain an existing ethereum block(encoded as PreLog), If contains,
// execute the imported block firstly and disable transact dispatch function.
if let Ok(log) = fp_consensus::find_pre_log(&frame_system::Pallet::<T>::digest()) {
let PreLog::Block(block) = log;

for transaction in block.transactions {
let source = Self::recover_signer(&transaction).expect(
"pre-block transaction signature invalid; the block cannot be built",
);

Self::validate_transaction_in_block(source, &transaction).expect(
"pre-block transaction verification failed; the block cannot be built",
);
let (r, _) = Self::apply_validated_transaction(source, transaction, None)
.expect("pre-block apply transaction failed; the block cannot be built");
// If the digest contains an existing ethereum block (encoded as PreLog),
// execute the imported block first and disable transact dispatch function.
match fp_consensus::find_pre_log(&frame_system::Pallet::<T>::digest()) {
Ok(log) => {
let PreLog::Block(block) = log;

for transaction in block.transactions {
let source = Self::recover_signer(&transaction).expect(
"pre-block transaction signature invalid; the block cannot be built",
);

Self::validate_transaction_in_block(source, &transaction).expect(
"pre-block transaction verification failed; the block cannot be built",
);
let (r, _) = Self::apply_validated_transaction(source, transaction, None)
.expect(
"pre-block apply transaction failed; the block cannot be built",
);

weight = weight.saturating_add(r.actual_weight.unwrap_or_default());
weight = weight.saturating_add(r.actual_weight.unwrap_or_default());
}
}
Err(fp_consensus::FindLogError::NotFound) => {}
Err(fp_consensus::FindLogError::MultipleLogs) => {
panic!("multiple pre-runtime Frontier logs; block is invalid")
}
}
// Account for `on_finalize` weight:
Expand Down Expand Up @@ -547,9 +562,11 @@ impl<T: Config> Pallet<T> {
CheckEvmTransactionConfig {
evm_config: T::config(),
block_gas_limit: T::BlockGasLimit::get(),
transaction_gas_limit: T::TransactionGasLimit::get(),
base_fee,
chain_id: T::ChainId::get(),
is_transactional: true,
allow_unprotected_txs: T::AllowUnprotectedTxs::get(),
},
transaction_data.clone().into(),
weight_limit,
Expand Down Expand Up @@ -996,9 +1013,11 @@ impl<T: Config> Pallet<T> {
CheckEvmTransactionConfig {
evm_config: T::config(),
block_gas_limit: T::BlockGasLimit::get(),
transaction_gas_limit: T::TransactionGasLimit::get(),
base_fee,
chain_id: T::ChainId::get(),
is_transactional: true,
allow_unprotected_txs: T::AllowUnprotectedTxs::get(),
},
transaction_data.into(),
weight_limit,
Expand Down Expand Up @@ -1121,9 +1140,11 @@ impl From<TransactionValidationError> for InvalidTransactionWrapper {
TransactionValidationError::GasLimitTooLow => InvalidTransactionWrapper(
InvalidTransaction::Custom(TransactionValidationError::GasLimitTooLow as u8),
),
TransactionValidationError::GasLimitTooHigh => InvalidTransactionWrapper(
InvalidTransaction::Custom(TransactionValidationError::GasLimitTooHigh as u8),
),
TransactionValidationError::GasLimitExceedsBlockLimit => {
InvalidTransactionWrapper(InvalidTransaction::Custom(
TransactionValidationError::GasLimitExceedsBlockLimit as u8,
))
}
TransactionValidationError::PriorityFeeTooHigh => InvalidTransactionWrapper(
InvalidTransaction::Custom(TransactionValidationError::PriorityFeeTooHigh as u8),
),
Expand Down Expand Up @@ -1158,6 +1179,11 @@ impl From<TransactionValidationError> for InvalidTransactionWrapper {
TransactionValidationError::AuthorizationListTooLarge as u8,
))
}
TransactionValidationError::TransactionGasLimitExceedsCap => {
InvalidTransactionWrapper(InvalidTransaction::Custom(
TransactionValidationError::TransactionGasLimitExceedsCap as u8,
))
}
TransactionValidationError::UnknownError => InvalidTransactionWrapper(
InvalidTransaction::Custom(TransactionValidationError::UnknownError as u8),
),
Expand Down
45 changes: 44 additions & 1 deletion frame/ethereum/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,8 @@ impl FindAuthor<H160> for FindAuthorTruncated {
parameter_types! {
pub const TransactionByteFee: u64 = 1;
pub const GasLimitStorageGrowthRatio: u64 = 366;
pub TransactionGasLimit: Option<U256> = Some(fp_evm::MAX_TRANSACTION_GAS_LIMIT);
pub static AllowUnprotectedTxs: bool = false;
// Alice is allowed to create contracts via CREATE and CALL(CREATE)
pub AllowedAddressesCreate: Vec<H160> = vec![H160::from_str("0x1a642f0e3c3af545e7acbd38b07251b3990914f1").expect("alice address")];
pub AllowedAddressesCreateInner: Vec<H160> = vec![H160::from_str("0x1a642f0e3c3af545e7acbd38b07251b3990914f1").expect("alice address")];
Expand All @@ -107,12 +109,15 @@ impl pallet_evm::Config for Test {
type PrecompilesValue = ();
type Runner = pallet_evm::runner::stack::Runner<Self>;
type FindAuthor = FindAuthorTruncated;
type TransactionGasLimit = TransactionGasLimit;
type GasLimitStorageGrowthRatio = GasLimitStorageGrowthRatio;
type Timestamp = Timestamp;
}

#[derive_impl(crate::config_preludes::TestDefaultConfig)]
impl Config for Test {}
impl Config for Test {
type AllowUnprotectedTxs = AllowUnprotectedTxs;
}

impl fp_self_contained::SelfContainedCall for RuntimeCall {
type SignedInfo = H160;
Expand Down Expand Up @@ -286,6 +291,17 @@ impl LegacyUnsignedTransaction {
H256::from(keccak_256(&stream.out()))
}

fn unprotected_signing_hash(&self) -> H256 {
let mut stream = RlpStream::new_list(6);
stream.append(&self.nonce);
stream.append(&self.gas_price);
stream.append(&self.gas_limit);
stream.append(&self.action);
stream.append(&self.value);
stream.append(&self.input);
H256::from(keccak_256(&stream.out()))
}

pub fn sign(&self, key: &H256) -> Transaction {
self.sign_with_chain_id(key, ChainId::get())
}
Expand Down Expand Up @@ -316,6 +332,33 @@ impl LegacyUnsignedTransaction {
signature: sig,
})
}

pub fn sign_without_chain_id(&self, key: &H256) -> Transaction {
let hash = self.unprotected_signing_hash();
let msg = libsecp256k1::Message::parse(hash.as_fixed_bytes());
let s = libsecp256k1::sign(
&msg,
&libsecp256k1::SecretKey::parse_slice(&key[..]).unwrap(),
);
let sig = s.0.serialize();

let sig = LegacyTransactionSignature::new(
s.1.serialize() as u64 % 2 + 27,
H256::from_slice(&sig[0..32]),
H256::from_slice(&sig[32..64]),
)
.unwrap();

Transaction::Legacy(ethereum::LegacyTransaction {
nonce: self.nonce,
gas_price: self.gas_price,
gas_limit: self.gas_limit,
action: self.action,
value: self.value,
input: self.input.clone(),
signature: sig,
})
}
}

pub struct EIP2930UnsignedTransaction {
Expand Down
28 changes: 22 additions & 6 deletions frame/ethereum/src/tests/eip1559.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
//! Consensus extension module tests for BABE consensus.

use super::*;
use evm::{ExitReason, ExitRevert, ExitSucceed};
use evm::{ExitError, ExitReason, ExitRevert, ExitSucceed};
use fp_ethereum::{TransactionData, ValidatedTransaction};
use frame_support::{dispatch::DispatchClass, traits::Get, weights::Weight};
use pallet_evm::{AddressMapping, GasWeightMapping};
Expand Down Expand Up @@ -51,12 +51,16 @@ fn transaction_with_max_extrinsic_gas_limit_should_success_pre_dispatch() {
let max_extrinsic_gas =
<Test as pallet_evm::Config>::GasWeightMapping::weight_to_gas(max_extrinsic);

// The effective max gas is the minimum of max_extrinsic_gas and the configured cap.
let transaction_gas_limit = TransactionGasLimit::get().unwrap_or_else(U256::max_value);
let effective_max_gas = U256::from(max_extrinsic_gas).min(transaction_gas_limit);

ext.execute_with(|| {
let transaction = EIP1559UnsignedTransaction {
nonce: U256::zero(),
max_priority_fee_per_gas: U256::from(1),
max_fee_per_gas: U256::from(1),
gas_limit: U256::from(max_extrinsic_gas),
gas_limit: effective_max_gas,
action: ethereum::TransactionAction::Call(bob.address),
value: U256::from(1),
input: Default::default(),
Expand Down Expand Up @@ -574,6 +578,8 @@ fn proof_size_weight_limit_validation_works() {
let alice = &pairs[0];

ext.execute_with(|| {
System::set_block_number(1);

let mut tx = EIP1559UnsignedTransaction {
nonce: U256::from(2),
max_priority_fee_per_gas: U256::zero(),
Expand All @@ -593,11 +599,21 @@ fn proof_size_weight_limit_validation_works() {
// Gas limit cannot afford the extra byte and thus is expected to exhaust.
tx.input = vec![0u8; (weight_limit.proof_size() + 1) as usize];
let tx = tx.sign(&alice.private_key, None);
let transaction_hash = tx.hash();

// Execute
assert!(
Ethereum::transact(RawOrigin::EthereumTransaction(alice.address).into(), tx,).is_err()
);
// Execute - transaction is applied but execution fails with OutOfGas
assert_ok!(Ethereum::apply_validated_transaction(
alice.address,
tx,
None
));
System::assert_last_event(RuntimeEvent::Ethereum(Event::Executed {
from: alice.address,
to: alice.address,
transaction_hash,
exit_reason: ExitReason::Error(ExitError::OutOfGas),
extra_data: vec![],
}));
});
}

Expand Down
22 changes: 17 additions & 5 deletions frame/ethereum/src/tests/eip2930.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
//! Consensus extension module tests for BABE consensus.

use super::*;
use evm::{ExitReason, ExitRevert, ExitSucceed};
use evm::{ExitError, ExitReason, ExitRevert, ExitSucceed};
use fp_ethereum::{TransactionData, ValidatedTransaction};
use frame_support::{
dispatch::{DispatchClass, GetDispatchInfo},
Expand Down Expand Up @@ -501,6 +501,8 @@ fn proof_size_weight_limit_validation_works() {
let alice = &pairs[0];

ext.execute_with(|| {
System::set_block_number(1);

let mut tx = EIP2930UnsignedTransaction {
nonce: U256::from(2),
gas_price: U256::from(1),
Expand All @@ -519,11 +521,21 @@ fn proof_size_weight_limit_validation_works() {
// Gas limit cannot afford the extra byte and thus is expected to exhaust.
tx.input = vec![0u8; (weight_limit.proof_size() + 1) as usize];
let tx = tx.sign(&alice.private_key, None);
let transaction_hash = tx.hash();

// Execute
assert!(
Ethereum::transact(RawOrigin::EthereumTransaction(alice.address).into(), tx,).is_err()
);
// Execute - transaction is applied but execution fails with OutOfGas
assert_ok!(Ethereum::apply_validated_transaction(
alice.address,
tx,
None
));
System::assert_last_event(RuntimeEvent::Ethereum(Event::Executed {
from: alice.address,
to: alice.address,
transaction_hash,
exit_reason: ExitReason::Error(ExitError::OutOfGas),
extra_data: vec![],
}));
});
}

Expand Down
Loading
Loading