Skip to content

Commit 3c3bdb1

Browse files
committed
Encourage signing middleware injection before txn modifying middleware
1 parent 31afba6 commit 3c3bdb1

File tree

9 files changed

+127
-29
lines changed

9 files changed

+127
-29
lines changed

docs/middleware.rst

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -423,14 +423,19 @@ The ``build`` method for this middleware builder takes a single argument:
423423
* An ``eth_keys.PrivateKey`` object
424424
* A raw private key as a hex string or byte string
425425

426+
.. note::
427+
Since this middleware signs the transaction, any middleware that modifies the
428+
transaction should run before this middleware. Therefore, it is recommended to
429+
inject the signing middleware at the 0th layer of the middleware onion.
430+
426431
.. code-block:: python
427432
428433
>>> from web3 import Web3, EthereumTesterProvider
429434
>>> w3 = Web3(EthereumTesterProvider)
430435
>>> from web3.middleware import SignAndSendRawMiddlewareBuilder
431436
>>> from eth_account import Account
432437
>>> acct = Account.create('KEYSMASH FJAFJKLDSKF7JKFDJ 1530')
433-
>>> w3.middleware_onion.add(SignAndSendRawMiddlewareBuilder.build(acct))
438+
>>> w3.middleware_onion.inject(SignAndSendRawMiddlewareBuilder.build(acct), layer=0)
434439
>>> w3.eth.default_account = acct.address
435440
436441
:ref:`Hosted nodes<local_vs_hosted>` (like Infura or Alchemy) only support signed
@@ -446,7 +451,7 @@ Instead, we can automate this process with
446451
>>> from eth_account import Account
447452
>>> import os
448453
>>> acct = w3.eth.account.from_key(os.environ.get('PRIVATE_KEY'))
449-
>>> w3.middleware_onion.add(SignAndSendRawMiddlewareBuilder.build(acct))
454+
>>> w3.middleware_onion.inject(SignAndSendRawMiddlewareBuilder.build(acct), layer=0)
450455
>>> w3.eth.default_account = acct.address
451456
452457
>>> # use `eth_sendTransaction` to automatically sign and send the raw transaction
@@ -463,7 +468,7 @@ Similarly, with AsyncWeb3:
463468
>>> from eth_account import Account
464469
>>> import os
465470
>>> acct = async_w3.eth.account.from_key(os.environ.get('PRIVATE_KEY'))
466-
>>> async_w3.middleware_onion.add(SignAndSendRawMiddlewareBuilder.build(acct))
471+
>>> async_w3.middleware_onion.inject(SignAndSendRawMiddlewareBuilder.build(acct), layer=0)
467472
>>> async_w3.eth.default_account = acct.address
468473
469474
>>> # use `eth_sendTransaction` to automatically sign and send the raw transaction

docs/migration.rst

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ with the necessary parameters via the ``build()`` method.
128128
129129
# v7
130130
from web3.middleware import SignAndSendRawMiddlewareBuilder
131-
w3.middleware_onion.add(SignAndSendRawMiddlewareBuilder.build(private_key))
131+
w3.middleware_onion.inject(SignAndSendRawMiddlewareBuilder.build(private_key), layer=0)
132132
133133
134134
Middleware Renaming and Removals

docs/transactions.rst

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,8 +90,8 @@ utilize web3.py middleware to sign transactions from a particular account:
9090
})
9191
9292
# Add acct2 as auto-signer:
93-
w3.middleware_onion.add(SignAndSendRawMiddlewareBuilder.build(acct2))
94-
# pk also works: w3.middleware_onion.add(SignAndSendRawMiddlewareBuilder.build(pk))
93+
w3.middleware_onion.inject(SignAndSendRawMiddlewareBuilder.build(acct2), layer=0)
94+
# pk also works: w3.middleware_onion.inject(SignAndSendRawMiddlewareBuilder.build(pk), layer=0)
9595
9696
# Transactions from `acct2` will then be signed, under the hood, in the middleware:
9797
tx_hash = w3.eth.send_transaction({

docs/web3.eth.account.rst

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ Example ``account_test_script.py``
151151
assert private_key.startswith("0x"), "Private key must start with 0x hex prefix"
152152
153153
account: LocalAccount = Account.from_key(private_key)
154-
w3.middleware_onion.add(SignAndSendRawMiddlewareBuilder.build(account))
154+
w3.middleware_onion.inject(SignAndSendRawMiddlewareBuilder.build(account), layer=0)
155155
156156
print(f"Your hot wallet address is {account.address}")
157157

newsfragments/3488.docs.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -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.

tests/core/middleware/test_transaction_signing.py

Lines changed: 104 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
import pytest
22

3+
from eth.vm.forks.london.transactions import (
4+
DynamicFeeTransaction,
5+
)
36
from eth_account import (
47
Account,
58
)
@@ -27,6 +30,7 @@
2730
HexBytes,
2831
)
2932
import pytest_asyncio
33+
import rlp
3034

3135
from web3 import (
3236
AsyncWeb3,
@@ -36,6 +40,7 @@
3640
InvalidAddress,
3741
)
3842
from web3.middleware import (
43+
BufferedGasEstimateMiddleware,
3944
SignAndSendRawMiddlewareBuilder,
4045
)
4146
from web3.middleware.signing import (
@@ -254,7 +259,9 @@ def hex_to_bytes(s):
254259
TEST_SIGN_AND_SEND_RAW_MIDDLEWARE_PARAMS,
255260
)
256261
def test_sign_and_send_raw_middleware(w3_dummy, method, from_, expected, key_object):
257-
w3_dummy.middleware_onion.add(SignAndSendRawMiddlewareBuilder.build(key_object))
262+
w3_dummy.middleware_onion.inject(
263+
SignAndSendRawMiddlewareBuilder.build(key_object), layer=0
264+
)
258265

259266
legacy_transaction = {
260267
"to": "0x7E5F4552091A69125d5DfCb7b8C2659029395Bdf",
@@ -364,7 +371,9 @@ def fund_account(w3):
364371
],
365372
)
366373
def test_signed_transaction(w3, fund_account, transaction, expected, key_object, from_):
367-
w3.middleware_onion.add(SignAndSendRawMiddlewareBuilder.build(key_object))
374+
w3.middleware_onion.inject(
375+
SignAndSendRawMiddlewareBuilder.build(key_object), layer=0
376+
)
368377

