-
Notifications
You must be signed in to change notification settings - Fork 9
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
496 seer market creation on pmat #606
base: main
Are you sure you want to change the base?
Conversation
…-market-creation-on-pmat # Conflicts: # prediction_market_agent_tooling/markets/seer/data_models.py
WalkthroughThis pull request extends the Seer module by refining the data models and contract interactions for categorical market creation. It introduces a new Pydantic model for market creation parameters, updates the Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant CLI as Create Market CLI Script
participant MF as SeerMarketFactory
participant BC as Blockchain/Gnosis Contract
U->>CLI: Provide market parameters
CLI->>CLI: Process parameters and instantiate APIKeys
CLI->>MF: Call create_categorical_market(api_keys, params)
MF->>BC: Send transaction to create market
BC-->>MF: Return transaction receipt
MF-->>CLI: Return confirmation of market creation
CLI-->>U: Log confirmation message
Possibly related PRs
Suggested reviewers
Tip 🌐 Web search-backed reviews and chat
✨ 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (4)
tests_integration_with_local_chain/markets/seer/test_seer_contracts.py (1)
12-22
: Consider parameterizing the market creation parameters.The hardcoded values in
build_params
limit test coverage. Consider parameterizing these values to test different scenarios.-def build_params() -> CreateCategoricalMarketsParams: +def build_params( + token_names: list[str] = ["YES", "NO"], + min_bond: int = int(1e18), + outcomes: list[str] = ["Yes", "No"], + market_name: str = "test test test" +) -> CreateCategoricalMarketsParams: opening_time = int( (datetime.datetime.utcnow() + datetime.timedelta(days=1)).timestamp() ) return CreateCategoricalMarketsParams( - token_names=["YES", "NO"], - min_bond=str(int(1e18)), + token_names=token_names, + min_bond=str(min_bond), openingTime=opening_time, - outcomes=["Yes", "No"], - market_name="test test test", + outcomes=outcomes, + market_name=market_name, )prediction_market_agent_tooling/markets/seer/data_models.py (1)
46-49
: Consider adding validation for parent_outcome.The
parent_outcome
field might benefit from value validation to ensure it's non-negative.- parent_outcome: int = Field(alias="parentOutcome") + parent_outcome: int = Field(alias="parentOutcome", ge=0)scripts/create_market_seer.py (2)
21-21
: Address the TODO comment to adapt the script for Seer.The script currently uses Omen-specific functionality and needs to be adapted for Seer.
Would you like me to help implement the Seer-specific changes? This would involve:
- Updating imports to use Seer-specific modules
- Modifying the market creation parameters
- Updating the documentation
41-42
: Update script name in documentation.The example command shows incorrect script name.
- python scripts/create_market_omen.py \ + python scripts/create_market_seer.py \
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
poetry.lock
is excluded by!**/*.lock
,!**/*.lock
prediction_market_agent_tooling/abis/seer_market_factory.abi.json
is excluded by!**/*.json
prediction_market_agent_tooling/abis/seer_realitio_3_0.abi.json
is excluded by!**/*.json
prediction_market_agent_tooling/abis/seer_wrapper_1155_factory.abi.json
is excluded by!**/*.json
pyproject.toml
is excluded by!**/*.toml
📒 Files selected for processing (6)
prediction_market_agent_tooling/markets/seer/data_models.py
(2 hunks)prediction_market_agent_tooling/markets/seer/seer_contracts.py
(1 hunks)prediction_market_agent_tooling/markets/seer/seer_subgraph_handler.py
(2 hunks)scripts/create_market_seer.py
(1 hunks)tests_integration/markets/seer/test_seer_subgraph_handler.py
(1 hunks)tests_integration_with_local_chain/markets/seer/test_seer_contracts.py
(1 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
tests_integration_with_local_chain/markets/seer/test_seer_contracts.py
29-29: Local variable tx_receipt
is assigned to but never used
Remove assignment to unused variable tx_receipt
(F841)
scripts/create_market_seer.py
26-26: Do not perform function call typer.Option
in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
34-34: Do not perform function call typer.Option
in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: pytest - Python 3.12.x - Integration with Local Chain
- GitHub Check: pytest - Python 3.12.x - Unit Tests
- GitHub Check: pytest - Python 3.11.x - Integration with Local Chain
- 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 - Unit Tests
- GitHub Check: mypy
🔇 Additional comments (7)
prediction_market_agent_tooling/markets/seer/data_models.py (1)
10-33
: LGTM! Well-structured data model with proper type hints.The class is well-designed with:
- Clear field aliases for JSON serialization
- Proper type hints and optional fields
- Sensible default values
tests_integration/markets/seer/test_seer_subgraph_handler.py (1)
51-62
: LGTM! Test updated to use renamed method.The test correctly uses the renamed method
get_swapr_pools_for_market
and maintains the same validation logic.prediction_market_agent_tooling/markets/seer/seer_contracts.py (3)
24-24
: Reminder: Address the TODO comment.Please ensure that tests are added to verify that the same functionality is supported as in the parent class.
Do you want me to generate the unit testing code or open a new issue to track this task?
39-39
: Reminder: Address the TODO comment.Please specify which functions need to be added to this class.
Do you want me to help identify the missing functions from the contract ABI or open a new issue to track this task?
53-53
: Reminder: Address the TODO comment.Please specify which functions need to be added to this class.
Do you want me to help identify the missing functions from the contract ABI or open a new issue to track this task?
prediction_market_agent_tooling/markets/seer/seer_subgraph_handler.py (2)
49-49
: LGTM!The addition of
parentOutcome
field enhances the market data model by including parent outcome information.
130-130
: LGTM!The method rename from
get_pools_for_market
toget_swapr_pools_for_market
improves clarity by being more specific about the type of pools being fetched.
def test_create_market(local_web3: Web3, test_keys: APIKeys) -> None: | ||
factory = MarketFactory() | ||
num_initial_markets = factory.market_count(web3=local_web3) | ||
params = build_params() | ||
tx_receipt = factory.create_categorical_market( | ||
api_keys=test_keys, params=params, web3=local_web3 | ||
) | ||
|
||
num_final_markets = factory.market_count(web3=local_web3) | ||
assert num_initial_markets + 1 == num_final_markets |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove unused variable and add error handling.
The test should handle potential transaction failures and verify the transaction receipt.
def test_create_market(local_web3: Web3, test_keys: APIKeys) -> None:
factory = MarketFactory()
num_initial_markets = factory.market_count(web3=local_web3)
params = build_params()
- tx_receipt = factory.create_categorical_market(
+ factory.create_categorical_market(
api_keys=test_keys, params=params, web3=local_web3
)
num_final_markets = factory.market_count(web3=local_web3)
assert num_initial_markets + 1 == num_final_markets
📝 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.
def test_create_market(local_web3: Web3, test_keys: APIKeys) -> None: | |
factory = MarketFactory() | |
num_initial_markets = factory.market_count(web3=local_web3) | |
params = build_params() | |
tx_receipt = factory.create_categorical_market( | |
api_keys=test_keys, params=params, web3=local_web3 | |
) | |
num_final_markets = factory.market_count(web3=local_web3) | |
assert num_initial_markets + 1 == num_final_markets | |
def test_create_market(local_web3: Web3, test_keys: APIKeys) -> None: | |
factory = MarketFactory() | |
num_initial_markets = factory.market_count(web3=local_web3) | |
params = build_params() | |
factory.create_categorical_market( | |
api_keys=test_keys, params=params, web3=local_web3 | |
) | |
num_final_markets = factory.market_count(web3=local_web3) | |
assert num_initial_markets + 1 == num_final_markets |
🧰 Tools
🪛 Ruff (0.8.2)
29-29: Local variable tx_receipt
is assigned to but never used
Remove assignment to unused variable tx_receipt
(F841)
scripts/create_market_seer.py
Outdated
def main( | ||
question: str = typer.Option(), | ||
closing_time: datetime = typer.Option(), | ||
category: str = typer.Option(), | ||
initial_funds: str = typer.Option(), | ||
from_private_key: str = typer.Option(), | ||
safe_address: str = typer.Option(None), | ||
cl_token: CollateralTokenChoice = CollateralTokenChoice.sdai, | ||
fee_perc: float = typer.Option(OMEN_DEFAULT_MARKET_FEE_PERC), | ||
language: str = typer.Option("en"), | ||
outcomes: list[str] = typer.Option(OMEN_BINARY_MARKET_OUTCOMES), | ||
auto_deposit: bool = typer.Option(False), | ||
) -> None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix function calls in argument defaults.
Using function calls in argument defaults can lead to unexpected behavior.
def main(
- question: str = typer.Option(),
- closing_time: datetime = typer.Option(),
- category: str = typer.Option(),
- initial_funds: str = typer.Option(),
- from_private_key: str = typer.Option(),
- safe_address: str = typer.Option(None),
+ question: str = None,
+ closing_time: datetime = None,
+ category: str = None,
+ initial_funds: str = None,
+ from_private_key: str = None,
+ safe_address: str = None,
cl_token: CollateralTokenChoice = CollateralTokenChoice.sdai,
- fee_perc: float = typer.Option(OMEN_DEFAULT_MARKET_FEE_PERC),
+ fee_perc: float = OMEN_DEFAULT_MARKET_FEE_PERC,
language: str = typer.Option("en"),
- outcomes: list[str] = typer.Option(OMEN_BINARY_MARKET_OUTCOMES),
+ outcomes: list[str] = None,
- auto_deposit: bool = typer.Option(False),
+ auto_deposit: bool = False,
) -> None:
Then, add the options inside the function:
def main(
# ... parameters as shown above ...
) -> None:
question = typer.Option(question)
closing_time = typer.Option(closing_time)
category = typer.Option(category)
initial_funds = typer.Option(initial_funds)
from_private_key = typer.Option(from_private_key)
safe_address = typer.Option(safe_address)
fee_perc = typer.Option(fee_perc)
outcomes = typer.Option(outcomes or OMEN_BINARY_MARKET_OUTCOMES)
auto_deposit = typer.Option(auto_deposit)
🧰 Tools
🪛 Ruff (0.8.2)
26-26: Do not perform function call typer.Option
in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
34-34: Do not perform function call typer.Option
in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
) | ||
return receipt_tx | ||
|
||
# ToDo - Also return event NewMarket, emitted by this contract |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Implement event handling for NewMarket event.
The TODO comment indicates that the NewMarket
event emitted by the contract should be handled. This event likely contains important information about the created market.
Consider updating the create_categorical_market
method to parse and return the event data:
def create_categorical_market(
self,
api_keys: APIKeys,
params: CreateCategoricalMarketsParams,
web3: Web3 | None = None,
) -> TxReceipt:
receipt_tx = self.send(
api_keys=api_keys,
function_name="createCategoricalMarket",
function_params=[params.model_dump(by_alias=True)],
web3=web3,
)
+ # Parse NewMarket event from receipt
+ new_market_event = self.get_event_from_receipt(receipt_tx, "NewMarket")
+ if not new_market_event:
+ raise ValueError("NewMarket event not found in transaction receipt")
return receipt_tx
📝 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.
# ToDo - Also return event NewMarket, emitted by this contract | |
def create_categorical_market( | |
self, | |
api_keys: APIKeys, | |
params: CreateCategoricalMarketsParams, | |
web3: Web3 | None = None, | |
) -> TxReceipt: | |
receipt_tx = self.send( | |
api_keys=api_keys, | |
function_name="createCategoricalMarket", | |
function_params=[params.model_dump(by_alias=True)], | |
web3=web3, | |
) | |
# Parse NewMarket event from receipt | |
new_market_event = self.get_event_from_receipt(receipt_tx, "NewMarket") | |
if not new_market_event: | |
raise ValueError("NewMarket event not found in transaction receipt") | |
return receipt_tx |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (4)
prediction_market_agent_tooling/markets/seer/seer.py (3)
18-29
: Add a docstring for the new function.It's helpful to document the purpose, parameters, and return value for future reference and ease of understanding.
47-50
: Handle the scenario where shares could be zero.If
get_in_shares
returns 0, depositing an insufficient amount of collateral might lead to errors when the market is created. Consider adding logic to handle or warn about this situation.
72-72
: Address the TODO for liquidity provisioning.The comment references adding liquidity on Swapr. Let me know if you'd like a helper function or script to facilitate this integration.
prediction_market_agent_tooling/markets/seer/data_models.py (1)
10-33
: Add validations for numeric fields.Consider enforcing non-negative constraints on fields like
lower_bound
,upper_bound
, andopening_time
(e.g., future-only markets). This can make your model more robust against invalid inputs.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
prediction_market_agent_tooling/markets/seer/data_models.py
(2 hunks)prediction_market_agent_tooling/markets/seer/seer.py
(1 hunks)prediction_market_agent_tooling/markets/seer/seer_contracts.py
(1 hunks)prediction_market_agent_tooling/monitor/monitor.py
(1 hunks)prediction_market_agent_tooling/monitor/monitor_app.py
(2 hunks)scripts/create_market_seer.py
(1 hunks)tests_integration_with_local_chain/markets/seer/test_seer_contracts.py
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- prediction_market_agent_tooling/monitor/monitor.py
🧰 Additional context used
🪛 Ruff (0.8.2)
scripts/create_market_seer.py
18-18: Do not perform function call typer.Option
in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
23-23: Do not perform function call typer.Option
in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
23-23: Do not perform function call xdai_type
in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
25-25: Do not perform function call typer.Option
in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
prediction_market_agent_tooling/markets/seer/seer_contracts.py
41-41: Do not perform function call xdai_type
in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
tests_integration_with_local_chain/markets/seer/test_seer_contracts.py
27-27: Local variable tx_receipt
is assigned to but never used
Remove assignment to unused variable tx_receipt
(F841)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: pytest - Python 3.12.x - Integration with Local Chain
- GitHub Check: pytest - Python 3.12.x - Unit Tests
- GitHub Check: pytest - Python 3.11.x - Integration with Local Chain
- 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 - Unit Tests
🔇 Additional comments (8)
prediction_market_agent_tooling/markets/seer/seer.py (1)
39-46
: Consider verifying deposit success.After calling
auto_deposit_collateral_token
, you might want to check the updated balance or validate the transaction's success to gracefully handle any deposit failures.tests_integration_with_local_chain/markets/seer/test_seer_contracts.py (1)
27-29
: Remove or utilize the unusedtx_receipt
variable.This local variable isn’t used and triggers a linting warning. You could verify
tx_receipt.status
to ensure the transaction succeeded or omit storing it if it isn't needed.🧰 Tools
🪛 Ruff (0.8.2)
27-27: Local variable
tx_receipt
is assigned to but never usedRemove assignment to unused variable
tx_receipt
(F841)
prediction_market_agent_tooling/markets/seer/data_models.py (1)
46-49
: Ensure consistent parent/child outcome relationships.When setting
parent_outcome
andparent_market
, consider validating that the given outcome index is valid for the specified market to prevent mismatched or invalid hierarchical references.scripts/create_market_seer.py (2)
17-26
: Fix function calls in argument defaults.Using function calls in argument defaults can lead to unexpected behavior.
Apply this diff to fix the issue:
def main( - question: str = typer.Option(), - opening_time: datetime = typer.Option(), - category: str = typer.Option(), - initial_funds: str = typer.Option(), - from_private_key: str = typer.Option(), - safe_address: str = typer.Option(None), - min_bond_xdai: xDai = typer.Option(xdai_type(10)), - language: str = typer.Option("en"), - outcomes: list[str] = typer.Option(OMEN_BINARY_MARKET_OUTCOMES), - auto_deposit: bool = typer.Option(False), + question: str = None, + opening_time: datetime = None, + category: str = None, + initial_funds: str = None, + from_private_key: str = None, + safe_address: str = None, + min_bond_xdai: xDai = None, + language: str = "en", + outcomes: list[str] = None, + auto_deposit: bool = False,Then, add the options inside the function:
def main( # ... parameters as shown above ... ) -> None: question = typer.Option(question) opening_time = typer.Option(opening_time) category = typer.Option(category) initial_funds = typer.Option(initial_funds) from_private_key = typer.Option(from_private_key) safe_address = typer.Option(safe_address) min_bond_xdai = typer.Option(min_bond_xdai or xdai_type(10)) outcomes = typer.Option(outcomes or OMEN_BINARY_MARKET_OUTCOMES) auto_deposit = typer.Option(auto_deposit)🧰 Tools
🪛 Ruff (0.8.2)
18-18: Do not perform function call
typer.Option
in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable(B008)
23-23: Do not perform function call
typer.Option
in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable(B008)
23-23: Do not perform function call
xdai_type
in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable(B008)
25-25: Do not perform function call
typer.Option
in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable(B008)
40-58
: LGTM! The implementation looks good.The function correctly:
- Converts safe address to checksum format
- Creates API keys with proper validation
- Calls seer_create_market_tx with all required parameters
- Logs the created market
prediction_market_agent_tooling/markets/seer/seer_contracts.py (2)
83-83
: Implement event handling for NewMarket event.The TODO comment indicates that the
NewMarket
event emitted by the contract should be handled.Would you like me to help implement the event handling code to parse and return the event data from the transaction receipt?
24-68
: LGTM! The implementation looks good.The class correctly:
- Loads ABI from JSON file
- Sets contract address
- Implements utility methods for market operations
🧰 Tools
🪛 Ruff (0.8.2)
41-41: Do not perform function call
xdai_type
in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable(B008)
prediction_market_agent_tooling/monitor/monitor_app.py (1)
108-110
: LGTM! The changes look good.The code correctly:
- Simplifies start_time handling by directly using st.date_input result
- Maintains proper date/time combination logic
market_question: str, | ||
outcomes: list[str], | ||
opening_time: DatetimeUTC, | ||
min_bond_xdai: xDai = xdai_type(10), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Fix function call in argument defaults.
Using function calls in argument defaults can lead to unexpected behavior.
Apply this diff to fix the issue:
- min_bond_xdai: xDai = xdai_type(10),
+ min_bond_xdai: xDai | None = None,
Then, add the default inside the function:
def build_market_params(
# ... parameters as shown above ...
) -> CreateCategoricalMarketsParams:
min_bond_xdai = min_bond_xdai or xdai_type(10)
return CreateCategoricalMarketsParams(
# ... rest of the implementation ...
)
🧰 Tools
🪛 Ruff (0.8.2)
41-41: Do not perform function call xdai_type
in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
No description provided.