Repository: ethereum/web3.py

Issue:
construct_sign_and_send_raw_middleware uses unbuffered gas estimation following PR #1999.
After the change that made buffered gas estimate a middleware 
https://github.com/ethereum/web3.py/commit/17264b423f24a6c4e88f690391d5e9fc56bb0714
Adding the construct_sign_and_send_raw_middleware using w3.middleware_onion.add like the example
in the documentation:
https://web3py.readthedocs.io/en/stable/middleware.html#web3.middleware.construct_sign_and_send_raw_middleware
causes the signing middleware to run before the buffered gas estimate middleware and use unbuffered gas estimation that is computed here:
https://github.com/ethereum/web3.py/blob/v5.23.1/web3/_utils/transactions.py#L67

This causes sporadic out of gas failures.

injecting construct_sign_and_send_raw_middleware as the innermost middleware does not work well with the exception_retry_middleware.




Discussion:
pacrob: We have the same suggestion for ExtraDataToPOAMiddleware:

It’s important to inject the middleware at the 0th layer of the middleware onion: w3.middleware_onion.inject(ExtraDataToPOAMiddleware, layer=0)

so should probably clarify if using both.

Patch:
diff --git a/docs/middleware.rst b/docs/middleware.rst
index 25c9f9e0..555a6306 100644
--- a/docs/middleware.rst
+++ b/docs/middleware.rst
@@ -319,9 +319,13 @@ Proof of Authority
 
 .. py:class:: web3.middleware.ExtraDataToPOAMiddleware
 
-.. note::
-    It's important to inject the middleware at the 0th layer of the middleware onion:
-    ``w3.middleware_onion.inject(ExtraDataToPOAMiddleware, layer=0)``
+.. important::
+    It is **crucial** that this middleware is injected at the 0th layer of the
+    middleware onion, using
+    ``w3.middleware_onion.inject(ExtraDataToPOAMiddleware, layer=0)``, to guarantee
+    it is the *first* middleware to process the response and modify the ``extraData``
+    field. This ensures it processes the field before any other middleware attempts
+    to validate it.
 
 ``ExtraDataToPOAMiddleware`` is required to connect to ``geth --dev`` and may
 also be needed for other EVM compatible blockchains like Polygon or BNB Chain
@@ -423,6 +427,21 @@ The ``build`` method for this middleware builder takes a single argument:
       * An ``eth_keys.PrivateKey`` object
       * A raw private key as a hex string or byte string
 
+.. important::
+    Since this middleware signs transactions, it must always run after any middleware
+    that modifies the transaction. Therefore, it is recommended to inject the signing
+    middleware at the 0th layer of the middleware onion using
+    ``w3.middleware_onion.inject(SignAndSendRawMiddlewareBuilder.build(...), layer=0)``.
+    Ensure that any transaction-modifying middleware exists in a higher layer within the
+    onion so that it runs before the signing middleware.
+
+.. note::
+    If used with ``ExtraDataToPOAMiddleware``, the injection order doesn't matter, as
+    the ``extraData`` field isn't involved in transaction signing. The key is ensuring
+    ``SignAndSendRawMiddlewareBuilder`` runs after any middleware that modifies the
+    transaction.
+
+
 .. code-block:: python
 
    >>> from web3 import Web3, EthereumTesterProvider
@@ -430,7 +449,7 @@ The ``build`` method for this middleware builder takes a single argument:
    >>> from web3.middleware import SignAndSendRawMiddlewareBuilder
    >>> from eth_account import Account
    >>> acct = Account.create('KEYSMASH FJAFJKLDSKF7JKFDJ 1530')
-   >>> w3.middleware_onion.add(SignAndSendRawMiddlewareBuilder.build(acct))
+   >>> w3.middleware_onion.inject(SignAndSendRawMiddlewareBuilder.build(acct), layer=0)
    >>> w3.eth.default_account = acct.address
 
 :ref:`Hosted nodes<local_vs_hosted>` (like Infura or Alchemy) only support signed
@@ -446,7 +465,7 @@ Instead, we can automate this process with
     >>> from eth_account import Account
     >>> import os
     >>> acct = w3.eth.account.from_key(os.environ.get('PRIVATE_KEY'))
-    >>> w3.middleware_onion.add(SignAndSendRawMiddlewareBuilder.build(acct))
+    >>> w3.middleware_onion.inject(SignAndSendRawMiddlewareBuilder.build(acct), layer=0)
     >>> w3.eth.default_account = acct.address
 
     >>> # use `eth_sendTransaction` to automatically sign and send the raw transaction
@@ -463,7 +482,7 @@ Similarly, with AsyncWeb3:
     >>> from eth_account import Account
     >>> import os
     >>> acct = async_w3.eth.account.from_key(os.environ.get('PRIVATE_KEY'))
-    >>> async_w3.middleware_onion.add(SignAndSendRawMiddlewareBuilder.build(acct))
+    >>> async_w3.middleware_onion.inject(SignAndSendRawMiddlewareBuilder.build(acct), layer=0)
     >>> async_w3.eth.default_account = acct.address
 
     >>> # use `eth_sendTransaction` to automatically sign and send the raw transaction
diff --git a/docs/migration.rst b/docs/migration.rst
index 83271c63..91794158 100644
--- a/docs/migration.rst
+++ b/docs/migration.rst
@@ -128,7 +128,7 @@ with the necessary parameters via the ``build()`` method.
 
     # v7
     from web3.middleware import SignAndSendRawMiddlewareBuilder
-    w3.middleware_onion.add(SignAndSendRawMiddlewareBuilder.build(private_key))
+    w3.middleware_onion.inject(SignAndSendRawMiddlewareBuilder.build(private_key), layer=0)
 
 
 Middleware Renaming and Removals
diff --git a/docs/transactions.rst b/docs/transactions.rst
index 6a764b47..b0066b29 100644
--- a/docs/transactions.rst
+++ b/docs/transactions.rst
@@ -90,8 +90,8 @@ utilize web3.py middleware to sign transactions from a particular account:
   })
 
   # Add acct2 as auto-signer:
-  w3.middleware_onion.add(SignAndSendRawMiddlewareBuilder.build(acct2))
-  # pk also works: w3.middleware_onion.add(SignAndSendRawMiddlewareBuilder.build(pk))
+  w3.middleware_onion.inject(SignAndSendRawMiddlewareBuilder.build(acct2), layer=0)
+  # pk also works: w3.middleware_onion.inject(SignAndSendRawMiddlewareBuilder.build(pk), layer=0)
 
   # Transactions from `acct2` will then be signed, under the hood, in the middleware:
   tx_hash = w3.eth.send_transaction({
diff --git a/docs/web3.eth.account.rst b/docs/web3.eth.account.rst
index 7d2720bb..ad0cec45 100644
--- a/docs/web3.eth.account.rst
+++ b/docs/web3.eth.account.rst
@@ -151,7 +151,7 @@ Example ``account_test_script.py``
     assert private_key.startswith("0x"), "Private key must start with 0x hex prefix"
 
     account: LocalAccount = Account.from_key(private_key)
-    w3.middleware_onion.add(SignAndSendRawMiddlewareBuilder.build(account))
+    w3.middleware_onion.inject(SignAndSendRawMiddlewareBuilder.build(account), layer=0)
 
     print(f"Your hot wallet address is {account.address}")
 
diff --git a/newsfragments/3488.docs.rst b/newsfragments/3488.docs.rst
new file mode 100644
index 00000000..36667c0b
--- /dev/null
+++ b/newsfragments/3488.docs.rst
@@ -0,0 +1,1 @@
+Update documentation around ``SignAndSendRawMiddlewareBuilder`` injection into the middleware onion. It should be injected lower in the stack than any middleware that modifies the transaction, in order to ensure those modified fields are signed.
diff --git a/web3/_utils/async_transactions.py b/web3/_utils/async_transactions.py
index c3040a84..857fc5a5 100644
--- a/web3/_utils/async_transactions.py
+++ b/web3/_utils/async_transactions.py
@@ -87,9 +87,7 @@ TRANSACTION_DEFAULTS = {
 async def get_block_gas_limit(
     web3_eth: "AsyncEth", block_identifier: Optional[BlockIdentifier] = None
 ) -> int:
-    if block_identifier is None:
-        block_identifier = await web3_eth.block_number
-    block = await web3_eth.get_block(block_identifier)
+    block = await web3_eth.get_block(block_identifier or "latest")
     return block["gasLimit"]
 
 
@@ -104,8 +102,8 @@ async def get_buffered_gas_estimate(
 
     if gas_estimate > gas_limit:
         raise Web3ValueError(
-            "Contract does not appear to be deployable within the "
-            f"current network gas limits.  Estimated: {gas_estimate}. "
+            "Gas estimate for transaction is higher than current network gas limits. "
+            f"Transaction could not be sent. Estimated: {gas_estimate}. "
             f"Current gas limit: {gas_limit}"
         )
 
diff --git a/web3/_utils/transactions.py b/web3/_utils/transactions.py
index fd9746fd..e2ffc7b5 100644
--- a/web3/_utils/transactions.py
+++ b/web3/_utils/transactions.py
@@ -149,9 +149,7 @@ def fill_transaction_defaults(w3: "Web3", transaction: TxParams) -> TxParams:
 def get_block_gas_limit(
     w3: "Web3", block_identifier: Optional[BlockIdentifier] = None
 ) -> int:
-    if block_identifier is None:
-        block_identifier = w3.eth.block_number
-    block = w3.eth.get_block(block_identifier)
+    block = w3.eth.get_block(block_identifier or "latest")
     return block["gasLimit"]
 
 
@@ -166,8 +164,8 @@ def get_buffered_gas_estimate(
 
     if gas_estimate > gas_limit:
         raise Web3ValueError(
-            "Contract does not appear to be deployable within the "
-            f"current network gas limits.  Estimated: {gas_estimate}. "
+            "Gas estimate for transaction is higher than current network gas limits. "
+            f"Transaction could not be sent. Estimated: {gas_estimate}. "
             f"Current gas limit: {gas_limit}"
         )
 


Test patch:
diff --git a/tests/core/middleware/test_transaction_signing.py b/tests/core/middleware/test_transaction_signing.py
index 16855f08..ee1030b9 100644
--- a/tests/core/middleware/test_transaction_signing.py
+++ b/tests/core/middleware/test_transaction_signing.py
@@ -1,5 +1,8 @@
 import pytest
 
+from eth.vm.forks.london.transactions import (
+    DynamicFeeTransaction,
+)
 from eth_account import (
     Account,
 )
@@ -27,6 +30,7 @@ from hexbytes import (
     HexBytes,
 )
 import pytest_asyncio
+import rlp
 
 from web3 import (
     AsyncWeb3,
@@ -36,6 +40,7 @@ from web3.exceptions import (
     InvalidAddress,
 )
 from web3.middleware import (
+    BufferedGasEstimateMiddleware,
     SignAndSendRawMiddlewareBuilder,
 )
 from web3.middleware.signing import (
@@ -254,7 +259,9 @@ TEST_SIGNED_TRANSACTION_PARAMS = (
     TEST_SIGN_AND_SEND_RAW_MIDDLEWARE_PARAMS,
 )
 def test_sign_and_send_raw_middleware(w3_dummy, method, from_, expected, key_object):
-    w3_dummy.middleware_onion.add(SignAndSendRawMiddlewareBuilder.build(key_object))
+    w3_dummy.middleware_onion.inject(
+        SignAndSendRawMiddlewareBuilder.build(key_object), layer=0
+    )
 
     legacy_transaction = {
         "to": "0x7E5F4552091A69125d5DfCb7b8C2659029395Bdf",
@@ -364,7 +371,9 @@ def fund_account(w3):
     ],
 )
 def test_signed_transaction(w3, fund_account, transaction, expected, key_object, from_):
-    w3.middleware_onion.add(SignAndSendRawMiddlewareBuilder.build(key_object))
+    w3.middleware_onion.inject(
+        SignAndSendRawMiddlewareBuilder.build(key_object), layer=0
+    )
 
     # Drop any falsy addresses
     to_from = valfilter(bool, {"to": w3.eth.default_account, "from": from_})
@@ -398,7 +407,9 @@ def test_sign_and_send_raw_middleware_with_byte_addresses(
     from_ = from_converter(ADDRESS_1)
     to_ = to_converter(ADDRESS_2)
 
-    w3_dummy.middleware_onion.add(SignAndSendRawMiddlewareBuilder.build(private_key))
+    w3_dummy.middleware_onion.inject(
+        SignAndSendRawMiddlewareBuilder.build(private_key), layer=0
+    )
 
     actual = w3_dummy.manager.request_blocking(
         "eth_sendTransaction",
@@ -419,6 +430,47 @@ def test_sign_and_send_raw_middleware_with_byte_addresses(
     assert is_hexstr(raw_txn)
 
 
+def test_sign_and_send_raw_middleware_with_buffered_gas_estimate_middleware(
+    w3_dummy, request_mocker
+):
+    gas_buffer = 100000  # the default internal value
+    gas_estimate = 12345 - gas_buffer
+
+    w3_dummy.middleware_onion.add(BufferedGasEstimateMiddleware)
+    w3_dummy.middleware_onion.inject(
+        SignAndSendRawMiddlewareBuilder.build(PRIVATE_KEY_1), layer=0
+    )
+
+    with request_mocker(
+        w3_dummy,
+        mock_results={
+            "eth_getBlockByNumber": {"gasLimit": 200000},  # arbitrary high number
+            "eth_estimateGas": gas_estimate,
+        },
+    ):
+        actual = w3_dummy.manager.request_blocking(
+            "eth_sendTransaction",
+            [
+                {
+                    "to": ADDRESS_2,
+                    "from": ADDRESS_1,
+                    "value": 1,
+                    "nonce": 0,
+                    "maxFeePerGas": 10**9,
+                    "maxPriorityFeePerGas": 10**9,
+                }
+            ],
+        )
+
+    raw_txn = actual[1][0]
+    actual_method = actual[0]
+    assert actual_method == "eth_sendRawTransaction"
+    assert is_hexstr(raw_txn)
+
+    decoded_txn = rlp.decode(HexBytes(raw_txn[4:]), sedes=DynamicFeeTransaction)
+    assert decoded_txn["gas"] == gas_estimate + gas_buffer
+
+
 # -- async -- #
 
 
@@ -478,8 +530,8 @@ async def test_async_sign_and_send_raw_middleware(
     expected,
     key_object,
 ):
-    async_w3_dummy.middleware_onion.add(
-        SignAndSendRawMiddlewareBuilder.build(key_object)
+    async_w3_dummy.middleware_onion.inject(
+        SignAndSendRawMiddlewareBuilder.build(key_object), layer=0
     )
 
     legacy_transaction = {
@@ -550,7 +602,9 @@ async def test_async_signed_transaction(
     key_object,
     from_,
 ):
-    async_w3.middleware_onion.add(SignAndSendRawMiddlewareBuilder.build(key_object))
+    async_w3.middleware_onion.inject(
+        SignAndSendRawMiddlewareBuilder.build(key_object), layer=0
+    )
 
     # Drop any falsy addresses
     to_from = valfilter(bool, {"to": async_w3.eth.default_account, "from": from_})
@@ -588,8 +642,8 @@ async def test_async_sign_and_send_raw_middleware_with_byte_addresses(
     from_ = from_converter(ADDRESS_1)
     to_ = to_converter(ADDRESS_2)
 
-    async_w3_dummy.middleware_onion.add(
-        SignAndSendRawMiddlewareBuilder.build(private_key)
+    async_w3_dummy.middleware_onion.inject(
+        SignAndSendRawMiddlewareBuilder.build(private_key), layer=0
     )
 
     actual = await async_w3_dummy.manager.coro_request(
@@ -609,3 +663,45 @@ async def test_async_sign_and_send_raw_middleware_with_byte_addresses(
     actual_method = actual[0]
     assert actual_method == "eth_sendRawTransaction"
     assert is_hexstr(raw_txn)
+
+
+@pytest.mark.asyncio
+async def test_async_sign_and_send_raw_middleware_with_buffered_gas_estimate_middleware(
+    async_w3_dummy, request_mocker
+):
+    gas_buffer = 100000  # the default internal value
+    gas_estimate = 12345 - gas_buffer
+
+    async_w3_dummy.middleware_onion.add(BufferedGasEstimateMiddleware)
+    async_w3_dummy.middleware_onion.inject(
+        SignAndSendRawMiddlewareBuilder.build(PRIVATE_KEY_1), layer=0
+    )
+
+    async with request_mocker(
+        async_w3_dummy,
+        mock_results={
+            "eth_getBlockByNumber": {"gasLimit": 200000},  # arbitrary high number
+            "eth_estimateGas": gas_estimate,
+        },
+    ):
+        actual = await async_w3_dummy.manager.coro_request(
+            "eth_sendTransaction",
+            [
+                {
+                    "to": ADDRESS_2,
+                    "from": ADDRESS_1,
+                    "value": 1,
+                    "nonce": 0,
+                    "maxFeePerGas": 10**9,
+                    "maxPriorityFeePerGas": 10**9,
+                }
+            ],
+        )
+
+    raw_txn = actual[1][0]
+    actual_method = actual[0]
+    assert actual_method == "eth_sendRawTransaction"
+    assert is_hexstr(raw_txn)
+
+    decoded_txn = rlp.decode(HexBytes(raw_txn[4:]), sedes=DynamicFeeTransaction)
+    assert decoded_txn["gas"] == gas_estimate + gas_buffer
diff --git a/web3/_utils/module_testing/eth_module.py b/web3/_utils/module_testing/eth_module.py
index 822a953e..68193d19 100644
--- a/web3/_utils/module_testing/eth_module.py
+++ b/web3/_utils/module_testing/eth_module.py
@@ -737,8 +737,8 @@ class AsyncEthModuleTest:
             "value": Wei(0),
             "gas": 21000,
         }
-        async_w3.middleware_onion.add(
-            SignAndSendRawMiddlewareBuilder.build(keyfile_account), "signing"
+        async_w3.middleware_onion.inject(
+            SignAndSendRawMiddlewareBuilder.build(keyfile_account), "signing", layer=0
         )
         txn_hash = await async_w3.eth.send_transaction(txn)
         assert isinstance(txn_hash, HexBytes)
@@ -3747,8 +3747,8 @@ class EthModuleTest:
             "value": Wei(0),
             "gas": 21000,
         }
-        w3.middleware_onion.add(
-            SignAndSendRawMiddlewareBuilder.build(keyfile_account), "signing"
+        w3.middleware_onion.inject(
+            SignAndSendRawMiddlewareBuilder.build(keyfile_account), "signing", layer=0
         )
         txn_hash = w3.eth.send_transaction(txn)
         assert isinstance(txn_hash, HexBytes)