Conversation
|
/describe |
|
PR Description updated to latest commit (5933846)
|
|
@coderabbitai resume |
WalkthroughThis update introduces offline transaction creation capabilities for the Tron blockchain by adding protobuf definitions, generated Python files, and utility modules. It refactors existing code to use new constants, utility functions, and type aliases, and adds comprehensive tests for offline transaction logic. Linting and build configurations are also updated. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Transaction
participant ProtobufUtils
participant tron_pb2
User->>Transaction: build_offline(owner, to, amount, ref_block_id)
Transaction->>ProtobufUtils: create_transaction_offline(...)
ProtobufUtils->>tron_pb2: Build protobuf TransferContract
ProtobufUtils->>ProtobufUtils: calculate_txid()
ProtobufUtils-->>Transaction: Return transaction dict
Transaction-->>User: Return Transaction instance
sequenceDiagram
participant User
participant Transaction
participant ProtobufUtils
participant tron_pb2
User->>Transaction: build_trc20_transfer_offline(from, to, amount, contract, ref_block_id, fee_limit)
Transaction->>ProtobufUtils: create_smart_contract_transaction_offline(...)
ProtobufUtils->>tron_pb2: Build protobuf TriggerSmartContract
ProtobufUtils->>ProtobufUtils: calculate_txid()
ProtobufUtils-->>Transaction: Return transaction dict
Transaction-->>User: Return Transaction instance
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
✅ Actions performedReviews resumed. |
|
@coderabbitai generate docstrings |
✅ Actions performedDocstrings generation started. |
Docstrings generation was requested by @rudolf-del. * #1 (comment) The following files were modified: * `tests/test_offline.py` * `tronpy/async_tron.py` * `tronpy/defaults.py` * `tronpy/proto/src/tron_pb2.pyi` * `tronpy/proto/transaction.py` * `tronpy/tron.py` * `tronpy/utils.py`
|
Note Generated docstrings for this pull request at #2 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (2)
tronpy/utils.py (1)
12-14: Likewise, emit raw hash bytes & add basic validation-def get_ref_block_hash(ref_block_id: str) -> str: - return ref_block_id[16:32] +def get_ref_block_hash(ref_block_id: str) -> bytes: + """ + Extract first 8 bytes of block hash from block id. + """ + if len(ref_block_id) != 64: + raise ValueError("ref_block_id must be 32-byte hex string") + return bytes.fromhex(ref_block_id[16:32])tronpy/tron.py (1)
145-145: Fix variable name in isinstance checkThe code is checking
json(which refers to the imported module) instead of thedataparameter.- if isinstance(json, str): + if isinstance(data, str):
🧹 Nitpick comments (7)
tronpy/types.py (1)
1-1: Consider usingtyping.NewTypefor stronger type-safety
TAddress = stris convenient but provides no protection against accidentally passing arbitrary strings. Defining it as aNewTypekeeps runtime cost at zero while allowing static checkers to flag misuse:-from typing import Final -TAddress = str +from typing import NewType, Final + +# Tron address as Base58/hex string. +TAddress: Final = NewType("TAddress", str)This change is optional but will catch many category-errors for free.
tronpy/utils.py (1)
4-6: Add docstring & prefertime.time_ns()to eliminate rounding loss
int(time.time() * 1000)truncates micro-seconds. Usingtime.time_ns() // 1_000_000is equally portable and precise:-def current_timestamp() -> int: - return int(time.time() * 1000) +def current_timestamp() -> int: + """Return current epoch time in milliseconds.""" + return time.time_ns() // 1_000_000tronpy/defaults.py (1)
36-37: Name constant to reflect units
SIXTY_SECONDS = 60_000is milliseconds but the name implies seconds. RecommendSIXTY_SECONDS_MSorONE_MINUTE_MSto avoid mis-use:-SIXTY_SECONDS: typing.Final[int] = 60_000 -"""Milliseconds in 60 seconds""" +ONE_MINUTE_MS: typing.Final[int] = 60_000 # milliseconds.flake8 (1)
5-15: Duplicate.venventry and trailing whitespaceCleaning this block removes redundancy and keeps config tidy:
exclude= .circleci, - .venv, + .venv, .eggs, .git, .tox, - .venv, build, dist, docs, .env - +tests/test_offline.py (1)
75-75: Consider extracting the encoded data for better readability.The long hex-encoded data string makes the test harder to read and understand.
Consider extracting the encoded data to a constant with a descriptive comment:
+# TRC-20 transfer(address,uint256) function call data for 1,000,000 tokens +TRC20_TRANSFER_DATA = "a9059cbb0000000000000000000000009ce29546a328107df7b685ed3183fa8ae70a46a700000000000000000000000000000000000000000000000000000000000f4240" - "data": "a9059cbb0000000000000000000000009ce29546a328107df7b685ed3183fa8ae70a46a700000000000000000000000000000000000000000000000000000000000f4240", # noqa: E501 + "data": TRC20_TRANSFER_DATA,tronpy/tron.py (1)
220-367: Well-implemented offline transaction builders with excellent documentationThe new methods provide comprehensive offline transaction creation capabilities with:
- Detailed docstrings explaining all parameters and usage
- Clear examples and references to external documentation
- Proper delegation to protobuf-based implementation
While static analysis flags the high argument count, this is reasonable for offline transaction construction where all details must be provided upfront.
For future enhancements, consider implementing a builder pattern to manage the multiple parameters more elegantly, especially if more transaction types are added.
tronpy/proto/src/tron_pb2.pyi (1)
216-223:SUCESStypo mirrors upstream but hurts code-completionThe enum value is spelled
SUCESS(missing “C”). This typo exists in the official Tron proto, so the stub is technically correct, but it still leaks a misspelled symbol into user code.If you can’t fix the upstream proto, consider adding an alias inside the generated Python module so consumers can use the correctly-spelled
SUCCESSwhile retaining wire compatibility:# in tron_pb2.py, after enum definition Transaction.Result.code.SUCCESS = Transaction.Result.code.SUCESS # type: ignore[attr-defined]No change needed in the stub, just flagging the ergonomic issue.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
.flake8(1 hunks)Makefile(1 hunks)pyproject.toml(1 hunks)tests/test_offline.py(1 hunks)tronpy/async_tron.py(6 hunks)tronpy/defaults.py(2 hunks)tronpy/proto/src/tron.proto(1 hunks)tronpy/proto/src/tron_pb2.py(1 hunks)tronpy/proto/src/tron_pb2.pyi(1 hunks)tronpy/proto/transaction.py(1 hunks)tronpy/tron.py(8 hunks)tronpy/types.py(1 hunks)tronpy/utils.py(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
tronpy/async_tron.py (3)
tronpy/defaults.py (1)
conf_for_name(40-41)tronpy/utils.py (3)
current_timestamp(4-5)get_ref_block_bytes(8-9)get_ref_block_hash(12-13)tronpy/tron.py (1)
get_latest_solid_block_id(988-992)
🪛 Buf (1.54.0)
tronpy/proto/src/tron.proto
1-1: Files must have a package defined.
(PACKAGE_DEFINED)
🪛 Pylint (3.3.7)
tronpy/proto/transaction.py
[error] 67-67: Module 'google.protobuf.any_pb2' has no 'Any' member
(E1101)
[refactor] 108-108: Too many arguments (6/5)
(R0913)
[refactor] 108-108: Too many positional arguments (6/5)
(R0917)
[refactor] 108-108: Too many local variables (21/15)
(R0914)
[error] 179-179: Module 'google.protobuf.any_pb2' has no 'Any' member
(E1101)
tronpy/tron.py
[refactor] 303-303: Too many arguments (7/5)
(R0913)
[refactor] 303-303: Too many positional arguments (7/5)
(R0917)
🔇 Additional comments (12)
pyproject.toml (1)
44-45: Re-evaluate pinning very oldfreezegunversion
freezegun==0.3.4dates back to 2018; current releases are 1.x with important bug-fixes (e.g. py3.11 compat, DST issues). Unless you have a hard constraint, bumping prevents avoidable test flakiness:-freezegun = "0.3.4" +freezegun = "^1.4" # latest at time of writingSame for
grpcio-tools; consider an upper cap rather than a patch-only pin to avoid silent ABI breaks.tronpy/async_tron.py (3)
11-11: LGTM! Good refactoring with centralized utilities.The imports properly bring in the new constants and utility functions that eliminate code duplication.
Also applies to: 31-31
206-206: LGTM! Elimination of magic numbers.Replacing hardcoded
60_000milliseconds with the named constantSIXTY_SECONDSimproves readability and maintainability.Also applies to: 240-240, 358-358, 362-362
209-211: LGTM! Centralized block reference utilities.Using
get_ref_block_bytesandget_ref_block_hashutility functions eliminates code duplication and improves consistency across the codebase.Also applies to: 282-284
tronpy/proto/src/tron.proto (1)
38-153: LGTM! Comprehensive transaction message structure.The
Transactionmessage definition properly captures all the necessary components for Tron blockchain transactions, including:
- Extensive contract type enumeration
- Proper nesting of contract, result, and raw data structures
- Support for multiple signatures and results
The structure aligns well with Tron's transaction format.
tronpy/proto/src/tron_pb2.py (1)
1-55: LGTM! Standard protobuf-generated code.This is properly generated Python protobuf code. The file header correctly indicates it's autogenerated and should not be manually edited. The structure follows standard protobuf Python bindings conventions.
Ensure this file is regenerated using the Makefile target whenever the
.protofile changes.tests/test_offline.py (2)
8-45: LGTM! Well-structured deterministic test.The test properly uses
freezegunto ensure deterministic timestamps and transaction IDs. The comprehensive assertion validates the complete offline transaction structure for TRX transfers.
48-93: LGTM! Comprehensive smart contract transaction test.The test validates TRC-20 token transfer transaction creation with proper assertion of the encoded data payload and fee limit. The frozen time approach ensures reproducible results.
tronpy/tron.py (1)
196-201: Good refactoring to use centralized constants and utility functionsThe changes improve code maintainability by:
- Using the
SIXTY_SECONDSconstant instead of hardcoded values- Replacing manual block ID slicing with dedicated utility functions
tronpy/proto/transaction.py (1)
136-160: Excellent implementation of ERC-20 ABI encodingThe encoding logic correctly:
- Removes Tron's address prefix to get the EVM-compatible address
- Properly pads both address and amount to 32 bytes
- Uses the correct function selector for
transfer(address,uint256)The detailed comments with references to external documentation are particularly valuable.
tronpy/proto/src/tron_pb2.pyi (2)
7-12: Depending ongoogle.protobuf.internalis brittleThe stub imports
_containersand_enum_type_wrapperfromgoogle.protobuf.internal.*.
Those APIs are private and may change without notice between protobuf releases, breaking type-checking.If possible, prefer relying on
google.protobuf.message.Messagepublic helpers or ship a minimal local wrapper that exposes only the names you need, so upgrades do not silently break the stubs.
59-68: Verify the lowercaseauthoritymessage nameEvery other message type follows the conventional PascalCase (
AccountId,TransferContract, …), but this one is declared as lowercaseauthority.
Double-check the source.proto: if the original message is actuallyAuthority, the stub will not match the generated implementation, causing import errors at runtime and mypy false positives.-class authority(_message.Message): +class Authority(_message.Message):Confirm the proto definition before renaming.
| def get_ref_block_bytes(ref_block_id: str) -> str: | ||
| return ref_block_id[12:16] | ||
|
|
There was a problem hiding this comment.
Return bytes rather than hex substring to match protobuf expectations
ref_block_bytes in Tron transactions is raw bytes; returning a hex slice as str shifts the conversion burden downstream and risks double-encoding. Suggest:
-def get_ref_block_bytes(ref_block_id: str) -> str:
- return ref_block_id[12:16]
+def get_ref_block_bytes(ref_block_id: str) -> bytes:
+ """
+ Extract last 2 bytes of block height (Big-Endian) from 32-byte block id.
+ """
+ return bytes.fromhex(ref_block_id[12:16])📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def get_ref_block_bytes(ref_block_id: str) -> str: | |
| return ref_block_id[12:16] | |
| def get_ref_block_bytes(ref_block_id: str) -> bytes: | |
| """ | |
| Extract last 2 bytes of block height (Big-Endian) from 32-byte block id. | |
| """ | |
| return bytes.fromhex(ref_block_id[12:16]) |
🤖 Prompt for AI Agents
In tronpy/utils.py around lines 8 to 10, the function get_ref_block_bytes
currently returns a hex substring as a string, but it should return raw bytes to
match protobuf expectations. Modify the function to extract the relevant slice
from the input string, decode it from hex to bytes, and return the bytes object
instead of a string.
| syntax = "proto3"; | ||
|
|
||
| import "google/protobuf/any.proto"; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add package definition to prevent namespace conflicts.
The protobuf file is missing a package definition, which can lead to naming conflicts and makes the generated code harder to organize.
Add a package definition after the syntax declaration:
syntax = "proto3";
+
+package tronpy;
import "google/protobuf/any.proto";This will generate the Python code with proper module organization and prevent potential conflicts with other protobuf definitions.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| syntax = "proto3"; | |
| import "google/protobuf/any.proto"; | |
| syntax = "proto3"; | |
| package tronpy; | |
| import "google/protobuf/any.proto"; |
🧰 Tools
🪛 Buf (1.54.0)
1-1: Files must have a package defined.
(PACKAGE_DEFINED)
🤖 Prompt for AI Agents
In tronpy/proto/src/tron.proto at lines 1 to 4, the protobuf file lacks a
package definition, which can cause namespace conflicts and disorganized
generated code. Add a package declaration immediately after the syntax line to
define a unique package name, ensuring proper module organization and avoiding
naming collisions in the generated Python code.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tronpy/proto/transaction.py (1)
136-264: Consider refactoring to reduce complexityThe function correctly implements TRC-20 token transfer transaction creation with proper ABI encoding. However, it has high complexity with 6 parameters and many local variables.
The ABI encoding logic for
transfer(address,uint256)is correctly implemented:
- Function selector
0xa9059cbbis correct- Address encoding properly removes Tron's 0x41 prefix and pads to 32 bytes
- Amount encoding correctly converts to big-endian hex and pads
Consider extracting the ABI encoding logic into a separate helper function to reduce complexity:
+def _encode_transfer_call_data(to_address: str, amount: int) -> bytes: + """Encode transfer(address,uint256) call data according to Ethereum ABI.""" + to_hex = to_hex_address(to_address) + # Remove the 0x41 prefix (1-byte Tron address prefix) → 20-byte EVM + to_evm = to_hex[2:] + to_padded = to_evm.rjust(64, "0") + amount_hex = hex(amount)[2:].rjust(64, "0") + data_hex = "a9059cbb" + to_padded + amount_hex # 4-byte selector + args + return bytes.fromhex(data_hex) + def create_smart_contract_transaction_offline( from_address: str, to_address: str, amount: int, ref_block_id: str, fee_limit: int, contract_address: str, ) -> Transaction: # ... existing code ... - # Encode `transfer(address,uint256)` call data section - to_hex = to_hex_address(to_address) - # Remove the 0x41 prefix (1-byte Tron address prefix) → 20-byte EVM - to_evm = to_hex[2:] - to_padded = to_evm.rjust(64, "0") - amount_hex = hex(amount)[2:].rjust(64, "0") - data_hex = "a9059cbb" + to_padded + amount_hex # 4-byte selector + args - data_bytes = bytes.fromhex(data_hex) + data_bytes = _encode_transfer_call_data(to_address, amount) + data_hex = data_bytes.hex()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
tronpy/proto/transaction.py(1 hunks)tronpy/utils.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tronpy/utils.py
🧰 Additional context used
🪛 Pylint (3.3.7)
tronpy/proto/transaction.py
[error] 95-95: Module 'google.protobuf.any_pb2' has no 'Any' member
(E1101)
[refactor] 136-136: Too many arguments (6/5)
(R0913)
[refactor] 136-136: Too many positional arguments (6/5)
(R0917)
[refactor] 136-136: Too many local variables (21/15)
(R0914)
[error] 212-212: Module 'google.protobuf.any_pb2' has no 'Any' member
(E1101)
🔇 Additional comments (4)
tronpy/proto/transaction.py (4)
1-17: LGTM: Clean imports and well-defined TypedDictThe imports are appropriate for the module's functionality, and the
TransactionTypedDict provides clear type annotations for the transaction structure.
19-31: LGTM: Correct transaction ID calculationThe
calculate_txidfunction correctly implements the transaction ID calculation by hashing the serialized raw transaction data using SHA-256, which aligns with Tron's transaction ID specification.
38-57: LGTM: Well-implemented TAPoS metadata extractionThe
_get_tapos_metahelper function correctly extracts TAPoS metadata using the appropriate utility functions and constants. The 60-second expiration time is reasonable for transaction validity.
59-134: LGTM: Correct offline TRX transfer transaction creationThe
create_transaction_offlinefunction correctly implements offline TRX transfer transaction creation. The protobuf usage and JSON conversion logic are properly implemented.
| contract=[ | ||
| tron_pb2.Transaction.Contract( | ||
| type=tron_pb2.Transaction.Contract.ContractType.TransferContract, | ||
| parameter=google.protobuf.any_pb2.Any( |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify protobuf imports are properly configured
The static analysis tool flags google.protobuf.any_pb2.Any as potentially missing, but this is likely a false positive since protobuf modules are dynamically generated.
Please verify that the protobuf imports work correctly by running the following script:
Also applies to: 212-212
🏁 Script executed:
#!/bin/bash
# Description: Verify protobuf imports and dependencies are properly configured
# Check if protobuf is installed and importable
python -c "
import sys
try:
import google.protobuf.any_pb2
print('✓ google.protobuf.any_pb2 is importable')
# Check if Any class exists
any_instance = google.protobuf.any_pb2.Any()
print('✓ Any class is accessible and can be instantiated')
# Check if tron_pb2 is importable
from tronpy.proto.src import tron_pb2
print('✓ tron_pb2 is importable')
# Check if required protobuf classes exist
transaction = tron_pb2.Transaction()
print('✓ tron_pb2.Transaction is accessible')
print('All protobuf imports are working correctly')
except ImportError as e:
print(f'❌ Import error: {e}')
sys.exit(1)
except Exception as e:
print(f'❌ Error: {e}')
sys.exit(1)
"Length of output: 909
Ensure ‘base58’ Is Declared as a Dependency
The protobuf imports themselves are valid, but from tronpy.proto.src import tron_pb2 currently fails with
ImportError: No module named 'base58'
because the generated tron_pb2 code relies on the base58 package.
Please add base58 to your project’s dependencies so that all protobuf modules import correctly.
• Add base58 to requirements.txt or setup.py (or pyproject.toml)
• After updating dependencies, verify with:
pip install base58
python - <<EOF
from tronpy.proto.src import tron_pb2
_ = tron_pb2.Transaction()
print("✓ tron_pb2.Transaction is accessible")
EOFAffected locations:
- tronpy/proto/transaction.py:95
- tronpy/proto/transaction.py:212
🧰 Tools
🪛 Pylint (3.3.7)
[error] 95-95: Module 'google.protobuf.any_pb2' has no 'Any' member
(E1101)
🤖 Prompt for AI Agents
In tronpy/proto/transaction.py around lines 95 and 212, the generated tron_pb2
module requires the base58 package, which is not currently declared as a
dependency. To fix this, add base58 to your project's dependency list in
requirements.txt, setup.py, or pyproject.toml. Then, install the package and
verify the import of tron_pb2.Transaction works without ImportError by running
the provided test script.
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Explore these optional code suggestions:
|
|||||||||
rudolf-del
force-pushed
the
feat-add-offline-transaction-methods
branch
from
33eb470 to
a868703
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
tronpy/proto/transaction.py (1)
95-95: Protobuf dependency issue already addressedBased on past review comments, the base58 dependency issue has been identified and should be resolved by ensuring base58 is properly declared in the project dependencies.
🧹 Nitpick comments (1)
tronpy/proto/transaction.py (1)
136-263: Consider refactoring to reduce complexityThe function correctly implements TRC-20 token transfer transaction creation with proper ABI encoding and comprehensive documentation. However, the high complexity could be improved.
Issues identified:
- 6 parameters (exceeds typical 5-parameter limit)
- 21 local variables (exceeds typical 15-variable limit)
- Function length of ~127 lines
Suggested improvements:
- Extract ABI encoding logic into a separate function
- Consider using a dataclass or configuration object for parameters
- Split the function into smaller, focused methods
Proposed refactor for ABI encoding:
+def _encode_transfer_abi(to_address: str, amount: int) -> bytes: + """Encode transfer(address,uint256) call data per Ethereum ABI spec.""" + to_hex = to_hex_address(to_address) + to_evm = to_hex[2:] # Remove 0x41 prefix + to_padded = to_evm.rjust(64, "0") + amount_hex = hex(amount)[2:].rjust(64, "0") + data_hex = "a9059cbb" + to_padded + amount_hex + return bytes.fromhex(data_hex) + def create_smart_contract_transaction_offline( from_address: str, to_address: str, amount: int, ref_block_id: str, fee_limit: int, contract_address: str, ) -> Transaction: - # ... existing ABI encoding logic ... + data_bytes = _encode_transfer_abi(to_address, amount)This would reduce the function's complexity while maintaining the excellent documentation and functionality.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
Makefile(1 hunks)tronpy/proto/transaction.py(1 hunks)tronpy/utils.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- Makefile
- tronpy/utils.py
🧰 Additional context used
🪛 Pylint (3.3.7)
tronpy/proto/transaction.py
[error] 95-95: Module 'google.protobuf.any_pb2' has no 'Any' member
(E1101)
[refactor] 136-136: Too many arguments (6/5)
(R0913)
[refactor] 136-136: Too many positional arguments (6/5)
(R0917)
[refactor] 136-136: Too many local variables (21/15)
(R0914)
[error] 212-212: Module 'google.protobuf.any_pb2' has no 'Any' member
(E1101)
🔇 Additional comments (5)
tronpy/proto/transaction.py (5)
1-9: LGTM: Import structure is well-organizedThe imports are logically grouped and include all necessary dependencies for offline transaction creation. The static analysis warning about
google.protobuf.any_pb2.Anyappears to be a false positive based on the past review verification.
12-16: LGTM: TypedDict provides clear transaction structureThe
TransactionTypedDict effectively defines the expected structure for offline transactions with proper type hints. The optionalpermissionfield correctly reflects the Tron transaction format.
19-30: LGTM: Transaction ID calculation is correctThe
calculate_txidfunction correctly implements Tron's transaction ID calculation by hashing the serialized raw data with SHA-256. The implementation is clean and well-documented.
38-56: LGTM: TAPoS metadata extraction is efficientThe
_get_tapos_metahelper function correctly extracts Transaction as Proof of Stake metadata and properly calculates the expiration timestamp. Good separation of concerns.
59-133: LGTM: TRX transfer transaction creation is comprehensiveThe
create_transaction_offlinefunction correctly implements offline TRX transfer transaction creation. The protobuf construction is accurate, address handling is proper, and the return structure matches the expected transaction format.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tronpy/async_tron.py (1)
317-386: Consider parameter consolidation for better maintainability.The
build_trc20_transfer_offlinemethod is well-implemented with excellent documentation, but the high parameter count flagged by static analysis could impact maintainability.Consider consolidating related parameters into a configuration object:
@classmethod def build_trc20_transfer_offline( cls, - from_address: TAddress, - to_address: TAddress, - amount: int, - contract_address: TAddress, - ref_block_id: str, - fee_limit: int = DEFAULT_CONF["fee_limit"], + transfer_params: dict, ) -> "AsyncTransaction": """Build an **unsigned** ``TriggerSmartContract`` transaction that calls the TRC-20 standard ``transfer`` function completely offline. Parameters ---------- + transfer_params : dict + Dictionary containing: from_address, to_address, amount, + contract_address, ref_block_id, and optionally fee_limitHowever, keeping the current signature may be preferred for API consistency with the synchronous version.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
tests/test_offline.py(1 hunks)tronpy/async_tron.py(7 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/test_offline.py
🧰 Additional context used
🧬 Code Graph Analysis (1)
tronpy/async_tron.py (4)
tronpy/defaults.py (1)
conf_for_name(40-41)tronpy/proto/transaction.py (2)
create_smart_contract_transaction_offline(136-263)create_transaction_offline(59-133)tronpy/utils.py (3)
current_timestamp(4-11)get_ref_block_bytes(14-24)get_ref_block_hash(27-37)tronpy/tron.py (4)
build_offline(221-300)txid(50-52)build_trc20_transfer_offline(303-367)fee_limit(411-414)
🪛 Pylint (3.3.7)
tronpy/async_tron.py
[refactor] 318-318: Too many arguments (7/5)
(R0913)
[refactor] 318-318: Too many positional arguments (7/5)
(R0917)
🔇 Additional comments (4)
tronpy/async_tron.py (4)
11-11: LGTM: Clean import organization.The import additions properly bring in the centralized constants, utility functions, and protobuf transaction creation capabilities needed for the new offline functionality.
Also applies to: 30-30, 32-32
207-207: Excellent refactoring to use centralized constant.Replacing hardcoded
60_000millisecond values with theSIXTY_SECONDSconstant improves maintainability and ensures consistency across the codebase.Also applies to: 398-398, 516-516, 520-520
210-210: Good refactoring to eliminate code duplication.Replacing manual string slicing with the utility functions
get_ref_block_bytesandget_ref_block_hashcentralizes the TAPoS metadata extraction logic and reduces the chance of errors.Also applies to: 212-212, 440-440, 442-442
231-315: Well-implemented offline TRX transfer method.The
build_offlinemethod provides a clean interface for creating unsigned TRX transfer transactions without network calls. The comprehensive docstring clearly explains the purpose, parameters, return value, and usage examples.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
pyproject.toml(2 hunks)setup.py(1 hunks)test-requirements.txt(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- test-requirements.txt
🚧 Files skipped from review as they are similar to previous changes (1)
- pyproject.toml
| ] | ||
|
|
||
| extras_hdwallet = {"mnemonic": ["mnemonic==0.20"]} | ||
| extras_hdwallet = {"mnemonic": ["mnemonic==0.20"], "offline": ["protobuf"]} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add version constraint for protobuf dependency.
The protobuf dependency lacks version constraints, which creates inconsistency with the existing mnemonic dependency pattern and could lead to compatibility issues, security vulnerabilities, and reproducibility problems.
Apply this diff to add appropriate version constraints:
-extras_hdwallet = {"mnemonic": ["mnemonic==0.20"], "offline": ["protobuf"]}
+extras_hdwallet = {"mnemonic": ["mnemonic==0.20"], "offline": ["protobuf>=3.20.0,<5.0.0"]}Consider using a version range that's compatible with the protobuf files generated in this PR to ensure stability and predictable behavior.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| extras_hdwallet = {"mnemonic": ["mnemonic==0.20"], "offline": ["protobuf"]} | |
| extras_hdwallet = {"mnemonic": ["mnemonic==0.20"], "offline": ["protobuf>=3.20.0,<5.0.0"]} |
🤖 Prompt for AI Agents
In setup.py at line 22, the protobuf dependency in extras_hdwallet lacks a
version constraint, unlike the mnemonic dependency which specifies
"mnemonic==0.20". To fix this, add an appropriate version range constraint to
the protobuf dependency that matches the protobuf files generated in this PR,
ensuring compatibility and stability. Update the extras_hdwallet dictionary to
include this version constraint for protobuf.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tronpy/defaults.py (1)
41-41: Remove redundantNoneparameter fromdict.get()The
Noneparameter is redundant sincedict.get()returnsNoneby default when the key is not found.- return ALL.get(name, None) + return ALL.get(name)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
tests/test_offline.py(1 hunks)tests/utils.py(2 hunks)tronpy/async_tron.py(8 hunks)tronpy/defaults.py(2 hunks)tronpy/exceptions.py(1 hunks)tronpy/proto/__init__.py(1 hunks)tronpy/proto/transaction.py(1 hunks)tronpy/tron.py(9 hunks)
✅ Files skipped from review due to trivial changes (2)
- tronpy/exceptions.py
- tronpy/proto/init.py
🚧 Files skipped from review as they are similar to previous changes (3)
- tests/test_offline.py
- tronpy/tron.py
- tronpy/async_tron.py
🧰 Additional context used
🧬 Code Graph Analysis (1)
tronpy/proto/transaction.py (4)
tronpy/exceptions.py (1)
ProtobufImportError(69-74)tronpy/keys/__init__.py (2)
to_raw_address(109-111)to_address(229-230)tronpy/utils.py (3)
current_timestamp(4-11)get_ref_block_bytes(14-24)get_ref_block_hash(27-37)tronpy/tron.py (3)
Transaction(107-378)expiration(418-420)fee_limit(422-425)
🪛 Ruff (0.11.9)
tronpy/defaults.py
41-41: Use ALL.get(name) instead of ALL.get(name, None)
Replace ALL.get(name, None) with ALL.get(name)
(SIM910)
🔇 Additional comments (10)
tronpy/defaults.py (2)
36-37: Well-defined constant with proper typingThe
SIXTY_SECONDSconstant is appropriately typed astyping.Final[int]and clearly documented. This improves code maintainability by replacing magic numbers with a named constant.
44-46: Consistent error message handlingThe
PROTOBUF_NOT_INSTALLED_ERROR_MESSAGEconstant ensures consistent error messaging across the codebase when protobuf dependencies are missing.tronpy/proto/transaction.py (6)
6-11: Proper protobuf import error handlingThe try-except block correctly handles protobuf import failures using the custom
ProtobufImportErrorexception, which provides better error categorization than genericImportError.
18-22: Well-structured TypedDict definitionThe
TransactionTypedDict provides clear type hints for the transaction structure, improving code maintainability and IDE support.
25-36: Correct transaction ID calculationThe
calculate_txidfunction properly computes the transaction ID using SHA-256 hash of the serialized raw transaction data, which aligns with TRON protocol specifications.
175-199: Comprehensive ABI encoding implementationThe smart contract call data encoding follows the Ethereum ABI specification correctly:
- Uses proper function selector (0xa9059cbb for
transfer(address,uint256))- Handles address conversion from TRON format to EVM format
- Properly pads arguments to 32-byte words
- Includes helpful comments explaining the encoding process
65-139: Robust offline transaction creationThe
create_transaction_offlinefunction correctly:
- Converts addresses to raw format for protobuf
- Extracts TAPoS metadata using utility functions
- Builds the protobuf transaction structure
- Returns a properly formatted transaction dictionary
142-269: Comprehensive TRC-20 transaction builderThe
create_smart_contract_transaction_offlinefunction provides a complete implementation for offline TRC-20 token transfers with proper:
- Address conversions
- ABI encoding for the transfer function
- TAPoS metadata extraction
- Protobuf transaction construction
- Fee limit handling
tests/utils.py (2)
14-18: Updated permission structure for offline transactionsThe permission structure assertion has been updated to expect a detailed dictionary instead of
None, which aligns with the new offline transaction functionality. The structure includes proper keys (keys,threshold,permission_name) that match the protobuf-based transaction format.
30-35: Improved code formattingThe multi-line set literal formatting improves readability while maintaining the same functionality.
2 participants
PR Type
Enhancement
Description
Add offline transaction building methods for TRX transfers
Add offline TRC-20 token transfer transaction building
Refactor timestamp and block reference utilities
Add Protocol Buffer definitions for transaction structures
Changes diagram
Changes walkthrough 📝
1 files
Add comprehensive tests for offline transaction methods4 files
Refactor to use shared utilities and constantsCore offline transaction building implementationAdd offline transaction class methods and refactor utilitiesExtract shared utility functions for timestamps and blocks3 files
Add SIXTY_SECONDS constant for transaction expirationUpdate linting configuration with exclusionsAdd Protocol Buffer generation target4 files
Generated Protocol Buffer Python bindingsAdd development dependencies for protobuf and testingProtocol Buffer schema for TRON transactionsType stubs for generated Protocol Buffer code1 files
Add TAddress type aliasSummary by CodeRabbit
New Features
Bug Fixes
Chores
Documentation