Repository: ethereum/web3.py (language: python)

Issue:
A race condition sometimes causes a transaction to have an invalid pair of maxFeePerGas/maxPriorityFeePerGas
When `Web3` is asked to construct a transaction without gas fees set explicitly, the values of the `maxFeePerGas` and `maxPriorityFeePerGas` fields are filled as part of the defaulting process, governed by this code:

https://github.com/ethereum/web3.py/blob/4b509a7d5fce0b9a67dbe93151e8b8a01e83b3cc/web3/_utils/transactions.py#L69-L80

https://github.com/ethereum/web3.py/blob/4b509a7d5fce0b9a67dbe93151e8b8a01e83b3cc/web3/_utils/transactions.py#L103-L137

If a new block is created right as the defaulting process takes place, then `w3.eth.max_priority_fee` will return different values for different fields of the same transaction. This can cause the transaction to end up with `tx['maxFeePerGas'] < tx['maxPriorityFeePerGas']`, which is invalid and will cause it to fail further down the line with something like
```json
{ "jsonrpc": "2.0"
, "id": 1383832
, "error":
    { "code": -32000
    , "message": "max priority fee per gas higher than max fee per gas"
    }
}
```

As for the fix, I propose to change the behavior like this:
1. Swap the order of the fields in the dictionary, so that `"maxPriorityFeePerGas"` goes before `"maxFeePerGas "`. Since Python dictionaries remember the insertion order of their keys and respect it during iteration, changing their order in the `TRANSACTION_DEFAULTS` map will correspondingly change the order the initializers are actually run.
2. In the loop, pass the `defaults` accumulator to the initializers together with `w3` and the initial transaction. Again, since Python guarantees to iterate over maps in the key insertion order, it's perfectly fine to carry state in-between iterations and rely on some actions being done before others.
3. In the `"maxFeePerGas"` initializer, instead of asking for `w3.eth.max_priority_fee` again, use `tx.get('maxPriorityFeePerGas', defaults['maxPriorityFeePerGas'])`.

So, with these considerations, the fix in code would look somewhat like this:
```py
# >>> add 3rd parameter `defs` to the lambda
TRANSACTION_DEFAULTS = {
    "value": 0,
    "data": b"",
    "gas": lambda w3, tx, defs: w3.eth.estimate_gas(tx),
    "gasPrice": lambda w3, tx, defs: w3.eth.generate_gas_price(tx),
# >>> swap the order of `maxFeePerGas` and `maxPriorityFeePerGas`
    "maxPriorityFeePerGas": lambda w3, tx, defs: w3.eth.max_priority_fee,
    # maxFeePerGas depends on maxPriorityFeePerGas having already run
    "maxFeePerGas": lambda w3, tx, defs: (
        tx.get("maxPriorityFeePerGas", defs["maxPriorityFeePerGas"])
        + (2 * w3.eth.get_block("latest")["baseFeePerGas"])
    ),
    "chainId": lambda w3, tx: w3.eth.chain_id,
}

<...>

@curry
def fill_transaction_defaults(w3: "Web3", transaction: TxParams) -> TxParams:
    """
    if w3 is None, fill as much as possible while offline
    """
    strategy_based_gas_price = w3.eth.generate_gas_price(transaction)
    is_dynamic_fee_transaction = strategy_based_gas_price is None and (
        "gasPrice" not in transaction  # default to dynamic fee transaction
        or any_in_dict(DYNAMIC_FEE_TXN_PARAMS, transaction)
    )

    defaults = {}
    for key, default_getter in TRANSACTION_DEFAULTS.items():
        if key not in transaction:
            if (
                is_dynamic_fee_transaction
                and key == "gasPrice"
                or not is_dynamic_fee_transaction
                and key in DYNAMIC_FEE_TXN_PARAMS
            ):
                # do not set default max fees if legacy txn or
                # gas price if dynamic fee txn
                continue

            if callable(default_getter):
                if w3 is None:
                    raise ValueError(
                        f"You must specify a '{key}' value in the transaction"
                    )
# >>> pass current `defaults` to the getter
                default_val = default_getter(w3, transaction, defaults)
            else:
                default_val = default_getter

            defaults[key] = default_val
    return merge(defaults, transaction)
```


