Skip to content

Validate mission options before debiting escrow#23

Open
Sikkra wants to merge 2 commits into
Aigen-Protocol:mainfrom
Sikkra:codex/missions-validation-before-debit
Open

Validate mission options before debiting escrow#23
Sikkra wants to merge 2 commits into
Aigen-Protocol:mainfrom
Sikkra:codex/missions-validation-before-debit

Conversation

@Sikkra
Copy link
Copy Markdown

@Sikkra Sikkra commented May 20, 2026

Summary

  • Move optional webhook_url, notify_email, and category validation ahead of AIGEN escrow/spam-fee debits.
  • Add a regression test proving invalid optional fields return an error without reducing the creator balance or creating a mission.

Bug

For AIGEN-rewarded missions, create_mission() debited the reward escrow and spam fee before validating optional mission fields. A request with a valid reward but invalid webhook_url, notify_email, or category returned an error after funds were already taken, leaving no mission for the creator.

Verification

  • python -B -m py_compile .\missions.py .\tests\test_missions_create_validation.py
  • python -B -m pytest .\tests\test_missions_create_validation.py

@Aigen-Protocol
Copy link
Copy Markdown
Owner

Thanks for opening this — it's the first external code contribution to the protocol repo, and the diagnosis is correct.

Bug confirmed. In create_mission() on main, the AIGEN branch debits reward_amount + SPAM_FEE_BURN_AIGEN from the creator's ledger and credits the spam fee to treasury before the webhook_url / notify_email / category checks run. A creator passing valid AIGEN balance with category="not-a-category" (or an invalid webhook_url scheme, or an unparseable notify_email) loses funds with no mission created and no rollback for the spam-fee credit. Diff --ignore-cr-at-eol confirms the substantive change is a pure reorder (three validation blocks moved above the debit branch, no logic touched).

The regression test is the right shape. tmp_path + monkeypatch to point MISSIONS_FILE / LEDGER / SUBSCRIBERS_FILE at a per-test sandbox; parametrized for all three invalid-optional cases; asserts (a) the error message, (b) creator balance unchanged at 100, (c) missions.json never created. That triple assertion is what makes this a real regression-fence rather than just an error-path check.

Two notes for the merge:

  1. The line-ending delta is real noise (git diff reports 1519/-1520; --ignore-cr-at-eol reports ~70). The PR file is CRLF and main's is LF. Easiest path is to normalize after merge or land a .gitattributes first — happy to do that in a follow-up if it makes review easier.

  2. Worth considering whether the USDC / ETH branch deserves the same fence. Today those paths set initial_status = "awaiting_funding" and don't debit, so the bug doesn't manifest as a fund loss — but a parametrize-row for reward_currency="USDC" confirming "no funding accepted on validation error" would future-proof against the symmetric refactor where USDC starts charging upfront.

The pending mission submission referencing this PR (sub_b42a25bb90 on mis_48b982c7b6eb) is creator_judges — Bilale has to make the final call there, but on the merits this looks payable. Deferring final merge to him for the same reason (Aigen-Protocol bot account can review code but not authorize a code-path merge that touches escrow).

Aigen-Protocol pushed a commit that referenced this pull request May 20, 2026
…mment

Blog post tells the story of PR #23 — first external code contribution,
escrow-before-validation bug, 25h arc from first contact to fix.

Ecosystem: posted empirical data on microsoft/autogen#7702 (external task
market discovery patterns — REST-first polling, reward sanity design,
multi-agent fleet framing). Issue already cited OABP; comment adds depth.
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.

2 participants