Skip to content
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

Post order using cow-py #580

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
151 changes: 150 additions & 1 deletion poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 8 additions & 0 deletions prediction_market_agent_tooling/config.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
import typing as t

from eth_account.signers.local import LocalAccount
from pydantic import Field
from pydantic.types import SecretStr
from pydantic.v1.types import SecretStr as SecretStrV1
from pydantic_settings import BaseSettings, SettingsConfigDict
from safe_eth.eth import EthereumClient
from safe_eth.safe.safe import SafeV141
from web3 import Account

from prediction_market_agent_tooling.gtypes import (
ChainID,
Expand Down Expand Up @@ -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()
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

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.

)
return acc

def model_dump_public(self) -> dict[str, t.Any]:
return {
k: v
Expand Down
84 changes: 84 additions & 0 deletions prediction_market_agent_tooling/tools/cow/cow_order.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
import asyncio
from datetime import timedelta

import httpx
from cow_py import swap_tokens
from cow_py.common.chains import Chain
from cow_py.common.constants import CowContractAddress
from cow_py.order_book.config import Envs
from cow_py.order_book.generated.model import OrderMetaData, OrderStatus
from eth_account.signers.local import LocalAccount
from web3 import Web3

from prediction_market_agent_tooling.config import APIKeys
from prediction_market_agent_tooling.gtypes import ChecksumAddress, Wei, xDai
from prediction_market_agent_tooling.loggers import logger
from prediction_market_agent_tooling.tools.contract import ContractERC20OnGnosisChain
from prediction_market_agent_tooling.tools.utils import utcnow
from prediction_market_agent_tooling.tools.web3_utils import xdai_to_wei


def swap_tokens_waiting(
amount: xDai,
sell_token: ChecksumAddress,
buy_token: ChecksumAddress,
api_keys: APIKeys,
chain: Chain = Chain.GNOSIS,
env: Envs = "prod",
web3: Web3 | None = None,
) -> OrderMetaData:
amount_wei = xdai_to_wei(amount)
account = api_keys.get_account()

# Approve the CoW Swap Vault Relayer to get the sell token.
ContractERC20OnGnosisChain(address=sell_token).approve(
api_keys,
Web3.to_checksum_address(CowContractAddress.VAULT_RELAYER.value),
amount_wei=amount_wei,
web3=web3,
)

# CoW library uses async, so we need to wrap the call in asyncio.run for us to use it.
return asyncio.run(
swap_tokens_waiting_async(
amount_wei, sell_token, buy_token, account, chain, env
)
)
Comment on lines +41 to +46
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Avoid using asyncio.run() inside library code to prevent event loop conflicts

Using asyncio.run() within a library function like swap_tokens_waiting can lead to issues if the caller is already running an event loop (common in asynchronous applications). This can cause a RuntimeError due to the event loop being already running. Instead, consider making swap_tokens_waiting an asynchronous function or provide both synchronous and asynchronous interfaces.

Apply this diff to refactor the function:

-def swap_tokens_waiting(
+async def swap_tokens_waiting(
     amount: xDai,
     sell_token: ChecksumAddress,
     buy_token: ChecksumAddress,
     api_keys: APIKeys,
     chain: Chain = Chain.GNOSIS,
     env: Envs = "prod",
     web3: Web3 | None = None,
 ) -> OrderMetaData:
     amount_wei = xdai_to_wei(amount)
     account = api_keys.get_account()

     # Approve the CoW Swap Vault Relayer to get the sell token.
     ContractERC20OnGnosisChain(address=sell_token).approve(
         api_keys,
         Web3.to_checksum_address(CowContractAddress.VAULT_RELAYER.value),
         amount_wei=amount_wei,
         web3=web3,
     )

     # CoW library uses async, so we need to call the async function directly.
-    return asyncio.run(
-        swap_tokens_waiting_async(
-            amount_wei, sell_token, buy_token, account, chain, env
-        )
-    )
+    return await swap_tokens_waiting_async(
+        amount_wei, sell_token, buy_token, account, chain, env
+    )
📝 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.

Suggested change
# CoW library uses async, so we need to wrap the call in asyncio.run for us to use it.
return asyncio.run(
swap_tokens_waiting_async(
amount_wei, sell_token, buy_token, account, chain, env
)
)
async def swap_tokens_waiting(
amount: xDai,
sell_token: ChecksumAddress,
buy_token: ChecksumAddress,
api_keys: APIKeys,
chain: Chain = Chain.GNOSIS,
env: Envs = "prod",
web3: Web3 | None = None,
) -> OrderMetaData:
amount_wei = xdai_to_wei(amount)
account = api_keys.get_account()
# Approve the CoW Swap Vault Relayer to get the sell token.
ContractERC20OnGnosisChain(address=sell_token).approve(
api_keys,
Web3.to_checksum_address(CowContractAddress.VAULT_RELAYER.value),
amount_wei=amount_wei,
web3=web3,
)
# CoW library uses async, so we need to call the async function directly.
return await swap_tokens_waiting_async(
amount_wei, sell_token, buy_token, account, chain, env
)



async def swap_tokens_waiting_async(
amount_wei: Wei,
sell_token: ChecksumAddress,
buy_token: ChecksumAddress,
account: LocalAccount,
chain: Chain,
env: Envs,
timeout: timedelta = timedelta(seconds=60),
) -> OrderMetaData:
order = await swap_tokens(
amount=amount_wei,
sell_token=sell_token,
buy_token=buy_token,
account=account,
chain=chain,
env=env,
)
logger.info(f"Order created: {order}")
start_time = utcnow()

while True:
async with httpx.AsyncClient() as client:
response = await client.get(order.url)
order_metadata = OrderMetaData.model_validate(response.json())

Comment on lines +70 to +73
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add exception handling for HTTP request failures

Currently, the HTTP GET request to order.url does not handle potential exceptions. Network errors like timeouts or connection issues could raise unhandled exceptions, causing the application to crash. It's important to handle httpx.RequestError to maintain application stability.

Apply this diff to add exception handling:

         while True:
             async with httpx.AsyncClient() as client:
+                try:
                     response = await client.get(order.url)
                     order_metadata = OrderMetaData.model_validate(response.json())
+                except httpx.RequestError as e:
+                    # Handle the exception (e.g., log the error or retry)
+                    print(f"Network error occurred: {e}")
+                    await asyncio.sleep(3.14)
+                    continue

Committable suggestion skipped: line range outside the PR's diff.

if order_metadata.status in (
OrderStatus.fulfilled,
OrderStatus.cancelled,
OrderStatus.expired,
):
return order_metadata

if utcnow() - start_time > timeout:
raise TimeoutError("Timeout waiting for order to be completed.")

await asyncio.sleep(3.14)
Comment on lines +81 to +84
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would log something so that the user knows it's in process.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes agreed, I added that already in the follow up PR that's ready for review as well: #585

2 changes: 2 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ pinatapy-vourhey = "^0.2.0"
hishel = "^0.0.31"
pytest-postgresql = "^6.1.1"
optuna = { version = "^4.1.0", optional = true}
httpx = "^0.25.2"
cow-py = {git = "https://github.com/kongzii/cow-py.git", branch = "peter/slippage_tolerance"}

[tool.poetry.extras]
openai = ["openai"]
Expand Down
25 changes: 25 additions & 0 deletions tests_integration_with_local_chain/tools/test_cow_order.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import pytest
from web3 import Web3

from prediction_market_agent_tooling.config import APIKeys
from prediction_market_agent_tooling.gtypes import xdai_type
from prediction_market_agent_tooling.markets.omen.omen_contracts import (
WrappedxDaiContract,
sDaiContract,
)
from prediction_market_agent_tooling.tools.cow.cow_order import swap_tokens_waiting


def test_swap_tokens_waiting(local_web3: Web3, test_keys: APIKeys) -> None:
with pytest.raises(Exception) as e:
swap_tokens_waiting(
amount=xdai_type(0.1),
sell_token=WrappedxDaiContract().address,
buy_token=sDaiContract().address,
api_keys=test_keys,
env="staging",
web3=local_web3,
)
# This is raised in `post_order` which is last call when swapping tokens, anvil's accounts don't have any balance on real chain, so this is expected,
# but still, it tests that all the logic behind calling CoW APIs is working correctly.
assert "InsufficientBalance" in str(e)
Comment on lines +23 to +25
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

Loading