[LOW] Error discriminant collision between campaign::Error and common::ErrorCode (1-3 swapped, 4 reused)

[Alqku]

Overview

campaign/src/types.rs::Error and common/src/lib.rs::ErrorCode are both #[contracterror] enums with overlapping discriminant assignments, and the existing comment in common/src/lib.rs says "All discriminants are stable — never renumber existing variants", yet two distinct crates depend on this invariant independently. If campaign/ ever pulls common into its dependency graph (the lib comment already claims the crate is "used by both campaign and core contracts"), a downstream panic site could fire the wrong discriminant code.

Evidence

common::ErrorCode::NotInitialized     = 1
campaign::Error::AlreadyInitialized  = 1

common::ErrorCode::AlreadyInitialized = 2
campaign::Error::NotInitialized      = 2

common::ErrorCode::Unauthorized      = 3
campaign::Error::Unauthorized        = 3

common::ErrorCode::InvalidAmount     = 4
campaign::Error::CampaignEnded       = 4
campaign::Error::CampaignNotActive   = 5
common::ErrorCode::Asset...          (none)

The values 1 and 2 are swapped between the two enums. Even where they align (Unauthorized = 3, InvalidAmount = 4), the meaning diverges: campaign's InvalidAmount = 70 lives far past common's InvalidAmount = 4. Both crates declare their errors as stable.

Impact

• If a third crate (likely crates/contracts/core/ or a future shared "orbit-error" crate) ever imports BOTH, discriminant conflicts produce skewed panic_with_error!(env, Error::Foo) semantics: panics fire the wrong u32, off-chain indexers misattribute failures, and Error(Contract, #N) symbolic names misalign.
• Off-host tooling that grep-builds Error(Contract, #N) per discriminant can produce a confusing mix of two definitions of "discs 1-3".
• The stability promise of one crate silently threatens the stability promise of the other.

Recommended Approach

Pick one of these, in order of preference:

1. De-duplicate or alias. Move the canonical Error enum into common/ under the existing name (ErrorCode is more accurate since this is shared error-space) and have campaign/src/types.rs::Error re-export the common type. This is the right answer when the two intent over the same concepts. Workspace-wide adoption.
2. Pick a disjoint discriminant range. Re-number one of the two (e.g. push common::ErrorCode to start at 1000) so they cannot collide even if both are imported. This is a non-stable-discriminant-promise change, so it must migrate the deployment sequence carefully.
3. Drop one of the two. If common::ErrorCode is unused outside its own crate, delete it entirely and document the explicit choice. (Grep for use common::ErrorCode to confirm.)

Acceptance Criteria

• Either enums are merged, or discriminants are placed in disjoint ranges, or common::ErrorCode is removed entirely — and the chosen approach is captured in common/src/lib.rs and campaign/src/types.rs doc comments
• cargo expand or compile-time grep of #[contracterror] discriminants shows no duplicates across the workspace
• A small unit test asserts no discriminant in one crate collides with any in the other (only feasible under Option 1 / 3; under Option 2 it's tautological)
• Migration notes added to docs/deployment.md if any renumbering is required for stability

Affected Files

• campaign/src/types.rs
• common/src/lib.rs
• crates/contracts/core/src/lib.rs (to check whether it imports common — it does not, currently, but future work might)
• docs/deployment.md

[OkeyAmy]

Hello Chief, I would like to work on this issue. I have 4 years of experience as an open source contributor