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

feat(primitives): effective_gas_tip and effective_tip_per_gas functions together #5144

Merged
merged 12 commits into from
Nov 2, 2023
Merged

feat(primitives): effective_gas_tip and effective_tip_per_gas functions together #5144

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
6cf8493
Update mod.rs
DoTheBestToGetTheBest Oct 23, 2023
8c708bb
Update fees.rs
DoTheBestToGetTheBest Oct 23, 2023
64051ae
Update bundle.rs
DoTheBestToGetTheBest Oct 23, 2023
ccf8505
Update gas_oracle.rs
DoTheBestToGetTheBest Oct 23, 2023
0f3835b
Update traits.rs
DoTheBestToGetTheBest Oct 23, 2023
518bed6
Update lib.rs
DoTheBestToGetTheBest Oct 23, 2023
0402b17
Update mod.rs
DoTheBestToGetTheBest Oct 23, 2023
f0163ea
Update lib.rs
DoTheBestToGetTheBest Oct 23, 2023
f9049e6
Update mod.rs
DoTheBestToGetTheBest Oct 25, 2023
0293455
Update mod.rs
DoTheBestToGetTheBest Oct 27, 2023
f71b5d6
Update mod.rs
DoTheBestToGetTheBest Oct 29, 2023
2569e83
Update crates/primitives/src/transaction/mod.rs
mattsse Nov 2, 2023
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
5 changes: 3 additions & 2 deletions crates/payload/basic/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -803,8 +803,9 @@ where
}));

// update add to total fees
let miner_fee =
tx.effective_tip_per_gas(base_fee).expect("fee is always valid; execution succeeded");
let miner_fee = tx
.effective_tip_per_gas(Some(base_fee))
.expect("fee is always valid; execution succeeded");
total_fees += U256::from(miner_fee) * U256::from(gas_used);

// append transaction to the list of executed transactions
Expand Down
88 changes: 58 additions & 30 deletions crates/primitives/src/transaction/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,50 +305,78 @@ impl Transaction {
}
}

// TODO: dedup with effective_tip_per_gas
/// Determine the effective gas limit for the given transaction and base fee.
/// If the base fee is `None`, the `max_priority_fee_per_gas`, or gas price for non-EIP1559
/// transactions is returned.
///
/// If the `max_fee_per_gas` is less than the base fee, `None` returned.
pub fn effective_gas_tip(&self, base_fee: Option<u64>) -> Option<u128> {
if let Some(base_fee) = base_fee {
let max_fee_per_gas = self.max_fee_per_gas();

if max_fee_per_gas < base_fee as u128 {
None
} else {
let effective_max_fee = max_fee_per_gas - base_fee as u128;
Some(std::cmp::min(effective_max_fee, self.priority_fee_or_price()))
}
} else {
Some(self.priority_fee_or_price())
}
}

// // TODO: dedup with effective_tip_per_gas
mattsse marked this conversation as resolved.
Show resolved Hide resolved
DoTheBestToGetTheBest marked this conversation as resolved.
Show resolved Hide resolved
// /// Determine the effective gas limit for the given transaction and base fee.
// /// If the base fee is `None`, the `max_priority_fee_per_gas`, or gas price for non-EIP1559
// /// transactions is returned.
// ///
// /// If the `max_fee_per_gas` is less than the base fee, `None` returned.
// pub fn effective_gas_tip(&self, base_fee: Option<u64>) -> Option<u128> {
// if let Some(base_fee) = base_fee {
// let max_fee_per_gas = self.max_fee_per_gas();

// if max_fee_per_gas < base_fee as u128 {
// None
// } else {
// let effective_max_fee = max_fee_per_gas - base_fee as u128;
// Some(std::cmp::min(effective_max_fee, self.priority_fee_or_price()))
// }
// } else {
// Some(self.priority_fee_or_price())
// }
// }

// /// Returns the effective miner gas tip cap (`gasTipCap`) for the given base fee:
// /// `min(maxFeePerGas - baseFee, maxPriorityFeePerGas)`
// ///
// /// Returns `None` if the basefee is higher than the [Transaction::max_fee_per_gas].
// pub fn effective_tip_per_gas(&self, base_fee: u64) -> Option<u128> {
// let base_fee = base_fee as u128;
// let max_fee_per_gas = self.max_fee_per_gas();

// if max_fee_per_gas < base_fee {
// return None
// }

// // the miner tip is the difference between the max fee and the base fee or the
// // max_priority_fee_per_gas, whatever is lower

// // SAFETY: max_fee_per_gas >= base_fee
// let fee = max_fee_per_gas - base_fee;

// if let Some(priority_fee) = self.max_priority_fee_per_gas() {
// return Some(fee.min(priority_fee))
// }

