Skip to content

propose_admin allows proposing the current admin as the new admin: a self-proposal emits adm_prop and adm_acc with no state change, polluting the governance trail #472

Description

@usmanimamu17-create

Problem

propose_admin (src/governance.rs) performs no check that new_admin != caller (the current admin):

pub fn propose_admin(env: &Env, caller: &Address, new_admin: &Address) -> Result<(), SLAError> {
    ...
    env.storage().instance().set(&PENDING_ADMIN_KEY, new_admin);
    env.events().publish((EVENT_ADMIN_PROP, EVENT_VERSION, caller.clone()), (new_admin.clone(),));
    Ok(())
}

Proposing the current admin (or yourself) creates a pending slot equal to the current admin; the admin then calls accept_admin (passes the equality check against pending) and nothing changes — but two governance events were emitted for a no-op.

Consequences:

  • The audit trail records phantom transitions: adm_prop + adm_acc for a role that never changed; a compliance review counting role changes over-counts.
  • The two-step protocol is reducible to a no-op ceremony: an admin can "confirm" their own role via the full propose/accept flow, which is indistinguishable in events from a real handoff followed by a rename back.
  • The pending slot becomes a trap: after a self-proposal, a later legitimate proposal silently overwrites it (companion issue), and any code reading get_pending_admin sees the admin themselves.

Root cause

The proposal path validates only that the caller is admin; it never validates that the proposal is a change.

Why this is architecturally hard

  1. Rejecting self-proposals (a new InvalidInput-style error or reuse of an existing code) is a small guard, but it must be paired with the decision on whether accept_admin should also reject accepting when pending == current admin (defense in depth).
  2. The same no-op possibility exists on the operator side (propose_operator with new_operator == current operator); the fix should cover both roles for consistency.
  3. Error-code choice is an ABI decision (companion error-catalog issue); alternatively the guard can be a silent no-op, but then the events must not be emitted — which is also a behavior change.

Acceptance criteria

  • Proposing the current role holder as their own successor is rejected (or performs no event-emitting state change).
  • accept_admin/accept_operator cannot complete a no-op transition.
  • Tests cover self-proposal for both roles.

Out of scope

The overwrite-supersession behavior (companion issue) and proposal expiry.

Getting started

just test

Good first files to read: apexchainx_calculator/src/governance.rs (propose_admin, accept_admin).

Activity

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

Metadata

Metadata

Assignees

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/lowImported campaign issue label

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions