Conversation
Contributor
📝 WalkthroughWalkthroughAdds a new end-to-end market demo script and a new rewards verification script, and removes two older verification scripts. The demo creates a market, seeds LP liquidity, executes a taker trade, waits for settlement, and exposes helpers for retries, client creation, and formatting; the verifier queries the indexer for distributions, rewards, and settlements. Changes
Sequence Diagram(s)sequenceDiagram
participant Demo as Demo Script
participant SDK as Client/SDK
participant Node as Blockchain Node
participant Indexer as Indexer API
participant Wallet as Wallets
Demo->>SDK: create_client() & create market
SDK->>Node: submit market creation tx
Node-->>SDK: tx confirmed / market_id
Demo->>Wallet: fund LP & buyer wallets
Demo->>SDK: place LP orders (split/midpoint/paired)
SDK->>Node: submit orders
Demo->>SDK: place buyer taker order (crosses LP)
SDK->>Node: submit taker tx
Node-->>SDK: trade confirmed
Demo->>Indexer: poll for settlement & distribution
Indexer-->>Demo: distribution, rewards, settlements
Demo->>Wallet: retrieve per-wallet rewards & P&L
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
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)
📝 Coding Plan for PR comments
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 |
Time Submission Status
You can submit time with the command. Example: See available commands to help comply with our Guidelines. |
![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: 8
🤖 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_run_market_demo.py`:
- Around line 33-39: Remove hard-coded private keys and addresses
(MARKET_CREATOR_KEY, MARKET_MAKER_KEY, MARKET_MAKER_ADDR, BUYER_TAKER_KEY,
BUYER_TAKER_ADDR) and load them from environment variables or generate throwaway
accounts at runtime; update the example to read process.env (or an env helper)
for each key/address and fall back to creating ephemeral testnet wallets when
not provided, and ensure any created keys are not written to the repository or
logs.
- Around line 187-195: The demo is inconsistent: the
with_retry(client_mm.place_buy_order, query_id, outcome=True, price=50,
amount=300) call is described as "matches NO sell@50" but the subsequent
expected LP summary still lists "NO sell@50" as open; update code so the
behavior and description agree—either change the buy order
(client_mm.place_buy_order) so it will not match (e.g., adjust price/amount or
use a non-matching flag) and update the print text to say it remains open, or
keep the buy to match and remove/update "NO sell@50" from the expected LP pairs
and adjust amounts/prints accordingly; ensure the printed messages around the
with_retry call and the expected LP pair block reflect the chosen behavior.
- Around line 109-133: The code currently drops the tx hash returned by
with_retry(client_creator.create_price_above_threshold_market, ...) and instead
scans the last 10 markets via client_creator.list_markets which can miss or
misidentify your market; capture the tx hash returned from
create_price_above_threshold_market, wait for the transaction to be mined using
the SDK (e.g., client_creator.get_tx / get_tx_receipt or any wait_for_tx
helper), then extract the created market id from the transaction receipt/events
and use that query_id directly instead of scanning list_markets; update the call
site that currently assigns nothing from with_retry(...) to store the tx hash
and replace the settle_time-based lookup logic with parsing the tx receipt for
the market id.
- Around line 51-65: The current with_retry function retries on every Exception
and is being used for non-idempotent writers
(create_price_above_threshold_market and place_*_order), which can cause
duplicate transactions; change with_retry to only retry idempotent/read-only
calls or update it to accept and match a whitelist of transient exception types
(e.g., network/timeouts, recoverable SDK errors) instead of catching Exception,
and stop using it for create_price_above_threshold_market and place_*_order
writes — either replace those calls with a non-retrying flow that checks
transaction receipt/status (or uses a deduplication idempotency key) or pass a
retry_exceptions parameter to with_retry and only retry when the exception
matches that set.
- Line 140: Remove the unused f-string prefixes on the print calls that do not
interpolate any values: change print(f" Saved to .query_id for
06_verify_rewards.py") to print(" Saved to .query_id for
06_verify_rewards.py") and do the same for the two other similar print
statements (the ones at the later occurrences with identical non-interpolated
messages). Locate these by searching for the print calls with those literal
message strings and remove the leading f prefix so they become regular string
literals.
- Line 245: The HTTP calls use with_retry(requests.get, ...) without a timeout
which can hang; update each call that passes requests.get (including the one
using dist_url and the three other requests.get usages in this file) to include
a timeout=10 argument so the call becomes with_retry(requests.get, ...,
timeout=10, max_retries=5, initial_backoff=5); ensure you add timeout=10 to the
same parameter position for all four sites so the retry wrapper forwards the
timeout to requests.get.
In `@examples/order_book/06_verify_rewards.py`:
- Around line 193-201: The current P&L fetch using pnl_url and
with_retry(requests.get, ...) only prints when resp.status_code == 200 and
silently ignores other responses; update the block that calls
with_retry/requests.get for pnl_url to handle non-200 responses by logging or
raising: check resp is not None, and if resp.status_code != 200, print a clear
error including the status code and resp.text (or resp.json() safely), and
optionally raise an exception; keep the existing print path for data when status
is 200 and still reference BUYER_TAKER_ADDR, pnl_url, with_retry, and
requests.get so the change is made in the same fetch/response handling code.
- Line 77: with_retry is calling requests.get without a timeout causing
indefinite hangs; update each with_retry(requests.get, ...) invocation (e.g.,
the one assigning resp from dist_url and the similar calls for distribution,
rewards, settlements, and P&L) to pass a callable that includes a timeout (use
functools.partial or a lambda, e.g., lambda url: requests.get(url, timeout=10))
so every HTTP request has a bounded timeout (choose a sensible value like 10s)
and keep using with_retry for retry/backoff behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c4696293-744f-4308-b843-1ddd47dc5d8b
📒 Files selected for processing (4)
examples/order_book/05_run_market_demo.pyexamples/order_book/05_verify_pnl.pyexamples/order_book/06_check_lp_rewards.pyexamples/order_book/06_verify_rewards.py
💤 Files with no reviewable changes (2)
- examples/order_book/05_verify_pnl.py
- examples/order_book/06_check_lp_rewards.py
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (4)
examples/order_book/06_verify_rewards.py (2)
102-102: Remove extraneousfprefixes from strings without placeholders.Several print statements use f-strings but contain no interpolation:
- Line 102:
f"\n Fee Split Verification:"- Line 110:
f" Settlement runs asynchronously after settle_time."- Line 118:
f"\n2. Participant Reward History..."- Line 165:
f"\n3. Buyer/Taker Settlement..."- Line 202:
f" P&L Summary:"🧹 Proposed fix
- print(f"\n Fee Split Verification:") + print("\n Fee Split Verification:")Apply similarly to lines 110, 118, 165, and 202.
Also applies to: 110-110, 118-118, 165-165, 202-202
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/order_book/06_verify_rewards.py` at line 102, Remove the unnecessary f-string prefixes from print statements that contain no interpolations: replace print(f"\n Fee Split Verification:"), print(f" Settlement runs asynchronously after settle_time."), print(f"\n2. Participant Reward History..."), print(f"\n3. Buyer/Taker Settlement..."), and print(f" P&L Summary:") with plain string prints (remove the leading 'f') so each print call uses standard string literals; this change should be applied where those exact string literals appear in the file.
96-107: Clarify variable naming in fee split calculation.The assignment
lp = totalon line 100 is potentially confusing. Iftotal_fees_distributedrepresents only the LP portion (as the calculation assumes), consider renaming to make this clearer:- total = int(data.get('total_fees_distributed', '0')) + lp_fees = int(data.get('total_fees_distributed', '0')) dp = int(data.get('total_dp_fees', '0')) val = int(data.get('total_validator_fees', '0')) - lp = total - if total > 0: + total = lp_fees + dp + val + if total > 0: print(f"\n Fee Split Verification:") - print(f" LP share: {lp / (lp + dp + val) * 100:.1f}% (expected: 75%)") - print(f" DP share: {dp / (lp + dp + val) * 100:.1f}% (expected: 12.5%)") - print(f" Validator share: {val / (lp + dp + val) * 100:.1f}% (expected: 12.5%)") + print(f" LP share: {lp_fees / total * 100:.1f}% (expected: 75%)") + print(f" DP share: {dp / total * 100:.1f}% (expected: 12.5%)") + print(f" Validator share: {val / total * 100:.1f}% (expected: 12.5%)")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/order_book/06_verify_rewards.py` around lines 96 - 107, The variable lp assigned from total is confusing; rename lp to a clearer name (e.g., lp_fees or lp_amount) and update its usage in the fee-split calculation and print statements so it's obvious lp_fees = int(data.get('total_fees_distributed', '0')) represents the LP portion; keep existing dp, val assignments (from total_dp_fees and total_validator_fees) and ensure the denominator uses (lp_fees + dp + val) to avoid misreading of total vs LP amount.examples/order_book/05_run_market_demo.py (2)
125-127: Consider catching more specific exceptions for market creation.Catching bare
Exceptioncan mask programming errors. Consider catching SDK-specific exceptions or at minimum re-raising unexpected errors:except requests.exceptions.RequestException as e: print(f"Network error creating market: {e}") return except Exception as e: print(f"Unexpected error creating market: {e}") raise # Re-raise unexpected errors for debugging🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/order_book/05_run_market_demo.py` around lines 125 - 127, Replace the broad "except Exception as e:" in the market creation block with specific exception handlers: catch SDK/network exceptions such as requests.exceptions.RequestException (or the SDK's specific error class) and handle them with a clear message like "Network error creating market", then add a final broad except that logs "Unexpected error creating market" and re-raises the exception to avoid masking programming errors; update the handlers around the market creation call where the current print(f"Error creating market: {e}") exists (the except block shown) to implement this pattern.
33-39: Consider environment variable fallback for demo keys.While the warning comment clarifies these are testnet-only throwaway keys, consider also supporting environment variable overrides for users who want to use their own test accounts:
MARKET_CREATOR_KEY = os.environ.get("DEMO_MARKET_CREATOR_KEY", "a537437df2ed8d3...")This makes the demo more flexible without removing the convenience of defaults.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/order_book/05_run_market_demo.py` around lines 33 - 39, Replace hard-coded demo keys and addresses with environment-variable fallbacks: for each constant (MARKET_CREATOR_KEY, MARKET_MAKER_KEY, MARKET_MAKER_ADDR, BUYER_TAKER_KEY, BUYER_TAKER_ADDR) read from os.environ with the current literal as the default; ensure os is imported at top and keep the existing warning comment intact so demo works out-of-the-box but can be overridden via DEMO_* environment variables.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@examples/order_book/05_run_market_demo.py`:
- Around line 125-127: Replace the broad "except Exception as e:" in the market
creation block with specific exception handlers: catch SDK/network exceptions
such as requests.exceptions.RequestException (or the SDK's specific error class)
and handle them with a clear message like "Network error creating market", then
add a final broad except that logs "Unexpected error creating market" and
re-raises the exception to avoid masking programming errors; update the handlers
around the market creation call where the current print(f"Error creating market:
{e}") exists (the except block shown) to implement this pattern.
- Around line 33-39: Replace hard-coded demo keys and addresses with
environment-variable fallbacks: for each constant (MARKET_CREATOR_KEY,
MARKET_MAKER_KEY, MARKET_MAKER_ADDR, BUYER_TAKER_KEY, BUYER_TAKER_ADDR) read
from os.environ with the current literal as the default; ensure os is imported
at top and keep the existing warning comment intact so demo works out-of-the-box
but can be overridden via DEMO_* environment variables.
In `@examples/order_book/06_verify_rewards.py`:
- Line 102: Remove the unnecessary f-string prefixes from print statements that
contain no interpolations: replace print(f"\n Fee Split Verification:"),
print(f" Settlement runs asynchronously after settle_time."), print(f"\n2.
Participant Reward History..."), print(f"\n3. Buyer/Taker Settlement..."), and
print(f" P&L Summary:") with plain string prints (remove the leading 'f') so
each print call uses standard string literals; this change should be applied
where those exact string literals appear in the file.
- Around line 96-107: The variable lp assigned from total is confusing; rename
lp to a clearer name (e.g., lp_fees or lp_amount) and update its usage in the
fee-split calculation and print statements so it's obvious lp_fees =
int(data.get('total_fees_distributed', '0')) represents the LP portion; keep
existing dp, val assignments (from total_dp_fees and total_validator_fees) and
ensure the denominator uses (lp_fees + dp + val) to avoid misreading of total vs
LP amount.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7a01d012-d333-4764-b20b-e3250988f579
📒 Files selected for processing (2)
examples/order_book/05_run_market_demo.pyexamples/order_book/06_verify_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.
This file committed since it useful to know end-to-end market demo
resolves: https://github.com/truflation/website/issues/3465
Summary by CodeRabbit
New Features
Chores