Skip to content

refactor(admin): typed AdminError and authenticated init - #1461

Open
kosikings wants to merge 1 commit into
CalloraOrg:mainfrom
kosikings:fix/admin-typed-errors
Open

kosikings wants to merge 1 commit into
CalloraOrg:mainfrom
kosikings:fix/admin-typed-errors

Conversation

@kosikings

Copy link
Copy Markdown
Contributor

Closes #1423

Summary of Changes

Typed Errors (AdminError):

Created #[contracterror] #[repr(u32)] pub enum AdminError with semantic error variants:
1: NotInitialized
2: AlreadyInitialized
3: Unauthorized
4: SameAdmin
5: NoPendingAdmin
6: UnauthorizedPending
7: NominationExpired
8: TimelockNotElapsed
Public functions in contracts/admin/src/admin.rs now return Result<_, AdminError>.
Eliminated all panic! and expect( calls from contracts/admin/src/admin.rs.

Authentication & Self-Nomination Guards:

init: Calls admin.require_auth() before inspecting or writing storage, preventing front-running on initialization.
set_admin: Rejects nominating the current admin with AdminError::SameAdmin.

Pinned Precedence in accept_admin:

Strictly enforced check order:
Pending nomination exists (AdminError::NoPendingAdmin)
Caller authentication (caller.require_auth())
Caller matches nominee (AdminError::UnauthorizedPending)
Nomination not expired (AdminError::NominationExpired)
Timelock delay elapsed (AdminError::TimelockNotElapsed)

Revived Test Suite & Consolidation of Experimental Limits:

Revived all 26 core admin transfer tests using a test-only AdminHarness (#[contract]) in contracts/admin/src/test.rs, covering scenarios 1–10.
Removed experimental contracts/admin/src/limits.rs and its 36 stale tests in line with Issue #1248 (consolidated into contracts/yield).
Added discriminant freeze test in contracts/admin/tests/err_stab.rs.

Documentation Updated:

Updated docs/ERROR_CODES.md with ## Admin table.
Updated docs/ACCESS_CONTROL.md with ## 5. Callora Admin Access Control.

Introduce typed AdminError enum covering all former panic/expect cases in callora-admin, authenticate init with admin.require_auth(), and enforce no self-nomination in set_admin. Revive previously ignored admin-transfer tests using a contract test harness, delete experimental limits.rs per issue CalloraOrg#1248 consolidation note, add error code stability tests, and update documentation.
@greatest0fallt1me

Copy link
Copy Markdown
Contributor

Skipped during rebuild review: this PR renumbers existing error codes, which main does not allow.

contracts/admin/src/errors.rs swaps the existing AdminLimitError for a new AdminError and gives codes 2-8 new meanings. For example, Unauthorized moves from 2 to 3, and 2 becomes AlreadyInitialized. Codes 3-8 (InvalidLimit, BetsAtCap, PositionsAtCap, SubscriptionsAtCap, CounterUnderflow, Overflow) are reused for unrelated admin-rotation errors. Any integrator that branches on these u32 codes would silently get the wrong meaning. The new tests/err_stab.rs and docs/ERROR_CODES.md lock in the reassigned numbering.

The PR also deletes contracts/admin/src/limits.rs (795 lines), its 9 event symbols in events.rs, and about 48 tests in src/test.rs (91 down to 43). That removal belongs to #1248, not #1423, and should not ride along here.

The new src/test.rs itself looked like plain Rust test code (no blobs or include_bytes).

What to do:

  1. Keep codes 1-8 with their current meanings. Add the new admin-lifecycle variants after the highest existing code (9 and up), or keep a separate enum that does not reuse released codes. Update err_stab.rs and ERROR_CODES.md to match.
  2. Put the removal of the limits module in its own PR that references Describe callora-admin limits scope and consumers #1248.
  3. Keep the typed-error refactor, the init require_auth, the SameAdmin guard and the ported tests in this PR, then rebase on main.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Replace callora-admin string panics with a typed AdminError, require auth in init, and revive its 62 ignored tests

2 participants