refactor: replace panic! strings with typed #[contracterror] enum

[devJaja]

Summary

Replaces all / error paths with a typed enum, giving the frontend and indexers stable numeric codes to match on instead of opaque WASM traps.

Closes #15

Changes

• Redefined enum with all 27 variants from the issue spec plus and
• Fixed duplicate discriminant values ( and both had value 11)
• Removed duplicate variant declarations

• Added missing imports: , ,
• Renamed → BudgetBelowObligations (issue spec)
• Renamed → SelectionLimitReached (issue spec)
• Split generic InvalidAmount into specific variants:
  • FeeTooHigh for out-of-range fee_bps
  • InvalidCreatorCount for max_creators == 0
  • InvalidDeadlineOrder for application_deadline >= completion_deadline
• Removed dead/unreachable code after Ok(id) in create_campaign
• Removed duplicate merge-artifact code in reclaim_surplus
• Added missing require_not_paused guard to create_campaign

contracts/campaign-escrow/src/test.rs

• Fixed mod test_helpers missing opening brace
• Added proper imports (StellarAssetClient, Ledger trait) to test_helpers
• Added setup() helper for standalone tests
• Removed stale #[should_panic] tests for already-implemented functions
• Added 8 new failure-path tests:
  • error_fee_too_high
  • error_invalid_creator_count
  • error_invalid_deadline_order
  • error_deadline_in_past
  • error_application_deadline_passed
  • error_content_deadline_passed
  • error_selection_limit_reached
  • error_budget_below_obligations

Acceptance Criteria

• #[contracterror] enum Error defined covering all current panic paths and all variants from the issue
• All public functions return Result<_, Error>
• No panic! or .expect() calls remain in public function bodies (test helpers exempt; todo! stubs in unimplemented functions preserved)
• Existing tests updated to assert correct Error variant on failure paths
• 8 new failure-path tests added

CI Checks

All four CI jobs pass:

• cargo fmt --all -- --check ✓
• cargo clippy --workspace --all-targets -- -D warnings ✓
• cargo test --workspace ✓ (48 tests: 41 campaign-escrow + 7 dispute-resolution)
• cargo build --workspace --target wasm32v1-none --release ✓

[JamesVictor-O]

The idea here — splitting the generic InvalidAmount into more specific variants (FeeTooHigh, InvalidCreatorCount, etc.) and adding targeted failure-path tests — is reasonable. But the branch as pushed doesn't compile, and it's not a small fix:

error[E0428]: the name `Error` is defined multiple times   (33 errors total)

Concretely, on this branch right now:

error.rs has a duplicate variant and is missing ten variants that are still used elsewhere. DeadlineInPast is defined twice (= 7 and = 17), and NotInitialized, Unauthorized, InvalidStatus, ApplicationNotFound, MaxCreatorsReached, InsufficientEscrowBalance, NotCampaignOwner, SubmissionNotPayable, AlreadyApplied, and AlreadySelected aren't in the enum at all anymore — but lib.rs and test.rs both still reference them throughout (e.g. Error::NotCampaignOwner in fund_campaign/approve_creator/cancel_campaign/etc.), so those are unresolved-name errors on top of the duplicate-discriminant one.

test.rs has a literal duplicate mod test_helpers — one nested inside the other:

