Quests Contract — Audit

┌──────────────────┐     deposit/withdraw     ┌──────────────────┐
│   quests.clar    │ ──────────────────────── │  treasury.clar   │
│                  │                           │                  │
│  create-quest ───┼─── deposit ──────────────►│  token-balances  │
│  cancel-quest ───┼─── withdraw ─────────────►│  (per-token map) │
│                  │                           │                  │
│  join-quest      │   reward-random-winners   │  admins map      │
│  complete-activity                           │  treasury-owner  │
│  refund-participant                          └──────────────────┘
│                  │
│  participants map│     ┌──────────────────┐
│  quests map      │     │ helper-contract  │
└──────────────────┘     │ (wSTX wrapper)   │
                         └──────────────────┘
                               │
                         ┌─────┴─────────────────────────────────┐
                         │ ZADAO-token-whitelist-v1 (quests)     │
                         │ ZADAO-token-whitelist-v2 (treasury)   │
                         └───────────────────────────────────────┘

F-01: Anyone can self-certify activity completion

Location: quests.clar → complete-activity

(define-public (complete-activity
    (quest-id (string-ascii 36))
    (use-token <token>)
  )
  (let (
      ;; ...
      (completed-count (get activities-completed participant))
      (new-count (+ completed-count u1))
      (is-completed (is-eq new-count ACTIVITIES_PER_QUEST))
    )
    (asserts! (is-eq tx-sender contract-caller) ERR_UNAUTHORIZED)
    ;; ← No verification that an activity was actually performed!
    ;; ← No oracle, no admin signature, no proof
    ;; Anyone who joined can call this 3 times and get auto-refunded
    ;; ...
  ))

Impact: The entire quest incentive model is broken. A participant can:

1. Call join-quest to lock tokens
2. Call complete-activity 3 times in the same block
3. Get auto-refunded immediately — no activities performed

Fix: Activity completion should require admin/oracle authorization. Options:

• Only quest creator or admin can call complete-activity for a participant
• Require a signed message from an off-chain oracle attesting to completion
• Add a cooldown between activity completions (e.g., minimum blocks between calls)

F-02: Unconditional refund defeats the quest model

Location: quests.clar → refund-participant

(define-public (refund-participant
    (quest-id (string-ascii 36))
    (use-token <token>)
  )
  (let (
      (quest (unwrap! (map-get? quests quest-id) ERR_INVALID_QUEST))
      (participant-key { quest-id: quest-id, participant: tx-sender })
      (participant (unwrap! (map-get? participants participant-key) ERR_NOT_PARTICIPATING))
    )
    ;; Only checks: tx-sender == contract-caller, token matches, amount is locked
    ;; Does NOT check: quest status, activities completed, time elapsed
    ;; Any participant can refund at any time, for any reason
    (asserts! (get amount-locked participant) ERR_AMOUNT_NOT_LOCKED)
    ;; ... transfers locked-amount back to participant
  ))

Impact: Combined with F-01, participants have zero risk. They can always get their tokens back — either by self-completing activities (F-01) or by calling refund-participant directly. The "commitment" is meaningless.

Fix: Refunds should be restricted:

• Only allow refund if quest is cancelled
• Or add a time-lock: no refund until quest expires
• Or require admin approval for early refunds

F-03: Treasury reward accounting — balance zeroed incorrectly

Location: treasury.clar → process-token-winners

(let ((amount-per-winner (/ fee winners-count)))
  (match (fold transfer-to-winner winners
    (ok { index: u0, token-contract: token-contract, amount: amount-per-winner }))

    transfer-result (begin
      ;; After distributing amount-per-winner to EACH winner...
      ;; ...ALSO sends 'fee' (50% of balance) to treasury-owner:
      (try! (as-contract?
        (try! (contract-call? token-contract transfer fee tx-sender
          (var-get treasury-owner) none))))

      ;; Then zeros the balance:
      (map-set token-balances token-principal (- balance balance))
      ;; ← (- balance balance) is ALWAYS 0, regardless of what was actually sent
    )))

Impact: The math is wrong. With balance=1000 and 2 winners:

• fee = 1000 / 2 = 500
• amount-per-winner = 500 / 2 = 250
• Sent to winners: 250 × 2 = 500 tokens
• Sent to owner: 500 tokens
• Total sent: 1000 tokens ✓
• Balance set to: 0 (via balance - balance)

This works when distributing all tokens, but the accounting is fragile. If any transfer-to-winner fails mid-fold, the balance map is never updated (the function errors). But if integer division leaves dust (e.g., 3 winners with fee=500 → 166 each = 498 sent, 2 dust lost), balance - balance = 0 hides the discrepancy. The real balance should be balance - (amount-per-winner * winners-count) - fee.

