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.

Sign up for GitHub

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

Fix challenging agents (replicator, challenger) #510

Merged
merged 4 commits into from
Oct 11, 2024
Merged

Fix challenging agents (replicator, challenger) #510

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
37b6125
Fix challenging agents (replicator, challenger)
kongzii Oct 10, 2024
2ae9642
Merge branch 'main' into peter/fixesreal
kongzii Oct 11, 2024
e0105b1
update lock
kongzii Oct 11, 2024
38a12f8
fix after merge
kongzii Oct 11, 2024
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
696 changes: 356 additions & 340 deletions poetry.lock
View file
Open in desktop

Large diffs are not rendered by default.

14 changes: 11 additions & 3 deletions prediction_market_agent/agents/ofvchallenger_agent/deploy.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
from datetime import timedelta
from functools import partial

from prediction_market_agent_tooling.deploy.agent import DeployableAgent
from prediction_market_agent_tooling.gtypes import ChecksumAddress, xdai_type
Expand Down Expand Up @@ -68,14 +69,21 @@ def challenge(self, api_keys: APIKeys) -> None:
# Claim the bonds as first thing, to have funds for the new challenges.
claim_all_bonds_on_reality(api_keys)

markets_open_for_answers = OmenSubgraphHandler().get_omen_binary_markets(
get_omen_binary_markets_common_filters = partial(
OmenSubgraphHandler().get_omen_binary_markets,
limit=None,
creator_in=MARKET_CREATORS_TO_CHALLENGE,
# We need markets already opened for answers.
opened_before=utcnow(),
question_opened_before=utcnow(),
)
markets_open_for_answers = get_omen_binary_markets_common_filters(
# With a little bandwidth for the market to be finalized,
# so we have time for processing it without erroring out at the end.
finalized_after=utcnow() + timedelta(minutes=30),
question_finalized_after=utcnow()
+ timedelta(minutes=30),
) + get_omen_binary_markets_common_filters(
# And also markets without any answer at all yet.
question_with_answers=False,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't realise before that if I use question_finalized_after, it won't include questions without any answer at all (where it's null)

Copy link
Contributor

@gabrielfior gabrielfior Oct 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Effectivelly what you are doing with the partial is an OR filter, since you want (in this code snippet) the questions that finalize after now() + 30min OR questions without answers.
For this, I find querying the subgraph using OR a lot cleaner - see code snippet below (example query).

{
  questions(first: 5,
  where: {
    or: [{
    id: "0x001dd5a9194948a903c0e3b624eb0d458cd23d828f29a57f517c858d7cf71f76"  
    },
    {id : "0x0005ef01269fbb37aba4cc16f518138d7d8bf5f4937a5572c294c2e62f0cf670"}
    ],    
  }) {
    id
    templateId
    data
    title
  }
}

What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

using OR a lot cleaner

It is clean in the simple example query you provided, but how do you suggest integrating or functionality into OmenSubgraphHandler? With any integration that I can imagine, it's in the end easier to just do it like I do it here.

  • finalized_after shouldn't be finalized_after OR without_answers, because sometimes we might need to really query only finalized_after markets
  • I didn't want to introduce finalize_after_or_null filter, because that we would have to have or_null variant of many other arguments as well

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree partial here is simple and works, but what I was thinking is something along these lines (inside the subgraph handler) if the use case becomes more complicated:

where_stm1 = -> build with params 1
where_stm2 -> build with params2
where_final = {"or": [where_stms1, where_stms2]}

The added benefit here is to execute just 1 query, whereas in yours we execute 2 queries.

Copy link
Contributor

Choose a reason for hiding this comment

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

But happy here with partial :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That will need some fancy refactoring of OmenSubgraphHandler, but I agree it could be nice!

I'm not sure how it would end up in the end, but if you are up to exploring it, it sounds good to me.

to execute just 1 query, whereas in yours we execute 2 queries.

That's a good benefit out of it.

)
logger.info(f"Found {len(markets_open_for_answers)} markets to challenge.")

Expand Down
2 changes: 1 addition & 1 deletion prediction_market_agent/agents/omen_cleaner_agent/deploy.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ def resolve_finalized_markets(self, api_keys: APIKeys) -> list[HexAddress]:
market
for market in OmenSubgraphHandler().get_omen_binary_markets(
limit=None,
finalized_before=utcnow(),
question_finalized_before=utcnow(),
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Inconsistent parameter naming in get_omen_binary_markets calls.

The question_finalized_before parameter was updated in deploy.py, but other calls to get_omen_binary_markets still use the old parameter name finalized_before. Please update these method calls to ensure consistent parameter naming across the codebase.

🔗 Analysis chain

Approve the parameter name change and verify consistency.

The change from finalized_before to question_finalized_before improves clarity by specifying that we're filtering based on the question's finalization time. This aligns well with the method's purpose of resolving finalized markets.

To ensure consistency across the codebase, please run the following script:

This will help confirm that the OmenSubgraphHandler class and its usage are consistent with this change.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of 'question_finalized_before' parameter in OmenSubgraphHandler

# Test: Search for OmenSubgraphHandler class definition and its method calls
rg --type python -A 10 'class OmenSubgraphHandler'
rg --type python 'OmenSubgraphHandler\(\)\.get_omen_binary_markets'

Length of output: 184

Script:

#!/bin/bash
# Description: Verify the usage of 'question_finalized_before' parameter in OmenSubgraphHandler

# Test: Search for OmenSubgraphHandler class definition and its method calls
rg --type py -A 10 'class OmenSubgraphHandler'
rg --type py 'OmenSubgraphHandler\(\)\.get_omen_binary_markets'

Length of output: 1431

Script:

#!/bin/bash
# Description: Verify the usage of 'question_finalized_before' parameter in all calls to 'get_omen_binary_markets'

# Search for 'get_omen_binary_markets' method calls with 'question_finalized_before' parameter
rg --type py 'get_omen_binary_markets\([^)]*question_finalized_before='

Length of output: 73

resolved=False,
)
if market.id not in IGNORED_FOR_RESOLUTION
Expand Down
2 changes: 1 addition & 1 deletion prediction_market_agent/agents/replicate_to_omen_agent/omen_replicate.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ def omen_unfund_replicated_known_markets_tx(
markets = OmenSubgraphHandler().get_omen_binary_markets(
limit=None,
creator=from_address,
opened_before=opened_before,
question_opened_before=opened_before,
liquidity_bigger_than=wei_type(0),
)

Expand Down
20 changes: 14 additions & 6 deletions prediction_market_agent/agents/replicate_to_omen_agent/omen_resolve_replicated.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
from datetime import timedelta
from functools import partial

from prediction_market_agent_tooling.config import APIKeys
from prediction_market_agent_tooling.gtypes import (
Expand Down Expand Up @@ -52,15 +53,22 @@ def omen_finalize_and_resolve_and_claim_back_all_markets_based_on_others_tx(

now = utcnow()

# Fetch markets created by us that are already open, but no answer was submitted yet.
created_opened_markets = OmenSubgraphHandler().get_omen_binary_markets(
# Fetch markets created by us that are already open, but no answer was submitted yet or they are challengable.
get_omen_binary_markets_common_filters = partial(
OmenSubgraphHandler().get_omen_binary_markets,
limit=None,
creator=public_key,
# We need markets already opened for answers.
opened_before=now,
# With a little bandwidth for the market to be finalized,
question_opened_before=now,
)
created_opened_markets = get_omen_binary_markets_common_filters(
# Markets with a little bandwidth for the market to be finalized,
# so we have time for processing it without erroring out at the end.
finalized_after=now + timedelta(minutes=30),
question_finalized_after=now
+ timedelta(minutes=30),
) + get_omen_binary_markets_common_filters(
# And markets without any answer at all.
question_with_answers=False,
)
logger.info(f"Found {len(created_opened_markets)} markets to answer.")
# Finalize them (set answer on Realitio).
Expand Down Expand Up @@ -89,7 +97,7 @@ def omen_finalize_and_resolve_and_claim_back_all_markets_based_on_others_tx(
created_finalized_markets = OmenSubgraphHandler().get_omen_binary_markets(
limit=None,
creator=public_key,
finalized_before=now,
question_finalized_before=now,
resolved=False,
)
# Resolve them (resolve them on Oracle).
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ poetry = "^1.7.1"
poetry-plugin-export = "^1.6.0"
functions-framework = "^3.5.0"
cron-validator = "^1.0.8"
prediction-market-agent-tooling = { version = "^0.51.1", extras = ["langchain", "google"] }
prediction-market-agent-tooling = { version = "^0.52.0", extras = ["langchain", "google"] }
pydantic-settings = "^2.1.0"
autoflake = "^2.2.1"
isort = "^5.13.2"
Expand Down
Loading