Discussion / hints:
On the probability and severity of this bug: we actually encountered it in the wild when working with the Polygon Mumbai testnet. It has two important properties with regards to this bug:
1. New blocks are created relatively often: currently, the average block time on Mumbai is 4 seconds.
2. Specifically on Mumbai, the gas fees are heavily dominated by the priority fee. The base fee here usually stays in the range of 16..20 wei, while the priority fee starts at 1'000'000'000 wei and can jump several times up from there. So, if the priority max and the total max are calculated at different blocks, it's entirely possible to end up with the `maxFeePerGas` much less than the `maxPriorityFeePerGas`.
The failed transaction that we actually got, as an example, had maxFeePerGas of 2.25 Gwei versus maxPriorityFeePerGas of 30 Gwei.

Fix patch:
diff --git a/newsfragments/3052.bugfix.rst b/newsfragments/3052.bugfix.rst
new file mode 100644
index 0000000000..c22aeae155
--- /dev/null
+++ b/newsfragments/3052.bugfix.rst
@@ -0,0 +1,1 @@
+Base default ``maxFeePerGas`` calculation off of existing ``maxPriorityFeePerGas`` key instead of separate RPC call
diff --git a/web3/_utils/async_transactions.py b/web3/_utils/async_transactions.py
index 6dd349e1f7..c3040a8434 100644
--- a/web3/_utils/async_transactions.py
+++ b/web3/_utils/async_transactions.py
@@ -1,6 +1,8 @@
 from typing import (
     TYPE_CHECKING,
+    Dict,
     Optional,
+    Union,
     cast,
 )
 
@@ -42,21 +44,32 @@
     )
 
 
-async def _estimate_gas(async_w3: "AsyncWeb3", tx: TxParams) -> int:
+# unused vars present in these funcs because they all need to have the same signature
+async def _estimate_gas(
+    async_w3: "AsyncWeb3", tx: TxParams, _defaults: Dict[str, Union[bytes, int]]
+) -> int:
     return await async_w3.eth.estimate_gas(tx)
 
 
-async def _max_fee_per_gas(async_w3: "AsyncWeb3", _tx: TxParams) -> Wei:
+async def _max_fee_per_gas(
+    async_w3: "AsyncWeb3", tx: TxParams, defaults: Dict[str, Union[bytes, int]]
+) -> Wei:
     block = await async_w3.eth.get_block("latest")
-    max_priority_fee = await async_w3.eth.max_priority_fee
-    return Wei(max_priority_fee + (2 * block["baseFeePerGas"]))
+    max_priority_fee = tx.get(
+        "maxPriorityFeePerGas", defaults.get("maxPriorityFeePerGas")
+    )
+    return Wei(int(max_priority_fee) + (2 * int(block["baseFeePerGas"])))
 
 
-async def _max_priority_fee_gas(async_w3: "AsyncWeb3", _tx: TxParams) -> Wei:
+async def _max_priority_fee_gas(
+    async_w3: "AsyncWeb3", _tx: TxParams, _defaults: Dict[str, Union[bytes, int]]
+) -> Wei:
     return await async_w3.eth.max_priority_fee
 
 
-async def _chain_id(async_w3: "AsyncWeb3", _tx: TxParams) -> int:
+async def _chain_id(
+    async_w3: "AsyncWeb3", _tx: TxParams, _defaults: Dict[str, Union[bytes, int]]
+) -> int:
     return await async_w3.eth.chain_id
 
 
@@ -64,9 +77,9 @@ async def _chain_id(async_w3: "AsyncWeb3", _tx: TxParams) -> int:
     "value": 0,
     "data": b"",
     "gas": _estimate_gas,
-    "gasPrice": lambda async_w3, tx: async_w3.eth.generate_gas_price(tx),
-    "maxFeePerGas": _max_fee_per_gas,
+    "gasPrice": lambda async_w3, tx, _defaults: async_w3.eth.generate_gas_price(tx),
     "maxPriorityFeePerGas": _max_priority_fee_gas,
+    "maxFeePerGas": _max_fee_per_gas,
     "chainId": _chain_id,
 }
 
@@ -122,7 +135,7 @@ async def async_fill_transaction_defaults(
         or any_in_dict(DYNAMIC_FEE_TXN_PARAMS, transaction)
     )
 
