fix(drive-abci): report configured main control group authority when unauthorized#4209
Conversation
…unauthorized validate_state_v1 delegated the authorization check to validate_state_v0, whose UnauthorizedTokenActionError reported the takers from authorized_action_takers_for_configuration_item, which hardcodes NoOne for MainControlGroup change items. The v0 report must stay unchanged where it is already live, so v1 now passes the controlling rule (main_control_group_can_be_modified) to a parameterized v0 helper and reports it instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 16 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdded override-aware token configuration validation and tests for group-controlled ChangesToken configuration authorization
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant GroupMember
participant TokenConfigUpdateValidation
participant TokenConfiguration
participant State
GroupMember->>TokenConfigUpdateValidation: submit MainControlGroup update
TokenConfigUpdateValidation->>TokenConfiguration: resolve authorized action takers
TokenConfigUpdateValidation->>TokenConfigUpdateValidation: validate with resolved action takers
TokenConfigUpdateValidation->>State: apply update after quorum
State-->>GroupMember: main control group becomes Some(0)
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## v4.1-dev #4209 +/- ##
==========================================
Coverage 87.51% 87.51%
==========================================
Files 2666 2667 +1
Lines 336904 337576 +672
==========================================
+ Hits 294837 295427 +590
- Misses 42067 42149 +82
🚀 New features to boost your workflow:
|
… validator Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Issue being fixed or feature implemented
Replaces #4000, reworked on top of the design introduced in #4154.
TokenConfigurationChangeItem::MainControlGroupis authorized by the token fieldmain_control_group_can_be_modified. The validation gate already used that field correctly, but theUnauthorizedTokenActionErrorproduced when the check fails reported the takers fromauthorized_action_takers_for_configuration_item, which hardcodesNoOneforMainControlGroup. That made failures look like the action was impossible for everyone, when the configured rule could beContractOwner,MainGroup,Group(0), or a specific identity:Since #4000 was opened, #4154 added
controlling_action_takers_for_configuration_item(which returnsmain_control_group_can_be_modified) and a version-gatedstate_v1validation that uses it for the group-binding check. That superseded the original approach here of changingauthorized_action_takers_for_configuration_itemdirectly — the legacy helper's report must stay unchanged for protocol versions where it is already live.However, the misleading report was still reachable at the new validation version:
validate_state_v1delegates the main authorization check tovalidate_state_v0, which still built the error from the legacy helper. This PR fixes that remaining path.What was done?
validate_state_v0_with_reported_action_takers, which takes an optional override for the action takers reported inUnauthorizedTokenActionError.validate_state_v0passesNone, so behavior at validation version 0 is byte-for-byte unchanged.validate_state_v1now passes the already-computed controlling rule, so unauthorizedMainControlGroupupdates reportmain_control_group_can_be_modifiedinstead ofNoOne. For every other change item the controlling rule equals the legacy report, so nothing else changes.controlling_action_takers_for_configuration_itemreports each configuredmain_control_group_can_be_modifiedvalue verbatim.test_token_config_update_main_control_group_unauthorized_reports_configured_rule— a non-member submitting aMainControlGroupchange getsUnauthorizedTokenActionErrorreportingGroup(0)(the configured rule), notNoOne.test_token_config_update_by_group_member_changing_main_control_group— the existing valid path where a configured group updates the main control group once enough group power has signed.How Has This Been Tested?
Breaking Changes
None. Validation version 0 output is unchanged; the corrected report only applies at validation version 1 (protocol v13, not yet released).
Checklist:
For repository code-owners and collaborators only
🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests