Conversation
WalkthroughThis update unifies the operator set abstraction across the codebase by replacing protocol-specific operator set handling (EigenLayer, Symbiotic) with a single "LinglongSubset" system using Changes
Sequence Diagram(s)sequenceDiagram
participant Deployer
participant DeployScript
participant RegistryCoordinator
participant Middleware
Deployer->>DeployScript: run(configFileName, minCollateral)
DeployScript->>RegistryCoordinator: createLinglongSubset(subsetId, minStake)
DeployScript->>Middleware: initialize with LinglongSubset IDs
Middleware->>RegistryCoordinator: isOperatorInLinglongSubset(subsetId, operator)
RegistryCoordinator-->>Middleware: returns membership status
Suggested reviewers
Poem
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🔭 Outside diff range comments (3)
src/slasher/LinglongSlasher.sol (1)
481-495:⚠️ Potential issuePossible silent slip-through when operator is not in any Symbiotic subset
subnetworkdefaults to0.
If the operator is not found in eitherSYMBIOTIC_VALIDATOR_SUBSET_IDorSYMBIOTIC_UNDERWRITER_SUBSET_ID, the loop exits withsubnetwork == 0, yet execution continues, potentially slashing the wrong subset.for (uint256 i = 0; i < subnetworks.length; i++) { uint32 subsetId = subnetworks[i]; if ( ITaiyiRegistryCoordinator(TAIYI_REGISTRY_COORDINATOR) - .isOperatorInLinglongSubset(baseId, operator) + .isOperatorInLinglongSubset(subsetId, operator) ) { - subnetwork = subnetworks[i]; + subnetwork = subsetId; break; } } +if (subnetwork == 0) { + // Operator is not part of any recognised Symbiotic subset + revert OperatorNotInSet(operator, 0); +}Fail-fast behaviour prevents accidental use of an invalid (zero) subset ID.
src/interfaces/ITaiyiRegistryCoordinator.sol (1)
40-44:⚠️ Potential issueMismatch between new subset model and type of
symbioticSets
AllocatedOperatorSets.symbioticSetsis still declared asuint96[], while every Symbiotic-related function now usesuint32Linglong subset IDs.
Unless there is a legacy path that still relies on the olduint96encoding, this type divergence will:
- Force unnecessary casts in the implementation.
- Risk silent truncation or overflow when a
uint96value >uint32.maxis stored in localuint32variables.- uint96[] symbioticSets; // Symbiotic operator set IDs + uint32[] symbioticSets; // Symbiotic operator subset IDssrc/operator-registries/TaiyiRegistryCoordinator.sol (1)
694-704:⚠️ Potential issueOperator can register to a non-existent subset with zero minStake
The EigenLayer registration path:
- Reads
_linglongSubsets.getMinStake(subsetId)– returns0for unknown IDs.- Accepts the registration if
stake >= 0(always true).- Adds the operator to the subset set, which creates the subset on-the-fly.
This bypasses the admin-set
minStakeand defeats economic gating.Fix by asserting the subset exists and has a configured
minStake:require( _linglongSubsets.isLinglongSubsetIdCreated(subsetId), "Subset not initialised" ); uint256 minStake = _linglongSubsets.getMinStake(subsetId); require(minStake > 0, "Min stake not set"); require(stakes[i] >= minStake, "Stake below minimum");
🧹 Nitpick comments (12)
src/interfaces/ILinglongSlasher.sol (1)
135-144: NatSpec comment still references “operator set ID” instead of newlinglongSubsetId.The parameter has been renamed and its type narrowed to
uint32, but the descriptive text (/// @param linglongSubsetId The operator set ID) still uses the old phrase.
Updating the wording improves self-documentation and avoids confusion for integrators.-/// @param linglongSubsetId The operator set ID +/// @param linglongSubsetId The Linglong subset IDtest/EigenLayerMiddleware.t.sol (1)
592-599: Redundant assertion & magic constant.
baseOperatorSetIdis alreadyopSet.id; asserting it equalsuint32(0)neither tests production logic nor guards against regressions— it only repeats the constant chosen in setup.
Removing the assertion (or comparing to the named constant) keeps the test intention clear.-assertEq(baseOperatorSetId, uint32(0), "Operator set ID should match"); +assertEq( + baseOperatorSetId, + OperatorSubsetLib.EIGENLAYER_VALIDATOR_SUBSET_ID, + "Operator set ID should match validator subset constant" +);test/SymbioticMiddleware.t.sol (1)
139-146: Variable rename is fine, but unused legacy constants remain.
VALIDATOR_SUBNETWORK/UNDERWRITER_SUBNETWORKconstants (lines 62-63) are no longer referenced after the switch toOperatorSubsetLib.*constants.
Keeping dead constants invites drift between specs & tests—consider deleting them to prevent accidental use.src/libs/SymbioticNetworkMiddlewareLib.sol (1)
18-23: Duplicate import & repetitive helper – clean-up needed.
SafeCast96To32Libis imported twice (lines 18 & 20). Solidity allows dupes but this adds noise and hints at merge-conflict remnants. Remove the extra line to keep the file tidy.-import { SafeCast96To32Lib } from "./SafeCast96To32Lib.sol"; ... -import { SafeCast96To32Lib } from "./SafeCast96To32Lib.sol";src/eigenlayer-avs/EigenLayerMiddleware.sol (1)
104-106: Avoid silent failure – bubble-up the membership error reasonWhen
isOperatorInLinglongSubsetreturnsfalse, you revert with a custom error that implicitly targets the validator subset.
If tomorrow we reuse the modifier for a different subset, the revert text may become misleading. Consider parameterising the error or using the return-value ofisOperatorInLinglongSubsetto compose an accurate revert reason.src/symbiotic-network/SymbioticNetworkMiddleware.sol (1)
74-78: Subset guard looks good, but consider library-level reuseThe validator-subset gate mirrors the one inside
EigenLayerMiddleware.
Extracting it intoOperatorSubsetLib(e.g.,assertValidator(address, ITaiyiRegistryCoordinator)) would keep both middleware contracts consistent and avoid drift.src/libs/EigenLayerMiddlewareLib.sol (2)
270-272: Out-of-date inline commentThe line still claims we “Get the current operator set count from allocationManager”, but the call was removed in the refactor. Please update the comment to avoid future confusion.
79-83: Variable name no longer reflects new semantics
baseIdis now a plain Linglong subset ID, not a “base-encoded” value.
Renaming tosubsetId(or similar) will improve readability across the file.Also applies to: 98-103
src/slasher/LinglongSlasher.sol (1)
380-398: Gas-heavy external self-call remains — consider internalising
_checkSlashingNotInProgressperformsthis.isSlashingInProgress, causing an external call to the same contract (extra 2.3k gas + re-entrancy surface).
Since the function being called isview, consider making an internal version that re-uses the storage directly:-(bool inProgress,) = this.isSlashingInProgress(operator, linglongSubsetId, challengerContract); +(bool inProgress,) = _isSlashingInProgress(operator, linglongSubsetId, challengerContract);…and move the existing logic into a private helper.
test/OperatorSubsetLib.t.sol (1)
20-24: Duplicate constants risk drift from library sourceThe four subset-ID constants are already defined in
OperatorSubsetLib; redeclaring them here introduces maintenance overhead and potential divergence.-uint32 internal constant EIGENLAYER_VALIDATOR_SUBSET_ID = 0; -uint32 internal constant EIGENLAYER_UNDERWRITER_SUBSET_ID = 1; -uint32 internal constant SYMBIOTIC_VALIDATOR_SUBSET_ID = 2; -uint32 internal constant SYMBIOTIC_UNDERWRITER_SUBSET_ID = 3; +uint32 internal constant EIGENLAYER_VALIDATOR_SUBSET_ID = + OperatorSubsetLib.EIGENLAYER_VALIDATOR_SUBSET_ID; +uint32 internal constant EIGENLAYER_UNDERWRITER_SUBSET_ID = + OperatorSubsetLib.EIGENLAYER_UNDERWRITER_SUBSET_ID; +uint32 internal constant SYMBIOTIC_VALIDATOR_SUBSET_ID = + OperatorSubsetLib.SYMBIOTIC_VALIDATOR_SUBSET_ID; +uint32 internal constant SYMBIOTIC_UNDERWRITER_SUBSET_ID = + OperatorSubsetLib.SYMBIOTIC_UNDERWRITER_SUBSET_ID;Keeps the test suite aligned with the single source of truth.
src/interfaces/ITaiyiRegistryCoordinator.sol (1)
90-95: Guard modifiers use custom errors that do not match the check they perform
onlySymbioticSubsetIdandonlyEigenLayerSubsetIdrevert withOnlySymbioticSubsetId()andOnlyEigenlayerSubsetId()respectively.
Because these modifiers are positive-logic checks (“require(isSymbioticProtocolID … )”) the error name should convey that an invalid ID was supplied; otherwise the revert reason is misleading to integrators and tooling that parse custom errors.
Consider renaming toInvalidSymbioticSubsetId/InvalidEigenlayerSubsetIdor invert the logic and message.[nitpick]
src/operator-registries/TaiyiRegistryCoordinator.sol (1)
360-366: Gas-heavy subset size computation
getLinglongSubsetSizefetches the whole operators array then reads.length.
Use the dedicated helper to avoid unnecessary memory allocation:- return _linglongSubsets.getOperatorsInLinglongSubset(linglongSubsetId).length; + return _linglongSubsets.getLinglongSubsetLength(linglongSubsetId);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
script/Deployments.s.sol(5 hunks)script/SetupContract.s.sol(1 hunks)script/deploy.sh(1 hunks)src/eigenlayer-avs/EigenLayerMiddleware.sol(5 hunks)src/eigenlayer-avs/EigenLayerRewardsHandler.sol(3 hunks)src/interfaces/ILinglongChallenger.sol(1 hunks)src/interfaces/ILinglongSlasher.sol(1 hunks)src/interfaces/ITaiyiRegistryCoordinator.sol(9 hunks)src/libs/EigenLayerMiddlewareLib.sol(4 hunks)src/libs/OperatorSubsetLib.sol(1 hunks)src/libs/SymbioticNetworkMiddlewareLib.sol(2 hunks)src/operator-registries/TaiyiRegistryCoordinator.sol(25 hunks)src/slasher/LinglongSlasher.sol(6 hunks)src/storage/TaiyiRegistryCoordinatorStorage.sol(1 hunks)src/symbiotic-network/SymbioticNetworkMiddleware.sol(3 hunks)src/taiyi/TaiyiInteractiveChallenger.sol(1 hunks)test/DeployTest.t.sol(1 hunks)test/EigenLayerMiddleware.t.sol(3 hunks)test/OperatorSubsetLib.t.sol(1 hunks)test/SymbioticMiddleware.t.sol(3 hunks)test/utils/MockChallenger.sol(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Foundry Project
🔇 Additional comments (15)
script/SetupContract.s.sol (2)
120-120: Updated parameter type to use new Linglong subset identifierThis change replaces the previous
VALIDATOR_SUBSET_TYPEconstant with the newEIGENLAYER_VALIDATOR_SUBSET_IDfrom theOperatorSubsetLib, aligning with the unified operator set abstraction usinguint32IDs.
123-123: Updated parameter type to use new Linglong subset identifierThis change replaces the previous
UNDERWRITER_SUBSET_TYPEconstant with the newEIGENLAYER_UNDERWRITER_SUBSET_IDfrom theOperatorSubsetLib, aligning with the unified operator set abstraction usinguint32IDs.test/utils/MockChallenger.sol (1)
75-75: Updated parameter type from uint96 to uint32The parameter type has been changed from
uint96touint32to align with the operator set refactoring that usesuint32Linglong subset IDs instead of the previousuint96operator set IDs. This change ensures the mock implementation is compatible with the updated interfaces.test/DeployTest.t.sol (1)
37-37: Updated call to include required minCollateral parameterThe
deploy.run()call now includes an additional0.1 etherparameter for the minimum collateral requirement, matching the updated function signature inscript/Deployments.s.sol.src/interfaces/ILinglongChallenger.sol (1)
116-121: Type change approved: Migrated fromuint96 operatorSetIdtouint32 linglongSubsetIdThis change is part of the broader refactor to unify operator set identifiers under the "LinglongSubset" concept with
uint32IDs. The parameter comment has been appropriately updated to reflect this change.src/eigenlayer-avs/EigenLayerRewardsHandler.sol (3)
18-18: Approved: Added import for OperatorSubsetLibThe import provides access to the subset ID constants used in the updated function calls below.
120-120: Approved: Updated to use unified LinglongSubset systemChanged from using hardcoded
getEigenLayerOperatorSetOperators(uint32(0))to the more semanticgetLinglongSubsetOperators(OperatorSubsetLib.EIGENLAYER_UNDERWRITER_SUBSET_ID), improving code clarity and maintainability.
163-163: Approved: Updated to use unified LinglongSubset systemChanged from using hardcoded
getEigenLayerOperatorSetOperators(uint32(0))to the more semanticgetLinglongSubsetOperators(OperatorSubsetLib.EIGENLAYER_VALIDATOR_SUBSET_ID), ensuring consistency in the operator subset access pattern.script/deploy.sh (1)
18-19: Approved: Updated deployment script signature and parametersThe deployment script now accepts an additional
minCollateralparameter set to0.1ether. This aligns with updated function signatures in the deployment scripts that now require explicit collateral values instead of using hardcoded defaults.src/taiyi/TaiyiInteractiveChallenger.sol (1)
518-518: Approved: Updated parameter type to match interface changesChanged parameter from
uint96 operatorSetIdtouint32 linglongSubsetIdto align with the interface changes and the unified operator subset model using LinglongSubset IDs.test/EigenLayerMiddleware.t.sol (1)
463-467: Hard-coding subset IDs may collide across multiple AVSes.
OperatorSubsetLib.EIGENLAYER_VALIDATOR_SUBSET_IDand…_UNDERWRITER_SUBSET_IDare currently0and1.
If another AVS in the same AllocationManager re-uses those constants,createOperatorSetwill revert (OperatorSetAlreadyExists). Consider deriving a namespace-unique ID (e.g., hashingaddress(this)with a constant) or letting the middleware allocate an unused ID.test/SymbioticMiddleware.t.sol (1)
545-552:Details
❌ Incorrect review comment
Return value of
createNewSubnetworkignored – potential mismatch.
middleware.createNewSubnetwork(...)likely returns the actual subnetwork ID assigned on-chain.
Relying on pre-defined constants and discarding the return value will silently break if the middleware ever chooses a different ID (e.g., due to an existing subset). Capture and assert instead:- middleware.createNewSubnetwork(OperatorSubsetLib.SYMBIOTIC_VALIDATOR_SUBSET_ID, 0); - middleware.createNewSubnetwork( - OperatorSubsetLib.SYMBIOTIC_UNDERWRITER_SUBSET_ID, 0 - ); - - validatorSubnetworkId = OperatorSubsetLib.SYMBIOTIC_VALIDATOR_SUBSET_ID; - underwriterSubnetworkId = OperatorSubsetLib.SYMBIOTIC_UNDERWRITER_SUBSET_ID; + validatorSubnetworkId = + middleware.createNewSubnetwork( + OperatorSubsetLib.SYMBIOTIC_VALIDATOR_SUBSET_ID, + 0 + ); + underwriterSubnetworkId = + middleware.createNewSubnetwork( + OperatorSubsetLib.SYMBIOTIC_UNDERWRITER_SUBSET_ID, + 0 + ); + + // Sanity-check that the middleware respected the requested IDs + assertEq( + validatorSubnetworkId, + OperatorSubsetLib.SYMBIOTIC_VALIDATOR_SUBSET_ID, + "Validator subnetwork ID mismatch" + ); + assertEq( + underwriterSubnetworkId, + OperatorSubsetLib.SYMBIOTIC_UNDERWRITER_SUBSET_ID, + "Underwriter subnetwork ID mismatch" + );
I’ll verify whether
createNewSubnetworkactually returns the new subnetwork ID by locating its definition in the codebase.
🏁 Script executed:
#!/bin/bash # Locate the createNewSubnetwork function signature rg -n "function createNewSubnetwork" -n .Length of output: 136
🏁 Script executed:
#!/bin/bash # Display the createNewSubnetwork signature and return type rg -A3 -n "function createNewSubnetwork" src/symbiotic-network/SymbioticNetworkMiddleware.solLength of output: 209
🏁 Script executed:
#!/bin/bash # Display surrounding context to inspect return type of createNewSubnetwork rg -C5 -n "function createNewSubnetwork" src/symbiotic-network/SymbioticNetworkMiddleware.solLength of output: 609
🏁 Script executed:
#!/bin/bash # Show extended context around the createNewSubnetwork signature to check for a return type rg -C8 -n "function createNewSubnetwork" src/symbiotic-network/SymbioticNetworkMiddleware.solLength of output: 845
No return value to capture –
createNewSubnetworkdoesn’t return an ID.The suggestion to assign and assert its return value isn’t applicable: in
src/symbiotic-network/SymbioticNetworkMiddleware.sol(around lines 124–130) thecreateNewSubnetworkfunction is declared without anyreturns (…)clause, so it doesn’t emit a value. The existing test correctly relies on the predefined constants.Please ignore the proposed assignment/
assertEqchanges.Likely an incorrect or invalid review comment.
script/Deployments.s.sol (2)
307-308: Guard against zero-collateral deploymentsPassing a zero value defeats the whole purpose of enforcing minimum collateral.
require(urcMinCollateral > 0, "minCollateral must be > 0");Place the check immediately before populating
IEigenLayerMiddleware.Config.
398-400: Signature change ripple-effect
run(string,uint256)is a breaking change for every script that previously calledrun(string).Please ensure:
- All CI scripts (
.github/workflows, Makefile, bash helpers) are updated.- Any README examples or docs are adjusted.
src/operator-registries/TaiyiRegistryCoordinator.sol (1)
210-218: Subset creation callable by any registered middleware, not only Symbiotic
createLinglongSubsetis protected byonlyMiddleware, which permits botheigenLayerMiddlewareandsymbioticMiddleware.
Creating subsets is only meaningful for Symbiotic (EigenLayer subsets are fixed at 0 & 1). Allowing the EigenLayer middleware to call this function opens the door to inconsistent state or accidental clashes.Either:
- Change the modifier to
onlySymbioticMiddleware, or- Add a runtime check inside the function.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/operator-registries/TaiyiRegistryCoordinator.sol (1)
780-785: Hard-coded deregistration assumes both Symbiotic subsets have been created.If either subset 2 or 3 was never initialised,
removeOperatorFromLinglongSubsetsreverts, blocking deregistration entirely.Option A: iterate over existing subsets obtained from storage.
Option B: guard each removal withisLinglongSubsetIdCreated.
🧹 Nitpick comments (1)
src/operator-registries/TaiyiRegistryCoordinator.sol (1)
713-745: Rigid subset ID requirements may limit flexibility.The EigenLayer deregistration function has very specific requirements about the number and specific IDs of subsets.
Consider generalizing this to handle variable subsets or validating against the actual registered subsets for the operator, which would make the code more flexible for future expansion.
- require( - _linglongSubsetIds.length == 2, - "Eigenlayer deregistration requires exactly one subset" - ); - - require( - _linglongSubsetIds[0] == OperatorSubsetLib.EIGENLAYER_VALIDATOR_SUBSET_ID - && _linglongSubsetIds[1] == OperatorSubsetLib.EIGENLAYER_UNDERWRITER_SUBSET_ID, - "Invalid eigenlayer subset IDs" - ); + // Validate that all subset IDs are EigenLayer protocol IDs + for (uint256 i = 0; i < _linglongSubsetIds.length; i++) { + require( + OperatorSubsetLib.isEigenlayerProtocolID(_linglongSubsetIds[i]), + "Invalid eigenlayer subset ID" + ); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (3)
script/Deployments.s.sol(6 hunks)src/operator-registries/TaiyiRegistryCoordinator.sol(25 hunks)src/storage/TaiyiRegistryCoordinatorStorage.sol(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- script/Deployments.s.sol
- src/storage/TaiyiRegistryCoordinatorStorage.sol
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Foundry Project
🔇 Additional comments (11)
src/operator-registries/TaiyiRegistryCoordinator.sol (11)
48-52: Clear and improved NatSpec documentation.The updated NatSpec comments provide a clearer description of the contract's purpose and the registries it maintains.
65-65: Successfully refactored to use LinglongSubsets.The storage usage has been updated to use the unified
OperatorSubsetLib.LinglongSubsetsabstraction instead of separate operator sets, aligning with the PR's objective.
82-88: Updated middleware access control.The
onlyMiddlewaremodifier correctly restricts function access to registered middleware contracts, supporting the new unified approach.
90-106: Protocol-specific modifiers implement clean separation.The new
onlySymbioticSubsetIdandonlyEigenLayerSubsetIdmodifiers properly validate the protocol type of Linglong subset IDs, enforcing type safety throughout the contract.
158-176: Registration function successfully updated to use subset IDs.The
registerOperatorfunction has been correctly updated to handle the new LinglongSubset abstraction.
207-218: Clean subset creation implementation.The
createLinglongSubsetfunction properly delegates to the LinglongSubsets library with appropriate access control.
268-277: Well-implemented subnet retrieval.The
getLinglongSubnetsfunction correctly retrieves and converts all subnet IDs to the expected return type.
308-321: Clean operator membership check using new abstraction.The
isOperatorInLinglongSubsetfunction correctly delegates to the LinglongSubsets library.
663-711: EigenLayer registration properly validates subset types.The function correctly ensures that only EigenLayer protocol subset IDs are used and that stake requirements are met.
747-770: Symbiotic registration properly validates subset types.The function correctly ensures that only Symbiotic protocol subset IDs are used during registration.
787-834: Stake validation adapts well to new subset model.The
_checkInitialEigenStakefunction has been successfully updated to work with the new LinglongSubset IDs while maintaining the same validation logic.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/operator-registries/TaiyiRegistryCoordinator.sol (1)
776-786:⚠️ Potential issueHard-coded deregistration still assumes both Symbiotic subsets exist
The function
_deregisterOperatorForSymbiotichardcodes both Symbiotic subset IDs and attempts to remove the operator from them without checking if they exist. If either subset was never initialized, the call toremoveOperatorFromLinglongSubsetswill revert, blocking deregistration completely.Modify the code to guard against non-existent subsets:
function _deregisterOperatorForSymbiotic(address operator) internal { OperatorInfo storage operatorInfo = _operatorInfo[operator]; require(operatorInfo.status == OperatorStatus.REGISTERED, OperatorNotRegistered()); uint32[] memory linglongSubsetIds = new uint32[](2); linglongSubsetIds[0] = OperatorSubsetLib.SYMBIOTIC_VALIDATOR_SUBSET_ID; linglongSubsetIds[1] = OperatorSubsetLib.SYMBIOTIC_UNDERWRITER_SUBSET_ID; - _linglongSubsets.removeOperatorFromLinglongSubsets(linglongSubsetIds, operator); + // Only remove from subsets that exist + if (_linglongSubsets.linglongSubsetIds.contains(OperatorSubsetLib.SYMBIOTIC_VALIDATOR_SUBSET_ID)) { + uint32[] memory validatorSubset = new uint32[](1); + validatorSubset[0] = OperatorSubsetLib.SYMBIOTIC_VALIDATOR_SUBSET_ID; + _linglongSubsets.removeOperatorFromLinglongSubsets(validatorSubset, operator); + } + + if (_linglongSubsets.linglongSubsetIds.contains(OperatorSubsetLib.SYMBIOTIC_UNDERWRITER_SUBSET_ID)) { + uint32[] memory underwriterSubset = new uint32[](1); + underwriterSubset[0] = OperatorSubsetLib.SYMBIOTIC_UNDERWRITER_SUBSET_ID; + _linglongSubsets.removeOperatorFromLinglongSubsets(underwriterSubset, operator); + }
🧹 Nitpick comments (3)
src/operator-registries/TaiyiRegistryCoordinator.sol (3)
734-749: Duplicate code for operator status update logicThe status checking logic in both
_deregisterOperatorForEigenlayerand_deregisterOperatorForSymbioticis duplicated. This makes the code harder to maintain and increases the risk of inconsistencies if one implementation is updated but not the other.Extract the common logic into a helper function:
+// Extract common deregistration logic to a helper function +function _updateOperatorStatusAfterRemoval(address operator) internal { + // Check if the operator is still in any Linglong subset + bool stillInAnySubset = false; + uint256[] memory allSubsetIds = _linglongSubsets.linglongSubsetIds.values(); + for (uint256 i = 0; i < allSubsetIds.length; i++) { + uint32 subsetId = uint32(allSubsetIds[i]); + if (_linglongSubsets.isOperatorInLinglongSubset(subsetId, operator)) { + stillInAnySubset = true; + break; + } + } + + // Only set status to DEREGISTERED if not in any subset + if (!stillInAnySubset) { + _operatorInfo[operator].status = OperatorStatus.DEREGISTERED; + } +} // In _deregisterOperatorForEigenlayer: -// Check if the operator is still in any Linglong subset -bool stillInAnySubset = false; -uint256[] memory allSubsetIds = _linglongSubsets.linglongSubsetIds.values(); -for (uint256 i = 0; i < allSubsetIds.length; i++) { - uint32 subsetId = uint32(allSubsetIds[i]); - if (_linglongSubsets.isOperatorInLinglongSubset(subsetId, operator)) { - stillInAnySubset = true; - break; - } -} - -// Only set status to DEREGISTERED if not in any subset -if (!stillInAnySubset) { - operatorInfo.status = OperatorStatus.DEREGISTERED; -} +_updateOperatorStatusAfterRemoval(operator); // In _deregisterOperatorForSymbiotic: -// Check if the operator is still in any Linglong subset -bool stillInAnySubset = false; -uint256[] memory allSubsetIds = _linglongSubsets.linglongSubsetIds.values(); -for (uint256 i = 0; i < allSubsetIds.length; i++) { - uint32 subsetId = uint32(allSubsetIds[i]); - if (_linglongSubsets.isOperatorInLinglongSubset(subsetId, operator)) { - stillInAnySubset = true; - break; - } -} - -// Only set status to DEREGISTERED if not in any subset -if (!stillInAnySubset) { - operatorInfo.status = OperatorStatus.DEREGISTERED; -} +_updateOperatorStatusAfterRemoval(operator);Also applies to: 788-803
843-843: Add comment explaining maxMag checkThe logic to skip strategies with zero maximum magnitude is correct (to prevent division by zero), but lacks documentation explaining why this check is necessary.
- if (maxMag == 0) continue; + // Skip strategies with zero maximum magnitude to prevent division by zero in stake calculation + if (maxMag == 0) continue;
268-277: Consider optimizing getLinglongSubnets implementationThe current implementation creates a new array and copies values to convert from
uint256[]touint32[]. This could be optimized for gas efficiency.function getLinglongSubnets() external view returns (uint32[] memory) { - uint256[] memory subnetIds = _linglongSubsets.linglongSubsetIds.values(); - uint32[] memory subsetIds = new uint32[](subnetIds.length); - for (uint256 i = 0; i < subnetIds.length; i++) { - subsetIds[i] = uint32(subnetIds[i]); - } - return subsetIds; + // Get all subset IDs + uint256[] memory subsetIds = _linglongSubsets.linglongSubsetIds.values(); + + // Convert in place using SafeCast + return subsetIds.toUint32Array(); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (1)
src/operator-registries/TaiyiRegistryCoordinator.sol(25 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Foundry Project
🔇 Additional comments (3)
src/operator-registries/TaiyiRegistryCoordinator.sol (3)
663-711: Registration and operator ID handling looks goodThe implementation properly handles operator registration for EigenLayer, including validating subset IDs, checking stakes against minimum requirements, and registering with the middleware when needed.
401-534: Strategy allocation query functions correctly updatedThe functions for querying strategy allocations have been properly updated to use Linglong subset IDs and include the appropriate protocol-specific subset ID validation modifiers.
82-106: Well-designed unified access control systemThe updated modifier system effectively unifies access control while maintaining separation of concerns. The
onlyMiddlewaremodifier handles authentication whileonlySymbioticSubsetIdandonlyEigenLayerSubsetIdensure proper authorization for protocol-specific operations.
|
|
||
| // Extract the base ID by masking out the protocol bits | ||
| baseId = encodedId & ID_MASK_32; | ||
| // the below are the available linglong subset id |
There was a problem hiding this comment.
not blocking the merge but just a question
how should we handle the id updates when we add more operator sets and subnetworks
There was a problem hiding this comment.
Just use different incremented number. I am not sure what kind of operator set and subnetwork you have in mind for the future. Because your original implementation doesn't actually address that either, I don't actually get what your future plan for it.
Are you talking about supporting more restaking protocol or something else?
There was a problem hiding this comment.
More operator set like gateway operator set, underwriter operator set. There could be one operator set for each based rollup etc
There was a problem hiding this comment.
This that case, do we need to implement some contracts for based rollup like eigenlayer middleware? These constant are actually used in the middleware.
3 participants
Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Tests
Chores