369378
# Drop any falsy addresses
370379
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(
398407
from_ = from_converter(ADDRESS_1)
399408
to_ = to_converter(ADDRESS_2)
400409

401-
w3_dummy.middleware_onion.add(SignAndSendRawMiddlewareBuilder.build(private_key))
410+
w3_dummy.middleware_onion.inject(
411+
SignAndSendRawMiddlewareBuilder.build(private_key), layer=0
412+
)
402413

403414
actual = w3_dummy.manager.request_blocking(
404415
"eth_sendTransaction",
@@ -419,6 +430,47 @@ def test_sign_and_send_raw_middleware_with_byte_addresses(
419430
assert is_hexstr(raw_txn)
420431

421432

433+
def test_sign_and_send_raw_middleware_with_buffered_gas_estimate_middleware(
434+
w3_dummy, request_mocker
435+
):
436+
gas_buffer = 100000 # the default internal value
437+
gas_estimate = 12345 - gas_buffer
438+
439+
w3_dummy.middleware_onion.add(BufferedGasEstimateMiddleware)
440+
w3_dummy.middleware_onion.inject(
441+
SignAndSendRawMiddlewareBuilder.build(PRIVATE_KEY_1), layer=0
442+
)
443+
444+
with request_mocker(
445+
w3_dummy,
446+
mock_results={
447+
"eth_getBlockByNumber": {"gasLimit": 200000}, # arbitrary high number
448+
"eth_estimateGas": gas_estimate,
449+
},
450+
):
451+
actual = w3_dummy.manager.request_blocking(
452+
"eth_sendTransaction",
453+
[
454+
{
455+
"to": ADDRESS_2,
456+
"from": ADDRESS_1,
457+
"value": 1,
458+
"nonce": 0,
459+
"maxFeePerGas": 10**9,
460+
"maxPriorityFeePerGas": 10**9,
461+
}
462+
],
463+
)
464+
465+
raw_txn = actual[1][0]
466+
actual_method = actual[0]
467+
assert actual_method == "eth_sendRawTransaction"
468+
assert is_hexstr(raw_txn)
469+
470+
decoded_txn = rlp.decode(HexBytes(raw_txn[4:]), sedes=DynamicFeeTransaction)
471+
assert decoded_txn["gas"] == gas_estimate + gas_buffer
472+
473+
422474
# -- async -- #
423475

424476

@@ -478,8 +530,8 @@ async def test_async_sign_and_send_raw_middleware(
478530
expected,
479531
key_object,
480532
):
481-
async_w3_dummy.middleware_onion.add(
482-
SignAndSendRawMiddlewareBuilder.build(key_object)
533+
async_w3_dummy.middleware_onion.inject(
534+
SignAndSendRawMiddlewareBuilder.build(key_object), layer=0
483535
)
484536

485537
legacy_transaction = {
@@ -550,7 +602,9 @@ async def test_async_signed_transaction(
550602
key_object,
551603
from_,
552604
):
553-
async_w3.middleware_onion.add(SignAndSendRawMiddlewareBuilder.build(key_object))
605+
async_w3.middleware_onion.inject(
606+
SignAndSendRawMiddlewareBuilder.build(key_object), layer=0
607+
)
554608

555609
# Drop any falsy addresses
556610
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(
588642
from_ = from_converter(ADDRESS_1)
589643
to_ = to_converter(ADDRESS_2)
590644

591-
async_w3_dummy.middleware_onion.add(
592-
SignAndSendRawMiddlewareBuilder.build(private_key)
645+
async_w3_dummy.middleware_onion.inject(
646+
SignAndSendRawMiddlewareBuilder.build(private_key), layer=0
593647
)
594648

595649
actual = await async_w3_dummy.manager.coro_request(
@@ -609,3 +663,45 @@ async def test_async_sign_and_send_raw_middleware_with_byte_addresses(
609663
actual_method = actual[0]
610664
assert actual_method == "eth_sendRawTransaction"
611665
assert is_hexstr(raw_txn)
666+
667+
668+
@pytest.mark.asyncio
669+
async def test_async_sign_and_send_raw_middleware_with_buffered_gas_estimate_middleware(
670+
async_w3_dummy, request_mocker
671+
):
672+
gas_buffer = 100000 # the default internal value
673+
gas_estimate = 12345 - gas_buffer
674+
675+
async_w3_dummy.middleware_onion.add(BufferedGasEstimateMiddleware)
676+
async_w3_dummy.middleware_onion.inject(
677+
SignAndSendRawMiddlewareBuilder.build(PRIVATE_KEY_1), layer=0
678+
)
679+
680+
async with request_mocker(
681+
async_w3_dummy,
682+
mock_results={
683+
"eth_getBlockByNumber": {"gasLimit": 200000}, # arbitrary high number
684+
"eth_estimateGas": gas_estimate,
685+
},
686+
):
687+
actual = await async_w3_dummy.manager.coro_request(
688+
"eth_sendTransaction",
689+
[
690+
{
691+
"to": ADDRESS_2,
692+
"from": ADDRESS_1,
693+
"value": 1,
694+
"nonce": 0,
695+
"maxFeePerGas": 10**9,
696+
"maxPriorityFeePerGas": 10**9,
697+
}
698+
],
699+
)
700+
701+
raw_txn = actual[1][0]
702+
actual_method = actual[0]
703+
assert actual_method == "eth_sendRawTransaction"
704+
assert is_hexstr(raw_txn)
705+
706+
decoded_txn = rlp.decode(HexBytes(raw_txn[4:]), sedes=DynamicFeeTransaction)
707+
assert decoded_txn["gas"] == gas_estimate + gas_buffer

web3/_utils/async_transactions.py

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -87,9 +87,7 @@ async def _chain_id(
8787
async def get_block_gas_limit(
8888
web3_eth: "AsyncEth", block_identifier: Optional[BlockIdentifier] = None
8989
) -> int:
90-
if block_identifier is None:
91-
block_identifier = await web3_eth.block_number
92-
block = await web3_eth.get_block(block_identifier)
90+
block = await web3_eth.get_block(block_identifier or "latest")
9391
return block["gasLimit"]
9492

9593

@@ -104,8 +102,8 @@ async def get_buffered_gas_estimate(
104102

105103
if gas_estimate > gas_limit:
106104
raise Web3ValueError(
107-
"Contract does not appear to be deployable within the "
108-
f"current network gas limits. Estimated: {gas_estimate}. "
105+
"Gas estimate for transaction is higher than current network gas limits. "
106+
f"Transaction could not be sent. Estimated: {gas_estimate}. "
109107
f"Current gas limit: {gas_limit}"
110108
)
111109

web3/_utils/module_testing/eth_module.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -737,8 +737,8 @@ async def test_async_sign_and_send_raw_middleware(
737737
"value": Wei(0),
738738
"gas": 21000,
739739
}
740-
async_w3.middleware_onion.add(
741-
SignAndSendRawMiddlewareBuilder.build(keyfile_account), "signing"
740+
async_w3.middleware_onion.inject(
741+
SignAndSendRawMiddlewareBuilder.build(keyfile_account), "signing", layer=0
742742
)
743743
txn_hash = await async_w3.eth.send_transaction(txn)
744744
assert isinstance(txn_hash, HexBytes)
@@ -3747,8 +3747,8 @@ def test_sign_and_send_raw_middleware(
37473747
"value": Wei(0),
37483748
"gas": 21000,
37493749
}
3750-
w3.middleware_onion.add(
3751-
SignAndSendRawMiddlewareBuilder.build(keyfile_account), "signing"
3750+
w3.middleware_onion.inject(
3751+
SignAndSendRawMiddlewareBuilder.build(keyfile_account), "signing", layer=0
37523752
)
37533753
txn_hash = w3.eth.send_transaction(txn)
37543754
assert isinstance(txn_hash, HexBytes)

web3/_utils/transactions.py

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -149,9 +149,7 @@ def fill_transaction_defaults(w3: "Web3", transaction: TxParams) -> TxParams:
149149
def get_block_gas_limit(
150150
w3: "Web3", block_identifier: Optional[BlockIdentifier] = None
151151
) -> int:
152-
if block_identifier is None:
153-
block_identifier = w3.eth.block_number
154-
block = w3.eth.get_block(block_identifier)
152+
block = w3.eth.get_block(block_identifier or "latest")
155153
return block["gasLimit"]
156154

157155

@@ -166,8 +164,8 @@ def get_buffered_gas_estimate(
166164

167165
if gas_estimate > gas_limit:
168166
raise Web3ValueError(
169-
"Contract does not appear to be deployable within the "
170-
f"current network gas limits. Estimated: {gas_estimate}. "
167+
"Gas estimate for transaction is higher than current network gas limits. "
168+
f"Transaction could not be sent. Estimated: {gas_estimate}. "
171169
f"Current gas limit: {gas_limit}"
172170
)
173171

0 commit comments

Comments
 (0)