Skip to content

Commit 892a497

Browse files
JereSaloCopilot
andauthored
fix(l1): use 256 bits for gas price in Legacy and EIP-2930 transactions (#4823)
**Motivation** <!-- Why does this pull request exist? What are its goals? --> **Description** <!-- A clear and concise general description of the changes this PR introduces --> Spec defines that legacy and eip 2930 txs should use 256 bits, we were using 64 bits. <!-- Link to issues: Resolves #111, Resolves #222 --> Closes #3629 --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent 6fedd57 commit 892a497

9 files changed

Lines changed: 47 additions & 48 deletions

File tree

crates/common/types/transaction.rs

Lines changed: 32 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,8 @@ impl RLPDecode for WrappedEIP4844Transaction {
163163
#[derive(Clone, Debug, PartialEq, Eq, Default, RSerialize, RDeserialize, Archive)]
164164
pub struct LegacyTransaction {
165165
pub nonce: u64,
166-
pub gas_price: u64,
166+
#[rkyv(with=crate::rkyv_utils::U256Wrapper)]
167+
pub gas_price: U256,
167168
pub gas: u64,
168169
/// The recipient of the transaction.
169170
/// Create transactions contain a [`null`](RLP_NULL) value in this field.
@@ -186,7 +187,8 @@ pub struct LegacyTransaction {
186187
pub struct EIP2930Transaction {
187188
pub chain_id: u64,
188189
pub nonce: u64,
189-
pub gas_price: u64,
190+
#[rkyv(with=crate::rkyv_utils::U256Wrapper)]
191+
pub gas_price: U256,
190192
pub gas_limit: u64,
191193
pub to: TxKind,
192194
#[rkyv(with=crate::rkyv_utils::U256Wrapper)]
@@ -351,20 +353,19 @@ impl Transaction {
351353
}
352354
}
353355

354-
fn calc_effective_gas_price(&self, base_fee_per_gas: Option<u64>) -> Option<u64> {
355-
if self.max_fee_per_gas()? < base_fee_per_gas? {
356+
fn calc_effective_gas_price(&self, base_fee_per_gas: Option<u64>) -> Option<U256> {
357+
let base_fee = base_fee_per_gas?;
358+
let max_fee = self.max_fee_per_gas()?;
359+
if max_fee < base_fee {
356360
// This is invalid, can't calculate
357361
return None;
358362
}
359363

360-
let priority_fee_per_gas = min(
361-
self.max_priority_fee()?,
362-
self.max_fee_per_gas()?.saturating_sub(base_fee_per_gas?),
363-
);
364-
Some(priority_fee_per_gas + base_fee_per_gas?)
364+
let priority_fee_per_gas = min(self.max_priority_fee()?, max_fee.saturating_sub(base_fee));
365+
Some(U256::from(priority_fee_per_gas) + U256::from(base_fee))
365366
}
366367

367-
pub fn effective_gas_price(&self, base_fee_per_gas: Option<u64>) -> Option<u64> {
368+
pub fn effective_gas_price(&self, base_fee_per_gas: Option<u64>) -> Option<U256> {
368369
match self.tx_type() {
369370
TxType::Legacy => Some(self.gas_price()),
370371
TxType::EIP2930 => Some(self.gas_price()),
@@ -379,14 +380,14 @@ impl Transaction {
379380
let price = match self.tx_type() {
380381
TxType::Legacy => self.gas_price(),
381382
TxType::EIP2930 => self.gas_price(),
382-
TxType::EIP1559 => self.max_fee_per_gas()?,
383-
TxType::EIP4844 => self.max_fee_per_gas()?,
384-
TxType::EIP7702 => self.max_fee_per_gas()?,
383+
TxType::EIP1559 => U256::from(self.max_fee_per_gas()?),
384+
TxType::EIP4844 => U256::from(self.max_fee_per_gas()?),
385+
TxType::EIP7702 => U256::from(self.max_fee_per_gas()?),
385386
TxType::Privileged => self.gas_price(),
386387
};
387388

388389
Some(U256::saturating_add(
389-
U256::saturating_mul(price.into(), self.gas_limit().into()),
390+
U256::saturating_mul(price, self.gas_limit().into()),
390391
self.value(),
391392
))
392393
}
@@ -1067,14 +1068,15 @@ impl Transaction {
10671068
}
10681069
}
10691070

1070-
pub fn gas_price(&self) -> u64 {
1071+
//TODO: It's not very correct to return gas price for legacy and eip-2930 txs but return the max fee per gas for the others, make necessary changes for it to be technically correct.
1072+
pub fn gas_price(&self) -> U256 {
10711073
match self {
10721074
Transaction::LegacyTransaction(tx) => tx.gas_price,
10731075
Transaction::EIP2930Transaction(tx) => tx.gas_price,
1074-
Transaction::EIP1559Transaction(tx) => tx.max_fee_per_gas,
1075-
Transaction::EIP7702Transaction(tx) => tx.max_fee_per_gas,
1076-
Transaction::EIP4844Transaction(tx) => tx.max_fee_per_gas,
1077-
Transaction::PrivilegedL2Transaction(tx) => tx.max_fee_per_gas,
1076+
Transaction::EIP1559Transaction(tx) => U256::from(tx.max_fee_per_gas),
1077+
Transaction::EIP7702Transaction(tx) => U256::from(tx.max_fee_per_gas),
1078+
Transaction::EIP4844Transaction(tx) => U256::from(tx.max_fee_per_gas),
1079+
Transaction::PrivilegedL2Transaction(tx) => U256::from(tx.max_fee_per_gas),
10781080
}
10791081
}
10801082

@@ -1236,11 +1238,11 @@ impl Transaction {
12361238
}
12371239

12381240
pub fn gas_tip_cap(&self) -> u64 {
1239-
self.max_priority_fee().unwrap_or(self.gas_price())
1241+
self.max_priority_fee().unwrap_or(self.gas_price().as_u64())
12401242
}
12411243

12421244
pub fn gas_fee_cap(&self) -> u64 {
1243-
self.max_fee_per_gas().unwrap_or(self.gas_price())
1245+
self.max_fee_per_gas().unwrap_or(self.gas_price().as_u64())
12441246
}
12451247

12461248
pub fn effective_gas_tip(&self, base_fee: Option<u64>) -> Option<u64> {
@@ -1941,7 +1943,7 @@ mod serde_impl {
19411943

19421944
Ok(LegacyTransaction {
19431945
nonce: deserialize_field::<U256, D>(&mut map, "nonce")?.as_u64(),
1944-
gas_price: deserialize_field::<U256, D>(&mut map, "gasPrice")?.as_u64(),
1946+
gas_price: deserialize_field::<U256, D>(&mut map, "gasPrice")?,
19451947
gas: deserialize_field::<U256, D>(&mut map, "gas")?.as_u64(),
19461948
to: deserialize_field::<TxKind, D>(&mut map, "to")?,
19471949
value: deserialize_field::<U256, D>(&mut map, "value")?,
@@ -1964,7 +1966,7 @@ mod serde_impl {
19641966
Ok(EIP2930Transaction {
19651967
chain_id: deserialize_field::<U256, D>(&mut map, "chainId")?.as_u64(),
19661968
nonce: deserialize_field::<U256, D>(&mut map, "nonce")?.as_u64(),
1967-
gas_price: deserialize_field::<U256, D>(&mut map, "gasPrice")?.as_u64(),
1969+
gas_price: deserialize_field::<U256, D>(&mut map, "gasPrice")?,
19681970
gas_limit: deserialize_field::<U256, D>(&mut map, "gas")?.as_u64(),
19691971
to: deserialize_field::<TxKind, D>(&mut map, "to")?,
19701972
value: deserialize_field::<U256, D>(&mut map, "value")?,
@@ -2452,7 +2454,7 @@ mod serde_impl {
24522454
from: Address::default(),
24532455
gas: Some(value.gas),
24542456
value: value.value,
2455-
gas_price: value.gas_price,
2457+
gas_price: value.gas_price.as_u64(),
24562458
max_priority_fee_per_gas: None,
24572459
max_fee_per_gas: None,
24582460
max_fee_per_blob_gas: None,
@@ -2475,7 +2477,7 @@ mod serde_impl {
24752477
from: Address::default(),
24762478
gas: Some(value.gas_limit),
24772479
value: value.value,
2478-
gas_price: value.gas_price,
2480+
gas_price: value.gas_price.as_u64(),
24792481
max_priority_fee_per_gas: None,
24802482
max_fee_per_gas: None,
24812483
max_fee_per_blob_gas: None,
@@ -2626,7 +2628,7 @@ mod tests {
26262628
let mut body = BlockBody::empty();
26272629
let tx = LegacyTransaction {
26282630
nonce: 0,
2629-
gas_price: 0x0a,
2631+
gas_price: U256::from(0x0a),
26302632
gas: 0x05f5e100,
26312633
to: TxKind::Call(hex!("1000000000000000000000000000000000000000").into()),
26322634
value: 0.into(),
@@ -2653,7 +2655,7 @@ mod tests {
26532655
let tx_eip2930 = EIP2930Transaction {
26542656
chain_id: 3503995874084926u64,
26552657
nonce: 7,
2656-
gas_price: 0x2dbf1f9a,
2658+
gas_price: U256::from(0x2dbf1f9a_u64),
26572659
gas_limit: 0x186A0,
26582660
to: TxKind::Call(hex!("7dcd17433742f4c0ca53122ab541d0ba67fc27df").into()),
26592661
value: 2.into(),
@@ -2707,7 +2709,7 @@ mod tests {
27072709
let tx = LegacyTransaction::decode(&encoded_tx_bytes).unwrap();
27082710
let expected_tx = LegacyTransaction {
27092711
nonce: 0,
2710-
gas_price: 1001000000,
2712+
gas_price: U256::from(1001000000u64),
27112713
gas: 63000,
27122714
to: TxKind::Call(Address::from_slice(
27132715
&hex::decode("6177843db3138ae69679A54b95cf345ED759450d").unwrap(),
@@ -3065,7 +3067,7 @@ mod tests {
30653067
fn test_legacy_transaction_into_generic() {
30663068
let legacy_tx = LegacyTransaction {
30673069
nonce: 1,
3068-
gas_price: 20_000_000_000,
3070+
gas_price: U256::from(20_000_000_000u64),
30693071
gas: 21000,
30703072
to: TxKind::Call(
30713073
Address::from_str("0x742d35Cc6634C0532925a3b844Bc454e4438f44e").unwrap(),
@@ -3104,7 +3106,7 @@ mod tests {
31043106
let eip2930_tx = EIP2930Transaction {
31053107
chain_id: 1,
31063108
nonce: 1,
3107-
gas_price: 20_000_000_000,
3109+
gas_price: U256::from(20_000_000_000u64),
31083110
gas_limit: 21000,
31093111
to: TxKind::Call(
31103112
Address::from_str("0x742d35Cc6634C0532925a3b844Bc454e4438f44e").unwrap(),

crates/networking/rpc/eth/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ pub mod test_utils {
110110
fn legacy_tx_for_test(nonce: u64) -> Transaction {
111111
Transaction::LegacyTransaction(LegacyTransaction {
112112
nonce,
113-
gas_price: nonce * BASE_PRICE_IN_WEI,
113+
gas_price: U256::from(nonce) * U256::from(BASE_PRICE_IN_WEI),
114114
gas: 10000,
115115
to: TxKind::Create,
116116
value: 100.into(),

crates/networking/rpc/types/receipt.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -177,12 +177,12 @@ impl RpcReceiptTxInfo {
177177
let nonce = transaction.nonce();
178178
let from = transaction.sender()?;
179179
let transaction_hash = transaction.hash();
180-
let effective_gas_price =
181-
transaction
182-
.effective_gas_price(base_fee_per_gas)
183-
.ok_or(RpcErr::Internal(
184-
"Could not get effective gas price from tx".into(),
185-
))?;
180+
let effective_gas_price = transaction
181+
.effective_gas_price(base_fee_per_gas)
182+
.ok_or(RpcErr::Internal(
183+
"Could not get effective gas price from tx".into(),
184+
))?
185+
.as_u64();
186186
let transaction_index = index;
187187
let (blob_gas_price, blob_gas_used) = match &transaction {
188188
Transaction::EIP4844Transaction(tx) => (

crates/vm/backends/levm/mod.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,8 +90,7 @@ impl LEVM {
9090
.effective_gas_price(block_header.base_fee_per_gas)
9191
.ok_or(VMError::TxValidation(
9292
TxValidationError::InsufficientMaxFeePerGas,
93-
))?
94-
.into();
93+
))?;
9594

9695
let config = EVMConfig::new_from_chain_config(&chain_config, block_header);
9796
let env = Environment {

crates/vm/levm/runner/src/input.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ impl From<InputTransaction> for ethrex_common::types::LegacyTransaction {
9393
fn from(tx: InputTransaction) -> Self {
9494
ethrex_common::types::LegacyTransaction {
9595
nonce: 0,
96-
gas_price: tx.gas_price.try_into().unwrap(),
96+
gas_price: tx.gas_price,
9797
gas: tx.gas_limit,
9898
to: match tx.to {
9999
Some(address) => ethrex_common::types::TxKind::Call(address),

tooling/ef_tests/blockchain/tests/all.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,6 @@ const SKIPPED_BASE: &[&str] = &[
1414
"static_Call50000_sha256",
1515
"CALLBlake2f_MaxRounds",
1616
"loopMul",
17-
// Gas price higher than u64::MAX; impractical scenario. Fix is on its way on https://github.com/lambdaclass/ethrex/pull/4823
18-
"HighGasPriceParis",
1917
// Skip because it tries to deserialize number > U256::MAX
2018
"ValueOverflowParis",
2119
// Skip because it's a "Create" Blob Transaction, which doesn't actually exist. It never reaches the EVM because we can't even parse it as an actual Transaction.

tooling/ef_tests/blockchain/types.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -514,7 +514,7 @@ impl From<Transaction> for LegacyTransaction {
514514
fn from(val: Transaction) -> Self {
515515
LegacyTransaction {
516516
nonce: val.nonce.as_u64(),
517-
gas_price: val.gas_price.unwrap_or_default().as_u64(), // TODO: Consider converting this into Option
517+
gas_price: val.gas_price.unwrap_or_default(), // TODO: Consider converting this into Option
518518
gas: val.gas_limit.as_u64(),
519519
// to: match val.to {
520520
// zero if zero == H160::zero() => TxKind::Create,
@@ -536,7 +536,7 @@ impl From<Transaction> for EIP2930Transaction {
536536
EIP2930Transaction {
537537
chain_id: val.chain_id.map(|id: U256| id.as_u64()).unwrap_or(1),
538538
nonce: val.nonce.as_u64(),
539-
gas_price: val.gas_price.unwrap_or_default().as_u64(),
539+
gas_price: val.gas_price.unwrap_or_default(),
540540
gas_limit: val.gas_limit.as_u64(),
541541
// to: match val.to {
542542
// zero if zero == H160::zero() => TxKind::Create,

tooling/ef_tests/state_v2/src/modules/runner.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,7 @@ pub async fn get_tx_from_test_case(test_case: &TestCase) -> Result<Transaction,
221221
Transaction::EIP2930Transaction(EIP2930Transaction {
222222
chain_id,
223223
nonce,
224-
gas_price: test_case.gas_price.unwrap().as_u64(),
224+
gas_price: test_case.gas_price.unwrap(),
225225
gas_limit: test_case.gas,
226226
to,
227227
value,
@@ -232,7 +232,7 @@ pub async fn get_tx_from_test_case(test_case: &TestCase) -> Result<Transaction,
232232
} else {
233233
Transaction::LegacyTransaction(LegacyTransaction {
234234
nonce,
235-
gas_price: test_case.gas_price.unwrap().as_u64(),
235+
gas_price: test_case.gas_price.unwrap(),
236236
gas: test_case.gas,
237237
to,
238238
value,

tooling/migrations/src/utils.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ pub fn migrate_transaction(tx: LibmdbxTransaction) -> Transaction {
8282
LibmdbxTransaction::LegacyTransaction(tx) => {
8383
Transaction::LegacyTransaction(LegacyTransaction {
8484
nonce: tx.nonce,
85-
gas_price: tx.gas_price,
85+
gas_price: tx.gas_price.into(),
8686
gas: tx.gas,
8787
to: match tx.to {
8888
LibmdbxTxKind::Create => TxKind::Create,
@@ -100,7 +100,7 @@ pub fn migrate_transaction(tx: LibmdbxTransaction) -> Transaction {
100100
Transaction::EIP2930Transaction(EIP2930Transaction {
101101
chain_id: tx.chain_id,
102102
nonce: tx.nonce,
103-
gas_price: tx.gas_price,
103+
gas_price: tx.gas_price.into(),
104104
gas_limit: tx.gas_limit,
105105
to: match tx.to {
106106
LibmdbxTxKind::Create => TxKind::Create,

0 commit comments

Comments
 (0)