// Some(fee)
// }
/// Returns the effective miner gas tip cap (`gasTipCap`) for the given base fee:
/// `min(maxFeePerGas - baseFee, maxPriorityFeePerGas)`
///
/// If the base fee is `None`, the `max_priority_fee_per_gas`, or gas price for non-EIP1559
/// transactions is returned.
///
/// Returns `None` if the basefee is higher than the [Transaction::max_fee_per_gas].
pub fn effective_tip_per_gas(&self, base_fee: u64) -> Option<u128> {
let base_fee = base_fee as u128;
pub fn effective_tip_per_gas(&self, base_fee: Option<u64>) -> Option<u128> {
// Convert the base fee to u128 if present, or provide a default value
let base_fee = base_fee.map_or(0, |fee| fee as u128);
Copy link
Member

Choose a reason for hiding this comment

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

Should use unwrap_or_default here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should use unwrap_or_default here

Hello, thank you for these information. Deleted comments + used unwrap_or_default :)


let max_fee_per_gas = self.max_fee_per_gas();

// Check if max_fee_per_gas is less than base_fee
if max_fee_per_gas < base_fee {
return None
}

// the miner tip is the difference between the max fee and the base fee or the
// max_priority_fee_per_gas, whatever is lower

// SAFETY: max_fee_per_gas >= base_fee
// Calculate the difference between max_fee_per_gas and base_fee
let fee = max_fee_per_gas - base_fee;

// Compare the fee with max_priority_fee_per_gas (or gas price for non-EIP1559 transactions)
if let Some(priority_fee) = self.max_priority_fee_per_gas() {
return Some(fee.min(priority_fee))
Some(fee.min(priority_fee))
} else {
Some(fee)
}

Some(fee)
}

/// Get the transaction's input field.
Expand Down
4 changes: 3 additions & 1 deletion crates/rpc/rpc-types-compat/src/transaction/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,9 @@ fn fill(
// baseFee`
let gas_price = base_fee
.and_then(|base_fee| {
signed_tx.effective_tip_per_gas(base_fee).map(|tip| tip + base_fee as u128)
signed_tx
.effective_tip_per_gas(Some(base_fee))
.map(|tip| tip + base_fee as u128)
})
.unwrap_or_else(|| signed_tx.max_fee_per_gas());

Expand Down
2 changes: 1 addition & 1 deletion crates/rpc/rpc/src/eth/api/fees.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ where

Some(TxGasAndReward {
gas_used,
reward: tx.effective_gas_tip(header.base_fee_per_gas).unwrap_or_default(),
reward: tx.effective_tip_per_gas(header.base_fee_per_gas).unwrap_or_default(),
})
})
.collect::<Vec<_>>();
Expand Down
2 changes: 1 addition & 1 deletion crates/rpc/rpc/src/eth/bundle.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ where
let tx = tx.into_ecrecovered_transaction();
hash_bytes.extend_from_slice(tx.hash().as_slice());
let gas_price = tx
.effective_gas_tip(basefee)
.effective_tip_per_gas(basefee)
.ok_or_else(|| RpcInvalidTransactionError::FeeCapTooLow)?;
tx.try_fill_tx_env(&mut evm.env.tx)?;
let ResultAndState { result, state } = evm.transact()?;
Expand Down
6 changes: 3 additions & 3 deletions crates/rpc/rpc/src/eth/gas_oracle.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,15 +237,15 @@ where
let parent_hash = block.parent_hash;

// sort the functions by ascending effective tip first
block.body.sort_by_cached_key(|tx| tx.effective_gas_tip(base_fee_per_gas));
block.body.sort_by_cached_key(|tx| tx.effective_tip_per_gas(base_fee_per_gas));

let mut prices = Vec::with_capacity(limit);

for tx in block.body.iter() {
let mut effective_gas_tip = None;
// ignore transactions with a tip under the configured threshold
if let Some(ignore_under) = self.ignore_price {
let tip = tx.effective_gas_tip(base_fee_per_gas);
let tip = tx.effective_tip_per_gas(base_fee_per_gas);
effective_gas_tip = Some(tip);
if tip < Some(ignore_under) {
continue
Expand All @@ -262,7 +262,7 @@ where
// a `None` effective_gas_tip represents a transaction where the max_fee_per_gas is
// less than the base fee which would be invalid
let effective_gas_tip = effective_gas_tip
.unwrap_or_else(|| tx.effective_gas_tip(base_fee_per_gas))
.unwrap_or_else(|| tx.effective_tip_per_gas(base_fee_per_gas))
.ok_or(RpcInvalidTransactionError::FeeCapTooLow)?;

prices.push(U256::from(effective_gas_tip));
Expand Down
2 changes: 1 addition & 1 deletion crates/transaction-pool/src/traits.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -938,7 +938,7 @@ impl PoolTransaction for EthPooledTransaction {
/// For EIP-1559 transactions: `min(max_fee_per_gas - base_fee, max_priority_fee_per_gas)`.
/// For legacy transactions: `gas_price - base_fee`.
fn effective_tip_per_gas(&self, base_fee: u64) -> Option<u128> {
self.transaction.effective_tip_per_gas(base_fee)
self.transaction.effective_tip_per_gas(Some(base_fee))
}

/// Returns the max priority fee per gas if the transaction is an EIP-1559 transaction, and
Expand Down