-    defaults = {}
+    defaults: Dict[str, Union[bytes, int]] = {}
     for key, default_getter in TRANSACTION_DEFAULTS.items():
         if key not in transaction:
             if (
@@ -143,9 +156,9 @@ async def async_fill_transaction_defaults(
                 if key == "gasPrice":
                     # `generate_gas_price()` is on the `BaseEth` class and does not
                     # need to be awaited
-                    default_val = default_getter(async_w3, transaction)
+                    default_val = default_getter(async_w3, transaction, defaults)
                 else:
-                    default_val = await default_getter(async_w3, transaction)
+                    default_val = await default_getter(async_w3, transaction, defaults)
             else:
                 default_val = default_getter
 
diff --git a/web3/_utils/transactions.py b/web3/_utils/transactions.py
index cd2f9667ad..fd9746fd3d 100644
--- a/web3/_utils/transactions.py
+++ b/web3/_utils/transactions.py
@@ -1,6 +1,7 @@
 import math
 from typing import (
     TYPE_CHECKING,
+    Dict,
     List,
     Literal,
     Optional,
@@ -75,14 +76,16 @@
 TRANSACTION_DEFAULTS = {
     "value": 0,
     "data": b"",
-    "gas": lambda w3, tx: w3.eth.estimate_gas(tx),
-    "gasPrice": lambda w3, tx: w3.eth.generate_gas_price(tx),
+    "gas": lambda w3, tx, _defaults: w3.eth.estimate_gas(tx),
+    "gasPrice": lambda w3, tx, _defaults: w3.eth.generate_gas_price(tx),
+    "maxPriorityFeePerGas": lambda w3, _tx, _defaults: w3.eth.max_priority_fee,
     "maxFeePerGas": (
-        lambda w3, tx: w3.eth.max_priority_fee
-        + (2 * w3.eth.get_block("latest")["baseFeePerGas"])
+        lambda w3, tx, defaults: (
+            tx.get("maxPriorityFeePerGas", defaults.get("maxPriorityFeePerGas"))
+            + (2 * w3.eth.get_block("latest")["baseFeePerGas"])
+        )
     ),
-    "maxPriorityFeePerGas": lambda w3, tx: w3.eth.max_priority_fee,
-    "chainId": lambda w3, tx: w3.eth.chain_id,
+    "chainId": lambda w3, _tx, _defaults: w3.eth.chain_id,
 }
 
 if TYPE_CHECKING:
@@ -117,7 +120,7 @@ def fill_transaction_defaults(w3: "Web3", transaction: TxParams) -> TxParams:
         or any_in_dict(DYNAMIC_FEE_TXN_PARAMS, transaction)
     )
 
-    defaults = {}
+    defaults: Dict[str, Union[bytes, int]] = {}
     for key, default_getter in TRANSACTION_DEFAULTS.items():
         if key not in transaction:
             if (
@@ -135,7 +138,7 @@ def fill_transaction_defaults(w3: "Web3", transaction: TxParams) -> TxParams:
                     raise Web3ValueError(
                         f"You must specify a '{key}' value in the transaction"
                     )
-                default_val = default_getter(w3, transaction)
+                default_val = default_getter(w3, transaction, defaults)
             else:
                 default_val = default_getter
 


Test patch:
diff --git a/tests/core/utilities/test_async_transaction.py b/tests/core/utilities/test_async_transaction.py
index c63604c7bf..4ec077451d 100644
--- a/tests/core/utilities/test_async_transaction.py
+++ b/tests/core/utilities/test_async_transaction.py
@@ -1,4 +1,8 @@
 import pytest
+from unittest.mock import (
+    AsyncMock,
+    patch,
+)
 
 from eth_typing import (
     BlockNumber,
@@ -15,6 +19,9 @@
 from web3.constants import (
     DYNAMIC_FEE_TXN_PARAMS,
 )
+from web3.eth import (
+    AsyncEth,
+)
 
 SIMPLE_CURRENT_TRANSACTION = {
     "blockHash": None,
@@ -79,6 +86,61 @@ async def test_async_fill_transaction_defaults_nondynamic_tranaction_fee(async_w
     assert none_in_dict(DYNAMIC_FEE_TXN_PARAMS, default_transaction)
 
 
+@pytest.mark.asyncio
+async def test_async_default_max_fee_per_gas_uses_max_priority_fee_if_exists_in_tx(
+    async_w3,
+):
+    fixed_base_fee = 500
+
+    async def get_block_func(block_number):
+        return {"baseFeePerGas": fixed_base_fee}
+
+    get_block_mock = AsyncMock(side_effect=get_block_func)
+
+    with patch.object(async_w3.eth, "get_block", side_effect=get_block_mock):
+        default_transaction = await async_fill_transaction_defaults(
+            async_w3, {"maxPriorityFeePerGas": 100}
+        )
+
+        assert default_transaction == {
+            "chainId": await async_w3.eth.chain_id,
+            "data": b"",
+            "gas": await async_w3.eth.estimate_gas({}),
+            "value": 0,
+            "maxPriorityFeePerGas": 100,
+            "maxFeePerGas": 1100,
+        }
+
+
+@pytest.mark.asyncio
+async def test_async_default_max_fee_per_gas_uses_default_mpfee_per_gas_if_not_in_tx(
+    async_w3,
+):
+    fixed_base_fee = 500
+
+    async def get_block_func(value):
+        return {"baseFeePerGas": fixed_base_fee}
+
+    get_block_mock = AsyncMock(side_effect=get_block_func)
+
+    @property
+    async def max_priority_fee_mock(arg):
+        return 45
+
+    with patch.object(async_w3.eth, "get_block", side_effect=get_block_mock):
+        with patch.object(AsyncEth, "max_priority_fee", new=max_priority_fee_mock):
+            default_transaction = await async_fill_transaction_defaults(async_w3, {})
+
+            assert default_transaction == {
+                "chainId": await async_w3.eth.chain_id,
+                "data": b"",
+                "gas": await async_w3.eth.estimate_gas({}),
+                "value": 0,
+                "maxPriorityFeePerGas": 45,
+                "maxFeePerGas": 1045,
+            }
+
+
 @pytest.mark.asyncio
 async def test_async_fill_transaction_defaults_for_zero_gas_price(async_w3):
     def gas_price_strategy(_w3, tx):
diff --git a/tests/core/utilities/test_valid_transaction_params.py b/tests/core/utilities/test_valid_transaction_params.py
index 7e104d3932..5c585bf106 100644
--- a/tests/core/utilities/test_valid_transaction_params.py
+++ b/tests/core/utilities/test_valid_transaction_params.py
@@ -1,10 +1,16 @@
 import pytest
+from unittest.mock import (
+    patch,
+)
 
 from web3._utils.transactions import (
     assert_valid_transaction_params,
     extract_valid_transaction_params,
     fill_transaction_defaults,
 )
+from web3.eth import (
+    Eth,
+)
 from web3.exceptions import (
     Web3AttributeError,
     Web3ValueError,
@@ -161,6 +167,47 @@ def test_fill_transaction_defaults_for_all_params(w3):
     }
 
 
+def test_default_max_fee_per_gas_uses_max_priority_fee_if_exists_in_tx(w3):
+    fixed_base_fee = 500
+
+    def get_block_mock(block_number):
+        return {"baseFeePerGas": fixed_base_fee}
+
+    with patch.object(w3.eth, "get_block", side_effect=get_block_mock):
+        default_transaction = fill_transaction_defaults(
+            w3, {"maxPriorityFeePerGas": 100}
+        )
+
+        assert default_transaction == {
+            "chainId": w3.eth.chain_id,
+            "data": b"",
+            "gas": w3.eth.estimate_gas({}),
+            "value": 0,
+            "maxPriorityFeePerGas": 100,
+            "maxFeePerGas": 1100,
+        }
+
+
+def test_default_max_fee_per_gas_based_on_default_max_prio_fee_per_gas_if_not_in_tx(w3):
+    fixed_base_fee = 500
+
+    def get_block_mock(value):
+        return {"baseFeePerGas": fixed_base_fee}
+
+    with patch.object(w3.eth, "get_block", side_effect=get_block_mock):
+        with patch.object(Eth, "max_priority_fee", 45):
+            default_transaction = fill_transaction_defaults(w3, {})
+
+            assert default_transaction == {
+                "chainId": w3.eth.chain_id,
+                "data": b"",
+                "gas": w3.eth.estimate_gas({}),
+                "value": 0,
+                "maxPriorityFeePerGas": 45,
+                "maxFeePerGas": 1045,
+            }
+
+
 def test_fill_transaction_defaults_for_zero_gas_price(w3):
     def gas_price_strategy(_w3, tx):
         return 0