-
Notifications
You must be signed in to change notification settings - Fork 18
Inference statistics #26
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
base: main
Are you sure you want to change the base?
Changes from all commits
967b196
d86e0e2
be0525e
abedbc1
6d98a1b
0d19bb0
69d6fce
b5d32dc
cfcfd4a
33d725f
92fcf84
90102c3
4bbad43
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,6 +12,7 @@ | |
| from aggregator.errors import InvalidProofError | ||
| from aggregator.server import AggregatorServer | ||
| from common.abis import ERC20_ABI, STRATEGY_ABI | ||
| from common.addresses import addresses | ||
| from common.auto_update import AutoUpdate | ||
| from common.config import AggregatorConfig | ||
| from common.constants import ( | ||
|
|
@@ -31,6 +32,7 @@ | |
|
|
||
|
|
||
| def run_aggregator(config: AggregatorConfig) -> None: | ||
| addresses.init_addresses(chain_id=config.chain_id) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chainVerify error handling for address initialization. The address initialization is correctly placed before any component that might need addresses. However, ensure proper error handling for initialization failures. Please verify:
🏁 Script executed: #!/bin/bash
# Check if init_addresses handles being called multiple times
ast-grep --pattern 'def init_addresses($$$) {
$$$
}'
# Check for any guard against multiple initialization
rg -n -A5 -B5 'def init_addresses' client/src/common/addresses.py
# Look for chain_id validation in config
rg -n -C3 'chain_id' client/src/common/config.pyLength of output: 776 I need to gather more detailed information about the Now I need to examine the actual implementation of I need to examine the actual implementation more carefully. Let me search for the complete context: Based on my verification through the script output and web research, I can now generate the rewritten review comment: Add error handling for address initialization failures and verify initialization safety. The
🏁 Script executed: #!/bin/bash
# Get full init_addresses implementation
rg -n -A20 'def init_addresses' client/src/common/addresses.py
# Check how addresses is imported and used in main.py
rg -n -B2 -A10 'addresses' client/src/aggregator/main.py | head -50Length of output: 140 Let me search for these files in the repository: 🏁 Script executed: #!/bin/bash
# Find addresses-related files
fd -name "addresses.py"
# Find aggregator main.py
fd -name "main.py" -path "*/aggregator/*"
# List directory structure to understand repo layout
find . -maxdepth 3 -type d -name "client" -o -name "aggregator" -o -name "common" 2>/dev/null | head -20Length of output: 514 Let me search for these files with corrected paths: 🏁 Script executed: #!/bin/bash
# Find addresses.py
fd addresses.py ./client/src/common
# Find main.py in aggregator
fd main.py ./client/src/aggregator
# Get full init_addresses implementation
cat ./client/src/common/addresses.py | grep -n -A 30 "def init_addresses"
# Check how addresses is used in main.py
cat ./client/src/aggregator/main.py | grep -n -B 3 -A 5 "addresses"Length of output: 3322 Add guard clause to prevent multiple initializations. Error handling for Suggested fix: Add a guard at the start of def init_addresses(self, chain_id: int) -> None:
if self._initialized:
return
# ... rest of implementationThe 🤖 Prompt for AI Agents |
||
| logger.info("Starting Sertn Aggregator...") | ||
| aggregator = Aggregator(config=config) | ||
| threading.Thread(target=aggregator.start_sending_new_tasks, args=[]).start() | ||
|
|
||
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.
🧩 Analysis chain
Verify Starlette dependency necessity.
Starlette is already a transitive dependency of FastAPI (line 14). Adding it explicitly could be intentional for version pinning, but may also introduce version conflicts if FastAPI expects a different Starlette version.
Please verify:
🏁 Script executed:
Length of output: 2168
I need to search more specifically for FastAPI 0.115.12's Starlette requirements to get a definitive answer.
Based on my verification, I've found a critical issue with the Starlette dependency constraint.
Critical: Starlette version 0.49.1 is incompatible with FastAPI 0.115.12.
FastAPI 0.115.12 requires starlette >=0.40.0,<0.47.0, but the added constraint specifies
starlette>=0.49.1, which falls outside the upper bound. This will cause dependency resolution failures during installation.This appears to be a typo. Verify the intended Starlette version—likely should be
>=0.40.1or similar to align with FastAPI's compatibility range.🤖 Prompt for AI Agents