Conversation
Signed-off-by: cyc60 <avsysoev60@gmail.com>
Signed-off-by: cyc60 <avsysoev60@gmail.com>
There was a problem hiding this comment.
Pull request overview
Adds an operator-side “state sync” flow to keep an individual operator’s on-chain state aligned with the latest globally-published NodesManager state (fetched from IPFS), optionally batching a vault harvest via multicall.
Changes:
- Introduces
StateSyncTaskto detect when the operator’s nonce lags the global nonce and submitupdateOperatorState(optionally withupdateVaultState). - Adds IPFS fetch + tx submission helpers (
fetch_operator_state_from_ipfs,submit_state_sync_transaction) and a new params typing. - Extends NodesManager contract wrapper/encoder to support querying state update events and encoding operator/vault update calls.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/node_manager/typings.py |
Adds OperatorStateUpdateParams dataclass for updateOperatorState. |
src/node_manager/tasks.py |
Adds StateSyncTask periodic loop that detects new global state and syncs the operator. |
src/node_manager/execution.py |
New helper module to fetch operator params from IPFS and submit the sync transaction (with optional multicall). |
src/common/contracts.py |
Adds NodesManager event query + nonce helpers, and encoder methods for operator/vault state updates. |
src/common/app_state.py |
Adds in-memory checkpoint (state_sync_block) for event scanning. |
src/commands/node_manager_start.py |
Runs the new StateSyncTask alongside the existing NodeManagerTask. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| """Fetch operator state data from IPFS and return update params if found.""" | ||
| ipfs_data = await ipfs_fetch_client.fetch_json(ipfs_hash) | ||
|
|
||
| for operator_data in ipfs_data.get('operators', []): # type: ignore |
There was a problem hiding this comment.
ipfs_fetch_client.fetch_json() is typed to return dict | list, but this code assumes a dict and calls .get('operators', ...). If IPFS returns a JSON array (or otherwise unexpected shape), this will raise at runtime. Consider validating ipfs_data is a dict with an operators key (and logging/returning None on unexpected data) before iterating.
| for operator_data in ipfs_data.get('operators', []): # type: ignore | |
| if not isinstance(ipfs_data, dict): | |
| logger.error( | |
| "Unexpected IPFS data type for hash %s: expected dict, got %s", | |
| ipfs_hash, | |
| type(ipfs_data).__name__, | |
| ) | |
| return None | |
| operators = ipfs_data.get('operators') | |
| if not isinstance(operators, list): | |
| logger.error( | |
| "Missing or invalid 'operators' field in IPFS data for hash %s", | |
| ipfs_hash, | |
| ) | |
| return None | |
| for operator_data in operators: |
| return OperatorStateUpdateParams( | ||
| total_assets=int(operator_data['totalAssets']), | ||
| cum_penalty_assets=int(operator_data['cumPenaltyAssets']), | ||
| cum_earned_fee_shares=int(operator_data['cumEarnedFeeShares']), | ||
| proof=[HexBytes(Web3.to_bytes(hexstr=HexStr(p))) for p in operator_data['proof']], | ||
| ) |
There was a problem hiding this comment.
The contract ABI expects totalAssets, cumPenaltyAssets, and cumEarnedFeeShares as uint128 and the proof as bytes32[], but the values parsed from IPFS are not validated. A negative/oversized int or a proof element not exactly 32 bytes will lead to an on-chain revert (wasting gas). Add explicit validation (0 <= value < 2**128, len(proof_item)==32) and fail fast with a clear log message when the IPFS payload is malformed.
| class StateSyncTask(BaseTask): | ||
| """Periodically syncs operator state after global state updates.""" | ||
|
|
||
| def __init__(self, operator_address: ChecksumAddress) -> None: | ||
| self.operator_address = operator_address | ||
|
|
||
| async def process_block(self, interrupt_handler: InterruptHandler) -> None: | ||
| nm_contract = NodesManagerContract() | ||
| app_state = AppState() |
There was a problem hiding this comment.
This PR introduces a new periodic task (StateSyncTask) and IPFS parsing/transaction submission logic, but there are no accompanying tests. The repo already has task-level tests for other modules (e.g. src/meta_vault/tests/test_tasks.py, src/withdrawals/tests/test_tasks.py). Consider adding unit tests that mock NodesManagerContract, ipfs_fetch_client, and transaction_gas_wrapper to cover: already-synced path, missing operator in IPFS, harvest+multicall path, and failed receipt status.
After the global state is updated (every 24 hours), the operator must sync their individual state:
StateUpdatedevents or pollGET /nodes-manager/state-voteto detect new state updatestotalAssets,cumPenaltyAssets,cumEarnedFeeShares, and merkle proof)updateVaultState(harvestParams)firstNodesManager.updateOperatorState(params)with the merkle proof — can be batched viamulticallThis should run as a periodic task. The operator must stay synced to the latest nonce — it's a prerequisite for entering the exit queue and claiming assets.