chore(sidecar): add parsing validator indexes as ranges #119
Conversation
WalkthroughThe update introduces a new Changes
The changes collectively transition the handling of validator indexes from a simple vector to a more structured and functional type, 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 Configration File (
|
merklefruit
changed the title
![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 (4)
- bolt-sidecar/bin/sidecar.rs (1 hunks)
- bolt-sidecar/src/config/mod.rs (5 hunks)
- bolt-sidecar/src/config/validator_indexes.rs (1 hunks)
- bolt-sidecar/src/state/consensus.rs (5 hunks)
Additional comments not posted (9)
bolt-sidecar/src/config/validator_indexes.rs (3)
6-9: LGTM! Thecontainsmethod is correctly implemented.The method correctly checks if a given index is in the internal
Vec<u64>.
47-50: LGTM! Thefrommethod is correctly implemented.The method correctly converts a
Vec<u64>into aValidatorIndexesobject.
53-71: LGTM! The test module is well-implemented.The tests cover different input formats and validate the correctness of the
from_strmethod.bolt-sidecar/bin/sidecar.rs (1)
39-43: LGTM! The updated function call toConsensusState::newis correct.The change correctly matches the updated function signature.
bolt-sidecar/src/state/consensus.rs (3)
62-68: LGTM! The updatednewfunction is correct.The change correctly matches the updated
ConsensusStatestruct.
150-151: LGTM! The updatedfind_validator_index_for_slotmethod is correct.The change correctly uses the new
containsmethod ofValidatorIndexes.
185-185: LGTM! The updated test forfind_validator_index_for_slotis correct.The change correctly matches the updated
ConsensusStatestruct.bolt-sidecar/src/config/mod.rs (2)
Line range hint
10-55:
LGTM! The updatedOptsstruct is correct.The change correctly matches the updated
ValidatorIndexestype.
Line range hint
104-128:
LGTM! The updatedConfigstruct is correct.The change correctly matches the updated
ValidatorIndexestype.
| impl FromStr for ValidatorIndexes { | ||
| type Err = eyre::Report; | ||
|
|
||
| /// Parse an array of validator indexes. Accepted values: | ||
| /// - a comma-separated list of indexes (e.g. "1,2,3,4") | ||
| /// - a contiguous range of indexes (e.g. "1..4") | ||
| /// - a mix of the above (e.g. "1,2..4,6..8") | ||
| /// | ||
| /// TODO: add parsing from a directory path, using the format of | ||
| /// validator definitions | ||
| fn from_str(s: &str) -> Result<Self, Self::Err> { | ||
| let s = s.trim(); | ||
| let mut vec = Vec::new(); | ||
|
|
||
| for comma_separated_part in s.split(',') { | ||
| if comma_separated_part.contains("..") { | ||
| let mut parts = comma_separated_part.split(".."); | ||
|
|
||
| let start = parts.next().ok_or_else(|| eyre::eyre!("Invalid range"))?; | ||
| let start = start.parse::<u64>()?; | ||
|
|
||
| let end = parts.next().ok_or_else(|| eyre::eyre!("Invalid range"))?; | ||
| let end = end.parse::<u64>()?; | ||
|
|
||
| vec.extend(start..=end); | ||
| } else { | ||
| let index = comma_separated_part.parse::<u64>()?; | ||
| vec.push(index); | ||
| } | ||
| } | ||
|
|
||
| Ok(Self(vec)) | ||
| } |
There was a problem hiding this comment.
LGTM! The from_str method is correctly implemented.
The method correctly handles different input formats and uses appropriate error handling.
Reminder: Add support for parsing from a directory path.
The TODO comment indicates a missing feature.
Do you want me to implement the parsing from a directory path or open a GitHub issue to track this task?
This PR adds the ability to parse validator index ranges from an array of comma-separated ranges.
ValidatorIndex also becomes a separate struct to allow for future parsing methods too, including from a keystore directory which would be handy for validators
example of valid values: