-
Notifications
You must be signed in to change notification settings - Fork 283
feat(client): add global max transaction fee configuration #2332
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
tech0priyanshu
wants to merge
5
commits into
hiero-ledger:main
Choose a base branch
from
tech0priyanshu:feat/add-client-set-max-transaction-fee
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,16 +1,25 @@ | ||
| 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 | ||
| from hiero_sdk_python.client.client import Client | ||
| from hiero_sdk_python.crypto.key import Key | ||
| from hiero_sdk_python.exceptions import PrecheckError | ||
| from hiero_sdk_python.executable import _Executable, _ExecutionState | ||
| from hiero_sdk_python.hapi.services import basic_types_pb2, transaction_contents_pb2, transaction_pb2 | ||
| from hiero_sdk_python.hapi.services.schedulable_transaction_body_pb2 import SchedulableTransactionBody | ||
| from hiero_sdk_python.hapi.services.transaction_response_pb2 import TransactionResponse as TransactionResponseProto | ||
| from hiero_sdk_python.hapi.services import ( | ||
| basic_types_pb2, | ||
| transaction_contents_pb2, | ||
| transaction_pb2, | ||
| ) | ||
| from hiero_sdk_python.hapi.services.schedulable_transaction_body_pb2 import ( | ||
| SchedulableTransactionBody, | ||
| ) | ||
| from hiero_sdk_python.hapi.services.transaction_response_pb2 import ( | ||
| TransactionResponse as TransactionResponseProto, | ||
| ) | ||
| from hiero_sdk_python.hbar import Hbar | ||
| from hiero_sdk_python.query.fee_estimate_query import FeeEstimateQuery | ||
| from hiero_sdk_python.response_code import ResponseCode | ||
|
|
@@ -64,8 +73,8 @@ def __init__(self) -> None: | |
| # This allows us to maintain the signatures for each unique transaction | ||
| # and ensures that the correct signatures are used when submitting transactions | ||
| self._signature_map: dict[bytes, basic_types_pb2.SignatureMap] = {} | ||
| # changed from int: 2_000_000 to Hbar: 0.02 | ||
| self._default_transaction_fee = Hbar(0.02) | ||
| # changed from Hbar: 0.02 to Hbar: 2 | ||
| self._default_transaction_fee = Hbar(2) | ||
| self.operator_account_id = None | ||
| self.batch_key: Key | None = None | ||
|
|
||
|
|
@@ -295,6 +304,17 @@ def freeze_with(self, client: Client): | |
| # For each node, set the node_account_id and build the transaction body | ||
| # This allows the transaction to be submitted to any node in the network | ||
|
|
||
| # Use all nodes from client network | ||
|
tech0priyanshu marked this conversation as resolved.
|
||
| # Resolve fee priority before building bodies: | ||
| # 1. Explicit transaction fee (self.transaction_fee) | ||
| # 2. Client default_max_transaction_fee | ||
| # 3. Transaction class default (_default_transaction_fee) | ||
| if self.transaction_fee is None: | ||
|
tech0priyanshu marked this conversation as resolved.
|
||
| if client is not None and getattr(client, "default_max_transaction_fee", None) is not None: | ||
|
tech0priyanshu marked this conversation as resolved.
|
||
| self.transaction_fee = client.default_max_transaction_fee | ||
| else: | ||
| self.transaction_fee = self._default_transaction_fee | ||
|
|
||
| if self.batch_key: | ||
| # For Inner Transaction of batch transaction node_account_id=0.0.0 | ||
| self.node_account_id = AccountId(0, 0, 0) | ||
|
|
@@ -314,7 +334,6 @@ def freeze_with(self, client: Client): | |
| self._transaction_body_bytes[node_account_id] = self.build_transaction_body().SerializeToString() | ||
|
|
||
| else: | ||
| # Use all nodes from client network | ||
| for node in client.network.nodes: | ||
| self.node_account_id = node._account_id | ||
| self._transaction_body_bytes[node._account_id] = self.build_transaction_body().SerializeToString() | ||
|
|
@@ -775,6 +794,23 @@ def from_bytes(transaction_bytes: bytes): | |
| transaction_body, signed_transaction.bodyBytes, signed_transaction.sigMap | ||
| ) | ||
|
|
||
| def set_max_transaction_fee(self, max_transaction_fee): | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Some of the logic here is quite complex, and duplicated elsewhere, which has also caused some typing curiosities why not extract the fee calculation and place it e.g. in the hbar file? Then create and call that from the multiple points |
||
| # Accept int, float, Decimal, or Hbar (but not bool) | ||
| self._require_not_frozen() | ||
|
|
||
| if isinstance(max_transaction_fee, bool) or not isinstance(max_transaction_fee, (int, float, Decimal, Hbar)): | ||
| raise TypeError( | ||
| f"max_transaction_fee must be int, float, Decimal, or Hbar, got {type(max_transaction_fee).__name__}" | ||
| ) | ||
|
|
||
| value = max_transaction_fee if isinstance(max_transaction_fee, Hbar) else Hbar(max_transaction_fee) | ||
|
|
||
| if value < Hbar(0): | ||
| raise ValueError("max_transaction_fee must be non-negative") | ||
|
|
||
| self.transaction_fee = value | ||
| return self | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| @staticmethod | ||
| def _get_transaction_class(transaction_type: str): | ||
| """ | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ideally can just call that same thing from the hbar class