More importantly: the contract sends fee (50%) to the owner as a separate transfer after already distributing fee / winners-count to each winner. This means 100% of the balance is always distributed (50% to winners + 50% to owner). The "rest remains as platform balance" comment in the code is misleading.

Fix: Use explicit accounting: (map-set token-balances token-principal (- balance (+ (* amount-per-winner winners-count) fee)))

F-04: Treasury deposit doesn't verify sender

Location: treasury.clar → deposit

(define-public (deposit
    (amount uint)
    (sender principal)      ;; ← caller-supplied, not verified
    (token-contract <token>)
  )
  ;; No check: (asserts! (is-eq sender tx-sender) ...)
  ;; Post-conditions may catch this, but the function signature
  ;; allows anyone to specify any sender
  (try! (contract-call? token-contract transfer amount sender current-contract none))
  ;; ...
)

Impact: The sender parameter is passed directly to transfer. In Clarity, ft-transfer? requires the sender to be tx-sender, so this will fail for unauthorized callers in practice. However, the function design is misleading — it looks like you can deposit on behalf of others. If called through an intermediary contract, tx-sender propagation could create unexpected behavior.

Fix: Add (asserts! (is-eq sender tx-sender) ERR_UNAUTHORIZED) or remove the sender param and use tx-sender directly.

F-05: Token whitelist version mismatch

Location: quests.clar line 276 vs treasury.clar line 214

;; In quests.clar:
(define-private (is-token-enabled (token-id principal))
  (contract-call?
    'SP2GW18TVQR75W1VT53HYGBRGKFRV5BFYNAF5SS5J.ZADAO-token-whitelist-v1
    is-token-enabled token-id))

;; In treasury.clar:
(define-private (is-token-enabled (token-id principal))
  (contract-call?
    'SP2GW18TVQR75W1VT53HYGBRGKFRV5BFYNAF5SS5J.ZADAO-token-whitelist-v2
    is-token-enabled token-id))

Impact: A token could be enabled in v1 but not v2 (or vice versa). This means create-quest could succeed (quests checks v1) but deposit in treasury could fail (treasury checks v2). Or a token could pass treasury's reward-random-winners check but not be valid for quest creation.

Fix: Both contracts should use the same whitelist version.

F-06: Cancelled quest doesn't refund participants

Location: quests.clar → cancel-quest

(define-public (cancel-quest
    (quest-id (string-ascii 36))
    (use-token <token>)
  )
  ;; Only refunds the CREATOR's commitment from treasury
  (try! (contract-call? .treasury withdraw (get commitment-amount quest) tx-sender use-token))
  ;; Sets status to QUEST_CANCELLED
  (ok (map-set quests quest-id (merge quest { status: QUEST_CANCELLED })))
  ;; ← Participants' locked tokens are NOT refunded
  ;; ← And complete-activity checks (is-eq status QUEST_ACTIVE), so it will FAIL
  ;; ← Participants must call refund-participant individually (which works due to F-02)
)

Impact: When a creator cancels, participants' tokens remain locked in the quests contract. They can't complete activities (quest is no longer active). They can call refund-participant individually, but there's no batch refund and participants may not know the quest was cancelled.

Fix: Either batch-refund all participants on cancellation, or emit an event so participants know to claim refunds.

F-07: No time bounds on quests

Quests have a created-time but no expiry. A quest stays QUEST_ACTIVE forever unless the creator cancels. Participants' tokens can be locked indefinitely. No mechanism for expiry-based resolution or automatic cleanup.

F-08: quest-counter is unused

(define-data-var quest-counter uint u0)
;; Incremented in create-quest but never read for logic
;; Quest IDs are caller-supplied 36-char strings (UUIDs)
;; Counter serves no purpose — remove or use as auto-increment ID

F-09: Helper contract hardcodes wSTX address

helper-contract.clar wraps all quests functions with hardcoded 'SP32AEEF6WW5Y0NMJ1S8SBSZDAY8R5J32NBZFPKKZ.wstx. This is fine for a convenience wrapper but means it can't be used with other tokens. Not a bug — just a design note.

Post-conditions on transfers

The contract uses restrict-assets? / with-ft / with-stx post-conditions on token transfers. This is good defensive practice — even if logic is wrong, post-conditions bound the damage.

tx-sender == contract-caller check

All public functions verify (is-eq tx-sender contract-caller), preventing intermediary contracts from acting on behalf of users without their direct involvement. This blocks a class of phishing attacks where a malicious contract calls quest functions.

Clean separation of concerns

Treasury handles token custody, quests handles game logic, helper provides convenience. The treasury only accepts withdrawals from the quests contract ((asserts! (is-eq contract-caller .quests) ...)). Good modular design.