feat: validate PayoutAsset.token is a live SEP-41 contract at create_…

[Wetshakat]

fix: validate payout token at create_campaign time via try_invoke_contract

Closes #46

Summary

Validate the payout token during create_campaign to ensure it is a real, responsive SEP-41 token contract before the campaign is persisted.

Problem

create_campaign previously accepted any PayoutAsset { token: Address, ... } without validating the token address.

As a result, a campaign could be created with a bogus or non-contract token address. The issue would only surface later when fund_campaign attempted the token transfer, potentially after creators had already applied and submitted proof.

A previous implementation using token::Client::new(&env, &addr).decimals() was rejected because token::Client internally uses invoke_contract, which converts failed contract dispatches into host traps (Err(Abort)) rather than returning a typed Rust error. This made Error::InvalidAsset effectively unreachable and forced tests to accept either outcome.

Solution

Use env.try_invoke_contract to probe the payout token during create_campaign.

The probe calls the SEP-41 decimals() entrypoint:

• Uses the host's try_call path instead of call.
• Treats non-contract addresses, missing entrypoints, and host-level dispatch failures as a Rust Result error.
• Maps any outer invocation failure to Error::InvalidAsset.
• Runs before storage::set_campaign, ensuring invalid campaigns are rejected without writing to storage.

Changes

• error.rs

  • Added Error::InvalidAsset = 26.

• lib.rs

  • Added payout-token validation in create_campaign using env.try_invoke_contract.
  • Updated the create_campaign documentation to describe the token validation behavior.

• test.rs

  • Replaced the previous hedged test that accepted either InvalidAsset or Abort.
  • Added create_campaign_rejects_non_contract_token, which explicitly asserts Err(Ok(Error::InvalidAsset)).

Testing

cargo test --workspace

Result: 124 passed, 0 failed.

This ensures invalid payout tokens are rejected at campaign creation rather than allowing invalid campaigns to progress until funding.

[JamesVictor-O]

The motivation (fail-fast on a bad payout token instead of only discovering it at fund_campaign) is good, but I verified experimentally that the implementation doesn't do what the PR describes.

I added a debug print in place of the test's assert! and ran it: for a non-contract bogus_token address, token::Client::new(&env, &asset.token).decimals() doesn't return Err(Error::InvalidAsset) — it produces a raw host trap:

RESULT_DEBUG: Err(Err(Abort))

So in practice:

• The new Error::InvalidAsset variant is never actually returned — it's dead code as written.
• create_campaign panics/traps on an invalid token rather than returning a typed Result::Err, which is the exact "opaque host trap" problem #69 (merged) explicitly moved away from elsewhere in this contract.
• The PR's own test already reflects this uncertainty — it accepts either Err(Ok(Error::InvalidAsset)) or Err(Err(_host_err)) as passing, with a comment saying "Accept either outcome here; later we can refine the implementation." That's a sign the feature isn't done yet, not something to merge and revisit later — a caller integrating against this contract can't rely on InvalidAsset since it's never actually surfaced.

The PR description itself notes the try_invoke_contract approach was tried and reverted due to host-level escalation/panic issues — that's the right instinct, but it means the underlying problem (cleanly catching a failed cross-contract call without trapping) isn't actually solved yet.

Suggested path: either (a) get try_invoke_contract (or env.try_invoke_contract_check_auth/whatever the SDK 27 equivalent is that returns a Result instead of trapping) working so InvalidAsset is genuinely reachable, and tighten the test to assert exactly that outcome; or (b) if trapping turns out to be unavoidable in soroban-sdk 27 for a non-contract address, drop the InvalidAsset error variant (it'd be misleading dead code) and document that invalid tokens cause the transaction to abort rather than return a typed contract error.

Happy to re-review once one of those lands.

[Wetshakat]

@JamesVictor-O please review i have done the correction

[JamesVictor-O]

The validation approach is technically sound, but the branch doesn't currently compile.

Blocker: duplicate error discriminant

Error::InvalidAsset = 26 collides with Error::EvidenceWindowOpen = 26 (added by #68, already merged to main). CI fails all four jobs with:

error[E0081]: discriminant value `26` assigned more than once

Fix: renumber InvalidAsset to 28 (27 is taken by NoDisputeOpen). No other open PR touches error.rs, so 28 is safe to claim.

Also failing: cargo fmt --check

A stray blank line at error.rs:1, and the try_invoke_contract call in lib.rs:274 needs to be reformatted to satisfy rustfmt's line-wrapping.

Verified once the discriminant is fixed locally

cargo test -p ads-bazaar-campaign-escrow passes 94+16 tests (including the new create_campaign_rejects_non_contract_token), and cargo build --workspace --target wasm32v1-none --release succeeds. The try_invoke_contract approach genuinely resolves the token::Client-panics-instead-of-erroring problem described in the PR — InvalidAsset really is reachable now.

Worth addressing (non-blocking)

• The doc comment overclaims: this only proves the address responds to decimals(), not full SEP-41 compliance — a paused, non-transferable, or otherwise broken token would still pass. Consider softening the wording to describe it as a liveness/smoke check.
• This does add the token contract's footprint requirement to create_campaign's transaction — not breaking for standard SDKs using simulateTransaction, but worth a one-line callout in the PR description for integrators building raw transactions.

Please fix the discriminant collision and formatting so CI goes green, and this looks good to merge.

[JamesVictor-O]

Good approach — probing with env.try_invoke_contract instead of token::Client to get a typed Result instead of a host trap is the right call, and rejecting before storage::set_campaign (rather than deferring the failure to fund_campaign) closes the gap described in #46.

Blocking: CI is red across the board, and it's a real compile error, not flakiness.

error[E0081]: discriminant value `26` assigned more than once
  --> contracts/campaign-escrow/src/error.rs:8:1
54 |     InvalidAsset = 26,
   |                    -- `26` assigned here
58 |     EvidenceWindowOpen = 26,
   |                          -- `26` assigned here

EvidenceWindowOpen = 26 merged into main after this branch was cut (from #68), so InvalidAsset needs to move off 26. Current main already has discriminants through 27 (NoDisputeOpen), so 28 is the next free one. This will need a rebase onto latest main to pick that up cleanly.

Minor: error.rs also has a stray extra blank line right after use soroban_sdk::contracterror; that cargo fmt --check is flagging — rustfmt will fix it automatically.

Once it compiles again and CI is green, happy to take another look.

[JamesVictor-O]

review found the cause of the failing CI (Build/Clippy/Format/Test all failing):

contracts/campaign-escrow/src/error.rs adds InvalidAsset = 26, but EvidenceWindowOpen = 26 was already added at that same discriminant value by a separate commit that landed on main after this branch diverged. cargo build --workspace fails with error[E0081]: discriminant value 26 assigned more than once.

Fix is simple: give InvalidAsset an unused discriminant (e.g. 27) and rebase onto latest main.

The actual feature change itself (calling token::Client::decimals() in create_campaign as a lightweight SEP-41 sanity check) looks reasonable and doesn't touch payout/transfer logic — just needs the discriminant collision fixed and a rebase.

[JamesVictor-O]

Merged manually after fixing the duplicate enum discriminant (InvalidAsset collided with EvidenceWindowOpen = 26, which was added to main after this branch was created). Fixed to InvalidAsset = 28. All CI checks pass. Thank you @Wetshakat!