Skip to content

Commit

Permalink
update fill_transaction_defaults() to suppoer 1559 params.
Browse files Browse the repository at this point in the history
Do not set a default 'gasPrice' if 1559 params are present and default to transaction 'type' of '0x2' if 1559 params are present.
  • Loading branch information
fselmo committed Jul 30, 2021
1 parent 88713f1 commit f36066b
Show file tree
Hide file tree
Showing 7 changed files with 177 additions and 36 deletions.
37 changes: 35 additions & 2 deletions docs/middleware.rst
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,9 @@ Signing

.. py:method:: web3.middleware.construct_sign_and_send_raw_middleware(private_key_or_account)
This middleware automatically captures transactions, signs them, and sends them as raw transactions. The from field on the transaction, or ``w3.eth.default_account`` must be set to the address of the private key for this middleware to have any effect.
This middleware automatically captures transactions, signs them, and sends them as raw transactions.
The ``from`` field on the transaction, or ``w3.eth.default_account`` must be set to the address of the private key for
this middleware to have any effect.

* ``private_key_or_account`` A single private key or a tuple, list or set of private keys.

Expand All @@ -437,4 +439,35 @@ This middleware automatically captures transactions, signs them, and sends them
>>> acct = Account.create('KEYSMASH FJAFJKLDSKF7JKFDJ 1530')
>>> w3.middleware_onion.add(construct_sign_and_send_raw_middleware(acct))
>>> w3.eth.default_account = acct.address
# Now you can send a tx from acct.address without having to build and sign each raw transaction
Now you can send a transaction from acct.address without having to build and sign each raw transaction.

When making use of this signing middleware, when sending EIP-1559 transactions (recommended over legacy transactions),
the transaction ``type`` of ``2`` (or ``'0x2'``) is necessary. This is because transaction signing is validated based
on the transaction ``type`` parameter. This value defaults to ``'0x2'`` when ``maxFeePerGas`` and / or
``maxPriorityFeePerGas`` are present as parameters in the transaction as these params imply a 1559-style transaction.
Since these values effectively replace the legacy ``gasPrice`` value, do not set a ``gasPrice`` for 1559 transactions.
Doing so will lead to validation issues.

.. code-block:: python
>>> transaction = {
... 'type': '0x2', # optional - defaults to '0x2' when 1559 fee params are present
... 'from': acct.address, # optional if w3.eth.default_account was set with acct.address
... 'to': receiving_account_address,
... 'value': 22,
... 'maxFeePerGas': 2000000000, # required for 1559 transaction (1559 fee param)
... 'maxPriorityFeePerGas': 1000000000, # required for 1559 transaction (1559 fee param)
... }
>>> w3.eth.send_transaction(transaction)
A legacy transaction still works in the same way as it did before EIP-1559 was introduced:

.. code-block:: python
>>> legacy_transaction = {
... 'to': receiving_account_address,
... 'value': 22,
... 'gasPrice': 123456, # optional - if not provided, gas_price_strategy (if exists) or eth_gasPrice is used
... }
>>> w3.eth.send_transaction(legacy_transaction)
1 change: 1 addition & 0 deletions newsfragments/2092.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
``fill_transaction_defaults()`` no longer sets a default ``gasPrice`` if 1559 fees are present in the transaction parameters. This fixes sign-and-send middleware issues with 1559 fees.
1 change: 1 addition & 0 deletions newsfragments/2092.doc.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Updated the sign-and-send middleware docs to include EIP-1559 as well as legacy transaction examples
104 changes: 72 additions & 32 deletions tests/core/middleware/test_transaction_signing.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
to_hex,
)
from eth_utils.toolz import (
assoc,
dissoc,
identity,
merge,
valfilter,
Expand Down Expand Up @@ -136,41 +138,50 @@ def hex_to_bytes(s):
)
def test_sign_and_send_raw_middleware(
w3_dummy,
w3,
method,
from_,
expected,
key_object):
w3_dummy.middleware_onion.add(
construct_sign_and_send_raw_middleware(key_object))

