diff --git a/src/hiero_sdk_python/client/client.py b/src/hiero_sdk_python/client/client.py index 361324d06..29e9f3895 100644 --- a/src/hiero_sdk_python/client/client.py +++ b/src/hiero_sdk_python/client/client.py @@ -59,6 +59,7 @@ def __init__(self, network: Network = None) -> None: self.max_attempts: int = 10 self.default_max_query_payment: Hbar = DEFAULT_MAX_QUERY_PAYMENT + self.default_max_transaction_fee: Hbar | None = None self._min_backoff: float = DEFAULT_MIN_BACKOFF self._max_backoff: float = DEFAULT_MAX_BACKOFF @@ -277,19 +278,21 @@ def set_default_max_query_payment(self, max_query_payment: int | float | Decimal Returns: Client: The current client instance for method chaining. """ - if isinstance(max_query_payment, bool) or not isinstance(max_query_payment, (int, float, Decimal, Hbar)): - raise TypeError( - f"max_query_payment must be int, float, Decimal, or Hbar, got {type(max_query_payment).__name__}" - ) - - value = max_query_payment if isinstance(max_query_payment, Hbar) else Hbar(max_query_payment) - - if value < Hbar(0): - raise ValueError("max_query_payment must be non-negative") - + value = Hbar._coerce_non_negative(max_query_payment, "max_query_payment") self.default_max_query_payment = value return self + def set_default_max_transaction_fee( + self, + max_transaction_fee: int | float | Decimal | Hbar, + ) -> Client: + """ + Sets the default maximum Hbar fee allowed for any transaction executed by this client. + """ + value = Hbar._coerce_non_negative(max_transaction_fee, "max_transaction_fee") + self.default_max_transaction_fee = value + return self + def set_max_attempts(self, max_attempts: int) -> Client: """ Set the maximum number of execution attempts for all transactions and queries diff --git a/src/hiero_sdk_python/hbar.py b/src/hiero_sdk_python/hbar.py index 22e3d03a6..18edf3d20 100644 --- a/src/hiero_sdk_python/hbar.py +++ b/src/hiero_sdk_python/hbar.py @@ -83,6 +83,29 @@ def negated(self) -> Hbar: """ return Hbar.from_tinybars(-self._amount_in_tinybar) + @staticmethod + def _coerce_non_negative(value: int | float | Decimal | Hbar, name: str = "value") -> Hbar: + """ + Coerce a value to a non-negative Hbar. Accepts int, float, Decimal, or Hbar (but not bool). + + Args: + value: The value to coerce. + name: The field name used in error messages. Defaults to "value". + + Returns: + Hbar: A non-negative Hbar instance. + + Raises: + TypeError: If value is not int, float, Decimal, or Hbar. + ValueError: If value is negative. + """ + if isinstance(value, bool) or not isinstance(value, (int, float, Decimal, Hbar)): + raise TypeError(f"{name} must be int, float, Decimal, or Hbar, got {type(value).__name__}") + result = value if isinstance(value, Hbar) else Hbar(value) + if result < Hbar.ZERO: + raise ValueError(f"{name} must be non-negative") + return result + @classmethod def of(cls, amount: int | float | Decimal, unit: HbarUnit) -> Hbar: """ diff --git a/src/hiero_sdk_python/transaction/transaction.py b/src/hiero_sdk_python/transaction/transaction.py index f83298bf9..baddc7d25 100644 --- a/src/hiero_sdk_python/transaction/transaction.py +++ b/src/hiero_sdk_python/transaction/transaction.py @@ -1,6 +1,7 @@ from __future__ import annotations import hashlib +from decimal import Decimal from typing import TYPE_CHECKING, Literal, overload from hiero_sdk_python.account.account_id import AccountId @@ -265,6 +266,20 @@ def _resolve_node_ids(self, client: Client): if self._node_account_ids.is_empty: self._node_account_ids.set_list([node._account_id for node in client.network.nodes]) + def _resolve_transaction_fee(self, client: Client | None) -> None: + """Resolve the max transaction fee: explicit fee, else client default, else per-type default.""" + if self._transaction_fee is not None: + return + + default = client.default_max_transaction_fee if client is not None else None + if not isinstance(default, Hbar): + default = None + + if default is not None: + self.transaction_fee = default + else: + self._transaction_fee = self._default_transaction_fee + def freeze(self): """ Freezes the transaction by building the transaction body and setting necessary IDs. @@ -304,6 +319,7 @@ def freeze_with(self, client: Client): # Resolve transaction_id and node_accountids to be set when using freeze() self._resolve_transaction_id(client) self._resolve_node_ids(client) + self._resolve_transaction_fee(client) required_chunks = self.get_required_chunks() self._generate_transaction_ids(self._transaction_ids.get(0), required_chunks) @@ -509,8 +525,8 @@ def build_base_transaction_body(self) -> transaction_pb2.TransactionBody: """ transaction_body = transaction_pb2.TransactionBody() - fee = self._transaction_fee or self._default_transaction_fee - if hasattr(fee, "to_tinybars"): + fee = self._transaction_fee if self._transaction_fee is not None else self._default_transaction_fee + if isinstance(fee, Hbar): transaction_body.transactionFee = int(fee.to_tinybars()) else: transaction_body.transactionFee = int(fee) @@ -537,8 +553,8 @@ def build_base_scheduled_body(self) -> SchedulableTransactionBody: """ schedulable_body = SchedulableTransactionBody() - fee = self._transaction_fee or self._default_transaction_fee - if hasattr(fee, "to_tinybars"): + fee = self._transaction_fee if self._transaction_fee is not None else self._default_transaction_fee + if isinstance(fee, Hbar): schedulable_body.transactionFee = int(fee.to_tinybars()) else: schedulable_body.transactionFee = int(fee) @@ -681,19 +697,7 @@ def transaction_fee(self, fee: Hbar | int): """ Set the maximum transaction fee for this transaction. """ - self._require_not_frozen() - - if isinstance(fee, Hbar): - tinybars = fee.to_tinybars() - elif isinstance(fee, bool) or not isinstance(fee, int): - raise TypeError("fee must be of type Hbar or int") - else: - tinybars = fee - - if tinybars < 0: - raise ValueError("fee must be greater than or equal to 0") - - self._transaction_fee = tinybars + self.set_max_transaction_fee(fee) def to_bytes(self) -> bytes: """ @@ -837,6 +841,25 @@ def from_bytes(transaction_bytes: bytes): transaction_body, signed_transaction.bodyBytes, signed_transaction.sigMap ) + def set_max_transaction_fee(self, max_transaction_fee: int | float | Decimal | Hbar) -> Transaction: + """ + Set the maximum transaction fee the payer is willing to pay for this transaction. + + Args: + max_transaction_fee (int | float | Decimal | Hbar): The maximum fee. + Numeric values are interpreted as Hbar. + + Returns: + Transaction: This transaction instance for method chaining. + Raises: + TypeError: If the value is not int, float, Decimal, or Hbar. + ValueError: If the value is negative. + Exception: If the transaction has already been frozen. + """ + self._require_not_frozen() + self._transaction_fee = Hbar._coerce_non_negative(max_transaction_fee, "max_transaction_fee").to_tinybars() + return self + @staticmethod def _get_transaction_class(transaction_type: str): """ @@ -942,7 +965,7 @@ def _from_protobuf(cls, transaction_body, body_bytes: bytes, sig_map): if transaction_body.HasField("nodeAccountID"): transaction._node_account_ids.set_list([AccountId._from_proto(transaction_body.nodeAccountID)]) - transaction.transaction_fee = transaction_body.transactionFee + transaction._transaction_fee = transaction_body.transactionFee transaction.transaction_valid_duration = transaction_body.transactionValidDuration.seconds transaction.generate_record = transaction_body.generateRecord transaction._high_volume = transaction_body.high_volume @@ -967,7 +990,6 @@ def _from_protobuf(cls, transaction_body, body_bytes: bytes, sig_map): if sig_map and sig_map.sigPair: transaction._signature_map[body_bytes] = sig_map - return transaction def set_batch_key(self, key: Key): diff --git a/tests/integration/account_update_transaction_e2e_test.py b/tests/integration/account_update_transaction_e2e_test.py index 88f0ed8cf..80dc4d536 100644 --- a/tests/integration/account_update_transaction_e2e_test.py +++ b/tests/integration/account_update_transaction_e2e_test.py @@ -12,6 +12,7 @@ from hiero_sdk_python.crypto.key_list import KeyList from hiero_sdk_python.crypto.private_key import PrivateKey from hiero_sdk_python.Duration import Duration +from hiero_sdk_python.exceptions import PrecheckError from hiero_sdk_python.hbar import Hbar from hiero_sdk_python.query.account_info_query import AccountInfoQuery from hiero_sdk_python.response_code import ResponseCode @@ -248,7 +249,6 @@ def _apply_tiny_max_fee_if_supported(tx, client) -> bool: # Try client-level default for attr in ( "set_default_max_transaction_fee", - "set_max_transaction_fee", "set_default_max_fee", "setMaxTransactionFee", ): @@ -283,16 +283,40 @@ def test_account_update_insufficient_fee_with_valid_expiration_bump(env): if not _apply_tiny_max_fee_if_supported(tx, env.client): pytest.skip("SDK lacks a max-fee API; cannot deterministically trigger INSUFFICIENT_TX_FEE.") - receipt = tx.execute(env.client) - assert receipt.status == ResponseCode.INSUFFICIENT_TX_FEE, ( - f"Expected INSUFFICIENT_TX_FEE but got {ResponseCode(receipt.status).name}" - ) + # If it succeeds or raises a different error, the test will fail. + with pytest.raises(PrecheckError) as exc_info: + tx.execute(env.client) + + assert exc_info.value.status == ResponseCode.INSUFFICIENT_TX_FEE # Confirm expiration time did not change info_after = AccountInfoQuery(account_id).execute(env.client) assert int(info_after.expiration_time.seconds) == base_expiry_secs +@pytest.mark.integration +def test_account_update_insufficient_fee_via_client_default(env): + """A client-level default max fee must apply to transactions that set no explicit fee.""" + receipt = ( + AccountCreateTransaction() + .set_key(env.operator_key.public_key()) + .set_initial_balance(Hbar(1)) + .execute(env.client) + ) + assert receipt.status == ResponseCode.SUCCESS + account_id = receipt.account_id + + env.client.set_default_max_transaction_fee(Hbar.from_tinybars(1)) + + # No tx-level fee: the 1-tinybar client default must be resolved at freeze and rejected at precheck. + tx = AccountUpdateTransaction().set_account_id(account_id).set_account_memo("client default fee test") + + with pytest.raises(PrecheckError) as exc_info: + tx.execute(env.client) + + assert exc_info.value.status == ResponseCode.INSUFFICIENT_TX_FEE + + @pytest.mark.integration def test_integration_account_update_transaction_with_only_account_id(env): """Test that AccountUpdateTransaction can execute with only account ID set.""" diff --git a/tests/unit/client_test.py b/tests/unit/client_test.py index 3cdc79c1f..15737a222 100644 --- a/tests/unit/client_test.py +++ b/tests/unit/client_test.py @@ -231,6 +231,49 @@ def test_set_default_max_query_payment_valid_param(valid_amount, expected): assert client.default_max_query_payment == expected +def test_default_max_transaction_fee_is_none(): + """Default `default_max_transaction_fee` should be None.""" + client = Client.for_testnet() + assert client.default_max_transaction_fee is None + + +@pytest.mark.parametrize( + "valid_amount,expected", + [ + (1, Hbar(1)), + (0.1, Hbar(0.1)), + (Decimal("0.1"), Hbar(Decimal("0.1"))), + (Hbar(1), Hbar(1)), + (Hbar(0), Hbar(0)), + ], +) +def test_set_default_max_transaction_fee_valid_param(valid_amount, expected): + """Test set_default_max_transaction_fee converts inputs to Hbar and stores them.""" + client = Client.for_testnet() + + returned = client.set_default_max_transaction_fee(valid_amount) + assert client.default_max_transaction_fee == expected + assert returned is client + + +@pytest.mark.parametrize("invalid_amount", ["1", True, False, None, object()]) +def test_set_default_max_transaction_fee_invalid_param(invalid_amount): + """Test set_default_max_transaction_fee rejects invalid types.""" + client = Client.for_testnet() + + with pytest.raises(TypeError): + client.set_default_max_transaction_fee(invalid_amount) + + +@pytest.mark.parametrize("negative_amount", [-1, -0.1, Decimal("-0.1"), Hbar(-1)]) +def test_set_default_max_transaction_fee_negative_value(negative_amount): + """Test set_default_max_transaction_fee rejects negative values.""" + client = Client.for_testnet() + + with pytest.raises(ValueError): + client.set_default_max_transaction_fee(negative_amount) + + @pytest.mark.parametrize("negative_amount", [-1, -0.1, Decimal("-0.1"), Decimal("-1"), Hbar(-1)]) def test_set_default_max_query_payment_negative_value(negative_amount): """Test set_default_max_query_payment for negative amount values.""" diff --git a/tests/unit/transaction_freeze_and_bytes_test.py b/tests/unit/transaction_freeze_and_bytes_test.py index 6916cb27e..ebf10b6d0 100644 --- a/tests/unit/transaction_freeze_and_bytes_test.py +++ b/tests/unit/transaction_freeze_and_bytes_test.py @@ -8,13 +8,18 @@ from __future__ import annotations +from decimal import Decimal +from unittest.mock import MagicMock + import pytest from hiero_sdk_python.account.account_id import AccountId from hiero_sdk_python.crypto.private_key import PrivateKey +from hiero_sdk_python.hapi.services import transaction_pb2 from hiero_sdk_python.hapi.services.transaction_response_pb2 import ( TransactionResponse as TransactionResponseProto, ) +from hiero_sdk_python.hbar import Hbar from hiero_sdk_python.transaction.transaction_id import TransactionId from hiero_sdk_python.transaction.transfer_transaction import TransferTransaction @@ -67,6 +72,45 @@ def test_freeze_with_valid_parameters(): assert node_id in transaction._transaction_body_bytes[transaction_id] +@pytest.mark.parametrize( + "valid_amount,expected", + [ + (1, 100_000_000), + (0.1, 10_000_000), + (Decimal("0.1"), 10_000_000), + (Hbar(1), 100_000_000), + (Hbar(0), 0), + ], +) +def test_set_max_transaction_fee_valid_param(valid_amount, expected): + """Transaction.set_max_transaction_fee should accept various numeric types and Hbar.""" + tx = TransferTransaction() + + returned = tx.set_max_transaction_fee(valid_amount) + assert tx.transaction_fee == expected + assert returned is tx + + +@pytest.mark.parametrize("invalid_amount", ["1", True, False, None, object()]) +def test_set_max_transaction_fee_invalid_param(invalid_amount): + """Transaction.set_max_transaction_fee should reject invalid types.""" + tx = TransferTransaction() + + with pytest.raises(TypeError): + tx.set_max_transaction_fee(invalid_amount) + + +@pytest.mark.parametrize("negative_amount", [-1, -0.1, Decimal("-0.1"), Hbar(-1)]) +def test_set_max_transaction_fee_negative_value(negative_amount): + """Transaction.set_max_transaction_fee should reject negative values.""" + tx = TransferTransaction() + + with pytest.raises(ValueError): + tx.set_max_transaction_fee(negative_amount) + # checking state un modified + assert len(tx._transaction_body_bytes) == 0 + + def test_freeze_is_idempotent(): """Test that calling freeze() multiple times doesn't cause issues.""" operator_id = AccountId.from_string("0.0.1234") @@ -370,7 +414,7 @@ def test_from_bytes_preserves_all_common_fields(): transaction.transaction_id = TransactionId.generate(operator_id) transaction.set_node_account_ids([node_id]) - transaction.transaction_fee = 5_000_000 # Custom fee + transaction._transaction_fee = 5_000_000 # Custom fee assert transaction.set_transaction_valid_duration(180) is transaction # 3 minutes - using setter transaction.generate_record = True @@ -715,3 +759,97 @@ def test_map_response_raises_if_proto_request_is_not_transaction(): node_id=mock_node_id, proto_request=invalid_proto_request, ) + + +def test_fee_resolution_transaction_precedence(mock_client): + """Transaction fee explicitly set should take precedence over client default.""" + tx = TransferTransaction() + tx.set_max_transaction_fee(Hbar(10)) + + # client has different default + mock_client.set_default_max_transaction_fee(Hbar(5)) + + before = tx.transaction_fee + tx.freeze_with(mock_client) + + assert tx.transaction_fee == 1_000_000_000 + assert tx.transaction_fee == before + + +def test_fee_resolution_client_default_used_when_transaction_missing(mock_client): + """When transaction fee is not set, client.default_max_transaction_fee should be used.""" + tx = TransferTransaction() + # leave tx.transaction_fee as None + + mock_client.set_default_max_transaction_fee(Hbar(7)) + + tx.freeze_with(mock_client) + + assert tx.transaction_fee == 700_000_000 + + +def test_fee_resolution_falls_back_to_transaction_default(mock_client): + """When neither transaction nor client provide a fee, fallback to transaction default Hbar(1).""" + tx = TransferTransaction() + tx.set_transaction_id(TransactionId.generate(AccountId.from_string("0.0.1234"))) + # Ensure client default is None + mock_client.default_max_transaction_fee = None + + tx.freeze_with(mock_client) + + assert tx.transaction_fee == 100_000_000 # Default fee for TransferTransaction + + +def test_resolved_fee_serialized_into_transaction_body(mock_client): + """The resolved fee must reach the serialized proto transactionFee.""" + tx = TransferTransaction() + + tx.set_max_transaction_fee(Hbar(2)) + + body = tx.build_base_scheduled_body() + + assert body.transactionFee == Hbar(2).to_tinybars() + + +def test_max_transaction_fee_survives_to_bytes_round_trip(mock_client): + """An explicitly set max fee must survive a to_bytes -> from_bytes round trip.""" + tx = TransferTransaction() + + tx.set_max_transaction_fee(Hbar(2)) + + tx.freeze_with(mock_client) # Serlize to_bytes() + data = tx.to_bytes() + + restored = TransferTransaction.from_bytes(data) # Deserialize from_bytes(). + assert restored._transaction_fee == Hbar(2).to_tinybars() + + +def test_explicit_zero_fee_is_honored_on_direct_and_scheduled_paths(mock_client): + """An explicit zero fee must reach both the direct and scheduled body unchanged.""" + tx = TransferTransaction() + tx.set_max_transaction_fee(Hbar(0)) + + scheduled_body = tx.build_base_scheduled_body() + assert scheduled_body.transactionFee == 0 + + tx.freeze_with(mock_client) + + transaction_id = tx._transaction_ids.current + node_account_id = tx._node_account_ids.current + body_bytes = tx._transaction_body_bytes[transaction_id][node_account_id] + + body = transaction_pb2.TransactionBody() + body.ParseFromString(body_bytes) + + assert body.transactionFee == 0 + + +def test_freeze_with_bare_magicmock_client_resolves_default_fee(): + """Freezing against a bare MagicMock client must not raise and must fall back to the per-type default fee.""" + tx = TransferTransaction() + tx.set_transaction_id(TransactionId.generate(AccountId.from_string("0.0.1234"))) + tx.set_node_account_ids([AccountId.from_string("0.0.3")]) + + tx.freeze_with(MagicMock()) + + assert tx.transaction_fee == 100_000_000 # TransferTransaction per-type default, Hbar(1) diff --git a/tests/unit/transaction_test.py b/tests/unit/transaction_test.py index 59205563b..595a8946b 100644 --- a/tests/unit/transaction_test.py +++ b/tests/unit/transaction_test.py @@ -564,7 +564,7 @@ def test_transaction_fee_rejects_bool(): """Test transaction_fee rejects boolean values.""" tx = AccountCreateTransaction() - with pytest.raises(TypeError, match="fee must be of type Hbar or int"): + with pytest.raises(TypeError, match="transaction_fee must be int, float, Decimal, or Hbar, got bool"): tx.transaction_fee = True @@ -572,7 +572,7 @@ def test_transaction_fee_rejects_invalid_type(): """Test transaction_fee rejects invalid types.""" tx = AccountCreateTransaction() - with pytest.raises(TypeError, match="fee must be of type Hbar or int"): + with pytest.raises(TypeError, match="transaction_fee must be int, float, Decimal, or Hbar, got str"): tx.transaction_fee = "100" @@ -580,7 +580,7 @@ def test_transaction_fee_rejects_negative_int(): """Test transaction_fee rejects negative integer values.""" tx = AccountCreateTransaction() - with pytest.raises(ValueError, match="fee must be greater than or equal to 0"): + with pytest.raises(ValueError, match="transaction_fee must be non-negative"): tx.transaction_fee = -1