Skip to content

set_operator can orphan a pending operator proposal: the two-step handoff state survives a direct assignment #469

Description

@usmanimamu17-create

Problem

Two operator-change paths coexist in src/governance.rs:

// Two-step: propose_operator -> accept_operator (writes PENDING_OP_KEY)
// Single-step: set_operator (writes OPERATOR_KEY directly, admin only)
pub fn set_operator(env: &Env, caller: &Address, new_operator: &Address) -> Result<(), SLAError> {
    ...
    env.storage().instance().set(&OPERATOR_KEY, new_operator);
    env.events().publish((EVENT_OP_SET, EVENT_VERSION, caller.clone()), (new_operator.clone(),));
    Ok(())
}

set_operator does not clear PENDING_OP_KEY. If a proposal is pending, a direct set_operator installs a new operator while the pending slot still names someone else; that someone can later call accept_operator and silently replace the operator that was just set directly.

Consequences:

  • A stale proposal can override a direct assignment: the sequence set_operator(B) → (later) accept_operator(C, pending from before) leaves C as operator, undoing the admin's direct decision — with no event ordering that explains it.
  • The two-step protocol is bypassable and then re-assertable: set_operator exists as a "single-step, admin only" escape hatch, but its failure to clear the pending slot lets the two-step path fire after the direct one, producing an operator the admin never intended.
  • Audit trails cannot reconcile the two paths: op_set and op_acc events are independent; nothing links a direct assignment to the pending proposal it should have invalidated.

Root cause

set_operator and the proposal lifecycle were written independently; the invariant "a direct assignment invalidates any pending proposal" was never encoded.

Why this is architecturally hard

  1. The fix (clear PENDING_OP_KEY in set_operator, possibly emitting op_can) is small, but it changes event behavior and must be paired with the state-machine definition from the propose-side companion issue.
  2. There is a design question whether set_operator should even exist alongside the two-step path (the event schema documents both, but only the two-step path has cancel semantics); the issue should decide the canonical path.
  3. Tests must cover the interleaving (pending → direct set → accept) and pin the resulting operator and event stream.

Acceptance criteria

  • A direct set_operator invalidates any pending operator proposal (cleared or rejected).
  • The event stream reflects the invalidation.
  • Tests cover pending → direct set → accept interleavings.

Out of scope

Removing set_operator entirely and the admin-side blind set (companion issues).

Getting started

just test

Good first files to read: apexchainx_calculator/src/governance.rs (set_operator, propose_operator, accept_operator).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

GrantFox OSSIssue tracked in GrantFox OSSMaybe RewardedIssue may be eligible for a GrantFox rewardStellar WaveIssues in the Stellar wave programThird CampaignCampaign: Third Campaignarea/governanceImported campaign issue labelpriority/highImported campaign issue label

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions