-
Notifications
You must be signed in to change notification settings - Fork 21
Add update redeemable positions command #630
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: master
Are you sure you want to change the base?
Conversation
Signed-off-by: cyc60 <avsysoev60@gmail.com>
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.
Pull request overview
This PR adds a new command to update redeemable positions for users who have minted osToken shares. The command fetches allocator data from the graph, filters out leverage positions, calculates kept tokens (both in wallets and locked in protocols), determines redeemable amounts, and uploads the results to IPFS.
Key Changes:
- Introduces the
update_redeemable_positionscommand with supporting graph queries and API client - Adds ERC20 contract wrapper and OS_TOKEN_CONTRACT_ADDRESS configuration for each network
- Expands IPFS configuration to support multiple upload clients (local, Infura, Pinata)
Reviewed changes
Copilot reviewed 11 out of 13 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/commands/internal/update_redeemable_positions.py | Main command implementation to fetch, calculate, and upload redeemable positions |
| src/redeem/typings.py | Data classes for Allocator and RedeemablePosition entities |
| src/redeem/graph.py | Graph queries to fetch allocators and leverage position proxies |
| src/redeem/api_client.py | API client to fetch locked osToken data from external protocol |
| src/common/contracts.py | ERC20 contract wrapper for token balance queries |
| src/common/clients.py | IPFS upload client builder supporting multiple providers |
| src/config/settings.py | IPFS upload settings and increased graph timeout/page size defaults |
| src/config/networks.py | OS_TOKEN_CONTRACT_ADDRESS added for each network |
| src/main.py | Command registration in CLI |
| src/reward_splitter/graph.py | Added pagination parameters to existing graph queries |
| src/common/abi/Erc20Token.json | ERC20 ABI definition |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: cyc60 <avsysoev60@gmail.com>
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.
Pull request overview
Copilot reviewed 13 out of 16 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| async def _fetch_json(self, url: str, params: dict | None = None) -> dict | list: | ||
| async with aiohttp.ClientSession() as session: | ||
| async with session.get( | ||
| url=url, | ||
| params=params, | ||
| headers={'user-agent': DEFAULT_USER_AGENT}, | ||
| ) as response: | ||
| response.raise_for_status() | ||
| return await response.json() |
Copilot
AI
Jan 12, 2026
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.
Creating a new aiohttp.ClientSession for each request is inefficient. Create the session once in init or use a session pool to reuse connections across multiple API calls.
Signed-off-by: cyc60 <avsysoev60@gmail.com>
Signed-off-by: cyc60 <avsysoev60@gmail.com>
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.
Pull request overview
Copilot reviewed 13 out of 16 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: cyc60 <avsysoev60@gmail.com>
Signed-off-by: cyc60 <avsysoev60@gmail.com>
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.
Pull request overview
Copilot reviewed 14 out of 18 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: cyc60 <avsysoev60@gmail.com>
Signed-off-by: cyc60 <avsysoev60@gmail.com>
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.
Pull request overview
Copilot reviewed 14 out of 18 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: cyc60 <avsysoev60@gmail.com> # Conflicts: # poetry.lock
Signed-off-by: cyc60 <avsysoev60@gmail.com>
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.
Pull request overview
Copilot reviewed 14 out of 18 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Pull request overview
Copilot reviewed 14 out of 18 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Pull request overview
Copilot reviewed 18 out of 23 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: cyc60 <avsysoev60@gmail.com>
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.
Pull request overview
Copilot reviewed 19 out of 24 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: cyc60 <avsysoev60@gmail.com>
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.
Pull request overview
Copilot reviewed 19 out of 24 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Code reviewFound 2 issues:
v3-operator/src/common/contracts.py Lines 346 to 349 in 921dcd2
v3-operator/src/config/networks.py Lines 20 to 52 in 921dcd2
v3-operator/src/commands/internal/update_redeemable_positions.py Lines 153 to 226 in 921dcd2
🤖 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
Signed-off-by: cyc60 <avsysoev60@gmail.com>
Signed-off-by: cyc60 <avsysoev60@gmail.com>
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.
Pull request overview
Copilot reviewed 19 out of 24 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: cyc60 <avsysoev60@gmail.com>
Signed-off-by: cyc60 <avsysoev60@gmail.com>
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.
Pull request overview
Copilot reviewed 19 out of 24 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: cyc60 <avsysoev60@gmail.com>
Signed-off-by: cyc60 <avsysoev60@gmail.com>
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.
Pull request overview
Copilot reviewed 19 out of 24 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
src/commands/internal/update_redeemable_positions.py:1
- Corrected spelling of 'cliente' to 'client'.
import asyncio
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: cyc60 <avsysoev60@gmail.com>
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.
Pull request overview
Copilot reviewed 19 out of 24 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: cyc60 <avsysoev60@gmail.com>
| 'Fetching %s from Arbitrum wallet balances...', | ||
| settings.network_config.OS_TOKEN_BALANCE_SYMBOL, | ||
| ) | ||
| arbitrum_endpoint = cast(str, arbitrum_endpoint) |
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.
(optional) You have 2 separate checks for arb contract address and arb endpoint. Btw they should be consistent, both empty or both non-empty.
Add NetworkConfig.ArbitrumConfig dataclass. Then you could check ArbitrumConfig is not None
@dataclass
class ArbitrumConfig:
OS_TOKEN_CONTRACT_ADDRESS
ENDPOINT: str
| if position.vault not in boosted_positions[position.user]: | ||
| boosted_positions[position.user][position.vault] = Wei(0) | ||
| boosted_positions[position.user][position.vault] = Wei( | ||
| boosted_positions[position.user][position.vault] + position_os_token_shares | ||
| ) |
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.
Use tuples as keys. Start from defaultdict.
| if position.vault not in boosted_positions[position.user]: | |
| boosted_positions[position.user][position.vault] = Wei(0) | |
| boosted_positions[position.user][position.vault] = Wei( | |
| boosted_positions[position.user][position.vault] + position_os_token_shares | |
| ) | |
| boosted_positions: defaultdict[tuple[ChecksumAddress, ChecksumAddress], Wei] = defaultdict(lambda: Wei(0)) | |
| for ... | |
| boosted_positions[position.user, position.vault] = Wei( | |
| boosted_positions[position.user, position.vault] + position_os_token_shares | |
| ) |
| vault_1 = faker.eth_address() | ||
| vault_2 = faker.eth_address() | ||
|
|
||
| # test zero allocators |
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.
Please split to test-cases. One function is one test-case. For this test and all tests below.
evgeny-stakewise
left a comment
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.
Added comments
No description provided.