Conversation
Time Submission Status
You can submit time with the command. Example: See available commands to help comply with our Guidelines. |
Contributor
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughAdds two new example scripts demonstrating market lifecycle orchestration and LP reward/P&L verification via TNClient and indexer HTTP endpoints, including exponential-backoff retry helpers and CLI entry for reward checks. Changes
Sequence Diagram(s)sequenceDiagram
participant Script
participant TNClient
participant Node
participant Indexer
participant MM_Wallet
participant Buyer_Wallet
Script->>TNClient: create_market(settle_time, params)
TNClient->>Node: submit tx (market creation)
Node-->>TNClient: tx result
Script->>TNClient: place_split_limit_order(MM_Wallet)
TNClient->>Node: submit tx (maker order)
Node-->>TNClient: tx result
Script->>TNClient: place_buy_order(Buyer_Wallet)
TNClient->>Node: submit tx (taker order)
Node-->>TNClient: tx result
Script->>Indexer: GET /distribution?query_id=...
Indexer-->>Script: distribution summary
Script->>Indexer: GET /rewards?query_id=...&wallet=MM_Wallet
Indexer-->>Script: MM rewards history
Script->>Indexer: GET /rewards?query_id=...&wallet=Buyer_Wallet
Indexer-->>Script: Buyer rewards history
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (5)
examples/order_book/05_verify_pnl.py (2)
43-46: Prefer bareraiseto preserve full traceback.Using
raise einstead ofraisecan truncate the traceback in some edge cases.🧹 Proposed fix
if retries >= max_retries: - raise e + raise🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/order_book/05_verify_pnl.py` around lines 43 - 46, In the except block that catches Exception as e (the retry loop using retries and max_retries), replace the explicit "raise e" with a bare "raise" to preserve the original traceback; keep the same logic that increments retries and re-raises only when retries >= max_retries (i.e., in the except Exception as e handler, use "raise" instead of re-raising the exception object).
120-131: Inconsistent with earlier examples: use SDK methods for distribution queries.The preceding order_book examples (01-04) consistently use SDK client methods. File 05 breaks this pattern by using direct HTTP requests. The SDK provides
get_distribution_summary(query_id)to fetch distribution data with type safety and consistent error handling. Examples 01-04 demonstrate that SDK methods are the standard approach for this example series; using them here would maintain consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/order_book/05_verify_pnl.py` around lines 120 - 131, Replace the direct HTTP call using with_retry(requests.get, dist_url) with the SDK method get_distribution_summary(query_id) on the existing SDK client (e.g., call client.get_distribution_summary(query_id)); check the SDK response for success/error using the SDK's return structure, extract the distribution data (total_fees_distributed, distributed_at) from the returned object instead of resp.json(), and print the same fields; remove reliance on with_retry/requests in this block so the example matches earlier files that use the SDK's type-safe method.examples/order_book/06_check_lp_rewards.py (3)
5-7: Remove unused imports.
timedeltaandWeb3are imported but never used in this script.🧹 Proposed fix
-from datetime import datetime, timezone, timedelta +from datetime import datetime, timezone import requests -from web3 import Web3🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/order_book/06_check_lp_rewards.py` around lines 5 - 7, The imports include unused symbols timedelta and Web3; remove them from the import statements so only used symbols remain (keep datetime, timezone and requests). Specifically edit the top import line that currently references timedelta and the Web3 import (the Web3 class) and delete those unused imports to eliminate dead code and lint warnings.
10-13: Remove unused constants.
NODE_URLandTEST_CHAIN_IDare defined but never used in this script.🧹 Proposed fix
# --- Configuration --- -# Kwil/Node Configuration -NODE_URL = "http://ec2-3-141-77-16.us-east-2.compute.amazonaws.com:8484" INDEXER_URL = "http://ec2-52-15-66-172.us-east-2.compute.amazonaws.com:8080" -TEST_CHAIN_ID = "testnet-v1"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/order_book/06_check_lp_rewards.py` around lines 10 - 13, Remove the unused constants NODE_URL and TEST_CHAIN_ID from the top of the script; keep only INDEXER_URL (or any other constants that are actually referenced later) to avoid dead code and confusion, and run a quick grep/IDE search to ensure no remaining references to NODE_URL or TEST_CHAIN_ID before committing.
20-31: Inconsistentwith_retryimplementation between example scripts.This implementation differs significantly from
05_verify_pnl.py:
- 3 retries vs 5 retries with exponential backoff
- Returns empty
Response()vs raises exception on failure- Fixed 2s sleep vs exponential backoff
The empty
Response()return is subtle—itsstatus_codeattribute isNone, which happens to work with the== 200checks but could cause confusion. Consider aligning implementations or extracting a shared utility.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/order_book/06_check_lp_rewards.py` around lines 20 - 31, The with_retry function currently retries 3 times with a fixed 2s sleep and returns an empty requests.Response() on failure; change it to match the behavior in 05_verify_pnl.py by performing 5 attempts with exponential backoff (e.g., sleep grows each retry), propagate/raise an exception when all retries fail instead of returning an empty Response(), and preserve checking resp.status_code == 200 before returning resp; update error handling for requests.exceptions.RequestException to include the exception when raising so callers of with_retry (calling code that checks resp.status_code) get a clear failure instead of a bogus Response object.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@examples/order_book/05_verify_pnl.py`:
- Around line 142-143: The print statement currently uses an unnecessary
f-string: change the line that prints " Summary P&L:" (the print after
assigning data = resp.json().get("data", {})) to use a normal string literal
(remove the leading "f") so it is print(" Summary P&L:") instead of print(f"
Summary P&L:"); no other behavior changes required.
- Around line 164-173: The loop that prints reward entries uses direct dict
access (r['query_id'], r['reward_amount'], r['distributed_at']) which can raise
KeyError if the API omits fields; update the printing logic in the block that
builds rewards_url/resp/data to use r.get('query_id'), r.get('reward_amount')
and r.get('distributed_at') (providing sensible defaults like None or 'N/A') or
guard each field before formatting so missing keys don't crash the script; keep
the rest of the rewards_url/resp handling the same.
In `@examples/order_book/06_check_lp_rewards.py`:
- Around line 54-55: The print statement in the else branch (the line containing
print(f" - Distributed At: Not yet distributed")) uses an unnecessary
f-string prefix even though there are no placeholders; change that print call to
a normal string literal by removing the leading "f" so it reads print(" -
Distributed At: Not yet distributed") to eliminate the extraneous f
prefix.
- Around line 72-74: The loop over rewards uses direct dict indexing
(r['reward_amount'] and r['distributed_at']) which can raise KeyError; update
the block that iterates the rewards list to access r.get('reward_amount',
<default>) and r.get('distributed_at') safely (e.g., reward_amount =
r.get('reward_amount', 0) and ts = r.get('distributed_at')), then only call
datetime.fromtimestamp(ts, timezone.utc).strftime(...) if ts is not None and is
a number, otherwise use a fallback string like "unknown time"; apply this change
where the rewards iteration/print occurs (variables: rewards, r, reward_amount,
distributed_at/ts).
---
Nitpick comments:
In `@examples/order_book/05_verify_pnl.py`:
- Around line 43-46: In the except block that catches Exception as e (the retry
loop using retries and max_retries), replace the explicit "raise e" with a bare
"raise" to preserve the original traceback; keep the same logic that increments
retries and re-raises only when retries >= max_retries (i.e., in the except
Exception as e handler, use "raise" instead of re-raising the exception object).
- Around line 120-131: Replace the direct HTTP call using
with_retry(requests.get, dist_url) with the SDK method
get_distribution_summary(query_id) on the existing SDK client (e.g., call
client.get_distribution_summary(query_id)); check the SDK response for
success/error using the SDK's return structure, extract the distribution data
(total_fees_distributed, distributed_at) from the returned object instead of
resp.json(), and print the same fields; remove reliance on with_retry/requests
in this block so the example matches earlier files that use the SDK's type-safe
method.
In `@examples/order_book/06_check_lp_rewards.py`:
- Around line 5-7: The imports include unused symbols timedelta and Web3; remove
them from the import statements so only used symbols remain (keep datetime,
timezone and requests). Specifically edit the top import line that currently
references timedelta and the Web3 import (the Web3 class) and delete those
unused imports to eliminate dead code and lint warnings.
- Around line 10-13: Remove the unused constants NODE_URL and TEST_CHAIN_ID from
the top of the script; keep only INDEXER_URL (or any other constants that are
actually referenced later) to avoid dead code and confusion, and run a quick
grep/IDE search to ensure no remaining references to NODE_URL or TEST_CHAIN_ID
before committing.
- Around line 20-31: The with_retry function currently retries 3 times with a
fixed 2s sleep and returns an empty requests.Response() on failure; change it to
match the behavior in 05_verify_pnl.py by performing 5 attempts with exponential
backoff (e.g., sleep grows each retry), propagate/raise an exception when all
retries fail instead of returning an empty Response(), and preserve checking
resp.status_code == 200 before returning resp; update error handling for
requests.exceptions.RequestException to include the exception when raising so
callers of with_retry (calling code that checks resp.status_code) get a clear
failure instead of a bogus Response object.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 688d191d-b8cb-4ea4-ba63-b10c11aafdcc
📒 Files selected for processing (2)
examples/order_book/05_verify_pnl.pyexamples/order_book/06_check_lp_rewards.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
resolves: https://github.com/truflation/website/issues/3419
Summary by CodeRabbit