Optionally skip failed question claimings #586
Conversation
WalkthroughThe changes modify the Changes
Sequence DiagramsequenceDiagram
participant Caller
participant ClaimBondsFunction
participant RealitioQuestion
Caller->>ClaimBondsFunction: Call with questions and skip_failed flag
loop For each question
ClaimBondsFunction->>RealitioQuestion: Attempt to claim bond
alt Claim Successful
ClaimBondsFunction->>ClaimBondsFunction: Record successful claim
else Claim Failed
alt skip_failed is True
ClaimBondsFunction->>ClaimBondsFunction: Log error, continue
else skip_failed is False
ClaimBondsFunction->>Caller: Raise exception
end
end
end
ClaimBondsFunction->>Caller: Return results
✨ 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
prediction_market_agent_tooling/markets/omen/omen_resolving.py (1)
50-55: Consider enhancing error logging for better debugging.The error logging could be improved to include more context about the exception type and provide structured logging for better error tracking.
- logger.error( - f"Failed to claim bond for {question.url=}, {question.questionId=}: {e}" - ) + logger.error( + "Failed to claim bond", + extra={ + "error_type": type(e).__name__, + "error_message": str(e), + "question_url": question.url, + "question_id": question.questionId.hex(), + } + )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pyproject.tomlis excluded by!**/*.toml
📒 Files selected for processing (1)
prediction_market_agent_tooling/markets/omen/omen_resolving.py(1 hunks)
⏰ 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 (3)
prediction_market_agent_tooling/markets/omen/omen_resolving.py (3)
37-37: LGTM! Well-designed parameter addition.The new
skip_failedparameter is well-named, properly typed, and maintains backward compatibility with its default value.
45-49: LGTM! Robust error handling implementation.The try-except block is well-structured with proper placement of the success tracking inside the try block.
37-55: Verify error handling consistency across related operations.The new error handling pattern for bond claiming operations should be consistent with other critical financial operations in the codebase.
✅ Verification successful
Error handling implementation is consistent with the codebase design
The skip_failed pattern is correctly implemented only in the bulk operation function, while individual transaction functions maintain strict error handling for safety. This is the intended design pattern across the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for similar error handling patterns in related functions # Look for similar financial operations that might benefit from the same error handling pattern rg -B 2 -A 2 "def \w*claim\w*|def \w*withdraw\w*|def \w*resolve\w*" --type py # Check for existing try-except patterns in financial operations rg -B 2 -A 5 "try:.*?except.*?Exception" --type py --multilineLength of output: 16609
Script:
#!/bin/bash # Search for the implementation of critical financial operations ast-grep --pattern 'def claim_bonds_on_realitio_questions($$$)' ast-grep --pattern 'def claim_bonds_on_realitio_question($$$)' ast-grep --pattern 'def resolve_markets($$$)' # Also check for similar skip_failed patterns rg -B 5 -A 5 "skip_failed.*?=.*?(True|False)" --type pyLength of output: 1248
Script:
#!/bin/bash # Get complete implementations of critical functions rg -B 10 -A 30 "^def (resolve_markets|claim_bonds_on_realitio_question|omen_resolve_market_tx)" --type py # Look for any other error handling patterns in financial operations rg -B 5 -A 10 "try:.*?(claim|resolve|withdraw)" --type py --multilineLength of output: 14458
| f"Failed to claim bond for {question.url=}, {question.questionId=}: {e}" | ||
| ) |
There was a problem hiding this comment.
According to GCP, here's the error
eb3.exceptions.ContractLogicError: ('execution reverted: History input provided did not match the expected hash',
I imagine there's a bug with the history input?
There was a problem hiding this comment.
No there isn't. That question has just 1 answer, not much to get wrong there. On localhost, it also failed a few times on "already claimed". And then it suddenly passed 🤷 Weird times!
deploy please