legacy_transaction = {
'to': '0x7E5F4552091A69125d5DfCb7b8C2659029395Bdf',
'from': from_,
'gas': 21000,
'gasPrice': 0,
'value': 1,
'nonce': 0
}
if isinstance(expected, type) and issubclass(expected, Exception):
with pytest.raises(expected):
w3_dummy.manager.request_blocking(
method,
[{
'to': '0x7E5F4552091A69125d5DfCb7b8C2659029395Bdf',
'from': from_,
'gas': 21000,
'gasPrice': 0,
'value': 1,
'nonce': 0
}])
w3_dummy.manager.request_blocking(method, [legacy_transaction])
else:
actual = w3_dummy.manager.request_blocking(
method,
[{
'to': '0x7E5F4552091A69125d5DfCb7b8C2659029395Bdf',
'from': from_,
'gas': 21000,
'gasPrice': 0,
'value': 1,
'nonce': 0
}])
raw_txn = actual[1][0]
actual_method = actual[0]
assert actual_method == expected
assert isinstance(raw_txn, bytes)
# assert with legacy txn params
actual = w3_dummy.manager.request_blocking(method, [legacy_transaction])
assert_method_and_txn_signed(actual, expected)

# assert with 1559 transaction params and explicit type
transaction_1559 = dissoc(legacy_transaction, 'gasPrice')
transaction_1559 = assoc(transaction_1559, 'maxFeePerGas', 2000000000)
transaction_1559 = assoc(transaction_1559, 'maxPriorityFeePerGas', 1000000000)
transaction_1559 = assoc(transaction_1559, 'type', '0x2')

actual_1559 = w3_dummy.manager.request_blocking(method, [transaction_1559])
assert_method_and_txn_signed(actual_1559, expected)

# assert with 1559 transaction params and no explicit type
transaction_1559_no_type = dissoc(transaction_1559, 'type')

actual_1559_no_type = w3_dummy.manager.request_blocking(method, [transaction_1559_no_type])
assert_method_and_txn_signed(actual_1559_no_type, expected)


def assert_method_and_txn_signed(actual, expected):
raw_txn = actual[1][0]
actual_method = actual[0]
assert actual_method == expected
assert isinstance(raw_txn, bytes)