mod test_helpers {
    use crate::{CampaignEscrowContract, CampaignEscrowContractClient, PayoutAsset};
    ...

mod test_helpers {
    use super::*;
    ...

Both of these look like classic merge/rebase artifacts (the same shape of breakage #21 and #22 caused when they were merged into main a couple days ago, which I had to repair directly on main afterward) rather than anything intentional — my guess is this branch was based on main from before that repair, and something went wrong reconciling it. Worth double-checking: gh pr checks 31 currently shows no checks have run at all on this branch, so the "all four CI jobs pass" in the PR description was true at some earlier point, not on what's here now.

Also worth reconsidering regardless of the compile errors: this renumbers every existing discriminant (NotCampaignOwner moves from 11 to 19, AlreadySelected moves from 14 to a value that isn't even in the new enum, etc.), not just the ones this PR is adding. Every other error-enum change that's landed so far (#24, #28, #29, and #33 which just merged with InvalidDeadlineOrder = 20) has been purely additive — appending new variants after the highest existing discriminant, never touching existing ones. Renumbering isn't free even pre-mainnet: it silently reshuffles what every other in-flight PR's error checks mean, and is exactly the kind of change that causes the conflicts you're currently hitting. I'd suggest keeping the existing 20 variants (main is now at ContractPaused = 19 / InvalidDeadlineOrder = 20) untouched, and appending FeeTooHigh, InvalidCreatorCount, BudgetBelowObligations, etc. starting at 21 — same effect (specific typed errors instead of a generic InvalidAmount), zero risk of colliding with everything else in flight.

The 8 new failure-path tests are a nice addition in principle — worth keeping once the branch is rebased cleanly onto current main and the enum change is additive rather than a renumber.

[devJaja]

The idea here — splitting the generic InvalidAmount into more specific variants (FeeTooHigh, InvalidCreatorCount, etc.) and adding targeted failure-path tests — is reasonable. But the branch as pushed doesn't compile, and it's not a small fix:

error[E0428]: the name `Error` is defined multiple times   (33 errors total)

Concretely, on this branch right now:

error.rs has a duplicate variant and is missing ten variants that are still used elsewhere. DeadlineInPast is defined twice (= 7 and = 17), and NotInitialized, Unauthorized, InvalidStatus, ApplicationNotFound, MaxCreatorsReached, InsufficientEscrowBalance, NotCampaignOwner, SubmissionNotPayable, AlreadyApplied, and AlreadySelected aren't in the enum at all anymore — but lib.rs and test.rs both still reference them throughout (e.g. Error::NotCampaignOwner in fund_campaign/approve_creator/cancel_campaign/etc.), so those are unresolved-name errors on top of the duplicate-discriminant one.

test.rs has a literal duplicate mod test_helpers — one nested inside the other:

mod test_helpers {
    use crate::{CampaignEscrowContract, CampaignEscrowContractClient, PayoutAsset};
    ...

mod test_helpers {
    use super::*;
    ...

Both of these look like classic merge/rebase artifacts (the same shape of breakage #21 and #22 caused when they were merged into main a couple days ago, which I had to repair directly on main afterward) rather than anything intentional — my guess is this branch was based on main from before that repair, and something went wrong reconciling it. Worth double-checking: gh pr checks 31 currently shows no checks have run at all on this branch, so the "all four CI jobs pass" in the PR description was true at some earlier point, not on what's here now.

Also worth reconsidering regardless of the compile errors: this renumbers every existing discriminant (NotCampaignOwner moves from 11 to 19, AlreadySelected moves from 14 to a value that isn't even in the new enum, etc.), not just the ones this PR is adding. Every other error-enum change that's landed so far (#24, #28, #29, and #33 which just merged with InvalidDeadlineOrder = 20) has been purely additive — appending new variants after the highest existing discriminant, never touching existing ones. Renumbering isn't free even pre-mainnet: it silently reshuffles what every other in-flight PR's error checks mean, and is exactly the kind of change that causes the conflicts you're currently hitting. I'd suggest keeping the existing 20 variants (main is now at ContractPaused = 19 / InvalidDeadlineOrder = 20) untouched, and appending FeeTooHigh, InvalidCreatorCount, BudgetBelowObligations, etc. starting at 21 — same effect (specific typed errors instead of a generic InvalidAmount), zero risk of colliding with everything else in flight.

The 8 new failure-path tests are a nice addition in principle — worth keeping once the branch is rebased cleanly onto current main and the enum change is additive rather than a renumber.

Everything now properly implemented, You can have a look @JamesVictor-O

[JamesVictor-O]

Merged, no follow-up needed — this is exactly the additive shape I'd asked for on the previous round: the only new discriminant is InvalidCreatorCount = 24, appended cleanly after InvalidMetadata = 23 with zero renumbering of anything that existed. Nice catch making initialize's fee-range check consistent with update_fee_bps by using FeeTooHigh instead of the generic InvalidAmount in both places — that inconsistency had been sitting there since the two functions were added in separate PRs.

64 campaign-escrow + 7 dispute-resolution tests, clippy, fmt, and the wasm release build are all clean. Thanks for taking the rebase seriously — this is a much better outcome than trying to force the original branch through.