Wrong gas price calculation in eth_send_raw_unsigned_tx could lead to suboptimal transaction pricing

[c4-bot-8]

Lines of code

https://github.com/kkrt-labs/kakarot/blob/7411a5520e8a00be6f5243a50c160e66ad285563/src/kakarot/eth_rpc.cairo#L304-L307

Vulnerability details

Proof of Concept

eth_rpc.cairo::eth_send_raw_unsigned_tx underprices transactions, which could lead to delayed or failed transactions in congested network conditions.

Take a look at this part of the code:

https://github.com/kkrt-labs/kakarot/blob/7411a5520e8a00be6f5243a50c160e66ad285563/src/kakarot/eth_rpc.cairo#L304-L307

let possible_priority_fee = tx.max_fee_per_gas - block_base_fee;
let priority_fee_is_max_priority_fee = is_nn(
    possible_priority_fee - tx.max_priority_fee_per_gas
);
let priority_fee_per_gas = priority_fee_is_max_priority_fee * tx.max_priority_fee_per_gas + (
    1 - priority_fee_is_max_priority_fee
) * possible_priority_fee;
let effective_gas_price = priority_fee_per_gas + block_base_fee;

The problem is in the calculation of priority_fee_per_gas. The logic attempts to choose between tx.max_priority_fee_per_gas and possible_priority_fee, but it does so incorrectly.

The issue is that priority_fee_is_max_priority_fee is set to 1 when possible_priority_fee is greater than or equal to tx.max_priority_fee_per_gas, which is the opposite of what we actually want. This means that when the possible_priority_fee is larger, the function will choose tx.max_priority_fee_per_gas instead of the smaller value.

As a result, in some cases, the priority_fee_per_gas could be set higher/lower than intended leading to users paying more/less in gas fees than necessary.

In most cases, transactions may be underpriced relative to what the user is willing to pay. In congested network conditions, transactions may end up being delayed or failing to be processed cos they're not utilizing the full gas price the user was willing to pay.

Short Note:

I discussed this issue with the sponsors and they confirmed this bug case

Recommended Mitigation Steps

The calculation of priority_fee_is_max_priority_fee should be inverted.

- let priority_fee_is_max_priority_fee = is_nn(
-     possible_priority_fee - tx.max_priority_fee_per_gas
- );
+ let priority_fee_is_max_priority_fee = is_nn(
+     tx.max_priority_fee_per_gas - possible_priority_fee
+ );

Assessed type

Error

[dmvt]

@Rhaydden thank you for flagging this. I've promoted it: code-423n4/2024-09-kakarot-findings#133