From 106cd525e905d557d4d05d664421a2f84bbed63c Mon Sep 17 00:00:00 2001 From: techdigger <72038289+techdigger@users.noreply.github.com> Date: Thu, 12 Nov 2026 17:36:12 +0000 Subject: [PATCH 1/5] Add --max-fee-per-gas and --max-priority-fee-per-gas safe-cli takes the fees of every transaction from eth_feeHistory: the next block's base fee plus the median tip. There is no way to change them. On chains whose fee history forecasts less than they charge (a fixed-fee chain whose node still projects a falling base fee) every transaction is refused, and on any chain you cannot pay more to be included sooner or to replace a pending transaction. Add two global options, for safe-cli (every command and the attended mode, anywhere on the command line) and safe-creator: --max-fee-per-gas cap of at least eth_gasPrice --max-fee-per-gas= that cap --max-priority-fee-per-gas= that tip SAFE_CLI_MAX_FEE_PER_GAS and SAFE_CLI_MAX_PRIORITY_FEE_PER_GAS set defaults; the command line overrides them. Transactions stay EIP-1559. The fees apply to approve_hash, to executing Safe transactions (local key or hardware wallet) and to deploying a Safe, and show in the confirmation prompt. safe-cli refuses a cap above 10 times the node's price, a tip above the cap, and the options on a network without EIP-1559, before sending anything. Without the options nothing changes. --- src/safe_cli/gas_fees.py | 158 +++++++++++ src/safe_cli/main.py | 58 +++- .../operators/hw_wallets/hw_wallet_manager.py | 10 +- src/safe_cli/operators/safe_operator.py | 56 +++- src/safe_cli/safe_creator.py | 92 ++++++- tests/test_gas_fees.py | 256 ++++++++++++++++++ 6 files changed, 612 insertions(+), 18 deletions(-) create mode 100644 src/safe_cli/gas_fees.py create mode 100644 tests/test_gas_fees.py diff --git a/src/safe_cli/gas_fees.py b/src/safe_cli/gas_fees.py new file mode 100644 index 00000000..5c547681 --- /dev/null +++ b/src/safe_cli/gas_fees.py @@ -0,0 +1,158 @@ +""" +EIP-1559 fees set by hand. + +By default safe-cli takes the fees of every transaction it sends from ``eth_feeHistory``: the next block's base fee +plus the median tip. Two global options override them: + +- ``--max-fee-per-gas``: a cap of at least ``eth_gasPrice``, for chains whose fee history forecasts less than they + charge; ``--max-fee-per-gas=``: that cap; +- ``--max-priority-fee-per-gas=``: that tip, for example to be included sooner on a busy chain or to replace + a pending transaction. + +The environment variables ``SAFE_CLI_MAX_FEE_PER_GAS`` (``node`` or gwei) and ``SAFE_CLI_MAX_PRIORITY_FEE_PER_GAS`` +(gwei) set defaults; the command line overrides them. +""" + +from decimal import Decimal, InvalidOperation + +from safe_eth.eth import EthereumClient, TxSpeed +from web3 import Web3 +from web3.types import TxParams + +MAX_FEE_FLAG = "--max-fee-per-gas" +MAX_FEE_ENVVAR = "SAFE_CLI_MAX_FEE_PER_GAS" +MAX_FEE_HELP = ( + "Cap the fee per gas of every transaction sent. Without a value (or with 'node'): at least eth_gasPrice, for " + "chains whose fee history forecasts less than they charge. With a value: that many gwei, at most 10 times the " + "node's price." +) +PRIORITY_FEE_FLAG = "--max-priority-fee-per-gas" +PRIORITY_FEE_ENVVAR = "SAFE_CLI_MAX_PRIORITY_FEE_PER_GAS" +PRIORITY_FEE_HELP = ( + "Tip per gas, in gwei, of every transaction sent, instead of the fee history's median: for example to be " + "included sooner, or to replace a pending transaction. At most the cap." +) +NODE_PRICE = "node" +# A cap above this many times the node's price is refused: far more likely a typing mistake (gwei taken for wei, +# extra zeros) than an intended fee. +MAX_FEE_MULTIPLE = 10 + +_max_fee: str | None = None +_priority_fee: str | None = None + + +def parse_gwei(value: str, flag: str, allow_zero: bool = False) -> int: + """ + :return: ``value`` gwei in wei + :raises ValueError: unless ``value`` is a finite number of gwei, positive (or zero if allowed), that is a whole + number of wei + """ + try: + gwei = Decimal(value) + valid = gwei.is_finite() and (gwei > 0 or (allow_zero and gwei == 0)) + except InvalidOperation: + valid = False + wei = gwei * 10**9 if valid else Decimal(0) + if not valid or wei != wei.to_integral_value(): + expected = ( + "a gas price in gwei" if allow_zero else "a positive gas price in gwei" + ) + if flag == MAX_FEE_FLAG: + expected = f"'{NODE_PRICE}' or {expected}" + raise ValueError(f"{flag}: expected {expected}, got {value!r}") + return int(wei) + + +def check_max_fee(value: str | None) -> str | None: + if value is not None and value != NODE_PRICE: + parse_gwei(value, MAX_FEE_FLAG) + return value + + +def check_priority_fee(value: str | None) -> str | None: + if value is not None: + parse_gwei(value, PRIORITY_FEE_FLAG, allow_zero=True) + return value + + +def set_fees(max_fee: str | None, priority_fee: str | None) -> None: + """ + Sets the fees of every transaction sent from now on. ``None`` for both keeps safe-cli's estimate. + """ + global _max_fee, _priority_fee + _max_fee, _priority_fee = check_max_fee(max_fee), check_priority_fee(priority_fee) + + +def take_fee_flags(arguments: list[str]) -> list[str]: + """ + Removes the fee options from ``arguments``, wherever they are, so they can be given anywhere on the command line, + and ``--max-fee-per-gas`` without a value. + + :return: the options in the one form the parsers declare (``--flag=value``), the last one of each given winning + """ + taken: dict[str, str] = {} + remaining: list[str] = [] + arguments_iter = iter(arguments) + for argument in arguments_iter: + if argument == MAX_FEE_FLAG: + taken[MAX_FEE_FLAG] = NODE_PRICE + elif argument == PRIORITY_FEE_FLAG: + taken[PRIORITY_FEE_FLAG] = next(arguments_iter, "") + elif argument.startswith((MAX_FEE_FLAG + "=", PRIORITY_FEE_FLAG + "=")): + flag, value = argument.split("=", 1) + taken[flag] = value + else: + remaining.append(argument) + arguments[:] = remaining + return [f"{flag}={value}" for flag, value in taken.items()] + + +def eip1559_fees(ethereum_client: EthereumClient) -> TxParams | None: + """ + :return: ``maxFeePerGas`` and ``maxPriorityFeePerGas`` for the next transaction, or ``None`` to keep safe-cli's + estimate + :raises ValueError: if the network has no EIP-1559 fee history, if the cap is more than ``MAX_FEE_MULTIPLE`` + times the node's price, or if the tip is above the cap + """ + if _max_fee is None and _priority_fee is None: + return None + if not ethereum_client.is_eip1559_supported(): + raise ValueError( + f"{MAX_FEE_FLAG} and {PRIORITY_FEE_FLAG} need a network with EIP-1559 fees, nothing was sent" + ) + base_fee, estimated_tip = ethereum_client.estimate_fee_eip1559(TxSpeed.NORMAL) + node_price = ethereum_client.w3.eth.gas_price + if _priority_fee is None: + tip = estimated_tip + else: + tip = parse_gwei(_priority_fee, PRIORITY_FEE_FLAG, allow_zero=True) + if _max_fee is None: + max_fee = base_fee + tip + elif _max_fee == NODE_PRICE: + max_fee = max(base_fee + tip, node_price) + else: + max_fee = parse_gwei(_max_fee, MAX_FEE_FLAG) + reference = max(node_price, base_fee + estimated_tip) + if reference and max_fee > reference * MAX_FEE_MULTIPLE: + raise ValueError( + f"{MAX_FEE_FLAG}: {gwei(max_fee)} gwei is more than {MAX_FEE_MULTIPLE} times the node's price " + f"({gwei(reference)} gwei), nothing was sent" + ) + if tip > max_fee: + if _priority_fee is not None: + raise ValueError( + f"{PRIORITY_FEE_FLAG}: {gwei(tip)} gwei is above the cap of {gwei(max_fee)} gwei, nothing was sent" + ) + tip = max_fee + return {"maxFeePerGas": max_fee, "maxPriorityFeePerGas": tip} + + +def gwei(wei: int) -> str: + return str(Web3.from_wei(wei, "gwei")) + + +def describe(fees: TxParams) -> str: + return ( + f"max fee per gas {gwei(fees['maxFeePerGas'])} gwei, " + f"max priority fee per gas {gwei(fees['maxPriorityFeePerGas'])} gwei" + ) diff --git a/src/safe_cli/main.py b/src/safe_cli/main.py index 84cbeb54..1fd78672 100644 --- a/src/safe_cli/main.py +++ b/src/safe_cli/main.py @@ -15,6 +15,18 @@ from . import VERSION from .argparse_validators import check_hex_str +from .gas_fees import ( + MAX_FEE_ENVVAR, + MAX_FEE_FLAG, + MAX_FEE_HELP, + PRIORITY_FEE_ENVVAR, + PRIORITY_FEE_FLAG, + PRIORITY_FEE_HELP, + check_max_fee, + check_priority_fee, + set_fees, + take_fee_flags, +) from .operators import SafeOperator from .safe_cli import SafeCli from .tx_builder.exceptions import SoliditySyntaxError, TxBuilderEncodingError @@ -30,6 +42,44 @@ app = typer.Typer(name="Safe CLI") +def _as_parameter_check(check): + def callback(value: str | None) -> str | None: + try: + return check(value) + except ValueError as error: + raise typer.BadParameter(str(error)) from error + + return callback + + +@app.callback() +def global_options( + max_fee_per_gas: Annotated[ + str | None, + typer.Option( + MAX_FEE_FLAG, + envvar=MAX_FEE_ENVVAR, + metavar="[node|GWEI]", + help=MAX_FEE_HELP, + callback=_as_parameter_check(check_max_fee), + show_default=False, + ), + ] = None, + max_priority_fee_per_gas: Annotated[ + str | None, + typer.Option( + PRIORITY_FEE_FLAG, + envvar=PRIORITY_FEE_ENVVAR, + metavar="GWEI", + help=PRIORITY_FEE_HELP, + callback=_as_parameter_check(check_priority_fee), + show_default=False, + ), + ] = None, +): + set_fees(max_fee_per_gas, max_priority_fee_per_gas) + + def _build_safe_operator_and_load_keys( safe_address: ChecksumAddress, node_url: str, @@ -343,7 +393,10 @@ def version(): for command in get_command(app).commands.keys() ] ) - + "\n\n\n\nUse the --help option of each command to see the usage options.", + + "\n\n\n\nUse the --help option of each command to see the usage options." + + "\n\n\n\nGlobal options, for every command and the attended mode, anywhere on the command line:" + + f"\n\n {MAX_FEE_FLAG}[=node|GWEI] {MAX_FEE_HELP} Default: ${MAX_FEE_ENVVAR}." + + f"\n\n {PRIORITY_FEE_FLAG}=GWEI {PRIORITY_FEE_HELP} Default: ${PRIORITY_FEE_ENVVAR}.", ) def default_attended_mode( address: Annotated[ @@ -401,7 +454,10 @@ def _is_safe_cli_default_command(arguments: list[str]) -> bool: def main(): + # The fee options are global; take them from wherever they were given and hand them to the application itself + fee_flags = take_fee_flags(sys.argv) # By default, the attended mode is initialised. Otherwise, the required command must be specified. if _is_safe_cli_default_command(sys.argv): sys.argv.insert(1, "attended-mode") + sys.argv[1:1] = fee_flags app() diff --git a/src/safe_cli/operators/hw_wallets/hw_wallet_manager.py b/src/safe_cli/operators/hw_wallets/hw_wallet_manager.py index 56ba0ccf..39639a3d 100644 --- a/src/safe_cli/operators/hw_wallets/hw_wallet_manager.py +++ b/src/safe_cli/operators/hw_wallets/hw_wallet_manager.py @@ -172,6 +172,7 @@ def execute_safe_tx( tx_gas_price: int | None = None, tx_nonce: int | None = None, eip1559_speed: TxSpeed | None = None, + eip1559_fees: TxParams | None = None, ) -> tuple[HexBytes, TxParams]: """ Send multisig tx to the Safe @@ -181,10 +182,17 @@ def execute_safe_tx( :param tx_gas_price: Gas price of the external tx. If not, `gas_price` will be used :param tx_nonce: Force nonce for `tx_sender` :param eip1559_speed: If provided, use EIP1559 transaction + :param eip1559_fees: If provided, use these ``maxFeePerGas`` and ``maxPriorityFeePerGas`` :return: Tuple(tx_hash, tx) """ - if eip1559_speed and safe_tx.ethereum_client.is_eip1559_supported(): + if eip1559_fees: + tx_parameters = { + "from": self.sender.address, + "chainId": safe_tx.ethereum_client.get_chain_id(), + **eip1559_fees, + } + elif eip1559_speed and safe_tx.ethereum_client.is_eip1559_supported(): tx_parameters = safe_tx.ethereum_client.set_eip1559_fees( { "from": self.sender.address, diff --git a/src/safe_cli/operators/safe_operator.py b/src/safe_cli/operators/safe_operator.py index 0c94779b..f8b09190 100644 --- a/src/safe_cli/operators/safe_operator.py +++ b/src/safe_cli/operators/safe_operator.py @@ -37,8 +37,10 @@ from web3 import Web3 from web3.contract import Contract from web3.exceptions import BadFunctionCallOutput +from web3.types import TxParams, Wei from safe_cli.ethereum_hd_wallet import get_account_from_words +from safe_cli.gas_fees import describe, eip1559_fees from safe_cli.operators.exceptions import ( AccountNotLoadedException, ExistingOwnerException, @@ -442,17 +444,18 @@ def approve_hash(self, hash_to_approve: HexBytes, sender: str) -> bool: sender_account_address, hash_to_approve ): raise HashAlreadyApproved(hash_to_approve, sender) + tx_params = { + "from": sender_account_address, + "nonce": self.ethereum_client.get_nonce_for_account(sender_account_address), + } + fees = eip1559_fees(self.ethereum_client) + if fees: + tx_params.update(fees) + print_formatted_text(HTML(f"{describe(fees)}")) transaction_to_send = self.safe_contract.functions.approveHash( hash_to_approve - ).build_transaction( - { - "from": sender_account_address, - "nonce": self.ethereum_client.get_nonce_for_account( - sender_account_address - ), - } - ) - if self.ethereum_client.is_eip1559_supported(): + ).build_transaction(tx_params) + if not fees and self.ethereum_client.is_eip1559_supported(): transaction_to_send = self.ethereum_client.set_eip1559_fees( transaction_to_send ) @@ -1035,16 +1038,20 @@ def execute_safe_transaction(self, safe_tx: SafeTx): else: call_result = safe_tx.call(self.hw_wallet_manager.sender.address) print_formatted_text(HTML(f"Result: {call_result}")) + fees = eip1559_fees(self.ethereum_client) + pricing = f" with {describe(fees)}" if fees else "" if not self.interactive or yes_or_no_question( - "Do you want to execute tx " + str(safe_tx) + "Do you want to execute tx " + str(safe_tx) + pricing ): - if self.default_sender: + if self.default_sender and fees: + tx_hash, tx = self._execute_with_fees(safe_tx, fees) + elif self.default_sender: tx_hash, tx = safe_tx.execute( self.default_sender.key, eip1559_speed=TxSpeed.NORMAL ) else: tx_hash, tx = self.hw_wallet_manager.execute_safe_tx( - safe_tx, eip1559_speed=TxSpeed.NORMAL + safe_tx, eip1559_speed=TxSpeed.NORMAL, eip1559_fees=fees ) self.executed_transactions.append(to_0x_hex_str(tx_hash)) print_formatted_text( @@ -1082,6 +1089,31 @@ def execute_safe_transaction(self, safe_tx: SafeTx): ) return False + def _execute_with_fees( + self, safe_tx: SafeTx, fees: TxParams + ) -> tuple[HexBytes, TxParams]: + """ + ``SafeTx.execute`` with the default sender and EIP-1559 fees given by hand (``SafeTx.execute`` only + estimates them). Same gas and nonce handling. + """ + sender = self.default_sender + safe_tx.tx = safe_tx.w3_tx.build_transaction( + { + "from": sender.address, + "chainId": self.ethereum_client.get_chain_id(), + **fees, + } + ) + safe_tx.tx["gas"] = Wei( + max(safe_tx.tx["gas"] + 75000, safe_tx.recommended_gas()) + ) + safe_tx.tx_hash = self.ethereum_client.send_unsigned_transaction( + safe_tx.tx, private_key=sender.key, retry=True, block_identifier="latest" + ) + # As SafeTx.execute: the Safe nonce increases even if the transaction fails, so the signatures are spent + safe_tx.signatures = b"" + return safe_tx.tx_hash, safe_tx.tx + # Batch_transactions multisend def batch_safe_txs( self, safe_nonce: int, safe_txs: Sequence[SafeTx] diff --git a/src/safe_cli/safe_creator.py b/src/safe_cli/safe_creator.py index b8256de2..84032597 100644 --- a/src/safe_cli/safe_creator.py +++ b/src/safe_cli/safe_creator.py @@ -1,5 +1,6 @@ #!/bin/env python3 import argparse +import os import secrets import sys @@ -15,6 +16,20 @@ from safe_eth.safe import ProxyFactory, Safe from safe_eth.util.util import to_0x_hex_str +from safe_cli.gas_fees import ( + MAX_FEE_ENVVAR, + MAX_FEE_FLAG, + MAX_FEE_HELP, + PRIORITY_FEE_ENVVAR, + PRIORITY_FEE_FLAG, + PRIORITY_FEE_HELP, + check_max_fee, + check_priority_fee, + describe, + eip1559_fees, + set_fees, + take_fee_flags, +) from safe_cli.safe_addresses import ( get_default_fallback_handler_address, get_proxy_factory_address, @@ -38,8 +53,32 @@ def get_epilog_msg(): """ +def _as_argument_type(check): + def argument_type(value: str) -> str: + try: + return check(value) + except ValueError as error: + raise argparse.ArgumentTypeError(str(error)) from error + + return argument_type + + def setup_argument_parser(): parser = argparse.ArgumentParser(description=get_epilog_msg()) + parser.add_argument( + MAX_FEE_FLAG, + metavar="[node|GWEI]", + help=MAX_FEE_HELP, + default=os.environ.get(MAX_FEE_ENVVAR), + type=_as_argument_type(check_max_fee), + ) + parser.add_argument( + PRIORITY_FEE_FLAG, + metavar="GWEI", + help=PRIORITY_FEE_HELP, + default=os.environ.get(PRIORITY_FEE_ENVVAR), + type=_as_argument_type(check_priority_fee), + ) parser.add_argument( "-v", "--version", @@ -112,11 +151,40 @@ def setup_argument_parser(): return parser +def deploy_proxy_with_fees( + ethereum_client: EthereumClient, + proxy_factory: ProxyFactory, + account: LocalAccount, + master_copy: str, + initializer: bytes, + salt_nonce: int, + fees: dict, +) -> EthereumTxSent: + """ + ``ProxyFactory.deploy_proxy_contract_with_nonce`` with EIP-1559 fees given by hand (it only takes a legacy + gas price). + """ + create_proxy = proxy_factory.get_deploy_function(chain_specific=False)( + master_copy, initializer, salt_nonce + ) + contract_address = create_proxy.call({"from": account.address}) + tx = create_proxy.build_transaction( + {"from": account.address, "chainId": ethereum_client.get_chain_id(), **fees} + ) + tx_hash = ethereum_client.send_unsigned_transaction(tx, private_key=account.key) + return EthereumTxSent(tx_hash, tx, contract_address) + + def main(*args, **kwargs) -> EthereumTxSent | None: print_formatted_text(text2art("Safe Creator")) # Print fancy text parser = setup_argument_parser() + sys.argv[1:1] = take_fee_flags(sys.argv) args = parser.parse_args() + set_fees( + getattr(args, "max_fee_per_gas", None), + getattr(args, "max_priority_fee_per_gas", None), + ) node_url: URI = args.node_url account: LocalAccount = Account.from_key(args.private_key) no_confirm: bool = args.no_confirm @@ -241,12 +309,28 @@ def yes_or_no(prompt: str) -> bool: ) sys.exit(1) + fees = eip1559_fees(ethereum_client) + pricing = f" with {describe(fees)}" if fees else "" if yes_or_no( - f"Safe will be deployed on {expected_safe_address}, would you like to proceed?" + f"Safe will be deployed on {expected_safe_address}{pricing}, would you like to proceed?" ): - ethereum_tx_sent = proxy_factory.deploy_proxy_contract_with_nonce( - account, safe_contract_address, safe_creation_tx_data, salt_nonce - ) + if fees: + ethereum_tx_sent = deploy_proxy_with_fees( + ethereum_client, + proxy_factory, + account, + safe_contract_address, + safe_creation_tx_data, + salt_nonce, + fees, + ) + else: + ethereum_tx_sent = proxy_factory.deploy_proxy_contract_with_nonce( + account, + safe_contract_address, + safe_creation_tx_data, + salt_nonce, + ) print_formatted_text( f"Sent tx with tx-hash={to_0x_hex_str(ethereum_tx_sent.tx_hash)} " f"Safe={ethereum_tx_sent.contract_address} is being created" diff --git a/tests/test_gas_fees.py b/tests/test_gas_fees.py new file mode 100644 index 00000000..8483e44a --- /dev/null +++ b/tests/test_gas_fees.py @@ -0,0 +1,256 @@ +import unittest +from unittest import mock + +from eth_account import Account +from typer.testing import CliRunner +from web3 import Web3 + +from safe_cli import gas_fees +from safe_cli.gas_fees import ( + MAX_FEE_MULTIPLE, + check_max_fee, + check_priority_fee, + eip1559_fees, + parse_gwei, + set_fees, + take_fee_flags, +) +from safe_cli.main import app +from safe_cli.operators.hw_wallets.hw_wallet_manager import HwWalletManager + +from .safe_cli_test_case_mixin import SafeCliTestCaseMixin + +runner = CliRunner() +GWEI = 10**9 + + +def client(base_fee: int, tip: int, node_price: int, eip1559: bool = True): + ethereum_client = mock.Mock() + ethereum_client.is_eip1559_supported.return_value = eip1559 + ethereum_client.estimate_fee_eip1559.return_value = (base_fee, tip) + ethereum_client.w3.eth.gas_price = node_price + return ethereum_client + + +class TestGasFees(unittest.TestCase): + def tearDown(self): + set_fees(None, None) + + def test_parse_gwei(self): + self.assertEqual(parse_gwei("30", "--x"), 30 * GWEI) + self.assertEqual(parse_gwei("1.5", "--x"), 1_500_000_000) + self.assertEqual(parse_gwei("0.000000001", "--x"), 1) + self.assertEqual(parse_gwei("0", "--x", allow_zero=True), 0) + for value in ("0", "-1", "abc", "", "nan", "inf", "0.0000000001"): + with self.assertRaises(ValueError): + parse_gwei(value, "--x") + + def test_checks(self): + for value in (None, "node", "30"): + self.assertEqual(check_max_fee(value), value) + for value in (None, "0", "2"): + self.assertEqual(check_priority_fee(value), value) + for check, value in ( + (check_max_fee, "fast"), + (check_max_fee, "0"), + (check_priority_fee, "node"), + (check_priority_fee, "-1"), + ): + with self.assertRaises(ValueError): + check(value) + + def test_take_flags_anywhere(self): + arguments = ["safe-cli", "0xSafe", "--max-fee-per-gas", "http://node"] + self.assertEqual(take_fee_flags(arguments), ["--max-fee-per-gas=node"]) + self.assertEqual(arguments, ["safe-cli", "0xSafe", "http://node"]) + + arguments = [ + "safe-cli", + "--max-priority-fee-per-gas", + "2", + "send-ether", + "--max-fee-per-gas=40", + "--max-fee-per-gas=50", + ] + self.assertEqual( + take_fee_flags(arguments), + ["--max-priority-fee-per-gas=2", "--max-fee-per-gas=50"], + ) + self.assertEqual(arguments, ["safe-cli", "send-ether"]) + + arguments = ["safe-cli", "0xSafe", "http://node"] + self.assertEqual(take_fee_flags(arguments), []) + self.assertEqual(arguments, ["safe-cli", "0xSafe", "http://node"]) + + def test_default_is_unchanged(self): + self.assertIsNone(eip1559_fees(client(10 * GWEI, 1 * GWEI, 11 * GWEI))) + + def test_cap_from_node(self): + # The fee history forecasts less than the node's price (a fixed-fee chain): the cap follows the node + set_fees("node", None) + self.assertEqual( + eip1559_fees(client(625_625 * GWEI, 0, 715_000 * GWEI)), + {"maxFeePerGas": 715_000 * GWEI, "maxPriorityFeePerGas": 0}, + ) + # The fee history forecasts more: keep it + self.assertEqual( + eip1559_fees(client(20 * GWEI, 2 * GWEI, 15 * GWEI)), + {"maxFeePerGas": 22 * GWEI, "maxPriorityFeePerGas": 2 * GWEI}, + ) + + def test_fixed_cap_and_tip(self): + set_fees("40", "3") + self.assertEqual( + eip1559_fees(client(20 * GWEI, 1 * GWEI, 21 * GWEI)), + {"maxFeePerGas": 40 * GWEI, "maxPriorityFeePerGas": 3 * GWEI}, + ) + # Only a tip: the cap is base fee plus that tip, as safe-cli computes it + set_fees(None, "3") + self.assertEqual( + eip1559_fees(client(20 * GWEI, 1 * GWEI, 21 * GWEI)), + {"maxFeePerGas": 23 * GWEI, "maxPriorityFeePerGas": 3 * GWEI}, + ) + # A fixed cap below the estimated tip lowers the tip to the cap + set_fees("1", None) + self.assertEqual( + eip1559_fees(client(1 * GWEI, 2 * GWEI, 3 * GWEI)), + {"maxFeePerGas": 1 * GWEI, "maxPriorityFeePerGas": 1 * GWEI}, + ) + + def test_refusals(self): + set_fees(str(21 * MAX_FEE_MULTIPLE), None) + self.assertEqual( + eip1559_fees(client(20 * GWEI, 1 * GWEI, 21 * GWEI))["maxFeePerGas"], + 21 * MAX_FEE_MULTIPLE * GWEI, + ) + set_fees(str(21 * MAX_FEE_MULTIPLE + 1), None) + with self.assertRaisesRegex(ValueError, "more than 10 times"): + eip1559_fees(client(20 * GWEI, 1 * GWEI, 21 * GWEI)) + set_fees("30", "31") + with self.assertRaisesRegex(ValueError, "above the cap"): + eip1559_fees(client(20 * GWEI, 1 * GWEI, 21 * GWEI)) + set_fees("node", None) + with self.assertRaisesRegex(ValueError, "need a network with EIP-1559"): + eip1559_fees(client(0, 0, 21 * GWEI, eip1559=False)) + + def test_command_line_options(self): + result = runner.invoke(app, ["--max-fee-per-gas=fast", "version"]) + self.assertEqual(result.exit_code, 2) + + result = runner.invoke( + app, ["--max-fee-per-gas=31", "--max-priority-fee-per-gas=0", "version"] + ) + self.assertEqual(result.exit_code, 0) + self.assertEqual((gas_fees._max_fee, gas_fees._priority_fee), ("31", "0")) + + result = runner.invoke(app, ["version"]) + self.assertEqual(result.exit_code, 0) + self.assertEqual((gas_fees._max_fee, gas_fees._priority_fee), (None, None)) + + with mock.patch.dict( + "os.environ", + { + "SAFE_CLI_MAX_FEE_PER_GAS": "node", + "SAFE_CLI_MAX_PRIORITY_FEE_PER_GAS": "1", + }, + ): + runner.invoke(app, ["version"]) + self.assertEqual((gas_fees._max_fee, gas_fees._priority_fee), ("node", "1")) + # The command line overrides the environment + runner.invoke(app, ["--max-fee-per-gas=32", "version"]) + self.assertEqual(gas_fees._max_fee, "32") + + with mock.patch.dict("os.environ", {"SAFE_CLI_MAX_FEE_PER_GAS": "30gwei"}): + result = runner.invoke(app, ["version"]) + self.assertEqual(result.exit_code, 2) + + +class TestGasFeesTransactions(SafeCliTestCaseMixin, unittest.TestCase): + def tearDown(self): + set_fees(None, None) + + def _operator(self): + safe_operator = self.setup_operator() + safe_operator.accounts.add(self.ethereum_test_account) + safe_operator.default_sender = self.ethereum_test_account + self.w3.eth.send_transaction( + { + "from": self.ethereum_test_account.address, + "to": safe_operator.address, + "value": 10, + } + ) + return safe_operator + + def _sent(self, safe_operator, block: int): + return [ + transaction + for number in range(block + 1, self.w3.eth.block_number + 1) + for transaction in self.w3.eth.get_block(number, True)["transactions"] + if transaction["to"] == safe_operator.address + ] + + def test_execute_fees(self): + safe_operator = self._operator() + to = Account.create().address + cap = self.w3.eth.gas_price * 3 + set_fees(str(Web3.from_wei(cap, "gwei")), "1.5") + block = self.w3.eth.block_number + self.assertTrue(safe_operator.send_ether(to, 1)) + (transaction,) = self._sent(safe_operator, block) + self.assertEqual(transaction["type"], 2) + self.assertEqual(transaction["maxFeePerGas"], cap) + self.assertEqual(transaction["maxPriorityFeePerGas"], 1_500_000_000) + + set_fees(str(Web3.from_wei(cap * 1000, "gwei")), None) + block = self.w3.eth.block_number + with self.assertRaises(ValueError): + safe_operator.send_ether(to, 1) + self.assertEqual(self._sent(safe_operator, block), []) + + def test_approve_hash_fees(self): + safe_operator = self._operator() + set_fees("node", "0.5") + block = self.w3.eth.block_number + self.assertTrue( + safe_operator.approve_hash( + Web3.keccak(text="fees"), self.ethereum_test_account.address + ) + ) + (transaction,) = self._sent(safe_operator, block) + self.assertEqual(transaction["type"], 2) + self.assertEqual(transaction["maxPriorityFeePerGas"], 500_000_000) + self.assertGreaterEqual(transaction["maxFeePerGas"], self.w3.eth.gas_price) + + def test_hardware_wallet_execute_fees(self): + owner = self.ethereum_test_account + safe = self.deploy_test_safe( + owners=[owner.address], + threshold=1, + initial_funding_wei=self.w3.to_wei(0.1, "ether"), + ) + safe_tx = safe.build_multisig_tx(Account.create().address, 10, b"") + safe_tx.sign(owner.key) + hw_wallet_manager = HwWalletManager() + sender = mock.Mock() + sender.address = owner.address + signed = [] + + def sign(tx, chain_id): + signed.append(dict(tx)) + return self.w3.eth.account.sign_transaction(tx, owner.key).raw_transaction + + sender.get_signed_raw_transaction.side_effect = sign + hw_wallet_manager.sender = sender + fees = {"maxFeePerGas": self.w3.eth.gas_price * 2, "maxPriorityFeePerGas": 7} + tx_hash, tx = hw_wallet_manager.execute_safe_tx(safe_tx, eip1559_fees=fees) + self.assertEqual(signed[0]["maxFeePerGas"], fees["maxFeePerGas"]) + self.assertEqual(signed[0]["maxPriorityFeePerGas"], 7) + self.assertNotIn("gasPrice", signed[0]) + transaction = self.w3.eth.get_transaction(tx_hash) + self.assertEqual(transaction["type"], 2) + self.assertEqual(transaction["maxPriorityFeePerGas"], 7) + + +if __name__ == "__main__": + unittest.main() From f823661ad1990f2c4abbbefe1a590f27af4ea2c6 Mon Sep 17 00:00:00 2001 From: techdigger <72038289+techdigger@users.noreply.github.com> Date: Thu, 12 Nov 2026 20:13:42 +0000 Subject: [PATCH 2/5] Do not cap EIP-1559 fees below the node's gas price Replace the fee options with one rule, in one place. EthereumClient.set_eip1559_fees takes maxFeePerGas from eth_feeHistory: the next block's forecast base fee plus the tip. A chain that charges more than its fee history forecasts (a fixed-fee chain whose node still projects a falling base fee) refuses such a transaction. There the node's eth_gasPrice is above the forecast, and it becomes the cap. Where the forecast is at or above eth_gasPrice, nothing changes. One override of set_eip1559_fees, in a subclass SafeOperator constructs, covers execution with a local key, approve_hash and execution with a hardware wallet. No options, and safe-creator is unchanged. --- src/safe_cli/ethereum_client.py | 24 ++ src/safe_cli/gas_fees.py | 158 ----------- src/safe_cli/main.py | 58 +--- .../operators/hw_wallets/hw_wallet_manager.py | 10 +- src/safe_cli/operators/safe_operator.py | 59 +--- src/safe_cli/safe_creator.py | 92 +------ tests/test_ethereum_client.py | 103 +++++++ tests/test_gas_fees.py | 256 ------------------ 8 files changed, 147 insertions(+), 613 deletions(-) create mode 100644 src/safe_cli/ethereum_client.py delete mode 100644 src/safe_cli/gas_fees.py create mode 100644 tests/test_ethereum_client.py delete mode 100644 tests/test_gas_fees.py diff --git a/src/safe_cli/ethereum_client.py b/src/safe_cli/ethereum_client.py new file mode 100644 index 00000000..eb41fe1c --- /dev/null +++ b/src/safe_cli/ethereum_client.py @@ -0,0 +1,24 @@ +from safe_eth.eth import EthereumClient, TxSpeed +from web3.types import TxParams, Wei + + +class SafeCliEthereumClient(EthereumClient): + """ + ``EthereumClient`` that does not cap the fee of an EIP-1559 transaction below the node's own gas price. + + ``EthereumClient.set_eip1559_fees`` takes ``maxFeePerGas`` from ``eth_feeHistory``: the next block's forecast + base fee plus the tip. A chain that charges more than its fee history forecasts (a fixed-fee chain whose node + still projects a falling base fee) refuses such a transaction. There the node's ``eth_gasPrice`` is above the + forecast, and it becomes the cap. Where the forecast is at or above ``eth_gasPrice``, nothing changes. + """ + + def set_eip1559_fees( + self, tx: TxParams, tx_speed: TxSpeed = TxSpeed.NORMAL + ) -> TxParams: + """ + :return: TxParams in EIP1559 format, with ``maxFeePerGas`` of at least ``eth_gasPrice`` + :raises: ValueError if EIP1559 not supported + """ + tx = super().set_eip1559_fees(tx, tx_speed) + tx["maxFeePerGas"] = Wei(max(tx["maxFeePerGas"], self.w3.eth.gas_price)) + return tx diff --git a/src/safe_cli/gas_fees.py b/src/safe_cli/gas_fees.py deleted file mode 100644 index 5c547681..00000000 --- a/src/safe_cli/gas_fees.py +++ /dev/null @@ -1,158 +0,0 @@ -""" -EIP-1559 fees set by hand. - -By default safe-cli takes the fees of every transaction it sends from ``eth_feeHistory``: the next block's base fee -plus the median tip. Two global options override them: - -- ``--max-fee-per-gas``: a cap of at least ``eth_gasPrice``, for chains whose fee history forecasts less than they - charge; ``--max-fee-per-gas=``: that cap; -- ``--max-priority-fee-per-gas=``: that tip, for example to be included sooner on a busy chain or to replace - a pending transaction. - -The environment variables ``SAFE_CLI_MAX_FEE_PER_GAS`` (``node`` or gwei) and ``SAFE_CLI_MAX_PRIORITY_FEE_PER_GAS`` -(gwei) set defaults; the command line overrides them. -""" - -from decimal import Decimal, InvalidOperation - -from safe_eth.eth import EthereumClient, TxSpeed -from web3 import Web3 -from web3.types import TxParams - -MAX_FEE_FLAG = "--max-fee-per-gas" -MAX_FEE_ENVVAR = "SAFE_CLI_MAX_FEE_PER_GAS" -MAX_FEE_HELP = ( - "Cap the fee per gas of every transaction sent. Without a value (or with 'node'): at least eth_gasPrice, for " - "chains whose fee history forecasts less than they charge. With a value: that many gwei, at most 10 times the " - "node's price." -) -PRIORITY_FEE_FLAG = "--max-priority-fee-per-gas" -PRIORITY_FEE_ENVVAR = "SAFE_CLI_MAX_PRIORITY_FEE_PER_GAS" -PRIORITY_FEE_HELP = ( - "Tip per gas, in gwei, of every transaction sent, instead of the fee history's median: for example to be " - "included sooner, or to replace a pending transaction. At most the cap." -) -NODE_PRICE = "node" -# A cap above this many times the node's price is refused: far more likely a typing mistake (gwei taken for wei, -# extra zeros) than an intended fee. -MAX_FEE_MULTIPLE = 10 - -_max_fee: str | None = None -_priority_fee: str | None = None - - -def parse_gwei(value: str, flag: str, allow_zero: bool = False) -> int: - """ - :return: ``value`` gwei in wei - :raises ValueError: unless ``value`` is a finite number of gwei, positive (or zero if allowed), that is a whole - number of wei - """ - try: - gwei = Decimal(value) - valid = gwei.is_finite() and (gwei > 0 or (allow_zero and gwei == 0)) - except InvalidOperation: - valid = False - wei = gwei * 10**9 if valid else Decimal(0) - if not valid or wei != wei.to_integral_value(): - expected = ( - "a gas price in gwei" if allow_zero else "a positive gas price in gwei" - ) - if flag == MAX_FEE_FLAG: - expected = f"'{NODE_PRICE}' or {expected}" - raise ValueError(f"{flag}: expected {expected}, got {value!r}") - return int(wei) - - -def check_max_fee(value: str | None) -> str | None: - if value is not None and value != NODE_PRICE: - parse_gwei(value, MAX_FEE_FLAG) - return value - - -def check_priority_fee(value: str | None) -> str | None: - if value is not None: - parse_gwei(value, PRIORITY_FEE_FLAG, allow_zero=True) - return value - - -def set_fees(max_fee: str | None, priority_fee: str | None) -> None: - """ - Sets the fees of every transaction sent from now on. ``None`` for both keeps safe-cli's estimate. - """ - global _max_fee, _priority_fee - _max_fee, _priority_fee = check_max_fee(max_fee), check_priority_fee(priority_fee) - - -def take_fee_flags(arguments: list[str]) -> list[str]: - """ - Removes the fee options from ``arguments``, wherever they are, so they can be given anywhere on the command line, - and ``--max-fee-per-gas`` without a value. - - :return: the options in the one form the parsers declare (``--flag=value``), the last one of each given winning - """ - taken: dict[str, str] = {} - remaining: list[str] = [] - arguments_iter = iter(arguments) - for argument in arguments_iter: - if argument == MAX_FEE_FLAG: - taken[MAX_FEE_FLAG] = NODE_PRICE - elif argument == PRIORITY_FEE_FLAG: - taken[PRIORITY_FEE_FLAG] = next(arguments_iter, "") - elif argument.startswith((MAX_FEE_FLAG + "=", PRIORITY_FEE_FLAG + "=")): - flag, value = argument.split("=", 1) - taken[flag] = value - else: - remaining.append(argument) - arguments[:] = remaining - return [f"{flag}={value}" for flag, value in taken.items()] - - -def eip1559_fees(ethereum_client: EthereumClient) -> TxParams | None: - """ - :return: ``maxFeePerGas`` and ``maxPriorityFeePerGas`` for the next transaction, or ``None`` to keep safe-cli's - estimate - :raises ValueError: if the network has no EIP-1559 fee history, if the cap is more than ``MAX_FEE_MULTIPLE`` - times the node's price, or if the tip is above the cap - """ - if _max_fee is None and _priority_fee is None: - return None - if not ethereum_client.is_eip1559_supported(): - raise ValueError( - f"{MAX_FEE_FLAG} and {PRIORITY_FEE_FLAG} need a network with EIP-1559 fees, nothing was sent" - ) - base_fee, estimated_tip = ethereum_client.estimate_fee_eip1559(TxSpeed.NORMAL) - node_price = ethereum_client.w3.eth.gas_price - if _priority_fee is None: - tip = estimated_tip - else: - tip = parse_gwei(_priority_fee, PRIORITY_FEE_FLAG, allow_zero=True) - if _max_fee is None: - max_fee = base_fee + tip - elif _max_fee == NODE_PRICE: - max_fee = max(base_fee + tip, node_price) - else: - max_fee = parse_gwei(_max_fee, MAX_FEE_FLAG) - reference = max(node_price, base_fee + estimated_tip) - if reference and max_fee > reference * MAX_FEE_MULTIPLE: - raise ValueError( - f"{MAX_FEE_FLAG}: {gwei(max_fee)} gwei is more than {MAX_FEE_MULTIPLE} times the node's price " - f"({gwei(reference)} gwei), nothing was sent" - ) - if tip > max_fee: - if _priority_fee is not None: - raise ValueError( - f"{PRIORITY_FEE_FLAG}: {gwei(tip)} gwei is above the cap of {gwei(max_fee)} gwei, nothing was sent" - ) - tip = max_fee - return {"maxFeePerGas": max_fee, "maxPriorityFeePerGas": tip} - - -def gwei(wei: int) -> str: - return str(Web3.from_wei(wei, "gwei")) - - -def describe(fees: TxParams) -> str: - return ( - f"max fee per gas {gwei(fees['maxFeePerGas'])} gwei, " - f"max priority fee per gas {gwei(fees['maxPriorityFeePerGas'])} gwei" - ) diff --git a/src/safe_cli/main.py b/src/safe_cli/main.py index 1fd78672..84cbeb54 100644 --- a/src/safe_cli/main.py +++ b/src/safe_cli/main.py @@ -15,18 +15,6 @@ from . import VERSION from .argparse_validators import check_hex_str -from .gas_fees import ( - MAX_FEE_ENVVAR, - MAX_FEE_FLAG, - MAX_FEE_HELP, - PRIORITY_FEE_ENVVAR, - PRIORITY_FEE_FLAG, - PRIORITY_FEE_HELP, - check_max_fee, - check_priority_fee, - set_fees, - take_fee_flags, -) from .operators import SafeOperator from .safe_cli import SafeCli from .tx_builder.exceptions import SoliditySyntaxError, TxBuilderEncodingError @@ -42,44 +30,6 @@ app = typer.Typer(name="Safe CLI") -def _as_parameter_check(check): - def callback(value: str | None) -> str | None: - try: - return check(value) - except ValueError as error: - raise typer.BadParameter(str(error)) from error - - return callback - - -@app.callback() -def global_options( - max_fee_per_gas: Annotated[ - str | None, - typer.Option( - MAX_FEE_FLAG, - envvar=MAX_FEE_ENVVAR, - metavar="[node|GWEI]", - help=MAX_FEE_HELP, - callback=_as_parameter_check(check_max_fee), - show_default=False, - ), - ] = None, - max_priority_fee_per_gas: Annotated[ - str | None, - typer.Option( - PRIORITY_FEE_FLAG, - envvar=PRIORITY_FEE_ENVVAR, - metavar="GWEI", - help=PRIORITY_FEE_HELP, - callback=_as_parameter_check(check_priority_fee), - show_default=False, - ), - ] = None, -): - set_fees(max_fee_per_gas, max_priority_fee_per_gas) - - def _build_safe_operator_and_load_keys( safe_address: ChecksumAddress, node_url: str, @@ -393,10 +343,7 @@ def version(): for command in get_command(app).commands.keys() ] ) - + "\n\n\n\nUse the --help option of each command to see the usage options." - + "\n\n\n\nGlobal options, for every command and the attended mode, anywhere on the command line:" - + f"\n\n {MAX_FEE_FLAG}[=node|GWEI] {MAX_FEE_HELP} Default: ${MAX_FEE_ENVVAR}." - + f"\n\n {PRIORITY_FEE_FLAG}=GWEI {PRIORITY_FEE_HELP} Default: ${PRIORITY_FEE_ENVVAR}.", + + "\n\n\n\nUse the --help option of each command to see the usage options.", ) def default_attended_mode( address: Annotated[ @@ -454,10 +401,7 @@ def _is_safe_cli_default_command(arguments: list[str]) -> bool: def main(): - # The fee options are global; take them from wherever they were given and hand them to the application itself - fee_flags = take_fee_flags(sys.argv) # By default, the attended mode is initialised. Otherwise, the required command must be specified. if _is_safe_cli_default_command(sys.argv): sys.argv.insert(1, "attended-mode") - sys.argv[1:1] = fee_flags app() diff --git a/src/safe_cli/operators/hw_wallets/hw_wallet_manager.py b/src/safe_cli/operators/hw_wallets/hw_wallet_manager.py index 39639a3d..56ba0ccf 100644 --- a/src/safe_cli/operators/hw_wallets/hw_wallet_manager.py +++ b/src/safe_cli/operators/hw_wallets/hw_wallet_manager.py @@ -172,7 +172,6 @@ def execute_safe_tx( tx_gas_price: int | None = None, tx_nonce: int | None = None, eip1559_speed: TxSpeed | None = None, - eip1559_fees: TxParams | None = None, ) -> tuple[HexBytes, TxParams]: """ Send multisig tx to the Safe @@ -182,17 +181,10 @@ def execute_safe_tx( :param tx_gas_price: Gas price of the external tx. If not, `gas_price` will be used :param tx_nonce: Force nonce for `tx_sender` :param eip1559_speed: If provided, use EIP1559 transaction - :param eip1559_fees: If provided, use these ``maxFeePerGas`` and ``maxPriorityFeePerGas`` :return: Tuple(tx_hash, tx) """ - if eip1559_fees: - tx_parameters = { - "from": self.sender.address, - "chainId": safe_tx.ethereum_client.get_chain_id(), - **eip1559_fees, - } - elif eip1559_speed and safe_tx.ethereum_client.is_eip1559_supported(): + if eip1559_speed and safe_tx.ethereum_client.is_eip1559_supported(): tx_parameters = safe_tx.ethereum_client.set_eip1559_fees( { "from": self.sender.address, diff --git a/src/safe_cli/operators/safe_operator.py b/src/safe_cli/operators/safe_operator.py index f8b09190..f3e20699 100644 --- a/src/safe_cli/operators/safe_operator.py +++ b/src/safe_cli/operators/safe_operator.py @@ -37,10 +37,9 @@ from web3 import Web3 from web3.contract import Contract from web3.exceptions import BadFunctionCallOutput -from web3.types import TxParams, Wei +from safe_cli.ethereum_client import SafeCliEthereumClient from safe_cli.ethereum_hd_wallet import get_account_from_words -from safe_cli.gas_fees import describe, eip1559_fees from safe_cli.operators.exceptions import ( AccountNotLoadedException, ExistingOwnerException, @@ -164,7 +163,7 @@ def __init__( ): self.address = address self.node_url = node_url - self.ethereum_client = EthereumClient(self.node_url) + self.ethereum_client = SafeCliEthereumClient(self.node_url) self.ens = ENS.from_web3(self.ethereum_client.w3) self.network: EthereumNetwork = self.ethereum_client.get_network() try: @@ -444,18 +443,17 @@ def approve_hash(self, hash_to_approve: HexBytes, sender: str) -> bool: sender_account_address, hash_to_approve ): raise HashAlreadyApproved(hash_to_approve, sender) - tx_params = { - "from": sender_account_address, - "nonce": self.ethereum_client.get_nonce_for_account(sender_account_address), - } - fees = eip1559_fees(self.ethereum_client) - if fees: - tx_params.update(fees) - print_formatted_text(HTML(f"{describe(fees)}")) transaction_to_send = self.safe_contract.functions.approveHash( hash_to_approve - ).build_transaction(tx_params) - if not fees and self.ethereum_client.is_eip1559_supported(): + ).build_transaction( + { + "from": sender_account_address, + "nonce": self.ethereum_client.get_nonce_for_account( + sender_account_address + ), + } + ) + if self.ethereum_client.is_eip1559_supported(): transaction_to_send = self.ethereum_client.set_eip1559_fees( transaction_to_send ) @@ -1038,20 +1036,16 @@ def execute_safe_transaction(self, safe_tx: SafeTx): else: call_result = safe_tx.call(self.hw_wallet_manager.sender.address) print_formatted_text(HTML(f"Result: {call_result}")) - fees = eip1559_fees(self.ethereum_client) - pricing = f" with {describe(fees)}" if fees else "" if not self.interactive or yes_or_no_question( - "Do you want to execute tx " + str(safe_tx) + pricing + "Do you want to execute tx " + str(safe_tx) ): - if self.default_sender and fees: - tx_hash, tx = self._execute_with_fees(safe_tx, fees) - elif self.default_sender: + if self.default_sender: tx_hash, tx = safe_tx.execute( self.default_sender.key, eip1559_speed=TxSpeed.NORMAL ) else: tx_hash, tx = self.hw_wallet_manager.execute_safe_tx( - safe_tx, eip1559_speed=TxSpeed.NORMAL, eip1559_fees=fees + safe_tx, eip1559_speed=TxSpeed.NORMAL ) self.executed_transactions.append(to_0x_hex_str(tx_hash)) print_formatted_text( @@ -1089,31 +1083,6 @@ def execute_safe_transaction(self, safe_tx: SafeTx): ) return False - def _execute_with_fees( - self, safe_tx: SafeTx, fees: TxParams - ) -> tuple[HexBytes, TxParams]: - """ - ``SafeTx.execute`` with the default sender and EIP-1559 fees given by hand (``SafeTx.execute`` only - estimates them). Same gas and nonce handling. - """ - sender = self.default_sender - safe_tx.tx = safe_tx.w3_tx.build_transaction( - { - "from": sender.address, - "chainId": self.ethereum_client.get_chain_id(), - **fees, - } - ) - safe_tx.tx["gas"] = Wei( - max(safe_tx.tx["gas"] + 75000, safe_tx.recommended_gas()) - ) - safe_tx.tx_hash = self.ethereum_client.send_unsigned_transaction( - safe_tx.tx, private_key=sender.key, retry=True, block_identifier="latest" - ) - # As SafeTx.execute: the Safe nonce increases even if the transaction fails, so the signatures are spent - safe_tx.signatures = b"" - return safe_tx.tx_hash, safe_tx.tx - # Batch_transactions multisend def batch_safe_txs( self, safe_nonce: int, safe_txs: Sequence[SafeTx] diff --git a/src/safe_cli/safe_creator.py b/src/safe_cli/safe_creator.py index 84032597..b8256de2 100644 --- a/src/safe_cli/safe_creator.py +++ b/src/safe_cli/safe_creator.py @@ -1,6 +1,5 @@ #!/bin/env python3 import argparse -import os import secrets import sys @@ -16,20 +15,6 @@ from safe_eth.safe import ProxyFactory, Safe from safe_eth.util.util import to_0x_hex_str -from safe_cli.gas_fees import ( - MAX_FEE_ENVVAR, - MAX_FEE_FLAG, - MAX_FEE_HELP, - PRIORITY_FEE_ENVVAR, - PRIORITY_FEE_FLAG, - PRIORITY_FEE_HELP, - check_max_fee, - check_priority_fee, - describe, - eip1559_fees, - set_fees, - take_fee_flags, -) from safe_cli.safe_addresses import ( get_default_fallback_handler_address, get_proxy_factory_address, @@ -53,32 +38,8 @@ def get_epilog_msg(): """ -def _as_argument_type(check): - def argument_type(value: str) -> str: - try: - return check(value) - except ValueError as error: - raise argparse.ArgumentTypeError(str(error)) from error - - return argument_type - - def setup_argument_parser(): parser = argparse.ArgumentParser(description=get_epilog_msg()) - parser.add_argument( - MAX_FEE_FLAG, - metavar="[node|GWEI]", - help=MAX_FEE_HELP, - default=os.environ.get(MAX_FEE_ENVVAR), - type=_as_argument_type(check_max_fee), - ) - parser.add_argument( - PRIORITY_FEE_FLAG, - metavar="GWEI", - help=PRIORITY_FEE_HELP, - default=os.environ.get(PRIORITY_FEE_ENVVAR), - type=_as_argument_type(check_priority_fee), - ) parser.add_argument( "-v", "--version", @@ -151,40 +112,11 @@ def setup_argument_parser(): return parser -def deploy_proxy_with_fees( - ethereum_client: EthereumClient, - proxy_factory: ProxyFactory, - account: LocalAccount, - master_copy: str, - initializer: bytes, - salt_nonce: int, - fees: dict, -) -> EthereumTxSent: - """ - ``ProxyFactory.deploy_proxy_contract_with_nonce`` with EIP-1559 fees given by hand (it only takes a legacy - gas price). - """ - create_proxy = proxy_factory.get_deploy_function(chain_specific=False)( - master_copy, initializer, salt_nonce - ) - contract_address = create_proxy.call({"from": account.address}) - tx = create_proxy.build_transaction( - {"from": account.address, "chainId": ethereum_client.get_chain_id(), **fees} - ) - tx_hash = ethereum_client.send_unsigned_transaction(tx, private_key=account.key) - return EthereumTxSent(tx_hash, tx, contract_address) - - def main(*args, **kwargs) -> EthereumTxSent | None: print_formatted_text(text2art("Safe Creator")) # Print fancy text parser = setup_argument_parser() - sys.argv[1:1] = take_fee_flags(sys.argv) args = parser.parse_args() - set_fees( - getattr(args, "max_fee_per_gas", None), - getattr(args, "max_priority_fee_per_gas", None), - ) node_url: URI = args.node_url account: LocalAccount = Account.from_key(args.private_key) no_confirm: bool = args.no_confirm @@ -309,28 +241,12 @@ def yes_or_no(prompt: str) -> bool: ) sys.exit(1) - fees = eip1559_fees(ethereum_client) - pricing = f" with {describe(fees)}" if fees else "" if yes_or_no( - f"Safe will be deployed on {expected_safe_address}{pricing}, would you like to proceed?" + f"Safe will be deployed on {expected_safe_address}, would you like to proceed?" ): - if fees: - ethereum_tx_sent = deploy_proxy_with_fees( - ethereum_client, - proxy_factory, - account, - safe_contract_address, - safe_creation_tx_data, - salt_nonce, - fees, - ) - else: - ethereum_tx_sent = proxy_factory.deploy_proxy_contract_with_nonce( - account, - safe_contract_address, - safe_creation_tx_data, - salt_nonce, - ) + ethereum_tx_sent = proxy_factory.deploy_proxy_contract_with_nonce( + account, safe_contract_address, safe_creation_tx_data, salt_nonce + ) print_formatted_text( f"Sent tx with tx-hash={to_0x_hex_str(ethereum_tx_sent.tx_hash)} " f"Safe={ethereum_tx_sent.contract_address} is being created" diff --git a/tests/test_ethereum_client.py b/tests/test_ethereum_client.py new file mode 100644 index 00000000..dc0916f7 --- /dev/null +++ b/tests/test_ethereum_client.py @@ -0,0 +1,103 @@ +import unittest +from unittest import mock + +from eth_account import Account +from safe_eth.eth import EthereumClient +from web3 import Web3 + +from safe_cli.ethereum_client import SafeCliEthereumClient +from safe_cli.operators.hw_wallets.hw_wallet_manager import HwWalletManager + +from .safe_cli_test_case_mixin import SafeCliTestCaseMixin + +GWEI = 10**9 + + +class TestSafeCliEthereumClient(SafeCliTestCaseMixin, unittest.TestCase): + def _fees(self, base_fee: int, tip: int, node_price: int) -> dict: + ethereum_client = SafeCliEthereumClient(self.ethereum_node_url) + with ( + mock.patch.object( + EthereumClient, "estimate_fee_eip1559", return_value=(base_fee, tip) + ), + mock.patch.object( + type(ethereum_client.w3.eth), + "gas_price", + new_callable=mock.PropertyMock, + return_value=node_price, + ), + ): + return ethereum_client.set_eip1559_fees({"gasPrice": 1}) + + def test_forecast_at_or_above_the_node_price_is_unchanged(self): + for node_price in (15 * GWEI, 22 * GWEI): + fees = self._fees(20 * GWEI, 2 * GWEI, node_price) + self.assertEqual(fees["maxFeePerGas"], 22 * GWEI) + self.assertEqual(fees["maxPriorityFeePerGas"], 2 * GWEI) + self.assertNotIn("gasPrice", fees) + + def test_forecast_below_the_node_price(self): + # A node that forecasts 625,625 gwei for the next block and charges 715,000 + fees = self._fees(625_625 * GWEI, 0, 715_000 * GWEI) + self.assertEqual(fees["maxFeePerGas"], 715_000 * GWEI) + self.assertEqual(fees["maxPriorityFeePerGas"], 0) + + def _sent_to(self, address: str, since_block: int) -> list: + return [ + transaction + for number in range(since_block + 1, self.w3.eth.block_number + 1) + for transaction in self.w3.eth.get_block(number, True)["transactions"] + if transaction["to"] == address + ] + + def _assert_included(self, transaction, forecast: int): + self.assertEqual(transaction["type"], 2) + base_fee = self.w3.eth.get_block(transaction["blockNumber"])["baseFeePerGas"] + self.assertLess(forecast, base_fee) + self.assertGreaterEqual(transaction["maxFeePerGas"], base_fee) + + def test_every_way_of_sending_uses_it(self): + safe_operator = self.setup_operator() + owner = self.ethereum_test_account + safe_operator.accounts.add(owner) + safe_operator.default_sender = owner + self.w3.eth.send_transaction( + {"from": owner.address, "to": safe_operator.address, "value": 10} + ) + # A fee history that forecasts 0.6 x the base fee the chain charges. A cap at the forecast is below the + # base fee and the node refuses the transaction: "max fee per gas less than block base fee". + forecast = self.w3.eth.get_block("latest")["baseFeePerGas"] * 6 // 10 + with mock.patch.object( + EthereumClient, "estimate_fee_eip1559", return_value=(forecast, 0) + ): + # Executing a Safe transaction with a local key + block = self.w3.eth.block_number + self.assertTrue(safe_operator.send_ether(Account.create().address, 1)) + (transaction,) = self._sent_to(safe_operator.address, block) + self._assert_included(transaction, forecast) + + # approve_hash + block = self.w3.eth.block_number + self.assertTrue( + safe_operator.approve_hash(Web3.keccak(text="fee cap"), owner.address) + ) + (transaction,) = self._sent_to(safe_operator.address, block) + self._assert_included(transaction, forecast) + + # Executing a Safe transaction with a hardware wallet + sender = mock.Mock() + sender.address = owner.address + sender.get_signed_raw_transaction.side_effect = lambda tx, chain_id: ( + self.w3.eth.account.sign_transaction(tx, owner.key).raw_transaction + ) + safe_operator.default_sender = None + safe_operator.hw_wallet_manager = HwWalletManager() + safe_operator.hw_wallet_manager.sender = sender + block = self.w3.eth.block_number + self.assertTrue(safe_operator.send_ether(Account.create().address, 1)) + (transaction,) = self._sent_to(safe_operator.address, block) + self._assert_included(transaction, forecast) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_gas_fees.py b/tests/test_gas_fees.py deleted file mode 100644 index 8483e44a..00000000 --- a/tests/test_gas_fees.py +++ /dev/null @@ -1,256 +0,0 @@ -import unittest -from unittest import mock - -from eth_account import Account -from typer.testing import CliRunner -from web3 import Web3 - -from safe_cli import gas_fees -from safe_cli.gas_fees import ( - MAX_FEE_MULTIPLE, - check_max_fee, - check_priority_fee, - eip1559_fees, - parse_gwei, - set_fees, - take_fee_flags, -) -from safe_cli.main import app -from safe_cli.operators.hw_wallets.hw_wallet_manager import HwWalletManager - -from .safe_cli_test_case_mixin import SafeCliTestCaseMixin - -runner = CliRunner() -GWEI = 10**9 - - -def client(base_fee: int, tip: int, node_price: int, eip1559: bool = True): - ethereum_client = mock.Mock() - ethereum_client.is_eip1559_supported.return_value = eip1559 - ethereum_client.estimate_fee_eip1559.return_value = (base_fee, tip) - ethereum_client.w3.eth.gas_price = node_price - return ethereum_client - - -class TestGasFees(unittest.TestCase): - def tearDown(self): - set_fees(None, None) - - def test_parse_gwei(self): - self.assertEqual(parse_gwei("30", "--x"), 30 * GWEI) - self.assertEqual(parse_gwei("1.5", "--x"), 1_500_000_000) - self.assertEqual(parse_gwei("0.000000001", "--x"), 1) - self.assertEqual(parse_gwei("0", "--x", allow_zero=True), 0) - for value in ("0", "-1", "abc", "", "nan", "inf", "0.0000000001"): - with self.assertRaises(ValueError): - parse_gwei(value, "--x") - - def test_checks(self): - for value in (None, "node", "30"): - self.assertEqual(check_max_fee(value), value) - for value in (None, "0", "2"): - self.assertEqual(check_priority_fee(value), value) - for check, value in ( - (check_max_fee, "fast"), - (check_max_fee, "0"), - (check_priority_fee, "node"), - (check_priority_fee, "-1"), - ): - with self.assertRaises(ValueError): - check(value) - - def test_take_flags_anywhere(self): - arguments = ["safe-cli", "0xSafe", "--max-fee-per-gas", "http://node"] - self.assertEqual(take_fee_flags(arguments), ["--max-fee-per-gas=node"]) - self.assertEqual(arguments, ["safe-cli", "0xSafe", "http://node"]) - - arguments = [ - "safe-cli", - "--max-priority-fee-per-gas", - "2", - "send-ether", - "--max-fee-per-gas=40", - "--max-fee-per-gas=50", - ] - self.assertEqual( - take_fee_flags(arguments), - ["--max-priority-fee-per-gas=2", "--max-fee-per-gas=50"], - ) - self.assertEqual(arguments, ["safe-cli", "send-ether"]) - - arguments = ["safe-cli", "0xSafe", "http://node"] - self.assertEqual(take_fee_flags(arguments), []) - self.assertEqual(arguments, ["safe-cli", "0xSafe", "http://node"]) - - def test_default_is_unchanged(self): - self.assertIsNone(eip1559_fees(client(10 * GWEI, 1 * GWEI, 11 * GWEI))) - - def test_cap_from_node(self): - # The fee history forecasts less than the node's price (a fixed-fee chain): the cap follows the node - set_fees("node", None) - self.assertEqual( - eip1559_fees(client(625_625 * GWEI, 0, 715_000 * GWEI)), - {"maxFeePerGas": 715_000 * GWEI, "maxPriorityFeePerGas": 0}, - ) - # The fee history forecasts more: keep it - self.assertEqual( - eip1559_fees(client(20 * GWEI, 2 * GWEI, 15 * GWEI)), - {"maxFeePerGas": 22 * GWEI, "maxPriorityFeePerGas": 2 * GWEI}, - ) - - def test_fixed_cap_and_tip(self): - set_fees("40", "3") - self.assertEqual( - eip1559_fees(client(20 * GWEI, 1 * GWEI, 21 * GWEI)), - {"maxFeePerGas": 40 * GWEI, "maxPriorityFeePerGas": 3 * GWEI}, - ) - # Only a tip: the cap is base fee plus that tip, as safe-cli computes it - set_fees(None, "3") - self.assertEqual( - eip1559_fees(client(20 * GWEI, 1 * GWEI, 21 * GWEI)), - {"maxFeePerGas": 23 * GWEI, "maxPriorityFeePerGas": 3 * GWEI}, - ) - # A fixed cap below the estimated tip lowers the tip to the cap - set_fees("1", None) - self.assertEqual( - eip1559_fees(client(1 * GWEI, 2 * GWEI, 3 * GWEI)), - {"maxFeePerGas": 1 * GWEI, "maxPriorityFeePerGas": 1 * GWEI}, - ) - - def test_refusals(self): - set_fees(str(21 * MAX_FEE_MULTIPLE), None) - self.assertEqual( - eip1559_fees(client(20 * GWEI, 1 * GWEI, 21 * GWEI))["maxFeePerGas"], - 21 * MAX_FEE_MULTIPLE * GWEI, - ) - set_fees(str(21 * MAX_FEE_MULTIPLE + 1), None) - with self.assertRaisesRegex(ValueError, "more than 10 times"): - eip1559_fees(client(20 * GWEI, 1 * GWEI, 21 * GWEI)) - set_fees("30", "31") - with self.assertRaisesRegex(ValueError, "above the cap"): - eip1559_fees(client(20 * GWEI, 1 * GWEI, 21 * GWEI)) - set_fees("node", None) - with self.assertRaisesRegex(ValueError, "need a network with EIP-1559"): - eip1559_fees(client(0, 0, 21 * GWEI, eip1559=False)) - - def test_command_line_options(self): - result = runner.invoke(app, ["--max-fee-per-gas=fast", "version"]) - self.assertEqual(result.exit_code, 2) - - result = runner.invoke( - app, ["--max-fee-per-gas=31", "--max-priority-fee-per-gas=0", "version"] - ) - self.assertEqual(result.exit_code, 0) - self.assertEqual((gas_fees._max_fee, gas_fees._priority_fee), ("31", "0")) - - result = runner.invoke(app, ["version"]) - self.assertEqual(result.exit_code, 0) - self.assertEqual((gas_fees._max_fee, gas_fees._priority_fee), (None, None)) - - with mock.patch.dict( - "os.environ", - { - "SAFE_CLI_MAX_FEE_PER_GAS": "node", - "SAFE_CLI_MAX_PRIORITY_FEE_PER_GAS": "1", - }, - ): - runner.invoke(app, ["version"]) - self.assertEqual((gas_fees._max_fee, gas_fees._priority_fee), ("node", "1")) - # The command line overrides the environment - runner.invoke(app, ["--max-fee-per-gas=32", "version"]) - self.assertEqual(gas_fees._max_fee, "32") - - with mock.patch.dict("os.environ", {"SAFE_CLI_MAX_FEE_PER_GAS": "30gwei"}): - result = runner.invoke(app, ["version"]) - self.assertEqual(result.exit_code, 2) - - -class TestGasFeesTransactions(SafeCliTestCaseMixin, unittest.TestCase): - def tearDown(self): - set_fees(None, None) - - def _operator(self): - safe_operator = self.setup_operator() - safe_operator.accounts.add(self.ethereum_test_account) - safe_operator.default_sender = self.ethereum_test_account - self.w3.eth.send_transaction( - { - "from": self.ethereum_test_account.address, - "to": safe_operator.address, - "value": 10, - } - ) - return safe_operator - - def _sent(self, safe_operator, block: int): - return [ - transaction - for number in range(block + 1, self.w3.eth.block_number + 1) - for transaction in self.w3.eth.get_block(number, True)["transactions"] - if transaction["to"] == safe_operator.address - ] - - def test_execute_fees(self): - safe_operator = self._operator() - to = Account.create().address - cap = self.w3.eth.gas_price * 3 - set_fees(str(Web3.from_wei(cap, "gwei")), "1.5") - block = self.w3.eth.block_number - self.assertTrue(safe_operator.send_ether(to, 1)) - (transaction,) = self._sent(safe_operator, block) - self.assertEqual(transaction["type"], 2) - self.assertEqual(transaction["maxFeePerGas"], cap) - self.assertEqual(transaction["maxPriorityFeePerGas"], 1_500_000_000) - - set_fees(str(Web3.from_wei(cap * 1000, "gwei")), None) - block = self.w3.eth.block_number - with self.assertRaises(ValueError): - safe_operator.send_ether(to, 1) - self.assertEqual(self._sent(safe_operator, block), []) - - def test_approve_hash_fees(self): - safe_operator = self._operator() - set_fees("node", "0.5") - block = self.w3.eth.block_number - self.assertTrue( - safe_operator.approve_hash( - Web3.keccak(text="fees"), self.ethereum_test_account.address - ) - ) - (transaction,) = self._sent(safe_operator, block) - self.assertEqual(transaction["type"], 2) - self.assertEqual(transaction["maxPriorityFeePerGas"], 500_000_000) - self.assertGreaterEqual(transaction["maxFeePerGas"], self.w3.eth.gas_price) - - def test_hardware_wallet_execute_fees(self): - owner = self.ethereum_test_account - safe = self.deploy_test_safe( - owners=[owner.address], - threshold=1, - initial_funding_wei=self.w3.to_wei(0.1, "ether"), - ) - safe_tx = safe.build_multisig_tx(Account.create().address, 10, b"") - safe_tx.sign(owner.key) - hw_wallet_manager = HwWalletManager() - sender = mock.Mock() - sender.address = owner.address - signed = [] - - def sign(tx, chain_id): - signed.append(dict(tx)) - return self.w3.eth.account.sign_transaction(tx, owner.key).raw_transaction - - sender.get_signed_raw_transaction.side_effect = sign - hw_wallet_manager.sender = sender - fees = {"maxFeePerGas": self.w3.eth.gas_price * 2, "maxPriorityFeePerGas": 7} - tx_hash, tx = hw_wallet_manager.execute_safe_tx(safe_tx, eip1559_fees=fees) - self.assertEqual(signed[0]["maxFeePerGas"], fees["maxFeePerGas"]) - self.assertEqual(signed[0]["maxPriorityFeePerGas"], 7) - self.assertNotIn("gasPrice", signed[0]) - transaction = self.w3.eth.get_transaction(tx_hash) - self.assertEqual(transaction["type"], 2) - self.assertEqual(transaction["maxPriorityFeePerGas"], 7) - - -if __name__ == "__main__": - unittest.main() From c49106d1a562974b10414dc77c136cbfdba7e52c Mon Sep 17 00:00:00 2001 From: techdigger <72038289+techdigger@users.noreply.github.com> Date: Thu, 12 Nov 2026 20:16:45 +0000 Subject: [PATCH 3/5] Replace the fee rule with one option: --max-fee-per-gas Without the option safe-cli behaves exactly as before. --max-fee-per-gas GWEI (or SAFE_CLI_MAX_FEE_PER_GAS) sets maxFeePerGas for every transaction safe-cli sends, instead of the estimate from eth_feeHistory: for a chain that refuses that estimate (a fixed-fee chain whose node forecasts less than it charges), or to raise the cap on a busy chain. Transactions stay EIP-1559 and the tip stays the estimate. A cap above 10 times the node's gas price is refused before anything is sent, a guard against typing mistakes. One override of EthereumClient.set_eip1559_fees, in a subclass SafeOperator constructs, covers execution with a local key, approve_hash and execution with a hardware wallet. The option is global: it goes before the command or the Safe address. --- src/safe_cli/ethereum_client.py | 36 +++++++--- src/safe_cli/main.py | 33 ++++++++- tests/test_ethereum_client.py | 116 +++++++++++++++++++++++++++----- 3 files changed, 156 insertions(+), 29 deletions(-) diff --git a/src/safe_cli/ethereum_client.py b/src/safe_cli/ethereum_client.py index eb41fe1c..1f4f5854 100644 --- a/src/safe_cli/ethereum_client.py +++ b/src/safe_cli/ethereum_client.py @@ -1,24 +1,40 @@ from safe_eth.eth import EthereumClient, TxSpeed +from web3 import Web3 from web3.types import TxParams, Wei +# A cap above this many times the node's gas price is refused: far more likely a typing mistake (extra zeros, wei +# taken for gwei) than an intended fee +MAX_FEE_PER_GAS_LIMIT = 10 + class SafeCliEthereumClient(EthereumClient): """ - ``EthereumClient`` that does not cap the fee of an EIP-1559 transaction below the node's own gas price. + ``EthereumClient`` whose EIP-1559 ``maxFeePerGas`` the user can set, with ``--max-fee-per-gas``. - ``EthereumClient.set_eip1559_fees`` takes ``maxFeePerGas`` from ``eth_feeHistory``: the next block's forecast - base fee plus the tip. A chain that charges more than its fee history forecasts (a fixed-fee chain whose node - still projects a falling base fee) refuses such a transaction. There the node's ``eth_gasPrice`` is above the - forecast, and it becomes the cap. Where the forecast is at or above ``eth_gasPrice``, nothing changes. + ``EthereumClient.set_eip1559_fees`` takes ``maxFeePerGas`` from ``eth_feeHistory`` and offers no way to change + it. Some chains refuse that estimate (a fixed-fee chain whose node forecasts less than it charges), and on a + busy chain the user may want a higher cap. A cap above ``MAX_FEE_PER_GAS_LIMIT`` times the node's gas price is + refused before anything is sent. """ + # Cap in wei for every transaction sent. ``None`` keeps the estimate. + max_fee_per_gas: int | None = None + def set_eip1559_fees( self, tx: TxParams, tx_speed: TxSpeed = TxSpeed.NORMAL ) -> TxParams: - """ - :return: TxParams in EIP1559 format, with ``maxFeePerGas`` of at least ``eth_gasPrice`` - :raises: ValueError if EIP1559 not supported - """ tx = super().set_eip1559_fees(tx, tx_speed) - tx["maxFeePerGas"] = Wei(max(tx["maxFeePerGas"], self.w3.eth.gas_price)) + if self.max_fee_per_gas is not None: + node_price = self.w3.eth.gas_price + if node_price and self.max_fee_per_gas > MAX_FEE_PER_GAS_LIMIT * node_price: + raise ValueError( + f"--max-fee-per-gas: {Web3.from_wei(self.max_fee_per_gas, 'gwei')} gwei is more than " + f"{MAX_FEE_PER_GAS_LIMIT} times the node's gas price " + f"({Web3.from_wei(node_price, 'gwei')} gwei), nothing was sent" + ) + tx["maxFeePerGas"] = Wei(self.max_fee_per_gas) + # The tip cannot be above the cap + tx["maxPriorityFeePerGas"] = min( + tx["maxPriorityFeePerGas"], tx["maxFeePerGas"] + ) return tx diff --git a/src/safe_cli/main.py b/src/safe_cli/main.py index 84cbeb54..5ffff288 100644 --- a/src/safe_cli/main.py +++ b/src/safe_cli/main.py @@ -15,6 +15,7 @@ from . import VERSION from .argparse_validators import check_hex_str +from .ethereum_client import SafeCliEthereumClient from .operators import SafeOperator from .safe_cli import SafeCli from .tx_builder.exceptions import SoliditySyntaxError, TxBuilderEncodingError @@ -29,6 +30,30 @@ app = typer.Typer(name="Safe CLI") +MAX_FEE_PER_GAS_OPTION = "--max-fee-per-gas" + + +@app.callback() +def global_options( + max_fee_per_gas: Annotated[ + float | None, + typer.Option( + MAX_FEE_PER_GAS_OPTION, + envvar="SAFE_CLI_MAX_FEE_PER_GAS", + help="maxFeePerGas, in gwei, for every transaction sent, instead of the estimate from the fee " + "history; at most 10 times the node's gas price. Goes before the command or the Safe address.", + show_default=False, + ), + ] = None, +): + if max_fee_per_gas is not None and not max_fee_per_gas > 0: + raise typer.BadParameter( + "expected a positive number of gwei", param_hint=MAX_FEE_PER_GAS_OPTION + ) + SafeCliEthereumClient.max_fee_per_gas = ( + None if max_fee_per_gas is None else Web3.to_wei(max_fee_per_gas, "gwei") + ) + def _build_safe_operator_and_load_keys( safe_address: ChecksumAddress, @@ -401,7 +426,11 @@ def _is_safe_cli_default_command(arguments: list[str]) -> bool: def main(): + # The global option and its value, if given, go before the command + first = 1 + if len(sys.argv) > 1 and sys.argv[1].startswith(MAX_FEE_PER_GAS_OPTION): + first = 2 if "=" in sys.argv[1] else 3 # By default, the attended mode is initialised. Otherwise, the required command must be specified. - if _is_safe_cli_default_command(sys.argv): - sys.argv.insert(1, "attended-mode") + if _is_safe_cli_default_command(sys.argv[:1] + sys.argv[first:]): + sys.argv.insert(first, "attended-mode") app() diff --git a/tests/test_ethereum_client.py b/tests/test_ethereum_client.py index dc0916f7..1529f788 100644 --- a/tests/test_ethereum_client.py +++ b/tests/test_ethereum_client.py @@ -3,17 +3,23 @@ from eth_account import Account from safe_eth.eth import EthereumClient +from typer.testing import CliRunner from web3 import Web3 -from safe_cli.ethereum_client import SafeCliEthereumClient +from safe_cli.ethereum_client import MAX_FEE_PER_GAS_LIMIT, SafeCliEthereumClient +from safe_cli.main import app, main from safe_cli.operators.hw_wallets.hw_wallet_manager import HwWalletManager from .safe_cli_test_case_mixin import SafeCliTestCaseMixin GWEI = 10**9 +runner = CliRunner() class TestSafeCliEthereumClient(SafeCliTestCaseMixin, unittest.TestCase): + def tearDown(self): + SafeCliEthereumClient.max_fee_per_gas = None + def _fees(self, base_fee: int, tip: int, node_price: int) -> dict: ethereum_client = SafeCliEthereumClient(self.ethereum_node_url) with ( @@ -29,18 +35,78 @@ def _fees(self, base_fee: int, tip: int, node_price: int) -> dict: ): return ethereum_client.set_eip1559_fees({"gasPrice": 1}) - def test_forecast_at_or_above_the_node_price_is_unchanged(self): - for node_price in (15 * GWEI, 22 * GWEI): - fees = self._fees(20 * GWEI, 2 * GWEI, node_price) - self.assertEqual(fees["maxFeePerGas"], 22 * GWEI) - self.assertEqual(fees["maxPriorityFeePerGas"], 2 * GWEI) - self.assertNotIn("gasPrice", fees) + def test_without_the_option_nothing_changes(self): + fees = self._fees(20 * GWEI, 2 * GWEI, 25 * GWEI) + self.assertEqual(fees["maxFeePerGas"], 22 * GWEI) + self.assertEqual(fees["maxPriorityFeePerGas"], 2 * GWEI) - def test_forecast_below_the_node_price(self): + def test_max_fee_per_gas(self): + SafeCliEthereumClient.max_fee_per_gas = 715_000 * GWEI # A node that forecasts 625,625 gwei for the next block and charges 715,000 fees = self._fees(625_625 * GWEI, 0, 715_000 * GWEI) self.assertEqual(fees["maxFeePerGas"], 715_000 * GWEI) self.assertEqual(fees["maxPriorityFeePerGas"], 0) + self.assertNotIn("gasPrice", fees) + + # A cap below the estimated tip lowers the tip to the cap + SafeCliEthereumClient.max_fee_per_gas = 1 * GWEI + fees = self._fees(20 * GWEI, 2 * GWEI, 22 * GWEI) + self.assertEqual(fees["maxFeePerGas"], 1 * GWEI) + self.assertEqual(fees["maxPriorityFeePerGas"], 1 * GWEI) + + def test_limit(self): + SafeCliEthereumClient.max_fee_per_gas = MAX_FEE_PER_GAS_LIMIT * 22 * GWEI + fees = self._fees(20 * GWEI, 2 * GWEI, 22 * GWEI) + self.assertEqual(fees["maxFeePerGas"], MAX_FEE_PER_GAS_LIMIT * 22 * GWEI) + + SafeCliEthereumClient.max_fee_per_gas += 1 + with self.assertRaisesRegex(ValueError, "more than 10 times"): + self._fees(20 * GWEI, 2 * GWEI, 22 * GWEI) + + # A node that reports no gas price (some development chains) gives nothing to compare with + self._fees(20 * GWEI, 2 * GWEI, 0) + + def test_command_line_option(self): + result = runner.invoke(app, ["--max-fee-per-gas", "31.5", "version"]) + self.assertEqual(result.exit_code, 0) + self.assertEqual(SafeCliEthereumClient.max_fee_per_gas, 31_500_000_000) + + result = runner.invoke(app, ["version"]) + self.assertEqual(result.exit_code, 0) + self.assertIsNone(SafeCliEthereumClient.max_fee_per_gas) + + for value in ("0", "-1", "nan", "fast"): + result = runner.invoke(app, [f"--max-fee-per-gas={value}", "version"]) + self.assertEqual(result.exit_code, 2, value) + + with mock.patch.dict("os.environ", {"SAFE_CLI_MAX_FEE_PER_GAS": "40"}): + runner.invoke(app, ["version"]) + self.assertEqual(SafeCliEthereumClient.max_fee_per_gas, 40 * GWEI) + # The command line overrides the environment + runner.invoke(app, ["--max-fee-per-gas=41", "version"]) + self.assertEqual(SafeCliEthereumClient.max_fee_per_gas, 41 * GWEI) + + @mock.patch.object(type(app), "__call__") + def test_attended_mode_after_the_option(self, mock_app: mock.MagicMock): + safe, node = Account.create().address, "http://localhost:8545" + for arguments, expected in ( + ([safe, node], ["attended-mode", safe, node]), + ( + ["--max-fee-per-gas", "30", safe, node], + ["--max-fee-per-gas", "30", "attended-mode", safe, node], + ), + ( + ["--max-fee-per-gas=30", safe, node], + ["--max-fee-per-gas=30", "attended-mode", safe, node], + ), + ( + ["--max-fee-per-gas", "30", "send-ether", safe, node], + ["--max-fee-per-gas", "30", "send-ether", safe, node], + ), + ): + with mock.patch("sys.argv", ["safe-cli", *arguments]) as argv: + main() + self.assertEqual(argv[1:], expected) def _sent_to(self, address: str, since_block: int) -> list: return [ @@ -50,12 +116,6 @@ def _sent_to(self, address: str, since_block: int) -> list: if transaction["to"] == address ] - def _assert_included(self, transaction, forecast: int): - self.assertEqual(transaction["type"], 2) - base_fee = self.w3.eth.get_block(transaction["blockNumber"])["baseFeePerGas"] - self.assertLess(forecast, base_fee) - self.assertGreaterEqual(transaction["maxFeePerGas"], base_fee) - def test_every_way_of_sending_uses_it(self): safe_operator = self.setup_operator() owner = self.ethereum_test_account @@ -67,6 +127,8 @@ def test_every_way_of_sending_uses_it(self): # A fee history that forecasts 0.6 x the base fee the chain charges. A cap at the forecast is below the # base fee and the node refuses the transaction: "max fee per gas less than block base fee". forecast = self.w3.eth.get_block("latest")["baseFeePerGas"] * 6 // 10 + cap = self.w3.eth.gas_price * 3 + SafeCliEthereumClient.max_fee_per_gas = cap with mock.patch.object( EthereumClient, "estimate_fee_eip1559", return_value=(forecast, 0) ): @@ -74,7 +136,9 @@ def test_every_way_of_sending_uses_it(self): block = self.w3.eth.block_number self.assertTrue(safe_operator.send_ether(Account.create().address, 1)) (transaction,) = self._sent_to(safe_operator.address, block) - self._assert_included(transaction, forecast) + self.assertEqual( + (transaction["type"], transaction["maxFeePerGas"]), (2, cap) + ) # approve_hash block = self.w3.eth.block_number @@ -82,7 +146,9 @@ def test_every_way_of_sending_uses_it(self): safe_operator.approve_hash(Web3.keccak(text="fee cap"), owner.address) ) (transaction,) = self._sent_to(safe_operator.address, block) - self._assert_included(transaction, forecast) + self.assertEqual( + (transaction["type"], transaction["maxFeePerGas"]), (2, cap) + ) # Executing a Safe transaction with a hardware wallet sender = mock.Mock() @@ -96,7 +162,23 @@ def test_every_way_of_sending_uses_it(self): block = self.w3.eth.block_number self.assertTrue(safe_operator.send_ether(Account.create().address, 1)) (transaction,) = self._sent_to(safe_operator.address, block) - self._assert_included(transaction, forecast) + self.assertEqual( + (transaction["type"], transaction["maxFeePerGas"]), (2, cap) + ) + + # Without the option the node refuses the same estimate + SafeCliEthereumClient.max_fee_per_gas = None + safe_operator.default_sender = owner + forecast = self.w3.eth.get_block("latest")["baseFeePerGas"] * 6 // 10 + with ( + mock.patch.object( + EthereumClient, "estimate_fee_eip1559", return_value=(forecast, 0) + ), + self.assertRaisesRegex( + Exception, "max fee per gas less than block base fee" + ), + ): + safe_operator.send_ether(Account.create().address, 1) if __name__ == "__main__": From dc816662281a29d7e4bdea4b682bc14e1b5b70a9 Mon Sep 17 00:00:00 2001 From: techdigger <72038289+techdigger@users.noreply.github.com> Date: Thu, 12 Nov 2026 20:47:58 +0000 Subject: [PATCH 4/5] Tighten the wording of --max-fee-per-gas --- src/safe_cli/ethereum_client.py | 37 ++++++++++++++------------------- src/safe_cli/main.py | 7 ++++--- tests/test_ethereum_client.py | 7 +++---- 3 files changed, 23 insertions(+), 28 deletions(-) diff --git a/src/safe_cli/ethereum_client.py b/src/safe_cli/ethereum_client.py index 1f4f5854..3c3f2da6 100644 --- a/src/safe_cli/ethereum_client.py +++ b/src/safe_cli/ethereum_client.py @@ -2,39 +2,34 @@ from web3 import Web3 from web3.types import TxParams, Wei -# A cap above this many times the node's gas price is refused: far more likely a typing mistake (extra zeros, wei -# taken for gwei) than an intended fee +# safe-cli refuses a cap above this many times the node's gas price. A cap that high is a typo more often than a +# choice: extra zeros, or wei typed as gwei. MAX_FEE_PER_GAS_LIMIT = 10 class SafeCliEthereumClient(EthereumClient): """ - ``EthereumClient`` whose EIP-1559 ``maxFeePerGas`` the user can set, with ``--max-fee-per-gas``. - - ``EthereumClient.set_eip1559_fees`` takes ``maxFeePerGas`` from ``eth_feeHistory`` and offers no way to change - it. Some chains refuse that estimate (a fixed-fee chain whose node forecasts less than it charges), and on a - busy chain the user may want a higher cap. A cap above ``MAX_FEE_PER_GAS_LIMIT`` times the node's gas price is - refused before anything is sent. + ``EthereumClient`` that sends EIP-1559 transactions with the ``maxFeePerGas`` the user gave in + ``--max-fee-per-gas``. Without the option it behaves as ``EthereumClient``. """ - # Cap in wei for every transaction sent. ``None`` keeps the estimate. + # The user's cap in wei, or None max_fee_per_gas: int | None = None def set_eip1559_fees( self, tx: TxParams, tx_speed: TxSpeed = TxSpeed.NORMAL ) -> TxParams: tx = super().set_eip1559_fees(tx, tx_speed) - if self.max_fee_per_gas is not None: - node_price = self.w3.eth.gas_price - if node_price and self.max_fee_per_gas > MAX_FEE_PER_GAS_LIMIT * node_price: - raise ValueError( - f"--max-fee-per-gas: {Web3.from_wei(self.max_fee_per_gas, 'gwei')} gwei is more than " - f"{MAX_FEE_PER_GAS_LIMIT} times the node's gas price " - f"({Web3.from_wei(node_price, 'gwei')} gwei), nothing was sent" - ) - tx["maxFeePerGas"] = Wei(self.max_fee_per_gas) - # The tip cannot be above the cap - tx["maxPriorityFeePerGas"] = min( - tx["maxPriorityFeePerGas"], tx["maxFeePerGas"] + if self.max_fee_per_gas is None: + return tx + node_price = self.w3.eth.gas_price + if node_price and self.max_fee_per_gas > MAX_FEE_PER_GAS_LIMIT * node_price: + raise ValueError( + f"--max-fee-per-gas: {Web3.from_wei(self.max_fee_per_gas, 'gwei')} gwei is more than " + f"{MAX_FEE_PER_GAS_LIMIT} times the node's gas price ({Web3.from_wei(node_price, 'gwei')} gwei). " + "safe-cli sent nothing." ) + tx["maxFeePerGas"] = Wei(self.max_fee_per_gas) + # A node rejects a tip above the cap + tx["maxPriorityFeePerGas"] = min(tx["maxPriorityFeePerGas"], tx["maxFeePerGas"]) return tx diff --git a/src/safe_cli/main.py b/src/safe_cli/main.py index 5ffff288..edb681e7 100644 --- a/src/safe_cli/main.py +++ b/src/safe_cli/main.py @@ -40,8 +40,9 @@ def global_options( typer.Option( MAX_FEE_PER_GAS_OPTION, envvar="SAFE_CLI_MAX_FEE_PER_GAS", - help="maxFeePerGas, in gwei, for every transaction sent, instead of the estimate from the fee " - "history; at most 10 times the node's gas price. Goes before the command or the Safe address.", + help="maxFeePerGas in gwei for each transaction safe-cli sends. Without it safe-cli estimates the " + "fee from the fee history. Limit: 10 times the node's gas price. Put it before the command or the " + "Safe address.", show_default=False, ), ] = None, @@ -426,7 +427,7 @@ def _is_safe_cli_default_command(arguments: list[str]) -> bool: def main(): - # The global option and its value, if given, go before the command + # --max-fee-per-gas and its value come before the command first = 1 if len(sys.argv) > 1 and sys.argv[1].startswith(MAX_FEE_PER_GAS_OPTION): first = 2 if "=" in sys.argv[1] else 3 diff --git a/tests/test_ethereum_client.py b/tests/test_ethereum_client.py index 1529f788..235ebfc3 100644 --- a/tests/test_ethereum_client.py +++ b/tests/test_ethereum_client.py @@ -63,7 +63,7 @@ def test_limit(self): with self.assertRaisesRegex(ValueError, "more than 10 times"): self._fees(20 * GWEI, 2 * GWEI, 22 * GWEI) - # A node that reports no gas price (some development chains) gives nothing to compare with + # Some development chains report a gas price of 0: no limit applies self._fees(20 * GWEI, 2 * GWEI, 0) def test_command_line_option(self): @@ -124,8 +124,7 @@ def test_every_way_of_sending_uses_it(self): self.w3.eth.send_transaction( {"from": owner.address, "to": safe_operator.address, "value": 10} ) - # A fee history that forecasts 0.6 x the base fee the chain charges. A cap at the forecast is below the - # base fee and the node refuses the transaction: "max fee per gas less than block base fee". + # The fee history forecasts 0.6 x the base fee the chain charges, so the node refuses the estimate forecast = self.w3.eth.get_block("latest")["baseFeePerGas"] * 6 // 10 cap = self.w3.eth.gas_price * 3 SafeCliEthereumClient.max_fee_per_gas = cap @@ -166,7 +165,7 @@ def test_every_way_of_sending_uses_it(self): (transaction["type"], transaction["maxFeePerGas"]), (2, cap) ) - # Without the option the node refuses the same estimate + # Without the option the node refuses SafeCliEthereumClient.max_fee_per_gas = None safe_operator.default_sender = owner forecast = self.w3.eth.get_block("latest")["baseFeePerGas"] * 6 // 10 From 9a4ceb5e2d7493166766c7c1ecea64c61260c2e2 Mon Sep 17 00:00:00 2001 From: techdigger <72038289+techdigger@users.noreply.github.com> Date: Thu, 12 Nov 2026 20:50:16 +0000 Subject: [PATCH 5/5] Test: match the node's refusal on Ganache and anvil --- tests/test_ethereum_client.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_ethereum_client.py b/tests/test_ethereum_client.py index 235ebfc3..0be179c5 100644 --- a/tests/test_ethereum_client.py +++ b/tests/test_ethereum_client.py @@ -165,7 +165,7 @@ def test_every_way_of_sending_uses_it(self): (transaction["type"], transaction["maxFeePerGas"]), (2, cap) ) - # Without the option the node refuses + # Without the option the node refuses: the wording differs between Ganache and anvil SafeCliEthereumClient.max_fee_per_gas = None safe_operator.default_sender = owner forecast = self.w3.eth.get_block("latest")["baseFeePerGas"] * 6 // 10 @@ -174,7 +174,7 @@ def test_every_way_of_sending_uses_it(self): EthereumClient, "estimate_fee_eip1559", return_value=(forecast, 0) ), self.assertRaisesRegex( - Exception, "max fee per gas less than block base fee" + Exception, r"(?i)max ?fee ?per ?gas.*less than.*base ?fee" ), ): safe_operator.send_ether(Account.create().address, 1)