feat(examples): Add Polygon and Arbitrum support to simulation examples#998
feat(examples): Add Polygon and Arbitrum support to simulation examples#998tvinagre wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
5c43e2e to
01ef7c6
Compare
| let tycho_api_key: Option<String> = env::var("TYCHO_API_KEY").ok(); | ||
| let no_tls = tycho_api_key.is_none(); | ||
| if no_tls { | ||
| eprintln!("Warning: TYCHO_API_KEY not set. Using plain HTTP/WS (no TLS)."); | ||
| } |
There was a problem hiding this comment.
I think it's overkill and not safe. Only useful when using on a local tycho instance no?
If yes then I'd rather make it a cli arg to manually disable it than risking external people using no TLS without noticing just because they forgot to set an env variable
There was a problem hiding this comment.
Only useful when using on a local tycho instance no?
Yes, for self-hosted or port-forwarded cases
Add Chain::Polygon and Chain::Arbitrum arms to price_printer and quickstart examples with their available exchanges. Auto-detect TLS based on TYCHO_API_KEY presence to support local indexer instances. ENG-5644 ENG-5612 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… flag Addresses PR review: auto-detecting TLS from TYCHO_API_KEY absence risks users accidentally running without TLS. Now requires an explicit --no-tls flag, with a warning when disabled. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
f0cdb9c to
4a0729e
Compare
| let no_tls = cli.no_tls; | ||
| if no_tls { | ||
| eprintln!("Warning: TLS is disabled. Only use this for local/self-hosted Tycho instances."); | ||
| } |
There was a problem hiding this comment.
The intermediate var is pretty shallow tbh... I would skip it and just use cli.no_tls directly.
| let no_tls = cli.no_tls; | |
| if no_tls { | |
| eprintln!("Warning: TLS is disabled. Only use this for local/self-hosted Tycho instances."); | |
| } | |
| if cli.no_tls { | |
| eprintln!("Warning: TLS is disabled. Only use this for local/self-hosted Tycho instances."); | |
| } |
| // Perform an early check to ensure `RPC_URL` is set. | ||
| // This prevents errors from occurring later during UI interactions for curve. |
There was a problem hiding this comment.
Why remove this? I like that this indicated to new users that this is needed for curve specifically.
| let no_tls = cli.no_tls; | ||
| if no_tls { | ||
| eprintln!("Warning: TLS is disabled. Only use this for local/self-hosted Tycho instances."); | ||
| } |
There was a problem hiding this comment.
Same comment as above
Summary
Chain::Polygonsupport toprice_printerandquickstartexamples with exchanges:uniswap_v2,quickswap_v2,uniswap_v3,uniswap_v4Chain::Arbitrumsupport with exchanges:uniswap_v2,uniswap_v3,pancakeswap_v3,uniswap_v4TYCHO_API_KEYpresence — when unset, uses plain HTTP/WS and prints a warning, enabling local indexer usageENG-5644 ENG-5612
Test plan
cargo checkpasses for both examplesprice_printeragainst local Polygon indexer atlocalhost:4242🤖 Generated with Claude Code