Express relay python searcher #1319

anihamde · 2024-02-22T18:35:14Z

Reorg Js searcher sdk
Drop python searcher sdk (w/o websocket; to be added)

vercel · 2024-02-22T18:35:20Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Ignored Deployments

Name	Status	Preview	Comments	Updated (UTC)
example-oracle-amm	⬜️ Ignored (Inspect)	Visit Preview		Mar 15, 2024 7:03pm
xc-admin-frontend	⬜️ Ignored (Inspect)	Visit Preview		Mar 15, 2024 7:03pm

express_relay/sdk/python/searcher/searcher_utils.py

express_relay/sdk/python/searcher/pyproject.toml

m30m · 2024-02-26T18:28:26Z

express_relay/sdk/python/searcher/searcher_utils.py

+        self.ws_msg_counter = 0
+
+    async def get_liquidation_opportunities(self) -> list[OpportunityParamsWithMetadata]:
+        async with httpx.AsyncClient() as client:


initialize the client in the construction and use it here. Also it would be great if you can accept some config to pass into this client. For example for tuning the timeout

This can lead to some resource leakage, better to instantiate the client in the context so that it shuts down upon the end of this function call.

express_relay/sdk/python/searcher/searcher_utils.py

express_relay/sdk/python/searcher/examples/simple_searcher.py

express_relay/sdk/python/searcher/searcher_utils.py

express_relay/sdk/python/searcher/examples/simple_searcher.py

express_relay/sdk/python/searcher/README.md

express_relay/sdk/python/express_relay_utils/express_relay_client.py

m30m · 2024-03-05T13:35:33Z

express_relay/sdk/python/express_relay_utils/express_relay_client.py

+        msg["id"] = str(self.ws_msg_counter)
+        self.ws_msg_counter += 1
+
+        await self.ws.send(json.dumps(msg))


we are not waiting for the response of this message. Server can return an error. This should be handled using Futures

express_relay/sdk/python/express_relay_utils/express_relay_client.py

m30m · 2024-03-13T14:31:48Z

.pre-commit-config.yaml

@@ -4,6 +4,7 @@ repos:
    hooks:
      - id: trailing-whitespace
      - id: end-of-file-fixer
+        exclude: openapi_client


this is not needed anymore

m30m · 2024-03-13T14:32:24Z

.pre-commit-config.yaml

+        name: black
+        entry: poetry -C express_relay/sdk/python/express_relay_utils run black express_relay/sdk/python/express_relay_utils
+        pass_filenames: false
+        language: system


what does this mean?

m30m · 2024-03-13T14:34:04Z

express_relay/sdk/python/README.md

