Post order using cow-py #580
Conversation
WalkthroughThe pull request introduces enhancements to the prediction market agent tooling by adding functionality for token swapping on the Gnosis chain using the CoW Swap protocol. The changes include extending the Changes
Sequence DiagramsequenceDiagram
participant User
participant SwapFunction
participant OrderBookAPI
participant CoWVaultRelayer
participant EthereumChain
User->>SwapFunction: Initiate token swap
SwapFunction->>OrderBookAPI: Get order quote
OrderBookAPI-->>SwapFunction: Return quote details
SwapFunction->>CoWVaultRelayer: Approve sell token
SwapFunction->>SwapFunction: Sign order
SwapFunction->>OrderBookAPI: Post order
OrderBookAPI-->>SwapFunction: Return order UID
SwapFunction-->>User: Return completed order details
Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
prediction_market_agent_tooling/tools/cow_order.py (1)
107-107: Verify the signature before submissionAfter signing the order, consider verifying the signature to ensure it's correct before posting the order. This can prevent issues related to invalid signatures.
tests_integration_with_local_chain/tools/test_cow_order.py (1)
13-25: Enhance test reliability by mocking external dependenciesThe test currently expects an exception due to insufficient balance, which depends on external conditions. Consider mocking the external API calls or setting up a controlled test environment to make the test deterministic and reliable.
prediction_market_agent_tooling/config.py (1)
189-194: Cache theLocalAccountinstanceRepeatedly creating a
LocalAccountcan be inefficient. Cache the account instance after the first creation to improve performance.Apply this diff to cache the account:
def get_account(self) -> LocalAccount: if not hasattr(self, "_local_account"): self._local_account = Account.from_key( self.bet_from_private_key.get_secret_value() ) return self._local_account
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
prediction_market_agent_tooling/config.py(2 hunks)prediction_market_agent_tooling/tools/cow_order.py(1 hunks)tests_integration_with_local_chain/tools/test_cow_order.py(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: pytest - Python 3.12.x - Unit Tests
- GitHub Check: pytest - Python 3.11.x - Unit Tests
- GitHub Check: pytest - Python 3.10.x - Integration with Local Chain
- GitHub Check: pytest - Python 3.10.x - Integration Tests
- GitHub Check: pytest - Python 3.10.x - Unit Tests
- GitHub Check: mypy
🔇 Additional comments (3)
prediction_market_agent_tooling/tools/cow_order.py (3)
66-66: Handle potential exceptions when retrieving the accountThe
get_account()method may raise an exception if the private key is missing or invalid. Consider adding error handling to manage this scenario gracefully.
80-86: Validate token addresses before useEnsure that
sell_tokenandbuy_tokenare valid and checksummed addresses before proceeding. This helps prevent errors due to incorrect address formats.
130-152: Ensure all required order fields are accurateWhen creating
OrderCreation, double-check that all fields are correctly populated and formatted according to the API requirements. Missing or incorrect fields may lead to order rejection.
| ContractERC20OnGnosisChain(address=sell_token).approve( | ||
| api_keys, | ||
| Web3.to_checksum_address(CowContractAddress.VAULT_RELAYER.value), | ||
| amount_wei=amount_wei, | ||
| web3=web3, | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for token approval
The approve function may fail due to various reasons (e.g., network issues, insufficient funds). It's recommended to handle exceptions to ensure the approval process is robust.
Apply this diff to add error handling:
try:
ContractERC20OnGnosisChain(address=sell_token).approve(
api_keys,
Web3.to_checksum_address(CowContractAddress.VAULT_RELAYER.value),
amount_wei=amount_wei,
web3=web3,
)
except Exception as e:
# Handle exception appropriately
raise RuntimeError(f"Token approval failed: {e}")| return asyncio.run( | ||
| swap_tokens_async( | ||
| amount, sell_token, buy_token, api_keys, chain, app_data, env, web3 | ||
| ) | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid using asyncio.run() in libraries
Using asyncio.run() within library code can cause issues if the caller is already running an event loop (e.g., in an asynchronous context). Consider refactoring swap_tokens to be asynchronous or providing separate synchronous and asynchronous interfaces.
Apply this diff to refactor the function:
-def swap_tokens(
+async def swap_tokens(
amount: xDai,
sell_token: ChecksumAddress,
buy_token: ChecksumAddress,
api_keys: APIKeys,
chain: Chain = Chain.GNOSIS,
app_data: str = ZERO_APP_DATA,
env: Envs = "prod",
web3: Web3 | None = None,
) -> CompletedOrder:
- # CoW library uses async, so we need to wrap the call in asyncio.run for us to use it.
- return asyncio.run(
- swap_tokens_async(
- amount, sell_token, buy_token, api_keys, chain, app_data, env, web3
- )
- )
+ return await swap_tokens_async(
+ amount, sell_token, buy_token, api_keys, chain, app_data, env, web3
+ )📝 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.
| return asyncio.run( | |
| swap_tokens_async( | |
| amount, sell_token, buy_token, api_keys, chain, app_data, env, web3 | |
| ) | |
| ) | |
| async def swap_tokens( | |
| amount: xDai, | |
| sell_token: ChecksumAddress, | |
| buy_token: ChecksumAddress, | |
| api_keys: APIKeys, | |
| chain: Chain = Chain.GNOSIS, | |
| app_data: str = ZERO_APP_DATA, | |
| env: Envs = "prod", | |
| web3: Web3 | None = None, | |
| ) -> CompletedOrder: | |
| return await swap_tokens_async( | |
| amount, sell_token, buy_token, api_keys, chain, app_data, env, web3 | |
| ) |
| @@ -184,6 +186,12 @@ def sqlalchemy_db_url(self) -> SecretStr: | |||
| self.SQLALCHEMY_DB_URL, "SQLALCHEMY_DB_URL missing in the environment." | |||
| ) | |||
|
|
|||
| def get_account(self) -> LocalAccount: | |||
| acc: LocalAccount = Account.from_key( | |||
| self.bet_from_private_key.get_secret_value() | |||
There was a problem hiding this comment.
Secure handling of private keys
Ensure that the private key is never logged or exposed in error messages. Consider adding validation to check if the private key is in the correct format and handle any exceptions securely.
This is basically cow-py's examples with some small modifications.
Before merging cow-py needs to be released on PyPi (atm I just installed it using
piplocally to develop this).