feat: add IRegistrar and IValidatorManager interfaces#5
Merged
Conversation
📝 WalkthroughWalkthroughTwo new Solidity interfaces are introduced: IRegistrar for epoch-based key-value storage with batch operations and event emission, and IValidatorManager for validator lifecycle management with epoch-aware retrieval functions. Both define storage structures, events, errors, and external function signatures. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/interfaces/IRegistrar.sol (1)
58-61: Consider pagination for historical reads.Line 61 returns all entries for a key in one call. As history grows, this can become impractical (especially for contract-to-contract calls). A paginated API is safer long-term.
Possible paginated shape
- function getEntries(bytes32 key) external view returns (Entry[] memory entries); + function getEntries( + bytes32 key, + uint256 offset, + uint256 limit + ) external view returns (Entry[] memory entries);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/interfaces/IRegistrar.sol` around lines 58 - 61, The current getEntries(bytes32 key) function returns all historical Entry[] results at once; change the API to support pagination by adding a paginated variant (e.g., getEntriesPaginated or overload getEntries) that accepts pagination parameters such as (bytes32 key, uint256 offset, uint256 limit) or (bytes32 key, uint256 page, uint256 pageSize) and returns (Entry[] memory entries, uint256 total) or similar so callers can fetch slices; update the interface signature and any implementations/callers referencing getEntries to use the new paginated method (and keep or deprecate the existing full-returning getEntries) to avoid unbounded memory/stack use for large histories.src/interfaces/IValidatorManager.sol (1)
23-28: Prefer raw network fields over formattedsocketAddressin core return type.Line 27 (
string socketAddress) bakes formatting into on-chain responses. Returning rawip/portkeeps the interface cheaper and avoids presentation coupling.Lean return struct shape
struct ValidatorInfo { uint64 index; bytes32 publicKey; string name; - string socketAddress; + uint32 ip; + uint16 port; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/interfaces/IValidatorManager.sol` around lines 23 - 28, The ValidatorInfo struct in IValidatorManager currently includes a formatted string socketAddress; change it to expose raw network fields instead (e.g. replace string socketAddress with a raw ip and port pair such as bytes4 ip and uint16 port or a bytes ip and uint16 port depending on IPv4/IPv6 needs) so the core interface stays presentation-agnostic; update the ValidatorInfo definition and then update all functions and return sites in IValidatorManager and any contract/tests that return or consume ValidatorInfo to use the new ip/port fields (adjust encoding/decoding and calldata/ABI handling accordingly).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/interfaces/IRegistrar.sol`:
- Around line 38-56: The current getters get, getNext, batchGet, and
batchGetNext return only bytes32 values which makes bytes32(0) ambiguous; modify
their signatures to also return existence flags so callers can distinguish
missing keys (e.g. change get(bytes32) returns (bytes32 value, bool found),
getNext(bytes32) returns (bytes32 value, bool found), batchGet(bytes32[]
calldata) returns (bytes32[] memory values, bool[] memory found), and
batchGetNext(bytes32[] calldata) returns (bytes32[] memory values, bool[] memory
found)); update any implementing contracts and callers to handle the additional
bool(s).
In `@src/interfaces/IValidatorManager.sol`:
- Around line 23-27: The interface mixes string and bytes32 for validator names
causing ambiguity; make name consistently bytes32 end-to-end by changing
ValidatorInfo.name from string to bytes32, update the ValidatorAdded and
ValidatorRemoved events to emit bytes32 name instead of string, and ensure any
function signatures and return types that reference ValidatorInfo (e.g.,
addValidator, getValidator/getValidators or similar) align to bytes32; this
matches the internal Validator and GenesisValidator structs and removes
conversion ambiguity for consumers and indexers.
---
Nitpick comments:
In `@src/interfaces/IRegistrar.sol`:
- Around line 58-61: The current getEntries(bytes32 key) function returns all
historical Entry[] results at once; change the API to support pagination by
adding a paginated variant (e.g., getEntriesPaginated or overload getEntries)
that accepts pagination parameters such as (bytes32 key, uint256 offset, uint256
limit) or (bytes32 key, uint256 page, uint256 pageSize) and returns (Entry[]
memory entries, uint256 total) or similar so callers can fetch slices; update
the interface signature and any implementations/callers referencing getEntries
to use the new paginated method (and keep or deprecate the existing
full-returning getEntries) to avoid unbounded memory/stack use for large
histories.
In `@src/interfaces/IValidatorManager.sol`:
- Around line 23-28: The ValidatorInfo struct in IValidatorManager currently
includes a formatted string socketAddress; change it to expose raw network
fields instead (e.g. replace string socketAddress with a raw ip and port pair
such as bytes4 ip and uint16 port or a bytes ip and uint16 port depending on
IPv4/IPv6 needs) so the core interface stays presentation-agnostic; update the
ValidatorInfo definition and then update all functions and return sites in
IValidatorManager and any contract/tests that return or consume ValidatorInfo to
use the new ip/port fields (adjust encoding/decoding and calldata/ABI handling
accordingly).
johnletey
changed the title
IRegistrar and IValidatorManager interfaces
johnletey
approved these changes
Mar 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit