Validate participant roles in IsValidEpochSetup - #8693
Conversation
IsValidEpochSetup already checked that an EpochSetup has enough active nodes for each known role, but it never rejected participants whose Role field was not a valid flow.Role. An invalid role (e.g. 0 or 42) would be accepted as long as the real roles were still represented, and later code paths that call Role.String() would panic on the bogus value. Add a defense-in-depth check that calls Role.Valid() for every participant and rejects the setup with an error if any participant has an invalid role. Also add a regression test that mutates a consensus participant to an invalid role and asserts that IsValidEpochSetup returns an error.
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 4 included reviews per hour; 2 remain after this review. 📝 WalkthroughWalkthroughEpoch setup validation now checks that every participant has a valid role. A test sets a participant role to ChangesEpoch setup validation
Estimated code review effort: 1 (Trivial) | ~5 minutes Merge Risk: ⚪ Minimal · up to Epoch setups with invalid participant roles are now rejected before later role handling can fail, with regression coverage for the invalid-role case. No current merge-blocking risk remains. Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Problem
IsValidEpochSetupalready checked that an EpochSetup has enough active nodes for each known role, but it never rejected participants whoseRolefield was not a validflow.Role. An invalid role (e.g. 0 or 42) would be accepted as long as the real roles were still represented, and later code paths that callRole.String()would panic on the bogus value.Changes
Role.Valid()for every participant and rejects the setup with an error if any participant has an invalid role.IsValidEpochSetupreturns an error.Labels: Bug, S-Epochs
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by CodeRabbit