feat(registry): add getallregistrants #158
Conversation
WalkthroughThe recent updates to the Changes
Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
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 as PR comments)
Additionally, you can add 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: 1
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files selected for processing (1)
- bolt-contracts/src/contracts/BoltRegistry.sol (3 hunks)
Additional comments not posted (2)
bolt-contracts/src/contracts/BoltRegistry.sol (2)
112-112: Visibility change approved.Changing the visibility of the
isActiveOperatorfunction fromexternaltopublicis appropriate, as it allows internal calls within the contract, which is necessary for the newgetAllRegistrantsfunction.
16-17: Ensure proper management of theoperatorsarray.The
operatorsarray is added to store operator addresses. Verify that operators are correctly added and removed to avoid stale or incorrect data.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files selected for processing (1)
- bolt-contracts/src/contracts/BoltRegistry.sol (3 hunks)
Additional comments not posted (2)
bolt-contracts/src/contracts/BoltRegistry.sol (2)
112-113: Visibility change approved.Changing the visibility of the
isActiveOperatorfunction fromexternaltopublicis a good change as it increases accessibility within the contract.
136-154: Optimize the allocation for active operators.The
getAllRegistrantsfunction currently allocates space for all operators, which might include inactive ones. Optimize the allocation to include only active operators.function getAllRegistrants() external view returns (Registrant[] memory) { uint256 activeCount = 0; for (uint256 i = 0; i < operators.length; i++) { if (isActiveOperator(operators[i])) { activeCount++; } } Registrant[] memory _registrants = new Registrant[](activeCount); uint256 index = 0; for (uint256 i = 0; i < operators.length; i++) { if (isActiveOperator(operators[i])) { _registrants[index] = registrants[operators[i]]; index++; } } return _registrants; }Likely invalid or redundant comment.
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files selected for processing (1)
- bolt-contracts/src/contracts/BoltRegistry.sol (4 hunks)
Additional comments not posted (3)
bolt-contracts/src/contracts/BoltRegistry.sol (3)
16-17: Add code to manage theoperatorsarray.The
operatorsarray is declared but there is no code to add or remove addresses from this array.Ensure that the
operatorsarray is properly managed by adding code to handle additions and removals.
114-114: Visibility change approved.The change in visibility from
externaltopublicenhances the function's accessibility and improves the modularity and efficiency of the contract's logic.
138-156: Optimize the allocation for active operators.The
getAllRegistrantsfunction currently allocates space for all operators, which might include inactive ones. Optimize the allocation to include only active operators.function getAllRegistrants() external view returns (Registrant[] memory) { uint256 activeCount = 0; for (uint256 i = 0; i < operators.length; i++) { if (isActiveOperator(operators[i])) { activeCount++; } } Registrant[] memory _registrants = new Registrant[](activeCount); uint256 index = 0; for (uint256 i = 0; i < operators.length; i++) { if (isActiveOperator(operators[i])) { _registrants[index] = registrants[operators[i]]; index++; } } return _registrants; }
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files selected for processing (1)
- bolt-contracts/src/contracts/BoltRegistry.sol (5 hunks)
Additional comments not posted (5)
bolt-contracts/src/contracts/BoltRegistry.sol (5)
16-18: Add code to manage theoperatorsarray.The
operatorsarray is declared but there is no code to add or remove addresses from this array.Ensure that the
operatorsarray is properly managed by adding code to handle additions and removals.
53-54: Ensure removal of operators from theoperatorsarray upon exit.The
registerfunction adds the operator to theoperatorsarray, but there is no corresponding code to remove the operator from the array when they exit. This may lead to stale data in theoperatorsarray.- delete registrants[msg.sender]; + // Remove operator from the operators array + for (uint256 i = 0; i < operators.length; i++) { + if (operators[i] == msg.sender) { + operators[i] = operators[operators.length - 1]; + operators.pop(); + break; + } + } + + delete registrants[msg.sender];
100-107: LGTM!The code correctly removes the operator from the
operatorsarray upon exit.
123-125: LGTM!Changing the visibility of the
isActiveOperatorfunction fromexternaltopublicis appropriate and enhances internal contract functionality.
147-165: Optimize the allocation for active operators.The
getAllRegistrantsfunction currently allocates space for all operators, which might include inactive ones. Optimize the allocation to include only active operators.function getAllRegistrants() external view returns (Registrant[] memory) { uint256 activeCount = 0; for (uint256 i = 0; i < operators.length; i++) { if (isActiveOperator(operators[i])) { activeCount++; } } Registrant[] memory _registrants = new Registrant[](activeCount); uint256 index = 0; for (uint256 i = 0; i < operators.length; i++) { if (isActiveOperator(operators[i])) { _registrants[index] = registrants[operators[i]]; index++; } } return _registrants; }
fixes #157
Summary by CodeRabbit