From f37fff43b283ba640fca2ee1fa03202b7dbcebd3 Mon Sep 17 00:00:00 2001 From: techdigger <72038289+techdigger@users.noreply.github.com> Date: Thu, 12 Nov 2026 19:41:41 +0000 Subject: [PATCH 1/2] Leave headroom in EIP-1559 fees EthereumClient.set_eip1559_fees sets maxFeePerGas to the next block's forecast base fee plus the tip, with no headroom. If the base fee rises before the transaction is included, the cap falls below it and the transaction waits. On chains that charge more than their fee history forecasts (a fixed-fee chain whose node still projects a falling base fee) every transaction is refused. Use 2 x base fee + tip, as ethers, viem and web3.py's default strategy do, for every transaction safe-cli sends. maxFeePerGas is only a cap: the sender still pays the base fee plus the tip. --- src/safe_cli/ethereum_client.py | 27 +++++++++ src/safe_cli/operators/safe_operator.py | 3 +- tests/test_ethereum_client.py | 81 +++++++++++++++++++++++++ 3 files changed, 110 insertions(+), 1 deletion(-) create mode 100644 src/safe_cli/ethereum_client.py create mode 100644 tests/test_ethereum_client.py diff --git a/src/safe_cli/ethereum_client.py b/src/safe_cli/ethereum_client.py new file mode 100644 index 00000000..c043f365 --- /dev/null +++ b/src/safe_cli/ethereum_client.py @@ -0,0 +1,27 @@ +from safe_eth.eth import EthereumClient, TxSpeed +from web3.types import TxParams, Wei + +# Headroom of maxFeePerGas over the forecast base fee, as ethers, viem and web3.py's default strategy use: the base +# fee can rise 12.5% per block, so 2x keeps a transaction valid for about six full blocks in a row. +BASE_FEE_MULTIPLIER = 2 + + +class SafeCliEthereumClient(EthereumClient): + """ + ``EthereumClient`` whose EIP-1559 fees leave headroom over the forecast base fee. + + ``EthereumClient.set_eip1559_fees`` sets ``maxFeePerGas`` to the next block's forecast base fee plus the tip, + with no headroom. If the base fee rises before the transaction is included, the cap falls below it and the + transaction waits; on chains that charge more than their fee history forecasts (a fixed-fee chain whose node + still projects a falling base fee) it is refused outright. ``maxFeePerGas`` is only a cap: the sender still pays + the base fee plus the tip. + """ + + def set_eip1559_fees( + self, tx: TxParams, tx_speed: TxSpeed = TxSpeed.NORMAL + ) -> TxParams: + tx = super().set_eip1559_fees(tx, tx_speed) + tip = tx["maxPriorityFeePerGas"] + base_fee = tx["maxFeePerGas"] - tip + tx["maxFeePerGas"] = Wei(BASE_FEE_MULTIPLIER * base_fee + tip) + return tx diff --git a/src/safe_cli/operators/safe_operator.py b/src/safe_cli/operators/safe_operator.py index 0c94779b..f3e20699 100644 --- a/src/safe_cli/operators/safe_operator.py +++ b/src/safe_cli/operators/safe_operator.py @@ -38,6 +38,7 @@ from web3.contract import Contract from web3.exceptions import BadFunctionCallOutput +from safe_cli.ethereum_client import SafeCliEthereumClient from safe_cli.ethereum_hd_wallet import get_account_from_words from safe_cli.operators.exceptions import ( AccountNotLoadedException, @@ -162,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: diff --git a/tests/test_ethereum_client.py b/tests/test_ethereum_client.py new file mode 100644 index 00000000..94d60d8f --- /dev/null +++ b/tests/test_ethereum_client.py @@ -0,0 +1,81 @@ +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_test_case_mixin import SafeCliTestCaseMixin + +GWEI = 10**9 + + +class TestSafeCliEthereumClient(SafeCliTestCaseMixin, unittest.TestCase): + def _fees(self, base_fee: int, tip: int) -> dict: + ethereum_client = SafeCliEthereumClient(self.ethereum_node_url) + with mock.patch.object( + EthereumClient, "estimate_fee_eip1559", return_value=(base_fee, tip) + ): + return ethereum_client.set_eip1559_fees({"gasPrice": 1}) + + def test_headroom_over_the_forecast(self): + fees = self._fees(20 * GWEI, 2 * GWEI) + self.assertEqual(fees["maxFeePerGas"], 42 * GWEI) + self.assertEqual(fees["maxPriorityFeePerGas"], 2 * GWEI) + self.assertNotIn("gasPrice", fees) + + def test_chain_charging_more_than_it_forecasts(self): + # The node forecasts 625,625 gwei for the next block and charges 715,000: the cap now clears it + fees = self._fees(625_625 * GWEI, 0) + self.assertEqual(fees["maxFeePerGas"], 1_251_250 * GWEI) + self.assertEqual(fees["maxPriorityFeePerGas"], 0) + + def test_operator_sends_with_it(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, + } + ) + # A fee history that forecasts 0.6 x the base fee the chain charges, as on a chain that charges more than + # it forecasts: stock fees (cap 0.6 x) would be refused, 2 x 0.6 clears it + forecast = self.w3.eth.get_block("latest")["baseFeePerGas"] * 6 // 10 + with mock.patch.object( + EthereumClient, "estimate_fee_eip1559", return_value=(forecast, 0) + ): + self.assertTrue(safe_operator.send_ether(Account.create().address, 1)) + executed = self.w3.eth.get_transaction( + safe_operator.executed_transactions[-1] + ) + self.assertEqual(executed["type"], 2) + self.assertGreaterEqual( + executed["maxFeePerGas"], + self.w3.eth.get_block(executed["blockNumber"])["baseFeePerGas"], + ) + + block = self.w3.eth.block_number + self.assertTrue( + safe_operator.approve_hash( + Web3.keccak(text="fee cap"), self.ethereum_test_account.address + ) + ) + (approval,) = [ + transaction + for number in range(block + 1, self.w3.eth.block_number + 1) + for transaction in self.w3.eth.get_block(number, True)["transactions"] + ] + self.assertEqual(approval["type"], 2) + self.assertGreaterEqual( + approval["maxFeePerGas"], + self.w3.eth.get_block(approval["blockNumber"])["baseFeePerGas"], + ) + + +if __name__ == "__main__": + unittest.main() From f87a6358caf5ccd87f95377657310e772ad27118 Mon Sep 17 00:00:00 2001 From: techdigger <72038289+techdigger@users.noreply.github.com> Date: Thu, 12 Nov 2026 19:52:25 +0000 Subject: [PATCH 2/2] Fee headroom: document the balance trade-off, test every send path Cover execution with a local key, approve_hash and execution with a hardware wallet in one test, each against a fee history that forecasts less than the chain charges. --- src/safe_cli/ethereum_client.py | 17 +++++-- tests/test_ethereum_client.py | 86 +++++++++++++++++++++------------ 2 files changed, 66 insertions(+), 37 deletions(-) diff --git a/src/safe_cli/ethereum_client.py b/src/safe_cli/ethereum_client.py index c043f365..e9bbc792 100644 --- a/src/safe_cli/ethereum_client.py +++ b/src/safe_cli/ethereum_client.py @@ -1,8 +1,8 @@ from safe_eth.eth import EthereumClient, TxSpeed from web3.types import TxParams, Wei -# Headroom of maxFeePerGas over the forecast base fee, as ethers, viem and web3.py's default strategy use: the base -# fee can rise 12.5% per block, so 2x keeps a transaction valid for about six full blocks in a row. +# Headroom of maxFeePerGas over the forecast base fee. ethers, viem and web3.py's default strategy use the same +# factor: the base fee can rise 12.5% per block, so 2x keeps a transaction valid for about six full blocks in a row. BASE_FEE_MULTIPLIER = 2 @@ -12,15 +12,22 @@ class SafeCliEthereumClient(EthereumClient): ``EthereumClient.set_eip1559_fees`` sets ``maxFeePerGas`` to the next block's forecast base fee plus the tip, with no headroom. If the base fee rises before the transaction is included, the cap falls below it and the - transaction waits; on chains that charge more than their fee history forecasts (a fixed-fee chain whose node - still projects a falling base fee) it is refused outright. ``maxFeePerGas`` is only a cap: the sender still pays - the base fee plus the tip. + transaction waits. A chain that charges more than its fee history forecasts (a fixed-fee chain whose node still + projects a falling base fee) refuses the transaction outright. + + ``maxFeePerGas`` is a cap, not a price: the sender still pays the base fee plus the tip. The sender's balance + must cover ``gas * maxFeePerGas``, so a higher cap needs a larger balance. """ def set_eip1559_fees( self, tx: TxParams, tx_speed: TxSpeed = TxSpeed.NORMAL ) -> TxParams: + """ + :return: TxParams in EIP1559 format, with ``maxFeePerGas = BASE_FEE_MULTIPLIER * base fee + tip`` + :raises: ValueError if EIP1559 not supported + """ tx = super().set_eip1559_fees(tx, tx_speed) + # The parent sets maxFeePerGas = base fee + tip tip = tx["maxPriorityFeePerGas"] base_fee = tx["maxFeePerGas"] - tip tx["maxFeePerGas"] = Wei(BASE_FEE_MULTIPLIER * base_fee + tip) diff --git a/tests/test_ethereum_client.py b/tests/test_ethereum_client.py index 94d60d8f..7c70541b 100644 --- a/tests/test_ethereum_client.py +++ b/tests/test_ethereum_client.py @@ -5,7 +5,8 @@ from safe_eth.eth import EthereumClient from web3 import Web3 -from safe_cli.ethereum_client import SafeCliEthereumClient +from safe_cli.ethereum_client import BASE_FEE_MULTIPLIER, SafeCliEthereumClient +from safe_cli.operators.hw_wallets.hw_wallet_manager import HwWalletManager from .safe_cli_test_case_mixin import SafeCliTestCaseMixin @@ -21,60 +22,81 @@ def _fees(self, base_fee: int, tip: int) -> dict: return ethereum_client.set_eip1559_fees({"gasPrice": 1}) def test_headroom_over_the_forecast(self): + self.assertEqual(BASE_FEE_MULTIPLIER, 2) fees = self._fees(20 * GWEI, 2 * GWEI) self.assertEqual(fees["maxFeePerGas"], 42 * GWEI) self.assertEqual(fees["maxPriorityFeePerGas"], 2 * GWEI) self.assertNotIn("gasPrice", fees) + fees = self._fees(0, 0) + self.assertEqual(fees["maxFeePerGas"], 0) + def test_chain_charging_more_than_it_forecasts(self): - # The node forecasts 625,625 gwei for the next block and charges 715,000: the cap now clears it + # A node that forecasts 625,625 gwei for the next block and charges 715,000: the cap clears it fees = self._fees(625_625 * GWEI, 0) self.assertEqual(fees["maxFeePerGas"], 1_251_250 * GWEI) self.assertEqual(fees["maxPriorityFeePerGas"], 0) - def test_operator_sends_with_it(self): + 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_with_headroom(self, transaction, forecast: int): + self.assertEqual(transaction["type"], 2) + self.assertEqual( + transaction["maxFeePerGas"], + BASE_FEE_MULTIPLIER * forecast + transaction["maxPriorityFeePerGas"], + ) + self.assertGreaterEqual( + transaction["maxFeePerGas"], + self.w3.eth.get_block(transaction["blockNumber"])["baseFeePerGas"], + ) + + def test_every_way_of_sending_uses_it(self): safe_operator = self.setup_operator() - safe_operator.accounts.add(self.ethereum_test_account) - safe_operator.default_sender = self.ethereum_test_account + owner = self.ethereum_test_account + safe_operator.accounts.add(owner) + safe_operator.default_sender = owner self.w3.eth.send_transaction( - { - "from": self.ethereum_test_account.address, - "to": safe_operator.address, - "value": 10, - } + {"from": owner.address, "to": safe_operator.address, "value": 10} ) - # A fee history that forecasts 0.6 x the base fee the chain charges, as on a chain that charges more than - # it forecasts: stock fees (cap 0.6 x) would be refused, 2 x 0.6 clears it + # A fee history that forecasts 0.6 x the base fee the chain charges. Without headroom the cap 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)) - executed = self.w3.eth.get_transaction( - safe_operator.executed_transactions[-1] - ) - self.assertEqual(executed["type"], 2) - self.assertGreaterEqual( - executed["maxFeePerGas"], - self.w3.eth.get_block(executed["blockNumber"])["baseFeePerGas"], - ) + (transaction,) = self._sent_to(safe_operator.address, block) + self._assert_included_with_headroom(transaction, forecast) + # approve_hash block = self.w3.eth.block_number self.assertTrue( - safe_operator.approve_hash( - Web3.keccak(text="fee cap"), self.ethereum_test_account.address - ) + safe_operator.approve_hash(Web3.keccak(text="headroom"), owner.address) ) - (approval,) = [ - transaction - for number in range(block + 1, self.w3.eth.block_number + 1) - for transaction in self.w3.eth.get_block(number, True)["transactions"] - ] - self.assertEqual(approval["type"], 2) - self.assertGreaterEqual( - approval["maxFeePerGas"], - self.w3.eth.get_block(approval["blockNumber"])["baseFeePerGas"], + (transaction,) = self._sent_to(safe_operator.address, block) + self._assert_included_with_headroom(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_with_headroom(transaction, forecast) if __name__ == "__main__":