+$ poetry add express-relay-utils
+```
+
+### openapi-generator


m30m · 2024-03-13T14:39:37Z

express_relay/sdk/python/express_relay_utils/express_relay_types.py

+    assert s_set.issubset(hex_set), "v must be a valid hex string"
+
+
+class HexString(BaseModel):


I think this is not a natural way to access these values.
For example I expect bid.calldata to be the actual calldata. But now I have to access it via bid.calldata.string which is awkward. Why not use something like HexBytes?
We can also leverage eth_typing library for these low level types.

m30m · 2024-03-13T14:45:14Z

express_relay/sdk/python/express_relay_utils/express_relay_types.py

+
+class Bid(BaseModel):
+    """
+    Args:


these are Attributes now

m30m · 2024-03-13T15:00:56Z

express_relay/sdk/python/express_relay_utils/express_relay_types.py

+    contract: Address
+    permission_key: HexString
+
+    def to_dict(self):


There should be a better way to do this only by customizing the amount field serialization instead of specifying everything.

In general defining to_dict and from_dict seems wrong as we are defining additional methods for serialization instead of overriding the default behavior that pydantic provides using model_dump/model_validate.

m30m · 2024-03-14T20:59:49Z

express_relay/sdk/python/express_relay_utils/express_relay_types.py

+from eth_account.datastructures import SignedMessage
+
+
+def maybe_hex_string(s: str):


maybe is not a good prefix here. maybe check or validate?

m30m · 2024-03-14T21:07:41Z

express_relay/sdk/python/express_relay_utils/express_relay_client.py

+                        opportunity = Opportunity.model_validate(
+                            msg_json.get("opportunity")
+                        )
+                        if opportunity:


I don't think opportunity can be None here. model_validate throws an exception if invalid

m30m · 2024-03-14T21:09:05Z

express_relay/sdk/python/express_relay_utils/express_relay_types.py

+            warnings.warn(
+                f"Cannot handle opportunity version: {self.version}. Please upgrade your client."
+            )
+            return None


according to the docs, we should either return the instance or throw an exception inside these validators. So maybe this is not the best place to handle this case. I don't know what the behavior is if we return None

yes but i think it's cleaner to handle the verification within this type. i'll add some more customized checking to throw an error but catch this while processing.

m30m · 2024-03-14T21:17:58Z

express_relay/sdk/python/express_relay_utils/express_relay_client.py

+        Args:
+            opportunity_bid: An object representing the bid to submit on an opportunity.
+            subscribe_to_updates: A boolean indicating whether to subscribe to the bid status updates.
+            kwargs: Keyword arguments to pass to the HTTP post request.


this parameter is now a bit ill defined. If subscribe_to_updates is True, this is useless, but we don't communicate that with the users. Anyway, I think we should reuse a single httpx client and allow the user to pass in any extra configuration required in the constructor.

m30m · 2024-03-14T21:19:17Z

express_relay/sdk/python/express_relay_utils/express_relay_client.py

+            )
+            bid_id = UUID(result.get("id"))
+        else:
+            async with httpx.AsyncClient() as client:


I searched a bit and defining a new client per method call doesn't seem like a good idea:
https://www.python-httpx.org/advanced/clients/
I suggest creating a single client in the constructor and use it here. Also we can accept the httpx client configuration in the constructor and set it once and for all the methods.

the same link advises using context manager for resource management reasons. but i agree that it is better to pass it in once in the constructor, refactoring to do that

This is because the link does not assume we are doing this in multiple functions. Each time the user call this functions, a new client is instantiated which is suboptimal:
https://www.python-httpx.org/async/#opening-and-closing-clients

m30m · 2024-03-14T21:22:12Z

.pre-commit-config.yaml

+        name: pyflakes
+        entry: poetry -C express_relay/sdk/python/express_relay_utils run pyflakes express_relay/sdk/python/express_relay_utils
+        pass_filenames: false
+        language: "system"


shouldn't these be python?

this hook directly invokes poetry, not python, see https://sam.hooke.me/note/2023/09/poetry-pre-commit-hooks/

m30m · 2024-03-14T21:22:36Z

express_relay/sdk/python/README.md

@@ -0,0 +1,25 @@
+# Pyth Express Relay Python SDK


Remove Pyth

m30m · 2024-03-14T21:23:48Z

express_relay/sdk/python/pyproject.toml

+[tool.poetry]
+name = "express-relay-utils"
+version = "0.0.1"
+description = "Utilities for searchers and protocols to interact with the express relay protocol."


Express Relay

m30m · 2024-03-14T21:25:01Z

express_relay/sdk/python/express_relay_utils/express_relay_client.py

+
+        return self.ws_loop
+
+    async def send_ws_message(self, method: str, params: dict) -> dict:


I think it makes more sense for this function to accept a ClientMessage and the callers be responsible to craft the correct payloads

yeah i wanted to avoid multiple duplicate calls to ClientMessage.model_validate throughout the code. but i dont have a strong take on this, so can refactor it

m30m · 2024-03-14T21:27:22Z

express_relay/sdk/python/express_relay_utils/express_relay_client.py

+        resp.raise_for_status()
+
+        opportunities = [
+            Opportunity.model_validate(opportunity) for opportunity in resp.json()


I think here is a better place to check the version before passing it to model_validate. Or we can catch the incompatible version errors here

m30m · 2024-03-15T07:18:44Z

express_relay/sdk/python/express_relay_utils/express_relay_client.py

@@ -37,14 +37,16 @@ def __init__(
        bid_status_callback: (
            Callable[[BidStatusUpdate], Coroutine[Any, Any, Any]] | None
        ) = None,
-        **kwargs,
+        ws_options: dict[str, Any] = {},
+        httpx_options: dict[str, Any] = {},


default argument value is mutable

m30m · 2024-03-15T07:21:20Z

express_relay/sdk/python/express_relay_utils/express_relay_client.py

-
-        # validate the format of msg and construct the message dict to send
-        client_msg_validated = ClientMessage.model_validate({"params": params})
-        msg = client_msg_validated.convert_to_server()


if we always need to add id to the output of convert_to_server. Let's add it as a parameter for the convert_to_server function. Therefore, we eliminate the chance of forgetting to add it.

m30m · 2024-03-15T07:28:30Z

express_relay/sdk/python/express_relay_utils/express_relay_client.py

+        await self.ws.send(json.dumps(msg))
+
+        # await the response for the sent ws message from the server
+        msg = await future


let's add some timeout there too. we don't want to wait indefinitely if there is a problem

m30m · 2024-03-15T07:31:57Z

express_relay/sdk/python/express_relay_utils/express_relay_client.py

+                future.set_result(msg_json)
+
+    async def get_opportunities(
+        self, chain_id: str | None = None, timeout_secs: int = 10


let's use self.httpx_options instead of this timeout parameter

m30m · 2024-03-15T07:32:01Z

express_relay/sdk/python/express_relay_utils/express_relay_client.py

+        return opportunities
+
+    async def submit_opportunity(
+        self, opportunity: OpportunityParams, timeout_secs: int = 10


let's use self.httpx_options instead of this timeout parameter

m30m · 2024-03-15T07:34:51Z

express_relay/sdk/python/express_relay_utils/express_relay_client.py

+        """
+        Returns the websocket handler loop.
+        """
+        await self.start_ws()


I don't think this function needs to start the websocket on its own. Maybe if we haven't started the ws yet, it's better to return None here

m30m · 2024-03-15T10:49:27Z

express_relay/sdk/python/express_relay_utils/express_relay_client.py

+        resp.raise_for_status()
+
+        opportunities = [
+            Opportunity.process_opportunity_dict(opportunity)


this list should not include None values

m30m · 2024-03-15T14:13:04Z

.github/workflows/pypi-express-relay-utils.yml

+on:
+  push:
+    tags:
+      - "v*"


let's make this a bit more specific. maybe python-v*

m30m · 2024-03-15T14:17:05Z

express_relay/sdk/python/pyproject.toml

@@ -0,0 +1,27 @@
+[tool.poetry]
+name = "express-relay-utils"
+version = "0.0.1"


let's make this 0.2.0 to match the server version

m30m · 2024-03-15T14:23:09Z

express_relay/sdk/python/mypy.ini

+check_untyped_defs = True
+no_implicit_reexport = True
+
+; # for strict mypy: (this is the tricky one :-))


I don't think we will enable this anytime soon let's just remove it

m30m · 2024-03-15T14:24:49Z

express_relay/sdk/python/pyproject.toml

@@ -0,0 +1,27 @@
+[tool.poetry]
+name = "express-relay-utils"


let's remove utils from here and the folder structure

m30m

Please fix this and the previous comments remaining before merge

m30m · 2024-03-15T16:15:09Z

express_relay/sdk/python/express_relay_utils/express_relay_types.py

+        "Cannot handle opportunity version: {%s}. Please upgrade your client."
+    )
+
+    @model_validator(mode="after")


I think we should do this before the validation. If the new schema doesn't have some of these fields we will receive another exception which we will not handle

m30m · 2024-03-15T16:15:49Z

express_relay/sdk/python/express_relay_utils/express_relay_types.py

+        """
+        try:
+            return cls.model_validate(opportunity_dict)
+        except Exception as e:


instead of catching a general exception we can throw something custom like UnsupportedOpportunity() and just catch that here.

ani added 3 commits February 22, 2024 13:14

move js searcher sdk

f4f2ba1

add the python searcher sdk

474501d

remove pycache

574b8ff

ani added 5 commits February 22, 2024 14:17

create class for simple searcher

64b7142

add websocket to python searcher sdk

244d8e0

finish ws, avoid storing liquidation opportunities within client

8dd3e74

python scripts now working w auto type generation

fcaff73

minor precommit changes

6cab8b5

m30m reviewed Feb 26, 2024

View reviewed changes

ani and others added 11 commits February 27, 2024 00:49

address comments 1

c14569e

add openapi type generations

2b819bb

fixed precommit issues on generated type files

7ad0165

reorg

069398f

fixed openapi_client generated precommit

e2acc99

fix js filepath issue

616ff4a

added close ws

26bc8d2

renamings and add send_ws_message method

74b3200

get rid of duplicate error parsing

206f481

cleanup

3e66a84

set up pypi workflow

35abeca

m30m reviewed Mar 5, 2024

View reviewed changes

ani added 8 commits March 5, 2024 22:18

address comments

ed32ff8

add python precommit

e9e6e50

changes + precommit changes

e64296c

test precommit change

a3a5961

test precommit change

ef8a447

test precommit change

0252a7d

test precommit change

458f483

test precommit change

b0c086d

ani added 11 commits March 7, 2024 20:31

more changes

342196e

precommit

2888be4

remove extraneous files, stick w actual_instance

6b5555f

added back http as nondefault

968f874

correction

8afe3e7

some cleanup and reorg

3e55b2f

minor changes

fbc5e6b

add back api response typing file

05f426e

minorer changes

c85480b

exclude openapi_client from end of file fixer

3af2730

build internal models via pydantic

946ff3a

m30m reviewed Mar 13, 2024

View reviewed changes

ani added 5 commits March 13, 2024 11:58

chgs

0f90814

start to reorg classes

c40c824

configure precommit to work

3423ee7

some cleanup

559533e

reorg a bit

c586c60

m30m reviewed Mar 14, 2024

View reviewed changes

address comments

5997608

m30m reviewed Mar 15, 2024

View reviewed changes

ani added 2 commits March 15, 2024 11:14

chgs

9b0f395

fgt

dce3eee

m30m approved these changes Mar 15, 2024

View reviewed changes

ani added 2 commits March 15, 2024 14:30

morechgs

b3d35b6

merge conflix

81e5956

vercel bot deployed to Preview – xc-admin-frontend March 15, 2024 18:34 View deployment

some more chgs

6090e3b

anihamde merged commit 9256673 into main Mar 15, 2024
5 checks passed

anihamde deleted the express-relay-python-searcher branch March 15, 2024 19:08

		assert s_set.issubset(hex_set), "v must be a valid hex string"


		class HexString(BaseModel):

		from eth_account.datastructures import SignedMessage


		def maybe_hex_string(s: str):


		return self.ws_loop

		async def send_ws_message(self, method: str, params: dict) -> dict:

Express relay python searcher #1319

Express relay python searcher #1319

Conversation

anihamde commented Feb 22, 2024

vercel bot commented Feb 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

m30m left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vercel bot commented Feb 22, 2024 •

edited

Loading