@pytest.fixture()
Expand Down Expand Up @@ -219,7 +230,6 @@ def fund_account(w3):
'transaction,expected,key_object,from_',
(
(
# Transaction with set gas
{
'gas': 21000,
'gasPrice': 0,
Expand All @@ -230,17 +240,15 @@ def fund_account(w3):
ADDRESS_1,
),
(
# Transaction with no set gas
{
'value': 1
},
-1,
MIXED_KEY_MIXED_TYPE,
ADDRESS_1,
),
# expect validation error + unmanaged account
(
# Transaction with mismatched sender
# expect a validation error with send_transaction + unmanaged account
{
'gas': 21000,
'value': 10
Expand All @@ -250,16 +258,49 @@ def fund_account(w3):
ADDRESS_2,
),
(
# Transaction with invalid sender
{
'gas': 21000,
'value': 10
},
InvalidAddress,
SAME_KEY_MIXED_TYPE,
'0x0000',
),
(
# TODO: Once eth-tester supports 1559 params, this test should fail and we will need to
# update this to appropriately test 'maxFeePerGas' and 'maxPriorityFeePerGas' as
# well as the transaction 'type'
{
'type': '0x2',
'value': 22,
'maxFeePerGas': 2000000000,
'maxPriorityFeePerGas': 1000000000,
},
ValidationError,
SAME_KEY_MIXED_TYPE,
ADDRESS_2,
),
(
# TODO: eth-tester support for 1559 message above applies to this test as well.
# type should default to '0x2` and send successfully based on 1559 fields being present
{
'value': 22,
'maxFeePerGas': 2000000000,
'maxPriorityFeePerGas': 1000000000,
},
ValidationError,
SAME_KEY_MIXED_TYPE,
ADDRESS_2,
)
)
),
ids=[
'with set gas',
'with no set gas',
'with mismatched sender',
'with invalid sender',
'with txn type and 1559 fees',
'with 1559 fees and no type',
]
)
def test_signed_transaction(
w3,
Expand All @@ -277,7 +318,6 @@ def test_signed_transaction(

if isinstance(expected, type) and issubclass(expected, Exception):
with pytest.raises(expected):
start_balance = w3.eth.get_balance(_transaction.get('from', w3.eth.accounts[0]))
w3.eth.send_transaction(_transaction)
else:
start_balance = w3.eth.get_balance(_transaction.get('from', w3.eth.accounts[0]))
Expand Down
52 changes: 51 additions & 1 deletion tests/core/utilities/test_valid_transaction_params.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,23 @@
from web3._utils.transactions import (
assert_valid_transaction_params,
extract_valid_transaction_params,
fill_transaction_defaults,
)


def test_assert_valid_transaction_params_all_params():
assert_valid_transaction_params({
'chainId': 1,
'type': '0x2',
'from': '0x0',
'to': '0x0',
'gas': 21000,
'gasPrice': 5000000,
'maxFeePerGas': 2000000000,
'maxPriorityFeePerGas': 1000000000,
'value': 1,
'data': '0x0',
'nonce': 2,
'chainId': 1,
})


Expand All @@ -38,10 +42,14 @@ def test_assert_valid_transaction_params_invalid_param():


FULL_TXN_DICT = {
'chainId': 1,
'type': '0x2',
'from': '0x0',
'to': '0x1',
'gas': 21000,
'gasPrice': 5000000,
'maxFeePerGas': 2000000000,
'maxPriorityFeePerGas': 1000000000,
'data': '0x2',
'value': 3,
'nonce': 2,
Expand Down Expand Up @@ -77,18 +85,60 @@ def test_extract_valid_transaction_params_invalid(transaction_params, expected_e

def test_extract_valid_transaction_params_includes_invalid():
input = {
'chainId': 1,
'type': '0x2',
'from': '0x0',
'to': '0x0',
'gas': 21000,
'gasPrice': 5000000,
'maxFeePerGas': 2000000000,
'maxPriorityFeePerGas': 1000000000,
'value': 1,
'tokens': 9000,
}
valid_transaction_params = extract_valid_transaction_params(input)
assert valid_transaction_params == {
'chainId': 1,
'type': '0x2',
'from': '0x0',
'to': '0x0',
'gas': 21000,
'gasPrice': 5000000,
'maxFeePerGas': 2000000000,
'maxPriorityFeePerGas': 1000000000,
'value': 1,
}


def test_fill_transaction_defaults_for_all_params(web3):
default_transaction = fill_transaction_defaults(web3, {})

assert default_transaction == {
'chainId': web3.eth.chain_id,
'data': b'',
'gas': web3.eth.estimate_gas({}),
'gasPrice': web3.eth.gas_price,
'value': 0,
}


def test_fill_transaction_defaults_sets_type_from_1559_params_and_no_gas_price(web3):
transaction_1559 = {
'chainId': 1,
'data': b'123',
'gas': 21000,
'maxFeePerGas': 2000000000,
'maxPriorityFeePerGas': 1000000000,
'value': 2,
}
transaction_1559_type_added = fill_transaction_defaults(web3, transaction_1559)

assert transaction_1559_type_added == {
'chainId': 1,
'data': b'123',
'gas': 21000,
'maxFeePerGas': 2000000000,
'maxPriorityFeePerGas': 1000000000,
'type': '0x2', # type is added and defaults to '0x2' when 1559 params are present
'value': 2,
}
16 changes: 15 additions & 1 deletion web3/_utils/transactions.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,11 @@
_Hash32,
)

TX_PARAM_LITERALS = Literal['from', 'to', 'gas', 'maxFeePerGas', 'maxPriorityFeePerGas',
TX_PARAM_LITERALS = Literal['type', 'from', 'to', 'gas', 'maxFeePerGas', 'maxPriorityFeePerGas',
'gasPrice', 'value', 'data', 'nonce', 'chainId']

VALID_TRANSACTION_PARAMS: List[TX_PARAM_LITERALS] = [
'type',
'from',
'to',
'gas',
Expand Down Expand Up @@ -85,14 +86,27 @@ def fill_transaction_defaults(web3: "Web3", transaction: TxParams) -> TxParams:
defaults = {}
for key, default_getter in TRANSACTION_DEFAULTS.items():
if key not in transaction:
if key == 'gasPrice':
if any(_ in transaction for _ in ('maxFeePerGas', 'maxPriorityFeePerGas')):
# if EIP-1559 params in transaction, do not set a default gasPrice if missing
continue

if callable(default_getter):
if web3 is not None:
default_val = default_getter(web3, transaction)
else:
raise ValueError("You must specify %s in the transaction" % key)

else:
default_val = default_getter
defaults[key] = default_val

if 'type' not in transaction and any(k in transaction for k in (
'maxFeePerGas', 'maxPriorityFeePerGas'
)):
# default transaction type to '2' if 1559 transaction params are present
defaults['type'] = '0x2'

return merge(defaults, transaction)


Expand Down
2 changes: 2 additions & 0 deletions web3/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ class LogReceipt(TypedDict):
"s": HexBytes,
"to": ChecksumAddress,
"transactionIndex": int,
"type": Union[int, HexStr],
"v": int,
"value": Wei,
}, total=False)
Expand All @@ -196,6 +197,7 @@ class LogReceipt(TypedDict):
"nonce": Nonce,
# addr or ens
"to": Union[Address, ChecksumAddress, str],
"type": Union[int, HexStr],
"value": Wei,
}, total=False)

Expand Down

0 comments on commit f36066b

Please sign in to comment.