Feat/add trade reward campaign endpoint #309
Merged
Conversation
This file contains 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
… returning any cookie
…s assistant to tackle the suggestion in the PR review
…policy fix/update_indexer_cookie_policy
…Added the use of `dotenv` to load private keys into environment variables. Removed `asyncio` package dependency
…Added the use of `dotenv` to load private keys into environment variables. Removed `asyncio` package dependency
…o new example scripts for all queries and unit tests
…the `distribution` module. Marked as deprecated old functions not following Python naming standards and created the replacements for them (with the correct name following standards)
…new example scripts for all the queries and unit tests.
…er. Added unit tests for new messages. Refactored example scripts to move them into separated subfolders for each module.
…new example scripts for all the queries and unit tests.
…er. Added unit tests for new messages. Refactored example scripts to move them into separated subfolders for each module.
…ule: TradeRewardCampaign
…o feat/add_trade_reward_campaign_endpoint
WalkthroughThe latest updates introduce enhanced query and message support in the distribution and exchange modules, improve private key loading via python-dotenv, and refine cookie assistant logic. Notably, Changes
TipsChatThere are 3 ways to chat with CodeRabbit:
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #309 +/- ##
==========================================
+ Coverage 74.60% 76.16% +1.55%
==========================================
Files 50 50
Lines 4099 4111 +12
Branches 349 351 +2
==========================================
+ Hits 3058 3131 +73
+ Misses 941 875 -66
- Partials 100 105 +5 ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 19
Configuration used: .coderabbit.yaml
Files selected for processing (24)
- CHANGELOG.md (1 hunks)
- examples/chain_client/exchange/11_MsgCreateDerivativeMarketOrder.py (1 hunks)
- examples/chain_client/exchange/query/41_TradeRewardCampaign.py (1 hunks)
- examples/chain_client/exchange/query/42_FeeDiscountAccountInfo.py (1 hunks)
- examples/chain_client/exchange/query/43_FeeDiscountSchedule.py (1 hunks)
- examples/chain_client/exchange/query/44_BalanceMismatches.py (1 hunks)
- examples/chain_client/exchange/query/45_BalanceWithBalanceHolds.py (1 hunks)
- examples/chain_client/exchange/query/46_FeeDiscountTierStatistics.py (1 hunks)
- examples/chain_client/exchange/query/47_MitoVaultInfos.py (1 hunks)
- examples/chain_client/exchange/query/48_QueryMarketIDFromVault.py (1 hunks)
- examples/chain_client/exchange/query/49_HistoricalTradeRecords.py (1 hunks)
- examples/chain_client/exchange/query/50_IsOptedOutOfRewards.py (1 hunks)
- examples/chain_client/exchange/query/51_OptedOutOfRewardsAccounts.py (1 hunks)
- examples/chain_client/exchange/query/52_MarketVolatility.py (1 hunks)
- examples/chain_client/exchange/query/53_BinaryOptionsMarkets.py (1 hunks)
- examples/chain_client/exchange/query/54_TraderDerivativeConditionalOrders.py (1 hunks)
- examples/chain_client/exchange/query/55_MarketAtomicExecutionFeeMultiplier.py (1 hunks)
- poetry.lock (14 hunks)
- pyinjective/async_client.py (1 hunks)
- pyinjective/client/chain/grpc/chain_grpc_exchange_api.py (1 hunks)
- pyinjective/core/network.py (4 hunks)
- tests/client/chain/grpc/configurable_exchange_query_servicer.py (2 hunks)
- tests/client/chain/grpc/test_chain_grpc_exchange_api.py (1 hunks)
- tests/core/test_network.py (1 hunks)
Files not summarized due to errors (1)
- poetry.lock: Error: Message exceeds token limit
Additional comments: 24
CHANGELOG.md (3)
- 5-9: The update for version 1.4.0 clearly documents the new features added, including support for queries and messages in the 'distribution' and 'exchange' modules, and the use of python-dotenv for loading private keys. This is a significant enhancement for users who rely on environment variables for configuration.
- 11-13: The changes in version 1.3.1, which update the cookie assistant logic to accommodate different server behaviors, are well-documented. This change is crucial for ensuring compatibility with various server configurations and enhances the robustness of the SDK.
- 14-14: The removal of
asynciofrom dependencies in version 1.3.0 is a significant change that could impact users. It's important to ensure that this change is communicated clearly to users who might be directly importingasyncioin their projects due to its previous inclusion.Verification successful
The removal of
asynciofrom dependencies, as noted in the CHANGELOG.md, is a significant change given the extensive use ofasynciothroughout the project, including in tests, examples, and core modules. Whileasynciois a standard library in Python 3.4 and later, making its explicit inclusion in dependencies unnecessary for running the project, it's crucial that this change is clearly communicated to users. Users need to be aware of the Python version requirements and any potential adjustments they might need to make in their projects due to this change. Ensuring that all documentation reflects this change is essential for minimizing user impact.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for mentions of asyncio in example scripts and documentation to ensure users are informed about its removal. rg --type md --type py 'asyncio'Length of output: 85922
pyinjective/core/network.py (3)
- 26-29: The logic for creating
metadatawhen a cookie is available is clear and straightforward. However, it's important to ensure that all callers ofexchange_metadataare prepared to handle aNonereturn value, as this could potentially lead to unhandled exceptions if not properly checked.- 73-73: Assigning an empty string to
cookie_infowhen it isNoneis a safe fallback. This ensures that the_exchange_cookieattribute is always a string, which simplifies type checking and handling in other parts of the code. Good approach for consistency.- 140-140: The same approach of assigning an empty string to
cookie_infowhen it isNoneis used here as well. This consistency in handling potentialNonevalues across different methods is commendable.tests/client/chain/grpc/configurable_exchange_query_servicer.py (1)
- 260-263: The addition of the
TradeRewardCampaignmethod to handle trade reward campaign responses is a necessary update to support the new functionality introduced in the PR. This method follows the established pattern of popping responses from a deque, which is consistent with the other methods in the class. Ensure that the deque is properly managed elsewhere in the code to avoid runtime errors due to popping from an empty deque.pyinjective/client/chain/grpc/chain_grpc_exchange_api.py (1)
- 454-458: The method
fetch_trade_reward_campaignis correctly implemented following the async pattern used throughout the class. It constructs the appropriate request and executes the gRPC call to fetch trade reward campaign information. This addition aligns with the PR's objective to enhance the chain exchange module's functionality.However, it's important to ensure that the corresponding gRPC stub method
TradeRewardCampaignexists and is correctly implemented on the server side. Additionally, consider adding error handling to manage potential exceptions that could arise from the gRPC call, enhancing the method's robustness.Verification successful
The search results confirm the existence of the
TradeRewardCampaignmethod within thepyinjective/proto/injective/exchange/v1beta1/query_pb2_grpc.pyfile, as well as its usage inpyinjective/client/chain/grpc/chain_grpc_exchange_api.pyand related test files. This verifies that the methodfetch_trade_reward_campaigncorrectly implements a gRPC call toTradeRewardCampaign, aligning with the review comment's approval of the method's implementation.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify the existence and correct implementation of the gRPC stub method TradeRewardCampaign. # This script is a placeholder and should be replaced with actual verification steps as needed. echo "Verify the gRPC stub method TradeRewardCampaign on the server side."Length of output: 144
Script:
#!/bin/bash # Search for the gRPC stub file or service definitions containing the TradeRewardCampaign method. fd 'exchange_query_pb.py' --exec rg 'TradeRewardCampaign'Length of output: 55
Script:
#!/bin/bash # Broaden the search for the TradeRewardCampaign method across all Python files. rg 'TradeRewardCampaign' --type pyLength of output: 39649
tests/client/chain/grpc/test_chain_grpc_exchange_api.py (1)
- 1912-2023: The test method
test_fetch_trade_reward_campaignis well-structured and covers a comprehensive scenario for fetching trade reward campaign information. It properly sets up mock responses and asserts the expected outcome against the actual API call result. However, there are a few areas for improvement and clarification:
Mock Data Completeness: Ensure that the mock data used in the test, especially for complex objects like
TradingRewardCampaignBoostInfo, accurately reflects the possible real-world data structures. This includes considering edge cases and varying data configurations.Assertion Granularity: While the final assertion checks if the entire response matches the expected structure, it might be beneficial to add more granular assertions. This could involve checking individual fields or subsets of the response to pinpoint issues more precisely if the test fails.
Async and Await Usage: The use of
asyncandawaitis correct, ensuring that the test properly handles asynchronous calls.Test Coverage: Consider if additional test cases are needed to cover different scenarios, such as error handling, empty responses, or responses with partial data.
Documentation and Comments: While the test method is straightforward, adding comments or documentation about the purpose of the test and the specific aspects it covers could enhance maintainability and clarity for future developers.
Overall, the test method is a valuable addition to the test suite, ensuring the new functionality for fetching trade reward campaign information is verified.
pyinjective/async_client.py (1)
- 968-970: The newly added method
fetch_trade_reward_campaigncorrectly implements an asynchronous call toself.chain_exchange_api.fetch_trade_reward_campaign(). This addition aligns with the PR's objective of enhancing the chain exchange module by allowing users to query trade reward campaign information. The method is well-structured, follows the async pattern used throughout the file, and returns a dictionary, which is consistent with the return types of similar methods in the class.poetry.lock (14)
- 638-694: The update to the
coveragepackage from an unspecified version to7.4.3includes a wide range of wheel files for different Python versions and platforms. This update seems to be a routine version bump to keep the package up-to-date. Ensure that this version ofcoverageis compatible with the project's testing framework and does not introduce any breaking changes.- 902-908: The update to the
eth-abipackage to version5.0.1is noted. Given the project's focus on blockchain-related functionalities, it's crucial to verify that this version ofeth-abisupports all necessary Ethereum ABI definitions and encoding/decoding operations required by the project. Compatibility with the project's Ethereum interaction code should be confirmed.- 917-919: The addition of development, documentation, and testing extras for the
eth-abipackage suggests an effort to enhance the development environment and testing capabilities. It's important to ensure these extras are utilized in the project's CI/CD pipelines or development setups to leverage their benefits fully.- 951-957: The
eth-hashpackage version0.7.0update should be checked for compatibility with the project's requirements for Ethereum hashing functionalities. Given the critical role of hashing in blockchain applications, any changes in the hashing behavior or supported algorithms must be thoroughly tested.- 972-978: The update to
eth-keyfilepackage version0.8.0is crucial for handling Ethereum private keys securely. Ensure that the updated version introduces no regressions in keyfile parsing, generation, or encryption/decryption processes.- 1233-1291: The
grpciopackage update to version1.62.0includes a significant number of wheel files for various environments. Givengrpcio's role in RPC and networking within the project, it's essential to verify that this update does not affect the stability and performance of network communications, especially in production environments.- 1398-1404: The
identifypackage update to version2.5.35is primarily used for file identification, which can be important for linting and code formatting tools. Ensure that this update integrates well with the project's code quality tools and does not introduce any issues with file detection.- 1788-1794: The
pre-commitpackage update to version3.6.2suggests an enhancement in the pre-commit hook framework used by the project. It's important to review the changelog for this version to ensure that all hooks are still compatible and functioning as expected.- 1806-1821: The
protobufpackage update to version4.25.3is critical for projects utilizing Protocol Buffers. Compatibility with the project's existing.protofiles and generated code should be verified to prevent any serialization or deserialization issues.- 1915-1921: The
pytestpackage update to version8.0.2is key for maintaining the project's test suite. Ensure that the new version is compatible with the project's test configurations and does not introduce any breaking changes in test discovery or execution.- 2270-2373: The addition of the
rpds-pypackage version0.18.0introduces Python bindings to Rust's persistent data structures. This addition could significantly impact the project's performance and memory usage patterns. It's crucial to evaluate the integration points ofrpds-pywithin the project to ensure they align with performance goals and do not introduce unexpected behavior.- 2388-2400: The
setuptoolspackage update to version69.1.1is a routine update but essential for the project's build and distribution process. Ensure that this update does not interfere with package building, especially if the project uses specificsetuptoolsfeatures or plugins.- 2448-2454: The
typing-extensionspackage update to version4.10.0supports backported and experimental type hints. Given the importance of type hints for code quality and maintainability, ensure that the updated version is fully compatible with the project's static type checking setup.- 2475-2481: The
virtualenvpackage update to version20.25.1is crucial for managing isolated Python environments. Verify that this update does not affect the project's development setup or CI/CD pipelines, especially ifvirtualenvis used extensively for environment management.
Comment on lines
965
to
973
| pending_pool_timestamp=pending_pool_timestamp, | ||
| ) | ||
|
|
||
| async def fetch_trade_reward_campaign(self) -> Dict[str, Any]: | ||
| return await self.chain_exchange_api.fetch_trade_reward_campaign() | ||
|
|
||
| async def fetch_fee_discount_account_info(self, account: str) -> Dict[str, Any]: | ||
| return await self.chain_exchange_api.fetch_fee_discount_account_info(account=account) | ||
|
|
There was a problem hiding this comment.
📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1-970]
Overall, the AsyncClient class in async_client.py is well-implemented, adhering to best practices for asynchronous programming in Python. The methods are logically structured, and the use of async/await syntax is consistent across the file, which is essential for non-blocking I/O operations. The newly added method fetch_trade_reward_campaign correctly aligns with the PR's objectives and follows the established patterns within the class.
However, a few general recommendations for improvement across the class include:
- Error Handling: Ensure robust error handling in all network calls and async operations. While some methods include try-except blocks, a more consistent approach across all methods would improve the resilience of the SDK.
- Documentation: Adding docstrings to methods, especially newly added ones, would enhance the maintainability of the code by providing clear descriptions of method functionalities, expected parameters, and return types.
- Performance Considerations: For methods making network requests, consider implementing caching mechanisms where appropriate to reduce the number of requests and improve the SDK's performance.
These recommendations aim to further enhance the quality and maintainability of the SDK.
# Example improvement for error handling in async methods
async def fetch_trade_reward_campaign(self) -> Dict[str, Any]:
try:
return await self.chain_exchange_api.fetch_trade_reward_campaign()
except Exception as e:
# Handle specific exceptions or log errors as appropriate
raise
Comment on lines
+89
to
+93
| expiration_data: Optional[str] = cookie.get("GCLB", {}).get("expires", None) | ||
| if expiration_data is None: | ||
| expiration_time = 0 | ||
| else: | ||
| expiration_time = datetime.datetime.strptime(expiration_data, "%a, %d-%b-%Y %H:%M:%S %Z").timestamp() |
There was a problem hiding this comment.
The handling of missing expiration data in _is_cookie_expired by setting expiration_time to 0 is a pragmatic solution. However, it's crucial to document the behavior and implications of this choice, especially how it affects cookie renewal logic. Consider adding a comment explaining why setting expiration_time to 0 is appropriate and how it impacts the session renewal process.
Comment on lines
+7
to
+19
| async def main() -> None: | ||
| # select network: local, testnet, mainnet | ||
| network = Network.testnet() | ||
|
|
||
| # initialize grpc client | ||
| client = AsyncClient(network) | ||
|
|
||
| infos = await client.fetch_mito_vault_infos() | ||
| print(infos) | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| asyncio.get_event_loop().run_until_complete(main()) |
There was a problem hiding this comment.
The example script is well-structured and demonstrates the usage of the fetch_mito_vault_infos method effectively. However, there are a couple of areas that could be improved for robustness and clarity:
- Consider adding error handling around the async call to
fetch_mito_vault_infosto manage potential exceptions that could arise from network issues or API changes. - Enhancing the output formatting could provide users with a clearer understanding of the data structure returned by the query, especially for complex data types.
- infos = await client.fetch_mito_vault_infos()
+ try:
+ infos = await client.fetch_mito_vault_infos()
+ # Consider adding more detailed formatting or processing here
+ print(infos)
+ except Exception as e:
+ print(f"Failed to fetch Mito Vault Infos: {e}")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.
Suggested change
| async def main() -> None: | |
| # select network: local, testnet, mainnet | |
| network = Network.testnet() | |
| # initialize grpc client | |
| client = AsyncClient(network) | |
| infos = await client.fetch_mito_vault_infos() | |
| print(infos) | |
| if __name__ == "__main__": | |
| asyncio.get_event_loop().run_until_complete(main()) | |
| async def main() -> None: | |
| # select network: local, testnet, mainnet | |
| network = Network.testnet() | |
| # initialize grpc client | |
| client = AsyncClient(network) | |
| try: | |
| infos = await client.fetch_mito_vault_infos() | |
| # Consider adding more detailed formatting or processing here | |
| print(infos) | |
| except Exception as e: | |
| print(f"Failed to fetch Mito Vault Infos: {e}") | |
| if __name__ == "__main__": | |
| asyncio.get_event_loop().run_until_complete(main()) |
Comment on lines
+11
to
+31
| async def main() -> None: | ||
| dotenv.load_dotenv() | ||
| configured_private_key = os.getenv("INJECTIVE_PRIVATE_KEY") | ||
|
|
||
| # select network: local, testnet, mainnet | ||
| network = Network.testnet() | ||
|
|
||
| # initialize grpc client | ||
| client = AsyncClient(network) | ||
|
|
||
| # load account | ||
| priv_key = PrivateKey.from_hex(configured_private_key) | ||
| pub_key = priv_key.to_public_key() | ||
| address = pub_key.to_address() | ||
| await client.fetch_account(address.to_acc_bech32()) | ||
|
|
||
| campaign = await client.fetch_trade_reward_campaign() | ||
| print(campaign) | ||
|
|
||
|
|
||
| if __name__ == "__main__": |
There was a problem hiding this comment.
The example script for querying trade reward campaigns is well-structured and follows the project's conventions for async operations. However, there are a few areas for improvement:
- Error Handling: Consider adding error handling around the
await client.fetch_trade_reward_campaign()call to gracefully handle potential exceptions that could arise from network issues or API changes. - Environment Variables: The script assumes the presence of an environment variable
INJECTIVE_PRIVATE_KEY. It's good practice to check if the variable is notNoneafter retrieval and provide a meaningful error message if it's missing. - Hardcoded Network: The network is hardcoded to
testnet. For better flexibility, consider allowing the network type to be passed as an argument or environment variable.
+ if configured_private_key is None:
+ raise ValueError("INJECTIVE_PRIVATE_KEY environment variable is not set.")
+ try:
+ campaign = await client.fetch_trade_reward_campaign()
+ print(campaign)
+ except Exception as e:
+ print(f"Failed to fetch trade reward campaign: {e}")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.
Suggested change
| async def main() -> None: | |
| dotenv.load_dotenv() | |
| configured_private_key = os.getenv("INJECTIVE_PRIVATE_KEY") | |
| # select network: local, testnet, mainnet | |
| network = Network.testnet() | |
| # initialize grpc client | |
| client = AsyncClient(network) | |
| # load account | |
| priv_key = PrivateKey.from_hex(configured_private_key) | |
| pub_key = priv_key.to_public_key() | |
| address = pub_key.to_address() | |
| await client.fetch_account(address.to_acc_bech32()) | |
| campaign = await client.fetch_trade_reward_campaign() | |
| print(campaign) | |
| if __name__ == "__main__": | |
| async def main() -> None: | |
| dotenv.load_dotenv() | |
| configured_private_key = os.getenv("INJECTIVE_PRIVATE_KEY") | |
| + if configured_private_key is None: | |
| + raise ValueError("INJECTIVE_PRIVATE_KEY environment variable is not set.") | |
| # select network: local, testnet, mainnet | |
| network = Network.testnet() | |
| # initialize grpc client | |
| client = AsyncClient(network) | |
| # load account | |
| priv_key = PrivateKey.from_hex(configured_private_key) | |
| pub_key = priv_key.to_public_key() | |
| address = pub_key.to_address() | |
| await client.fetch_account(address.to_acc_bech32()) | |
| + try: | |
| + campaign = await client.fetch_trade_reward_campaign() | |
| + print(campaign) | |
| + except Exception as e: | |
| + print(f"Failed to fetch trade reward campaign: {e}") | |
| if __name__ == "__main__": |
Comment on lines
+7
to
+19
| async def main() -> None: | ||
| # select network: local, testnet, mainnet | ||
| network = Network.testnet() | ||
|
|
||
| # initialize grpc client | ||
| client = AsyncClient(network) | ||
|
|
||
| schedule = await client.fetch_fee_discount_schedule() | ||
| print(schedule) | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| asyncio.get_event_loop().run_until_complete(main()) |
There was a problem hiding this comment.
Similar to the previous example, this script demonstrates the usage of the fetch_fee_discount_schedule method effectively. To improve robustness and clarity:
- Implement error handling around the async call to manage potential exceptions gracefully.
- Enhance the output formatting for a better understanding of the returned data, particularly for users unfamiliar with the data structure.
- schedule = await client.fetch_fee_discount_schedule()
+ try:
+ schedule = await client.fetch_fee_discount_schedule()
+ # Consider adding more detailed formatting or processing here
+ print(schedule)
+ except Exception as e:
+ print(f"Failed to fetch Fee Discount Schedule: {e}")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.
Suggested change
| async def main() -> None: | |
| # select network: local, testnet, mainnet | |
| network = Network.testnet() | |
| # initialize grpc client | |
| client = AsyncClient(network) | |
| schedule = await client.fetch_fee_discount_schedule() | |
| print(schedule) | |
| if __name__ == "__main__": | |
| asyncio.get_event_loop().run_until_complete(main()) | |
| async def main() -> None: | |
| # select network: local, testnet, mainnet | |
| network = Network.testnet() | |
| # initialize grpc client | |
| client = AsyncClient(network) | |
| try: | |
| schedule = await client.fetch_fee_discount_schedule() | |
| # Consider adding more detailed formatting or processing here | |
| print(schedule) | |
| except Exception as e: | |
| print(f"Failed to fetch Fee Discount Schedule: {e}") | |
| if __name__ == "__main__": | |
| asyncio.get_event_loop().run_until_complete(main()) |
Comment on lines
+11
to
+37
| async def main() -> None: | ||
| dotenv.load_dotenv() | ||
| configured_private_key = os.getenv("INJECTIVE_PRIVATE_KEY") | ||
|
|
||
| # select network: local, testnet, mainnet | ||
| network = Network.testnet() | ||
|
|
||
| # initialize grpc client | ||
| client = AsyncClient(network) | ||
|
|
||
| # load account | ||
| priv_key = PrivateKey.from_hex(configured_private_key) | ||
| pub_key = priv_key.to_public_key() | ||
| address = pub_key.to_address() | ||
| await client.fetch_account(address.to_acc_bech32()) | ||
|
|
||
| subaccount_id = address.get_subaccount_id(index=0) | ||
|
|
||
| orders = await client.fetch_trader_derivative_conditional_orders( | ||
| subaccount_id=subaccount_id, | ||
| market_id="0x17ef48032cb24375ba7c2e39f384e56433bcab20cbee9a7357e4cba2eb00abe6", | ||
| ) | ||
| print(orders) | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| asyncio.get_event_loop().run_until_complete(main()) |
There was a problem hiding this comment.
The script for querying trader derivative conditional orders is well-structured. Recommendations for further improvement include:
- Error Handling: Implement error handling around the
await client.fetch_trader_derivative_conditional_orders(...)call to better manage potential exceptions. - Parameterization: The
subaccount_idandmarket_idare derived from the loaded account and hardcoded, respectively. Consider allowing these values to be passed as arguments or read from environment variables for greater flexibility.
+ try:
+ orders = await client.fetch_trader_derivative_conditional_orders(
+ subaccount_id=subaccount_id,
+ market_id="0x17ef48032cb24375ba7c2e39f384e56433bcab20cbee9a7357e4cba2eb00abe6",
+ )
+ print(orders)
+ except Exception as e:
+ print(f"Failed to fetch trader derivative conditional orders: {e}")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.
Suggested change
| async def main() -> None: | |
| dotenv.load_dotenv() | |
| configured_private_key = os.getenv("INJECTIVE_PRIVATE_KEY") | |
| # select network: local, testnet, mainnet | |
| network = Network.testnet() | |
| # initialize grpc client | |
| client = AsyncClient(network) | |
| # load account | |
| priv_key = PrivateKey.from_hex(configured_private_key) | |
| pub_key = priv_key.to_public_key() | |
| address = pub_key.to_address() | |
| await client.fetch_account(address.to_acc_bech32()) | |
| subaccount_id = address.get_subaccount_id(index=0) | |
| orders = await client.fetch_trader_derivative_conditional_orders( | |
| subaccount_id=subaccount_id, | |
| market_id="0x17ef48032cb24375ba7c2e39f384e56433bcab20cbee9a7357e4cba2eb00abe6", | |
| ) | |
| print(orders) | |
| if __name__ == "__main__": | |
| asyncio.get_event_loop().run_until_complete(main()) | |
| async def main() -> None: | |
| dotenv.load_dotenv() | |
| configured_private_key = os.getenv("INJECTIVE_PRIVATE_KEY") | |
| # select network: local, testnet, mainnet | |
| network = Network.testnet() | |
| # initialize grpc client | |
| client = AsyncClient(network) | |
| # load account | |
| priv_key = PrivateKey.from_hex(configured_private_key) | |
| pub_key = priv_key.to_public_key() | |
| address = pub_key.to_address() | |
| await client.fetch_account(address.to_acc_bech32()) | |
| subaccount_id = address.get_subaccount_id(index=0) | |
| try: | |
| orders = await client.fetch_trader_derivative_conditional_orders( | |
| subaccount_id=subaccount_id, | |
| market_id="0x17ef48032cb24375ba7c2e39f384e56433bcab20cbee9a7357e4cba2eb00abe6", | |
| ) | |
| print(orders) | |
| except Exception as e: | |
| print(f"Failed to fetch trader derivative conditional orders: {e}") | |
| if __name__ == "__main__": | |
| asyncio.get_event_loop().run_until_complete(main()) |
Comment on lines
+46
to
+50
| quantity=Decimal(0.1), | ||
| margin=composer.calculate_margin( | ||
| quantity=Decimal(0.01), price=Decimal(50000), leverage=Decimal(3), is_reduce_only=False | ||
| quantity=Decimal(0.1), price=Decimal(50000), leverage=Decimal(1), is_reduce_only=False | ||
| ), | ||
| order_type="BUY", |
There was a problem hiding this comment.
The adjustments made to the main function in creating a derivative market order are clear and align with the described objectives. However, consider the following:
- Validation and Error Handling: Ensure that the values for
quantity,price, andleverageare validated before use to prevent invalid transactions. Additionally, consider adding error handling around the transaction simulation and broadcast steps to gracefully handle potential failures. - Parameterization: The
market_id,fee_recipient,price,quantity,leverage, andorder_typeare hardcoded. For enhanced usability, consider allowing these values to be passed as arguments or read from environment variables.
+ # Example validation (simplified)
+ if quantity <= 0 or price <= 0 or leverage <= 0:
+ raise ValueError("Quantity, price, and leverage must be greater than 0.")
+ try:
+ # Transaction simulation and broadcast code...
+ except Exception as e:
+ print(f"Failed to create derivative market order: {e}")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.
Suggested change
| quantity=Decimal(0.1), | |
| margin=composer.calculate_margin( | |
| quantity=Decimal(0.01), price=Decimal(50000), leverage=Decimal(3), is_reduce_only=False | |
| quantity=Decimal(0.1), price=Decimal(50000), leverage=Decimal(1), is_reduce_only=False | |
| ), | |
| order_type="BUY", | |
| quantity=Decimal(0.1), | |
| margin=composer.calculate_margin( | |
| quantity=Decimal(0.1), price=Decimal(50000), leverage=Decimal(1), is_reduce_only=False | |
| ), | |
| order_type="BUY", |
Comment on lines
+6
to
+105
| class TestBareMetalLoadBalancedCookieAssistant: | ||
| @pytest.mark.asyncio | ||
| async def test_chain_metadata(self): | ||
| assistant = BareMetalLoadBalancedCookieAssistant() | ||
| dummy_metadata = [("set-cookie", "expected_cookie")] | ||
|
|
||
| async def dummy_metadata_provider(): | ||
| return dummy_metadata | ||
|
|
||
| metadata = await assistant.chain_metadata(metadata_query_provider=dummy_metadata_provider) | ||
| expected_metadata = (("cookie", "expected_cookie"),) | ||
|
|
||
| assert expected_metadata == metadata | ||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_chain_metadata_fails_when_cookie_info_not_included_in_server_response(self): | ||
| assistant = BareMetalLoadBalancedCookieAssistant() | ||
| dummy_metadata = [("invalid_key", "invalid_value")] | ||
|
|
||
| async def dummy_metadata_provider(): | ||
| return dummy_metadata | ||
|
|
||
| with pytest.raises(RuntimeError, match=f"Error fetching chain cookie ({dummy_metadata})"): | ||
| await assistant.chain_metadata(metadata_query_provider=dummy_metadata_provider) | ||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_exchange_metadata(self): | ||
| assistant = BareMetalLoadBalancedCookieAssistant() | ||
| dummy_metadata = [("set-cookie", "expected_cookie")] | ||
|
|
||
| async def dummy_metadata_provider(): | ||
| return dummy_metadata | ||
|
|
||
| metadata = await assistant.exchange_metadata(metadata_query_provider=dummy_metadata_provider) | ||
| expected_metadata = (("cookie", "expected_cookie"),) | ||
|
|
||
| assert expected_metadata == metadata | ||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_exchange_metadata_is_none_when_cookie_info_not_included_in_server_response(self): | ||
| assistant = BareMetalLoadBalancedCookieAssistant() | ||
| dummy_metadata = [("invalid_key", "invalid_value")] | ||
|
|
||
| async def dummy_metadata_provider(): | ||
| return dummy_metadata | ||
|
|
||
| metadata = await assistant.exchange_metadata(metadata_query_provider=dummy_metadata_provider) | ||
|
|
||
| assert metadata is None | ||
|
|
||
|
|
||
| class TestKubernetesLoadBalancedCookieAssistant: | ||
| @pytest.mark.asyncio | ||
| async def test_chain_metadata(self): | ||
| assistant = KubernetesLoadBalancedCookieAssistant() | ||
| dummy_metadata = [("set-cookie", "expected_cookie")] | ||
|
|
||
| async def dummy_metadata_provider(): | ||
| return dummy_metadata | ||
|
|
||
| metadata = await assistant.chain_metadata(metadata_query_provider=dummy_metadata_provider) | ||
| expected_metadata = (("cookie", "expected_cookie"),) | ||
|
|
||
| assert expected_metadata == metadata | ||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_chain_metadata_fails_when_cookie_info_not_included_in_server_response(self): | ||
| assistant = KubernetesLoadBalancedCookieAssistant() | ||
| dummy_metadata = [("invalid_key", "invalid_value")] | ||
|
|
||
| async def dummy_metadata_provider(): | ||
| return dummy_metadata | ||
|
|
||
| with pytest.raises(RuntimeError, match=f"Error fetching chain cookie ({dummy_metadata})"): | ||
| await assistant.chain_metadata(metadata_query_provider=dummy_metadata_provider) | ||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_exchange_metadata(self): | ||
| assistant = KubernetesLoadBalancedCookieAssistant() | ||
| dummy_metadata = [("set-cookie", "expected_cookie")] | ||
|
|
||
| async def dummy_metadata_provider(): | ||
| return dummy_metadata | ||
|
|
||
| metadata = await assistant.exchange_metadata(metadata_query_provider=dummy_metadata_provider) | ||
| expected_metadata = (("cookie", "expected_cookie"),) | ||
|
|
||
| assert expected_metadata == metadata | ||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_exchange_metadata_is_none_when_cookie_info_not_included_in_server_response(self): | ||
| assistant = KubernetesLoadBalancedCookieAssistant() | ||
| dummy_metadata = [("invalid_key", "invalid_value")] | ||
|
|
||
| async def dummy_metadata_provider(): | ||
| return dummy_metadata | ||
|
|
||
| metadata = await assistant.exchange_metadata(metadata_query_provider=dummy_metadata_provider) | ||
|
|
||
| assert metadata is None |
There was a problem hiding this comment.
The tests for BareMetalLoadBalancedCookieAssistant and KubernetesLoadBalancedCookieAssistant are comprehensive and cover both successful scenarios and error handling for missing cookie information. A few suggestions for improvement:
- Mocking External Calls: Ensure that any external calls made during these tests (e.g., to a metadata provider) are properly mocked to avoid making actual network requests during test execution.
- Test Coverage: Consider adding tests for additional edge cases, such as handling malformed cookie values or network errors during metadata retrieval.
+ # Example of mocking an external call
+ @mock.patch('path.to.metadata_provider', return_value=dummy_metadata)
+ async def test_chain_metadata_with_mocked_provider(self, mock_metadata_provider):
+ # Test code...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.
Suggested change
| class TestBareMetalLoadBalancedCookieAssistant: | |
| @pytest.mark.asyncio | |
| async def test_chain_metadata(self): | |
| assistant = BareMetalLoadBalancedCookieAssistant() | |
| dummy_metadata = [("set-cookie", "expected_cookie")] | |
| async def dummy_metadata_provider(): | |
| return dummy_metadata | |
| metadata = await assistant.chain_metadata(metadata_query_provider=dummy_metadata_provider) | |
| expected_metadata = (("cookie", "expected_cookie"),) | |
| assert expected_metadata == metadata | |
| @pytest.mark.asyncio | |
| async def test_chain_metadata_fails_when_cookie_info_not_included_in_server_response(self): | |
| assistant = BareMetalLoadBalancedCookieAssistant() | |
| dummy_metadata = [("invalid_key", "invalid_value")] | |
| async def dummy_metadata_provider(): | |
| return dummy_metadata | |
| with pytest.raises(RuntimeError, match=f"Error fetching chain cookie ({dummy_metadata})"): | |
| await assistant.chain_metadata(metadata_query_provider=dummy_metadata_provider) | |
| @pytest.mark.asyncio | |
| async def test_exchange_metadata(self): | |
| assistant = BareMetalLoadBalancedCookieAssistant() | |
| dummy_metadata = [("set-cookie", "expected_cookie")] | |
| async def dummy_metadata_provider(): | |
| return dummy_metadata | |
| metadata = await assistant.exchange_metadata(metadata_query_provider=dummy_metadata_provider) | |
| expected_metadata = (("cookie", "expected_cookie"),) | |
| assert expected_metadata == metadata | |
| @pytest.mark.asyncio | |
| async def test_exchange_metadata_is_none_when_cookie_info_not_included_in_server_response(self): | |
| assistant = BareMetalLoadBalancedCookieAssistant() | |
| dummy_metadata = [("invalid_key", "invalid_value")] | |
| async def dummy_metadata_provider(): | |
| return dummy_metadata | |
| metadata = await assistant.exchange_metadata(metadata_query_provider=dummy_metadata_provider) | |
| assert metadata is None | |
| class TestKubernetesLoadBalancedCookieAssistant: | |
| @pytest.mark.asyncio | |
| async def test_chain_metadata(self): | |
| assistant = KubernetesLoadBalancedCookieAssistant() | |
| dummy_metadata = [("set-cookie", "expected_cookie")] | |
| async def dummy_metadata_provider(): | |
| return dummy_metadata | |
| metadata = await assistant.chain_metadata(metadata_query_provider=dummy_metadata_provider) | |
| expected_metadata = (("cookie", "expected_cookie"),) | |
| assert expected_metadata == metadata | |
| @pytest.mark.asyncio | |
| async def test_chain_metadata_fails_when_cookie_info_not_included_in_server_response(self): | |
| assistant = KubernetesLoadBalancedCookieAssistant() | |
| dummy_metadata = [("invalid_key", "invalid_value")] | |
| async def dummy_metadata_provider(): | |
| return dummy_metadata | |
| with pytest.raises(RuntimeError, match=f"Error fetching chain cookie ({dummy_metadata})"): | |
| await assistant.chain_metadata(metadata_query_provider=dummy_metadata_provider) | |
| @pytest.mark.asyncio | |
| async def test_exchange_metadata(self): | |
| assistant = KubernetesLoadBalancedCookieAssistant() | |
| dummy_metadata = [("set-cookie", "expected_cookie")] | |
| async def dummy_metadata_provider(): | |
| return dummy_metadata | |
| metadata = await assistant.exchange_metadata(metadata_query_provider=dummy_metadata_provider) | |
| expected_metadata = (("cookie", "expected_cookie"),) | |
| assert expected_metadata == metadata | |
| @pytest.mark.asyncio | |
| async def test_exchange_metadata_is_none_when_cookie_info_not_included_in_server_response(self): | |
| assistant = KubernetesLoadBalancedCookieAssistant() | |
| dummy_metadata = [("invalid_key", "invalid_value")] | |
| async def dummy_metadata_provider(): | |
| return dummy_metadata | |
| metadata = await assistant.exchange_metadata(metadata_query_provider=dummy_metadata_provider) | |
| assert metadata is None | |
| # Example of mocking an external call | |
| @mock.patch('path.to.metadata_provider', return_value=dummy_metadata) | |
| async def test_chain_metadata_with_mocked_provider(self, mock_metadata_provider): | |
| # Test code... |
Comment on lines
+7
to
+20
| async def main() -> None: | ||
| # select network: local, testnet, mainnet | ||
| network = Network.testnet() | ||
|
|
||
| # initialize grpc client | ||
| client = AsyncClient(network) | ||
|
|
||
| records = await client.fetch_historical_trade_records( | ||
| market_id="0x0611780ba69656949525013d947713300f56c37b6175e02f26bffa495c3208fe" | ||
| ) | ||
| print(records) | ||
|
|
||
|
|
||
| if __name__ == "__main__": |
There was a problem hiding this comment.
This script, showcasing the fetch_historical_trade_records method, could be improved by:
- Adding error handling to manage exceptions from the async call effectively.
- Improving the output formatting to aid users in understanding the returned data, particularly for complex data structures.
- records = await client.fetch_historical_trade_records(
+ try:
+ records = await client.fetch_historical_trade_records(
+ market_id="0x0611780ba69656949525013d947713300f56c37b6175e02f26bffa495c3208fe"
+ )
+ # Recommend enhancing output formatting for clarity
+ print(records)
+ except Exception as e:
+ print(f"Failed to fetch Historical Trade Records: {e}")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.
Suggested change
| async def main() -> None: | |
| # select network: local, testnet, mainnet | |
| network = Network.testnet() | |
| # initialize grpc client | |
| client = AsyncClient(network) | |
| records = await client.fetch_historical_trade_records( | |
| market_id="0x0611780ba69656949525013d947713300f56c37b6175e02f26bffa495c3208fe" | |
| ) | |
| print(records) | |
| if __name__ == "__main__": | |
| async def main() -> None: | |
| # select network: local, testnet, mainnet | |
| network = Network.testnet() | |
| # initialize grpc client | |
| client = AsyncClient(network) | |
| try: | |
| records = await client.fetch_historical_trade_records( | |
| market_id="0x0611780ba69656949525013d947713300f56c37b6175e02f26bffa495c3208fe" | |
| ) | |
| # Recommend enhancing output formatting for clarity | |
| print(records) | |
| except Exception as e: | |
| print(f"Failed to fetch Historical Trade Records: {e}") | |
| if __name__ == "__main__": |
Comment on lines
+7
to
+20
| async def main() -> None: | ||
| # select network: local, testnet, mainnet | ||
| network = Network.testnet() | ||
|
|
||
| # initialize grpc client | ||
| client = AsyncClient(network) | ||
|
|
||
| multiplier = await client.fetch_market_atomic_execution_fee_multiplier( | ||
| market_id="0x17ef48032cb24375ba7c2e39f384e56433bcab20cbee9a7357e4cba2eb00abe6", | ||
| ) | ||
| print(multiplier) | ||
|
|
||
|
|
||
| if __name__ == "__main__": |
There was a problem hiding this comment.
In this final example, which introduces the fetch_market_atomic_execution_fee_multiplier method, consider:
- Implementing error handling to ensure the script can handle exceptions from the async call.
- Enhancing the output formatting to make the complex data structures more understandable for users.
- multiplier = await client.fetch_market_atomic_execution_fee_multiplier(
+ try:
+ multiplier = await client.fetch_market_atomic_execution_fee_multiplier(
+ market_id="0x17ef48032cb24375ba7c2e39f384e56433bcab20cbee9a7357e4cba2eb00abe6",
+ )
+ # Suggest improving output formatting for data clarity
+ print(multiplier)
+ except Exception as e:
+ print(f"Failed to fetch Market Atomic Execution Fee Multiplier: {e}")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.
Suggested change
| async def main() -> None: | |
| # select network: local, testnet, mainnet | |
| network = Network.testnet() | |
| # initialize grpc client | |
| client = AsyncClient(network) | |
| multiplier = await client.fetch_market_atomic_execution_fee_multiplier( | |
| market_id="0x17ef48032cb24375ba7c2e39f384e56433bcab20cbee9a7357e4cba2eb00abe6", | |
| ) | |
| print(multiplier) | |
| if __name__ == "__main__": | |
| async def main() -> None: | |
| # select network: local, testnet, mainnet | |
| network = Network.testnet() | |
| # initialize grpc client | |
| client = AsyncClient(network) | |
| try: | |
| multiplier = await client.fetch_market_atomic_execution_fee_multiplier( | |
| market_id="0x17ef48032cb24375ba7c2e39f384e56433bcab20cbee9a7357e4cba2eb00abe6", | |
| ) | |
| # Suggest improving output formatting for data clarity | |
| print(multiplier) | |
| except Exception as e: | |
| print(f"Failed to fetch Market Atomic Execution Fee Multiplier: {e}") | |
| if __name__ == "__main__": |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
devbranch with the latest changes inmasterSummary by CodeRabbit