-
Notifications
You must be signed in to change notification settings - Fork 7
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
Raise OutOfFunds in withdraw_wxdai_to_xdai_to_keep_balance #553
Conversation
WalkthroughThe pull request introduces two custom exception classes, Changes
Possibly related PRs
Suggested reviewers
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 (
|
@@ -1339,7 +1340,7 @@ def withdraw_wxdai_to_xdai_to_keep_balance( | |||
) | |||
|
|||
if current_balances.wxdai < need_to_withdraw: | |||
raise ValueError( | |||
raise OutOfFundsError( |
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.
This can happen if agent has all his money locked in bets. If we raise OutOfFundsError instead of ValueError, these errors are reported to our slack channel only once a week, so it won't spam us.
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: 0
🧹 Outside diff range and nitpick comments (4)
prediction_market_agent_tooling/tools/custom_exceptions.py (2)
1-6
: LGTM! Consider adding docstrings for better documentation.The exception classes are well-named and appropriately inherit from ValueError. To improve maintainability, consider adding docstrings to document their specific use cases.
class CantPayForGasError(ValueError): + """Raised when there are insufficient funds to pay for gas fees in blockchain operations.""" pass class OutOfFundsError(ValueError): + """Raised when there are insufficient funds to perform trading operations.""" pass
6-6
: Add newline at end of fileAdd a trailing newline at the end of the file to follow standard file formatting conventions.
class OutOfFundsError(ValueError): pass +
prediction_market_agent_tooling/deploy/agent.py (2)
Line range hint
476-481
: Security: Avoid exposing API keys in error messagesThe error message includes the full
api_keys
object which might expose sensitive information in logs or error reports.Consider this safer alternative:
- raise CantPayForGasError( - f"{api_keys=} doesn't have enough operational balance." - ) + raise CantPayForGasError( + "Insufficient operational balance to execute transactions." + )
Line range hint
711-723
: Consider making the balance buffer configurableThe 1% buffer added to the minimum required balance is a good practice, but consider making it configurable for different market conditions or risk profiles.
Consider this enhancement:
class DeployableTraderAgent(DeployablePredictionAgent): + # Buffer percentage above maximum possible bet amount (e.g., 0.01 for 1%) + min_balance_buffer: float = 0.01 + def check_min_required_balance_to_trade(self, market: AgentMarket) -> None: api_keys = APIKeys() # Get the strategy to know how much it will bet. strategy = self.get_betting_strategy(market) # Have a little bandwidth after the bet. - min_required_balance_to_trade = strategy.maximum_possible_bet_amount * 1.01 + min_required_balance_to_trade = strategy.maximum_possible_bet_amount * (1 + self.min_balance_buffer)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
pyproject.toml
is excluded by!**/*.toml
📒 Files selected for processing (3)
prediction_market_agent_tooling/deploy/agent.py
(1 hunks)prediction_market_agent_tooling/markets/omen/omen.py
(2 hunks)prediction_market_agent_tooling/tools/custom_exceptions.py
(1 hunks)
🔇 Additional comments (3)
prediction_market_agent_tooling/deploy/agent.py (1)
62-65
: LGTM: Clean import of custom exceptions
The custom exceptions are properly imported and well-organized within the imports section.
prediction_market_agent_tooling/markets/omen/omen.py (2)
78-78
: LGTM!
The import statement for OutOfFundsError
is correctly placed with other imports from the same module.
1343-1345
: LGTM! Good error handling improvement.
The change from ValueError
to OutOfFundsError
makes the error handling more specific and descriptive. This will help in better error handling and debugging when insufficient funds are available for withdrawal.
#552) * Lower amount of DB connections (#547) * Single shared engine instance * fixes * revert accidental bump * fix pickle in github ci * fiiiix * fix loguru * revert * ah * ah2 * revert * Added gnosis_rpc_config, google_credentials_filename and chain_id to BaseSettings * Fixed black * Fixed black (2) | changed rpc * Raise OutOfFunds in withdraw_wxdai_to_xdai_to_keep_balance (#553) * Implemented PR comments * Fixed CI * Merged --------- Co-authored-by: Peter Jung <[email protected]>
* Added block_time fetching for more accurate outcome tokens received. * Financial metrics being published * Reverted caching to diskcache * Fixing CI * lock file * Updated network foundry * Updated RPC to gateway * Moved local_chain test to proper folder * Fixed black * Applied PR comments * Small fixes after PR review * Small refactoring to address PR comments * Added gnosis_rpc_config, google_credentials_filename and chain_id to … (#552) * Lower amount of DB connections (#547) * Single shared engine instance * fixes * revert accidental bump * fix pickle in github ci * fiiiix * fix loguru * revert * ah * ah2 * revert * Added gnosis_rpc_config, google_credentials_filename and chain_id to BaseSettings * Fixed black * Fixed black (2) | changed rpc * Raise OutOfFunds in withdraw_wxdai_to_xdai_to_keep_balance (#553) * Implemented PR comments * Fixed CI * Merged --------- Co-authored-by: Peter Jung <[email protected]> --------- Co-authored-by: Peter Jung <[email protected]>
No description provided.